From 07277e2ab4b54e5844c28f0cb33e64a91aa5f492 Mon Sep 17 00:00:00 2001
From: Julian Hall <julian.hall@arm.com>
Date: Wed, 16 Feb 2022 10:37:04 +0000
Subject: [PATCH] Fix multi-part termination on error

For multi-part operations, the PSA Crypto API specifies that if
the final operation does not return PSA_SUCCESS, the abort
operaion must be called by a client to clean-up the operation.
This change modifies behaviour in-line with the API definition.

Signed-off-by: Julian Hall <julian.hall@arm.com>
Change-Id: Ia3d3ec004164647a7ab5988cac45c39c22e76e9a

Upstream-Status: Pending [Not submitted to upstream yet]
Signed-off-by: Emekcan Aras <Emekcan.Aras@arm.com>


---
 components/service/crypto/client/psa/psa_aead.c       |  8 ++++++++
 components/service/crypto/client/psa/psa_cipher.c     |  4 ++++
 components/service/crypto/client/psa/psa_hash.c       | 10 ++++++++++
 components/service/crypto/client/psa/psa_mac.c        | 10 ++++++++++
 .../crypto/provider/extension/aead/aead_provider.c    | 10 +++++-----
 .../provider/extension/cipher/cipher_provider.c       |  6 +++---
 .../crypto/provider/extension/hash/hash_provider.c    |  6 +++---
 .../crypto/provider/extension/mac/mac_provider.c      | 11 +++++++----
 8 files changed, 50 insertions(+), 15 deletions(-)

diff --git a/components/service/crypto/client/psa/psa_aead.c b/components/service/crypto/client/psa/psa_aead.c
index e4579e63..559eb6a3 100644
--- a/components/service/crypto/client/psa/psa_aead.c
+++ b/components/service/crypto/client/psa/psa_aead.c
@@ -241,6 +241,10 @@ psa_status_t psa_aead_encrypt(psa_key_id_t key,
 
 			*aeadtext_length = bytes_output + remaining_aead_len + tag_len;
 		}
+		else {
+
+			psa_aead_abort(&operation);
+		}
 	}
 	else {
 
@@ -292,6 +296,10 @@ psa_status_t psa_aead_decrypt(psa_key_id_t key,
 
 			*plaintext_length = bytes_output + remaining_plaintext_len;
 		}
+		else {
+
+			psa_aead_abort(&operation);
+		}
 	}
 	else {
 
diff --git a/components/service/crypto/client/psa/psa_cipher.c b/components/service/crypto/client/psa/psa_cipher.c
index 111af829..4e4264b6 100644
--- a/components/service/crypto/client/psa/psa_cipher.c
+++ b/components/service/crypto/client/psa/psa_cipher.c
@@ -146,6 +146,10 @@ static psa_status_t multi_cipher_update(psa_cipher_operation_t *operation,
 
 				*output_length = bytes_output + finish_output_len;
 			}
+			else {
+
+				psa_cipher_abort(operation);
+			}
 		}
 		else {
 
diff --git a/components/service/crypto/client/psa/psa_hash.c b/components/service/crypto/client/psa/psa_hash.c
index 83278de6..e5dd0030 100644
--- a/components/service/crypto/client/psa/psa_hash.c
+++ b/components/service/crypto/client/psa/psa_hash.c
@@ -137,6 +137,11 @@ psa_status_t psa_hash_compare(psa_algorithm_t alg,
 	if (psa_status == PSA_SUCCESS) {
 
 		psa_status = psa_hash_verify(&operation, hash, hash_length);
+
+		if (psa_status != PSA_SUCCESS) {
+
+			psa_hash_abort(&operation);
+		}
 	}
 
 	return psa_status;
@@ -155,6 +160,11 @@ psa_status_t psa_hash_compute(psa_algorithm_t alg,
 	if (psa_status == PSA_SUCCESS) {
 
 		psa_status = psa_hash_finish(&operation, hash, hash_size, hash_length);
+
+		if (psa_status != PSA_SUCCESS) {
+
+			psa_hash_abort(&operation);
+		}
 	}
 
 	return psa_status;
diff --git a/components/service/crypto/client/psa/psa_mac.c b/components/service/crypto/client/psa/psa_mac.c
index 5c5eb32a..a3db8644 100644
--- a/components/service/crypto/client/psa/psa_mac.c
+++ b/components/service/crypto/client/psa/psa_mac.c
@@ -129,6 +129,11 @@ psa_status_t psa_mac_verify(psa_key_id_t key,
 	if (psa_status == PSA_SUCCESS) {
 
 		psa_status = psa_mac_verify_finish(&operation, mac, mac_length);
+
+		if (psa_status != PSA_SUCCESS) {
+
+			psa_mac_abort(&operation);
+		}
 	}
 
 	return psa_status;
@@ -153,6 +158,11 @@ psa_status_t psa_mac_compute(psa_key_id_t key,
 	if (psa_status == PSA_SUCCESS) {
 
 		psa_status = psa_mac_sign_finish(&operation, mac, mac_size, mac_length);
+
+		if (psa_status != PSA_SUCCESS) {
+
+			psa_mac_abort(&operation);
+		}
 	}
 
 	return psa_status;
diff --git a/components/service/crypto/provider/extension/aead/aead_provider.c b/components/service/crypto/provider/extension/aead/aead_provider.c
index f4e81a03..14a25436 100644
--- a/components/service/crypto/provider/extension/aead/aead_provider.c
+++ b/components/service/crypto/provider/extension/aead/aead_provider.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, Arm Limited and Contributors. All rights reserved.
+ * Copyright (c) 2021-2022, Arm Limited and Contributors. All rights reserved.
  *
  * SPDX-License-Identifier: BSD-3-Clause
  */
@@ -369,9 +369,9 @@ static rpc_status_t aead_finish_handler(void *context, struct call_req *req)
 				rpc_status = serializer->serialize_aead_finish_resp(resp_buf,
 					ciphertext, ciphertext_len,
 					tag, tag_len);
-			}
 
-			crypto_context_pool_free(&this_instance->context_pool, crypto_context);
+				crypto_context_pool_free(&this_instance->context_pool, crypto_context);
+			}
 		}
 
 		call_req_set_opstatus(req, psa_status);
@@ -418,9 +418,9 @@ static rpc_status_t aead_verify_handler(void *context, struct call_req *req)
 				struct call_param_buf *resp_buf = call_req_get_resp_buf(req);
 				rpc_status = serializer->serialize_aead_verify_resp(resp_buf,
 					plaintext, plaintext_len);
-			}
 
-			crypto_context_pool_free(&this_instance->context_pool, crypto_context);
+				crypto_context_pool_free(&this_instance->context_pool, crypto_context);
+			}
 		}
 
 		call_req_set_opstatus(req, psa_status);
diff --git a/components/service/crypto/provider/extension/cipher/cipher_provider.c b/components/service/crypto/provider/extension/cipher/cipher_provider.c
index 8e7a86de..a5dd0371 100644
--- a/components/service/crypto/provider/extension/cipher/cipher_provider.c
+++ b/components/service/crypto/provider/extension/cipher/cipher_provider.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, Arm Limited and Contributors. All rights reserved.
+ * Copyright (c) 2021-2022, Arm Limited and Contributors. All rights reserved.
  *
  * SPDX-License-Identifier: BSD-3-Clause
  */
@@ -283,9 +283,9 @@ static rpc_status_t cipher_finish_handler(void *context, struct call_req* req)
 
 				struct call_param_buf *resp_buf = call_req_get_resp_buf(req);
 				rpc_status = serializer->serialize_cipher_finish_resp(resp_buf, output, output_len);
-			}
 
-			crypto_context_pool_free(&this_instance->context_pool, crypto_context);
+				crypto_context_pool_free(&this_instance->context_pool, crypto_context);
+			}
 		}
 
 		call_req_set_opstatus(req, psa_status);
diff --git a/components/service/crypto/provider/extension/hash/hash_provider.c b/components/service/crypto/provider/extension/hash/hash_provider.c
index 2c560513..fd39d440 100644
--- a/components/service/crypto/provider/extension/hash/hash_provider.c
+++ b/components/service/crypto/provider/extension/hash/hash_provider.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, Arm Limited and Contributors. All rights reserved.
+ * Copyright (c) 2021-2022, Arm Limited and Contributors. All rights reserved.
  *
  * SPDX-License-Identifier: BSD-3-Clause
  */
@@ -179,9 +179,9 @@ static rpc_status_t hash_finish_handler(void *context, struct call_req* req)
 
 				struct call_param_buf *resp_buf = call_req_get_resp_buf(req);
 				rpc_status = serializer->serialize_hash_finish_resp(resp_buf, hash, hash_len);
-			}
 
-			crypto_context_pool_free(&this_instance->context_pool, crypto_context);
+				crypto_context_pool_free(&this_instance->context_pool, crypto_context);
+			}
 		}
 
 		call_req_set_opstatus(req, psa_status);
diff --git a/components/service/crypto/provider/extension/mac/mac_provider.c b/components/service/crypto/provider/extension/mac/mac_provider.c
index 96fe4cf3..eef55586 100644
--- a/components/service/crypto/provider/extension/mac/mac_provider.c
+++ b/components/service/crypto/provider/extension/mac/mac_provider.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, Arm Limited and Contributors. All rights reserved.
+ * Copyright (c) 2021-2022, Arm Limited and Contributors. All rights reserved.
  *
  * SPDX-License-Identifier: BSD-3-Clause
  */
@@ -181,9 +181,9 @@ static rpc_status_t mac_sign_finish_handler(void *context, struct call_req* req)
 
 				struct call_param_buf *resp_buf = call_req_get_resp_buf(req);
 				rpc_status = serializer->serialize_mac_sign_finish_resp(resp_buf, mac, mac_len);
-			}
 
-			crypto_context_pool_free(&this_instance->context_pool, crypto_context);
+				crypto_context_pool_free(&this_instance->context_pool, crypto_context);
+			}
 		}
 
 		call_req_set_opstatus(req, psa_status);
@@ -220,7 +220,10 @@ static rpc_status_t mac_verify_finish_handler(void *context, struct call_req* re
 
 			psa_status = psa_mac_verify_finish(&crypto_context->op.mac, mac, mac_len);
 
-			crypto_context_pool_free(&this_instance->context_pool, crypto_context);
+			if (psa_status == PSA_SUCCESS) {
+
+				crypto_context_pool_free(&this_instance->context_pool, crypto_context);
+			}
 		}
 
 		call_req_set_opstatus(req, psa_status);