On 25.03.2013 18:12, Chaoxing Lin wrote: > 2nd ping.... > > Nobody is maintaining crypto/gcm.c? > > > > -----Original Message----- > From: Chaoxing Lin > Sent: Friday, March 08, 2013 11:38 AM > To: 'linux-crypto@xxxxxxxxxxxxxxx' > Subject: potential bug in GMAC implementation. not work in ESN mode > > I was testing ipsec with GMAC and found that the rfc4543 GMAC implementation in kernel software crypto work in "esp=aes256gmac-noesn!" mode. > It does not work in in "esp=aes256gmac-esn!" mode. The tunnel was established but no data traffic is possible. > > Looking at source code, I found this piece of code is suspicious. > Line 1146~1147 tries to put req->assoc to assoc[1]. But I think this way only works when req->assoc has only one segment. In ESN mode, req->assoc contains 3 segments (SPI, SN-hi, SN-low). Line 1146~1147 will only attach SPI segment(with total length) in assoc. > > Please let me know whether I understand it right. Your analysis seems correct. Does attached the patch fix the problem? (I've only compile tested it.) -Jussi > Thanks, > > Chaoxing > > > Source from kernel 3.8.2 > path: root/crypto/gcm.c > > 1136: /* construct the aad */ > 1137: dstp = sg_page(dst); > vdst = PageHighMem(dstp) ? NULL : page_address(dstp) + dst->offset; > > sg_init_table(payload, 2); > sg_set_buf(payload, req->iv, 8); > scatterwalk_crypto_chain(payload, dst, vdst == req->iv + 8, 2); > assoclen += 8 + req->cryptlen - (enc ? 0 : authsize); > > sg_init_table(assoc, 2); > 1146: sg_set_page(assoc, sg_page(req->assoc), req->assoc->length, > 1147: req->assoc->offset); > scatterwalk_crypto_chain(assoc, payload, 0, 2); > > aead_request_set_tfm(subreq, ctx->child); > aead_request_set_callback(subreq, req->base.flags, req->base.complete, > req->base.data); > aead_request_set_crypt(subreq, cipher, cipher, enc ? 0 : authsize, iv); > 1154: aead_request_set_assoc(subreq, assoc, assoclen); > -- > To unsubscribe from this list: send the line "unsubscribe linux-crypto" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html >
crypto: gcm - fix assumption that assoc has one segment
From: Jussi Kivilinna <jussi.kivilinna@xxxxxx>
Signed-off-by: Jussi Kivilinna <jussi.kivilinna@xxxxxx>
---
crypto/gcm.c | 17 ++++++++++++++---
crypto/tcrypt.c | 4 ++++
2 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/crypto/gcm.c b/crypto/gcm.c
index 137ad1e..13ccbda 100644
--- a/crypto/gcm.c
+++ b/crypto/gcm.c
@@ -44,6 +44,7 @@ struct crypto_rfc4543_ctx {
struct crypto_rfc4543_req_ctx {
u8 auth_tag[16];
+ u8 assocbuf[32];
struct scatterlist cipher[1];
struct scatterlist payload[2];
struct scatterlist assoc[2];
@@ -1133,9 +1134,19 @@ static struct aead_request *crypto_rfc4543_crypt(struct aead_request *req,
scatterwalk_crypto_chain(payload, dst, vdst == req->iv + 8, 2);
assoclen += 8 + req->cryptlen - (enc ? 0 : authsize);
- sg_init_table(assoc, 2);
- sg_set_page(assoc, sg_page(req->assoc), req->assoc->length,
- req->assoc->offset);
+ if (req->assoc->length == req->assoclen) {
+ sg_init_table(assoc, 2);
+ sg_set_page(assoc, sg_page(req->assoc), req->assoc->length,
+ req->assoc->offset);
+ } else {
+ BUG_ON(req->assoclen > sizeof(rctx->assocbuf));
+
+ scatterwalk_map_and_copy(rctx->assocbuf, req->assoc, 0,
+ req->assoclen, 0);
+
+ sg_init_table(assoc, 2);
+ sg_set_buf(assoc, rctx->assocbuf, req->assoclen);
+ }
scatterwalk_crypto_chain(assoc, payload, 0, 2);
aead_request_set_tfm(subreq, ctx->child);
diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index 87ef7d6..6b911ef 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -1225,6 +1225,10 @@ static int do_test(int m)
ret += tcrypt_test("rfc4106(gcm(aes))");
break;
+ case 152:
+ ret += tcrypt_test("rfc4543(gcm(aes))");
+ break;
+
case 200:
test_cipher_speed("ecb(aes)", ENCRYPT, sec, NULL, 0,
speed_template_16_24_32);
Attachment:
signature.asc
Description: OpenPGP digital signature
