Akashi-san,
- {
EFI_CERT_X509_SHA256_GUID,"sha256",256,=> SHA256_SUM_LEN * 8
Sure
- },
- {
EFI_CERT_SHA256_GUID,"sha256",256,- },
- {
EFI_CERT_X509_SHA384_GUID,"sha384",384,- },
- {
EFI_CERT_X509_SHA512_GUID,"sha512",512,- },
+};
+#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;
+}
+/** guid_to_sha_len - return the sha size in bytes for a given guid
used on EFI security databases
- @guid: guid to check
- Return: len or 0 if no match is found
- */
+int guid_to_sha_len(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].bits / 8;- }
- return 0;
+}
If we have algo_to_len(), we don't need guid_to_sha_len().
True, I'll get rid of this
+/** 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)
This function seems to be quite generic and useful for others. Why not implement it along with hash_calculate() in hash-checksum.c (without using guid_to_hash[]).
Tbh I have similar stuff in efi_tcg2.c, but they are all bound to EFI atm. We could refactor all of the structures a bit and have a more generic version. Can we leave it for now and I can send a refactoring series after this gets merged?
+{
- 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..cf01f21b4d04 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,32 @@ 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;
- if (!hash_algo || !len)
return false;- *len = algo_to_len(hash_algo);
Please modify it this way; int hash_len;
hash_len = algo_to_len(hash_algo); if (len) *len = hash_len;Then use hash_len instead of *len hereafterr.
This way, we don't have to define a unused local variable below code.
Sure
[...]
- /* We just need any sha256 algo to start the matching */
- hash_algo = guid_to_sha_str(&efi_guid_sha256);
Why not check if hash_algo is NULL?
- len = guid_to_sha_len(&efi_guid_sha256);
=> len = algo_to_len(hash_algo);Or even we don't need this line as efi_has_regions() will set it for us.
Yea good catch. I was playing with a version where len wasn't a ptr in efi_hash_regions() and that's a leftover (as well as guid_to_sha_len()).
[...]
Thanks for having a look /Ilias