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

[Nick Child]

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 
or. 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>
---

The first two patches in this set were taken from patch 3 and 4 from
https://lists.ozlabs.org/pipermail/skiboot/2021-June/017619.html

This specific patch was broken into two patches: this one and the third 
one in this patchset. The reason for spliting this patch into two was 
because the changes to `get_esl_signature_list` were independent of the 
changes in this patch and will likely need more reviews.

---
 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 5204fdd5..03547875 100644
--- a/libstb/secvar/backend/edk2-compat-process.c
+++ b/libstb/secvar/backend/edk2-compat-process.c
@@ -267,8 +267,11 @@ int validate_esl_list(const char *key, const char *esl, const size_t size)
 
 	while (eslvarsize > 0) {
 		prlog(PR_DEBUG, "esl var size is %d offset is %lu\n", eslvarsize, size - eslvarsize);
-		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, eslvarsize);
diff --git a/libstb/secvar/test/secvar-test-edk2-compat.c b/libstb/secvar/test/secvar-test-edk2-compat.c
index 3cdc6ef2..1edce112 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 Header and its data */
+	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.25.1