From b3e6eecfcef37b9e4f91c1ac04ea2219c84c7859 Mon Sep 17 00:00:00 2001 From: Jochen Hoenicke Date: Mon, 29 Aug 2016 20:33:35 +0200 Subject: [PATCH 1/2] sha2: Fix unaligned access --- sha2.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/sha2.c b/sha2.c index b3bf14386..241265f5e 100644 --- a/sha2.c +++ b/sha2.c @@ -480,7 +480,6 @@ void sha256_Update(SHA256_CTX* context, const sha2_byte *data, size_t len) { } void sha256_Final(SHA256_CTX* context, sha2_byte digest[]) { - sha2_word32 *d = (sha2_word32*)(void*)digest; unsigned int usedspace; /* If no digest buffer is passed, we don't bother doing this: */ @@ -528,7 +527,7 @@ void sha256_Final(SHA256_CTX* context, sha2_byte digest[]) { REVERSE32(context->state[j],context->state[j]); } #endif - MEMCPY_BCOPY(d, context->state, SHA256_DIGEST_LENGTH); + MEMCPY_BCOPY(digest, context->state, SHA256_DIGEST_LENGTH); } /* Clean up state data: */ @@ -770,7 +769,7 @@ void sha512_Update(SHA512_CTX* context, const sha2_byte *data, size_t len) { #if BYTE_ORDER == LITTLE_ENDIAN /* Convert TO host byte order */ for (int j = 0; j < 16; j++) { - REVERSE64(((sha2_word64*)data)[j],context->buffer[j]); + REVERSE64(context->buffer[j],context->buffer[j]); } #endif sha512_Transform(context->state, context->buffer, context->state); @@ -831,8 +830,6 @@ static void sha512_Last(SHA512_CTX* context) { } void sha512_Final(SHA512_CTX* context, sha2_byte digest[]) { - sha2_word64 *d = (sha2_word64*)(void*)digest; - /* If no digest buffer is passed, we don't bother doing this: */ if (digest != (sha2_byte*)0) { sha512_Last(context); @@ -844,7 +841,7 @@ void sha512_Final(SHA512_CTX* context, sha2_byte digest[]) { REVERSE64(context->state[j],context->state[j]); } #endif - MEMCPY_BCOPY(d, context->state, SHA512_DIGEST_LENGTH); + MEMCPY_BCOPY(digest, context->state, SHA512_DIGEST_LENGTH); } /* Zero out state data */ From 19a1f501c4294f25f47b235899b58f78aa2c04c7 Mon Sep 17 00:00:00 2001 From: Jochen Hoenicke Date: Mon, 29 Aug 2016 21:41:23 +0200 Subject: [PATCH 2/2] Simplified sha256_Final/sha512_Last - Fix the bug where we zero too many bytes in sha512_Last (SHORT_BLOCK_LENGTH != BLOCK_LENGTH -2). - Get rid of an if branch. - Don't reverse the last two words in 512_Last that are written later. - make 256_Final and 512_Last look the same. --- sha2.c | 40 +++++++++++++++++----------------------- 1 file changed, 17 insertions(+), 23 deletions(-) diff --git a/sha2.c b/sha2.c index 241265f5e..5831e3dd9 100644 --- a/sha2.c +++ b/sha2.c @@ -488,13 +488,9 @@ void sha256_Final(SHA256_CTX* context, sha2_byte digest[]) { /* Begin padding with a 1 bit: */ ((uint8_t*)context->buffer)[usedspace++] = 0x80; - if (usedspace <= SHA256_SHORT_BLOCK_LENGTH) { - /* Set-up for the last transform: */ - MEMSET_BZERO(((uint8_t*)context->buffer) + usedspace, SHA256_SHORT_BLOCK_LENGTH - usedspace); - } else { - if (usedspace < SHA256_BLOCK_LENGTH) { - MEMSET_BZERO(((uint8_t*)context->buffer) + usedspace, SHA256_BLOCK_LENGTH - usedspace); - } + if (usedspace > SHA256_SHORT_BLOCK_LENGTH) { + MEMSET_BZERO(((uint8_t*)context->buffer) + usedspace, SHA256_BLOCK_LENGTH - usedspace); + #if BYTE_ORDER == LITTLE_ENDIAN /* Convert TO host byte order */ for (int j = 0; j < 16; j++) { @@ -504,9 +500,11 @@ void sha256_Final(SHA256_CTX* context, sha2_byte digest[]) { /* Do second-to-last transform: */ sha256_Transform(context->state, context->buffer, context->state); - /* And set-up for the last transform: */ - MEMSET_BZERO(context->buffer, SHA256_SHORT_BLOCK_LENGTH); + /* And prepare the last transform: */ + usedspace = 0; } + /* Set-up for the last transform: */ + MEMSET_BZERO(((uint8_t*)context->buffer) + usedspace, SHA256_SHORT_BLOCK_LENGTH - usedspace); #if BYTE_ORDER == LITTLE_ENDIAN /* Convert TO host byte order */ @@ -793,13 +791,9 @@ static void sha512_Last(SHA512_CTX* context) { /* Begin padding with a 1 bit: */ ((uint8_t*)context->buffer)[usedspace++] = 0x80; - if (usedspace <= SHA512_SHORT_BLOCK_LENGTH) { - /* Set-up for the last transform: */ - MEMSET_BZERO(((uint8_t*)context->buffer) + usedspace, SHA512_SHORT_BLOCK_LENGTH - usedspace); - } else { - if (usedspace < SHA512_BLOCK_LENGTH) { - MEMSET_BZERO(((uint8_t*)context->buffer) + usedspace, SHA512_BLOCK_LENGTH - usedspace); - } + if (usedspace > SHA512_SHORT_BLOCK_LENGTH) { + MEMSET_BZERO(((uint8_t*)context->buffer) + usedspace, SHA512_BLOCK_LENGTH - usedspace); + #if BYTE_ORDER == LITTLE_ENDIAN /* Convert TO host byte order */ for (int j = 0; j < 16; j++) { @@ -809,21 +803,21 @@ static void sha512_Last(SHA512_CTX* context) { /* Do second-to-last transform: */ sha512_Transform(context->state, context->buffer, context->state); - /* And set-up for the last transform: */ - MEMSET_BZERO(context->buffer, SHA512_BLOCK_LENGTH - 2); + /* And prepare the last transform: */ + usedspace = 0; } + /* Set-up for the last transform: */ + MEMSET_BZERO(((uint8_t*)context->buffer) + usedspace, SHA512_SHORT_BLOCK_LENGTH - usedspace); #if BYTE_ORDER == LITTLE_ENDIAN /* Convert TO host byte order */ - for (int j = 0; j < 16; j++) { + for (int j = 0; j < 14; j++) { REVERSE64(context->buffer[j],context->buffer[j]); } #endif /* Store the length of input data (in bits): */ - sha2_word64 *t; - t = &context->buffer[SHA512_SHORT_BLOCK_LENGTH/sizeof(sha2_word64)]; - t[0] = context->bitcount[1]; - t[1] = context->bitcount[0]; + context->buffer[14] = context->bitcount[1]; + context->buffer[15] = context->bitcount[0]; /* Final transform: */ sha512_Transform(context->state, context->buffer, context->state);