From b9f217a4cbe8c808a34f824503d15996a7f8be13 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Wed, 8 Jul 2020 07:48:06 +0200 Subject: [PATCH 01/37] lib/crypto: use qualified path for x509_parser.h Use the path relative to /include for x509_parser.h in pkcs7_parser.h. Signed-off-by: Heinrich Schuchardt --- include/crypto/pkcs7_parser.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/crypto/pkcs7_parser.h b/include/crypto/pkcs7_parser.h index b8234da45a..906033a90e 100644 --- a/include/crypto/pkcs7_parser.h +++ b/include/crypto/pkcs7_parser.h @@ -10,7 +10,7 @@ #include #include -#include "x509_parser.h" +#include #define kenter(FMT, ...) \ pr_devel("==> %s("FMT")\n", __func__, ##__VA_ARGS__) From 1a44b7059c183a227f2fc4519df24da09d403cba Mon Sep 17 00:00:00 2001 From: AKASHI Takahiro Date: Wed, 8 Jul 2020 14:01:52 +0900 Subject: [PATCH 02/37] efi_loader: image_loader: add a check against certificate type of authenticode UEFI specification requires that we shall support three type of certificates of authenticode in PE image: WIN_CERT_TYPE_EFI_GUID with the guid, EFI_CERT_TYPE_PCKS7_GUID WIN_CERT_TYPE_PKCS_SIGNED_DATA WIN_CERT_TYPE_EFI_PKCS1_15 As EDK2 does, we will support the first two that are pkcs7 SignedData. Signed-off-by: AKASHI Takahiro --- lib/efi_loader/efi_image_loader.c | 56 ++++++++++++++++++++++++------- 1 file changed, 44 insertions(+), 12 deletions(-) diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c index 06a2ebdb90..9b01e1ec90 100644 --- a/lib/efi_loader/efi_image_loader.c +++ b/lib/efi_loader/efi_image_loader.c @@ -483,7 +483,8 @@ static bool efi_image_authenticate(void *efi, size_t efi_size) struct efi_signature_store *db = NULL, *dbx = NULL; struct x509_certificate *cert = NULL; void *new_efi = NULL; - size_t new_efi_size; + u8 *auth, *wincerts_end; + size_t new_efi_size, auth_size; bool ret = false; if (!efi_secure_boot_enabled()) @@ -532,21 +533,52 @@ static bool efi_image_authenticate(void *efi, size_t efi_size) } /* go through WIN_CERTIFICATE list */ - for (wincert = wincerts; - (void *)wincert < (void *)wincerts + wincerts_len; - wincert = (void *)wincert + ALIGN(wincert->dwLength, 8)) { - if (wincert->dwLength < sizeof(*wincert)) { - EFI_PRINT("%s: dwLength too small: %u < %zu\n", - __func__, wincert->dwLength, - sizeof(*wincert)); - goto err; + for (wincert = wincerts, wincerts_end = (u8 *)wincerts + wincerts_len; + (u8 *)wincert < wincerts_end; + wincert = (WIN_CERTIFICATE *) + ((u8 *)wincert + ALIGN(wincert->dwLength, 8))) { + if ((u8 *)wincert + sizeof(*wincert) >= wincerts_end) + break; + + if (wincert->dwLength <= sizeof(*wincert)) { + EFI_PRINT("dwLength too small: %u < %zu\n", + wincert->dwLength, sizeof(*wincert)); + continue; } - msg = pkcs7_parse_message((void *)wincert + sizeof(*wincert), - wincert->dwLength - sizeof(*wincert)); + + EFI_PRINT("WIN_CERTIFICATE_TYPE: 0x%x\n", + wincert->wCertificateType); + + auth = (u8 *)wincert + sizeof(*wincert); + auth_size = wincert->dwLength - sizeof(*wincert); + if (wincert->wCertificateType == WIN_CERT_TYPE_EFI_GUID) { + if (auth + sizeof(efi_guid_t) >= wincerts_end) + break; + + if (auth_size <= sizeof(efi_guid_t)) { + EFI_PRINT("dwLength too small: %u < %zu\n", + wincert->dwLength, sizeof(*wincert)); + continue; + } + if (guidcmp(auth, &efi_guid_cert_type_pkcs7)) { + EFI_PRINT("Certificate type not supported: %pUl\n", + auth); + continue; + } + + auth += sizeof(efi_guid_t); + auth_size -= sizeof(efi_guid_t); + } else if (wincert->wCertificateType + != WIN_CERT_TYPE_PKCS_SIGNED_DATA) { + EFI_PRINT("Certificate type not supported\n"); + continue; + } + + msg = pkcs7_parse_message(auth, auth_size); if (IS_ERR(msg)) { EFI_PRINT("Parsing image's signature failed\n"); msg = NULL; - goto err; + continue; } /* try black-list first */ From eb537fd7eb05665a088766128eebd45b585679d3 Mon Sep 17 00:00:00 2001 From: AKASHI Takahiro Date: Wed, 8 Jul 2020 14:01:53 +0900 Subject: [PATCH 03/37] efi_loader: image_loader: retrieve authenticode only if it exists Since the certificate table, which is indexed by IMAGE_DIRECTORY_ENTRY_SECURITY and contains authenticode in PE image, doesn't always exist, we should make sure that we will retrieve its pointer only if it exists. Signed-off-by: AKASHI Takahiro --- lib/efi_loader/efi_image_loader.c | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c index 9b01e1ec90..de230409e3 100644 --- a/lib/efi_loader/efi_image_loader.c +++ b/lib/efi_loader/efi_image_loader.c @@ -267,6 +267,8 @@ bool efi_image_parse(void *efi, size_t len, struct efi_image_regions **regp, dos = (void *)efi; nt = (void *)(efi + dos->e_lfanew); + authoff = 0; + authsz = 0; /* * Count maximum number of regions to be digested. @@ -305,25 +307,36 @@ bool efi_image_parse(void *efi, size_t len, struct efi_image_regions **regp, efi_image_region_add(regs, &opt->DataDirectory[ctidx] + 1, efi + opt->SizeOfHeaders, 0); + + authoff = opt->DataDirectory[ctidx].VirtualAddress; + authsz = opt->DataDirectory[ctidx].Size; } bytes_hashed = opt->SizeOfHeaders; align = opt->FileAlignment; - authoff = opt->DataDirectory[ctidx].VirtualAddress; - authsz = opt->DataDirectory[ctidx].Size; } else if (nt->OptionalHeader.Magic == IMAGE_NT_OPTIONAL_HDR32_MAGIC) { IMAGE_OPTIONAL_HEADER32 *opt = &nt->OptionalHeader; + /* Skip CheckSum */ efi_image_region_add(regs, efi, &opt->CheckSum, 0); - efi_image_region_add(regs, &opt->Subsystem, - &opt->DataDirectory[ctidx], 0); - efi_image_region_add(regs, &opt->DataDirectory[ctidx] + 1, - efi + opt->SizeOfHeaders, 0); + if (nt->OptionalHeader.NumberOfRvaAndSizes <= ctidx) { + efi_image_region_add(regs, + &opt->Subsystem, + efi + opt->SizeOfHeaders, 0); + } else { + /* Skip Certificates Table */ + efi_image_region_add(regs, &opt->Subsystem, + &opt->DataDirectory[ctidx], 0); + efi_image_region_add(regs, + &opt->DataDirectory[ctidx] + 1, + efi + opt->SizeOfHeaders, 0); + + authoff = opt->DataDirectory[ctidx].VirtualAddress; + authsz = opt->DataDirectory[ctidx].Size; + } bytes_hashed = opt->SizeOfHeaders; align = opt->FileAlignment; - authoff = opt->DataDirectory[ctidx].VirtualAddress; - authsz = opt->DataDirectory[ctidx].Size; } else { EFI_PRINT("%s: Invalid optional header magic %x\n", __func__, nt->OptionalHeader.Magic); From 36b2f9da5ccd38581c80098b0cad6dd1a9224705 Mon Sep 17 00:00:00 2001 From: AKASHI Takahiro Date: Wed, 8 Jul 2020 14:01:54 +0900 Subject: [PATCH 04/37] efi_loader: signature: fix a size check against revocation list Since the size check against an entry in efi_search_siglist() is incorrect, this function will never find out a to-be-matched certificate and its associated revocation time in the signature list. Signed-off-by: AKASHI Takahiro --- lib/efi_loader/efi_signature.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c index e05c471c61..cd2df46264 100644 --- a/lib/efi_loader/efi_signature.c +++ b/lib/efi_loader/efi_signature.c @@ -433,10 +433,11 @@ static bool efi_search_siglist(struct x509_certificate *cert, * time64_t revocation_time; * }; */ - if ((sig_data->size == SHA256_SUM_LEN) && - !memcmp(sig_data->data, hash, SHA256_SUM_LEN)) { + if ((sig_data->size >= SHA256_SUM_LEN + sizeof(time64_t)) && + !memcmp(sig_data->data, msg, SHA256_SUM_LEN)) { memcpy(revoc_time, sig_data->data + SHA256_SUM_LEN, sizeof(*revoc_time)); + EFI_PRINT("revocation time: 0x%llx\n", *revoc_time); found = true; goto out; } From 1e64d0b5a4ca8de67e79f332dd4d51a243f51eb8 Mon Sep 17 00:00:00 2001 From: AKASHI Takahiro Date: Wed, 8 Jul 2020 14:01:55 +0900 Subject: [PATCH 05/37] efi_loader: signature: make efi_hash_regions more generic There are a couple of occurrences of hash calculations in which a new efi_hash_regions will be commonly used. Signed-off-by: AKASHI Takahiro --- lib/efi_loader/efi_signature.c | 46 +++++++++++++--------------------- 1 file changed, 17 insertions(+), 29 deletions(-) diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c index cd2df46264..b14d104094 100644 --- a/lib/efi_loader/efi_signature.c +++ b/lib/efi_loader/efi_signature.c @@ -28,7 +28,8 @@ const efi_guid_t efi_guid_cert_type_pkcs7 = EFI_CERT_TYPE_PKCS7_GUID; /** * efi_hash_regions - calculate a hash value - * @regs: List of regions + * @regs: Array of regions + * @count: Number of regions * @hash: Pointer to a pointer to buffer holding a hash value * @size: Size of buffer to be returned * @@ -36,18 +37,20 @@ const efi_guid_t efi_guid_cert_type_pkcs7 = EFI_CERT_TYPE_PKCS7_GUID; * * Return: true on success, false on error */ -static bool efi_hash_regions(struct efi_image_regions *regs, void **hash, - size_t *size) +static bool efi_hash_regions(struct image_region *regs, int count, + void **hash, size_t *size) { - *size = 0; - *hash = calloc(1, SHA256_SUM_LEN); if (!*hash) { - EFI_PRINT("Out of memory\n"); - return false; + *hash = calloc(1, SHA256_SUM_LEN); + if (!*hash) { + EFI_PRINT("Out of memory\n"); + return false; + } } - *size = SHA256_SUM_LEN; + if (size) + *size = SHA256_SUM_LEN; - hash_calculate("sha256", regs->reg, regs->num, *hash); + hash_calculate("sha256", regs, count, *hash); #ifdef DEBUG EFI_PRINT("hash calculated:\n"); print_hex_dump(" ", DUMP_PREFIX_OFFSET, 16, 1, @@ -72,26 +75,10 @@ static bool efi_hash_msg_content(struct pkcs7_message *msg, void **hash, { struct image_region regtmp; - *size = 0; - *hash = calloc(1, SHA256_SUM_LEN); - if (!*hash) { - EFI_PRINT("Out of memory\n"); - free(msg); - return false; - } - *size = SHA256_SUM_LEN; - regtmp.data = msg->data; regtmp.size = msg->data_len; - hash_calculate("sha256", ®tmp, 1, *hash); -#ifdef DEBUG - EFI_PRINT("hash calculated based on contentInfo:\n"); - print_hex_dump(" ", DUMP_PREFIX_OFFSET, 16, 1, - *hash, SHA256_SUM_LEN, false); -#endif - - return true; + return efi_hash_regions(®tmp, 1, hash, size); } /** @@ -169,9 +156,10 @@ static bool efi_signature_verify(struct efi_image_regions *regs, false); #endif /* against contentInfo first */ + hash = NULL; if ((msg->data && efi_hash_msg_content(msg, &hash, &size)) || /* for signed image */ - efi_hash_regions(regs, &hash, &size)) { + efi_hash_regions(regs->reg, regs->num, &hash, &size)) { /* for authenticated variable */ if (ps_info->msgdigest_len != size || memcmp(hash, ps_info->msgdigest, size)) { @@ -239,7 +227,7 @@ bool efi_signature_verify_with_list(struct efi_image_regions *regs, regs, signed_info, siglist, valid_cert); if (!signed_info) { - void *hash; + void *hash = NULL; size_t size; EFI_PRINT("%s: unsigned image\n", __func__); @@ -253,7 +241,7 @@ bool efi_signature_verify_with_list(struct efi_image_regions *regs, goto out; } - if (!efi_hash_regions(regs, &hash, &size)) { + if (!efi_hash_regions(regs->reg, regs->num, &hash, &size)) { EFI_PRINT("Digesting unsigned image failed\n"); goto out; } From 11bafb259648dea054e07dc5c8003eb8c736f36c Mon Sep 17 00:00:00 2001 From: AKASHI Takahiro Date: Wed, 8 Jul 2020 14:01:56 +0900 Subject: [PATCH 06/37] efi_loader: image_loader: verification for all signatures should pass A signed image may have multiple signatures in - each WIN_CERTIFICATE in authenticode, and/or - each SignerInfo in pkcs7 SignedData (of WIN_CERTIFICATE) In the initial implementation of efi_image_authenticate(), the criteria of verification check for multiple signatures case is a bit ambiguous and it may cause inconsistent result. With this patch, we will make sure that verification check in efi_image_authenticate() should pass against all the signatures. The only exception would be - the case where a digest algorithm used in signature is not supported by U-Boot, or - the case where parsing some portion of authenticode has failed In those cases, we don't know how the signature be handled and should just ignore them. Please note that, due to this change, efi_signature_verify_with_sigdb()'s function prototype will be modified, taking "dbx" as well as "db" instead of outputing a "certificate." If "dbx" is null, the behavior would be the exact same as before. The function's name will be changed to efi_signature_verify() once current efi_signature_verify() has gone due to further improvement in intermediate certificates support. Signed-off-by: AKASHI Takahiro --- include/efi_loader.h | 13 +- lib/efi_loader/efi_image_loader.c | 43 ++-- lib/efi_loader/efi_signature.c | 336 +++++++++++++++++------------- 3 files changed, 217 insertions(+), 175 deletions(-) diff --git a/include/efi_loader.h b/include/efi_loader.h index fc9344c742..2f9fb112b3 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -765,14 +765,15 @@ struct efi_signature_store { struct x509_certificate; struct pkcs7_message; -bool efi_signature_verify_cert(struct x509_certificate *cert, - struct efi_signature_store *dbx); -bool efi_signature_verify_signers(struct pkcs7_message *msg, - struct efi_signature_store *dbx); +bool efi_signature_verify_one(struct efi_image_regions *regs, + struct pkcs7_message *msg, + struct efi_signature_store *db); bool efi_signature_verify_with_sigdb(struct efi_image_regions *regs, struct pkcs7_message *msg, - struct efi_signature_store *db, - struct x509_certificate **cert); + struct efi_signature_store *db, + struct efi_signature_store *dbx); +bool efi_signature_check_signers(struct pkcs7_message *msg, + struct efi_signature_store *dbx); efi_status_t efi_image_region_add(struct efi_image_regions *regs, const void *start, const void *end, diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c index de230409e3..058359fc25 100644 --- a/lib/efi_loader/efi_image_loader.c +++ b/lib/efi_loader/efi_image_loader.c @@ -448,13 +448,13 @@ static bool efi_image_unsigned_authenticate(struct efi_image_regions *regs) } /* try black-list first */ - if (efi_signature_verify_with_sigdb(regs, NULL, dbx, NULL)) { + if (efi_signature_verify_one(regs, NULL, dbx)) { EFI_PRINT("Image is not signed and rejected by \"dbx\"\n"); goto out; } /* try white-list */ - if (efi_signature_verify_with_sigdb(regs, NULL, db, NULL)) + if (efi_signature_verify_one(regs, NULL, db)) ret = true; else EFI_PRINT("Image is not signed and not found in \"db\" or \"dbx\"\n"); @@ -494,12 +494,13 @@ static bool efi_image_authenticate(void *efi, size_t efi_size) size_t wincerts_len; struct pkcs7_message *msg = NULL; struct efi_signature_store *db = NULL, *dbx = NULL; - struct x509_certificate *cert = NULL; void *new_efi = NULL; u8 *auth, *wincerts_end; size_t new_efi_size, auth_size; bool ret = false; + debug("%s: Enter, %d\n", __func__, ret); + if (!efi_secure_boot_enabled()) return true; @@ -545,7 +546,17 @@ static bool efi_image_authenticate(void *efi, size_t efi_size) goto err; } - /* go through WIN_CERTIFICATE list */ + /* + * go through WIN_CERTIFICATE list + * NOTE: + * We may have multiple signatures either as WIN_CERTIFICATE's + * in PE header, or as pkcs7 SignerInfo's in SignedData. + * So the verification policy here is: + * - Success if, at least, one of signatures is verified + * - unless + * any of signatures is rejected explicitly, or + * none of digest algorithms are supported + */ for (wincert = wincerts, wincerts_end = (u8 *)wincerts + wincerts_len; (u8 *)wincert < wincerts_end; wincert = (WIN_CERTIFICATE *) @@ -595,42 +606,32 @@ static bool efi_image_authenticate(void *efi, size_t efi_size) } /* try black-list first */ - if (efi_signature_verify_with_sigdb(regs, msg, dbx, NULL)) { + if (efi_signature_verify_one(regs, msg, dbx)) { EFI_PRINT("Signature was rejected by \"dbx\"\n"); goto err; } - if (!efi_signature_verify_signers(msg, dbx)) { - EFI_PRINT("Signer was rejected by \"dbx\"\n"); + if (!efi_signature_check_signers(msg, dbx)) { + EFI_PRINT("Signer(s) in \"dbx\"\n"); goto err; - } else { - ret = true; } /* try white-list */ - if (!efi_signature_verify_with_sigdb(regs, msg, db, &cert)) { - EFI_PRINT("Verifying signature with \"db\" failed\n"); + if (!efi_signature_verify_with_sigdb(regs, msg, db, dbx)) { + EFI_PRINT("Signature was not verified by \"db\"\n"); goto err; - } else { - ret = true; - } - - if (!efi_signature_verify_cert(cert, dbx)) { - EFI_PRINT("Certificate was rejected by \"dbx\"\n"); - goto err; - } else { - ret = true; } } + ret = true; err: - x509_free_certificate(cert); efi_sigstore_free(db); efi_sigstore_free(dbx); pkcs7_free_message(msg); free(regs); free(new_efi); + debug("%s: Exit, %d\n", __func__, ret); return ret; } #else diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c index b14d104094..52392d139a 100644 --- a/lib/efi_loader/efi_signature.c +++ b/lib/efi_loader/efi_signature.c @@ -301,27 +301,110 @@ out: } /** - * efi_signature_verify_with_sigdb - verify a signature with db + * efi_signature_check_revocation - check revocation with dbx + * @sinfo: Signer's info + * @cert: x509 certificate + * @dbx: Revocation signature database + * + * Search revocation signature database pointed to by @dbx and find + * an entry matching to certificate pointed to by @cert. + * + * While this entry contains revocation time, we don't support timestamp + * protocol at this time and any image will be unconditionally revoked + * when this match occurs. + * + * Return: true if check passed, false otherwise. + */ +static bool efi_signature_check_revocation(struct pkcs7_signed_info *sinfo, + struct x509_certificate *cert, + struct efi_signature_store *dbx) +{ + struct efi_signature_store *siglist; + struct efi_sig_data *sig_data; + struct image_region reg[1]; + void *hash = NULL; + size_t size = 0; + time64_t revoc_time; + bool revoked = false; + + EFI_PRINT("%s: Enter, %p, %p, %p\n", __func__, sinfo, cert, dbx); + + if (!sinfo || !cert || !dbx || !dbx->sig_data_list) + goto out; + + 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)) + continue; + + /* calculate hash of TBSCertificate */ + reg[0].data = cert->tbs; + reg[0].size = cert->tbs_size; + if (!efi_hash_regions(reg, 1, &hash, &size)) + goto out; + + for (sig_data = siglist->sig_data_list; sig_data; + sig_data = sig_data->next) { + /* + * struct efi_cert_x509_sha256 { + * u8 tbs_hash[256/8]; + * time64_t revocation_time; + * }; + */ +#ifdef DEBUG + if (sig_data->size >= size) { + EFI_PRINT("hash in db:\n"); + print_hex_dump(" ", DUMP_PREFIX_OFFSET, + 16, 1, + sig_data->data, size, false); + } +#endif + if ((sig_data->size < size + sizeof(time64_t)) || + memcmp(sig_data->data, hash, size)) + continue; + + memcpy(&revoc_time, sig_data->data + size, + sizeof(revoc_time)); + EFI_PRINT("revocation time: 0x%llx\n", revoc_time); + /* + * TODO: compare signing timestamp in sinfo + * with revocation time + */ + + revoked = true; + free(hash); + goto out; + } + free(hash); + hash = NULL; + } +out: + EFI_PRINT("%s: Exit, revoked: %d\n", __func__, revoked); + return !revoked; +} + +/** + * efi_signature_verify_one - verify signatures with database * @regs: List of regions to be authenticated * @msg: Signature - * @db: Signature database for trusted certificates - * @cert: x509 certificate that verifies this signature + * @db: Signature database * - * Signature pointed to by @msg against image pointed to by @regs - * is verified by signature database pointed to by @db. + * All the signature pointed to by @msg against image pointed to by @regs + * will be verified by signature database pointed to by @db. * - * Return: true if signature is verified, false if not + * Return: true if verification for one of signatures passed, false + * otherwise */ -bool efi_signature_verify_with_sigdb(struct efi_image_regions *regs, - struct pkcs7_message *msg, - struct efi_signature_store *db, - struct x509_certificate **cert) +bool efi_signature_verify_one(struct efi_image_regions *regs, + struct pkcs7_message *msg, + struct efi_signature_store *db) { - struct pkcs7_signed_info *info; + struct pkcs7_signed_info *sinfo; struct efi_signature_store *siglist; + struct x509_certificate *cert; bool verified = false; - EFI_PRINT("%s: Enter, %p, %p, %p, %p\n", __func__, regs, msg, db, cert); + EFI_PRINT("%s: Enter, %p, %p, %p\n", __func__, regs, msg, db); if (!db) goto out; @@ -334,11 +417,72 @@ bool efi_signature_verify_with_sigdb(struct efi_image_regions *regs, EFI_PRINT("%s: Verify unsigned image with db\n", __func__); for (siglist = db; siglist; siglist = siglist->next) if (efi_signature_verify_with_list(regs, NULL, NULL, - siglist, cert)) { + siglist, &cert)) { + verified = true; + break; + } + goto out; + } + + /* for signed image or variable */ + EFI_PRINT("%s: Verify signed image with db\n", __func__); + for (sinfo = msg->signed_infos; sinfo; sinfo = sinfo->next) { + EFI_PRINT("Signed Info: digest algo: %s, pkey algo: %s\n", + sinfo->sig->hash_algo, sinfo->sig->pkey_algo); + + for (siglist = db; siglist; siglist = siglist->next) + if (efi_signature_verify_with_list(regs, msg, sinfo, + siglist, &cert)) { verified = true; goto out; } + EFI_PRINT("Valid certificate not in \"db\"\n"); + } +out: + EFI_PRINT("%s: Exit, verified: %d\n", __func__, verified); + return verified; +} + +/** + * efi_signature_verify_with_sigdb - verify signatures with db and dbx + * @regs: List of regions to be authenticated + * @msg: Signature + * @db: Signature database for trusted certificates + * @dbx: Revocation signature database + * + * All the signature pointed to by @msg against image pointed to by @regs + * will be verified by signature database pointed to by @db and @dbx. + * + * Return: true if verification for all signatures passed, false otherwise + */ +bool efi_signature_verify_with_sigdb(struct efi_image_regions *regs, + struct pkcs7_message *msg, + struct efi_signature_store *db, + struct efi_signature_store *dbx) +{ + struct pkcs7_signed_info *info; + struct efi_signature_store *siglist; + struct x509_certificate *cert; + bool verified = false; + + EFI_PRINT("%s: Enter, %p, %p, %p, %p\n", __func__, regs, msg, db, dbx); + + if (!db) + goto out; + + if (!db->sig_data_list) + goto out; + + /* for unsigned image */ + if (!msg) { + EFI_PRINT("%s: Verify unsigned image with db\n", __func__); + for (siglist = db; siglist; siglist = siglist->next) + if (efi_signature_verify_with_list(regs, NULL, NULL, + siglist, &cert)) { + verified = true; + break; + } goto out; } @@ -350,12 +494,21 @@ bool efi_signature_verify_with_sigdb(struct efi_image_regions *regs, for (siglist = db; siglist; siglist = siglist->next) { if (efi_signature_verify_with_list(regs, msg, info, - siglist, cert)) { - verified = true; - goto out; - } + siglist, &cert)) + break; } + if (!siglist) { + EFI_PRINT("Valid certificate not in \"db\"\n"); + goto out; + } + + if (!dbx || efi_signature_check_revocation(info, cert, dbx)) + continue; + + EFI_PRINT("Certificate in \"dbx\"\n"); + goto out; } + verified = true; out: EFI_PRINT("%s: Exit, verified: %d\n", __func__, verified); @@ -363,150 +516,37 @@ out: } /** - * efi_search_siglist - search signature list for a certificate - * @cert: x509 certificate - * @siglist: Signature list - * @revoc_time: Pointer to buffer for revocation time - * - * Search signature list pointed to by @siglist and find a certificate - * pointed to by @cert. - * If found, revocation time that is specified in signature database is - * returned in @revoc_time. - * - * Return: true if certificate is found, false if not - */ -static bool efi_search_siglist(struct x509_certificate *cert, - struct efi_signature_store *siglist, - time64_t *revoc_time) -{ - struct image_region reg[1]; - void *hash = NULL, *msg = NULL; - struct efi_sig_data *sig_data; - bool found = false; - - /* can be null */ - if (!siglist->sig_data_list) - return false; - - if (guidcmp(&siglist->sig_type, &efi_guid_cert_x509_sha256)) { - /* TODO: other hash algos */ - EFI_PRINT("Certificate's digest type is not supported: %pUl\n", - &siglist->sig_type); - goto out; - } - - /* calculate hash of TBSCertificate */ - msg = calloc(1, SHA256_SUM_LEN); - if (!msg) { - EFI_PRINT("Out of memory\n"); - goto out; - } - - hash = calloc(1, SHA256_SUM_LEN); - if (!hash) { - EFI_PRINT("Out of memory\n"); - goto out; - } - - reg[0].data = cert->tbs; - reg[0].size = cert->tbs_size; - hash_calculate("sha256", reg, 1, msg); - - /* go through signature list */ - for (sig_data = siglist->sig_data_list; sig_data; - sig_data = sig_data->next) { - /* - * struct efi_cert_x509_sha256 { - * u8 tbs_hash[256/8]; - * time64_t revocation_time; - * }; - */ - if ((sig_data->size >= SHA256_SUM_LEN + sizeof(time64_t)) && - !memcmp(sig_data->data, msg, SHA256_SUM_LEN)) { - memcpy(revoc_time, sig_data->data + SHA256_SUM_LEN, - sizeof(*revoc_time)); - EFI_PRINT("revocation time: 0x%llx\n", *revoc_time); - found = true; - goto out; - } - } - -out: - free(hash); - free(msg); - - return found; -} - -/** - * efi_signature_verify_cert - verify a certificate with dbx - * @cert: x509 certificate - * @dbx: Signature database - * - * Search signature database pointed to by @dbx and find a certificate - * pointed to by @cert. - * This function is expected to be used against "dbx". - * - * Return: true if a certificate is not rejected, false otherwise. - */ -bool efi_signature_verify_cert(struct x509_certificate *cert, - struct efi_signature_store *dbx) -{ - struct efi_signature_store *siglist; - time64_t revoc_time; - bool found = false; - - EFI_PRINT("%s: Enter, %p, %p\n", __func__, dbx, cert); - - if (!cert) - return false; - - for (siglist = dbx; siglist; siglist = siglist->next) { - if (efi_search_siglist(cert, siglist, &revoc_time)) { - /* TODO */ - /* compare signing time with revocation time */ - - found = true; - break; - } - } - - EFI_PRINT("%s: Exit, verified: %d\n", __func__, !found); - return !found; -} - -/** - * efi_signature_verify_signers - verify signers' certificates with dbx + * efi_signature_check_signers - check revocation against all signers with dbx * @msg: Signature - * @dbx: Signature database + * @dbx: Revocation signature database * - * Determine if any of signers' certificates in @msg may be verified - * by any of certificates in signature database pointed to by @dbx. - * This function is expected to be used against "dbx". + * Determine if none of signers' certificates in @msg are revoked + * by signature database pointed to by @dbx. * - * Return: true if none of certificates is rejected, false otherwise. + * Return: true if all signers passed, false otherwise. */ -bool efi_signature_verify_signers(struct pkcs7_message *msg, - struct efi_signature_store *dbx) +bool efi_signature_check_signers(struct pkcs7_message *msg, + struct efi_signature_store *dbx) { - struct pkcs7_signed_info *info; - bool found = false; + struct pkcs7_signed_info *sinfo; + bool revoked = false; EFI_PRINT("%s: Enter, %p, %p\n", __func__, msg, dbx); - if (!msg) + if (!msg || !dbx) goto out; - for (info = msg->signed_infos; info; info = info->next) { - if (info->signer && - !efi_signature_verify_cert(info->signer, dbx)) { - found = true; - goto out; + for (sinfo = msg->signed_infos; sinfo; sinfo = sinfo->next) { + if (sinfo->signer && + !efi_signature_check_revocation(sinfo, sinfo->signer, + dbx)) { + revoked = true; + break; } } out: - EFI_PRINT("%s: Exit, verified: %d\n", __func__, !found); - return !found; + EFI_PRINT("%s: Exit, revoked: %d\n", __func__, revoked); + return !revoked; } /** From 7926dfb579cb17efc62ede2ce6d5c0a6f7e2f855 Mon Sep 17 00:00:00 2001 From: AKASHI Takahiro Date: Wed, 8 Jul 2020 14:01:57 +0900 Subject: [PATCH 07/37] efi_loader: image_loader: add digest-based verification for signed image In case that a type of certificate in "db" or "dbx" is EFI_CERT_X509_SHA256_GUID, it is actually not a certificate which contains a public key for RSA decryption, but a digest of image to be loaded. If the value matches to a value calculated from a given binary image, it is granted for loading. With this patch, common digest check code, which used to be used for unsigned image verification, will be extracted from efi_signature_verify_with_sigdb() into efi_signature_lookup_digest(), and extra step for digest check will be added to efi_image_authenticate(). Signed-off-by: AKASHI Takahiro --- include/efi_loader.h | 2 + lib/efi_loader/efi_image_loader.c | 44 ++++++++-- lib/efi_loader/efi_signature.c | 134 ++++++++++++++---------------- 3 files changed, 102 insertions(+), 78 deletions(-) diff --git a/include/efi_loader.h b/include/efi_loader.h index 2f9fb112b3..ceabbaadd0 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -765,6 +765,8 @@ struct efi_signature_store { struct x509_certificate; struct pkcs7_message; +bool efi_signature_lookup_digest(struct efi_image_regions *regs, + struct efi_signature_store *db); bool efi_signature_verify_one(struct efi_image_regions *regs, struct pkcs7_message *msg, struct efi_signature_store *db); diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c index 058359fc25..b7cf26046e 100644 --- a/lib/efi_loader/efi_image_loader.c +++ b/lib/efi_loader/efi_image_loader.c @@ -448,16 +448,16 @@ static bool efi_image_unsigned_authenticate(struct efi_image_regions *regs) } /* try black-list first */ - if (efi_signature_verify_one(regs, NULL, dbx)) { - EFI_PRINT("Image is not signed and rejected by \"dbx\"\n"); + if (efi_signature_lookup_digest(regs, dbx)) { + EFI_PRINT("Image is not signed and its digest found in \"dbx\"\n"); goto out; } /* try white-list */ - if (efi_signature_verify_one(regs, NULL, db)) + if (efi_signature_lookup_digest(regs, db)) ret = true; else - EFI_PRINT("Image is not signed and not found in \"db\" or \"dbx\"\n"); + EFI_PRINT("Image is not signed and its digest not found in \"db\" or \"dbx\"\n"); out: efi_sigstore_free(db); @@ -605,6 +605,25 @@ static bool efi_image_authenticate(void *efi, size_t efi_size) continue; } + /* + * NOTE: + * UEFI specification defines two signature types possible + * in signature database: + * a. x509 certificate, where a signature in image is + * a message digest encrypted by RSA public key + * (EFI_CERT_X509_GUID) + * b. bare hash value of message digest + * (EFI_CERT_SHAxxx_GUID) + * + * efi_signature_verify() handles case (a), while + * efi_signature_lookup_digest() handles case (b). + * + * There is a third type: + * c. message digest of a certificate + * (EFI_CERT_X509_SHAAxxx_GUID) + * This type of signature is used only in revocation list + * (dbx) and handled as part of efi_signatgure_verify(). + */ /* try black-list first */ if (efi_signature_verify_one(regs, msg, dbx)) { EFI_PRINT("Signature was rejected by \"dbx\"\n"); @@ -616,11 +635,22 @@ static bool efi_image_authenticate(void *efi, size_t efi_size) goto err; } - /* try white-list */ - if (!efi_signature_verify_with_sigdb(regs, msg, db, dbx)) { - EFI_PRINT("Signature was not verified by \"db\"\n"); + if (efi_signature_lookup_digest(regs, dbx)) { + EFI_PRINT("Image's digest was found in \"dbx\"\n"); goto err; } + + /* try white-list */ + if (efi_signature_verify_with_sigdb(regs, msg, db, dbx)) + continue; + + debug("Signature was not verified by \"db\"\n"); + + if (efi_signature_lookup_digest(regs, db)) + continue; + + debug("Image's digest was not found in \"db\" or \"dbx\"\n"); + goto err; } ret = true; diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c index 52392d139a..fc0314e6d4 100644 --- a/lib/efi_loader/efi_signature.c +++ b/lib/efi_loader/efi_signature.c @@ -197,6 +197,68 @@ out: return verified; } +/** + * efi_signature_lookup_digest - search for an image's digest in sigdb + * @regs: List of regions to be authenticated + * @db: Signature database for trusted certificates + * + * A message digest of image pointed to by @regs is calculated and + * its hash value is compared to entries in signature database pointed + * to by @db. + * + * Return: true if found, false if not + */ +bool efi_signature_lookup_digest(struct efi_image_regions *regs, + struct efi_signature_store *db) +{ + struct efi_signature_store *siglist; + struct efi_sig_data *sig_data; + void *hash = NULL; + size_t size = 0; + bool found = false; + + EFI_PRINT("%s: Enter, %p, %p\n", __func__, regs, db); + + if (!regs || !db || !db->sig_data_list) + goto out; + + for (siglist = db; siglist; siglist = siglist->next) { + /* TODO: support other hash algorithms */ + if (guidcmp(&siglist->sig_type, &efi_guid_sha256)) { + EFI_PRINT("Digest algorithm is not supported: %pUl\n", + &siglist->sig_type); + break; + } + + if (!efi_hash_regions(regs->reg, regs->num, &hash, &size)) { + EFI_PRINT("Digesting an image failed\n"); + break; + } + + for (sig_data = siglist->sig_data_list; sig_data; + sig_data = sig_data->next) { +#ifdef DEBUG + EFI_PRINT("Msg digest in database:\n"); + 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)) { + found = true; + free(hash); + goto out; + } + } + + free(hash); + hash = NULL; + } + +out: + EFI_PRINT("%s: Exit, found: %d\n", __func__, found); + return found; +} + /** * efi_signature_verify_with_list - verify a signature with signature list * @regs: List of regions to be authenticated @@ -226,46 +288,6 @@ bool efi_signature_verify_with_list(struct efi_image_regions *regs, EFI_PRINT("%s: Enter, %p, %p, %p, %p\n", __func__, regs, signed_info, siglist, valid_cert); - if (!signed_info) { - void *hash = NULL; - size_t size; - - EFI_PRINT("%s: unsigned image\n", __func__); - /* - * verify based on calculated hash value - * TODO: support other hash algorithms - */ - if (guidcmp(&siglist->sig_type, &efi_guid_sha256)) { - EFI_PRINT("Digest algorithm is not supported: %pUl\n", - &siglist->sig_type); - goto out; - } - - if (!efi_hash_regions(regs->reg, regs->num, &hash, &size)) { - EFI_PRINT("Digesting unsigned image failed\n"); - goto out; - } - - /* go through the list */ - for (sig_data = siglist->sig_data_list; sig_data; - sig_data = sig_data->next) { -#ifdef DEBUG - EFI_PRINT("Msg digest in database:\n"); - 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)) { - verified = true; - free(hash); - goto out; - } - } - free(hash); - goto out; - } - - EFI_PRINT("%s: signed image\n", __func__); if (guidcmp(&siglist->sig_type, &efi_guid_cert_x509)) { EFI_PRINT("Signature type is not supported: %pUl\n", &siglist->sig_type); @@ -412,19 +434,6 @@ bool efi_signature_verify_one(struct efi_image_regions *regs, if (!db->sig_data_list) goto out; - /* for unsigned image */ - if (!msg) { - EFI_PRINT("%s: Verify unsigned image with db\n", __func__); - for (siglist = db; siglist; siglist = siglist->next) - if (efi_signature_verify_with_list(regs, NULL, NULL, - siglist, &cert)) { - verified = true; - break; - } - goto out; - } - - /* for signed image or variable */ EFI_PRINT("%s: Verify signed image with db\n", __func__); for (sinfo = msg->signed_infos; sinfo; sinfo = sinfo->next) { EFI_PRINT("Signed Info: digest algo: %s, pkey algo: %s\n", @@ -468,26 +477,9 @@ bool efi_signature_verify_with_sigdb(struct efi_image_regions *regs, EFI_PRINT("%s: Enter, %p, %p, %p, %p\n", __func__, regs, msg, db, dbx); - if (!db) + if (!regs || !msg || !db || !db->sig_data_list) goto out; - if (!db->sig_data_list) - goto out; - - /* for unsigned image */ - if (!msg) { - EFI_PRINT("%s: Verify unsigned image with db\n", __func__); - for (siglist = db; siglist; siglist = siglist->next) - if (efi_signature_verify_with_list(regs, NULL, NULL, - siglist, &cert)) { - verified = true; - break; - } - goto out; - } - - /* for signed image or variable */ - EFI_PRINT("%s: Verify signed image with db\n", __func__); for (info = msg->signed_infos; info; info = info->next) { EFI_PRINT("Signed Info: digest algo: %s, pkey algo: %s\n", info->sig->hash_algo, info->sig->pkey_algo); From 91d2b6216b643f4c932b594d186b31d49a53deeb Mon Sep 17 00:00:00 2001 From: AKASHI Takahiro Date: Wed, 8 Jul 2020 14:01:58 +0900 Subject: [PATCH 08/37] test/py: efi_secboot: apply autopep8 Python's autopep8 can automatically correct some of warnings from pylint and rewrite the code in a pretty print format. So just do it. Suggested-by: Heinrich Schuchardt Signed-off-by: AKASHI Takahiro --- test/py/tests/test_efi_secboot/conftest.py | 74 ++++++++++--------- test/py/tests/test_efi_secboot/defs.py | 14 ++-- .../py/tests/test_efi_secboot/test_authvar.py | 1 + test/py/tests/test_efi_secboot/test_signed.py | 1 + .../tests/test_efi_secboot/test_unsigned.py | 37 +++++----- 5 files changed, 67 insertions(+), 60 deletions(-) diff --git a/test/py/tests/test_efi_secboot/conftest.py b/test/py/tests/test_efi_secboot/conftest.py index ac5a780fdb..82bc8886c4 100644 --- a/test/py/tests/test_efi_secboot/conftest.py +++ b/test/py/tests/test_efi_secboot/conftest.py @@ -10,6 +10,8 @@ from subprocess import call, check_call, check_output, CalledProcessError from defs import * # from test/py/conftest.py + + def tool_is_in_path(tool): for path in os.environ["PATH"].split(os.pathsep): fn = os.path.join(path, tool) @@ -20,13 +22,15 @@ def tool_is_in_path(tool): # # Fixture for UEFI secure boot test # + + @pytest.fixture(scope='session') def efi_boot_env(request, u_boot_config): """Set up a file system to be used in UEFI secure boot test. Args: request: Pytest request object. - u_boot_config: U-boot configuration. + u_boot_config: U-boot configuration. Return: A path to disk image to be used for testing @@ -48,20 +52,20 @@ def efi_boot_env(request, u_boot_config): # create a disk/partition check_call('dd if=/dev/zero of=%s bs=1MiB count=%d' - % (image_path, image_size), shell=True) + % (image_path, image_size), shell=True) check_call('sgdisk %s -n 1:0:+%dMiB' - % (image_path, part_size), shell=True) + % (image_path, part_size), shell=True) # create a file system check_call('dd if=/dev/zero of=%s.tmp bs=1MiB count=%d' - % (image_path, part_size), shell=True) + % (image_path, part_size), shell=True) check_call('mkfs -t %s %s.tmp' % (fs_type, image_path), shell=True) check_call('dd if=%s.tmp of=%s bs=1MiB seek=1 count=%d conv=notrunc' - % (image_path, image_path, 1), shell=True) + % (image_path, image_path, 1), shell=True) check_call('rm %s.tmp' % image_path, shell=True) loop_dev = check_output('sudo losetup -o 1MiB --sizelimit %dMiB --show -f %s | tr -d "\n"' % (part_size, image_path), shell=True).decode() check_output('sudo mount -t %s -o umask=000 %s %s' - % (fs_type, loop_dev, mnt_point), shell=True) + % (fs_type, loop_dev, mnt_point), shell=True) # suffix # *.key: RSA private key in PEM @@ -73,53 +77,53 @@ def efi_boot_env(request, u_boot_config): # *.efi.signed: signed UEFI image # Create signature database - ## PK + # PK check_call('cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_PK/ -keyout PK.key -out PK.crt -nodes -days 365' - % mnt_point, shell=True) + % mnt_point, shell=True) check_call('cd %s; %scert-to-efi-sig-list -g %s PK.crt PK.esl; %ssign-efi-sig-list -t "2020-04-01" -c PK.crt -k PK.key PK PK.esl PK.auth' - % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH), - shell=True) - ## PK_null for deletion + % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH), + shell=True) + # PK_null for deletion check_call('cd %s; touch PK_null.esl; %ssign-efi-sig-list -t "2020-04-02" -c PK.crt -k PK.key PK PK_null.esl PK_null.auth' - % (mnt_point, EFITOOLS_PATH), shell=True) - ## KEK + % (mnt_point, EFITOOLS_PATH), shell=True) + # KEK check_call('cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_KEK/ -keyout KEK.key -out KEK.crt -nodes -days 365' - % mnt_point, shell=True) + % mnt_point, shell=True) check_call('cd %s; %scert-to-efi-sig-list -g %s KEK.crt KEK.esl; %ssign-efi-sig-list -t "2020-04-03" -c PK.crt -k PK.key KEK KEK.esl KEK.auth' - % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH), - shell=True) - ## db + % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH), + shell=True) + # db check_call('cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_db/ -keyout db.key -out db.crt -nodes -days 365' - % mnt_point, shell=True) + % mnt_point, shell=True) check_call('cd %s; %scert-to-efi-sig-list -g %s db.crt db.esl; %ssign-efi-sig-list -t "2020-04-04" -c KEK.crt -k KEK.key db db.esl db.auth' - % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH), - shell=True) - ## db1 + % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH), + shell=True) + # db1 check_call('cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_db1/ -keyout db1.key -out db1.crt -nodes -days 365' - % mnt_point, shell=True) + % mnt_point, shell=True) check_call('cd %s; %scert-to-efi-sig-list -g %s db1.crt db1.esl; %ssign-efi-sig-list -t "2020-04-05" -c KEK.crt -k KEK.key db db1.esl db1.auth' - % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH), - shell=True) - ## db1-update + % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH), + shell=True) + # db1-update check_call('cd %s; %ssign-efi-sig-list -t "2020-04-06" -a -c KEK.crt -k KEK.key db db1.esl db1-update.auth' - % (mnt_point, EFITOOLS_PATH), shell=True) - ## dbx + % (mnt_point, EFITOOLS_PATH), shell=True) + # dbx check_call('cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_dbx/ -keyout dbx.key -out dbx.crt -nodes -days 365' - % mnt_point, shell=True) + % mnt_point, shell=True) check_call('cd %s; %scert-to-efi-sig-list -g %s dbx.crt dbx.esl; %ssign-efi-sig-list -t "2020-04-05" -c KEK.crt -k KEK.key dbx dbx.esl dbx.auth' - % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH), - shell=True) + % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH), + shell=True) # Copy image check_call('cp %s %s' % (HELLO_PATH, mnt_point), shell=True) - ## Sign image + # Sign image check_call('cd %s; sbsign --key db.key --cert db.crt helloworld.efi' - % mnt_point, shell=True) - ## Digest image + % mnt_point, shell=True) + # Digest image check_call('cd %s; %shash-to-efi-sig-list helloworld.efi db_hello.hash; %ssign-efi-sig-list -t "2020-04-07" -c KEK.crt -k KEK.key db db_hello.hash db_hello.auth' - % (mnt_point, EFITOOLS_PATH, EFITOOLS_PATH), - shell=True) + % (mnt_point, EFITOOLS_PATH, EFITOOLS_PATH), + shell=True) check_call('sudo umount %s' % loop_dev, shell=True) check_call('sudo losetup -d %s' % loop_dev, shell=True) diff --git a/test/py/tests/test_efi_secboot/defs.py b/test/py/tests/test_efi_secboot/defs.py index d6222809c5..099f453979 100644 --- a/test/py/tests/test_efi_secboot/defs.py +++ b/test/py/tests/test_efi_secboot/defs.py @@ -1,21 +1,21 @@ # SPDX-License-Identifier: GPL-2.0+ # Disk image name -EFI_SECBOOT_IMAGE_NAME='test_efi_secboot.img' +EFI_SECBOOT_IMAGE_NAME = 'test_efi_secboot.img' # Size in MiB -EFI_SECBOOT_IMAGE_SIZE=16 -EFI_SECBOOT_PART_SIZE=8 +EFI_SECBOOT_IMAGE_SIZE = 16 +EFI_SECBOOT_PART_SIZE = 8 # Partition file system type -EFI_SECBOOT_FS_TYPE='vfat' +EFI_SECBOOT_FS_TYPE = 'vfat' # Owner guid -GUID='11111111-2222-3333-4444-123456789abc' +GUID = '11111111-2222-3333-4444-123456789abc' # v1.5.1 or earlier of efitools has a bug in sha256 calculation, and # you need build a newer version on your own. -EFITOOLS_PATH='' +EFITOOLS_PATH = '' # Hello World application for sandbox -HELLO_PATH='' +HELLO_PATH = '' diff --git a/test/py/tests/test_efi_secboot/test_authvar.py b/test/py/tests/test_efi_secboot/test_authvar.py index 148aa3123e..359adba4b4 100644 --- a/test/py/tests/test_efi_secboot/test_authvar.py +++ b/test/py/tests/test_efi_secboot/test_authvar.py @@ -11,6 +11,7 @@ This test verifies variable authentication import pytest from defs import * + @pytest.mark.boardspec('sandbox') @pytest.mark.buildconfigspec('efi_secure_boot') @pytest.mark.buildconfigspec('cmd_fat') diff --git a/test/py/tests/test_efi_secboot/test_signed.py b/test/py/tests/test_efi_secboot/test_signed.py index 19d78b1b64..6d4b03ef41 100644 --- a/test/py/tests/test_efi_secboot/test_signed.py +++ b/test/py/tests/test_efi_secboot/test_signed.py @@ -11,6 +11,7 @@ This test verifies image authentication for signed images. import pytest from defs import * + @pytest.mark.boardspec('sandbox') @pytest.mark.buildconfigspec('efi_secure_boot') @pytest.mark.buildconfigspec('cmd_efidebug') diff --git a/test/py/tests/test_efi_secboot/test_unsigned.py b/test/py/tests/test_efi_secboot/test_unsigned.py index c42c5ddc47..3748b51ee7 100644 --- a/test/py/tests/test_efi_secboot/test_unsigned.py +++ b/test/py/tests/test_efi_secboot/test_unsigned.py @@ -11,6 +11,7 @@ This test verifies image authentication for unsigned images. import pytest from defs import * + @pytest.mark.boardspec('sandbox') @pytest.mark.buildconfigspec('efi_secure_boot') @pytest.mark.buildconfigspec('cmd_efidebug') @@ -28,10 +29,10 @@ class TestEfiUnsignedImage(object): # Test Case 1 output = u_boot_console.run_command_list([ 'host bind 0 %s' % disk_img, - 'fatload host 0:1 4000000 KEK.auth', - 'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK', - 'fatload host 0:1 4000000 PK.auth', - 'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK']) + 'fatload host 0:1 4000000 KEK.auth', + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK', + 'fatload host 0:1 4000000 PK.auth', + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK']) assert(not 'Failed to set EFI variable' in ''.join(output)) output = u_boot_console.run_command_list([ @@ -55,12 +56,12 @@ class TestEfiUnsignedImage(object): # Test Case 2 output = u_boot_console.run_command_list([ 'host bind 0 %s' % disk_img, - 'fatload host 0:1 4000000 db_hello.auth', - 'setenv -e -nv -bs -rt -at -i 4000000,$filesize db', - 'fatload host 0:1 4000000 KEK.auth', - 'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK', - 'fatload host 0:1 4000000 PK.auth', - 'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK']) + 'fatload host 0:1 4000000 db_hello.auth', + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize db', + 'fatload host 0:1 4000000 KEK.auth', + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK', + 'fatload host 0:1 4000000 PK.auth', + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK']) assert(not 'Failed to set EFI variable' in ''.join(output)) output = u_boot_console.run_command_list([ @@ -79,12 +80,12 @@ class TestEfiUnsignedImage(object): # Test Case 3a, rejected by dbx output = u_boot_console.run_command_list([ 'host bind 0 %s' % disk_img, - 'fatload host 0:1 4000000 db_hello.auth', - 'setenv -e -nv -bs -rt -at -i 4000000,$filesize dbx', - 'fatload host 0:1 4000000 KEK.auth', - 'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK', - 'fatload host 0:1 4000000 PK.auth', - 'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK']) + 'fatload host 0:1 4000000 db_hello.auth', + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize dbx', + 'fatload host 0:1 4000000 KEK.auth', + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK', + 'fatload host 0:1 4000000 PK.auth', + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK']) assert(not 'Failed to set EFI variable' in ''.join(output)) output = u_boot_console.run_command_list([ @@ -101,8 +102,8 @@ class TestEfiUnsignedImage(object): with u_boot_console.log.section('Test Case 3b'): # Test Case 3b, rejected by dbx even if db allows output = u_boot_console.run_command_list([ - 'fatload host 0:1 4000000 db_hello.auth', - 'setenv -e -nv -bs -rt -at -i 4000000,$filesize db']) + 'fatload host 0:1 4000000 db_hello.auth', + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize db']) assert(not 'Failed to set EFI variable' in ''.join(output)) output = u_boot_console.run_command_list([ From d863b307e8079867575335f584db0949467cc220 Mon Sep 17 00:00:00 2001 From: AKASHI Takahiro Date: Wed, 8 Jul 2020 14:01:59 +0900 Subject: [PATCH 09/37] test/py: efi_secboot: more fixes against pylint More fixes against pylint warnings that autopep8 didn't handle in the previous commit. Signed-off-by: AKASHI Takahiro --- test/py/tests/test_efi_secboot/conftest.py | 11 +-- .../py/tests/test_efi_secboot/test_authvar.py | 91 +++++++++---------- test/py/tests/test_efi_secboot/test_signed.py | 31 +++---- .../tests/test_efi_secboot/test_unsigned.py | 29 +++--- 4 files changed, 79 insertions(+), 83 deletions(-) diff --git a/test/py/tests/test_efi_secboot/conftest.py b/test/py/tests/test_efi_secboot/conftest.py index 82bc8886c4..c0ee8be9f7 100644 --- a/test/py/tests/test_efi_secboot/conftest.py +++ b/test/py/tests/test_efi_secboot/conftest.py @@ -4,9 +4,8 @@ import os import os.path -import pytest -import re from subprocess import call, check_call, check_output, CalledProcessError +import pytest from defs import * # from test/py/conftest.py @@ -14,8 +13,8 @@ from defs import * def tool_is_in_path(tool): for path in os.environ["PATH"].split(os.pathsep): - fn = os.path.join(path, tool) - if os.path.isfile(fn) and os.access(fn, os.X_OK): + full_path = os.path.join(path, tool) + if os.path.isfile(full_path) and os.access(full_path, os.X_OK): return True return False @@ -128,8 +127,8 @@ def efi_boot_env(request, u_boot_config): check_call('sudo umount %s' % loop_dev, shell=True) check_call('sudo losetup -d %s' % loop_dev, shell=True) - except CalledProcessError as e: - pytest.skip('Setup failed: %s' % e.cmd) + except CalledProcessError as exception: + pytest.skip('Setup failed: %s' % exception.cmd) return else: yield image_path diff --git a/test/py/tests/test_efi_secboot/test_authvar.py b/test/py/tests/test_efi_secboot/test_authvar.py index 359adba4b4..d0c6b9035b 100644 --- a/test/py/tests/test_efi_secboot/test_authvar.py +++ b/test/py/tests/test_efi_secboot/test_authvar.py @@ -9,7 +9,6 @@ This test verifies variable authentication """ import pytest -from defs import * @pytest.mark.boardspec('sandbox') @@ -29,18 +28,18 @@ class TestEfiAuthVar(object): output = u_boot_console.run_command_list([ 'host bind 0 %s' % disk_img, 'printenv -e SecureBoot']) - assert('00000000: 00' in ''.join(output)) + assert '00000000: 00' in ''.join(output) output = u_boot_console.run_command( 'printenv -e SetupMode') - assert('00000000: 01' in output) + assert '00000000: 01' in output with u_boot_console.log.section('Test Case 1b'): # Test Case 1b, PK without AUTHENTICATED_WRITE_ACCESS output = u_boot_console.run_command_list([ 'fatload host 0:1 4000000 PK.auth', 'setenv -e -nv -bs -rt -i 4000000,$filesize PK']) - assert('Failed to set EFI variable' in ''.join(output)) + assert 'Failed to set EFI variable' in ''.join(output) with u_boot_console.log.section('Test Case 1c'): # Test Case 1c, install PK @@ -48,79 +47,79 @@ class TestEfiAuthVar(object): 'fatload host 0:1 4000000 PK.auth', 'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK', 'printenv -e -n PK']) - assert('PK:' in ''.join(output)) + assert 'PK:' in ''.join(output) output = u_boot_console.run_command( 'printenv -e SecureBoot') - assert('00000000: 01' in output) + assert '00000000: 01' in output output = u_boot_console.run_command( 'printenv -e SetupMode') - assert('00000000: 00' in output) + assert '00000000: 00' in output with u_boot_console.log.section('Test Case 1d'): # Test Case 1d, db/dbx without KEK output = u_boot_console.run_command_list([ 'fatload host 0:1 4000000 db.auth', 'setenv -e -nv -bs -rt -at -i 4000000,$filesize db']) - assert('Failed to set EFI variable' in ''.join(output)) + assert 'Failed to set EFI variable' in ''.join(output) output = u_boot_console.run_command_list([ 'fatload host 0:1 4000000 db.auth', 'setenv -e -nv -bs -rt -at -i 4000000,$filesize dbx']) - assert('Failed to set EFI variable' in ''.join(output)) + assert 'Failed to set EFI variable' in ''.join(output) with u_boot_console.log.section('Test Case 1e'): # Test Case 1e, install KEK output = u_boot_console.run_command_list([ 'fatload host 0:1 4000000 KEK.auth', 'setenv -e -nv -bs -rt -i 4000000,$filesize KEK']) - assert('Failed to set EFI variable' in ''.join(output)) + assert 'Failed to set EFI variable' in ''.join(output) output = u_boot_console.run_command_list([ 'fatload host 0:1 4000000 KEK.auth', 'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK', 'printenv -e -n KEK']) - assert('KEK:' in ''.join(output)) + assert 'KEK:' in ''.join(output) output = u_boot_console.run_command( 'printenv -e SecureBoot') - assert('00000000: 01' in output) + assert '00000000: 01' in output with u_boot_console.log.section('Test Case 1f'): # Test Case 1f, install db output = u_boot_console.run_command_list([ 'fatload host 0:1 4000000 db.auth', 'setenv -e -nv -bs -rt -i 4000000,$filesize db']) - assert('Failed to set EFI variable' in ''.join(output)) + assert 'Failed to set EFI variable' in ''.join(output) output = u_boot_console.run_command_list([ 'fatload host 0:1 4000000 db.auth', 'setenv -e -nv -bs -rt -at -i 4000000,$filesize db', 'printenv -e -n -guid d719b2cb-3d3a-4596-a3bc-dad00e67656f db']) - assert(not 'Failed to set EFI variable' in ''.join(output)) - assert('db:' in ''.join(output)) + assert 'Failed to set EFI variable' not in ''.join(output) + assert 'db:' in ''.join(output) output = u_boot_console.run_command( 'printenv -e SecureBoot') - assert('00000000: 01' in output) + assert '00000000: 01' in output with u_boot_console.log.section('Test Case 1g'): # Test Case 1g, install dbx output = u_boot_console.run_command_list([ 'fatload host 0:1 4000000 dbx.auth', 'setenv -e -nv -bs -rt -i 4000000,$filesize dbx']) - assert('Failed to set EFI variable' in ''.join(output)) + assert 'Failed to set EFI variable' in ''.join(output) output = u_boot_console.run_command_list([ 'fatload host 0:1 4000000 dbx.auth', 'setenv -e -nv -bs -rt -at -i 4000000,$filesize dbx', 'printenv -e -n -guid d719b2cb-3d3a-4596-a3bc-dad00e67656f dbx']) - assert(not 'Failed to set EFI variable' in ''.join(output)) - assert('dbx:' in ''.join(output)) + assert 'Failed to set EFI variable' not in ''.join(output) + assert 'dbx:' in ''.join(output) output = u_boot_console.run_command( 'printenv -e SecureBoot') - assert('00000000: 01' in output) + assert '00000000: 01' in output def test_efi_var_auth2(self, u_boot_console, efi_boot_env): """ @@ -139,20 +138,20 @@ class TestEfiAuthVar(object): 'fatload host 0:1 4000000 db.auth', 'setenv -e -nv -bs -rt -at -i 4000000,$filesize db', 'printenv -e -n -guid d719b2cb-3d3a-4596-a3bc-dad00e67656f db']) - assert(not 'Failed to set EFI variable' in ''.join(output)) - assert('db:' in ''.join(output)) + assert 'Failed to set EFI variable' not in ''.join(output) + assert 'db:' in ''.join(output) output = u_boot_console.run_command_list([ 'fatload host 0:1 4000000 db1.auth', 'setenv -e -nv -bs -rt -i 4000000,$filesize db']) - assert('Failed to set EFI variable' in ''.join(output)) + assert 'Failed to set EFI variable' in ''.join(output) with u_boot_console.log.section('Test Case 2b'): # Test Case 2b, update without correct signature output = u_boot_console.run_command_list([ 'fatload host 0:1 4000000 db.esl', 'setenv -e -nv -bs -rt -at -i 4000000,$filesize db']) - assert('Failed to set EFI variable' in ''.join(output)) + assert 'Failed to set EFI variable' in ''.join(output) with u_boot_console.log.section('Test Case 2c'): # Test Case 2c, update with correct signature @@ -160,8 +159,8 @@ class TestEfiAuthVar(object): 'fatload host 0:1 4000000 db1.auth', 'setenv -e -nv -bs -rt -at -i 4000000,$filesize db', 'printenv -e -n -guid d719b2cb-3d3a-4596-a3bc-dad00e67656f db']) - assert(not 'Failed to set EFI variable' in ''.join(output)) - assert('db:' in ''.join(output)) + assert 'Failed to set EFI variable' not in ''.join(output) + assert 'db:' in ''.join(output) def test_efi_var_auth3(self, u_boot_console, efi_boot_env): """ @@ -180,20 +179,20 @@ class TestEfiAuthVar(object): 'fatload host 0:1 4000000 db.auth', 'setenv -e -nv -bs -rt -at -i 4000000,$filesize db', 'printenv -e -n -guid d719b2cb-3d3a-4596-a3bc-dad00e67656f db']) - assert(not 'Failed to set EFI variable' in ''.join(output)) - assert('db:' in ''.join(output)) + assert 'Failed to set EFI variable' not in ''.join(output) + assert 'db:' in ''.join(output) output = u_boot_console.run_command_list([ 'fatload host 0:1 4000000 db1.auth', 'setenv -e -nv -bs -rt -a -i 4000000,$filesize db']) - assert('Failed to set EFI variable' in ''.join(output)) + assert 'Failed to set EFI variable' in ''.join(output) with u_boot_console.log.section('Test Case 3b'): # Test Case 3b, update without correct signature output = u_boot_console.run_command_list([ 'fatload host 0:1 4000000 db.esl', 'setenv -e -nv -bs -rt -at -a -i 4000000,$filesize db']) - assert('Failed to set EFI variable' in ''.join(output)) + assert 'Failed to set EFI variable' in ''.join(output) with u_boot_console.log.section('Test Case 3c'): # Test Case 3c, update with correct signature @@ -201,8 +200,8 @@ class TestEfiAuthVar(object): 'fatload host 0:1 4000000 db1.auth', 'setenv -e -nv -bs -rt -at -a -i 4000000,$filesize db', 'printenv -e -n -guid d719b2cb-3d3a-4596-a3bc-dad00e67656f db']) - assert(not 'Failed to set EFI variable' in ''.join(output)) - assert('db:' in ''.join(output)) + assert 'Failed to set EFI variable' not in ''.join(output) + assert 'db:' in ''.join(output) def test_efi_var_auth4(self, u_boot_console, efi_boot_env): """ @@ -221,22 +220,22 @@ class TestEfiAuthVar(object): 'fatload host 0:1 4000000 db.auth', 'setenv -e -nv -bs -rt -at -i 4000000,$filesize db', 'printenv -e -n -guid d719b2cb-3d3a-4596-a3bc-dad00e67656f db']) - assert(not 'Failed to set EFI variable' in ''.join(output)) - assert('db:' in ''.join(output)) + assert 'Failed to set EFI variable' not in ''.join(output) + assert 'db:' in ''.join(output) output = u_boot_console.run_command_list([ 'setenv -e -nv -bs -rt db', 'printenv -e -n -guid d719b2cb-3d3a-4596-a3bc-dad00e67656f db']) - assert('Failed to set EFI variable' in ''.join(output)) - assert('db:' in ''.join(output)) + assert 'Failed to set EFI variable' in ''.join(output) + assert 'db:' in ''.join(output) with u_boot_console.log.section('Test Case 4b'): # Test Case 4b, update without correct signature/data output = u_boot_console.run_command_list([ 'setenv -e -nv -bs -rt -at db', 'printenv -e -n -guid d719b2cb-3d3a-4596-a3bc-dad00e67656f db']) - assert('Failed to set EFI variable' in ''.join(output)) - assert('db:' in ''.join(output)) + assert 'Failed to set EFI variable' in ''.join(output) + assert 'db:' in ''.join(output) def test_efi_var_auth5(self, u_boot_console, efi_boot_env): """ @@ -255,15 +254,15 @@ class TestEfiAuthVar(object): 'fatload host 0:1 4000000 db.auth', 'setenv -e -nv -bs -rt -at -i 4000000,$filesize db', 'printenv -e -n PK']) - assert(not 'Failed to set EFI variable' in ''.join(output)) - assert('PK:' in ''.join(output)) + assert 'Failed to set EFI variable' not in ''.join(output) + assert 'PK:' in ''.join(output) output = u_boot_console.run_command_list([ 'fatload host 0:1 4000000 PK_null.esl', 'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK', 'printenv -e -n PK']) - assert('Failed to set EFI variable' in ''.join(output)) - assert('PK:' in ''.join(output)) + assert 'Failed to set EFI variable' in ''.join(output) + assert 'PK:' in ''.join(output) with u_boot_console.log.section('Test Case 5b'): # Test Case 5b, Uninstall PK with correct signature @@ -271,12 +270,12 @@ class TestEfiAuthVar(object): 'fatload host 0:1 4000000 PK_null.auth', 'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK', 'printenv -e -n PK']) - assert(not 'Failed to set EFI variable' in ''.join(output)) - assert('\"PK\" not defined' in ''.join(output)) + assert 'Failed to set EFI variable' not in ''.join(output) + assert '\"PK\" not defined' in ''.join(output) output = u_boot_console.run_command( 'printenv -e SecureBoot') - assert('00000000: 00' in output) + assert '00000000: 00' in output output = u_boot_console.run_command( 'printenv -e SetupMode') - assert('00000000: 01' in output) + assert '00000000: 01' in output diff --git a/test/py/tests/test_efi_secboot/test_signed.py b/test/py/tests/test_efi_secboot/test_signed.py index 6d4b03ef41..4e6f129b7f 100644 --- a/test/py/tests/test_efi_secboot/test_signed.py +++ b/test/py/tests/test_efi_secboot/test_signed.py @@ -9,7 +9,6 @@ This test verifies image authentication for signed images. """ import pytest -from defs import * @pytest.mark.boardspec('sandbox') @@ -32,7 +31,7 @@ class TestEfiSignedImage(object): 'efidebug boot add 1 HELLO1 host 0:1 /helloworld.efi.signed ""', 'efidebug boot next 1', 'bootefi bootmgr']) - assert('Hello, world!' in ''.join(output)) + assert 'Hello, world!' in ''.join(output) with u_boot_console.log.section('Test Case 1b'): # Test Case 1b, run unsigned image if no db/dbx @@ -40,7 +39,7 @@ class TestEfiSignedImage(object): 'efidebug boot add 2 HELLO2 host 0:1 /helloworld.efi ""', 'efidebug boot next 2', 'bootefi bootmgr']) - assert('Hello, world!' in ''.join(output)) + assert 'Hello, world!' in ''.join(output) with u_boot_console.log.section('Test Case 1c'): # Test Case 1c, not authenticated by db @@ -51,23 +50,23 @@ class TestEfiSignedImage(object): 'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK', 'fatload host 0:1 4000000 PK.auth', 'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK']) - assert(not 'Failed to set EFI variable' in ''.join(output)) + assert 'Failed to set EFI variable' not in ''.join(output) output = u_boot_console.run_command_list([ 'efidebug boot next 2', 'bootefi bootmgr']) - assert('\'HELLO2\' failed' in ''.join(output)) + assert '\'HELLO2\' failed' in ''.join(output) output = u_boot_console.run_command_list([ 'efidebug boot next 2', 'efidebug test bootmgr']) - assert('efi_start_image() returned: 26' in ''.join(output)) - assert(not 'Hello, world!' in ''.join(output)) + assert 'efi_start_image() returned: 26' in ''.join(output) + assert 'Hello, world!' not in ''.join(output) with u_boot_console.log.section('Test Case 1d'): # Test Case 1d, authenticated by db output = u_boot_console.run_command_list([ 'efidebug boot next 1', 'bootefi bootmgr']) - assert('Hello, world!' in ''.join(output)) + assert 'Hello, world!' in ''.join(output) def test_efi_signed_image_auth2(self, u_boot_console, efi_boot_env): """ @@ -85,30 +84,30 @@ class TestEfiSignedImage(object): 'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK', 'fatload host 0:1 4000000 PK.auth', 'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK']) - assert(not 'Failed to set EFI variable' in ''.join(output)) + assert 'Failed to set EFI variable' not in ''.join(output) output = u_boot_console.run_command_list([ 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed ""', 'efidebug boot next 1', 'bootefi bootmgr']) - assert('\'HELLO\' failed' in ''.join(output)) + assert '\'HELLO\' failed' in ''.join(output) output = u_boot_console.run_command_list([ 'efidebug boot next 1', 'efidebug test bootmgr']) - assert('efi_start_image() returned: 26' in ''.join(output)) - assert(not 'Hello, world!' in ''.join(output)) + assert 'efi_start_image() returned: 26' in ''.join(output) + assert 'Hello, world!' not in ''.join(output) with u_boot_console.log.section('Test Case 2b'): # Test Case 2b, rejected by dbx even if db allows output = u_boot_console.run_command_list([ 'fatload host 0:1 4000000 db.auth', 'setenv -e -nv -bs -rt -at -i 4000000,$filesize db']) - assert(not 'Failed to set EFI variable' in ''.join(output)) + assert 'Failed to set EFI variable' not in ''.join(output) output = u_boot_console.run_command_list([ 'efidebug boot next 1', 'bootefi bootmgr']) - assert('\'HELLO\' failed' in ''.join(output)) + assert '\'HELLO\' failed' in ''.join(output) output = u_boot_console.run_command_list([ 'efidebug boot next 1', 'efidebug test bootmgr']) - assert('efi_start_image() returned: 26' in ''.join(output)) - assert(not 'Hello, world!' in ''.join(output)) + assert 'efi_start_image() returned: 26' in ''.join(output) + assert 'Hello, world!' not in ''.join(output) diff --git a/test/py/tests/test_efi_secboot/test_unsigned.py b/test/py/tests/test_efi_secboot/test_unsigned.py index 3748b51ee7..c4c3f4c202 100644 --- a/test/py/tests/test_efi_secboot/test_unsigned.py +++ b/test/py/tests/test_efi_secboot/test_unsigned.py @@ -9,7 +9,6 @@ This test verifies image authentication for unsigned images. """ import pytest -from defs import * @pytest.mark.boardspec('sandbox') @@ -33,18 +32,18 @@ class TestEfiUnsignedImage(object): 'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK', 'fatload host 0:1 4000000 PK.auth', 'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK']) - assert(not 'Failed to set EFI variable' in ''.join(output)) + assert 'Failed to set EFI variable' not in ''.join(output) output = u_boot_console.run_command_list([ 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""', 'efidebug boot next 1', 'bootefi bootmgr']) - assert('\'HELLO\' failed' in ''.join(output)) + assert '\'HELLO\' failed' in ''.join(output) output = u_boot_console.run_command_list([ 'efidebug boot next 1', 'efidebug test bootmgr']) - assert('efi_start_image() returned: 26' in ''.join(output)) - assert(not 'Hello, world!' in ''.join(output)) + assert 'efi_start_image() returned: 26' in ''.join(output) + assert 'Hello, world!' not in ''.join(output) def test_efi_unsigned_image_auth2(self, u_boot_console, efi_boot_env): """ @@ -62,13 +61,13 @@ class TestEfiUnsignedImage(object): 'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK', 'fatload host 0:1 4000000 PK.auth', 'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK']) - assert(not 'Failed to set EFI variable' in ''.join(output)) + assert 'Failed to set EFI variable' not in ''.join(output) output = u_boot_console.run_command_list([ 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""', 'efidebug boot next 1', 'bootefi bootmgr']) - assert('Hello, world!' in ''.join(output)) + assert 'Hello, world!' in ''.join(output) def test_efi_unsigned_image_auth3(self, u_boot_console, efi_boot_env): """ @@ -86,33 +85,33 @@ class TestEfiUnsignedImage(object): 'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK', 'fatload host 0:1 4000000 PK.auth', 'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK']) - assert(not 'Failed to set EFI variable' in ''.join(output)) + assert 'Failed to set EFI variable' not in ''.join(output) output = u_boot_console.run_command_list([ 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""', 'efidebug boot next 1', 'bootefi bootmgr']) - assert('\'HELLO\' failed' in ''.join(output)) + assert '\'HELLO\' failed' in ''.join(output) output = u_boot_console.run_command_list([ 'efidebug boot next 1', 'efidebug test bootmgr']) - assert('efi_start_image() returned: 26' in ''.join(output)) - assert(not 'Hello, world!' in ''.join(output)) + assert 'efi_start_image() returned: 26' in ''.join(output) + assert 'Hello, world!' not in ''.join(output) with u_boot_console.log.section('Test Case 3b'): # Test Case 3b, rejected by dbx even if db allows output = u_boot_console.run_command_list([ 'fatload host 0:1 4000000 db_hello.auth', 'setenv -e -nv -bs -rt -at -i 4000000,$filesize db']) - assert(not 'Failed to set EFI variable' in ''.join(output)) + assert 'Failed to set EFI variable' not in ''.join(output) output = u_boot_console.run_command_list([ 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""', 'efidebug boot next 1', 'bootefi bootmgr']) - assert('\'HELLO\' failed' in ''.join(output)) + assert '\'HELLO\' failed' in ''.join(output) output = u_boot_console.run_command_list([ 'efidebug boot next 1', 'efidebug test bootmgr']) - assert('efi_start_image() returned: 26' in ''.join(output)) - assert(not 'Hello, world!' in ''.join(output)) + assert 'efi_start_image() returned: 26' in ''.join(output) + assert 'Hello, world!' not in ''.join(output) From 48ead6097bb614698a53c898243c456e8d6737a3 Mon Sep 17 00:00:00 2001 From: AKASHI Takahiro Date: Wed, 8 Jul 2020 14:02:00 +0900 Subject: [PATCH 10/37] test/py: efi_secboot: split "signed image" test case-1 into two cases Split the existing test case-1 into case1 and a new case-2: case-1 for non-SecureBoot mode; case-2 for SecureBoot mode. In addition, one corner case is added to case-2; a image is signed but a corresponding certificate is not yet installed in "db." Signed-off-by: AKASHI Takahiro --- test/py/tests/test_efi_secboot/test_signed.py | 66 +++++++++++-------- 1 file changed, 38 insertions(+), 28 deletions(-) diff --git a/test/py/tests/test_efi_secboot/test_signed.py b/test/py/tests/test_efi_secboot/test_signed.py index 4e6f129b7f..8ea45c8486 100644 --- a/test/py/tests/test_efi_secboot/test_signed.py +++ b/test/py/tests/test_efi_secboot/test_signed.py @@ -20,12 +20,12 @@ import pytest class TestEfiSignedImage(object): def test_efi_signed_image_auth1(self, u_boot_console, efi_boot_env): """ - Test Case 1 - authenticated by db + Test Case 1 - Secure boot is not in force """ u_boot_console.restart_uboot() disk_img = efi_boot_env with u_boot_console.log.section('Test Case 1a'): - # Test Case 1a, run signed image if no db/dbx + # Test Case 1a, run signed image if no PK output = u_boot_console.run_command_list([ 'host bind 0 %s' % disk_img, 'efidebug boot add 1 HELLO1 host 0:1 /helloworld.efi.signed ""', @@ -34,48 +34,66 @@ class TestEfiSignedImage(object): assert 'Hello, world!' in ''.join(output) with u_boot_console.log.section('Test Case 1b'): - # Test Case 1b, run unsigned image if no db/dbx + # Test Case 1b, run unsigned image if no PK output = u_boot_console.run_command_list([ 'efidebug boot add 2 HELLO2 host 0:1 /helloworld.efi ""', 'efidebug boot next 2', 'bootefi bootmgr']) assert 'Hello, world!' in ''.join(output) - with u_boot_console.log.section('Test Case 1c'): - # Test Case 1c, not authenticated by db + def test_efi_signed_image_auth2(self, u_boot_console, efi_boot_env): + """ + Test Case 2 - Secure boot is in force, + authenticated by db (TEST_db certificate in db) + """ + u_boot_console.restart_uboot() + disk_img = efi_boot_env + with u_boot_console.log.section('Test Case 2a'): + # Test Case 2a, db is not yet installed output = u_boot_console.run_command_list([ - 'fatload host 0:1 4000000 db.auth', - 'setenv -e -nv -bs -rt -at -i 4000000,$filesize db', + 'host bind 0 %s' % disk_img, 'fatload host 0:1 4000000 KEK.auth', 'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK', 'fatload host 0:1 4000000 PK.auth', 'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK']) assert 'Failed to set EFI variable' not in ''.join(output) output = u_boot_console.run_command_list([ + 'efidebug boot add 1 HELLO1 host 0:1 /helloworld.efi.signed ""', + 'efidebug boot next 1', + 'efidebug test bootmgr']) + assert('\'HELLO1\' failed' in ''.join(output)) + assert('efi_start_image() returned: 26' in ''.join(output)) + output = u_boot_console.run_command_list([ + 'efidebug boot add 2 HELLO2 host 0:1 /helloworld.efi ""', 'efidebug boot next 2', - 'bootefi bootmgr']) + 'efidebug test bootmgr']) assert '\'HELLO2\' failed' in ''.join(output) + assert 'efi_start_image() returned: 26' in ''.join(output) + + with u_boot_console.log.section('Test Case 2b'): + # Test Case 2b, authenticated by db + output = u_boot_console.run_command_list([ + 'fatload host 0:1 4000000 db.auth', + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize db']) + assert 'Failed to set EFI variable' not in ''.join(output) output = u_boot_console.run_command_list([ 'efidebug boot next 2', 'efidebug test bootmgr']) + assert '\'HELLO2\' failed' in ''.join(output) assert 'efi_start_image() returned: 26' in ''.join(output) - assert 'Hello, world!' not in ''.join(output) - - with u_boot_console.log.section('Test Case 1d'): - # Test Case 1d, authenticated by db output = u_boot_console.run_command_list([ 'efidebug boot next 1', 'bootefi bootmgr']) assert 'Hello, world!' in ''.join(output) - def test_efi_signed_image_auth2(self, u_boot_console, efi_boot_env): + def test_efi_signed_image_auth3(self, u_boot_console, efi_boot_env): """ - Test Case 2 - rejected by dbx + Test Case 3 - rejected by dbx (TEST_db certificate in dbx) """ u_boot_console.restart_uboot() disk_img = efi_boot_env - with u_boot_console.log.section('Test Case 2a'): - # Test Case 2a, rejected by dbx + with u_boot_console.log.section('Test Case 3a'): + # Test Case 3a, rejected by dbx output = u_boot_console.run_command_list([ 'host bind 0 %s' % disk_img, 'fatload host 0:1 4000000 db.auth', @@ -87,27 +105,19 @@ class TestEfiSignedImage(object): assert 'Failed to set EFI variable' not in ''.join(output) output = u_boot_console.run_command_list([ 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed ""', - 'efidebug boot next 1', - 'bootefi bootmgr']) - assert '\'HELLO\' failed' in ''.join(output) - output = u_boot_console.run_command_list([ 'efidebug boot next 1', 'efidebug test bootmgr']) + assert '\'HELLO\' failed' in ''.join(output) assert 'efi_start_image() returned: 26' in ''.join(output) - assert 'Hello, world!' not in ''.join(output) - with u_boot_console.log.section('Test Case 2b'): - # Test Case 2b, rejected by dbx even if db allows + with u_boot_console.log.section('Test Case 3b'): + # Test Case 3b, rejected by dbx even if db allows output = u_boot_console.run_command_list([ 'fatload host 0:1 4000000 db.auth', 'setenv -e -nv -bs -rt -at -i 4000000,$filesize db']) assert 'Failed to set EFI variable' not in ''.join(output) - output = u_boot_console.run_command_list([ - 'efidebug boot next 1', - 'bootefi bootmgr']) - assert '\'HELLO\' failed' in ''.join(output) output = u_boot_console.run_command_list([ 'efidebug boot next 1', 'efidebug test bootmgr']) + assert '\'HELLO\' failed' in ''.join(output) assert 'efi_start_image() returned: 26' in ''.join(output) - assert 'Hello, world!' not in ''.join(output) From c6361e73b5c21216fc49ed46721015283414d77e Mon Sep 17 00:00:00 2001 From: AKASHI Takahiro Date: Wed, 8 Jul 2020 14:02:01 +0900 Subject: [PATCH 11/37] test/py: efi_secboot: add a test against certificate revocation Revocation database (dbx) may have not only certificates, but also message digests of certificates with revocation time (EFI_CERT_X509_SHA256_GUILD). In this test case, if the database has such a digest and if the value matches to a certificate that created a given image's signature, authentication should fail. Signed-off-by: AKASHI Takahiro Set defined time stamp for dbx_hash.auth. Signed-off-by: Heinrich Schuchardt --- test/py/tests/test_efi_secboot/conftest.py | 6 ++++- test/py/tests/test_efi_secboot/test_signed.py | 26 +++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/test/py/tests/test_efi_secboot/conftest.py b/test/py/tests/test_efi_secboot/conftest.py index c0ee8be9f7..c3d56622a2 100644 --- a/test/py/tests/test_efi_secboot/conftest.py +++ b/test/py/tests/test_efi_secboot/conftest.py @@ -106,12 +106,16 @@ def efi_boot_env(request, u_boot_config): # db1-update check_call('cd %s; %ssign-efi-sig-list -t "2020-04-06" -a -c KEK.crt -k KEK.key db db1.esl db1-update.auth' % (mnt_point, EFITOOLS_PATH), shell=True) - # dbx + ## dbx (TEST_dbx certificate) check_call('cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_dbx/ -keyout dbx.key -out dbx.crt -nodes -days 365' % mnt_point, shell=True) check_call('cd %s; %scert-to-efi-sig-list -g %s dbx.crt dbx.esl; %ssign-efi-sig-list -t "2020-04-05" -c KEK.crt -k KEK.key dbx dbx.esl dbx.auth' % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH), shell=True) + ## dbx_hash (digest of TEST_db certificate) + check_call('cd %s; %scert-to-efi-hash-list -g %s -t 0 -s 256 db.crt dbx_hash.crl; %ssign-efi-sig-list -t "2020-04-05" -c KEK.crt -k KEK.key dbx dbx_hash.crl dbx_hash.auth' + % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH), + shell=True) # Copy image check_call('cp %s %s' % (HELLO_PATH, mnt_point), shell=True) diff --git a/test/py/tests/test_efi_secboot/test_signed.py b/test/py/tests/test_efi_secboot/test_signed.py index 8ea45c8486..6dabecb669 100644 --- a/test/py/tests/test_efi_secboot/test_signed.py +++ b/test/py/tests/test_efi_secboot/test_signed.py @@ -121,3 +121,29 @@ class TestEfiSignedImage(object): 'efidebug test bootmgr']) assert '\'HELLO\' failed' in ''.join(output) assert 'efi_start_image() returned: 26' in ''.join(output) + + def test_efi_signed_image_auth4(self, u_boot_console, efi_boot_env): + """ + Test Case 4 - revoked by dbx (digest of TEST_db certificate in dbx) + """ + u_boot_console.restart_uboot() + disk_img = efi_boot_env + with u_boot_console.log.section('Test Case 4'): + # Test Case 4, rejected by dbx + output = u_boot_console.run_command_list([ + 'host bind 0 %s' % disk_img, + 'fatload host 0:1 4000000 dbx_hash.auth', + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize dbx', + 'fatload host 0:1 4000000 db.auth', + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize db', + 'fatload host 0:1 4000000 KEK.auth', + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK', + 'fatload host 0:1 4000000 PK.auth', + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK']) + assert 'Failed to set EFI variable' not in ''.join(output) + output = u_boot_console.run_command_list([ + 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed ""', + 'efidebug boot next 1', + 'efidebug test bootmgr']) + assert '\'HELLO\' failed' in ''.join(output) + assert 'efi_start_image() returned: 26' in ''.join(output) From 0c7772d40f7994b1ff75ad9acd220101241c1feb Mon Sep 17 00:00:00 2001 From: AKASHI Takahiro Date: Wed, 8 Jul 2020 14:02:02 +0900 Subject: [PATCH 12/37] test/py: efi_secboot: add a test for multiple signatures In this test case, an image is signed multiple times with different keys. If any of signatures contained is not verified, the whole authentication check should fail. Signed-off-by: AKASHI Takahiro Provide a defined time stamp for dbx_hash1.auth. Signed-off-by: Heinrich Schuchardt --- test/py/tests/test_efi_secboot/conftest.py | 9 +++- test/py/tests/test_efi_secboot/test_signed.py | 51 +++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/test/py/tests/test_efi_secboot/conftest.py b/test/py/tests/test_efi_secboot/conftest.py index c3d56622a2..7bb2e1d765 100644 --- a/test/py/tests/test_efi_secboot/conftest.py +++ b/test/py/tests/test_efi_secboot/conftest.py @@ -116,6 +116,10 @@ def efi_boot_env(request, u_boot_config): check_call('cd %s; %scert-to-efi-hash-list -g %s -t 0 -s 256 db.crt dbx_hash.crl; %ssign-efi-sig-list -t "2020-04-05" -c KEK.crt -k KEK.key dbx dbx_hash.crl dbx_hash.auth' % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH), shell=True) + ## dbx_hash1 (digest of TEST_db1 certificate) + check_call('cd %s; %scert-to-efi-hash-list -g %s -t 0 -s 256 db1.crt dbx_hash1.crl; %ssign-efi-sig-list -t "2020-04-05" -c KEK.crt -k KEK.key dbx dbx_hash1.crl dbx_hash1.auth' + % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH), + shell=True) # Copy image check_call('cp %s %s' % (HELLO_PATH, mnt_point), shell=True) @@ -123,7 +127,10 @@ def efi_boot_env(request, u_boot_config): # Sign image check_call('cd %s; sbsign --key db.key --cert db.crt helloworld.efi' % mnt_point, shell=True) - # Digest image + ## Sign already-signed image with another key + check_call('cd %s; sbsign --key db1.key --cert db1.crt --output helloworld.efi.signed_2sigs helloworld.efi.signed' + % mnt_point, shell=True) + ## Digest image check_call('cd %s; %shash-to-efi-sig-list helloworld.efi db_hello.hash; %ssign-efi-sig-list -t "2020-04-07" -c KEK.crt -k KEK.key db db_hello.hash db_hello.auth' % (mnt_point, EFITOOLS_PATH, EFITOOLS_PATH), shell=True) diff --git a/test/py/tests/test_efi_secboot/test_signed.py b/test/py/tests/test_efi_secboot/test_signed.py index 6dabecb669..1a31a57e12 100644 --- a/test/py/tests/test_efi_secboot/test_signed.py +++ b/test/py/tests/test_efi_secboot/test_signed.py @@ -147,3 +147,54 @@ class TestEfiSignedImage(object): 'efidebug test bootmgr']) assert '\'HELLO\' failed' in ''.join(output) assert 'efi_start_image() returned: 26' in ''.join(output) + + def test_efi_signed_image_auth5(self, u_boot_console, efi_boot_env): + """ + Test Case 5 - multiple signatures + one signed with TEST_db, and + one signed with TEST_db1 + """ + u_boot_console.restart_uboot() + disk_img = efi_boot_env + with u_boot_console.log.section('Test Case 5a'): + # Test Case 5a, rejected if any of signatures is not verified + output = u_boot_console.run_command_list([ + 'host bind 0 %s' % disk_img, + 'fatload host 0:1 4000000 db.auth', + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize db', + 'fatload host 0:1 4000000 KEK.auth', + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK', + 'fatload host 0:1 4000000 PK.auth', + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK']) + assert 'Failed to set EFI variable' not in ''.join(output) + output = u_boot_console.run_command_list([ + 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed_2sigs ""', + 'efidebug boot next 1', + 'efidebug test bootmgr']) + assert '\'HELLO\' failed' in ''.join(output) + assert 'efi_start_image() returned: 26' in ''.join(output) + + with u_boot_console.log.section('Test Case 5b'): + # Test Case 5b, authenticated if both signatures are verified + output = u_boot_console.run_command_list([ + 'fatload host 0:1 4000000 db1.auth', + 'setenv -e -nv -bs -rt -at -a -i 4000000,$filesize db']) + assert 'Failed to set EFI variable' not in ''.join(output) + output = u_boot_console.run_command_list([ + 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed_2sigs ""', + 'efidebug boot next 1', + 'bootefi bootmgr']) + assert 'Hello, world!' in ''.join(output) + + with u_boot_console.log.section('Test Case 5c'): + # Test Case 5c, rejected if any of signatures is revoked + output = u_boot_console.run_command_list([ + 'fatload host 0:1 4000000 dbx_hash1.auth', + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize dbx']) + assert 'Failed to set EFI variable' not in ''.join(output) + output = u_boot_console.run_command_list([ + 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed_2sigs ""', + 'efidebug boot next 1', + 'efidebug test bootmgr']) + assert '\'HELLO\' failed' in ''.join(output) + assert 'efi_start_image() returned: 26' in ''.join(output) From 7fdc02b3d7d2085231f44d44c0556f4a592c8daf Mon Sep 17 00:00:00 2001 From: AKASHI Takahiro Date: Wed, 8 Jul 2020 14:02:03 +0900 Subject: [PATCH 13/37] test/py: efi_secboot: add a test for verifying with digest of signed image Signature database (db or dbx) may have not only certificates that contain a public key for RSA decryption, but also digests of signed images. In this test case, if database has an image's digest (EFI_CERT_SHA256_GUID) and if the value matches to a hash value calculated from image's binary, authentication should pass in case of db, and fail in case of dbx. Signed-off-by: AKASHI Takahiro Use defined time stamps for sign-efi-sig-list. Signed-off-by: Heinrich Schuchardt --- test/py/tests/test_efi_secboot/conftest.py | 10 ++++ test/py/tests/test_efi_secboot/test_signed.py | 49 +++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/test/py/tests/test_efi_secboot/conftest.py b/test/py/tests/test_efi_secboot/conftest.py index 7bb2e1d765..71ef723e59 100644 --- a/test/py/tests/test_efi_secboot/conftest.py +++ b/test/py/tests/test_efi_secboot/conftest.py @@ -120,6 +120,10 @@ def efi_boot_env(request, u_boot_config): check_call('cd %s; %scert-to-efi-hash-list -g %s -t 0 -s 256 db1.crt dbx_hash1.crl; %ssign-efi-sig-list -t "2020-04-05" -c KEK.crt -k KEK.key dbx dbx_hash1.crl dbx_hash1.auth' % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH), shell=True) + ## dbx_db (with TEST_db certificate) + check_call('cd %s; %ssign-efi-sig-list -t "2020-04-05" -c KEK.crt -k KEK.key dbx db.esl dbx_db.auth' + % (mnt_point, EFITOOLS_PATH), + shell=True) # Copy image check_call('cp %s %s' % (HELLO_PATH, mnt_point), shell=True) @@ -134,6 +138,12 @@ def efi_boot_env(request, u_boot_config): check_call('cd %s; %shash-to-efi-sig-list helloworld.efi db_hello.hash; %ssign-efi-sig-list -t "2020-04-07" -c KEK.crt -k KEK.key db db_hello.hash db_hello.auth' % (mnt_point, EFITOOLS_PATH, EFITOOLS_PATH), shell=True) + check_call('cd %s; %shash-to-efi-sig-list helloworld.efi.signed db_hello_signed.hash; %ssign-efi-sig-list -t "2020-04-03" -c KEK.crt -k KEK.key db db_hello_signed.hash db_hello_signed.auth' + % (mnt_point, EFITOOLS_PATH, EFITOOLS_PATH), + shell=True) + check_call('cd %s; %ssign-efi-sig-list -t "2020-04-07" -c KEK.crt -k KEK.key dbx db_hello_signed.hash dbx_hello_signed.auth' + % (mnt_point, EFITOOLS_PATH), + shell=True) check_call('sudo umount %s' % loop_dev, shell=True) check_call('sudo losetup -d %s' % loop_dev, shell=True) diff --git a/test/py/tests/test_efi_secboot/test_signed.py b/test/py/tests/test_efi_secboot/test_signed.py index 1a31a57e12..7531bbac6a 100644 --- a/test/py/tests/test_efi_secboot/test_signed.py +++ b/test/py/tests/test_efi_secboot/test_signed.py @@ -198,3 +198,52 @@ class TestEfiSignedImage(object): 'efidebug test bootmgr']) assert '\'HELLO\' failed' in ''.join(output) assert 'efi_start_image() returned: 26' in ''.join(output) + + def test_efi_signed_image_auth6(self, u_boot_console, efi_boot_env): + """ + Test Case 6 - using digest of signed image in database + """ + u_boot_console.restart_uboot() + disk_img = efi_boot_env + with u_boot_console.log.section('Test Case 6a'): + # Test Case 6a, verified by image's digest in db + output = u_boot_console.run_command_list([ + 'host bind 0 %s' % disk_img, + 'fatload host 0:1 4000000 db_hello_signed.auth', + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize db', + 'fatload host 0:1 4000000 KEK.auth', + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK', + 'fatload host 0:1 4000000 PK.auth', + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK']) + assert 'Failed to set EFI variable' not in ''.join(output) + output = u_boot_console.run_command_list([ + 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed ""', + 'efidebug boot next 1', + 'bootefi bootmgr']) + assert 'Hello, world!' in ''.join(output) + + with u_boot_console.log.section('Test Case 6b'): + # Test Case 6b, rejected by TEST_db certificate in dbx + output = u_boot_console.run_command_list([ + 'fatload host 0:1 4000000 dbx_db.auth', + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize dbx']) + assert 'Failed to set EFI variable' not in ''.join(output) + output = u_boot_console.run_command_list([ + 'efidebug boot next 1', + 'efidebug test bootmgr']) + assert '\'HELLO\' failed' in ''.join(output) + assert 'efi_start_image() returned: 26' in ''.join(output) + + with u_boot_console.log.section('Test Case 6c'): + # Test Case 6c, rejected by image's digest in dbx + output = u_boot_console.run_command_list([ + 'fatload host 0:1 4000000 db.auth', + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize db', + 'fatload host 0:1 4000000 dbx_hello_signed.auth', + 'setenv -e -nv -bs -rt -at -i 4000000,$filesize dbx']) + assert 'Failed to set EFI variable' not in ''.join(output) + output = u_boot_console.run_command_list([ + 'efidebug boot next 1', + 'efidebug test bootmgr']) + assert '\'HELLO\' failed' in ''.join(output) + assert 'efi_start_image() returned: 26' in ''.join(output) From 53ce9a6ed98b69a82e54a28254b014e480fc98ca Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Sat, 11 Jul 2020 23:05:18 +0200 Subject: [PATCH 14/37] test: use virt-make-fs to build image Avoid sudo for test/py/tests/test_efi_secboot by using virt-make-fs. Signed-off-by: Heinrich Schuchardt --- test/py/tests/test_efi_secboot/conftest.py | 27 ++++------------------ test/py/tests/test_efi_secboot/defs.py | 7 ------ 2 files changed, 4 insertions(+), 30 deletions(-) diff --git a/test/py/tests/test_efi_secboot/conftest.py b/test/py/tests/test_efi_secboot/conftest.py index 71ef723e59..c6709700a8 100644 --- a/test/py/tests/test_efi_secboot/conftest.py +++ b/test/py/tests/test_efi_secboot/conftest.py @@ -38,34 +38,15 @@ def efi_boot_env(request, u_boot_config): image_path = u_boot_config.persistent_data_dir image_path = image_path + '/' + EFI_SECBOOT_IMAGE_NAME - image_size = EFI_SECBOOT_IMAGE_SIZE - part_size = EFI_SECBOOT_PART_SIZE - fs_type = EFI_SECBOOT_FS_TYPE if HELLO_PATH == '': HELLO_PATH = u_boot_config.build_dir + '/lib/efi_loader/helloworld.efi' try: - mnt_point = u_boot_config.persistent_data_dir + '/mnt_efisecure' + mnt_point = u_boot_config.build_dir + '/mnt_efisecure' + check_call('rm -rf {}'.format(mnt_point), shell=True) check_call('mkdir -p {}'.format(mnt_point), shell=True) - # create a disk/partition - check_call('dd if=/dev/zero of=%s bs=1MiB count=%d' - % (image_path, image_size), shell=True) - check_call('sgdisk %s -n 1:0:+%dMiB' - % (image_path, part_size), shell=True) - # create a file system - check_call('dd if=/dev/zero of=%s.tmp bs=1MiB count=%d' - % (image_path, part_size), shell=True) - check_call('mkfs -t %s %s.tmp' % (fs_type, image_path), shell=True) - check_call('dd if=%s.tmp of=%s bs=1MiB seek=1 count=%d conv=notrunc' - % (image_path, image_path, 1), shell=True) - check_call('rm %s.tmp' % image_path, shell=True) - loop_dev = check_output('sudo losetup -o 1MiB --sizelimit %dMiB --show -f %s | tr -d "\n"' - % (part_size, image_path), shell=True).decode() - check_output('sudo mount -t %s -o umask=000 %s %s' - % (fs_type, loop_dev, mnt_point), shell=True) - # suffix # *.key: RSA private key in PEM # *.crt: X509 certificate (self-signed) in PEM @@ -145,8 +126,8 @@ def efi_boot_env(request, u_boot_config): % (mnt_point, EFITOOLS_PATH), shell=True) - check_call('sudo umount %s' % loop_dev, shell=True) - check_call('sudo losetup -d %s' % loop_dev, shell=True) + check_call('virt-make-fs --partition=gpt --size=+1M --type=vfat {} {}'.format(mnt_point, image_path), shell=True) + check_call('rm -rf {}'.format(mnt_point), shell=True) except CalledProcessError as exception: pytest.skip('Setup failed: %s' % exception.cmd) diff --git a/test/py/tests/test_efi_secboot/defs.py b/test/py/tests/test_efi_secboot/defs.py index 099f453979..ba6b9f391e 100644 --- a/test/py/tests/test_efi_secboot/defs.py +++ b/test/py/tests/test_efi_secboot/defs.py @@ -3,13 +3,6 @@ # Disk image name EFI_SECBOOT_IMAGE_NAME = 'test_efi_secboot.img' -# Size in MiB -EFI_SECBOOT_IMAGE_SIZE = 16 -EFI_SECBOOT_PART_SIZE = 8 - -# Partition file system type -EFI_SECBOOT_FS_TYPE = 'vfat' - # Owner guid GUID = '11111111-2222-3333-4444-123456789abc' From 491bfe86062464b892386e8c8dfe0069975ecedc Mon Sep 17 00:00:00 2001 From: AKASHI Takahiro Date: Tue, 16 Jun 2020 14:26:48 +0900 Subject: [PATCH 15/37] lib: rsa: export rsa_verify_with_pkey() This function will be used to implement public_key_verify_signature() in a later patch. rsa_verify() is not suitable here because calculation of message digest is not necessary. Signed-off-by: AKASHI Takahiro Reviewed-by: Heinrich Schuchardt --- include/u-boot/rsa.h | 3 +++ lib/rsa/rsa-verify.c | 8 ++++---- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/include/u-boot/rsa.h b/include/u-boot/rsa.h index a0bae495f0..bed1c097c2 100644 --- a/include/u-boot/rsa.h +++ b/include/u-boot/rsa.h @@ -112,6 +112,9 @@ int rsa_verify(struct image_sign_info *info, const struct image_region region[], int region_count, uint8_t *sig, uint sig_len); +int rsa_verify_with_pkey(struct image_sign_info *info, + const void *hash, uint8_t *sig, uint sig_len); + int padding_pkcs_15_verify(struct image_sign_info *info, uint8_t *msg, int msg_len, const uint8_t *hash, int hash_len); diff --git a/lib/rsa/rsa-verify.c b/lib/rsa/rsa-verify.c index 6c4bbc4625..2057f6819d 100644 --- a/lib/rsa/rsa-verify.c +++ b/lib/rsa/rsa-verify.c @@ -387,8 +387,8 @@ static int rsa_verify_key(struct image_sign_info *info, * * Return 0 if verified, -ve on error */ -static int rsa_verify_with_pkey(struct image_sign_info *info, - const void *hash, uint8_t *sig, uint sig_len) +int rsa_verify_with_pkey(struct image_sign_info *info, + const void *hash, uint8_t *sig, uint sig_len) { struct key_prop *prop; int ret; @@ -408,8 +408,8 @@ static int rsa_verify_with_pkey(struct image_sign_info *info, return ret; } #else -static int rsa_verify_with_pkey(struct image_sign_info *info, - const void *hash, uint8_t *sig, uint sig_len) +int rsa_verify_with_pkey(struct image_sign_info *info, + const void *hash, uint8_t *sig, uint sig_len) { return -EACCES; } From ecb833a0c623172cf72e1c6dda9e6362c30d5a86 Mon Sep 17 00:00:00 2001 From: Ilias Apalodimas Date: Wed, 1 Jul 2020 16:41:25 +0300 Subject: [PATCH 16/37] efi_loader: cleanup for tee backed variables There's 2 variables in efi_get_next_variable_name() checking the size of the variable name. Let's get rid of the reduntant definition and simplitfy the code a bit. Signed-off-by: Ilias Apalodimas Signed-off-by: Heinrich Schuchardt --- lib/efi_loader/efi_variable_tee.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c index cacc76e23d..1d127720cd 100644 --- a/lib/efi_loader/efi_variable_tee.c +++ b/lib/efi_loader/efi_variable_tee.c @@ -351,7 +351,6 @@ efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size, efi_uintn_t out_name_size; efi_uintn_t in_name_size; efi_uintn_t tmp_dsize; - efi_uintn_t name_size; u8 *comm_buf = NULL; efi_status_t ret; @@ -370,19 +369,18 @@ efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size, goto out; } - name_size = u16_strsize(variable_name); - if (name_size > max_payload_size - MM_VARIABLE_GET_NEXT_HEADER_SIZE) { + if (in_name_size > max_payload_size - MM_VARIABLE_GET_NEXT_HEADER_SIZE) { ret = EFI_INVALID_PARAMETER; goto out; } /* Trim output buffer size */ tmp_dsize = *variable_name_size; - if (name_size + tmp_dsize > + if (in_name_size + tmp_dsize > max_payload_size - MM_VARIABLE_GET_NEXT_HEADER_SIZE) { tmp_dsize = max_payload_size - MM_VARIABLE_GET_NEXT_HEADER_SIZE - - name_size; + in_name_size; } payload_size = MM_VARIABLE_GET_NEXT_HEADER_SIZE + out_name_size; From 39a75f5af139eaa53758a8cb0e0788cfafdaf54c Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Tue, 7 Jul 2020 07:23:44 +0200 Subject: [PATCH 17/37] efi_loader: wrong printf format in efi_image_parse MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit 1b6c08548c85 ("efi_loader: image_loader: replace debug to EFI_PRINT") leads to a build warning on 32bit systems: lib/efi_loader/efi_image_loader.c: In function ‘efi_image_parse’: include/efi_loader.h:123:8: warning: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 8 has type ‘size_t’ {aka ‘unsigned int’} [-Wformat=] Use %zu for printing size_t. Fixes: 1b6c08548c85 ("efi_loader: image_loader: replace debug to EFI_PRINT") Signed-off-by: Heinrich Schuchardt --- lib/efi_loader/efi_image_loader.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c index b7cf26046e..fef0bb870c 100644 --- a/lib/efi_loader/efi_image_loader.c +++ b/lib/efi_loader/efi_image_loader.c @@ -382,7 +382,7 @@ bool efi_image_parse(void *efi, size_t len, struct efi_image_regions **regp, /* 3. Extra data excluding Certificates Table */ if (bytes_hashed + authsz < len) { - EFI_PRINT("extra data for hash: %lu\n", + EFI_PRINT("extra data for hash: %zu\n", len - (bytes_hashed + authsz)); efi_image_region_add(regs, efi + bytes_hashed, efi + len - authsz, 0); From 1047c6e23c9bf7e2e0b0c0539b6d7289627c9f71 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Tue, 7 Jul 2020 04:21:26 +0200 Subject: [PATCH 18/37] efi_loader: fix efi_get_child_controllers() Don't call calloc(0, ..). Consider return value of efi_get_child_controllers(). Signed-off-by: Heinrich Schuchardt --- lib/efi_loader/efi_boottime.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 1591ad8300..0b16554ba2 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -3459,6 +3459,8 @@ static efi_status_t efi_get_child_controllers( * the buffer will be too large. But that does not harm. */ *number_of_children = 0; + if (!count) + return EFI_SUCCESS; *child_handle_buffer = calloc(count, sizeof(efi_handle_t)); if (!*child_handle_buffer) return EFI_OUT_OF_RESOURCES; @@ -3536,10 +3538,12 @@ static efi_status_t EFIAPI efi_disconnect_controller( number_of_children = 1; child_handle_buffer = &child_handle; } else { - efi_get_child_controllers(efiobj, - driver_image_handle, - &number_of_children, - &child_handle_buffer); + r = efi_get_child_controllers(efiobj, + driver_image_handle, + &number_of_children, + &child_handle_buffer); + if (r != EFI_SUCCESS) + return r; } /* Get the driver binding protocol */ From a27c78fddbbafcace558368a92f9b9fdaeee8b5c Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Tue, 7 Jul 2020 03:10:12 +0200 Subject: [PATCH 19/37] efi_loader: NULL dereference in efi_convert_pointer Avoid a possible NULL pointer dereference in efi_convert_pointer(). Signed-off-by: Heinrich Schuchardt --- lib/efi_loader/efi_runtime.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c index c0bd99b867..121e2f65c6 100644 --- a/lib/efi_loader/efi_runtime.c +++ b/lib/efi_loader/efi_runtime.c @@ -499,7 +499,7 @@ static __efi_runtime efi_status_t EFIAPI efi_convert_pointer_runtime( static __efi_runtime efi_status_t EFIAPI efi_convert_pointer( efi_uintn_t debug_disposition, void **address) { - efi_physical_addr_t addr = (uintptr_t)*address; + efi_physical_addr_t addr; efi_uintn_t i; efi_status_t ret = EFI_NOT_FOUND; @@ -515,6 +515,7 @@ static __efi_runtime efi_status_t EFIAPI efi_convert_pointer( goto out; } + addr = (uintptr_t)*address; for (i = 0; i < efi_descriptor_count; i++) { struct efi_mem_desc *map = (void *)efi_virtmap + (efi_descriptor_size * i); From 5a8d1f60b2505cf2ee116ac38e54e65b757a1773 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Mon, 6 Jul 2020 07:48:14 +0200 Subject: [PATCH 20/37] fs/fat: reduce data size for FAT_WRITE Allocated tmpbuf_cluster dynamically to reduce the data size added by compiling with CONFIG_FAT_WRITE. Reported-by: Tom Rini Signed-off-by: Heinrich Schuchardt --- fs/fat/fat_write.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c index b16a39d3ff..a2682b5f46 100644 --- a/fs/fat/fat_write.c +++ b/fs/fat/fat_write.c @@ -500,8 +500,6 @@ flush_dir(fat_itr *itr) nsects * mydata->sect_size); } -static __u8 tmpbuf_cluster[MAX_CLUSTSIZE] __aligned(ARCH_DMA_MINALIGN); - /* * Read and modify data on existing and consecutive cluster blocks */ @@ -509,6 +507,7 @@ static int get_set_cluster(fsdata *mydata, __u32 clustnum, loff_t pos, __u8 *buffer, loff_t size, loff_t *gotsize) { + static u8 *tmpbuf_cluster; unsigned int bytesperclust = mydata->clust_size * mydata->sect_size; __u32 startsect; loff_t wsize; @@ -518,6 +517,12 @@ get_set_cluster(fsdata *mydata, __u32 clustnum, loff_t pos, __u8 *buffer, if (!size) return 0; + if (!tmpbuf_cluster) { + tmpbuf_cluster = memalign(ARCH_DMA_MINALIGN, MAX_CLUSTSIZE); + if (!tmpbuf_cluster) + return -1; + } + assert(pos < bytesperclust); startsect = clust_to_sect(mydata, clustnum); From f2d2b3a11ce18663ea95c29eb2c609efd77b7999 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Mon, 22 Jun 2020 18:10:27 +0200 Subject: [PATCH 21/37] efi_loader: prepare for read only OP-TEE variables We currently have two implementations of UEFI variables: * variables provided via an OP-TEE module * variables stored in the U-Boot environment Read only variables are up to now only implemented in the U-Boot environment implementation. Provide a common interface for both implementations that allows handling read-only variables. As variable access is limited to very few source files put variable related definitions into new include efi_variable.h instead of efi_loader. Signed-off-by: Heinrich Schuchardt --- doc/api/efi.rst | 2 + include/efi_variable.h | 43 ++++++++ lib/efi_loader/Makefile | 1 + lib/efi_loader/efi_var_common.c | 78 +++++++++++++ lib/efi_loader/efi_variable.c | 175 ++++++++---------------------- lib/efi_loader/efi_variable_tee.c | 75 ++++--------- 6 files changed, 193 insertions(+), 181 deletions(-) create mode 100644 include/efi_variable.h create mode 100644 lib/efi_loader/efi_var_common.c diff --git a/doc/api/efi.rst b/doc/api/efi.rst index d5114f05b3..cb2a1c897e 100644 --- a/doc/api/efi.rst +++ b/doc/api/efi.rst @@ -93,6 +93,8 @@ Runtime services Variable services ~~~~~~~~~~~~~~~~~ +.. kernel-doc:: include/efi_variable.h + :internal: .. kernel-doc:: lib/efi_loader/efi_variable.c :internal: diff --git a/include/efi_variable.h b/include/efi_variable.h new file mode 100644 index 0000000000..6789118eba --- /dev/null +++ b/include/efi_variable.h @@ -0,0 +1,43 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * Copyright (c) 2020, Heinrich Schuchardt + */ + +#ifndef _EFI_VARIABLE_H +#define _EFI_VARIABLE_H + +#include + +#define EFI_VARIABLE_READ_ONLY BIT(31) + +/** + * efi_get_variable() - retrieve value of a UEFI variable + * + * @variable_name: name of the variable + * @vendor: vendor GUID + * @attributes: attributes of the variable + * @data_size: size of the buffer to which the variable value is copied + * @data: buffer to which the variable value is copied + * @timep: authentication time (seconds since start of epoch) + * Return: status code + */ +efi_status_t efi_get_variable_int(u16 *variable_name, const efi_guid_t *vendor, + u32 *attributes, efi_uintn_t *data_size, + void *data, u64 *timep); + +/** + * efi_set_variable() - set value of a UEFI variable + * + * @variable_name: name of the variable + * @vendor: vendor GUID + * @attributes: attributes of the variable + * @data_size: size of the buffer with the variable value + * @data: buffer with the variable value + * @ro_check: check the read only read only bit in attributes + * Return: status code + */ +efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor, + u32 attributes, efi_uintn_t data_size, + const void *data, bool ro_check); + +#endif diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile index 57c7e66ea0..7eddd7ef37 100644 --- a/lib/efi_loader/Makefile +++ b/lib/efi_loader/Makefile @@ -35,6 +35,7 @@ obj-y += efi_root_node.o obj-y += efi_runtime.o obj-y += efi_setup.o obj-$(CONFIG_EFI_UNICODE_COLLATION_PROTOCOL2) += efi_unicode_collation.o +obj-y += efi_var_common.o ifeq ($(CONFIG_EFI_MM_COMM_TEE),y) obj-y += efi_variable_tee.o else diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c new file mode 100644 index 0000000000..6a4efa3f27 --- /dev/null +++ b/lib/efi_loader/efi_var_common.c @@ -0,0 +1,78 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * UEFI runtime variable services + * + * Copyright (c) 2020, Heinrich Schuchardt + */ + +#include +#include +#include + +/** + * efi_efi_get_variable() - retrieve value of a UEFI variable + * + * This function implements the GetVariable runtime service. + * + * See the Unified Extensible Firmware Interface (UEFI) specification for + * details. + * + * @variable_name: name of the variable + * @vendor: vendor GUID + * @attributes: attributes of the variable + * @data_size: size of the buffer to which the variable value is copied + * @data: buffer to which the variable value is copied + * Return: status code + */ +efi_status_t EFIAPI efi_get_variable(u16 *variable_name, + const efi_guid_t *vendor, u32 *attributes, + efi_uintn_t *data_size, void *data) +{ + efi_status_t ret; + + EFI_ENTRY("\"%ls\" %pUl %p %p %p", variable_name, vendor, attributes, + data_size, data); + + ret = efi_get_variable_int(variable_name, vendor, attributes, + data_size, data, NULL); + + /* Remove EFI_VARIABLE_READ_ONLY flag */ + if (attributes) + *attributes &= EFI_VARIABLE_MASK; + + return EFI_EXIT(ret); +} + +/** + * efi_set_variable() - set value of a UEFI variable + * + * This function implements the SetVariable runtime service. + * + * See the Unified Extensible Firmware Interface (UEFI) specification for + * details. + * + * @variable_name: name of the variable + * @vendor: vendor GUID + * @attributes: attributes of the variable + * @data_size: size of the buffer with the variable value + * @data: buffer with the variable value + * Return: status code + */ +efi_status_t EFIAPI efi_set_variable(u16 *variable_name, + const efi_guid_t *vendor, u32 attributes, + efi_uintn_t data_size, const void *data) +{ + efi_status_t ret; + + EFI_ENTRY("\"%ls\" %pUl %x %zu %p", variable_name, vendor, attributes, + data_size, data); + + /* Make sure that the EFI_VARIABLE_READ_ONLY flag is not set */ + if (attributes & ~(u32)EFI_VARIABLE_MASK) + ret = EFI_INVALID_PARAMETER; + else + ret = efi_set_variable_int(variable_name, vendor, attributes, + data_size, data, true); + + return EFI_EXIT(ret); +} diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index efaba869ef..6ec1f97326 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -15,7 +16,6 @@ #include #include #include -#include #include #include @@ -30,21 +30,6 @@ static bool efi_secure_boot; static enum efi_secure_mode efi_secure_mode; static u8 efi_vendor_keys; -#define READ_ONLY BIT(31) - -static efi_status_t efi_get_variable_common(u16 *variable_name, - const efi_guid_t *vendor, - u32 *attributes, - efi_uintn_t *data_size, void *data, - u64 *timep); - -static efi_status_t efi_set_variable_common(u16 *variable_name, - const efi_guid_t *vendor, - u32 attributes, - efi_uintn_t data_size, - const void *data, - bool ro_check); - /* * Mapping between EFI variables and u-boot variables: * @@ -155,7 +140,7 @@ static const char *parse_attr(const char *str, u32 *attrp, u64 *timep) str++; if ((s = prefix(str, "ro"))) { - attr |= READ_ONLY; + attr |= EFI_VARIABLE_READ_ONLY; } else if ((s = prefix(str, "nv"))) { attr |= EFI_VARIABLE_NON_VOLATILE; } else if ((s = prefix(str, "boot"))) { @@ -203,29 +188,29 @@ static efi_status_t efi_set_secure_state(u8 secure_boot, u8 setup_mode, attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | - READ_ONLY; - ret = efi_set_variable_common(L"SecureBoot", &efi_global_variable_guid, - attributes, sizeof(secure_boot), - &secure_boot, false); + EFI_VARIABLE_READ_ONLY; + ret = efi_set_variable_int(L"SecureBoot", &efi_global_variable_guid, + attributes, sizeof(secure_boot), + &secure_boot, false); if (ret != EFI_SUCCESS) goto err; - ret = efi_set_variable_common(L"SetupMode", &efi_global_variable_guid, - attributes, sizeof(setup_mode), - &setup_mode, false); + ret = efi_set_variable_int(L"SetupMode", &efi_global_variable_guid, + attributes, sizeof(setup_mode), + &setup_mode, false); if (ret != EFI_SUCCESS) goto err; - ret = efi_set_variable_common(L"AuditMode", &efi_global_variable_guid, - attributes, sizeof(audit_mode), - &audit_mode, false); + ret = efi_set_variable_int(L"AuditMode", &efi_global_variable_guid, + attributes, sizeof(audit_mode), + &audit_mode, false); if (ret != EFI_SUCCESS) goto err; - ret = efi_set_variable_common(L"DeployedMode", - &efi_global_variable_guid, attributes, - sizeof(deployed_mode), &deployed_mode, - false); + ret = efi_set_variable_int(L"DeployedMode", + &efi_global_variable_guid, attributes, + sizeof(deployed_mode), &deployed_mode, + false); err: return ret; } @@ -235,7 +220,7 @@ err: * @mode: new state * * Depending on @mode, secure boot related variables are updated. - * Those variables are *read-only* for users, efi_set_variable_common() + * Those variables are *read-only* for users, efi_set_variable_int() * is called here. * * Return: status code @@ -254,10 +239,10 @@ static efi_status_t efi_transfer_secure_state(enum efi_secure_mode mode) efi_secure_boot = true; } else if (mode == EFI_MODE_AUDIT) { - ret = efi_set_variable_common(L"PK", &efi_global_variable_guid, - EFI_VARIABLE_BOOTSERVICE_ACCESS | - EFI_VARIABLE_RUNTIME_ACCESS, - 0, NULL, false); + ret = efi_set_variable_int(L"PK", &efi_global_variable_guid, + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS, + 0, NULL, false); if (ret != EFI_SUCCESS) goto err; @@ -309,8 +294,8 @@ static efi_status_t efi_init_secure_state(void) */ size = 0; - ret = efi_get_variable_common(L"PK", &efi_global_variable_guid, - NULL, &size, NULL, NULL); + ret = efi_get_variable_int(L"PK", &efi_global_variable_guid, + NULL, &size, NULL, NULL); if (ret == EFI_BUFFER_TOO_SMALL) { if (IS_ENABLED(CONFIG_EFI_SECURE_BOOT)) mode = EFI_MODE_USER; @@ -327,13 +312,13 @@ static efi_status_t efi_init_secure_state(void) ret = efi_transfer_secure_state(mode); if (ret == EFI_SUCCESS) - ret = efi_set_variable_common(L"VendorKeys", - &efi_global_variable_guid, - EFI_VARIABLE_BOOTSERVICE_ACCESS | - EFI_VARIABLE_RUNTIME_ACCESS | - READ_ONLY, - sizeof(efi_vendor_keys), - &efi_vendor_keys, false); + ret = efi_set_variable_int(L"VendorKeys", + &efi_global_variable_guid, + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS | + EFI_VARIABLE_READ_ONLY, + sizeof(efi_vendor_keys), + &efi_vendor_keys, false); err: return ret; @@ -599,11 +584,9 @@ static efi_status_t efi_variable_authenticate(u16 *variable, } #endif /* CONFIG_EFI_SECURE_BOOT */ -static efi_status_t efi_get_variable_common(u16 *variable_name, - const efi_guid_t *vendor, - u32 *attributes, - efi_uintn_t *data_size, void *data, - u64 *timep) +efi_status_t efi_get_variable_int(u16 *variable_name, const efi_guid_t *vendor, + u32 *attributes, efi_uintn_t *data_size, + void *data, u64 *timep) { char *native_name; efi_status_t ret; @@ -684,40 +667,11 @@ static efi_status_t efi_get_variable_common(u16 *variable_name, out: if (attributes) - *attributes = attr & EFI_VARIABLE_MASK; + *attributes = attr; return ret; } -/** - * efi_efi_get_variable() - retrieve value of a UEFI variable - * - * This function implements the GetVariable runtime service. - * - * See the Unified Extensible Firmware Interface (UEFI) specification for - * details. - * - * @variable_name: name of the variable - * @vendor: vendor GUID - * @attributes: attributes of the variable - * @data_size: size of the buffer to which the variable value is copied - * @data: buffer to which the variable value is copied - * Return: status code - */ -efi_status_t EFIAPI efi_get_variable(u16 *variable_name, - const efi_guid_t *vendor, u32 *attributes, - efi_uintn_t *data_size, void *data) -{ - efi_status_t ret; - - EFI_ENTRY("\"%ls\" %pUl %p %p %p", variable_name, vendor, attributes, - data_size, data); - - ret = efi_get_variable_common(variable_name, vendor, attributes, - data_size, data, NULL); - return EFI_EXIT(ret); -} - static char *efi_variables_list; static char *efi_cur_variable; @@ -881,12 +835,9 @@ efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size, return EFI_EXIT(ret); } -static efi_status_t efi_set_variable_common(u16 *variable_name, - const efi_guid_t *vendor, - u32 attributes, - efi_uintn_t data_size, - const void *data, - bool ro_check) +efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor, + u32 attributes, efi_uintn_t data_size, + const void *data, bool ro_check) { char *native_name = NULL, *old_data = NULL, *val = NULL, *s; efi_uintn_t old_size; @@ -909,15 +860,15 @@ static efi_status_t efi_set_variable_common(u16 *variable_name, /* check if a variable exists */ old_size = 0; attr = 0; - ret = efi_get_variable_common(variable_name, vendor, &attr, - &old_size, NULL, &time); + ret = efi_get_variable_int(variable_name, vendor, &attr, + &old_size, NULL, &time); append = !!(attributes & EFI_VARIABLE_APPEND_WRITE); attributes &= ~(u32)EFI_VARIABLE_APPEND_WRITE; delete = !append && (!data_size || !attributes); /* check attributes */ if (old_size) { - if (ro_check && (attr & READ_ONLY)) { + if (ro_check && (attr & EFI_VARIABLE_READ_ONLY)) { ret = EFI_WRITE_PROTECTED; goto err; } @@ -925,8 +876,8 @@ static efi_status_t efi_set_variable_common(u16 *variable_name, /* attributes won't be changed */ if (!delete && ((ro_check && attr != attributes) || - (!ro_check && ((attr & ~(u32)READ_ONLY) - != (attributes & ~(u32)READ_ONLY))))) { + (!ro_check && ((attr & ~(u32)EFI_VARIABLE_READ_ONLY) + != (attributes & ~(u32)EFI_VARIABLE_READ_ONLY))))) { ret = EFI_INVALID_PARAMETER; goto err; } @@ -1000,8 +951,8 @@ static efi_status_t efi_set_variable_common(u16 *variable_name, ret = EFI_OUT_OF_RESOURCES; goto err; } - ret = efi_get_variable_common(variable_name, vendor, - &attr, &old_size, old_data, NULL); + ret = efi_get_variable_int(variable_name, vendor, + &attr, &old_size, old_data, NULL); if (ret != EFI_SUCCESS) goto err; } else { @@ -1021,7 +972,7 @@ static efi_status_t efi_set_variable_common(u16 *variable_name, /* * store attributes */ - attributes &= (READ_ONLY | + attributes &= (EFI_VARIABLE_READ_ONLY | EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | @@ -1030,7 +981,7 @@ static efi_status_t efi_set_variable_common(u16 *variable_name, while (attributes) { attr = 1 << (ffs(attributes) - 1); - if (attr == READ_ONLY) { + if (attr == EFI_VARIABLE_READ_ONLY) { s += sprintf(s, "ro"); } else if (attr == EFI_VARIABLE_NON_VOLATILE) { s += sprintf(s, "nv"); @@ -1084,12 +1035,12 @@ out: /* update VendorKeys */ if (vendor_keys_modified & efi_vendor_keys) { efi_vendor_keys = 0; - ret = efi_set_variable_common( + ret = efi_set_variable_int( L"VendorKeys", &efi_global_variable_guid, EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS - | READ_ONLY, + | EFI_VARIABLE_READ_ONLY, sizeof(efi_vendor_keys), &efi_vendor_keys, false); @@ -1106,36 +1057,6 @@ err: return ret; } -/** - * efi_set_variable() - set value of a UEFI variable - * - * This function implements the SetVariable runtime service. - * - * See the Unified Extensible Firmware Interface (UEFI) specification for - * details. - * - * @variable_name: name of the variable - * @vendor: vendor GUID - * @attributes: attributes of the variable - * @data_size: size of the buffer with the variable value - * @data: buffer with the variable value - * Return: status code - */ -efi_status_t EFIAPI efi_set_variable(u16 *variable_name, - const efi_guid_t *vendor, u32 attributes, - efi_uintn_t data_size, const void *data) -{ - EFI_ENTRY("\"%ls\" %pUl %x %zu %p", variable_name, vendor, attributes, - data_size, data); - - /* READ_ONLY bit is not part of API */ - attributes &= ~(u32)READ_ONLY; - - return EFI_EXIT(efi_set_variable_common(variable_name, vendor, - attributes, data_size, data, - true)); -} - /** * efi_query_variable_info() - get information about EFI variables * diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c index 1d127720cd..1d6d4fe807 100644 --- a/lib/efi_loader/efi_variable_tee.c +++ b/lib/efi_loader/efi_variable_tee.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -243,24 +244,9 @@ out: return ret; } -/** - * efi_get_variable() - retrieve value of a UEFI variable - * - * This function implements the GetVariable runtime service. - * - * See the Unified Extensible Firmware Interface (UEFI) specification for - * details. - * - * @name: name of the variable - * @guid: vendor GUID - * @attr: attributes of the variable - * @data_size: size of the buffer to which the variable value is copied - * @data: buffer to which the variable value is copied - * Return: status code - */ -efi_status_t EFIAPI efi_get_variable(u16 *name, const efi_guid_t *guid, - u32 *attr, efi_uintn_t *data_size, - void *data) +efi_status_t efi_get_variable_int(u16 *variable_name, const efi_guid_t *vendor, + u32 *attributes, efi_uintn_t *data_size, + void *data, u64 *timep) { struct smm_variable_access *var_acc; efi_uintn_t payload_size; @@ -269,15 +255,13 @@ efi_status_t EFIAPI efi_get_variable(u16 *name, const efi_guid_t *guid, u8 *comm_buf = NULL; efi_status_t ret; - EFI_ENTRY("\"%ls\" %pUl %p %p %p", name, guid, attr, data_size, data); - - if (!name || !guid || !data_size) { + if (!variable_name || !vendor || !data_size) { ret = EFI_INVALID_PARAMETER; goto out; } /* Check payload size */ - name_size = u16_strsize(name); + name_size = u16_strsize(variable_name); if (name_size > max_payload_size - MM_VARIABLE_ACCESS_HEADER_SIZE) { ret = EFI_INVALID_PARAMETER; goto out; @@ -300,11 +284,11 @@ efi_status_t EFIAPI efi_get_variable(u16 *name, const efi_guid_t *guid, goto out; /* Fill in contents */ - guidcpy(&var_acc->guid, guid); + guidcpy(&var_acc->guid, vendor); var_acc->data_size = tmp_dsize; var_acc->name_size = name_size; - var_acc->attr = attr ? *attr : 0; - memcpy(var_acc->name, name, name_size); + var_acc->attr = attributes ? *attributes : 0; + memcpy(var_acc->name, variable_name, name_size); /* Communicate */ ret = mm_communicate(comm_buf, payload_size); @@ -315,8 +299,8 @@ efi_status_t EFIAPI efi_get_variable(u16 *name, const efi_guid_t *guid, if (ret != EFI_SUCCESS) goto out; - if (attr) - *attr = var_acc->attr; + if (attributes) + *attributes = var_acc->attr; if (data) memcpy(data, (u8 *)var_acc->name + var_acc->name_size, var_acc->data_size); @@ -325,7 +309,7 @@ efi_status_t EFIAPI efi_get_variable(u16 *name, const efi_guid_t *guid, out: free(comm_buf); - return EFI_EXIT(ret); + return ret; } /** @@ -415,24 +399,9 @@ out: return EFI_EXIT(ret); } -/** - * efi_set_variable() - set value of a UEFI variable - * - * This function implements the SetVariable runtime service. - * - * See the Unified Extensible Firmware Interface (UEFI) specification for - * details. - * - * @name: name of the variable - * @guid: vendor GUID - * @attr: attributes of the variable - * @data_size: size of the buffer with the variable value - * @data: buffer with the variable value - * Return: status code - */ -efi_status_t EFIAPI efi_set_variable(u16 *name, const efi_guid_t *guid, - u32 attr, efi_uintn_t data_size, - const void *data) +efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor, + u32 attributes, efi_uintn_t data_size, + const void *data, bool ro_check) { struct smm_variable_access *var_acc; efi_uintn_t payload_size; @@ -440,9 +409,7 @@ efi_status_t EFIAPI efi_set_variable(u16 *name, const efi_guid_t *guid, u8 *comm_buf = NULL; efi_status_t ret; - EFI_ENTRY("\"%ls\" %pUl %x %zu %p", name, guid, attr, data_size, data); - - if (!name || name[0] == 0 || !guid) { + if (!variable_name || variable_name[0] == 0 || !vendor) { ret = EFI_INVALID_PARAMETER; goto out; } @@ -452,7 +419,7 @@ efi_status_t EFIAPI efi_set_variable(u16 *name, const efi_guid_t *guid, } /* Check payload size */ - name_size = u16_strsize(name); + name_size = u16_strsize(variable_name); payload_size = MM_VARIABLE_ACCESS_HEADER_SIZE + name_size + data_size; if (payload_size > max_payload_size) { ret = EFI_INVALID_PARAMETER; @@ -466,11 +433,11 @@ efi_status_t EFIAPI efi_set_variable(u16 *name, const efi_guid_t *guid, goto out; /* Fill in contents */ - guidcpy(&var_acc->guid, guid); + guidcpy(&var_acc->guid, vendor); var_acc->data_size = data_size; var_acc->name_size = name_size; - var_acc->attr = attr; - memcpy(var_acc->name, name, name_size); + var_acc->attr = attributes; + memcpy(var_acc->name, variable_name, name_size); memcpy((u8 *)var_acc->name + name_size, data, data_size); /* Communicate */ @@ -478,7 +445,7 @@ efi_status_t EFIAPI efi_set_variable(u16 *name, const efi_guid_t *guid, out: free(comm_buf); - return EFI_EXIT(ret); + return ret; } /** From 2a79c352caf9bd84301dcfe8d4a08075457d33d6 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Wed, 24 Jun 2020 19:55:13 +0200 Subject: [PATCH 22/37] efi_loader: display RO attribute in printenv -e Let the 'printenv -e' command display the read only flag. If the variable is time authenticated write the time stamp. Avoid EFI_CALL() when calling SetVariable() and GetVariable(). Signed-off-by: Heinrich Schuchardt --- cmd/nvedit_efi.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/cmd/nvedit_efi.c b/cmd/nvedit_efi.c index 29cad38e19..3f61d5d6cc 100644 --- a/cmd/nvedit_efi.c +++ b/cmd/nvedit_efi.c @@ -9,11 +9,13 @@ #include #include #include +#include #include #include #include #include #include +#include #include #include @@ -34,6 +36,7 @@ static const struct { {EFI_VARIABLE_RUNTIME_ACCESS, "RT"}, {EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS, "AW"}, {EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS, "AT"}, + {EFI_VARIABLE_READ_ONLY, "RO"}, }; static const struct { @@ -87,20 +90,22 @@ static void efi_dump_single_var(u16 *name, const efi_guid_t *guid, bool verbose) { u32 attributes; u8 *data; + u64 time; + struct rtc_time tm; efi_uintn_t size; int count, i; efi_status_t ret; data = NULL; size = 0; - ret = EFI_CALL(efi_get_variable(name, guid, &attributes, &size, data)); + ret = efi_get_variable_int(name, guid, &attributes, &size, data, &time); if (ret == EFI_BUFFER_TOO_SMALL) { data = malloc(size); if (!data) goto out; - ret = EFI_CALL(efi_get_variable(name, guid, &attributes, &size, - data)); + ret = efi_get_variable_int(name, guid, &attributes, &size, + data, &time); } if (ret == EFI_NOT_FOUND) { printf("Error: \"%ls\" not defined\n", name); @@ -109,13 +114,16 @@ static void efi_dump_single_var(u16 *name, const efi_guid_t *guid, bool verbose) if (ret != EFI_SUCCESS) goto out; - printf("%ls:\n %s:", name, efi_guid_to_str(guid)); + rtc_to_tm(time, &tm); + printf("%ls:\n %s:\n", name, efi_guid_to_str(guid)); + if (attributes & EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) + printf(" %04d-%02d-%02d %02d:%02d:%02d\n", tm.tm_year, + tm.tm_mon, tm.tm_mday, tm.tm_hour, tm.tm_min, tm.tm_sec); + printf(" "); for (count = 0, i = 0; i < ARRAY_SIZE(efi_var_attrs); i++) if (attributes & efi_var_attrs[i].mask) { if (count) putc('|'); - else - putc(' '); count++; puts(efi_var_attrs[i].text); } @@ -592,8 +600,8 @@ int do_env_set_efi(struct cmd_tbl *cmdtp, int flag, int argc, p = var_name16; utf8_utf16_strncpy(&p, var_name, len + 1); - ret = EFI_CALL(efi_set_variable(var_name16, &guid, attributes, - size, value)); + ret = efi_set_variable_int(var_name16, &guid, attributes, size, value, + true); unmap_sysmem(value); if (ret == EFI_SUCCESS) { ret = CMD_RET_SUCCESS; From 01df8cf33634a36566281af475cc520c3febad4d Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Fri, 26 Jun 2020 17:57:48 +0200 Subject: [PATCH 23/37] efi_loader: separate UEFI variable API from implemementation Separate the remaining UEFI variable API functions GetNextVariableName and QueryVariableInfo() from internal functions implementing them. Signed-off-by: Heinrich Schuchardt --- include/efi_variable.h | 39 +++++++++++++++++++ lib/efi_loader/efi_var_common.c | 62 +++++++++++++++++++++++++++++++ lib/efi_loader/efi_variable.c | 56 ++++++++++++---------------- lib/efi_loader/efi_variable_tee.c | 55 +++++---------------------- 4 files changed, 133 insertions(+), 79 deletions(-) diff --git a/include/efi_variable.h b/include/efi_variable.h index 6789118eba..3ba274fce1 100644 --- a/include/efi_variable.h +++ b/include/efi_variable.h @@ -40,4 +40,43 @@ efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor, u32 attributes, efi_uintn_t data_size, const void *data, bool ro_check); +/** + * efi_get_next_variable_name_int() - enumerate the current variable names + * + * @variable_name_size: size of variable_name buffer in byte + * @variable_name: name of uefi variable's name in u16 + * @vendor: vendor's guid + * + * See the Unified Extensible Firmware Interface (UEFI) specification for + * details. + * + * Return: status code + */ +efi_status_t efi_get_next_variable_name_int(efi_uintn_t *variable_name_size, + u16 *variable_name, + efi_guid_t *vendor); + +/** + * efi_query_variable_info_int() - get information about EFI variables + * + * This function implements the QueryVariableInfo() runtime service. + * + * See the Unified Extensible Firmware Interface (UEFI) specification for + * details. + * + * @attributes: bitmask to select variables to be + * queried + * @maximum_variable_storage_size: maximum size of storage area for the + * selected variable types + * @remaining_variable_storage_size: remaining size of storage are for the + * selected variable types + * @maximum_variable_size: maximum size of a variable of the + * selected type + * Returns: status code + */ +efi_status_t efi_query_variable_info_int(u32 attributes, + u64 *maximum_variable_storage_size, + u64 *remaining_variable_storage_size, + u64 *maximum_variable_size); + #endif diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c index 6a4efa3f27..1e2be1135b 100644 --- a/lib/efi_loader/efi_var_common.c +++ b/lib/efi_loader/efi_var_common.c @@ -76,3 +76,65 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name, return EFI_EXIT(ret); } + +/** + * efi_get_next_variable_name() - enumerate the current variable names + * + * @variable_name_size: size of variable_name buffer in byte + * @variable_name: name of uefi variable's name in u16 + * @vendor: vendor's guid + * + * See the Unified Extensible Firmware Interface (UEFI) specification for + * details. + * + * Return: status code + */ +efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size, + u16 *variable_name, + efi_guid_t *vendor) +{ + efi_status_t ret; + + EFI_ENTRY("%p \"%ls\" %pUl", variable_name_size, variable_name, vendor); + + ret = efi_get_next_variable_name_int(variable_name_size, variable_name, + vendor); + + return EFI_EXIT(ret); +} + +/** + * efi_query_variable_info() - get information about EFI variables + * + * This function implements the QueryVariableInfo() runtime service. + * + * See the Unified Extensible Firmware Interface (UEFI) specification for + * details. + * + * @attributes: bitmask to select variables to be + * queried + * @maximum_variable_storage_size: maximum size of storage area for the + * selected variable types + * @remaining_variable_storage_size: remaining size of storage are for the + * selected variable types + * @maximum_variable_size: maximum size of a variable of the + * selected type + * Returns: status code + */ +efi_status_t EFIAPI efi_query_variable_info( + u32 attributes, u64 *maximum_variable_storage_size, + u64 *remaining_variable_storage_size, + u64 *maximum_variable_size) +{ + efi_status_t ret; + + EFI_ENTRY("%x %p %p %p", attributes, maximum_variable_storage_size, + remaining_variable_storage_size, maximum_variable_size); + + ret = efi_query_variable_info_int(attributes, + maximum_variable_storage_size, + remaining_variable_storage_size, + maximum_variable_size); + + return EFI_EXIT(ret); +} diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index 6ec1f97326..36bac86702 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -745,23 +745,9 @@ static efi_status_t parse_uboot_variable(char *variable, return EFI_SUCCESS; } -/** - * efi_get_next_variable_name() - enumerate the current variable names - * - * @variable_name_size: size of variable_name buffer in byte - * @variable_name: name of uefi variable's name in u16 - * @vendor: vendor's guid - * - * This function implements the GetNextVariableName service. - * - * See the Unified Extensible Firmware Interface (UEFI) specification for - * details. - * - * Return: status code - */ -efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size, - u16 *variable_name, - efi_guid_t *vendor) +efi_status_t efi_get_next_variable_name_int(efi_uintn_t *variable_name_size, + u16 *variable_name, + efi_guid_t *vendor) { char *native_name, *variable; ssize_t name_len, list_len; @@ -771,10 +757,8 @@ efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size, int i; efi_status_t ret; - EFI_ENTRY("%p \"%ls\" %pUl", variable_name_size, variable_name, vendor); - if (!variable_name_size || !variable_name || !vendor) - return EFI_EXIT(EFI_INVALID_PARAMETER); + return EFI_INVALID_PARAMETER; if (variable_name[0]) { /* check null-terminated string */ @@ -782,12 +766,12 @@ efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size, if (!variable_name[i]) break; if (i >= *variable_name_size) - return EFI_EXIT(EFI_INVALID_PARAMETER); + return EFI_INVALID_PARAMETER; /* search for the last-returned variable */ ret = efi_to_native(&native_name, variable_name, vendor); if (ret) - return EFI_EXIT(ret); + return ret; name_len = strlen(native_name); for (variable = efi_variables_list; variable && *variable;) { @@ -802,14 +786,14 @@ efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size, free(native_name); if (!(variable && *variable)) - return EFI_EXIT(EFI_INVALID_PARAMETER); + return EFI_INVALID_PARAMETER; /* next variable */ variable = strchr(variable, '\n'); if (variable) variable++; if (!(variable && *variable)) - return EFI_EXIT(EFI_NOT_FOUND); + return EFI_NOT_FOUND; } else { /* *new search: free a list used in the previous search @@ -824,7 +808,7 @@ efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size, &efi_variables_list, 0, 1, regexlist); if (list_len <= 1) - return EFI_EXIT(EFI_NOT_FOUND); + return EFI_NOT_FOUND; variable = efi_variables_list; } @@ -832,7 +816,7 @@ efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size, ret = parse_uboot_variable(variable, variable_name_size, variable_name, vendor, &attributes); - return EFI_EXIT(ret); + return ret; } efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor, @@ -1057,13 +1041,17 @@ err: return ret; } +efi_status_t efi_query_variable_info_int(u32 attributes, + u64 *maximum_variable_storage_size, + u64 *remaining_variable_storage_size, + u64 *maximum_variable_size) +{ + return EFI_UNSUPPORTED; +} + /** - * efi_query_variable_info() - get information about EFI variables - * - * This function implements the QueryVariableInfo() runtime service. - * - * See the Unified Extensible Firmware Interface (UEFI) specification for - * details. + * efi_query_variable_info_runtime() - runtime implementation of + * QueryVariableInfo() * * @attributes: bitmask to select variables to be * queried @@ -1075,7 +1063,7 @@ err: * selected type * Returns: status code */ -efi_status_t __efi_runtime EFIAPI efi_query_variable_info( +efi_status_t __efi_runtime EFIAPI efi_query_variable_info_runtime( u32 attributes, u64 *maximum_variable_storage_size, u64 *remaining_variable_storage_size, @@ -1144,6 +1132,8 @@ void efi_variables_boot_exit_notify(void) efi_runtime_services.get_next_variable_name = efi_get_next_variable_name_runtime; efi_runtime_services.set_variable = efi_set_variable_runtime; + efi_runtime_services.query_variable_info = + efi_query_variable_info_runtime; efi_update_table_header_crc32(&efi_runtime_services.hdr); } diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c index 1d6d4fe807..ff90aa8e81 100644 --- a/lib/efi_loader/efi_variable_tee.c +++ b/lib/efi_loader/efi_variable_tee.c @@ -312,23 +312,9 @@ out: return ret; } -/** - * efi_get_next_variable_name() - enumerate the current variable names - * - * @variable_name_size: size of variable_name buffer in bytes - * @variable_name: name of uefi variable's name in u16 - * @guid: vendor's guid - * - * This function implements the GetNextVariableName service. - * - * See the Unified Extensible Firmware Interface (UEFI) specification for - * details. - * - * Return: status code - */ -efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size, - u16 *variable_name, - efi_guid_t *guid) +efi_status_t efi_get_next_variable_name_int(efi_uintn_t *variable_name_size, + u16 *variable_name, + efi_guid_t *guid) { struct smm_variable_getnext *var_getnext; efi_uintn_t payload_size; @@ -338,8 +324,6 @@ efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size, u8 *comm_buf = NULL; efi_status_t ret; - EFI_ENTRY("%p \"%ls\" %pUl", variable_name_size, variable_name, guid); - if (!variable_name_size || !variable_name || !guid) { ret = EFI_INVALID_PARAMETER; goto out; @@ -396,7 +380,7 @@ efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size, out: free(comm_buf); - return EFI_EXIT(ret); + return ret; } efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor, @@ -448,37 +432,16 @@ out: return ret; } -/** - * efi_query_variable_info() - get information about EFI variables - * - * This function implements the QueryVariableInfo() runtime service. - * - * See the Unified Extensible Firmware Interface (UEFI) specification for - * details. - * - * @attributes: bitmask to select variables to be - * queried - * @maximum_variable_storage_size: maximum size of storage area for the - * selected variable types - * @remaining_variable_storage_size: remaining size of storage are for the - * selected variable types - * @maximum_variable_size: maximum size of a variable of the - * selected type - * Returns: status code - */ -efi_status_t EFIAPI __efi_runtime -efi_query_variable_info(u32 attributes, u64 *max_variable_storage_size, - u64 *remain_variable_storage_size, - u64 *max_variable_size) +efi_status_t efi_query_variable_info_int(u32 attributes, + u64 *max_variable_storage_size, + u64 *remain_variable_storage_size, + u64 *max_variable_size) { struct smm_variable_query_info *mm_query_info; efi_uintn_t payload_size; efi_status_t ret; u8 *comm_buf; - EFI_ENTRY("%x %p %p %p", attributes, max_variable_storage_size, - remain_variable_storage_size, max_variable_size); - payload_size = sizeof(*mm_query_info); comm_buf = setup_mm_hdr((void **)&mm_query_info, payload_size, SMM_VARIABLE_FUNCTION_QUERY_VARIABLE_INFO, @@ -497,7 +460,7 @@ efi_query_variable_info(u32 attributes, u64 *max_variable_storage_size, out: free(comm_buf); - return EFI_EXIT(ret); + return ret; } /** From dc90e5c491fb975c577f4cd874052c8b91078915 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Wed, 24 Jun 2020 19:38:29 +0200 Subject: [PATCH 24/37] efi_loader: OsIndicationsSupported, PlatformLangCodes UEFI variables OsIndicationsSupported, PlatformLangCodes should be read only. Avoid EFI_CALL() for SetVariable(). Signed-off-by: Heinrich Schuchardt --- lib/efi_loader/efi_setup.c | 59 ++++++++++++++++++++------------------ 1 file changed, 31 insertions(+), 28 deletions(-) diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c index a3b05a4a9b..6196c0a06c 100644 --- a/lib/efi_loader/efi_setup.c +++ b/lib/efi_loader/efi_setup.c @@ -8,6 +8,7 @@ #include #include #include +#include #define OBJ_LIST_NOT_INITIALIZED 1 @@ -40,12 +41,13 @@ static efi_status_t efi_init_platform_lang(void) * Variable PlatformLangCodes defines the language codes that the * machine can support. */ - ret = EFI_CALL(efi_set_variable(L"PlatformLangCodes", - &efi_global_variable_guid, - EFI_VARIABLE_BOOTSERVICE_ACCESS | - EFI_VARIABLE_RUNTIME_ACCESS, - sizeof(CONFIG_EFI_PLATFORM_LANG_CODES), - CONFIG_EFI_PLATFORM_LANG_CODES)); + ret = efi_set_variable_int(L"PlatformLangCodes", + &efi_global_variable_guid, + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS | + EFI_VARIABLE_READ_ONLY, + sizeof(CONFIG_EFI_PLATFORM_LANG_CODES), + CONFIG_EFI_PLATFORM_LANG_CODES, false); if (ret != EFI_SUCCESS) goto out; @@ -53,9 +55,9 @@ static efi_status_t efi_init_platform_lang(void) * Variable PlatformLang defines the language that the machine has been * configured for. */ - ret = EFI_CALL(efi_get_variable(L"PlatformLang", - &efi_global_variable_guid, - NULL, &data_size, &pos)); + ret = efi_get_variable_int(L"PlatformLang", + &efi_global_variable_guid, + NULL, &data_size, &pos, NULL); if (ret == EFI_BUFFER_TOO_SMALL) { /* The variable is already set. Do not change it. */ ret = EFI_SUCCESS; @@ -70,12 +72,12 @@ static efi_status_t efi_init_platform_lang(void) if (pos) *pos = 0; - ret = EFI_CALL(efi_set_variable(L"PlatformLang", - &efi_global_variable_guid, - EFI_VARIABLE_NON_VOLATILE | - EFI_VARIABLE_BOOTSERVICE_ACCESS | - EFI_VARIABLE_RUNTIME_ACCESS, - 1 + strlen(lang), lang)); + ret = efi_set_variable_int(L"PlatformLang", + &efi_global_variable_guid, + EFI_VARIABLE_NON_VOLATILE | + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS, + 1 + strlen(lang), lang, false); out: if (ret != EFI_SUCCESS) printf("EFI: cannot initialize platform language settings\n"); @@ -96,13 +98,13 @@ static efi_status_t efi_init_secure_boot(void) }; efi_status_t ret; - /* TODO: read-only */ - ret = EFI_CALL(efi_set_variable(L"SignatureSupport", - &efi_global_variable_guid, - EFI_VARIABLE_BOOTSERVICE_ACCESS - | EFI_VARIABLE_RUNTIME_ACCESS, - sizeof(signature_types), - &signature_types)); + ret = efi_set_variable_int(L"SignatureSupport", + &efi_global_variable_guid, + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS | + EFI_VARIABLE_READ_ONLY, + sizeof(signature_types), + &signature_types, false); if (ret != EFI_SUCCESS) printf("EFI: cannot initialize SignatureSupport variable\n"); @@ -160,12 +162,13 @@ efi_status_t efi_init_obj_list(void) goto out; /* Indicate supported features */ - ret = EFI_CALL(efi_set_variable(L"OsIndicationsSupported", - &efi_global_variable_guid, - EFI_VARIABLE_BOOTSERVICE_ACCESS | - EFI_VARIABLE_RUNTIME_ACCESS, - sizeof(os_indications_supported), - &os_indications_supported)); + ret = efi_set_variable_int(L"OsIndicationsSupported", + &efi_global_variable_guid, + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS | + EFI_VARIABLE_READ_ONLY, + sizeof(os_indications_supported), + &os_indications_supported, false); if (ret != EFI_SUCCESS) goto out; From dda8c7163a2b4f3d30f54cd3553b2c81bc64e787 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Wed, 24 Jun 2020 19:09:18 +0200 Subject: [PATCH 25/37] efi_loader: simplify boot manager Simplify the implementation of the UEFI boot manager: * avoid EFI_CALL for SetVariable() and GetVariable() * remove unnecessary type conversions Signed-off-by: Heinrich Schuchardt --- lib/efi_loader/efi_bootmgr.c | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index e268e9c4b8..e03198b57a 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -12,6 +12,7 @@ #include #include #include +#include #include static const struct efi_boot_services *bs; @@ -147,15 +148,14 @@ unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8 **data) static void *get_var(u16 *name, const efi_guid_t *vendor, efi_uintn_t *size) { - efi_guid_t *v = (efi_guid_t *)vendor; efi_status_t ret; void *buf = NULL; *size = 0; - EFI_CALL(ret = rs->get_variable(name, v, NULL, size, buf)); + ret = efi_get_variable_int(name, vendor, NULL, size, buf, NULL); if (ret == EFI_BUFFER_TOO_SMALL) { buf = malloc(*size); - EFI_CALL(ret = rs->get_variable(name, v, NULL, size, buf)); + ret = efi_get_variable_int(name, vendor, NULL, size, buf, NULL); } if (ret != EFI_SUCCESS) { @@ -219,10 +219,9 @@ static efi_status_t try_load_entry(u16 n, efi_handle_t *handle) attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS; size = sizeof(n); - ret = EFI_CALL(efi_set_variable( - L"BootCurrent", - (efi_guid_t *)&efi_global_variable_guid, - attributes, size, &n)); + ret = efi_set_variable_int(L"BootCurrent", + &efi_global_variable_guid, + attributes, size, &n, false); if (ret != EFI_SUCCESS) { if (EFI_CALL(efi_unload_image(*handle)) != EFI_SUCCESS) @@ -262,22 +261,19 @@ efi_status_t efi_bootmgr_load(efi_handle_t *handle) rs = systab.runtime; /* BootNext */ - bootnext = 0; size = sizeof(bootnext); - ret = EFI_CALL(efi_get_variable(L"BootNext", - (efi_guid_t *)&efi_global_variable_guid, - NULL, &size, &bootnext)); + ret = efi_get_variable_int(L"BootNext", + &efi_global_variable_guid, + NULL, &size, &bootnext, NULL); if (ret == EFI_SUCCESS || ret == EFI_BUFFER_TOO_SMALL) { /* BootNext does exist here */ if (ret == EFI_BUFFER_TOO_SMALL || size != sizeof(u16)) log_err("BootNext must be 16-bit integer\n"); /* delete BootNext */ - ret = EFI_CALL(efi_set_variable( - L"BootNext", - (efi_guid_t *)&efi_global_variable_guid, - EFI_VARIABLE_NON_VOLATILE, 0, - &bootnext)); + ret = efi_set_variable_int(L"BootNext", + &efi_global_variable_guid, + 0, 0, NULL, false); /* load BootNext */ if (ret == EFI_SUCCESS) { From 37fa7cb045cc15b3a22377fa2e3fb00b2d132d7a Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Wed, 17 Jun 2020 12:20:46 +0200 Subject: [PATCH 26/37] efi_loader: keep attributes in efi_set_variable_int Do not change the value of parameter attributes in function efi_set_variable_int(). This allows to use it later. Do not use variable attr for different purposes but declare separate variables (attr and old_attr). Signed-off-by: Heinrich Schuchardt --- lib/efi_loader/efi_variable.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index 36bac86702..c9980ca692 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -827,7 +827,7 @@ efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor, efi_uintn_t old_size; bool append, delete; u64 time = 0; - u32 attr; + u32 old_attr; efi_status_t ret = EFI_SUCCESS; if (!variable_name || !*variable_name || !vendor || @@ -843,8 +843,8 @@ efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor, /* check if a variable exists */ old_size = 0; - attr = 0; - ret = efi_get_variable_int(variable_name, vendor, &attr, + old_attr = 0; + ret = efi_get_variable_int(variable_name, vendor, &old_attr, &old_size, NULL, &time); append = !!(attributes & EFI_VARIABLE_APPEND_WRITE); attributes &= ~(u32)EFI_VARIABLE_APPEND_WRITE; @@ -852,15 +852,15 @@ efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor, /* check attributes */ if (old_size) { - if (ro_check && (attr & EFI_VARIABLE_READ_ONLY)) { + if (ro_check && (old_attr & EFI_VARIABLE_READ_ONLY)) { ret = EFI_WRITE_PROTECTED; goto err; } /* attributes won't be changed */ if (!delete && - ((ro_check && attr != attributes) || - (!ro_check && ((attr & ~(u32)EFI_VARIABLE_READ_ONLY) + ((ro_check && old_attr != attributes) || + (!ro_check && ((old_attr & ~(u32)EFI_VARIABLE_READ_ONLY) != (attributes & ~(u32)EFI_VARIABLE_READ_ONLY))))) { ret = EFI_INVALID_PARAMETER; goto err; @@ -902,7 +902,7 @@ efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor, EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) { ret = efi_variable_authenticate(variable_name, vendor, &data_size, &data, - attributes, &attr, + attributes, &old_attr, &time); if (ret != EFI_SUCCESS) goto err; @@ -936,7 +936,7 @@ efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor, goto err; } ret = efi_get_variable_int(variable_name, vendor, - &attr, &old_size, old_data, NULL); + &old_attr, &old_size, old_data, NULL); if (ret != EFI_SUCCESS) goto err; } else { @@ -962,8 +962,8 @@ efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor, EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS); s += sprintf(s, "{"); - while (attributes) { - attr = 1 << (ffs(attributes) - 1); + for (u32 attr_rem = attributes; attr_rem;) { + u32 attr = 1 << (ffs(attr_rem) - 1); if (attr == EFI_VARIABLE_READ_ONLY) { s += sprintf(s, "ro"); @@ -979,8 +979,8 @@ efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor, s = bin2hex(s, (u8 *)&time, sizeof(time)); } - attributes &= ~attr; - if (attributes) + attr_rem &= ~attr; + if (attr_rem) s += sprintf(s, ","); } s += sprintf(s, "}"); From fd7b6730aba4540fef8d59cfe10a7dce2649cd8e Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Sat, 4 Jul 2020 22:41:26 +0200 Subject: [PATCH 27/37] efi_loader: value of VendorKeys According to the UEFI specification the variable VendorKeys is 1 if the "system is configured to use only vendor-provided keys". As we do not supply any vendor keys yet the variable VendorKeys must be zero. Signed-off-by: Heinrich Schuchardt --- lib/efi_loader/efi_variable.c | 75 +++++++---------------------------- 1 file changed, 15 insertions(+), 60 deletions(-) diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index c9980ca692..2f8005bd95 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -282,45 +282,29 @@ err: */ static efi_status_t efi_init_secure_state(void) { - enum efi_secure_mode mode; - efi_uintn_t size; + enum efi_secure_mode mode = EFI_MODE_SETUP; + efi_uintn_t size = 0; efi_status_t ret; - /* - * TODO: - * Since there is currently no "platform-specific" installation - * method of Platform Key, we can't say if VendorKeys is 0 or 1 - * precisely. - */ - - size = 0; ret = efi_get_variable_int(L"PK", &efi_global_variable_guid, NULL, &size, NULL, NULL); if (ret == EFI_BUFFER_TOO_SMALL) { if (IS_ENABLED(CONFIG_EFI_SECURE_BOOT)) mode = EFI_MODE_USER; - else - mode = EFI_MODE_SETUP; - - efi_vendor_keys = 0; - } else if (ret == EFI_NOT_FOUND) { - mode = EFI_MODE_SETUP; - efi_vendor_keys = 1; - } else { - goto err; } ret = efi_transfer_secure_state(mode); - if (ret == EFI_SUCCESS) - ret = efi_set_variable_int(L"VendorKeys", - &efi_global_variable_guid, - EFI_VARIABLE_BOOTSERVICE_ACCESS | - EFI_VARIABLE_RUNTIME_ACCESS | - EFI_VARIABLE_READ_ONLY, - sizeof(efi_vendor_keys), - &efi_vendor_keys, false); + if (ret != EFI_SUCCESS) + return ret; -err: + /* As we do not provide vendor keys this variable is always 0. */ + ret = efi_set_variable_int(L"VendorKeys", + &efi_global_variable_guid, + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS | + EFI_VARIABLE_READ_ONLY, + sizeof(efi_vendor_keys), + &efi_vendor_keys, false); return ret; } @@ -998,39 +982,10 @@ out: if (env_set(native_name, val)) { ret = EFI_DEVICE_ERROR; } else { - bool vendor_keys_modified = false; - - if ((u16_strcmp(variable_name, L"PK") == 0 && - guidcmp(vendor, &efi_global_variable_guid) == 0)) { - ret = efi_transfer_secure_state( - (delete ? EFI_MODE_SETUP : - EFI_MODE_USER)); - if (ret != EFI_SUCCESS) - goto err; - - if (efi_secure_mode != EFI_MODE_SETUP) - vendor_keys_modified = true; - } else if ((u16_strcmp(variable_name, L"KEK") == 0 && - guidcmp(vendor, &efi_global_variable_guid) == 0)) { - if (efi_secure_mode != EFI_MODE_SETUP) - vendor_keys_modified = true; - } - - /* update VendorKeys */ - if (vendor_keys_modified & efi_vendor_keys) { - efi_vendor_keys = 0; - ret = efi_set_variable_int( - L"VendorKeys", - &efi_global_variable_guid, - EFI_VARIABLE_BOOTSERVICE_ACCESS - | EFI_VARIABLE_RUNTIME_ACCESS - | EFI_VARIABLE_READ_ONLY, - sizeof(efi_vendor_keys), - &efi_vendor_keys, - false); - } else { + if (!u16_strcmp(variable_name, L"PK")) + ret = efi_init_secure_state(); + else ret = EFI_SUCCESS; - } } err: From 55a830560efafd8ae5a61a2beaaece6701e077c6 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Sat, 4 Jul 2020 18:34:15 +0200 Subject: [PATCH 28/37] efi_loader: read-only AuditMode and DeployedMode Set the read only property of the UEFI variables AuditMode and DeployedMode conforming to the UEFI specification. Signed-off-by: Heinrich Schuchardt --- lib/efi_loader/efi_variable.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index 2f8005bd95..4bd976e44a 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -183,32 +183,36 @@ static const char *parse_attr(const char *str, u32 *attrp, u64 *timep) static efi_status_t efi_set_secure_state(u8 secure_boot, u8 setup_mode, u8 audit_mode, u8 deployed_mode) { - u32 attributes; efi_status_t ret; + const u32 attributes_ro = EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS | + EFI_VARIABLE_READ_ONLY; + const u32 attributes_rw = EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS; - attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS | - EFI_VARIABLE_RUNTIME_ACCESS | - EFI_VARIABLE_READ_ONLY; ret = efi_set_variable_int(L"SecureBoot", &efi_global_variable_guid, - attributes, sizeof(secure_boot), + attributes_ro, sizeof(secure_boot), &secure_boot, false); if (ret != EFI_SUCCESS) goto err; ret = efi_set_variable_int(L"SetupMode", &efi_global_variable_guid, - attributes, sizeof(setup_mode), + attributes_ro, sizeof(setup_mode), &setup_mode, false); if (ret != EFI_SUCCESS) goto err; ret = efi_set_variable_int(L"AuditMode", &efi_global_variable_guid, - attributes, sizeof(audit_mode), - &audit_mode, false); + audit_mode || setup_mode ? + attributes_ro : attributes_rw, + sizeof(audit_mode), &audit_mode, false); if (ret != EFI_SUCCESS) goto err; ret = efi_set_variable_int(L"DeployedMode", - &efi_global_variable_guid, attributes, + &efi_global_variable_guid, + audit_mode || deployed_mode || setup_mode ? + attributes_ro : attributes_rw, sizeof(deployed_mode), &deployed_mode, false); err: From 198bf6418efa282ddb9392d352a2afef20585020 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Sun, 5 Jul 2020 02:29:50 +0200 Subject: [PATCH 29/37] efi_loader: secure boot flag In audit mode the UEFI variable SecureBoot is set to zero but the efi_secure_boot flag is set to true. The efi_secure_boot flag should match the UEFIvariable SecureBoot. Signed-off-by: Heinrich Schuchardt --- lib/efi_loader/efi_variable.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index 4bd976e44a..0d6bafc76d 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -190,6 +190,8 @@ static efi_status_t efi_set_secure_state(u8 secure_boot, u8 setup_mode, const u32 attributes_rw = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS; + efi_secure_boot = secure_boot; + ret = efi_set_variable_int(L"SecureBoot", &efi_global_variable_guid, attributes_ro, sizeof(secure_boot), &secure_boot, false); @@ -240,8 +242,6 @@ static efi_status_t efi_transfer_secure_state(enum efi_secure_mode mode) ret = efi_set_secure_state(1, 0, 0, 1); if (ret != EFI_SUCCESS) goto err; - - efi_secure_boot = true; } else if (mode == EFI_MODE_AUDIT) { ret = efi_set_variable_int(L"PK", &efi_global_variable_guid, EFI_VARIABLE_BOOTSERVICE_ACCESS | @@ -253,14 +253,10 @@ static efi_status_t efi_transfer_secure_state(enum efi_secure_mode mode) ret = efi_set_secure_state(0, 1, 1, 0); if (ret != EFI_SUCCESS) goto err; - - efi_secure_boot = true; } else if (mode == EFI_MODE_USER) { ret = efi_set_secure_state(1, 0, 0, 0); if (ret != EFI_SUCCESS) goto err; - - efi_secure_boot = true; } else if (mode == EFI_MODE_SETUP) { ret = efi_set_secure_state(0, 1, 0, 0); if (ret != EFI_SUCCESS) From 5f7dcf079de8caa5ef9c332c928bd6a3c3d12335 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Thu, 19 Mar 2020 18:21:58 +0000 Subject: [PATCH 30/37] efi_loader: UEFI variable persistence Persist non-volatile UEFI variables in a file on the EFI system partition. The file is written whenever a non-volatile UEFI variable is changed after initialization of the UEFI sub-system. The file is read during the UEFI sub-system initialization to restore non-volatile UEFI variables. Signed-off-by: Heinrich Schuchardt --- include/efi_variable.h | 62 +++++++++ lib/efi_loader/Kconfig | 8 ++ lib/efi_loader/Makefile | 1 + lib/efi_loader/efi_var_file.c | 239 ++++++++++++++++++++++++++++++++++ lib/efi_loader/efi_variable.c | 10 +- 5 files changed, 319 insertions(+), 1 deletion(-) create mode 100644 lib/efi_loader/efi_var_file.c diff --git a/include/efi_variable.h b/include/efi_variable.h index 3ba274fce1..01054209c4 100644 --- a/include/efi_variable.h +++ b/include/efi_variable.h @@ -79,4 +79,66 @@ efi_status_t efi_query_variable_info_int(u32 attributes, u64 *remaining_variable_storage_size, u64 *maximum_variable_size); +#define EFI_VAR_FILE_NAME "ubootefi.var" + +#define EFI_VAR_BUF_SIZE 0x4000 + +#define EFI_VAR_FILE_MAGIC 0x0161566966456255 /* UbEfiVa, version 1 */ + +/** + * struct efi_var_entry - UEFI variable file entry + * + * @length: length of enty, multiple of 8 + * @attr: variable attributes + * @time: authentication time (seconds since start of epoch) + * @guid: vendor GUID + * @name: UTF16 variable name + */ +struct efi_var_entry { + u32 length; + u32 attr; + u64 time; + efi_guid_t guid; + u16 name[]; +}; + +/** + * struct efi_var_file - file for storing UEFI variables + * + * @reserved: unused, may be overwritten by memory probing + * @magic: identifies file format + * @length: length including header + * @crc32: CRC32 without header + * @var: variables + */ +struct efi_var_file { + u64 reserved; + u64 magic; + u32 length; + u32 crc32; + struct efi_var_entry var[]; +}; + +/** + * efi_var_to_file() - save non-volatile variables as file + * + * File ubootefi.var is created on the EFI system partion. + * + * Return: status code + */ +efi_status_t efi_var_to_file(void); + +/** + * efi_var_from_file() - read variables from file + * + * File ubootefi.var is read from the EFI system partitions and the variables + * stored in the file are created. + * + * In case the file does not exist yet or a variable cannot be set EFI_SUCCESS + * is returned. + * + * Return: status code + */ +efi_status_t efi_var_from_file(void); + #endif diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index 6c9df3a767..4324694d48 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -27,6 +27,14 @@ config EFI_LOADER if EFI_LOADER +config EFI_VARIABLE_FILE_STORE + bool "Store non-volatile UEFI variables as file" + depends on FAT_WRITE + default y + help + Select tis option if you want non-volatile UEFI variables to be stored + as file /ubootefi.var on the EFI system partition. + config EFI_GET_TIME bool "GetTime() runtime service" depends on DM_RTC diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile index 7eddd7ef37..c87b82db32 100644 --- a/lib/efi_loader/Makefile +++ b/lib/efi_loader/Makefile @@ -40,6 +40,7 @@ ifeq ($(CONFIG_EFI_MM_COMM_TEE),y) obj-y += efi_variable_tee.o else obj-y += efi_variable.o +obj-y += efi_var_file.o endif obj-y += efi_watchdog.o obj-$(CONFIG_LCD) += efi_gop.o diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c new file mode 100644 index 0000000000..b1b7532495 --- /dev/null +++ b/lib/efi_loader/efi_var_file.c @@ -0,0 +1,239 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * File interface for UEFI variables + * + * Copyright (c) 2020, Heinrich Schuchardt + */ + +#define LOG_CATEGORY LOGC_EFI + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define PART_STR_LEN 10 + +/** + * efi_set_blk_dev_to_system_partition() - select EFI system partition + * + * Set the EFI system partition as current block device. + * + * Return: status code + */ +static efi_status_t __maybe_unused efi_set_blk_dev_to_system_partition(void) +{ + char part_str[PART_STR_LEN]; + int r; + + if (!efi_system_partition.if_type) { + log_err("No EFI system partition\n"); + return EFI_DEVICE_ERROR; + } + snprintf(part_str, PART_STR_LEN, "%u:%u", + efi_system_partition.devnum, efi_system_partition.part); + r = fs_set_blk_dev(blk_get_if_type_name(efi_system_partition.if_type), + part_str, FS_TYPE_ANY); + if (r) { + log_err("Cannot read EFI system partition\n"); + return EFI_DEVICE_ERROR; + } + return EFI_SUCCESS; +} + +/** + * efi_var_collect() - collect non-volatile variables in buffer + * + * A buffer is allocated and filled with all non-volatile variables in a + * format ready to be written to disk. + * + * @bufp: pointer to pointer of buffer with collected variables + * @lenp: pointer to length of buffer + * Return: status code + */ +static efi_status_t __maybe_unused efi_var_collect(struct efi_var_file **bufp, + loff_t *lenp) +{ + size_t len = EFI_VAR_BUF_SIZE; + struct efi_var_file *buf; + struct efi_var_entry *var, *old_var; + size_t old_var_name_length = 2; + + *bufp = NULL; /* Avoid double free() */ + buf = calloc(1, len); + if (!buf) + return EFI_OUT_OF_RESOURCES; + var = buf->var; + old_var = var; + for (;;) { + efi_uintn_t data_length, var_name_length; + u8 *data; + efi_status_t ret; + + if ((uintptr_t)buf + len <= + (uintptr_t)var->name + old_var_name_length) + return EFI_BUFFER_TOO_SMALL; + + var_name_length = (uintptr_t)buf + len - (uintptr_t)var->name; + memcpy(var->name, old_var->name, old_var_name_length); + guidcpy(&var->guid, &old_var->guid); + ret = efi_get_next_variable_name_int( + &var_name_length, var->name, &var->guid); + if (ret == EFI_NOT_FOUND) + break; + if (ret != EFI_SUCCESS) { + free(buf); + return ret; + } + old_var_name_length = var_name_length; + old_var = var; + + data = (u8 *)var->name + old_var_name_length; + data_length = (uintptr_t)buf + len - (uintptr_t)data; + ret = efi_get_variable_int(var->name, &var->guid, + &var->attr, &data_length, data, + &var->time); + if (ret != EFI_SUCCESS) { + free(buf); + return ret; + } + if (!(var->attr & EFI_VARIABLE_NON_VOLATILE)) + continue; + var->length = data_length; + var = (struct efi_var_entry *) + ALIGN((uintptr_t)data + data_length, 8); + } + + buf->reserved = 0; + buf->magic = EFI_VAR_FILE_MAGIC; + len = (uintptr_t)var - (uintptr_t)buf; + buf->crc32 = crc32(0, (u8 *)buf->var, + len - sizeof(struct efi_var_file)); + buf->length = len; + *bufp = buf; + *lenp = len; + + return EFI_SUCCESS; +} + +/** + * efi_var_to_file() - save non-volatile variables as file + * + * File ubootefi.var is created on the EFI system partion. + * + * Return: status code + */ +efi_status_t efi_var_to_file(void) +{ +#ifdef CONFIG_EFI_VARIABLE_FILE_STORE + efi_status_t ret; + struct efi_var_file *buf; + loff_t len; + loff_t actlen; + int r; + + ret = efi_var_collect(&buf, &len); + if (ret != EFI_SUCCESS) + goto error; + + ret = efi_set_blk_dev_to_system_partition(); + if (ret != EFI_SUCCESS) + goto error; + + r = fs_write(EFI_VAR_FILE_NAME, map_to_sysmem(buf), 0, len, &actlen); + if (r || len != actlen) + ret = EFI_DEVICE_ERROR; + +error: + if (ret != EFI_SUCCESS) + log_err("Failed to persist EFI variables\n"); + free(buf); + return ret; +#else + return EFI_SUCCESS; +#endif +} + +/** + * efi_var_restore() - restore EFI variables from buffer + * + * @buf: buffer + * Return: status code + */ +static efi_status_t __maybe_unused efi_var_restore(struct efi_var_file *buf) +{ + struct efi_var_entry *var, *last_var; + efi_status_t ret; + + if (buf->reserved || buf->magic != EFI_VAR_FILE_MAGIC || + buf->crc32 != crc32(0, (u8 *)buf->var, + buf->length - sizeof(struct efi_var_file))) { + log_err("Invalid EFI variables file\n"); + return EFI_INVALID_PARAMETER; + } + + var = buf->var; + last_var = (struct efi_var_entry *)((u8 *)buf + buf->length); + while (var < last_var) { + u16 *data = var->name + u16_strlen(var->name) + 1; + + if (var->attr & EFI_VARIABLE_NON_VOLATILE && var->length) { + ret = efi_set_variable_int(var->name, &var->guid, + var->attr, var->length, + data, true); + if (ret != EFI_SUCCESS) + log_err("Failed to set EFI variable %ls\n", + var->name); + } + var = (struct efi_var_entry *) + ALIGN((uintptr_t)data + var->length, 8); + } + return EFI_SUCCESS; +} + +/** + * efi_var_from_file() - read variables from file + * + * File ubootefi.var is read from the EFI system partitions and the variables + * stored in the file are created. + * + * In case the file does not exist yet or a variable cannot be set EFI_SUCCESS + * is returned. + * + * Return: status code + */ +efi_status_t efi_var_from_file(void) +{ +#ifdef CONFIG_EFI_VARIABLE_FILE_STORE + struct efi_var_file *buf; + loff_t len; + efi_status_t ret; + int r; + + buf = calloc(1, EFI_VAR_BUF_SIZE); + if (!buf) { + log_err("Out of memory\n"); + return EFI_OUT_OF_RESOURCES; + } + + ret = efi_set_blk_dev_to_system_partition(); + if (ret != EFI_SUCCESS) + goto error; + r = fs_read(EFI_VAR_FILE_NAME, map_to_sysmem(buf), 0, EFI_VAR_BUF_SIZE, + &len); + if (r || len < sizeof(struct efi_var_file)) { + log_err("Failed to load EFI variables\n"); + goto error; + } + if (buf->length != len || efi_var_restore(buf) != EFI_SUCCESS) + log_err("Invalid EFI variables file\n"); +error: + free(buf); +#endif + return EFI_SUCCESS; +} diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index 0d6bafc76d..8ed4b0830b 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -988,6 +988,11 @@ out: ret = EFI_SUCCESS; } + /* Write non-volatile EFI variables to file */ + if (attributes & EFI_VARIABLE_NON_VOLATILE && + ret == EFI_SUCCESS && efi_obj_list_initialized == EFI_SUCCESS) + efi_var_to_file(); + err: free(native_name); free(old_data); @@ -1083,6 +1088,7 @@ efi_set_variable_runtime(u16 *variable_name, const efi_guid_t *vendor, */ void efi_variables_boot_exit_notify(void) { + /* Switch variable services functions to runtime version */ efi_runtime_services.get_variable = efi_get_variable_runtime; efi_runtime_services.get_next_variable_name = efi_get_next_variable_name_runtime; @@ -1102,6 +1108,8 @@ efi_status_t efi_init_variables(void) efi_status_t ret; ret = efi_init_secure_state(); + if (ret != EFI_SUCCESS) + return ret; - return ret; + return efi_var_from_file(); } From a44d2a23992ba70dfeefbad9c55249bfc6be9fbc Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Tue, 24 Mar 2020 18:05:22 +0100 Subject: [PATCH 31/37] efi_loader: export efi_convert_pointer() We need ConvertPointer() to adjust pointers when implementing runtime services within U-Boot. After ExitBootServices() gd is not available anymore. So we should not use EFI_ENTRY() and EFI_EXIT(). Signed-off-by: Heinrich Schuchardt --- include/efi_loader.h | 3 +++ lib/efi_loader/efi_runtime.c | 8 +++----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/include/efi_loader.h b/include/efi_loader.h index ceabbaadd0..be6cede92f 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -397,6 +397,9 @@ efi_status_t efi_root_node_register(void); efi_status_t efi_initialize_system_table(void); /* efi_runtime_detach() - detach unimplemented runtime functions */ void efi_runtime_detach(void); +/* efi_convert_pointer() - convert pointer to virtual address */ +efi_status_t EFIAPI efi_convert_pointer(efi_uintn_t debug_disposition, + void **address); /* Called by bootefi to make console interface available */ efi_status_t efi_console_register(void); /* Called by bootefi to make all disk storage accessible as EFI objects */ diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c index 121e2f65c6..45baa2fd3e 100644 --- a/lib/efi_loader/efi_runtime.c +++ b/lib/efi_loader/efi_runtime.c @@ -496,15 +496,13 @@ static __efi_runtime efi_status_t EFIAPI efi_convert_pointer_runtime( * @address: pointer to be converted * Return: status code */ -static __efi_runtime efi_status_t EFIAPI efi_convert_pointer( - efi_uintn_t debug_disposition, void **address) +__efi_runtime efi_status_t EFIAPI +efi_convert_pointer(efi_uintn_t debug_disposition, void **address) { efi_physical_addr_t addr; efi_uintn_t i; efi_status_t ret = EFI_NOT_FOUND; - EFI_ENTRY("%zu %p", debug_disposition, address); - if (!efi_virtmap) { ret = EFI_UNSUPPORTED; goto out; @@ -533,7 +531,7 @@ static __efi_runtime efi_status_t EFIAPI efi_convert_pointer( } out: - return EFI_EXIT(ret); + return ret; } static __efi_runtime void efi_relocate_runtime_table(ulong offset) From 724d28171e5bd239866cdf885a7b70c059ddd563 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Tue, 24 Mar 2020 17:52:40 +0100 Subject: [PATCH 32/37] efi_loader: optional pointer for ConvertPointer If the EFI_OPTIONAL_PTR is set in DebugDisposition, a NULL pointer does not constitute an invalid parameter. Signed-off-by: Heinrich Schuchardt --- include/efi_api.h | 2 ++ lib/efi_loader/efi_runtime.c | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/include/efi_api.h b/include/efi_api.h index 759d911875..5744f6aed8 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -251,6 +251,8 @@ struct efi_rt_properties_table { u32 runtime_services_supported; }; +#define EFI_OPTIONAL_PTR 0x00000001 + struct efi_runtime_services { struct efi_table_hdr hdr; efi_status_t (EFIAPI *get_time)(struct efi_time *time, diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c index 45baa2fd3e..a4aa1d8b6c 100644 --- a/lib/efi_loader/efi_runtime.c +++ b/lib/efi_loader/efi_runtime.c @@ -512,6 +512,12 @@ efi_convert_pointer(efi_uintn_t debug_disposition, void **address) ret = EFI_INVALID_PARAMETER; goto out; } + if (!*address) { + if (debug_disposition & EFI_OPTIONAL_PTR) + return EFI_SUCCESS; + else + return EFI_INVALID_PARAMETER; + } addr = (uintptr_t)*address; for (i = 0; i < efi_descriptor_count; i++) { From b0dd8cb4076ea178df93ad56be6636dcdb444c33 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Sun, 28 Jun 2020 16:30:29 +0200 Subject: [PATCH 33/37] efi_loader: new function efi_memcpy_runtime() Provide a memcpy() function that we can use at UEFI runtime. Signed-off-by: Heinrich Schuchardt --- include/efi_loader.h | 3 +++ lib/efi_loader/efi_runtime.c | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/include/efi_loader.h b/include/efi_loader.h index be6cede92f..98944640be 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -792,6 +792,9 @@ bool efi_secure_boot_enabled(void); bool efi_image_parse(void *efi, size_t len, struct efi_image_regions **regp, WIN_CERTIFICATE **auth, size_t *auth_len); +/* runtime implementation of memcpy() */ +void efi_memcpy_runtime(void *dest, const void *src, size_t n); + #else /* CONFIG_IS_ENABLED(EFI_LOADER) */ /* Without CONFIG_EFI_LOADER we don't have a runtime section, stub it out */ diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c index a4aa1d8b6c..5b6506fbdc 100644 --- a/lib/efi_loader/efi_runtime.c +++ b/lib/efi_loader/efi_runtime.c @@ -137,6 +137,25 @@ efi_status_t efi_init_runtime_supported(void) return ret; } +/** + * efi_memcpy_runtime() - copy memory area + * + * At runtime memcpy() is not available. + * + * @dest: destination buffer + * @src: source buffer + * @n: number of bytes to copy + * Return: pointer to destination buffer + */ +void __efi_runtime efi_memcpy_runtime(void *dest, const void *src, size_t n) +{ + u8 *d = dest; + const u8 *s = src; + + for (; n; --n) + *d++ = *s++; +} + /** * efi_update_table_header_crc32() - Update crc32 in table header * From f1f990a8c9584b558bb1449dad3807858bbbcc49 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Sun, 22 Mar 2020 09:07:50 +0100 Subject: [PATCH 34/37] efi_loader: memory buffer for variables Saving UEFI variable as encoded U-Boot environment variables does not allow support at runtime. Provide functions to manage a memory buffer with UEFI variables. Signed-off-by: Heinrich Schuchardt --- include/efi_variable.h | 54 +++++++ lib/efi_loader/Makefile | 1 + lib/efi_loader/efi_var_mem.c | 266 +++++++++++++++++++++++++++++++++++ 3 files changed, 321 insertions(+) create mode 100644 lib/efi_loader/efi_var_mem.c diff --git a/include/efi_variable.h b/include/efi_variable.h index 01054209c4..bc5985cfdb 100644 --- a/include/efi_variable.h +++ b/include/efi_variable.h @@ -141,4 +141,58 @@ efi_status_t efi_var_to_file(void); */ efi_status_t efi_var_from_file(void); +/** + * efi_var_mem_init() - set-up variable list + * + * Return: status code + */ +efi_status_t efi_var_mem_init(void); + +/** + * efi_var_mem_find() - find a variable in the list + * + * @guid: GUID of the variable + * @name: name of the variable + * @next: on exit pointer to the next variable after the found one + * Return: found variable + */ +struct efi_var_entry *efi_var_mem_find(const efi_guid_t *guid, const u16 *name, + struct efi_var_entry **next); + +/** + * efi_var_mem_del() - delete a variable from the list of variables + * + * @var: variable to delete + */ +void efi_var_mem_del(struct efi_var_entry *var); + +/** + * efi_var_mem_ins() - append a variable to the list of variables + * + * The variable is appended without checking if a variable of the same name + * already exists. The two data buffers are concatenated. + * + * @variable_name: variable name + * @vendor: GUID + * @attributes: variable attributes + * @size1: size of the first data buffer + * @data1: first data buffer + * @size2: size of the second data field + * @data2: second data buffer + * @time: time of authentication (as seconds since start of epoch) + * Result: status code + */ +efi_status_t efi_var_mem_ins(u16 *variable_name, + const efi_guid_t *vendor, u32 attributes, + const efi_uintn_t size1, const void *data1, + const efi_uintn_t size2, const void *data2, + const u64 time); + +/** + * efi_var_mem_free() - determine free memory for variables + * + * Return: maximum data size plus variable name size + */ +u64 efi_var_mem_free(void); + #endif diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile index c87b82db32..f81ec8d277 100644 --- a/lib/efi_loader/Makefile +++ b/lib/efi_loader/Makefile @@ -36,6 +36,7 @@ obj-y += efi_runtime.o obj-y += efi_setup.o obj-$(CONFIG_EFI_UNICODE_COLLATION_PROTOCOL2) += efi_unicode_collation.o obj-y += efi_var_common.o +obj-y += efi_var_mem.o ifeq ($(CONFIG_EFI_MM_COMM_TEE),y) obj-y += efi_variable_tee.o else diff --git a/lib/efi_loader/efi_var_mem.c b/lib/efi_loader/efi_var_mem.c new file mode 100644 index 0000000000..7a2dba7dc2 --- /dev/null +++ b/lib/efi_loader/efi_var_mem.c @@ -0,0 +1,266 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * File interface for UEFI variables + * + * Copyright (c) 2020, Heinrich Schuchardt + */ + +#include +#include +#include +#include + +static struct efi_var_file __efi_runtime_data *efi_var_buf; +static struct efi_var_entry __efi_runtime_data *efi_current_var; + +/** + * efi_var_mem_compare() - compare GUID and name with a variable + * + * @var: variable to compare + * @guid: GUID to compare + * @name: variable name to compare + * @next: pointer to next variable + * Return: true if match + */ +static bool __efi_runtime +efi_var_mem_compare(struct efi_var_entry *var, const efi_guid_t *guid, + const u16 *name, struct efi_var_entry **next) +{ + int i; + u8 *guid1, *guid2; + const u16 *data, *var_name; + bool match = true; + + for (guid1 = (u8 *)&var->guid, guid2 = (u8 *)guid, i = 0; + i < sizeof(efi_guid_t) && match; ++i) + match = (guid1[i] == guid2[i]); + + for (data = var->name, var_name = name;; ++data, ++var_name) { + if (match) + match = (*data == *var_name); + if (!*data) + break; + } + + ++data; + + if (next) + *next = (struct efi_var_entry *) + ALIGN((uintptr_t)data + var->length, 8); + + if (match) + efi_current_var = var; + + return match; +} + +struct efi_var_entry __efi_runtime +*efi_var_mem_find(const efi_guid_t *guid, const u16 *name, + struct efi_var_entry **next) +{ + struct efi_var_entry *var, *last; + + last = (struct efi_var_entry *) + ((uintptr_t)efi_var_buf + efi_var_buf->length); + + if (!*name) { + if (next) { + *next = efi_var_buf->var; + if (*next >= last) + *next = NULL; + } + return NULL; + } + if (efi_current_var && + efi_var_mem_compare(efi_current_var, guid, name, next)) { + if (next && *next >= last) + *next = NULL; + return efi_current_var; + } + + var = efi_var_buf->var; + if (var < last) { + for (; var;) { + struct efi_var_entry *pos; + bool match; + + match = efi_var_mem_compare(var, guid, name, &pos); + if (pos >= last) + pos = NULL; + if (match) { + if (next) + *next = pos; + return var; + } + var = pos; + } + } + if (next) + *next = NULL; + return NULL; +} + +void __efi_runtime efi_var_mem_del(struct efi_var_entry *var) +{ + u16 *data; + struct efi_var_entry *next, *last; + + if (!var) + return; + + last = (struct efi_var_entry *) + ((uintptr_t)efi_var_buf + efi_var_buf->length); + if (var <= efi_current_var) + efi_current_var = NULL; + + for (data = var->name; *data; ++data) + ; + ++data; + next = (struct efi_var_entry *) + ALIGN((uintptr_t)data + var->length, 8); + efi_var_buf->length -= (uintptr_t)next - (uintptr_t)var; + + memmove(var, next, (uintptr_t)last - (uintptr_t)next); + efi_var_buf->crc32 = crc32(0, (u8 *)efi_var_buf->var, + efi_var_buf->length - + sizeof(struct efi_var_file)); +} + +efi_status_t __efi_runtime efi_var_mem_ins( + u16 *variable_name, + const efi_guid_t *vendor, u32 attributes, + const efi_uintn_t size1, const void *data1, + const efi_uintn_t size2, const void *data2, + const u64 time) +{ + u16 *data; + struct efi_var_entry *var; + u32 var_name_len; + + var = (struct efi_var_entry *) + ((uintptr_t)efi_var_buf + efi_var_buf->length); + for (var_name_len = 0; variable_name[var_name_len]; ++var_name_len) + ; + ++var_name_len; + data = var->name + var_name_len; + + if ((uintptr_t)data - (uintptr_t)efi_var_buf + size1 + size2 > + EFI_VAR_BUF_SIZE) + return EFI_OUT_OF_RESOURCES; + + var->attr = attributes; + var->length = size1 + size2; + var->time = time; + + efi_memcpy_runtime(&var->guid, vendor, sizeof(efi_guid_t)); + efi_memcpy_runtime(var->name, variable_name, + sizeof(u16) * var_name_len); + efi_memcpy_runtime(data, data1, size1); + efi_memcpy_runtime((u8 *)data + size1, data2, size2); + + var = (struct efi_var_entry *) + ALIGN((uintptr_t)data + var->length, 8); + efi_var_buf->length = (uintptr_t)var - (uintptr_t)efi_var_buf; + efi_var_buf->crc32 = crc32(0, (u8 *)efi_var_buf->var, + efi_var_buf->length - + sizeof(struct efi_var_file)); + + return EFI_SUCCESS; +} + +u64 __efi_runtime efi_var_mem_free(void) +{ + return EFI_VAR_BUF_SIZE - efi_var_buf->length - + sizeof(struct efi_var_entry); +} + +/** + * efi_var_mem_bs_del() - delete boot service only variables + */ +static void efi_var_mem_bs_del(void) +{ + struct efi_var_entry *var = efi_var_buf->var; + + for (;;) { + struct efi_var_entry *last; + + last = (struct efi_var_entry *) + ((uintptr_t)efi_var_buf + efi_var_buf->length); + if (var >= last) + break; + if (var->attr & EFI_VARIABLE_RUNTIME_ACCESS) { + u16 *data; + + /* skip variable */ + for (data = var->name; *data; ++data) + ; + ++data; + var = (struct efi_var_entry *) + ALIGN((uintptr_t)data + var->length, 8); + } else { + /* delete variable */ + efi_var_mem_del(var); + } + } +} + +/** + * efi_var_mem_notify_exit_boot_services() - ExitBootService callback + * + * @event: callback event + * @context: callback context + */ +static void EFIAPI __efi_runtime +efi_var_mem_notify_exit_boot_services(struct efi_event *event, void *context) +{ + EFI_ENTRY("%p, %p", event, context); + + /* Delete boot service only variables */ + efi_var_mem_bs_del(); + + EFI_EXIT(EFI_SUCCESS); +} + +/** + * efi_var_mem_notify_exit_boot_services() - SetVirtualMemoryMap callback + * + * @event: callback event + * @context: callback context + */ +static void EFIAPI __efi_runtime +efi_var_mem_notify_virtual_address_map(struct efi_event *event, void *context) +{ + efi_convert_pointer(0, (void **)&efi_var_buf); +} + +efi_status_t efi_var_mem_init(void) +{ + u64 memory; + efi_status_t ret; + struct efi_event *event; + + ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES, + EFI_RUNTIME_SERVICES_DATA, + efi_size_in_pages(EFI_VAR_BUF_SIZE), + &memory); + if (ret != EFI_SUCCESS) + return ret; + efi_var_buf = (struct efi_var_file *)(uintptr_t)memory; + memset(efi_var_buf, 0, EFI_VAR_BUF_SIZE); + efi_var_buf->magic = EFI_VAR_FILE_MAGIC; + efi_var_buf->length = (uintptr_t)efi_var_buf->var - + (uintptr_t)efi_var_buf; + /* crc32 for 0 bytes = 0 */ + + ret = efi_create_event(EVT_SIGNAL_EXIT_BOOT_SERVICES, TPL_CALLBACK, + efi_var_mem_notify_exit_boot_services, NULL, + NULL, &event); + if (ret != EFI_SUCCESS) + return ret; + ret = efi_create_event(EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE, TPL_CALLBACK, + efi_var_mem_notify_virtual_address_map, NULL, + NULL, &event); + if (ret != EFI_SUCCESS) + return ret; + return ret; +} From ab7296c99c273736801f9480cd21fbe862671987 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Sat, 4 Jul 2020 18:34:15 +0200 Subject: [PATCH 35/37] efi_loader: use memory based variable storage Saving UEFI variable as encoded U-Boot environment variables does not allow implement run-time support. Use a memory buffer for storing UEFI variables. Signed-off-by: Heinrich Schuchardt --- lib/efi_loader/efi_variable.c | 560 ++++++---------------------------- 1 file changed, 93 insertions(+), 467 deletions(-) diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index 8ed4b0830b..bbdc071126 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -30,145 +30,6 @@ static bool efi_secure_boot; static enum efi_secure_mode efi_secure_mode; static u8 efi_vendor_keys; -/* - * Mapping between EFI variables and u-boot variables: - * - * efi_$guid_$varname = {attributes}(type)value - * - * For example: - * - * efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_OsIndicationsSupported= - * "{ro,boot,run}(blob)0000000000000000" - * efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_BootOrder= - * "(blob)00010000" - * - * The attributes are a comma separated list of these possible - * attributes: - * - * + ro - read-only - * + boot - boot-services access - * + run - runtime access - * - * NOTE: with current implementation, no variables are available after - * ExitBootServices, and all are persisted (if possible). - * - * If not specified, the attributes default to "{boot}". - * - * The required type is one of: - * - * + utf8 - raw utf8 string - * + blob - arbitrary length hex string - * - * Maybe a utf16 type would be useful to for a string value to be auto - * converted to utf16? - */ - -#define PREFIX_LEN (strlen("efi_xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx_")) - -/** - * efi_to_native() - convert the UEFI variable name and vendor GUID to U-Boot - * variable name - * - * The U-Boot variable name is a concatenation of prefix 'efi', the hexstring - * encoded vendor GUID, and the UTF-8 encoded UEFI variable name separated by - * underscores, e.g. 'efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_BootOrder'. - * - * @native: pointer to pointer to U-Boot variable name - * @variable_name: UEFI variable name - * @vendor: vendor GUID - * Return: status code - */ -static efi_status_t efi_to_native(char **native, const u16 *variable_name, - const efi_guid_t *vendor) -{ - size_t len; - char *pos; - - len = PREFIX_LEN + utf16_utf8_strlen(variable_name) + 1; - *native = malloc(len); - if (!*native) - return EFI_OUT_OF_RESOURCES; - - pos = *native; - pos += sprintf(pos, "efi_%pUl_", vendor); - utf16_utf8_strcpy(&pos, variable_name); - - return EFI_SUCCESS; -} - -/** - * prefix() - skip over prefix - * - * Skip over a prefix string. - * - * @str: string with prefix - * @prefix: prefix string - * Return: string without prefix, or NULL if prefix not found - */ -static const char *prefix(const char *str, const char *prefix) -{ - size_t n = strlen(prefix); - if (!strncmp(prefix, str, n)) - return str + n; - return NULL; -} - -/** - * parse_attr() - decode attributes part of variable value - * - * Convert the string encoded attributes of a UEFI variable to a bit mask. - * TODO: Several attributes are not supported. - * - * @str: value of U-Boot variable - * @attrp: pointer to UEFI attributes - * @timep: pointer to time attribute - * Return: pointer to remainder of U-Boot variable value - */ -static const char *parse_attr(const char *str, u32 *attrp, u64 *timep) -{ - u32 attr = 0; - char sep = '{'; - - if (*str != '{') { - *attrp = EFI_VARIABLE_BOOTSERVICE_ACCESS; - return str; - } - - while (*str == sep) { - const char *s; - - str++; - - if ((s = prefix(str, "ro"))) { - attr |= EFI_VARIABLE_READ_ONLY; - } else if ((s = prefix(str, "nv"))) { - attr |= EFI_VARIABLE_NON_VOLATILE; - } else if ((s = prefix(str, "boot"))) { - attr |= EFI_VARIABLE_BOOTSERVICE_ACCESS; - } else if ((s = prefix(str, "run"))) { - attr |= EFI_VARIABLE_RUNTIME_ACCESS; - } else if ((s = prefix(str, "time="))) { - attr |= EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS; - hex2bin((u8 *)timep, s, sizeof(*timep)); - s += sizeof(*timep) * 2; - } else if (*str == '}') { - break; - } else { - printf("invalid attribute: %s\n", str); - break; - } - - str = s; - sep = ','; - } - - str++; - - *attrp = attr; - - return str; -} - /** * efi_set_secure_state - modify secure boot state variables * @secure_boot: value of SecureBoot @@ -568,296 +429,115 @@ static efi_status_t efi_variable_authenticate(u16 *variable, } #endif /* CONFIG_EFI_SECURE_BOOT */ -efi_status_t efi_get_variable_int(u16 *variable_name, const efi_guid_t *vendor, - u32 *attributes, efi_uintn_t *data_size, - void *data, u64 *timep) +efi_status_t __efi_runtime +efi_get_variable_int(u16 *variable_name, const efi_guid_t *vendor, + u32 *attributes, efi_uintn_t *data_size, void *data, + u64 *timep) { - char *native_name; - efi_status_t ret; - unsigned long in_size; - const char *val = NULL, *s; - u64 time = 0; - u32 attr; + efi_uintn_t old_size; + struct efi_var_entry *var; + u16 *pdata; if (!variable_name || !vendor || !data_size) return EFI_INVALID_PARAMETER; - - ret = efi_to_native(&native_name, variable_name, vendor); - if (ret) - return ret; - - EFI_PRINT("get '%s'\n", native_name); - - val = env_get(native_name); - free(native_name); - if (!val) + var = efi_var_mem_find(vendor, variable_name, NULL); + if (!var) return EFI_NOT_FOUND; - val = parse_attr(val, &attr, &time); - - if (timep) - *timep = time; - - in_size = *data_size; - - if ((s = prefix(val, "(blob)"))) { - size_t len = strlen(s); - - /* number of hexadecimal digits must be even */ - if (len & 1) - return EFI_DEVICE_ERROR; - - /* two characters per byte: */ - len /= 2; - *data_size = len; - - if (in_size < len) { - ret = EFI_BUFFER_TOO_SMALL; - goto out; - } - - if (!data) { - EFI_PRINT("Variable with no data shouldn't exist.\n"); - return EFI_INVALID_PARAMETER; - } - - if (hex2bin(data, s, len)) - return EFI_DEVICE_ERROR; - - EFI_PRINT("got value: \"%s\"\n", s); - } else if ((s = prefix(val, "(utf8)"))) { - unsigned len = strlen(s) + 1; - - *data_size = len; - - if (in_size < len) { - ret = EFI_BUFFER_TOO_SMALL; - goto out; - } - - if (!data) { - EFI_PRINT("Variable with no data shouldn't exist.\n"); - return EFI_INVALID_PARAMETER; - } - - memcpy(data, s, len); - ((char *)data)[len] = '\0'; - - EFI_PRINT("got value: \"%s\"\n", (char *)data); - } else { - EFI_PRINT("invalid value: '%s'\n", val); - return EFI_DEVICE_ERROR; - } - -out: if (attributes) - *attributes = attr; + *attributes = var->attr; + if (timep) + *timep = var->time; - return ret; -} - -static char *efi_variables_list; -static char *efi_cur_variable; - -/** - * parse_uboot_variable() - parse a u-boot variable and get uefi-related - * information - * @variable: whole data of u-boot variable (ie. name=value) - * @variable_name_size: size of variable_name buffer in byte - * @variable_name: name of uefi variable in u16, null-terminated - * @vendor: vendor's guid - * @attributes: attributes - * - * A uefi variable is encoded into a u-boot variable as described above. - * This function parses such a u-boot variable and retrieve uefi-related - * information into respective parameters. In return, variable_name_size - * is the size of variable name including NULL. - * - * Return: EFI_SUCCESS if parsing is OK, EFI_NOT_FOUND when - * the entire variable list has been returned, - * otherwise non-zero status code - */ -static efi_status_t parse_uboot_variable(char *variable, - efi_uintn_t *variable_name_size, - u16 *variable_name, - const efi_guid_t *vendor, - u32 *attributes) -{ - char *guid, *name, *end, c; - size_t name_len; - efi_uintn_t old_variable_name_size; - u64 time; - u16 *p; - - guid = strchr(variable, '_'); - if (!guid) - return EFI_INVALID_PARAMETER; - guid++; - name = strchr(guid, '_'); - if (!name) - return EFI_INVALID_PARAMETER; - name++; - end = strchr(name, '='); - if (!end) - return EFI_INVALID_PARAMETER; - - name_len = end - name; - old_variable_name_size = *variable_name_size; - *variable_name_size = sizeof(u16) * (name_len + 1); - if (old_variable_name_size < *variable_name_size) + old_size = *data_size; + *data_size = var->length; + if (old_size < var->length) return EFI_BUFFER_TOO_SMALL; - end++; /* point to value */ - - /* variable name */ - p = variable_name; - utf8_utf16_strncpy(&p, name, name_len); - variable_name[name_len] = 0; - - /* guid */ - c = *(name - 1); - *(name - 1) = '\0'; /* guid need be null-terminated here */ - if (uuid_str_to_bin(guid, (unsigned char *)vendor, - UUID_STR_FORMAT_GUID)) - /* The only error would be EINVAL. */ + if (!data) return EFI_INVALID_PARAMETER; - *(name - 1) = c; - /* attributes */ - parse_attr(end, attributes, &time); + for (pdata = var->name; *pdata; ++pdata) + ; + ++pdata; + + efi_memcpy_runtime(data, pdata, var->length); return EFI_SUCCESS; } -efi_status_t efi_get_next_variable_name_int(efi_uintn_t *variable_name_size, - u16 *variable_name, - efi_guid_t *vendor) +efi_status_t __efi_runtime +efi_get_next_variable_name_int(efi_uintn_t *variable_name_size, + u16 *variable_name, efi_guid_t *vendor) { - char *native_name, *variable; - ssize_t name_len, list_len; - char regex[256]; - char * const regexlist[] = {regex}; - u32 attributes; - int i; - efi_status_t ret; + struct efi_var_entry *var; + efi_uintn_t old_size; + u16 *pdata; if (!variable_name_size || !variable_name || !vendor) return EFI_INVALID_PARAMETER; - if (variable_name[0]) { - /* check null-terminated string */ - for (i = 0; i < *variable_name_size; i++) - if (!variable_name[i]) - break; - if (i >= *variable_name_size) - return EFI_INVALID_PARAMETER; + efi_var_mem_find(vendor, variable_name, &var); - /* search for the last-returned variable */ - ret = efi_to_native(&native_name, variable_name, vendor); - if (ret) - return ret; + if (!var) + return EFI_NOT_FOUND; - name_len = strlen(native_name); - for (variable = efi_variables_list; variable && *variable;) { - if (!strncmp(variable, native_name, name_len) && - variable[name_len] == '=') - break; + for (pdata = var->name; *pdata; ++pdata) + ; + ++pdata; - variable = strchr(variable, '\n'); - if (variable) - variable++; - } + old_size = *variable_name_size; + *variable_name_size = (uintptr_t)pdata - (uintptr_t)var->name; - free(native_name); - if (!(variable && *variable)) - return EFI_INVALID_PARAMETER; + if (old_size < *variable_name_size) + return EFI_BUFFER_TOO_SMALL; - /* next variable */ - variable = strchr(variable, '\n'); - if (variable) - variable++; - if (!(variable && *variable)) - return EFI_NOT_FOUND; - } else { - /* - *new search: free a list used in the previous search - */ - free(efi_variables_list); - efi_variables_list = NULL; - efi_cur_variable = NULL; + efi_memcpy_runtime(variable_name, var->name, *variable_name_size); + efi_memcpy_runtime(vendor, &var->guid, sizeof(efi_guid_t)); - snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_.*"); - list_len = hexport_r(&env_htab, '\n', - H_MATCH_REGEX | H_MATCH_KEY, - &efi_variables_list, 0, 1, regexlist); - - if (list_len <= 1) - return EFI_NOT_FOUND; - - variable = efi_variables_list; - } - - ret = parse_uboot_variable(variable, variable_name_size, variable_name, - vendor, &attributes); - - return ret; + return EFI_SUCCESS; } efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor, u32 attributes, efi_uintn_t data_size, const void *data, bool ro_check) { - char *native_name = NULL, *old_data = NULL, *val = NULL, *s; - efi_uintn_t old_size; + struct efi_var_entry *var; + efi_uintn_t ret; bool append, delete; u64 time = 0; - u32 old_attr; - efi_status_t ret = EFI_SUCCESS; if (!variable_name || !*variable_name || !vendor || ((attributes & EFI_VARIABLE_RUNTIME_ACCESS) && - !(attributes & EFI_VARIABLE_BOOTSERVICE_ACCESS))) { - ret = EFI_INVALID_PARAMETER; - goto err; - } - - ret = efi_to_native(&native_name, variable_name, vendor); - if (ret) - goto err; + !(attributes & EFI_VARIABLE_BOOTSERVICE_ACCESS))) + return EFI_INVALID_PARAMETER; /* check if a variable exists */ - old_size = 0; - old_attr = 0; - ret = efi_get_variable_int(variable_name, vendor, &old_attr, - &old_size, NULL, &time); + var = efi_var_mem_find(vendor, variable_name, NULL); append = !!(attributes & EFI_VARIABLE_APPEND_WRITE); attributes &= ~(u32)EFI_VARIABLE_APPEND_WRITE; delete = !append && (!data_size || !attributes); /* check attributes */ - if (old_size) { - if (ro_check && (old_attr & EFI_VARIABLE_READ_ONLY)) { - ret = EFI_WRITE_PROTECTED; - goto err; - } + if (var) { + if (ro_check && (var->attr & EFI_VARIABLE_READ_ONLY)) + return EFI_WRITE_PROTECTED; /* attributes won't be changed */ if (!delete && - ((ro_check && old_attr != attributes) || - (!ro_check && ((old_attr & ~(u32)EFI_VARIABLE_READ_ONLY) + ((ro_check && var->attr != attributes) || + (!ro_check && ((var->attr & ~(u32)EFI_VARIABLE_READ_ONLY) != (attributes & ~(u32)EFI_VARIABLE_READ_ONLY))))) { - ret = EFI_INVALID_PARAMETER; - goto err; + return EFI_INVALID_PARAMETER; } + time = var->time; } else { - if (delete || append) { + if (delete || append) /* * Trying to delete or to update a non-existent * variable. */ - ret = EFI_NOT_FOUND; - goto err; - } + return EFI_NOT_FOUND; } if (((!u16_strcmp(variable_name, L"PK") || @@ -869,27 +549,26 @@ efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor, /* authentication is mandatory */ if (!(attributes & EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS)) { - EFI_PRINT("%ls: AUTHENTICATED_WRITE_ACCESS required\n", + EFI_PRINT("%ls: TIME_BASED_AUTHENTICATED_WRITE_ACCESS required\n", variable_name); - ret = EFI_INVALID_PARAMETER; - goto err; + return EFI_INVALID_PARAMETER; } } /* authenticate a variable */ if (IS_ENABLED(CONFIG_EFI_SECURE_BOOT)) { - if (attributes & EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS) { - ret = EFI_INVALID_PARAMETER; - goto err; - } + if (attributes & EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS) + return EFI_INVALID_PARAMETER; if (attributes & EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) { + u32 env_attr; + ret = efi_variable_authenticate(variable_name, vendor, &data_size, &data, - attributes, &old_attr, + attributes, &env_attr, &time); if (ret != EFI_SUCCESS) - goto err; + return ret; /* last chance to check for delete */ if (!data_size) @@ -900,105 +579,42 @@ efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor, (EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS | EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS)) { EFI_PRINT("Secure boot is not configured\n"); - ret = EFI_INVALID_PARAMETER; - goto err; + return EFI_INVALID_PARAMETER; } } - /* delete a variable */ if (delete) { - /* !old_size case has been handled before */ - val = NULL; + /* EFI_NOT_FOUND has been handled before */ ret = EFI_SUCCESS; - goto out; - } + } else if (append) { + u16 *old_data = var->name; - if (append) { - old_data = malloc(old_size); - if (!old_data) { - ret = EFI_OUT_OF_RESOURCES; - goto err; - } - ret = efi_get_variable_int(variable_name, vendor, - &old_attr, &old_size, old_data, NULL); - if (ret != EFI_SUCCESS) - goto err; + for (; *old_data; ++old_data) + ; + ++old_data; + ret = efi_var_mem_ins(variable_name, vendor, attributes, + var->length, old_data, data_size, data, + time); } else { - old_size = 0; + ret = efi_var_mem_ins(variable_name, vendor, attributes, + data_size, data, 0, NULL, time); } + efi_var_mem_del(var); - val = malloc(2 * old_size + 2 * data_size - + strlen("{ro,run,boot,nv,time=0123456701234567}(blob)") - + 1); - if (!val) { - ret = EFI_OUT_OF_RESOURCES; - goto err; - } + if (ret != EFI_SUCCESS) + return ret; - s = val; - - /* - * store attributes - */ - attributes &= (EFI_VARIABLE_READ_ONLY | - EFI_VARIABLE_NON_VOLATILE | - EFI_VARIABLE_BOOTSERVICE_ACCESS | - EFI_VARIABLE_RUNTIME_ACCESS | - EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS); - s += sprintf(s, "{"); - for (u32 attr_rem = attributes; attr_rem;) { - u32 attr = 1 << (ffs(attr_rem) - 1); - - if (attr == EFI_VARIABLE_READ_ONLY) { - s += sprintf(s, "ro"); - } else if (attr == EFI_VARIABLE_NON_VOLATILE) { - s += sprintf(s, "nv"); - } else if (attr == EFI_VARIABLE_BOOTSERVICE_ACCESS) { - s += sprintf(s, "boot"); - } else if (attr == EFI_VARIABLE_RUNTIME_ACCESS) { - s += sprintf(s, "run"); - } else if (attr == - EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) { - s += sprintf(s, "time="); - s = bin2hex(s, (u8 *)&time, sizeof(time)); - } - - attr_rem &= ~attr; - if (attr_rem) - s += sprintf(s, ","); - } - s += sprintf(s, "}"); - s += sprintf(s, "(blob)"); - - /* store payload: */ - if (append) - s = bin2hex(s, old_data, old_size); - s = bin2hex(s, data, data_size); - *s = '\0'; - - EFI_PRINT("setting: %s=%s\n", native_name, val); - -out: - if (env_set(native_name, val)) { - ret = EFI_DEVICE_ERROR; - } else { - if (!u16_strcmp(variable_name, L"PK")) - ret = efi_init_secure_state(); - else - ret = EFI_SUCCESS; - } + if (!u16_strcmp(variable_name, L"PK")) + ret = efi_init_secure_state(); + else + ret = EFI_SUCCESS; /* Write non-volatile EFI variables to file */ if (attributes & EFI_VARIABLE_NON_VOLATILE && ret == EFI_SUCCESS && efi_obj_list_initialized == EFI_SUCCESS) efi_var_to_file(); -err: - free(native_name); - free(old_data); - free(val); - - return ret; + return EFI_SUCCESS; } efi_status_t efi_query_variable_info_int(u32 attributes, @@ -1006,7 +622,13 @@ efi_status_t efi_query_variable_info_int(u32 attributes, u64 *remaining_variable_storage_size, u64 *maximum_variable_size) { - return EFI_UNSUPPORTED; + *maximum_variable_storage_size = EFI_VAR_BUF_SIZE - + sizeof(struct efi_var_file); + *remaining_variable_storage_size = efi_var_mem_free(); + *maximum_variable_size = EFI_VAR_BUF_SIZE - + sizeof(struct efi_var_file) - + sizeof(struct efi_var_entry); + return EFI_SUCCESS; } /** @@ -1107,6 +729,10 @@ efi_status_t efi_init_variables(void) { efi_status_t ret; + ret = efi_var_mem_init(); + if (ret != EFI_SUCCESS) + return ret; + ret = efi_init_secure_state(); if (ret != EFI_SUCCESS) return ret; From b02a707152dc8046de6777c633f1d719c329a133 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Tue, 24 Mar 2020 19:54:53 +0000 Subject: [PATCH 36/37] efi_loader: enable UEFI variables at runtime Enable UEFI variables at runtime. Signed-off-by: Heinrich Schuchardt --- lib/efi_loader/efi_runtime.c | 2 ++ lib/efi_loader/efi_var_file.c | 6 +++--- lib/efi_loader/efi_variable.c | 14 ++++++++++++-- 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c index 5b6506fbdc..91a4551448 100644 --- a/lib/efi_loader/efi_runtime.c +++ b/lib/efi_loader/efi_runtime.c @@ -121,6 +121,8 @@ efi_status_t efi_init_runtime_supported(void) rt_table->version = EFI_RT_PROPERTIES_TABLE_VERSION; rt_table->length = sizeof(struct efi_rt_properties_table); rt_table->runtime_services_supported = + EFI_RT_SUPPORTED_GET_VARIABLE | + EFI_RT_SUPPORTED_GET_NEXT_VARIABLE_NAME | EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP | EFI_RT_SUPPORTED_CONVERT_POINTER; diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c index b1b7532495..880c279aef 100644 --- a/lib/efi_loader/efi_var_file.c +++ b/lib/efi_loader/efi_var_file.c @@ -183,9 +183,9 @@ static efi_status_t __maybe_unused efi_var_restore(struct efi_var_file *buf) u16 *data = var->name + u16_strlen(var->name) + 1; if (var->attr & EFI_VARIABLE_NON_VOLATILE && var->length) { - ret = efi_set_variable_int(var->name, &var->guid, - var->attr, var->length, - data, true); + ret = efi_var_mem_ins(var->name, &var->guid, var->attr, + var->length, data, 0, NULL, + var->time); if (ret != EFI_SUCCESS) log_err("Failed to set EFI variable %ls\n", var->name); diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index bbdc071126..eab5f005da 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -668,7 +668,16 @@ static efi_status_t __efi_runtime EFIAPI efi_get_variable_runtime(u16 *variable_name, const efi_guid_t *vendor, u32 *attributes, efi_uintn_t *data_size, void *data) { - return EFI_UNSUPPORTED; + efi_status_t ret; + + ret = efi_get_variable_int(variable_name, vendor, attributes, + data_size, data, NULL); + + /* Remove EFI_VARIABLE_READ_ONLY flag */ + if (attributes) + *attributes &= EFI_VARIABLE_MASK; + + return ret; } /** @@ -684,7 +693,8 @@ static efi_status_t __efi_runtime EFIAPI efi_get_next_variable_name_runtime(efi_uintn_t *variable_name_size, u16 *variable_name, efi_guid_t *vendor) { - return EFI_UNSUPPORTED; + return efi_get_next_variable_name_int(variable_name_size, variable_name, + vendor); } /** From 4a3155de3dbadfcb933287dbb84c8eff0fd951eb Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Mon, 6 Jul 2020 17:06:07 +0200 Subject: [PATCH 37/37] efi_selftest: adjust runtime test for variables As variable services are available at runtime we have to expect EFI_SUCCESS when calling the services. Signed-off-by: Heinrich Schuchardt --- lib/efi_selftest/efi_selftest_variables_runtime.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/lib/efi_selftest/efi_selftest_variables_runtime.c b/lib/efi_selftest/efi_selftest_variables_runtime.c index b3b40ad2cf..3226069c0b 100644 --- a/lib/efi_selftest/efi_selftest_variables_runtime.c +++ b/lib/efi_selftest/efi_selftest_variables_runtime.c @@ -16,9 +16,7 @@ static struct efi_boot_services *boottime; static struct efi_runtime_services *runtime; -static const efi_guid_t guid_vendor0 = - EFI_GUID(0x67029eb5, 0x0af2, 0xf6b1, - 0xda, 0x53, 0xfc, 0xb5, 0x66, 0xdd, 0x1c, 0xe6); +static const efi_guid_t guid_vendor0 = EFI_GLOBAL_VARIABLE_GUID; /* * Setup unit test. @@ -68,17 +66,18 @@ static int execute(void) efi_st_error("SetVariable failed\n"); return EFI_ST_FAILURE; } - len = 3; - ret = runtime->get_variable(L"efi_st_var0", &guid_vendor0, + len = EFI_ST_MAX_DATA_SIZE; + ret = runtime->get_variable(L"PlatformLangCodes", &guid_vendor0, &attr, &len, data); - if (ret != EFI_UNSUPPORTED) { + if (ret != EFI_SUCCESS) { efi_st_error("GetVariable failed\n"); return EFI_ST_FAILURE; } memset(&guid, 0, 16); *varname = 0; + len = 2 * EFI_ST_MAX_VARNAME_SIZE; ret = runtime->get_next_variable_name(&len, varname, &guid); - if (ret != EFI_UNSUPPORTED) { + if (ret != EFI_SUCCESS) { efi_st_error("GetNextVariableName failed\n"); return EFI_ST_FAILURE; }