Skip to contentSkip to conversation

crypto: krb5enc - fix async decrypt skipping hash verification

PATCH In review Dudu Lu 2 participants 2 messages Apr 16 – Apr 17 Crypto API

lore
Share & tools
lore

Dudu Lu

author
#0 lore raw

PATCH 1/1 crypto: krb5enc - fix async decrypt skipping hash verification

krb5enc_dispatch_decrypt() sets req->base.complete as the skcipher callback, which is the caller's own completion handler. When the skcipher completes asynchronously, this signals "done" to the caller without executing krb5enc_dispatch_decrypt_hash(), completely bypassing the integrity verification (hash check).

Compare with the encrypt path which correctly uses krb5enc_encrypt_done as an intermediate callback to chain into the hash computation on async completion.

Fix by adding krb5enc_decrypt_done as an intermediate callback that chains into krb5enc_dispatch_decrypt_hash() upon async skcipher completion, matching the encrypt path's callback pattern. Handle both -EINPROGRESS and -EBUSY notifications from backlogged requests, consistent with authenc's authenc_request_complete(). Also fix krb5enc_request_complete() to filter -EBUSY in addition to -EINPROGRESS, matching the authenc reference implementation.

Fixes d1775a177f7f ("crypto: Add 'krb5enc' hash and cipher AEAD algorithm")
Signed-off-by Dudu Lu <[email protected]>
---
 crypto/krb5enc.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)
crypto/krb5enc.c +20 -2
index a1de55994d92..2490343873a9 100644
@@ -41,7 +41,7 @@ struct krb5enc_request_ctx {
 
 static void krb5enc_request_complete(struct aead_request *req, int err)
 {
-	if (err != -EINPROGRESS)
+	if (err != -EINPROGRESS && err != -EBUSY)
 		aead_request_complete(req, err);
 }
 

@@ -300,6 +300,24 @@ static int krb5enc_dispatch_decrypt_hash(struct aead_request *req)
 	return krb5enc_verify_hash(req);
 }
 
+static void krb5enc_decrypt_done(void *data, int err)
+{
+	struct aead_request *req = data;
+
+	if (err == -EINPROGRESS || err == -EBUSY)
+		return krb5enc_request_complete(req, err);
+
+	if (err)
+		goto out;
+
+	err = krb5enc_dispatch_decrypt_hash(req);
+	if (err == -EINPROGRESS || err == -EBUSY)
+		return;
+
+out:
+	aead_request_complete(req, err);
+}
+
 /*
  * Dispatch the decryption of the ciphertext.
  */

@@ -323,7 +341,7 @@ static int krb5enc_dispatch_decrypt(struct aead_request *req)
 
 	skcipher_request_set_tfm(skreq, ctx->enc);
 	skcipher_request_set_callback(skreq, aead_request_flags(req),
-				      req->base.complete, req->base.data);
+				      krb5enc_decrypt_done, req);
 	skcipher_request_set_crypt(skreq, src, dst,
 				   req->cryptlen - authsize, req->iv);
Signature · 1 line
2.39.3 (Apple Git-145)
maintainer
#1 lore raw

crypto/krb5enc.c struct krb5enc_request_ctx { · line 44

Quoted 11 lines · Dudu Lu
crypto/krb5enc.c +1 -1
index a1de55994d92..2490343873a9 100644
@@ -41,7 +41,7 @@ struct krb5enc_request_ctx {
 
 static void krb5enc_request_complete(struct aead_request *req, int err)
 {
-	if (err != -EINPROGRESS)
+	if (err != -EINPROGRESS && err != -EBUSY)

This shouldn't filter anything out. The filtering needs to occur further up the call stack. In fact just get rid of it and use aead_request_complete directly.

The encrypt path is just as broken as the decrypt path and needs to be fixed accordingly to filter out EBUSY/EINPROGRESS. In particular, this should be done in krb5enc_encrypt_ahash_done. Currently it's only filtering out EINPROGRESS.

crypto/krb5enc.c static int krb5enc_dispatch_decrypt_hash(struct aead_request *req) · line 308

Quoted 6 lines
crypto/krb5enc.c +6 -0
+static void krb5enc_decrypt_done(void *data, int err)
+{
+	struct aead_request *req = data;
+
+	if (err == -EINPROGRESS || err == -EBUSY)
+		return krb5enc_request_complete(req, err);

EINPROGRESS should always get passed up here because it means that we originally returned an EBUSY and the caller is potentially blocking on this notification.

EBUSY cannot occur in the context of the callback so there is no need to check for it.

So this should simply become

	if (err)
		goto out;

crypto/krb5enc.c static int krb5enc_dispatch_decrypt_hash(struct aead_request *req) · line 315

Quoted 3 lines
crypto/krb5enc.c +3 -0
+	err = krb5enc_dispatch_decrypt_hash(req);
+	if (err == -EINPROGRESS || err == -EBUSY)
+		return;

This is the only place where EBUSY needs to be checked.

Thanks,

Signature · 3 lines
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt