From 643cc8a41c0ce9b38dd577ea62ec8d01cdc9eb60 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Thu, 2 May 2019 23:03:39 +0100 Subject: [PATCH] Add patches to enable loading dbx and MOKX blacklists Import patches from: https://lore.kernel.org/patchwork/cover/933178/ that allow to also load dbx and MOKX as blacklists for modules. These patches also disable loading MOK/MOKX when secure boot is not enabled, as the variables will not be safe, and to check the variables attributes before accepting them. --- debian/changelog | 3 +- ...t-load-mok-when-secure-boot-disabled.patch | 59 +++++++++ ...002-MODSIGN-load-blacklist-from-MOKx.patch | 55 ++++++++ ...-hash-before-loading-a-kernel-module.patch | 124 ++++++++++++++++++ ...N-check-the-attributes-of-db-and-mok.patch | 103 +++++++++++++++ debian/patches/series | 4 + 6 files changed, 347 insertions(+), 1 deletion(-) create mode 100644 debian/patches/features/all/db-mok-keyring/0001-MODSIGN-do-not-load-mok-when-secure-boot-disabled.patch create mode 100644 debian/patches/features/all/db-mok-keyring/0002-MODSIGN-load-blacklist-from-MOKx.patch create mode 100644 debian/patches/features/all/db-mok-keyring/0003-MODSIGN-checking-the-blacklisted-hash-before-loading-a-kernel-module.patch create mode 100644 debian/patches/features/all/db-mok-keyring/0004-MODSIGN-check-the-attributes-of-db-and-mok.patch diff --git a/debian/changelog b/debian/changelog index b17f71d9b..2174aa1cc 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1105,7 +1105,8 @@ linux (4.19.37-1) UNRELEASED; urgency=medium libbpf-generate-pkg-config.patch from bpf-next. * Import patches to enable loading keys from UEFI db and MOK from http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git to - allow kernel modules built by users (eg: by dkms) to be verified. + allow kernel modules built by users (eg: by dkms) to be verified, and + to load dbx and MOKX for the equivalent blacklisting functionality. [ Bastian Blank ] * Don't longer recommend irqbalance. (closes: #926967) diff --git a/debian/patches/features/all/db-mok-keyring/0001-MODSIGN-do-not-load-mok-when-secure-boot-disabled.patch b/debian/patches/features/all/db-mok-keyring/0001-MODSIGN-do-not-load-mok-when-secure-boot-disabled.patch new file mode 100644 index 000000000..d7d7fa96b --- /dev/null +++ b/debian/patches/features/all/db-mok-keyring/0001-MODSIGN-do-not-load-mok-when-secure-boot-disabled.patch @@ -0,0 +1,59 @@ +Origin: https://lore.kernel.org/patchwork/cover/933178/ +From: "Lee, Chun-Yi" +Subject: [PATCH 1/5] MODSIGN: do not load mok when secure boot disabled + +The mok can not be trusted when the secure boot is disabled. Which +means that the kernel embedded certificate is the only trusted key. + +Due to db/dbx are authenticated variables, they needs manufacturer's +KEK for update. So db/dbx are secure when secureboot disabled. + +Signed-off-by: "Lee, Chun-Yi" +--- + certs/load_uefi.c | 26 +++++++++++++++----------- + 1 file changed, 15 insertions(+), 11 deletions(-) + +diff --git a/certs/load_uefi.c b/certs/load_uefi.c +index 3d88459..d6de4d0 100644 +--- a/certs/load_uefi.c ++++ b/certs/load_uefi.c +@@ -171,17 +171,6 @@ + } + } + +- rc = get_cert_list(L"MokListRT", &mok_var, &moksize, &mok); +- if (rc < 0) { +- pr_info("MODSIGN: Couldn't get UEFI MokListRT\n"); +- } else if (moksize != 0) { +- rc = parse_efi_signature_list("UEFI:MokListRT", +- mok, moksize, get_handler_for_db); +- if (rc) +- pr_err("Couldn't parse MokListRT signatures: %d\n", rc); +- kfree(mok); +- } +- + rc = get_cert_list(L"dbx", &secure_var, &dbxsize, &dbx); + if (rc < 0) { + pr_info("MODSIGN: Couldn't get UEFI dbx list\n"); +@@ -194,6 +183,21 @@ + kfree(dbx); + } + ++ /* the MOK can not be trusted when secure boot is disabled */ ++ if (!efi_enabled(EFI_SECURE_BOOT)) ++ return 0; ++ ++ rc = get_cert_list(L"MokListRT", &mok_var, &moksize, &mok); ++ if (rc < 0) { ++ pr_info("MODSIGN: Couldn't get UEFI MokListRT\n"); ++ } else if (moksize != 0) { ++ rc = parse_efi_signature_list("UEFI:MokListRT", ++ mok, moksize, get_handler_for_db); ++ if (rc) ++ pr_err("Couldn't parse MokListRT signatures: %d\n", rc); ++ kfree(mok); ++ } ++ + return rc; + } + late_initcall(load_uefi_certs); diff --git a/debian/patches/features/all/db-mok-keyring/0002-MODSIGN-load-blacklist-from-MOKx.patch b/debian/patches/features/all/db-mok-keyring/0002-MODSIGN-load-blacklist-from-MOKx.patch new file mode 100644 index 000000000..490c44103 --- /dev/null +++ b/debian/patches/features/all/db-mok-keyring/0002-MODSIGN-load-blacklist-from-MOKx.patch @@ -0,0 +1,55 @@ +Origin: https://lore.kernel.org/patchwork/cover/933178/ +From: "Lee, Chun-Yi" +Subject: [PATCH 2/4] MODSIGN: load blacklist from MOKx + +This patch adds the logic to load the blacklisted hash and +certificates from MOKx which is maintained by shim bootloader. + +Signed-off-by: "Lee, Chun-Yi" +--- + certs/load_uefi.c | 16 +++++++++++++--- + 1 file changed, 13 insertions(+), 3 deletions(-) + +diff --git a/certs/load_uefi.c b/certs/load_uefi.c +index f2f372b..dc66a79 100644 +--- a/certs/load_uefi.c ++++ b/certs/load_uefi.c +@@ -148,8 +148,8 @@ + { + efi_guid_t secure_var = EFI_IMAGE_SECURITY_DATABASE_GUID; + efi_guid_t mok_var = EFI_SHIM_LOCK_GUID; +- void *db = NULL, *dbx = NULL, *mok = NULL; +- unsigned long dbsize = 0, dbxsize = 0, moksize = 0; ++ void *db = NULL, *dbx = NULL, *mok = NULL, *mokx = NULL; ++ unsigned long dbsize = 0, dbxsize = 0, moksize = 0, mokxsize = 0; + int rc = 0; + + if (!efi.get_variable) +@@ -183,7 +183,7 @@ + kfree(dbx); + } + +- /* the MOK can not be trusted when secure boot is disabled */ ++ /* the MOK and MOKx can not be trusted when secure boot is disabled */ + if (!efi_enabled(EFI_SECURE_BOOT)) + return 0; + +@@ -198,6 +198,18 @@ + kfree(mok); + } + ++ rc = get_cert_list(L"MokListXRT", &mok_var, &mokxsize, &mokx); ++ if (rc < 0) { ++ pr_info("MODSIGN: Couldn't get UEFI MokListXRT\n"); ++ } else if (mokxsize != 0) { ++ rc = parse_efi_signature_list("UEFI:mokx", ++ mokx, mokxsize, ++ get_handler_for_dbx); ++ if (rc) ++ pr_err("Couldn't parse MokListXRT signatures: %d\n", rc); ++ kfree(mokx); ++ } ++ + return rc; + } + late_initcall(load_uefi_certs); diff --git a/debian/patches/features/all/db-mok-keyring/0003-MODSIGN-checking-the-blacklisted-hash-before-loading-a-kernel-module.patch b/debian/patches/features/all/db-mok-keyring/0003-MODSIGN-checking-the-blacklisted-hash-before-loading-a-kernel-module.patch new file mode 100644 index 000000000..757e8edd4 --- /dev/null +++ b/debian/patches/features/all/db-mok-keyring/0003-MODSIGN-checking-the-blacklisted-hash-before-loading-a-kernel-module.patch @@ -0,0 +1,124 @@ +Origin: https://lore.kernel.org/patchwork/cover/933178/ +From: "Lee, Chun-Yi" +Subject: [PATCH 3/4] MODSIGN: checking the blacklisted hash before loading a + kernel module + +This patch adds the logic for checking the kernel module's hash +base on blacklist. The hash must be generated by sha256 and enrolled +to dbx/mokx. + +For example: + sha256sum sample.ko + mokutil --mokx --import-hash $HASH_RESULT + +Whether the signature on ko file is stripped or not, the hash can be +compared by kernel. + +Signed-off-by: "Lee, Chun-Yi" +--- + kernel/module_signing.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++-- + 1 file changed, 60 insertions(+), 2 deletions(-) + +diff --git a/kernel/module_signing.c b/kernel/module_signing.c +index d3d6f95..d30ac74 100644 +--- a/kernel/module_signing.c ++++ b/kernel/module_signing.c +@@ -11,9 +11,12 @@ + + #include + #include ++#include + #include + #include + #include ++#include ++#include + #include "module-internal.h" + + enum pkey_id_type { +@@ -42,19 +45,67 @@ + __be32 sig_len; /* Length of signature data */ + }; + ++static int mod_is_hash_blacklisted(const void *mod, size_t verifylen) ++{ ++ struct crypto_shash *tfm; ++ struct shash_desc *desc; ++ size_t digest_size, desc_size; ++ u8 *digest; ++ int ret = 0; ++ ++ tfm = crypto_alloc_shash("sha256", 0, 0); ++ if (IS_ERR(tfm)) ++ goto error_return; ++ ++ desc_size = crypto_shash_descsize(tfm) + sizeof(*desc); ++ digest_size = crypto_shash_digestsize(tfm); ++ digest = kzalloc(digest_size + desc_size, GFP_KERNEL); ++ if (!digest) { ++ pr_err("digest memory buffer allocate fail\n"); ++ ret = -ENOMEM; ++ goto error_digest; ++ } ++ desc = (void *)digest + digest_size; ++ desc->tfm = tfm; ++ desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP; ++ ret = crypto_shash_init(desc); ++ if (ret < 0) ++ goto error_shash; ++ ++ ret = crypto_shash_finup(desc, mod, verifylen, digest); ++ if (ret < 0) ++ goto error_shash; ++ ++ pr_debug("%ld digest: %*phN\n", verifylen, (int) digest_size, digest); ++ ++ ret = is_hash_blacklisted(digest, digest_size, "bin"); ++ if (ret == -EKEYREJECTED) ++ pr_err("Module hash %*phN is blacklisted\n", ++ (int) digest_size, digest); ++ ++error_shash: ++ kfree(digest); ++error_digest: ++ crypto_free_shash(tfm); ++error_return: ++ return ret; ++} ++ + /* + * Verify the signature on a module. + */ + int mod_verify_sig(const void *mod, struct load_info *info) + { + struct module_signature ms; +- size_t sig_len, modlen = info->len; ++ size_t sig_len, modlen = info->len, wholelen; ++ int ret;; + + pr_devel("==>%s(,%zu)\n", __func__, modlen); + + if (modlen <= sizeof(ms)) + return -EBADMSG; + ++ wholelen = modlen + sizeof(MODULE_SIG_STRING) - 1; + memcpy(&ms, mod + (modlen - sizeof(ms)), sizeof(ms)); + modlen -= sizeof(ms); + +@@ -82,8 +133,15 @@ + return -EBADMSG; + } + +- return verify_pkcs7_signature(mod, modlen, mod + modlen, sig_len, ++ ret = verify_pkcs7_signature(mod, modlen, mod + modlen, sig_len, + VERIFY_USE_SECONDARY_KEYRING, + VERIFYING_MODULE_SIGNATURE, + NULL, NULL); ++ pr_devel("verify_pkcs7_signature() = %d\n", ret); ++ ++ /* checking hash of module is in blacklist */ ++ if (!ret) ++ ret = mod_is_hash_blacklisted(mod, wholelen); ++ ++ return ret; + } diff --git a/debian/patches/features/all/db-mok-keyring/0004-MODSIGN-check-the-attributes-of-db-and-mok.patch b/debian/patches/features/all/db-mok-keyring/0004-MODSIGN-check-the-attributes-of-db-and-mok.patch new file mode 100644 index 000000000..24857e236 --- /dev/null +++ b/debian/patches/features/all/db-mok-keyring/0004-MODSIGN-check-the-attributes-of-db-and-mok.patch @@ -0,0 +1,103 @@ +Origin: https://lore.kernel.org/patchwork/cover/933178/ +From: "Lee, Chun-Yi" +Subject: [PATCH 4/4] MODSIGN: check the attributes of db and mok + +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. + +Signed-off-by: "Lee, Chun-Yi" +--- + certs/load_uefi.c | 35 +++++++++++++++++++++++------------ + 1 file changed, 23 insertions(+), 12 deletions(-) + +diff --git a/certs/load_uefi.c b/certs/load_uefi.c +index dc66a79..52526bd 100644 +--- a/certs/load_uefi.c ++++ b/certs/load_uefi.c +@@ -36,12 +36,14 @@ + * Get a certificate list blob from the named EFI variable. + */ + static __init int get_cert_list(efi_char16_t *name, efi_guid_t *guid, +- unsigned long *size, void **cert_list) ++ unsigned long *size, void **cert_list, ++ u32 pos_attr, u32 neg_attr) + { + efi_status_t status; + 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) { +@@ -61,12 +63,19 @@ + return -ENOMEM; + } + +- 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 efi_status_to_err(status); + } ++ /* 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); ++ return -1; ++ } + + *size = lsize; + *cert_list = db; +@@ -159,7 +168,8 @@ + * an error if we can't get them. + */ + if (!uefi_check_ignore_db()) { +- rc = get_cert_list(L"db", &secure_var, &dbsize, &db); ++ rc = get_cert_list(L"db", &secure_var, &dbsize, &db, ++ EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS, 0); + if (rc < 0) { + pr_err("MODSIGN: Couldn't get UEFI db list\n"); + } else if (dbsize != 0) { +@@ -171,7 +181,8 @@ + } + } + +- rc = get_cert_list(L"dbx", &secure_var, &dbxsize, &dbx); ++ rc = get_cert_list(L"dbx", &secure_var, &dbxsize, &dbx, ++ EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS, 0); + if (rc < 0) { + pr_info("MODSIGN: Couldn't get UEFI dbx list\n"); + } else if (dbxsize != 0) { +@@ -187,7 +198,8 @@ + if (!efi_enabled(EFI_SECURE_BOOT)) + return 0; + +- rc = get_cert_list(L"MokListRT", &mok_var, &moksize, &mok); ++ rc = get_cert_list(L"MokListRT", &mok_var, &moksize, &mok, ++ 0, EFI_VARIABLE_NON_VOLATILE); + if (rc < 0) { + pr_info("MODSIGN: Couldn't get UEFI MokListRT\n"); + } else if (moksize != 0) { +@@ -198,7 +210,8 @@ + kfree(mok); + } + +- rc = get_cert_list(L"MokListXRT", &mok_var, &mokxsize, &mokx); ++ rc = get_cert_list(L"MokListXRT", &mok_var, &mokxsize, &mokx, ++ 0, EFI_VARIABLE_NON_VOLATILE); + if (rc < 0) { + pr_info("MODSIGN: Couldn't get UEFI MokListXRT\n"); + } else if (mokxsize != 0) { diff --git a/debian/patches/series b/debian/patches/series index 32ced0d13..f713f9f68 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -151,6 +151,10 @@ features/all/db-mok-keyring/0004-MODSIGN-Import-certificates-from-UEFI-Secure-Bo features/all/db-mok-keyring/0005-MODSIGN-Allow-the-db-UEFI-variable-to-be-suppressed.patch features/all/db-mok-keyring/0006-Make-get_cert_list-not-complain-about-cert-lists-tha.patch features/all/db-mok-keyring/0007-modsign-Use-secondary-trust-keyring-for-module-signi.patch +features/all/db-mok-keyring/0001-MODSIGN-do-not-load-mok-when-secure-boot-disabled.patch +features/all/db-mok-keyring/0002-MODSIGN-load-blacklist-from-MOKx.patch +features/all/db-mok-keyring/0003-MODSIGN-checking-the-blacklisted-hash-before-loading-a-kernel-module.patch +features/all/db-mok-keyring/0004-MODSIGN-check-the-attributes-of-db-and-mok.patch # Security fixes debian/i386-686-pae-pci-set-pci-nobios-by-default.patch