security EIA1: bugfix: memory alignment + read out of array
Compiling the nrUE with -fsanitize=undefined leads to a crash because the function nas_stream_encrypt_eia1() accesses the data as uint32_t but the data is not aligned. Actually this function wants 64 bits (big-endian) of data, so let's introduce a simple function to read a big-endian 64 bits value. It may seem bad (unoptimized), but looking at the output of gcc without -fsanitize=undefined this will be translated as just one movbeq instruction (on the machine where I did the test), which is actually less instructions than the previous version which was doing too movbel plus orq. Moreover, after the main loop we need to process the last remaining bytes (so less than 8). The code was wrong, reading past the input data (and also not caring about alignment). It may work, it may fail, it depends on many things. But it's plain wrong. So this was replaced for something better, simpler. No need for mask32bit() anymore, only one call to U64() is needed. And now -fsanitize=undefined is happy, so all is good. We also restrict the input length to be multiple of 8 bits. I don't think it's a problem. To be refined if I'm wrong. (I think RRC and NAS messages are 8-bits aligned, so it shouldn't be a problem.)
This commit is contained in:
@@ -88,25 +88,36 @@ static uint64_t MUL64(uint64_t V, uint64_t P, uint64_t c)
|
|||||||
return result;
|
return result;
|
||||||
}
|
}
|
||||||
|
|
||||||
/* mask32bit.
|
/* read a big endian uint64_t at given address (potentially not 64-bits aligned)
|
||||||
* Input n: an integer in 1-32.
|
* don't read more than 'available_bytes'
|
||||||
* Output : a 32 bit mask.
|
* (use 0 if no byte to read)
|
||||||
* Prepares a 32 bit mask with required number of 1 bits on the MSB side.
|
* (note: the compiler will optimize this, no need to do better)
|
||||||
*/
|
*/
|
||||||
static uint32_t mask32bit(int n)
|
static inline uint64_t U64(uint8_t *p, int available_bytes)
|
||||||
{
|
{
|
||||||
uint32_t mask=0x0;
|
uint64_t a = 0;
|
||||||
|
uint64_t b = 0;
|
||||||
|
uint64_t c = 0;
|
||||||
|
uint64_t d = 0;
|
||||||
|
uint64_t e = 0;
|
||||||
|
uint64_t f = 0;
|
||||||
|
uint64_t g = 0;
|
||||||
|
uint64_t h = 0;
|
||||||
|
switch (available_bytes) {
|
||||||
|
case 8: h = p[7]; /* falltrough */
|
||||||
|
case 7: g = p[6]; /* falltrough */
|
||||||
|
case 6: f = p[5]; /* falltrough */
|
||||||
|
case 5: e = p[4]; /* falltrough */
|
||||||
|
case 4: d = p[3]; /* falltrough */
|
||||||
|
case 3: c = p[2]; /* falltrough */
|
||||||
|
case 2: b = p[1]; /* falltrough */
|
||||||
|
case 1: a = p[0];
|
||||||
|
}
|
||||||
|
|
||||||
if ( n%32 == 0 )
|
return (a << (32+24)) | (b << (32+16)) | (c << (32+8)) | (d << 32)
|
||||||
return 0xffffffff;
|
| (e << 24) | (f << 16) | (g << 8) | h;
|
||||||
|
|
||||||
while (n--)
|
|
||||||
mask = (mask>>1) ^ 0x80000000;
|
|
||||||
|
|
||||||
return mask;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
/*!
|
/*!
|
||||||
* @brief Create integrity cmac t for a given message.
|
* @brief Create integrity cmac t for a given message.
|
||||||
* @param[in] stream_cipher Structure containing various variables to setup encoding
|
* @param[in] stream_cipher Structure containing various variables to setup encoding
|
||||||
@@ -125,11 +136,8 @@ void nas_stream_encrypt_eia1(nas_stream_cipher_t const *stream_cipher, uint8_t o
|
|||||||
uint64_t c;
|
uint64_t c;
|
||||||
uint64_t M_D_2;
|
uint64_t M_D_2;
|
||||||
int rem_bits;
|
int rem_bits;
|
||||||
uint32_t mask = 0;
|
|
||||||
uint32_t *message;
|
|
||||||
uint8_t *key = (uint8_t *)stream_cipher->context;
|
uint8_t *key = (uint8_t *)stream_cipher->context;
|
||||||
|
|
||||||
message = (uint32_t*)stream_cipher->message; /* To operate 32 bit message internally. */
|
|
||||||
/* Load the Integrity Key for SNOW3G initialization as in section 4.4. */
|
/* Load the Integrity Key for SNOW3G initialization as in section 4.4. */
|
||||||
memcpy(K+3,key+0,4); /*K[3] = key[0]; we assume
|
memcpy(K+3,key+0,4); /*K[3] = key[0]; we assume
|
||||||
K[3]=key[0]||key[1]||...||key[31] , with key[0] the
|
K[3]=key[0]||key[1]||...||key[31] , with key[0] the
|
||||||
@@ -181,11 +189,14 @@ void nas_stream_encrypt_eia1(nas_stream_cipher_t const *stream_cipher, uint8_t o
|
|||||||
EVAL = 0;
|
EVAL = 0;
|
||||||
c = 0x1b;
|
c = 0x1b;
|
||||||
|
|
||||||
|
AssertFatal(stream_cipher->blength % 8 == 0, "unsupported buffer length\n");
|
||||||
|
|
||||||
|
uint8_t *message = stream_cipher->message;
|
||||||
|
|
||||||
/* for 0 <= i <= D-3 */
|
/* for 0 <= i <= D-3 */
|
||||||
for (i=0; i<D-2; i++) {
|
for (i=0; i<D-2; i++) {
|
||||||
V = EVAL ^ ( (uint64_t)hton_int32(message[2*i]) << 32 | (uint64_t)hton_int32(message[2*i+1]) );
|
V = EVAL ^ U64(&message[4 * 2*i], 8);
|
||||||
EVAL = MUL64(V,P,c);
|
EVAL = MUL64(V,P,c);
|
||||||
//printf ("Mi: %16X %16X\tEVAL: %16lX\n",hton_int32(message[2*i]),hton_int32(message[2*i+1]), EVAL);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/* for D-2 */
|
/* for D-2 */
|
||||||
@@ -194,14 +205,7 @@ void nas_stream_encrypt_eia1(nas_stream_cipher_t const *stream_cipher, uint8_t o
|
|||||||
if (rem_bits == 0)
|
if (rem_bits == 0)
|
||||||
rem_bits = 64;
|
rem_bits = 64;
|
||||||
|
|
||||||
mask = mask32bit(rem_bits%32);
|
M_D_2 = U64(&message[4 * (2*(D-2))], rem_bits/8);
|
||||||
|
|
||||||
if (rem_bits > 32) {
|
|
||||||
M_D_2 = ( (uint64_t) hton_int32(message[2*(D-2)]) << 32 ) |
|
|
||||||
(uint64_t) (hton_int32(message[2*(D-2)+1]) & mask);
|
|
||||||
} else {
|
|
||||||
M_D_2 = ( (uint64_t) hton_int32(message[2*(D-2)]) & mask) << 32 ;
|
|
||||||
}
|
|
||||||
|
|
||||||
V = EVAL ^ M_D_2;
|
V = EVAL ^ M_D_2;
|
||||||
EVAL = MUL64(V,P,c);
|
EVAL = MUL64(V,P,c);
|
||||||
|
|||||||
Reference in New Issue
Block a user