Re: [PATCH v8 5/8] ima: make process_buffer_measurement() generic
On Sat, 2019-10-19 at 14:06 -0400, Nayna Jain wrote: > process_buffer_measurement() is limited to measuring the kexec boot > command line. This patch makes process_buffer_measurement() more > generic, allowing it to measure other types of buffer data (e.g. > blacklisted binary hashes or key hashes). based on "func". > > This patch modifies the function to conditionally retrieve the policy > defined pcr and template based on the func. This would be done in a subsequent patch, not here. > @@ -642,19 +642,38 @@ static void process_buffer_measurement(const void *buf, > int size, > .filename = eventname, > .buf = buf, > .buf_len = size}; > - struct ima_template_desc *template_desc = NULL; > + struct ima_template_desc *template = NULL; > struct { > struct ima_digest_data hdr; > char digest[IMA_MAX_DIGEST_SIZE]; > } hash = {}; > int violation = 0; > - int pcr = CONFIG_IMA_MEASURE_PCR_IDX; > int action = 0; > + u32 secid; > > - action = ima_get_action(NULL, cred, secid, 0, KEXEC_CMDLINE, &pcr, > - &template_desc); > - if (!(action & IMA_MEASURE)) > - return; > + if (func) { > + security_task_getsecid(current, &secid); > + action = ima_get_action(NULL, current_cred(), secid, 0, func, > + &pcr, &template); > + if (!(action & IMA_MEASURE)) > + return; > + } > + Initially there is no need to test "func". A specific "func" test would be added as needed. Mimi
Re: [PATCH v8 7/8] ima: check against blacklisted hashes for files with modsig
On Sat, 2019-10-19 at 14:06 -0400, Nayna Jain wrote: > diff --git a/Documentation/ABI/testing/ima_policy > b/Documentation/ABI/testing/ima_policy > index 29ebe9afdac4..4c97afcc0f3c 100644 > --- a/Documentation/ABI/testing/ima_policy > +++ b/Documentation/ABI/testing/ima_policy > @@ -25,6 +25,7 @@ Description: > lsm:[[subj_user=] [subj_role=] [subj_type=] >[obj_user=] [obj_role=] [obj_type=]] > option: [[appraise_type=]] [template=] [permit_directio] > + [appraise_flag=[check_blacklist]] Like the other options, only "[[appraise_flag=]]" should be defined here. The values should be defined in the "option:" section. > base: func:= > [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK] > [FIRMWARE_CHECK] > > [KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK] > diff --git a/security/integrity/ima/ima_appraise.c > b/security/integrity/ima/ima_appraise.c > index 136ae4e0ee92..7a002b08dde8 100644 > --- a/security/integrity/ima/ima_appraise.c > +++ b/security/integrity/ima/ima_appraise.c > @@ -303,6 +304,36 @@ static int modsig_verify(enum ima_hooks func, const > struct modsig *modsig, > return rc; > } > > +/* > + * ima_blacklist_measurement - Checks whether the binary is blacklisted. If Please update the function name to reflect the actual function name. > + * yes, then adds the hash of the blacklisted binary to the measurement list. Refer to Documentation/process/coding-style.rst section "8) Commenting" on how to format function comments. Don't start a sentence with "If yes,". > + * > + * Returns -EPERM if the hash is blacklisted. > + */ > +int ima_check_blacklist(struct integrity_iint_cache *iint, > + const struct modsig *modsig, int pcr) > +{ > + enum hash_algo hash_algo; > diff --git a/security/integrity/ima/ima_policy.c > b/security/integrity/ima/ima_policy.c > index 5380aca2b351..bfaae7a8443a 100644 > --- a/security/integrity/ima/ima_policy.c > +++ b/security/integrity/ima/ima_policy.c > @@ -1172,6 +1173,11 @@ static int ima_parse_rule(char *rule, struct > ima_rule_entry *entry) > else > result = -EINVAL; > break; > + case Opt_appraise_flag: > + ima_log_string(ab, "appraise_flag", args[0].from); > + if (strstr(args[0].from, "blacklist")) > + entry->flags |= IMA_CHECK_BLACKLIST; > + break; When adding a new policy rule option, ima_policy_show() needs to be updated as well. Mimi > case Opt_permit_directio: > entry->flags |= IMA_PERMIT_DIRECTIO; > break; >
Re: [PATCH v8 2/8] powerpc/ima: add support to initialize ima policy rules
On Sat, 2019-10-19 at 14:06 -0400, Nayna Jain wrote: > index ..65d82ee74ea4 > --- /dev/null > +++ b/arch/powerpc/kernel/ima_arch.c > @@ -0,0 +1,39 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2019 IBM Corporation > + * Author: Nayna Jain > + */ > + > +#include > +#include > + > +bool arch_ima_get_secureboot(void) > +{ > + return is_ppc_secureboot_enabled(); > +} > + > +/* > + * The "secure_rules" are enabled only on "secureboot" enabled systems. > + * These rules verify the file signatures against known good values. > + * The "appraise_type=imasig|modsig" option allows the known good signature > + * to be stored as an xattr or as an appended signature. Please add another sentence or two as a separate paragraph with an explanation why the kernel module rule is conditional (eg. Only verify the appended kernel module signatures once.) > + */ > +static const char *const secure_rules[] = { > + "appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig|modsig", > +#ifndef CONFIG_MODULE_SIG_FORCE > + "appraise func=MODULE_CHECK appraise_type=imasig|modsig", > +#endif > + NULL > +}; > + Mimi
Re: [PATCH v8 4/8] powerpc/ima: add measurement rules to ima arch specific policy
On Sat, 2019-10-19 at 14:06 -0400, Nayna Jain wrote: > This patch adds the measurement rules to the arch specific policies on > trusted boot enabled systems. This version does not add rules to the existing arch specific policy, but defines an arch specific trusted boot only policy and a combined secure and trusted boot policy. > > Signed-off-by: Nayna Jain > --- > arch/powerpc/kernel/ima_arch.c | 34 +- > 1 file changed, 33 insertions(+), 1 deletion(-) > > diff --git a/arch/powerpc/kernel/ima_arch.c b/arch/powerpc/kernel/ima_arch.c > index 65d82ee74ea4..710872ea8f35 100644 > --- a/arch/powerpc/kernel/ima_arch.c > +++ b/arch/powerpc/kernel/ima_arch.c > @@ -26,6 +26,32 @@ static const char *const secure_rules[] = { > NULL > }; > > +/* > + * The "measure_rules" are enabled only on "trustedboot" enabled systems. Please update the policy name to reflect the new "trusted_rules" name. > + * These rules add the kexec kernel image and kernel modules file hashes to > + * the IMA measurement list. > + */ > +static const char *const trusted_rules[] = { > + "measure func=KEXEC_KERNEL_CHECK", > + "measure func=MODULE_CHECK", > + NULL > +}; > + > +/* > + * The "secure_and_trusted_rules" contains rules for both the secure boot and > + * trusted boot. The "template=ima-modsig" option includes the appended > + * signature, when available, in the IMA measurement list. > + */ > +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_type=imasig|modsig", > +#ifndef CONFIG_MODULE_SIG_FORCE > + "appraise func=MODULE_CHECK appraise_type=imasig|modsig", > +#endif > + NULL > +}; > + > /* > * Returns the relevant IMA arch-specific policies based on the system secure > * boot state. > @@ -33,7 +59,13 @@ static const char *const secure_rules[] = { > const char *const *arch_get_ima_policy(void) > { > if (is_ppc_secureboot_enabled()) > - return secure_rules; > + if (is_ppc_trustedboot_enabled()) > + return secure_and_trusted_rules; > + else > + return secure_rules; > + else > + if (is_ppc_trustedboot_enabled()) No need for the "if" statement to be on a separate line. Please combine the "else" and "if" statements. Mimi > + return trusted_rules; > > return NULL; > }
Re: passing NULL to clock_getres (VDSO): terminated by unexpected signal 11
On Okt 19 2019, Christophe Leroy wrote: > Hi Nathan, > > While trying to switch powerpc VDSO to C version of gettimeofday(), I'm > getting the following kind of error with vdsotest: > > passing NULL to clock_getres (VDSO): terminated by unexpected signal 11 > > Looking at commit a9446a906f52 ("lib/vdso/32: Remove inconsistent NULL > pointer checks"), it seems that signal 11 is expected when passing NULL > pointer. > > Any plan to fix vdsotest ? Passing NULL to clock_getres is valid, and required to return successfully if the clock id is valid. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different."
Re: [PATCH v7 7/8] ima: check against blacklisted hashes for files with modsig
Hi Mimi, On 10/11/2019 09:19 AM, Mimi Zohar wrote: On Mon, 2019-10-07 at 21:14 -0400, Nayna Jain wrote: Asymmetric private keys are used to sign multiple files. The kernel currently support checking against the blacklisted keys. However, if the public key is blacklisted, any file signed by the blacklisted key will automatically fail signature verification. We might not want to blacklist all the files signed by a particular key, but just a single file. Blacklisting the public key is not fine enough granularity. This patch adds support for blacklisting binaries with appended signatures, based on the IMA policy. Defined is a new policy option "appraise_flag=check_blacklist". The blacklisted hash is not the same as the file hash, but is the file hash without the appended signature. Are there tools for calculating the blacklisted hash? Can you provide an example? I have updated the patch description to specify that the blacklisted hash is the file hash without the appended signature. I hope that makes it clear now. Thanks & Regards, - Nayna
Re: [PATCH v7 4/8] powerpc/ima: add measurement rules to ima arch specific policy
Hi Michael, On 10/15/2019 07:29 AM, Michael Ellerman wrote: Nayna Jain writes: This patch adds the measurement rules to the arch specific policies on trusted boot enabled systems. Signed-off-by: Nayna Jain Reviewed-by: Mimi Zohar --- arch/powerpc/kernel/ima_arch.c | 45 +++--- 1 file changed, 42 insertions(+), 3 deletions(-) diff --git a/arch/powerpc/kernel/ima_arch.c b/arch/powerpc/kernel/ima_arch.c index c22d82965eb4..88bfe4a1a9a5 100644 --- a/arch/powerpc/kernel/ima_arch.c +++ b/arch/powerpc/kernel/ima_arch.c @@ -12,8 +12,19 @@ bool arch_ima_get_secureboot(void) return is_powerpc_os_secureboot_enabled(); } -/* Defines IMA appraise rules for secureboot */ +/* + * The "arch_rules" contains both the securebot and trustedboot rules for adding + * the kexec kernel image and kernel modules file hashes to the IMA measurement + * list and verifying the file signatures against known good values. + * + * The "appraise_type=imasig|modsig" option allows the good signature to be + * stored as an xattr or as an appended signature. The "template=ima-modsig" + * option includes the appended signature, when available, in the IMA + * measurement list. + */ static const char *const arch_rules[] = { + "measure func=KEXEC_KERNEL_CHECK template=ima-modsig", + "measure func=MODULE_CHECK template=ima-modsig", "appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig|modsig", #if !IS_ENABLED(CONFIG_MODULE_SIG_FORCE) "appraise func=MODULE_CHECK appraise_type=imasig|modsig", @@ -22,12 +33,40 @@ static const char *const arch_rules[] = { }; /* - * Returns the relevant IMA arch policies based on the system secureboot state. + * The "measure_rules" are enabled only on "trustedboot" enabled systems. + * These rules add the kexec kernel image and kernel modules file hashes to + * the IMA measurement list. + */ +static const char *const measure_rules[] = { + "measure func=KEXEC_KERNEL_CHECK", + "measure func=MODULE_CHECK", Why do these ones not have "template=ima-modsig" on the end? ima-modsig template is applicable only when IMA "collects" the appended signatures. IMA can then include it in the measurement list. + NULL +}; + +/* + * Returns the relevant IMA arch policies based on the system secureboot + * and trustedboot state. */ const char *const *arch_get_ima_policy(void) { - if (is_powerpc_os_secureboot_enabled()) + const char *const *rules; + int offset = 0; + + for (rules = arch_rules; *rules != NULL; rules++) { + if (strncmp(*rules, "appraise", 8) == 0) + break; + offset++; + } This seems like kind of a hack, doesn't it? :) What we really want is three sets of rules isn't it? But some of them are shared between the different sets. But they just have to be flat arrays of strings. I think it would probably be cleaner to just use a #define for the shared part of the rules, eg something like: #ifdef CONFIG_MODULE_SIG_FORCE #define APPRAISE_MODULE #else #define APPRAISE_MODULE \ "appraise func=MODULE_CHECK appraise_flag=check_blacklist appraise_type=imasig|modsig", #endif #define APPRAISE_KERNEL \ "appraise func=KEXEC_KERNEL_CHECK appraise_flag=check_blacklist appraise_type=imasig|modsig" #define MEASURE_KERNEL \ "measure func=KEXEC_KERNEL_CHECK" #define MEASURE_MODULE \ "measure func=MODULE_CHECK" #define APPEND_TEMPLATE_IMA_MODSIG \ " template=ima-modsig" static const char *const secure_and_trusted_rules[] = { MEASURE_KERNEL APPEND_TEMPLATE_IMA_MODSIG, MEASURE_MODULE APPEND_TEMPLATE_IMA_MODSIG, APPRAISE_KERNEL, APPRAISE_MODULE NULL }; static const char *const secure_rules[] = { APPRAISE_KERNEL, APPRAISE_MODULE NULL }; static const char *const trusted_rules[] = { MEASURE_KERNEL, MEASURE_MODULE, NULL }; Yes, I agree it is sort of a hack to walk through the rules to find the start of the appraise policy. While trying your suggestion, I realized that defining three arrays, with some rule duplication, can fix the hack without #defines. This also improves readability of the rules. I have just now posted the new version with the changes. Please let me know if this looks ok. Thanks & Regards, - Nayna
[PATCH v8 2/8] powerpc/ima: add support to initialize ima policy rules
PowerNV system use a Linux-based bootloader, which relies on the IMA subsystem to enforce different secure boot modes. Since the verification policy may differ based on the secure boot mode of the system, the policies must be defined at runtime. This patch implements arch-specific support to define IMA policy rules based on the runtime secure boot mode of the system. This patch provides arch-specific IMA policies if PPC_SECURE_BOOT config is enabled. Signed-off-by: Nayna Jain --- arch/powerpc/Kconfig | 1 + arch/powerpc/kernel/Makefile | 2 +- arch/powerpc/kernel/ima_arch.c | 39 ++ include/linux/ima.h| 3 ++- 4 files changed, 43 insertions(+), 2 deletions(-) create mode 100644 arch/powerpc/kernel/ima_arch.c diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 56ea0019b616..c795039bdc73 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -938,6 +938,7 @@ 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/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile index e2a54fa240ac..e8eb2955b7d5 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 +obj-$(CONFIG_PPC_SECURE_BOOT) += secure_boot.o ima_arch.o # Disable GCOV, KCOV & sanitizers in odd or sensitive code GCOV_PROFILE_prom_init.o := n diff --git a/arch/powerpc/kernel/ima_arch.c b/arch/powerpc/kernel/ima_arch.c new file mode 100644 index ..65d82ee74ea4 --- /dev/null +++ b/arch/powerpc/kernel/ima_arch.c @@ -0,0 +1,39 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2019 IBM Corporation + * Author: Nayna Jain + */ + +#include +#include + +bool arch_ima_get_secureboot(void) +{ + return is_ppc_secureboot_enabled(); +} + +/* + * The "secure_rules" are enabled only on "secureboot" enabled systems. + * These rules verify the file signatures against known good values. + * The "appraise_type=imasig|modsig" option allows the known good signature + * to be stored as an xattr or as an appended signature. + */ +static const char *const secure_rules[] = { + "appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig|modsig", +#ifndef CONFIG_MODULE_SIG_FORCE + "appraise func=MODULE_CHECK appraise_type=imasig|modsig", +#endif + NULL +}; + +/* + * Returns the relevant IMA arch-specific policies based on the system secure + * boot state. + */ +const char *const *arch_get_ima_policy(void) +{ + if (is_ppc_secureboot_enabled()) + return secure_rules; + + return NULL; +} diff --git a/include/linux/ima.h b/include/linux/ima.h index 1c37f17f7203..6d904754d858 100644 --- a/include/linux/ima.h +++ b/include/linux/ima.h @@ -29,7 +29,8 @@ 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) +#if (defined(CONFIG_X86) && defined(CONFIG_EFI)) || defined(CONFIG_S390) \ + || defined(CONFIG_PPC_SECURE_BOOT) extern bool arch_ima_get_secureboot(void); extern const char * const *arch_get_ima_policy(void); #else -- 2.20.1
[PATCH v8 8/8] powerpc/ima: update ima arch policy to check for blacklist
This patch updates the arch-specific policies for PowerNV system to make sure that the binary hash is not blacklisted. Signed-off-by: Nayna Jain Reviewed-by: Mimi Zohar --- arch/powerpc/kernel/ima_arch.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/arch/powerpc/kernel/ima_arch.c b/arch/powerpc/kernel/ima_arch.c index 710872ea8f35..98f3f8d9f9e4 100644 --- a/arch/powerpc/kernel/ima_arch.c +++ b/arch/powerpc/kernel/ima_arch.c @@ -19,9 +19,9 @@ bool arch_ima_get_secureboot(void) * to be stored as an xattr or as an appended signature. */ static const char *const secure_rules[] = { - "appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig|modsig", + "appraise func=KEXEC_KERNEL_CHECK appraise_flag=check_blacklist appraise_type=imasig|modsig", #ifndef CONFIG_MODULE_SIG_FORCE - "appraise func=MODULE_CHECK appraise_type=imasig|modsig", + "appraise func=MODULE_CHECK appraise_flag=check_blacklist appraise_type=imasig|modsig", #endif NULL }; @@ -45,9 +45,9 @@ static const char *const trusted_rules[] = { 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_type=imasig|modsig", + "appraise func=KEXEC_KERNEL_CHECK appraise_flag=check_blacklist appraise_type=imasig|modsig", #ifndef CONFIG_MODULE_SIG_FORCE - "appraise func=MODULE_CHECK appraise_type=imasig|modsig", + "appraise func=MODULE_CHECK appraise_flag=check_blacklist appraise_type=imasig|modsig", #endif NULL }; -- 2.20.1
[PATCH v8 7/8] ima: check against blacklisted hashes for files with modsig
Asymmetric private keys are used to sign multiple files. The kernel currently support checking against blacklisted keys. However, if the public key is blacklisted, any file signed by the blacklisted key will automatically fail signature verification. We might not want to blacklist all the files signed by a particular key, but just a single file. Blacklisting the public key is not fine enough granularity. This patch adds support for checking against the blacklisted hash of the file based on the IMA policy. The blacklisted hash is the file hash without the appended signature. Defined is a new policy option "appraise_flag=check_blacklist". Signed-off-by: Nayna Jain --- Documentation/ABI/testing/ima_policy | 1 + security/integrity/ima/ima.h | 8 +++ security/integrity/ima/ima_appraise.c | 31 +++ security/integrity/ima/ima_main.c | 12 +++ security/integrity/ima/ima_policy.c | 10 +++-- security/integrity/integrity.h| 1 + 6 files changed, 57 insertions(+), 6 deletions(-) diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy index 29ebe9afdac4..4c97afcc0f3c 100644 --- a/Documentation/ABI/testing/ima_policy +++ b/Documentation/ABI/testing/ima_policy @@ -25,6 +25,7 @@ Description: lsm:[[subj_user=] [subj_role=] [subj_type=] [obj_user=] [obj_role=] [obj_type=]] option: [[appraise_type=]] [template=] [permit_directio] + [appraise_flag=[check_blacklist]] base: func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK] [FIRMWARE_CHECK] [KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK] diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index a65772ffa427..df4ca482fb53 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -256,6 +256,8 @@ int ima_policy_show(struct seq_file *m, void *v); #define IMA_APPRAISE_KEXEC 0x40 #ifdef CONFIG_IMA_APPRAISE +int ima_check_blacklist(struct integrity_iint_cache *iint, + const struct modsig *modsig, int pcr); int ima_appraise_measurement(enum ima_hooks func, struct integrity_iint_cache *iint, struct file *file, const unsigned char *filename, @@ -271,6 +273,12 @@ int ima_read_xattr(struct dentry *dentry, struct evm_ima_xattr_data **xattr_value); #else +static inline int ima_check_blacklist(struct integrity_iint_cache *iint, + const struct modsig *modsig, int pcr) +{ + return 0; +} + static inline int ima_appraise_measurement(enum ima_hooks func, struct integrity_iint_cache *iint, struct file *file, diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 136ae4e0ee92..7a002b08dde8 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -12,6 +12,7 @@ #include #include #include +#include #include "ima.h" @@ -303,6 +304,36 @@ static int modsig_verify(enum ima_hooks func, const struct modsig *modsig, return rc; } +/* + * ima_blacklist_measurement - Checks whether the binary is blacklisted. If + * yes, then adds the hash of the blacklisted binary to the measurement list. + * + * Returns -EPERM if the hash is blacklisted. + */ +int ima_check_blacklist(struct integrity_iint_cache *iint, + const struct modsig *modsig, int pcr) +{ + enum hash_algo hash_algo; + const u8 *digest = NULL; + u32 digestsize = 0; + int rc = 0; + + if (!(iint->flags & IMA_CHECK_BLACKLIST)) + return 0; + + if (iint->flags & IMA_MODSIG_ALLOWED && modsig) { + ima_get_modsig_digest(modsig, &hash_algo, &digest, &digestsize); + + rc = is_binary_blacklisted(digest, digestsize); + if ((rc == -EPERM) && (iint->flags & IMA_MEASURE)) + process_buffer_measurement(digest, digestsize, + "blacklisted-hash", NONE, + pcr); + } + + return rc; +} + /* * ima_appraise_measurement - appraise file measurement * diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index fe0b704ffdeb..13a0d64580ef 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -335,10 +335,14 @@ static int process_measurement(struct file *file, const struct cred *cred, xattr_value, xattr_len, modsig, pcr, template_desc); if (rc == 0 && (action & IMA_APPRAISE_SUBMASK)) { -
[PATCH v8 6/8] certs: add wrapper function to check blacklisted binary hash
The -EKEYREJECTED error returned by existing is_hash_blacklisted() is misleading when called for checking against blacklisted hash of a binary. This patch adds a wrapper function is_binary_blacklisted() to return -EPERM error if binary is blacklisted. Signed-off-by: Nayna Jain Reviewed-by: Mimi Zohar --- certs/blacklist.c | 9 + include/keys/system_keyring.h | 6 ++ 2 files changed, 15 insertions(+) diff --git a/certs/blacklist.c b/certs/blacklist.c index ec00bf337eb6..6514f9ebc943 100644 --- a/certs/blacklist.c +++ b/certs/blacklist.c @@ -135,6 +135,15 @@ int is_hash_blacklisted(const u8 *hash, size_t hash_len, const char *type) } EXPORT_SYMBOL_GPL(is_hash_blacklisted); +int is_binary_blacklisted(const u8 *hash, size_t hash_len) +{ + if (is_hash_blacklisted(hash, hash_len, "bin") == -EKEYREJECTED) + return -EPERM; + + return 0; +} +EXPORT_SYMBOL_GPL(is_binary_blacklisted); + /* * Initialise the blacklist */ diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h index c1a96fdf598b..fb8b07daa9d1 100644 --- a/include/keys/system_keyring.h +++ b/include/keys/system_keyring.h @@ -35,12 +35,18 @@ extern int restrict_link_by_builtin_and_secondary_trusted( extern int mark_hash_blacklisted(const char *hash); extern int is_hash_blacklisted(const u8 *hash, size_t hash_len, const char *type); +extern int is_binary_blacklisted(const u8 *hash, size_t hash_len); #else static inline int is_hash_blacklisted(const u8 *hash, size_t hash_len, const char *type) { return 0; } + +static inline int is_binary_blacklisted(const u8 *hash, size_t hash_len) +{ + return 0; +} #endif #ifdef CONFIG_IMA_BLACKLIST_KEYRING -- 2.20.1
[PATCH v8 5/8] ima: make process_buffer_measurement() generic
process_buffer_measurement() is limited to measuring the kexec boot command line. This patch makes process_buffer_measurement() more generic, allowing it to measure other types of buffer data (e.g. blacklisted binary hashes or key hashes). This patch modifies the function to conditionally retrieve the policy defined pcr and template based on the func. Signed-off-by: Nayna Jain --- security/integrity/ima/ima.h | 3 ++ security/integrity/ima/ima_main.c | 51 --- 2 files changed, 36 insertions(+), 18 deletions(-) diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 3689081aaf38..a65772ffa427 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -217,6 +217,9 @@ void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file, struct evm_ima_xattr_data *xattr_value, int xattr_len, const struct modsig *modsig, int pcr, struct ima_template_desc *template_desc); +void process_buffer_measurement(const void *buf, int size, + const char *eventname, enum ima_hooks func, + int pcr); void ima_audit_measurement(struct integrity_iint_cache *iint, const unsigned char *filename); int ima_alloc_init_template(struct ima_event_data *event_data, diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 60027c643ecd..fe0b704ffdeb 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -626,14 +626,14 @@ int ima_load_data(enum kernel_load_data_id id) * @buf: pointer to the buffer that needs to be added to the log. * @size: size of buffer(in bytes). * @eventname: event name to be used for the buffer entry. - * @cred: a pointer to a credentials structure for user validation. - * @secid: the secid of the task to be validated. + * @func: IMA hook + * @pcr: pcr to extend the measurement * * Based on policy, the buffer is measured into the ima log. */ -static void process_buffer_measurement(const void *buf, int size, - const char *eventname, - const struct cred *cred, u32 secid) +void process_buffer_measurement(const void *buf, int size, + const char *eventname, enum ima_hooks func, + int pcr) { int ret = 0; struct ima_template_entry *entry = NULL; @@ -642,19 +642,38 @@ static void process_buffer_measurement(const void *buf, int size, .filename = eventname, .buf = buf, .buf_len = size}; - struct ima_template_desc *template_desc = NULL; + struct ima_template_desc *template = NULL; struct { struct ima_digest_data hdr; char digest[IMA_MAX_DIGEST_SIZE]; } hash = {}; int violation = 0; - int pcr = CONFIG_IMA_MEASURE_PCR_IDX; int action = 0; + u32 secid; - action = ima_get_action(NULL, cred, secid, 0, KEXEC_CMDLINE, &pcr, - &template_desc); - if (!(action & IMA_MEASURE)) - return; + if (func) { + security_task_getsecid(current, &secid); + action = ima_get_action(NULL, current_cred(), secid, 0, func, + &pcr, &template); + if (!(action & IMA_MEASURE)) + return; + } + + if (!pcr) + pcr = CONFIG_IMA_MEASURE_PCR_IDX; + + if (!template) { + template = lookup_template_desc("ima-buf"); + ret = template_desc_init_fields(template->fmt, + &(template->fields), + &(template->num_fields)); + if (ret < 0) { + pr_err("template %s init failed, result: %d\n", + (strlen(template->name) ? + template->name : template->fmt), ret); + return; + } + } iint.ima_hash = &hash.hdr; iint.ima_hash->algo = ima_hash_algo; @@ -664,7 +683,7 @@ static void process_buffer_measurement(const void *buf, int size, if (ret < 0) goto out; - ret = ima_alloc_init_template(&event_data, &entry, template_desc); + ret = ima_alloc_init_template(&event_data, &entry, template); if (ret < 0) goto out; @@ -686,13 +705,9 @@ static void process_buffer_measurement(const void *buf, int size, */ void ima_kexec_cmdline(const void *buf, int size) { - u32 secid; - - if (buf && size != 0) { - security_task_getsecid(current, &secid)
[PATCH v8 4/8] powerpc/ima: add measurement rules to ima arch specific policy
This patch adds the measurement rules to the arch specific policies on trusted boot enabled systems. Signed-off-by: Nayna Jain --- arch/powerpc/kernel/ima_arch.c | 34 +- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/arch/powerpc/kernel/ima_arch.c b/arch/powerpc/kernel/ima_arch.c index 65d82ee74ea4..710872ea8f35 100644 --- a/arch/powerpc/kernel/ima_arch.c +++ b/arch/powerpc/kernel/ima_arch.c @@ -26,6 +26,32 @@ static const char *const secure_rules[] = { NULL }; +/* + * The "measure_rules" are enabled only on "trustedboot" enabled systems. + * These rules add the kexec kernel image and kernel modules file hashes to + * the IMA measurement list. + */ +static const char *const trusted_rules[] = { + "measure func=KEXEC_KERNEL_CHECK", + "measure func=MODULE_CHECK", + NULL +}; + +/* + * The "secure_and_trusted_rules" contains rules for both the secure boot and + * trusted boot. The "template=ima-modsig" option includes the appended + * signature, when available, in the IMA measurement list. + */ +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_type=imasig|modsig", +#ifndef CONFIG_MODULE_SIG_FORCE + "appraise func=MODULE_CHECK appraise_type=imasig|modsig", +#endif + NULL +}; + /* * Returns the relevant IMA arch-specific policies based on the system secure * boot state. @@ -33,7 +59,13 @@ static const char *const secure_rules[] = { const char *const *arch_get_ima_policy(void) { if (is_ppc_secureboot_enabled()) - return secure_rules; + if (is_ppc_trustedboot_enabled()) + return secure_and_trusted_rules; + else + return secure_rules; + else + if (is_ppc_trustedboot_enabled()) + return trusted_rules; return NULL; } -- 2.20.1
[PATCH v8 3/8] powerpc: detect the trusted boot state of the system
While secure boot permits only properly verified signed kernels to be booted, trusted boot takes a measurement of the kernel image prior to boot that can be subsequently compared against good known values via attestation services. This patch reads the trusted boot state of a PowerNV system. The state is used to conditionally enable additional measurement rules in the IMA arch-specific policies. Signed-off-by: Nayna Jain --- arch/powerpc/include/asm/secure_boot.h | 6 ++ arch/powerpc/kernel/secure_boot.c | 24 2 files changed, 30 insertions(+) diff --git a/arch/powerpc/include/asm/secure_boot.h b/arch/powerpc/include/asm/secure_boot.h index 07d0fe0ca81f..a2ff556916c6 100644 --- a/arch/powerpc/include/asm/secure_boot.h +++ b/arch/powerpc/include/asm/secure_boot.h @@ -11,6 +11,7 @@ #ifdef CONFIG_PPC_SECURE_BOOT bool is_ppc_secureboot_enabled(void); +bool is_ppc_trustedboot_enabled(void); #else @@ -19,5 +20,10 @@ static inline bool is_ppc_secureboot_enabled(void) return false; } +static inline bool is_ppc_trustedboot_enabled(void) +{ + return false; +} + #endif #endif diff --git a/arch/powerpc/kernel/secure_boot.c b/arch/powerpc/kernel/secure_boot.c index 99bba7915629..9753470ab08a 100644 --- a/arch/powerpc/kernel/secure_boot.c +++ b/arch/powerpc/kernel/secure_boot.c @@ -7,6 +7,17 @@ #include #include +static struct device_node *get_ppc_fw_sb_node(void) +{ + static const struct of_device_id ids[] = { + { .compatible = "ibm,secureboot-v1", }, + { .compatible = "ibm,secureboot-v2", }, + {}, + }; + + return of_find_matching_node(NULL, ids); +} + bool is_ppc_secureboot_enabled(void) { struct device_node *node; @@ -28,3 +39,16 @@ bool is_ppc_secureboot_enabled(void) pr_info("Secure boot mode %s\n", enabled ? "enabled" : "disabled"); return enabled; } + +bool is_ppc_trustedboot_enabled(void) +{ + struct device_node *node; + bool enabled = false; + + node = get_ppc_fw_sb_node(); + enabled = of_property_read_bool(node, "trusted-enabled"); + + pr_info("Trusted boot mode %s\n", enabled ? "enabled" : "disabled"); + + return enabled; +} -- 2.20.1
[PATCH v8 1/8] powerpc: detect the secure boot mode of the system
This patch defines a function to detect the secure boot state of a PowerNV system. The PPC_SECURE_BOOT config represents the base enablement of secure boot for powerpc. Signed-off-by: Nayna Jain --- arch/powerpc/Kconfig | 10 + arch/powerpc/include/asm/secure_boot.h | 23 arch/powerpc/kernel/Makefile | 2 ++ arch/powerpc/kernel/secure_boot.c | 30 ++ 4 files changed, 65 insertions(+) create mode 100644 arch/powerpc/include/asm/secure_boot.h create mode 100644 arch/powerpc/kernel/secure_boot.c diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 3e56c9c2f16e..56ea0019b616 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -934,6 +934,16 @@ config PPC_MEM_KEYS If unsure, say y. +config PPC_SECURE_BOOT + prompt "Enable secure boot support" + bool + depends on PPC_POWERNV + help + Systems with firmware secure boot enabled need to define security + policies to extend secure boot to the OS. This config allows a user + to enable OS secure boot on systems that have firmware support for + it. If in doubt say N. + endmenu config ISA_DMA_API diff --git a/arch/powerpc/include/asm/secure_boot.h b/arch/powerpc/include/asm/secure_boot.h new file mode 100644 index ..07d0fe0ca81f --- /dev/null +++ b/arch/powerpc/include/asm/secure_boot.h @@ -0,0 +1,23 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Secure boot definitions + * + * Copyright (C) 2019 IBM Corporation + * Author: Nayna Jain + */ +#ifndef _ASM_POWER_SECURE_BOOT_H +#define _ASM_POWER_SECURE_BOOT_H + +#ifdef CONFIG_PPC_SECURE_BOOT + +bool is_ppc_secureboot_enabled(void); + +#else + +static inline bool is_ppc_secureboot_enabled(void) +{ + return false; +} + +#endif +#endif diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile index a7ca8fe62368..e2a54fa240ac 100644 --- a/arch/powerpc/kernel/Makefile +++ b/arch/powerpc/kernel/Makefile @@ -161,6 +161,8 @@ ifneq ($(CONFIG_PPC_POWERNV)$(CONFIG_PPC_SVM),) obj-y += ucall.o endif +obj-$(CONFIG_PPC_SECURE_BOOT) += secure_boot.o + # Disable GCOV, KCOV & sanitizers in odd or sensitive code GCOV_PROFILE_prom_init.o := n KCOV_INSTRUMENT_prom_init.o := n diff --git a/arch/powerpc/kernel/secure_boot.c b/arch/powerpc/kernel/secure_boot.c new file mode 100644 index ..99bba7915629 --- /dev/null +++ b/arch/powerpc/kernel/secure_boot.c @@ -0,0 +1,30 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2019 IBM Corporation + * Author: Nayna Jain + */ +#include +#include +#include + +bool is_ppc_secureboot_enabled(void) +{ + struct device_node *node; + bool enabled = false; + + node = of_find_compatible_node(NULL, NULL, "ibm,secvar-v1"); + if (!of_device_is_available(node)) { + pr_err("Cannot find secure variable node in device tree; failing to secure state\n"); + goto out; + } + + /* +* secureboot is enabled if os-secure-enforcing property exists, +* else disabled. +*/ + enabled = of_property_read_bool(node, "os-secure-enforcing"); + +out: + pr_info("Secure boot mode %s\n", enabled ? "enabled" : "disabled"); + return enabled; +} -- 2.20.1
[PATCH v8 0/8] powerpc: Enabling IMA arch specific secure boot policies
This patchset extends the previous version[1] by adding support for checking against a blacklist of binary hashes. The IMA subsystem supports custom, built-in, arch-specific policies to define the files to be measured and appraised. These policies are honored based on priority, where arch-specific policy is the highest and custom is the lowest. PowerNV system uses a Linux-based bootloader to kexec the OS. The bootloader kernel relies on IMA for signature verification of the OS kernel before doing the kexec. This patchset adds support for powerpc arch-specific IMA policies that are conditionally defined based on a system's secure boot and trusted boot states. The OS secure boot and trusted boot states are determined via device-tree properties. The verification needs to be performed only for binaries that are not blacklisted. The kernel currently only checks against the blacklist of keys. However, doing so results in blacklisting all the binaries that are signed by the same key. In order to prevent just one particular binary from being loaded, it must be checked against a blacklist of binary hashes. This patchset also adds support to IMA for checking against a hash blacklist for files. signed by appended signature. [1] http://patchwork.ozlabs.org/cover/1149262/ Changelog: v8: * Updates the Patch Description as per Michael's and Mimi's feedback * Includes feedbacks from Michael for the device tree and policies * removes the arch-policy hack by defining three arrays. * fixes related to device-tree calls * other code specific feedbacks * Includes feedbacks from Mimi on the blacklist * generic blacklist function is modified than previous version * other coding fixes v7: * Removes patch related to dt-bindings as per input from Rob Herring. * fixes Patch 1/8 to use new device-tree updates as per Oliver feedback to device-tree documentation in skiboot mailing list. (https://lists.ozlabs.org/pipermail/skiboot/2019-September/015329.html) * Includes feedbacks from Mimi, Thiago * moves function get_powerpc_fw_sb_node() from Patch 1 to Patch 3 * fixes Patch 2/8 to use CONFIG_MODULE_SIG_FORCE. * updates Patch description in Patch 5/8 * adds a new patch to add wrapper is_binary_blacklisted() * removes the patch that deprecated permit_directio v6: * includes feedbacks from Michael Ellerman on the patchset v5 * removed email ids from comments * add the doc for the device-tree * renames the secboot.c to secure_boot.c and secboot.h to secure_boot.h * other code specific fixes * split the patches to differentiate between secureboot and trustedboot state of the system * adds the patches to support the blacklisting of the binary hash. v5: * secureboot state is now read via device tree entry rather than OPAL secure variables * ima arch policies are updated to use policy based template for measurement rules v4: * Fixed the build issue as reported by Satheesh Rajendran. v3: * OPAL APIs in Patch 1 are updated to provide generic interface based on key/keylen. This patchset updates kernel OPAL APIs to be compatible with generic interface. * Patch 2 is cleaned up to use new OPAL APIs. * Since OPAL can support different types of backend which can vary in the variable interpretation, the Patch 2 is updated to add a check for the backend version * OPAL API now expects consumer to first check the supported backend version before calling other secvar OPAL APIs. This check is now added in patch 2. * IMA policies in Patch 3 is updated to specify appended signature and per policy template. * The patches now are free of any EFIisms. v2: * Removed Patch 1: powerpc/include: Override unneeded early ioremap functions * Updated Subject line and patch description of the Patch 1 of this series * Removed dependency of OPAL_SECVAR on EFI, CPU_BIG_ENDIAN and UCS2_STRING * Changed OPAL APIs from static to non-static. Added opal-secvar.h for the same * Removed EFI hooks from opal_secvar.c * Removed opal_secvar_get_next(), opal_secvar_enqueue() and opal_query_variable_info() function * get_powerpc_sb_mode() in secboot.c now directly calls OPAL Runtime API rather than via EFI hooks. * Fixed log messages in get_powerpc_sb_mode() function. * Added dependency for PPC_SECURE_BOOT on configs PPC64 and OPAL_SECVAR * Replaced obj-$(CONFIG_IMA) with obj-$(CONFIG_PPC_SECURE_BOOT) in arch/powerpc/kernel/Makefile Nayna Jain (8): powerpc: detect the secure boot mode of the system powerpc/ima: add support to initialize ima policy rules powerpc: detect the trusted boot state of the system powerpc/ima: add measurement rules to ima arch specific policy ima: make process_buffer_measurement() generic certs: add wrapper function to check blacklisted binary hash ima: check against blacklisted hashes for files with modsig powerpc/ima: update ima arch policy to check for blacklist Documentation/ABI/testing/ima_policy | 1 + arch/powerpc/Kconfig | 11 arch/powerpc/include/asm/secure_boot.h |
passing NULL to clock_getres (VDSO): terminated by unexpected signal 11
Hi Nathan, While trying to switch powerpc VDSO to C version of gettimeofday(), I'm getting the following kind of error with vdsotest: passing NULL to clock_getres (VDSO): terminated by unexpected signal 11 Looking at commit a9446a906f52 ("lib/vdso/32: Remove inconsistent NULL pointer checks"), it seems that signal 11 is expected when passing NULL pointer. Any plan to fix vdsotest ? Thanks Christophe