| Message ID | 348160a7b7d93d976654c3d74dfe4b4aa5d3c6d1.1526923746.git.joseph.salisbury@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | Revert "ima: limit file hash setting by user to fix and log modes" | expand |
On 23.05.2018 11:00, Joseph Salisbury wrote: > From: Mimi Zohar <zohar@linux.vnet.ibm.com> > > BugLink: http://bugs.launchpad.net/bugs/1771826 > > Userspace applications have been modified to write security xattrs, > but they are not context aware. In the case of security.ima, the > security xattr can be either a file hash or a file signature. > Permitting writing one, but not the other requires the application to > be context aware. > > In addition, userspace applications might write files to a staging > area, which might not be in policy, and then change some file metadata > (eg. owner) making it in policy. As a result, these files are not > labeled properly. > > This reverts commit c68ed80c97d9720f51ef31fe91560fdd1e121533, which > prevents writing file hashes as security.ima xattrs. > > Requested-by: Patrick Ohly <patrick.ohly@intel.com> > Cc: Dmitry Kasatkin <dmitry.kasatkin@gmail.com> > Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com> > (cherry picked from commit f5acb3dcba1ffb7f0b8cbb9dba61500eea5d610b) > Signed-off-by: Joseph Salisbury <joseph.salisbury@canonical.com> Acked-by: Stefan Bader <stefan.bader@canonical.com> > --- > security/integrity/ima/ima_appraise.c | 8 ++------ > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c > index d62ae31..80f2c6d 100644 > --- a/security/integrity/ima/ima_appraise.c > +++ b/security/integrity/ima/ima_appraise.c > @@ -384,14 +384,10 @@ int ima_inode_setxattr(struct dentry *dentry, const char *xattr_name, > result = ima_protect_xattr(dentry, xattr_name, xattr_value, > xattr_value_len); > if (result == 1) { > - bool digsig; > - > if (!xattr_value_len || (xvalue->type >= IMA_XATTR_LAST)) > return -EINVAL; > - digsig = (xvalue->type == EVM_IMA_XATTR_DIGSIG); > - if (!digsig && (ima_appraise & IMA_APPRAISE_ENFORCE)) > - return -EPERM; > - ima_reset_appraise_flags(d_backing_inode(dentry), digsig); > + ima_reset_appraise_flags(d_backing_inode(dentry), > + (xvalue->type == EVM_IMA_XATTR_DIGSIG) ? 1 : 0); > result = 0; > } > return result; >
On 05/23/2018 08:00 PM, Joseph Salisbury wrote: > From: Mimi Zohar <zohar@linux.vnet.ibm.com> > > BugLink: http://bugs.launchpad.net/bugs/1771826 > > Userspace applications have been modified to write security xattrs, > but they are not context aware. In the case of security.ima, the > security xattr can be either a file hash or a file signature. > Permitting writing one, but not the other requires the application to > be context aware. > > In addition, userspace applications might write files to a staging > area, which might not be in policy, and then change some file metadata > (eg. owner) making it in policy. As a result, these files are not > labeled properly. > > This reverts commit c68ed80c97d9720f51ef31fe91560fdd1e121533, which > prevents writing file hashes as security.ima xattrs. > > Requested-by: Patrick Ohly <patrick.ohly@intel.com> > Cc: Dmitry Kasatkin <dmitry.kasatkin@gmail.com> > Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com> > (cherry picked from commit f5acb3dcba1ffb7f0b8cbb9dba61500eea5d610b) > Signed-off-by: Joseph Salisbury <joseph.salisbury@canonical.com> Acked-by: Juerg Haefliger <juergh@canonical.com> Applied to xenial/master-next. ...Juerg > --- > security/integrity/ima/ima_appraise.c | 8 ++------ > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c > index d62ae31..80f2c6d 100644 > --- a/security/integrity/ima/ima_appraise.c > +++ b/security/integrity/ima/ima_appraise.c > @@ -384,14 +384,10 @@ int ima_inode_setxattr(struct dentry *dentry, const char *xattr_name, > result = ima_protect_xattr(dentry, xattr_name, xattr_value, > xattr_value_len); > if (result == 1) { > - bool digsig; > - > if (!xattr_value_len || (xvalue->type >= IMA_XATTR_LAST)) > return -EINVAL; > - digsig = (xvalue->type == EVM_IMA_XATTR_DIGSIG); > - if (!digsig && (ima_appraise & IMA_APPRAISE_ENFORCE)) > - return -EPERM; > - ima_reset_appraise_flags(d_backing_inode(dentry), digsig); > + ima_reset_appraise_flags(d_backing_inode(dentry), > + (xvalue->type == EVM_IMA_XATTR_DIGSIG) ? 1 : 0); > result = 0; > } > return result; >
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index d62ae31..80f2c6d 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -384,14 +384,10 @@ int ima_inode_setxattr(struct dentry *dentry, const char *xattr_name, result = ima_protect_xattr(dentry, xattr_name, xattr_value, xattr_value_len); if (result == 1) { - bool digsig; - if (!xattr_value_len || (xvalue->type >= IMA_XATTR_LAST)) return -EINVAL; - digsig = (xvalue->type == EVM_IMA_XATTR_DIGSIG); - if (!digsig && (ima_appraise & IMA_APPRAISE_ENFORCE)) - return -EPERM; - ima_reset_appraise_flags(d_backing_inode(dentry), digsig); + ima_reset_appraise_flags(d_backing_inode(dentry), + (xvalue->type == EVM_IMA_XATTR_DIGSIG) ? 1 : 0); result = 0; } return result;