6:51 p.m.
Some pre-4.9 kernels have an AEAD bug that makes recvmsg() return an
incorrect value that does not include the AAD length. This could cause
an erroneous successful return from l_aead_cipher_{en,de}crypt() if
ad_len was longer than the expected output. operate_cipher() now returns
the number of bytes copied to the 'out' buffer, and the AEAD functions
only succeed if the expected number of bytes were copied.
---
ell/cipher.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/ell/cipher.c b/ell/cipher.c
index bbf0a48..bfbe5d4 100644
--- a/ell/cipher.c
+++ b/ell/cipher.c
@@ -378,8 +378,10 @@ static ssize_t operate_cipher(int sk, __u32 operation,
result = recvmsg(sk, &msg, 0);
- if (result > (ssize_t) ad_len)
+ if (result >= (ssize_t) ad_len)
result -= ad_len;
+ else if (result > 0)
+ result = 0;
l_free(iov[0].iov_base);
} else {
@@ -546,7 +548,8 @@ LIB_EXPORT bool l_aead_cipher_encrypt(struct l_aead_cipher *cipher,
return operate_cipher(cipher->encrypt_sk, ALG_OP_ENCRYPT, in, in_len,
ad, ad_len, nonce, nonce_len, out, out_len,
- l_aead_cipher_get_ivlen(cipher)) >= 0;
+ l_aead_cipher_get_ivlen(cipher)) ==
+ (ssize_t)out_len;
}
LIB_EXPORT bool l_aead_cipher_decrypt(struct l_aead_cipher *cipher,
@@ -563,7 +566,8 @@ LIB_EXPORT bool l_aead_cipher_decrypt(struct l_aead_cipher *cipher,
return operate_cipher(cipher->decrypt_sk, ALG_OP_DECRYPT, in, in_len,
ad, ad_len, nonce, nonce_len, out, out_len,
- l_aead_cipher_get_ivlen(cipher)) >= 0;
+ l_aead_cipher_get_ivlen(cipher)) ==
+ (ssize_t)out_len;
}
static void init_supported()
--
2.16.1
6:51 p.m.
New subject: [PATCH 2/4] cipher: Check AAD length when setting up AEAD result reads
Use read() instead of recvmsg() when AAD length is zero, instead of
checking the AAD data pointer to make the decision. The length is used
in the affected block of code and the pointer is not.
---
ell/cipher.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ell/cipher.c b/ell/cipher.c
index bfbe5d4..8fe3610 100644
--- a/ell/cipher.c
+++ b/ell/cipher.c
@@ -357,7 +357,7 @@ static ssize_t operate_cipher(int sk, __u32 operation,
if (result < 0)
return -errno;
- if (ad) {
+ if (ad_len) {
/*
* When AEAD additional data is passed to sendmsg() for
* use in computing the tag, those bytes also appear at
--
2.16.1
6:51 p.m.
New subject: [PATCH 3/4] unit: Check l_aead_cipher_* return values
---
unit/test-cipher.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/unit/test-cipher.c b/unit/test-cipher.c
index 6a365df..c99650c 100644
--- a/unit/test-cipher.c
+++ b/unit/test-cipher.c
@@ -175,6 +175,7 @@ static void test_aes_ccm(const void *data)
char *decbuf;
size_t decbuflen;
int r;
+ bool success;
const struct ccm_test_vector *tv = data;
size_t ptlen;
@@ -200,16 +201,18 @@ static void test_aes_ccm(const void *data)
cipher = l_aead_cipher_new(L_AEAD_CIPHER_AES_CCM, key, keylen, taglen);
assert(cipher);
- l_aead_cipher_encrypt(cipher, pt, ptlen, aad, aadlen,
- nonce, noncelen, encbuf, encbuflen);
+ success = l_aead_cipher_encrypt(cipher, pt, ptlen, aad, aadlen,
+ nonce, noncelen, encbuf, encbuflen);
+ assert(success);
r = memcmp(encbuf, ct, ctlen);
assert(!r);
r = memcmp(encbuf + ctlen, tag, taglen);
assert(!r);
- l_aead_cipher_decrypt(cipher, encbuf, encbuflen, aad, aadlen, nonce,
- noncelen, decbuf, decbuflen);
+ success = l_aead_cipher_decrypt(cipher, encbuf, encbuflen, aad, aadlen,
+ nonce, noncelen, decbuf, decbuflen);
+ assert (success);
r = memcmp(decbuf, pt, ptlen);
assert(!r);
--
2.16.1
6:51 p.m.
New subject: [PATCH 4/4] unit: Add kernel bug information to AEAD encryption failure output
---
unit/test-cipher.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/unit/test-cipher.c b/unit/test-cipher.c
index c99650c..4765c3e 100644
--- a/unit/test-cipher.c
+++ b/unit/test-cipher.c
@@ -26,6 +26,7 @@
#include <assert.h>
#include <alloca.h>
+#include <stdio.h>
#include <ell/ell.h>
@@ -203,6 +204,12 @@ static void test_aes_ccm(const void *data)
success = l_aead_cipher_encrypt(cipher, pt, ptlen, aad, aadlen,
nonce, noncelen, encbuf, encbuflen);
+ if (!success) {
+ printf("* Some kernel versions before v4.9 have a known AEAD\n"
+ "* bug. If the system running this test is using a\n"
+ "* v4.8 or earlier kernel, a failure here is likely\n"
+ "* due to that kernel bug.\n");
+ }
assert(success);
r = memcmp(encbuf, ct, ctlen);
--
2.16.1
8:10 p.m.
Hi Mat,
On 01/24/2018 03:51 PM, Mat Martineau wrote:
Some pre-4.9 kernels have an AEAD bug that makes recvmsg() return an
incorrect value that does not include the AAD length. This could cause
an erroneous successful return from l_aead_cipher_{en,de}crypt() if
ad_len was longer than the expected output. operate_cipher() now returns
the number of bytes copied to the 'out' buffer, and the AEAD functions
only succeed if the expected number of bytes were copied.
---
ell/cipher.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
All 4 applied, thanks.
Regards,
-Denis
861
days inactive
861
days old
4 comments
2 participants
participants (2)
-
Denis Kenzior -
Mat Martineau