2022-02-09 09:37:27

by Mimi Zohar

[permalink] [raw]
Subject: [PATCH v4 0/8] ima: support fs-verity digests and signatures

Support for including fs-verity file digests and signatures in the IMA
measurement list as well as verifying the fs-verity file digest based
signatures, both based on IMA policy rules, was discussed prior to
fs-verity being upstreamed[1,2].

Support for including fs-verity file digests in the 'd-ng' template field
is based on a new policy rule option named 'digest_type=verity'. A new
template field named 'd-type' as well as a new template named 'ima-ngv2'
are defined to differentiate between the regular IMA file hashes from the
fs-verity file digests (tree-hash based file hashes) stored in the 'd-ng'
template field.

Support for verifying fs-verity based file signatures stored in the
'security.ima' xattr is similarly based on the policy rule option
'digest_type=verity'.

To differentiate IMA from fs-verity file signatures a new xattr_type
named IMA_VERITY_DIGSIG is defined. Signature version 3, which is a hash
of the ima_file_id struct, disambiguates the signatures stored as
'security.ima' xattr. fs-verity only supports the new signature format
(version 3). To prevent abuse of the different signature formats, policy
rules must be limited to a specific signature version.

[1] https://events19.linuxfoundation.org/wp-content/uploads/2017/11/fs-verify_Mike-Halcrow_Eric-Biggers.pdf
[2] Documentation/filesystems/fsverity.rst

Changelog v4:
- Based on Eric Bigger's signature verification concerns of replacing the
contents of a file with the ima_file_id struct hash, require per policy
rule signature versions.
- Addressed Eric Bigger's other comments.
- Added new audit messages "causes".
- Updated patch descriptions.

Changelog v3:
- Addressed Eric Bigger's comments: included Ack, incremented the
signature format version, the crypto issues are generic and will be
addressed by him separately.
- Addressed Vitaly Chikunov's comments: hard coded maximum digest size
rather than using a flexible array, removed unnecessary assignment, and
fixed comment to match variable name.
- Defined new "ima_max_digest_size" struct to avoid wrapping the
"ima_digest_data" struct inside a function local structure or
having to dynamically allocate it with enough memory for the specific
hash algo size.

Changelog v2:
- Addressed Eric Bigger's comments: sign the hash of fsverity's digest
and the digest's metadata, use match_string, use preferred function
name fsverity_get_digest(), support including unsigned fs-verity's
digests in the IMA measurement list.
- Remove signatures requirement for including fs-verity's file digests in
the 'd-ng' field of the measurement list.

Changelog v1:
- Updated both fsverity and IMA documentation.
- Addressed both Eric Bigger's and Lakshmi's comments.

Mimi Zohar (8):
ima: rename IMA_ACTION_FLAGS to IMA_NONACTION_FLAGS
ima: define ima_max_digest_data struct without a flexible array
variable
fs-verity: define a function to return the integrity protected file
digest
ima: define a new template field 'd-type' and a new template
'ima-ngv2'
ima: permit fsverity's file digests in the IMA measurement list
ima: define signature version 3
ima: support fs-verity file digest based version 3 signatures
fsverity: update the documentation

Documentation/ABI/testing/ima_policy | 22 +++++
Documentation/filesystems/fsverity.rst | 22 +++--
Documentation/security/IMA-templates.rst | 11 ++-
fs/verity/Kconfig | 1 +
fs/verity/fsverity_private.h | 7 --
fs/verity/measure.c | 41 ++++++++
include/linux/fsverity.h | 18 ++++
security/integrity/digsig.c | 3 +-
security/integrity/ima/ima_api.c | 59 ++++++++---
security/integrity/ima/ima_appraise.c | 113 +++++++++++++++++++++-
security/integrity/ima/ima_init.c | 10 +-
security/integrity/ima/ima_main.c | 2 +-
security/integrity/ima/ima_policy.c | 45 ++++++++-
security/integrity/ima/ima_template.c | 3 +
security/integrity/ima/ima_template_lib.c | 23 ++++-
security/integrity/ima/ima_template_lib.h | 2 +
security/integrity/integrity.h | 53 +++++++++-
17 files changed, 385 insertions(+), 50 deletions(-)

--
2.27.0



2022-02-09 09:53:46

by Eric Biggers

[permalink] [raw]
Subject: Re: [PATCH v4 0/8] ima: support fs-verity digests and signatures

On Mon, Feb 07, 2022 at 08:41:32PM -0500, Mimi Zohar wrote:
> Support for including fs-verity file digests and signatures in the IMA
> measurement list as well as verifying the fs-verity file digest based
> signatures, both based on IMA policy rules, was discussed prior to
> fs-verity being upstreamed[1,2].
>
> Support for including fs-verity file digests in the 'd-ng' template field
> is based on a new policy rule option named 'digest_type=verity'. A new
> template field named 'd-type' as well as a new template named 'ima-ngv2'
> are defined to differentiate between the regular IMA file hashes from the
> fs-verity file digests (tree-hash based file hashes) stored in the 'd-ng'
> template field.
>
> Support for verifying fs-verity based file signatures stored in the
> 'security.ima' xattr is similarly based on the policy rule option
> 'digest_type=verity'.
>
> To differentiate IMA from fs-verity file signatures a new xattr_type
> named IMA_VERITY_DIGSIG is defined. Signature version 3, which is a hash
> of the ima_file_id struct, disambiguates the signatures stored as
> 'security.ima' xattr. fs-verity only supports the new signature format
> (version 3). To prevent abuse of the different signature formats, policy
> rules must be limited to a specific signature version.
>
> [1] https://events19.linuxfoundation.org/wp-content/uploads/2017/11/fs-verify_Mike-Halcrow_Eric-Biggers.pdf
> [2] Documentation/filesystems/fsverity.rst

What does this patchset apply to? I'm no longer able to apply it. I tried
both v5.17-rc3, and the next-integrity branch of
https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git.

- Eric

2022-02-09 09:59:38

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH v4 0/8] ima: support fs-verity digests and signatures

On Mon, 2022-02-07 at 21:50 -0800, Eric Biggers wrote:
> On Mon, Feb 07, 2022 at 08:41:32PM -0500, Mimi Zohar wrote:
> > Support for including fs-verity file digests and signatures in the IMA
> > measurement list as well as verifying the fs-verity file digest based
> > signatures, both based on IMA policy rules, was discussed prior to
> > fs-verity being upstreamed[1,2].
> >
> > Support for including fs-verity file digests in the 'd-ng' template field
> > is based on a new policy rule option named 'digest_type=verity'. A new
> > template field named 'd-type' as well as a new template named 'ima-ngv2'
> > are defined to differentiate between the regular IMA file hashes from the
> > fs-verity file digests (tree-hash based file hashes) stored in the 'd-ng'
> > template field.
> >
> > Support for verifying fs-verity based file signatures stored in the
> > 'security.ima' xattr is similarly based on the policy rule option
> > 'digest_type=verity'.
> >
> > To differentiate IMA from fs-verity file signatures a new xattr_type
> > named IMA_VERITY_DIGSIG is defined. Signature version 3, which is a hash
> > of the ima_file_id struct, disambiguates the signatures stored as
> > 'security.ima' xattr. fs-verity only supports the new signature format
> > (version 3). To prevent abuse of the different signature formats, policy
> > rules must be limited to a specific signature version.
> >
> > [1] https://events19.linuxfoundation.org/wp-content/uploads/2017/11/fs-verify_Mike-Halcrow_Eric-Biggers.pdf
> > [2] Documentation/filesystems/fsverity.rst
>
> What does this patchset apply to? I'm no longer able to apply it. I tried
> both v5.17-rc3, and the next-integrity branch of
> https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git.

Just refreshed 'next-integrity' now.

--
thanks,

Mimi


2022-02-09 11:00:07

by Mimi Zohar

[permalink] [raw]
Subject: [PATCH v4 6/8] ima: define signature version 3

To disambiguate the signed data stored in the 'security.ima' xattr,
define signature version 3 as the hash of the ima_file_id structure.

Signed-off-by: Mimi Zohar <[email protected]>
---
security/integrity/digsig.c | 3 ++-
security/integrity/ima/ima_appraise.c | 36 +++++++++++++++++++++++++++
security/integrity/integrity.h | 20 +++++++++++++--
3 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index 3b06a01bd0fd..fd8f77d92a62 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -74,7 +74,8 @@ int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
/* v1 API expect signature without xattr type */
return digsig_verify(keyring, sig + 1, siglen - 1, digest,
digestlen);
- case 2:
+ case 2: /* regular file data hash based sginature */
+ case 3: /* struct ima_file_id data base signature */
return asymmetric_verify(keyring, sig, siglen, digest,
digestlen);
}
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 17232bbfb9f9..7bc180bd808e 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -225,6 +225,34 @@ int ima_read_xattr(struct dentry *dentry,
return ret;
}

+/*
+ * calc_file_id_hash - calculate the hash of the ima_file_id struct data
+ * @type: xattr type [enum evm_ima_xattr_type]
+ * @algo: hash algorithm [enum hash_algo]
+ * @digest: pointer to the digest to be hashed
+ * @hash: (out) pointer to the hash
+ *
+ * IMA signature version 3 disambiguates the data that is signed by
+ * indirectly signing the hash of the ima_file_id structure data.
+ *
+ * Return 0 on success, error code otherwise.
+ */
+static int calc_file_id_hash(enum evm_ima_xattr_type type,
+ enum hash_algo algo, const u8 *digest,
+ struct ima_max_digest_data *hash)
+{
+ struct ima_file_id file_id = {.hash_algorithm = algo};
+ uint unused = HASH_MAX_DIGESTSIZE - hash_digest_size[algo];
+
+ memcpy(file_id.hash, digest, hash_digest_size[algo]);
+
+ hash->algo = algo;
+ hash->length = hash_digest_size[algo];
+
+ return ima_calc_buffer_hash(&file_id, sizeof(file_id) - unused,
+ (struct ima_digest_data *)hash);
+}
+
/*
* xattr_verify - verify xattr digest or signature
*
@@ -236,6 +264,7 @@ 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 signature_v2_hdr *sig;
int rc = -EINVAL, hash_start = 0;

switch (xattr_value->type) {
@@ -274,6 +303,13 @@ static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint,
break;
case EVM_IMA_XATTR_DIGSIG:
set_bit(IMA_DIGSIG, &iint->atomic_flags);
+
+ sig = (typeof(sig))xattr_value;
+ if (sig->version != 2) {
+ *cause = "invalid-signature-version";
+ *status = INTEGRITY_FAIL;
+ break;
+ }
rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA,
(const char *)xattr_value,
xattr_len,
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index cbc41d4288ed..f59f83527c0c 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -94,7 +94,7 @@ struct evm_xattr {
u8 digest[SHA1_DIGEST_SIZE];
} __packed;

-#define IMA_MAX_DIGEST_SIZE 64
+#define IMA_MAX_DIGEST_SIZE HASH_MAX_DIGESTSIZE

struct ima_digest_data {
u8 algo;
@@ -137,7 +137,11 @@ struct ima_max_digest_data {
} __packed;

/*
- * signature format v2 - for using with asymmetric keys
+ * signature header format v2 - for using with asymmetric keys
+ *
+ * signature format:
+ * version 2: regular file data hash based signature
+ * version 3: struct ima_file_id data based signature
*/
struct signature_v2_hdr {
uint8_t type; /* xattr type */
@@ -148,6 +152,18 @@ struct signature_v2_hdr {
uint8_t sig[]; /* signature payload */
} __packed;

+/*
+ * IMA signature version 3 disambiguates the data that is signed, by
+ * indirectly signing the hash of the ima_file_id structure data.
+ *
+ * (The hash of the ima_file_id structure is only of the portion used.)
+ */
+struct ima_file_id {
+ __u8 hash_type; /* xattr type [enum evm_ima_xattr_type] */
+ __u8 hash_algorithm; /* Digest algorithm [enum hash_algo] */
+ __u8 hash[HASH_MAX_DIGESTSIZE];
+} __packed;
+
/* integrity data associated with an inode */
struct integrity_iint_cache {
struct rb_node rb_node; /* rooted in integrity_iint_tree */
--
2.27.0