| Message ID | 20220506123602.73303-1-ilias.apalodimas@linaro.org |
|---|---|
| State | Accepted |
| Commit | b436cc6a57cae017343a549f4b701e748d7e6448 |
| Headers | show |
| Series | [1/2,v4] efi_loader: add sha384/512 on certificate revocation | expand |
On 5/6/22 14:36, Ilias Apalodimas wrote: > Currently we don't support sha384/512 for the X.509 certificate > in dbx. Moreover if we come across such a hash we skip the check > and approve the image, although the image might needs to be rejected. > > Rework the code a bit and fix it by adding an array of structs with the > supported GUIDs, len and literal used in the U-Boot crypto APIs instead > of hardcoding the GUID types. > > It's worth noting here that efi_hash_regions() can now be reused from > efi_signature_lookup_digest() and add sha348/512 support there as well > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > --- > changes since v3: > - Get rid of guid_to_sha_len() > - define a local variable to efi_hash_regions() so we can use it with > a NULL ptr for len > changes since v2: > - updated changelog (there was no v1) > changes since RFC: > - add an array of structs with the algo info info of a function > - checking hash_calculate result in efi_hash_regions() > include/efi_api.h | 6 +++ > include/efi_loader.h | 6 +++ > lib/efi_loader/efi_helper.c | 66 +++++++++++++++++++++++++++++ > lib/efi_loader/efi_signature.c | 76 ++++++++++++++++++++++++---------- > 4 files changed, 131 insertions(+), 23 deletions(-) > > diff --git a/include/efi_api.h b/include/efi_api.h > index c7f7873b5d48..83c01085fded 100644 > --- a/include/efi_api.h > +++ b/include/efi_api.h > @@ -1873,6 +1873,12 @@ struct efi_system_resource_table { > #define EFI_CERT_X509_SHA256_GUID \ > EFI_GUID(0x3bd2a492, 0x96c0, 0x4079, 0xb4, 0x20, \ > 0xfc, 0xf9, 0x8e, 0xf1, 0x03, 0xed) > +#define EFI_CERT_X509_SHA384_GUID \ > + EFI_GUID(0x7076876e, 0x80c2, 0x4ee6, \ > + 0xaa, 0xd2, 0x28, 0xb3, 0x49, 0xa6, 0x86, 0x5b) > +#define EFI_CERT_X509_SHA512_GUID \ > + EFI_GUID(0x446dbf63, 0x2502, 0x4cda, \ > + 0xbc, 0xfa, 0x24, 0x65, 0xd2, 0xb0, 0xfe, 0x9d) > #define EFI_CERT_TYPE_PKCS7_GUID \ > EFI_GUID(0x4aafd29d, 0x68df, 0x49ee, 0x8a, 0xa9, \ > 0x34, 0x7d, 0x37, 0x56, 0x65, 0xa7) > diff --git a/include/efi_loader.h b/include/efi_loader.h > index effb43369d96..733ee03cd77a 100644 > --- a/include/efi_loader.h > +++ b/include/efi_loader.h > @@ -300,6 +300,8 @@ extern const efi_guid_t efi_guid_image_security_database; > extern const efi_guid_t efi_guid_sha256; > extern const efi_guid_t efi_guid_cert_x509; > extern const efi_guid_t efi_guid_cert_x509_sha256; > +extern const efi_guid_t efi_guid_cert_x509_sha384; > +extern const efi_guid_t efi_guid_cert_x509_sha512; > extern const efi_guid_t efi_guid_cert_type_pkcs7; > > /* GUID of RNG protocol */ > @@ -677,6 +679,10 @@ efi_status_t efi_file_size(struct efi_file_handle *fh, efi_uintn_t *size); > /* get a device path from a Boot#### option */ > struct efi_device_path *efi_get_dp_from_boot(const efi_guid_t guid); > > +/* get len, string (used in u-boot crypto from a guid */ > +const char *guid_to_sha_str(const efi_guid_t *guid); > +int algo_to_len(const char *algo); > + > /** > * efi_size_in_pages() - convert size in bytes to size in pages > * > diff --git a/lib/efi_loader/efi_helper.c b/lib/efi_loader/efi_helper.c > index 802d39ed97b6..849658d47bc7 100644 > --- a/lib/efi_loader/efi_helper.c > +++ b/lib/efi_loader/efi_helper.c > @@ -92,3 +92,69 @@ err: > free(var_value); > return NULL; > } > + > +const struct guid_to_hash_map { > + efi_guid_t guid; > + const char algo[32]; > + u32 bits; > +} guid_to_hash[] = { > + { > + EFI_CERT_X509_SHA256_GUID, > + "sha256", > + SHA256_SUM_LEN * 8, > + }, > + { > + EFI_CERT_SHA256_GUID, > + "sha256", > + SHA256_SUM_LEN * 8, > + }, > + { > + EFI_CERT_X509_SHA384_GUID, > + "sha384", > + SHA384_SUM_LEN * 8, > + }, > + { > + EFI_CERT_X509_SHA512_GUID, > + "sha512", > + SHA512_SUM_LEN * 8, > + }, > +}; > + > +#define MAX_GUID_TO_HASH_COUNT ARRAY_SIZE(guid_to_hash) > + > +/** guid_to_sha_str - return the sha string e.g "sha256" for a given guid > + * used on EFI security databases > + * > + * @guid: guid to check > + * > + * Return: len or 0 if no match is found > + */ > +const char *guid_to_sha_str(const efi_guid_t *guid) > +{ > + size_t i; > + > + for (i = 0; i < MAX_GUID_TO_HASH_COUNT; i++) { > + if (!guidcmp(guid, &guid_to_hash[i].guid)) > + return guid_to_hash[i].algo; > + } > + > + return NULL; > +} > + > +/** algo_to_len - return the sha size in bytes for a given string > + * > + * @guid: string to check The parameter is called algo not guid. > + * > + * Return: len or 0 if no match is found > + */ > +int algo_to_len(const char *algo) > +{ > + size_t i; > + > + for (i = 0; i < MAX_GUID_TO_HASH_COUNT; i++) { > + if (!strcmp(algo, guid_to_hash[i].algo)) > + return guid_to_hash[i].bits / 8; > + } > + > + return 0; > +} > diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c > index 79ed077ae7dd..ddac751d128e 100644 > --- a/lib/efi_loader/efi_signature.c > +++ b/lib/efi_loader/efi_signature.c > @@ -24,6 +24,8 @@ const efi_guid_t efi_guid_sha256 = EFI_CERT_SHA256_GUID; > const efi_guid_t efi_guid_cert_rsa2048 = EFI_CERT_RSA2048_GUID; > const efi_guid_t efi_guid_cert_x509 = EFI_CERT_X509_GUID; > const efi_guid_t efi_guid_cert_x509_sha256 = EFI_CERT_X509_SHA256_GUID; > +const efi_guid_t efi_guid_cert_x509_sha384 = EFI_CERT_X509_SHA384_GUID; > +const efi_guid_t efi_guid_cert_x509_sha512 = EFI_CERT_X509_SHA512_GUID; > const efi_guid_t efi_guid_cert_type_pkcs7 = EFI_CERT_TYPE_PKCS7_GUID; > > static u8 pkcs7_hdr[] = { > @@ -124,23 +126,35 @@ struct pkcs7_message *efi_parse_pkcs7_header(const void *buf, > * Return: true on success, false on error > */ > static bool efi_hash_regions(struct image_region *regs, int count, > - void **hash, size_t *size) > + void **hash, const char *hash_algo, int *len) > { > + int ret, hash_len; > + > + if (!hash_algo) > + return false; > + > + hash_len = algo_to_len(hash_algo); > + if (!hash_len) > + return false; > + > if (!*hash) { > - *hash = calloc(1, SHA256_SUM_LEN); > + *hash = calloc(1, hash_len); > if (!*hash) { > EFI_PRINT("Out of memory\n"); > return false; > } > } > - if (size) > - *size = SHA256_SUM_LEN; > > - hash_calculate("sha256", regs, count, *hash); > + ret = hash_calculate(hash_algo, regs, count, *hash); > + if (ret) > + return false; > + > + if (len) > + *len = hash_len; > #ifdef DEBUG > EFI_PRINT("hash calculated:\n"); > print_hex_dump(" ", DUMP_PREFIX_OFFSET, 16, 1, > - *hash, SHA256_SUM_LEN, false); > + *hash, hash_len, false); > #endif > > return true; > @@ -190,7 +204,6 @@ bool efi_signature_lookup_digest(struct efi_image_regions *regs, > struct efi_signature_store *siglist; > struct efi_sig_data *sig_data; > void *hash = NULL; > - size_t size = 0; > bool found = false; > bool hash_done = false; > > @@ -200,6 +213,8 @@ bool efi_signature_lookup_digest(struct efi_image_regions *regs, > goto out; > > for (siglist = db; siglist; siglist = siglist->next) { > + int len = 0; > + const char *hash_algo = NULL; > /* > * if the hash algorithm is unsupported and we get an entry in > * dbx reject the image > @@ -215,8 +230,14 @@ bool efi_signature_lookup_digest(struct efi_image_regions *regs, > if (guidcmp(&siglist->sig_type, &efi_guid_sha256)) > continue; > > + hash_algo = guid_to_sha_str(&efi_guid_sha256); > + /* > + * We could check size and hash_algo but efi_hash_regions() > + * will do that for us > + */ > if (!hash_done && > - !efi_hash_regions(regs->reg, regs->num, &hash, &size)) { > + !efi_hash_regions(regs->reg, regs->num, &hash, hash_algo, > + &len)) { > EFI_PRINT("Digesting an image failed\n"); > break; > } > @@ -229,8 +250,8 @@ bool efi_signature_lookup_digest(struct efi_image_regions *regs, > print_hex_dump(" ", DUMP_PREFIX_OFFSET, 16, 1, > sig_data->data, sig_data->size, false); > #endif > - if (sig_data->size == size && > - !memcmp(sig_data->data, hash, size)) { > + if (sig_data->size == len && > + !memcmp(sig_data->data, hash, len)) { > found = true; > free(hash); > goto out; > @@ -263,8 +284,9 @@ static bool efi_lookup_certificate(struct x509_certificate *cert, > struct efi_sig_data *sig_data; > struct image_region reg[1]; > void *hash = NULL, *hash_tmp = NULL; > - size_t size = 0; > + int len = 0; > bool found = false; > + const char *hash_algo = NULL; > > EFI_PRINT("%s: Enter, %p, %p\n", __func__, cert, db); > > @@ -278,7 +300,10 @@ static bool efi_lookup_certificate(struct x509_certificate *cert, > /* calculate hash of TBSCertificate */ > reg[0].data = cert->tbs; > reg[0].size = cert->tbs_size; > - if (!efi_hash_regions(reg, 1, &hash, &size)) > + > + /* We just need any sha256 algo to start the matching */ > + hash_algo = guid_to_sha_str(&efi_guid_sha256); > + if (!efi_hash_regions(reg, 1, &hash, hash_algo, &len)) > goto out; > > EFI_PRINT("%s: searching for %s\n", __func__, cert->subject); > @@ -300,12 +325,13 @@ static bool efi_lookup_certificate(struct x509_certificate *cert, > cert_tmp->subject); > reg[0].data = cert_tmp->tbs; > reg[0].size = cert_tmp->tbs_size; > - if (!efi_hash_regions(reg, 1, &hash_tmp, NULL)) > + if (!efi_hash_regions(reg, 1, &hash_tmp, hash_algo, > + NULL)) > goto out; > > x509_free_certificate(cert_tmp); > > - if (!memcmp(hash, hash_tmp, size)) { > + if (!memcmp(hash, hash_tmp, len)) { > found = true; > goto out; > } > @@ -400,9 +426,10 @@ static bool efi_signature_check_revocation(struct pkcs7_signed_info *sinfo, > struct efi_sig_data *sig_data; > struct image_region reg[1]; > void *hash = NULL; > - size_t size = 0; > + int len = 0; > time64_t revoc_time; > bool revoked = false; > + const char *hash_algo = NULL; > > EFI_PRINT("%s: Enter, %p, %p, %p\n", __func__, sinfo, cert, dbx); > > @@ -411,13 +438,14 @@ static bool efi_signature_check_revocation(struct pkcs7_signed_info *sinfo, > > EFI_PRINT("Checking revocation against %s\n", cert->subject); > for (siglist = dbx; siglist; siglist = siglist->next) { > - if (guidcmp(&siglist->sig_type, &efi_guid_cert_x509_sha256)) > + hash_algo = guid_to_sha_str(&siglist->sig_type); > + if (!hash_algo) > continue; > > /* calculate hash of TBSCertificate */ > reg[0].data = cert->tbs; > reg[0].size = cert->tbs_size; > - if (!efi_hash_regions(reg, 1, &hash, &size)) > + if (!efi_hash_regions(reg, 1, &hash, hash_algo, &len)) > goto out; > > for (sig_data = siglist->sig_data_list; sig_data; > @@ -429,18 +457,18 @@ static bool efi_signature_check_revocation(struct pkcs7_signed_info *sinfo, > * }; > */ > #ifdef DEBUG > - if (sig_data->size >= size) { > + if (sig_data->size >= len) { > EFI_PRINT("hash in db:\n"); > print_hex_dump(" ", DUMP_PREFIX_OFFSET, > 16, 1, > - sig_data->data, size, false); > + sig_data->data, len, false); > } > #endif > - if ((sig_data->size < size + sizeof(time64_t)) || > - memcmp(sig_data->data, hash, size)) > + if ((sig_data->size < len + sizeof(time64_t)) || > + memcmp(sig_data->data, hash, len)) > continue; > > - memcpy(&revoc_time, sig_data->data + size, > + memcpy(&revoc_time, sig_data->data + len, > sizeof(revoc_time)); > EFI_PRINT("revocation time: 0x%llx\n", revoc_time); > /* > @@ -500,7 +528,9 @@ bool efi_signature_verify(struct efi_image_regions *regs, > */ > if (!msg->data && > !efi_hash_regions(regs->reg, regs->num, > - (void **)&sinfo->sig->digest, NULL)) { > + (void **)&sinfo->sig->digest, > + guid_to_sha_str(&efi_guid_sha256), The UEFI spec knows certificate types like EFI_CERT_X509_SHA512_GUID. Why do we assume SHA256 here? Best regards Heinrich > + NULL)) { > EFI_PRINT("Digesting an image failed\n"); > goto out; > }
Hi Heinrich, [...] > > /* > > @@ -500,7 +528,9 @@ bool efi_signature_verify(struct efi_image_regions *regs, > > */ > > if (!msg->data && > > !efi_hash_regions(regs->reg, regs->num, > > - (void **)&sinfo->sig->digest, NULL)) { > > + (void **)&sinfo->sig->digest, > > + guid_to_sha_str(&efi_guid_sha256), > > The UEFI spec knows certificate types like EFI_CERT_X509_SHA512_GUID. > Why do we assume SHA256 here? This part is only used for variable authentication. This was using sha256 only before the patch, but isn't that the only thing the spec mandates for authenticated variables? > > Best regards > > Heinrich > > > + NULL)) { > > EFI_PRINT("Digesting an image failed\n"); > > goto out; > > } >
diff --git a/include/efi_api.h b/include/efi_api.h index c7f7873b5d48..83c01085fded 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -1873,6 +1873,12 @@ struct efi_system_resource_table { #define EFI_CERT_X509_SHA256_GUID \ EFI_GUID(0x3bd2a492, 0x96c0, 0x4079, 0xb4, 0x20, \ 0xfc, 0xf9, 0x8e, 0xf1, 0x03, 0xed) +#define EFI_CERT_X509_SHA384_GUID \ + EFI_GUID(0x7076876e, 0x80c2, 0x4ee6, \ + 0xaa, 0xd2, 0x28, 0xb3, 0x49, 0xa6, 0x86, 0x5b) +#define EFI_CERT_X509_SHA512_GUID \ + EFI_GUID(0x446dbf63, 0x2502, 0x4cda, \ + 0xbc, 0xfa, 0x24, 0x65, 0xd2, 0xb0, 0xfe, 0x9d) #define EFI_CERT_TYPE_PKCS7_GUID \ EFI_GUID(0x4aafd29d, 0x68df, 0x49ee, 0x8a, 0xa9, \ 0x34, 0x7d, 0x37, 0x56, 0x65, 0xa7) diff --git a/include/efi_loader.h b/include/efi_loader.h index effb43369d96..733ee03cd77a 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -300,6 +300,8 @@ extern const efi_guid_t efi_guid_image_security_database; extern const efi_guid_t efi_guid_sha256; extern const efi_guid_t efi_guid_cert_x509; extern const efi_guid_t efi_guid_cert_x509_sha256; +extern const efi_guid_t efi_guid_cert_x509_sha384; +extern const efi_guid_t efi_guid_cert_x509_sha512; extern const efi_guid_t efi_guid_cert_type_pkcs7; /* GUID of RNG protocol */ @@ -677,6 +679,10 @@ efi_status_t efi_file_size(struct efi_file_handle *fh, efi_uintn_t *size); /* get a device path from a Boot#### option */ struct efi_device_path *efi_get_dp_from_boot(const efi_guid_t guid); +/* get len, string (used in u-boot crypto from a guid */ +const char *guid_to_sha_str(const efi_guid_t *guid); +int algo_to_len(const char *algo); + /** * efi_size_in_pages() - convert size in bytes to size in pages * diff --git a/lib/efi_loader/efi_helper.c b/lib/efi_loader/efi_helper.c index 802d39ed97b6..849658d47bc7 100644 --- a/lib/efi_loader/efi_helper.c +++ b/lib/efi_loader/efi_helper.c @@ -92,3 +92,69 @@ err: free(var_value); return NULL; } + +const struct guid_to_hash_map { + efi_guid_t guid; + const char algo[32]; + u32 bits; +} guid_to_hash[] = { + { + EFI_CERT_X509_SHA256_GUID, + "sha256", + SHA256_SUM_LEN * 8, + }, + { + EFI_CERT_SHA256_GUID, + "sha256", + SHA256_SUM_LEN * 8, + }, + { + EFI_CERT_X509_SHA384_GUID, + "sha384", + SHA384_SUM_LEN * 8, + }, + { + EFI_CERT_X509_SHA512_GUID, + "sha512", + SHA512_SUM_LEN * 8, + }, +}; + +#define MAX_GUID_TO_HASH_COUNT ARRAY_SIZE(guid_to_hash) + +/** guid_to_sha_str - return the sha string e.g "sha256" for a given guid + * used on EFI security databases + * + * @guid: guid to check + * + * Return: len or 0 if no match is found + */ +const char *guid_to_sha_str(const efi_guid_t *guid) +{ + size_t i; + + for (i = 0; i < MAX_GUID_TO_HASH_COUNT; i++) { + if (!guidcmp(guid, &guid_to_hash[i].guid)) + return guid_to_hash[i].algo; + } + + return NULL; +} + +/** algo_to_len - return the sha size in bytes for a given string + * + * @guid: string to check + * + * Return: len or 0 if no match is found + */ +int algo_to_len(const char *algo) +{ + size_t i; + + for (i = 0; i < MAX_GUID_TO_HASH_COUNT; i++) { + if (!strcmp(algo, guid_to_hash[i].algo)) + return guid_to_hash[i].bits / 8; + } + + return 0; +} diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c index 79ed077ae7dd..ddac751d128e 100644 --- a/lib/efi_loader/efi_signature.c +++ b/lib/efi_loader/efi_signature.c @@ -24,6 +24,8 @@ const efi_guid_t efi_guid_sha256 = EFI_CERT_SHA256_GUID; const efi_guid_t efi_guid_cert_rsa2048 = EFI_CERT_RSA2048_GUID; const efi_guid_t efi_guid_cert_x509 = EFI_CERT_X509_GUID; const efi_guid_t efi_guid_cert_x509_sha256 = EFI_CERT_X509_SHA256_GUID; +const efi_guid_t efi_guid_cert_x509_sha384 = EFI_CERT_X509_SHA384_GUID; +const efi_guid_t efi_guid_cert_x509_sha512 = EFI_CERT_X509_SHA512_GUID; const efi_guid_t efi_guid_cert_type_pkcs7 = EFI_CERT_TYPE_PKCS7_GUID; static u8 pkcs7_hdr[] = { @@ -124,23 +126,35 @@ struct pkcs7_message *efi_parse_pkcs7_header(const void *buf, * Return: true on success, false on error */ static bool efi_hash_regions(struct image_region *regs, int count, - void **hash, size_t *size) + void **hash, const char *hash_algo, int *len) { + int ret, hash_len; + + if (!hash_algo) + return false; + + hash_len = algo_to_len(hash_algo); + if (!hash_len) + return false; + if (!*hash) { - *hash = calloc(1, SHA256_SUM_LEN); + *hash = calloc(1, hash_len); if (!*hash) { EFI_PRINT("Out of memory\n"); return false; } } - if (size) - *size = SHA256_SUM_LEN; - hash_calculate("sha256", regs, count, *hash); + ret = hash_calculate(hash_algo, regs, count, *hash); + if (ret) + return false; + + if (len) + *len = hash_len; #ifdef DEBUG EFI_PRINT("hash calculated:\n"); print_hex_dump(" ", DUMP_PREFIX_OFFSET, 16, 1, - *hash, SHA256_SUM_LEN, false); + *hash, hash_len, false); #endif return true; @@ -190,7 +204,6 @@ bool efi_signature_lookup_digest(struct efi_image_regions *regs, struct efi_signature_store *siglist; struct efi_sig_data *sig_data; void *hash = NULL; - size_t size = 0; bool found = false; bool hash_done = false; @@ -200,6 +213,8 @@ bool efi_signature_lookup_digest(struct efi_image_regions *regs, goto out; for (siglist = db; siglist; siglist = siglist->next) { + int len = 0; + const char *hash_algo = NULL; /* * if the hash algorithm is unsupported and we get an entry in * dbx reject the image @@ -215,8 +230,14 @@ bool efi_signature_lookup_digest(struct efi_image_regions *regs, if (guidcmp(&siglist->sig_type, &efi_guid_sha256)) continue; + hash_algo = guid_to_sha_str(&efi_guid_sha256); + /* + * We could check size and hash_algo but efi_hash_regions() + * will do that for us + */ if (!hash_done && - !efi_hash_regions(regs->reg, regs->num, &hash, &size)) { + !efi_hash_regions(regs->reg, regs->num, &hash, hash_algo, + &len)) { EFI_PRINT("Digesting an image failed\n"); break; } @@ -229,8 +250,8 @@ bool efi_signature_lookup_digest(struct efi_image_regions *regs, print_hex_dump(" ", DUMP_PREFIX_OFFSET, 16, 1, sig_data->data, sig_data->size, false); #endif - if (sig_data->size == size && - !memcmp(sig_data->data, hash, size)) { + if (sig_data->size == len && + !memcmp(sig_data->data, hash, len)) { found = true; free(hash); goto out; @@ -263,8 +284,9 @@ static bool efi_lookup_certificate(struct x509_certificate *cert, struct efi_sig_data *sig_data; struct image_region reg[1]; void *hash = NULL, *hash_tmp = NULL; - size_t size = 0; + int len = 0; bool found = false; + const char *hash_algo = NULL; EFI_PRINT("%s: Enter, %p, %p\n", __func__, cert, db); @@ -278,7 +300,10 @@ static bool efi_lookup_certificate(struct x509_certificate *cert, /* calculate hash of TBSCertificate */ reg[0].data = cert->tbs; reg[0].size = cert->tbs_size; - if (!efi_hash_regions(reg, 1, &hash, &size)) + + /* We just need any sha256 algo to start the matching */ + hash_algo = guid_to_sha_str(&efi_guid_sha256); + if (!efi_hash_regions(reg, 1, &hash, hash_algo, &len)) goto out; EFI_PRINT("%s: searching for %s\n", __func__, cert->subject); @@ -300,12 +325,13 @@ static bool efi_lookup_certificate(struct x509_certificate *cert, cert_tmp->subject); reg[0].data = cert_tmp->tbs; reg[0].size = cert_tmp->tbs_size; - if (!efi_hash_regions(reg, 1, &hash_tmp, NULL)) + if (!efi_hash_regions(reg, 1, &hash_tmp, hash_algo, + NULL)) goto out; x509_free_certificate(cert_tmp); - if (!memcmp(hash, hash_tmp, size)) { + if (!memcmp(hash, hash_tmp, len)) { found = true; goto out; } @@ -400,9 +426,10 @@ static bool efi_signature_check_revocation(struct pkcs7_signed_info *sinfo, struct efi_sig_data *sig_data; struct image_region reg[1]; void *hash = NULL; - size_t size = 0; + int len = 0; time64_t revoc_time; bool revoked = false; + const char *hash_algo = NULL; EFI_PRINT("%s: Enter, %p, %p, %p\n", __func__, sinfo, cert, dbx); @@ -411,13 +438,14 @@ static bool efi_signature_check_revocation(struct pkcs7_signed_info *sinfo, EFI_PRINT("Checking revocation against %s\n", cert->subject); for (siglist = dbx; siglist; siglist = siglist->next) { - if (guidcmp(&siglist->sig_type, &efi_guid_cert_x509_sha256)) + hash_algo = guid_to_sha_str(&siglist->sig_type); + if (!hash_algo) continue; /* calculate hash of TBSCertificate */ reg[0].data = cert->tbs; reg[0].size = cert->tbs_size; - if (!efi_hash_regions(reg, 1, &hash, &size)) + if (!efi_hash_regions(reg, 1, &hash, hash_algo, &len)) goto out; for (sig_data = siglist->sig_data_list; sig_data; @@ -429,18 +457,18 @@ static bool efi_signature_check_revocation(struct pkcs7_signed_info *sinfo, * }; */ #ifdef DEBUG - if (sig_data->size >= size) { + if (sig_data->size >= len) { EFI_PRINT("hash in db:\n"); print_hex_dump(" ", DUMP_PREFIX_OFFSET, 16, 1, - sig_data->data, size, false); + sig_data->data, len, false); } #endif - if ((sig_data->size < size + sizeof(time64_t)) || - memcmp(sig_data->data, hash, size)) + if ((sig_data->size < len + sizeof(time64_t)) || + memcmp(sig_data->data, hash, len)) continue; - memcpy(&revoc_time, sig_data->data + size, + memcpy(&revoc_time, sig_data->data + len, sizeof(revoc_time)); EFI_PRINT("revocation time: 0x%llx\n", revoc_time); /* @@ -500,7 +528,9 @@ bool efi_signature_verify(struct efi_image_regions *regs, */ if (!msg->data && !efi_hash_regions(regs->reg, regs->num, - (void **)&sinfo->sig->digest, NULL)) { + (void **)&sinfo->sig->digest, + guid_to_sha_str(&efi_guid_sha256), + NULL)) { EFI_PRINT("Digesting an image failed\n"); goto out; }
Currently we don't support sha384/512 for the X.509 certificate in dbx. Moreover if we come across such a hash we skip the check and approve the image, although the image might needs to be rejected. Rework the code a bit and fix it by adding an array of structs with the supported GUIDs, len and literal used in the U-Boot crypto APIs instead of hardcoding the GUID types. It's worth noting here that efi_hash_regions() can now be reused from efi_signature_lookup_digest() and add sha348/512 support there as well Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> --- changes since v3: - Get rid of guid_to_sha_len() - define a local variable to efi_hash_regions() so we can use it with a NULL ptr for len changes since v2: - updated changelog (there was no v1) changes since RFC: - add an array of structs with the algo info info of a function - checking hash_calculate result in efi_hash_regions() include/efi_api.h | 6 +++ include/efi_loader.h | 6 +++ lib/efi_loader/efi_helper.c | 66 +++++++++++++++++++++++++++++ lib/efi_loader/efi_signature.c | 76 ++++++++++++++++++++++++---------- 4 files changed, 131 insertions(+), 23 deletions(-)