[Skiboot] [PATCH 3/4] secvar: return error if validate_esl has extra data

Nick Child nnac123 at gmail.com
Fri Jun 25 06:51:07 AEST 2021


Currently, in `validate_esl_list`, the return code is initialized to zero
(our success value). While looping though the ESL's in the submitted ESL
chain, the loop will break if there is not enough data to meet minimum ESL
requirements. This condition was not setting a return code, meaning that the
successful return code can pass to the end of the function if there is extra
data at the end of the ESL. As a consequence, any properly signed update can
successfully commit any data (as long as it is less than the min size of an
ESL) to the secvars.

This commit will return an error if the described condition is met. This
means all data in the appended ESL of an auth file must be accounted for. No
extra bytes can be added to the end since, on success, this data will become
the updated secvar.

Additionally, a test case has been added to ensure that this commit
addresses the issue correctly.

Signed-off-by: Nick Child <nick.child at ibm.com>
---
 libstb/secvar/backend/edk2-compat-process.c  |  5 ++++-
 libstb/secvar/test/secvar-test-edk2-compat.c | 15 +++++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/libstb/secvar/backend/edk2-compat-process.c b/libstb/secvar/backend/edk2-compat-process.c
index d12d1134..5cb54c95 100644
--- a/libstb/secvar/backend/edk2-compat-process.c
+++ b/libstb/secvar/backend/edk2-compat-process.c
@@ -265,8 +265,11 @@ int validate_esl_list(const char *key, const char *esl, const size_t size)
 
 	while (eslvarsize > 0) {
 		prlog(PR_DEBUG, "esl var size size is %d offset is %d\n", eslvarsize, offset);
-		if (eslvarsize < sizeof(EFI_SIGNATURE_LIST))
+		if (eslvarsize < sizeof(EFI_SIGNATURE_LIST)) {
+			prlog(PR_ERR, "ESL has %d unknown extra bytes\n", eslvarsize);
+			rc = OPAL_PARAMETER;
 			break;
+		}
 
 		/* Check Supported ESL Type */
 		list = get_esl_signature_list(esl + offset, eslvarsize);
diff --git a/libstb/secvar/test/secvar-test-edk2-compat.c b/libstb/secvar/test/secvar-test-edk2-compat.c
index 93ade920..0e1a8211 100644
--- a/libstb/secvar/test/secvar-test-edk2-compat.c
+++ b/libstb/secvar/test/secvar-test-edk2-compat.c
@@ -165,6 +165,21 @@ int run_test()
 	ASSERT(5 == list_length(&variable_bank));
 	ASSERT(setup_mode);
 
+	/* Add PK with bad ESL. should fail since data is not big enough to be ESL*/
+	printf("Add PK with invalid appended ESL");
+	/* 1014 is length of appended ESL */
+	tmp = new_secvar("PK", 3, PK_auth, PK_auth_len - 1014 + sizeof(EFI_SIGNATURE_LIST) - 1, 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(5 == list_length(&variable_bank));
+	ASSERT(0 == list_length(&update_bank));
+	rc = edk2_compat_post_process(&variable_bank, &update_bank);
+	ASSERT(5 == list_length(&variable_bank));
+	ASSERT(setup_mode);
+
+
 	/* Add PK to update and .process(). */
 	printf("Add PK");
 	tmp = new_secvar("PK", 3, PK_auth, PK_auth_len, 0);
-- 
2.17.1



More information about the Skiboot mailing list