[PATCH v4] powerpc/pseries: make max polling consistent for longer H_CALLs
Currently, plpks_confirm_object_flushed() function polls for 5msec in total instead of 5sec. Keep max polling time consistent for all the H_CALLs, which take longer than expected, to be 5sec. Also, make use of fsleep() everywhere to insert delay. Reported-by: Nageswara R Sastry Fixes: 2454a7af0f2a ("powerpc/pseries: define driver for Platform KeyStore") Signed-off-by: Nayna Jain Tested-by: Nageswara R Sastry --- v4: * As per Andrew's feedback, squashed Patch 2 with Patch 1. Now it is single patch. v3: * Addition to Patch 1 timeout patch based on Andrew's feedback. v2: * Updated based on feedback from Michael Ellerman Replaced usleep_range with fsleep. Since there is no more need to specify range, sleep time is reverted back to 10 msec. arch/powerpc/include/asm/plpks.h | 5 ++--- arch/powerpc/platforms/pseries/plpks.c | 10 +- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/arch/powerpc/include/asm/plpks.h b/arch/powerpc/include/asm/plpks.h index 23b77027c916..7a84069759b0 100644 --- a/arch/powerpc/include/asm/plpks.h +++ b/arch/powerpc/include/asm/plpks.h @@ -44,9 +44,8 @@ #define PLPKS_MAX_DATA_SIZE4000 // Timeouts for PLPKS operations -#define PLPKS_MAX_TIMEOUT 5000 // msec -#define PLPKS_FLUSH_SLEEP 10 // msec -#define PLPKS_FLUSH_SLEEP_RANGE400 +#define PLPKS_MAX_TIMEOUT (5 * USEC_PER_SEC) +#define PLPKS_FLUSH_SLEEP 1 // usec struct plpks_var { char *component; diff --git a/arch/powerpc/platforms/pseries/plpks.c b/arch/powerpc/platforms/pseries/plpks.c index febe18f251d0..4a595493d28a 100644 --- a/arch/powerpc/platforms/pseries/plpks.c +++ b/arch/powerpc/platforms/pseries/plpks.c @@ -415,8 +415,7 @@ static int plpks_confirm_object_flushed(struct label *label, break; } - usleep_range(PLPKS_FLUSH_SLEEP, -PLPKS_FLUSH_SLEEP + PLPKS_FLUSH_SLEEP_RANGE); + fsleep(PLPKS_FLUSH_SLEEP); timeout = timeout + PLPKS_FLUSH_SLEEP; } while (timeout < PLPKS_MAX_TIMEOUT); @@ -464,9 +463,10 @@ int plpks_signed_update_var(struct plpks_var *var, u64 flags) continuetoken = retbuf[0]; if (pseries_status_to_err(rc) == -EBUSY) { - int delay_ms = get_longbusy_msecs(rc); - mdelay(delay_ms); - timeout += delay_ms; + int delay_us = get_longbusy_msecs(rc) * 1000; + + fsleep(delay_us); + timeout += delay_us; } rc = pseries_status_to_err(rc); } while (rc == -EBUSY && timeout < PLPKS_MAX_TIMEOUT); -- 2.31.1
[PATCH v3 2/2] powerpc/pseries: increase timeout value for plpks_signed_update_var() H_CALL
Signed update H_CALL currently polls PHYP for 5msec. Update this to 5sec. Signed-off-by: Nayna Jain Tested-by: Nageswara R Sastry --- v3: * Addition to Patch 1 timeout patch based on Andrew's feedback. arch/powerpc/platforms/pseries/plpks.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/arch/powerpc/platforms/pseries/plpks.c b/arch/powerpc/platforms/pseries/plpks.c index bcfcd5acc5c2..4a595493d28a 100644 --- a/arch/powerpc/platforms/pseries/plpks.c +++ b/arch/powerpc/platforms/pseries/plpks.c @@ -463,9 +463,10 @@ int plpks_signed_update_var(struct plpks_var *var, u64 flags) continuetoken = retbuf[0]; if (pseries_status_to_err(rc) == -EBUSY) { - int delay_ms = get_longbusy_msecs(rc); - mdelay(delay_ms); - timeout += delay_ms; + int delay_us = get_longbusy_msecs(rc) * 1000; + + fsleep(delay_us); + timeout += delay_us; } rc = pseries_status_to_err(rc); } while (rc == -EBUSY && timeout < PLPKS_MAX_TIMEOUT); -- 2.31.1
[PATCH v3 1/2] powerpc/pseries: fix max polling time in plpks_confirm_object_flushed() function
usleep_range() function takes input time and range in usec. However, currently it is assumed in msec in the function plpks_confirm_object_flushed(). Fix the total polling time for the object flushing from 5msec to 5sec. Reported-by: Nageswara R Sastry Fixes: 2454a7af0f2a ("powerpc/pseries: define driver for Platform KeyStore") Signed-off-by: Nayna Jain Tested-by: Nageswara R Sastry --- v3: No change v2: * Updated based on feedback from Michael Ellerman Replaced usleep_range with fsleep. Since there is no more need to specify range, sleep time is reverted back to 10 msec. arch/powerpc/include/asm/plpks.h | 5 ++--- arch/powerpc/platforms/pseries/plpks.c | 3 +-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/arch/powerpc/include/asm/plpks.h b/arch/powerpc/include/asm/plpks.h index 23b77027c916..7a84069759b0 100644 --- a/arch/powerpc/include/asm/plpks.h +++ b/arch/powerpc/include/asm/plpks.h @@ -44,9 +44,8 @@ #define PLPKS_MAX_DATA_SIZE4000 // Timeouts for PLPKS operations -#define PLPKS_MAX_TIMEOUT 5000 // msec -#define PLPKS_FLUSH_SLEEP 10 // msec -#define PLPKS_FLUSH_SLEEP_RANGE400 +#define PLPKS_MAX_TIMEOUT (5 * USEC_PER_SEC) +#define PLPKS_FLUSH_SLEEP 1 // usec struct plpks_var { char *component; diff --git a/arch/powerpc/platforms/pseries/plpks.c b/arch/powerpc/platforms/pseries/plpks.c index febe18f251d0..bcfcd5acc5c2 100644 --- a/arch/powerpc/platforms/pseries/plpks.c +++ b/arch/powerpc/platforms/pseries/plpks.c @@ -415,8 +415,7 @@ static int plpks_confirm_object_flushed(struct label *label, break; } - usleep_range(PLPKS_FLUSH_SLEEP, -PLPKS_FLUSH_SLEEP + PLPKS_FLUSH_SLEEP_RANGE); + fsleep(PLPKS_FLUSH_SLEEP); timeout = timeout + PLPKS_FLUSH_SLEEP; } while (timeout < PLPKS_MAX_TIMEOUT); -- 2.31.1
[PATCH v2] powerpc/pseries: fix max polling time in plpks_confirm_object_flushed() function
usleep_range() function takes input time and range in usec. However, currently it is assumed in msec in the function plpks_confirm_object_flushed(). Fix the total polling time for the object flushing from 5msec to 5sec. Reported-by: Nageswara R Sastry Fixes: 2454a7af0f2a ("powerpc/pseries: define driver for Platform KeyStore") Suggested-by: Michael Ellerman Signed-off-by: Nayna Jain Tested-by: Nageswara R Sastry --- v2: * Updated based on feedback from Michael Ellerman Replaced usleep_range with fsleep. Since there is no more need to specify range, sleep time is reverted back to 10 msec. arch/powerpc/include/asm/plpks.h | 5 ++--- arch/powerpc/platforms/pseries/plpks.c | 3 +-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/arch/powerpc/include/asm/plpks.h b/arch/powerpc/include/asm/plpks.h index 23b77027c916..7a84069759b0 100644 --- a/arch/powerpc/include/asm/plpks.h +++ b/arch/powerpc/include/asm/plpks.h @@ -44,9 +44,8 @@ #define PLPKS_MAX_DATA_SIZE4000 // Timeouts for PLPKS operations -#define PLPKS_MAX_TIMEOUT 5000 // msec -#define PLPKS_FLUSH_SLEEP 10 // msec -#define PLPKS_FLUSH_SLEEP_RANGE400 +#define PLPKS_MAX_TIMEOUT (5 * USEC_PER_SEC) +#define PLPKS_FLUSH_SLEEP 1 // usec struct plpks_var { char *component; diff --git a/arch/powerpc/platforms/pseries/plpks.c b/arch/powerpc/platforms/pseries/plpks.c index febe18f251d0..bcfcd5acc5c2 100644 --- a/arch/powerpc/platforms/pseries/plpks.c +++ b/arch/powerpc/platforms/pseries/plpks.c @@ -415,8 +415,7 @@ static int plpks_confirm_object_flushed(struct label *label, break; } - usleep_range(PLPKS_FLUSH_SLEEP, -PLPKS_FLUSH_SLEEP + PLPKS_FLUSH_SLEEP_RANGE); + fsleep(PLPKS_FLUSH_SLEEP); timeout = timeout + PLPKS_FLUSH_SLEEP; } while (timeout < PLPKS_MAX_TIMEOUT); -- 2.31.1
[PATCH] powerpc/pseries: fix max polling time in plpks_confirm_object_flushed() function
usleep_range() function takes input time and range in usec. However, currently it is assumed in msec in the function plpks_confirm_object_flushed(). Fix the total polling time for the object flushing from 5msec to 5sec. Reported-by: Nageswara R Sastry Fixes: 2454a7af0f2a ("powerpc/pseries: define driver for Platform KeyStore") Signed-off-by: Nayna Jain Tested-by: Nageswara R Sastry --- arch/powerpc/include/asm/plpks.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/powerpc/include/asm/plpks.h b/arch/powerpc/include/asm/plpks.h index 23b77027c916..8721d97f32c1 100644 --- a/arch/powerpc/include/asm/plpks.h +++ b/arch/powerpc/include/asm/plpks.h @@ -44,9 +44,9 @@ #define PLPKS_MAX_DATA_SIZE4000 // Timeouts for PLPKS operations -#define PLPKS_MAX_TIMEOUT 5000 // msec -#define PLPKS_FLUSH_SLEEP 10 // msec -#define PLPKS_FLUSH_SLEEP_RANGE400 +#define PLPKS_MAX_TIMEOUT 500 // usec +#define PLPKS_FLUSH_SLEEP 5000 // usec +#define PLPKS_FLUSH_SLEEP_RANGE5000 struct plpks_var { char *component; -- 2.31.1
Re: [PATCH] integrity: powerpc: Do not select CA_MACHINE_KEYRING
On 9/7/23 13:32, Michal Suchánek wrote: Adding more CC's from the original patch, looks like get_maintainers is not that great for this file. On Thu, Sep 07, 2023 at 06:52:19PM +0200, Michal Suchanek wrote: No other platform needs CA_MACHINE_KEYRING, either. This is policy that should be decided by the administrator, not Kconfig dependencies. We certainly agree that flexibility is important. However, in this case, this also implies that we are expecting system admins to be security experts. As per our understanding, CA based infrastructure(PKI) is the standard to be followed and not the policy decision. And we can only speak for Power. INTEGRITY_CA_MACHINE_KEYRING ensures that we always have CA signed leaf certs. INTEGRITY_CA_MACHINE_KEYRING_MAX ensures that CA is only allowed to do key signing and not code signing. Having CA signed certs also permits easy revocation of all leaf certs. Loading certificates is completely new for Power Systems. We would like to make it as clean as possible from the start. We want to enforce CA signed leaf certificates(INTEGRITY_CA_MACHINE_KEYRING). As per keyUsage(INTEGRITY_CA_MACHINE_KEYRING_MAX), if we want more flexibility, probably a boot time override can be considered. Thanks & Regards, - Nayna cc: joeyli Signed-off-by: Michal Suchanek --- security/integrity/Kconfig | 2 -- 1 file changed, 2 deletions(-) diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig index 232191ee09e3..b6e074ac0227 100644 --- a/security/integrity/Kconfig +++ b/security/integrity/Kconfig @@ -68,8 +68,6 @@ config INTEGRITY_MACHINE_KEYRING depends on INTEGRITY_ASYMMETRIC_KEYS depends on SYSTEM_BLACKLIST_KEYRING 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 -- 2.41.0
[PATCH v4 5/6] 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 --- 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 -- 2.31.1
[PATCH v4 1/6] 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 --- .../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); -- 2.31.1
[PATCH v4 0/6] Enable loading local and third party keys on PowerVM guest
On a secure boot enabled PowerVM guest, local and third party code signing keys are needed to verify signed applications, configuration files, and kernel modules. Loading these keys onto either the .secondary_trusted_keys or .ima keyrings requires the certificates be signed by keys on the .builtin_trusted_keys, .machine or .secondary_trusted_keys keyrings. Keys on the .builtin_trusted_keys keyring are trusted because of the chain of trust from secure boot up to and including the linux kernel. Keys on the .machine keyring that derive their trust from an entity such as a security officer, administrator, system owner, or machine owner are said to have "imputed trust." The type of certificates and the mechanism for loading them onto the .machine keyring is platform dependent. Userspace may load certificates onto the .secondary_trusted_keys or .ima keyrings. However, keys may also need to be loaded by the kernel if they are needed for verification in early boot time. On PowerVM guest, third party code signing keys are loaded from the moduledb variable in the Platform KeyStore(PKS) onto the .secondary_trusted_keys. The purpose of this patch set is to allow loading of local and third party code signing keys on PowerVM. Changelog: v4: * Fixed build error reported by Nageswara R Sastry , as part of his testing of patches. * Included Jarkko's and Mimi's feedback v3: * Included Jarkko's feedback for Patch 6/6. v2: * Patch 5/6: Update CA restriction to allow only key signing CA's. * Rebase on Jarkko's master tree - https://kernel.googlesource.com/pub/scm/linux/kernel/git/jarkko/linux-tpmdd * Tested after reverting cfa7522f280aa95 because of build failure due to this commit. Nayna Jain (6): integrity: PowerVM support for loading CA keys on machine keyring integrity: ignore keys failing CA restrictions on non-UEFI platform integrity: remove global variable from machine_keyring.c integrity: check whether imputed trust is enabled integrity: PowerVM machine keyring enablement integrity: PowerVM support for loading third party code signing keys certs/system_keyring.c| 30 include/keys/system_keyring.h | 4 +++ security/integrity/Kconfig| 4 ++- security/integrity/digsig.c | 2 +- security/integrity/integrity.h| 5 +-- .../platform_certs/keyring_handler.c | 19 ++- .../platform_certs/keyring_handler.h | 10 ++ .../integrity/platform_certs/load_powerpc.c | 34 +++ .../platform_certs/machine_keyring.c | 22 +--- 9 files changed, 121 insertions(+), 9 deletions(-) -- 2.31.1
[PATCH v4 4/6] 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-off-by: Mimi Zohar --- 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; +} -- 2.31.1
[PATCH v4 6/6] 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 --- 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, *dat
[PATCH v4 3/6] 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 --- 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; -- 2.31.1
[PATCH v4 2/6] 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 --- 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); -- 2.31.1
[PATCH v3 6/6] 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 --- certs/system_keyring.c| 30 +++ include/keys/system_keyring.h | 7 + security/integrity/integrity.h| 1 + .../platform_certs/keyring_handler.c | 8 + .../platform_certs/keyring_handler.h | 5 .../integrity/platform_certs/load_powerpc.c | 18 ++- 6 files changed, 68 insertions(+), 1 deletion(-) diff --git a/certs/system_keyring.c b/certs/system_keyring.c index b348e0898d34..e458d414918d 100644 --- a/certs/system_keyring.c +++ b/certs/system_keyring.c @@ -396,3 +396,33 @@ void __init set_platform_trusted_keys(struct key *keyring) platform_trusted_keys = keyring; } #endif + +/** + * 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); +} diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h index 7e2583208820..4188f75d1bac 100644 --- a/include/keys/system_keyring.h +++ b/include/keys/system_keyring.h @@ -50,9 +50,16 @@ 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 +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/integrity.h b/security/integrity/integrity.h index d7553c93f5c0..efaa2eb789ad 100644 --- a/security/integrity/integrity.h +++ b/security/integrity/integrity.h @@ -228,6 +228,7 @@ static inline int __init integrity_load_cert(const unsigned int id, { return 0; } + #endif /* CONFIG_INTEGRITY_SIGNATURE */ #ifdef CONFIG_INTEGRITY_ASYMMETRIC_KEYS 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 s
[PATCH v3 5/6] 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 --- 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 -- 2.31.1
[PATCH v3 2/6] 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 --- 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); -- 2.31.1
[PATCH v3 4/6] 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-off-by: Mimi Zohar --- 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; +} -- 2.31.1
[PATCH v3 1/6] 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 --- .../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..6263ce3b3f1e 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 = NULL; 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); -- 2.31.1
[PATCH v3 3/6] 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 --- 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; -- 2.31.1
[PATCH v3 0/6] Enable loading local and third party keys on PowerVM guest
On a secure boot enabled PowerVM guest, local and third party code signing keys are needed to verify signed applications, configuration files, and kernel modules. Loading these keys onto either the .secondary_trusted_keys or .ima keyrings requires the certificates be signed by keys on the .builtin_trusted_keys, .machine or .secondary_trusted_keys keyrings. Keys on the .builtin_trusted_keys keyring are trusted because of the chain of trust from secure boot up to and including the linux kernel. Keys on the .machine keyring that derive their trust from an entity such as a security officer, administrator, system owner, or machine owner are said to have "imputed trust." The type of certificates and the mechanism for loading them onto the .machine keyring is platform dependent. Userspace may load certificates onto the .secondary_trusted_keys or .ima keyrings. However, keys may also need to be loaded by the kernel if they are needed for verification in early boot time. On PowerVM guest, third party code signing keys are loaded from the moduledb variable in the Platform KeyStore(PKS) onto the .secondary_trusted_keys. The purpose of this patch set is to allow loading of local and third party code signing keys on PowerVM. Changelog: v3: * Included Jarkko's feedback for Patch 6/6. v2: * Patch 5/6: Update CA restriction to allow only key signing CA's. * Rebase on Jarkko's master tree - https://kernel.googlesource.com/pub/scm/linux/kernel/git/jarkko/linux-tpmdd * Tested after reverting cfa7522f280aa95 because of build failure due to this commit. Nayna Jain (6): integrity: PowerVM support for loading CA keys on machine keyring integrity: ignore keys failing CA restrictions on non-UEFI platform integrity: remove global variable from machine_keyring.c integrity: check whether imputed trust is enabled integrity: PowerVM machine keyring enablement integrity: PowerVM support for loading third party code signing keys certs/system_keyring.c| 30 + include/keys/system_keyring.h | 7 security/integrity/Kconfig| 4 ++- security/integrity/digsig.c | 2 +- security/integrity/integrity.h| 6 ++-- .../platform_certs/keyring_handler.c | 19 ++- .../platform_certs/keyring_handler.h | 10 ++ .../integrity/platform_certs/load_powerpc.c | 33 +++ .../platform_certs/machine_keyring.c | 22 ++--- 9 files changed, 124 insertions(+), 9 deletions(-) -- 2.31.1
[PATCH v2 1/6] 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 --- .../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..6263ce3b3f1e 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 = NULL; 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); -- 2.31.1
[PATCH v2 6/6] 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 --- certs/system_keyring.c| 23 +++ include/keys/system_keyring.h | 7 ++ security/integrity/integrity.h| 1 + .../platform_certs/keyring_handler.c | 8 +++ .../platform_certs/keyring_handler.h | 5 .../integrity/platform_certs/load_powerpc.c | 18 ++- 6 files changed, 61 insertions(+), 1 deletion(-) diff --git a/certs/system_keyring.c b/certs/system_keyring.c index b348e0898d34..3435d4936fb2 100644 --- a/certs/system_keyring.c +++ b/certs/system_keyring.c @@ -396,3 +396,26 @@ void __init set_platform_trusted_keys(struct key *keyring) platform_trusted_keys = keyring; } #endif + +void __init add_to_secondary_keyring(const char *source, const void *data, +size_t len) +{ + key_ref_t key; + key_perm_t perm; + int rc = 0; + + 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)) { + rc = PTR_ERR(key); + pr_err("Problem loading X.509 certificate %d\n", rc); + } else { + pr_notice("Loaded X.509 cert '%s'\n", + key_ref_to_ptr(key)->description); + key_ref_put(key); + } +} diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h index 7e2583208820..4188f75d1bac 100644 --- a/include/keys/system_keyring.h +++ b/include/keys/system_keyring.h @@ -50,9 +50,16 @@ 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 +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/integrity.h b/security/integrity/integrity.h index d7553c93f5c0..efaa2eb789ad 100644 --- a/security/integrity/integrity.h +++ b/security/integrity/integrity.h @@ -228,6 +228,7 @@ static inline int __init integrity_load_cert(const unsigned int id, { return 0; } + #endif /* CONFIG_INTEGRITY_SIGNATURE */ #ifdef CONFIG_INTEGRITY_ASYMMETRIC_KEYS 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 6263ce3b3f1e..32c4e5fbf0fb 100644 --- a/security/integrity/platform_certs/load_po
[PATCH v2 5/6] 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 --- 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 -- 2.31.1
[PATCH v2 4/6] 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-off-by: Mimi Zohar --- 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; +} -- 2.31.1
[PATCH v2 3/6] 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 --- 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; -- 2.31.1
[PATCH v2 2/6] 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 --- 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); -- 2.31.1
[PATCH v2 0/6] Enable loading local and third party keys on PowerVM guest
On a secure boot enabled PowerVM guest, local and third party code signing keys are needed to verify signed applications, configuration files, and kernel modules. Loading these keys onto either the .secondary_trusted_keys or .ima keyrings requires the certificates be signed by keys on the .builtin_trusted_keys, .machine or .secondary_trusted_keys keyrings. Keys on the .builtin_trusted_keys keyring are trusted because of the chain of trust from secure boot up to and including the linux kernel. Keys on the .machine keyring that derive their trust from an entity such as a security officer, administrator, system owner, or machine owner are said to have "imputed trust." The type of certificates and the mechanism for loading them onto the .machine keyring is platform dependent. Userspace may load certificates onto the .secondary_trusted_keys or .ima keyrings. However, keys may also need to be loaded by the kernel if they are needed for verification in early boot time. On PowerVM guest, third party code signing keys are loaded from the moduledb variable in the Platform KeyStore(PKS) onto the .secondary_trusted_keys. The purpose of this patch set is to allow loading of local and third party code signing keys on PowerVM. Changelog: v2: * Patch 5/6: Update CA restriction to allow only key signing CA's. * Rebase on Jarkko's master tree - https://kernel.googlesource.com/pub/scm/linux/kernel/git/jarkko/linux-tpmdd * Tested after reverting cfa7522f280aa95 because of build failure due to this commit. Nayna Jain (6): integrity: PowerVM support for loading CA keys on machine keyring integrity: ignore keys failing CA restrictions on non-UEFI platform integrity: remove global variable from machine_keyring.c integrity: check whether imputed trust is enabled integrity: PowerVM machine keyring enablement integrity: PowerVM support for loading third party code signing keys certs/system_keyring.c| 23 + include/keys/system_keyring.h | 7 security/integrity/Kconfig| 4 ++- security/integrity/digsig.c | 2 +- security/integrity/integrity.h| 6 ++-- .../platform_certs/keyring_handler.c | 19 ++- .../platform_certs/keyring_handler.h | 10 ++ .../integrity/platform_certs/load_powerpc.c | 33 +++ .../platform_certs/machine_keyring.c | 22 ++--- 9 files changed, 117 insertions(+), 9 deletions(-) -- 2.31.1
[PATCH 6/6] 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 --- Jarkko, this patch is based on Linus master tree branch, which does not contain the following commits yet: c9d004712300 integrity: Enforce digitalSignature usage in the ima and evm keyrings 59b656eb58fe KEYS: DigitalSignature link restriction certs/system_keyring.c| 22 +++ include/keys/system_keyring.h | 8 +++ security/integrity/integrity.h| 1 + .../platform_certs/keyring_handler.c | 8 +++ .../platform_certs/keyring_handler.h | 5 + .../integrity/platform_certs/load_powerpc.c | 18 ++- 6 files changed, 61 insertions(+), 1 deletion(-) diff --git a/certs/system_keyring.c b/certs/system_keyring.c index a7a49b17ceb1..b0235732c1d4 100644 --- a/certs/system_keyring.c +++ b/certs/system_keyring.c @@ -347,3 +347,25 @@ void __init set_platform_trusted_keys(struct key *keyring) platform_trusted_keys = keyring; } #endif + +void __init add_to_secondary_keyring(const char *source, const void *data, +size_t len) +{ + key_ref_t key; + key_perm_t perm; + int rc = 0; + + 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)) { + rc = PTR_ERR(key); + pr_err("Problem loading X.509 certificate %d\n", rc); + } else { + pr_notice("Loaded X.509 cert '%s'\n", + key_ref_to_ptr(key)->description); + key_ref_put(key); + } +} diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h index 91e080efb918..a57a77ccf003 100644 --- a/include/keys/system_keyring.h +++ b/include/keys/system_keyring.h @@ -41,8 +41,16 @@ extern int restrict_link_by_builtin_and_secondary_trusted( 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 +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/integrity.h b/security/integrity/integrity.h index d7553c93f5c0..efaa2eb789ad 100644 --- a/security/integrity/integrity.h +++ b/security/integrity/integrity.h @@ -228,6 +228,7 @@ static inline int __init integrity_load_cert(const unsigned int id, { return 0; } + #endif /* CONFIG_INTEGRITY_SIGNATURE */ #ifdef CONFIG_INTEGRITY_ASYMMETRIC_KEYS diff --git a/security/integrity/platform_certs/keyring_handler.c b/security/integrity/platform_certs/keyring_handler.c index b3e5df136e50..6095df043498 100644 --- a/security/integrity/platform_certs/keyring_handler.c +++ b/security/integrity/platform_certs/keyring_handler.c @@ -77,6 +77,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 6263ce3b3f1e..32c4e5fbf0fb 100644 --- a/security/integrity/platform_certs/load_powerpc.c +++ b/s
[PATCH 5/6] integrity: PowerVM machine keyring enablement.
Update Kconfig to enable machine keyring and limit to CA certificates on PowerVM. Signed-off-by: Nayna Jain --- security/integrity/Kconfig | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig index ec6e0d789da1..03c40ade0214 100644 --- a/security/integrity/Kconfig +++ b/security/integrity/Kconfig @@ -67,7 +67,8 @@ 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 help If set, provide a keyring to which Machine Owner Keys (MOK) may be added. This keyring shall contain just MOK keys. Unlike keys -- 2.31.1
[PATCH 4/6] 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 --- security/integrity/digsig.c | 2 +- security/integrity/integrity.h| 5 +++-- .../integrity/platform_certs/keyring_handler.c| 2 +- .../integrity/platform_certs/machine_keyring.c| 15 ++- 4 files changed, 19 insertions(+), 5 deletions(-) diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c index 6f31ffe23c48..48d505cacd81 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..b3e5df136e50 100644 --- a/security/integrity/platform_certs/keyring_handler.c +++ b/security/integrity/platform_certs/keyring_handler.c @@ -61,7 +61,7 @@ __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..58cd72b193e6 100644 --- a/security/integrity/platform_certs/machine_keyring.c +++ b/security/integrity/platform_certs/machine_keyring.c @@ -60,7 +60,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 +75,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; +} -- 2.31.1
[PATCH 3/6] 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 --- 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; -- 2.31.1
[PATCH 1/6] 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 --- .../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..6263ce3b3f1e 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 = NULL; 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); -- 2.31.1
[PATCH 2/6] 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 --- 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); -- 2.31.1
[PATCH 0/6] Enable loading local and third party keys on PowerVM guest
On a secure boot enabled PowerVM guest, local and third party code signing keys are needed to verify signed applications, configuration files, and kernel modules. Loading these keys onto either the .secondary_trusted_keys or .ima keyrings requires the certificates be signed by keys on the .builtin_trusted_keys, .machine or .secondary_trusted_keys keyrings. Keys on the .builtin_trusted_keys keyring are trusted because of the chain of trust from secure boot up to and including the linux kernel. Keys on the .machine keyring that derive their trust from an entity such as a security officer, administrator, system owner, or machine owner are said to have "imputed trust." The type of certificates and the mechanism for loading them onto the .machine keyring is platform dependent. Userspace may load certificates onto the .secondary_trusted_keys or .ima keyrings. However, keys may also need to be loaded by the kernel if they are needed for verification in early boot time. On PowerVM guest, third party code signing keys are loaded from the moduledb variable in the Platform KeyStore(PKS) onto the .secondary_trusted_keys. The purpose of this patch set is to allow loading of local and third party code signing keys on PowerVM. Nayna Jain (6): integrity: PowerVM support for loading CA keys on machine keyring integrity: ignore keys failing CA restrictions on non-UEFI platform integrity: remove global variable from machine_keyring.c integrity: check whether imputed trust is enabled integrity: PowerVM machine keyring enablement. integrity: PowerVM support for loading third party code signing keys certs/system_keyring.c| 22 + include/keys/system_keyring.h | 8 + security/integrity/Kconfig| 3 +- security/integrity/digsig.c | 2 +- security/integrity/integrity.h| 6 ++-- .../platform_certs/keyring_handler.c | 18 +- .../platform_certs/keyring_handler.h | 10 ++ .../integrity/platform_certs/load_powerpc.c | 33 +++ .../platform_certs/machine_keyring.c | 21 +--- 9 files changed, 114 insertions(+), 9 deletions(-) base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5 -- 2.31.1
[PATCH v2] security/integrity: fix pointer to ESL data and its size on pseries
On PowerVM guest, variable data is prefixed with 8 bytes of timestamp. Extract ESL by stripping off the timestamp before passing to ESL parser. Fixes: 4b3e71e9a34c ("integrity/powerpc: Support loading keys from PLPKS") Cc: sta...@vger.kenrnel.org # v6.3 Signed-off-by: Nayna Jain --- Changelog: v2: Fixed feedback from Jarkko * added CC to stable * moved *data declaration to same line as *db,*dbx Renamed extract_data() macro to extract_esl() for clarity .../integrity/platform_certs/load_powerpc.c | 40 --- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/security/integrity/platform_certs/load_powerpc.c b/security/integrity/platform_certs/load_powerpc.c index b9de70b90826..170789dc63d2 100644 --- a/security/integrity/platform_certs/load_powerpc.c +++ b/security/integrity/platform_certs/load_powerpc.c @@ -15,6 +15,9 @@ #include "keyring_handler.h" #include "../integrity.h" +#define extract_esl(db, data, size, offset)\ + do { db = data + offset; size = size - offset; } while (0) + /* * Get a certificate list blob from the named secure variable. * @@ -55,8 +58,9 @@ 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; - u64 dbsize = 0, dbxsize = 0; + void *db = NULL, *dbx = NULL, *data = NULL; + u64 dsize = 0; + u64 offset = 0; int rc = 0; ssize_t len; char buf[32]; @@ -74,38 +78,46 @@ static int __init load_powerpc_certs(void) return -ENODEV; } + if (strcmp("ibm,plpks-sb-v1", buf) == 0) + /* PLPKS authenticated variables ESL data is prefixed with 8 bytes of timestamp */ + offset = 8; + /* * Get db, and dbx. They might not exist, so it isn't an error if we * can't get them. */ - db = get_cert_list("db", 3, &dbsize); - if (!db) { + data = get_cert_list("db", 3, &dsize); + if (!data) { pr_info("Couldn't get db list from firmware\n"); - } else if (IS_ERR(db)) { - rc = PTR_ERR(db); + } else if (IS_ERR(data)) { + rc = PTR_ERR(data); pr_err("Error reading db from firmware: %d\n", rc); return rc; } else { - rc = parse_efi_signature_list("powerpc:db", db, dbsize, + extract_esl(db, data, dsize, offset); + + rc = parse_efi_signature_list("powerpc:db", db, dsize, get_handler_for_db); if (rc) pr_err("Couldn't parse db signatures: %d\n", rc); - kfree(db); + kfree(data); } - dbx = get_cert_list("dbx", 4, &dbxsize); - if (!dbx) { + data = get_cert_list("dbx", 4, &dsize); + if (!data) { pr_info("Couldn't get dbx list from firmware\n"); - } else if (IS_ERR(dbx)) { - rc = PTR_ERR(dbx); + } else if (IS_ERR(data)) { + rc = PTR_ERR(data); pr_err("Error reading dbx from firmware: %d\n", rc); return rc; } else { - rc = parse_efi_signature_list("powerpc:dbx", dbx, dbxsize, + extract_esl(dbx, data, dsize, offset); + + rc = parse_efi_signature_list("powerpc:dbx", dbx, dsize, get_handler_for_dbx); if (rc) pr_err("Couldn't parse dbx signatures: %d\n", rc); - kfree(dbx); + kfree(data); } return rc; -- 2.31.1
Re: [PATCH] security/integrity: fix pointer to ESL data and its size on pseries
On 6/6/23 16:51, Jarkko Sakkinen wrote: On Tue Jun 6, 2023 at 8:26 PM EEST, Nayna Jain wrote: On PowerVM guest, variable data is prefixed with 8 bytes of timestamp. Extract ESL by stripping off the timestamp before passing to ESL parser. Cc: sta...@vger.kenrnel.org # v6.3 ? Aah yes. Missed that.. Thanks.. Fixes: 4b3e71e9a34c ("integrity/powerpc: Support loading keys from PLPKS") Signed-off-by: Nayna Jain --- .../integrity/platform_certs/load_powerpc.c | 39 --- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/security/integrity/platform_certs/load_powerpc.c b/security/integrity/platform_certs/load_powerpc.c index b9de70b90826..57768cbf1fd3 100644 --- a/security/integrity/platform_certs/load_powerpc.c +++ b/security/integrity/platform_certs/load_powerpc.c @@ -15,6 +15,9 @@ #include "keyring_handler.h" #include "../integrity.h" +#define extract_data(db, data, size, offset) \ + do { db = data + offset; size = size - offset; } while (0) + /* * Get a certificate list blob from the named secure variable. * @@ -55,8 +58,10 @@ static __init void *get_cert_list(u8 *key, unsigned long keylen, u64 *size) */ static int __init load_powerpc_certs(void) { + void *data = NULL; + u64 dsize = 0; + u64 offset = 0; void *db = NULL, *dbx = NULL; So... what do you need db still for? If you meant to rename 'db' to 'data', then you should not do it, since this is a bug fix. It is zero gain, and a factor harder backport. In case of PowerVM guest, data points to timestamp + ESL. And then with offset of 8 bytes, db points to ESL. While db is used for parsing ESL, data is then later used to free (timestamp + ESL) memory. Hope it answers the question. Thanks & Regards, - Nayna - u64 dbsize = 0, dbxsize = 0; int rc = 0; ssize_t len; char buf[32]; @@ -74,38 +79,46 @@ static int __init load_powerpc_certs(void) return -ENODEV; } + if (strcmp("ibm,plpks-sb-v1", buf) == 0) + /* PLPKS authenticated variables ESL data is prefixed with 8 bytes of timestamp */ + offset = 8; + /* * Get db, and dbx. They might not exist, so it isn't an error if we * can't get them. */ - db = get_cert_list("db", 3, &dbsize); - if (!db) { + data = get_cert_list("db", 3, &dsize); + if (!data) { pr_info("Couldn't get db list from firmware\n"); - } else if (IS_ERR(db)) { - rc = PTR_ERR(db); + } else if (IS_ERR(data)) { + rc = PTR_ERR(data); pr_err("Error reading db from firmware: %d\n", rc); return rc; } else { - rc = parse_efi_signature_list("powerpc:db", db, dbsize, + extract_data(db, data, dsize, offset); + + rc = parse_efi_signature_list("powerpc:db", db, dsize, get_handler_for_db); if (rc) pr_err("Couldn't parse db signatures: %d\n", rc); - kfree(db); + kfree(data); } - dbx = get_cert_list("dbx", 4, &dbxsize); - if (!dbx) { + data = get_cert_list("dbx", 4, &dsize); + if (!data) { pr_info("Couldn't get dbx list from firmware\n"); - } else if (IS_ERR(dbx)) { - rc = PTR_ERR(dbx); + } else if (IS_ERR(data)) { + rc = PTR_ERR(data); pr_err("Error reading dbx from firmware: %d\n", rc); return rc; } else { - rc = parse_efi_signature_list("powerpc:dbx", dbx, dbxsize, + extract_data(dbx, data, dsize, offset); + + rc = parse_efi_signature_list("powerpc:dbx", dbx, dsize, get_handler_for_dbx); if (rc) pr_err("Couldn't parse dbx signatures: %d\n", rc); - kfree(dbx); + kfree(data); } return rc; -- 2.31.1 BR, Jarkko
[PATCH] security/integrity: fix pointer to ESL data and its size on pseries
On PowerVM guest, variable data is prefixed with 8 bytes of timestamp. Extract ESL by stripping off the timestamp before passing to ESL parser. Fixes: 4b3e71e9a34c ("integrity/powerpc: Support loading keys from PLPKS") Signed-off-by: Nayna Jain --- .../integrity/platform_certs/load_powerpc.c | 39 --- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/security/integrity/platform_certs/load_powerpc.c b/security/integrity/platform_certs/load_powerpc.c index b9de70b90826..57768cbf1fd3 100644 --- a/security/integrity/platform_certs/load_powerpc.c +++ b/security/integrity/platform_certs/load_powerpc.c @@ -15,6 +15,9 @@ #include "keyring_handler.h" #include "../integrity.h" +#define extract_data(db, data, size, offset) \ + do { db = data + offset; size = size - offset; } while (0) + /* * Get a certificate list blob from the named secure variable. * @@ -55,8 +58,10 @@ static __init void *get_cert_list(u8 *key, unsigned long keylen, u64 *size) */ static int __init load_powerpc_certs(void) { + void *data = NULL; + u64 dsize = 0; + u64 offset = 0; void *db = NULL, *dbx = NULL; - u64 dbsize = 0, dbxsize = 0; int rc = 0; ssize_t len; char buf[32]; @@ -74,38 +79,46 @@ static int __init load_powerpc_certs(void) return -ENODEV; } + if (strcmp("ibm,plpks-sb-v1", buf) == 0) + /* PLPKS authenticated variables ESL data is prefixed with 8 bytes of timestamp */ + offset = 8; + /* * Get db, and dbx. They might not exist, so it isn't an error if we * can't get them. */ - db = get_cert_list("db", 3, &dbsize); - if (!db) { + data = get_cert_list("db", 3, &dsize); + if (!data) { pr_info("Couldn't get db list from firmware\n"); - } else if (IS_ERR(db)) { - rc = PTR_ERR(db); + } else if (IS_ERR(data)) { + rc = PTR_ERR(data); pr_err("Error reading db from firmware: %d\n", rc); return rc; } else { - rc = parse_efi_signature_list("powerpc:db", db, dbsize, + extract_data(db, data, dsize, offset); + + rc = parse_efi_signature_list("powerpc:db", db, dsize, get_handler_for_db); if (rc) pr_err("Couldn't parse db signatures: %d\n", rc); - kfree(db); + kfree(data); } - dbx = get_cert_list("dbx", 4, &dbxsize); - if (!dbx) { + data = get_cert_list("dbx", 4, &dsize); + if (!data) { pr_info("Couldn't get dbx list from firmware\n"); - } else if (IS_ERR(dbx)) { - rc = PTR_ERR(dbx); + } else if (IS_ERR(data)) { + rc = PTR_ERR(data); pr_err("Error reading dbx from firmware: %d\n", rc); return rc; } else { - rc = parse_efi_signature_list("powerpc:dbx", dbx, dbxsize, + extract_data(dbx, data, dsize, offset); + + rc = parse_efi_signature_list("powerpc:dbx", dbx, dsize, get_handler_for_dbx); if (rc) pr_err("Couldn't parse dbx signatures: %d\n", rc); - kfree(dbx); + kfree(data); } return rc; -- 2.31.1
Re: [PATCH 2/4] fs: define a firmware security filesystem named fwsecurityfs
On 11/23/22 10:57, Greg Kroah-Hartman wrote: On Wed, Nov 23, 2022 at 10:05:49AM -0500, Nayna wrote: On 11/22/22 18:21, Nayna wrote: From the perspective of our use case, we need to expose firmware security objects to userspace for management. Not all of the objects pre-exist and we would like to allow root to create them from userspace. From a unification perspective, I have considered a common location at /sys/firmware/security for managing any platform's security objects. And I've proposed a generic filesystem, which could be used by any platform to represent firmware security objects via /sys/firmware/security. Here are some alternatives to generic filesystem in discussion: 1. Start with a platform-specific filesystem. If more platforms would like to use the approach, it can be made generic. We would still have a common location of /sys/firmware/security and new code would live in arch. This is my preference and would be the best fit for our use case. 2. Use securityfs. This would mean modifying it to satisfy other use cases, including supporting userspace file creation. I don't know if the securityfs maintainer would find that acceptable. I would also still want some way to expose variables at /sys/firmware/security. 3. Use a sysfs-based approach. This would be a platform-specific implementation. However, sysfs has a similar issue to securityfs for file creation. When I tried it in RFC v1[1], I had to implement a workaround to achieve that. [1] https://lore.kernel.org/linuxppc-dev/20220122005637.28199-3-na...@linux.ibm.com/ Hi Greg, Based on the discussions so far, is Option 1, described above, an acceptable next step? No, as I said almost a year ago, I do not want to see platform-only filesystems going and implementing stuff that should be shared by all platforms. Given there are no other exploiters for fwsecurityfs and there should be no platform-specific fs, would modifying sysfs now to let userspace create files cleanly be the way forward? Or, if we should strongly consider securityfs, which would result in updating securityfs to allow userspace creation of files and then expose variables via a more platform-specific directory /sys/kernel/security/pks? We want to pick the best available option and would find some hints on direction helpful before we develop the next patch. Thanks & Regards, - Nayna
Re: [PATCH 2/4] fs: define a firmware security filesystem named fwsecurityfs
On 11/22/22 18:21, Nayna wrote: From the perspective of our use case, we need to expose firmware security objects to userspace for management. Not all of the objects pre-exist and we would like to allow root to create them from userspace. From a unification perspective, I have considered a common location at /sys/firmware/security for managing any platform's security objects. And I've proposed a generic filesystem, which could be used by any platform to represent firmware security objects via /sys/firmware/security. Here are some alternatives to generic filesystem in discussion: 1. Start with a platform-specific filesystem. If more platforms would like to use the approach, it can be made generic. We would still have a common location of /sys/firmware/security and new code would live in arch. This is my preference and would be the best fit for our use case. 2. Use securityfs. This would mean modifying it to satisfy other use cases, including supporting userspace file creation. I don't know if the securityfs maintainer would find that acceptable. I would also still want some way to expose variables at /sys/firmware/security. 3. Use a sysfs-based approach. This would be a platform-specific implementation. However, sysfs has a similar issue to securityfs for file creation. When I tried it in RFC v1[1], I had to implement a workaround to achieve that. [1] https://lore.kernel.org/linuxppc-dev/20220122005637.28199-3-na...@linux.ibm.com/ Hi Greg, Based on the discussions so far, is Option 1, described above, an acceptable next step? Thanks & Regards, - Nayna
Re: [PATCH 2/4] fs: define a firmware security filesystem named fwsecurityfs
On 11/19/22 06:48, Ritesh Harjani (IBM) wrote: Hello Nayna, Hi Ritesh, On 22/11/09 03:10PM, Nayna wrote: On 11/9/22 08:46, Greg Kroah-Hartman wrote: On Sun, Nov 06, 2022 at 04:07:42PM -0500, Nayna Jain wrote: securityfs is meant for Linux security subsystems to expose policies/logs or any other information. However, there are various firmware security features which expose their variables for user management via the kernel. There is currently no single place to expose these variables. Different platforms use sysfs/platform specific filesystem(efivarfs)/securityfs interface as they find it appropriate. Thus, there is a gap in kernel interfaces to expose variables for security features. Define a firmware security filesystem (fwsecurityfs) to be used by security features enabled by the firmware. These variables are platform specific. This filesystem provides platforms a way to implement their own underlying semantics by defining own inode and file operations. Similar to securityfs, the firmware security filesystem is recommended to be exposed on a well known mount point /sys/firmware/security. Platforms can define their own directory or file structure under this path. Example: # mount -t fwsecurityfs fwsecurityfs /sys/firmware/security Why not juset use securityfs in /sys/security/firmware/ instead? Then you don't have to create a new filesystem and convince userspace to mount it in a specific location? I am also curious to know on why not use securityfs, given the similarity between the two. :) More specifics on that below... From man 5 sysfs page: /sys/firmware: This subdirectory contains interfaces for viewing and manipulating firmware-specific objects and attributes. /sys/kernel: This subdirectory contains various files and subdirectories that provide information about the running kernel. The security variables which are being exposed via fwsecurityfs are managed by firmware, stored in firmware managed space and also often consumed by firmware for enabling various security features. That's ok. As I see it users of securityfs can define their own fileops (like how you are doing in fwsecurityfs). See securityfs_create_file() & securityfs_create_symlink(), can accept the fops & iops. Except maybe securityfs_create_dir(), that could be since there might not be a usecase for it. But do you also need it in your case is the question to ask. Please refer to the function plpks_secvars_init() in Patch 4/4. From git commit b67dbf9d4c1987c370fd18fdc4cf9d8aaea604c2, the purpose of securityfs(/sys/kernel/security) is to provide a common place for all kernel LSMs. The idea of Which was then seperated out by commit, da31894ed7b654e2 ("securityfs: do not depend on CONFIG_SECURITY"). securityfs now has a seperate CONFIG_SECURITYFS config option. In fact I was even thinking of why shouldn't we move security/inode.c into fs/securityfs/inode.c . fs/* is a common place for all filesystems. Users of securityfs can call it's exported kernel APIs to create files/dirs/symlinks. If we move security/inode.c to fs/security/inode.c, then... ...below call within securityfs_init() should be moved into some lsm sepecific file. #ifdef CONFIG_SECURITY static struct dentry *lsm_dentry; static ssize_t lsm_read(struct file *filp, char __user *buf, size_t count, loff_t *ppos) { return simple_read_from_buffer(buf, count, ppos, lsm_names, strlen(lsm_names)); } static const struct file_operations lsm_ops = { .read = lsm_read, .llseek = generic_file_llseek, }; #endif securityfs_init() #ifdef CONFIG_SECURITY lsm_dentry = securityfs_create_file("lsm", 0444, NULL, NULL, &lsm_ops); #endif So why not move it? Maybe others, can comment more on whether it's a good idea to move security/inode.c into fs/security/inode.c? This should then help others identify securityfs filesystem in fs/security/ for everyone to notice and utilize for their use? fwsecurityfs(/sys/firmware/security) is to similarly provide a common place for all firmware security objects. /sys/firmware already exists. The patch now defines a new /security directory in it for firmware security features. Using /sys/kernel/security would mean scattering firmware objects in multiple places and confusing the purpose of /sys/kernel and /sys/firmware. We can also think of it this way that, all security related exports should happen via /sys/kernel/security/. Then /sys/kernel/security/firmware/ becomes the security related firmware exports. If you see find /sys -iname firmware, I am sure you will find other firmware specifics directories related to other specific subsystems (e.g. root@qemu:/home/qemu# find /sys -iname firmware /sys/devices/ndbus0/nmem0/firmware /sys/devices/ndbus0/firmware /sys/firmware ) But it could be, I am not an expert here, although I was thinking a goo
Re: [PATCH 2/4] fs: define a firmware security filesystem named fwsecurityfs
On 11/20/22 22:14, James Bottomley wrote: On Sun, 2022-11-20 at 17:13 +0100, Greg Kroah-Hartman wrote: On Sat, Nov 19, 2022 at 01:20:09AM -0500, Nayna wrote: On 11/17/22 16:27, Greg Kroah-Hartman wrote: On Mon, Nov 14, 2022 at 06:03:43PM -0500, Nayna wrote: On 11/10/22 04:58, Greg Kroah-Hartman wrote: [...] [...] You are correct. There's no namespace for these. So again, I do not understand. Do you want to use filesystem namespaces, or do you not? Since this seems to go back to my email quoted again, let me repeat: the question isn't if this patch is namespaced; I think you've agreed several times it isn't. The question is if the exposed properties would ever need to be namespaced. This is a subtle and complex question which isn't at all explored by the above interchange. How again can you not use sysfs or securityfs due to namespaces? What is missing? I already explained in the email that sysfs contains APIs like simple_pin_... which are completely inimical to namespacing. Currently securityfs contains them as well, so in that regard they're both no better than each other. The point I was making is that securityfs is getting namespaced by the IMA namespace rework (which is pretty complex due to having to replace the simple_pin_... APIs), so when (perhaps if) the IMA namespace is accepted, securityfs will make a good home for quantities that need namespacing. That's not to say you can't namespace things in sysfs, you can, in the same way that you can get a round peg into a square hole if you bang hard enough. So perhaps we could get back to the original question of whether these quantities would ever be namespaced ... or, conversely, whether they would never need namespacing. To clarify, I brought up in the discussion about namespacing considerations because I was asked about them. However, I determined there were none because firmware object interactions are invariant across namespaces. I don't see this changing in the future given that the firmware objects have no notion of namespacing. Thanks & Regards, - Nayna
Re: [PATCH 2/4] fs: define a firmware security filesystem named fwsecurityfs
On 11/17/22 16:27, Greg Kroah-Hartman wrote: On Mon, Nov 14, 2022 at 06:03:43PM -0500, Nayna wrote: On 11/10/22 04:58, Greg Kroah-Hartman wrote: On Wed, Nov 09, 2022 at 03:10:37PM -0500, Nayna wrote: On 11/9/22 08:46, Greg Kroah-Hartman wrote: On Sun, Nov 06, 2022 at 04:07:42PM -0500, Nayna Jain wrote: securityfs is meant for Linux security subsystems to expose policies/logs or any other information. However, there are various firmware security features which expose their variables for user management via the kernel. There is currently no single place to expose these variables. Different platforms use sysfs/platform specific filesystem(efivarfs)/securityfs interface as they find it appropriate. Thus, there is a gap in kernel interfaces to expose variables for security features. Define a firmware security filesystem (fwsecurityfs) to be used by security features enabled by the firmware. These variables are platform specific. This filesystem provides platforms a way to implement their own underlying semantics by defining own inode and file operations. Similar to securityfs, the firmware security filesystem is recommended to be exposed on a well known mount point /sys/firmware/security. Platforms can define their own directory or file structure under this path. Example: # mount -t fwsecurityfs fwsecurityfs /sys/firmware/security Why not juset use securityfs in /sys/security/firmware/ instead? Then you don't have to create a new filesystem and convince userspace to mount it in a specific location? From man 5 sysfs page: /sys/firmware: This subdirectory contains interfaces for viewing and manipulating firmware-specific objects and attributes. /sys/kernel: This subdirectory contains various files and subdirectories that provide information about the running kernel. The security variables which are being exposed via fwsecurityfs are managed by firmware, stored in firmware managed space and also often consumed by firmware for enabling various security features. Ok, then just use the normal sysfs interface for /sys/firmware, why do you need a whole new filesystem type? From git commit b67dbf9d4c1987c370fd18fdc4cf9d8aaea604c2, the purpose of securityfs(/sys/kernel/security) is to provide a common place for all kernel LSMs. The idea of fwsecurityfs(/sys/firmware/security) is to similarly provide a common place for all firmware security objects. /sys/firmware already exists. The patch now defines a new /security directory in it for firmware security features. Using /sys/kernel/security would mean scattering firmware objects in multiple places and confusing the purpose of /sys/kernel and /sys/firmware. sysfs is confusing already, no problem with making it more confusing :) Just document where you add things and all should be fine. Even though fwsecurityfs code is based on securityfs, since the two filesystems expose different types of objects and have different requirements, there are distinctions: 1. fwsecurityfs lets users create files in userspace, securityfs only allows kernel subsystems to create files. Wait, why would a user ever create a file in this filesystem? If you need that, why not use configfs? That's what that is for, right? The purpose of fwsecurityfs is not to expose configuration items but rather security objects used for firmware security features. I think these are more comparable to EFI variables, which are exposed via an EFI-specific filesystem, efivarfs, rather than configfs. 2. firmware and kernel objects may have different requirements. For example, consideration of namespacing. As per my understanding, namespacing is applied to kernel resources and not firmware resources. That's why it makes sense to add support for namespacing in securityfs, but we concluded that fwsecurityfs currently doesn't need it. Another but similar example of it is: TPM space, which is exposed from hardware. For containers, the TPM would be made as virtual/software TPM. Similarly for firmware space for containers, it would have to be something virtualized/software version of it. I do not understand, sorry. What does namespaces have to do with this? sysfs can already handle namespaces just fine, why not use that? Firmware objects are not namespaced. I mentioned it here as an example of the difference between firmware and kernel objects. It is also in response to the feedback from James Bottomley in RFC v2 [https://lore.kernel.org/linuxppc-dev/41ca51e8db9907d9060cc38adb59a66dcae4c59b.ca...@hansenpartnership.com/]. I do not understand, sorry. Do you want to use a namespace for these or not? The code does not seem to be using namespaces. You can use sysfs with, or without, a namespace so I don't understand the issue here. With your code, there is no namespace. You are correct. There's no namespace for these. 3. firmware objects are persistent and read at boot time by interaction with firmware, unlike kernel objects which are not pe
Re: [PATCH 2/4] fs: define a firmware security filesystem named fwsecurityfs
On 11/10/22 04:58, Greg Kroah-Hartman wrote: On Wed, Nov 09, 2022 at 03:10:37PM -0500, Nayna wrote: On 11/9/22 08:46, Greg Kroah-Hartman wrote: On Sun, Nov 06, 2022 at 04:07:42PM -0500, Nayna Jain wrote: securityfs is meant for Linux security subsystems to expose policies/logs or any other information. However, there are various firmware security features which expose their variables for user management via the kernel. There is currently no single place to expose these variables. Different platforms use sysfs/platform specific filesystem(efivarfs)/securityfs interface as they find it appropriate. Thus, there is a gap in kernel interfaces to expose variables for security features. Define a firmware security filesystem (fwsecurityfs) to be used by security features enabled by the firmware. These variables are platform specific. This filesystem provides platforms a way to implement their own underlying semantics by defining own inode and file operations. Similar to securityfs, the firmware security filesystem is recommended to be exposed on a well known mount point /sys/firmware/security. Platforms can define their own directory or file structure under this path. Example: # mount -t fwsecurityfs fwsecurityfs /sys/firmware/security Why not juset use securityfs in /sys/security/firmware/ instead? Then you don't have to create a new filesystem and convince userspace to mount it in a specific location? From man 5 sysfs page: /sys/firmware: This subdirectory contains interfaces for viewing and manipulating firmware-specific objects and attributes. /sys/kernel: This subdirectory contains various files and subdirectories that provide information about the running kernel. The security variables which are being exposed via fwsecurityfs are managed by firmware, stored in firmware managed space and also often consumed by firmware for enabling various security features. Ok, then just use the normal sysfs interface for /sys/firmware, why do you need a whole new filesystem type? From git commit b67dbf9d4c1987c370fd18fdc4cf9d8aaea604c2, the purpose of securityfs(/sys/kernel/security) is to provide a common place for all kernel LSMs. The idea of fwsecurityfs(/sys/firmware/security) is to similarly provide a common place for all firmware security objects. /sys/firmware already exists. The patch now defines a new /security directory in it for firmware security features. Using /sys/kernel/security would mean scattering firmware objects in multiple places and confusing the purpose of /sys/kernel and /sys/firmware. sysfs is confusing already, no problem with making it more confusing :) Just document where you add things and all should be fine. Even though fwsecurityfs code is based on securityfs, since the two filesystems expose different types of objects and have different requirements, there are distinctions: 1. fwsecurityfs lets users create files in userspace, securityfs only allows kernel subsystems to create files. Wait, why would a user ever create a file in this filesystem? If you need that, why not use configfs? That's what that is for, right? The purpose of fwsecurityfs is not to expose configuration items but rather security objects used for firmware security features. I think these are more comparable to EFI variables, which are exposed via an EFI-specific filesystem, efivarfs, rather than configfs. 2. firmware and kernel objects may have different requirements. For example, consideration of namespacing. As per my understanding, namespacing is applied to kernel resources and not firmware resources. That's why it makes sense to add support for namespacing in securityfs, but we concluded that fwsecurityfs currently doesn't need it. Another but similar example of it is: TPM space, which is exposed from hardware. For containers, the TPM would be made as virtual/software TPM. Similarly for firmware space for containers, it would have to be something virtualized/software version of it. I do not understand, sorry. What does namespaces have to do with this? sysfs can already handle namespaces just fine, why not use that? Firmware objects are not namespaced. I mentioned it here as an example of the difference between firmware and kernel objects. It is also in response to the feedback from James Bottomley in RFC v2 [https://lore.kernel.org/linuxppc-dev/41ca51e8db9907d9060cc38adb59a66dcae4c59b.ca...@hansenpartnership.com/]. 3. firmware objects are persistent and read at boot time by interaction with firmware, unlike kernel objects which are not persistent. That doesn't matter, sysfs exports what the hardware provides, and that might persist over boot. So I don't see why a new filesystem is needed. You didn't explain why sysfs, or securitfs (except for the location in the tree) does not work at all for your needs. The location really doesn't matter all that much as you are creating a brand new location anyway so we can just declar
Re: [PATCH 2/4] fs: define a firmware security filesystem named fwsecurityfs
On 11/9/22 08:46, Greg Kroah-Hartman wrote: On Sun, Nov 06, 2022 at 04:07:42PM -0500, Nayna Jain wrote: securityfs is meant for Linux security subsystems to expose policies/logs or any other information. However, there are various firmware security features which expose their variables for user management via the kernel. There is currently no single place to expose these variables. Different platforms use sysfs/platform specific filesystem(efivarfs)/securityfs interface as they find it appropriate. Thus, there is a gap in kernel interfaces to expose variables for security features. Define a firmware security filesystem (fwsecurityfs) to be used by security features enabled by the firmware. These variables are platform specific. This filesystem provides platforms a way to implement their own underlying semantics by defining own inode and file operations. Similar to securityfs, the firmware security filesystem is recommended to be exposed on a well known mount point /sys/firmware/security. Platforms can define their own directory or file structure under this path. Example: # mount -t fwsecurityfs fwsecurityfs /sys/firmware/security Why not juset use securityfs in /sys/security/firmware/ instead? Then you don't have to create a new filesystem and convince userspace to mount it in a specific location? From man 5 sysfs page: /sys/firmware: This subdirectory contains interfaces for viewing and manipulating firmware-specific objects and attributes. /sys/kernel: This subdirectory contains various files and subdirectories that provide information about the running kernel. The security variables which are being exposed via fwsecurityfs are managed by firmware, stored in firmware managed space and also often consumed by firmware for enabling various security features. From git commit b67dbf9d4c1987c370fd18fdc4cf9d8aaea604c2, the purpose of securityfs(/sys/kernel/security) is to provide a common place for all kernel LSMs. The idea of fwsecurityfs(/sys/firmware/security) is to similarly provide a common place for all firmware security objects. /sys/firmware already exists. The patch now defines a new /security directory in it for firmware security features. Using /sys/kernel/security would mean scattering firmware objects in multiple places and confusing the purpose of /sys/kernel and /sys/firmware. Even though fwsecurityfs code is based on securityfs, since the two filesystems expose different types of objects and have different requirements, there are distinctions: 1. fwsecurityfs lets users create files in userspace, securityfs only allows kernel subsystems to create files. 2. firmware and kernel objects may have different requirements. For example, consideration of namespacing. As per my understanding, namespacing is applied to kernel resources and not firmware resources. That's why it makes sense to add support for namespacing in securityfs, but we concluded that fwsecurityfs currently doesn't need it. Another but similar example of it is: TPM space, which is exposed from hardware. For containers, the TPM would be made as virtual/software TPM. Similarly for firmware space for containers, it would have to be something virtualized/software version of it. 3. firmware objects are persistent and read at boot time by interaction with firmware, unlike kernel objects which are not persistent. For a more detailed explanation refer to the LSS-NA 2022 "PowerVM Platform Keystore - Securing Linux Credentials Locally" talk and slides[1]. The link to previously posted RFC version is [2]. [1] https://static.sched.com/hosted_files/lssna2022/25/NaynaJain_PowerVM_PlatformKeyStore_SecuringLinuxCredentialsLocally.pdf [2] https://lore.kernel.org/linuxppc-dev/yrqqphi4+jhz1...@kroah.com/ Thanks & Regards, - Nayna thanks, greg k-h
[PATCH 4/4] powerpc/pseries: expose authenticated variables stored in LPAR PKS
PowerVM Guest Secure boot feature need to expose firmware managed secure variables for user management. These variables store keys for grub/kernel verification and also corresponding denied list. Expose these variables to the userpace via fwsecurityfs. Example: $ pwd /sys/firmware/security/plpks/secvars $ ls -ltrh total 0 -rw-r--r-- 1 root root 831 Sep 12 18:34 PK -rw-r--r-- 1 root root 831 Sep 12 18:34 KEK -rw-r--r-- 1 root root 831 Sep 12 18:34 db $ hexdump -C db 00 00 00 08 a1 59 c0 a5 e4 94 a7 4a 87 b5 ab 15 |.Y.J| 0010 5c 2b f0 72 3f 03 00 00 00 00 00 00 23 03 00 00 |\+.r?...#...| 0020 ca 18 1d 1c 01 7d eb 11 9a 71 08 94 ef 31 fb e4 |.}...q...1..| 0030 30 82 03 0f 30 82 01 f7 a0 03 02 01 02 02 14 22 |0...0.."| 0040 ab 18 2f d5 aa dd c5 ba 98 27 60 26 f1 63 89 54 |../..'`&.c.T| 0050 4c 52 d9 30 0d 06 09 2a 86 48 86 f7 0d 01 01 0b |LR.0...*.H..| 0060 05 00 30 17 31 15 30 13 06 03 55 04 03 0c 0c 72 |..0.1.0...Ur| ... Signed-off-by: Nayna Jain --- arch/powerpc/platforms/pseries/Kconfig| 10 + arch/powerpc/platforms/pseries/Makefile | 1 + .../platforms/pseries/fwsecurityfs_arch.c | 8 + arch/powerpc/platforms/pseries/plpks.h| 3 + arch/powerpc/platforms/pseries/secvars.c | 365 ++ 5 files changed, 387 insertions(+) create mode 100644 arch/powerpc/platforms/pseries/secvars.c diff --git a/arch/powerpc/platforms/pseries/Kconfig b/arch/powerpc/platforms/pseries/Kconfig index 5fb45e601982..41c17f60dfe9 100644 --- a/arch/powerpc/platforms/pseries/Kconfig +++ b/arch/powerpc/platforms/pseries/Kconfig @@ -172,6 +172,16 @@ config PSERIES_FWSECURITYFS_ARCH If you are unsure how to use it, say N. +config PSERIES_PLPKS_SECVARS + depends on PSERIES_PLPKS + select PSERIES_FWSECURITYFS_ARCH + tristate "Support for secvars" + help + This interface exposes authenticated variables stored in the LPAR + Platform KeyStore using fwsecurityfs interface. + + If you are unsure how to use it, say N. + config PAPR_SCM depends on PPC_PSERIES && MEMORY_HOTPLUG && LIBNVDIMM tristate "Support for the PAPR Storage Class Memory interface" diff --git a/arch/powerpc/platforms/pseries/Makefile b/arch/powerpc/platforms/pseries/Makefile index 2903cff26258..6833f6b02798 100644 --- a/arch/powerpc/platforms/pseries/Makefile +++ b/arch/powerpc/platforms/pseries/Makefile @@ -29,6 +29,7 @@ obj-$(CONFIG_PPC_SVM) += svm.o obj-$(CONFIG_FA_DUMP) += rtas-fadump.o obj-$(CONFIG_PSERIES_PLPKS) += plpks.o obj-$(CONFIG_PSERIES_FWSECURITYFS_ARCH) += fwsecurityfs_arch.o +obj-$(CONFIG_PSERIES_PLPKS_SECVARS) += secvars.o obj-$(CONFIG_SUSPEND) += suspend.o obj-$(CONFIG_PPC_VAS) += vas.o vas-sysfs.o diff --git a/arch/powerpc/platforms/pseries/fwsecurityfs_arch.c b/arch/powerpc/platforms/pseries/fwsecurityfs_arch.c index b43bd3cf7889..1cc651ad6434 100644 --- a/arch/powerpc/platforms/pseries/fwsecurityfs_arch.c +++ b/arch/powerpc/platforms/pseries/fwsecurityfs_arch.c @@ -58,6 +58,7 @@ static int create_plpks_dir(void) { struct dentry *config_dir; struct dentry *fdentry; + int rc; if (!IS_ENABLED(CONFIG_PSERIES_PLPKS) || !plpks_is_available()) { pr_warn("Platform KeyStore is not available on this LPAR\n"); @@ -107,6 +108,13 @@ static int create_plpks_dir(void) if (IS_ERR(fdentry)) pr_err("Could not create version %ld\n", PTR_ERR(fdentry)); + if (IS_ENABLED(CONFIG_PSERIES_PLPKS_SECVARS)) { + rc = plpks_secvars_init(plpks_dir); + if (rc) + pr_err("Secure Variables initialization failed with error %d\n", rc); + return rc; + } + return 0; } diff --git a/arch/powerpc/platforms/pseries/plpks.h b/arch/powerpc/platforms/pseries/plpks.h index fb483658549f..2d572fe4b522 100644 --- a/arch/powerpc/platforms/pseries/plpks.h +++ b/arch/powerpc/platforms/pseries/plpks.h @@ -11,6 +11,7 @@ #include #include +#include #define OSSECBOOTAUDIT 0x4000 #define OSSECBOOTENFORCE 0x2000 @@ -103,4 +104,6 @@ u32 plpks_get_totalsize(void); */ u32 plpks_get_usedspace(void); +int plpks_secvars_init(struct dentry *parent); + #endif diff --git a/arch/powerpc/platforms/pseries/secvars.c b/arch/powerpc/platforms/pseries/secvars.c new file mode 100644 index ..3d5a251d0571 --- /dev/null +++ b/arch/powerpc/platforms/pseries/secvars.c @@ -0,0 +1,365 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Expose secure(authenticated) variables for user key management. + * Copyright (C) 2022 IBM Corporation + * Author: Nayna Jain + * + */ + +#include +#include "plpks.h" + +static struct dentry *secvar_dir; + +static const char * c
[PATCH 1/4] powerpc/pseries: Add new functions to PLPKS driver
PowerVM stores authenticated variables in the PowerVM LPAR Platform KeyStore(PLPKS). Add signed update H_CALL to PLPKS driver to support authenticated variables. Additionally, expose config values outside the PLPKS driver. Signed-off-by: Nayna Jain --- arch/powerpc/include/asm/hvcall.h | 3 +- arch/powerpc/platforms/pseries/plpks.c | 112 +++-- arch/powerpc/platforms/pseries/plpks.h | 35 3 files changed, 144 insertions(+), 6 deletions(-) diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h index 95fd7f9485d5..33b26c0cb69b 100644 --- a/arch/powerpc/include/asm/hvcall.h +++ b/arch/powerpc/include/asm/hvcall.h @@ -336,7 +336,8 @@ #define H_SCM_FLUSH0x44C #define H_GET_ENERGY_SCALE_INFO0x450 #define H_WATCHDOG 0x45C -#define MAX_HCALL_OPCODE H_WATCHDOG +#define H_PKS_SIGNED_UPDATE0x454 +#define MAX_HCALL_OPCODE H_PKS_SIGNED_UPDATE /* Scope args for H_SCM_UNBIND_ALL */ #define H_UNBIND_SCOPE_ALL (0x1) diff --git a/arch/powerpc/platforms/pseries/plpks.c b/arch/powerpc/platforms/pseries/plpks.c index 4edd1585e245..d0fa7b55a900 100644 --- a/arch/powerpc/platforms/pseries/plpks.c +++ b/arch/powerpc/platforms/pseries/plpks.c @@ -40,6 +40,10 @@ static u16 ospasswordlength; // Retrieved with H_PKS_GET_CONFIG static u16 maxpwsize; static u16 maxobjsize; +static s16 maxobjlabelsize; +static u32 totalsize; +static u32 usedspace; +static u8 version; struct plpks_auth { u8 version; @@ -87,6 +91,12 @@ static int pseries_status_to_err(int rc) err = -ENOENT; break; case H_BUSY: + case H_LONG_BUSY_ORDER_1_MSEC: + case H_LONG_BUSY_ORDER_10_MSEC: + case H_LONG_BUSY_ORDER_100_MSEC: + case H_LONG_BUSY_ORDER_1_SEC: + case H_LONG_BUSY_ORDER_10_SEC: + case H_LONG_BUSY_ORDER_100_SEC: err = -EBUSY; break; case H_AUTHORITY: @@ -187,14 +197,17 @@ static struct label *construct_label(char *component, u8 varos, u8 *name, u16 namelen) { struct label *label; - size_t slen; + size_t slen = 0; if (!name || namelen > MAX_NAME_SIZE) return ERR_PTR(-EINVAL); - slen = strlen(component); - if (component && slen > sizeof(label->attr.prefix)) - return ERR_PTR(-EINVAL); + /* Support NULL component for signed updates */ + if (component) { + slen = strlen(component); + if (slen > sizeof(label->attr.prefix)) + return ERR_PTR(-EINVAL); + } label = kzalloc(sizeof(*label), GFP_KERNEL); if (!label) @@ -240,10 +253,51 @@ static int _plpks_get_config(void) maxpwsize = be16_to_cpu(config.maxpwsize); maxobjsize = be16_to_cpu(config.maxobjsize); + maxobjlabelsize = be16_to_cpu(config.maxobjlabelsize) - + MAX_LABEL_ATTR_SIZE; + maxobjlabelsize = maxobjlabelsize < 0 ? 0 : maxobjlabelsize; + totalsize = be32_to_cpu(config.totalsize); + usedspace = be32_to_cpu(config.usedspace); return 0; } +u8 plpks_get_version(void) +{ + return version; +} + +u16 plpks_get_maxobjectsize(void) +{ + return maxobjsize; +} + +u16 plpks_get_maxobjectlabelsize(void) +{ + return maxobjlabelsize; +} + +u32 plpks_get_totalsize(void) +{ + return totalsize; +} + +u32 plpks_get_usedspace(void) +{ + return usedspace; +} + +bool plpks_is_available(void) +{ + int rc; + + rc = _plpks_get_config(); + if (rc) + return false; + + return true; +} + static int plpks_confirm_object_flushed(struct label *label, struct plpks_auth *auth) { @@ -277,6 +331,54 @@ static int plpks_confirm_object_flushed(struct label *label, return rc; } +int plpks_signed_update_var(struct plpks_var var, u64 flags) +{ + unsigned long retbuf[PLPAR_HCALL9_BUFSIZE] = {0}; + int rc; + struct label *label; + struct plpks_auth *auth; + u64 continuetoken = 0; + + if (!var.data || var.datalen <= 0 || var.namelen > MAX_NAME_SIZE) + return -EINVAL; + + if (!(var.policy & SIGNEDUPDATE)) + return -EINVAL; + + auth = construct_auth(PKS_OS_OWNER); + if (IS_ERR(auth)) + return PTR_ERR(auth); + + label = construct_label(var.component, var.os, var.name, var.namelen); + if (IS_ERR(label)) { + rc = PTR_ERR(label); + goto out; + } + + do { + rc = plpar_hcall9(H_PKS_SIGNED_UPDATE, retbuf, + virt_to_phys(auth), virt_to_phys(label), + label->size, var.policy, flags, +
[PATCH 3/4] powerpc/pseries: initialize fwsecurityfs with plpks arch-specific structure
PowerVM PLPKS variables are exposed via fwsecurityfs. Initialize fwsecurityfs arch-specific structure with plpks configuration. Eg: [root@ltcfleet35-lp1 config]# pwd /sys/firmware/security/plpks/config [root@ltcfleet35-lp1 config]# ls -ltrh total 0 -r--r--r-- 1 root root 1 Sep 28 15:01 version -r--r--r-- 1 root root 4 Sep 28 15:01 used_space -r--r--r-- 1 root root 4 Sep 28 15:01 total_size -r--r--r-- 1 root root 2 Sep 28 15:01 max_object_size -r--r--r-- 1 root root 2 Sep 28 15:01 max_object_label_size Signed-off-by: Nayna Jain --- arch/powerpc/platforms/pseries/Kconfig| 10 ++ arch/powerpc/platforms/pseries/Makefile | 1 + .../platforms/pseries/fwsecurityfs_arch.c | 116 ++ include/linux/fwsecurityfs.h | 4 + 4 files changed, 131 insertions(+) create mode 100644 arch/powerpc/platforms/pseries/fwsecurityfs_arch.c diff --git a/arch/powerpc/platforms/pseries/Kconfig b/arch/powerpc/platforms/pseries/Kconfig index a3b4d99567cb..5fb45e601982 100644 --- a/arch/powerpc/platforms/pseries/Kconfig +++ b/arch/powerpc/platforms/pseries/Kconfig @@ -162,6 +162,16 @@ config PSERIES_PLPKS If unsure, select N. +config PSERIES_FWSECURITYFS_ARCH + select FWSECURITYFS + bool "Support fwsecurityfs for pseries" + help + Enable fwsecurityfs arch specific code. This would initialize + the firmware security filesystem with initial platform specific + structure. + + If you are unsure how to use it, say N. + config PAPR_SCM depends on PPC_PSERIES && MEMORY_HOTPLUG && LIBNVDIMM tristate "Support for the PAPR Storage Class Memory interface" diff --git a/arch/powerpc/platforms/pseries/Makefile b/arch/powerpc/platforms/pseries/Makefile index 92310202bdd7..2903cff26258 100644 --- a/arch/powerpc/platforms/pseries/Makefile +++ b/arch/powerpc/platforms/pseries/Makefile @@ -28,6 +28,7 @@ obj-$(CONFIG_PPC_SPLPAR) += vphn.o obj-$(CONFIG_PPC_SVM) += svm.o obj-$(CONFIG_FA_DUMP) += rtas-fadump.o obj-$(CONFIG_PSERIES_PLPKS) += plpks.o +obj-$(CONFIG_PSERIES_FWSECURITYFS_ARCH) += fwsecurityfs_arch.o obj-$(CONFIG_SUSPEND) += suspend.o obj-$(CONFIG_PPC_VAS) += vas.o vas-sysfs.o diff --git a/arch/powerpc/platforms/pseries/fwsecurityfs_arch.c b/arch/powerpc/platforms/pseries/fwsecurityfs_arch.c new file mode 100644 index ..b43bd3cf7889 --- /dev/null +++ b/arch/powerpc/platforms/pseries/fwsecurityfs_arch.c @@ -0,0 +1,116 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Initialize fwsecurityfs with POWER LPAR Platform KeyStore (PLPKS) + * Copyright (C) 2022 IBM Corporation + * Author: Nayna Jain + * + */ + +#include +#include "plpks.h" + +static struct dentry *plpks_dir; + +static ssize_t plpks_config_file_read(struct file *file, char __user *userbuf, + size_t count, loff_t *ppos) +{ + u8 out[4]; + u32 outlen; + size_t size; + char *name; + u32 data; + + name = file_dentry(file)->d_iname; + + if (strcmp(name, "max_object_size") == 0) { + outlen = sizeof(u16); + data = plpks_get_maxobjectsize(); + } else if (strcmp(name, "max_object_label_size") == 0) { + outlen = sizeof(u16); + data = plpks_get_maxobjectlabelsize(); + } else if (strcmp(name, "total_size") == 0) { + outlen = sizeof(u32); + data = plpks_get_totalsize(); + } else if (strcmp(name, "used_space") == 0) { + outlen = sizeof(u32); + data = plpks_get_usedspace(); + } else if (strcmp(name, "version") == 0) { + outlen = sizeof(u8); + data = plpks_get_version(); + } else { + return -EINVAL; + } + + memcpy(out, &data, outlen); + + size = simple_read_from_buffer(userbuf, count, ppos, out, outlen); + + return size; +} + +static const struct file_operations plpks_config_file_operations = { + .open = simple_open, + .read = plpks_config_file_read, + .llseek = no_llseek, +}; + +static int create_plpks_dir(void) +{ + struct dentry *config_dir; + struct dentry *fdentry; + + if (!IS_ENABLED(CONFIG_PSERIES_PLPKS) || !plpks_is_available()) { + pr_warn("Platform KeyStore is not available on this LPAR\n"); + return 0; + } + + plpks_dir = fwsecurityfs_create_dir("plpks", S_IFDIR | 0755, NULL, + NULL); + if (IS_ERR(plpks_dir)) { + pr_err("Unable to create PLPKS dir: %ld\n", PTR_ERR(plpks_dir)); + return PTR_ERR(plpks_dir); + } + + config_dir = fwsecurityfs_create_dir("config", S_IFDIR | 0755, plpks_dir, NULL); +
[PATCH 2/4] fs: define a firmware security filesystem named fwsecurityfs
securityfs is meant for Linux security subsystems to expose policies/logs or any other information. However, there are various firmware security features which expose their variables for user management via the kernel. There is currently no single place to expose these variables. Different platforms use sysfs/platform specific filesystem(efivarfs)/securityfs interface as they find it appropriate. Thus, there is a gap in kernel interfaces to expose variables for security features. Define a firmware security filesystem (fwsecurityfs) to be used by security features enabled by the firmware. These variables are platform specific. This filesystem provides platforms a way to implement their own underlying semantics by defining own inode and file operations. Similar to securityfs, the firmware security filesystem is recommended to be exposed on a well known mount point /sys/firmware/security. Platforms can define their own directory or file structure under this path. Example: # mount -t fwsecurityfs fwsecurityfs /sys/firmware/security # cd /sys/firmware/security/ Signed-off-by: Nayna Jain --- fs/Kconfig | 1 + fs/Makefile | 1 + fs/fwsecurityfs/Kconfig | 14 ++ fs/fwsecurityfs/Makefile | 10 ++ fs/fwsecurityfs/super.c | 263 +++ include/linux/fwsecurityfs.h | 29 include/uapi/linux/magic.h | 1 + 7 files changed, 319 insertions(+) create mode 100644 fs/fwsecurityfs/Kconfig create mode 100644 fs/fwsecurityfs/Makefile create mode 100644 fs/fwsecurityfs/super.c create mode 100644 include/linux/fwsecurityfs.h diff --git a/fs/Kconfig b/fs/Kconfig index 2685a4d0d353..2a24f1c779dd 100644 --- a/fs/Kconfig +++ b/fs/Kconfig @@ -275,6 +275,7 @@ config ARCH_HAS_GIGANTIC_PAGE source "fs/configfs/Kconfig" source "fs/efivarfs/Kconfig" +source "fs/fwsecurityfs/Kconfig" endmenu diff --git a/fs/Makefile b/fs/Makefile index 4dea17840761..b945019a9bbe 100644 --- a/fs/Makefile +++ b/fs/Makefile @@ -134,6 +134,7 @@ obj-$(CONFIG_F2FS_FS) += f2fs/ obj-$(CONFIG_CEPH_FS) += ceph/ obj-$(CONFIG_PSTORE) += pstore/ obj-$(CONFIG_EFIVAR_FS)+= efivarfs/ +obj-$(CONFIG_FWSECURITYFS) += fwsecurityfs/ obj-$(CONFIG_EROFS_FS) += erofs/ obj-$(CONFIG_VBOXSF_FS)+= vboxsf/ obj-$(CONFIG_ZONEFS_FS)+= zonefs/ diff --git a/fs/fwsecurityfs/Kconfig b/fs/fwsecurityfs/Kconfig new file mode 100644 index ..1dc2ee831eda --- /dev/null +++ b/fs/fwsecurityfs/Kconfig @@ -0,0 +1,14 @@ +# SPDX-License-Identifier: GPL-2.0-only +# +# Copyright (C) 2022 IBM Corporation +# Author: Nayna Jain +# + +config FWSECURITYFS + bool "Enable the fwsecurityfs filesystem" + help + This will build fwsecurityfs filesystem which should be mounted + on /sys/firmware/security. This filesystem can be used by + platform to expose firmware-managed variables. + + If you are unsure how to answer this question, answer N. diff --git a/fs/fwsecurityfs/Makefile b/fs/fwsecurityfs/Makefile new file mode 100644 index ..5c7b76228ebb --- /dev/null +++ b/fs/fwsecurityfs/Makefile @@ -0,0 +1,10 @@ +# SPDX-License-Identifier: GPL-2.0-only +# +# Copyright (C) 2022 IBM Corporation +# Author: Nayna Jain +# +# Makefile for the firmware security filesystem + +obj-$(CONFIG_FWSECURITYFS) += fwsecurityfs.o + +fwsecurityfs-objs := super.o diff --git a/fs/fwsecurityfs/super.c b/fs/fwsecurityfs/super.c new file mode 100644 index ..99ca4da4ab63 --- /dev/null +++ b/fs/fwsecurityfs/super.c @@ -0,0 +1,263 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2022 IBM Corporation + * Author: Nayna Jain + */ + +#include +#include +#include +#include +#include +#include +#include + +static struct super_block *fwsecsb; +static struct vfsmount *mount; +static int mount_count; +static bool fwsecurityfs_initialized; + +static void fwsecurityfs_free_inode(struct inode *inode) +{ + free_inode_nonrcu(inode); +} + +static const struct super_operations fwsecurityfs_super_operations = { + .statfs = simple_statfs, + .free_inode = fwsecurityfs_free_inode, +}; + +static int fwsecurityfs_fill_super(struct super_block *sb, + struct fs_context *fc) +{ + static const struct tree_descr files[] = {{""}}; + int rc; + + rc = simple_fill_super(sb, FWSECURITYFS_MAGIC, files); + if (rc) + return rc; + + sb->s_op = &fwsecurityfs_super_operations; + + fwsecsb = sb; + + rc = arch_fwsecurityfs_init(); + + if (!rc) + fwsecurityfs_initialized = true; + + return rc; +} + +static int fwsecurityfs_get_tree(struct fs_context *fc) +{ + return get_tree_single(fc, fwsecurityfs_fill_super); +} + +static co
[PATCH 0/4] powerpc/pseries: expose firmware security variables via filesystem
PowerVM provides an isolated Platform KeyStore (PKS)[1] storage allocation for each logical partition (LPAR) with individually managed access controls to store sensitive information securely. The Linux kernel can access this storage by interfacing with the hypervisor using a new set of hypervisor calls. The PowerVM guest secure boot feature intends to use PKS for the purpose of storing public keys. Secure boot requires public keys in order to verify GRUB and the boot kernel. To allow authenticated manipulation of keys, PKS supports variables to store key authorities namely, PK and KEK. Other variables are used to store code signing keys, db and grubdb. It also supports deny lists to disallow booting GRUB or kernels even if they are signed with valid keys. This is done via deny list databases stored in dbx and sbat variables. These variables are stored in PKS and are managed and controlled by firmware. The purpose of this patchset is to add the userspace interface to manage these variables. Currently, OpenPOWER exposes variables via sysfs, while EFI platforms have used sysfs and then moved to their own efivarfs filesystem. The recent coco feature uses securityfs to expose secrets to TEEs. All of these environments are different both syntactically and semantically. securityfs is meant for Linux security subsystems to expose policies, logs, and other information and it does not interact with firmware for managing these variables. However, there are various firmware security features that expose their variables for user management via pseudo filesystems as discussed above. There is currently no single place to expose these variables. Different platforms use sysfs, platform-specific filesystems such as efivars, or securityfs as they have found appropriate. This has resulted in interfaces scattered around the tree. The multiple interfac problem can be addressed by providing a single pseudo filesystem for all platforms to expose their variables for firmware security features. Doing so simplifies the interface for users of these platforms. This patchset introduces a new firmware security pseudo filesystem, fwsecurityfs. Any platform can expose the variables that are required by firmware security features via this interface. It provides a common place for exposing variables managed by firmware while still allowing platforms to implement their own underlying semantics. This design consists of two parts: 1. Firmware security filesystem (fwsecurityfs) that provides platforms with APIs to create their own underlying directory and file structure. It should be mounted on a new well-known mountpoint, /sys/firmware/security. 2. Platform-specific layer for these variables that implements underlying semantics. Platforms can expose their variables as files allowing read/write/add/delete operations by defining their own inode and file functions. This patchset adds: 1. An update to the PLPKS driver to support the signed update H_CALL for authenticated variables used in guest secure boot. 2. A firmware security filesystem named fwsecurityfs. 3. An interface to expose secure variables stored in the LPAR's PKS via fwsecurityfs. Note: This patchset is not intended to modify existing interfaces already used by OpenPOWER or EFI but rather to ensure that new similar interfaces have a common base going forward. The first patch related to PLPKS driver is dependent on bugfixes posted as part of patchset[4]. Changelog: First non-RFC version after RFC versions[2,3]. Feedback from non-RFC version are included to update fwsecurityfs. * PLPKS driver patch had been upstreamed separately. In this set, Patch 1 updates existing driver to include signed update support. * Fix fwsecurityfs to also pin the file system, refactor and cleanup. The consideration of namespacing has been done and is concluded that currently no firmware object or entity is handled by namespacing. The purpose of fwsecurityfs is to expose firmware space which is similar to exposing space in TPM. And TPM is also not currently namespaced. If containers have to make use of some such space in the future, it would have to be some software space. With that, this currently only considers the host using the firmware space. * Fix secvars support for powerpc. It supports policy handling within the kernel, supports UCS2 naming and cleanups. * Read-only PLPKS configuration is exposed. * secvars directory is now moved within a new parent directory plpks. * Patch is now no more an RFC version. [1] https://community.ibm.com/community/user/power/blogs/chris-engel1/2020/11/20/powervm-introduces-the-platform-keystore [2] RFC v2: https://lore.kernel.org/linuxppc-dev/20220622215648.96723-1-na...@linux.ibm.com/ [3] RFC v1: https://lore.kernel.org/linuxppc-dev/20220122005637.28199-1-na...@linux.ibm.com/ [4] https://lore.kernel.org/linuxppc-dev/20221106205839.600442-1-na...@linux.ibm.com/T/#t Nayna Jain (4): powerpc/pseries: Ad
[PATCH 6/6] powerpc/pseries: fix plpks_read_var() code for different consumers
Even though plpks_read_var() is currently called to read variables owned by different consumers, it internally supports only OS consumer. Fix plpks_read_var() to handle different consumers correctly. Fixes: 2454a7af0f2a ("powerpc/pseries: define driver for Platform KeyStore") Signed-off-by: Nayna Jain --- arch/powerpc/platforms/pseries/plpks.c | 28 +- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/arch/powerpc/platforms/pseries/plpks.c b/arch/powerpc/platforms/pseries/plpks.c index e8c02735b702..4edd1585e245 100644 --- a/arch/powerpc/platforms/pseries/plpks.c +++ b/arch/powerpc/platforms/pseries/plpks.c @@ -354,22 +354,24 @@ static int plpks_read_var(u8 consumer, struct plpks_var *var) { unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = { 0 }; struct plpks_auth *auth; - struct label *label; + struct label *label = NULL; u8 *output; int rc; if (var->namelen > MAX_NAME_SIZE) return -EINVAL; - auth = construct_auth(PKS_OS_OWNER); + auth = construct_auth(consumer); if (IS_ERR(auth)) return PTR_ERR(auth); - label = construct_label(var->component, var->os, var->name, - var->namelen); - if (IS_ERR(label)) { - rc = PTR_ERR(label); - goto out_free_auth; + if (consumer == PKS_OS_OWNER) { + label = construct_label(var->component, var->os, var->name, + var->namelen); + if (IS_ERR(label)) { + rc = PTR_ERR(label); + goto out_free_auth; + } } output = kzalloc(maxobjsize, GFP_KERNEL); @@ -378,9 +380,15 @@ static int plpks_read_var(u8 consumer, struct plpks_var *var) goto out_free_label; } - rc = plpar_hcall(H_PKS_READ_OBJECT, retbuf, virt_to_phys(auth), -virt_to_phys(label), label->size, virt_to_phys(output), -maxobjsize); + if (consumer == PKS_OS_OWNER) + rc = plpar_hcall(H_PKS_READ_OBJECT, retbuf, virt_to_phys(auth), +virt_to_phys(label), label->size, virt_to_phys(output), +maxobjsize); + else + rc = plpar_hcall(H_PKS_READ_OBJECT, retbuf, virt_to_phys(auth), +virt_to_phys(var->name), var->namelen, virt_to_phys(output), +maxobjsize); + if (rc != H_SUCCESS) { rc = pseries_status_to_err(rc); -- 2.31.1
[PATCH 5/6] powerpc/pseries: replace kmalloc with kzalloc in PLPKS driver
Replace kmalloc with kzalloc in construct_auth() function to default initialize structure with zeroes. Signed-off-by: Nayna Jain --- arch/powerpc/platforms/pseries/plpks.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/arch/powerpc/platforms/pseries/plpks.c b/arch/powerpc/platforms/pseries/plpks.c index 72d9debf18c0..e8c02735b702 100644 --- a/arch/powerpc/platforms/pseries/plpks.c +++ b/arch/powerpc/platforms/pseries/plpks.c @@ -162,19 +162,15 @@ static struct plpks_auth *construct_auth(u8 consumer) if (consumer > PKS_OS_OWNER) return ERR_PTR(-EINVAL); - auth = kmalloc(struct_size(auth, password, maxpwsize), GFP_KERNEL); + auth = kzalloc(struct_size(auth, password, maxpwsize), GFP_KERNEL); if (!auth) return ERR_PTR(-ENOMEM); auth->version = 1; auth->consumer = consumer; - auth->rsvd0 = 0; - auth->rsvd1 = 0; - if (consumer == PKS_FW_OWNER || consumer == PKS_BOOTLOADER_OWNER) { - auth->passwordlength = 0; + if (consumer == PKS_FW_OWNER || consumer == PKS_BOOTLOADER_OWNER) return auth; - } memcpy(auth->password, ospassword, ospasswordlength); -- 2.31.1
[PATCH 4/6] powerpc/pseries: cleanup error logs in plpks driver
Logging H_CALL return codes in PLPKS driver are easy to confuse with Linux error codes. Let the caller of the function log the converted linux error code. Signed-off-by: Nayna Jain --- arch/powerpc/platforms/pseries/plpks.c | 10 -- 1 file changed, 10 deletions(-) diff --git a/arch/powerpc/platforms/pseries/plpks.c b/arch/powerpc/platforms/pseries/plpks.c index cbea447122ca..72d9debf18c0 100644 --- a/arch/powerpc/platforms/pseries/plpks.c +++ b/arch/powerpc/platforms/pseries/plpks.c @@ -312,10 +312,6 @@ int plpks_write_var(struct plpks_var var) if (!rc) rc = plpks_confirm_object_flushed(label, auth); - if (rc) - pr_err("Failed to write variable %s for component %s with error %d\n", - var.name, var.component, rc); - rc = pseries_status_to_err(rc); kfree(label); out: @@ -350,10 +346,6 @@ int plpks_remove_var(char *component, u8 varos, struct plpks_var_name vname) if (!rc) rc = plpks_confirm_object_flushed(label, auth); - if (rc) - pr_err("Failed to remove variable %s for component %s with error %d\n", - vname.name, component, rc); - rc = pseries_status_to_err(rc); kfree(label); out: @@ -395,8 +387,6 @@ static int plpks_read_var(u8 consumer, struct plpks_var *var) maxobjsize); if (rc != H_SUCCESS) { - pr_err("Failed to read variable %s for component %s with error %d\n", - var->name, var->component, rc); rc = pseries_status_to_err(rc); goto out_free_output; } -- 2.31.1
[PATCH 3/6] powerpc/pseries: Return -EIO instead of -EINTR for H_ABORTED error
Some commands for eg. "cat" might continue to retry on encountering EINTR. This is not expected for original error code H_ABORTED. Map H_ABORTED to more relevant Linux error code EIO. Fixes: 2454a7af0f2a ("powerpc/pseries: define driver for Platform KeyStore") Signed-off-by: Nayna Jain --- arch/powerpc/platforms/pseries/plpks.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/platforms/pseries/plpks.c b/arch/powerpc/platforms/pseries/plpks.c index 32ce4d780d8f..cbea447122ca 100644 --- a/arch/powerpc/platforms/pseries/plpks.c +++ b/arch/powerpc/platforms/pseries/plpks.c @@ -111,7 +111,7 @@ static int pseries_status_to_err(int rc) err = -EEXIST; break; case H_ABORTED: - err = -EINTR; + err = -EIO; break; default: err = -EINVAL; -- 2.31.1
[PATCH 2/6] powerpc/pseries: Fix the H_CALL error code in PLPKS driver
PAPR Spec defines H_P1 actually as H_PARAMETER and maps H_ABORTED to a different numerical value. Fix the error codes as per PAPR Specification. Fixes: 2454a7af0f2a ("powerpc/pseries: define driver for Platform KeyStore") Signed-off-by: Nayna Jain --- arch/powerpc/include/asm/hvcall.h | 3 +-- arch/powerpc/platforms/pseries/plpks.c | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h index 8abae463f6c1..95fd7f9485d5 100644 --- a/arch/powerpc/include/asm/hvcall.h +++ b/arch/powerpc/include/asm/hvcall.h @@ -79,7 +79,7 @@ #define H_NOT_ENOUGH_RESOURCES -44 #define H_R_STATE -45 #define H_RESCINDED -46 -#define H_P1 -54 +#define H_ABORTED -54 #define H_P2 -55 #define H_P3 -56 #define H_P4 -57 @@ -100,7 +100,6 @@ #define H_COP_HW -74 #define H_STATE-75 #define H_IN_USE -77 -#define H_ABORTED -78 #define H_UNSUPPORTED_FLAG_START -256 #define H_UNSUPPORTED_FLAG_END -511 #define H_MULTI_THREADS_ACTIVE -9005 diff --git a/arch/powerpc/platforms/pseries/plpks.c b/arch/powerpc/platforms/pseries/plpks.c index f4b5b5a64db3..32ce4d780d8f 100644 --- a/arch/powerpc/platforms/pseries/plpks.c +++ b/arch/powerpc/platforms/pseries/plpks.c @@ -75,7 +75,7 @@ static int pseries_status_to_err(int rc) case H_FUNCTION: err = -ENXIO; break; - case H_P1: + case H_PARAMETER: case H_P2: case H_P3: case H_P4: -- 2.31.1
[PATCH 0/6] powerpc/pseries - bugfixes/cleanups for PLPKS driver
This patchset fixes some bugs and does some cleanups. Nayna Jain (6): powerpc/pseries: fix the object owners enum value in plpks driver powerpc/pseries: Fix the H_CALL error code in PLPKS driver powerpc/pseries: Return -EIO instead of -EINTR for H_ABORTED error powerpc/pseries: cleanup error logs in plpks driver powerpc/pseries: replace kmalloc with kzalloc in PLPKS driver powerpc/pseries: fix plpks_read_var() code for different consumers arch/powerpc/include/asm/hvcall.h | 3 +- arch/powerpc/platforms/pseries/plpks.c | 50 -- arch/powerpc/platforms/pseries/plpks.h | 2 +- 3 files changed, 24 insertions(+), 31 deletions(-) -- 2.31.1
[PATCH 1/6] powerpc/pseries: fix the object owners enum value in plpks driver
OS_VAR_LINUX enum in PLPKS driver should be 0x02 instead of 0x01. Fixes: 2454a7af0f2a ("powerpc/pseries: define driver for Platform KeyStore") Signed-off-by: Nayna Jain --- arch/powerpc/platforms/pseries/plpks.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/platforms/pseries/plpks.h b/arch/powerpc/platforms/pseries/plpks.h index c6a291367bb1..275ccd86bfb5 100644 --- a/arch/powerpc/platforms/pseries/plpks.h +++ b/arch/powerpc/platforms/pseries/plpks.h @@ -17,7 +17,7 @@ #define WORLDREADABLE 0x0800 #define SIGNEDUPDATE 0x0100 -#define PLPKS_VAR_LINUX0x01 +#define PLPKS_VAR_LINUX0x02 #define PLPKS_VAR_COMMON 0x04 struct plpks_var { -- 2.31.1
Re: [PATCH v3 1/2] lib: generic accessor functions for arch keystore
On 8/1/22 09:40, Michal Suchánek wrote: Hello, On Mon, Aug 01, 2022 at 07:34:25AM -0500, gjo...@linux.vnet.ibm.com wrote: From: Greg Joyce Generic kernel subsystems may rely on platform specific persistent KeyStore to store objects containing sensitive key material. In such case, they need to access architecture specific functions to perform read/write operations on these variables. Define the generic variable read/write prototypes to be implemented by architecture specific versions. The default(weak) implementations of these prototypes return -EOPNOTSUPP unless overridden by architecture versions. Signed-off-by: Greg Joyce --- include/linux/arch_vars.h | 23 +++ lib/Makefile | 2 +- lib/arch_vars.c | 25 + 3 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 include/linux/arch_vars.h create mode 100644 lib/arch_vars.c diff --git a/include/linux/arch_vars.h b/include/linux/arch_vars.h new file mode 100644 index ..9c280ff9432e --- /dev/null +++ b/include/linux/arch_vars.h @@ -0,0 +1,23 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Platform variable opearations. + * + * Copyright (C) 2022 IBM Corporation + * + * These are the accessor functions (read/write) for architecture specific + * variables. Specific architectures can provide overrides. + * + */ + +#include + +enum arch_variable_type { + ARCH_VAR_OPAL_KEY = 0, /* SED Opal Authentication Key */ + ARCH_VAR_OTHER = 1, /* Other type of variable */ + ARCH_VAR_MAX = 1, /* Maximum type value */ +}; + +int arch_read_variable(enum arch_variable_type type, char *varname, + void *varbuf, u_int *varlen); +int arch_write_variable(enum arch_variable_type type, char *varname, + void *varbuf, u_int varlen); diff --git a/lib/Makefile b/lib/Makefile index f99bf61f8bbc..b90c4cb0dbbb 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -48,7 +48,7 @@ obj-y += bcd.o sort.o parser.o debug_locks.o random32.o \ bsearch.o find_bit.o llist.o memweight.o kfifo.o \ percpu-refcount.o rhashtable.o \ once.o refcount.o usercopy.o errseq.o bucket_locks.o \ -generic-radix-tree.o +generic-radix-tree.o arch_vars.o obj-$(CONFIG_STRING_SELFTEST) += test_string.o obj-y += string_helpers.o obj-$(CONFIG_TEST_STRING_HELPERS) += test-string_helpers.o diff --git a/lib/arch_vars.c b/lib/arch_vars.c new file mode 100644 index ..e6f16d7d09c1 --- /dev/null +++ b/lib/arch_vars.c @@ -0,0 +1,25 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Platform variable operations. + * + * Copyright (C) 2022 IBM Corporation + * + * These are the accessor functions (read/write) for architecture specific + * variables. Specific architectures can provide overrides. + * + */ + +#include +#include + +int __weak arch_read_variable(enum arch_variable_type type, char *varname, + void *varbuf, u_int *varlen) +{ + return -EOPNOTSUPP; +} + +int __weak arch_write_variable(enum arch_variable_type type, char *varname, + void *varbuf, u_int varlen) +{ + return -EOPNOTSUPP; +} -- Doesn't EFI already have some variables? And even powernv? Shouldn't this generalize the already existing variables? Or move to powerpc and at least generalize the powerpc ones? Yes, EFI and PowerNV do have variables, but I am not exactly clear about your reference to them in this context. What do you mean by generalize already existing variables ? This interface is actually generalizing calls to access platform specific keystores. It is explained in cover letter that this patch is defining generic interface and these are default implementations which needs to be overridden by arch specific versions. For PowerVM PLPAR Platform KeyStore, the arch specific version is implemented in Patch 2. Access to EFI variables should be implemented by EFI arch specific interface and PowerNV will have to do the same if it needs to. Hope it helps. Thanks & Regards, - Nayna
[PATCH v2 3/3] powerpc/pseries: Override lib/arch_vars.c with PowerPC architecture specific version
From: Greg Joyce Self Encrypting Drives(SED) make use of POWER LPAR Platform KeyStore for storing its variables. Thus the block subsystem needs to access PowerPC specific functions to read/write objects in PLPKS. Override the default implementations in lib/arch_vars.c file with PowerPC specific versions. Signed-off-by: Greg Joyce --- arch/powerpc/platforms/pseries/Makefile | 1 + .../platforms/pseries/plpks_arch_ops.c| 166 ++ 2 files changed, 167 insertions(+) create mode 100644 arch/powerpc/platforms/pseries/plpks_arch_ops.c diff --git a/arch/powerpc/platforms/pseries/Makefile b/arch/powerpc/platforms/pseries/Makefile index 14e143b946a3..3a545422eae5 100644 --- a/arch/powerpc/platforms/pseries/Makefile +++ b/arch/powerpc/platforms/pseries/Makefile @@ -29,6 +29,7 @@ obj-$(CONFIG_PPC_SPLPAR) += vphn.o obj-$(CONFIG_PPC_SVM) += svm.o obj-$(CONFIG_FA_DUMP) += rtas-fadump.o obj-$(CONFIG_PSERIES_PLPKS) += plpks.o +obj-$(CONFIG_PSERIES_PLPKS) += plpks_arch_ops.o obj-$(CONFIG_SUSPEND) += suspend.o obj-$(CONFIG_PPC_VAS) += vas.o vas-sysfs.o diff --git a/arch/powerpc/platforms/pseries/plpks_arch_ops.c b/arch/powerpc/platforms/pseries/plpks_arch_ops.c new file mode 100644 index ..48fa19f0c9c5 --- /dev/null +++ b/arch/powerpc/platforms/pseries/plpks_arch_ops.c @@ -0,0 +1,166 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * POWER Platform arch specific code for SED + * Copyright (C) 2022 IBM Corporation + * + * Define operations for generic kernel subsystems to read/write keys from + * POWER LPAR Platform KeyStore(PLPKS). + * + * List of subsystems/usecase using PLPKS: + * - Self Encrypting Drives(SED) + */ + +#include +#include +#include +#include +#include +#include +#include +#include "plpks.h" + +/* + * variable structure that contains all SED data + */ +struct plpks_sed_object_data { + u_char version; + u_char pad1[7]; + u_long authority; + u_long range; + u_int key_len; + u_char key[32]; +}; + +/* + * ext_type values + * 00no extension exists + * 01-1F common + * 20-3F AIX + * 40-5F Linux + * 60-7F IBMi + */ + +/* + * This extension is optional for version 1 sed_object_data + */ +struct sed_object_extension { + u8 ext_type; + u8 rsvd[3]; + u8 ext_data[64]; +}; + +#define PKS_SED_OBJECT_DATA_V1 1 +#define PKS_SED_MANGLED_LABEL "/default/pri" +#define PLPKS_SED_COMPONENT "sed-opal" +#define PLPKS_SED_POLICYWORLDREADABLE +#define PLPKS_SED_OS_COMMON 4 + +#ifndef CONFIG_BLK_SED_OPAL +#defineOPAL_AUTH_KEY "" +#endif + +/* + * Read the variable data from PKS given the label + */ +int arch_read_variable(enum arch_variable_type type, char *varname, + void *varbuf, u_int *varlen) +{ + struct plpks_var var; + struct plpks_sed_object_data *data; + u_int offset = 0; + char *buf = (char *)varbuf; + int ret; + + var.name = varname; + var.namelen = strlen(varname); + var.policy = PLPKS_SED_POLICY; + var.os = PLPKS_SED_OS_COMMON; + var.data = NULL; + var.datalen = 0; + + switch (type) { + case ARCH_VAR_OPAL_KEY: + var.component = PLPKS_SED_COMPONENT; + if (strcmp(OPAL_AUTH_KEY, varname) == 0) { + var.name = PKS_SED_MANGLED_LABEL; + var.namelen = strlen(varname); + } + offset = offsetof(struct plpks_sed_object_data, key); + break; + case ARCH_VAR_OTHER: + var.component = ""; + break; + } + + ret = plpks_read_os_var(&var); + if (ret != 0) + return ret; + + if (offset > var.datalen) + offset = 0; + + switch (type) { + case ARCH_VAR_OPAL_KEY: + data = (struct plpks_sed_object_data *)var.data; + *varlen = data->key_len; + break; + case ARCH_VAR_OTHER: + *varlen = var.datalen; + break; + } + + if (var.data) { + memcpy(varbuf, var.data + offset, var.datalen - offset); + buf[*varlen] = '\0'; + kfree(var.data); + } + + return 0; +} + +/* + * Write the variable data to PKS given the label + */ +int arch_write_variable(enum arch_variable_type type, char *varname, + void *varbuf, u_int varlen) +{ + struct plpks_var var; + struct plpks_sed_object_data data; + struct plpks_var_name vname; + + var.name = varname; + var.namelen = strlen(varname); + var.policy = PLPKS_SED_POLICY; + var.os = PLPKS_SED_OS_COMMON; + var.datalen = varlen; + var.data = varbuf; + + switch (type) { + case ARCH_VAR_OPAL_KEY: + var.co
[PATCH v2 2/3] lib: define generic accessor functions for arch specific keystore
From: Greg Joyce Generic kernel subsystems may rely on platform specific persistent KeyStore to store objects containing sensitive key material. In such case, they need to access architecture specific functions to perform read/write operations on these variables. Define the generic variable read/write prototypes to be implemented by architecture specific versions. The default(weak) implementations of these prototypes return -EOPNOTSUPP unless overridden by architecture versions. Signed-off-by: Greg Joyce --- include/linux/arch_vars.h | 23 +++ lib/Makefile | 2 +- lib/arch_vars.c | 25 + 3 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 include/linux/arch_vars.h create mode 100644 lib/arch_vars.c diff --git a/include/linux/arch_vars.h b/include/linux/arch_vars.h new file mode 100644 index ..9c280ff9432e --- /dev/null +++ b/include/linux/arch_vars.h @@ -0,0 +1,23 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Platform variable opearations. + * + * Copyright (C) 2022 IBM Corporation + * + * These are the accessor functions (read/write) for architecture specific + * variables. Specific architectures can provide overrides. + * + */ + +#include + +enum arch_variable_type { + ARCH_VAR_OPAL_KEY = 0, /* SED Opal Authentication Key */ + ARCH_VAR_OTHER = 1, /* Other type of variable */ + ARCH_VAR_MAX = 1, /* Maximum type value */ +}; + +int arch_read_variable(enum arch_variable_type type, char *varname, + void *varbuf, u_int *varlen); +int arch_write_variable(enum arch_variable_type type, char *varname, + void *varbuf, u_int varlen); diff --git a/lib/Makefile b/lib/Makefile index f99bf61f8bbc..b90c4cb0dbbb 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -48,7 +48,7 @@ obj-y += bcd.o sort.o parser.o debug_locks.o random32.o \ bsearch.o find_bit.o llist.o memweight.o kfifo.o \ percpu-refcount.o rhashtable.o \ once.o refcount.o usercopy.o errseq.o bucket_locks.o \ -generic-radix-tree.o +generic-radix-tree.o arch_vars.o obj-$(CONFIG_STRING_SELFTEST) += test_string.o obj-y += string_helpers.o obj-$(CONFIG_TEST_STRING_HELPERS) += test-string_helpers.o diff --git a/lib/arch_vars.c b/lib/arch_vars.c new file mode 100644 index ..e6f16d7d09c1 --- /dev/null +++ b/lib/arch_vars.c @@ -0,0 +1,25 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Platform variable operations. + * + * Copyright (C) 2022 IBM Corporation + * + * These are the accessor functions (read/write) for architecture specific + * variables. Specific architectures can provide overrides. + * + */ + +#include +#include + +int __weak arch_read_variable(enum arch_variable_type type, char *varname, + void *varbuf, u_int *varlen) +{ + return -EOPNOTSUPP; +} + +int __weak arch_write_variable(enum arch_variable_type type, char *varname, + void *varbuf, u_int varlen) +{ + return -EOPNOTSUPP; +} -- 2.27.0
[PATCH v2 1/3] powerpc/pseries: define driver for Platform KeyStore
PowerVM provides an isolated Platform Keystore(PKS) storage allocation for each LPAR with individually managed access controls to store sensitive information securely. It provides a new set of hypervisor calls for Linux kernel to access PKS storage. Define POWER LPAR Platform KeyStore(PLPKS) driver using H_CALL interface to access PKS storage. Signed-off-by: Nayna Jain --- arch/powerpc/include/asm/hvcall.h | 11 + arch/powerpc/platforms/pseries/Kconfig | 13 + arch/powerpc/platforms/pseries/Makefile | 1 + arch/powerpc/platforms/pseries/plpks.c | 460 arch/powerpc/platforms/pseries/plpks.h | 71 5 files changed, 556 insertions(+) create mode 100644 arch/powerpc/platforms/pseries/plpks.c create mode 100644 arch/powerpc/platforms/pseries/plpks.h diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h index d92a20a85395..9f707974af1a 100644 --- a/arch/powerpc/include/asm/hvcall.h +++ b/arch/powerpc/include/asm/hvcall.h @@ -79,6 +79,7 @@ #define H_NOT_ENOUGH_RESOURCES -44 #define H_R_STATE -45 #define H_RESCINDED -46 +#define H_P1 -54 #define H_P2 -55 #define H_P3 -56 #define H_P4 -57 @@ -97,6 +98,8 @@ #define H_OP_MODE -73 #define H_COP_HW -74 #define H_STATE-75 +#define H_IN_USE -77 +#define H_ABORTED -78 #define H_UNSUPPORTED_FLAG_START -256 #define H_UNSUPPORTED_FLAG_END -511 #define H_MULTI_THREADS_ACTIVE -9005 @@ -321,6 +324,14 @@ #define H_SCM_UNBIND_ALL0x3FC #define H_SCM_HEALTH0x400 #define H_SCM_PERFORMANCE_STATS 0x418 +#define H_PKS_GET_CONFIG 0x41C +#define H_PKS_SET_PASSWORD 0x420 +#define H_PKS_GEN_PASSWORD 0x424 +#define H_PKS_WRITE_OBJECT 0x42C +#define H_PKS_GEN_KEY 0x430 +#define H_PKS_READ_OBJECT 0x434 +#define H_PKS_REMOVE_OBJECT0x438 +#define H_PKS_CONFIRM_OBJECT_FLUSHED 0x43C #define H_RPT_INVALIDATE 0x448 #define H_SCM_FLUSH0x44C #define H_GET_ENERGY_SCALE_INFO0x450 diff --git a/arch/powerpc/platforms/pseries/Kconfig b/arch/powerpc/platforms/pseries/Kconfig index f7fd91d153a4..c4a6d4083a7a 100644 --- a/arch/powerpc/platforms/pseries/Kconfig +++ b/arch/powerpc/platforms/pseries/Kconfig @@ -142,6 +142,19 @@ config IBMEBUS help Bus device driver for GX bus based adapters. +config PSERIES_PLPKS + depends on PPC_PSERIES + bool "Support for the Platform Key Storage" + help + PowerVM provides an isolated Platform Keystore(PKS) storage + allocation for each LPAR with individually managed access + controls to store sensitive information securely. It can be + used to store asymmetric public keys or secrets as required + by different usecases. Select this config to enable + operating system interface to hypervisor to access this space. + + If unsure, select N. + config PAPR_SCM depends on PPC_PSERIES && MEMORY_HOTPLUG && LIBNVDIMM tristate "Support for the PAPR Storage Class Memory interface" diff --git a/arch/powerpc/platforms/pseries/Makefile b/arch/powerpc/platforms/pseries/Makefile index 7aaff5323544..14e143b946a3 100644 --- a/arch/powerpc/platforms/pseries/Makefile +++ b/arch/powerpc/platforms/pseries/Makefile @@ -28,6 +28,7 @@ obj-$(CONFIG_PAPR_SCM)+= papr_scm.o obj-$(CONFIG_PPC_SPLPAR) += vphn.o obj-$(CONFIG_PPC_SVM) += svm.o obj-$(CONFIG_FA_DUMP) += rtas-fadump.o +obj-$(CONFIG_PSERIES_PLPKS) += plpks.o obj-$(CONFIG_SUSPEND) += suspend.o obj-$(CONFIG_PPC_VAS) += vas.o vas-sysfs.o diff --git a/arch/powerpc/platforms/pseries/plpks.c b/arch/powerpc/platforms/pseries/plpks.c new file mode 100644 index ..52aaa2894606 --- /dev/null +++ b/arch/powerpc/platforms/pseries/plpks.c @@ -0,0 +1,460 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * POWER LPAR Platform KeyStore(PLPKS) + * Copyright (C) 2022 IBM Corporation + * Author: Nayna Jain + * + * Provides access to variables stored in Power LPAR Platform KeyStore(PLPKS). + */ + +#define pr_fmt(fmt) "plpks: " fmt + +#include +#include +#include +#include +#include +#include +#include +#include + +#include "plpks.h" + +#define PKS_FW_OWNER0x1 +#define PKS_BOOTLOADER_OWNER 0x2 +#define PKS_OS_OWNER0x3 + +#define LABEL_VERSION 0 +#define MAX_LABEL_ATTR_SIZE 16 +#define MAX_NAME_SIZE 239 +#define MAX_DATA_SIZE 4000 + +#define PKS_FLUSH_MAX_TIMEOUT 5000 //msec +#define PKS_FLUSH_SLEEP 10 //msec +#define PKS_FLUSH_SLEEP_RANGE 400 + +static u8 *ospassword; +static u16 ospasswordlength; + +// Retrieved with H_PKS_GET_CONFIG +static u16 maxpwsize; +static u16 maxobjsize; + +struct plpks_auth { + u8 version; + u8 consumer; + __be64 rsvd0; + __be32 r
[PATCH v2 0/3] Provide PowerVM LPAR Platform KeyStore driver for Self Encrypting Drives
PowerVM provides an isolated Platform KeyStore(PKS)[1] storage allocation for each partition(LPAR) with individually managed access controls to store sensitive information securely. The Linux Kernel can access this storage by interfacing with the hypervisor using a new set of hypervisor calls. This storage can be used for multiple purposes. The current two usecases are: 1. Guest Secure Boot on PowerVM[2] 2. Self Encrypting Drives(SED) on PowerVM[3] Initially, the PowerVM LPAR Platform KeyStore(PLPKS) driver was defined as part of RFC patches which included the user interface design for guest secure boot[2]. While this interface is still in progress, the same driver is also required for Self Encrypting Drives(SED) support. For this reason, the driver is being split from the patchset[1] and is now separately posted with SED arch-specific code. This patchset provides driver for PowerVM LPAR Platform KeyStore and also arch-specific code for SED to make use of it. The dependency patch from patch series[3] is moved to this patchset. This patchset now builds completely of its own. [1]https://community.ibm.com/community/user/power/blogs/chris-engel1/2020/11/20/powervm-introduces-the-platform-keystore [2]https://lore.kernel.org/linuxppc-dev/20220622215648.96723-1-na...@linux.ibm.com/ [3]https://lore.kernel.org/keyrings/20220718210156.1535955-1-gjo...@linux.vnet.ibm.com/T/#m8e7b2cbbd26ee1de711bd70967fd0124c85c479f Changelog: v2: * Include feedback from Gregory Joyce, Eric Richter and Murilo Opsfelder Araújo. * Include suggestions from Michael Ellerman. * Moved a dependency from generic SED code to this patchset. This patchset now builds of its own. Greg Joyce (2): lib: define generic accessor functions for arch specific keystore powerpc/pseries: Override lib/arch_vars.c with PowerPC architecture specific version Nayna Jain (1): powerpc/pseries: define driver for Platform KeyStore arch/powerpc/include/asm/hvcall.h | 11 + arch/powerpc/platforms/pseries/Kconfig| 13 + arch/powerpc/platforms/pseries/Makefile | 2 + arch/powerpc/platforms/pseries/plpks.c| 460 ++ arch/powerpc/platforms/pseries/plpks.h| 71 +++ .../platforms/pseries/plpks_arch_ops.c| 166 +++ include/linux/arch_vars.h | 23 + lib/Makefile | 2 +- lib/arch_vars.c | 25 + 9 files changed, 772 insertions(+), 1 deletion(-) create mode 100644 arch/powerpc/platforms/pseries/plpks.c create mode 100644 arch/powerpc/platforms/pseries/plpks.h create mode 100644 arch/powerpc/platforms/pseries/plpks_arch_ops.c create mode 100644 include/linux/arch_vars.h create mode 100644 lib/arch_vars.c -- 2.27.0
[PATCH 1/2] powerpc/pseries: define driver for Platform KeyStore
PowerVM provides an isolated Platform Keystore(PKS) storage allocation for each LPAR with individually managed access controls to store sensitive information securely. It provides a new set of hypervisor calls for Linux kernel to access PKS storage. Define PLPKS driver using H_CALL interface to access PKS storage. Signed-off-by: Nayna Jain --- arch/powerpc/include/asm/hvcall.h | 9 + arch/powerpc/include/asm/plpks.h | 90 arch/powerpc/platforms/pseries/Kconfig| 13 + arch/powerpc/platforms/pseries/Makefile | 2 + arch/powerpc/platforms/pseries/plpks/Makefile | 7 + arch/powerpc/platforms/pseries/plpks/plpks.c | 509 ++ 6 files changed, 630 insertions(+) create mode 100644 arch/powerpc/include/asm/plpks.h create mode 100644 arch/powerpc/platforms/pseries/plpks/Makefile create mode 100644 arch/powerpc/platforms/pseries/plpks/plpks.c diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h index d92a20a85395..24b661b0717c 100644 --- a/arch/powerpc/include/asm/hvcall.h +++ b/arch/powerpc/include/asm/hvcall.h @@ -97,6 +97,7 @@ #define H_OP_MODE -73 #define H_COP_HW -74 #define H_STATE-75 +#define H_IN_USE -77 #define H_UNSUPPORTED_FLAG_START -256 #define H_UNSUPPORTED_FLAG_END -511 #define H_MULTI_THREADS_ACTIVE -9005 @@ -321,6 +322,14 @@ #define H_SCM_UNBIND_ALL0x3FC #define H_SCM_HEALTH0x400 #define H_SCM_PERFORMANCE_STATS 0x418 +#define H_PKS_GET_CONFIG 0x41C +#define H_PKS_SET_PASSWORD 0x420 +#define H_PKS_GEN_PASSWORD 0x424 +#define H_PKS_WRITE_OBJECT 0x42C +#define H_PKS_GEN_KEY 0x430 +#define H_PKS_READ_OBJECT 0x434 +#define H_PKS_REMOVE_OBJECT0x438 +#define H_PKS_CONFIRM_OBJECT_FLUSHED 0x43C #define H_RPT_INVALIDATE 0x448 #define H_SCM_FLUSH0x44C #define H_GET_ENERGY_SCALE_INFO0x450 diff --git a/arch/powerpc/include/asm/plpks.h b/arch/powerpc/include/asm/plpks.h new file mode 100644 index ..cf60e53e1f15 --- /dev/null +++ b/arch/powerpc/include/asm/plpks.h @@ -0,0 +1,90 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (C) 2022 IBM Corporation + * Author: Nayna Jain + * + * Platform keystore for pseries LPAR(PLPKS). + */ + +#ifndef _PSERIES_PLPKS_H +#define _PSERIES_PLPKS_H + +#include +#include + +#define OSSECBOOTAUDIT 0x4000 +#define OSSECBOOTENFORCE 0x2000 +#define WORLDREADABLE 0x0800 +#define SIGNEDUPDATE 0x0100 + +#define PLPKS_VAR_LINUX0x01 +#define PLPKS_VAR_COMMON 0x04 + +struct plpks_var { + char *component; + u8 os; + u8 *name; + u16 namelen; + u32 policy; + u16 datalen; + u8 *data; +}; + +struct plpks_var_name { + u16 namelen; + u8 *name; +}; + +struct plpks_var_name_list { + u32 varcount; + struct plpks_var_name varlist[]; +}; + +struct plpks_config { + u8 version; + u8 flags; + u32 rsvd0; + u16 maxpwsize; + u16 maxobjlabelsize; + u16 maxobjsize; + u32 totalsize; + u32 usedspace; + u32 supportedpolicies; + u64 rsvd1; +} __packed; + +/** + * Successful return from this API implies PKS is available. + * This is used to initialize kernel driver and user interfaces. + */ +struct plpks_config *plpks_get_config(void); + +/** + * Writes the specified var and its data to PKS. + * Any caller of PKS driver should present a valid component type for + * their variable. + */ +int plpks_write_var(struct plpks_var var); + +/** + * Removes the specified var and its data from PKS. + */ +int plpks_remove_var(char *component, u8 varos, +struct plpks_var_name vname); + +/** + * Returns the data for the specified os variable. + */ +int plpks_read_os_var(struct plpks_var *var); + +/** + * Returns the data for the specified firmware variable. + */ +int plpks_read_fw_var(struct plpks_var *var); + +/** + * Returns the data for the specified bootloader variable. + */ +int plpks_read_bootloader_var(struct plpks_var *var); + +#endif diff --git a/arch/powerpc/platforms/pseries/Kconfig b/arch/powerpc/platforms/pseries/Kconfig index f7fd91d153a4..de6efe5d18c2 100644 --- a/arch/powerpc/platforms/pseries/Kconfig +++ b/arch/powerpc/platforms/pseries/Kconfig @@ -142,6 +142,19 @@ config IBMEBUS help Bus device driver for GX bus based adapters. +config PSERIES_PLPKS + depends on PPC_PSERIES + tristate "Support for the Platform Key Storage" + help + PowerVM provides an isolated Platform Keystore(PKS) storage + allocation for each LPAR with individually managed access + controls to store sensitive information securely. It can be + used to store asymmetric public keys or secrets as required + by different usecases. Select this config to enable + operating system interface to hypervisor to a
[PATCH 2/2] powerpc/pseries: kernel interfaces to PLPKS platform driver
From: Greg Joyce Add platform specific interfaces arch_read_variable() and arch_variable() to allow platform agnostic access to platform variable stores. Signed-off-by: Greg Joyce --- arch/powerpc/platforms/pseries/plpks/Makefile | 1 + .../platforms/pseries/plpks/plpks_arch_ops.c | 163 ++ 2 files changed, 164 insertions(+) create mode 100644 arch/powerpc/platforms/pseries/plpks/plpks_arch_ops.c diff --git a/arch/powerpc/platforms/pseries/plpks/Makefile b/arch/powerpc/platforms/pseries/plpks/Makefile index e651ace920db..3e9ce6f16575 100644 --- a/arch/powerpc/platforms/pseries/plpks/Makefile +++ b/arch/powerpc/platforms/pseries/plpks/Makefile @@ -5,3 +5,4 @@ # obj-$(CONFIG_PSERIES_PLPKS) += plpks.o +obj-$(CONFIG_PSERIES_PLPKS) += plpks_arch_ops.o diff --git a/arch/powerpc/platforms/pseries/plpks/plpks_arch_ops.c b/arch/powerpc/platforms/pseries/plpks/plpks_arch_ops.c new file mode 100644 index ..11cd03da08b7 --- /dev/null +++ b/arch/powerpc/platforms/pseries/plpks/plpks_arch_ops.c @@ -0,0 +1,163 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * POWER platform keystore + * Copyright (C) 2022 IBM Corporation + * + * This pseries platform device driver provides access to + * variables stored in platform keystore. + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +/* + * variable structure that contains all SED data + */ +struct plpks_sed_object_data { + u_char version; + u_char pad1[7]; + u_long authority; + u_long range; + u_int key_len; + u_char key[32]; +}; + +/* + * ext_type values + * 00no extension exists + * 01-1F common + * 20-3F AIX + * 40-5F Linux + * 60-7F IBMi + */ + +/* + * This extension is optional for version 1 sed_object_data + */ +struct sed_object_extension { + u8 ext_type; + u8 rsvd[3]; + u8 ext_data[64]; +}; + +#define PKS_SED_OBJECT_DATA_V1 1 +#define PKS_SED_MANGLED_LABEL "/default/pri" +#define PLPKS_SED_COMPONENT "sed-opal" +#define PLPKS_SED_POLICYWORLDREADABLE +#define PLPKS_SED_OS_COMMON 4 + +#ifndef CONFIG_BLK_SED_OPAL +#defineOPAL_AUTH_KEY "" +#endif + +/* + * Read the variable data from PKS given the label + */ +int arch_read_variable(enum arch_variable_type type, char *varname, + void *varbuf, u_int *varlen) +{ + struct plpks_var var; + struct plpks_sed_object_data *data; + u_int offset = 0; + char *buf = (char *)varbuf; + int ret; + + var.name = varname; + var.namelen = strlen(varname); + var.policy = PLPKS_SED_POLICY; + var.os = PLPKS_SED_OS_COMMON; + var.data = NULL; + var.datalen = 0; + + switch (type) { + case ARCH_VAR_OPAL_KEY: + var.component = PLPKS_SED_COMPONENT; + if (strcmp(OPAL_AUTH_KEY, varname) == 0) { + var.name = PKS_SED_MANGLED_LABEL; + var.namelen = strlen(varname); + } + offset = offsetof(struct plpks_sed_object_data, key); + break; + case ARCH_VAR_OTHER: + var.component = ""; + break; + } + + ret = plpks_read_os_var(&var); + if (ret != 0) + return ret; + + if (offset > var.datalen) + offset = 0; + + switch (type) { + case ARCH_VAR_OPAL_KEY: + data = (struct plpks_sed_object_data *)var.data; + *varlen = data->key_len; + break; + case ARCH_VAR_OTHER: + *varlen = var.datalen; + break; + } + + if (var.data) { + memcpy(varbuf, var.data + offset, var.datalen - offset); + buf[*varlen] = '\0'; + kfree(var.data); + } + + return 0; +} + +/* + * Write the variable data to PKS given the label + */ +int arch_write_variable(enum arch_variable_type type, char *varname, + void *varbuf, u_int varlen) +{ + struct plpks_var var; + struct plpks_sed_object_data data; + struct plpks_var_name vname; + + var.name = varname; + var.namelen = strlen(varname); + var.policy = PLPKS_SED_POLICY; + var.os = PLPKS_SED_OS_COMMON; + var.datalen = varlen; + var.data = varbuf; + + switch (type) { + case ARCH_VAR_OPAL_KEY: + var.component = PLPKS_SED_COMPONENT; + if (strcmp(OPAL_AUTH_KEY, varname) == 0) { + var.name = PKS_SED_MANGLED_LABEL; + var.namelen = strlen(varname); + } + var.datalen = sizeof(struct plpks_sed_object_data); + var.data = (u8 *)&data; + + /* initialize SED object */ + data.version = PKS_SED_OBJECT_DATA_V1; + data.aut
[PATCH 0/2] Provide PowerVM LPAR Platform KeyStore driver for Self Encrypting Drives
PowerVM provides an isolated Platform KeyStore(PKS)[1] storage allocation for each partition(LPAR) with individually managed access controls to store sensitive information securely. The Linux Kernel can access this storage by interfacing with the hypervisor using a new set of hypervisor calls. This storage can be used for multiple purposes. The current two usecases are: 1. Guest Secure Boot on PowerVM[2] 2. Self Encrypting Drives(SED) on PowerVM[3] Initially, the PowerVM LPAR Platform KeyStore(PLPKS) driver was defined as part of RFC patches which included the user interface design for guest secure boot[2]. While this interface is still in progress, the same driver is also required for Self Encrypting Drives(SED) support. For this reason, the driver is being split from the patchset[1] and is now separately posted with SED arch-specific code. This patchset provides driver for PowerVM LPAR Platform KeyStore and also arch-specific code for SED to make use of it. The patch series[3] is pre-requisite to build Patch 2/2. The PLPKS driver can be built of its own. [1]https://community.ibm.com/community/user/power/blogs/chris-engel1/2020/11/20/powervm-introduces-the-platform-keystore [2]https://lore.kernel.org/linuxppc-dev/20220622215648.96723-1-na...@linux.ibm.com/ [3]https://lore.kernel.org/keyrings/20220706023935.875994-1-gjo...@linux.vnet.ibm.com/T/#mc32b51991bf825ec6f90af010998ec7cd2b9624a Greg Joyce (1): powerpc/pseries: kernel interfaces to PLPKS platform driver Nayna Jain (1): powerpc/pseries: define driver for Platform KeyStore arch/powerpc/include/asm/hvcall.h | 9 + arch/powerpc/include/asm/plpks.h | 90 arch/powerpc/platforms/pseries/Kconfig| 13 + arch/powerpc/platforms/pseries/Makefile | 2 + arch/powerpc/platforms/pseries/plpks/Makefile | 8 + arch/powerpc/platforms/pseries/plpks/plpks.c | 509 ++ .../platforms/pseries/plpks/plpks_arch_ops.c | 163 ++ 7 files changed, 794 insertions(+) create mode 100644 arch/powerpc/include/asm/plpks.h create mode 100644 arch/powerpc/platforms/pseries/plpks/Makefile create mode 100644 arch/powerpc/platforms/pseries/plpks/plpks.c create mode 100644 arch/powerpc/platforms/pseries/plpks/plpks_arch_ops.c -- 2.27.0
Re: [RFC PATCH v2 2/3] fs: define a firmware security filesystem named fwsecurityfs
On 6/22/22 18:29, Casey Schaufler wrote: On 6/22/2022 2:56 PM, Nayna Jain wrote: securityfs is meant for linux security subsystems to expose policies/logs or any other information. However, there are various firmware security features which expose their variables for user management via kernel. There is currently no single place to expose these variables. Different platforms use sysfs/platform specific filesystem(efivarfs)/securityfs interface as find appropriate. Thus, there is a gap in kernel interfaces to expose variables for security features. Why not put the firmware entries under /sys/kernel/security/firmware? From man 5 sysfs page: /sys/firmware: This subdirectory contains interfaces for viewing and manipulating firmware-specific objects and attributes. /sys/kernel: This subdirectory contains various files and subdirectories that provide information about the running kernel. The security variables which are supposed to be exposed via fwsecurityfs are managed by firmware, stored in firmware managed space and also often consumed by firmware for enabling various security features. From git commit b67dbf9d4c1987c370fd18fdc4cf9d8aaea604c2, the purpose of securityfs(/sys/kernel/security) is to provide a common place for all kernel LSMs to use a common place. The idea of fwsecurityfs(/sys/firmware/security) is to similarly provide a common place for all firmware security objects. By having another firmware directory within /sys/kernel/security would mean scattering firmware objects at multiple places and confusing the purpose of /sys/kernel and /sys/firmware. Thanks & Regards, - Nayna
[RFC PATCH v2 3/3] powerpc/pseries: expose authenticated variables stored in LPAR PKS
PowerVM Guest Secure boot feature need to expose firmware managed secure variables for user management. These variables store keys for grub/kernel verification and also corresponding denied list. Expose these variables to the userpace via fwsecurityfs. Example: Example: # cd /sys/firmware/security/secvars # pwd /sys/firmware/security/secvars # cat /tmp/PK.bin > PK # ls -l total 0 -rw-r--r-- 1 root root 2497 Jun 22 08:34 PK # hexdump -C PK 00 00 00 00 00 08 00 00 a1 59 c0 a5 e4 94 a7 4a |.Y.J| 0010 87 b5 ab 15 5c 2b f0 72 3f 03 00 00 00 00 00 00 |\+.r?...| 0020 23 03 00 00 ca 18 1d 1c 01 7d eb 11 9a 71 08 94 |#}...q..| 0030 ef 31 fb e4 30 82 03 0f 30 82 01 f7 a0 03 02 01 |.1..0...0...| 0040 02 02 14 22 ab 18 2f d5 aa dd c5 ba 98 27 60 26 |..."../..'`&| 0050 f1 63 89 54 4c 52 d9 30 0d 06 09 2a 86 48 86 f7 |.c.TLR.0...*.H..| ... Signed-off-by: Nayna Jain --- arch/powerpc/platforms/pseries/Kconfig| 17 ++ arch/powerpc/platforms/pseries/plpks/Makefile | 2 + .../pseries/plpks/fwsecurityfs_arch.c | 16 ++ .../platforms/pseries/plpks/internal.h| 18 ++ .../powerpc/platforms/pseries/plpks/secvars.c | 239 ++ include/linux/fwsecurityfs.h | 4 + 6 files changed, 296 insertions(+) create mode 100644 arch/powerpc/platforms/pseries/plpks/fwsecurityfs_arch.c create mode 100644 arch/powerpc/platforms/pseries/plpks/internal.h create mode 100644 arch/powerpc/platforms/pseries/plpks/secvars.c diff --git a/arch/powerpc/platforms/pseries/Kconfig b/arch/powerpc/platforms/pseries/Kconfig index 6c1ca487103f..9c52095e20c4 100644 --- a/arch/powerpc/platforms/pseries/Kconfig +++ b/arch/powerpc/platforms/pseries/Kconfig @@ -152,6 +152,23 @@ config PSERIES_PLPKS config to enable operating system interface to hypervisor to access this space. +config PSERIES_FWSECURITYFS_ARCH + depends on FWSECURITYFS + bool "Support fwsecurityfs for pseries" + help + Enable fwsecuirtyfs arch specific code. This would initialize + the firmware security filesystem with initial platform specific + structure. + +config PSERIES_PLPKS_SECVARS + depends on PSERIES_PLPKS + select PSERIES_FWSECURITYFS_ARCH + tristate "Support for secvars" + help + This interface exposes authenticated variables stored in the LPAR + Platform KeyStore using fwsecurityfs interface. + If you are unsure how to use it, say N. + config PAPR_SCM depends on PPC_PSERIES && MEMORY_HOTPLUG && LIBNVDIMM tristate "Support for the PAPR Storage Class Memory interface" diff --git a/arch/powerpc/platforms/pseries/plpks/Makefile b/arch/powerpc/platforms/pseries/plpks/Makefile index e651ace920db..ff3d4b4cd3d7 100644 --- a/arch/powerpc/platforms/pseries/plpks/Makefile +++ b/arch/powerpc/platforms/pseries/plpks/Makefile @@ -5,3 +5,5 @@ # obj-$(CONFIG_PSERIES_PLPKS) += plpks.o +obj-$(CONFIG_PSERIES_FWSECURITYFS_ARCH) += fwsecurityfs_arch.o +obj-$(CONFIG_PSERIES_PLPKS_SECVARS) += secvars.o diff --git a/arch/powerpc/platforms/pseries/plpks/fwsecurityfs_arch.c b/arch/powerpc/platforms/pseries/plpks/fwsecurityfs_arch.c new file mode 100644 index ..6ccdfe4000a6 --- /dev/null +++ b/arch/powerpc/platforms/pseries/plpks/fwsecurityfs_arch.c @@ -0,0 +1,16 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * POWER LPAR Platform KeyStore (PLPKS) + * Copyright (C) 2022 IBM Corporation + * Author: Nayna Jain + * + */ + +#include + +#include "internal.h" + +int arch_fwsecurity_init(void) +{ + return plpks_secvars_init(); +} diff --git a/arch/powerpc/platforms/pseries/plpks/internal.h b/arch/powerpc/platforms/pseries/plpks/internal.h new file mode 100644 index ..6061ffd37677 --- /dev/null +++ b/arch/powerpc/platforms/pseries/plpks/internal.h @@ -0,0 +1,18 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (C) 2022 IBM Corporation + * Author: Nayna Jain + * + */ +#ifndef PKS_FWSEC_INTERNAL +#define PKS_FWSEC_INTERNAL + +#ifdef CONFIG_PSERIES_PLPKS_SECVARS +int plpks_secvars_init(void); +#else +int plpks_secvars_init(void) +{ + return 0; +} +#endif +#endif diff --git a/arch/powerpc/platforms/pseries/plpks/secvars.c b/arch/powerpc/platforms/pseries/plpks/secvars.c new file mode 100644 index ..8852cb8f2f3c --- /dev/null +++ b/arch/powerpc/platforms/pseries/plpks/secvars.c @@ -0,0 +1,239 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * POWER LPAR Platform KeyStore (PLPKS) + * Copyright (C) 2022 IBM Corporation + * Author: Nayna Jain + * + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "internal.h" + +static struct dentry *secvar_dir; + +static const char * const names[] = { + "PK", +
[RFC PATCH v2 2/3] fs: define a firmware security filesystem named fwsecurityfs
securityfs is meant for linux security subsystems to expose policies/logs or any other information. However, there are various firmware security features which expose their variables for user management via kernel. There is currently no single place to expose these variables. Different platforms use sysfs/platform specific filesystem(efivarfs)/securityfs interface as find appropriate. Thus, there is a gap in kernel interfaces to expose variables for security features. Define a firmware security filesystem (fwsecurityfs) to be used for exposing variables managed by firmware and to be used by firmware enabled security features. These variables are platform specific. Filesystem provides platforms to implement their own underlying semantics by defining own inode and file operations. Similar to securityfs, the firmware security filesystem is recommended to be exposed on a well known mount point /sys/firmware/security. Platforms can define their own directory or file structure under this path. Example: # mount -t fwsecurityfs fwsecurityfs /sys/firmware/security # cd /sys/firmware/security/ Signed-off-by: Nayna Jain --- fs/Kconfig | 1 + fs/Makefile | 1 + fs/fwsecurityfs/Kconfig | 14 +++ fs/fwsecurityfs/Makefile | 10 +++ fs/fwsecurityfs/inode.c | 159 +++ fs/fwsecurityfs/internal.h | 13 +++ fs/fwsecurityfs/super.c | 154 + include/linux/fwsecurityfs.h | 29 +++ include/uapi/linux/magic.h | 1 + 9 files changed, 382 insertions(+) create mode 100644 fs/fwsecurityfs/Kconfig create mode 100644 fs/fwsecurityfs/Makefile create mode 100644 fs/fwsecurityfs/inode.c create mode 100644 fs/fwsecurityfs/internal.h create mode 100644 fs/fwsecurityfs/super.c create mode 100644 include/linux/fwsecurityfs.h diff --git a/fs/Kconfig b/fs/Kconfig index 5976eb33535f..19ea28143428 100644 --- a/fs/Kconfig +++ b/fs/Kconfig @@ -276,6 +276,7 @@ config ARCH_HAS_GIGANTIC_PAGE source "fs/configfs/Kconfig" source "fs/efivarfs/Kconfig" +source "fs/fwsecurityfs/Kconfig" endmenu diff --git a/fs/Makefile b/fs/Makefile index 208a74e0b00e..5792cd0443cb 100644 --- a/fs/Makefile +++ b/fs/Makefile @@ -137,6 +137,7 @@ obj-$(CONFIG_F2FS_FS) += f2fs/ obj-$(CONFIG_CEPH_FS) += ceph/ obj-$(CONFIG_PSTORE) += pstore/ obj-$(CONFIG_EFIVAR_FS)+= efivarfs/ +obj-$(CONFIG_FWSECURITYFS) += fwsecurityfs/ obj-$(CONFIG_EROFS_FS) += erofs/ obj-$(CONFIG_VBOXSF_FS)+= vboxsf/ obj-$(CONFIG_ZONEFS_FS)+= zonefs/ diff --git a/fs/fwsecurityfs/Kconfig b/fs/fwsecurityfs/Kconfig new file mode 100644 index ..f1665511eeb9 --- /dev/null +++ b/fs/fwsecurityfs/Kconfig @@ -0,0 +1,14 @@ +# SPDX-License-Identifier: GPL-2.0-only +# +# Copyright (C) 2022 IBM Corporation +# Author: Nayna Jain +# + +config FWSECURITYFS + bool "Enable the fwsecurityfs filesystem" + help + This will build the fwsecurityfs file system which is recommended + to be mounted on /sys/firmware/security. This can be used by + platforms to expose their variables which are managed by firmware. + + If you are unsure how to answer this question, answer N. diff --git a/fs/fwsecurityfs/Makefile b/fs/fwsecurityfs/Makefile new file mode 100644 index ..b9931d180178 --- /dev/null +++ b/fs/fwsecurityfs/Makefile @@ -0,0 +1,10 @@ +# SPDX-License-Identifier: GPL-2.0-only +# +# Copyright (C) 2022 IBM Corporation +# Author: Nayna Jain +# +# Makefile for the firmware security filesystem + +obj-$(CONFIG_FWSECURITYFS) += fwsecurityfs.o + +fwsecurityfs-objs := inode.o super.o diff --git a/fs/fwsecurityfs/inode.c b/fs/fwsecurityfs/inode.c new file mode 100644 index ..5d06dc0de059 --- /dev/null +++ b/fs/fwsecurityfs/inode.c @@ -0,0 +1,159 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2022 IBM Corporation + * Author: Nayna Jain + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "internal.h" + +int fwsecurityfs_remove_file(struct dentry *dentry) +{ + drop_nlink(d_inode(dentry)); + dput(dentry); + return 0; +}; +EXPORT_SYMBOL_GPL(fwsecurityfs_remove_file); + +int fwsecurityfs_create_file(const char *name, umode_t mode, + u16 filesize, struct dentry *parent, + struct dentry *dentry, + const struct file_operations *fops) +{ + struct inode *inode; + int error; + struct inode *dir; + + if (!parent) + return -EINVAL; + + dir = d_inode(parent); + pr_debug("securityfs: creating file '%s'\n&qu
[RFC PATCH v2 0/3] powerpc/pseries: add support for local secure storage called Platform KeyStore(PKS)
PowerVM provides an isolated Platform KeyStore(PKS)[1] storage allocation for each partition(LPAR) with individually managed access controls to store sensitive information securely. Linux Kernel can access this storage by interfacing with hypervisor using a new set of hypervisor calls. PowerVM guest secure boot feature intend to use Platform KeyStore for the purpose of storing public keys. Secure boot requires public keys to be able to verify the grub and boot kernel. To allow authenticated manipulation of keys, it supports variables to store key authorities - PK/KEK. Other variables are used to store code signing keys - db/grubdb. It also supports denied list to disallow booting even if signed with valid key. This is done via denied list database - dbx or sbat. These variables would be stored in PKS, and are managed and controlled by firmware. The purpose of this patchset is to add userspace interface to manage these variables. For v1[2] version, we received following feedback "Ok, this is like the 3rd or 4th different platform-specific proposal for this type of functionality. I think we need to give up on platform-specific user/kernel apis on this (random sysfs/securityfs files scattered around the tree), and come up with a standard place for all of this." Currently, OpenPOWER exposes variables via sysfs, while EFI platforms have used sysfs and then moved to their own efivarfs filesystem. Recently, coco feature is using securityfs to expose their secrets. All of these environments are different both syntactically and semantically. securityfs is meant for linux security subsystems to expose policies/logs or any other information, and do not interact with firmware for managing these variables. However, there are various firmware security features which expose their variables for user management via kernel as discussed above. There is currently no single place to expose these variables. Different platforms use sysfs/platform specific filesystem(efivarfs)/securityfs interface as find appropriate. This has resulted in interfaces scattered around the tree. This resulted in demand of a need for a common single place for new platform interfaces to expose their variables for firmware security features. This would simplify the interface for users of these platforms. This patchset proposes firmware security filesystem(fwsecurityfs). Any platform can expose the variables which are required by firmware security features via this interface. Going forward, this would give a common place for exposing variables managed by firmware while still allowing platforms to implement their own underlying semantics. This design consists of two parts: 1. firmware security filesystem(fwsecurityfs) that provides platforms with APIs to create their own underlying directory and file structure. It is recommended to establish a well known mount point: i.e. /sys/firmware/security/ 2. platform specific implementation for these variables which implements underlying semantics. Platforms can expose their variables as files allowing read/write/add/delete operations by defining their own inode and file operations. This patchset defines: 1. pseries driver to access LPAR Platform Key Store(PLPKS) 2. firmware security filesystem named fwsecurityfs 3. Interface to expose secure variables stored in LPAR PKS via fwsecurityfs [1] https://community.ibm.com/community/user/power/blogs/chris-engel1/2020/11/20/powervm-introduces-the-platform-keystore [2] https://lore.kernel.org/linuxppc-dev/20220122005637.28199-1-na...@linux.ibm.com/ Changelog: v1: * Defined unified interface(firmware security filesystem) for all platforms to expose their variables used for security features. * Expose secvars using firmware security fileystem. * Renamed PKS driver to PLPKS to avoid naming conflict as mentioned by Dave Hanson. Nayna Jain (3): powerpc/pseries: define driver for Platform KeyStore fs: define a firmware security filesystem named fwsecurityfs powerpc/pseries: expose authenticated variables stored in LPAR PKS arch/powerpc/include/asm/hvcall.h | 12 +- arch/powerpc/include/asm/plpks.h | 92 arch/powerpc/platforms/pseries/Kconfig| 27 + arch/powerpc/platforms/pseries/Makefile | 2 + arch/powerpc/platforms/pseries/plpks/Makefile | 9 + .../pseries/plpks/fwsecurityfs_arch.c | 16 + .../platforms/pseries/plpks/internal.h| 18 + arch/powerpc/platforms/pseries/plpks/plpks.c | 517 ++ .../powerpc/platforms/pseries/plpks/secvars.c | 239 fs/Kconfig| 1 + fs/Makefile | 1 + fs/fwsecurityfs/Kconfig | 14 + fs/fwsecurityfs/Makefile | 10 + fs/fwsecurityfs/inode.c | 159 ++ fs/fwsecurityfs/internal.h| 13 + fs/fwsecurityfs/super.c | 154 ++ inc
[RFC PATCH v2 1/3] powerpc/pseries: define driver for Platform KeyStore
PowerVM provides an isolated Platform Keystore(PKS) storage allocation for each LPAR with individually managed access controls to store sensitive information securely. It provides a new set of hypervisor calls for Linux kernel to access PKS storage. Define PLPKS driver using H_CALL interface to access PKS storage. Signed-off-by: Nayna Jain --- arch/powerpc/include/asm/hvcall.h | 12 +- arch/powerpc/include/asm/plpks.h | 92 arch/powerpc/platforms/pseries/Kconfig| 10 + arch/powerpc/platforms/pseries/Makefile | 2 + arch/powerpc/platforms/pseries/plpks/Makefile | 7 + arch/powerpc/platforms/pseries/plpks/plpks.c | 517 ++ 6 files changed, 639 insertions(+), 1 deletion(-) create mode 100644 arch/powerpc/include/asm/plpks.h create mode 100644 arch/powerpc/platforms/pseries/plpks/Makefile create mode 100644 arch/powerpc/platforms/pseries/plpks/plpks.c diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h index d92a20a85395..1da429235632 100644 --- a/arch/powerpc/include/asm/hvcall.h +++ b/arch/powerpc/include/asm/hvcall.h @@ -97,6 +97,7 @@ #define H_OP_MODE -73 #define H_COP_HW -74 #define H_STATE-75 +#define H_IN_USE -77 #define H_UNSUPPORTED_FLAG_START -256 #define H_UNSUPPORTED_FLAG_END -511 #define H_MULTI_THREADS_ACTIVE -9005 @@ -321,10 +322,19 @@ #define H_SCM_UNBIND_ALL0x3FC #define H_SCM_HEALTH0x400 #define H_SCM_PERFORMANCE_STATS 0x418 +#define H_PKS_GET_CONFIG 0x41C +#define H_PKS_SET_PASSWORD 0x420 +#define H_PKS_GEN_PASSWORD 0x424 +#define H_PKS_WRITE_OBJECT 0x42C +#define H_PKS_GEN_KEY 0x430 +#define H_PKS_READ_OBJECT 0x434 +#define H_PKS_REMOVE_OBJECT0x438 +#define H_PKS_CONFIRM_OBJECT_FLUSHED 0x43C #define H_RPT_INVALIDATE 0x448 #define H_SCM_FLUSH0x44C #define H_GET_ENERGY_SCALE_INFO0x450 -#define MAX_HCALL_OPCODE H_GET_ENERGY_SCALE_INFO +#define H_PKS_SIGNED_UPDATE0x454 +#define MAX_HCALL_OPCODE H_PKS_SIGNED_UPDATE /* Scope args for H_SCM_UNBIND_ALL */ #define H_UNBIND_SCOPE_ALL (0x1) diff --git a/arch/powerpc/include/asm/plpks.h b/arch/powerpc/include/asm/plpks.h new file mode 100644 index ..c3b544bdbcb6 --- /dev/null +++ b/arch/powerpc/include/asm/plpks.h @@ -0,0 +1,92 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (C) 2022 IBM Corporation + * Author: Nayna Jain + * + * Platform keystore for pseries LPAR(PLPKS). + */ + +#ifndef _PSERIES_PLPKS_H +#define _PSERIES_PLPKS_H + + +#include +#include + + +#define OSSECBOOTAUDIT 0x4000 +#define OSSECBOOTENFORCE 0x2000 +#define WORLDREADABLE 0x0800 +#define SIGNEDUPDATE 0x0100 + +struct plpks_var { + char *component; + u8 *name; + u16 namelen; + u32 policy; + u16 datalen; + u8 *data; +}; + +struct plpks_var_name { + u16 namelen; + u8 *name; +}; + +struct plpks_var_name_list { + u32 varcount; + struct plpks_var_name varlist[]; +}; + +struct plpks_config { + u8 version; + u8 flags; + u32 rsvd0; + u16 maxpwsize; + u16 maxobjlabelsize; + u16 maxobjsize; + u32 totalsize; + u32 usedspace; + u32 supportedpolicies; + u64 rsvd1; +} __packed; + +/** + * Successful return from this API implies PKS is available. + * This is used to initialize kernel driver and user interfaces. + */ +extern struct plpks_config *plpks_get_config(void); + +/** + * Updates the authenticated variable. It expects NULL as the component. + */ +extern int plpks_signed_update_var(struct plpks_var var); + +/** + * Writes the specified var and its data to PKS. + * Any caller of PKS driver should present a valid component type for + * their variable. + */ +extern int plpks_write_var(struct plpks_var var); + +/** + * Removes the specified var and its data from PKS. + */ +extern int plpks_remove_var(char *component, struct plpks_var_name vname); + +/** + * Returns the data for the specified os variable. + */ +extern int plpks_read_os_var(struct plpks_var *var); + +/** + * Returns the data for the specified firmware variable. + */ +extern int plpks_read_fw_var(struct plpks_var *var); + +/** + * Returns the data for the specified bootloader variable. + */ +extern int plpks_read_bootloader_var(struct plpks_var *var); + +#endif diff --git a/arch/powerpc/platforms/pseries/Kconfig b/arch/powerpc/platforms/pseries/Kconfig index f7fd91d153a4..6c1ca487103f 100644 --- a/arch/powerpc/platforms/pseries/Kconfig +++ b/arch/powerpc/platforms/pseries/Kconfig @@ -142,6 +142,16 @@ config IBMEBUS help Bus device driver for GX bus based adapters. +config PSERIES_PLPKS + depends on PPC_PSERIES + tristate "Support for the Platform Key Storage" + help + PowerVM provides an isolated Platform Keystore(PKS) storage + allocatio
Re: [PATCH v7 0/5] Allow guest access to EFI confidential computing secret area
s://lore.kernel.org/all/20220122005637.28199-1-na...@linux.ibm.com/ Thanks & Regards, - Nayna
Re: [PATCH v7 0/5] Allow guest access to EFI confidential computing secret area
ave its own platform specific interface. I would be happy to hear the ideas. [1] https://lore.kernel.org/linux-efi/yrzuiivizmfgj...@google.com/ [2] https://lore.kernel.org/all/20220122005637.28199-1-na...@linux.ibm.com/ Thanks & Regards, - Nayna
[RFC PATCH 2/2] pseries: define sysfs interface to expose PKS variables
PowerVM guest secure boot intend to use Platform Keystore(PKS) for the purpose of storing public keys to verify digital signature. Define sysfs interface to expose PKS variables to userspace to allow read/write/add/delete operations. Each variable is shown as a read/write attribute file. The size of the file represents the size of the current content of the variable. create_var and delete_var attribute files are always present which allow users to create/delete variables. These are write only attributes.The design has tried to be compliant with sysfs semantic to represent single value per attribute. Thus, rather than mapping a complete data structure representation to create_var, it only accepts a single formatted string to create an empty variable. The sysfs interface is designed such as to expose PKS configuration properties, operating system variables and firmware variables. Current version exposes configuration and operating system variables. The support for exposing firmware variables will be added in the future version. Example of pksvar sysfs interface: # cd /sys/firmware/pksvar/ # ls config os # cd config # ls -ltrh total 0 -r--r--r-- 1 root root 64K Jan 21 17:55 version -r--r--r-- 1 root root 64K Jan 21 17:55 used_space -r--r--r-- 1 root root 64K Jan 21 17:55 total_size -r--r--r-- 1 root root 64K Jan 21 17:55 supported_policies -r--r--r-- 1 root root 64K Jan 21 17:55 max_object_size -r--r--r-- 1 root root 64K Jan 21 17:55 max_object_label_size -r--r--r-- 1 root root 64K Jan 21 17:55 flags # cd os # ls -ltrh total 0 -rw--- 1 root root 104 Jan 21 17:56 var4 -rw--- 1 root root 104 Jan 21 17:56 var3 -rw--- 1 root root 831 Jan 21 17:56 GLOBAL_PK -rw--- 1 root root 831 Jan 21 17:56 GLOBAL_KEK -rw--- 1 root root 76 Jan 21 17:56 GLOBAL_dbx -rw--- 1 root root 831 Jan 21 17:56 GLOBAL_db --w--- 1 root root 64K Jan 21 17:56 delete_var --w--- 1 root root 64K Jan 21 17:56 create_var 1. Read variable # hexdump -C GLOBAL_db 00 00 00 00 a1 59 c0 a5 e4 94 a7 4a 87 b5 ab 15 |.Y.J| 0010 5c 2b f0 72 3f 03 00 00 00 00 00 00 23 03 00 00 |\+.r?...#...| 0330 02 a8 e8 ed 0f 20 60 3f 40 04 7c a8 91 21 37 eb |. `?@.|..!7.| 0340 f3 f1 4e |..N| 0343 2. Write variable cat /tmp/data.bin > 3. Create variable # echo "var1" > create_var # ls -ltrh total 0 -rw--- 1 root root 104 Jan 21 17:56 var4 -rw--- 1 root root 104 Jan 21 17:56 var3 -rw--- 1 root root 831 Jan 21 17:56 GLOBAL_PK -rw--- 1 root root 831 Jan 21 17:56 GLOBAL_KEK -rw--- 1 root root 76 Jan 21 17:56 GLOBAL_dbx -rw--- 1 root root 831 Jan 21 17:56 GLOBAL_db --w--- 1 root root 64K Jan 21 17:56 delete_var --w--- 1 root root 64K Jan 21 17:57 create_var -rw--- 1 root root 0 Jan 21 17:57 var1.tmp Current design creates a zero size temporary variable. This implies it is not yet persisted to PKS. Only once data is written to newly created temporary variable and if it is successfully stored in the PKS, that the variable is permanent. The temporary variable will get removed on reboot. The code currently doesn't remove .tmp suffix immediately when persisted. The future version will fix this. To avoid the additional .tmp semantic, alternative option is to consider any zero size variable as temporary variable. This option is under evaluation. This would avoid any runtime sysfs magic to replace .tmp variable with real variable. Also, the formatted string to pass to create_var will have following format in the future version: : 4. Delete variable # echo "var3" > delete_var # ls -ltrh total 0 -rw--- 1 root root 104 Jan 21 17:56 var4 -rw--- 1 root root 831 Jan 21 17:56 GLOBAL_PK -rw--- 1 root root 831 Jan 21 17:56 GLOBAL_KEK -rw--- 1 root root 76 Jan 21 17:56 GLOBAL_dbx -rw--- 1 root root 831 Jan 21 17:56 GLOBAL_db --w--- 1 root root 64K Jan 21 17:57 create_var -rw--- 1 root root 0 Jan 21 17:57 var1.tmp --w--- 1 root root 64K Jan 21 17:58 delete_var The var3 file is removed at runtime, if variable is successfully removed from the PKS storage. NOTE: We are evaluating two design for userspace interface: using the sysfs or defining a new filesystem based. Any feedback on this sysfs based approach would be highly appreciated. We have tried to follow one value per attribute semantic. If that or any other semantics aren't followed properly, please let us know. Signed-off-by: Nayna Jain --- Documentation/ABI/testing/sysfs-pksvar| 77 arch/powerpc/platforms/pseries/Kconfig| 7 + arch/powerpc/platforms/pseries/Makefile | 1 + arch/powerpc/platforms/pseries/pksvar-sysfs.c | 356 ++ 4 files changed, 441 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-pksvar create mode 100644 arch/powerpc/platforms/pseries/pksvar-sysfs.c diff --git a/Documentation/ABI/testing/sy
[RFC PATCH 1/2] pseries: define driver for Platform Keystore
PowerVM provides an isolated Platform Keystore(PKS) storage allocation for each partition with individually managed access controls to store sensitive information securely. It provides a new set of hypervisor calls for Linux kernel to access PKS storage. Define PKS driver using H_CALL interface to access PKS storage. Signed-off-by: Nayna Jain --- arch/powerpc/include/asm/hvcall.h | 13 +- arch/powerpc/include/asm/pks.h | 84 arch/powerpc/platforms/pseries/Kconfig | 10 + arch/powerpc/platforms/pseries/Makefile | 1 + arch/powerpc/platforms/pseries/pks.c| 494 5 files changed, 601 insertions(+), 1 deletion(-) create mode 100644 arch/powerpc/include/asm/pks.h create mode 100644 arch/powerpc/platforms/pseries/pks.c diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h index 9bcf345cb208..08108dcf8677 100644 --- a/arch/powerpc/include/asm/hvcall.h +++ b/arch/powerpc/include/asm/hvcall.h @@ -97,6 +97,7 @@ #define H_OP_MODE -73 #define H_COP_HW -74 #define H_STATE-75 +#define H_IN_USE -77 #define H_UNSUPPORTED_FLAG_START -256 #define H_UNSUPPORTED_FLAG_END -511 #define H_MULTI_THREADS_ACTIVE -9005 @@ -321,9 +322,19 @@ #define H_SCM_UNBIND_ALL0x3FC #define H_SCM_HEALTH0x400 #define H_SCM_PERFORMANCE_STATS 0x418 +#define H_PKS_GET_CONFIG 0x41C +#define H_PKS_SET_PASSWORD 0x420 +#define H_PKS_GEN_PASSWORD 0x424 +#define H_PKS_GET_OBJECT_LABELS 0x428 +#define H_PKS_WRITE_OBJECT 0x42C +#define H_PKS_GEN_KEY 0x430 +#define H_PKS_READ_OBJECT 0x434 +#define H_PKS_REMOVE_OBJECT0x438 +#define H_PKS_CONFIRM_OBJECT_FLUSHED 0x43C #define H_RPT_INVALIDATE 0x448 #define H_SCM_FLUSH0x44C -#define MAX_HCALL_OPCODE H_SCM_FLUSH +#define H_PKS_SB_SIGNED_UPDATE 0x454 +#define MAX_HCALL_OPCODE H_PKS_SB_SIGNED_UPDATE /* Scope args for H_SCM_UNBIND_ALL */ #define H_UNBIND_SCOPE_ALL (0x1) diff --git a/arch/powerpc/include/asm/pks.h b/arch/powerpc/include/asm/pks.h new file mode 100644 index ..ef6f541d75d3 --- /dev/null +++ b/arch/powerpc/include/asm/pks.h @@ -0,0 +1,84 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (C) 2022 IBM Corporation + * Author: Nayna Jain + * + * Platform keystore for pseries. + */ +#ifndef _PSERIES_PKS_H +#define _PSERIES_PKS_H + + +#include +#include + +struct pks_var { + char *prefix; + u8 *name; + u16 namelen; + u32 policy; + u16 datalen; + u8 *data; +}; + +struct pks_var_name { + u16 namelen; + u8 *name; +}; + +struct pks_var_name_list { + u32 varcount; + struct pks_var_name *varlist; +}; + +struct pks_config { + u8 version; + u8 flags; + u32 rsvd0; + u16 maxpwsize; + u16 maxobjlabelsize; + u16 maxobjsize; + u32 totalsize; + u32 usedspace; + u32 supportedpolicies; + u64 rsvd1; +} __packed; + +/** + * Successful return from this API implies PKS is available. + * This is used to initialize kernel driver and user interfaces. + */ +extern struct pks_config *pks_get_config(void); + +/** + * Returns all the var names for this prefix. + * This only returns name list. If the caller needs data, it has to specifically + * call read for the required var name. + */ +int pks_get_var_ids_for_type(char *prefix, struct pks_var_name_list *list); + +/** + * Writes the specified var and its data to PKS. + * Any caller of PKS driver should present a valid prefix type for their + * variable. This is an exception only for signed variables exposed via + * sysfs which do not have any prefixes. + * The prefix should always start with '/'. For eg. '/sysfs'. + */ +extern int pks_write_var(struct pks_var var); + +/** + * Writes the specified signed var and its data to PKS. + */ +extern int pks_update_signed_var(struct pks_var var); + +/** + * Removes the specified var and its data from PKS. + */ +extern int pks_remove_var(char *prefix, struct pks_var_name vname); + +/** + * Returns the data for the specified variable. + */ +extern int pks_read_var(struct pks_var *var); + +#endif diff --git a/arch/powerpc/platforms/pseries/Kconfig b/arch/powerpc/platforms/pseries/Kconfig index 2e57391e0778..32d0df84e611 100644 --- a/arch/powerpc/platforms/pseries/Kconfig +++ b/arch/powerpc/platforms/pseries/Kconfig @@ -147,6 +147,16 @@ config IBMEBUS help Bus device driver for GX bus based adapters. +config PSERIES_PKS + depends on PPC_PSERIES + tristate "Support for the Platform Key Storage" + help + PowerVM provides an isolated Platform Keystore(PKS) storage + allocation for each partition with individually managed + access controls to store sensitive information securely. Select + this config to enable operating system interface to hypervisor to + access
[RFC PATCH 0/2] powerpc/pseries: add support for local secure storage called Platform Keystore(PKS)
PowerVM provides an isolated Platform Keystore(PKS) storage allocation for each partition with individually managed access controls to store sensitive information securely. Linux Kernel can access this storage by interfacing with hypervisor using a new set of hypervisor calls. PowerVM guest secure boot intend to use Platform Keystore for the purpose of storing public keys. Secure boot requires public keys to be able to verify the grub and boot kernel. To allow authenticated manipulation of keys, it supports variables to store key authorities - PK/KEK and code signing keys - db. It also supports denied list to disallow booting even if signed with valid key. This is done via denied list database - dbx or sbat. These variables would be stored in PKS, and are managed and controlled by firmware. The purpose of this patchset is to add support for users to read/write/add/delete variables required for secure boot on PowerVM. Nayna Jain (2): pseries: define driver for Platform Keystore pseries: define sysfs interface to expose PKS variables Documentation/ABI/testing/sysfs-pksvar| 77 +++ arch/powerpc/include/asm/hvcall.h | 13 +- arch/powerpc/include/asm/pks.h| 84 +++ arch/powerpc/platforms/pseries/Kconfig| 17 + arch/powerpc/platforms/pseries/Makefile | 2 + arch/powerpc/platforms/pseries/pks.c | 494 ++ arch/powerpc/platforms/pseries/pksvar-sysfs.c | 356 + 7 files changed, 1042 insertions(+), 1 deletion(-) create mode 100644 Documentation/ABI/testing/sysfs-pksvar create mode 100644 arch/powerpc/include/asm/pks.h create mode 100644 arch/powerpc/platforms/pseries/pks.c create mode 100644 arch/powerpc/platforms/pseries/pksvar-sysfs.c -- 2.27.0
Re: [PATCH v2 2/6] powerpc/kexec_file: Add KEXEC_SIG support.
On 11/25/21 13:02, Michal Suchanek wrote: Copy the code from s390x Signed-off-by: Michal Suchanek --- arch/powerpc/Kconfig| 11 +++ arch/powerpc/kexec/elf_64.c | 36 2 files changed, 47 insertions(+) diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index ac0c515552fd..ecc1227a77f1 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -561,6 +561,17 @@ config KEXEC_FILE config ARCH_HAS_KEXEC_PURGATORY def_bool KEXEC_FILE +config KEXEC_SIG + bool "Verify kernel signature during kexec_file_load() syscall" + depends on KEXEC_FILE && MODULE_SIG_FORMAT + help + This option makes kernel signature verification mandatory for + the kexec_file_load() syscall. + Resending my last response as looks like it didn't go through mailing list because of some wrong formatting. My apologies to those who are receiving it twice. Since powerpc also supports IMA_ARCH_POLICY for kernel image signature verification, please include the following: "An alternative implementation for the powerpc arch is IMA_ARCH_POLICY. It verifies the appended kernel image signature and additionally includes both the signed and unsigned file hashes in the IMA measurement list, extends the IMA PCR in the TPM, and prevents blacklisted binary kernel images from being kexec'd." Thanks & Regards, - Nayna
Re: [PATCH v2 2/6] powerpc/kexec_file: Add KEXEC_SIG support.
On 11/25/21 13:02, Michal Suchanek wrote: Copy the code from s390x Signed-off-by: Michal Suchanek --- arch/powerpc/Kconfig| 11 +++ arch/powerpc/kexec/elf_64.c | 36 2 files changed, 47 insertions(+) diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index ac0c515552fd..ecc1227a77f1 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -561,6 +561,17 @@ config KEXEC_FILE config ARCH_HAS_KEXEC_PURGATORY def_bool KEXEC_FILE +config KEXEC_SIG + bool "Verify kernel signature during kexec_file_load() syscall" + depends on KEXEC_FILE && MODULE_SIG_FORMAT + help + This option makes kernel signature verification mandatory for + the kexec_file_load() syscall. + Since powerpc also supports IMA_ARCH_POLICY for kernel image signature verification, please include the following: "An alternative implementation for the powerpc arch is IMA_ARCH_POLICY. It verifies the appended kernel image signature and additionally includes both the signed and unsigned file hashes in the IMA measurement list, extends the IMA PCR in the TPM, and prevents blacklisted binary kernel images from being kexec'd" Thanks & Regards, - Nayna
Re: [PATCH v2 2/6] powerpc/kexec_file: Add KEXEC_SIG support.
On 12/9/21 04:21, Michal Suchánek wrote: Hello, Hi, On Wed, Dec 08, 2021 at 08:51:47PM -0500, Nayna wrote: On 11/25/21 13:02, Michal Suchanek wrote: Copy the code from s390x Signed-off-by: Michal Suchanek --- arch/powerpc/Kconfig| 11 +++ arch/powerpc/kexec/elf_64.c | 36 2 files changed, 47 insertions(+) diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index ac0c515552fd..ecc1227a77f1 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -561,6 +561,17 @@ config KEXEC_FILE config ARCH_HAS_KEXEC_PURGATORY def_bool KEXEC_FILE +config KEXEC_SIG + bool "Verify kernel signature during kexec_file_load() syscall" + depends on KEXEC_FILE && MODULE_SIG_FORMAT After manually applying the patch, the build is failing with the following error: build failed with error "arch/powerpc/kexec/elf_64.o: In function `elf64_verify_sig': /root/kernel/linus/linux/arch/powerpc/kexec/elf_64.c:160: undefined reference to `verify_appended_signature'" This patch does not add call to verify_appended_signature. Maybe you applied the following patch as well? Yes, I tried build after applying all the patches. Thanks & Regards, - Nayna
Re: [PATCH v2 2/6] powerpc/kexec_file: Add KEXEC_SIG support.
On 11/25/21 13:02, Michal Suchanek wrote: Copy the code from s390x Signed-off-by: Michal Suchanek --- arch/powerpc/Kconfig| 11 +++ arch/powerpc/kexec/elf_64.c | 36 2 files changed, 47 insertions(+) diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index ac0c515552fd..ecc1227a77f1 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -561,6 +561,17 @@ config KEXEC_FILE config ARCH_HAS_KEXEC_PURGATORY def_bool KEXEC_FILE +config KEXEC_SIG + bool "Verify kernel signature during kexec_file_load() syscall" + depends on KEXEC_FILE && MODULE_SIG_FORMAT After manually applying the patch, the build is failing with the following error: build failed with error "arch/powerpc/kexec/elf_64.o: In function `elf64_verify_sig': /root/kernel/linus/linux/arch/powerpc/kexec/elf_64.c:160: undefined reference to `verify_appended_signature'" I see it happened because I didn't have MODULE_SIG enabled. Enabling MODULE_SIG fixes it. I wonder why not to add "depends on MODULE_SIG" rather than on MODULE_SIG_FORMAT. Thanks & Regards, - Nayna
Re: [PATCH v2 0/6] KEXEC_SIG with appended signature
On 11/25/21 13:02, Michal Suchanek wrote: Hello, Hi Michael, This is resend of the KEXEC_SIG patchset. The first patch is new because it'a a cleanup that does not require any change to the module verification code. The second patch is the only one that is intended to change any functionality. The rest only deduplicates code but I did not receive any review on that part so I don't know if it's desirable as implemented. The first two patches can be applied separately without the rest. Patch 2 fails to apply on v5.16-rc4. Can you please also include git tree/branch while posting the patches ? Secondly, I see that you add the powerpc support in Patch 2 and then modify it again in Patch 5 after cleanup. Why not add the support for powerpc after the clean up ? This will reduce some rework and also probably simplify patches. Thanks & Regards, - Nayna
Re: [PATCH 0/3] KEXEC_SIG with appended signature
On 11/16/21 04:53, Michal Suchánek wrote: On Mon, Nov 15, 2021 at 06:53:53PM -0500, Nayna wrote: On 11/12/21 03:30, Michal Suchánek wrote: Hello, On Thu, Nov 11, 2021 at 05:26:41PM -0500, Nayna wrote: On 11/8/21 07:05, Michal Suchánek wrote: Hello, The other part is that distributions apply 'lockdown' patches that change the security policy depending on secure boot status which were rejected by upstream which only hook into the _SIG options, and not into the IMA_ options. Of course, I expect this to change when the IMA options are universally available across architectures and the support picked up by distributions. Which brings the third point: IMA features vary across architectures, and KEXEC_SIG is more common than IMA_KEXEC. config/arm64/default:CONFIG_HAVE_IMA_KEXEC=y config/ppc64le/default:CONFIG_HAVE_IMA_KEXEC=y config/arm64/default:CONFIG_KEXEC_SIG=y config/s390x/default:CONFIG_KEXEC_SIG=y config/x86_64/default:CONFIG_KEXEC_SIG=y KEXEC_SIG makes it much easier to get uniform features across architectures. Architectures use KEXEC_SIG vs IMA_KEXEC based on their requirement. IMA_KEXEC is for the kernel images signed using sign-file (appended signatures, not PECOFF), provides measurement along with verification, and That's certainly not the case. S390 uses appended signatures with KEXEC_SIG, arm64 uses PECOFF with both KEXEC_SIG and IMA_KEXEC. Yes, S390 uses appended signature, but they also do not support measurements. On the other hand for arm64/x86, PECOFF works only with KEXEC_SIG. Look at the KEXEC_IMAGE_VERIFY_SIG config dependencies in arch/arm64/Kconfig and KEXEC_BZIMAGE_VERIFY_SIG config dependencies in arch/x86/Kconfig. Now, if KEXEC_SIG is not enabled, then IMA appraisal policies are enforced if secure boot is enabled, refer to security/integrity/ima_efi.c . IMA would fail verification if kernel is not signed with module sig appended signatures or signature verification fails. In short, IMA is used to enforce the existence of a policy if secure boot is enabled. If they don't support module sig appended signatures, by definition it fails. Thus PECOFF doesn't work with both KEXEC_SIG and IMA_KEXEC, but only with KEXEC_SIG. Then IMA_KEXEC is a no-go. It is not supported on all architectures and it principially cannot be supported because it does not support PECOFF which is needed to boot the kernel on EFI platforms. To get feature parity across architectures KEXEC_SIG is required. I would not say "a no-go", it is based on user requirements. The key takeaway from this discussion is that both KEXEC_SIG and IMA_KEXEC support functionality with some small degree of overlap, and that documenting the differences is needed. This will help kernel consumers to understand the difference and enable the appropriate functionality for their environment. As per my understanding: KEXEC_SIG: * Supports kernel image verification * Linked with secureboot state using downstream patch * Supports PECOFF and module sig appended signature format * Supports blocklisting of keys IMA_KEXEC: * Supports kernel image verification * Linked with secureboot state in upstream * Supports module sig appended signature format and signatures in extended attribute. * Supports blocklisting of keys * Supports blocklisting single kernel binary * Supports measurements for attestation * Supports audit log Users can enable the option based on their requirements. Thanks for the good discussion and enabling KEXEC_SIG for POWER as well. It would be good to have updated kernel documentation to go along with KEXEC_SIG support in the patchset. Thanks & Regards, - Nayna
Re: [PATCH 0/3] KEXEC_SIG with appended signature
On 11/12/21 03:30, Michal Suchánek wrote: Hello, On Thu, Nov 11, 2021 at 05:26:41PM -0500, Nayna wrote: On 11/8/21 07:05, Michal Suchánek wrote: Hello, On Mon, Nov 08, 2021 at 09:18:56AM +1100, Daniel Axtens wrote: Michal Suchánek writes: On Fri, Nov 05, 2021 at 09:55:52PM +1100, Daniel Axtens wrote: Michal Suchanek writes: S390 uses appended signature for kernel but implements the check separately from module loader. Support for secure boot on powerpc with appended signature is planned - grub patches submitted upstream but not yet merged. Power Non-Virtualised / OpenPower already supports secure boot via kexec with signature verification via IMA. I think you have now sent a follow-up series that merges some of the IMA implementation, I just wanted to make sure it was clear that we actually already have support So is IMA_KEXEC and KEXEC_SIG redundant? I see some architectures have both. I also see there is a lot of overlap between the IMA framework and the KEXEC_SIG and MODULE_SIg. Mimi would be much better placed than me to answer this. The limits of my knowledge are basically that signature verification for modules and kexec kernels can be enforced by IMA policies. For example a secure booted powerpc kernel with module support will have the following IMA policy set at the arch level: "appraise func=KEXEC_KERNEL_CHECK appraise_flag=check_blacklist appraise_type=imasig|modsig", (in arch/powerpc/kernel/ima_arch.c) Module signature enforcement can be set with either IMA (policy like "appraise func=MODULE_CHECK appraise_flag=check_blacklist appraise_type=imasig|modsig" ) or with CONFIG_MODULE_SIG_FORCE/module.sig_enforce=1. Sometimes this leads to arguably unexpected interactions - for example commit fa4f3f56ccd2 ("powerpc/ima: Fix secure boot rules in ima arch policy"), so it might be interesting to see if we can make things easier to understand. I suspect that is the root of the problem here. Until distributions pick up IMA and properly document step by step in detail how to implement, enable, and debug it the _SIG options are required for users to be able to make use of signatures. For secureboot, IMA appraisal policies are configured in kernel at boot time based on secureboot state of the system, refer arch/powerpc/kernel/ima_arch.c and security/integrity/ima/ima_efi.c. This doesn't require any user configuration. Yes, I agree it would be helpful to update kernel documentation specifying steps to sign the kernel image using sign-file. The other part is that distributions apply 'lockdown' patches that change the security policy depending on secure boot status which were rejected by upstream which only hook into the _SIG options, and not into the IMA_ options. Of course, I expect this to change when the IMA options are universally available across architectures and the support picked up by distributions. Which brings the third point: IMA features vary across architectures, and KEXEC_SIG is more common than IMA_KEXEC. config/arm64/default:CONFIG_HAVE_IMA_KEXEC=y config/ppc64le/default:CONFIG_HAVE_IMA_KEXEC=y config/arm64/default:CONFIG_KEXEC_SIG=y config/s390x/default:CONFIG_KEXEC_SIG=y config/x86_64/default:CONFIG_KEXEC_SIG=y KEXEC_SIG makes it much easier to get uniform features across architectures. Architectures use KEXEC_SIG vs IMA_KEXEC based on their requirement. IMA_KEXEC is for the kernel images signed using sign-file (appended signatures, not PECOFF), provides measurement along with verification, and That's certainly not the case. S390 uses appended signatures with KEXEC_SIG, arm64 uses PECOFF with both KEXEC_SIG and IMA_KEXEC. Yes, S390 uses appended signature, but they also do not support measurements. On the other hand for arm64/x86, PECOFF works only with KEXEC_SIG. Look at the KEXEC_IMAGE_VERIFY_SIG config dependencies in arch/arm64/Kconfig and KEXEC_BZIMAGE_VERIFY_SIG config dependencies in arch/x86/Kconfig. Now, if KEXEC_SIG is not enabled, then IMA appraisal policies are enforced if secure boot is enabled, refer to security/integrity/ima_efi.c . IMA would fail verification if kernel is not signed with module sig appended signatures or signature verification fails. In short, IMA is used to enforce the existence of a policy if secure boot is enabled. If they don't support module sig appended signatures, by definition it fails. Thus PECOFF doesn't work with both KEXEC_SIG and IMA_KEXEC, but only with KEXEC_SIG. Lakshmi, do you agree with my reasoning ? is tied to secureboot state of the system at boot time. In distrubutions it's also the case with KEXEC_SIG, it's only upstream where this is different. I don't know why Linux upstream has rejected this support for KEXEC_SIG. Anyway, sounds like the difference is that IMA provides measurement but if you don't use it it does not makes any difference except more comlex code. I am unsure what do you m
Re: [PATCH 0/3] KEXEC_SIG with appended signature
On 11/8/21 07:05, Michal Suchánek wrote: Hello, On Mon, Nov 08, 2021 at 09:18:56AM +1100, Daniel Axtens wrote: Michal Suchánek writes: On Fri, Nov 05, 2021 at 09:55:52PM +1100, Daniel Axtens wrote: Michal Suchanek writes: S390 uses appended signature for kernel but implements the check separately from module loader. Support for secure boot on powerpc with appended signature is planned - grub patches submitted upstream but not yet merged. Power Non-Virtualised / OpenPower already supports secure boot via kexec with signature verification via IMA. I think you have now sent a follow-up series that merges some of the IMA implementation, I just wanted to make sure it was clear that we actually already have support So is IMA_KEXEC and KEXEC_SIG redundant? I see some architectures have both. I also see there is a lot of overlap between the IMA framework and the KEXEC_SIG and MODULE_SIg. Mimi would be much better placed than me to answer this. The limits of my knowledge are basically that signature verification for modules and kexec kernels can be enforced by IMA policies. For example a secure booted powerpc kernel with module support will have the following IMA policy set at the arch level: "appraise func=KEXEC_KERNEL_CHECK appraise_flag=check_blacklist appraise_type=imasig|modsig", (in arch/powerpc/kernel/ima_arch.c) Module signature enforcement can be set with either IMA (policy like "appraise func=MODULE_CHECK appraise_flag=check_blacklist appraise_type=imasig|modsig" ) or with CONFIG_MODULE_SIG_FORCE/module.sig_enforce=1. Sometimes this leads to arguably unexpected interactions - for example commit fa4f3f56ccd2 ("powerpc/ima: Fix secure boot rules in ima arch policy"), so it might be interesting to see if we can make things easier to understand. I suspect that is the root of the problem here. Until distributions pick up IMA and properly document step by step in detail how to implement, enable, and debug it the _SIG options are required for users to be able to make use of signatures. For secureboot, IMA appraisal policies are configured in kernel at boot time based on secureboot state of the system, refer arch/powerpc/kernel/ima_arch.c and security/integrity/ima/ima_efi.c. This doesn't require any user configuration. Yes, I agree it would be helpful to update kernel documentation specifying steps to sign the kernel image using sign-file. The other part is that distributions apply 'lockdown' patches that change the security policy depending on secure boot status which were rejected by upstream which only hook into the _SIG options, and not into the IMA_ options. Of course, I expect this to change when the IMA options are universally available across architectures and the support picked up by distributions. Which brings the third point: IMA features vary across architectures, and KEXEC_SIG is more common than IMA_KEXEC. config/arm64/default:CONFIG_HAVE_IMA_KEXEC=y config/ppc64le/default:CONFIG_HAVE_IMA_KEXEC=y config/arm64/default:CONFIG_KEXEC_SIG=y config/s390x/default:CONFIG_KEXEC_SIG=y config/x86_64/default:CONFIG_KEXEC_SIG=y KEXEC_SIG makes it much easier to get uniform features across architectures. Architectures use KEXEC_SIG vs IMA_KEXEC based on their requirement. IMA_KEXEC is for the kernel images signed using sign-file (appended signatures, not PECOFF), provides measurement along with verification, and is tied to secureboot state of the system at boot time. Thanks & Regards, - Nayna
Re: [PATCH 0/3] KEXEC_SIG with appended signature
On 11/5/21 09:14, Michal Suchánek wrote: On Fri, Nov 05, 2021 at 09:55:52PM +1100, Daniel Axtens wrote: Michal Suchanek writes: S390 uses appended signature for kernel but implements the check separately from module loader. Support for secure boot on powerpc with appended signature is planned - grub patches submitted upstream but not yet merged. Power Non-Virtualised / OpenPower already supports secure boot via kexec with signature verification via IMA. I think you have now sent a follow-up series that merges some of the IMA implementation, I just wanted to make sure it was clear that we actually already have support So is IMA_KEXEC and KEXEC_SIG redundant? I see some architectures have both. I also see there is a lot of overlap between the IMA framework and the KEXEC_SIG and MODULE_SIg. Originally, KEXEC_SIG was meant for PECOFF based signatures, while IMA_KEXEC mainly supported xattr based signatures. Power (Non-virtualized/OpenPOWER) doesn't support PECOFF. Extended attributes based signature verification doesn't work with netboot. That's when appended signature support was added to IMA. Using IMA_KEXEC has the benefit of being able to enable both signature verification and measurement of the kernel image. Thanks & Regards, - Nayna
Re: [PATCH] linux: configure CONFIG_I2C_OPAL as in-built.
On 9/29/20 2:14 AM, Joel Stanley wrote: On Fri, 25 Sep 2020 at 18:19, Mimi Zohar wrote: Hi Nayna, On Wed, 2020-09-23 at 14:25 -0400, Nayna Jain wrote: Currently, skiroot_defconfig CONFIG_I2C_OPAL is built as a loadable module rather than builtin, even if CONFIG_I2C=y is defined. This results in a delay in the TPM initialization, causing IMA to go into TPM bypass mode. As a result, the IMA measurements are added to the measurement list, but do not extend the TPM. Because of this, it is impossible to verify or attest to the system's integrity, either from skiroot or the target Host OS. The patch description is good, but perhaps we could provide a bit more context before. The concept of trusted boot requires the measurement to be added to the measurement list and extend the TPM, prior to allowing access to the file. By allowing access to a file before its measurement is included in the measurement list and extended into the TPM PCR, a malicious file could potentially prevent its own measurement from being added. As the PCRs are tamper proof, measuring and extending the TPM prior to giving access to the file, guarantees that all file measurements are included in the measurement list, including the malicious file. IMA needs to be enabled before any files are accessed in order to verify a file's integrity and extend the TPM with the file measurement. Queueing file measurements breaks the measure and extend, before usage, trusted boot paradigm. The ima-evm-utils package includes a test for walking the IMA measurement list, calculating the expected TPM PCRs, and comparing the calculated PCR values with the physical TPM. Testing is important to ensure the TPM is initialized prior to IMA. Failure to validate the IMA measurement list may indicate IMA went into TPM bypass mode, like in this case. Thanks for the explanation Mimi. It's lucky that the TPM drivers can be loaded early enough! Should we add something like this to security/integrity/ima/Kconfig? select I2C_OPAL if PPC_POWERNV It's generally frowned upon to select user visible symbols, but IMA does this for the TCG options already. I think yes, but in drivers/i2c/Kconfig and not in IMA Kconfig. IMA is dependent on TPM, and it is specified in IMA Kconfig. TPM NUVOTON driver has its dependency on I2C, which is taken care in drivers/char/tpm/Kconfig It is I2C driver which is dependent on I2C_OPAL for its functioning on POWERNV Systems. Thanks & Regards, - Nayna
Re: [PATCH v6] ima: move APPRAISE_BOOTPARAM dependency on ARCH_POLICY to runtime
On 7/13/20 12:48 PM, Bruno Meneguele wrote: The IMA_APPRAISE_BOOTPARAM config allows enabling different "ima_appraise=" modes - log, fix, enforce - at run time, but not when IMA architecture specific policies are enabled. This prevents properly labeling the filesystem on systems where secure boot is supported, but not enabled on the platform. Only when secure boot is actually enabled should these IMA appraise modes be disabled. This patch removes the compile time dependency and makes it a runtime decision, based on the secure boot state of that platform. Test results as follows: -> x86-64 with secure boot enabled [0.015637] Kernel command line: <...> ima_policy=appraise_tcb ima_appraise=fix [0.015668] ima: Secure boot enabled: ignoring ima_appraise=fix boot parameter option -> powerpc with secure boot disabled [0.00] Kernel command line: <...> ima_policy=appraise_tcb ima_appraise=fix [0.00] Secure boot mode disabled -> Running the system without secure boot and with both options set: CONFIG_IMA_APPRAISE_BOOTPARAM=y CONFIG_IMA_ARCH_POLICY=y Audit prompts "missing-hash" but still allow execution and, consequently, filesystem labeling: type=INTEGRITY_DATA msg=audit(07/09/2020 12:30:27.778:1691) : pid=4976 uid=root auid=root ses=2 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 op=appraise_data cause=missing-hash comm=bash name=/usr/bin/evmctl dev="dm-0" ino=493150 res=no Cc: sta...@vger.kernel.org Fixes: d958083a8f64 ("x86/ima: define arch_get_ima_policy() for x86") Signed-off-by: Bruno Meneguele Reviewed-by: Nayna Jain Tested-by: Nayna Jain Thanks & Regards, - Nayna
[PATCH v3] powerpc/pseries: detect secure and trusted boot state of the system.
The device-tree property to check secure and trusted boot state is different for guests(pseries) compared to baremetal(powernv). This patch updates the existing is_ppc_secureboot_enabled() and is_ppc_trustedboot_enabled() functions to add support for pseries. The secureboot and trustedboot state are exposed via device-tree property: /proc/device-tree/ibm,secure-boot and /proc/device-tree/ibm,trusted-boot The values of ibm,secure-boot under pseries are interpreted as: 0 - Disabled 1 - Enabled in Log-only mode. This patch interprets this value as disabled, since audit mode is currently not supported for Linux. 2 - Enabled and enforced. 3-9 - Enabled and enforcing; requirements are at the discretion of the operating system. The values of ibm,trusted-boot under pseries are interpreted as: 0 - Disabled 1 - Enabled Signed-off-by: Nayna Jain Reviewed-by: Daniel Axtens --- v3: * fixed double check. Thanks Daniel for noticing it. * updated patch description. v2: * included Michael Ellerman's feedback. * added Daniel Axtens's Reviewed-by. arch/powerpc/kernel/secure_boot.c | 19 +-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/kernel/secure_boot.c b/arch/powerpc/kernel/secure_boot.c index 4b982324d368..118bcb5f79c4 100644 --- a/arch/powerpc/kernel/secure_boot.c +++ b/arch/powerpc/kernel/secure_boot.c @@ -6,6 +6,7 @@ #include #include #include +#include static struct device_node *get_ppc_fw_sb_node(void) { @@ -23,12 +24,19 @@ bool is_ppc_secureboot_enabled(void) { struct device_node *node; bool enabled = false; + u32 secureboot; node = get_ppc_fw_sb_node(); enabled = of_property_read_bool(node, "os-secureboot-enforcing"); - of_node_put(node); + if (enabled) + goto out; + + if (!of_property_read_u32(of_root, "ibm,secure-boot", &secureboot)) + enabled = (secureboot > 1); + +out: pr_info("Secure boot mode %s\n", enabled ? "enabled" : "disabled"); return enabled; @@ -38,12 +46,19 @@ bool is_ppc_trustedboot_enabled(void) { struct device_node *node; bool enabled = false; + u32 trustedboot; node = get_ppc_fw_sb_node(); enabled = of_property_read_bool(node, "trusted-enabled"); - of_node_put(node); + if (enabled) + goto out; + + if (!of_property_read_u32(of_root, "ibm,trusted-boot", &trustedboot)) + enabled = (trustedboot > 0); + +out: pr_info("Trusted boot mode %s\n", enabled ? "enabled" : "disabled"); return enabled; -- 2.26.2
[PATCH v2] powerpc/pseries: detect secure and trusted boot state of the system.
The device-tree property to check secure and trusted boot state is different for guests(pseries) compared to baremetal(powernv). This patch updates the existing is_ppc_secureboot_enabled() and is_ppc_trustedboot_enabled() function to add support for pseries. Signed-off-by: Nayna Jain Reviewed-by: Daniel Axtens --- v2: * included Michael Ellerman's feedback. * added Daniel Axtens's Reviewed-by. arch/powerpc/kernel/secure_boot.c | 23 +-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/kernel/secure_boot.c b/arch/powerpc/kernel/secure_boot.c index 4b982324d368..efb325cbd42f 100644 --- a/arch/powerpc/kernel/secure_boot.c +++ b/arch/powerpc/kernel/secure_boot.c @@ -6,6 +6,7 @@ #include #include #include +#include static struct device_node *get_ppc_fw_sb_node(void) { @@ -23,12 +24,21 @@ bool is_ppc_secureboot_enabled(void) { struct device_node *node; bool enabled = false; + u32 secureboot; node = get_ppc_fw_sb_node(); enabled = of_property_read_bool(node, "os-secureboot-enforcing"); - of_node_put(node); + if (enabled) + goto out; + + if (!of_property_read_u32(of_root, "ibm,secure-boot", &secureboot)) { + if (secureboot) + enabled = (secureboot > 1) ? true : false; + } + +out: pr_info("Secure boot mode %s\n", enabled ? "enabled" : "disabled"); return enabled; @@ -38,12 +48,21 @@ bool is_ppc_trustedboot_enabled(void) { struct device_node *node; bool enabled = false; + u32 trustedboot; node = get_ppc_fw_sb_node(); enabled = of_property_read_bool(node, "trusted-enabled"); - of_node_put(node); + if (enabled) + goto out; + + if (!of_property_read_u32(of_root, "ibm,trusted-boot", &trustedboot)) { + if (trustedboot) + enabled = (trustedboot > 0) ? true : false; + } + +out: pr_info("Trusted boot mode %s\n", enabled ? "enabled" : "disabled"); return enabled; -- 2.26.2
[PATCH] powerpc/pseries: detect secure and trusted boot state of the system.
The device-tree property to check secure and trusted boot state is different for guests(pseries) compared to baremetal(powernv). This patch updates the existing is_ppc_secureboot_enabled() and is_ppc_trustedboot_enabled() function to add support for pseries. Signed-off-by: Nayna Jain --- arch/powerpc/kernel/secure_boot.c | 31 +-- 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/arch/powerpc/kernel/secure_boot.c b/arch/powerpc/kernel/secure_boot.c index 4b982324d368..43fc6607c7a5 100644 --- a/arch/powerpc/kernel/secure_boot.c +++ b/arch/powerpc/kernel/secure_boot.c @@ -6,6 +6,7 @@ #include #include #include +#include static struct device_node *get_ppc_fw_sb_node(void) { @@ -23,11 +24,20 @@ bool is_ppc_secureboot_enabled(void) { struct device_node *node; bool enabled = false; + const u32 *secureboot; - node = get_ppc_fw_sb_node(); - enabled = of_property_read_bool(node, "os-secureboot-enforcing"); + if (machine_is(powernv)) { + node = get_ppc_fw_sb_node(); + enabled = + of_property_read_bool(node, "os-secureboot-enforcing"); + of_node_put(node); + } - of_node_put(node); + if (machine_is(pseries)) { + secureboot = of_get_property(of_root, "ibm,secure-boot", NULL); + if (secureboot) + enabled = (*secureboot > 1) ? true : false; + } pr_info("Secure boot mode %s\n", enabled ? "enabled" : "disabled"); @@ -38,11 +48,20 @@ bool is_ppc_trustedboot_enabled(void) { struct device_node *node; bool enabled = false; + const u32 *trustedboot; - node = get_ppc_fw_sb_node(); - enabled = of_property_read_bool(node, "trusted-enabled"); + if (machine_is(powernv)) { + node = get_ppc_fw_sb_node(); + enabled = of_property_read_bool(node, "trusted-enabled"); + of_node_put(node); + } - of_node_put(node); + if (machine_is(pseries)) { + trustedboot = + of_get_property(of_root, "ibm,trusted-boot", NULL); + if (trustedboot) + enabled = (*trustedboot > 0) ? true : false; + } pr_info("Trusted boot mode %s\n", enabled ? "enabled" : "disabled"); -- 2.18.1
[PATCH v2] powerpc/ima: fix secure boot rules in ima arch policy
To prevent verifying the kernel module appended signature twice (finit_module), once by the module_sig_check() and again by IMA, powerpc secure boot rules define an IMA architecture specific policy rule only if CONFIG_MODULE_SIG_FORCE is not enabled. This, unfortunately, does not take into account the ability of enabling "sig_enforce" on the boot command line (module.sig_enforce=1). Including the IMA module appraise rule results in failing the finit_module syscall, unless the module signing public key is loaded onto the IMA keyring. This patch fixes secure boot policy rules to be based on CONFIG_MODULE_SIG instead. Fixes: 4238fad366a6 ("powerpc/ima: Add support to initialize ima policy rules") Signed-off-by: Nayna Jain --- v2: * Fixes the patch description to specify the problem more clearly as asked by Michael Ellerman. arch/powerpc/kernel/ima_arch.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/powerpc/kernel/ima_arch.c b/arch/powerpc/kernel/ima_arch.c index e34116255ced..957abd592075 100644 --- a/arch/powerpc/kernel/ima_arch.c +++ b/arch/powerpc/kernel/ima_arch.c @@ -19,12 +19,12 @@ bool arch_ima_get_secureboot(void) * to be stored as an xattr or as an appended signature. * * To avoid duplicate signature verification as much as possible, the IMA - * policy rule for module appraisal is added only if CONFIG_MODULE_SIG_FORCE + * policy rule for module appraisal is added only if CONFIG_MODULE_SIG * is not enabled. */ static const char *const secure_rules[] = { "appraise func=KEXEC_KERNEL_CHECK appraise_flag=check_blacklist appraise_type=imasig|modsig", -#ifndef CONFIG_MODULE_SIG_FORCE +#ifndef CONFIG_MODULE_SIG "appraise func=MODULE_CHECK appraise_flag=check_blacklist appraise_type=imasig|modsig", #endif NULL @@ -50,7 +50,7 @@ static const char *const secure_and_trusted_rules[] = { "measure func=KEXEC_KERNEL_CHECK template=ima-modsig", "measure func=MODULE_CHECK template=ima-modsig", "appraise func=KEXEC_KERNEL_CHECK appraise_flag=check_blacklist appraise_type=imasig|modsig", -#ifndef CONFIG_MODULE_SIG_FORCE +#ifndef CONFIG_MODULE_SIG "appraise func=MODULE_CHECK appraise_flag=check_blacklist appraise_type=imasig|modsig", #endif NULL -- 2.18.1
[PATCH] powerpc/ima: fix secure boot rules in ima arch policy
To prevent verifying the kernel module appended signature twice (finit_module), once by the module_sig_check() and again by IMA, powerpc IMA secure boot rules define an IMA architecture specific policy rule only if CONFIG_MODULE_SIG_FORCE is not enabled. This, unfortunately, does not take into account the ability of enabling "sig_enforce" on the boot command line (module.sig_enforce=1). This patch fixes secure boot policy rules to be based on CONFIG_MODULE_SIG instead. Fixes: 4238fad366a6 ("powerpc/ima: Add support to initialize ima policy rules") Signed-off-by: Nayna Jain --- arch/powerpc/kernel/ima_arch.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/powerpc/kernel/ima_arch.c b/arch/powerpc/kernel/ima_arch.c index e34116255ced..957abd592075 100644 --- a/arch/powerpc/kernel/ima_arch.c +++ b/arch/powerpc/kernel/ima_arch.c @@ -19,12 +19,12 @@ bool arch_ima_get_secureboot(void) * to be stored as an xattr or as an appended signature. * * To avoid duplicate signature verification as much as possible, the IMA - * policy rule for module appraisal is added only if CONFIG_MODULE_SIG_FORCE + * policy rule for module appraisal is added only if CONFIG_MODULE_SIG * is not enabled. */ static const char *const secure_rules[] = { "appraise func=KEXEC_KERNEL_CHECK appraise_flag=check_blacklist appraise_type=imasig|modsig", -#ifndef CONFIG_MODULE_SIG_FORCE +#ifndef CONFIG_MODULE_SIG "appraise func=MODULE_CHECK appraise_flag=check_blacklist appraise_type=imasig|modsig", #endif NULL @@ -50,7 +50,7 @@ static const char *const secure_and_trusted_rules[] = { "measure func=KEXEC_KERNEL_CHECK template=ima-modsig", "measure func=MODULE_CHECK template=ima-modsig", "appraise func=KEXEC_KERNEL_CHECK appraise_flag=check_blacklist appraise_type=imasig|modsig", -#ifndef CONFIG_MODULE_SIG_FORCE +#ifndef CONFIG_MODULE_SIG "appraise func=MODULE_CHECK appraise_flag=check_blacklist appraise_type=imasig|modsig", #endif NULL -- 2.25.1
[PATCH v3] ima: add a new CONFIG for loading arch-specific policies
From: Nayna Jain Every time a new architecture defines the IMA architecture specific functions - arch_ima_get_secureboot() and arch_ima_get_policy(), the IMA include file needs to be updated. To avoid this "noise", this patch defines a new IMA Kconfig IMA_SECURE_AND_OR_TRUSTED_BOOT option, allowing the different architectures to select it. Suggested-by: Linus Torvalds Signed-off-by: Nayna Jain Acked-by: Ard Biesheuvel Cc: Ard Biesheuvel Cc: Philipp Rudo Cc: Michael Ellerman --- v3: * Removes CONFIG_IMA dependency. Thanks Ard. * Updated the patch with improvements suggested by Michael. It now uses "imply" instead of "select". Thanks Michael. * Replaced the CONFIG_IMA in x86 and s390 with new config, else it was resulting in redefinition when the IMA_SECURE_AND_OR_TRUSTED_BOOT is not enabled. Thanks to Mimi for identifying the problem. * Removed "#ifdef EFI" check in the arch/x86/Makefile for compiling ima_arch.c file. * Ard, Thanks for your Acked-by. I have changed the arch/x86/Makefile in this version. Can you please review again and confirm ? * Rudo, Thanks for your review. I have changed arch/s390/Makefile as well. Can you also please review again ? v2: * Fixed the issue identified by Mimi. Thanks Mimi, Ard, Heiko and Michael for discussing the fix. arch/powerpc/Kconfig | 1 + arch/s390/Kconfig | 1 + arch/s390/kernel/Makefile | 2 +- arch/x86/Kconfig | 1 + arch/x86/kernel/Makefile | 4 +--- include/linux/ima.h| 3 +-- security/integrity/ima/Kconfig | 7 +++ 7 files changed, 13 insertions(+), 6 deletions(-) diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 497b7d0b2d7e..5b9f1cba2a44 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -979,6 +979,7 @@ config PPC_SECURE_BOOT bool depends on PPC_POWERNV depends on IMA_ARCH_POLICY + imply IMA_SECURE_AND_OR_TRUSTED_BOOT help Systems with firmware secure boot enabled need to define security policies to extend secure boot to the OS. This config allows a user diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig index 8abe77536d9d..59c216af6264 100644 --- a/arch/s390/Kconfig +++ b/arch/s390/Kconfig @@ -195,6 +195,7 @@ config S390 select ARCH_HAS_FORCE_DMA_UNENCRYPTED select SWIOTLB select GENERIC_ALLOCATOR + imply IMA_SECURE_AND_OR_TRUSTED_BOOT config SCHED_OMIT_FRAME_POINTER diff --git a/arch/s390/kernel/Makefile b/arch/s390/kernel/Makefile index 2b1203cf7be6..578a6fa82ea4 100644 --- a/arch/s390/kernel/Makefile +++ b/arch/s390/kernel/Makefile @@ -70,7 +70,7 @@ obj-$(CONFIG_JUMP_LABEL) += jump_label.o obj-$(CONFIG_KEXEC_FILE) += machine_kexec_file.o kexec_image.o obj-$(CONFIG_KEXEC_FILE) += kexec_elf.o -obj-$(CONFIG_IMA) += ima_arch.o +obj-$(CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT) += ima_arch.o obj-$(CONFIG_PERF_EVENTS) += perf_event.o perf_cpum_cf_common.o obj-$(CONFIG_PERF_EVENTS) += perf_cpum_cf.o perf_cpum_sf.o diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index beea77046f9b..dcf5b1729f7c 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -230,6 +230,7 @@ config X86 select VIRT_TO_BUS select X86_FEATURE_NAMESif PROC_FS select PROC_PID_ARCH_STATUS if PROC_FS + imply IMA_SECURE_AND_OR_TRUSTED_BOOTif EFI config INSTRUCTION_DECODER def_bool y diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile index 9b294c13809a..cfef37a27def 100644 --- a/arch/x86/kernel/Makefile +++ b/arch/x86/kernel/Makefile @@ -154,6 +154,4 @@ ifeq ($(CONFIG_X86_64),y) obj-y += vsmp_64.o endif -ifdef CONFIG_EFI -obj-$(CONFIG_IMA) += ima_arch.o -endif +obj-$(CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT) += ima_arch.o diff --git a/include/linux/ima.h b/include/linux/ima.h index 1659217e9b60..aefe758f4466 100644 --- a/include/linux/ima.h +++ b/include/linux/ima.h @@ -30,8 +30,7 @@ extern void ima_kexec_cmdline(const void *buf, int size); extern void ima_add_kexec_buffer(struct kimage *image); #endif -#if (defined(CONFIG_X86) && defined(CONFIG_EFI)) || defined(CONFIG_S390) \ - || defined(CONFIG_PPC_SECURE_BOOT) +#ifdef CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT extern bool arch_ima_get_secureboot(void); extern const char * const *arch_get_ima_policy(void); #else diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig index 3f3ee4e2eb0d..edde88dbe576 100644 --- a/security/integrity/ima/Kconfig +++ b/security/integrity/ima/Kconfig @@ -327,3 +327,10 @@ config IMA_QUEUE_EARLY_BOOT_KEYS depends on IMA_MEASURE_ASYMMETRIC_KEYS depends on SYSTEM_TRUSTED_KEYRING default y + +config IMA_SECURE_AND_OR_TRUSTED_BOOT + bool + depends on IMA_ARCH_POLICY + help + This option is selected by architectures
Re: [PATCH v3] ima: add a new CONFIG for loading arch-specific policies
Oops, Please ignore this patch. By mistake I posted the wrong version. I am sorry for the confusion, I will resend the right version. Thanks & Regards, - Nayna On 3/6/20 12:39 PM, Nayna Jain wrote: Every time a new architecture defines the IMA architecture specific functions - arch_ima_get_secureboot() and arch_ima_get_policy(), the IMA include file needs to be updated. To avoid this "noise", this patch defines a new IMA Kconfig IMA_SECURE_AND_OR_TRUSTED_BOOT option, allowing the different architectures to select it. Suggested-by: Linus Torvalds Signed-off-by: Nayna Jain Cc: Ard Biesheuvel Cc: Philipp Rudo Cc: Michael Ellerman --- v3: * Updated and tested the patch with improvements suggested by Michael. It now uses "imply" instead of "select". Thanks Michael. * Have missed replacing the CONFIG_IMA in x86 and s390 with new config, that was resulting in redefinition when the IMA_SECURE_AND_OR_TRUSTED_BOOT is not enabled. Thanks to Mimi for recognizing the problem. v2: * Fixed the issue identified by Mimi. Thanks Mimi, Ard, Heiko and Michael for discussing the fix. arch/powerpc/Kconfig | 1 + arch/s390/Kconfig | 1 + arch/s390/kernel/Makefile | 2 +- arch/x86/Kconfig | 1 + arch/x86/kernel/Makefile | 2 +- include/linux/ima.h| 3 +-- security/integrity/ima/Kconfig | 8 7 files changed, 14 insertions(+), 4 deletions(-) diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 497b7d0b2d7e..a5cfde432983 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -979,6 +979,7 @@ config PPC_SECURE_BOOT bool depends on PPC_POWERNV depends on IMA_ARCH_POLICY + select IMA_SECURE_AND_OR_TRUSTED_BOOT help Systems with firmware secure boot enabled need to define security policies to extend secure boot to the OS. This config allows a user diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig index 8abe77536d9d..4a502fbcb800 100644 --- a/arch/s390/Kconfig +++ b/arch/s390/Kconfig @@ -195,6 +195,7 @@ config S390 select ARCH_HAS_FORCE_DMA_UNENCRYPTED select SWIOTLB select GENERIC_ALLOCATOR + select IMA_SECURE_AND_OR_TRUSTED_BOOT if IMA_ARCH_POLICY config SCHED_OMIT_FRAME_POINTER diff --git a/arch/s390/kernel/Makefile b/arch/s390/kernel/Makefile index 2b1203cf7be6..578a6fa82ea4 100644 --- a/arch/s390/kernel/Makefile +++ b/arch/s390/kernel/Makefile @@ -70,7 +70,7 @@ obj-$(CONFIG_JUMP_LABEL) += jump_label.o obj-$(CONFIG_KEXEC_FILE) += machine_kexec_file.o kexec_image.o obj-$(CONFIG_KEXEC_FILE) += kexec_elf.o -obj-$(CONFIG_IMA) += ima_arch.o +obj-$(CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT) += ima_arch.o obj-$(CONFIG_PERF_EVENTS) += perf_event.o perf_cpum_cf_common.o obj-$(CONFIG_PERF_EVENTS) += perf_cpum_cf.o perf_cpum_sf.o diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index beea77046f9b..7f5bfaf0cbd2 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -230,6 +230,7 @@ config X86 select VIRT_TO_BUS select X86_FEATURE_NAMESif PROC_FS select PROC_PID_ARCH_STATUS if PROC_FS + select IMA_SECURE_AND_OR_TRUSTED_BOOT if EFI && IMA_ARCH_POLICY config INSTRUCTION_DECODER def_bool y diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile index 9b294c13809a..7f131ceba136 100644 --- a/arch/x86/kernel/Makefile +++ b/arch/x86/kernel/Makefile @@ -155,5 +155,5 @@ ifeq ($(CONFIG_X86_64),y) endif ifdef CONFIG_EFI -obj-$(CONFIG_IMA) += ima_arch.o +obj-$(CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT) += ima_arch.o endif diff --git a/include/linux/ima.h b/include/linux/ima.h index 1659217e9b60..aefe758f4466 100644 --- a/include/linux/ima.h +++ b/include/linux/ima.h @@ -30,8 +30,7 @@ extern void ima_kexec_cmdline(const void *buf, int size); extern void ima_add_kexec_buffer(struct kimage *image); #endif -#if (defined(CONFIG_X86) && defined(CONFIG_EFI)) || defined(CONFIG_S390) \ - || defined(CONFIG_PPC_SECURE_BOOT) +#ifdef CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT extern bool arch_ima_get_secureboot(void); extern const char * const *arch_get_ima_policy(void); #else diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig index 3f3ee4e2eb0d..2baaf196c6d8 100644 --- a/security/integrity/ima/Kconfig +++ b/security/integrity/ima/Kconfig @@ -327,3 +327,11 @@ config IMA_QUEUE_EARLY_BOOT_KEYS depends on IMA_MEASURE_ASYMMETRIC_KEYS depends on SYSTEM_TRUSTED_KEYRING default y + +config IMA_SECURE_AND_OR_TRUSTED_BOOT + bool + depends on IMA_ARCH_POLICY + default n + help + This option is selected by architectures to enable secure and/or + trusted boot based on IMA runtime policies.
[PATCH v3] ima: add a new CONFIG for loading arch-specific policies
Every time a new architecture defines the IMA architecture specific functions - arch_ima_get_secureboot() and arch_ima_get_policy(), the IMA include file needs to be updated. To avoid this "noise", this patch defines a new IMA Kconfig IMA_SECURE_AND_OR_TRUSTED_BOOT option, allowing the different architectures to select it. Suggested-by: Linus Torvalds Signed-off-by: Nayna Jain Cc: Ard Biesheuvel Cc: Philipp Rudo Cc: Michael Ellerman --- v3: * Updated and tested the patch with improvements suggested by Michael. It now uses "imply" instead of "select". Thanks Michael. * Have missed replacing the CONFIG_IMA in x86 and s390 with new config, that was resulting in redefinition when the IMA_SECURE_AND_OR_TRUSTED_BOOT is not enabled. Thanks to Mimi for recognizing the problem. v2: * Fixed the issue identified by Mimi. Thanks Mimi, Ard, Heiko and Michael for discussing the fix. arch/powerpc/Kconfig | 1 + arch/s390/Kconfig | 1 + arch/s390/kernel/Makefile | 2 +- arch/x86/Kconfig | 1 + arch/x86/kernel/Makefile | 2 +- include/linux/ima.h| 3 +-- security/integrity/ima/Kconfig | 8 7 files changed, 14 insertions(+), 4 deletions(-) diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 497b7d0b2d7e..a5cfde432983 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -979,6 +979,7 @@ config PPC_SECURE_BOOT bool depends on PPC_POWERNV depends on IMA_ARCH_POLICY + select IMA_SECURE_AND_OR_TRUSTED_BOOT help Systems with firmware secure boot enabled need to define security policies to extend secure boot to the OS. This config allows a user diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig index 8abe77536d9d..4a502fbcb800 100644 --- a/arch/s390/Kconfig +++ b/arch/s390/Kconfig @@ -195,6 +195,7 @@ config S390 select ARCH_HAS_FORCE_DMA_UNENCRYPTED select SWIOTLB select GENERIC_ALLOCATOR + select IMA_SECURE_AND_OR_TRUSTED_BOOT if IMA_ARCH_POLICY config SCHED_OMIT_FRAME_POINTER diff --git a/arch/s390/kernel/Makefile b/arch/s390/kernel/Makefile index 2b1203cf7be6..578a6fa82ea4 100644 --- a/arch/s390/kernel/Makefile +++ b/arch/s390/kernel/Makefile @@ -70,7 +70,7 @@ obj-$(CONFIG_JUMP_LABEL) += jump_label.o obj-$(CONFIG_KEXEC_FILE) += machine_kexec_file.o kexec_image.o obj-$(CONFIG_KEXEC_FILE) += kexec_elf.o -obj-$(CONFIG_IMA) += ima_arch.o +obj-$(CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT) += ima_arch.o obj-$(CONFIG_PERF_EVENTS) += perf_event.o perf_cpum_cf_common.o obj-$(CONFIG_PERF_EVENTS) += perf_cpum_cf.o perf_cpum_sf.o diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index beea77046f9b..7f5bfaf0cbd2 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -230,6 +230,7 @@ config X86 select VIRT_TO_BUS select X86_FEATURE_NAMESif PROC_FS select PROC_PID_ARCH_STATUS if PROC_FS + select IMA_SECURE_AND_OR_TRUSTED_BOOT if EFI && IMA_ARCH_POLICY config INSTRUCTION_DECODER def_bool y diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile index 9b294c13809a..7f131ceba136 100644 --- a/arch/x86/kernel/Makefile +++ b/arch/x86/kernel/Makefile @@ -155,5 +155,5 @@ ifeq ($(CONFIG_X86_64),y) endif ifdef CONFIG_EFI -obj-$(CONFIG_IMA) += ima_arch.o +obj-$(CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT) += ima_arch.o endif diff --git a/include/linux/ima.h b/include/linux/ima.h index 1659217e9b60..aefe758f4466 100644 --- a/include/linux/ima.h +++ b/include/linux/ima.h @@ -30,8 +30,7 @@ extern void ima_kexec_cmdline(const void *buf, int size); extern void ima_add_kexec_buffer(struct kimage *image); #endif -#if (defined(CONFIG_X86) && defined(CONFIG_EFI)) || defined(CONFIG_S390) \ - || defined(CONFIG_PPC_SECURE_BOOT) +#ifdef CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT extern bool arch_ima_get_secureboot(void); extern const char * const *arch_get_ima_policy(void); #else diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig index 3f3ee4e2eb0d..2baaf196c6d8 100644 --- a/security/integrity/ima/Kconfig +++ b/security/integrity/ima/Kconfig @@ -327,3 +327,11 @@ config IMA_QUEUE_EARLY_BOOT_KEYS depends on IMA_MEASURE_ASYMMETRIC_KEYS depends on SYSTEM_TRUSTED_KEYRING default y + +config IMA_SECURE_AND_OR_TRUSTED_BOOT + bool + depends on IMA_ARCH_POLICY + default n + help + This option is selected by architectures to enable secure and/or + trusted boot based on IMA runtime policies. -- 2.18.1
[PATCH v2] ima: add a new CONFIG for loading arch-specific policies
Every time a new architecture defines the IMA architecture specific functions - arch_ima_get_secureboot() and arch_ima_get_policy(), the IMA include file needs to be updated. To avoid this "noise", this patch defines a new IMA Kconfig IMA_SECURE_AND_OR_TRUSTED_BOOT option, allowing the different architectures to select it. Suggested-by: Linus Torvalds Signed-off-by: Nayna Jain Cc: Ard Biesheuvel Cc: Philipp Rudo Cc: Michael Ellerman --- v2: * Fixed the issue identified by Mimi. Thanks Mimi, Ard, Heiko and Michael for discussing the fix. arch/powerpc/Kconfig | 1 + arch/s390/Kconfig | 1 + arch/x86/Kconfig | 1 + include/linux/ima.h| 3 +-- security/integrity/ima/Kconfig | 9 + 5 files changed, 13 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 497b7d0b2d7e..a5cfde432983 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -979,6 +979,7 @@ config PPC_SECURE_BOOT bool depends on PPC_POWERNV depends on IMA_ARCH_POLICY + select IMA_SECURE_AND_OR_TRUSTED_BOOT help Systems with firmware secure boot enabled need to define security policies to extend secure boot to the OS. This config allows a user diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig index 8abe77536d9d..4a502fbcb800 100644 --- a/arch/s390/Kconfig +++ b/arch/s390/Kconfig @@ -195,6 +195,7 @@ config S390 select ARCH_HAS_FORCE_DMA_UNENCRYPTED select SWIOTLB select GENERIC_ALLOCATOR + select IMA_SECURE_AND_OR_TRUSTED_BOOT if IMA_ARCH_POLICY config SCHED_OMIT_FRAME_POINTER diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index beea77046f9b..7f5bfaf0cbd2 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -230,6 +230,7 @@ config X86 select VIRT_TO_BUS select X86_FEATURE_NAMESif PROC_FS select PROC_PID_ARCH_STATUS if PROC_FS + select IMA_SECURE_AND_OR_TRUSTED_BOOT if EFI && IMA_ARCH_POLICY config INSTRUCTION_DECODER def_bool y diff --git a/include/linux/ima.h b/include/linux/ima.h index 1659217e9b60..aefe758f4466 100644 --- a/include/linux/ima.h +++ b/include/linux/ima.h @@ -30,8 +30,7 @@ extern void ima_kexec_cmdline(const void *buf, int size); extern void ima_add_kexec_buffer(struct kimage *image); #endif -#if (defined(CONFIG_X86) && defined(CONFIG_EFI)) || defined(CONFIG_S390) \ - || defined(CONFIG_PPC_SECURE_BOOT) +#ifdef CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT extern bool arch_ima_get_secureboot(void); extern const char * const *arch_get_ima_policy(void); #else diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig index 3f3ee4e2eb0d..d17972aa413a 100644 --- a/security/integrity/ima/Kconfig +++ b/security/integrity/ima/Kconfig @@ -327,3 +327,12 @@ config IMA_QUEUE_EARLY_BOOT_KEYS depends on IMA_MEASURE_ASYMMETRIC_KEYS depends on SYSTEM_TRUSTED_KEYRING default y + +config IMA_SECURE_AND_OR_TRUSTED_BOOT + bool + depends on IMA + depends on IMA_ARCH_POLICY + default n + help + This option is selected by architectures to enable secure and/or + trusted boot based on IMA runtime policies. -- 2.13.6
[PATCH] ima: add a new CONFIG for loading arch-specific policies
Every time a new architecture defines the IMA architecture specific functions - arch_ima_get_secureboot() and arch_ima_get_policy(), the IMA include file needs to be updated. To avoid this "noise", this patch defines a new IMA Kconfig IMA_SECURE_AND_OR_TRUSTED_BOOT option, allowing the different architectures to select it. Suggested-by: Linus Torvalds Signed-off-by: Nayna Jain Cc: Ard Biesheuvel Cc: Martin Schwidefsky Cc: Philipp Rudo Cc: Michael Ellerman --- arch/powerpc/Kconfig | 2 +- arch/s390/Kconfig | 1 + arch/x86/Kconfig | 1 + include/linux/ima.h| 3 +-- security/integrity/ima/Kconfig | 9 + 5 files changed, 13 insertions(+), 3 deletions(-) diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 497b7d0b2d7e..b8ce1b995633 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -246,6 +246,7 @@ config PPC select SYSCTL_EXCEPTION_TRACE select THREAD_INFO_IN_TASK select VIRT_TO_BUS if !PPC64 + select IMA_SECURE_AND_OR_TRUSTED_BOOT if PPC_SECURE_BOOT # # Please keep this list sorted alphabetically. # @@ -978,7 +979,6 @@ config PPC_SECURE_BOOT prompt "Enable secure boot support" bool depends on PPC_POWERNV - depends on IMA_ARCH_POLICY help Systems with firmware secure boot enabled need to define security policies to extend secure boot to the OS. This config allows a user diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig index 8abe77536d9d..90ff3633ade6 100644 --- a/arch/s390/Kconfig +++ b/arch/s390/Kconfig @@ -195,6 +195,7 @@ config S390 select ARCH_HAS_FORCE_DMA_UNENCRYPTED select SWIOTLB select GENERIC_ALLOCATOR + select IMA_SECURE_AND_OR_TRUSTED_BOOT config SCHED_OMIT_FRAME_POINTER diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index beea77046f9b..cafa66313fe2 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -230,6 +230,7 @@ config X86 select VIRT_TO_BUS select X86_FEATURE_NAMESif PROC_FS select PROC_PID_ARCH_STATUS if PROC_FS + select IMA_SECURE_AND_OR_TRUSTED_BOOT if EFI config INSTRUCTION_DECODER def_bool y diff --git a/include/linux/ima.h b/include/linux/ima.h index 1659217e9b60..aefe758f4466 100644 --- a/include/linux/ima.h +++ b/include/linux/ima.h @@ -30,8 +30,7 @@ extern void ima_kexec_cmdline(const void *buf, int size); extern void ima_add_kexec_buffer(struct kimage *image); #endif -#if (defined(CONFIG_X86) && defined(CONFIG_EFI)) || defined(CONFIG_S390) \ - || defined(CONFIG_PPC_SECURE_BOOT) +#ifdef CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT extern bool arch_ima_get_secureboot(void); extern const char * const *arch_get_ima_policy(void); #else diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig index 3f3ee4e2eb0d..d17972aa413a 100644 --- a/security/integrity/ima/Kconfig +++ b/security/integrity/ima/Kconfig @@ -327,3 +327,12 @@ config IMA_QUEUE_EARLY_BOOT_KEYS depends on IMA_MEASURE_ASYMMETRIC_KEYS depends on SYSTEM_TRUSTED_KEYRING default y + +config IMA_SECURE_AND_OR_TRUSTED_BOOT + bool + depends on IMA + depends on IMA_ARCH_POLICY + default n + help + This option is selected by architectures to enable secure and/or + trusted boot based on IMA runtime policies. -- 2.18.1
Re: [GIT PULL] Please pull powerpc/linux.git powerpc-5.5-1 tag
On 11/30/19 5:42 PM, Linus Torvalds wrote: [ Only tangentially related to the power parts ] On Sat, Nov 30, 2019 at 2:41 AM Michael Ellerman wrote: There's some changes in security/integrity as part of the secure boot work. They were all either written by or acked/reviewed by Mimi. -#if (defined(CONFIG_X86) && defined(CONFIG_EFI)) || defined(CONFIG_S390) +#if (defined(CONFIG_X86) && defined(CONFIG_EFI)) || defined(CONFIG_S390) \ + || defined(CONFIG_PPC_SECURE_BOOT) This clearly should be its own CONFIG variable, and be generated by having the different architectures just select it. IOW, IMA should probably have a config IMA_SECURE_BOOT and then s390 would just do the select unconditionally, while x86 and ppc would do select IMA_SECURE_BOOT if EFI and select IMA_SECURE_BOOT if PPC_SECURE_BOOT respectively. And then we wouldn't have random architectures adding random "me me me tooo!!!" type code. Thanks Linus for your feedback. I will do the patch for Kconfig cleanup. Thanks & Regards, - Nayna
[PATCH v9 4/4] powerpc: load firmware trusted keys/hashes into kernel keyring
The keys used to verify the Host OS kernel are managed by firmware as secure variables. This patch loads the verification keys into the .platform keyring and revocation hashes into .blacklist keyring. This enables verification and loading of the kernels signed by the boot time keys which are trusted by firmware. Signed-off-by: Nayna Jain Reviewed-by: Mimi Zohar Signed-off-by: Eric Richter --- security/integrity/Kconfig | 9 +++ security/integrity/Makefile | 4 +- security/integrity/platform_certs/load_powerpc.c | 99 3 files changed, 111 insertions(+), 1 deletion(-) create mode 100644 security/integrity/platform_certs/load_powerpc.c diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig index 0bae6adb63a9..71f0177e8716 100644 --- a/security/integrity/Kconfig +++ b/security/integrity/Kconfig @@ -72,6 +72,15 @@ config LOAD_IPL_KEYS depends on S390 def_bool y +config LOAD_PPC_KEYS + bool "Enable loading of platform and blacklisted keys for POWER" + depends on INTEGRITY_PLATFORM_KEYRING + depends on PPC_SECURE_BOOT + default y + help + Enable loading of keys to the .platform keyring and blacklisted + hashes to the .blacklist keyring for powerpc based platforms. + config INTEGRITY_AUDIT bool "Enables integrity auditing support " depends on AUDIT diff --git a/security/integrity/Makefile b/security/integrity/Makefile index 351c9662994b..7ee39d66cf16 100644 --- a/security/integrity/Makefile +++ b/security/integrity/Makefile @@ -14,6 +14,8 @@ integrity-$(CONFIG_LOAD_UEFI_KEYS) += platform_certs/efi_parser.o \ platform_certs/load_uefi.o \ platform_certs/keyring_handler.o integrity-$(CONFIG_LOAD_IPL_KEYS) += platform_certs/load_ipl_s390.o - +integrity-$(CONFIG_LOAD_PPC_KEYS) += platform_certs/efi_parser.o \ + platform_certs/load_powerpc.o \ + platform_certs/keyring_handler.o obj-$(CONFIG_IMA) += ima/ obj-$(CONFIG_EVM) += evm/ diff --git a/security/integrity/platform_certs/load_powerpc.c b/security/integrity/platform_certs/load_powerpc.c new file mode 100644 index ..9b2596e838ac --- /dev/null +++ b/security/integrity/platform_certs/load_powerpc.c @@ -0,0 +1,99 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2019 IBM Corporation + * Author: Nayna Jain + * + * - loads keys and hashes stored and controlled by the firmware. + */ +#include +#include +#include +#include +#include +#include +#include +#include +#include "keyring_handler.h" + +/* + * Get a certificate list blob from the named secure variable. + */ +static __init void *get_cert_list(u8 *key, unsigned long keylen, uint64_t *size) +{ + int rc; + void *db; + + rc = secvar_ops->get(key, keylen, NULL, size); + if (rc) { + pr_err("Couldn't get size: %d\n", rc); + return NULL; + } + + db = kmalloc(*size, GFP_KERNEL); + if (!db) + return NULL; + + rc = secvar_ops->get(key, keylen, db, size); + if (rc) { + kfree(db); + pr_err("Error reading %s var: %d\n", key, rc); + return NULL; + } + + return db; +} + +/* + * Load the certs contained in the keys databases into the platform trusted + * keyring and the blacklisted X.509 cert SHA256 hashes into the blacklist + * keyring. + */ +static int __init load_powerpc_certs(void) +{ + void *db = NULL, *dbx = NULL; + uint64_t dbsize = 0, dbxsize = 0; + int rc = 0; + struct device_node *node; + + if (!secvar_ops) + return -ENODEV; + + /* The following only applies for the edk2-compat backend. +* Return early if it is not set. +*/ + + node = of_find_compatible_node(NULL, NULL, "ibm,secvar-backend"); + rc = of_property_match_string(node, "format", "ibm,edk2-compat-v1"); + if (rc) + return -ENODEV; + + /* Get db, and dbx. They might not exist, so it isn't +* an error if we can't get them. +*/ + db = get_cert_list("db", 3, &dbsize); + if (!db) { + pr_err("Couldn't get db list from firmware\n"); + } else { + rc = parse_efi_signature_list("powerpc:db", db, dbsize, + get_handler_for_db); + if (rc) + pr_err("Couldn't parse db signatures: %d\n", rc); + kfree(db); + } + + dbx = get_cert_list("dbx", 4, &dbxsize); +
[PATCH v9 3/4] x86/efi: move common keyring handler functions to new file
The handlers to add the keys to the .platform keyring and blacklisted hashes to the .blacklist keyring is common for both the uefi and powerpc mechanisms of loading the keys/hashes from the firmware. This patch moves the common code from load_uefi.c to keyring_handler.c Signed-off-by: Nayna Jain Acked-by: Mimi Zohar Signed-off-by: Eric Richter --- security/integrity/Makefile| 3 +- .../integrity/platform_certs/keyring_handler.c | 80 ++ .../integrity/platform_certs/keyring_handler.h | 32 + security/integrity/platform_certs/load_uefi.c | 67 +- 4 files changed, 115 insertions(+), 67 deletions(-) create mode 100644 security/integrity/platform_certs/keyring_handler.c create mode 100644 security/integrity/platform_certs/keyring_handler.h diff --git a/security/integrity/Makefile b/security/integrity/Makefile index 35e6ca773734..351c9662994b 100644 --- a/security/integrity/Makefile +++ b/security/integrity/Makefile @@ -11,7 +11,8 @@ integrity-$(CONFIG_INTEGRITY_SIGNATURE) += digsig.o integrity-$(CONFIG_INTEGRITY_ASYMMETRIC_KEYS) += digsig_asymmetric.o integrity-$(CONFIG_INTEGRITY_PLATFORM_KEYRING) += platform_certs/platform_keyring.o integrity-$(CONFIG_LOAD_UEFI_KEYS) += platform_certs/efi_parser.o \ - platform_certs/load_uefi.o + platform_certs/load_uefi.o \ + platform_certs/keyring_handler.o integrity-$(CONFIG_LOAD_IPL_KEYS) += platform_certs/load_ipl_s390.o obj-$(CONFIG_IMA) += ima/ diff --git a/security/integrity/platform_certs/keyring_handler.c b/security/integrity/platform_certs/keyring_handler.c new file mode 100644 index ..c5ba695c10e3 --- /dev/null +++ b/security/integrity/platform_certs/keyring_handler.c @@ -0,0 +1,80 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include +#include +#include +#include +#include +#include +#include +#include +#include "../integrity.h" + +static efi_guid_t efi_cert_x509_guid __initdata = EFI_CERT_X509_GUID; +static efi_guid_t efi_cert_x509_sha256_guid __initdata = + EFI_CERT_X509_SHA256_GUID; +static efi_guid_t efi_cert_sha256_guid __initdata = EFI_CERT_SHA256_GUID; + +/* + * Blacklist a hash. + */ +static __init void uefi_blacklist_hash(const char *source, const void *data, + size_t len, const char *type, + size_t type_len) +{ + char *hash, *p; + + hash = kmalloc(type_len + len * 2 + 1, GFP_KERNEL); + if (!hash) + return; + p = memcpy(hash, type, type_len); + p += type_len; + bin2hex(p, data, len); + p += len * 2; + *p = 0; + + mark_hash_blacklisted(hash); + kfree(hash); +} + +/* + * Blacklist an X509 TBS hash. + */ +static __init void uefi_blacklist_x509_tbs(const char *source, + const void *data, size_t len) +{ + uefi_blacklist_hash(source, data, len, "tbs:", 4); +} + +/* + * Blacklist the hash of an executable. + */ +static __init void uefi_blacklist_binary(const char *source, +const void *data, size_t len) +{ + uefi_blacklist_hash(source, data, len, "bin:", 4); +} + +/* + * Return the appropriate handler for particular signature list types found in + * the UEFI db and MokListRT tables. + */ +__init efi_element_handler_t get_handler_for_db(const efi_guid_t *sig_type) +{ + if (efi_guidcmp(*sig_type, efi_cert_x509_guid) == 0) + return add_to_platform_keyring; + return 0; +} + +/* + * Return the appropriate handler for particular signature list types found in + * the UEFI dbx and MokListXRT tables. + */ +__init efi_element_handler_t get_handler_for_dbx(const efi_guid_t *sig_type) +{ + if (efi_guidcmp(*sig_type, efi_cert_x509_sha256_guid) == 0) + return uefi_blacklist_x509_tbs; + if (efi_guidcmp(*sig_type, efi_cert_sha256_guid) == 0) + return uefi_blacklist_binary; + return 0; +} diff --git a/security/integrity/platform_certs/keyring_handler.h b/security/integrity/platform_certs/keyring_handler.h new file mode 100644 index ..2462bfa08fe3 --- /dev/null +++ b/security/integrity/platform_certs/keyring_handler.h @@ -0,0 +1,32 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +#ifndef PLATFORM_CERTS_INTERNAL_H +#define PLATFORM_CERTS_INTERNAL_H + +#include + +void blacklist_hash(const char *source, const void *data, + size_t len, const char *type, + size_t type_len); + +/* + * Blacklist an X509 TBS hash. + */ +void blacklist_x509_tbs(const char *source, const void *data, size_t len); + +/* + * Blacklist the hash of an executable. + */ +void blacklist_binary(const char *source, const void *data, size_t len); + +/* + * Return the handler for p
[PATCH v9 2/4] powerpc: expose secure variables to userspace via sysfs
PowerNV secure variables, which store the keys used for OS kernel verification, are managed by the firmware. These secure variables need to be accessed by the userspace for addition/deletion of the certificates. This patch adds the sysfs interface to expose secure variables for PowerNV secureboot. The users shall use this interface for manipulating the keys stored in the secure variables. Signed-off-by: Nayna Jain Reviewed-by: Greg Kroah-Hartman Signed-off-by: Eric Richter --- Documentation/ABI/testing/sysfs-secvar | 46 ++ arch/powerpc/Kconfig | 11 ++ arch/powerpc/kernel/Makefile | 1 + arch/powerpc/kernel/secvar-sysfs.c | 248 + 4 files changed, 306 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-secvar create mode 100644 arch/powerpc/kernel/secvar-sysfs.c diff --git a/Documentation/ABI/testing/sysfs-secvar b/Documentation/ABI/testing/sysfs-secvar new file mode 100644 index ..feebb8c57294 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-secvar @@ -0,0 +1,46 @@ +What: /sys/firmware/secvar +Date: August 2019 +Contact: Nayna Jain +Description: This directory is created if the POWER firmware supports OS + secureboot, thereby secure variables. It exposes interface + for reading/writing the secure variables + +What: /sys/firmware/secvar/vars +Date: August 2019 +Contact: Nayna Jain +Description: This directory lists all the secure variables that are supported + by the firmware. + +What: /sys/firmware/secvar/format +Date: August 2019 +Contact: Nayna Jain +Description: A string indicating which backend is in use by the firmware. + This determines the format of the variable and the accepted + format of variable updates. + +What: /sys/firmware/secvar/vars/ +Date: August 2019 +Contact: Nayna Jain +Description: Each secure variable is represented as a directory named as + . The variable name is unique and is in ASCII + representation. The data and size can be determined by reading + their respective attribute files. + +What: /sys/firmware/secvar/vars//size +Date: August 2019 +Contact: Nayna Jain +Description: An integer representation of the size of the content of the + variable. In other words, it represents the size of the data. + +What: /sys/firmware/secvar/vars//data +Date: August 2019 +Contact: Nayna Jain h +Description: A read-only file containing the value of the variable. The size + of the file represents the maximum size of the variable data. + +What: /sys/firmware/secvar/vars//update +Date: August 2019 +Contact: Nayna Jain +Description: A write-only file that is used to submit the new value for the + variable. The size of the file represents the maximum size of + the variable data that can be written. diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index c795039bdc73..cabc091f3fe1 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -945,6 +945,17 @@ config PPC_SECURE_BOOT to enable OS secure boot on systems that have firmware support for it. If in doubt say N. +config PPC_SECVAR_SYSFS + bool "Enable sysfs interface for POWER secure variables" + default y + depends on PPC_SECURE_BOOT + depends on SYSFS + help + POWER secure variables are managed and controlled by firmware. + These variables are exposed to userspace via sysfs to enable + read/write operations on these variables. Say Y if you have + secure boot enabled and want to expose variables to userspace. + endmenu config ISA_DMA_API diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile index 3cf26427334f..b216e9f316ee 100644 --- a/arch/powerpc/kernel/Makefile +++ b/arch/powerpc/kernel/Makefile @@ -162,6 +162,7 @@ obj-y += ucall.o endif obj-$(CONFIG_PPC_SECURE_BOOT) += secure_boot.o ima_arch.o secvar-ops.o +obj-$(CONFIG_PPC_SECVAR_SYSFS) += secvar-sysfs.o # Disable GCOV, KCOV & sanitizers in odd or sensitive code GCOV_PROFILE_prom_init.o := n diff --git a/arch/powerpc/kernel/secvar-sysfs.c b/arch/powerpc/kernel/secvar-sysfs.c new file mode 100644 index ..a0a78aba2083 --- /dev/null +++ b/arch/powerpc/kernel/secvar-sysfs.c @@ -0,0 +1,248 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2019 IBM Corporation + * + * This code exposes secure variables to user via sysfs + */ + +#define pr_fmt(fmt) "secvar-sysfs: "fmt + +#include +#include +#include +#include +#include + +#define NAME_MAX_SIZE 1024 + +static struct kobject *secvar_kobj; +static struct kset *secvar
[PATCH v9 1/4] powerpc/powernv: Add OPAL API interface to access secure variable
The X.509 certificates trusted by the platform and required to secure boot the OS kernel are wrapped in secure variables, which are controlled by OPAL. This patch adds firmware/kernel interface to read and write OPAL secure variables based on the unique key. This support can be enabled using CONFIG_OPAL_SECVAR. Signed-off-by: Claudio Carvalho Signed-off-by: Nayna Jain Signed-off-by: Eric Richter --- arch/powerpc/include/asm/opal-api.h | 5 +- arch/powerpc/include/asm/opal.h | 7 ++ arch/powerpc/include/asm/secvar.h| 35 +++ arch/powerpc/kernel/Makefile | 2 +- arch/powerpc/kernel/secvar-ops.c | 16 +++ arch/powerpc/platforms/powernv/Makefile | 2 +- arch/powerpc/platforms/powernv/opal-call.c | 3 + arch/powerpc/platforms/powernv/opal-secvar.c | 140 +++ arch/powerpc/platforms/powernv/opal.c| 3 + 9 files changed, 210 insertions(+), 3 deletions(-) create mode 100644 arch/powerpc/include/asm/secvar.h create mode 100644 arch/powerpc/kernel/secvar-ops.c create mode 100644 arch/powerpc/platforms/powernv/opal-secvar.c diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h index 378e3997845a..c1f25a760eb1 100644 --- a/arch/powerpc/include/asm/opal-api.h +++ b/arch/powerpc/include/asm/opal-api.h @@ -211,7 +211,10 @@ #define OPAL_MPIPL_UPDATE 173 #define OPAL_MPIPL_REGISTER_TAG174 #define OPAL_MPIPL_QUERY_TAG 175 -#define OPAL_LAST 175 +#define OPAL_SECVAR_GET176 +#define OPAL_SECVAR_GET_NEXT 177 +#define OPAL_SECVAR_ENQUEUE_UPDATE 178 +#define OPAL_LAST 178 #define QUIESCE_HOLD 1 /* Spin all calls at entry */ #define QUIESCE_REJECT 2 /* Fail all calls with OPAL_BUSY */ diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h index a0cf8fba4d12..9986ac34b8e2 100644 --- a/arch/powerpc/include/asm/opal.h +++ b/arch/powerpc/include/asm/opal.h @@ -298,6 +298,13 @@ int opal_sensor_group_clear(u32 group_hndl, int token); int opal_sensor_group_enable(u32 group_hndl, int token, bool enable); int opal_nx_coproc_init(uint32_t chip_id, uint32_t ct); +int opal_secvar_get(const char *key, uint64_t key_len, u8 *data, + uint64_t *data_size); +int opal_secvar_get_next(const char *key, uint64_t *key_len, +uint64_t key_buf_size); +int opal_secvar_enqueue_update(const char *key, uint64_t key_len, u8 *data, + uint64_t data_size); + s64 opal_mpipl_update(enum opal_mpipl_ops op, u64 src, u64 dest, u64 size); s64 opal_mpipl_register_tag(enum opal_mpipl_tags tag, u64 addr); s64 opal_mpipl_query_tag(enum opal_mpipl_tags tag, u64 *addr); diff --git a/arch/powerpc/include/asm/secvar.h b/arch/powerpc/include/asm/secvar.h new file mode 100644 index ..4cc35b58b986 --- /dev/null +++ b/arch/powerpc/include/asm/secvar.h @@ -0,0 +1,35 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (C) 2019 IBM Corporation + * Author: Nayna Jain + * + * PowerPC secure variable operations. + */ +#ifndef SECVAR_OPS_H +#define SECVAR_OPS_H + +#include +#include + +extern const struct secvar_operations *secvar_ops; + +struct secvar_operations { + int (*get)(const char *key, uint64_t key_len, u8 *data, + uint64_t *data_size); + int (*get_next)(const char *key, uint64_t *key_len, + uint64_t keybufsize); + int (*set)(const char *key, uint64_t key_len, u8 *data, + uint64_t data_size); +}; + +#ifdef CONFIG_PPC_SECURE_BOOT + +extern void set_secvar_ops(const struct secvar_operations *ops); + +#else + +static inline void set_secvar_ops(const struct secvar_operations *ops) { } + +#endif + +#endif diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile index e8eb2955b7d5..3cf26427334f 100644 --- a/arch/powerpc/kernel/Makefile +++ b/arch/powerpc/kernel/Makefile @@ -161,7 +161,7 @@ ifneq ($(CONFIG_PPC_POWERNV)$(CONFIG_PPC_SVM),) obj-y += ucall.o endif -obj-$(CONFIG_PPC_SECURE_BOOT) += secure_boot.o ima_arch.o +obj-$(CONFIG_PPC_SECURE_BOOT) += secure_boot.o ima_arch.o secvar-ops.o # Disable GCOV, KCOV & sanitizers in odd or sensitive code GCOV_PROFILE_prom_init.o := n diff --git a/arch/powerpc/kernel/secvar-ops.c b/arch/powerpc/kernel/secvar-ops.c new file mode 100644 index ..4cfa7dbd8850 --- /dev/null +++ b/arch/powerpc/kernel/secvar-ops.c @@ -0,0 +1,16 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2019 IBM Corporation + * Author: Nayna Jain + * + * This file initializes secvar operations for PowerPC Secureboot + */ + +#include + +const struct secvar_operations *secvar_ops; + +void set_secvar_ops(const st