Mimi, On Sun, Jan 09, 2022 at 01:55:16PM -0500, Mimi Zohar wrote: > Instead of calculating a file hash and verifying the signature stored > in the security.ima xattr against the calculated file hash, verify the > signature based on a hash of fs-verity's file digest and the digest's > metadata. > > To differentiate between a regular file hash and an fs-verity file digest > based signature stored as security.ima xattr, define a new signature type > named IMA_VERITY_DIGSIG. > > The hash format of fs-verity's file digest and the digest's metadata to > be signed is defined as a structure named "ima_tbs_hash". > > Update the 'ima-sig' template field to display the new fs-verity signature > type as well. > > For example: > appraise func=BPRM_CHECK digest_type=hash|verity > > Signed-off-by: Mimi Zohar <zohar@xxxxxxxxxxxxx> > --- > Documentation/ABI/testing/ima_policy | 10 +++ > Documentation/security/IMA-templates.rst | 4 +- > include/uapi/linux/ima.h | 26 ++++++++ > security/integrity/ima/ima_appraise.c | 81 +++++++++++++++++++++++ > security/integrity/ima/ima_template_lib.c | 3 +- > security/integrity/integrity.h | 1 + > 6 files changed, 122 insertions(+), 3 deletions(-) > create mode 100644 include/uapi/linux/ima.h > > diff --git a/include/uapi/linux/ima.h b/include/uapi/linux/ima.h > new file mode 100644 > index 000000000000..6a2a68fc0fad > --- /dev/null > +++ b/include/uapi/linux/ima.h > @@ -0,0 +1,26 @@ > +/* SPDX-License-Identifier: GPL-2.0-only WITH Linux-syscall-note */ > +/* > + * IMA user API > + * > + */ > +#ifndef _UAPI_LINUX_IMA_H > +#define _UAPI_LINUX_IMA_H > + > +#include <linux/types.h> > + > +/* > + * The hash format of fs-verity's file digest and other file metadata > + * to be signed. The resulting signature is stored as a security.ima > + * xattr. > + * > + * "type" is defined as IMA_VERITY_DIGSIG > + * "algo" is the hash_algo enum of fs-verity's file digest > + * (e.g. HASH_ALGO_SHA256, HASH_ALGO_SHA512). > + */ > +struct ima_tbs_hash { > + __u8 type; /* xattr type [enum evm_ima_xattr_type] */ > + __u8 algo; /* Digest algorithm [enum hash_algo] */ > + __u8 digest[]; /* fs-verity digest */ Maximum digest size is known to be FS_VERITY_MAX_DIGEST_SIZE. If it's allocated here then ima_tbs_hash could be allocated temporarily on stack instead of kalloc. > +}; > + > +#endif /* _UAPI_LINUX_IMA_H */ > diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c > index dbba51583e7c..4e092c189ed0 100644 > --- a/security/integrity/ima/ima_appraise.c > +++ b/security/integrity/ima/ima_appraise.c > @@ -13,7 +13,10 @@ > #include <linux/magic.h> > #include <linux/ima.h> > #include <linux/evm.h> > +#include <linux/fsverity.h> > #include <keys/system_keyring.h> > +#include <uapi/linux/fsverity.h> > +#include <uapi/linux/ima.h> > > #include "ima.h" > > @@ -183,6 +186,8 @@ enum hash_algo ima_get_hash_algo(const struct evm_ima_xattr_data *xattr_value, > return ima_hash_algo; > > switch (xattr_value->type) { > + case IMA_VERITY_DIGSIG: > + fallthrough; I think fallthrough is not needed there, since it's just multiple case's and no code. > case EVM_IMA_XATTR_DIGSIG: > sig = (typeof(sig))xattr_value; > if (sig->version != 2 || xattr_len <= sizeof(*sig) > @@ -225,6 +230,47 @@ int ima_read_xattr(struct dentry *dentry, > return ret; > } > > +/* > + * calc_tbs_hash - calculate hash of a digest and digest metadata > + * @type: signature type [IMA_VERITY_DIGSIG] Parameter seems renamed, but why it's ever need if it's called once and ever with IMA_VERITY_DIGSIG? If it's deleted then its value no need to be checked below. > + * @algo: hash algorithm [enum hash_algo] > + * @digest: pointer to the digest to be hashed > + * @hash: (out) pointer to the hash > + * > + * The IMA signature is a signature over the hash of fs-verity's file digest > + * with other digest metadata, not just fs-verity's file digest. Calculate > + * the to be signed hash. > + * > + * Return 0 on success, error code otherwise. > + */ > +static int calc_tbs_hash(enum evm_ima_xattr_type xattr_type, > + enum hash_algo algo, const u8 *digest, > + struct ima_digest_data *hash) > +{ > + struct ima_tbs_hash *tbs_h; > + int rc = 0; > + > + if (xattr_type != IMA_VERITY_DIGSIG) > + return -EINVAL; > + > + tbs_h = kzalloc(sizeof(*tbs_h) + hash_digest_size[algo], GFP_KERNEL); > + if (!tbs_h) > + return -ENOMEM; > + > + tbs_h->type = xattr_type; > + tbs_h->algo = algo; > + memcpy(tbs_h->digest, digest, hash_digest_size[algo]); > + > + hash->algo = algo; As I understood all of this - hash algo used in fs-verity and algo used to hash it here are the same. Ultimate source of which is algo id from xattr - if fs-verity digest algo differs from xattr's then fs-verity digest is ignored. Thanks, > + hash->length = hash_digest_size[algo]; > + > + rc = ima_calc_buffer_hash(tbs_h, > + sizeof(*tbs_h) + hash_digest_size[algo], > + hash); > + kfree(tbs_h); > + return rc; > +} > + > /* > * xattr_verify - verify xattr digest or signature > * > @@ -236,7 +282,9 @@ static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint, > struct evm_ima_xattr_data *xattr_value, int xattr_len, > enum integrity_status *status, const char **cause) > { > + struct ima_digest_data *hash = NULL; > int rc = -EINVAL, hash_start = 0; > + u8 algo; /* Digest algorithm [enum hash_algo] */ > > switch (xattr_value->type) { > case IMA_XATTR_DIGEST_NG: > @@ -271,6 +319,38 @@ static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint, > break; > } > *status = INTEGRITY_PASS; > + break; > + case IMA_VERITY_DIGSIG: > + set_bit(IMA_DIGSIG, &iint->atomic_flags); > + > + algo = iint->ima_hash->algo; > + hash = kzalloc(sizeof(*hash) + hash_digest_size[algo], > + GFP_KERNEL); > + if (!hash) { > + *cause = "verity-hashing-error"; > + *status = INTEGRITY_FAIL; > + break; > + } > + > + rc = calc_tbs_hash(IMA_VERITY_DIGSIG, iint->ima_hash->algo, > + iint->ima_hash->digest, hash); > + if (rc) { > + *cause = "verity-hashing-error"; > + *status = INTEGRITY_FAIL; > + break; > + } > + > + rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA, > + (const char *)xattr_value, > + xattr_len, hash->digest, > + hash->length); > + if (rc) { > + *cause = "invalid-verity-signature"; > + *status = INTEGRITY_FAIL; > + } else { > + *status = INTEGRITY_PASS; > + } > + > break; > case EVM_IMA_XATTR_DIGSIG: > set_bit(IMA_DIGSIG, &iint->atomic_flags); > @@ -303,6 +383,7 @@ static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint, > break; > } > > + kfree(hash); > return rc; > } > > diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c > index 1c0cea2b805f..31a14943e459 100644 > --- a/security/integrity/ima/ima_template_lib.c > +++ b/security/integrity/ima/ima_template_lib.c > @@ -498,7 +498,8 @@ int ima_eventsig_init(struct ima_event_data *event_data, > { > struct evm_ima_xattr_data *xattr_value = event_data->xattr_value; > > - if ((!xattr_value) || (xattr_value->type != EVM_IMA_XATTR_DIGSIG)) > + if ((!xattr_value) || > + !(xattr_value->type & (EVM_IMA_XATTR_DIGSIG | IMA_VERITY_DIGSIG))) > return ima_eventevmsig_init(event_data, field_data); > > return ima_write_template_field_data(xattr_value, event_data->xattr_len, > diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h > index e7ac1086d1d9..51124708c072 100644 > --- a/security/integrity/integrity.h > +++ b/security/integrity/integrity.h > @@ -79,6 +79,7 @@ enum evm_ima_xattr_type { > EVM_IMA_XATTR_DIGSIG, > IMA_XATTR_DIGEST_NG, > EVM_XATTR_PORTABLE_DIGSIG, > + IMA_VERITY_DIGSIG, > IMA_XATTR_LAST > }; > > -- > 2.27.0
![]() |