________________________________________ From: Petko Manolov [pet...@mip-labs.com] Sent: Friday, October 23, 2015 4:05 PM To: Dmitry Kasatkin Cc: zo...@linux.vnet.ibm.com; linux-ima-de...@lists.sourceforge.net; linux-security-mod...@vger.kernel.org; linux-kernel@vger.kernel.org; Dmitry Kasatkin Subject: Re: [PATCHv3 1/6] integrity: define '.evm' as a builtin 'trusted' keyring
On 15-10-22 21:49:25, Dmitry Kasatkin wrote: > Require all keys added to the EVM keyring be signed by an > existing trusted key on the system trusted keyring. > > This patch also switches IMA to use integrity_init_keyring(). > > Changes in v3: > * Added 'init_keyring' config based variable to skip initializing > keyring instead of using __integrity_init_keyring() wrapper. > * Added dependency back to CONFIG_IMA_TRUSTED_KEYRING > > Changes in v2: > * Replace CONFIG_EVM_TRUSTED_KEYRING with IMA and EVM common > CONFIG_INTEGRITY_TRUSTED_KEYRING configuration option > * Deprecate CONFIG_IMA_TRUSTED_KEYRING but keep it for config > file compatibility. (Mimi Zohar) > > Signed-off-by: Dmitry Kasatkin <dmitry.kasat...@huawei.com> > --- > security/integrity/Kconfig | 11 +++++++++++ > security/integrity/digsig.c | 14 ++++++++++++-- > security/integrity/evm/evm_main.c | 8 +++++--- > security/integrity/ima/Kconfig | 5 ++++- > security/integrity/ima/ima.h | 12 ------------ > security/integrity/ima/ima_init.c | 2 +- > security/integrity/integrity.h | 5 ++--- > 7 files changed, 35 insertions(+), 22 deletions(-) > > diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig > index 73c457b..21d7568 100644 > --- a/security/integrity/Kconfig > +++ b/security/integrity/Kconfig > @@ -41,6 +41,17 @@ config INTEGRITY_ASYMMETRIC_KEYS > This option enables digital signature verification using > asymmetric keys. > > +config INTEGRITY_TRUSTED_KEYRING > + bool "Require all keys on the integrity keyrings be signed" > + depends on SYSTEM_TRUSTED_KEYRING > + depends on INTEGRITY_ASYMMETRIC_KEYS > + select KEYS_DEBUG_PROC_KEYS > + default y > + help > + This option requires that all keys added to the .ima and > + .evm keyrings be signed by a key on the system trusted > + keyring. > + > config INTEGRITY_AUDIT > bool "Enables integrity auditing support " > depends on AUDIT > diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c > index 5be9ffb..8ef1511 100644 > --- a/security/integrity/digsig.c > +++ b/security/integrity/digsig.c > @@ -24,15 +24,22 @@ > static struct key *keyring[INTEGRITY_KEYRING_MAX]; > > static const char *keyring_name[INTEGRITY_KEYRING_MAX] = { > +#ifndef CONFIG_INTEGRITY_TRUSTED_KEYRING > "_evm", > - "_module", > -#ifndef CONFIG_IMA_TRUSTED_KEYRING > "_ima", > #else > + ".evm", > ".ima", > #endif > + "_module", > }; > > +#ifdef CONFIG_INTEGRITY_TRUSTED_KEYRING > +static bool init_keyring __initdata = true; > +#else > +static bool init_keyring __initdata; > +#endif > + > int integrity_digsig_verify(const unsigned int id, const char *sig, int > siglen, > const char *digest, int digestlen) > { > @@ -68,6 +75,9 @@ int __init integrity_init_keyring(const unsigned int id) > const struct cred *cred = current_cred(); > int err = 0; > > + if (!init_keyring) > + return 0; > + > keyring[id] = keyring_alloc(keyring_name[id], KUIDT_INIT(0), > KGIDT_INIT(0), cred, > ((KEY_POS_ALL & ~KEY_POS_SETATTR) | > diff --git a/security/integrity/evm/evm_main.c > b/security/integrity/evm/evm_main.c > index 1334e02..75b7e30 100644 > --- a/security/integrity/evm/evm_main.c > +++ b/security/integrity/evm/evm_main.c > @@ -478,15 +478,17 @@ static int __init init_evm(void) > > evm_init_config(); > > + error = integrity_init_keyring(INTEGRITY_KEYRING_EVM); > + if (error) > + return error; > + > error = evm_init_secfs(); > if (error < 0) { > pr_info("Error registering secfs\n"); > - goto err; > + return error; > } > > return 0; > -err: > - return error; > } > > /* > diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig > index df30334..a292b88 100644 > --- a/security/integrity/ima/Kconfig > +++ b/security/integrity/ima/Kconfig > @@ -123,14 +123,17 @@ config IMA_APPRAISE > If unsure, say N. > > config IMA_TRUSTED_KEYRING > - bool "Require all keys on the .ima keyring be signed" > + bool "Require all keys on the .ima keyring be signed (deprecated)" > depends on IMA_APPRAISE && SYSTEM_TRUSTED_KEYRING > depends on INTEGRITY_ASYMMETRIC_KEYS > + select INTEGRITY_TRUSTED_KEYRING > default y > help > This option requires that all keys added to the .ima > keyring be signed by a key on the system trusted keyring. > > + This option is deprecated in favor of INTEGRITY_TRUSTED_KEYRING > + > config IMA_LOAD_X509 > bool "Load X509 certificate onto the '.ima' trusted keyring" > depends on IMA_TRUSTED_KEYRING I guess we may as well remove this switch. Otherwise somebody have to remember to post a patch that does so a few kernel releases after this one goes mainline. It was actually removed in one of first versions, but Mimi suggested to keep it 1 release to allow "seamless" migration from kernel version to kernel version. In the next release we may remove it completely. Dmitry > diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h > index e2a60c3..9e82367 100644 > --- a/security/integrity/ima/ima.h > +++ b/security/integrity/ima/ima.h > @@ -251,16 +251,4 @@ static inline int security_filter_rule_match(u32 secid, > u32 field, u32 op, > return -EINVAL; > } > #endif /* CONFIG_IMA_LSM_RULES */ > - > -#ifdef CONFIG_IMA_TRUSTED_KEYRING > -static inline int ima_init_keyring(const unsigned int id) > -{ > - return integrity_init_keyring(id); > -} > -#else > -static inline int ima_init_keyring(const unsigned int id) > -{ > - return 0; > -} > -#endif /* CONFIG_IMA_TRUSTED_KEYRING */ > #endif > diff --git a/security/integrity/ima/ima_init.c > b/security/integrity/ima/ima_init.c > index e600cad..bd79f25 100644 > --- a/security/integrity/ima/ima_init.c > +++ b/security/integrity/ima/ima_init.c > @@ -116,7 +116,7 @@ int __init ima_init(void) > if (!ima_used_chip) > pr_info("No TPM chip found, activating TPM-bypass!\n"); > > - rc = ima_init_keyring(INTEGRITY_KEYRING_IMA); > + rc = integrity_init_keyring(INTEGRITY_KEYRING_IMA); > if (rc) > return rc; > > diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h > index 9c61687..07726a7 100644 > --- a/security/integrity/integrity.h > +++ b/security/integrity/integrity.h > @@ -125,8 +125,8 @@ int integrity_kernel_read(struct file *file, loff_t > offset, > int __init integrity_read_file(const char *path, char **data); > > #define INTEGRITY_KEYRING_EVM 0 > -#define INTEGRITY_KEYRING_MODULE 1 > -#define INTEGRITY_KEYRING_IMA 2 > +#define INTEGRITY_KEYRING_IMA 1 > +#define INTEGRITY_KEYRING_MODULE 2 > #define INTEGRITY_KEYRING_MAX 3 > > #ifdef CONFIG_INTEGRITY_SIGNATURE > @@ -149,7 +149,6 @@ static inline int integrity_init_keyring(const unsigned > int id) > { > return 0; > } > - > #endif /* CONFIG_INTEGRITY_SIGNATURE */ > > #ifdef CONFIG_INTEGRITY_ASYMMETRIC_KEYS > -- > 2.1.4 ACK to the rest of the code. Petko -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/