From 4896a4895588ab803663fcd3ff603f53fd8c6d04 Mon Sep 17 00:00:00 2001 From: Takashi Kojo Date: Wed, 20 Nov 2019 19:32:18 +0900 Subject: [PATCH 1/4] fix EVP_CipherUpdate padding --- wolfcrypt/src/evp.c | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/wolfcrypt/src/evp.c b/wolfcrypt/src/evp.c index 9ff2ff59d..6f35df7ed 100644 --- a/wolfcrypt/src/evp.c +++ b/wolfcrypt/src/evp.c @@ -375,10 +375,12 @@ WOLFSSL_API int wolfSSL_EVP_CipherUpdate(WOLFSSL_EVP_CIPHER_CTX *ctx, { int blocks; int fill; + int copied; if ((ctx == NULL) || (inl < 0) || (outl == NULL)|| (in == NULL)) return BAD_FUNC_ARG; WOLFSSL_ENTER("wolfSSL_EVP_CipherUpdate"); + printf("wolfSSL_EVP_CipherUpdate\n"); *outl = 0; if (inl == 0) return WOLFSSL_SUCCESS; @@ -400,34 +402,42 @@ WOLFSSL_API int wolfSSL_EVP_CipherUpdate(WOLFSSL_EVP_CIPHER_CTX *ctx, return BAD_FUNC_ARG; if (ctx->bufUsed > 0) { /* concatinate them if there is anything */ + printf("head fillBuff inl=%d, ctx->bufUsed=%d\n", inl, ctx->bufUsed); fill = fillBuff(ctx, in, inl); inl -= fill; in += fill; + printf("head fillBuff inl=%d, ctx->bufUsed=%d\n", inl, ctx->bufUsed); } - if ((ctx->enc == 0)&& (ctx->lastUsed == 1)) { + if ((ctx->enc == 0) && (ctx->lastUsed == 1) && (inl > 0/*ctx->block_size*/)) { PRINT_BUF(ctx->lastBlock, ctx->block_size); XMEMCPY(out, ctx->lastBlock, ctx->block_size); *outl+= ctx->block_size; out += ctx->block_size; + ctx->lastUsed = 0; + copied = 1; + printf("Copied out last block - 1\n"); } - if (ctx->bufUsed == ctx->block_size) { + if (ctx->bufUsed == ctx->block_size /* && inl/ctx->block_size != 0*/) { /* the buff is full, flash out */ PRINT_BUF(ctx->buf, ctx->block_size); if (evpCipherBlock(ctx, out, ctx->buf, ctx->block_size) == 0) return WOLFSSL_FAILURE; PRINT_BUF(out, ctx->block_size); - if (ctx->enc == 0) { - ctx->lastUsed = 1; - XMEMCPY(ctx->lastBlock, out, ctx->block_size); - } else { - *outl+= ctx->block_size; - out += ctx->block_size; - } + *outl+= ctx->block_size; + out += ctx->block_size; ctx->bufUsed = 0; } blocks = inl / ctx->block_size; if (blocks > 0) { + if ((ctx->enc == 0) && (ctx->lastUsed == 1) && (copied == 0)) { + PRINT_BUF(ctx->lastBlock, ctx->block_size); + XMEMCPY(out, ctx->lastBlock, ctx->block_size); + *outl += ctx->block_size; + out += ctx->block_size; + ctx->lastUsed = 0; + printf("Copied out last block - 2\n"); + } /* process blocks */ if (evpCipherBlock(ctx, out, in, blocks * ctx->block_size) == 0) return WOLFSSL_FAILURE; @@ -447,6 +457,7 @@ WOLFSSL_API int wolfSSL_EVP_CipherUpdate(WOLFSSL_EVP_CIPHER_CTX *ctx, * EVP_CipherFinal call */ XMEMCPY(ctx->lastBlock, &out[ctx->block_size * blocks], ctx->block_size); + printf("Saved last block\n"); } *outl+= ctx->block_size * blocks; } @@ -457,9 +468,10 @@ WOLFSSL_API int wolfSSL_EVP_CipherUpdate(WOLFSSL_EVP_CIPHER_CTX *ctx, if (inl > 0) { /* put fraction into buff */ fillBuff(ctx, in, inl); + printf("tailing fillBuff inl=%d\n", inl); /* no increase of outl */ } - + printf("Returning outl=%d, ctx->lastUsed= %d, ctx->bufUsed=%d\n", *outl, ctx->lastUsed, ctx->bufUsed); (void)out; /* silence warning in case not read */ return WOLFSSL_SUCCESS; @@ -515,6 +527,7 @@ int wolfSSL_EVP_CipherFinal(WOLFSSL_EVP_CIPHER_CTX *ctx, *outl = 0; } else if (ctx->enc) { + printf("EVP_CipherFinal(ctx->enc)\n"); if (ctx->block_size == 1) { *outl = 0; } @@ -547,6 +560,7 @@ int wolfSSL_EVP_CipherFinal(WOLFSSL_EVP_CIPHER_CTX *ctx, if (ctx->lastUsed == 0 && ctx->bufUsed == 0) { /* return error in cases where the block length is incorrect */ ret = WOLFSSL_FAILURE; + printf("EVP_CipherFinal(ctx->lastUsed == 0 && ctx->bufUsed == 0) fl=%d\n", fl); } } else { From 9880ad6926dcc138fc1e947622388f9395527541 Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Wed, 20 Nov 2019 11:57:06 -0700 Subject: [PATCH 2/4] updates to EVP_CipherUpdate for handling storage of last block --- wolfcrypt/src/evp.c | 89 +++++++++++++++++++++++++++------------------ 1 file changed, 53 insertions(+), 36 deletions(-) diff --git a/wolfcrypt/src/evp.c b/wolfcrypt/src/evp.c index 6f35df7ed..00ef9f57e 100644 --- a/wolfcrypt/src/evp.c +++ b/wolfcrypt/src/evp.c @@ -369,21 +369,24 @@ static int wolfSSL_EVP_CipherUpdate_GCM(WOLFSSL_EVP_CIPHER_CTX *ctx, } #endif +/* returns WOLFSSL_SUCCESS on success and WOLFSSL_FAILURE on failure */ WOLFSSL_API int wolfSSL_EVP_CipherUpdate(WOLFSSL_EVP_CIPHER_CTX *ctx, unsigned char *out, int *outl, const unsigned char *in, int inl) { int blocks; int fill; - int copied; - if ((ctx == NULL) || (inl < 0) || - (outl == NULL)|| (in == NULL)) return BAD_FUNC_ARG; WOLFSSL_ENTER("wolfSSL_EVP_CipherUpdate"); - printf("wolfSSL_EVP_CipherUpdate\n"); + if ((ctx == NULL) || (inl < 0) || (outl == NULL)|| (in == NULL)) { + WOLFSSL_MSG("Bad argument"); + return WOLFSSL_FAILURE; + } *outl = 0; - if (inl == 0) return WOLFSSL_SUCCESS; + if (inl == 0) { + return WOLFSSL_SUCCESS; + } #if !defined(NO_AES) && defined(HAVE_AESGCM) switch (ctx->cipherType) { @@ -398,49 +401,64 @@ WOLFSSL_API int wolfSSL_EVP_CipherUpdate(WOLFSSL_EVP_CIPHER_CTX *ctx, } #endif /* !defined(NO_AES) && defined(HAVE_AESGCM) */ - if (out == NULL) - return BAD_FUNC_ARG; + if (out == NULL) { + return WOLFSSL_FAILURE; + } - if (ctx->bufUsed > 0) { /* concatinate them if there is anything */ - printf("head fillBuff inl=%d, ctx->bufUsed=%d\n", inl, ctx->bufUsed); + + if (ctx->bufUsed > 0) { /* concatenate them if there is anything */ fill = fillBuff(ctx, in, inl); inl -= fill; in += fill; - printf("head fillBuff inl=%d, ctx->bufUsed=%d\n", inl, ctx->bufUsed); } - if ((ctx->enc == 0) && (ctx->lastUsed == 1) && (inl > 0/*ctx->block_size*/)) { - PRINT_BUF(ctx->lastBlock, ctx->block_size); - XMEMCPY(out, ctx->lastBlock, ctx->block_size); - *outl+= ctx->block_size; - out += ctx->block_size; - ctx->lastUsed = 0; - copied = 1; - printf("Copied out last block - 1\n"); - } - if (ctx->bufUsed == ctx->block_size /* && inl/ctx->block_size != 0*/) { - /* the buff is full, flash out */ + + /* check if the buff is full, and if so flash it out */ + if (ctx->bufUsed == ctx->block_size) { + byte* output = out; + + /* During decryption we save the last block to check padding on Final. + * Update the last block stored if one has already been stored */ + if ((ctx->enc == 0)) { + if (ctx->lastUsed == 1) { + XMEMCPY(out, ctx->lastBlock, ctx->block_size); + *outl+= ctx->block_size; + out += ctx->block_size; + } + output = ctx->lastBlock; /* redirect output to last block buffer */ + ctx->lastUsed = 1; + } + PRINT_BUF(ctx->buf, ctx->block_size); - if (evpCipherBlock(ctx, out, ctx->buf, ctx->block_size) == 0) + if (evpCipherBlock(ctx, output, ctx->buf, ctx->block_size) == 0) { return WOLFSSL_FAILURE; + } PRINT_BUF(out, ctx->block_size); - *outl+= ctx->block_size; - out += ctx->block_size; ctx->bufUsed = 0; + + /* if doing encryption update the new output block, decryption will + * always have the last block saved for when Final is called */ + if ((ctx->enc != 0)) { + *outl+= ctx->block_size; + out += ctx->block_size; + } } blocks = inl / ctx->block_size; if (blocks > 0) { - if ((ctx->enc == 0) && (ctx->lastUsed == 1) && (copied == 0)) { + /* During decryption we save the last block to check padding on Final. + * Update the last block stored if one has already been stored */ + if ((ctx->enc == 0) && (ctx->lastUsed == 1)) { PRINT_BUF(ctx->lastBlock, ctx->block_size); XMEMCPY(out, ctx->lastBlock, ctx->block_size); *outl += ctx->block_size; out += ctx->block_size; ctx->lastUsed = 0; - printf("Copied out last block - 2\n"); } + /* process blocks */ - if (evpCipherBlock(ctx, out, in, blocks * ctx->block_size) == 0) + if (evpCipherBlock(ctx, out, in, blocks * ctx->block_size) == 0) { return WOLFSSL_FAILURE; + } PRINT_BUF(in, ctx->block_size*blocks); PRINT_BUF(out,ctx->block_size*blocks); inl -= ctx->block_size * blocks; @@ -449,29 +467,30 @@ WOLFSSL_API int wolfSSL_EVP_CipherUpdate(WOLFSSL_EVP_CIPHER_CTX *ctx, if ((ctx->flags & WOLFSSL_EVP_CIPH_NO_PADDING) || (ctx->block_size == 1)) { ctx->lastUsed = 0; - *outl+= ctx->block_size * blocks; + *outl += ctx->block_size * blocks; } else { - if (inl == 0) { + /* in the case of decryption and padding, store the last block + * here in order to verify the padding when Final is called */ + if (inl == 0) { /* if not 0 then we know leftovers are checked*/ ctx->lastUsed = 1; blocks = blocks - 1; /* save last block to check padding in * EVP_CipherFinal call */ XMEMCPY(ctx->lastBlock, &out[ctx->block_size * blocks], ctx->block_size); - printf("Saved last block\n"); } - *outl+= ctx->block_size * blocks; + *outl += ctx->block_size * blocks; } } else { - *outl+= ctx->block_size * blocks; + *outl += ctx->block_size * blocks; } } + + if (inl > 0) { /* put fraction into buff */ fillBuff(ctx, in, inl); - printf("tailing fillBuff inl=%d\n", inl); /* no increase of outl */ } - printf("Returning outl=%d, ctx->lastUsed= %d, ctx->bufUsed=%d\n", *outl, ctx->lastUsed, ctx->bufUsed); (void)out; /* silence warning in case not read */ return WOLFSSL_SUCCESS; @@ -527,7 +546,6 @@ int wolfSSL_EVP_CipherFinal(WOLFSSL_EVP_CIPHER_CTX *ctx, *outl = 0; } else if (ctx->enc) { - printf("EVP_CipherFinal(ctx->enc)\n"); if (ctx->block_size == 1) { *outl = 0; } @@ -560,7 +578,6 @@ int wolfSSL_EVP_CipherFinal(WOLFSSL_EVP_CIPHER_CTX *ctx, if (ctx->lastUsed == 0 && ctx->bufUsed == 0) { /* return error in cases where the block length is incorrect */ ret = WOLFSSL_FAILURE; - printf("EVP_CipherFinal(ctx->lastUsed == 0 && ctx->bufUsed == 0) fl=%d\n", fl); } } else { From 1eb1755f071f3bad961307927405754046b97a5d Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Wed, 20 Nov 2019 12:29:22 -0700 Subject: [PATCH 3/4] add another evp decrypt test case --- wolfcrypt/test/test.c | 61 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index a750ac373..c00adfe11 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -13522,6 +13522,7 @@ static int openssl_aes_test(void) EVP_CIPHER_CTX de; int outlen ; int total = 0; + int i; EVP_CIPHER_CTX_init(&en); if (EVP_CipherInit(&en, EVP_aes_128_cbc(), @@ -13654,6 +13655,66 @@ static int openssl_aes_test(void) if (XMEMCMP(plain, cbcPlain, 18)) return -7334; + + /* test byte by byte decrypt */ + for (i = 0; i < AES_BLOCK_SIZE * 3; i++) { + plain[i] = i; + } + + total = 0; + EVP_CIPHER_CTX_init(&en); + if (EVP_CipherInit(&en, EVP_aes_128_cbc(), + (unsigned char*)key, (unsigned char*)iv, 1) == 0) + return -7335; + if (EVP_CipherUpdate(&en, (byte*)cipher, &outlen, + (byte*)plain, AES_BLOCK_SIZE * 3) == 0) + return -7336; + if (outlen != AES_BLOCK_SIZE * 3) + return -7337; + total += outlen; + + if (EVP_CipherFinal(&en, (byte*)&cipher[total], &outlen) == 0) + return -7338; + if (outlen != AES_BLOCK_SIZE) + return -7339; + total += outlen; + if (total != sizeof(plain)) + return -7340; + + total = 0; + EVP_CIPHER_CTX_init(&de); + if (EVP_CipherInit(&de, EVP_aes_128_cbc(), + (unsigned char*)key, (unsigned char*)iv, 0) == 0) + return -7341; + + for (i = 0; i < AES_BLOCK_SIZE * 4; i++) { + if (EVP_CipherUpdate(&de, (byte*)plain + total, &outlen, + (byte*)cipher + i, 1) == 0) + return -7342; + + if (outlen > 0) { + int j; + + total += outlen; + for (j = 0; j < total; j++) { + if (plain[j] != j) { + return -7343; + } + } + } + } + + if (EVP_CipherFinal(&de, (byte*)&plain[total], &outlen) == 0) + return -7344; + total += outlen; + if (total != AES_BLOCK_SIZE * 3) { + return -7345; + } + for (i = 0; i < AES_BLOCK_SIZE * 3; i++) { + if (plain[i] != i) { + return -7346; + } + } } /* set buffers to be exact size to catch potential over read/write */ From 6f98d5d3484c6da28592a490e61269b1f9f98f90 Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Wed, 20 Nov 2019 14:49:47 -0700 Subject: [PATCH 4/4] remove extra parentheses that clang complained about --- wolfcrypt/src/evp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wolfcrypt/src/evp.c b/wolfcrypt/src/evp.c index 00ef9f57e..6e0a7b133 100644 --- a/wolfcrypt/src/evp.c +++ b/wolfcrypt/src/evp.c @@ -418,7 +418,7 @@ WOLFSSL_API int wolfSSL_EVP_CipherUpdate(WOLFSSL_EVP_CIPHER_CTX *ctx, /* During decryption we save the last block to check padding on Final. * Update the last block stored if one has already been stored */ - if ((ctx->enc == 0)) { + if (ctx->enc == 0) { if (ctx->lastUsed == 1) { XMEMCPY(out, ctx->lastBlock, ctx->block_size); *outl+= ctx->block_size;