[PATCH 4/4] MODSIGN: check the attributes of db and mok
authorLee, Chun-Yi <joeyli.kernel@gmail.com>
Tue, 13 Mar 2018 10:38:03 +0000 (18:38 +0800)
committerSalvatore Bonaccorso <carnil@debian.org>
Mon, 6 Jun 2022 18:45:23 +0000 (19:45 +0100)
Origin: https://lore.kernel.org/patchwork/patch/933176/

That's better for checking the attributes of db and mok variables
before loading certificates to kernel keyring.

For db and dbx, both of them are authenticated variables. Which
means that they can only be modified by manufacturer's key. So
the kernel should checks EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS
attribute before we trust it.

For mok-rt and mokx-rt, both of them are created by shim boot loader
to forward the mok/mokx content to runtime. They must be runtime-volatile
variables. So kernel should checks that the attributes map did not set
EFI_VARIABLE_NON_VOLATILE bit before we trust it.

Cc: David Howells <dhowells@redhat.com>
Cc: Josh Boyer <jwboyer@fedoraproject.org>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
[Rebased by Luca Boccassi]
[bwh: Forward-ported to 5.5.9:
 - get_cert_list() takes a pointer to status and returns the cert list
 - Adjust filename, context]
[bwh: Forward-ported to 5.10: MokListRT and MokListXRT are now both
 loaded through a single code path.]
[bwh: Forward-ported to 5.13: No they aren't]

Gbp-Pq: Topic features/all/db-mok-keyring
Gbp-Pq: Name 0004-MODSIGN-check-the-attributes-of-db-and-mok.patch

security/integrity/platform_certs/load_uefi.c

index 5fc03fc333fe1331d6d82e9b2fba101a6a9ab03b..d3c41856a8c35a5b303c07300fa9f7e35e282a22 100644 (file)
@@ -36,11 +36,13 @@ static __init bool uefi_check_ignore_db(void)
  * Get a certificate list blob from the named EFI variable.
  */
 static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid,
-                                 unsigned long *size, efi_status_t *status)
+                                 unsigned long *size, efi_status_t *status,
+                                 u32 pos_attr, u32 neg_attr)
 {
        unsigned long lsize = 4;
        unsigned long tmpdb[4];
        void *db;
+       u32 attr = 0;
 
        *status = efi.get_variable(name, guid, NULL, &lsize, &tmpdb);
        if (*status == EFI_NOT_FOUND)
@@ -55,13 +57,22 @@ static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid,
        if (!db)
                return NULL;
 
-       *status = efi.get_variable(name, guid, NULL, &lsize, db);
+       *status = efi.get_variable(name, guid, &attr, &lsize, db);
        if (*status != EFI_SUCCESS) {
                kfree(db);
                pr_err("Error reading db var: 0x%lx\n", *status);
                return NULL;
        }
 
+       /* must have positive attributes and no negative attributes */
+       if ((pos_attr && !(attr & pos_attr)) ||
+           (neg_attr && (attr & neg_attr))) {
+               kfree(db);
+               pr_err("Error reading db var attributes: 0x%016x\n", attr);
+               *status = EFI_SECURITY_VIOLATION;
+               return NULL;
+       }
+
        *size = lsize;
        return db;
 }
@@ -107,7 +118,8 @@ static int __init load_moklist_certs(void)
        /* Get MokListRT. It might not exist, so it isn't an error
         * if we can't get it.
         */
-       mok = get_cert_list(L"MokListRT", &mok_var, &moksize, &status);
+       mok = get_cert_list(L"MokListRT", &mok_var, &moksize, &status,
+                               0, EFI_VARIABLE_NON_VOLATILE);
        if (mok) {
                rc = parse_efi_signature_list("UEFI:MokListRT",
                                              mok, moksize, get_handler_for_mok);
@@ -146,7 +158,8 @@ static int __init load_uefi_certs(void)
         * if we can't get them.
         */
        if (!uefi_check_ignore_db()) {
-               db = get_cert_list(L"db", &secure_var, &dbsize, &status);
+               db = get_cert_list(L"db", &secure_var, &dbsize, &status,
+                       EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS, 0);
                if (!db) {
                        if (status == EFI_NOT_FOUND)
                                pr_debug("MODSIGN: db variable wasn't found\n");
@@ -162,7 +175,8 @@ static int __init load_uefi_certs(void)
                }
        }
 
-       dbx = get_cert_list(L"dbx", &secure_var, &dbxsize, &status);
+       dbx = get_cert_list(L"dbx", &secure_var, &dbxsize, &status,
+               EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS, 0);
        if (!dbx) {
                if (status == EFI_NOT_FOUND)
                        pr_debug("dbx variable wasn't found\n");
@@ -181,7 +195,8 @@ static int __init load_uefi_certs(void)
        if (!efi_enabled(EFI_SECURE_BOOT))
                return 0;
 
-       mokx = get_cert_list(L"MokListXRT", &mok_var, &mokxsize, &status);
+       mokx = get_cert_list(L"MokListXRT", &mok_var, &mokxsize, &status,
+                               0, EFI_VARIABLE_NON_VOLATILE);
        if (!mokx) {
                if (status == EFI_NOT_FOUND)
                        pr_debug("mokx variable wasn't found\n");