From 25af184cd59b1769c0588678362adb5fd41a50ed Mon Sep 17 00:00:00 2001 From: Sebastian Verschoor Date: Fri, 21 Aug 2015 14:40:15 +0200 Subject: [PATCH 1/4] Quickfix for issue #73 The API of the function is changed (for decryption, tag is now an input parameter). With the old API it is impossible to confirm to the NIST specification and a timing sidechannel leak is inevitable. --- src/encauth/ccm/ccm_memory.c | 49 +++++++++++++++++++++++++----------- src/encauth/ccm/ccm_test.c | 16 +++++++----- 2 files changed, 45 insertions(+), 20 deletions(-) diff --git a/src/encauth/ccm/ccm_memory.c b/src/encauth/ccm/ccm_memory.c index 1b4328d..166b13c 100644 --- a/src/encauth/ccm/ccm_memory.c +++ b/src/encauth/ccm/ccm_memory.c @@ -20,7 +20,7 @@ /** CCM encrypt/decrypt and produce an authentication tag - *1 'pt' and 'ct' can both be 'in' or 'out', depending on 'direction' + *1 'pt', 'ct' and 'tag' can both be 'in' or 'out', depending on 'direction' @param cipher The index of the cipher desired @param key The secret key to use @@ -33,8 +33,8 @@ @param pt [*1] The plaintext @param ptlen The length of the plaintext (octets) @param ct [*1] The ciphertext - @param tag [out] The destination tag - @param taglen [in/out] The max size and resulting size of the authentication tag + @param tag [*1] The destination tag + @param taglen The max size and resulting size of the authentication tag @param direction Encrypt or Decrypt direction (0 or 1) @return CRYPT_OK if successful */ @@ -48,7 +48,7 @@ int ccm_memory(int cipher, unsigned char *tag, unsigned long *taglen, int direction) { - unsigned char PAD[16], ctr[16], CTRPAD[16], b; + unsigned char PAD[16], ctr[16], CTRPAD[16], ptTag[16], b; symmetric_key *skey; int err; unsigned long len, L, x, y, z, CTRlen; @@ -203,11 +203,9 @@ int ccm_memory(int cipher, PAD[x++] ^= header[y]; } - /* remainder? */ - if (x != 0) { - if ((err = cipher_descriptor[cipher].ecb_encrypt(PAD, PAD, skey)) != CRYPT_OK) { - goto error; - } + /* remainder */ + if ((err = cipher_descriptor[cipher].ecb_encrypt(PAD, PAD, skey)) != CRYPT_OK) { + goto error; } } @@ -254,7 +252,7 @@ int ccm_memory(int cipher, goto error; } } - } else { + } else { /* direction == CCM_DECRYPT */ for (; y < (ptlen & ~15); y += 16) { /* increment the ctr? */ for (z = 15; z > 15-L; z--) { @@ -328,11 +326,34 @@ int ccm_memory(int cipher, cipher_descriptor[cipher].done(skey); } - /* store the TAG */ - for (x = 0; x < 16 && x < *taglen; x++) { - tag[x] = PAD[x] ^ CTRPAD[x]; + if (direction == CCM_ENCRYPT) { + /* store the TAG */ + for (x = 0; x < 16 && x < *taglen; x++) { + tag[x] = PAD[x] ^ CTRPAD[x]; + } + *taglen = x; + } else { /* direction == CCM_DECRYPT */ + /* decrypt the tag */ + for (x = 0; x < 16 && x < *taglen; x++) { + ptTag[x] = tag[x] ^ CTRPAD[x]; + } + *taglen = x; + + /* check validity of the decrypted tag against the computed PAD (in constant time) */ + /* HACK: the boolean value of XMEM_NEQ becomes either 0 (CRYPT_OK) or 1 (CRYPT_ERR). + * there should be a better way of setting the correct error code in constant + * time. + */ + err = XMEM_NEQ(ptTag, PAD, *taglen); + + /* TODO: pt should not be revealed when the tag is invalid. However, resetting the + * memory should be done in constant time, which is not the case in the + * (commented) code below. + if (err != CRYPT_OK) { + zeromem(pt, ptlen); + } + */ } - *taglen = x; #ifdef LTC_CLEAN_STACK zeromem(skey, sizeof(*skey)); diff --git a/src/encauth/ccm/ccm_test.c b/src/encauth/ccm/ccm_test.c index f8eb3de..d3b20e4 100644 --- a/src/encauth/ccm/ccm_test.c +++ b/src/encauth/ccm/ccm_test.c @@ -195,7 +195,7 @@ int ccm_test(void) tests[x].header, tests[x].headerlen, buf2, tests[x].ptlen, buf, - tag2, &taglen, 1 )) != CRYPT_OK) { + tests[x].tag, &taglen, 1 )) != CRYPT_OK) { return err; } } else { @@ -224,13 +224,17 @@ int ccm_test(void) #endif return CRYPT_FAIL_TESTVECTOR; } - if (XMEMCMP(tag2, tests[x].tag, tests[x].taglen)) { + /* Only check the tag if ccm_memory was not called: ccm_memory already + validates the tag */ + if (y != 0) { + if (XMEMCMP(tag2, tests[x].tag, tests[x].taglen)) { #if defined(LTC_TEST_DBG) - printf("\n%d: x=%lu y=%lu\n", __LINE__, x, y); - print_hex("tag is ", tag, tests[x].taglen); - print_hex("tag should", tests[x].tag, tests[x].taglen); + printf("\n%d: x=%lu y=%lu\n", __LINE__, x, y); + print_hex("tag is ", tag, tests[x].taglen); + print_hex("tag should", tests[x].tag, tests[x].taglen); #endif - return CRYPT_FAIL_TESTVECTOR; + return CRYPT_FAIL_TESTVECTOR; + } } if (y == 0) { cipher_descriptor[idx].done(&skey); From 6c11ca771b2d805989decf35ed210d4cf0555d0c Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Fri, 21 Aug 2015 21:32:42 +0200 Subject: [PATCH 2/4] fix compile error of tests --- src/encauth/ccm/ccm_test.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/encauth/ccm/ccm_test.c b/src/encauth/ccm/ccm_test.c index d3b20e4..55e064e 100644 --- a/src/encauth/ccm/ccm_test.c +++ b/src/encauth/ccm/ccm_test.c @@ -114,7 +114,7 @@ int ccm_test(void) }; unsigned long taglen, x, y; - unsigned char buf[64], buf2[64], tag2[16], tag[16]; + unsigned char buf[64], buf2[64], tag[16], tag2[16], tag3[16]; int err, idx; symmetric_key skey; ccm_state ccm; @@ -188,6 +188,9 @@ int ccm_test(void) } if (y == 0) { + + XMEMCPY(tag3, tests[x].tag, tests[x].taglen); + taglen = tests[x].taglen; if ((err = ccm_memory(idx, tests[x].key, 16, NULL, @@ -195,7 +198,7 @@ int ccm_test(void) tests[x].header, tests[x].headerlen, buf2, tests[x].ptlen, buf, - tests[x].tag, &taglen, 1 )) != CRYPT_OK) { + tag3, &taglen, 1 )) != CRYPT_OK) { return err; } } else { From 09e4b0ec9b46ac4db3fa904658821c54106a1d0e Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Fri, 21 Aug 2015 21:47:43 +0200 Subject: [PATCH 3/4] don't reveal plaintext if authentication failed Create two buffers of the same size as the input data. Copy the input data to the first one and work with that version to hold the decrypted data, zeroize the second one. Copy depending on the verification result, either the zero-buffer or the real plaintext to the output buffer. --- src/encauth/ccm/ccm_memory.c | 36 ++++++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/src/encauth/ccm/ccm_memory.c b/src/encauth/ccm/ccm_memory.c index 166b13c..4b7de92 100644 --- a/src/encauth/ccm/ccm_memory.c +++ b/src/encauth/ccm/ccm_memory.c @@ -48,7 +48,8 @@ int ccm_memory(int cipher, unsigned char *tag, unsigned long *taglen, int direction) { - unsigned char PAD[16], ctr[16], CTRPAD[16], ptTag[16], b; + unsigned char PAD[16], ctr[16], CTRPAD[16], ptTag[16], b, *pt_real; + unsigned char *pt_work[2] = {0}; symmetric_key *skey; int err; unsigned long len, L, x, y, z, CTRlen; @@ -65,6 +66,8 @@ int ccm_memory(int cipher, LTC_ARGCHK(tag != NULL); LTC_ARGCHK(taglen != NULL); + pt_real = pt; + #ifdef LTC_FAST if (16 % sizeof(LTC_FAST_TYPE)) { return CRYPT_INVALID_ARG; @@ -140,6 +143,17 @@ int ccm_memory(int cipher, } else { skey = uskey; } + if (direction != CCM_ENCRYPT) { + pt_work[0] = XMALLOC(ptlen); + pt_work[1] = XCALLOC(1, ptlen); + + if ((pt_work[0] == NULL) || (pt_work[1] == NULL)) { + goto error; + } + + XMEMCPY(pt_work[0], pt, ptlen); + pt = pt_work[0]; + } /* form B_0 == flags | Nonce N | l(m) */ x = 0; @@ -346,13 +360,13 @@ int ccm_memory(int cipher, */ err = XMEM_NEQ(ptTag, PAD, *taglen); - /* TODO: pt should not be revealed when the tag is invalid. However, resetting the - * memory should be done in constant time, which is not the case in the - * (commented) code below. - if (err != CRYPT_OK) { - zeromem(pt, ptlen); - } - */ + /* Here err is 0 or 1, so we just copy either the real plaintext + * or the zeroized buffer. + */ + XMEMCPY(pt_real, pt_work[err], ptlen); +#ifdef LTC_CLEAN_STACK + zeromem(pt_work[0], ptlen); +#endif } #ifdef LTC_CLEAN_STACK @@ -361,6 +375,12 @@ int ccm_memory(int cipher, zeromem(CTRPAD, sizeof(CTRPAD)); #endif error: + if (pt_work[1]) { + XFREE(pt_work[1]); + } + if (pt_work[0]) { + XFREE(pt_work[0]); + } if (skey != uskey) { XFREE(skey); } From 75b114517a3f8db2075a45b0af87d4d74778ad66 Mon Sep 17 00:00:00 2001 From: Sebastian Verschoor Date: Tue, 25 Aug 2015 15:45:10 +0200 Subject: [PATCH 4/4] make sure no cache-based timing attack is possible instead of two different buffers, there is just one buffer. Based upon the verification result, a mask is applied to the buffer before it is written to the output buffer. --- src/encauth/ccm/ccm_memory.c | 55 ++++++++++++++++++----------- src/encauth/ccm/ccm_test.c | 67 ++++++++++++++++++++++++++---------- 2 files changed, 83 insertions(+), 39 deletions(-) diff --git a/src/encauth/ccm/ccm_memory.c b/src/encauth/ccm/ccm_memory.c index 4b7de92..5750f34 100644 --- a/src/encauth/ccm/ccm_memory.c +++ b/src/encauth/ccm/ccm_memory.c @@ -49,10 +49,14 @@ int ccm_memory(int cipher, int direction) { unsigned char PAD[16], ctr[16], CTRPAD[16], ptTag[16], b, *pt_real; - unsigned char *pt_work[2] = {0}; + unsigned char *pt_work = NULL; symmetric_key *skey; int err; unsigned long len, L, x, y, z, CTRlen; +#ifdef LTC_FAST + LTC_FAST_TYPE fastMask = -1; /* initialize fastMask at all zeroes */ +#endif + unsigned char mask = 0xff; /* initialize mask at all zeroes */ if (uskey == NULL) { LTC_ARGCHK(key != NULL); @@ -143,16 +147,14 @@ int ccm_memory(int cipher, } else { skey = uskey; } - if (direction != CCM_ENCRYPT) { - pt_work[0] = XMALLOC(ptlen); - pt_work[1] = XCALLOC(1, ptlen); - - if ((pt_work[0] == NULL) || (pt_work[1] == NULL)) { + + /* initialize buffer for pt */ + if (direction == CCM_DECRYPT) { + pt_work = XMALLOC(ptlen); + if (pt_work == NULL) { goto error; } - - XMEMCPY(pt_work[0], pt, ptlen); - pt = pt_work[0]; + pt = pt_work; } /* form B_0 == flags | Nonce N | l(m) */ @@ -360,26 +362,39 @@ int ccm_memory(int cipher, */ err = XMEM_NEQ(ptTag, PAD, *taglen); - /* Here err is 0 or 1, so we just copy either the real plaintext - * or the zeroized buffer. - */ - XMEMCPY(pt_real, pt_work[err], ptlen); -#ifdef LTC_CLEAN_STACK - zeromem(pt_work[0], ptlen); + /* Zero the plaintext if the tag was invalid (in constant time) */ + if (ptlen > 0) { + y = 0; + mask *= 1 - err; /* mask = ( err ? 0 : 0xff ) */ +#ifdef LTC_FAST + fastMask *= 1 - err; + if (ptlen & ~15) { + for (; y < (ptlen & ~15); y += 16) { + for (z = 0; z < 16; z += sizeof(LTC_FAST_TYPE)) { + *((LTC_FAST_TYPE*)(&pt_real[y+z])) = *((LTC_FAST_TYPE*)(&pt[y+z])) & fastMask; + } + } + } #endif + for (; y < ptlen; y++) { + pt_real[y] = pt[y] & mask; + } + } } #ifdef LTC_CLEAN_STACK + fastMask = 0; + mask = 0; zeromem(skey, sizeof(*skey)); zeromem(PAD, sizeof(PAD)); zeromem(CTRPAD, sizeof(CTRPAD)); + if (pt_work != NULL) { + zeromem(pt_work, ptlen); + } #endif error: - if (pt_work[1]) { - XFREE(pt_work[1]); - } - if (pt_work[0]) { - XFREE(pt_work[0]); + if (pt_work) { + XFREE(pt_work); } if (skey != uskey) { XFREE(skey); diff --git a/src/encauth/ccm/ccm_test.c b/src/encauth/ccm/ccm_test.c index 55e064e..7384151 100644 --- a/src/encauth/ccm/ccm_test.c +++ b/src/encauth/ccm/ccm_test.c @@ -114,10 +114,12 @@ int ccm_test(void) }; unsigned long taglen, x, y; - unsigned char buf[64], buf2[64], tag[16], tag2[16], tag3[16]; + unsigned char buf[64], buf2[64], tag[16], tag2[16], tag3[16], zero[64]; int err, idx; symmetric_key skey; ccm_state ccm; + + zeromem(zero, 64); idx = find_cipher("aes"); if (idx == -1) { @@ -166,8 +168,8 @@ int ccm_test(void) if (XMEMCMP(buf, tests[x].ct, tests[x].ptlen)) { #if defined(LTC_TEST_DBG) printf("\n%d: x=%lu y=%lu\n", __LINE__, x, y); - print_hex("ct is ", tag, taglen); - print_hex("ct should", tests[x].tag, taglen); + print_hex("ct is ", buf, tests[x].ptlen); + print_hex("ct should", tests[x].ct, tests[x].ptlen); #endif return CRYPT_FAIL_TESTVECTOR; } @@ -188,10 +190,9 @@ int ccm_test(void) } if (y == 0) { - - XMEMCPY(tag3, tests[x].tag, tests[x].taglen); - taglen = tests[x].taglen; - if ((err = ccm_memory(idx, + XMEMCPY(tag3, tests[x].tag, tests[x].taglen); + taglen = tests[x].taglen; + if ((err = ccm_memory(idx, tests[x].key, 16, NULL, tests[x].nonce, tests[x].noncelen, @@ -222,28 +223,56 @@ int ccm_test(void) if (XMEMCMP(buf2, tests[x].pt, tests[x].ptlen)) { #if defined(LTC_TEST_DBG) printf("\n%d: x=%lu y=%lu\n", __LINE__, x, y); - print_hex("pt is ", tag, taglen); - print_hex("pt should", tests[x].tag, taglen); + print_hex("pt is ", buf2, tests[x].ptlen); + print_hex("pt should", tests[x].pt, tests[x].ptlen); #endif return CRYPT_FAIL_TESTVECTOR; } - /* Only check the tag if ccm_memory was not called: ccm_memory already - validates the tag */ - if (y != 0) { - if (XMEMCMP(tag2, tests[x].tag, tests[x].taglen)) { -#if defined(LTC_TEST_DBG) - printf("\n%d: x=%lu y=%lu\n", __LINE__, x, y); - print_hex("tag is ", tag, tests[x].taglen); - print_hex("tag should", tests[x].tag, tests[x].taglen); + if (y == 0) { + /* check if decryption with the wrong tag does not reveal the plaintext */ + XMEMCPY(tag3, tests[x].tag, tests[x].taglen); + tag3[0] ^= 0xff; /* set the tag to the wrong value */ + taglen = tests[x].taglen; + if ((err = ccm_memory(idx, + tests[x].key, 16, + NULL, + tests[x].nonce, tests[x].noncelen, + tests[x].header, tests[x].headerlen, + buf2, tests[x].ptlen, + buf, + tag3, &taglen, 1 )) != CRYPT_ERROR) { + return CRYPT_FAIL_TESTVECTOR; + } + if (XMEMCMP(buf2, zero, tests[x].ptlen)) { +#if defined(LTC_CCM_TEST_DBG) + printf("\n%d: x=%lu y=%lu\n", __LINE__, x, y); + print_hex("pt is ", buf2, tests[x].ptlen); + print_hex("pt should", zero, tests[x].ptlen); #endif - return CRYPT_FAIL_TESTVECTOR; - } + return CRYPT_FAIL_TESTVECTOR; + } + } else { + /* FIXME: Only check the tag if ccm_memory was not called: ccm_memory already + validates the tag. ccm_process and ccm_done should somehow do the same, + although with current setup it is impossible to keep the plaintext hidden + if the tag is incorrect. + */ + if (XMEMCMP(tag2, tests[x].tag, tests[x].taglen)) { +#if defined(LTC_TEST_DBG) + printf("\n%d: x=%lu y=%lu\n", __LINE__, x, y); + print_hex("tag is ", tag2, tests[x].taglen); + print_hex("tag should", tests[x].tag, tests[x].taglen); +#endif + return CRYPT_FAIL_TESTVECTOR; + } } + if (y == 0) { cipher_descriptor[idx].done(&skey); } } } + return CRYPT_OK; #endif }