* [PATCH 0/2] evm: disable EVM on overlayfs @ 2023-12-19 13:48 Mimi Zohar 2023-12-19 13:49 ` [PATCH 1/2] evm: don't copy up 'security.evm' xattr Mimi Zohar 2023-12-19 13:49 ` [PATCH 2/2] evm: add support to disable EVM on unsupported filesystems Mimi Zohar 0 siblings, 2 replies; 5+ messages in thread From: Mimi Zohar @ 2023-12-19 13:48 UTC (permalink / raw) To: linux-unionfs Cc: Mimi Zohar, linux-integrity, linux-kernel, Amir Goldstein, Christian Brauner, Seth Forshee, Roberto Sassu EVM verifies the existing 'security.evm' value, before allowing it to be updated. The EVM HMAC and the original file signatures contain filesystem specific metadata (e.g. i_ino, i_generation and s_uuid). This poses a challenge when transitioning from the lower backing file to the upper backing file. Until a complete solution is developed, disable EVM on overlayfs. Mimi Zohar (2): evm: don't copy up 'security.evm' xattr evm: add support to disable EVM on unsupported filesystems include/linux/evm.h | 6 +++++ security/integrity/evm/evm_main.c | 42 ++++++++++++++++++++++++++++++- security/security.c | 4 +++ 3 files changed, 51 insertions(+), 1 deletion(-) -- 2.39.3 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] evm: don't copy up 'security.evm' xattr 2023-12-19 13:48 [PATCH 0/2] evm: disable EVM on overlayfs Mimi Zohar @ 2023-12-19 13:49 ` Mimi Zohar 2023-12-19 14:41 ` Amir Goldstein 2023-12-19 13:49 ` [PATCH 2/2] evm: add support to disable EVM on unsupported filesystems Mimi Zohar 1 sibling, 1 reply; 5+ messages in thread From: Mimi Zohar @ 2023-12-19 13:49 UTC (permalink / raw) To: linux-unionfs Cc: Mimi Zohar, linux-integrity, linux-kernel, Amir Goldstein, Christian Brauner, Seth Forshee, Roberto Sassu The security.evm HMAC and the original file signatures contain filesystem specific data. As a result, the HMAC and signature are not the same on the stacked and backing filesystems. Don't copy up 'security.evm'. Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> --- include/linux/evm.h | 6 ++++++ security/integrity/evm/evm_main.c | 7 +++++++ security/security.c | 4 ++++ 3 files changed, 17 insertions(+) diff --git a/include/linux/evm.h b/include/linux/evm.h index 01fc495a83e2..36ec884320d9 100644 --- a/include/linux/evm.h +++ b/include/linux/evm.h @@ -31,6 +31,7 @@ extern void evm_inode_post_setxattr(struct dentry *dentry, const char *xattr_name, const void *xattr_value, size_t xattr_value_len); +extern int evm_inode_copy_up_xattr(const char *name); extern int evm_inode_removexattr(struct mnt_idmap *idmap, struct dentry *dentry, const char *xattr_name); extern void evm_inode_post_removexattr(struct dentry *dentry, @@ -117,6 +118,11 @@ static inline void evm_inode_post_setxattr(struct dentry *dentry, return; } +static inline int evm_inode_copy_up_xattr(const char *name) +{ + return 0; +} + static inline int evm_inode_removexattr(struct mnt_idmap *idmap, struct dentry *dentry, const char *xattr_name) diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index 894570fe39bc..02adba635b02 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -863,6 +863,13 @@ void evm_inode_post_setattr(struct dentry *dentry, int ia_valid) evm_update_evmxattr(dentry, NULL, NULL, 0); } +int evm_inode_copy_up_xattr(const char *name) +{ + if (strcmp(name, XATTR_NAME_EVM) == 0) + return 1; /* Discard */ + return -EOPNOTSUPP; +} + /* * evm_inode_init_security - initializes security.evm HMAC value */ diff --git a/security/security.c b/security/security.c index dcb3e7014f9b..a02e78c45007 100644 --- a/security/security.c +++ b/security/security.c @@ -2539,6 +2539,10 @@ int security_inode_copy_up_xattr(const char *name) return rc; } + rc = evm_inode_copy_up_xattr(name); + if (rc != LSM_RET_DEFAULT(inode_copy_up_xattr)) + return rc; + return LSM_RET_DEFAULT(inode_copy_up_xattr); } EXPORT_SYMBOL(security_inode_copy_up_xattr); -- 2.39.3 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] evm: don't copy up 'security.evm' xattr 2023-12-19 13:49 ` [PATCH 1/2] evm: don't copy up 'security.evm' xattr Mimi Zohar @ 2023-12-19 14:41 ` Amir Goldstein 0 siblings, 0 replies; 5+ messages in thread From: Amir Goldstein @ 2023-12-19 14:41 UTC (permalink / raw) To: Mimi Zohar Cc: linux-unionfs, linux-integrity, linux-kernel, Christian Brauner, Seth Forshee, Roberto Sassu On Tue, Dec 19, 2023 at 3:49 PM Mimi Zohar <zohar@linux.ibm.com> wrote: > > The security.evm HMAC and the original file signatures contain > filesystem specific data. As a result, the HMAC and signature > are not the same on the stacked and backing filesystems. > > Don't copy up 'security.evm'. > > Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> > --- > include/linux/evm.h | 6 ++++++ > security/integrity/evm/evm_main.c | 7 +++++++ > security/security.c | 4 ++++ > 3 files changed, 17 insertions(+) > > diff --git a/include/linux/evm.h b/include/linux/evm.h > index 01fc495a83e2..36ec884320d9 100644 > --- a/include/linux/evm.h > +++ b/include/linux/evm.h > @@ -31,6 +31,7 @@ extern void evm_inode_post_setxattr(struct dentry *dentry, > const char *xattr_name, > const void *xattr_value, > size_t xattr_value_len); > +extern int evm_inode_copy_up_xattr(const char *name); > extern int evm_inode_removexattr(struct mnt_idmap *idmap, > struct dentry *dentry, const char *xattr_name); > extern void evm_inode_post_removexattr(struct dentry *dentry, > @@ -117,6 +118,11 @@ static inline void evm_inode_post_setxattr(struct dentry *dentry, > return; > } > > +static inline int evm_inode_copy_up_xattr(const char *name) > +{ > + return 0; > +} > + > static inline int evm_inode_removexattr(struct mnt_idmap *idmap, > struct dentry *dentry, > const char *xattr_name) > diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c > index 894570fe39bc..02adba635b02 100644 > --- a/security/integrity/evm/evm_main.c > +++ b/security/integrity/evm/evm_main.c > @@ -863,6 +863,13 @@ void evm_inode_post_setattr(struct dentry *dentry, int ia_valid) > evm_update_evmxattr(dentry, NULL, NULL, 0); > } > > +int evm_inode_copy_up_xattr(const char *name) > +{ > + if (strcmp(name, XATTR_NAME_EVM) == 0) > + return 1; /* Discard */ > + return -EOPNOTSUPP; > +} > + > /* > * evm_inode_init_security - initializes security.evm HMAC value > */ > diff --git a/security/security.c b/security/security.c > index dcb3e7014f9b..a02e78c45007 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -2539,6 +2539,10 @@ int security_inode_copy_up_xattr(const char *name) > return rc; > } > > + rc = evm_inode_copy_up_xattr(name); > + if (rc != LSM_RET_DEFAULT(inode_copy_up_xattr)) > + return rc; > + > return LSM_RET_DEFAULT(inode_copy_up_xattr); The rest of the hooks call evm last, e.g.: return evm_inode_setattr(idmap, dentry, attr); return evm_inode_remove_acl(idmap, dentry, acl_name); evm_inode_post_setxattr(dentry, name, value, size); return evm_inode_removexattr(idmap, dentry, name); best keep a consistent LSM order. Other than that, feel free to add: Reviewed-by: Amir Goldstein <amir73il@gmail.com> Thanks, Amir. ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] evm: add support to disable EVM on unsupported filesystems 2023-12-19 13:48 [PATCH 0/2] evm: disable EVM on overlayfs Mimi Zohar 2023-12-19 13:49 ` [PATCH 1/2] evm: don't copy up 'security.evm' xattr Mimi Zohar @ 2023-12-19 13:49 ` Mimi Zohar 2023-12-19 14:47 ` Amir Goldstein 1 sibling, 1 reply; 5+ messages in thread From: Mimi Zohar @ 2023-12-19 13:49 UTC (permalink / raw) To: linux-unionfs Cc: Mimi Zohar, linux-integrity, linux-kernel, Amir Goldstein, Christian Brauner, Seth Forshee, Roberto Sassu Don't verify, write, remove or update 'security.evm' on unsupported filesystems. Temporarily define overlayfs as an unsupported filesystem until a complete solution is developed. Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> --- security/integrity/evm/evm_main.c | 35 ++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index 02adba635b02..aa6d32a07d20 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -151,6 +151,17 @@ static int evm_find_protected_xattrs(struct dentry *dentry) return count; } +static int is_unsupported_fs(struct dentry *dentry) +{ + struct inode *inode = d_backing_inode(dentry); + + if (strcmp(inode->i_sb->s_type->name, "overlay") == 0) { + pr_info_once("overlayfs not supported\n"); + return 1; + } + return 0; +} + /* * evm_verify_hmac - calculate and compare the HMAC with the EVM xattr * @@ -181,6 +192,9 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry, iint->evm_status == INTEGRITY_PASS_IMMUTABLE)) return iint->evm_status; + if (is_unsupported_fs(dentry)) + return INTEGRITY_UNKNOWN; + /* if status is not PASS, try to check again - against -ENOMEM */ /* first need to know the sig type */ @@ -408,6 +422,9 @@ enum integrity_status evm_verifyxattr(struct dentry *dentry, if (!evm_key_loaded() || !evm_protected_xattr(xattr_name)) return INTEGRITY_UNKNOWN; + if (is_unsupported_fs(dentry)) + return INTEGRITY_UNKNOWN; + if (!iint) { iint = integrity_iint_find(d_backing_inode(dentry)); if (!iint) @@ -491,15 +508,21 @@ static int evm_protect_xattr(struct mnt_idmap *idmap, if (strcmp(xattr_name, XATTR_NAME_EVM) == 0) { if (!capable(CAP_SYS_ADMIN)) return -EPERM; + if (is_unsupported_fs(dentry)) + return -EPERM; } else if (!evm_protected_xattr(xattr_name)) { if (!posix_xattr_acl(xattr_name)) return 0; + if (is_unsupported_fs(dentry)) + return 0; + evm_status = evm_verify_current_integrity(dentry); if ((evm_status == INTEGRITY_PASS) || (evm_status == INTEGRITY_NOXATTRS)) return 0; goto out; - } + } else if (is_unsupported_fs(dentry)) + return 0; evm_status = evm_verify_current_integrity(dentry); if (evm_status == INTEGRITY_NOXATTRS) { @@ -750,6 +773,9 @@ void evm_inode_post_setxattr(struct dentry *dentry, const char *xattr_name, if (!(evm_initialized & EVM_INIT_HMAC)) return; + if (is_unsupported_fs(dentry)) + return; + evm_update_evmxattr(dentry, xattr_name, xattr_value, xattr_value_len); } @@ -814,8 +840,12 @@ int evm_inode_setattr(struct mnt_idmap *idmap, struct dentry *dentry, if (evm_initialized & EVM_ALLOW_METADATA_WRITES) return 0; + if (is_unsupported_fs(dentry)) + return 0; + if (!(ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID))) return 0; + evm_status = evm_verify_current_integrity(dentry); /* * Writing attrs is safe for portable signatures, as portable signatures @@ -859,6 +889,9 @@ void evm_inode_post_setattr(struct dentry *dentry, int ia_valid) if (!(evm_initialized & EVM_INIT_HMAC)) return; + if (is_unsupported_fs(dentry)) + return; + if (ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID)) evm_update_evmxattr(dentry, NULL, NULL, 0); } -- 2.39.3 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] evm: add support to disable EVM on unsupported filesystems 2023-12-19 13:49 ` [PATCH 2/2] evm: add support to disable EVM on unsupported filesystems Mimi Zohar @ 2023-12-19 14:47 ` Amir Goldstein 0 siblings, 0 replies; 5+ messages in thread From: Amir Goldstein @ 2023-12-19 14:47 UTC (permalink / raw) To: Mimi Zohar Cc: linux-unionfs, linux-integrity, linux-kernel, Christian Brauner, Seth Forshee, Roberto Sassu On Tue, Dec 19, 2023 at 3:49 PM Mimi Zohar <zohar@linux.ibm.com> wrote: > > Don't verify, write, remove or update 'security.evm' on unsupported > filesystems. > > Temporarily define overlayfs as an unsupported filesystem until > a complete solution is developed. > > Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> > --- > security/integrity/evm/evm_main.c | 35 ++++++++++++++++++++++++++++++- > 1 file changed, 34 insertions(+), 1 deletion(-) > > diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c > index 02adba635b02..aa6d32a07d20 100644 > --- a/security/integrity/evm/evm_main.c > +++ b/security/integrity/evm/evm_main.c > @@ -151,6 +151,17 @@ static int evm_find_protected_xattrs(struct dentry *dentry) > return count; > } > > +static int is_unsupported_fs(struct dentry *dentry) > +{ > + struct inode *inode = d_backing_inode(dentry); > + > + if (strcmp(inode->i_sb->s_type->name, "overlay") == 0) { > + pr_info_once("overlayfs not supported\n"); > + return 1; > + } Please do not special case overlayfs in and please do not use the fs name to detect support. Please define an sb flag like SB_I_IMA_UNVERIFIABLE_SIGNATURE to disable EVM and set this flag in ovl_fill_super(). Thanks, Amir. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-12-19 14:47 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-12-19 13:48 [PATCH 0/2] evm: disable EVM on overlayfs Mimi Zohar 2023-12-19 13:49 ` [PATCH 1/2] evm: don't copy up 'security.evm' xattr Mimi Zohar 2023-12-19 14:41 ` Amir Goldstein 2023-12-19 13:49 ` [PATCH 2/2] evm: add support to disable EVM on unsupported filesystems Mimi Zohar 2023-12-19 14:47 ` Amir Goldstein
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox