From d2d0c6225c2daa9091332a13c4d6832c3c085223 Mon Sep 17 00:00:00 2001 From: Dirk Mueller Date: Mon, 30 Sep 2019 09:11:00 +0200 Subject: [PATCH 1/2] Remove duplicated sha1 implementation (Fixes #6568) The Hash library had its own copy of a loop-unrolled sha1 implementation adding a large code footprint for no good reason, as there are several sha1 implementations already in tree (one in NONOS-SDK as well as one in bearssl). Switching to the bearssl one is straightforward and removes about 3kb of code size overhead. Also cleanup some obvious inefficiencies (copy by value, string summing, no reservation causing repeated reallocations) in the implementation. --- libraries/Hash/src/Hash.cpp | 24 ++-- libraries/Hash/src/Hash.h | 4 +- libraries/Hash/src/sha1/sha1.c | 208 --------------------------------- libraries/Hash/src/sha1/sha1.h | 32 ----- 4 files changed, 13 insertions(+), 255 deletions(-) delete mode 100644 libraries/Hash/src/sha1/sha1.c delete mode 100644 libraries/Hash/src/sha1/sha1.h diff --git a/libraries/Hash/src/Hash.cpp b/libraries/Hash/src/Hash.cpp index 5a58c961d2..62c5664eb9 100644 --- a/libraries/Hash/src/Hash.cpp +++ b/libraries/Hash/src/Hash.cpp @@ -23,13 +23,10 @@ */ #include +#include #include "Hash.h" -extern "C" { -#include "sha1/sha1.h" -} - /** * create a sha1 hash from data * @param data uint8_t * @@ -38,7 +35,7 @@ extern "C" { */ void sha1(uint8_t * data, uint32_t size, uint8_t hash[20]) { - SHA1_CTX ctx; + br_sha1_context ctx; #ifdef DEBUG_SHA1 os_printf("DATA:"); @@ -53,9 +50,9 @@ void sha1(uint8_t * data, uint32_t size, uint8_t hash[20]) { os_printf("\n"); #endif - SHA1Init(&ctx); - SHA1Update(&ctx, data, size); - SHA1Final(hash, &ctx); + br_sha1_init(&ctx); + br_sha1_update(&ctx, data, size); + br_sha1_out(&ctx, hash); #ifdef DEBUG_SHA1 os_printf("SHA1:"); @@ -78,20 +75,21 @@ void sha1(const char * data, uint32_t size, uint8_t hash[20]) { sha1((uint8_t *) data, size, hash); } -void sha1(String data, uint8_t hash[20]) { +void sha1(const String& data, uint8_t hash[20]) { sha1(data.c_str(), data.length(), hash); } String sha1(uint8_t* data, uint32_t size) { uint8_t hash[20]; - String hashStr = ""; + String hashStr((const char*)nullptr); + hashStr.reserve(20 * 2 + 1); sha1(&data[0], size, &hash[0]); for(uint16_t i = 0; i < 20; i++) { String hex = String(hash[i], HEX); - if(hex.length() < 2) { - hex = "0" + hex; + if(hash[i] < 0x10) { + hashStr += '0'; } hashStr += hex; } @@ -111,7 +109,7 @@ String sha1(const char* data, uint32_t size) { return sha1((uint8_t*) data, size); } -String sha1(String data) { +String sha1(const String& data) { return sha1(data.c_str(), data.length()); } diff --git a/libraries/Hash/src/Hash.h b/libraries/Hash/src/Hash.h index 774b8aad12..0abb150bdb 100644 --- a/libraries/Hash/src/Hash.h +++ b/libraries/Hash/src/Hash.h @@ -31,12 +31,12 @@ void sha1(uint8_t * data, uint32_t size, uint8_t hash[20]); void sha1(char * data, uint32_t size, uint8_t hash[20]); void sha1(const uint8_t * data, uint32_t size, uint8_t hash[20]); void sha1(const char * data, uint32_t size, uint8_t hash[20]); -void sha1(String data, uint8_t hash[20]); +void sha1(const String& data, uint8_t hash[20]); String sha1(uint8_t* data, uint32_t size); String sha1(char* data, uint32_t size); String sha1(const uint8_t* data, uint32_t size); String sha1(const char* data, uint32_t size); -String sha1(String data); +String sha1(const String& data); #endif /* HASH_H_ */ diff --git a/libraries/Hash/src/sha1/sha1.c b/libraries/Hash/src/sha1/sha1.c deleted file mode 100644 index fae926462d..0000000000 --- a/libraries/Hash/src/sha1/sha1.c +++ /dev/null @@ -1,208 +0,0 @@ -/** - * @file sha1.c - * @date 20.05.2015 - * @author Steve Reid - * - * from: http://www.virtualbox.org/svn/vbox/trunk/src/recompiler/tests/sha1.c - */ - -/* from valgrind tests */ - -/* ================ sha1.c ================ */ -/* - SHA-1 in C - By Steve Reid - 100% Public Domain - - Test Vectors (from FIPS PUB 180-1) - "abc" - A9993E36 4706816A BA3E2571 7850C26C 9CD0D89D - "abcdbcdecdefdefgefghfghighijhijkijkljklmklmnlmnomnopnopq" - 84983E44 1C3BD26E BAAE4AA1 F95129E5 E54670F1 - A million repetitions of "a" - 34AA973C D4C4DAA4 F61EEB2B DBAD2731 6534016F -*/ - -/* #define LITTLE_ENDIAN * This should be #define'd already, if true. */ -/* #define SHA1HANDSOFF * Copies data before messing with it. */ - -#define SHA1HANDSOFF - -#include -#include -#include -#include - -#include "sha1.h" - -//#include - -#define rol(value, bits) (((value) << (bits)) | ((value) >> (32 - (bits)))) - -/* blk0() and blk() perform the initial expand. */ -/* I got the idea of expanding during the round function from SSLeay */ -#if BYTE_ORDER == LITTLE_ENDIAN -#define blk0(i) (block->l[i] = (rol(block->l[i],24)&0xFF00FF00) \ - |(rol(block->l[i],8)&0x00FF00FF)) -#elif BYTE_ORDER == BIG_ENDIAN -#define blk0(i) block->l[i] -#else -#error "Endianness not defined!" -#endif -#define blk(i) (block->l[i&15] = rol(block->l[(i+13)&15]^block->l[(i+8)&15] \ - ^block->l[(i+2)&15]^block->l[i&15],1)) - -/* (R0+R1), R2, R3, R4 are the different operations used in SHA1 */ -#define R0(v,w,x,y,z,i) z+=((w&(x^y))^y)+blk0(i)+0x5A827999+rol(v,5);w=rol(w,30); -#define R1(v,w,x,y,z,i) z+=((w&(x^y))^y)+blk(i)+0x5A827999+rol(v,5);w=rol(w,30); -#define R2(v,w,x,y,z,i) z+=(w^x^y)+blk(i)+0x6ED9EBA1+rol(v,5);w=rol(w,30); -#define R3(v,w,x,y,z,i) z+=(((w|x)&y)|(w&x))+blk(i)+0x8F1BBCDC+rol(v,5);w=rol(w,30); -#define R4(v,w,x,y,z,i) z+=(w^x^y)+blk(i)+0xCA62C1D6+rol(v,5);w=rol(w,30); - - -/* Hash a single 512-bit block. This is the core of the algorithm. */ - -void ICACHE_FLASH_ATTR SHA1Transform(uint32_t state[5], uint8_t buffer[64]) -{ -uint32_t a, b, c, d, e; -typedef union { - unsigned char c[64]; - uint32_t l[16]; -} CHAR64LONG16; -#ifdef SHA1HANDSOFF -CHAR64LONG16 block[1]; /* use array to appear as a pointer */ - memcpy(block, buffer, 64); -#else - /* The following had better never be used because it causes the - * pointer-to-const buffer to be cast into a pointer to non-const. - * And the result is written through. I threw a "const" in, hoping - * this will cause a diagnostic. - */ -CHAR64LONG16* block = (const CHAR64LONG16*)buffer; -#endif - /* Copy context->state[] to working vars */ - a = state[0]; - b = state[1]; - c = state[2]; - d = state[3]; - e = state[4]; - /* 4 rounds of 20 operations each. Loop unrolled. */ - R0(a,b,c,d,e, 0); R0(e,a,b,c,d, 1); R0(d,e,a,b,c, 2); R0(c,d,e,a,b, 3); - R0(b,c,d,e,a, 4); R0(a,b,c,d,e, 5); R0(e,a,b,c,d, 6); R0(d,e,a,b,c, 7); - R0(c,d,e,a,b, 8); R0(b,c,d,e,a, 9); R0(a,b,c,d,e,10); R0(e,a,b,c,d,11); - R0(d,e,a,b,c,12); R0(c,d,e,a,b,13); R0(b,c,d,e,a,14); R0(a,b,c,d,e,15); - R1(e,a,b,c,d,16); R1(d,e,a,b,c,17); R1(c,d,e,a,b,18); R1(b,c,d,e,a,19); - R2(a,b,c,d,e,20); R2(e,a,b,c,d,21); R2(d,e,a,b,c,22); R2(c,d,e,a,b,23); - R2(b,c,d,e,a,24); R2(a,b,c,d,e,25); R2(e,a,b,c,d,26); R2(d,e,a,b,c,27); - R2(c,d,e,a,b,28); R2(b,c,d,e,a,29); R2(a,b,c,d,e,30); R2(e,a,b,c,d,31); - R2(d,e,a,b,c,32); R2(c,d,e,a,b,33); R2(b,c,d,e,a,34); R2(a,b,c,d,e,35); - R2(e,a,b,c,d,36); R2(d,e,a,b,c,37); R2(c,d,e,a,b,38); R2(b,c,d,e,a,39); - R3(a,b,c,d,e,40); R3(e,a,b,c,d,41); R3(d,e,a,b,c,42); R3(c,d,e,a,b,43); - R3(b,c,d,e,a,44); R3(a,b,c,d,e,45); R3(e,a,b,c,d,46); R3(d,e,a,b,c,47); - R3(c,d,e,a,b,48); R3(b,c,d,e,a,49); R3(a,b,c,d,e,50); R3(e,a,b,c,d,51); - R3(d,e,a,b,c,52); R3(c,d,e,a,b,53); R3(b,c,d,e,a,54); R3(a,b,c,d,e,55); - R3(e,a,b,c,d,56); R3(d,e,a,b,c,57); R3(c,d,e,a,b,58); R3(b,c,d,e,a,59); - R4(a,b,c,d,e,60); R4(e,a,b,c,d,61); R4(d,e,a,b,c,62); R4(c,d,e,a,b,63); - R4(b,c,d,e,a,64); R4(a,b,c,d,e,65); R4(e,a,b,c,d,66); R4(d,e,a,b,c,67); - R4(c,d,e,a,b,68); R4(b,c,d,e,a,69); R4(a,b,c,d,e,70); R4(e,a,b,c,d,71); - R4(d,e,a,b,c,72); R4(c,d,e,a,b,73); R4(b,c,d,e,a,74); R4(a,b,c,d,e,75); - R4(e,a,b,c,d,76); R4(d,e,a,b,c,77); R4(c,d,e,a,b,78); R4(b,c,d,e,a,79); - /* Add the working vars back into context.state[] */ - state[0] += a; - state[1] += b; - state[2] += c; - state[3] += d; - state[4] += e; - /* Wipe variables */ - a = b = c = d = e = 0; -#ifdef SHA1HANDSOFF - memset(block, '\0', sizeof(block)); -#endif -} - - -/* SHA1Init - Initialize new context */ - -void ICACHE_FLASH_ATTR SHA1Init(SHA1_CTX* context) -{ - /* SHA1 initialization constants */ - context->state[0] = 0x67452301; - context->state[1] = 0xEFCDAB89; - context->state[2] = 0x98BADCFE; - context->state[3] = 0x10325476; - context->state[4] = 0xC3D2E1F0; - context->count[0] = context->count[1] = 0; -} - - -/* Run your data through this. */ - -void ICACHE_FLASH_ATTR SHA1Update(SHA1_CTX* context, uint8_t* data, uint32_t len) -{ - uint32_t i; - uint32_t j; - - j = context->count[0]; - if ((context->count[0] += len << 3) < j) - context->count[1]++; - context->count[1] += (len>>29); - j = (j >> 3) & 63; - if ((j + len) > 63) { - memcpy(&context->buffer[j], data, (i = 64-j)); - SHA1Transform(context->state, context->buffer); - for ( ; i + 63 < len; i += 64) { - SHA1Transform(context->state, &data[i]); - } - j = 0; - } - else i = 0; - memcpy(&context->buffer[j], &data[i], len - i); -} - - -/* Add padding and return the message digest. */ - -void ICACHE_FLASH_ATTR SHA1Final(unsigned char digest[20], SHA1_CTX* context) -{ -unsigned i; -unsigned char finalcount[8]; -unsigned char c; - -#if 0 /* untested "improvement" by DHR */ - /* Convert context->count to a sequence of bytes - * in finalcount. Second element first, but - * big-endian order within element. - * But we do it all backwards. - */ - unsigned char *fcp = &finalcount[8]; - - for (i = 0; i < 2; i++) - { - uint32_t t = context->count[i]; - int j; - - for (j = 0; j < 4; t >>= 8, j++) - *--fcp = (unsigned char) t; - } -#else - for (i = 0; i < 8; i++) { - finalcount[i] = (unsigned char)((context->count[(i >= 4 ? 0 : 1)] - >> ((3-(i & 3)) * 8) ) & 255); /* Endian independent */ - } -#endif - c = 0200; - SHA1Update(context, &c, 1); - while ((context->count[0] & 504) != 448) { - c = 0000; - SHA1Update(context, &c, 1); - } - SHA1Update(context, finalcount, 8); /* Should cause a SHA1Transform() */ - for (i = 0; i < 20; i++) { - digest[i] = (unsigned char) - ((context->state[i>>2] >> ((3-(i & 3)) * 8) ) & 255); - } - /* Wipe variables */ - memset(context, '\0', sizeof(*context)); - memset(&finalcount, '\0', sizeof(finalcount)); -} -/* ================ end of sha1.c ================ */ diff --git a/libraries/Hash/src/sha1/sha1.h b/libraries/Hash/src/sha1/sha1.h deleted file mode 100644 index 158bd76b36..0000000000 --- a/libraries/Hash/src/sha1/sha1.h +++ /dev/null @@ -1,32 +0,0 @@ -/** - * @file sha1.h - * @date 20.05.2015 - * @author Steve Reid - * - * from: http://www.virtualbox.org/svn/vbox/trunk/src/recompiler/tests/sha1.c - */ - -/* ================ sha1.h ================ */ -/* - SHA-1 in C - By Steve Reid - 100% Public Domain -*/ - -#ifndef SHA1_H_ -#define SHA1_H_ - -typedef struct { - uint32_t state[5]; - uint32_t count[2]; - unsigned char buffer[64]; -} SHA1_CTX; - -void SHA1Transform(uint32_t state[5], uint8_t buffer[64]); -void SHA1Init(SHA1_CTX* context); -void SHA1Update(SHA1_CTX* context, uint8_t* data, uint32_t len); -void SHA1Final(unsigned char digest[20], SHA1_CTX* context); - -#endif /* SHA1_H_ */ - -/* ================ end of sha1.h ================ */ From 0bc1061aaf736246afcf368748fd8603076228bc Mon Sep 17 00:00:00 2001 From: Dirk Mueller Date: Mon, 30 Sep 2019 15:50:57 +0200 Subject: [PATCH 2/2] Remove overload variants for sha1(...) that accept nonconst data The data is always remaining unmodified, so non-const overloads are confusing and redundant. Also optimize the hexify variant a bit more. --- libraries/Hash/src/Hash.cpp | 33 +++++++-------------------------- libraries/Hash/src/Hash.h | 8 ++------ 2 files changed, 9 insertions(+), 32 deletions(-) diff --git a/libraries/Hash/src/Hash.cpp b/libraries/Hash/src/Hash.cpp index 62c5664eb9..2d88663616 100644 --- a/libraries/Hash/src/Hash.cpp +++ b/libraries/Hash/src/Hash.cpp @@ -33,8 +33,7 @@ * @param size uint32_t * @param hash uint8_t[20] */ -void sha1(uint8_t * data, uint32_t size, uint8_t hash[20]) { - +void sha1(const uint8_t* data, uint32_t size, uint8_t hash[20]) { br_sha1_context ctx; #ifdef DEBUG_SHA1 @@ -63,23 +62,15 @@ void sha1(uint8_t * data, uint32_t size, uint8_t hash[20]) { #endif } -void sha1(char * data, uint32_t size, uint8_t hash[20]) { - sha1((uint8_t *) data, size, hash); -} - -void sha1(const uint8_t * data, uint32_t size, uint8_t hash[20]) { - sha1((uint8_t *) data, size, hash); -} - -void sha1(const char * data, uint32_t size, uint8_t hash[20]) { - sha1((uint8_t *) data, size, hash); +void sha1(const char* data, uint32_t size, uint8_t hash[20]) { + sha1((const uint8_t *) data, size, hash); } void sha1(const String& data, uint8_t hash[20]) { sha1(data.c_str(), data.length(), hash); } -String sha1(uint8_t* data, uint32_t size) { +String sha1(const uint8_t* data, uint32_t size) { uint8_t hash[20]; String hashStr((const char*)nullptr); hashStr.reserve(20 * 2 + 1); @@ -87,26 +78,16 @@ String sha1(uint8_t* data, uint32_t size) { sha1(&data[0], size, &hash[0]); for(uint16_t i = 0; i < 20; i++) { - String hex = String(hash[i], HEX); - if(hash[i] < 0x10) { - hashStr += '0'; - } + char hex[3]; + snprintf(hex, sizeof(hex), "%02x", hash[i]); hashStr += hex; } return hashStr; } -String sha1(char* data, uint32_t size) { - return sha1((uint8_t*) data, size); -} - -String sha1(const uint8_t* data, uint32_t size) { - return sha1((uint8_t*) data, size); -} - String sha1(const char* data, uint32_t size) { - return sha1((uint8_t*) data, size); + return sha1((const uint8_t*) data, size); } String sha1(const String& data) { diff --git a/libraries/Hash/src/Hash.h b/libraries/Hash/src/Hash.h index 0abb150bdb..67a3151124 100644 --- a/libraries/Hash/src/Hash.h +++ b/libraries/Hash/src/Hash.h @@ -27,14 +27,10 @@ //#define DEBUG_SHA1 -void sha1(uint8_t * data, uint32_t size, uint8_t hash[20]); -void sha1(char * data, uint32_t size, uint8_t hash[20]); -void sha1(const uint8_t * data, uint32_t size, uint8_t hash[20]); -void sha1(const char * data, uint32_t size, uint8_t hash[20]); +void sha1(const uint8_t* data, uint32_t size, uint8_t hash[20]); +void sha1(const char* data, uint32_t size, uint8_t hash[20]); void sha1(const String& data, uint8_t hash[20]); -String sha1(uint8_t* data, uint32_t size); -String sha1(char* data, uint32_t size); String sha1(const uint8_t* data, uint32_t size); String sha1(const char* data, uint32_t size); String sha1(const String& data);