| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20220110012403.naqx7el2t5i72xm2@altlinux.org>
Date: Mon, 10 Jan 2022 04:24:03 +0300
From: Vitaly Chikunov <vt@...linux.org>
To: Mimi Zohar <zohar@...ux.ibm.com>
Cc: linux-integrity@...r.kernel.org,
Eric Biggers <ebiggers@...nel.org>,
linux-fscrypt@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 5/6] ima: support fs-verity file digest based
signatures
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@...ux.ibm.com>
> ---
> 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
Powered by blists - more mailing lists