[PATCH 1/2] blkcipher: Copy iv from desc even for 0-len walks

Subsystems: crypto api, the rest

STALE3903d

4 messages, 2 authors, 2015-12-09 · open the first message on its own page

[PATCH 1/2] blkcipher: Copy iv from desc even for 0-len walks

From: "Jason A. Donenfeld" <Jason@zx2c4.com>
Date: 2015-12-06 01:52:04

Some ciphers actually support encrypting zero length plaintexts. For
example, many AEAD modes support this. The resulting ciphertext for
those winds up being only the authentication tag, which is a result of
the key, the iv, the additional data, and the fact that the plaintext
had zero length. The blkcipher constructors won't copy the IV to the
right place, however, when using a zero length input, resulting in
some significant problems when ciphers call their initialization
routines, only to find that the ->iv parameter is uninitialized. One
such example of this would be using chacha20poly1305 with a zero length
input, which then calls chacha20, which calls the key setup routine,
which eventually OOPSes due to the uninitialized ->iv member.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: <redacted>
---
 crypto/ablkcipher.c | 2 +-
 crypto/blkcipher.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/crypto/ablkcipher.c b/crypto/ablkcipher.c
index b4ffc5b..e5b5721 100644
--- a/crypto/ablkcipher.c
+++ b/crypto/ablkcipher.c
@@ -277,12 +277,12 @@ static int ablkcipher_walk_first(struct ablkcipher_request *req,
 	if (WARN_ON_ONCE(in_irq()))
 		return -EDEADLK;
 
+	walk->iv = req->info;
 	walk->nbytes = walk->total;
 	if (unlikely(!walk->total))
 		return 0;
 
 	walk->iv_buffer = NULL;
-	walk->iv = req->info;
 	if (unlikely(((unsigned long)walk->iv & alignmask))) {
 		int err = ablkcipher_copy_iv(walk, tfm, alignmask);
 
diff --git a/crypto/blkcipher.c b/crypto/blkcipher.c
index 11b9814..8cc1622 100644
--- a/crypto/blkcipher.c
+++ b/crypto/blkcipher.c
@@ -326,12 +326,12 @@ static int blkcipher_walk_first(struct blkcipher_desc *desc,
 	if (WARN_ON_ONCE(in_irq()))
 		return -EDEADLK;
 
+	walk->iv = desc->info;
 	walk->nbytes = walk->total;
 	if (unlikely(!walk->total))
 		return 0;
 
 	walk->buffer = NULL;
-	walk->iv = desc->info;
 	if (unlikely(((unsigned long)walk->iv & walk->alignmask))) {
 		int err = blkcipher_copy_iv(walk);
 		if (err)
-- 
2.6.3

[PATCH 2/2] chacha20poly1305: Skip encryption/decryption for 0-len

From: "Jason A. Donenfeld" <Jason@zx2c4.com>
Date: 2015-12-06 01:52:08

If the length of the plaintext is zero, there's no need to waste cycles
on encryption and decryption. Using the chacha20poly1305 construction
for zero-length plaintexts is a common way of using a shared encryption
key for AAD authentication.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: <redacted>
---
 crypto/chacha20poly1305.c | 8 ++++++++
 1 file changed, 8 insertions(+)
diff --git a/crypto/chacha20poly1305.c b/crypto/chacha20poly1305.c
index 99c3cce..7b6b935 100644
--- a/crypto/chacha20poly1305.c
+++ b/crypto/chacha20poly1305.c
@@ -130,6 +130,9 @@ static int chacha_decrypt(struct aead_request *req)
 	struct scatterlist *src, *dst;
 	int err;
 
+	if (rctx->cryptlen == 0)
+		goto skip;
+
 	chacha_iv(creq->iv, req, 1);
 
 	sg_init_table(rctx->src, 2);
@@ -150,6 +153,7 @@ static int chacha_decrypt(struct aead_request *req)
 	if (err)
 		return err;
 
+skip:
 	return poly_verify_tag(req);
 }
 
@@ -415,6 +419,9 @@ static int chacha_encrypt(struct aead_request *req)
 	struct scatterlist *src, *dst;
 	int err;
 
+	if (req->cryptlen == 0)
+		goto skip;
+
 	chacha_iv(creq->iv, req, 1);
 
 	sg_init_table(rctx->src, 2);
@@ -435,6 +442,7 @@ static int chacha_encrypt(struct aead_request *req)
 	if (err)
 		return err;
 
+skip:
 	return poly_genkey(req);
 }
 
-- 
2.6.3

Re: [PATCH 1/2] blkcipher: Copy iv from desc even for 0-len walks

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: 2015-12-09 12:19:35

On Sun, Dec 06, 2015 at 02:51:37AM +0100, Jason A. Donenfeld wrote:
Some ciphers actually support encrypting zero length plaintexts. For
example, many AEAD modes support this. The resulting ciphertext for
those winds up being only the authentication tag, which is a result of
the key, the iv, the additional data, and the fact that the plaintext
had zero length. The blkcipher constructors won't copy the IV to the
right place, however, when using a zero length input, resulting in
some significant problems when ciphers call their initialization
routines, only to find that the ->iv parameter is uninitialized. One
such example of this would be using chacha20poly1305 with a zero length
input, which then calls chacha20, which calls the key setup routine,
which eventually OOPSes due to the uninitialized ->iv member.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: <redacted>
Applied to crypto.
-- 
Email: Herbert Xu [off-list ref]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 2/2] chacha20poly1305: Skip encryption/decryption for 0-len

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: 2015-12-09 12:20:28

On Sun, Dec 06, 2015 at 02:51:38AM +0100, Jason A. Donenfeld wrote:
If the length of the plaintext is zero, there's no need to waste cycles
on encryption and decryption. Using the chacha20poly1305 construction
for zero-length plaintexts is a common way of using a shared encryption
key for AAD authentication.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Patch applied.  Thanks.
-- 
Email: Herbert Xu [off-list ref]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help