This patch set includes various fixes for IMA and EVM.
Patches 1-3 are trivial fixes. The remaining improve support and usability
of EVM portable signatures. In particular patch 4 allows EVM to be used
without an HMAC key. Patch 5 avoids appraisal verification of public keys
(they are already verified by the key subsystem).
Patches 6-7 allow metadata verification to be turned off when the HMAC key
is not already loaded and to use this mode in a safe way (by ensuring that
IMA revalidates metadata when there is a change).
Patches 8-9 make portable signatures more usable if metadata verification
cannot be turned off (because the HMAC key is loaded) by accepting any
metadata modification until signature verification succeeds (useful when
xattrs/attrs are copied in a sequence from a source) and by allowing
operations that don't change metadata.
Patch 10 makes it possible to use portable signatures when the IMA policy
requires file signatures and patch 11 shows portable signatures when the
ima-sig measurement list template is selected.
Lastly, patch 12 avoids undesired removal of security.ima when a file is
not selected by the IMA policy.
Roberto Sassu (12):
ima: Don't ignore errors from crypto_shash_update()
ima: Remove semicolon at the end of ima_get_binary_runtime_size()
evm: Check size of security.evm before using it
evm: Execute evm_inode_init_security() only when the HMAC key is
loaded
evm: Load EVM key in ima_load_x509() to avoid appraisal
evm: Refuse EVM_ALLOW_METADATA_WRITES only if the HMAC key is loaded
evm: Introduce EVM_RESET_STATUS atomic flag
evm: Allow xattr/attr operations for portable signatures if check
fails
evm: Allow setxattr() and setattr() if metadata digest won't change
ima: Allow imasig requirement to be satisfied by EVM portable
signatures
ima: Introduce template field evmsig and write to field sig as
fallback
ima: Don't remove security.ima if file must not be appraised
Documentation/ABI/testing/evm | 6 +-
Documentation/security/IMA-templates.rst | 4 +-
include/linux/integrity.h | 1 +
security/integrity/evm/evm_main.c | 151 ++++++++++++++++++++--
security/integrity/evm/evm_secfs.c | 2 +-
security/integrity/iint.c | 2 +
security/integrity/ima/ima_appraise.c | 26 ++--
security/integrity/ima/ima_crypto.c | 2 +
security/integrity/ima/ima_init.c | 4 +
security/integrity/ima/ima_main.c | 8 +-
security/integrity/ima/ima_queue.c | 2 +-
security/integrity/ima/ima_template.c | 2 +
security/integrity/ima/ima_template_lib.c | 39 +++++-
security/integrity/ima/ima_template_lib.h | 2 +
security/integrity/integrity.h | 1 +
15 files changed, 225 insertions(+), 27 deletions(-)
--
2.27.GIT
Errors returned by crypto_shash_update() are not checked in
ima_calc_boot_aggregate_tfm() and thus can be overwritten at the next
iteration of the loop. This patch adds a check after calling
crypto_shash_update() and returns immediately if the result is not zero.
Cc: stable@vger.kernel.org
Fixes: 3323eec921efd ("integrity: IMA as an integrity service provider")
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
---
security/integrity/ima/ima_crypto.c | 2 ++
1 file changed, 2 insertions(+)
@@ -829,6 +829,8 @@ static int ima_calc_boot_aggregate_tfm(char *digest, u16 alg_id,/* now accumulate with current aggregate */rc=crypto_shash_update(shash,d.digest,crypto_shash_digestsize(tfm));+if(rc!=0)+returnrc;}/**ExtendcumulativedigestoverTPMregisters8-9,whichcontain
evm_inode_init_security() requires the HMAC key to calculate the HMAC on
initial xattrs provided by LSMs. Unfortunately, with the evm_key_loaded()
check, the function continues even if the HMAC key is not loaded
(evm_key_loaded() returns true also if EVM has been initialized only with a
public key). If the HMAC key is not loaded, evm_inode_init_security()
returns an error later when it calls evm_init_hmac().
Thus, this patch replaces the evm_key_loaded() check with a check of the
EVM_INIT_HMAC flag in evm_initialized, so that evm_inode_init_security()
returns 0 instead of an error.
Cc: stable@vger.kernel.org # 4.5.x
Fixes: 26ddabfe96b ("evm: enable EVM when X509 certificate is loaded")
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
---
security/integrity/evm/evm_main.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
This patch checks the size for the EVM_IMA_XATTR_DIGSIG and
EVM_XATTR_PORTABLE_DIGSIG types to ensure that the algorithm is read from
the buffer returned by vfs_getxattr_alloc().
Cc: stable@vger.kernel.org # 4.19.x
Fixes: 5feeb61183dde ("evm: Allow non-SHA1 digital signatures")
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
---
security/integrity/evm/evm_main.c | 6 ++++++
1 file changed, 6 insertions(+)
EVM_ALLOW_METADATA_WRITES is an EVM initialization flag that can be set to
temporarily disable metadata verification until all xattrs/attrs necessary
to verify an EVM portable signature are copied to the file. This flag is
cleared when EVM is initialized with an HMAC key, to avoid that the HMAC is
calculated on unverified xattrs/attrs.
Currently EVM unnecessarily denies setting this flag if EVM is initialized
with public key, which is not a concern as it cannot be used to trust
xattrs/attrs updates. This patch removes this limitation.
Cc: stable@vger.kernel.org # 4.16.x
Fixes: ae1ba1676b88e ("EVM: Allow userland to permit modification of EVM-protected metadata")
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
Documentation/ABI/testing/evm | 6 ++++--
security/integrity/evm/evm_secfs.c | 2 +-
2 files changed, 5 insertions(+), 3 deletions(-)
@@ -42,8 +42,10 @@ Description: modification of EVM-protected metadata and disable all further modification of policy- Note that once a key has been loaded, it will no longer be- possible to enable metadata modification.+ Note that once HMAC validation and creation is enabled,+ it will no longer be possible to enable metadata modification+ and if metadata modification is already enabled, it will be+ disabled. Until key loading has been signaled EVM can not create or validate the 'security.evm' xattr, but returns
With the patch to allow xattr/attr operations if a portable signature
verification fails, cp and tar can copy all xattrs/attrs so that at the
end of the process verification succeeds.
However, it might happen that xattrs/attrs are already set to the correct
value (taken at signing time) and signature verification succeeds before
the copy is completed. For example, an archive might contains files owned
by root and the archive is extracted by root.
Then, since portable signatures are immutable, all subsequent operations
fail (e.g. fchown()), even if the operation is legitimate (does not alter
the current value).
This patch avoids this problem by reporting successful operation to user
space when that operation does not alter the current value of xattrs/attrs.
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
security/integrity/evm/evm_main.c | 94 +++++++++++++++++++++++++++++++
1 file changed, 94 insertions(+)
@@ -314,6 +315,78 @@ static enum integrity_status evm_verify_current_integrity(struct dentry *dentry)returnevm_verify_hmac(dentry,NULL,NULL,0,NULL);}+/*+*evm_xattr_acl_change-checkifpassedACLchangestheinodemode+*@dentry:pointertotheaffecteddentry+*@xattr_name:requestedxattr+*@xattr_value:requestedxattrvalue+*@xattr_value_len:requestedxattrvaluelength+*+*CheckifpassedACLchangestheinodemode,whichisprotectedbyEVM.+*+*Returns1ifpassedACLcausesinodemodechange,0otherwise.+*/+staticintevm_xattr_acl_change(structdentry*dentry,constchar*xattr_name,+constvoid*xattr_value,size_txattr_value_len)+{+umode_tmode;+structposix_acl*acl=NULL,*acl_res;+structinode*inode=d_backing_inode(dentry);+intrc;++/* UID/GID in ACL have been already converted from user to init ns */+acl=posix_acl_from_xattr(&init_user_ns,xattr_value,xattr_value_len);+if(!acl)+return1;++acl_res=acl;+rc=posix_acl_update_mode(inode,&mode,&acl_res);++posix_acl_release(acl);++if(rc)+return1;++if(acl_res&&inode->i_mode!=mode)+return1;++return0;+}++/*+*evm_xattr_change-checkifpassedxattrvaluediffersfromcurrentvalue+*@dentry:pointertotheaffecteddentry+*@xattr_name:requestedxattr+*@xattr_value:requestedxattrvalue+*@xattr_value_len:requestedxattrvaluelength+*+*Checkifpassedxattrvaluediffersfromcurrentvalue.+*+*Returns1ifpassedxattrvaluediffersfromcurrentvalue,0otherwise.+*/+staticintevm_xattr_change(structdentry*dentry,constchar*xattr_name,+constvoid*xattr_value,size_txattr_value_len)+{+char*xattr_data=NULL;+intrc=0;++if(posix_xattr_acl(xattr_name))+returnevm_xattr_acl_change(dentry,xattr_name,xattr_value,+xattr_value_len);++rc=vfs_getxattr_alloc(dentry,xattr_name,&xattr_data,0,GFP_NOFS);+if(rc<0)+return1;++if(rc==xattr_value_len)+rc=memcmp(xattr_value,xattr_data,rc);+else+rc=1;++kfree(xattr_data);+returnrc;+}+/**evm_protect_xattr-protecttheEVMextendedattribute*
When EVM_ALLOW_METADATA_WRITES is set, EVM allows any operation on
metadata. Its main purpose is to allow users to freely set metadata when
they are protected by a portable signature, until the HMAC key is loaded.
However, IMA is not notified about metadata changes and, after the first
successful appraisal, always allows access to the files without checking
metadata again.
This patch introduces the new atomic flag EVM_RESET_STATUS in
integrity_iint_cache that is set in the EVM post hooks and cleared in
evm_verify_hmac(). IMA checks the new flag in process_measurement() and if
it is set, it clears the appraisal flags.
Although the flag could be cleared also by evm_inode_setxattr() and
evm_inode_setattr() before IMA sees it, this does not happen if
EVM_ALLOW_METADATA_WRITES is set. Since the only remaining caller is
evm_verifyxattr(), this ensures that IMA always sees the flag set before it
is cleared.
This patch also adds a call to evm_reset_status() in
evm_inode_post_setattr() so that EVM won't return the cached status the
next time appraisal is performed.
Cc: stable@vger.kernel.org # 4.16.x
Fixes: ae1ba1676b88e ("EVM: Allow userland to permit modification of EVM-protected metadata")
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
security/integrity/evm/evm_main.c | 17 +++++++++++++++--
security/integrity/ima/ima_main.c | 8 ++++++--
security/integrity/integrity.h | 1 +
3 files changed, 22 insertions(+), 4 deletions(-)
If files with portable signatures are copied from one location to another
or are extracted from an archive, verification can temporarily fail until
all xattrs/attrs are set in the destination. Only portable signatures may
be moved or copied from one file to another, as they don't depend on
system-specific information such as the inode generation. Instead portable
signatures must include security.ima.
Unlike other security.evm types, EVM portable signatures are also
immutable. Thus, it wouldn't be a problem to allow xattr/attr operations
when verification fails, as portable signatures will never be replaced with
an HMAC on possibly corrupted xattrs/attrs.
This patch first introduces a new integrity status called
INTEGRITY_FAIL_IMMUTABLE, that allows callers of
evm_verify_current_integrity() to detect that a portable signature didn't
pass verification and then adds an exception in evm_protect_xattr() and
evm_inode_setattr() for this status and returns 0 instead of -EPERM.
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
---
include/linux/integrity.h | 1 +
security/integrity/evm/evm_main.c | 31 +++++++++++++++++++++------
security/integrity/ima/ima_appraise.c | 2 ++
3 files changed, 27 insertions(+), 7 deletions(-)
@@ -358,6 +364,12 @@ static int evm_protect_xattr(struct dentry *dentry, const char *xattr_name,-EPERM,0);}out:+/* Writing other xattrs is safe for portable signatures, as portable+*signaturesareimmutableandcanneverbeupdated.+*/+if(evm_status==INTEGRITY_FAIL_IMMUTABLE)+return0;+if(evm_status!=INTEGRITY_PASS)integrity_audit_msg(AUDIT_INTEGRITY_METADATA,d_backing_inode(dentry),dentry->d_name.name,"appraise_metadata",
@@ -499,9 +511,14 @@ int evm_inode_setattr(struct dentry *dentry, struct iattr *attr)if(!(ia_valid&(ATTR_MODE|ATTR_UID|ATTR_GID)))return0;evm_status=evm_verify_current_integrity(dentry);+/* Writing attrs is safe for portable signatures, as portable signatures+*areimmutableandcanneverbeupdated.+*/if((evm_status==INTEGRITY_PASS)||-(evm_status==INTEGRITY_NOXATTRS))+(evm_status==INTEGRITY_NOXATTRS)||+(evm_status==INTEGRITY_FAIL_IMMUTABLE))return0;+integrity_audit_msg(AUDIT_INTEGRITY_METADATA,d_backing_inode(dentry),dentry->d_name.name,"appraise_metadata",integrity_status_msg[evm_status],-EPERM,0);
Public keys do not need to be appraised by IMA as the restriction on the
IMA/EVM keyrings ensures that a key is loaded only if it is signed with a
key in the primary or secondary keyring.
However, when evm_load_x509() is loaded, appraisal is already enabled and
a valid IMA signature must be added to the EVM key to pass verification.
Since the restriction is applied on both IMA and EVM keyrings, it is safe
to disable appraisal also when the EVM key is loaded. This patch calls
evm_load_x509() inside ima_load_x509() if CONFIG_IMA_LOAD_X509 is defined.
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
---
security/integrity/iint.c | 2 ++
security/integrity/ima/ima_init.c | 4 ++++
2 files changed, 6 insertions(+)
With the patch to accept EVM portable signatures when the
appraise_type=imasig requirement is specified in the policy, appraisal can
be successfully done even if the file does not have an IMA signature.
However, remote attestation would not see that a different signature type
was used, as only IMA signatures can be included in the measurement list.
This patch solves the issue by introducing the new template field 'evmsig'
to show EVM portable signatures and by including its value in the existing
field 'sig' if the IMA signature is not found.
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
---
Documentation/security/IMA-templates.rst | 4 ++-
security/integrity/ima/ima_template.c | 2 ++
security/integrity/ima/ima_template_lib.c | 39 ++++++++++++++++++++++-
security/integrity/ima/ima_template_lib.h | 2 ++
4 files changed, 45 insertions(+), 2 deletions(-)
@@ -70,9 +70,11 @@ descriptors by adding their identifier to the format string prefix is shown only if the hash algorithm is not SHA1 or MD5);- 'd-modsig': the digest of the event without the appended modsig;- 'n-ng': the name of the event, without size limitations;-- 'sig': the file signature;+- 'sig': the file signature, or the EVM portable signature if the file+ signature is not found;- 'modsig' the appended file signature;- 'buf': the buffer data that was used to generate the hash without size limitations;+- 'evmsig': the EVM portable signature; Below, there is the list of defined template descriptors:
Files might come from a remote source and might have xattrs, including
security.ima. It should not be IMA task to decide whether security.ima
should be kept or not. This patch removes the removexattr() system
call in ima_inode_post_setattr().
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
---
security/integrity/ima/ima_appraise.c | 2 --
1 file changed, 2 deletions(-)
System administrators can require that all accessed files have a signature
by specifying appraise_type=imasig in a policy rule.
Currently, IMA signatures satisfy this requirement. Appended signatures may
also satisfy this requirement, but are not applicable as IMA signatures.
IMA/appended signatures ensure data source authentication for file content
and prevent any change. EVM signatures instead ensure data source
authentication for file metadata. Given that the digest or signature of the
file content must be included in the metadata, EVM signatures provide the
same file data guarantees of IMA signatures, as well as providing file
metadata guarantees.
This patch lets systems protected with EVM signatures pass appraisal
verification if the appraise_type=imasig requirement is specified in the
policy. This facilitates deployment in the scenarios where only EVM
signatures are available.
The patch makes the following changes:
file xattr types:
security.ima: IMA_XATTR_DIGEST/IMA_XATTR_DIGEST_NG
security.evm: EVM_XATTR_PORTABLE_DIGSIG
execve(), mmap(), open() behavior (with appraise_type=imasig):
before: denied (file without IMA signature, imasig requirement not met)
after: allowed (file with EVM portable signature, imasig requirement met)
open(O_WRONLY) behavior (without appraise_type=imasig):
before: allowed (file without IMA signature, not immutable)
after: denied (file with EVM portable signature, immutable)
In addition, similarly to IMA signatures, this patch temporarily allows
new files without or with incomplete metadata to be opened so that content
can be written.
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
---
security/integrity/ima/ima_appraise.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
@@ -444,9 +449,12 @@ int ima_appraise_measurement(enum ima_hooks func,status=INTEGRITY_PASS;}-/* Permit new files with file signatures, but without data. */+/*+*Permitnewfileswithfile/EVMportablesignatures,but+*withoutdata.+*/if(inode->i_size==0&&iint->flags&IMA_NEW_FILE&&-xattr_value&&xattr_value->type==EVM_IMA_XATTR_DIGSIG){+test_bit(IMA_DIGSIG,&iint->atomic_flags)){status=INTEGRITY_PASS;}
Hi Roberto,
On Fri, 2020-09-04 at 11:23 +0200, Roberto Sassu wrote:
evm_inode_init_security() requires the HMAC key to calculate the HMAC on
initial xattrs provided by LSMs. Unfortunately, with the evm_key_loaded()
check, the function continues even if the HMAC key is not loaded
(evm_key_loaded() returns true also if EVM has been initialized only with a
public key). If the HMAC key is not loaded, evm_inode_init_security()
returns an error later when it calls evm_init_hmac().
This is all true, but the context for why it wasn't an issue previously
is missing.
The original usecase for allowing signature verificaton prior to
loading the HMAC key was a fully signed, possibly immutable, initrd.
No new files were created or, at least, were in policy until the HMAC
key was loaded. More recently support for requiring an EVM HMAC key
was removed. Files having a portable and immutable signature were
given additional privileges.
Please update the patch description with the context of what has
changed.
Mimi
Hi Roberto,
On Fri, 2020-09-04 at 11:23 +0200, Roberto Sassu wrote:
This patch set includes various fixes for IMA and EVM.
Patches 1-3 are trivial fixes.
I've queued these patches in the next-integrity-testing branch for now.
When reposting this patch set please replace the cover letter subject
line with a more appropriate one.
The remaining improve support and usability
of EVM portable signatures. In particular patch 4 allows EVM to be used
without an HMAC key.
EVM already supports using EVM portable and immutable sigatures without
an HMAC key.
I was able to apply this patch set, but patch 10/12 does not apply
cleanly due to the "fallthrough" line. Please hold off on reposting,
until I've finished reviewing the entire patch set.
thanks,
Mimi
Hi Roberto,
"if check fails" in the Subject line is unnecessary.
On Fri, 2020-09-04 at 11:26 +0200, Roberto Sassu wrote:
If files with portable signatures are copied from one location to another
or are extracted from an archive, verification can temporarily fail until
all xattrs/attrs are set in the destination. Only portable signatures may
be moved or copied from one file to another, as they don't depend on
system-specific information such as the inode generation. Instead portable
signatures must include security.ima.
Unlike other security.evm types, EVM portable signatures are also
immutable. Thus, it wouldn't be a problem to allow xattr/attr operations
when verification fails, as portable signatures will never be replaced with
an HMAC on possibly corrupted xattrs/attrs.
This patch first introduces a new integrity status called
INTEGRITY_FAIL_IMMUTABLE, that allows callers of
evm_verify_current_integrity() to detect that a portable signature didn't
pass verification and then adds an exception in evm_protect_xattr() and
evm_inode_setattr() for this status and returns 0 instead of -EPERM.
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
@@ -358,6 +364,12 @@ static int evm_protect_xattr(struct dentry *dentry, const char *xattr_name,-EPERM,0);}out:+/* Writing other xattrs is safe for portable signatures, as portable+*signaturesareimmutableandcanneverbeupdated.+*/
This is the second time I'm seeing this comment format style. Why?
What changed?
Mimi
+ if (evm_status == INTEGRITY_FAIL_IMMUTABLE)
+ return 0;
+
if (evm_status != INTEGRITY_PASS)
integrity_audit_msg(AUDIT_INTEGRITY_METADATA, d_backing_inode(dentry),
dentry->d_name.name, "appraise_metadata",
[Cc'ing John Johansen]
Hi Roberto,
On Fri, 2020-09-04 at 11:26 +0200, Roberto Sassu wrote:
quoted hunk
When EVM_ALLOW_METADATA_WRITES is set, EVM allows any operation on
metadata. Its main purpose is to allow users to freely set metadata when
they are protected by a portable signature, until the HMAC key is loaded.
However, IMA is not notified about metadata changes and, after the first
successful appraisal, always allows access to the files without checking
metadata again.
This patch introduces the new atomic flag EVM_RESET_STATUS in
integrity_iint_cache that is set in the EVM post hooks and cleared in
evm_verify_hmac(). IMA checks the new flag in process_measurement() and if
it is set, it clears the appraisal flags.
Although the flag could be cleared also by evm_inode_setxattr() and
evm_inode_setattr() before IMA sees it, this does not happen if
EVM_ALLOW_METADATA_WRITES is set. Since the only remaining caller is
evm_verifyxattr(), this ensures that IMA always sees the flag set before it
is cleared.
This patch also adds a call to evm_reset_status() in
evm_inode_post_setattr() so that EVM won't return the cached status the
next time appraisal is performed.
Cc: stable@vger.kernel.org # 4.16.x
Fixes: ae1ba1676b88e ("EVM: Allow userland to permit modification of EVM-protected metadata")
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
security/integrity/evm/evm_main.c | 17 +++++++++++++++--
security/integrity/ima/ima_main.c | 8 ++++++--
security/integrity/integrity.h | 1 +
3 files changed, 22 insertions(+), 4 deletions(-)
True IMA is currently the only caller of evm_verifyxattr() in the
upstreamed kernel, but it is an exported function, which may be called
from elsewhere. The previous version crossed the boundary between EVM
& IMA with EVM modifying the IMA flag directly. This version assumes
that IMA will be the only caller. Otherwise, I like this version.
Mimi
Hi Roberto,
"if" doesn't belong in the subject line. In this case, instead of "if
metadata ...", how about something like "for unmodified metadata"?
On Fri, 2020-09-04 at 11:26 +0200, Roberto Sassu wrote:
With the patch to allow xattr/attr operations if a portable signature
verification fails, cp and tar can copy all xattrs/attrs so that at the
end of the process verification succeeds.
However, it might happen that xattrs/attrs are already set to the correct
^ the xattrs/attrs
value (taken at signing time) and signature verification succeeds before
the copy is completed. For example, an archive might contains files owned
^ has completed.
by root and the archive is extracted by root.
Then, since portable signatures are immutable, all subsequent operations
fail (e.g. fchown()), even if the operation is legitimate (does not alter
the current value).
This patch avoids this problem by reporting successful operation to user
space when that operation does not alter the current value of xattrs/attrs.
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
<snip>
+/*
+ * evm_xattr_change - check if passed xattr value differs from current value
+ * @dentry: pointer to the affected dentry
+ * @xattr_name: requested xattr
+ * @xattr_value: requested xattr value
+ * @xattr_value_len: requested xattr value length
+ *
+ * Check if passed xattr value differs from current value.
+ *
+ * Returns 1 if passed xattr value differs from current value, 0 otherwise.
+ */
+static int evm_xattr_change(struct dentry *dentry, const char *xattr_name,
+ const void *xattr_value, size_t xattr_value_len)
+{
+ char *xattr_data = NULL;
+ int rc = 0;
+
+ if (posix_xattr_acl(xattr_name))
+ return evm_xattr_acl_change(dentry, xattr_name, xattr_value,
+ xattr_value_len);
+
+ rc = vfs_getxattr_alloc(dentry, xattr_name, &xattr_data, 0, GFP_NOFS);
+ if (rc < 0)
+ return 1;
+
+ if (rc == xattr_value_len)
+ rc = memcmp(xattr_value, xattr_data, rc);
This should probably be changed to crypto_memneq(). Refer to commit
613317bd212c ("EVM: Use crypto_memneq() for digest comparisons").
thanks,
Mimi
Hi Roberto,
On Fri, 2020-09-04 at 11:26 +0200, Roberto Sassu wrote:
With the patch to accept EVM portable signatures when the
appraise_type=imasig requirement is specified in the policy, appraisal can
be successfully done even if the file does not have an IMA signature.
However, remote attestation would not see that a different signature type
was used, as only IMA signatures can be included in the measurement list.
This patch solves the issue by introducing the new template field 'evmsig'
to show EVM portable signatures and by including its value in the existing
field 'sig' if the IMA signature is not found.
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
From: Mimi Zohar [mailto:zohar@linux.ibm.com]
Sent: Thursday, September 17, 2020 4:25 PM
Hi Roberto,
On Fri, 2020-09-04 at 11:26 +0200, Roberto Sassu wrote:
quoted
With the patch to accept EVM portable signatures when the
appraise_type=imasig requirement is specified in the policy, appraisal can
be successfully done even if the file does not have an IMA signature.
However, remote attestation would not see that a different signature
type
quoted
was used, as only IMA signatures can be included in the measurement list.
This patch solves the issue by introducing the new template field 'evmsig'
to show EVM portable signatures and by including its value in the existing
field 'sig' if the IMA signature is not found.
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
We're including the EVM signature on a best effort basis to help with
attestation. Do we really care why it failed? Are we going to act on
it?
Hi Mimi
other template field functions have a similar behavior. They return
an error if an operation necessary to retrieve the data cannot be
performed. Should I always return 0?
Thanks
Roberto
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Li Jian, Shi Yanli
Hi Roberto,
On Wed, 2020-09-16 at 12:14 -0400, Mimi Zohar wrote:
On Fri, 2020-09-04 at 11:23 +0200, Roberto Sassu wrote:
quoted
This patch set includes various fixes for IMA and EVM.
Patches 1-3 are trivial fixes.
I've queued these patches in the next-integrity-testing branch for now.
When reposting this patch set please replace the cover letter subject
line with a more appropriate one.
quoted
The remaining improve support and usability
of EVM portable signatures. In particular patch 4 allows EVM to be used
without an HMAC key.
EVM already supports using EVM portable and immutable sigatures without
an HMAC key.
I was able to apply this patch set, but patch 10/12 does not apply
cleanly due to the "fallthrough" line. Please hold off on reposting,
until I've finished reviewing the entire patch set.
Done. Other than the one issue of clearing the EVM_RESET_STATUS in
evm_verifyxattr(), the remaining changes are straight forward.
thanks,
Mimi
On Thu, 2020-09-17 at 15:05 +0000, Roberto Sassu wrote:
quoted
From: Mimi Zohar [mailto:zohar@linux.ibm.com]
Sent: Thursday, September 17, 2020 4:25 PM
Hi Roberto,
On Fri, 2020-09-04 at 11:26 +0200, Roberto Sassu wrote:
quoted
With the patch to accept EVM portable signatures when the
appraise_type=imasig requirement is specified in the policy, appraisal can
be successfully done even if the file does not have an IMA signature.
However, remote attestation would not see that a different signature
type
quoted
was used, as only IMA signatures can be included in the measurement list.
This patch solves the issue by introducing the new template field 'evmsig'
to show EVM portable signatures and by including its value in the existing
field 'sig' if the IMA signature is not found.
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
We're including the EVM signature on a best effort basis to help with
attestation. Do we really care why it failed? Are we going to act on
it?
Hi Mimi
other template field functions have a similar behavior. They return
an error if an operation necessary to retrieve the data cannot be
performed. Should I always return 0?
The EVM signature case is more similar to the IMA signature case, than
to other fields. In the signature cases, if the signature exists, it
is included. My suggestion is based on the difference in how the
vfs_getxattr_alloc() results are handled.
thanks,
Mimi
From: Mimi Zohar [mailto:zohar@linux.ibm.com]
Sent: Thursday, September 17, 2020 2:01 PM
[Cc'ing John Johansen]
Hi Roberto,
On Fri, 2020-09-04 at 11:26 +0200, Roberto Sassu wrote:
quoted
When EVM_ALLOW_METADATA_WRITES is set, EVM allows any operation
on
quoted
metadata. Its main purpose is to allow users to freely set metadata when
they are protected by a portable signature, until the HMAC key is loaded.
However, IMA is not notified about metadata changes and, after the first
successful appraisal, always allows access to the files without checking
metadata again.
This patch introduces the new atomic flag EVM_RESET_STATUS in
integrity_iint_cache that is set in the EVM post hooks and cleared in
evm_verify_hmac(). IMA checks the new flag in process_measurement()
and if
quoted
it is set, it clears the appraisal flags.
Although the flag could be cleared also by evm_inode_setxattr() and
evm_inode_setattr() before IMA sees it, this does not happen if
EVM_ALLOW_METADATA_WRITES is set. Since the only remaining caller is
evm_verifyxattr(), this ensures that IMA always sees the flag set before it
is cleared.
This patch also adds a call to evm_reset_status() in
evm_inode_post_setattr() so that EVM won't return the cached status
the
quoted
next time appraisal is performed.
Cc: stable@vger.kernel.org # 4.16.x
Fixes: ae1ba1676b88e ("EVM: Allow userland to permit modification of
True IMA is currently the only caller of evm_verifyxattr() in the
upstreamed kernel, but it is an exported function, which may be called
from elsewhere. The previous version crossed the boundary between EVM
& IMA with EVM modifying the IMA flag directly. This version assumes
that IMA will be the only caller. Otherwise, I like this version.
Ok, I think it is better, as you suggested, to export a new EVM function
that tells if evm_reset_status() will be executed in the EVM post hooks, and
to call this function from IMA. IMA would then call ima_reset_appraise_flags()
also depending on the result of the new EVM function.
ima_reset_appraise_flags() should be called in a post hook in IMA.
Should I introduce it?
Thanks
Roberto
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Li Jian, Shi Yanli
True IMA is currently the only caller of evm_verifyxattr() in the
upstreamed kernel, but it is an exported function, which may be called
from elsewhere. The previous version crossed the boundary between EVM
& IMA with EVM modifying the IMA flag directly. This version assumes
that IMA will be the only caller. Otherwise, I like this version.
Ok, I think it is better, as you suggested, to export a new EVM function
that tells if evm_reset_status() will be executed in the EVM post hooks, and
to call this function from IMA. IMA would then call ima_reset_appraise_flags()
also depending on the result of the new EVM function.
ima_reset_appraise_flags() should be called in a post hook in IMA.
Should I introduce it?
Yes, so any callers of evm_verifyxattr() will need to implement the
post hook as well. As much as possible, please limit code duplication.
The last time I looked, there didn't seem to be a locking concern, but
please make sure.
thanks,
Mimi