From 873c61c77f8a3ce1c8677e374a47a0ac04d9ebfa Mon Sep 17 00:00:00 2001 From: Robert Lukotka Date: Fri, 6 Oct 2023 16:56:44 +0200 Subject: [PATCH] Refactor DH crypto functions --- MakefileTest.mk | 2 +- src/decodeDH.c | 5 +- src/diffieHellman.c | 591 +++++++++++++++++++++++++------------- src/diffieHellman.h | 185 ++++++++---- src/diffieHellmann_test.c | 129 ++++++--- src/getPublicKey.c | 2 +- src/keyDerivation.c | 42 ++- src/signTransaction.c | 127 ++++---- 8 files changed, 698 insertions(+), 385 deletions(-) diff --git a/MakefileTest.mk b/MakefileTest.mk index 6be3a75d..84e2029a 100644 --- a/MakefileTest.mk +++ b/MakefileTest.mk @@ -33,7 +33,7 @@ endef .PHONY: speculos_port_5001_test_internal speculos_port_5001_test_internal: $(call run_announce,$@) -# $(call run_nodejs_test,5001,40001,getVersion.js) + $(call run_nodejs_test,5001,40001,getVersion.js) $(call run_nodejs_test,5001,40001,getSerial.js) $(call run_nodejs_test,5001,40001,getPublicKey.js) $(call run_nodejs_test,5001,40001,decodeMessage.js) diff --git a/src/decodeDH.c b/src/decodeDH.c index 7e1a28cf..9ceb3c0b 100644 --- a/src/decodeDH.c +++ b/src/decodeDH.c @@ -503,7 +503,10 @@ void decode_handleAPDU(uint8_t p1, TRACE("Decoding DH"); ASSERT(ctx->bufferLen <= SIZEOF(ctx->buffer)); TRACE_BUFFER(ctx->buffer, ctx->bufferLen); - ctx->bufferLen = dh_decode(&ctx->pathSpec, &ctx->otherPubKey, ctx->buffer, ctx->bufferLen); + uint16_t err = dh_decode(&ctx->pathSpec, &ctx->otherPubKey, ctx->buffer, &ctx->bufferLen); + if (err != SUCCESS) { + THROW(err); + } ctx->messageDecodedMagic = DECODING_FINISHED_MAGIC; ctx->bufferSentLen = 0; TRACE_BUFFER(ctx->buffer, ctx->bufferLen); diff --git a/src/diffieHellman.c b/src/diffieHellman.c index f0a315d4..5e4ea8ef 100644 --- a/src/diffieHellman.c +++ b/src/diffieHellman.c @@ -16,12 +16,12 @@ static void base64EncBlock(uint8_t in[3], uint8_t out[4]) { out[3] = BASE64[in[2] & 0x3F]; } -// Returns number of bytes written +// Base64 encodes as many block as possible. // Move unencoded part of inBuffer to the beginning and sets inSize accordingly -static size_t base64EncWholeBlocks(uint8_t* inBuffer, - uint8_t* inSize, - uint8_t* outBuffer, - size_t outSize) { +static WARN_UNUSED_RESULT uint16_t base64EncWholeBlocks(uint8_t *inBuffer, + uint8_t *inSize, + uint8_t *outBuffer, + uint16_t *outSize) { uint8_t processedBlocks = 0; while (1) { // We cannot process whole block @@ -32,45 +32,61 @@ static size_t base64EncWholeBlocks(uint8_t* inBuffer, break; } - // process one block, inBuffer + BASE64_IN_BLOCK_SIZE*processedBlocks and next two values - // exist - ASSERT(outSize >= BASE64_OUT_BLOCK_SIZE * (processedBlocks + 1)); + // process one block + if (*outSize < BASE64_OUT_BLOCK_SIZE * (processedBlocks + 1)) { + return ERR_ASSERT; + }; base64EncBlock(inBuffer + BASE64_IN_BLOCK_SIZE * processedBlocks, outBuffer + BASE64_OUT_BLOCK_SIZE * processedBlocks); processedBlocks++; } - return BASE64_OUT_BLOCK_SIZE * processedBlocks; + *outSize = BASE64_OUT_BLOCK_SIZE * processedBlocks; + return SUCCESS; } -static size_t processDHOneBlockFromCache(dh_context_t* ctx, - const dh_aes_key_t* aes_key, - uint8_t* outBuffer, - size_t outSize) { - ASSERT(ctx->cacheLength == CX_AES_BLOCK_SIZE); +// Encodes one block from cache using AES key +// Move unencoded part of inBuffer to the beginning and sets inSize accordingly +static WARN_UNUSED_RESULT uint16_t processDHOneBlockFromCache(dh_context_t *ctx, + const dh_aes_key_t *aesKey, + uint8_t *outBuffer, + uint16_t *outSize) { + // precondition sanity check + if (ctx->cacheLength != CX_AES_BLOCK_SIZE) { + return ERR_ASSERT; + } STATIC_ASSERT(SIZEOF(ctx->IV) == CX_AES_BLOCK_SIZE, "Incompatible IV size"); // We work in CBC mode // 1. IV xor plaintext - for (size_t i = 0; i < CX_AES_BLOCK_SIZE; i++) { + for (uint16_t i = 0; i < CX_AES_BLOCK_SIZE; i++) { ctx->cache[i] ^= ctx->IV[i]; } // 2. encrypt the result to obtain cyphertext 3. use cyphertext as new IV - cx_err_t err = cx_aes_enc_block(&aes_key->aesKey, ctx->cache, ctx->IV); - ASSERT(err == CX_OK); + cx_err_t err = cx_aes_enc_block(&aesKey->aesKey, ctx->cache, ctx->IV); + if (err != CX_OK) { + return ERR_ASSERT; + } // append cyphertext (not base64 encrypted) to hmac and clear cache - err = cx_hmac_update((cx_hmac_t*) &ctx->hmacCtx, ctx->IV, CX_AES_BLOCK_SIZE); - ASSERT(err == CX_OK); + err = cx_hmac_update((cx_hmac_t *) &ctx->hmacCtx, ctx->IV, CX_AES_BLOCK_SIZE); + if (err != CX_OK) { + return ERR_ASSERT; + } explicit_bzero(ctx->cache, SIZEOF(ctx->cache)); ctx->cacheLength = 0; // base64 encode - ASSERT(ctx->base64EncodingCacheLen < BASE64_IN_BLOCK_SIZE); + + // sanity check, we expect that cache does not contain whole block first + if (ctx->base64EncodingCacheLen >= BASE64_IN_BLOCK_SIZE) { + return ERR_ASSERT; + } STATIC_ASSERT(SIZEOF(ctx->base64EncodingCache) >= BASE64_IN_BLOCK_SIZE + CX_AES_BLOCK_SIZE, "Cache too small"); memmove(ctx->base64EncodingCache + ctx->base64EncodingCacheLen, ctx->IV, CX_AES_BLOCK_SIZE); ctx->base64EncodingCacheLen += CX_AES_BLOCK_SIZE; + // This also sets outSize as desired return base64EncWholeBlocks(ctx->base64EncodingCache, &ctx->base64EncodingCacheLen, outBuffer, @@ -79,9 +95,8 @@ static size_t processDHOneBlockFromCache(dh_context_t* ctx, //---------------------------- DH ENCODING --------------------------------------- -__noinline_due_to_stack__ void dh_init_aes_key(dh_aes_key_t* dhKey, - const bip44_path_t* pathSpec, - const public_key_t* publicKey) { +__noinline_due_to_stack__ WARN_UNUSED_RESULT uint16_t +dh_init_aes_key(dh_aes_key_t *dhKey, const bip44_path_t *pathSpec, const public_key_t *publicKey) { TRACE_STACK_USAGE(); private_key_t privateKey; unsigned char basicSecret[32]; @@ -89,70 +104,107 @@ __noinline_due_to_stack__ void dh_init_aes_key(dh_aes_key_t* dhKey, uint8_t K[SHA_512_SIZE]; TRACE("dh_init_aesKey"); - BEGIN_TRY { - TRY { - TRACE_STACK_USAGE(); - { - uint16_t err = derivePrivateKey(pathSpec, &privateKey); - if (err != SUCCESS) { - THROW(err); - } - } - cx_err_t err = cx_ecdh_no_throw(&privateKey, - CX_ECDH_X, - publicKey->W, - publicKey->W_len, - basicSecret, - SIZEOF(basicSecret)); - CX_THROW(err); - err = sha_512_hash(basicSecret, SIZEOF(basicSecret), secret, SIZEOF(secret)); - CX_THROW(err); - err = sha_512_hash(secret, SIZEOF(secret), K, SIZEOF(K)); - CX_THROW(err); - - // First DH_AES_SECRET_SIZE bytes are used to compute shared secret, then DH_KM_SIZE are - // used as Km for HMAC calculation - STATIC_ASSERT(SIZEOF(K) == DH_AES_SECRET_SIZE + DH_KM_SIZE, "Incompatible types"); - err = cx_aes_init_key_no_throw(K, DH_AES_SECRET_SIZE, &dhKey->aesKey); - CX_THROW(err); - STATIC_ASSERT(SIZEOF(dhKey->km) == DH_KM_SIZE, "Incompatible types"); - memmove(dhKey->km, K + DH_AES_SECRET_SIZE, DH_KM_SIZE); - dhKey->initialized_magic = DH_AES_KEY_INITIALIZED_MAGIC; - } - FINALLY { - explicit_bzero(&privateKey, sizeof(privateKey)); - explicit_bzero(basicSecret, SIZEOF(basicSecret)); - explicit_bzero(secret, SIZEOF(secret)); - explicit_bzero(K, SIZEOF(K)); + explicit_bzero(&privateKey, SIZEOF(privateKey)); + explicit_bzero(basicSecret, SIZEOF(basicSecret)); + explicit_bzero(secret, SIZEOF(secret)); + explicit_bzero(K, SIZEOF(K)); + explicit_bzero(dhKey, SIZEOF(*dhKey)); + + { + uint16_t err = derivePrivateKey(pathSpec, &privateKey); + if (err != SUCCESS) { + explicit_bzero(&privateKey, SIZEOF(privateKey)); + return err; } } - END_TRY; + + cx_err_t err = cx_ecdh_no_throw(&privateKey, + CX_ECDH_X, + publicKey->W, + publicKey->W_len, + basicSecret, + SIZEOF(basicSecret)); + explicit_bzero(&privateKey, SIZEOF(privateKey)); + if (err != CX_OK) { + explicit_bzero(basicSecret, SIZEOF(basicSecret)); + return ERR_ASSERT; + } + + err = sha_512_hash(basicSecret, SIZEOF(basicSecret), secret, SIZEOF(secret)); + explicit_bzero(basicSecret, SIZEOF(basicSecret)); + if (err != CX_OK) { + explicit_bzero(secret, SIZEOF(secret)); + return ERR_ASSERT; + } + + err = sha_512_hash(secret, SIZEOF(secret), K, SIZEOF(K)); + explicit_bzero(secret, SIZEOF(secret)); + if (err != CX_OK) { + explicit_bzero(K, SIZEOF(K)); + return ERR_ASSERT; + } + + // First DH_AES_SECRET_SIZE bytes are used to compute shared secret, then DH_KM_SIZE are + // used as Km for HMAC calculation + STATIC_ASSERT(SIZEOF(K) == DH_AES_SECRET_SIZE + DH_KM_SIZE, "Incompatible types"); + err = cx_aes_init_key_no_throw(K, DH_AES_SECRET_SIZE, &dhKey->aesKey); + if (err != CX_OK) { + explicit_bzero(K, SIZEOF(K)); + explicit_bzero(dhKey, SIZEOF(*dhKey)); + return ERR_ASSERT; + } + + STATIC_ASSERT(SIZEOF(dhKey->km) == DH_KM_SIZE, "Incompatible types"); + memmove(dhKey->km, K + DH_AES_SECRET_SIZE, DH_KM_SIZE); + return SUCCESS; } -__noinline_due_to_stack__ size_t dh_encode_init(dh_context_t* ctx, - const dh_aes_key_t* aes_key, - const uint8_t* iv, - size_t ivSize, - uint8_t* outBuffer, - size_t outSize) { +__noinline_due_to_stack__ WARN_UNUSED_RESULT uint16_t dh_encode_init(dh_context_t *ctx, + const bip44_path_t *pathSpec, + const public_key_t *publicKey, + const uint8_t *iv, + uint16_t ivSize, + uint8_t *outBuffer, + uint16_t *outSize) { + // Crypto assets to be cleared + dh_aes_key_t aesKey; + explicit_bzero(&aesKey, SIZEOF(aesKey)); + TRACE_STACK_USAGE(); TRACE("dh_encode_init"); - ASSERT(ivSize == SIZEOF(ctx->IV)); - ASSERT(aes_key->initialized_magic == DH_AES_KEY_INITIALIZED_MAGIC); + STATIC_ASSERT(SIZEOF(ctx->IV) == CX_AES_BLOCK_SIZE, "Unexpected IV length"); + if (ivSize != SIZEOF(ctx->IV)) { + return ERR_ASSERT; + } + // initialize DH context ctx->cacheLength = 0; explicit_bzero(ctx->cache, SIZEOF(ctx->cache)); ctx->base64EncodingCacheLen = 0; explicit_bzero(ctx->base64EncodingCache, SIZEOF(ctx->base64EncodingCache)); memcpy(ctx->IV, iv, SIZEOF(ctx->IV)); + // Compute AES key + { + uint16_t err = dh_init_aes_key(&aesKey, pathSpec, publicKey); + if (err != SUCCESS) { + explicit_bzero(&aesKey, SIZEOF(aesKey)); + return err; + } + } + explicit_bzero(&ctx->hmacCtx, SIZEOF(ctx->hmacCtx)); - cx_err_t err = cx_hmac_sha256_init_no_throw(&ctx->hmacCtx, aes_key->km, SIZEOF(aes_key->km)); - ASSERT(err == CX_OK); - err = cx_hmac_update((cx_hmac_t*) &ctx->hmacCtx, iv, ivSize); - ASSERT(err == CX_OK); + cx_err_t err = cx_hmac_sha256_init_no_throw(&ctx->hmacCtx, aesKey.km, SIZEOF(aesKey.km)); + explicit_bzero(&aesKey, SIZEOF(aesKey)); + if (err != CX_OK) { + return ERR_ASSERT; + } + err = cx_hmac_update((cx_hmac_t *) &ctx->hmacCtx, iv, ivSize); + if (err != CX_OK) { + return ERR_ASSERT; + } ctx->initialized_magic = HASH_CONTEXT_INITIALIZED_MAGIC; @@ -160,32 +212,54 @@ __noinline_due_to_stack__ size_t dh_encode_init(dh_context_t* ctx, STATIC_ASSERT(SIZEOF(ctx->base64EncodingCache) >= SIZEOF(ctx->IV), "Cache too small"); memcpy(ctx->base64EncodingCache, ctx->IV, SIZEOF(ctx->IV)); ctx->base64EncodingCacheLen = SIZEOF(ctx->IV); + // This also sets outSize as desired return base64EncWholeBlocks(ctx->base64EncodingCache, &ctx->base64EncodingCacheLen, outBuffer, outSize); } -__noinline_due_to_stack__ size_t dh_encode_append(dh_context_t* ctx, - const dh_aes_key_t* aes_key, - const uint8_t* inBuffer, - size_t inSize, - uint8_t* outBuffer, - size_t outSize) { +__noinline_due_to_stack__ WARN_UNUSED_RESULT uint16_t +dh_encode_append(dh_context_t *ctx, + const bip44_path_t *pathSpec, + const public_key_t *publicKey, + const uint8_t *inBuffer, + uint16_t inSize, + uint8_t *outBuffer, + uint16_t *outSize) { + // Crypto assets to be cleared + dh_aes_key_t aesKey; + explicit_bzero(&aesKey, SIZEOF(aesKey)); + TRACE("dh_encode_append"); TRACE_STACK_USAGE(); - ASSERT(inSize < BUFFER_SIZE_PARANOIA); - ASSERT(ctx->initialized_magic == HASH_CONTEXT_INITIALIZED_MAGIC); - ASSERT(aes_key->initialized_magic == DH_AES_KEY_INITIALIZED_MAGIC); - ASSERT(ctx->cacheLength < CX_AES_BLOCK_SIZE); + // sanity checks + if (inSize >= BUFFER_SIZE_PARANOIA) { + return ERR_ASSERT; + } + if (ctx->initialized_magic != HASH_CONTEXT_INITIALIZED_MAGIC) { + return ERR_ASSERT; + } + if (ctx->cacheLength >= CX_AES_BLOCK_SIZE) { + return ERR_ASSERT; + } STATIC_ASSERT(SIZEOF(ctx->cache) >= CX_AES_BLOCK_SIZE, "dh_context_t->cache too small"); - size_t processedInput = 0; - size_t written = 0; + // Compute AES key + { + uint16_t err = dh_init_aes_key(&aesKey, pathSpec, publicKey); + if (err != SUCCESS) { + explicit_bzero(&aesKey, SIZEOF(aesKey)); + return err; + } + } + + uint16_t processedInput = 0; + uint16_t written = 0; while (1) { // fill ctx->buffer - size_t to_read = MIN(CX_AES_BLOCK_SIZE - ctx->cacheLength, inSize - processedInput); + uint16_t to_read = MIN(CX_AES_BLOCK_SIZE - ctx->cacheLength, inSize - processedInput); memcpy(ctx->cache + ctx->cacheLength, inBuffer + processedInput, to_read); ctx->cacheLength += to_read; processedInput += to_read; @@ -197,49 +271,104 @@ __noinline_due_to_stack__ size_t dh_encode_append(dh_context_t* ctx, break; } - written += processDHOneBlockFromCache(ctx, aes_key, outBuffer + written, outSize - written); + uint16_t restLength = *outSize - written; // remaining buffer + uint16_t err = processDHOneBlockFromCache(ctx, &aesKey, outBuffer + written, &restLength); + if (err != SUCCESS) { + explicit_bzero(&aesKey, SIZEOF(aesKey)); + return err; + } + written += restLength; // processDHOneBlockFromCache returns number of bytes written } TRACE("Leaving dh_encode_append, written: %d", (int) written); - return written; + *outSize = written; + return SUCCESS; } -__noinline_due_to_stack__ size_t dh_encode_finalize(dh_context_t* ctx, - const dh_aes_key_t* aes_key, - uint8_t* outBuffer, - size_t outSize) { +__noinline_due_to_stack__ WARN_UNUSED_RESULT uint16_t +dh_encode_finalize(dh_context_t *ctx, + const bip44_path_t *pathSpec, + const public_key_t *publicKey, + uint8_t *outBuffer, + uint16_t *outSize) { + // Crypto assets to be cleared + dh_aes_key_t aesKey; + explicit_bzero(&aesKey, SIZEOF(aesKey)); + // Crypto assets to be cleared in case of failure + explicit_bzero(outBuffer, *outSize); // It suffices to clear this after final hmac + TRACE("dh_encode_finalize"); + TRACE_STACK_USAGE(); - ASSERT(ctx->initialized_magic == HASH_CONTEXT_INITIALIZED_MAGIC); - ASSERT(aes_key->initialized_magic == DH_AES_KEY_INITIALIZED_MAGIC); - ASSERT(ctx->cacheLength < CX_AES_BLOCK_SIZE); - TRACE("Cache length %d", (int) ctx->cacheLength); + // sanity checks + if (ctx->initialized_magic != HASH_CONTEXT_INITIALIZED_MAGIC) { + return ERR_ASSERT; + } + if (ctx->cacheLength >= CX_AES_BLOCK_SIZE) { + return ERR_ASSERT; + } + STATIC_ASSERT(SIZEOF(ctx->cache) >= CX_AES_BLOCK_SIZE, "dh_context_t->cache too small"); TRACE_BUFFER(ctx->cache, ctx->cacheLength); + // Compute AES key + { + uint16_t err = dh_init_aes_key(&aesKey, pathSpec, publicKey); + if (err != SUCCESS) { + explicit_bzero(&aesKey, SIZEOF(aesKey)); + return err; + } + } + // fill the last cache block with integers equal to number of elements missing // if the next block is empty we create a block full of 0x10 (CX_AES_BLOCK_SIZE) STATIC_ASSERT(SIZEOF(ctx->cache) >= CX_AES_BLOCK_SIZE, "dh_context_t->cache too small"); uint8_t fillValue = CX_AES_BLOCK_SIZE - ctx->cacheLength; - for (size_t i = ctx->cacheLength; i < CX_AES_BLOCK_SIZE; i++) { + for (uint16_t i = ctx->cacheLength; i < CX_AES_BLOCK_SIZE; i++) { ctx->cache[i] = fillValue; } ctx->cacheLength = CX_AES_BLOCK_SIZE; uint8_t written = 0; - written += processDHOneBlockFromCache(ctx, aes_key, outBuffer + written, outSize - written); + + // Encode last block + { + uint16_t restLength = *outSize - written; // remaining buffer + uint16_t err = processDHOneBlockFromCache(ctx, &aesKey, outBuffer + written, &restLength); + explicit_bzero(&aesKey, SIZEOF(aesKey)); + if (err != SUCCESS) { + return err; + } + written += restLength; + } // finalize hmac and append base64 encode it and append to cyphertext - size_t hmacOutSize = SIZEOF(ctx->base64EncodingCache) - ctx->base64EncodingCacheLen; - cx_err_t err = cx_hmac_final((cx_hmac_t*) &ctx->hmacCtx, - ctx->base64EncodingCache + ctx->base64EncodingCacheLen, - &hmacOutSize); - ASSERT(err == CX_OK); - ASSERT(hmacOutSize == DH_HMAC_SIZE); - ctx->base64EncodingCacheLen += DH_HMAC_SIZE; - written += base64EncWholeBlocks(ctx->base64EncodingCache, - &ctx->base64EncodingCacheLen, - outBuffer + written, - outSize - written); + { + size_t hmacOutSize = SIZEOF(ctx->base64EncodingCache) - ctx->base64EncodingCacheLen; + cx_err_t err = cx_hmac_final((cx_hmac_t *) &ctx->hmacCtx, + ctx->base64EncodingCache + ctx->base64EncodingCacheLen, + &hmacOutSize); + // cache now contains possibly sensitive data + if (err != CX_OK || hmacOutSize != DH_HMAC_SIZE) { + explicit_bzero(ctx->base64EncodingCache, SIZEOF(ctx->base64EncodingCache)); + return ERR_ASSERT; + } + ctx->base64EncodingCacheLen += DH_HMAC_SIZE; + } + + // Encode cache content + { + uint16_t restLength = *outSize - written; // remaining buffer + uint16_t err = base64EncWholeBlocks(ctx->base64EncodingCache, + &ctx->base64EncodingCacheLen, + outBuffer + written, + &restLength); + if (err != SUCCESS) { + explicit_bzero(ctx->base64EncodingCache, SIZEOF(ctx->base64EncodingCache)); + explicit_bzero(outBuffer, *outSize); + return err; + } + written += restLength; // processDHOneBlockFromCache returns number of bytes written + } // the last base64 encoding block uint8_t lastBlock[3] = {0, 0, 0}; @@ -247,7 +376,11 @@ __noinline_due_to_stack__ size_t dh_encode_finalize(dh_context_t* ctx, case 0: break; case 1: - ASSERT(outSize >= written + BASE64_OUT_BLOCK_SIZE); + if (*outSize < written + BASE64_OUT_BLOCK_SIZE) { + explicit_bzero(ctx->base64EncodingCache, SIZEOF(ctx->base64EncodingCache)); + explicit_bzero(outBuffer, *outSize); + return ERR_ASSERT; + } lastBlock[0] = ctx->base64EncodingCache[0]; base64EncBlock(lastBlock, outBuffer + written); *(outBuffer + written + 2) = '='; @@ -255,7 +388,11 @@ __noinline_due_to_stack__ size_t dh_encode_finalize(dh_context_t* ctx, written += BASE64_OUT_BLOCK_SIZE; break; case 2: - ASSERT(outSize >= written + BASE64_OUT_BLOCK_SIZE); + if (*outSize < written + BASE64_OUT_BLOCK_SIZE) { + explicit_bzero(ctx->base64EncodingCache, SIZEOF(ctx->base64EncodingCache)); + explicit_bzero(outBuffer, *outSize); + return ERR_ASSERT; + } lastBlock[0] = ctx->base64EncodingCache[0]; lastBlock[1] = ctx->base64EncodingCache[1]; base64EncBlock(lastBlock, outBuffer + written); @@ -263,122 +400,166 @@ __noinline_due_to_stack__ size_t dh_encode_finalize(dh_context_t* ctx, written += BASE64_OUT_BLOCK_SIZE; break; default: - ASSERT(false); + explicit_bzero(ctx->base64EncodingCache, SIZEOF(ctx->base64EncodingCache)); + explicit_bzero(outBuffer, *outSize); + return ERR_ASSERT; } - return written; + *outSize = written; + return SUCCESS; + ; } -__noinline_due_to_stack__ size_t dh_encode(bip44_path_t* pathSpec, - public_key_t* publicKey, - const uint8_t* iv, - size_t ivSize, - const uint8_t* inBuffer, - size_t inSize, - uint8_t* outBuffer, - size_t outSize) { - dh_aes_key_t key; +#ifdef DEVEL +__noinline_due_to_stack__ WARN_UNUSED_RESULT uint16_t dh_encode(bip44_path_t *pathSpec, + public_key_t *publicKey, + const uint8_t *iv, + uint16_t ivSize, + const uint8_t *inBuffer, + uint16_t inSize, + uint8_t *outBuffer, + uint16_t *outSize) { TRACE("dh_encode"); - ASSERT(inSize < BUFFER_SIZE_PARANOIA); - size_t written = 0; - - BEGIN_TRY { - TRY { - TRACE_STACK_USAGE(); - dh_init_aes_key(&key, pathSpec, publicKey); - - dh_context_t ctx; - written += - dh_encode_init(&ctx, &key, iv, ivSize, outBuffer + written, outSize - written); - TRACE("Init: written %d", written); - TRACE_BUFFER(outBuffer, written); - written += dh_encode_append(&ctx, - &key, - inBuffer, - inSize, - outBuffer + written, - outSize - written); - TRACE("Append: written %d", written); - TRACE_BUFFER(outBuffer, written); - written += dh_encode_finalize(&ctx, &key, outBuffer + written, outSize - written); - TRACE("Finalize: written %d", written); - TRACE_BUFFER(outBuffer, written); - } - FINALLY { - explicit_bzero(&key, sizeof(key)); - } + dh_context_t ctx; + + uint16_t written = 0; + uint16_t remaining = *outSize - written; + uint16_t err = + dh_encode_init(&ctx, pathSpec, publicKey, iv, ivSize, outBuffer + written, &remaining); + if (err != SUCCESS) { + return err; } - END_TRY; + written += remaining; + + remaining = *outSize - written; + err = dh_encode_append(&ctx, + pathSpec, + publicKey, + inBuffer, + inSize, + outBuffer + written, + &remaining); + if (err != SUCCESS) { + return err; + } + written += remaining; + + remaining = *outSize - written; + err = dh_encode_finalize(&ctx, pathSpec, publicKey, outBuffer + written, &remaining); + if (err != SUCCESS) { + return err; + } + written += remaining; - return written; + *outSize = written; + return SUCCESS; } +#endif -__noinline_due_to_stack__ static void validateHmac(dh_aes_key_t* aes_key, - const uint8_t* buffer, - size_t inSize) { - VALIDATE(inSize >= DH_AES_IV_SIZE + CX_AES_BLOCK_SIZE + DH_HMAC_SIZE, ERR_INVALID_DATA); +__noinline_due_to_stack__ static WARN_UNUSED_RESULT uint16_t validateHmac(dh_aes_key_t *aesKey, + const uint8_t *buffer, + uint16_t inSize) { + if (inSize < DH_AES_IV_SIZE + CX_AES_BLOCK_SIZE + DH_HMAC_SIZE) { + return ERR_INVALID_DATA; + } cx_hmac_sha256_t hmac; - cx_err_t err = cx_hmac_sha256_init_no_throw(&hmac, aes_key->km, SIZEOF(aes_key->km)); - ASSERT(err == CX_OK); - err = cx_hmac_update((cx_hmac_t*) &hmac, buffer, inSize - DH_HMAC_SIZE); - ASSERT(err == CX_OK); + cx_err_t err = cx_hmac_sha256_init_no_throw(&hmac, aesKey->km, SIZEOF(aesKey->km)); + if (err != CX_OK) { + return ERR_ASSERT; + } + err = cx_hmac_update((cx_hmac_t *) &hmac, buffer, inSize - DH_HMAC_SIZE); + if (err != CX_OK) { + return ERR_ASSERT; + } uint8_t hmacBuf[DH_HMAC_SIZE]; size_t outLen = SIZEOF(hmacBuf); - err = cx_hmac_final((cx_hmac_t*) &hmac, hmacBuf, &outLen); - ASSERT(err == CX_OK); - ASSERT(outLen == DH_HMAC_SIZE); - VALIDATE(!memcmp(hmacBuf, buffer + inSize - DH_HMAC_SIZE, DH_HMAC_SIZE), ERR_INVALID_HMAC); + err = cx_hmac_final((cx_hmac_t *) &hmac, hmacBuf, &outLen); + if (err != CX_OK || outLen != DH_HMAC_SIZE) { + return ERR_ASSERT; + } + if (memcmp(hmacBuf, buffer + inSize - DH_HMAC_SIZE, DH_HMAC_SIZE)) { + return ERR_INVALID_HMAC; + } + return SUCCESS; } -__noinline_due_to_stack__ size_t dh_decode(bip44_path_t* pathSpec, - public_key_t* publicKey, - uint8_t* buffer, - size_t inSize) { - VALIDATE(inSize >= DH_AES_IV_SIZE + CX_AES_BLOCK_SIZE + DH_HMAC_SIZE, ERR_INVALID_DATA); - VALIDATE(inSize % CX_AES_BLOCK_SIZE == 0, ERR_INVALID_DATA); - - size_t read = - DH_AES_IV_SIZE; // we do not decode IV, this also creates a buffer so we can decode inplace - size_t written = 0; - dh_aes_key_t aes_key; - BEGIN_TRY { - TRY { - dh_init_aes_key(&aes_key, pathSpec, publicKey); - - // validate HMAC - validateHmac(&aes_key, buffer, inSize); - TRACE("HMAC validation succesfull."); - - // initiate DH decryptions - uint8_t IV[CX_AES_BLOCK_SIZE]; - memcpy(IV, buffer, SIZEOF(IV)); - - for (; read < inSize - DH_HMAC_SIZE; read += CX_AES_BLOCK_SIZE) { - // 1. Decode next block - ASSERT(read - written == CX_AES_BLOCK_SIZE); - cx_err_t err = cx_aes_dec_block(&aes_key.aesKey, buffer + read, buffer + written); - ASSERT(err == CX_OK); - // 2. XOR with IV - for (size_t i = 0; i < CX_AES_BLOCK_SIZE; i++) { - buffer[written + i] ^= IV[i]; - } - // 3. Cyphertext is the new IV ... we do not care that we copy part of HMAC in last - // iteration here - memcpy(IV, buffer + read, CX_AES_BLOCK_SIZE); - written += CX_AES_BLOCK_SIZE; - } +__noinline_due_to_stack__ WARN_UNUSED_RESULT uint16_t dh_decode(bip44_path_t *pathSpec, + public_key_t *publicKey, + uint8_t *buffer, + uint16_t *size) { + // Crypto assets to be cleared + dh_aes_key_t aesKey; + explicit_bzero(&aesKey, SIZEOF(aesKey)); + // Crypto assets to be cleared in case of failure + TRACE_BUFFER(buffer, *size); // buffer after decoding + + if (*size < DH_AES_IV_SIZE + CX_AES_BLOCK_SIZE + DH_HMAC_SIZE) { + return ERR_INVALID_DATA; + } + if (*size % CX_AES_BLOCK_SIZE != 0) { + return ERR_INVALID_DATA; + } + + // we do not decode IV, this also creates a buffer so we can decode inplace + uint16_t read = DH_AES_IV_SIZE; + uint16_t written = 0; + { + uint16_t err = dh_init_aes_key(&aesKey, pathSpec, publicKey); + if (err != SUCCESS) { + explicit_bzero(&aesKey, SIZEOF(aesKey)); + return err; } - FINALLY { - explicit_bzero(&aes_key, sizeof(aes_key)); + } + + // validate HMAC + { + uint16_t err = validateHmac(&aesKey, buffer, *size); + if (err != SUCCESS) { + explicit_bzero(&aesKey, SIZEOF(aesKey)); + return err; } + + TRACE("HMAC validation succesfull."); } - END_TRY; + + // initiate DH decryptions + uint8_t IV[CX_AES_BLOCK_SIZE]; + memcpy(IV, buffer, SIZEOF(IV)); + + for (; read < *size - DH_HMAC_SIZE; read += CX_AES_BLOCK_SIZE) { + // 1. Decode next block + if (read - written != CX_AES_BLOCK_SIZE) { + explicit_bzero(&aesKey, SIZEOF(aesKey)); + explicit_bzero(buffer, *size); + return ERR_ASSERT; + } + cx_err_t err = cx_aes_dec_block(&aesKey.aesKey, buffer + read, buffer + written); + if (err != CX_OK) { + explicit_bzero(&aesKey, SIZEOF(aesKey)); + explicit_bzero(buffer, *size); + return ERR_ASSERT; + } + // 2. XOR with IV + for (uint16_t i = 0; i < CX_AES_BLOCK_SIZE; i++) { + buffer[written + i] ^= IV[i]; + } + // 3. Cyphertext is the new IV ... we do not care that we copy part of HMAC in last + // iteration here + memcpy(IV, buffer + read, CX_AES_BLOCK_SIZE); + written += CX_AES_BLOCK_SIZE; + } + + explicit_bzero(&aesKey, SIZEOF(aesKey)); TRACE("Finishing decription, written:%d, lastCharacter:%d", written, buffer[written - 1]); // Calculate resulting length based on the last decoded value - ASSERT(written != 0); - ASSERT(written >= buffer[written - 1]); - return written - buffer[written - 1]; + if (written == 0 || written < buffer[written - 1]) { + explicit_bzero(buffer, *size); + return ERR_INVALID_DATA; + } + + *size = written - buffer[written - 1]; + return SUCCESS; } diff --git a/src/diffieHellman.h b/src/diffieHellman.h index 4006ed2c..931e6fe1 100644 --- a/src/diffieHellman.h +++ b/src/diffieHellman.h @@ -39,63 +39,146 @@ typedef struct { // Aes key is secret shared between us and the second party typedef struct { - uint16_t initialized_magic; cx_aes_key_t aesKey; uint8_t km[DH_KM_SIZE]; } dh_aes_key_t; -// You may need to call multiple times if encryption spans multiple APDU's -// You may want to do something like -// APDU1 - dh_init_aes_key, dh_encode_init, dh_encode_append -// APDU2 - dh_init_aes_key, dh_encode_append -// APDU3 - dh_init_aes_key, dh_encode_append, dh_encode_finalize -// Do not forget to guarantee that dh_aes_key_t is zeroes at all times - -__noinline_due_to_stack__ void dh_init_aes_key(dh_aes_key_t* dhKey, - const bip44_path_t* pathSpec, - const public_key_t* publicKey); - -// Output data are base64 encrypted -__noinline_due_to_stack__ size_t dh_encode_init(dh_context_t* ctx, - const dh_aes_key_t* aes_key, - const uint8_t* iv, - size_t ivSize, - uint8_t* outBuffer, - size_t outSize); - -// Output data are base64 encrypted -__noinline_due_to_stack__ size_t dh_encode_append(dh_context_t* ctx, - const dh_aes_key_t* aes_key, - const uint8_t* inBuffer, - size_t inSize, - uint8_t* outBuffer, - size_t outSize); - -// Output data are base64 encrypted -__noinline_due_to_stack__ size_t dh_encode_finalize(dh_context_t* ctx, - const dh_aes_key_t* aes_key, - uint8_t* outBuffer, - size_t outSize); - -// Convenience function to make all in one step -// Output data are base64 encrypted -__noinline_due_to_stack__ size_t dh_encode(bip44_path_t* pathSpec, - public_key_t* publicKey, - const uint8_t* iv, - size_t ivSize, - const uint8_t* inBuffer, - size_t inSize, - uint8_t* outBuffer, - size_t outSize); - -// Inplace decoding function. -// Input data is NOT base64 encrypted -__noinline_due_to_stack__ size_t dh_decode(bip44_path_t* pathSpec, - public_key_t* publicKey, - uint8_t* buffer, - size_t inSize); +/** + * @brief Generates shared secret AES key. Use only in crypto functions to guarantee cleanup. + * + * @param[out] dhKey Resulting AES key. + * + * @param[in] pathSpec Our derivation path. + * + * @param[in] publicKey Their public key. + * + * @return Error code: + * - SUCCESS on success + * - ERR_REJECTED_BY_POLICY + * - ERR_ASSERT for other unexpected errors + */ +__noinline_due_to_stack__ WARN_UNUSED_RESULT uint16_t +dh_init_aes_key(dh_aes_key_t *dhKey, const bip44_path_t *pathSpec, const public_key_t *publicKey); + +/** + * @brief Initiates DH+Base64 ecoding. + * + * @param[out] ctx DH encoding context. + * + * @param[in] pathSpec Our derivation path. + * + * @param[in] publicKey Their public key. + * + * @param[in] iv Initiatlization vector. + * + * @param[in] ivSize Size of initialization vector. + * + * @param[out] outBuffer Output buffer, initialization produces cyphertext. + * + * @param[in, out] outSize Size of output buffer. returns the number of new bytes. + * + * @return Error code: + * - SUCCESS on success + * - ERR_REJECTED_BY_POLICY + * - ERR_ASSERT for other unexpected errors + */ +__noinline_due_to_stack__ WARN_UNUSED_RESULT uint16_t dh_encode_init(dh_context_t *ctx, + const bip44_path_t *pathSpec, + const public_key_t *publicKey, + const uint8_t *iv, + uint16_t ivSize, + uint8_t *outBuffer, + uint16_t *outSize); + +/** + * @brief Append to DH+Base64 ecoded data. + * + * @param[in, out] ctx DH encoding context. + * + * @param[in] pathSpec Our derivation path. + * + * @param[in] publicKey Their public key. + * + * @param[in] inBuffer Input buffer. + * + * @param[in] inSize Input buffer size. + * + * @param[out] outBuffer Output buffer, initialization produces cyphertext. + * + * @param[in, out] outSize Size of output buffer. returns the number of new bytes. + * + * @return Error code: + * - SUCCESS on success + * - ERR_REJECTED_BY_POLICY + * - ERR_ASSERT for other unexpected errors + */ +__noinline_due_to_stack__ WARN_UNUSED_RESULT uint16_t +dh_encode_append(dh_context_t *ctx, + const bip44_path_t *pathSpec, + const public_key_t *publicKey, + const uint8_t *inBuffer, + uint16_t inSize, + uint8_t *outBuffer, + uint16_t *outSize); + +/** + * @brief Finish DH+Base64 encoding. + * + * @param[in] ctx DH encoding context. + * + * @param[in] pathSpec Our derivation path. + * + * @param[in] publicKey Their public key. + * + * @param[out] outBuffer Output buffer, initialization produces cyphertext. + * + * @param[in, out] outSize Size of output buffer. returns the number of new bytes. + * + * @return Error code: + * - SUCCESS on success + * - ERR_REJECTED_BY_POLICY + * - ERR_ASSERT for other unexpected errors + */ +__noinline_due_to_stack__ WARN_UNUSED_RESULT uint16_t +dh_encode_finalize(dh_context_t *ctx, + const bip44_path_t *pathSpec, + const public_key_t *publicKey, + uint8_t *outBuffer, + uint16_t *outSize); + +/** + * @brief Inplace DH decoding function. Input data is NOT base64 encrypted + * + * @param[in] pathSpec Our derivation path. + * + * @param[in] publicKey Their public key. + * + * @param[out] buffer Output buffer, initialization produces cyphertext. + * + * @param[in, out] size Size of output buffer. returns the number of new bytes. + * + * @return Error code: + * - SUCCESS on success + * - ERR_REJECTED_BY_POLICY + * - ERR_INVALID_DATA + * - ERR_INVALID_HMAC + * - ERR_ASSERT for other unexpected errors + */ +__noinline_due_to_stack__ WARN_UNUSED_RESULT uint16_t dh_decode(bip44_path_t *pathSpec, + public_key_t *publicKey, + uint8_t *buffer, + uint16_t *size); #ifdef DEVEL +__noinline_due_to_stack__ WARN_UNUSED_RESULT uint16_t dh_encode(bip44_path_t *pathSpec, + public_key_t *publicKey, + const uint8_t *iv, + uint16_t ivSize, + const uint8_t *inBuffer, + uint16_t inSize, + uint8_t *outBuffer, + uint16_t *outSize); + __noinline_due_to_stack__ void run_diffieHellman_test(); #endif // DEVEL diff --git a/src/diffieHellmann_test.c b/src/diffieHellmann_test.c index 76d7fd31..e26f9e72 100644 --- a/src/diffieHellmann_test.c +++ b/src/diffieHellmann_test.c @@ -57,14 +57,18 @@ __noinline_due_to_stack__ static void run_dh_encode_tests() { uint8_t msg[50]; \ size_t msgLen = decode_hex(msgHex, msg, SIZEOF(msg)); \ uint8_t encMsg[200]; \ - size_t encMsgLength = dh_encode(&pathSpec, \ - &publicKey, \ - IV, \ - DH_AES_IV_SIZE, \ - msg, \ - msgLen, \ - encMsg, \ - SIZEOF(encMsg)); \ + uint16_t encMsgLength = SIZEOF(encMsg); \ + uint16_t err = dh_encode(&pathSpec, \ + &publicKey, \ + IV, \ + DH_AES_IV_SIZE, \ + msg, \ + msgLen, \ + encMsg, \ + &encMsgLength); \ + if (err != SUCCESS) { \ + THROW(err); \ + } \ TRACE_BUFFER(encMsg, encMsgLength); \ ASSERT(encMsgLength == strlen(expectedEncMsg)); \ EXPECT_EQ_BYTES(encMsg, expectedEncMsg, strlen(expectedEncMsg)); \ @@ -132,9 +136,6 @@ __noinline_due_to_stack__ static void run_dh_encode_init_append_finalize_tests() const uint8_t IV[DH_AES_IV_SIZE] = {100, 100, 100, 100, 100, 100, 100, 100, 100, 100, 100, 100, 100, 100, 100, 100}; - dh_aes_key_t key; - dh_init_aes_key(&key, &pathSpec, &publicKey); - uint8_t inBuffer[50]; uint8_t outBuffer[160]; const char* inBufferHex = @@ -146,15 +147,16 @@ __noinline_due_to_stack__ static void run_dh_encode_init_append_finalize_tests() "llUGwVFl+lJ3Qpxm5u0fLG9kxuEGaN18XnBMEu6iJXmhERjaQhHhN4L/" "gZPs9AD9BNvPMXpd9H9EnR27den0hCjQqECg6QE5dZrTnSiZDLN/Yg=="; - size_t outBufferLength = 0; - outBufferLength = dh_encode(&pathSpec, - &publicKey, - IV, - DH_AES_IV_SIZE, - inBuffer, - inBufferLength, - outBuffer, - SIZEOF(outBuffer)); + uint16_t outBufferLength = SIZEOF(outBuffer); + uint16_t err = dh_encode(&pathSpec, + &publicKey, + IV, + DH_AES_IV_SIZE, + inBuffer, + inBufferLength, + outBuffer, + &outBufferLength); + ASSERT(err == SUCCESS); TRACE_BUFFER(outBuffer, outBufferLength); ASSERT(outBufferLength == strlen(expected)); EXPECT_EQ_BYTES(expected, outBuffer, outBufferLength); @@ -165,43 +167,87 @@ __noinline_due_to_stack__ static void run_dh_encode_init_append_finalize_tests() TRACE_STACK_USAGE(); // due to memory limitations we reuse static tx data dh_context_t* ctx = &instructionState.signTransactionContext.dhContext; - outBufferLength = - dh_encode_init(ctx, &key, IV, SIZEOF(IV), outBuffer, SIZEOF(outBuffer)); + outBufferLength = SIZEOF(outBuffer); + err = dh_encode_init(ctx, + &pathSpec, + &publicKey, + IV, + SIZEOF(IV), + outBuffer, + &outBufferLength); + ASSERT(err == SUCCESS); ASSERT(outBufferLength == 20); EXPECT_EQ_BYTES(expected + written, outBuffer, 20); written += 20; // Cache 0b, 1b { - outBufferLength = - dh_encode_append(ctx, &key, inBuffer + read, 1, outBuffer, SIZEOF(outBuffer)); + outBufferLength = SIZEOF(outBuffer); + err = dh_encode_append(ctx, + &pathSpec, + &publicKey, + inBuffer + read, + 1, + outBuffer, + &outBufferLength); + ASSERT(err == SUCCESS); ASSERT(outBufferLength == 0); read += 1; // Cache 1b, 1b } { - outBufferLength = - dh_encode_append(ctx, &key, inBuffer + read, 14, outBuffer, SIZEOF(outBuffer)); + outBufferLength = SIZEOF(outBuffer); + err = dh_encode_append(ctx, + &pathSpec, + &publicKey, + inBuffer + read, + 14, + outBuffer, + &outBufferLength); + ASSERT(err == SUCCESS); ASSERT(outBufferLength == 0); read += 14; // Cache 15b, 1b } - outBufferLength = - dh_encode_append(ctx, &key, inBuffer + read, 18, outBuffer, SIZEOF(outBuffer)); + outBufferLength = SIZEOF(outBuffer); + err = dh_encode_append(ctx, + &pathSpec, + &publicKey, + inBuffer + read, + 18, + outBuffer, + &outBufferLength); + ASSERT(err == SUCCESS); ASSERT(outBufferLength == 44); EXPECT_EQ_BYTES(expected + written, outBuffer, 44); read += 18, written += 44; // Cache 1b, 0b - outBufferLength = - dh_encode_append(ctx, &key, inBuffer + read, 1, outBuffer, SIZEOF(outBuffer)); + outBufferLength = SIZEOF(outBuffer); + err = dh_encode_append(ctx, + &pathSpec, + &publicKey, + inBuffer + read, + 1, + outBuffer, + &outBufferLength); + ASSERT(err == SUCCESS); ASSERT(outBufferLength == 0); read += 1; // Cache 2b, 0b - outBufferLength = - dh_encode_append(ctx, &key, inBuffer + read, 14, outBuffer, SIZEOF(outBuffer)); + outBufferLength = SIZEOF(outBuffer); + err = dh_encode_append(ctx, + &pathSpec, + &publicKey, + inBuffer + read, + 14, + outBuffer, + &outBufferLength); + ASSERT(err == SUCCESS); ASSERT(outBufferLength == 20); EXPECT_EQ_BYTES(expected + written, outBuffer, 20); read += 14, written += 20; // Cache 0b, 1b - outBufferLength = dh_encode_finalize(ctx, &key, outBuffer, SIZEOF(outBuffer)); + outBufferLength = SIZEOF(outBuffer); + err = dh_encode_finalize(ctx, &pathSpec, &publicKey, outBuffer, &outBufferLength); + ASSERT(err == SUCCESS); ASSERT(outBufferLength == 68); EXPECT_EQ_BYTES(expected + written, outBuffer, 68); written += 68; @@ -240,15 +286,18 @@ __noinline_due_to_stack__ static void run_dh_decode_tests() { { \ const char* msgHex = msgHex_; \ uint8_t msg[120]; \ - size_t msgLen = decode_hex(msgHex, msg, SIZEOF(msg)); \ + uint16_t msgLen = decode_hex(msgHex, msg, SIZEOF(msg)); \ const char* expectedDecMsgHex = expectedDecMsgHex_; \ uint8_t expectedDecMsg[100]; \ size_t expectedMsgLen = \ decode_hex(expectedDecMsgHex, expectedDecMsg, SIZEOF(expectedDecMsg)); \ - size_t decMsgLen = dh_decode(&pathSpec, &publicKey, msg, msgLen); \ - TRACE("Decoded mesage %d, %d", decMsgLen, expectedMsgLen); \ - ASSERT(decMsgLen == expectedMsgLen); \ - EXPECT_EQ_BYTES(msg, expectedDecMsg, decMsgLen); \ + uint16_t err = dh_decode(&pathSpec, &publicKey, msg, &msgLen); \ + if (err != SUCCESS) { \ + THROW(err); \ + } \ + TRACE("Decoded mesage %d, %d", msgLen, expectedMsgLen); \ + ASSERT(msgLen == expectedMsgLen); \ + EXPECT_EQ_BYTES(msg, expectedDecMsg, msgLen); \ } TESTCASE( "000102030405060708090a0b0c0d0e0f9508b492f96f067cc72ef8c7c24ac2072310c4e1d36bd6737958f0" @@ -321,9 +370,9 @@ __noinline_due_to_stack__ static void run_dh_decode_failed_hmac_tests() { "05576d60a63b30e52db993fdb53f67ba03cd0abed894f54929ac6addfd7076970597a43a36c525ad1fc4349c69" "be21718ab07bc639172663927cb075fa777797e0c1c4"; uint8_t msg[120]; - size_t msgLen = decode_hex(msgHex, msg, SIZEOF(msg)); + uint16_t msgLen = decode_hex(msgHex, msg, SIZEOF(msg)); - EXPECT_THROWS(dh_decode(&pathSpec, &publicKey, msg, msgLen), ERR_INVALID_HMAC); + ASSERT(dh_decode(&pathSpec, &publicKey, msg, &msgLen) == ERR_INVALID_HMAC); } __noinline_due_to_stack__ void run_diffieHellman_test() { diff --git a/src/getPublicKey.c b/src/getPublicKey.c index 25fa0f41..f97df1a5 100644 --- a/src/getPublicKey.c +++ b/src/getPublicKey.c @@ -80,7 +80,7 @@ static void runGetPublicKeyUIFlow() { { // Calculation uint16_t err = derivePublicKey(&ctx->pathSpec, &ctx->pubKey); - if (err != CX_OK) { + if (err != SUCCESS) { THROW(err); } ctx->responseReadyMagic = RESPONSE_READY_MAGIC; diff --git a/src/keyDerivation.c b/src/keyDerivation.c index 5b2530af..7e946f2c 100644 --- a/src/keyDerivation.c +++ b/src/keyDerivation.c @@ -17,6 +17,12 @@ static uint8_t const SECP256K1_N[] = { __noinline_due_to_stack__ WARN_UNUSED_RESULT uint16_t derivePrivateKey(const bip44_path_t* pathSpec, private_key_t* privateKey) { + // Crypto assets to be cleared + uint8_t privateKeySeed[PRIVATE_KEY_SEED_LEN]; + explicit_bzero(privateKeySeed, SIZEOF(privateKeySeed)); + // Crypto assets to be cleared in case of failure + explicit_bzero(privateKey, SIZEOF(*privateKey)); + if (policyDerivePrivateKey(pathSpec) == POLICY_DENY) { return ERR_REJECTED_BY_POLICY; } @@ -26,11 +32,8 @@ __noinline_due_to_stack__ WARN_UNUSED_RESULT uint16_t derivePrivateKey(const bip return ERR_ASSERT; } - TRACE(); - uint8_t privateKeySeed[PRIVATE_KEY_SEED_LEN]; - explicit_bzero(privateKeySeed, SIZEOF(privateKeySeed)); - STATIC_ASSERT(CX_APILEVEL >= 5, "unsupported api level"); + TRACE(); cx_err_t err = os_derive_bip32_with_seed_no_throw(HDW_NORMAL, CX_CURVE_SECP256K1, @@ -46,8 +49,8 @@ __noinline_due_to_stack__ WARN_UNUSED_RESULT uint16_t derivePrivateKey(const bip } err = cx_ecfp_init_private_key_no_throw(CX_CURVE_SECP256K1, privateKeySeed, 32, privateKey); + explicit_bzero(privateKeySeed, SIZEOF(privateKeySeed)); if (err != CX_OK) { - explicit_bzero(privateKeySeed, SIZEOF(privateKeySeed)); explicit_bzero(privateKey, SIZEOF(*privateKey)); return ERR_ASSERT; } @@ -57,6 +60,7 @@ __noinline_due_to_stack__ WARN_UNUSED_RESULT uint16_t derivePrivateKey(const bip __noinline_due_to_stack__ WARN_UNUSED_RESULT uint16_t derivePublicKey(const bip44_path_t* pathSpec, public_key_t* publicKey) { + // Crypto assets to be cleared private_key_t privateKey; explicit_bzero(&privateKey, SIZEOF(privateKey)); @@ -78,11 +82,12 @@ __noinline_due_to_stack__ WARN_UNUSED_RESULT uint16_t derivePublicKey(const bip4 publicKey, &privateKey, 1); // 1 - private key preserved + explicit_bzero(&privateKey, SIZEOF(privateKey)); if (err != CX_OK) { - explicit_bzero(&privateKey, SIZEOF(privateKey)); return ERR_ASSERT; } + TRACE(); return SUCCESS; } @@ -92,14 +97,20 @@ __noinline_due_to_stack__ WARN_UNUSED_RESULT uint16_t signTransaction(bip44_path uint8_t hashBuf[SHA_256_SIZE], uint8_t* signature, size_t signatureLen) { + // Crypto assets to be cleared + private_key_t privateKey; + explicit_bzero(&privateKey, SIZEOF(privateKey)); + uint8_t V[33]; + explicit_bzero(V, SIZEOF(V)); + uint8_t K[32]; + explicit_bzero(K, SIZEOF(K)); + // Crypto assets to be cleared in case of failure + explicit_bzero(signature, signatureLen); + if (signatureLen < 200) { return ERR_ASSERT; } - // Derive keys and sign the transaction, setup - private_key_t privateKey; - explicit_bzero(&privateKey, SIZEOF(privateKey)); - // We derive the private key { uint16_t err = derivePrivateKey(wittnessPath, &privateKey); @@ -112,9 +123,6 @@ __noinline_due_to_stack__ WARN_UNUSED_RESULT uint16_t signTransaction(bip44_path } // We sign the hash - explicit_bzero(signature, signatureLen); - uint8_t V[33]; - uint8_t K[32]; int tries = 0; // Loop until a candidate matching the canonical signature is found @@ -131,6 +139,8 @@ __noinline_due_to_stack__ WARN_UNUSED_RESULT uint16_t signTransaction(bip44_path if (err != CX_OK) { explicit_bzero(&privateKey, SIZEOF(privateKey)); explicit_bzero(signature, signatureLen); + explicit_bzero(V, SIZEOF(V)); + explicit_bzero(K, SIZEOF(K)); return ERR_ASSERT; } } else { @@ -138,6 +148,8 @@ __noinline_due_to_stack__ WARN_UNUSED_RESULT uint16_t signTransaction(bip44_path if (err != CX_OK) { explicit_bzero(&privateKey, SIZEOF(privateKey)); explicit_bzero(signature, signatureLen); + explicit_bzero(V, SIZEOF(V)); + explicit_bzero(K, SIZEOF(K)); return ERR_ASSERT; } } @@ -155,6 +167,8 @@ __noinline_due_to_stack__ WARN_UNUSED_RESULT uint16_t signTransaction(bip44_path if (err != CX_OK) { explicit_bzero(&privateKey, SIZEOF(privateKey)); explicit_bzero(signature, signatureLen); + explicit_bzero(V, SIZEOF(V)); + explicit_bzero(K, SIZEOF(K)); return ERR_ASSERT; } TRACE_BUFFER(signature + 100, 100); @@ -176,5 +190,7 @@ __noinline_due_to_stack__ WARN_UNUSED_RESULT uint16_t signTransaction(bip44_path } explicit_bzero(&privateKey, sizeof(privateKey)); + explicit_bzero(V, SIZEOF(V)); + explicit_bzero(K, SIZEOF(K)); return SUCCESS; } diff --git a/src/signTransaction.c b/src/signTransaction.c index b44deedb..97f479a0 100644 --- a/src/signTransaction.c +++ b/src/signTransaction.c @@ -40,35 +40,27 @@ enum { // &ctx->dhContext) are needed for encryption static void processShaAndPosibleDHAndPrepareResponse() { if (ctx->dhIsActive) { - dh_aes_key_t aesKey; - BEGIN_TRY { - TRY { - // Compute AES key - dh_init_aes_key(&aesKey, &ctx->wittnessPath, &ctx->otherPubkey); - - // Encode message chunk - ctx->responseLength = dh_encode_append(&ctx->dhContext, - &aesKey, - ctx->dataToAppendToTx, - ctx->dataToAppendToTxLen, - G_io_apdu_buffer, - SIZEOF(G_io_apdu_buffer)); - CX_THROW(sha_256_append(&ctx->hashContext, G_io_apdu_buffer, ctx->responseLength)); - VALIDATE( - ctx->countedSectionDifference + ctx->responseLength >= ctx->dataToAppendToTxLen, - ERR_INVALID_STATE); - ctx->countedSectionDifference = - ctx->countedSectionDifference + ctx->responseLength - ctx->dataToAppendToTxLen; - TRACE("CS diff %d from:%d, %d", - (int) ctx->countedSectionDifference, - (int) ctx->responseLength, - (int) ctx->dataToAppendToTxLen); - } - FINALLY { - explicit_bzero(&aesKey, SIZEOF(aesKey)); - } + uint16_t bufferLen = SIZEOF(G_io_apdu_buffer); + uint16_t err = dh_encode_append(&ctx->dhContext, + &ctx->wittnessPath, + &ctx->otherPubkey, + ctx->dataToAppendToTx, + ctx->dataToAppendToTxLen, + G_io_apdu_buffer, + &bufferLen); + if (err != SUCCESS) { + THROW(err); } - END_TRY; + ctx->responseLength = bufferLen; + CX_THROW(sha_256_append(&ctx->hashContext, G_io_apdu_buffer, ctx->responseLength)); + VALIDATE(ctx->countedSectionDifference + ctx->responseLength >= ctx->dataToAppendToTxLen, + ERR_INVALID_STATE); + ctx->countedSectionDifference = + ctx->countedSectionDifference + ctx->responseLength - ctx->dataToAppendToTxLen; + TRACE("CS diff %d from:%d, %d", + (int) ctx->countedSectionDifference, + (int) ctx->responseLength, + (int) ctx->dataToAppendToTxLen); } else { cx_err_t err = sha_256_append(&ctx->hashContext, ctx->dataToAppendToTx, ctx->dataToAppendToTxLen); @@ -703,37 +695,32 @@ __noinline_due_to_stack__ void signTx_handleStartDHEncodingAPDU( TRACE_STACK_USAGE(); VALIDATE(!ctx->dhIsActive, ERR_INVALID_STATE); - dh_aes_key_t aesKey; - BEGIN_TRY { - TRY { - TRACE_STACK_USAGE(); - // Compute AES key - dh_init_aes_key(&aesKey, &ctx->wittnessPath, &ctx->otherPubkey); - - // Generate IV - uint8_t IV[DH_AES_IV_SIZE]; - cx_rng_no_throw(IV, SIZEOF(IV)); - - // INIT dh context - STATIC_ASSERT(DH_AES_IV_SIZE == CX_AES_BLOCK_SIZE, "Unexpected IV length"); - ctx->dhCountedSectionEntryLevel = ctx->countedSections.currentLevel; - ctx->responseLength = dh_encode_init(&ctx->dhContext, - &aesKey, - IV, - SIZEOF(IV), - G_io_apdu_buffer, - SIZEOF(G_io_apdu_buffer)); - if (ctx->responseLength != 20) { // first 5 blocks - THROW(ERR_ASSERT); - } - ctx->countedSectionDifference = ctx->responseLength; - TRACE("CS diff %d", (int) ctx->responseLength); - } - FINALLY { - explicit_bzero(&aesKey, SIZEOF(aesKey)); - } + TRACE_STACK_USAGE(); + // Generate IV + uint8_t IV[DH_AES_IV_SIZE]; + cx_rng_no_throw(IV, SIZEOF(IV)); + + // INIT dh context + STATIC_ASSERT(DH_AES_IV_SIZE == CX_AES_BLOCK_SIZE, "Unexpected IV length"); + ctx->dhCountedSectionEntryLevel = ctx->countedSections.currentLevel; + uint16_t bufferLen = SIZEOF(G_io_apdu_buffer); + + uint16_t err = dh_encode_init(&ctx->dhContext, + &ctx->wittnessPath, + &ctx->otherPubkey, + IV, + SIZEOF(IV), + G_io_apdu_buffer, + &bufferLen); + if (err != SUCCESS) { + THROW(err); + } + ctx->responseLength = bufferLen; + if (ctx->responseLength != 20) { // first 5 blocks + THROW(ERR_ASSERT); } - END_TRY; + ctx->countedSectionDifference = ctx->responseLength; + TRACE("CS diff %d", (int) ctx->responseLength); ASSERT(sha_256_append(&ctx->hashContext, G_io_apdu_buffer, ctx->responseLength) == CX_OK); ctx->dhIsActive = true; @@ -809,22 +796,16 @@ __noinline_due_to_stack__ void signTx_handleEndDHEncodingAPDU( VALIDATE(ctx->dhCountedSectionEntryLevel == ctx->countedSections.currentLevel, ERR_INVALID_STATE); - dh_aes_key_t aesKey; - BEGIN_TRY { - TRY { - // Compute AES key - dh_init_aes_key(&aesKey, &ctx->wittnessPath, &ctx->otherPubkey); - - ctx->responseLength = dh_encode_finalize(&ctx->dhContext, - &aesKey, - G_io_apdu_buffer, - SIZEOF(G_io_apdu_buffer)); - } - FINALLY { - explicit_bzero(&aesKey, SIZEOF(aesKey)); - } + uint16_t bufferLen = SIZEOF(G_io_apdu_buffer); + uint16_t err = dh_encode_finalize(&ctx->dhContext, + &ctx->wittnessPath, + &ctx->otherPubkey, + G_io_apdu_buffer, + &bufferLen); + if (err != SUCCESS) { + THROW(err); } - END_TRY; + ctx->responseLength = bufferLen; ctx->countedSectionDifference += ctx->responseLength; TRACE("CS diff %d from:%d",