diff --git a/src/decodeDH.c b/src/decodeDH.c index 9ceb3c0b..d78f2eee 100644 --- a/src/decodeDH.c +++ b/src/decodeDH.c @@ -505,6 +505,9 @@ void decode_handleAPDU(uint8_t p1, TRACE_BUFFER(ctx->buffer, ctx->bufferLen); uint16_t err = dh_decode(&ctx->pathSpec, &ctx->otherPubKey, ctx->buffer, &ctx->bufferLen); if (err != SUCCESS) { + explicit_bzero(ctx->buffer, ctx->bufferLen); + TRACE(); + PRINTF("Error: %d\n", err); THROW(err); } ctx->messageDecodedMagic = DECODING_FINISHED_MAGIC; diff --git a/src/diffieHellman.c b/src/diffieHellman.c index 44c28537..a7d9c6e7 100644 --- a/src/diffieHellman.c +++ b/src/diffieHellman.c @@ -1,6 +1,12 @@ #include "diffieHellman.h" #include "os_math.h" +// This is crypto module, we do not want to jump from it +// To avoid mistakes, we undef most common macros we do not want to use +#undef ASSERT +#undef THROW +#undef VALIDATE + //---------------------------- UTILS --------------------------------------- static const uint8_t BASE64[64] = {'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', @@ -22,6 +28,9 @@ static WARN_UNUSED_RESULT uint16_t base64EncWholeBlocks(uint8_t *inBuffer, uint8_t *inSize, uint8_t *outBuffer, uint16_t *outSize) { + // Variable for CRYPTO_ error handling macros + uint16_t error_to_return = ERR_ASSERT; + uint8_t processedBlocks = 0; while (1) { // We cannot process whole block @@ -33,15 +42,15 @@ static WARN_UNUSED_RESULT uint16_t base64EncWholeBlocks(uint8_t *inBuffer, } // process one block - if (*outSize < BASE64_OUT_BLOCK_SIZE * (processedBlocks + 1)) { - return ERR_ASSERT; - }; + CRYPTO_ASSERT(*outSize >= BASE64_OUT_BLOCK_SIZE * (processedBlocks + 1)); base64EncBlock(inBuffer + BASE64_IN_BLOCK_SIZE * processedBlocks, outBuffer + BASE64_OUT_BLOCK_SIZE * processedBlocks); processedBlocks++; } *outSize = BASE64_OUT_BLOCK_SIZE * processedBlocks; - return SUCCESS; + error_to_return = SUCCESS; +end: + return error_to_return; } // Encodes one block from cache using AES key @@ -50,10 +59,11 @@ static WARN_UNUSED_RESULT uint16_t processDHOneBlockFromCache(dh_context_t *ctx, const dh_aes_key_t *aesKey, uint8_t *outBuffer, uint16_t *outSize) { + // Variable for CRYPTO_ error handling macros + uint16_t error_to_return = ERR_ASSERT; + // precondition sanity check - if (ctx->cacheLength != CX_AES_BLOCK_SIZE) { - return ERR_ASSERT; - } + CRYPTO_ASSERT(ctx->cacheLength == CX_AES_BLOCK_SIZE); STATIC_ASSERT(SIZEOF(ctx->IV) == CX_AES_BLOCK_SIZE, "Incompatible IV size"); // We work in CBC mode @@ -63,34 +73,28 @@ static WARN_UNUSED_RESULT uint16_t processDHOneBlockFromCache(dh_context_t *ctx, } // 2. encrypt the result to obtain cyphertext 3. use cyphertext as new IV - cx_err_t err = cx_aes_enc_block(&aesKey->aesKey, ctx->cache, ctx->IV); - if (err != CX_OK) { - return ERR_ASSERT; - } + CRYPTO_CX_CHECK(cx_aes_enc_block(&aesKey->aesKey, ctx->cache, ctx->IV)); // 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); - if (err != CX_OK) { - return ERR_ASSERT; - } + CRYPTO_CX_CHECK(cx_hmac_update((cx_hmac_t *) &ctx->hmacCtx, ctx->IV, CX_AES_BLOCK_SIZE)); explicit_bzero(ctx->cache, SIZEOF(ctx->cache)); ctx->cacheLength = 0; // base64 encode - // sanity check, we expect that cache does not contain whole block first - if (ctx->base64EncodingCacheLen >= BASE64_IN_BLOCK_SIZE) { - return ERR_ASSERT; - } + CRYPTO_ASSERT(ctx->base64EncodingCacheLen < BASE64_IN_BLOCK_SIZE); 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, - outSize); + CRYPTO_FORWARD_ERROR(base64EncWholeBlocks(ctx->base64EncodingCache, + &ctx->base64EncodingCacheLen, + outBuffer, + outSize)); + error_to_return = SUCCESS; +end: // CRYPTO_ macros jump here in case of error + return error_to_return; } //---------------------------- DH ENCODING --------------------------------------- @@ -99,66 +103,48 @@ __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) { // Crypto assets to be cleared private_key_t privateKey; - explicit_bzero(&privateKey, SIZEOF(privateKey)); unsigned char basicSecret[32]; - explicit_bzero(basicSecret, SIZEOF(basicSecret)); unsigned char secret[SHA_512_SIZE]; - explicit_bzero(secret, SIZEOF(secret)); uint8_t K[SHA_512_SIZE]; + 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)); + // Variable for CRYPTO_ error handling macros + uint16_t error_to_return = ERR_ASSERT; + TRACE_STACK_USAGE(); TRACE("dh_init_aesKey"); - { - uint16_t err = derivePrivateKey(pathSpec, &privateKey); - if (err != SUCCESS) { - explicit_bzero(&privateKey, SIZEOF(privateKey)); - return err; - } - } - - 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; - } + CRYPTO_FORWARD_ERROR(derivePrivateKey(pathSpec, &privateKey)); - 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; - } + // Derive aes key. + CRYPTO_CX_CHECK(cx_ecdh_no_throw(&privateKey, + CX_ECDH_X, + publicKey->W, + publicKey->W_len, + basicSecret, + SIZEOF(basicSecret))); + CRYPTO_CX_CHECK(sha_512_hash(basicSecret, SIZEOF(basicSecret), secret, SIZEOF(secret))); + CRYPTO_CX_CHECK(sha_512_hash(secret, SIZEOF(secret), K, SIZEOF(K))); // 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; - } + CRYPTO_CX_CHECK(cx_aes_init_key_no_throw(K, DH_AES_SECRET_SIZE, &dhKey->aesKey)); STATIC_ASSERT(SIZEOF(dhKey->km) == DH_KM_SIZE, "Incompatible types"); memmove(dhKey->km, K + DH_AES_SECRET_SIZE, DH_KM_SIZE); + + error_to_return = SUCCESS; +end: // CRYPTO_ macros jump here in case of error + explicit_bzero(&privateKey, SIZEOF(privateKey)); + explicit_bzero(basicSecret, SIZEOF(basicSecret)); + explicit_bzero(secret, SIZEOF(secret)); explicit_bzero(K, SIZEOF(K)); - return SUCCESS; + return error_to_return; } __noinline_due_to_stack__ WARN_UNUSED_RESULT uint16_t dh_encode_init(dh_context_t *ctx, @@ -172,52 +158,47 @@ __noinline_due_to_stack__ WARN_UNUSED_RESULT uint16_t dh_encode_init(dh_context_ dh_aes_key_t aesKey; explicit_bzero(&aesKey, SIZEOF(aesKey)); + // Variable for CRYPTO_ error handling macros + uint16_t error_to_return = ERR_ASSERT; + TRACE_STACK_USAGE(); TRACE("dh_encode_init"); STATIC_ASSERT(SIZEOF(ctx->IV) == CX_AES_BLOCK_SIZE, "Unexpected IV length"); - if (ivSize != SIZEOF(ctx->IV)) { - return ERR_ASSERT; - } + CRYPTO_ASSERT(ivSize == SIZEOF(ctx->IV)); - // initialize DH context + // 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; - } - } + // Compute AES key and forward error + CRYPTO_FORWARD_ERROR(dh_init_aes_key(&aesKey, pathSpec, publicKey)); explicit_bzero(&ctx->hmacCtx, SIZEOF(ctx->hmacCtx)); - 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; + CRYPTO_CX_CHECK(cx_hmac_sha256_init_no_throw(&ctx->hmacCtx, aesKey.km, SIZEOF(aesKey.km))); + CRYPTO_CX_CHECK(cx_hmac_update((cx_hmac_t *) &ctx->hmacCtx, iv, ivSize)); // Base64 We encode IV 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); + + ctx->initialized_magic = HASH_CONTEXT_INITIALIZED_MAGIC; + // Context initialization finished + + // This also sets outSize as desired. + CRYPTO_FORWARD_ERROR(base64EncWholeBlocks(ctx->base64EncodingCache, + &ctx->base64EncodingCacheLen, + outBuffer, + outSize)); + error_to_return = SUCCESS; +end: + // CX_CHECK macro jumps her in case of an error. Cleanup. + explicit_bzero(&aesKey, SIZEOF(aesKey)); + return error_to_return; } __noinline_due_to_stack__ WARN_UNUSED_RESULT uint16_t @@ -232,29 +213,20 @@ dh_encode_append(dh_context_t *ctx, dh_aes_key_t aesKey; explicit_bzero(&aesKey, SIZEOF(aesKey)); + // Variable for CRYPTO_ error handling macros + uint16_t error_to_return = ERR_ASSERT; + TRACE("dh_encode_append"); TRACE_STACK_USAGE(); // 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; - } + CRYPTO_ASSERT(inSize < BUFFER_SIZE_PARANOIA); + CRYPTO_ASSERT(ctx->initialized_magic == HASH_CONTEXT_INITIALIZED_MAGIC); + CRYPTO_ASSERT(ctx->cacheLength < CX_AES_BLOCK_SIZE); STATIC_ASSERT(SIZEOF(ctx->cache) >= CX_AES_BLOCK_SIZE, "dh_context_t->cache too small"); - // Compute AES key - { - uint16_t err = dh_init_aes_key(&aesKey, pathSpec, publicKey); - if (err != SUCCESS) { - explicit_bzero(&aesKey, SIZEOF(aesKey)); - return err; - } - } + // Compute AES key and forward error + CRYPTO_FORWARD_ERROR(dh_init_aes_key(&aesKey, pathSpec, publicKey)); uint16_t processedInput = 0; uint16_t written = 0; @@ -273,18 +245,18 @@ dh_encode_append(dh_context_t *ctx, } 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; - } + CRYPTO_FORWARD_ERROR( + processDHOneBlockFromCache(ctx, &aesKey, outBuffer + written, &restLength)); written += restLength; // processDHOneBlockFromCache returns number of bytes written } - - explicit_bzero(&aesKey, SIZEOF(aesKey)); TRACE("Leaving dh_encode_append, written: %d", (int) written); + + // Cleanup *outSize = written; - return SUCCESS; + error_to_return = SUCCESS; +end: + explicit_bzero(&aesKey, SIZEOF(aesKey)); + return error_to_return; } __noinline_due_to_stack__ WARN_UNUSED_RESULT uint16_t @@ -296,34 +268,25 @@ dh_encode_finalize(dh_context_t *ctx, // Crypto assets to be cleared dh_aes_key_t aesKey; explicit_bzero(&aesKey, SIZEOF(aesKey)); - // ctx->base64EncodingCache will contain final HMAC at one point - // Crypto assets to be cleared in case of failure - explicit_bzero(outBuffer, *outSize); // It suffices to clear this after final hmac + // ctx->base64EncodingCache + + // Variable for CRYPTO_ error handling macros + uint16_t error_to_return = ERR_ASSERT; TRACE("dh_encode_finalize"); TRACE_STACK_USAGE(); // sanity checks - if (ctx->initialized_magic != HASH_CONTEXT_INITIALIZED_MAGIC) { - return ERR_ASSERT; - } - if (ctx->cacheLength >= CX_AES_BLOCK_SIZE) { - return ERR_ASSERT; - } + CRYPTO_ASSERT(ctx->initialized_magic == HASH_CONTEXT_INITIALIZED_MAGIC); + CRYPTO_ASSERT(ctx->cacheLength < CX_AES_BLOCK_SIZE); 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; - } - } + CRYPTO_FORWARD_ERROR(dh_init_aes_key(&aesKey, pathSpec, publicKey)); - // 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) + // 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 (uint16_t i = ctx->cacheLength; i < CX_AES_BLOCK_SIZE; i++) { @@ -336,40 +299,28 @@ dh_encode_finalize(dh_context_t *ctx, // 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; - } + CRYPTO_FORWARD_ERROR( + processDHOneBlockFromCache(ctx, &aesKey, outBuffer + written, &restLength)); 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); - // cache now contains possibly sensitive data - if (err != CX_OK || hmacOutSize != DH_HMAC_SIZE) { - explicit_bzero(ctx->base64EncodingCache, SIZEOF(ctx->base64EncodingCache)); - return ERR_ASSERT; - } + CRYPTO_CX_CHECK(cx_hmac_final((cx_hmac_t *) &ctx->hmacCtx, + ctx->base64EncodingCache + ctx->base64EncodingCacheLen, + &hmacOutSize)); + CRYPTO_ASSERT(hmacOutSize == DH_HMAC_SIZE); 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; - } + CRYPTO_FORWARD_ERROR(base64EncWholeBlocks(ctx->base64EncodingCache, + &ctx->base64EncodingCacheLen, + outBuffer + written, + &restLength)); written += restLength; // processDHOneBlockFromCache returns number of bytes written } @@ -379,11 +330,7 @@ dh_encode_finalize(dh_context_t *ctx, case 0: break; case 1: - if (*outSize < written + BASE64_OUT_BLOCK_SIZE) { - explicit_bzero(ctx->base64EncodingCache, SIZEOF(ctx->base64EncodingCache)); - explicit_bzero(outBuffer, *outSize); - return ERR_ASSERT; - } + CRYPTO_ASSERT(*outSize >= written + BASE64_OUT_BLOCK_SIZE); lastBlock[0] = ctx->base64EncodingCache[0]; base64EncBlock(lastBlock, outBuffer + written); *(outBuffer + written + 2) = '='; @@ -391,11 +338,7 @@ dh_encode_finalize(dh_context_t *ctx, written += BASE64_OUT_BLOCK_SIZE; break; case 2: - if (*outSize < written + BASE64_OUT_BLOCK_SIZE) { - explicit_bzero(ctx->base64EncodingCache, SIZEOF(ctx->base64EncodingCache)); - explicit_bzero(outBuffer, *outSize); - return ERR_ASSERT; - } + CRYPTO_ASSERT(*outSize >= written + BASE64_OUT_BLOCK_SIZE); lastBlock[0] = ctx->base64EncodingCache[0]; lastBlock[1] = ctx->base64EncodingCache[1]; base64EncBlock(lastBlock, outBuffer + written); @@ -403,13 +346,15 @@ dh_encode_finalize(dh_context_t *ctx, written += BASE64_OUT_BLOCK_SIZE; break; default: - explicit_bzero(ctx->base64EncodingCache, SIZEOF(ctx->base64EncodingCache)); - explicit_bzero(outBuffer, *outSize); - return ERR_ASSERT; + CRYPTO_ASSERT(false); } - explicit_bzero(ctx->base64EncodingCache, SIZEOF(ctx->base64EncodingCache)); + *outSize = written; - return SUCCESS; + error_to_return = SUCCESS; +end: + explicit_bzero(&aesKey, SIZEOF(aesKey)); + explicit_bzero(ctx->base64EncodingCache, SIZEOF(ctx->base64EncodingCache)); + return error_to_return; } #ifdef DEVEL @@ -421,71 +366,62 @@ __noinline_due_to_stack__ WARN_UNUSED_RESULT uint16_t dh_encode(bip44_path_t *pa uint16_t inSize, uint8_t *outBuffer, uint16_t *outSize) { + // Variable for CRYPTO_ error handling macros + uint16_t error_to_return = ERR_ASSERT; + TRACE("dh_encode"); - ASSERT(inSize < BUFFER_SIZE_PARANOIA); + CRYPTO_ASSERT(inSize < BUFFER_SIZE_PARANOIA); 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; - } + CRYPTO_FORWARD_ERROR( + dh_encode_init(&ctx, pathSpec, publicKey, iv, ivSize, outBuffer + written, &remaining)); written += remaining; remaining = *outSize - written; - err = dh_encode_append(&ctx, - pathSpec, - publicKey, - inBuffer, - inSize, - outBuffer + written, - &remaining); - if (err != SUCCESS) { - return err; - } + CRYPTO_FORWARD_ERROR(dh_encode_append(&ctx, + pathSpec, + publicKey, + inBuffer, + inSize, + outBuffer + written, + &remaining)); written += remaining; remaining = *outSize - written; - err = dh_encode_finalize(&ctx, pathSpec, publicKey, outBuffer + written, &remaining); - if (err != SUCCESS) { - return err; - } + CRYPTO_FORWARD_ERROR( + dh_encode_finalize(&ctx, pathSpec, publicKey, outBuffer + written, &remaining)); written += remaining; *outSize = written; - return SUCCESS; + error_to_return = SUCCESS; +end: + return error_to_return; } #endif __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; - } + // Variable for CRYPTO_ error handling macros + uint16_t error_to_return = ERR_ASSERT; + + CRYPTO_VALIDATE(inSize >= DH_AES_IV_SIZE + CX_AES_BLOCK_SIZE + DH_HMAC_SIZE, ERR_INVALID_DATA); cx_hmac_sha256_t hmac; - 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; - } + CRYPTO_CX_CHECK(cx_hmac_sha256_init_no_throw(&hmac, aesKey->km, SIZEOF(aesKey->km))); + CRYPTO_CX_CHECK(cx_hmac_update((cx_hmac_t *) &hmac, buffer, inSize - DH_HMAC_SIZE)); uint8_t hmacBuf[DH_HMAC_SIZE]; size_t outLen = SIZEOF(hmacBuf); - 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; + CRYPTO_CX_CHECK(cx_hmac_final((cx_hmac_t *) &hmac, hmacBuf, &outLen)); + CRYPTO_ASSERT(outLen == DH_HMAC_SIZE); + CRYPTO_VALIDATE(!memcmp(hmacBuf, buffer + inSize - DH_HMAC_SIZE, DH_HMAC_SIZE), + ERR_INVALID_HMAC); + error_to_return = SUCCESS; +end: + return error_to_return; } __noinline_due_to_stack__ WARN_UNUSED_RESULT uint16_t dh_decode(bip44_path_t *pathSpec, @@ -495,37 +431,21 @@ __noinline_due_to_stack__ WARN_UNUSED_RESULT uint16_t dh_decode(bip44_path_t *pa // 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); // only 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; - } + // Variable for CRYPTO_ error handling macros + uint16_t error_to_return = ERR_ASSERT; + + CRYPTO_VALIDATE(*size >= DH_AES_IV_SIZE + CX_AES_BLOCK_SIZE + DH_HMAC_SIZE, ERR_INVALID_DATA); + CRYPTO_VALIDATE(*size % CX_AES_BLOCK_SIZE == 0, 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; - } - } + CRYPTO_FORWARD_ERROR(dh_init_aes_key(&aesKey, pathSpec, publicKey)); // validate HMAC - { - uint16_t err = validateHmac(&aesKey, buffer, *size); - if (err != SUCCESS) { - explicit_bzero(&aesKey, SIZEOF(aesKey)); - return err; - } - - TRACE("HMAC validation succesfull."); - } + CRYPTO_FORWARD_ERROR(validateHmac(&aesKey, buffer, *size)); + TRACE("HMAC validation succesfull."); // initiate DH decryptions uint8_t IV[CX_AES_BLOCK_SIZE]; @@ -533,17 +453,8 @@ __noinline_due_to_stack__ WARN_UNUSED_RESULT uint16_t dh_decode(bip44_path_t *pa 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; - } + CRYPTO_ASSERT(read - written == CX_AES_BLOCK_SIZE); + CRYPTO_CX_CHECK(cx_aes_dec_block(&aesKey.aesKey, buffer + read, buffer + written)); // 2. XOR with IV for (uint16_t i = 0; i < CX_AES_BLOCK_SIZE; i++) { buffer[written + i] ^= IV[i]; @@ -554,15 +465,13 @@ __noinline_due_to_stack__ WARN_UNUSED_RESULT uint16_t dh_decode(bip44_path_t *pa 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 - if (written == 0 || written < buffer[written - 1]) { - explicit_bzero(buffer, *size); - return ERR_INVALID_DATA; - } + CRYPTO_VALIDATE(written != 0 && written >= buffer[written - 1], ERR_INVALID_DATA); *size = written - buffer[written - 1]; - return SUCCESS; + error_to_return = SUCCESS; +end: + explicit_bzero(&aesKey, SIZEOF(aesKey)); + return error_to_return; } diff --git a/src/keyDerivation.c b/src/keyDerivation.c index 977f845b..cd835208 100644 --- a/src/keyDerivation.c +++ b/src/keyDerivation.c @@ -15,53 +15,38 @@ static uint8_t const SECP256K1_N[] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfe, 0xba, 0xae, 0xdc, 0xe6, 0xaf, 0x48, 0xa0, 0x3b, 0xbf, 0xd2, 0x5e, 0x8c, 0xd0, 0x36, 0x41, 0x41}; -#define FORWARD_CX_ERROR(call) \ - { \ - cx_err_t callResult = (call); \ - if (callResult != CX_OK) return callResult; \ - } - __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; - } + // Variable for CRYPTO_ error handling macros + uint16_t error_to_return = ERR_ASSERT; - // Sanity check - if (pathSpec->length >= ARRAY_LEN(pathSpec->path)) { - return ERR_ASSERT; - } + CRYPTO_VALIDATE(policyDerivePrivateKey(pathSpec) != POLICY_DENY, ERR_REJECTED_BY_POLICY); + // Sanity check + CRYPTO_ASSERT(pathSpec->length < ARRAY_LEN(pathSpec->path)); 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, - pathSpec->path, - pathSpec->length, - privateKeySeed, - NULL, - NULL, - 0); - if (err != CX_OK) { - explicit_bzero(privateKeySeed, SIZEOF(privateKeySeed)); - return ERR_ASSERT; - } - - err = cx_ecfp_init_private_key_no_throw(CX_CURVE_SECP256K1, privateKeySeed, 32, privateKey); + CRYPTO_CX_CHECK(os_derive_bip32_with_seed_no_throw(HDW_NORMAL, + CX_CURVE_SECP256K1, + pathSpec->path, + pathSpec->length, + privateKeySeed, + NULL, + NULL, + 0)); + CRYPTO_CX_CHECK( + cx_ecfp_init_private_key_no_throw(CX_CURVE_SECP256K1, privateKeySeed, 32, privateKey)); + + error_to_return = SUCCESS; +end: explicit_bzero(privateKeySeed, SIZEOF(privateKeySeed)); - if (err != CX_OK) { - explicit_bzero(privateKey, SIZEOF(*privateKey)); - return ERR_ASSERT; - } - - return SUCCESS; + return error_to_return; } __noinline_due_to_stack__ WARN_UNUSED_RESULT uint16_t derivePublicKey(const bip44_path_t *pathSpec, @@ -70,31 +55,20 @@ __noinline_due_to_stack__ WARN_UNUSED_RESULT uint16_t derivePublicKey(const bip4 private_key_t privateKey; explicit_bzero(&privateKey, SIZEOF(privateKey)); - { - uint16_t err = derivePrivateKey(pathSpec, &privateKey); - if (err != SUCCESS) { - explicit_bzero(&privateKey, SIZEOF(privateKey)); - return err; - } - } + // Variable for CRYPTO_ error handling macros + uint16_t error_to_return = ERR_ASSERT; - cx_err_t err = cx_ecfp_init_public_key_no_throw(CX_CURVE_SECP256K1, NULL, 0, publicKey); - if (err != CX_OK) { - explicit_bzero(&privateKey, SIZEOF(privateKey)); - return ERR_ASSERT; - } + CRYPTO_FORWARD_ERROR(derivePrivateKey(pathSpec, &privateKey)); - err = cx_ecfp_generate_pair_no_throw(CX_CURVE_SECP256K1, - publicKey, - &privateKey, - 1); // 1 - private key preserved - explicit_bzero(&privateKey, SIZEOF(privateKey)); - if (err != CX_OK) { - return ERR_ASSERT; - } + CRYPTO_CX_CHECK(cx_ecfp_init_public_key_no_throw(CX_CURVE_SECP256K1, NULL, 0, publicKey)); - TRACE(); - return SUCCESS; + CRYPTO_CX_CHECK(cx_ecfp_generate_pair_no_throw(CX_CURVE_SECP256K1, + publicKey, + &privateKey, + 1)); // 1 - private key preserved + error_to_return = SUCCESS; +end: + return error_to_return; } // EOS way to check if a signature is canonical :/ @@ -155,7 +129,7 @@ static int ecdsa_der_to_sig(const uint8_t *der, uint8_t *sig) { * - V, out * - K, out */ -static cx_err_t rng_rfc6979(unsigned char *rnd, +static uint16_t rng_rfc6979(unsigned char *rnd, unsigned char *h1, unsigned char *x, unsigned int x_len, @@ -163,6 +137,9 @@ static cx_err_t rng_rfc6979(unsigned char *rnd, unsigned int q_len, unsigned char *V, unsigned char *K) { + // Variable for CRYPTO_ error handling macros + uint16_t error_to_return = ERR_ASSERT; + unsigned int h_len, found, i; cx_hmac_sha256_t hmac; @@ -179,32 +156,32 @@ static cx_err_t rng_rfc6979(unsigned char *rnd, memset(K, 0x00, h_len); // d. Set: K = HMAC_K(V || 0x00 || int2octets(x) || bits2octets(h1)) V[h_len] = 0; - FORWARD_CX_ERROR(cx_hmac_sha256_init_no_throw(&hmac, K, 32)); - FORWARD_CX_ERROR(cx_hmac_no_throw((cx_hmac_t *) &hmac, 0, V, h_len + 1, K, 32)); - FORWARD_CX_ERROR(cx_hmac_no_throw((cx_hmac_t *) &hmac, 0, x, x_len, K, 32)); - FORWARD_CX_ERROR(cx_hmac_no_throw((cx_hmac_t *) &hmac, CX_LAST, h1, h_len, K, 32)); + CRYPTO_CX_CHECK(cx_hmac_sha256_init_no_throw(&hmac, K, 32)); + CRYPTO_CX_CHECK(cx_hmac_no_throw((cx_hmac_t *) &hmac, 0, V, h_len + 1, K, 32)); + CRYPTO_CX_CHECK(cx_hmac_no_throw((cx_hmac_t *) &hmac, 0, x, x_len, K, 32)); + CRYPTO_CX_CHECK(cx_hmac_no_throw((cx_hmac_t *) &hmac, CX_LAST, h1, h_len, K, 32)); // e. Set: V = HMAC_K(V) - FORWARD_CX_ERROR(cx_hmac_sha256_init_no_throw(&hmac, K, 32)); - FORWARD_CX_ERROR(cx_hmac_no_throw((cx_hmac_t *) &hmac, CX_LAST, V, h_len, V, 32)); + CRYPTO_CX_CHECK(cx_hmac_sha256_init_no_throw(&hmac, K, 32)); + CRYPTO_CX_CHECK(cx_hmac_no_throw((cx_hmac_t *) &hmac, CX_LAST, V, h_len, V, 32)); // f. Set: K = HMAC_K(V || 0x01 || int2octets(x) || bits2octets(h1)) V[h_len] = 1; - FORWARD_CX_ERROR(cx_hmac_sha256_init_no_throw(&hmac, K, 32)); - FORWARD_CX_ERROR(cx_hmac_no_throw((cx_hmac_t *) &hmac, 0, V, h_len + 1, K, 32)); - FORWARD_CX_ERROR(cx_hmac_no_throw((cx_hmac_t *) &hmac, 0, x, x_len, K, 32)); - FORWARD_CX_ERROR(cx_hmac_no_throw((cx_hmac_t *) &hmac, CX_LAST, h1, h_len, K, 32)); + CRYPTO_CX_CHECK(cx_hmac_sha256_init_no_throw(&hmac, K, 32)); + CRYPTO_CX_CHECK(cx_hmac_no_throw((cx_hmac_t *) &hmac, 0, V, h_len + 1, K, 32)); + CRYPTO_CX_CHECK(cx_hmac_no_throw((cx_hmac_t *) &hmac, 0, x, x_len, K, 32)); + CRYPTO_CX_CHECK(cx_hmac_no_throw((cx_hmac_t *) &hmac, CX_LAST, h1, h_len, K, 32)); // g. Set: V = HMAC_K(V) -- - FORWARD_CX_ERROR(cx_hmac_sha256_init_no_throw(&hmac, K, 32)); - FORWARD_CX_ERROR(cx_hmac_no_throw((cx_hmac_t *) &hmac, CX_LAST, V, h_len, V, 32)); + CRYPTO_CX_CHECK(cx_hmac_sha256_init_no_throw(&hmac, K, 32)); + CRYPTO_CX_CHECK(cx_hmac_no_throw((cx_hmac_t *) &hmac, CX_LAST, V, h_len, V, 32)); // initial setup only once x = NULL; } else { // h.3 K = HMAC_K(V || 0x00) V[h_len] = 0; - FORWARD_CX_ERROR(cx_hmac_sha256_init_no_throw(&hmac, K, 32)); - FORWARD_CX_ERROR(cx_hmac_no_throw((cx_hmac_t *) &hmac, CX_LAST, V, h_len + 1, K, 32)); + CRYPTO_CX_CHECK(cx_hmac_sha256_init_no_throw(&hmac, K, 32)); + CRYPTO_CX_CHECK(cx_hmac_no_throw((cx_hmac_t *) &hmac, CX_LAST, V, h_len + 1, K, 32)); // h.3 V = HMAC_K(V) - FORWARD_CX_ERROR(cx_hmac_sha256_init_no_throw(&hmac, K, 32)); - FORWARD_CX_ERROR(cx_hmac_no_throw((cx_hmac_t *) &hmac, CX_LAST, V, h_len, V, 32)); + CRYPTO_CX_CHECK(cx_hmac_sha256_init_no_throw(&hmac, K, 32)); + CRYPTO_CX_CHECK(cx_hmac_no_throw((cx_hmac_t *) &hmac, CX_LAST, V, h_len, V, 32)); } // generate candidate @@ -220,8 +197,8 @@ static cx_err_t rng_rfc6979(unsigned char *rnd, if (x_len < h_len) { h_len = x_len; } - FORWARD_CX_ERROR(cx_hmac_sha256_init_no_throw(&hmac, K, 32)); - FORWARD_CX_ERROR(cx_hmac_no_throw((cx_hmac_t *) &hmac, CX_LAST, V, h_len, V, 32)); + CRYPTO_CX_CHECK(cx_hmac_sha256_init_no_throw(&hmac, K, 32)); + CRYPTO_CX_CHECK(cx_hmac_no_throw((cx_hmac_t *) &hmac, CX_LAST, V, h_len, V, 32)); memcpy(rnd, V, h_len); x_len -= h_len; } @@ -235,7 +212,9 @@ static cx_err_t rng_rfc6979(unsigned char *rnd, } } - return CX_OK; + error_to_return = SUCCESS; +end: + return error_to_return; } // This function contains code producing signatures that is taken from EOS app @@ -246,78 +225,43 @@ __noinline_due_to_stack__ WARN_UNUSED_RESULT uint16_t signTransaction(bip44_path 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(&privateKey, SIZEOF(privateKey)); + explicit_bzero(V, SIZEOF(V)); explicit_bzero(K, SIZEOF(K)); - // Crypto assets to be cleared in case of failure explicit_bzero(signature, signatureLen); - if (signatureLen < 200) { - return ERR_ASSERT; - } + // Variable for CRYPTO_ error handling macros + uint16_t error_to_return = ERR_ASSERT; - // We derive the private key - { - uint16_t err = derivePrivateKey(wittnessPath, &privateKey); - if (err != SUCCESS) { - explicit_bzero(&privateKey, SIZEOF(privateKey)); - return err; - } - TRACE("privateKey.d:"); - TRACE_BUFFER(privateKey.d, privateKey.d_len); - } + CRYPTO_ASSERT(signatureLen >= 200); + + CRYPTO_FORWARD_ERROR(derivePrivateKey(wittnessPath, &privateKey)); + TRACE("privateKey.d:"); + TRACE_BUFFER(privateKey.d, privateKey.d_len); // We sign the hash int tries = 0; - // Loop until a candidate matching the canonical signature is found for (;;) { if (tries == 0) { - cx_err_t err = rng_rfc6979(signature + 100, - hashBuf, - privateKey.d, - privateKey.d_len, - SECP256K1_N, - 32, - V, - K); - 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; - } + CRYPTO_FORWARD_ERROR(rng_rfc6979 + (signature + 100, hashBuf, privateKey.d, privateKey.d_len, SECP256K1_N, 32, V, K)); } else { - cx_err_t err = rng_rfc6979(signature + 100, hashBuf, NULL, 0, SECP256K1_N, 32, V, K); - 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; - } + CRYPTO_FORWARD_ERROR(rng_rfc6979(signature + 100, hashBuf, NULL, 0, SECP256K1_N, 32, V, K)); } uint32_t infos; size_t sig_len_ = 100; - cx_err_t err = cx_ecdsa_sign_no_throw(&privateKey, - CX_NO_CANONICAL | CX_RND_PROVIDED | CX_LAST, - CX_SHA256, - hashBuf, - 32, - signature + 100, - &sig_len_, - &infos); - 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; - } + CRYPTO_CX_CHECK(cx_ecdsa_sign_no_throw(&privateKey, + CX_NO_CANONICAL | CX_RND_PROVIDED | CX_LAST, + CX_SHA256, + hashBuf, + 32, + signature + 100, + &sig_len_, + &infos)); TRACE_BUFFER(signature + 100, 100); if ((infos & CX_ECCINFO_PARITY_ODD) != 0) { @@ -336,8 +280,10 @@ __noinline_due_to_stack__ WARN_UNUSED_RESULT uint16_t signTransaction(bip44_path } } + error_to_return = SUCCESS; +end: explicit_bzero(&privateKey, sizeof(privateKey)); explicit_bzero(V, SIZEOF(V)); explicit_bzero(K, SIZEOF(K)); - return SUCCESS; + return error_to_return; } diff --git a/src/signTransaction.c b/src/signTransaction.c index 2f500d54..da680227 100644 --- a/src/signTransaction.c +++ b/src/signTransaction.c @@ -803,6 +803,7 @@ __noinline_due_to_stack__ void signTx_handleEndDHEncodingAPDU( G_io_apdu_buffer, &bufferLen); if (err != SUCCESS) { + explicit_bzero(G_io_apdu_buffer, SIZEOF(G_io_apdu_buffer)); THROW(err); } ctx->responseLength = bufferLen; @@ -932,6 +933,7 @@ __noinline_due_to_stack__ void signTx_handleFinishAPDU( uint16_t err = signTransaction(&ctx->wittnessPath, hashBuf, G_io_apdu_buffer, SIZEOF(G_io_apdu_buffer)); if (err != SUCCESS) { + explicit_bzero(G_io_apdu_buffer, SIZEOF(G_io_apdu_buffer)); THROW(err); } diff --git a/src/utils.h b/src/utils.h index 1124e405..576dfb16 100644 --- a/src/utils.h +++ b/src/utils.h @@ -119,4 +119,43 @@ extern unsigned int app_stack_canary; #define TRACE_STACK_USAGE() #endif // DEVEL +// In crypto functions we do not want to throw errors, just return them +// To make this easy we use these macros +// They expect that the function has +// uint16_t error_to_return variable +// end: that contains cleanup +#define CRYPTO_CX_CHECK(call) \ + { \ + if ((call) != CX_OK) { \ + TRACE(); \ + error_to_return = ERR_ASSERT; \ + goto end; \ + } \ + } +#define CRYPTO_FORWARD_ERROR(call) \ + { \ + uint16_t err = (call); \ + if (err != SUCCESS) { \ + TRACE(); \ + error_to_return = err; \ + goto end; \ + } \ + } +#define CRYPTO_ASSERT(condition) \ + { \ + if (!(condition)) { \ + TRACE(); \ + error_to_return = ERR_ASSERT; \ + goto end; \ + } \ + } +#define CRYPTO_VALIDATE(condition, error) \ + { \ + if (!(condition)) { \ + TRACE(); \ + error_to_return = error; \ + goto end; \ + } \ + } + #endif // H_FIO_APP_UTILS