* RE: [PATCH 1/6] IMA: Allow EVM validation on appraisal even without a symmetric key @ 2017-10-19 16:12 Dmitry Kasatkin 0 siblings, 0 replies; 5+ messages in thread From: Dmitry Kasatkin @ 2017-10-19 16:12 UTC (permalink / raw) To: linux-integrity; +Cc: Matthew Garrett, Mimi Zohar https://www.spinics.net/lists/linux-integrity/msg00036.html Missed those because of ML switch.. Why is this needed? This patch in upstream enables EVM when X509 is loaded evm: enable EVM when X509 certificate is loaded https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=26ddabfe96bb7468763c9c92791404d991b16250 -- Thanks, Dmitry ^ permalink raw reply [flat|nested] 5+ messages in thread
* RFC: Make it practical to ship EVM signatures @ 2017-09-27 22:16 Matthew Garrett 2017-09-27 22:16 ` [PATCH 1/6] IMA: Allow EVM validation on appraisal even without a symmetric key Matthew Garrett 0 siblings, 1 reply; 5+ messages in thread From: Matthew Garrett @ 2017-09-27 22:16 UTC (permalink / raw) To: linux-integrity; +Cc: zohar These are basically untested, but I'd like to get some feedback on the problem I'm trying to solve here. We'd like to be able to ship packages with verifiable security xattrs, but right now EVM makes this difficult due to its requirement that the inode number be encoded in the hmac. This patchset is intended to make it possible to protect a subset of metadata rather than all of it, and also to permit using EVM digital signatures in a similar way to how IMA digital signatures can be used now (ie, protecting the metadata using public/private crypto rather than having a local symmetric key and generating the HMACs locally). The expected workflow is: 1) During package build or mirroring process, appropriate security metadata is added (IMA hash, selinux label, etc) 2) An EVM digital signature is generated based purely on the security metadata present during the build or mirroring process 3) IMA is extended to allow it to force EVM validation during appraisal even if no symmetric EVM key has been added, which allows IMA appraisal to appraise not only the IMA hash but also the additional metadata 4) If EVM is never enabled, binaries are purely validated using the EVM digital signatures and are not transitioned to using HMACs 5) If EVM is desired, userland can set the set of metadata to be incorporated into the EVM HMAC before enabling EVM ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/6] IMA: Allow EVM validation on appraisal even without a symmetric key 2017-09-27 22:16 RFC: Make it practical to ship EVM signatures Matthew Garrett @ 2017-09-27 22:16 ` Matthew Garrett 2017-10-01 2:08 ` Mimi Zohar 0 siblings, 1 reply; 5+ messages in thread From: Matthew Garrett @ 2017-09-27 22:16 UTC (permalink / raw) To: linux-integrity; +Cc: zohar, Matthew Garrett A reasonable configuration is to use IMA to appraise a subset of files (based on user, security label or other features supported by IMA) but to also want to use EVM to validate not only the state of the IMA hash but also additional metadata on the file. Right now this is only possible if a symmetric key has been loaded, which may not be desirable in all cases (eg, one where EVM digital signatures are shipped to end systems rather than EVM HMACs being generated locally). Add an additional "require_evm" keyword to the IMA policy language in order to permit the local admin to indicate that they wish EVM validation to occur even if no symmetric key has been loaded. Signed-off-by: Matthew Garrett <mjg59@google.com> --- Documentation/ABI/testing/ima_policy | 3 ++- include/linux/evm.h | 6 ++++-- security/integrity/evm/evm_main.c | 6 ++++-- security/integrity/ima/ima_appraise.c | 11 ++++++++++- security/integrity/ima/ima_policy.c | 12 +++++++++++- security/integrity/integrity.h | 3 ++- 6 files changed, 33 insertions(+), 8 deletions(-) diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy index 5dc9eed035fb..ea2703c847f6 100644 --- a/Documentation/ABI/testing/ima_policy +++ b/Documentation/ABI/testing/ima_policy @@ -23,7 +23,8 @@ Description: [euid=] [fowner=]] lsm: [[subj_user=] [subj_role=] [subj_type=] [obj_user=] [obj_role=] [obj_type=]] - option: [[appraise_type=]] [permit_directio] + option: [[appraise_type=] [permit_directio] + [require_evm]] base: func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK] [FIRMWARE_CHECK] diff --git a/include/linux/evm.h b/include/linux/evm.h index 35ed9a8a403a..7661f3085942 100644 --- a/include/linux/evm.h +++ b/include/linux/evm.h @@ -19,7 +19,8 @@ extern enum integrity_status evm_verifyxattr(struct dentry *dentry, const char *xattr_name, void *xattr_value, size_t xattr_value_len, - struct integrity_iint_cache *iint); + struct integrity_iint_cache *iint, + bool force); extern int evm_inode_setattr(struct dentry *dentry, struct iattr *attr); extern void evm_inode_post_setattr(struct dentry *dentry, int ia_valid); extern int evm_inode_setxattr(struct dentry *dentry, const char *name, @@ -54,7 +55,8 @@ static inline enum integrity_status evm_verifyxattr(struct dentry *dentry, const char *xattr_name, void *xattr_value, size_t xattr_value_len, - struct integrity_iint_cache *iint) + struct integrity_iint_cache *iint, + bool force) { return INTEGRITY_UNKNOWN; } diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index 063d38aef64e..44e4f4fda965 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -223,6 +223,7 @@ static int evm_protected_xattr(const char *req_xattr_name) * @xattr_name: requested xattr * @xattr_value: requested xattr value * @xattr_value_len: requested xattr value length + * @force: force verification even if no EVM symmetric key is loaded * * Calculate the HMAC for the given dentry and verify it against the stored * security.evm xattr. For performance, use the xattr value and length @@ -236,9 +237,10 @@ static int evm_protected_xattr(const char *req_xattr_name) enum integrity_status evm_verifyxattr(struct dentry *dentry, const char *xattr_name, void *xattr_value, size_t xattr_value_len, - struct integrity_iint_cache *iint) + struct integrity_iint_cache *iint, + bool force) { - if (!evm_initialized || !evm_protected_xattr(xattr_name)) + if ((!evm_initialized || !evm_protected_xattr(xattr_name)) && !force) return INTEGRITY_UNKNOWN; if (!iint) { diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index edb82e722a0d..9df1148f17cc 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -217,6 +217,7 @@ int ima_appraise_measurement(enum ima_hooks func, struct inode *inode = d_backing_inode(dentry); enum integrity_status status = INTEGRITY_UNKNOWN; int rc = xattr_len, hash_start = 0; + bool evm_force = false; if (!(inode->i_opflags & IOP_XATTR)) return INTEGRITY_UNKNOWN; @@ -236,7 +237,15 @@ int ima_appraise_measurement(enum ima_hooks func, goto out; } - status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value, rc, iint); + /* + * Check if policy specifies that we should perform EVM + * validation even in the absence of an EVM symmetric key + */ + if (iint->flags & IMA_EVM_REQUIRED) + evm_force = true; + + status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value, rc, iint, + evm_force); if ((status != INTEGRITY_PASS) && (status != INTEGRITY_UNKNOWN)) { if ((status == INTEGRITY_NOLABEL) || (status == INTEGRITY_NOXATTRS)) diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index a6e14c532627..db4a0c968e00 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -531,7 +531,7 @@ enum { Opt_uid_gt, Opt_euid_gt, Opt_fowner_gt, Opt_uid_lt, Opt_euid_lt, Opt_fowner_lt, Opt_appraise_type, Opt_permit_directio, - Opt_pcr + Opt_pcr, Opt_require_evm, }; static match_table_t policy_tokens = { @@ -562,6 +562,7 @@ static match_table_t policy_tokens = { {Opt_appraise_type, "appraise_type=%s"}, {Opt_permit_directio, "permit_directio"}, {Opt_pcr, "pcr=%s"}, + {Opt_require_evm, "require_evm"}, {Opt_err, NULL} }; @@ -876,6 +877,13 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) else entry->flags |= IMA_PCR; + break; + case Opt_require_evm: + if (entry->action != APPRAISE) { + result = -EINVAL; + break; + } + entry->flags |= IMA_EVM_REQUIRED; break; case Opt_err: ima_log_string(ab, "UNKNOWN", p); @@ -1142,6 +1150,8 @@ int ima_policy_show(struct seq_file *m, void *v) } } } + if (entry->flags & IMA_EVM_REQUIRED) + seq_puts(m, "require_evm "); if (entry->flags & IMA_DIGSIG_REQUIRED) seq_puts(m, "appraise_type=imasig "); if (entry->flags & IMA_PERMIT_DIRECTIO) diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h index 45ba0e4501d6..2fa0d7bc55fb 100644 --- a/security/integrity/integrity.h +++ b/security/integrity/integrity.h @@ -28,11 +28,12 @@ /* iint cache flags */ #define IMA_ACTION_FLAGS 0xff000000 -#define IMA_ACTION_RULE_FLAGS 0x06000000 +#define IMA_ACTION_RULE_FLAGS 0x16000000 #define IMA_DIGSIG 0x01000000 #define IMA_DIGSIG_REQUIRED 0x02000000 #define IMA_PERMIT_DIRECTIO 0x04000000 #define IMA_NEW_FILE 0x08000000 +#define IMA_EVM_REQUIRED 0x10000000 #define IMA_DO_MASK (IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \ IMA_APPRAISE_SUBMASK) -- 2.14.2.822.g60be5d43e6-goog ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/6] IMA: Allow EVM validation on appraisal even without a symmetric key 2017-09-27 22:16 ` [PATCH 1/6] IMA: Allow EVM validation on appraisal even without a symmetric key Matthew Garrett @ 2017-10-01 2:08 ` Mimi Zohar 2017-10-02 17:02 ` Matthew Garrett 0 siblings, 1 reply; 5+ messages in thread From: Mimi Zohar @ 2017-10-01 2:08 UTC (permalink / raw) To: Matthew Garrett, linux-integrity On Wed, 2017-09-27 at 15:16 -0700, Matthew Garrett wrote: > A reasonable configuration is to use IMA to appraise a subset of files > (based on user, security label or other features supported by IMA) but > to also want to use EVM to validate not only the state of the IMA hash > but also additional metadata on the file. Right now this is only > possible if a symmetric key has been loaded, which may not be desirable > in all cases (eg, one where EVM digital signatures are shipped to end > systems rather than EVM HMACs being generated locally). Commit 26ddabfe96bb "evm: enable EVM when X509 certificate is loaded" already allows EVM to be enabled without loading a symmetric key. Mimi > Add an > additional "require_evm" keyword to the IMA policy language in order to > permit the local admin to indicate that they wish EVM validation to > occur even if no symmetric key has been loaded. > > Signed-off-by: Matthew Garrett <mjg59@google.com> > --- > Documentation/ABI/testing/ima_policy | 3 ++- > include/linux/evm.h | 6 ++++-- > security/integrity/evm/evm_main.c | 6 ++++-- > security/integrity/ima/ima_appraise.c | 11 ++++++++++- > security/integrity/ima/ima_policy.c | 12 +++++++++++- > security/integrity/integrity.h | 3 ++- > 6 files changed, 33 insertions(+), 8 deletions(-) > > diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy > index 5dc9eed035fb..ea2703c847f6 100644 > --- a/Documentation/ABI/testing/ima_policy > +++ b/Documentation/ABI/testing/ima_policy > @@ -23,7 +23,8 @@ Description: > [euid=] [fowner=]] > lsm: [[subj_user=] [subj_role=] [subj_type=] > [obj_user=] [obj_role=] [obj_type=]] > - option: [[appraise_type=]] [permit_directio] > + option: [[appraise_type=] [permit_directio] > + [require_evm]] > > base: func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK] > [FIRMWARE_CHECK] > diff --git a/include/linux/evm.h b/include/linux/evm.h > index 35ed9a8a403a..7661f3085942 100644 > --- a/include/linux/evm.h > +++ b/include/linux/evm.h > @@ -19,7 +19,8 @@ extern enum integrity_status evm_verifyxattr(struct dentry *dentry, > const char *xattr_name, > void *xattr_value, > size_t xattr_value_len, > - struct integrity_iint_cache *iint); > + struct integrity_iint_cache *iint, > + bool force); > extern int evm_inode_setattr(struct dentry *dentry, struct iattr *attr); > extern void evm_inode_post_setattr(struct dentry *dentry, int ia_valid); > extern int evm_inode_setxattr(struct dentry *dentry, const char *name, > @@ -54,7 +55,8 @@ static inline enum integrity_status evm_verifyxattr(struct dentry *dentry, > const char *xattr_name, > void *xattr_value, > size_t xattr_value_len, > - struct integrity_iint_cache *iint) > + struct integrity_iint_cache *iint, > + bool force) > { > return INTEGRITY_UNKNOWN; > } > diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c > index 063d38aef64e..44e4f4fda965 100644 > --- a/security/integrity/evm/evm_main.c > +++ b/security/integrity/evm/evm_main.c > @@ -223,6 +223,7 @@ static int evm_protected_xattr(const char *req_xattr_name) > * @xattr_name: requested xattr > * @xattr_value: requested xattr value > * @xattr_value_len: requested xattr value length > + * @force: force verification even if no EVM symmetric key is loaded > * > * Calculate the HMAC for the given dentry and verify it against the stored > * security.evm xattr. For performance, use the xattr value and length > @@ -236,9 +237,10 @@ static int evm_protected_xattr(const char *req_xattr_name) > enum integrity_status evm_verifyxattr(struct dentry *dentry, > const char *xattr_name, > void *xattr_value, size_t xattr_value_len, > - struct integrity_iint_cache *iint) > + struct integrity_iint_cache *iint, > + bool force) > { > - if (!evm_initialized || !evm_protected_xattr(xattr_name)) > + if ((!evm_initialized || !evm_protected_xattr(xattr_name)) && !force) > return INTEGRITY_UNKNOWN; > > if (!iint) { > diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c > index edb82e722a0d..9df1148f17cc 100644 > --- a/security/integrity/ima/ima_appraise.c > +++ b/security/integrity/ima/ima_appraise.c > @@ -217,6 +217,7 @@ int ima_appraise_measurement(enum ima_hooks func, > struct inode *inode = d_backing_inode(dentry); > enum integrity_status status = INTEGRITY_UNKNOWN; > int rc = xattr_len, hash_start = 0; > + bool evm_force = false; > > if (!(inode->i_opflags & IOP_XATTR)) > return INTEGRITY_UNKNOWN; > @@ -236,7 +237,15 @@ int ima_appraise_measurement(enum ima_hooks func, > goto out; > } > > - status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value, rc, iint); > + /* > + * Check if policy specifies that we should perform EVM > + * validation even in the absence of an EVM symmetric key > + */ > + if (iint->flags & IMA_EVM_REQUIRED) > + evm_force = true; > + > + status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value, rc, iint, > + evm_force); > if ((status != INTEGRITY_PASS) && (status != INTEGRITY_UNKNOWN)) { > if ((status == INTEGRITY_NOLABEL) > || (status == INTEGRITY_NOXATTRS)) > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c > index a6e14c532627..db4a0c968e00 100644 > --- a/security/integrity/ima/ima_policy.c > +++ b/security/integrity/ima/ima_policy.c > @@ -531,7 +531,7 @@ enum { > Opt_uid_gt, Opt_euid_gt, Opt_fowner_gt, > Opt_uid_lt, Opt_euid_lt, Opt_fowner_lt, > Opt_appraise_type, Opt_permit_directio, > - Opt_pcr > + Opt_pcr, Opt_require_evm, > }; > > static match_table_t policy_tokens = { > @@ -562,6 +562,7 @@ static match_table_t policy_tokens = { > {Opt_appraise_type, "appraise_type=%s"}, > {Opt_permit_directio, "permit_directio"}, > {Opt_pcr, "pcr=%s"}, > + {Opt_require_evm, "require_evm"}, > {Opt_err, NULL} > }; > > @@ -876,6 +877,13 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) > else > entry->flags |= IMA_PCR; > > + break; > + case Opt_require_evm: > + if (entry->action != APPRAISE) { > + result = -EINVAL; > + break; > + } > + entry->flags |= IMA_EVM_REQUIRED; > break; > case Opt_err: > ima_log_string(ab, "UNKNOWN", p); > @@ -1142,6 +1150,8 @@ int ima_policy_show(struct seq_file *m, void *v) > } > } > } > + if (entry->flags & IMA_EVM_REQUIRED) > + seq_puts(m, "require_evm "); > if (entry->flags & IMA_DIGSIG_REQUIRED) > seq_puts(m, "appraise_type=imasig "); > if (entry->flags & IMA_PERMIT_DIRECTIO) > diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h > index 45ba0e4501d6..2fa0d7bc55fb 100644 > --- a/security/integrity/integrity.h > +++ b/security/integrity/integrity.h > @@ -28,11 +28,12 @@ > > /* iint cache flags */ > #define IMA_ACTION_FLAGS 0xff000000 > -#define IMA_ACTION_RULE_FLAGS 0x06000000 > +#define IMA_ACTION_RULE_FLAGS 0x16000000 > #define IMA_DIGSIG 0x01000000 > #define IMA_DIGSIG_REQUIRED 0x02000000 > #define IMA_PERMIT_DIRECTIO 0x04000000 > #define IMA_NEW_FILE 0x08000000 > +#define IMA_EVM_REQUIRED 0x10000000 > > #define IMA_DO_MASK (IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \ > IMA_APPRAISE_SUBMASK) ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/6] IMA: Allow EVM validation on appraisal even without a symmetric key 2017-10-01 2:08 ` Mimi Zohar @ 2017-10-02 17:02 ` Matthew Garrett 2017-10-02 19:41 ` Mimi Zohar 0 siblings, 1 reply; 5+ messages in thread From: Matthew Garrett @ 2017-10-02 17:02 UTC (permalink / raw) To: Mimi Zohar; +Cc: linux-integrity On Sat, Sep 30, 2017 at 7:08 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > On Wed, 2017-09-27 at 15:16 -0700, Matthew Garrett wrote: >> A reasonable configuration is to use IMA to appraise a subset of files >> (based on user, security label or other features supported by IMA) but >> to also want to use EVM to validate not only the state of the IMA hash >> but also additional metadata on the file. Right now this is only >> possible if a symmetric key has been loaded, which may not be desirable >> in all cases (eg, one where EVM digital signatures are shipped to end >> systems rather than EVM HMACs being generated locally). > > Commit 26ddabfe96bb "evm: enable EVM when X509 certificate is loaded" > already allows EVM to be enabled without loading a symmetric key. This only seems to be set if CONFIG_EVM_LOAD_X509 is set. Should there be some sort of callback to set this if a key is loaded onto the evm keyring at runtime? ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/6] IMA: Allow EVM validation on appraisal even without a symmetric key 2017-10-02 17:02 ` Matthew Garrett @ 2017-10-02 19:41 ` Mimi Zohar 0 siblings, 0 replies; 5+ messages in thread From: Mimi Zohar @ 2017-10-02 19:41 UTC (permalink / raw) To: Matthew Garrett; +Cc: linux-integrity On Mon, 2017-10-02 at 10:02 -0700, Matthew Garrett wrote: > On Sat, Sep 30, 2017 at 7:08 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > > On Wed, 2017-09-27 at 15:16 -0700, Matthew Garrett wrote: > >> A reasonable configuration is to use IMA to appraise a subset of files > >> (based on user, security label or other features supported by IMA) but > >> to also want to use EVM to validate not only the state of the IMA hash > >> but also additional metadata on the file. Right now this is only > >> possible if a symmetric key has been loaded, which may not be desirable > >> in all cases (eg, one where EVM digital signatures are shipped to end > >> systems rather than EVM HMACs being generated locally). > > > > Commit 26ddabfe96bb "evm: enable EVM when X509 certificate is loaded" > > already allows EVM to be enabled without loading a symmetric key. > > This only seems to be set if CONFIG_EVM_LOAD_X509 is set. Should there > be some sort of callback to set this if a key is loaded onto the evm > keyring at runtime? Currently writing 1 to the securityfs file causes the EVM key to be loaded. I would extend the existing evm_write_key(). Writing 2, for example, might skip attempting to load the EVM key. You'll probably want to make sure that a public key has been loaded first. Mimi ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-10-19 16:12 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-10-19 16:12 [PATCH 1/6] IMA: Allow EVM validation on appraisal even without a symmetric key Dmitry Kasatkin -- strict thread matches above, loose matches on Subject: below -- 2017-09-27 22:16 RFC: Make it practical to ship EVM signatures Matthew Garrett 2017-09-27 22:16 ` [PATCH 1/6] IMA: Allow EVM validation on appraisal even without a symmetric key Matthew Garrett 2017-10-01 2:08 ` Mimi Zohar 2017-10-02 17:02 ` Matthew Garrett 2017-10-02 19:41 ` Mimi Zohar
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).