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: [email protected] # 4.5.x
Fixes: 26ddabfe96b ("evm: enable EVM when X509 certificate is loaded")
Signed-off-by: Roberto Sassu <[email protected]>
---
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);
--
2.17.1
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: [email protected] # 4.19.x
Fixes: 5feeb61183dde ("evm: Allow non-SHA1 digital signatures")
Signed-off-by: Roberto Sassu <[email protected]>
---
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,
--
2.17.1
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: [email protected] # 4.5.x
> Fixes: 26ddabfe96b ("evm: enable EVM when X509 certificate is loaded")
> Signed-off-by: Roberto Sassu <[email protected]>
Reviewed-by: Mimi Zohar <[email protected]>
> ---
> 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);
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: [email protected] # 4.19.x
> Fixes: 5feeb61183dde ("evm: Allow non-SHA1 digital signatures")
> Signed-off-by: Roberto Sassu <[email protected]>
Reviewed-by: Mimi Zohar <[email protected]>
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:[email protected]]
> 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:[email protected]]
> > 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