[Skiboot] [PATCH v2 3/4] secvar: return error if validate_esl has extra data
Nick Child
nnac123 at gmail.com
Tue Jun 29 05:37:31 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.
Also, a test case has been added to ensure that this commit
addresses the issue correctly.
Additionally, some changes have been made in `get_esl_signature_list` that
adds a check to the ESL size (which was being done outside of all of the
functions calls) and also combines its functionality with
`get_esl_signature_list_size`. The purpose of this is to have for
`get_esl_signature_list` to have at least some data checks and save
repetitive code. The function now has a new set of input and output parameters
as well as a different return value. All calling functions have been edited to
respond to this change appropriately.
Lastly, some size variables in `verify_signature` and `validate_esl_list`
have been edited from being type integer to an unsigned type that better matches their specifications.
Signed-off-by: Nick Child <nick.child at ibm.com>
---
libstb/secvar/backend/edk2-compat-process.c | 82 +++++++++-----------
libstb/secvar/test/secvar-test-edk2-compat.c | 15 ++++
2 files changed, 52 insertions(+), 45 deletions(-)
diff --git a/libstb/secvar/backend/edk2-compat-process.c b/libstb/secvar/backend/edk2-compat-process.c
index 7d83c912..ff445d45 100644
--- a/libstb/secvar/backend/edk2-compat-process.c
+++ b/libstb/secvar/backend/edk2-compat-process.c
@@ -85,27 +85,28 @@ static void get_key_authority(const char *ret[3], const char *key)
ret[i] = NULL;
}
-static EFI_SIGNATURE_LIST* get_esl_signature_list(const char *buf, size_t buflen)
+static int get_esl_signature_list(const char *buf, size_t buflen, EFI_SIGNATURE_LIST** list, uint32_t *list_size)
{
- EFI_SIGNATURE_LIST *list = NULL;
+ EFI_SIGNATURE_LIST *list_tmp = NULL;
if (buflen < sizeof(EFI_SIGNATURE_LIST) || !buf)
- return NULL;
-
- list = (EFI_SIGNATURE_LIST *)buf;
-
- return list;
-}
+ return OPAL_PARAMETER;
-/* Returns the size of the complete ESL. */
-static int32_t get_esl_signature_list_size(const char *buf, const size_t buflen)
-{
- EFI_SIGNATURE_LIST *list = get_esl_signature_list(buf, buflen);
+ list_tmp = (EFI_SIGNATURE_LIST *)buf;
+ /* Calculate the size of the ESL */
+ *list_size = le32_to_cpu(list_tmp->SignatureListSize);
- if (!list)
+ /* If could not extract the size or it is larger than available data */
+ if (*list_size == 0 || *list_size > (uint32_t) buflen) {
+ prlog(PR_ERR, "Invalid size of the ESL: %u\n",
+ *list_size);
return OPAL_PARAMETER;
+ }
+
+ if (list != NULL)
+ *list = list_tmp;
- return le32_to_cpu(list->SignatureListSize);
+ return OPAL_SUCCESS;
}
/*
@@ -116,17 +117,20 @@ static int get_esl_cert(const char *buf, const size_t buflen, char **cert)
{
size_t sig_data_offset;
size_t size;
- EFI_SIGNATURE_LIST *list = get_esl_signature_list(buf, buflen);
+ uint32_t list_size;
+ int rc;
+ EFI_SIGNATURE_LIST *list;
- if (!list)
- return OPAL_PARAMETER;
+ rc = get_esl_signature_list(buf, buflen, &list, &list_size);
+ if (rc)
+ return rc;
assert(cert != NULL);
size = le32_to_cpu(list->SignatureSize) - sizeof(uuid_t);
prlog(PR_DEBUG,"size of signature list size is %u\n",
- le32_to_cpu(list->SignatureListSize));
+ list_size);
prlog(PR_DEBUG, "size of signature header size is %u\n",
le32_to_cpu(list->SignatureHeaderSize));
prlog(PR_DEBUG, "size of signature size is %u\n",
@@ -257,33 +261,24 @@ int validate_esl_list(const char *key, const char *esl, const size_t size)
int count = 0;
int dsize;
char *data = NULL;
- int eslvarsize = size;
- int eslsize;
+ size_t eslvarsize = size;
+ uint32_t eslsize;
int rc = OPAL_SUCCESS;
EFI_SIGNATURE_LIST *list = NULL;
while (eslvarsize > 0) {
- prlog(PR_DEBUG, "esl var size size is %d offset is %lu\n", eslvarsize, size - eslvarsize);
- if (eslvarsize < sizeof(EFI_SIGNATURE_LIST))
- break;
-
- /* Check Supported ESL Type */
- list = get_esl_signature_list(esl, eslvarsize);
-
- if (!list)
- return OPAL_PARAMETER;
-
- /* Calculate the size of the ESL */
- eslsize = le32_to_cpu(list->SignatureListSize);
-
- /* If could not extract the size */
- if (eslsize <= 0) {
- prlog(PR_ERR, "Invalid size of the ESL: %u\n",
- le32_to_cpu(list->SignatureListSize));
+ prlog(PR_DEBUG, "esl var size size is %zu offset is %zu\n", eslvarsize, size - eslvarsize);
+ if (eslvarsize < sizeof(EFI_SIGNATURE_LIST)) {
+ prlog(PR_ERR, "ESL has %zu unknown extra bytes\n", eslvarsize);
rc = OPAL_PARAMETER;
break;
}
+ /* Check Supported ESL Type */
+ rc = get_esl_signature_list(esl, eslvarsize, &list, &eslsize);
+ if (rc)
+ return rc;
+
/* Extract the certificate from the ESL */
dsize = get_esl_cert(esl, eslvarsize, &data);
if (dsize < 0) {
@@ -466,9 +461,9 @@ static int verify_signature(const struct efi_variable_authentication_2 *auth,
int signing_cert_size;
int rc = 0;
char *errbuf;
- int eslvarsize;
+ size_t eslvarsize;
int eslsize;
- int offset = 0;
+ size_t offset = 0;
if (!auth)
return OPAL_PARAMETER;
@@ -485,18 +480,15 @@ static int verify_signature(const struct efi_variable_authentication_2 *auth,
/* Variable is not empty */
while (eslvarsize > 0) {
- prlog(PR_DEBUG, "esl var size size is %d offset is %d\n", eslvarsize, offset);
+ prlog(PR_DEBUG, "esl var size size is %zu offset is %zu\n", eslvarsize, offset);
if (eslvarsize < sizeof(EFI_SIGNATURE_LIST))
break;
/* Calculate the size of the ESL */
- eslsize = get_esl_signature_list_size(avar->data + offset,
- eslvarsize);
+ rc = get_esl_signature_list(avar->data + offset, eslvarsize, NULL, &eslsize);
/* If could not extract the size */
- if (eslsize <= 0) {
- rc = OPAL_PARAMETER;
+ if (rc)
break;
- }
/* Extract the certificate from the ESL */
signing_cert_size = get_esl_cert(avar->data + offset,
diff --git a/libstb/secvar/test/secvar-test-edk2-compat.c b/libstb/secvar/test/secvar-test-edk2-compat.c
index 93ade920..d401f952 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
More information about the Skiboot
mailing list