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 --- security/integrity/evm/evm_main.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index 0d36259b690d..744c105b48d1 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -521,7 +521,8 @@ int evm_inode_init_security(struct inode *inode, struct evm_xattr *xattr_data; int rc;
- if (!evm_key_loaded() || !evm_protected_xattr(lsm_xattr->name)) + if (!(evm_initialized & EVM_INIT_HMAC) || + !evm_protected_xattr(lsm_xattr->name)) return 0;
xattr_data = kzalloc(sizeof(*xattr_data), GFP_NOFS);
Granting metadata write is safe if the HMAC key is not loaded, as it won't let an attacker obtain a valid HMAC from corrupted xattrs. evm_write_key() however does not allow it if any key is loaded, including a public key, which should not be a problem.
This patch allows setting EVM_ALLOW_METADATA_WRITES if the EVM_INIT_HMAC flag is not set.
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_secfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c index cfc3075769bb..92fe26ace797 100644 --- a/security/integrity/evm/evm_secfs.c +++ b/security/integrity/evm/evm_secfs.c @@ -84,7 +84,7 @@ static ssize_t evm_write_key(struct file *file, const char __user *buf, * keys are loaded. */ if ((i & EVM_ALLOW_METADATA_WRITES) && - ((evm_initialized & EVM_KEY_MASK) != 0) && + ((evm_initialized & EVM_INIT_HMAC) != 0) && !(evm_initialized & EVM_ALLOW_METADATA_WRITES)) return -EPERM;
Hi Roberto,
On Thu, 2020-06-18 at 18:01 +0200, Roberto Sassu wrote:
Granting metadata write is safe if the HMAC key is not loaded, as it won't let an attacker obtain a valid HMAC from corrupted xattrs. evm_write_key() however does not allow it if any key is loaded, including a public key, which should not be a problem.
Why is the existing hebavior a problem? What is the problem being solved?
This patch allows setting EVM_ALLOW_METADATA_WRITES if the EVM_INIT_HMAC flag is not set.
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_secfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c index cfc3075769bb..92fe26ace797 100644 --- a/security/integrity/evm/evm_secfs.c +++ b/security/integrity/evm/evm_secfs.c @@ -84,7 +84,7 @@ static ssize_t evm_write_key(struct file *file, const char __user *buf, * keys are loaded. */ if ((i & EVM_ALLOW_METADATA_WRITES) &&
((evm_initialized & EVM_KEY_MASK) != 0) &&
((evm_initialized & EVM_INIT_HMAC) != 0) && !(evm_initialized & EVM_ALLOW_METADATA_WRITES))
Documentation/ABI/testing/evm needs to be updated as well.
thanks,
Mimi
From: Mimi Zohar [mailto:zohar@linux.ibm.com] Sent: Friday, August 21, 2020 10:15 PM Hi Roberto,
On Thu, 2020-06-18 at 18:01 +0200, Roberto Sassu wrote:
Granting metadata write is safe if the HMAC key is not loaded, as it won't let an attacker obtain a valid HMAC from corrupted xattrs.
evm_write_key()
however does not allow it if any key is loaded, including a public key, which should not be a problem.
Why is the existing hebavior a problem? What is the problem being solved?
Hi Mimi
currently it is not possible to set EVM_ALLOW_METADATA_WRITES when only a public key is loaded and the HMAC key is not. The patch removes this limitation.
This patch allows setting EVM_ALLOW_METADATA_WRITES if the
EVM_INIT_HMAC
flag is not set.
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_secfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/security/integrity/evm/evm_secfs.c
b/security/integrity/evm/evm_secfs.c
index cfc3075769bb..92fe26ace797 100644 --- a/security/integrity/evm/evm_secfs.c +++ b/security/integrity/evm/evm_secfs.c @@ -84,7 +84,7 @@ static ssize_t evm_write_key(struct file *file, const
char __user *buf,
* keys are loaded. */if ((i & EVM_ALLOW_METADATA_WRITES) &&
((evm_initialized & EVM_KEY_MASK) != 0) &&
((evm_initialized & EVM_INIT_HMAC) != 0) && !(evm_initialized & EVM_ALLOW_METADATA_WRITES))Documentation/ABI/testing/evm needs to be updated as well.
Ok.
Thanks
Roberto
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Li Peng, Li Jian, Shi Yanli
thanks,
Mimi
On Mon, 2020-08-31 at 08:24 +0000, Roberto Sassu wrote:
From: Mimi Zohar [mailto:zohar@linux.ibm.com] Sent: Friday, August 21, 2020 10:15 PM Hi Roberto,
On Thu, 2020-06-18 at 18:01 +0200, Roberto Sassu wrote:
Granting metadata write is safe if the HMAC key is not loaded, as it won't let an attacker obtain a valid HMAC from corrupted xattrs.
evm_write_key()
however does not allow it if any key is loaded, including a public key, which should not be a problem.
Why is the existing hebavior a problem? What is the problem being solved?
Hi Mimi
currently it is not possible to set EVM_ALLOW_METADATA_WRITES when only a public key is loaded and the HMAC key is not. The patch removes this limitation.
Yes, I understand. You're describing "what" the problem is, not "why" this is a problem. Support for loading EVM HMAC and x509 certificates isn't new. Please add a line or two prior to this paragraph providing the context for why this is now a problem.
Is the problem related to previoulsy not beginning EVM verification until after the EVM HMAC key was loaded? Or perhaps EVM signatures were not that common since they weren't portable. Now, with portable and immutable signatures loading x509 certificates is more common.
thanks,
Mimi
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 --- security/integrity/evm/evm_main.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index 744c105b48d1..4e9f5e8b21d5 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -181,6 +181,12 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry, break; case EVM_IMA_XATTR_DIGSIG: case EVM_XATTR_PORTABLE_DIGSIG: + /* accept xattr with non-empty signature field */ + if (xattr_len <= sizeof(struct signature_v2_hdr)) { + evm_status = INTEGRITY_FAIL; + goto out; + } + hdr = (struct signature_v2_hdr *)xattr_data; digest.hdr.algo = hdr->hash_algo; rc = evm_calc_hash(dentry, xattr_name, xattr_value,
On Thu, 2020-06-18 at 18:01 +0200, Roberto Sassu wrote:
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
Hi Roberto,
Sorry for the delay in reviewing these patches. Missing from this patch set is a cover letter with an explanation for grouping these patches into a patch set, other than for convenience. In this case, it would be along the lines that the original use case for EVM portable and immutable keys support was for a few critical files, not combined with an EVM encrypted key type. This patch set more fully integrates the initial EVM portable and immutable signature support.
On Thu, 2020-06-18 at 18:01 +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().
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(-)
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index 0d36259b690d..744c105b48d1 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -521,7 +521,8 @@ int evm_inode_init_security(struct inode *inode, struct evm_xattr *xattr_data; int rc;
- if (!evm_key_loaded() || !evm_protected_xattr(lsm_xattr->name))
- if (!(evm_initialized & EVM_INIT_HMAC) ||
!evm_protected_xattr(lsm_xattr->name))xattr_data = kzalloc(sizeof(*xattr_data), GFP_NOFS);
Hi Roberto,
On Fri, 2020-08-21 at 14:30 -0400, Mimi Zohar wrote:
Sorry for the delay in reviewing these patches. Missing from this patch set is a cover letter with an explanation for grouping these patches into a patch set, other than for convenience. In this case, it would be along the lines that the original use case for EVM portable and immutable keys support was for a few critical files, not combined with an EVM encrypted key type. This patch set more fully integrates the initial EVM portable and immutable signature support.
Thank you for more fully integrating the EVM portable signatures into IMA.
" [PATCH 08/11] ima: Allow imasig requirement to be satisfied by EVM portable signatures" equates an IMA signature to having a portable and immutable EVM signature. That is true in terms of signature verification, but from an attestation perspective the "ima-sig" template will not contain a signature. If not the EVM signature, then at least some other indication should be included in the measurement list.
Are you planning on posting the associated IMA/EVM regression tests?
Mimi
From: Mimi Zohar [mailto:zohar@linux.ibm.com] Sent: Monday, August 24, 2020 7:45 PM Hi Roberto,
On Fri, 2020-08-21 at 14:30 -0400, Mimi Zohar wrote:
Sorry for the delay in reviewing these patches. Missing from this patch set is a cover letter with an explanation for grouping these patches into a patch set, other than for convenience. In this case, it would be along the lines that the original use case for EVM portable and immutable keys support was for a few critical files, not combined with an EVM encrypted key type. This patch set more fully integrates the initial EVM portable and immutable signature support.
Thank you for more fully integrating the EVM portable signatures into IMA.
" [PATCH 08/11] ima: Allow imasig requirement to be satisfied by EVM portable signatures" equates an IMA signature to having a portable and immutable EVM signature. That is true in terms of signature verification, but from an attestation perspective the "ima-sig" template will not contain a signature. If not the EVM signature, then at least some other indication should be included in the measurement list.
Would it be ok to print the EVM portable signature in the sig field if the IMA signature is not found? Later we can introduce the new template evm-sig to include all metadata necessary to verify the EVM portable signature.
Are you planning on posting the associated IMA/EVM regression tests?
I didn't have a look yet at the code. I will try to write some later.
Roberto
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Li Peng, Li Jian, Shi Yanli
On Wed, 2020-09-02 at 11:42 +0000, Roberto Sassu wrote:
From: Mimi Zohar [mailto:zohar@linux.ibm.com] Sent: Monday, August 24, 2020 7:45 PM Hi Roberto,
On Fri, 2020-08-21 at 14:30 -0400, Mimi Zohar wrote:
Sorry for the delay in reviewing these patches. Missing from this patch set is a cover letter with an explanation for grouping these patches into a patch set, other than for convenience. In this case, it would be along the lines that the original use case for EVM portable and immutable keys support was for a few critical files, not combined with an EVM encrypted key type. This patch set more fully integrates the initial EVM portable and immutable signature support.
Thank you for more fully integrating the EVM portable signatures into IMA.
" [PATCH 08/11] ima: Allow imasig requirement to be satisfied by EVM portable signatures" equates an IMA signature to having a portable and immutable EVM signature. That is true in terms of signature verification, but from an attestation perspective the "ima-sig" template will not contain a signature. If not the EVM signature, then at least some other indication should be included in the measurement list.
Would it be ok to print the EVM portable signature in the sig field if the IMA signature is not found? Later we can introduce the new template evm-sig to include all metadata necessary to verify the EVM portable signature.
As long as the attestation server can differentiate between the signature types, including the EVM signature should be fine.
Are you planning on posting the associated IMA/EVM regression tests?
I didn't have a look yet at the code. I will try to write some later.
It looks like ima_verify_signature() in ima-evm-utils could be extended to support the EVM portable signature or at least not to fail the signature verification.
Mimi
linux-stable-mirror@lists.linaro.org
-
Mimi Zohar -
Roberto Sassu