| Message ID | 20200618160133.937-1-roberto.sassu@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/11] evm: Execute evm_inode_init_security() only when the HMAC key is loaded | expand |
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)) > return 0; > > 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
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);
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(-)