diff options
author | Nick Child <nnac123@gmail.com> | 2021-07-01 14:58:12 -0400 |
---|---|---|
committer | Vasant Hegde <hegdevasant@linux.vnet.ibm.com> | 2021-07-19 20:51:22 +0530 |
commit | 8a31163a0271f11b4597bca4e803f559e38e3d24 (patch) | |
tree | 1c57bfc63c2918c54ee62021ae900ec1ba6850b9 /libstb/secvar/test/secvar-test-edk2-compat.c | |
parent | 1daaf0ee564e3edb5964fbd2877a293088656820 (diff) | |
download | skiboot-8a31163a0271f11b4597bca4e803f559e38e3d24.zip skiboot-8a31163a0271f11b4597bca4e803f559e38e3d24.tar.gz skiboot-8a31163a0271f11b4597bca4e803f559e38e3d24.tar.bz2 |
secvar: ensure ESL buf size is at least what ESL header expects
Currently, `get_esl_cert` receives a data buffer containing an ESL and its
length. It is to return a data buffer of the certificate that is contained
inside the ESL. The ESL has header info that contains the certificates
`size` and the size of the header (`sig_data_offset`). We use this
information to copy `size` bytes starting `sig_data_offset` bytes after the
given ESL buffer. Currently we are checking that the length of the ESL
buffer is at least `sig_data_offset` bytes but we are not checking that it
also has enough bytes to also contain `size` bytes of the certificate. This
becomes problematic if some data at the end of the ESL gets lost. Since the
ESL claims it has more than it actually does, this will lead to a buffer
over-read. What is even worse, is that this buffer over-read can go
unnoticed since the last 256 bytes of the ESL are usually the x509 2048 bit
signature so the extra garbage bytes that are copied will appear to be a
valid rsa signature.
To resolve this, this commit ensures that the ESL buffer length is large
enough to hold the data that it claims it contains.
Lastly, a new test case is added to test the described condition. It
includes a new test file `trimmedKEK.h` which contains a struct a valid KEK
auth file minus 5 bytes, therefore making it invalid.
Signed-off-by: Nick Child <nick.child@ibm.com>
Reviewed-by: Daniel Axtens <dja@axtens.net>
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>
Diffstat (limited to 'libstb/secvar/test/secvar-test-edk2-compat.c')
-rw-r--r-- | libstb/secvar/test/secvar-test-edk2-compat.c | 16 |
1 files changed, 16 insertions, 0 deletions
diff --git a/libstb/secvar/test/secvar-test-edk2-compat.c b/libstb/secvar/test/secvar-test-edk2-compat.c index 87c3fe3..b322e49 100644 --- a/libstb/secvar/test/secvar-test-edk2-compat.c +++ b/libstb/secvar/test/secvar-test-edk2-compat.c @@ -12,6 +12,7 @@ #include "./data/KEK.h" #include "./data/invalidkek.h" #include "./data/malformedkek.h" +#include "./data/trimmedKEK.h" #include "./data/db.h" #include "./data/dbsigneddata.h" #include "./data/OldTSKEK.h" @@ -197,6 +198,21 @@ int run_test() tmp = find_secvar("db", 3, &variable_bank); ASSERT(NULL != tmp); + /* Add trimmed KEK, .process(), should fail. */ + printf("Add trimmed KEK\n"); + tmp = new_secvar("KEK", 4, trimmedKEK_auth, trimmedKEK_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_PARAMETER == rc); + ASSERT(5 == list_length(&variable_bank)); + ASSERT(0 == list_length(&update_bank)); + tmp = find_secvar("KEK", 4, &variable_bank); + ASSERT(NULL != tmp); + ASSERT(0 == tmp->data_size); + /* Add valid KEK, .process(), succeeds. */ printf("Add KEK"); tmp = new_secvar("KEK", 4, KEK_auth, KEK_auth_len, 0); |