* [RFC] kexec: Allow kexec_file() with appropriate IMA policy when locked down @ 2019-03-12 19:57 Matthew Garrett 2019-03-13 11:58 ` Mimi Zohar 0 siblings, 1 reply; 15+ messages in thread From: Matthew Garrett @ 2019-03-12 19:57 UTC (permalink / raw) To: linux-integrity Cc: dhowells, zohar, dmitry.kasatkin, Matthew Garrett, Matthew Garrett Systems in lockdown mode should block the kexec of untrusted kernels. For x86 and ARM we can ensure that a kernel is trustworthy by validating a PE signature, but this isn't possible on other architectures. On those platforms we can use IMA instead, either with native IMA digital signatures or EVM-protected IMA hashes. Add a function to determine whether IMA will verify signatures on kexec files, and if so permit kexec_file() even if the kernel is otherwise locked down. This is restricted to cases where CONFIG_INTEGRITY_TRUSTED_KEYRING is set in order to prevent an attacker from loading additional keys at runtime. Signed-off-by: Matthew Garrett <mjg59@google.com> --- include/linux/evm.h | 6 +++++ include/linux/ima.h | 9 ++++++++ kernel/kexec_file.c | 9 ++++++-- security/integrity/evm/evm_main.c | 2 +- security/integrity/ima/ima_policy.c | 35 +++++++++++++++++++++++++++++ 5 files changed, 58 insertions(+), 3 deletions(-) diff --git a/include/linux/evm.h b/include/linux/evm.h index 8302bc29bb35..6e89d046b716 100644 --- a/include/linux/evm.h +++ b/include/linux/evm.h @@ -15,6 +15,7 @@ struct integrity_iint_cache; #ifdef CONFIG_EVM +extern bool evm_key_loaded(void); extern int evm_set_key(void *key, size_t keylen); extern enum integrity_status evm_verifyxattr(struct dentry *dentry, const char *xattr_name, @@ -45,6 +46,11 @@ static inline int posix_xattr_acl(const char *xattrname) #endif #else +static inline bool evm_key_loaded(void) +{ + return false; +} + static inline int evm_set_key(void *key, size_t keylen) { return -EOPNOTSUPP; diff --git a/include/linux/ima.h b/include/linux/ima.h index dc12fbcf484c..2ec593537c9b 100644 --- a/include/linux/ima.h +++ b/include/linux/ima.h @@ -132,4 +132,13 @@ static inline int ima_inode_removexattr(struct dentry *dentry, return 0; } #endif /* CONFIG_IMA_APPRAISE */ + +#if defined(CONFIG_IMA_APPRAISE) && defined(CONFIG_INTEGRITY_TRUSTED_KEYRING) +extern bool ima_appraise_kexec_signature(void); +#else +static inline bool ima_appraise_kexec_signature(void) +{ + return false; +} +#endif /* CONFIG_IMA_APPRAISE && CONFIG_INTEGRITY_TRUSTED_KEYRING */ #endif /* _LINUX_IMA_H */ diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c index 0cfe4f6f7f85..8ca607f1b515 100644 --- a/kernel/kexec_file.c +++ b/kernel/kexec_file.c @@ -20,11 +20,11 @@ #include <linux/mutex.h> #include <linux/list.h> #include <linux/fs.h> -#include <linux/ima.h> #include <crypto/hash.h> #include <crypto/sha.h> #include <linux/elf.h> #include <linux/elfcore.h> +#include <linux/ima.h> #include <linux/kernel.h> #include <linux/syscalls.h> #include <linux/vmalloc.h> @@ -240,7 +240,12 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd, ret = 0; - if (kernel_is_locked_down(reason)) { + /* If IMA is guaranteed to appraise a signature on the kexec + * image, permit it even if the kernel is otherwise locked + * down. + */ + if (!ima_appraise_kexec_signature() && + kernel_is_locked_down(reason)) { ret = -EPERM; goto out; } diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index b6d9f14bc234..aad61bc0f774 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -87,7 +87,7 @@ static void __init evm_init_config(void) pr_info("HMAC attrs: 0x%x\n", evm_hmac_attrs); } -static bool evm_key_loaded(void) +bool evm_key_loaded(void) { return (bool)(evm_initialized & EVM_KEY_MASK); } diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 8bc8a1c8cb3f..c06b1a6b3528 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -21,6 +21,7 @@ #include <linux/genhd.h> #include <linux/seq_file.h> #include <linux/ima.h> +#include <linux/evm.h> #include "ima.h" @@ -1336,4 +1337,38 @@ int ima_policy_show(struct seq_file *m, void *v) seq_puts(m, "\n"); return 0; } + #endif /* CONFIG_IMA_READ_POLICY */ + +#if defined(CONFIG_IMA_APPRAISE) && defined(CONFIG_INTEGRITY_TRUSTED_KEYRING) +/* + * ima_appraise_kexec: whether IMA will appraise a kexec image, either via + * IMA digital signatures or with a hash and EVM validation + */ +bool ima_appraise_kexec_signature(void) +{ + struct ima_rule_entry *entry; + bool found = false; + + rcu_read_lock(); + list_for_each_entry_rcu(entry, ima_rules, list) { + if (entry->func != KEXEC_KERNEL_CHECK || + entry->action != APPRAISE) + continue; + + /* + * We require this to be a digital signature, not a raw IMA + * hash. An IMA hash is acceptable as long as it's covered + * by an EVM signature. + */ + if (entry->flags & IMA_DIGSIG_REQUIRED || + evm_key_loaded()) { + found = true; + break; + } + } + + rcu_read_unlock(); + return found; +} +#endif /* CONFIG_IMA_APPRAISE && CONFIG_INTEGRITY_TRUSTED_KEYRING */ -- 2.21.0.360.g471c308f928-goog ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [RFC] kexec: Allow kexec_file() with appropriate IMA policy when locked down 2019-03-12 19:57 [RFC] kexec: Allow kexec_file() with appropriate IMA policy when locked down Matthew Garrett @ 2019-03-13 11:58 ` Mimi Zohar 2019-03-13 20:36 ` Matthew Garrett 0 siblings, 1 reply; 15+ messages in thread From: Mimi Zohar @ 2019-03-13 11:58 UTC (permalink / raw) To: Matthew Garrett, linux-integrity Cc: dhowells, dmitry.kasatkin, Matthew Garrett On Tue, 2019-03-12 at 12:57 -0700, Matthew Garrett wrote: > Systems in lockdown mode should block the kexec of untrusted kernels. > For x86 and ARM we can ensure that a kernel is trustworthy by validating > a PE signature, but this isn't possible on other architectures. On those > platforms we can use IMA instead, either with native IMA digital > signatures or EVM-protected IMA hashes. Add a function to determine > whether IMA will verify signatures on kexec files, and if so permit > kexec_file() even if the kernel is otherwise locked down. This is > restricted to cases where CONFIG_INTEGRITY_TRUSTED_KEYRING is set in > order to prevent an attacker from loading additional keys at runtime. Thank you for working on this! With the changes suggested below, it might work. :) > > Signed-off-by: Matthew Garrett <mjg59@google.com> > --- > include/linux/evm.h | 6 +++++ > include/linux/ima.h | 9 ++++++++ > kernel/kexec_file.c | 9 ++++++-- > security/integrity/evm/evm_main.c | 2 +- > security/integrity/ima/ima_policy.c | 35 +++++++++++++++++++++++++++++ > 5 files changed, 58 insertions(+), 3 deletions(-) > > diff --git a/include/linux/evm.h b/include/linux/evm.h > index 8302bc29bb35..6e89d046b716 100644 > --- a/include/linux/evm.h > +++ b/include/linux/evm.h > @@ -15,6 +15,7 @@ > struct integrity_iint_cache; > > #ifdef CONFIG_EVM > +extern bool evm_key_loaded(void); > extern int evm_set_key(void *key, size_t keylen); > extern enum integrity_status evm_verifyxattr(struct dentry *dentry, > const char *xattr_name, > @@ -45,6 +46,11 @@ static inline int posix_xattr_acl(const char *xattrname) > #endif > #else > > +static inline bool evm_key_loaded(void) > +{ > + return false; > +} > + > static inline int evm_set_key(void *key, size_t keylen) > { > return -EOPNOTSUPP; > diff --git a/include/linux/ima.h b/include/linux/ima.h > index dc12fbcf484c..2ec593537c9b 100644 > --- a/include/linux/ima.h > +++ b/include/linux/ima.h > @@ -132,4 +132,13 @@ static inline int ima_inode_removexattr(struct dentry *dentry, > return 0; > } > #endif /* CONFIG_IMA_APPRAISE */ > + > +#if defined(CONFIG_IMA_APPRAISE) && defined(CONFIG_INTEGRITY_TRUSTED_KEYRING) > +extern bool ima_appraise_kexec_signature(void); > +#else > +static inline bool ima_appraise_kexec_signature(void) > +{ > + return false; > +} > +#endif /* CONFIG_IMA_APPRAISE && CONFIG_INTEGRITY_TRUSTED_KEYRING */ > #endif /* _LINUX_IMA_H */ > diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c > index 0cfe4f6f7f85..8ca607f1b515 100644 > --- a/kernel/kexec_file.c > +++ b/kernel/kexec_file.c > @@ -20,11 +20,11 @@ > #include <linux/mutex.h> > #include <linux/list.h> > #include <linux/fs.h> > -#include <linux/ima.h> > #include <crypto/hash.h> > #include <crypto/sha.h> > #include <linux/elf.h> > #include <linux/elfcore.h> > +#include <linux/ima.h> > #include <linux/kernel.h> > #include <linux/syscalls.h> > #include <linux/vmalloc.h> > @@ -240,7 +240,12 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd, > > ret = 0; > > - if (kernel_is_locked_down(reason)) { > + /* If IMA is guaranteed to appraise a signature on the kexec > + * image, permit it even if the kernel is otherwise locked > + * down. > + */ > + if (!ima_appraise_kexec_signature() && > + kernel_is_locked_down(reason)) { > ret = -EPERM; > goto out; > } > diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c > index b6d9f14bc234..aad61bc0f774 100644 > --- a/security/integrity/evm/evm_main.c > +++ b/security/integrity/evm/evm_main.c > @@ -87,7 +87,7 @@ static void __init evm_init_config(void) > pr_info("HMAC attrs: 0x%x\n", evm_hmac_attrs); > } > > -static bool evm_key_loaded(void) > +bool evm_key_loaded(void) > { > return (bool)(evm_initialized & EVM_KEY_MASK); > } This might be sufficient for your environment, but in general it isn't. > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c > index 8bc8a1c8cb3f..c06b1a6b3528 100644 > --- a/security/integrity/ima/ima_policy.c > +++ b/security/integrity/ima/ima_policy.c > @@ -21,6 +21,7 @@ > #include <linux/genhd.h> > #include <linux/seq_file.h> > #include <linux/ima.h> > +#include <linux/evm.h> > > #include "ima.h" > > @@ -1336,4 +1337,38 @@ int ima_policy_show(struct seq_file *m, void *v) > seq_puts(m, "\n"); > return 0; > } > + > #endif /* CONFIG_IMA_READ_POLICY */ > + > +#if defined(CONFIG_IMA_APPRAISE) && defined(CONFIG_INTEGRITY_TRUSTED_KEYRING) With these defines, the function isn't limited to just "lockdown". Either fix the defines or the patch description. > +/* > + * ima_appraise_kexec: whether IMA will appraise a kexec image, either via > + * IMA digital signatures or with a hash and EVM validation > + */ > +bool ima_appraise_kexec_signature(void) Instead of making this specific to kexec, how about naming the function something like ima_require_appraise_signature() and pass the kernel_read_file_id (eg. READING_KEXEC_IMAGE, READING_MODULE). > +{ > + struct ima_rule_entry *entry; > + bool found = false; > + > + rcu_read_lock(); > + list_for_each_entry_rcu(entry, ima_rules, list) { > + if (entry->func != KEXEC_KERNEL_CHECK || > + entry->action != APPRAISE) > + continue; > + > + /* > + * We require this to be a digital signature, not a raw IMA > + * hash. An IMA hash is acceptable as long as it's covered > + * by an EVM signature. > + */ > + if (entry->flags & IMA_DIGSIG_REQUIRED || > + evm_key_loaded()) { evm_key_loaded() is a problem. > + found = true; > + break; > + } The first matching rule dictates the policy. Move the "break" here. Walking the list looking for a specific rule might not be a true indication of the policy. For example, with a generic rule prior to a specific rule, the generic rule might take precedence. As long as the generic rules require a signature, there isn't a problem. I would stop walking the policy rules after the first "appraise" rule that doesn't require a signature. This will prevent returning a false positive. The builtin and arch policy rules are by design forced to be first. Mimi > + } > + > + rcu_read_unlock(); > + return found; > +} > +#endif /* CONFIG_IMA_APPRAISE && CONFIG_INTEGRITY_TRUSTED_KEYRING */ ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC] kexec: Allow kexec_file() with appropriate IMA policy when locked down 2019-03-13 11:58 ` Mimi Zohar @ 2019-03-13 20:36 ` Matthew Garrett 2019-03-13 21:29 ` Mimi Zohar 0 siblings, 1 reply; 15+ messages in thread From: Matthew Garrett @ 2019-03-13 20:36 UTC (permalink / raw) To: Mimi Zohar; +Cc: linux-integrity, David Howells, Dmitry Kasatkin On Wed, Mar 13, 2019 at 4:58 AM Mimi Zohar <zohar@linux.ibm.com> wrote: > > On Tue, 2019-03-12 at 12:57 -0700, Matthew Garrett wrote: > > Systems in lockdown mode should block the kexec of untrusted kernels. > > For x86 and ARM we can ensure that a kernel is trustworthy by validating > > a PE signature, but this isn't possible on other architectures. On those > > platforms we can use IMA instead, either with native IMA digital > > signatures or EVM-protected IMA hashes. Add a function to determine > > whether IMA will verify signatures on kexec files, and if so permit > > kexec_file() even if the kernel is otherwise locked down. This is > > restricted to cases where CONFIG_INTEGRITY_TRUSTED_KEYRING is set in > > order to prevent an attacker from loading additional keys at runtime. > > Thank you for working on this! With the changes suggested below, it > might work. :) Ok, I'll incorporate them - just one question: > > +bool evm_key_loaded(void) > > { > > return (bool)(evm_initialized & EVM_KEY_MASK); > > } > > This might be sufficient for your environment, but in general it > isn't. Oh hm. The only case I can see where this isn't sufficient is if the filesystem returns EOPNOTSUPP for the EVM xattr, but in that case we should already have failed to get the IMA xattr and will fail appraisal as a result? > > +#if defined(CONFIG_IMA_APPRAISE) && defined(CONFIG_INTEGRITY_TRUSTED_KEYRING) > > With these defines, the function isn't limited to just "lockdown". > Either fix the defines or the patch description. The function will be called even when lockdown isn't enabled, but it won't have any impact on the logic flow. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC] kexec: Allow kexec_file() with appropriate IMA policy when locked down 2019-03-13 20:36 ` Matthew Garrett @ 2019-03-13 21:29 ` Mimi Zohar 2019-03-13 21:59 ` Matthew Garrett 0 siblings, 1 reply; 15+ messages in thread From: Mimi Zohar @ 2019-03-13 21:29 UTC (permalink / raw) To: Matthew Garrett; +Cc: linux-integrity, David Howells, Dmitry Kasatkin On Wed, 2019-03-13 at 13:36 -0700, Matthew Garrett wrote: > On Wed, Mar 13, 2019 at 4:58 AM Mimi Zohar <zohar@linux.ibm.com> wrote: > > > > On Tue, 2019-03-12 at 12:57 -0700, Matthew Garrett wrote: > > > Systems in lockdown mode should block the kexec of untrusted kernels. > > > For x86 and ARM we can ensure that a kernel is trustworthy by validating > > > a PE signature, but this isn't possible on other architectures. On those > > > platforms we can use IMA instead, either with native IMA digital > > > signatures or EVM-protected IMA hashes. Add a function to determine > > > whether IMA will verify signatures on kexec files, and if so permit > > > kexec_file() even if the kernel is otherwise locked down. This is > > > restricted to cases where CONFIG_INTEGRITY_TRUSTED_KEYRING is set in > > > order to prevent an attacker from loading additional keys at runtime. > > > > Thank you for working on this! With the changes suggested below, it > > might work. :) > > Ok, I'll incorporate them - just one question: > > > > +bool evm_key_loaded(void) > > > { > > > return (bool)(evm_initialized & EVM_KEY_MASK); > > > } > > > > This might be sufficient for your environment, but in general it > > isn't. > > Oh hm. The only case I can see where this isn't sufficient is if the > filesystem returns EOPNOTSUPP for the EVM xattr, but in that case we > should already have failed to get the IMA xattr and will fail > appraisal as a result? The evm_initialized flag is an indication that EVM has been initialized on the system. Both hmac and signatures could be supported. Even checking for EVM_INIT_X509 doesn't provide any guarantees that the particular file has an EVM signature. (The hmac can be updated (eg. change in security xattrs, remove/additional of protected xattr), so we can't rely on them.) > > > > +#if defined(CONFIG_IMA_APPRAISE) && defined(CONFIG_INTEGRITY_TRUSTED_KEYRING) > > > > With these defines, the function isn't limited to just "lockdown". > > Either fix the defines or the patch description. > > The function will be called even when lockdown isn't enabled, but it > won't have any impact on the logic flow. Ok, so inverting the test order should prevent unnecessarily calling ima_apprase_kexec_signature(). + if (!ima_appraise_kexec_signature() && + kernel_is_locked_down(reason)) { Mimi ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC] kexec: Allow kexec_file() with appropriate IMA policy when locked down 2019-03-13 21:29 ` Mimi Zohar @ 2019-03-13 21:59 ` Matthew Garrett 2019-03-14 1:08 ` Mimi Zohar 0 siblings, 1 reply; 15+ messages in thread From: Matthew Garrett @ 2019-03-13 21:59 UTC (permalink / raw) To: Mimi Zohar; +Cc: linux-integrity, David Howells, Dmitry Kasatkin On Wed, Mar 13, 2019 at 2:29 PM Mimi Zohar <zohar@linux.ibm.com> wrote: > > On Wed, 2019-03-13 at 13:36 -0700, Matthew Garrett wrote: > > Oh hm. The only case I can see where this isn't sufficient is if the > > filesystem returns EOPNOTSUPP for the EVM xattr, but in that case we > > should already have failed to get the IMA xattr and will fail > > appraisal as a result? > > The evm_initialized flag is an indication that EVM has been > initialized on the system. Both hmac and signatures could be > supported. Even checking for EVM_INIT_X509 doesn't provide any > guarantees that the particular file has an EVM signature. > > (The hmac can be updated (eg. change in security xattrs, > remove/additional of protected xattr), so we can't rely on them.) So having IMA appraisal of the hash and hmac-based EVM validation of the xattr security isn't sufficient? Is this just because of the offline attack case? > > > > +#if defined(CONFIG_IMA_APPRAISE) && defined(CONFIG_INTEGRITY_TRUSTED_KEYRING) > > > > > > With these defines, the function isn't limited to just "lockdown". > > > Either fix the defines or the patch description. > > > > The function will be called even when lockdown isn't enabled, but it > > won't have any impact on the logic flow. > > Ok, so inverting the test order should prevent unnecessarily calling > ima_apprase_kexec_signature(). Unfortunately kernel_is_locked_down(reason) will print a message telling us that something was blocked, even if ima_appraise_signature() then permits it, so we need to do it in this order to shortcut that. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC] kexec: Allow kexec_file() with appropriate IMA policy when locked down 2019-03-13 21:59 ` Matthew Garrett @ 2019-03-14 1:08 ` Mimi Zohar 2019-03-14 21:08 ` Matthew Garrett 0 siblings, 1 reply; 15+ messages in thread From: Mimi Zohar @ 2019-03-14 1:08 UTC (permalink / raw) To: Matthew Garrett; +Cc: linux-integrity, David Howells, Dmitry Kasatkin On Wed, 2019-03-13 at 14:59 -0700, Matthew Garrett wrote: > On Wed, Mar 13, 2019 at 2:29 PM Mimi Zohar <zohar@linux.ibm.com> wrote: > > > > On Wed, 2019-03-13 at 13:36 -0700, Matthew Garrett wrote: > > > Oh hm. The only case I can see where this isn't sufficient is if the > > > filesystem returns EOPNOTSUPP for the EVM xattr, but in that case we > > > should already have failed to get the IMA xattr and will fail > > > appraisal as a result? > > > > The evm_initialized flag is an indication that EVM has been > > initialized on the system. Both hmac and signatures could be > > supported. Even checking for EVM_INIT_X509 doesn't provide any > > guarantees that the particular file has an EVM signature. > > > > (The hmac can be updated (eg. change in security xattrs, > > remove/additional of protected xattr), so we can't rely on them.) > > So having IMA appraisal of the hash and hmac-based EVM validation of > the xattr security isn't sufficient? Is this just because of the > offline attack case? The IMA hash and EVM hmac combination is fine for offline protection. It's used for mutable files. For immutable files, there must be either an IMA or EVM signature. Mimi ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC] kexec: Allow kexec_file() with appropriate IMA policy when locked down 2019-03-14 1:08 ` Mimi Zohar @ 2019-03-14 21:08 ` Matthew Garrett 2019-03-14 22:31 ` Mimi Zohar 0 siblings, 1 reply; 15+ messages in thread From: Matthew Garrett @ 2019-03-14 21:08 UTC (permalink / raw) To: Mimi Zohar; +Cc: linux-integrity, David Howells, Dmitry Kasatkin On Wed, Mar 13, 2019 at 6:08 PM Mimi Zohar <zohar@linux.ibm.com> wrote: > The IMA hash and EVM hmac combination is fine for offline protection. > It's used for mutable files. For immutable files, there must be > either an IMA or EVM signature. Ok. Is the correct way to handle this to check that the file has a signature, or to extend IMA policy to allow it to provide a requirement that EVM verify a signature rather than an HMAC and have the arch policy set that? ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC] kexec: Allow kexec_file() with appropriate IMA policy when locked down 2019-03-14 21:08 ` Matthew Garrett @ 2019-03-14 22:31 ` Mimi Zohar 2019-03-14 22:54 ` Matthew Garrett 0 siblings, 1 reply; 15+ messages in thread From: Mimi Zohar @ 2019-03-14 22:31 UTC (permalink / raw) To: Matthew Garrett; +Cc: linux-integrity, David Howells, Dmitry Kasatkin On Thu, 2019-03-14 at 14:08 -0700, Matthew Garrett wrote: > On Wed, Mar 13, 2019 at 6:08 PM Mimi Zohar <zohar@linux.ibm.com> wrote: > > The IMA hash and EVM hmac combination is fine for offline protection. > > It's used for mutable files. For immutable files, there must be > > either an IMA or EVM signature. > > Ok. Is the correct way to handle this to check that the file has a > signature, or to extend IMA policy to allow it to provide a > requirement that EVM verify a signature rather than an HMAC and have > the arch policy set that? I'm not sure what you mean by "check that the file has a signature". EVM and IMA are separate subsystems with a defined interface for interaction between them. evm_verifyxattr() isn't, but could be called by LSMs. So evm_verifyxattr() would need to be extended to return the EVM xattr type. The IMA policy could then require a specific evmxattr type. Possible. Perhaps for now require IMA signatures and defer supporting EVM signatures? Mimi ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC] kexec: Allow kexec_file() with appropriate IMA policy when locked down 2019-03-14 22:31 ` Mimi Zohar @ 2019-03-14 22:54 ` Matthew Garrett 2019-03-14 23:58 ` Mimi Zohar 0 siblings, 1 reply; 15+ messages in thread From: Matthew Garrett @ 2019-03-14 22:54 UTC (permalink / raw) To: Mimi Zohar; +Cc: linux-integrity, David Howells, Dmitry Kasatkin On Thu, Mar 14, 2019 at 3:31 PM Mimi Zohar <zohar@linux.ibm.com> wrote: > > On Thu, 2019-03-14 at 14:08 -0700, Matthew Garrett wrote: > > On Wed, Mar 13, 2019 at 6:08 PM Mimi Zohar <zohar@linux.ibm.com> wrote: > > > The IMA hash and EVM hmac combination is fine for offline protection. > > > It's used for mutable files. For immutable files, there must be > > > either an IMA or EVM signature. > > > > Ok. Is the correct way to handle this to check that the file has a > > signature, or to extend IMA policy to allow it to provide a > > requirement that EVM verify a signature rather than an HMAC and have > > the arch policy set that? > > I'm not sure what you mean by "check that the file has a signature". Call getxattr(XATTR_SECURITY_EVM) and parse the type to determine whether it's an hmac or a signature. > EVM and IMA are separate subsystems with a defined interface for > interaction between them. evm_verifyxattr() isn't, but could be called > by LSMs. So evm_verifyxattr() would need to be extended to return the > EVM xattr type. The IMA policy could then require a specific evmxattr > type. Possible. I'd been thinking of doing it the other way (ie, pass the set of permitted EVM xattr types to evm_verifyxattr()), but yes. > Perhaps for now require IMA signatures and defer supporting EVM > signatures? If that's sufficient for you, happy to do that. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC] kexec: Allow kexec_file() with appropriate IMA policy when locked down 2019-03-14 22:54 ` Matthew Garrett @ 2019-03-14 23:58 ` Mimi Zohar 2019-03-15 22:03 ` Matthew Garrett 0 siblings, 1 reply; 15+ messages in thread From: Mimi Zohar @ 2019-03-14 23:58 UTC (permalink / raw) To: Matthew Garrett; +Cc: linux-integrity, David Howells, Dmitry Kasatkin On Thu, 2019-03-14 at 15:54 -0700, Matthew Garrett wrote: > On Thu, Mar 14, 2019 at 3:31 PM Mimi Zohar <zohar@linux.ibm.com> wrote: > > > > On Thu, 2019-03-14 at 14:08 -0700, Matthew Garrett wrote: > > > On Wed, Mar 13, 2019 at 6:08 PM Mimi Zohar <zohar@linux.ibm.com> wrote: > > > > The IMA hash and EVM hmac combination is fine for offline protection. > > > > It's used for mutable files. For immutable files, there must be > > > > either an IMA or EVM signature. > > > > > > Ok. Is the correct way to handle this to check that the file has a > > > signature, or to extend IMA policy to allow it to provide a > > > requirement that EVM verify a signature rather than an HMAC and have > > > the arch policy set that? > > > > I'm not sure what you mean by "check that the file has a signature". > > Call getxattr(XATTR_SECURITY_EVM) and parse the type to determine > whether it's an hmac or a signature. The other option is preferable. > > > EVM and IMA are separate subsystems with a defined interface for > > interaction between them. evm_verifyxattr() isn't, but could be called > > by LSMs. So evm_verifyxattr() would need to be extended to return the > > EVM xattr type. The IMA policy could then require a specific evmxattr > > type. Possible. > > I'd been thinking of doing it the other way (ie, pass the set of > permitted EVM xattr types to evm_verifyxattr()), but yes. Just as IMA decides how it wants to handle "no xattrs", let IMA decide how it wants to handle the different EVM xattr types. > > > Perhaps for now require IMA signatures and defer supporting EVM > > signatures? > > If that's sufficient for you, happy to do that. Definitely Mimi ^ permalink raw reply [flat|nested] 15+ messages in thread
* [RFC] kexec: Allow kexec_file() with appropriate IMA policy when locked down 2019-03-14 23:58 ` Mimi Zohar @ 2019-03-15 22:03 ` Matthew Garrett 2019-03-17 11:39 ` Mimi Zohar 0 siblings, 1 reply; 15+ messages in thread From: Matthew Garrett @ 2019-03-15 22:03 UTC (permalink / raw) To: linux-integrity Cc: dhowells, zohar, dmitry.kasatkin, Matthew Garrett, Matthew Garrett Systems in lockdown mode should block the kexec of untrusted kernels. For x86 and ARM we can ensure that a kernel is trustworthy by validating a PE signature, but this isn't possible on other architectures. On those platforms we can use IMA digital signatures instead. Add a function to determine whether IMA will verify signatures for a given event type, and if so permit kexec_file() even if the kernel is otherwise locked down. This is restricted to cases where CONFIG_INTEGRITY_TRUSTED_KEYRING is set in order to prevent an attacker from loading additional keys at runtime. Signed-off-by: Matthew Garrett <mjg59@google.com> --- include/linux/evm.h | 6 ++++ include/linux/ima.h | 28 +++++++++++++++++++ kernel/kexec_file.c | 9 ++++-- security/integrity/evm/evm_main.c | 2 +- security/integrity/ima/ima.h | 20 +------------- security/integrity/ima/ima_policy.c | 43 +++++++++++++++++++++++++++++ 6 files changed, 86 insertions(+), 22 deletions(-) diff --git a/include/linux/evm.h b/include/linux/evm.h index 8302bc29bb35..6e89d046b716 100644 --- a/include/linux/evm.h +++ b/include/linux/evm.h @@ -15,6 +15,7 @@ struct integrity_iint_cache; #ifdef CONFIG_EVM +extern bool evm_key_loaded(void); extern int evm_set_key(void *key, size_t keylen); extern enum integrity_status evm_verifyxattr(struct dentry *dentry, const char *xattr_name, @@ -45,6 +46,11 @@ static inline int posix_xattr_acl(const char *xattrname) #endif #else +static inline bool evm_key_loaded(void) +{ + return false; +} + static inline int evm_set_key(void *key, size_t keylen) { return -EOPNOTSUPP; diff --git a/include/linux/ima.h b/include/linux/ima.h index dc12fbcf484c..a42e2a9a08b7 100644 --- a/include/linux/ima.h +++ b/include/linux/ima.h @@ -27,6 +27,25 @@ extern int ima_post_read_file(struct file *file, void *buf, loff_t size, enum kernel_read_file_id id); extern void ima_post_path_mknod(struct dentry *dentry); +#define __ima_hooks(hook) \ + hook(NONE) \ + hook(FILE_CHECK) \ + hook(MMAP_CHECK) \ + hook(BPRM_CHECK) \ + hook(CREDS_CHECK) \ + hook(POST_SETATTR) \ + hook(MODULE_CHECK) \ + hook(FIRMWARE_CHECK) \ + hook(KEXEC_KERNEL_CHECK) \ + hook(KEXEC_INITRAMFS_CHECK) \ + hook(POLICY_CHECK) \ + hook(MAX_CHECK) +#define __ima_hook_enumify(ENUM) ENUM, + +enum ima_hooks { + __ima_hooks(__ima_hook_enumify) +}; + #ifdef CONFIG_IMA_KEXEC extern void ima_add_kexec_buffer(struct kimage *image); #endif @@ -132,4 +151,13 @@ static inline int ima_inode_removexattr(struct dentry *dentry, return 0; } #endif /* CONFIG_IMA_APPRAISE */ + +#if defined(CONFIG_IMA_APPRAISE) && defined(CONFIG_INTEGRITY_TRUSTED_KEYRING) +extern bool ima_appraise_signature(enum ima_hooks func); +#else +static inline bool ima_appraise_kexec_signature(enum ima_hooks func) +{ + return false; +} +#endif /* CONFIG_IMA_APPRAISE && CONFIG_INTEGRITY_TRUSTED_KEYRING */ #endif /* _LINUX_IMA_H */ diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c index 0cfe4f6f7f85..3e04506a00a2 100644 --- a/kernel/kexec_file.c +++ b/kernel/kexec_file.c @@ -20,11 +20,11 @@ #include <linux/mutex.h> #include <linux/list.h> #include <linux/fs.h> -#include <linux/ima.h> #include <crypto/hash.h> #include <crypto/sha.h> #include <linux/elf.h> #include <linux/elfcore.h> +#include <linux/ima.h> #include <linux/kernel.h> #include <linux/syscalls.h> #include <linux/vmalloc.h> @@ -240,7 +240,12 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd, ret = 0; - if (kernel_is_locked_down(reason)) { + /* If IMA is guaranteed to appraise a signature on the kexec + * image, permit it even if the kernel is otherwise locked + * down. + */ + if (!ima_appraise_signature(KEXEC_KERNEL_CHECK) && + kernel_is_locked_down(reason)) { ret = -EPERM; goto out; } diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index b6d9f14bc234..aad61bc0f774 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -87,7 +87,7 @@ static void __init evm_init_config(void) pr_info("HMAC attrs: 0x%x\n", evm_hmac_attrs); } -static bool evm_key_loaded(void) +bool evm_key_loaded(void) { return (bool)(evm_initialized & EVM_KEY_MASK); } diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index cc12f3449a72..71614a8ed2aa 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -20,6 +20,7 @@ #include <linux/types.h> #include <linux/crypto.h> #include <linux/fs.h> +#include <linux/ima.h> #include <linux/security.h> #include <linux/hash.h> #include <linux/tpm.h> @@ -171,25 +172,6 @@ static inline unsigned long ima_hash_key(u8 *digest) return hash_long(*digest, IMA_HASH_BITS); } -#define __ima_hooks(hook) \ - hook(NONE) \ - hook(FILE_CHECK) \ - hook(MMAP_CHECK) \ - hook(BPRM_CHECK) \ - hook(CREDS_CHECK) \ - hook(POST_SETATTR) \ - hook(MODULE_CHECK) \ - hook(FIRMWARE_CHECK) \ - hook(KEXEC_KERNEL_CHECK) \ - hook(KEXEC_INITRAMFS_CHECK) \ - hook(POLICY_CHECK) \ - hook(MAX_CHECK) -#define __ima_hook_enumify(ENUM) ENUM, - -enum ima_hooks { - __ima_hooks(__ima_hook_enumify) -}; - /* LIM API function definitions */ int ima_get_action(struct inode *inode, const struct cred *cred, u32 secid, int mask, enum ima_hooks func, int *pcr); diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 8bc8a1c8cb3f..adeae1ab9ee9 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -21,6 +21,7 @@ #include <linux/genhd.h> #include <linux/seq_file.h> #include <linux/ima.h> +#include <linux/evm.h> #include "ima.h" @@ -1336,4 +1337,46 @@ int ima_policy_show(struct seq_file *m, void *v) seq_puts(m, "\n"); return 0; } + #endif /* CONFIG_IMA_READ_POLICY */ + +#if defined(CONFIG_IMA_APPRAISE) && defined(CONFIG_INTEGRITY_TRUSTED_KEYRING) +/* + * ima_appraise_signature: whether IMA will appraise a given function using + * an IMA digital signature. This is restricted to cases where the kernel + * has a set of built-in trusted keys in order to avoid an attacker simply + * loading additional keys. + */ +bool ima_appraise_signature(enum ima_hooks func) +{ + struct ima_rule_entry *entry; + bool found = false; + + rcu_read_lock(); + list_for_each_entry_rcu(entry, ima_rules, list) { + if (entry->action != APPRAISE) + continue; + + /* A generic entry will match, but otherwise require that it + * match the func we're looking for + */ + if (entry->func && entry->func != func) + continue; + + /* We require this to be a digital signature, not a raw IMA + * hash. + */ + if (entry->flags & IMA_DIGSIG_REQUIRED) + found = true; + + /* We've found a rule that matches, so break now even if it + * didn't require a digital signature - a later rule that does + * won't override it, so would be a false positive. + */ + break; + } + + rcu_read_unlock(); + return found; +} +#endif /* CONFIG_IMA_APPRAISE && CONFIG_INTEGRITY_TRUSTED_KEYRING */ -- 2.21.0.225.g810b269d1ac-goog ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [RFC] kexec: Allow kexec_file() with appropriate IMA policy when locked down 2019-03-15 22:03 ` Matthew Garrett @ 2019-03-17 11:39 ` Mimi Zohar 0 siblings, 0 replies; 15+ messages in thread From: Mimi Zohar @ 2019-03-17 11:39 UTC (permalink / raw) To: Matthew Garrett, linux-integrity Cc: dhowells, dmitry.kasatkin, Matthew Garrett, kexec, Dave Young, Eric W. Biederman On Fri, 2019-03-15 at 15:03 -0700, Matthew Garrett wrote: > Systems in lockdown mode should block the kexec of untrusted kernels. > For x86 and ARM we can ensure that a kernel is trustworthy by validating > a PE signature, but this isn't possible on other architectures. On those > platforms we can use IMA digital signatures instead. Add a function to > determine whether IMA will verify signatures for a given event type, In both the kexec and kernel modules cases, this should be in the past tense. Perhaps change it to something like, "whether IMA has already or will verify signatures ...". > and > if so permit kexec_file() even if the kernel is otherwise locked down. > This is restricted to cases where CONFIG_INTEGRITY_TRUSTED_KEYRING is set > in order to prevent an attacker from loading additional keys at runtime. > > Signed-off-by: Matthew Garrett <mjg59@google.com> > --- > include/linux/evm.h | 6 ++++ > include/linux/ima.h | 28 +++++++++++++++++++ > kernel/kexec_file.c | 9 ++++-- > security/integrity/evm/evm_main.c | 2 +- > security/integrity/ima/ima.h | 20 +------------- > security/integrity/ima/ima_policy.c | 43 +++++++++++++++++++++++++++++ > 6 files changed, 86 insertions(+), 22 deletions(-) > > diff --git a/include/linux/evm.h b/include/linux/evm.h > index 8302bc29bb35..6e89d046b716 100644 > --- a/include/linux/evm.h > +++ b/include/linux/evm.h > @@ -15,6 +15,7 @@ > struct integrity_iint_cache; > > #ifdef CONFIG_EVM > +extern bool evm_key_loaded(void); > extern int evm_set_key(void *key, size_t keylen); > extern enum integrity_status evm_verifyxattr(struct dentry *dentry, > const char *xattr_name, > @@ -45,6 +46,11 @@ static inline int posix_xattr_acl(const char *xattrname) > #endif > #else > > +static inline bool evm_key_loaded(void) > +{ > + return false; > +} > + Remove remaining EVM fragment. > static inline int evm_set_key(void *key, size_t keylen) > { > return -EOPNOTSUPP; > diff --git a/include/linux/ima.h b/include/linux/ima.h > index dc12fbcf484c..a42e2a9a08b7 100644 > --- a/include/linux/ima.h > +++ b/include/linux/ima.h > @@ -27,6 +27,25 @@ extern int ima_post_read_file(struct file *file, void *buf, loff_t size, > enum kernel_read_file_id id); > extern void ima_post_path_mknod(struct dentry *dentry); > > +#define __ima_hooks(hook) \ > + hook(NONE) \ > + hook(FILE_CHECK) \ > + hook(MMAP_CHECK) \ > + hook(BPRM_CHECK) \ > + hook(CREDS_CHECK) \ > + hook(POST_SETATTR) \ > + hook(MODULE_CHECK) \ > + hook(FIRMWARE_CHECK) \ > + hook(KEXEC_KERNEL_CHECK) \ > + hook(KEXEC_INITRAMFS_CHECK) \ > + hook(POLICY_CHECK) \ > + hook(MAX_CHECK) > +#define __ima_hook_enumify(ENUM) ENUM, > + > +enum ima_hooks { > + __ima_hooks(__ima_hook_enumify) > +}; > + > #ifdef CONFIG_IMA_KEXEC > extern void ima_add_kexec_buffer(struct kimage *image); > #endif > @@ -132,4 +151,13 @@ static inline int ima_inode_removexattr(struct dentry *dentry, > return 0; > } > #endif /* CONFIG_IMA_APPRAISE */ > + > +#if defined(CONFIG_IMA_APPRAISE) && defined(CONFIG_INTEGRITY_TRUSTED_KEYRING) > +extern bool ima_appraise_signature(enum ima_hooks func); > +#else > +static inline bool ima_appraise_kexec_signature(enum ima_hooks func) > +{ > + return false; > +} > +#endif /* CONFIG_IMA_APPRAISE && CONFIG_INTEGRITY_TRUSTED_KEYRING */ > #endif /* _LINUX_IMA_H */ > diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c > index 0cfe4f6f7f85..3e04506a00a2 100644 > --- a/kernel/kexec_file.c > +++ b/kernel/kexec_file.c > @@ -20,11 +20,11 @@ > #include <linux/mutex.h> > #include <linux/list.h> > #include <linux/fs.h> > -#include <linux/ima.h> > #include <crypto/hash.h> > #include <crypto/sha.h> > #include <linux/elf.h> > #include <linux/elfcore.h> > +#include <linux/ima.h> > #include <linux/kernel.h> > #include <linux/syscalls.h> > #include <linux/vmalloc.h> > @@ -240,7 +240,12 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd, > > ret = 0; > > - if (kernel_is_locked_down(reason)) { > + /* If IMA is guaranteed to appraise a signature on the kexec > + * image, permit it even if the kernel is otherwise locked > + * down. > + */ > + if (!ima_appraise_signature(KEXEC_KERNEL_CHECK) && > + kernel_is_locked_down(reason)) { > ret = -EPERM; > goto out; [Cc'ing Dave Young, Eric Biederman, kexec mailing list] There was a discussion about using KEXEC_KERNEL_CHECK as an argument when replacing copy_file_from_fd() with kernel_read_file_from_fd(). There was a subsequent discussion when adding a security call in kexec_load_check. The end result was defining two enumerations named kernel_read_file_id and kernel_load_data_id with READING_KEXEC_IMAGE and LOADING_KECEC_IMAGE respectively. Instead of making the ima_hooks enumeration global, as we're already relying on READING_KEXEC_IMAGE, use it. > } > diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c > index b6d9f14bc234..aad61bc0f774 100644 > --- a/security/integrity/evm/evm_main.c > +++ b/security/integrity/evm/evm_main.c > @@ -87,7 +87,7 @@ static void __init evm_init_config(void) > pr_info("HMAC attrs: 0x%x\n", evm_hmac_attrs); > } > > -static bool evm_key_loaded(void) > +bool evm_key_loaded(void) > { > return (bool)(evm_initialized & EVM_KEY_MASK); > } Remove remaining EVM fragment. > diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h > index cc12f3449a72..71614a8ed2aa 100644 > --- a/security/integrity/ima/ima.h > +++ b/security/integrity/ima/ima.h > @@ -20,6 +20,7 @@ > #include <linux/types.h> > #include <linux/crypto.h> > #include <linux/fs.h> > +#include <linux/ima.h> > #include <linux/security.h> > #include <linux/hash.h> > #include <linux/tpm.h> > @@ -171,25 +172,6 @@ static inline unsigned long ima_hash_key(u8 *digest) > return hash_long(*digest, IMA_HASH_BITS); > } > > -#define __ima_hooks(hook) \ > - hook(NONE) \ > - hook(FILE_CHECK) \ > - hook(MMAP_CHECK) \ > - hook(BPRM_CHECK) \ > - hook(CREDS_CHECK) \ > - hook(POST_SETATTR) \ > - hook(MODULE_CHECK) \ > - hook(FIRMWARE_CHECK) \ > - hook(KEXEC_KERNEL_CHECK) \ > - hook(KEXEC_INITRAMFS_CHECK) \ > - hook(POLICY_CHECK) \ > - hook(MAX_CHECK) > -#define __ima_hook_enumify(ENUM) ENUM, > - > -enum ima_hooks { > - __ima_hooks(__ima_hook_enumify) > -}; > - > /* LIM API function definitions */ > int ima_get_action(struct inode *inode, const struct cred *cred, u32 secid, > int mask, enum ima_hooks func, int *pcr); > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c > index 8bc8a1c8cb3f..adeae1ab9ee9 100644 > --- a/security/integrity/ima/ima_policy.c > +++ b/security/integrity/ima/ima_policy.c > @@ -21,6 +21,7 @@ > #include <linux/genhd.h> > #include <linux/seq_file.h> > #include <linux/ima.h> > +#include <linux/evm.h> > > #include "ima.h" > > @@ -1336,4 +1337,46 @@ int ima_policy_show(struct seq_file *m, void *v) > seq_puts(m, "\n"); > return 0; > } > + > #endif /* CONFIG_IMA_READ_POLICY */ > + > +#if defined(CONFIG_IMA_APPRAISE) && defined(CONFIG_INTEGRITY_TRUSTED_KEYRING) > +/* > + * ima_appraise_signature: whether IMA will appraise a given function using > + * an IMA digital signature. This is restricted to cases where the kernel > + * has a set of built-in trusted keys in order to avoid an attacker simply > + * loading additional keys. > + */ > +bool ima_appraise_signature(enum ima_hooks func) > +{ > + struct ima_rule_entry *entry; > + bool found = false; > + > + rcu_read_lock(); > + list_for_each_entry_rcu(entry, ima_rules, list) { > + if (entry->action != APPRAISE) > + continue; > + > + /* A generic entry will match, but otherwise require that it > + * match the func we're looking for > + */ > + if (entry->func && entry->func != func) > + continue; > + > + /* We require this to be a digital signature, not a raw IMA > + * hash. > + */ Comments should either be a single line or "/*" on a separate line. > + if (entry->flags & IMA_DIGSIG_REQUIRED) > + found = true; > + > + /* We've found a rule that matches, so break now even if it > + * didn't require a digital signature - a later rule that does > + * won't override it, so would be a false positive. > + */ > + break; > + } > + Much better. thanks, Mimi > + rcu_read_unlock(); > + return found; > +} > +#endif /* CONFIG_IMA_APPRAISE && CONFIG_INTEGRITY_TRUSTED_KEYRING */ ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC] kexec: Allow kexec_file() with appropriate IMA policy when locked down @ 2019-03-17 11:39 ` Mimi Zohar 0 siblings, 0 replies; 15+ messages in thread From: Mimi Zohar @ 2019-03-17 11:39 UTC (permalink / raw) To: Matthew Garrett, linux-integrity Cc: dmitry.kasatkin, kexec, Matthew Garrett, dhowells, Eric W. Biederman, Dave Young On Fri, 2019-03-15 at 15:03 -0700, Matthew Garrett wrote: > Systems in lockdown mode should block the kexec of untrusted kernels. > For x86 and ARM we can ensure that a kernel is trustworthy by validating > a PE signature, but this isn't possible on other architectures. On those > platforms we can use IMA digital signatures instead. Add a function to > determine whether IMA will verify signatures for a given event type, In both the kexec and kernel modules cases, this should be in the past tense. Perhaps change it to something like, "whether IMA has already or will verify signatures ...". > and > if so permit kexec_file() even if the kernel is otherwise locked down. > This is restricted to cases where CONFIG_INTEGRITY_TRUSTED_KEYRING is set > in order to prevent an attacker from loading additional keys at runtime. > > Signed-off-by: Matthew Garrett <mjg59@google.com> > --- > include/linux/evm.h | 6 ++++ > include/linux/ima.h | 28 +++++++++++++++++++ > kernel/kexec_file.c | 9 ++++-- > security/integrity/evm/evm_main.c | 2 +- > security/integrity/ima/ima.h | 20 +------------- > security/integrity/ima/ima_policy.c | 43 +++++++++++++++++++++++++++++ > 6 files changed, 86 insertions(+), 22 deletions(-) > > diff --git a/include/linux/evm.h b/include/linux/evm.h > index 8302bc29bb35..6e89d046b716 100644 > --- a/include/linux/evm.h > +++ b/include/linux/evm.h > @@ -15,6 +15,7 @@ > struct integrity_iint_cache; > > #ifdef CONFIG_EVM > +extern bool evm_key_loaded(void); > extern int evm_set_key(void *key, size_t keylen); > extern enum integrity_status evm_verifyxattr(struct dentry *dentry, > const char *xattr_name, > @@ -45,6 +46,11 @@ static inline int posix_xattr_acl(const char *xattrname) > #endif > #else > > +static inline bool evm_key_loaded(void) > +{ > + return false; > +} > + Remove remaining EVM fragment. > static inline int evm_set_key(void *key, size_t keylen) > { > return -EOPNOTSUPP; > diff --git a/include/linux/ima.h b/include/linux/ima.h > index dc12fbcf484c..a42e2a9a08b7 100644 > --- a/include/linux/ima.h > +++ b/include/linux/ima.h > @@ -27,6 +27,25 @@ extern int ima_post_read_file(struct file *file, void *buf, loff_t size, > enum kernel_read_file_id id); > extern void ima_post_path_mknod(struct dentry *dentry); > > +#define __ima_hooks(hook) \ > + hook(NONE) \ > + hook(FILE_CHECK) \ > + hook(MMAP_CHECK) \ > + hook(BPRM_CHECK) \ > + hook(CREDS_CHECK) \ > + hook(POST_SETATTR) \ > + hook(MODULE_CHECK) \ > + hook(FIRMWARE_CHECK) \ > + hook(KEXEC_KERNEL_CHECK) \ > + hook(KEXEC_INITRAMFS_CHECK) \ > + hook(POLICY_CHECK) \ > + hook(MAX_CHECK) > +#define __ima_hook_enumify(ENUM) ENUM, > + > +enum ima_hooks { > + __ima_hooks(__ima_hook_enumify) > +}; > + > #ifdef CONFIG_IMA_KEXEC > extern void ima_add_kexec_buffer(struct kimage *image); > #endif > @@ -132,4 +151,13 @@ static inline int ima_inode_removexattr(struct dentry *dentry, > return 0; > } > #endif /* CONFIG_IMA_APPRAISE */ > + > +#if defined(CONFIG_IMA_APPRAISE) && defined(CONFIG_INTEGRITY_TRUSTED_KEYRING) > +extern bool ima_appraise_signature(enum ima_hooks func); > +#else > +static inline bool ima_appraise_kexec_signature(enum ima_hooks func) > +{ > + return false; > +} > +#endif /* CONFIG_IMA_APPRAISE && CONFIG_INTEGRITY_TRUSTED_KEYRING */ > #endif /* _LINUX_IMA_H */ > diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c > index 0cfe4f6f7f85..3e04506a00a2 100644 > --- a/kernel/kexec_file.c > +++ b/kernel/kexec_file.c > @@ -20,11 +20,11 @@ > #include <linux/mutex.h> > #include <linux/list.h> > #include <linux/fs.h> > -#include <linux/ima.h> > #include <crypto/hash.h> > #include <crypto/sha.h> > #include <linux/elf.h> > #include <linux/elfcore.h> > +#include <linux/ima.h> > #include <linux/kernel.h> > #include <linux/syscalls.h> > #include <linux/vmalloc.h> > @@ -240,7 +240,12 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd, > > ret = 0; > > - if (kernel_is_locked_down(reason)) { > + /* If IMA is guaranteed to appraise a signature on the kexec > + * image, permit it even if the kernel is otherwise locked > + * down. > + */ > + if (!ima_appraise_signature(KEXEC_KERNEL_CHECK) && > + kernel_is_locked_down(reason)) { > ret = -EPERM; > goto out; [Cc'ing Dave Young, Eric Biederman, kexec mailing list] There was a discussion about using KEXEC_KERNEL_CHECK as an argument when replacing copy_file_from_fd() with kernel_read_file_from_fd(). There was a subsequent discussion when adding a security call in kexec_load_check. The end result was defining two enumerations named kernel_read_file_id and kernel_load_data_id with READING_KEXEC_IMAGE and LOADING_KECEC_IMAGE respectively. Instead of making the ima_hooks enumeration global, as we're already relying on READING_KEXEC_IMAGE, use it. > } > diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c > index b6d9f14bc234..aad61bc0f774 100644 > --- a/security/integrity/evm/evm_main.c > +++ b/security/integrity/evm/evm_main.c > @@ -87,7 +87,7 @@ static void __init evm_init_config(void) > pr_info("HMAC attrs: 0x%x\n", evm_hmac_attrs); > } > > -static bool evm_key_loaded(void) > +bool evm_key_loaded(void) > { > return (bool)(evm_initialized & EVM_KEY_MASK); > } Remove remaining EVM fragment. > diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h > index cc12f3449a72..71614a8ed2aa 100644 > --- a/security/integrity/ima/ima.h > +++ b/security/integrity/ima/ima.h > @@ -20,6 +20,7 @@ > #include <linux/types.h> > #include <linux/crypto.h> > #include <linux/fs.h> > +#include <linux/ima.h> > #include <linux/security.h> > #include <linux/hash.h> > #include <linux/tpm.h> > @@ -171,25 +172,6 @@ static inline unsigned long ima_hash_key(u8 *digest) > return hash_long(*digest, IMA_HASH_BITS); > } > > -#define __ima_hooks(hook) \ > - hook(NONE) \ > - hook(FILE_CHECK) \ > - hook(MMAP_CHECK) \ > - hook(BPRM_CHECK) \ > - hook(CREDS_CHECK) \ > - hook(POST_SETATTR) \ > - hook(MODULE_CHECK) \ > - hook(FIRMWARE_CHECK) \ > - hook(KEXEC_KERNEL_CHECK) \ > - hook(KEXEC_INITRAMFS_CHECK) \ > - hook(POLICY_CHECK) \ > - hook(MAX_CHECK) > -#define __ima_hook_enumify(ENUM) ENUM, > - > -enum ima_hooks { > - __ima_hooks(__ima_hook_enumify) > -}; > - > /* LIM API function definitions */ > int ima_get_action(struct inode *inode, const struct cred *cred, u32 secid, > int mask, enum ima_hooks func, int *pcr); > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c > index 8bc8a1c8cb3f..adeae1ab9ee9 100644 > --- a/security/integrity/ima/ima_policy.c > +++ b/security/integrity/ima/ima_policy.c > @@ -21,6 +21,7 @@ > #include <linux/genhd.h> > #include <linux/seq_file.h> > #include <linux/ima.h> > +#include <linux/evm.h> > > #include "ima.h" > > @@ -1336,4 +1337,46 @@ int ima_policy_show(struct seq_file *m, void *v) > seq_puts(m, "\n"); > return 0; > } > + > #endif /* CONFIG_IMA_READ_POLICY */ > + > +#if defined(CONFIG_IMA_APPRAISE) && defined(CONFIG_INTEGRITY_TRUSTED_KEYRING) > +/* > + * ima_appraise_signature: whether IMA will appraise a given function using > + * an IMA digital signature. This is restricted to cases where the kernel > + * has a set of built-in trusted keys in order to avoid an attacker simply > + * loading additional keys. > + */ > +bool ima_appraise_signature(enum ima_hooks func) > +{ > + struct ima_rule_entry *entry; > + bool found = false; > + > + rcu_read_lock(); > + list_for_each_entry_rcu(entry, ima_rules, list) { > + if (entry->action != APPRAISE) > + continue; > + > + /* A generic entry will match, but otherwise require that it > + * match the func we're looking for > + */ > + if (entry->func && entry->func != func) > + continue; > + > + /* We require this to be a digital signature, not a raw IMA > + * hash. > + */ Comments should either be a single line or "/*" on a separate line. > + if (entry->flags & IMA_DIGSIG_REQUIRED) > + found = true; > + > + /* We've found a rule that matches, so break now even if it > + * didn't require a digital signature - a later rule that does > + * won't override it, so would be a false positive. > + */ > + break; > + } > + Much better. thanks, Mimi > + rcu_read_unlock(); > + return found; > +} > +#endif /* CONFIG_IMA_APPRAISE && CONFIG_INTEGRITY_TRUSTED_KEYRING */ _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply [flat|nested] 15+ messages in thread
* [RFC] kexec: Allow kexec_file() with appropriate IMA policy when locked down 2019-03-17 11:39 ` Mimi Zohar @ 2019-03-18 19:51 ` Matthew Garrett -1 siblings, 0 replies; 15+ messages in thread From: Matthew Garrett @ 2019-03-18 19:51 UTC (permalink / raw) To: linux-integrity Cc: dhowells, zohar, dmitry.kasatkin, kexec, dyoung, ebiederm, Matthew Garrett, Matthew Garrett Systems in lockdown mode should block the kexec of untrusted kernels. For x86 and ARM we can ensure that a kernel is trustworthy by validating a PE signature, but this isn't possible on other architectures. On those platforms we can use IMA digital signatures instead. Add a function to determine whether IMA has or will verify signatures for a given event type, and if so permit kexec_file() even if the kernel is otherwise locked down. This is restricted to cases where CONFIG_INTEGRITY_TRUSTED_KEYRING is set in order to prevent an attacker from loading additional keys at runtime. Signed-off-by: Matthew Garrett <mjg59@google.com> --- include/linux/ima.h | 9 +++++ kernel/kexec_file.c | 9 +++-- security/integrity/ima/ima.h | 3 ++ security/integrity/ima/ima_main.c | 2 +- security/integrity/ima/ima_policy.c | 52 +++++++++++++++++++++++++++++ 5 files changed, 72 insertions(+), 3 deletions(-) diff --git a/include/linux/ima.h b/include/linux/ima.h index dc12fbcf484c..a78c04580a3c 100644 --- a/include/linux/ima.h +++ b/include/linux/ima.h @@ -132,4 +132,13 @@ static inline int ima_inode_removexattr(struct dentry *dentry, return 0; } #endif /* CONFIG_IMA_APPRAISE */ + +#if defined(CONFIG_IMA_APPRAISE) && defined(CONFIG_INTEGRITY_TRUSTED_KEYRING) +extern bool ima_appraise_signature(enum kernel_read_file_id func); +#else +static inline bool ima_appraise_kexec_signature(enum kernel_read_file_id func) +{ + return false; +} +#endif /* CONFIG_IMA_APPRAISE && CONFIG_INTEGRITY_TRUSTED_KEYRING */ #endif /* _LINUX_IMA_H */ diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c index 0cfe4f6f7f85..3598f93a731a 100644 --- a/kernel/kexec_file.c +++ b/kernel/kexec_file.c @@ -20,11 +20,11 @@ #include <linux/mutex.h> #include <linux/list.h> #include <linux/fs.h> -#include <linux/ima.h> #include <crypto/hash.h> #include <crypto/sha.h> #include <linux/elf.h> #include <linux/elfcore.h> +#include <linux/ima.h> #include <linux/kernel.h> #include <linux/syscalls.h> #include <linux/vmalloc.h> @@ -240,7 +240,12 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd, ret = 0; - if (kernel_is_locked_down(reason)) { + /* If IMA is guaranteed to appraise a signature on the kexec + * image, permit it even if the kernel is otherwise locked + * down. + */ + if (!ima_appraise_signature(READING_KEXEC_IMAGE) && + kernel_is_locked_down(reason)) { ret = -EPERM; goto out; } diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index cc12f3449a72..93a6825bfcaf 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -20,6 +20,7 @@ #include <linux/types.h> #include <linux/crypto.h> #include <linux/fs.h> +#include <linux/ima.h> #include <linux/security.h> #include <linux/hash.h> #include <linux/tpm.h> @@ -115,6 +116,8 @@ struct ima_kexec_hdr { u64 count; }; +extern const int read_idmap[]; + #ifdef CONFIG_HAVE_IMA_KEXEC void ima_load_kexec_buffer(void); #else diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 357edd140c09..927fe889201a 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -473,7 +473,7 @@ int ima_read_file(struct file *file, enum kernel_read_file_id read_id) return 0; } -static const int read_idmap[READING_MAX_ID] = { +const int read_idmap[READING_MAX_ID] = { [READING_FIRMWARE] = FIRMWARE_CHECK, [READING_FIRMWARE_PREALLOC_BUFFER] = FIRMWARE_CHECK, [READING_MODULE] = MODULE_CHECK, diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 8bc8a1c8cb3f..bbd1d5b3d45a 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -21,6 +21,7 @@ #include <linux/genhd.h> #include <linux/seq_file.h> #include <linux/ima.h> +#include <linux/evm.h> #include "ima.h" @@ -1336,4 +1337,55 @@ int ima_policy_show(struct seq_file *m, void *v) seq_puts(m, "\n"); return 0; } + #endif /* CONFIG_IMA_READ_POLICY */ + +#if defined(CONFIG_IMA_APPRAISE) && defined(CONFIG_INTEGRITY_TRUSTED_KEYRING) +/* + * ima_appraise_signature: whether IMA will appraise a given function using + * an IMA digital signature. This is restricted to cases where the kernel + * has a set of built-in trusted keys in order to avoid an attacker simply + * loading additional keys. + */ +bool ima_appraise_signature(enum kernel_read_file_id id) +{ + struct ima_rule_entry *entry; + bool found = false; + enum ima_hooks func; + + if (id >= READING_MAX_ID) + return false; + + func = read_idmap[id] ?: FILE_CHECK; + + rcu_read_lock(); + list_for_each_entry_rcu(entry, ima_rules, list) { + if (entry->action != APPRAISE) + continue; + + /* + * A generic entry will match, but otherwise require that it + * match the func we're looking for + */ + if (entry->func && entry->func != func) + continue; + + /* + * We require this to be a digital signature, not a raw IMA + * hash. + */ + if (entry->flags & IMA_DIGSIG_REQUIRED) + found = true; + + /* + * We've found a rule that matches, so break now even if it + * didn't require a digital signature - a later rule that does + * won't override it, so would be a false positive. + */ + break; + } + + rcu_read_unlock(); + return found; +} +#endif /* CONFIG_IMA_APPRAISE && CONFIG_INTEGRITY_TRUSTED_KEYRING */ -- 2.21.0.225.g810b269d1ac-goog ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [RFC] kexec: Allow kexec_file() with appropriate IMA policy when locked down @ 2019-03-18 19:51 ` Matthew Garrett 0 siblings, 0 replies; 15+ messages in thread From: Matthew Garrett @ 2019-03-18 19:51 UTC (permalink / raw) To: linux-integrity Cc: Matthew Garrett, dmitry.kasatkin, kexec, zohar, dhowells, Matthew Garrett, ebiederm, dyoung Systems in lockdown mode should block the kexec of untrusted kernels. For x86 and ARM we can ensure that a kernel is trustworthy by validating a PE signature, but this isn't possible on other architectures. On those platforms we can use IMA digital signatures instead. Add a function to determine whether IMA has or will verify signatures for a given event type, and if so permit kexec_file() even if the kernel is otherwise locked down. This is restricted to cases where CONFIG_INTEGRITY_TRUSTED_KEYRING is set in order to prevent an attacker from loading additional keys at runtime. Signed-off-by: Matthew Garrett <mjg59@google.com> --- include/linux/ima.h | 9 +++++ kernel/kexec_file.c | 9 +++-- security/integrity/ima/ima.h | 3 ++ security/integrity/ima/ima_main.c | 2 +- security/integrity/ima/ima_policy.c | 52 +++++++++++++++++++++++++++++ 5 files changed, 72 insertions(+), 3 deletions(-) diff --git a/include/linux/ima.h b/include/linux/ima.h index dc12fbcf484c..a78c04580a3c 100644 --- a/include/linux/ima.h +++ b/include/linux/ima.h @@ -132,4 +132,13 @@ static inline int ima_inode_removexattr(struct dentry *dentry, return 0; } #endif /* CONFIG_IMA_APPRAISE */ + +#if defined(CONFIG_IMA_APPRAISE) && defined(CONFIG_INTEGRITY_TRUSTED_KEYRING) +extern bool ima_appraise_signature(enum kernel_read_file_id func); +#else +static inline bool ima_appraise_kexec_signature(enum kernel_read_file_id func) +{ + return false; +} +#endif /* CONFIG_IMA_APPRAISE && CONFIG_INTEGRITY_TRUSTED_KEYRING */ #endif /* _LINUX_IMA_H */ diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c index 0cfe4f6f7f85..3598f93a731a 100644 --- a/kernel/kexec_file.c +++ b/kernel/kexec_file.c @@ -20,11 +20,11 @@ #include <linux/mutex.h> #include <linux/list.h> #include <linux/fs.h> -#include <linux/ima.h> #include <crypto/hash.h> #include <crypto/sha.h> #include <linux/elf.h> #include <linux/elfcore.h> +#include <linux/ima.h> #include <linux/kernel.h> #include <linux/syscalls.h> #include <linux/vmalloc.h> @@ -240,7 +240,12 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd, ret = 0; - if (kernel_is_locked_down(reason)) { + /* If IMA is guaranteed to appraise a signature on the kexec + * image, permit it even if the kernel is otherwise locked + * down. + */ + if (!ima_appraise_signature(READING_KEXEC_IMAGE) && + kernel_is_locked_down(reason)) { ret = -EPERM; goto out; } diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index cc12f3449a72..93a6825bfcaf 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -20,6 +20,7 @@ #include <linux/types.h> #include <linux/crypto.h> #include <linux/fs.h> +#include <linux/ima.h> #include <linux/security.h> #include <linux/hash.h> #include <linux/tpm.h> @@ -115,6 +116,8 @@ struct ima_kexec_hdr { u64 count; }; +extern const int read_idmap[]; + #ifdef CONFIG_HAVE_IMA_KEXEC void ima_load_kexec_buffer(void); #else diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 357edd140c09..927fe889201a 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -473,7 +473,7 @@ int ima_read_file(struct file *file, enum kernel_read_file_id read_id) return 0; } -static const int read_idmap[READING_MAX_ID] = { +const int read_idmap[READING_MAX_ID] = { [READING_FIRMWARE] = FIRMWARE_CHECK, [READING_FIRMWARE_PREALLOC_BUFFER] = FIRMWARE_CHECK, [READING_MODULE] = MODULE_CHECK, diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 8bc8a1c8cb3f..bbd1d5b3d45a 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -21,6 +21,7 @@ #include <linux/genhd.h> #include <linux/seq_file.h> #include <linux/ima.h> +#include <linux/evm.h> #include "ima.h" @@ -1336,4 +1337,55 @@ int ima_policy_show(struct seq_file *m, void *v) seq_puts(m, "\n"); return 0; } + #endif /* CONFIG_IMA_READ_POLICY */ + +#if defined(CONFIG_IMA_APPRAISE) && defined(CONFIG_INTEGRITY_TRUSTED_KEYRING) +/* + * ima_appraise_signature: whether IMA will appraise a given function using + * an IMA digital signature. This is restricted to cases where the kernel + * has a set of built-in trusted keys in order to avoid an attacker simply + * loading additional keys. + */ +bool ima_appraise_signature(enum kernel_read_file_id id) +{ + struct ima_rule_entry *entry; + bool found = false; + enum ima_hooks func; + + if (id >= READING_MAX_ID) + return false; + + func = read_idmap[id] ?: FILE_CHECK; + + rcu_read_lock(); + list_for_each_entry_rcu(entry, ima_rules, list) { + if (entry->action != APPRAISE) + continue; + + /* + * A generic entry will match, but otherwise require that it + * match the func we're looking for + */ + if (entry->func && entry->func != func) + continue; + + /* + * We require this to be a digital signature, not a raw IMA + * hash. + */ + if (entry->flags & IMA_DIGSIG_REQUIRED) + found = true; + + /* + * We've found a rule that matches, so break now even if it + * didn't require a digital signature - a later rule that does + * won't override it, so would be a false positive. + */ + break; + } + + rcu_read_unlock(); + return found; +} +#endif /* CONFIG_IMA_APPRAISE && CONFIG_INTEGRITY_TRUSTED_KEYRING */ -- 2.21.0.225.g810b269d1ac-goog _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply related [flat|nested] 15+ messages in thread
end of thread, other threads:[~2019-03-18 19:51 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-03-12 19:57 [RFC] kexec: Allow kexec_file() with appropriate IMA policy when locked down Matthew Garrett 2019-03-13 11:58 ` Mimi Zohar 2019-03-13 20:36 ` Matthew Garrett 2019-03-13 21:29 ` Mimi Zohar 2019-03-13 21:59 ` Matthew Garrett 2019-03-14 1:08 ` Mimi Zohar 2019-03-14 21:08 ` Matthew Garrett 2019-03-14 22:31 ` Mimi Zohar 2019-03-14 22:54 ` Matthew Garrett 2019-03-14 23:58 ` Mimi Zohar 2019-03-15 22:03 ` Matthew Garrett 2019-03-17 11:39 ` Mimi Zohar 2019-03-17 11:39 ` Mimi Zohar 2019-03-18 19:51 ` Matthew Garrett 2019-03-18 19:51 ` Matthew Garrett
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.