diff mbox series

[v1,4/4] secvar: return error if verify_signature runs out of ESLs

Message ID 20210720160501.9850-4-nick.child@ibm.com
State Accepted
Headers show
Series [v1,1/4] secvar: ensure ESL buf size is at least what ESL header expects | expand

Commit Message

Nick Child July 20, 2021, 4:05 p.m. UTC 
From: Nick Child <nnac123@gmail.com>

commit ID: 56658ad4a0249cdf516e6bc21781cce901965998 from upstream.

Currently, in `verify_signature`, the return code `rc` is initialized
as 0 (our success value). While looping through the ESL's in the given
secvar, the function will break if the remaining data in the secvar is
not enough to contain another ESL. This break from the loop was not
setting a return code, this means that the successful return code
can pass to the end of the function if the first iteration meets
this condition. In other words, if a current secvar has a size that
is less than minimum size for an ESL, than it will approve any update.

In response to this bug, this commit will return an error code if
the described condition is met. Additionally, a test case has been
added to ensure that this unlikely event is handled correctly.

Fixes: 87562bc5c1a6 ("secvar/backend: add edk2 derived key updates processing")
Signed-off-by: Nick Child <nick.child@ibm.com>
Reviewed-by: Nayna Jain <nayna@linux.ibm.com>
Tested-by: Nayna Jain <nayna@linux.ibm.com>
Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
---
 libstb/secvar/backend/edk2-compat-process.c  |  5 +++-
 libstb/secvar/test/secvar-test-edk2-compat.c | 25 ++++++++++++++++++++
 2 files changed, 29 insertions(+), 1 deletion(-)
diff mbox series

Patch

diff --git a/libstb/secvar/backend/edk2-compat-process.c b/libstb/secvar/backend/edk2-compat-process.c
index ecba3c2b..c0006a5e 100644
--- a/libstb/secvar/backend/edk2-compat-process.c
+++ b/libstb/secvar/backend/edk2-compat-process.c
@@ -491,8 +491,11 @@  static int verify_signature(const struct efi_variable_authentication_2 *auth,
 	/* Variable is not empty */
 	while (eslvarsize > 0) {
 		prlog(PR_DEBUG, "esl var size is %d offset is %d\n", eslvarsize, offset);
-		if (eslvarsize < sizeof(EFI_SIGNATURE_LIST))
+		if (eslvarsize < sizeof(EFI_SIGNATURE_LIST)) {
+			rc = OPAL_INTERNAL_ERROR;
+			prlog(PR_ERR, "ESL data is corrupted\n");
 			break;
+		}
 
 		/* Calculate the size of the ESL */
 		eslsize = get_esl_signature_list_size(avar->data + offset,
diff --git a/libstb/secvar/test/secvar-test-edk2-compat.c b/libstb/secvar/test/secvar-test-edk2-compat.c
index 1edce112..100fda7d 100644
--- a/libstb/secvar/test/secvar-test-edk2-compat.c
+++ b/libstb/secvar/test/secvar-test-edk2-compat.c
@@ -89,6 +89,7 @@  int run_test()
 {
 	int rc = -1;
 	struct secvar *tmp;
+	size_t tmp_size;
 	char empty[64] = {0};
 
 	/* The sequence of test cases here is important to ensure that
@@ -213,6 +214,30 @@  int run_test()
 	tmp = find_secvar("db", 3, &variable_bank);
 	ASSERT(NULL != tmp);
 
+	/* Add db, should fail with no KEK and invalid PK size */
+	printf("Add db, corrupt PK");
+	/* Somehow PK gets assigned wrong size */
+	tmp = find_secvar("PK", 3, &variable_bank);
+	ASSERT(NULL != tmp);
+	tmp_size = tmp->data_size;
+	tmp->data_size = sizeof(EFI_SIGNATURE_LIST) - 1;
+	tmp = new_secvar("db", 3, DB_auth, DB_auth_len, 0);
+	ASSERT(0 == edk2_compat_validate(tmp));
+	list_add_tail(&update_bank, &tmp->link);
+	ASSERT(1 == list_length(&update_bank));
+
+	rc = edk2_compat_process(&variable_bank, &update_bank);
+	ASSERT(OPAL_INTERNAL_ERROR == rc);
+	ASSERT(5 == list_length(&variable_bank));
+	ASSERT(0 == list_length(&update_bank));
+	tmp = find_secvar("db", 3, &variable_bank);
+	ASSERT(NULL != tmp);
+	ASSERT(0 == tmp->data_size);
+	/* Restore PK data size */
+	tmp = find_secvar("PK", 3, &variable_bank);
+	ASSERT(NULL != tmp);
+	tmp->data_size = tmp_size;
+
 	/* Add trimmed KEK, .process(), should fail. */
 	printf("Add trimmed KEK\n");
 	tmp = new_secvar("KEK", 4, trimmedKEK_auth, trimmedKEK_auth_len, 0);