From bff24699b94a34c5fcb8d3283794e7d39adb092c Mon Sep 17 00:00:00 2001 From: Jarkko Sakkinen Date: Mon, 14 Aug 2023 19:17:49 +0300 Subject: [PATCH 01/17] tpm_tis: Revert "tpm_tis: Disable interrupts on ThinkPad T490s" Since for MMIO driver using FIFO registers, also known as tpm_tis, the default (and tbh recommended) behaviour is now the polling mode, the "tristate" workaround is no longer for benefit. If someone wants to explicitly enable IRQs for a TPM chip that should be without question allowed. It could very well be a piece hardware in the existing deny list because of e.g. firmware update or something similar. While at it, document the module parameter, as this was not done in 2006 when it first appeared in the mainline. Link: https://lore.kernel.org/linux-integrity/20201015214430.17937-1-jsnitsel@redhat.com/ Link: https://lore.kernel.org/all/1145393776.4829.19.camel@localhost.localdomain/ Reviewed-by: Jerry Snitselaar Signed-off-by: Jarkko Sakkinen --- .../admin-guide/kernel-parameters.txt | 7 ++ drivers/char/tpm/tpm_tis.c | 93 +------------------ 2 files changed, 9 insertions(+), 91 deletions(-) diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 722b6eca2e93..155101120438 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -6340,6 +6340,13 @@ This will guarantee that all the other pcrs are saved. + tpm_tis.interrupts= [HW,TPM] + Enable interrupts for the MMIO based physical layer + for the FIFO interface. By default it is set to false + (0). For more information about TPM hardware interfaces + defined by Trusted Computing Group (TCG) see + https://trustedcomputinggroup.org/resource/pc-client-platform-tpm-profile-ptp-specification/ + tp_printk [FTRACE] Have the tracepoints sent to printk as well as the tracing ring buffer. This is useful for early boot up diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c index 7fa3d91042b2..077fdb73740c 100644 --- a/drivers/char/tpm/tpm_tis.c +++ b/drivers/char/tpm/tpm_tis.c @@ -27,7 +27,6 @@ #include #include #include -#include #include "tpm.h" #include "tpm_tis_core.h" @@ -89,8 +88,8 @@ static inline void tpm_tis_iowrite32(u32 b, void __iomem *iobase, u32 addr) tpm_tis_flush(iobase); } -static int interrupts; -module_param(interrupts, int, 0444); +static bool interrupts; +module_param(interrupts, bool, 0444); MODULE_PARM_DESC(interrupts, "Enable interrupts"); static bool itpm; @@ -103,92 +102,6 @@ module_param(force, bool, 0444); MODULE_PARM_DESC(force, "Force device probe rather than using ACPI entry"); #endif -static int tpm_tis_disable_irq(const struct dmi_system_id *d) -{ - if (interrupts == -1) { - pr_notice("tpm_tis: %s detected: disabling interrupts.\n", d->ident); - interrupts = 0; - } - - return 0; -} - -static const struct dmi_system_id tpm_tis_dmi_table[] = { - { - .callback = tpm_tis_disable_irq, - .ident = "Framework Laptop (12th Gen Intel Core)", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "Framework"), - DMI_MATCH(DMI_PRODUCT_NAME, "Laptop (12th Gen Intel Core)"), - }, - }, - { - .callback = tpm_tis_disable_irq, - .ident = "Framework Laptop (13th Gen Intel Core)", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "Framework"), - DMI_MATCH(DMI_PRODUCT_NAME, "Laptop (13th Gen Intel Core)"), - }, - }, - { - .callback = tpm_tis_disable_irq, - .ident = "ThinkPad T490s", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), - DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T490s"), - }, - }, - { - .callback = tpm_tis_disable_irq, - .ident = "ThinkStation P360 Tiny", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), - DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkStation P360 Tiny"), - }, - }, - { - .callback = tpm_tis_disable_irq, - .ident = "ThinkPad L490", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), - DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad L490"), - }, - }, - { - .callback = tpm_tis_disable_irq, - .ident = "ThinkPad L590", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), - DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad L590"), - }, - }, - { - .callback = tpm_tis_disable_irq, - .ident = "ThinkStation P620", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), - DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkStation P620"), - }, - }, - { - .callback = tpm_tis_disable_irq, - .ident = "TUXEDO InfinityBook S 15/17 Gen7", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "TUXEDO"), - DMI_MATCH(DMI_PRODUCT_NAME, "TUXEDO InfinityBook S 15/17 Gen7"), - }, - }, - { - .callback = tpm_tis_disable_irq, - .ident = "UPX-TGL", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "AAEON"), - DMI_MATCH(DMI_PRODUCT_NAME, "UPX-TGL01"), - }, - }, - {} -}; - #if defined(CONFIG_PNP) && defined(CONFIG_ACPI) static int has_hid(struct acpi_device *dev, const char *hid) { @@ -312,8 +225,6 @@ static int tpm_tis_init(struct device *dev, struct tpm_info *tpm_info) int irq = -1; int rc; - dmi_check_system(tpm_tis_dmi_table); - rc = check_acpi_tpm2(dev); if (rc) return rc; From 4cfb908054456ad8b6b8cd5108bbdf80faade8cd Mon Sep 17 00:00:00 2001 From: Eric Snowberg Date: Mon, 22 May 2023 19:09:42 -0400 Subject: [PATCH 02/17] KEYS: DigitalSignature link restriction Add a new link restriction. Restrict the addition of keys in a keyring based on the key having digitalSignature usage set. Additionally, verify the new certificate against the ones in the system keyrings. Add two additional functions to use the new restriction within either the builtin or secondary keyrings. [jarkko@kernel.org: Fix checkpatch.pl --strict issues] Signed-off-by: Eric Snowberg Reviewed-and-tested-by: Mimi Zohar Reviewed-by: Jarkko Sakkinen Signed-off-by: Jarkko Sakkinen --- certs/system_keyring.c | 49 +++++++++++++++++++++++++++++++ crypto/asymmetric_keys/restrict.c | 44 +++++++++++++++++++++++++++ include/crypto/public_key.h | 12 ++++++++ include/keys/system_keyring.h | 10 +++++++ 4 files changed, 115 insertions(+) diff --git a/certs/system_keyring.c b/certs/system_keyring.c index a7a49b17ceb1..b348e0898d34 100644 --- a/certs/system_keyring.c +++ b/certs/system_keyring.c @@ -51,6 +51,26 @@ int restrict_link_by_builtin_trusted(struct key *dest_keyring, builtin_trusted_keys); } +/** + * restrict_link_by_digsig_builtin - Restrict digitalSignature key additions by the built-in keyring + * @dest_keyring: Keyring being linked to. + * @type: The type of key being added. + * @payload: The payload of the new key. + * @restriction_key: A ring of keys that can be used to vouch for the new cert. + * + * Restrict the addition of keys into a keyring based on the key-to-be-added + * being vouched for by a key in the built in system keyring. The new key + * must have the digitalSignature usage field set. + */ +int restrict_link_by_digsig_builtin(struct key *dest_keyring, + const struct key_type *type, + const union key_payload *payload, + struct key *restriction_key) +{ + return restrict_link_by_digsig(dest_keyring, type, payload, + builtin_trusted_keys); +} + #ifdef CONFIG_SECONDARY_TRUSTED_KEYRING /** * restrict_link_by_builtin_and_secondary_trusted - Restrict keyring @@ -83,6 +103,35 @@ int restrict_link_by_builtin_and_secondary_trusted( secondary_trusted_keys); } +/** + * restrict_link_by_digsig_builtin_and_secondary - Restrict by digitalSignature. + * @dest_keyring: Keyring being linked to. + * @type: The type of key being added. + * @payload: The payload of the new key. + * @restrict_key: A ring of keys that can be used to vouch for the new cert. + * + * Restrict the addition of keys into a keyring based on the key-to-be-added + * being vouched for by a key in either the built-in or the secondary system + * keyrings. The new key must have the digitalSignature usage field set. + */ +int restrict_link_by_digsig_builtin_and_secondary(struct key *dest_keyring, + const struct key_type *type, + const union key_payload *payload, + struct key *restrict_key) +{ + /* If we have a secondary trusted keyring, then that contains a link + * through to the builtin keyring and the search will follow that link. + */ + if (type == &key_type_keyring && + dest_keyring == secondary_trusted_keys && + payload == &builtin_trusted_keys->payload) + /* Allow the builtin keyring to be added to the secondary */ + return 0; + + return restrict_link_by_digsig(dest_keyring, type, payload, + secondary_trusted_keys); +} + /* * Allocate a struct key_restriction for the "builtin and secondary trust" * keyring. Only for use in system_trusted_keyring_init(). diff --git a/crypto/asymmetric_keys/restrict.c b/crypto/asymmetric_keys/restrict.c index 276bdb627498..6b69ea40da23 100644 --- a/crypto/asymmetric_keys/restrict.c +++ b/crypto/asymmetric_keys/restrict.c @@ -148,6 +148,50 @@ int restrict_link_by_ca(struct key *dest_keyring, return 0; } +/** + * restrict_link_by_digsig - Restrict additions to a ring of digsig keys + * @dest_keyring: Keyring being linked to. + * @type: The type of key being added. + * @payload: The payload of the new key. + * @trust_keyring: A ring of keys that can be used to vouch for the new cert. + * + * Check if the new certificate has digitalSignature usage set. If it is, + * then mark the new certificate as being ok to link. Afterwards verify + * the new certificate against the ones in the trust_keyring. + * + * Returns 0 if the new certificate was accepted, -ENOKEY if the + * certificate is not a digsig. -ENOPKG if the signature uses unsupported + * crypto, or some other error if there is a matching certificate but + * the signature check cannot be performed. + */ +int restrict_link_by_digsig(struct key *dest_keyring, + const struct key_type *type, + const union key_payload *payload, + struct key *trust_keyring) +{ + const struct public_key *pkey; + + if (type != &key_type_asymmetric) + return -EOPNOTSUPP; + + pkey = payload->data[asym_crypto]; + + if (!pkey) + return -ENOPKG; + + if (!test_bit(KEY_EFLAG_DIGITALSIG, &pkey->key_eflags)) + return -ENOKEY; + + if (test_bit(KEY_EFLAG_CA, &pkey->key_eflags)) + return -ENOKEY; + + if (test_bit(KEY_EFLAG_KEYCERTSIGN, &pkey->key_eflags)) + return -ENOKEY; + + return restrict_link_by_signature(dest_keyring, type, payload, + trust_keyring); +} + static bool match_either_id(const struct asymmetric_key_id **pair, const struct asymmetric_key_id *single) { diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h index 8fadd561c50e..462f8a34cdf8 100644 --- a/include/crypto/public_key.h +++ b/include/crypto/public_key.h @@ -78,6 +78,10 @@ extern int restrict_link_by_ca(struct key *dest_keyring, const struct key_type *type, const union key_payload *payload, struct key *trust_keyring); +int restrict_link_by_digsig(struct key *dest_keyring, + const struct key_type *type, + const union key_payload *payload, + struct key *trust_keyring); #else static inline int restrict_link_by_ca(struct key *dest_keyring, const struct key_type *type, @@ -86,6 +90,14 @@ static inline int restrict_link_by_ca(struct key *dest_keyring, { return 0; } + +static inline int restrict_link_by_digsig(struct key *dest_keyring, + const struct key_type *type, + const union key_payload *payload, + struct key *trust_keyring) +{ + return 0; +} #endif extern int query_asymmetric_key(const struct kernel_pkey_params *, diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h index 91e080efb918..7e2583208820 100644 --- a/include/keys/system_keyring.h +++ b/include/keys/system_keyring.h @@ -23,10 +23,15 @@ extern int restrict_link_by_builtin_trusted(struct key *keyring, const struct key_type *type, const union key_payload *payload, struct key *restriction_key); +int restrict_link_by_digsig_builtin(struct key *dest_keyring, + const struct key_type *type, + const union key_payload *payload, + struct key *restriction_key); extern __init int load_module_cert(struct key *keyring); #else #define restrict_link_by_builtin_trusted restrict_link_reject +#define restrict_link_by_digsig_builtin restrict_link_reject static inline __init int load_module_cert(struct key *keyring) { @@ -41,8 +46,13 @@ extern int restrict_link_by_builtin_and_secondary_trusted( const struct key_type *type, const union key_payload *payload, struct key *restriction_key); +int restrict_link_by_digsig_builtin_and_secondary(struct key *keyring, + const struct key_type *type, + const union key_payload *payload, + struct key *restriction_key); #else #define restrict_link_by_builtin_and_secondary_trusted restrict_link_by_builtin_trusted +#define restrict_link_by_digsig_builtin_and_secondary restrict_link_by_digsig_builtin #endif #ifdef CONFIG_INTEGRITY_MACHINE_KEYRING From 90f6f691a706754e33d2d0c6fa2e1dacedb477f6 Mon Sep 17 00:00:00 2001 From: Eric Snowberg Date: Mon, 22 May 2023 19:09:43 -0400 Subject: [PATCH 03/17] integrity: Enforce digitalSignature usage in the ima and evm keyrings After being vouched for by a system keyring, only allow keys into the .ima and .evm keyrings that have the digitalSignature usage field set. Link: https://lore.kernel.org/all/41dffdaeb7eb7840f7e38bc691fbda836635c9f9.camel@linux.ibm.com Suggested-by: Mimi Zohar Signed-off-by: Eric Snowberg Acked-and-tested-by: Mimi Zohar Reviewed-by: Jarkko Sakkinen Signed-off-by: Jarkko Sakkinen --- security/integrity/digsig.c | 4 ++-- security/integrity/evm/Kconfig | 3 ++- security/integrity/ima/Kconfig | 3 ++- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c index 6f31ffe23c48..d0704b1597d4 100644 --- a/security/integrity/digsig.c +++ b/security/integrity/digsig.c @@ -34,9 +34,9 @@ static const char * const keyring_name[INTEGRITY_KEYRING_MAX] = { }; #ifdef CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY -#define restrict_link_to_ima restrict_link_by_builtin_and_secondary_trusted +#define restrict_link_to_ima restrict_link_by_digsig_builtin_and_secondary #else -#define restrict_link_to_ima restrict_link_by_builtin_trusted +#define restrict_link_to_ima restrict_link_by_digsig_builtin #endif static struct key *integrity_keyring_from_id(const unsigned int id) diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig index a6e19d23e700..fba9ee359bc9 100644 --- a/security/integrity/evm/Kconfig +++ b/security/integrity/evm/Kconfig @@ -64,7 +64,8 @@ config EVM_LOAD_X509 This option enables X509 certificate loading from the kernel onto the '.evm' trusted keyring. A public key can be used to - verify EVM integrity starting from the 'init' process. + verify EVM integrity starting from the 'init' process. The + key must have digitalSignature usage set. config EVM_X509_PATH string "EVM X509 certificate path" diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig index 60a511c6b583..684425936c53 100644 --- a/security/integrity/ima/Kconfig +++ b/security/integrity/ima/Kconfig @@ -270,7 +270,8 @@ config IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY help Keys may be added to the IMA or IMA blacklist keyrings, if the key is validly signed by a CA cert in the system built-in or - secondary trusted keyrings. + secondary trusted keyrings. The key must also have the + digitalSignature usage set. Intermediate keys between those the kernel has compiled in and the IMA keys to be added may be added to the system secondary keyring, From a3af7188e360aea343611dd385056eec44e10175 Mon Sep 17 00:00:00 2001 From: Nayna Jain Date: Tue, 15 Aug 2023 07:27:17 -0400 Subject: [PATCH 04/17] integrity: PowerVM support for loading CA keys on machine keyring Keys that derive their trust from an entity such as a security officer, administrator, system owner, or machine owner are said to have "imputed trust". CA keys with imputed trust can be loaded onto the machine keyring. The mechanism for loading these keys onto the machine keyring is platform dependent. Load keys stored in the variable trustedcadb onto the .machine keyring on PowerVM platform. Signed-off-by: Nayna Jain Reviewed-and-tested-by: Mimi Zohar Reviewed-by: Jarkko Sakkinen Tested-by: Nageswara R Sastry Signed-off-by: Jarkko Sakkinen --- .../integrity/platform_certs/keyring_handler.c | 8 ++++++++ .../integrity/platform_certs/keyring_handler.h | 5 +++++ .../integrity/platform_certs/load_powerpc.c | 17 +++++++++++++++++ 3 files changed, 30 insertions(+) diff --git a/security/integrity/platform_certs/keyring_handler.c b/security/integrity/platform_certs/keyring_handler.c index 8a1124e4d769..1649d047e3b8 100644 --- a/security/integrity/platform_certs/keyring_handler.c +++ b/security/integrity/platform_certs/keyring_handler.c @@ -69,6 +69,14 @@ __init efi_element_handler_t get_handler_for_mok(const efi_guid_t *sig_type) return NULL; } +__init efi_element_handler_t get_handler_for_ca_keys(const efi_guid_t *sig_type) +{ + if (efi_guidcmp(*sig_type, efi_cert_x509_guid) == 0) + return add_to_machine_keyring; + + return NULL; +} + /* * Return the appropriate handler for particular signature list types found in * the UEFI dbx and MokListXRT tables. diff --git a/security/integrity/platform_certs/keyring_handler.h b/security/integrity/platform_certs/keyring_handler.h index 212d894a8c0c..6f15bb4cc8dc 100644 --- a/security/integrity/platform_certs/keyring_handler.h +++ b/security/integrity/platform_certs/keyring_handler.h @@ -29,6 +29,11 @@ efi_element_handler_t get_handler_for_db(const efi_guid_t *sig_type); */ efi_element_handler_t get_handler_for_mok(const efi_guid_t *sig_type); +/* + * Return the handler for particular signature list types for CA keys. + */ +efi_element_handler_t get_handler_for_ca_keys(const efi_guid_t *sig_type); + /* * Return the handler for particular signature list types found in the dbx. */ diff --git a/security/integrity/platform_certs/load_powerpc.c b/security/integrity/platform_certs/load_powerpc.c index 170789dc63d2..339053d9726d 100644 --- a/security/integrity/platform_certs/load_powerpc.c +++ b/security/integrity/platform_certs/load_powerpc.c @@ -59,6 +59,7 @@ static __init void *get_cert_list(u8 *key, unsigned long keylen, u64 *size) static int __init load_powerpc_certs(void) { void *db = NULL, *dbx = NULL, *data = NULL; + void *trustedca; u64 dsize = 0; u64 offset = 0; int rc = 0; @@ -120,6 +121,22 @@ static int __init load_powerpc_certs(void) kfree(data); } + data = get_cert_list("trustedcadb", 12, &dsize); + if (!data) { + pr_info("Couldn't get trustedcadb list from firmware\n"); + } else if (IS_ERR(data)) { + rc = PTR_ERR(data); + pr_err("Error reading trustedcadb from firmware: %d\n", rc); + } else { + extract_esl(trustedca, data, dsize, offset); + + rc = parse_efi_signature_list("powerpc:trustedca", trustedca, dsize, + get_handler_for_ca_keys); + if (rc) + pr_err("Couldn't parse trustedcadb signatures: %d\n", rc); + kfree(data); + } + return rc; } late_initcall(load_powerpc_certs); From bc02667698b0e828f566e031d5020298b8d0c5de Mon Sep 17 00:00:00 2001 From: Nayna Jain Date: Tue, 15 Aug 2023 07:27:18 -0400 Subject: [PATCH 05/17] integrity: ignore keys failing CA restrictions on non-UEFI platform On non-UEFI platforms, handle restrict_link_by_ca failures differently. Certificates which do not satisfy CA restrictions on non-UEFI platforms are ignored. Signed-off-by: Nayna Jain Reviewed-and-tested-by: Mimi Zohar Acked-by: Jarkko Sakkinen Tested-by: Nageswara R Sastry Signed-off-by: Jarkko Sakkinen --- security/integrity/platform_certs/machine_keyring.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/security/integrity/platform_certs/machine_keyring.c b/security/integrity/platform_certs/machine_keyring.c index 7aaed7950b6e..389a6e7c9245 100644 --- a/security/integrity/platform_certs/machine_keyring.c +++ b/security/integrity/platform_certs/machine_keyring.c @@ -36,7 +36,7 @@ void __init add_to_machine_keyring(const char *source, const void *data, size_t * If the restriction check does not pass and the platform keyring * is configured, try to add it into that keyring instead. */ - if (rc && IS_ENABLED(CONFIG_INTEGRITY_PLATFORM_KEYRING)) + if (rc && efi_enabled(EFI_BOOT) && IS_ENABLED(CONFIG_INTEGRITY_PLATFORM_KEYRING)) rc = integrity_load_cert(INTEGRITY_KEYRING_PLATFORM, source, data, len, perm); From 7b9de406582d12b72cce72c056b5678e8c0627eb Mon Sep 17 00:00:00 2001 From: Nayna Jain Date: Tue, 15 Aug 2023 07:27:19 -0400 Subject: [PATCH 06/17] integrity: remove global variable from machine_keyring.c trust_mok variable is accessed within a single function locally. Change trust_mok from global to local static variable. Signed-off-by: Nayna Jain Reviewed-and-tested-by: Mimi Zohar Reviewed-by: Jarkko Sakkinen Tested-by: Nageswara R Sastry Signed-off-by: Jarkko Sakkinen --- security/integrity/platform_certs/machine_keyring.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/security/integrity/platform_certs/machine_keyring.c b/security/integrity/platform_certs/machine_keyring.c index 389a6e7c9245..9482e16cb2ca 100644 --- a/security/integrity/platform_certs/machine_keyring.c +++ b/security/integrity/platform_certs/machine_keyring.c @@ -8,8 +8,6 @@ #include #include "../integrity.h" -static bool trust_mok; - static __init int machine_keyring_init(void) { int rc; @@ -65,9 +63,11 @@ static __init bool uefi_check_trust_mok_keys(void) bool __init trust_moklist(void) { static bool initialized; + static bool trust_mok; if (!initialized) { initialized = true; + trust_mok = false; if (uefi_check_trust_mok_keys()) trust_mok = true; From 4cb1ed94f18047d0863f976bc95aa7c0584cc51c Mon Sep 17 00:00:00 2001 From: Nayna Jain Date: Tue, 15 Aug 2023 07:27:20 -0400 Subject: [PATCH 07/17] integrity: check whether imputed trust is enabled trust_moklist() is specific to UEFI enabled systems. Other platforms rely only on the Kconfig. Define a generic wrapper named imputed_trust_enabled(). Signed-off-by: Nayna Jain Reviewed-by: Mimi Zohar Tested-by: Nageswara R Sastry Signed-off-by: Jarkko Sakkinen --- security/integrity/digsig.c | 2 +- security/integrity/integrity.h | 5 +++-- .../integrity/platform_certs/keyring_handler.c | 3 ++- .../integrity/platform_certs/machine_keyring.c | 18 ++++++++++++++++-- 4 files changed, 22 insertions(+), 6 deletions(-) diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c index d0704b1597d4..df387de29bfa 100644 --- a/security/integrity/digsig.c +++ b/security/integrity/digsig.c @@ -113,7 +113,7 @@ static int __init __integrity_init_keyring(const unsigned int id, } else { if (id == INTEGRITY_KEYRING_PLATFORM) set_platform_trusted_keys(keyring[id]); - if (id == INTEGRITY_KEYRING_MACHINE && trust_moklist()) + if (id == INTEGRITY_KEYRING_MACHINE && imputed_trust_enabled()) set_machine_trusted_keys(keyring[id]); if (id == INTEGRITY_KEYRING_IMA) load_module_cert(keyring[id]); diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h index 7167a6e99bdc..d7553c93f5c0 100644 --- a/security/integrity/integrity.h +++ b/security/integrity/integrity.h @@ -320,13 +320,14 @@ static inline void __init add_to_platform_keyring(const char *source, #ifdef CONFIG_INTEGRITY_MACHINE_KEYRING void __init add_to_machine_keyring(const char *source, const void *data, size_t len); -bool __init trust_moklist(void); +bool __init imputed_trust_enabled(void); #else static inline void __init add_to_machine_keyring(const char *source, const void *data, size_t len) { } -static inline bool __init trust_moklist(void) + +static inline bool __init imputed_trust_enabled(void) { return false; } diff --git a/security/integrity/platform_certs/keyring_handler.c b/security/integrity/platform_certs/keyring_handler.c index 1649d047e3b8..586027b9a3f5 100644 --- a/security/integrity/platform_certs/keyring_handler.c +++ b/security/integrity/platform_certs/keyring_handler.c @@ -61,7 +61,8 @@ __init efi_element_handler_t get_handler_for_db(const efi_guid_t *sig_type) __init efi_element_handler_t get_handler_for_mok(const efi_guid_t *sig_type) { if (efi_guidcmp(*sig_type, efi_cert_x509_guid) == 0) { - if (IS_ENABLED(CONFIG_INTEGRITY_MACHINE_KEYRING) && trust_moklist()) + if (IS_ENABLED(CONFIG_INTEGRITY_MACHINE_KEYRING) && + imputed_trust_enabled()) return add_to_machine_keyring; else return add_to_platform_keyring; diff --git a/security/integrity/platform_certs/machine_keyring.c b/security/integrity/platform_certs/machine_keyring.c index 9482e16cb2ca..a401640a63cd 100644 --- a/security/integrity/platform_certs/machine_keyring.c +++ b/security/integrity/platform_certs/machine_keyring.c @@ -34,7 +34,8 @@ void __init add_to_machine_keyring(const char *source, const void *data, size_t * If the restriction check does not pass and the platform keyring * is configured, try to add it into that keyring instead. */ - if (rc && efi_enabled(EFI_BOOT) && IS_ENABLED(CONFIG_INTEGRITY_PLATFORM_KEYRING)) + if (rc && efi_enabled(EFI_BOOT) && + IS_ENABLED(CONFIG_INTEGRITY_PLATFORM_KEYRING)) rc = integrity_load_cert(INTEGRITY_KEYRING_PLATFORM, source, data, len, perm); @@ -60,7 +61,7 @@ static __init bool uefi_check_trust_mok_keys(void) return false; } -bool __init trust_moklist(void) +static bool __init trust_moklist(void) { static bool initialized; static bool trust_mok; @@ -75,3 +76,16 @@ bool __init trust_moklist(void) return trust_mok; } + +/* + * Provides platform specific check for trusting imputed keys before loading + * on .machine keyring. UEFI systems enable this trust based on a variable, + * and for other platforms, it is always enabled. + */ +bool __init imputed_trust_enabled(void) +{ + if (efi_enabled(EFI_BOOT)) + return trust_moklist(); + + return true; +} From d7d91c4743c4ef0f60b7556d2794b6dd27cda373 Mon Sep 17 00:00:00 2001 From: Nayna Jain Date: Tue, 15 Aug 2023 07:27:21 -0400 Subject: [PATCH 08/17] integrity: PowerVM machine keyring enablement Update Kconfig to enable machine keyring and limit to CA certificates on PowerVM. Only key signing CA keys are allowed. Signed-off-by: Nayna Jain Reviewed-and-tested-by: Mimi Zohar Reviewed-by: Jarkko Sakkinen Tested-by: Nageswara R Sastry Signed-off-by: Jarkko Sakkinen --- security/integrity/Kconfig | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig index ec6e0d789da1..232191ee09e3 100644 --- a/security/integrity/Kconfig +++ b/security/integrity/Kconfig @@ -67,7 +67,9 @@ config INTEGRITY_MACHINE_KEYRING depends on SECONDARY_TRUSTED_KEYRING depends on INTEGRITY_ASYMMETRIC_KEYS depends on SYSTEM_BLACKLIST_KEYRING - depends on LOAD_UEFI_KEYS + depends on LOAD_UEFI_KEYS || LOAD_PPC_KEYS + select INTEGRITY_CA_MACHINE_KEYRING if LOAD_PPC_KEYS + select INTEGRITY_CA_MACHINE_KEYRING_MAX if LOAD_PPC_KEYS help If set, provide a keyring to which Machine Owner Keys (MOK) may be added. This keyring shall contain just MOK keys. Unlike keys From 44e69ea53892f18e8753943a4376de20b076c3fe Mon Sep 17 00:00:00 2001 From: Nayna Jain Date: Tue, 15 Aug 2023 07:27:22 -0400 Subject: [PATCH 09/17] integrity: PowerVM support for loading third party code signing keys On secure boot enabled PowerVM LPAR, third party code signing keys are needed during early boot to verify signed third party modules. These third party keys are stored in moduledb object in the Platform KeyStore (PKS). Load third party code signing keys onto .secondary_trusted_keys keyring. Signed-off-by: Nayna Jain Reviewed-and-tested-by: Mimi Zohar Reviewed-by: Jarkko Sakkinen Tested-by: Nageswara R Sastry Signed-off-by: Jarkko Sakkinen --- certs/system_keyring.c | 30 +++++++++++++++++++ include/keys/system_keyring.h | 4 +++ .../platform_certs/keyring_handler.c | 8 +++++ .../platform_certs/keyring_handler.h | 5 ++++ .../integrity/platform_certs/load_powerpc.c | 17 +++++++++++ 5 files changed, 64 insertions(+) diff --git a/certs/system_keyring.c b/certs/system_keyring.c index b348e0898d34..33841c91f12c 100644 --- a/certs/system_keyring.c +++ b/certs/system_keyring.c @@ -152,6 +152,36 @@ static __init struct key_restriction *get_builtin_and_secondary_restriction(void return restriction; } + +/** + * add_to_secondary_keyring - Add to secondary keyring. + * @source: Source of key + * @data: The blob holding the key + * @len: The length of the data blob + * + * Add a key to the secondary keyring. The key must be vouched for by a key in the builtin, + * machine or secondary keyring itself. + */ +void __init add_to_secondary_keyring(const char *source, const void *data, size_t len) +{ + key_ref_t key; + key_perm_t perm; + + perm = (KEY_POS_ALL & ~KEY_POS_SETATTR) | KEY_USR_VIEW; + + key = key_create_or_update(make_key_ref(secondary_trusted_keys, 1), + "asymmetric", + NULL, data, len, perm, + KEY_ALLOC_NOT_IN_QUOTA); + if (IS_ERR(key)) { + pr_err("Problem loading X.509 certificate from %s to secondary keyring %ld\n", + source, PTR_ERR(key)); + return; + } + + pr_notice("Loaded X.509 cert '%s'\n", key_ref_to_ptr(key)->description); + key_ref_put(key); +} #endif #ifdef CONFIG_INTEGRITY_MACHINE_KEYRING void __init set_machine_trusted_keys(struct key *keyring) diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h index 7e2583208820..8365adf842ef 100644 --- a/include/keys/system_keyring.h +++ b/include/keys/system_keyring.h @@ -50,9 +50,13 @@ int restrict_link_by_digsig_builtin_and_secondary(struct key *keyring, const struct key_type *type, const union key_payload *payload, struct key *restriction_key); +void __init add_to_secondary_keyring(const char *source, const void *data, size_t len); #else #define restrict_link_by_builtin_and_secondary_trusted restrict_link_by_builtin_trusted #define restrict_link_by_digsig_builtin_and_secondary restrict_link_by_digsig_builtin +static inline void __init add_to_secondary_keyring(const char *source, const void *data, size_t len) +{ +} #endif #ifdef CONFIG_INTEGRITY_MACHINE_KEYRING diff --git a/security/integrity/platform_certs/keyring_handler.c b/security/integrity/platform_certs/keyring_handler.c index 586027b9a3f5..13ea17207902 100644 --- a/security/integrity/platform_certs/keyring_handler.c +++ b/security/integrity/platform_certs/keyring_handler.c @@ -78,6 +78,14 @@ __init efi_element_handler_t get_handler_for_ca_keys(const efi_guid_t *sig_type) return NULL; } +__init efi_element_handler_t get_handler_for_code_signing_keys(const efi_guid_t *sig_type) +{ + if (efi_guidcmp(*sig_type, efi_cert_x509_guid) == 0) + return add_to_secondary_keyring; + + return NULL; +} + /* * Return the appropriate handler for particular signature list types found in * the UEFI dbx and MokListXRT tables. diff --git a/security/integrity/platform_certs/keyring_handler.h b/security/integrity/platform_certs/keyring_handler.h index 6f15bb4cc8dc..f92895cc50f6 100644 --- a/security/integrity/platform_certs/keyring_handler.h +++ b/security/integrity/platform_certs/keyring_handler.h @@ -34,6 +34,11 @@ efi_element_handler_t get_handler_for_mok(const efi_guid_t *sig_type); */ efi_element_handler_t get_handler_for_ca_keys(const efi_guid_t *sig_type); +/* + * Return the handler for particular signature list types for code signing keys. + */ +efi_element_handler_t get_handler_for_code_signing_keys(const efi_guid_t *sig_type); + /* * Return the handler for particular signature list types found in the dbx. */ diff --git a/security/integrity/platform_certs/load_powerpc.c b/security/integrity/platform_certs/load_powerpc.c index 339053d9726d..c85febca3343 100644 --- a/security/integrity/platform_certs/load_powerpc.c +++ b/security/integrity/platform_certs/load_powerpc.c @@ -60,6 +60,7 @@ static int __init load_powerpc_certs(void) { void *db = NULL, *dbx = NULL, *data = NULL; void *trustedca; + void *moduledb; u64 dsize = 0; u64 offset = 0; int rc = 0; @@ -137,6 +138,22 @@ static int __init load_powerpc_certs(void) kfree(data); } + data = get_cert_list("moduledb", 9, &dsize); + if (!data) { + pr_info("Couldn't get moduledb list from firmware\n"); + } else if (IS_ERR(data)) { + rc = PTR_ERR(data); + pr_err("Error reading moduledb from firmware: %d\n", rc); + } else { + extract_esl(moduledb, data, dsize, offset); + + rc = parse_efi_signature_list("powerpc:moduledb", moduledb, dsize, + get_handler_for_code_signing_keys); + if (rc) + pr_err("Couldn't parse moduledb signatures: %d\n", rc); + kfree(data); + } + return rc; } late_initcall(load_powerpc_certs); From 604b8e7558857814bf7c109bc651697129543383 Mon Sep 17 00:00:00 2001 From: Azeem Shaikh Date: Thu, 18 May 2023 04:15:13 +0000 Subject: [PATCH 10/17] KEYS: Replace all non-returning strlcpy with strscpy strlcpy() reads the entire source buffer first. This read may exceed the destination size limit. This is both inefficient and can lead to linear read overflows if a source string is not NUL-terminated [1]. In an effort to remove strlcpy() completely [2], replace strlcpy() here with strscpy(). No return values were used, so direct replacement is safe. [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy [2] https://github.com/KSPP/linux/issues/89 Signed-off-by: Azeem Shaikh Reviewed-by: Jarkko Sakkinen Signed-off-by: Jarkko Sakkinen --- security/keys/request_key_auth.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c index 41e9735006d0..8f33cd170e42 100644 --- a/security/keys/request_key_auth.c +++ b/security/keys/request_key_auth.c @@ -178,7 +178,7 @@ struct key *request_key_auth_new(struct key *target, const char *op, if (!rka->callout_info) goto error_free_rka; rka->callout_len = callout_len; - strlcpy(rka->op, op, sizeof(rka->op)); + strscpy(rka->op, op, sizeof(rka->op)); /* see if the calling process is already servicing the key request of * another process */ From a86a42ac2bd652fdc7836a9d880c306a2485c142 Mon Sep 17 00:00:00 2001 From: Krishna Yarlagadda Date: Fri, 21 Apr 2023 14:43:08 +0530 Subject: [PATCH 11/17] tpm_tis_spi: Add hardware wait polling TPM devices may insert wait state on last clock cycle of ADDR phase. For SPI controllers that support full-duplex transfers, this can be detected using software by reading the MISO line. For SPI controllers that only support half-duplex transfers, such as the Tegra QSPI, it is not possible to detect the wait signal from software. The QSPI controller in Tegra234 and Tegra241 implement hardware detection of the wait signal which can be enabled in the controller for TPM devices. The current TPM TIS driver only supports software detection of the wait signal. To support SPI controllers that use hardware to detect the wait signal, add the function tpm_tis_spi_transfer_half() and move the existing code for software based detection into a function called tpm_tis_spi_transfer_full(). SPI controllers that only support half-duplex transfers will always call tpm_tis_spi_transfer_half() because they cannot support software based detection. The bit SPI_TPM_HW_FLOW is set to indicate to the SPI controller that hardware detection is required and it is the responsibility of the SPI controller driver to determine if this is supported or not. For hardware flow control, CMD-ADDR-DATA messages are combined into a single message where as for software flow control exiting method of CMD-ADDR in a message and DATA in another is followed. [jarkko: Fixed the function names to match the code change, and the tag in the short summary.] Signed-off-by: Krishna Yarlagadda Reviewed-by: Jarkko Sakkinen Signed-off-by: Jarkko Sakkinen --- drivers/char/tpm/tpm_tis_spi_main.c | 91 ++++++++++++++++++++++++++++- 1 file changed, 89 insertions(+), 2 deletions(-) diff --git a/drivers/char/tpm/tpm_tis_spi_main.c b/drivers/char/tpm/tpm_tis_spi_main.c index 9bfaba092a06..a62f5c7f38d3 100644 --- a/drivers/char/tpm/tpm_tis_spi_main.c +++ b/drivers/char/tpm/tpm_tis_spi_main.c @@ -71,8 +71,74 @@ static int tpm_tis_spi_flow_control(struct tpm_tis_spi_phy *phy, return 0; } -int tpm_tis_spi_transfer(struct tpm_tis_data *data, u32 addr, u16 len, - u8 *in, const u8 *out) +/* + * Half duplex controller with support for TPM wait state detection like + * Tegra QSPI need CMD, ADDR & DATA sent in single message to manage HW flow + * control. Each phase sent in different transfer for controller to idenity + * phase. + */ +static int tpm_tis_spi_transfer_half(struct tpm_tis_data *data, u32 addr, + u16 len, u8 *in, const u8 *out) +{ + struct tpm_tis_spi_phy *phy = to_tpm_tis_spi_phy(data); + struct spi_transfer spi_xfer[3]; + struct spi_message m; + u8 transfer_len; + int ret; + + while (len) { + transfer_len = min_t(u16, len, MAX_SPI_FRAMESIZE); + + spi_message_init(&m); + phy->iobuf[0] = (in ? 0x80 : 0) | (transfer_len - 1); + phy->iobuf[1] = 0xd4; + phy->iobuf[2] = addr >> 8; + phy->iobuf[3] = addr; + + memset(&spi_xfer, 0, sizeof(spi_xfer)); + + spi_xfer[0].tx_buf = phy->iobuf; + spi_xfer[0].len = 1; + spi_message_add_tail(&spi_xfer[0], &m); + + spi_xfer[1].tx_buf = phy->iobuf + 1; + spi_xfer[1].len = 3; + spi_message_add_tail(&spi_xfer[1], &m); + + if (out) { + spi_xfer[2].tx_buf = &phy->iobuf[4]; + spi_xfer[2].rx_buf = NULL; + memcpy(&phy->iobuf[4], out, transfer_len); + out += transfer_len; + } + + if (in) { + spi_xfer[2].tx_buf = NULL; + spi_xfer[2].rx_buf = &phy->iobuf[4]; + } + + spi_xfer[2].len = transfer_len; + spi_message_add_tail(&spi_xfer[2], &m); + + reinit_completion(&phy->ready); + + ret = spi_sync(phy->spi_device, &m); + if (ret < 0) + return ret; + + if (in) { + memcpy(in, &phy->iobuf[4], transfer_len); + in += transfer_len; + } + + len -= transfer_len; + } + + return ret; +} + +static int tpm_tis_spi_transfer_full(struct tpm_tis_data *data, u32 addr, + u16 len, u8 *in, const u8 *out) { struct tpm_tis_spi_phy *phy = to_tpm_tis_spi_phy(data); int ret = 0; @@ -148,6 +214,24 @@ exit: return ret; } +int tpm_tis_spi_transfer(struct tpm_tis_data *data, u32 addr, u16 len, + u8 *in, const u8 *out) +{ + struct tpm_tis_spi_phy *phy = to_tpm_tis_spi_phy(data); + struct spi_controller *ctlr = phy->spi_device->controller; + + /* + * TPM flow control over SPI requires full duplex support. + * Send entire message to a half duplex controller to handle + * wait polling in controller. + * Set TPM HW flow control flag.. + */ + if (ctlr->flags & SPI_CONTROLLER_HALF_DUPLEX) + return tpm_tis_spi_transfer_half(data, addr, len, in, out); + else + return tpm_tis_spi_transfer_full(data, addr, len, in, out); +} + static int tpm_tis_spi_read_bytes(struct tpm_tis_data *data, u32 addr, u16 len, u8 *result, enum tpm_tis_io_mode io_mode) { @@ -189,6 +273,9 @@ static int tpm_tis_spi_probe(struct spi_device *dev) phy->flow_control = tpm_tis_spi_flow_control; + if (dev->controller->flags & SPI_CONTROLLER_HALF_DUPLEX) + dev->mode |= SPI_TPM_HW_FLOW; + /* If the SPI device has an IRQ then use that */ if (dev->irq > 0) irq = dev->irq; From 32a0c860ff48543299772f7a98f32dd3ee9281b7 Mon Sep 17 00:00:00 2001 From: Alexander Steffen Date: Tue, 13 Jun 2023 20:02:57 +0200 Subject: [PATCH 12/17] tpm_tis: Move CRC check to generic send routine The CRC functionality is initialized before tpm_tis_core, so it can be used on all code paths within the module. Therefore, move the CRC check to the generic send routine, that also contains all other checks for successful command transmission, so that all those checks are in one place. Also, this ensures that tpm_tis_ready is called when a CRC failure is detected, to clear the invalid data from the TPM, which did not happen previously. Signed-off-by: Alexander Steffen Reviewed-by: Jarkko Sakkinen Signed-off-by: Jarkko Sakkinen --- drivers/char/tpm/tpm_tis_core.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c index b95963095729..469e4cd7fb4a 100644 --- a/drivers/char/tpm/tpm_tis_core.c +++ b/drivers/char/tpm/tpm_tis_core.c @@ -469,6 +469,12 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len) goto out_err; } + rc = tpm_tis_verify_crc(priv, len, buf); + if (rc < 0) { + dev_err(&chip->dev, "CRC mismatch for command.\n"); + goto out_err; + } + return 0; out_err: @@ -517,12 +523,6 @@ static int tpm_tis_send_main(struct tpm_chip *chip, const u8 *buf, size_t len) if (rc < 0) return rc; - rc = tpm_tis_verify_crc(priv, len, buf); - if (rc < 0) { - dev_err(&chip->dev, "CRC mismatch for command.\n"); - return rc; - } - /* go and do it */ rc = tpm_tis_write8(priv, TPM_STS(priv->locality), TPM_STS_GO); if (rc < 0) From b400f9d33fc21726bef465c49e71cd7364d0b8ef Mon Sep 17 00:00:00 2001 From: Alexander Steffen Date: Tue, 13 Jun 2023 20:02:58 +0200 Subject: [PATCH 13/17] tpm_tis: Use responseRetry to recover from data transfer errors TPM responses may become damaged during transmission, for example due to bit flips on the wire. Instead of aborting when detecting such issues, the responseRetry functionality can be used to make the TPM retransmit its response and receive it again without errors. Signed-off-by: Alexander Steffen Reviewed-by: Jarkko Sakkinen Signed-off-by: Jarkko Sakkinen --- drivers/char/tpm/tpm_tis_core.c | 37 ++++++++++++++++++++++++++------- drivers/char/tpm/tpm_tis_core.h | 1 + 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c index 469e4cd7fb4a..52e2dc7a9f81 100644 --- a/drivers/char/tpm/tpm_tis_core.c +++ b/drivers/char/tpm/tpm_tis_core.c @@ -340,7 +340,7 @@ static int recv_data(struct tpm_chip *chip, u8 *buf, size_t count) return size; } -static int tpm_tis_recv(struct tpm_chip *chip, u8 *buf, size_t count) +static int tpm_tis_try_recv(struct tpm_chip *chip, u8 *buf, size_t count) { struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev); int size = 0; @@ -348,11 +348,6 @@ static int tpm_tis_recv(struct tpm_chip *chip, u8 *buf, size_t count) u32 expected; int rc; - if (count < TPM_HEADER_SIZE) { - size = -EIO; - goto out; - } - size = recv_data(chip, buf, TPM_HEADER_SIZE); /* read first 10 bytes, including tag, paramsize, and result */ if (size < TPM_HEADER_SIZE) { @@ -385,7 +380,7 @@ static int tpm_tis_recv(struct tpm_chip *chip, u8 *buf, size_t count) goto out; } status = tpm_tis_status(chip); - if (status & TPM_STS_DATA_AVAIL) { /* retry? */ + if (status & TPM_STS_DATA_AVAIL) { dev_err(&chip->dev, "Error left over data\n"); size = -EIO; goto out; @@ -399,10 +394,36 @@ static int tpm_tis_recv(struct tpm_chip *chip, u8 *buf, size_t count) } out: - tpm_tis_ready(chip); return size; } +static int tpm_tis_recv(struct tpm_chip *chip, u8 *buf, size_t count) +{ + struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev); + unsigned int try; + int rc = 0; + + if (count < TPM_HEADER_SIZE) + return -EIO; + + for (try = 0; try < TPM_RETRY; try++) { + rc = tpm_tis_try_recv(chip, buf, count); + + if (rc == -EIO) + /* Data transfer errors, indicated by EIO, can be + * recovered by rereading the response. + */ + tpm_tis_write8(priv, TPM_STS(priv->locality), + TPM_STS_RESPONSE_RETRY); + else + break; + } + + tpm_tis_ready(chip); + + return rc; +} + /* * If interrupts are used (signaled by an irq set in the vendor structure) * tpm.c can skip polling for the data to be available as the interrupt is diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h index b1a169d7d1ca..13e99cf65efe 100644 --- a/drivers/char/tpm/tpm_tis_core.h +++ b/drivers/char/tpm/tpm_tis_core.h @@ -34,6 +34,7 @@ enum tis_status { TPM_STS_GO = 0x20, TPM_STS_DATA_AVAIL = 0x10, TPM_STS_DATA_EXPECT = 0x08, + TPM_STS_RESPONSE_RETRY = 0x02, TPM_STS_READ_ZERO = 0x23, /* bits that must be zero on read */ }; From 280db21e153d8810ce3b93640c63ae922bcb9e8e Mon Sep 17 00:00:00 2001 From: Alexander Steffen Date: Tue, 13 Jun 2023 20:02:59 +0200 Subject: [PATCH 14/17] tpm_tis: Resend command to recover from data transfer errors Similar to the transmission of TPM responses, also the transmission of TPM commands may become corrupted. Instead of aborting when detecting such issues, try resending the command again. Signed-off-by: Alexander Steffen Reviewed-by: Jarkko Sakkinen Signed-off-by: Jarkko Sakkinen --- drivers/char/tpm/tpm_tis_core.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c index 52e2dc7a9f81..1b350412d8a6 100644 --- a/drivers/char/tpm/tpm_tis_core.c +++ b/drivers/char/tpm/tpm_tis_core.c @@ -539,10 +539,17 @@ static int tpm_tis_send_main(struct tpm_chip *chip, const u8 *buf, size_t len) int rc; u32 ordinal; unsigned long dur; + unsigned int try; - rc = tpm_tis_send_data(chip, buf, len); - if (rc < 0) - return rc; + for (try = 0; try < TPM_RETRY; try++) { + rc = tpm_tis_send_data(chip, buf, len); + if (rc >= 0) + /* Data transfer done successfully */ + break; + else if (rc != -EIO) + /* Data transfer failed, not recoverable */ + return rc; + } /* go and do it */ rc = tpm_tis_write8(priv, TPM_STS(priv->locality), TPM_STS_GO); From df333d058a60a8dc282009a483124983669b4203 Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Fri, 16 Jun 2023 18:07:30 +0100 Subject: [PATCH 15/17] tpm: remove redundant variable len Variable len is being accumulated but the value is never read. It is redundant and can be removed. Cleans up clang scan build warning: drivers/char/tpm/eventlog/tpm1.c:276:2: warning: Value stored to 'len' is never read [deadcode.DeadStores] Signed-off-by: Colin Ian King Reviewed-by: Jarkko Sakkinen Reviewed-by: Jerry Snitselaar Signed-off-by: Jarkko Sakkinen --- drivers/char/tpm/eventlog/tpm1.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/char/tpm/eventlog/tpm1.c b/drivers/char/tpm/eventlog/tpm1.c index 8aa9057601d6..12ee42a31c71 100644 --- a/drivers/char/tpm/eventlog/tpm1.c +++ b/drivers/char/tpm/eventlog/tpm1.c @@ -251,7 +251,6 @@ static int tpm1_binary_bios_measurements_show(struct seq_file *m, void *v) static int tpm1_ascii_bios_measurements_show(struct seq_file *m, void *v) { - int len = 0; char *eventname; struct tcpa_event *event = v; unsigned char *event_entry = @@ -273,7 +272,7 @@ static int tpm1_ascii_bios_measurements_show(struct seq_file *m, void *v) /* 3rd: event type identifier */ seq_printf(m, " %02x", do_endian_conversion(event->event_type)); - len += get_event_name(eventname, event, event_entry); + get_event_name(eventname, event, event_entry); /* 4th: eventname <= max + \'0' delimiter */ seq_printf(m, " %s\n", eventname); From 2ccf8c76b4fb38deb25af3830aa9489b6a374621 Mon Sep 17 00:00:00 2001 From: Li Zetao Date: Tue, 15 Aug 2023 16:06:37 +0800 Subject: [PATCH 16/17] tpm/tpm_tis_synquacer: Use module_platform_driver macro to simplify the code Use the module_platform_driver macro to simplify the code, which is the same as declaring with module_init() and module_exit(). Signed-off-by: Li Zetao Reviewed-by: Jarkko Sakkinen Signed-off-by: Jarkko Sakkinen --- drivers/char/tpm/tpm_tis_synquacer.c | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/drivers/char/tpm/tpm_tis_synquacer.c b/drivers/char/tpm/tpm_tis_synquacer.c index 49278746b0e2..f7d5e76b505e 100644 --- a/drivers/char/tpm/tpm_tis_synquacer.c +++ b/drivers/char/tpm/tpm_tis_synquacer.c @@ -162,23 +162,7 @@ static struct platform_driver tis_synquacer_drv = { }, }; -static int __init tpm_tis_synquacer_module_init(void) -{ - int rc; +module_platform_driver(tis_synquacer_drv); - rc = platform_driver_register(&tis_synquacer_drv); - if (rc) - return rc; - - return 0; -} - -static void __exit tpm_tis_synquacer_module_exit(void) -{ - platform_driver_unregister(&tis_synquacer_drv); -} - -module_init(tpm_tis_synquacer_module_init); -module_exit(tpm_tis_synquacer_module_exit); MODULE_DESCRIPTION("TPM MMIO Driver for Socionext SynQuacer platform"); MODULE_LICENSE("GPL"); From 218a2680624cba1611e3dfc7d9b646d240e5f855 Mon Sep 17 00:00:00 2001 From: Eric Snowberg Date: Thu, 17 Aug 2023 10:04:07 -0400 Subject: [PATCH 17/17] certs: Reference revocation list for all keyrings Systems booted with shim have a Forbidden Signature Database called mokx. During boot, hashes and certs contained within the mokx are loaded into the blacklist keyring. When calling verify_pkcs7_message_sig the contents of the blacklist keyring (or revocation list) are referenced when validating keys on the platform keyring. Currently, when validating against the secondary or builtin keyrings, the revocation list is not referenced. Move up the check to allow the revocation list to be used with all keyrings, including the secondary and builtin, allowing the system owner to take corrective action should a vulnerability be found within keys contained within either keyring. Signed-off-by: Eric Snowberg Reviewed-by: Mimi Zohar Reviewed-by: Jarkko Sakkinen Signed-off-by: Jarkko Sakkinen --- certs/system_keyring.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/certs/system_keyring.c b/certs/system_keyring.c index 33841c91f12c..9de610bf1f4b 100644 --- a/certs/system_keyring.c +++ b/certs/system_keyring.c @@ -330,6 +330,12 @@ int verify_pkcs7_message_sig(const void *data, size_t len, if (ret < 0) goto error; + ret = is_key_on_revocation_list(pkcs7); + if (ret != -ENOKEY) { + pr_devel("PKCS#7 key is on revocation list\n"); + goto error; + } + if (!trusted_keys) { trusted_keys = builtin_trusted_keys; } else if (trusted_keys == VERIFY_USE_SECONDARY_KEYRING) { @@ -349,12 +355,6 @@ int verify_pkcs7_message_sig(const void *data, size_t len, pr_devel("PKCS#7 platform keyring is not available\n"); goto error; } - - ret = is_key_on_revocation_list(pkcs7); - if (ret != -ENOKEY) { - pr_devel("PKCS#7 platform key is on revocation list\n"); - goto error; - } } ret = pkcs7_validate_trust(pkcs7, trusted_keys); if (ret < 0) {