2022-04-29 16:50:50

by Mimi Zohar

[permalink] [raw]
Subject: [PATCH v8 0/7] 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 was discussed prior to fs-verity being upstreamed[1,2].

Including fs-verity file digests and signatures in the IMA measurement
list need to be based on policy and be identifiable. To address being
based on policy, a new policy rule option "digest_type=verity", applicable
to both "measure" and "appraise" policy rules, is defined. To address
being identifiable, a new template field 'd-ngv2' and two new template
formats 'ima-ngv2' and 'ima-sigv2' are defined.

d-ngv2: prefixes the digest type ("ima", "verity") to the digest
algorithm and digest.

ima-ngv2', ima-sigv2: templates with the new d-ngv2 field defined.

In addition the signatures stored in 'security.ima' xattr need to be
disambiguated. So instead of directly signing the fs-verity digest, the
fs-verity digest is indirectly signed, by signing the hash of the new
ima_file_id structure data (signature version 3) containing the fs-verity
digest and other metadata.

New policy rule option:
appraise_type=sigv3: support for new IMA signature version 3


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

Changelog v8:
- Based on Eric Bigger's review, fixed the original 'd-ng' and the new
DATA_FMT_DIGEST_WITH_TYPE_ALGO comments and documentation, moved the
buffer measurement test to after fs-verity measurements, renamed
appraise_type "sigv2" to "sigv3", require the new "digest_type=verity"
policy option be specified prior to "appraise_type=sigv3", and updated
the fs-verity documentation.

Changelog v7:
- Based on Stefan Berger's review, cleaned up code by defining an enum,
removed unnecessary memcpy, fs-verity documentation suggestions.
- Add comment in ima_get_verify_digest() with explanation for always
returning the fs-verity digest.

Changelog v6:
- As suggested by Eric Bigger's, instead of defining a new field to
differentiate between IMA and fs-verity signatures, prepend the
digest type to the digest field.
- Addressed Eric Bigger's comments: updated the patch description,
corrected comment, squashed patches, fixed enumeration usage,and
added assumption to fsverity_get_digest.
- Removed the now unnecessary IMA_VERITY_DIGEST flag
- Updated kernel-parameters.txt

Changelog v5:
- Define ima_max_digest_size struct, removing the locally defined versions.
- Don't overload the 'digest_type=verity' to imply a verity signature,
but extend the 'appraise_type' policy rule option to define 'sigv3'.

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 (7):
ima: fix 'd-ng' comments and documentation
ima: use IMA default hash algorithm for integrity violations
fs-verity: define a function to return the integrity protected file
digest
ima: define a new template field named 'd-ngv2' and templates
ima: permit fsverity's file digests in the IMA measurement list
ima: support fs-verity file digest based version 3 signatures
fsverity: update the documentation

Documentation/ABI/testing/ima_policy | 45 ++++++-
.../admin-guide/kernel-parameters.txt | 3 +-
Documentation/filesystems/fsverity.rst | 35 ++++--
Documentation/security/IMA-templates.rst | 11 +-
fs/verity/Kconfig | 1 +
fs/verity/fsverity_private.h | 7 --
fs/verity/measure.c | 43 +++++++
include/linux/fsverity.h | 18 +++
security/integrity/digsig.c | 3 +-
security/integrity/ima/ima_api.c | 49 +++++++-
security/integrity/ima/ima_appraise.c | 114 +++++++++++++++++-
security/integrity/ima/ima_main.c | 2 +-
security/integrity/ima/ima_policy.c | 79 ++++++++++--
security/integrity/ima/ima_template.c | 4 +
security/integrity/ima/ima_template_lib.c | 97 ++++++++++++---
security/integrity/ima/ima_template_lib.h | 4 +
security/integrity/integrity.h | 27 ++++-
17 files changed, 480 insertions(+), 62 deletions(-)

--
2.27.0


2022-04-29 17:57:21

by Mimi Zohar

[permalink] [raw]
Subject: [PATCH v8 3/7] fs-verity: define a function to return the integrity protected file digest

Define a function named fsverity_get_digest() to return the verity file
digest and the associated hash algorithm (enum hash_algo).

This assumes that before calling fsverity_get_digest() the file must have
been opened, which is even true for the IMA measure/appraise on file
open policy rule use case (func=FILE_CHECK). do_open() calls vfs_open()
immediately prior to ima_file_check().

Acked-by: Eric Biggers <[email protected]>
Signed-off-by: Mimi Zohar <[email protected]>
---
fs/verity/Kconfig | 1 +
fs/verity/fsverity_private.h | 7 ------
fs/verity/measure.c | 43 ++++++++++++++++++++++++++++++++++++
include/linux/fsverity.h | 18 +++++++++++++++
4 files changed, 62 insertions(+), 7 deletions(-)

diff --git a/fs/verity/Kconfig b/fs/verity/Kconfig
index 24d1b54de807..54598cd80145 100644
--- a/fs/verity/Kconfig
+++ b/fs/verity/Kconfig
@@ -3,6 +3,7 @@
config FS_VERITY
bool "FS Verity (read-only file-based authenticity protection)"
select CRYPTO
+ select CRYPTO_HASH_INFO
# SHA-256 is implied as it's intended to be the default hash algorithm.
# To avoid bloat, other wanted algorithms must be selected explicitly.
# Note that CRYPTO_SHA256 denotes the generic C implementation, but
diff --git a/fs/verity/fsverity_private.h b/fs/verity/fsverity_private.h
index a7920434bae5..c6fb62e0ef1a 100644
--- a/fs/verity/fsverity_private.h
+++ b/fs/verity/fsverity_private.h
@@ -14,7 +14,6 @@

#define pr_fmt(fmt) "fs-verity: " fmt

-#include <crypto/sha2.h>
#include <linux/fsverity.h>
#include <linux/mempool.h>

@@ -26,12 +25,6 @@ struct ahash_request;
*/
#define FS_VERITY_MAX_LEVELS 8

-/*
- * Largest digest size among all hash algorithms supported by fs-verity.
- * Currently assumed to be <= size of fsverity_descriptor::root_hash.
- */
-#define FS_VERITY_MAX_DIGEST_SIZE SHA512_DIGEST_SIZE
-
/* A hash algorithm supported by fs-verity */
struct fsverity_hash_alg {
struct crypto_ahash *tfm; /* hash tfm, allocated on demand */
diff --git a/fs/verity/measure.c b/fs/verity/measure.c
index f0d7b30c62db..e99c00350c28 100644
--- a/fs/verity/measure.c
+++ b/fs/verity/measure.c
@@ -57,3 +57,46 @@ int fsverity_ioctl_measure(struct file *filp, void __user *_uarg)
return 0;
}
EXPORT_SYMBOL_GPL(fsverity_ioctl_measure);
+
+/**
+ * fsverity_get_digest() - get a verity file's digest
+ * @inode: inode to get digest of
+ * @digest: (out) pointer to the digest
+ * @alg: (out) pointer to the hash algorithm enumeration
+ *
+ * Return the file hash algorithm and digest of an fsverity protected file.
+ * Assumption: before calling fsverity_get_digest(), the file must have been
+ * opened.
+ *
+ * Return: 0 on success, -errno on failure
+ */
+int fsverity_get_digest(struct inode *inode,
+ u8 digest[FS_VERITY_MAX_DIGEST_SIZE],
+ enum hash_algo *alg)
+{
+ const struct fsverity_info *vi;
+ const struct fsverity_hash_alg *hash_alg;
+ int i;
+
+ vi = fsverity_get_info(inode);
+ if (!vi)
+ return -ENODATA; /* not a verity file */
+
+ hash_alg = vi->tree_params.hash_alg;
+ memset(digest, 0, FS_VERITY_MAX_DIGEST_SIZE);
+
+ /* convert the verity hash algorithm name to a hash_algo_name enum */
+ i = match_string(hash_algo_name, HASH_ALGO__LAST, hash_alg->name);
+ if (i < 0)
+ return -EINVAL;
+ *alg = i;
+
+ if (WARN_ON_ONCE(hash_alg->digest_size != hash_digest_size[*alg]))
+ return -EINVAL;
+ memcpy(digest, vi->file_digest, hash_alg->digest_size);
+
+ pr_debug("file digest %s:%*phN\n", hash_algo_name[*alg],
+ hash_digest_size[*alg], digest);
+
+ return 0;
+}
diff --git a/include/linux/fsverity.h b/include/linux/fsverity.h
index a7afc800bd8d..7af030fa3c36 100644
--- a/include/linux/fsverity.h
+++ b/include/linux/fsverity.h
@@ -12,8 +12,16 @@
#define _LINUX_FSVERITY_H

#include <linux/fs.h>
+#include <crypto/hash_info.h>
+#include <crypto/sha2.h>
#include <uapi/linux/fsverity.h>

+/*
+ * Largest digest size among all hash algorithms supported by fs-verity.
+ * Currently assumed to be <= size of fsverity_descriptor::root_hash.
+ */
+#define FS_VERITY_MAX_DIGEST_SIZE SHA512_DIGEST_SIZE
+
/* Verity operations for filesystems */
struct fsverity_operations {

@@ -131,6 +139,9 @@ int fsverity_ioctl_enable(struct file *filp, const void __user *arg);
/* measure.c */

int fsverity_ioctl_measure(struct file *filp, void __user *arg);
+int fsverity_get_digest(struct inode *inode,
+ u8 digest[FS_VERITY_MAX_DIGEST_SIZE],
+ enum hash_algo *alg);

/* open.c */

@@ -170,6 +181,13 @@ static inline int fsverity_ioctl_measure(struct file *filp, void __user *arg)
return -EOPNOTSUPP;
}

+static inline int fsverity_get_digest(struct inode *inode,
+ u8 digest[FS_VERITY_MAX_DIGEST_SIZE],
+ enum hash_algo *alg)
+{
+ return -EOPNOTSUPP;
+}
+
/* open.c */

static inline int fsverity_file_open(struct inode *inode, struct file *filp)
--
2.27.0

2022-04-29 22:02:49

by Mimi Zohar

[permalink] [raw]
Subject: [PATCH v8 1/7] ima: fix 'd-ng' comments and documentation

Initially the 'd-ng' template field did not prefix the digest with either
"md5" or "sha1" hash algorithms. Prior to being upstreamed this changed,
but the comments and documentation were not updated. Fix the comments
and documentation.

Fixes: 4d7aeee73f53 ("ima: define new template ima-ng and template fields d-ng and n-ng")
Reported-by: Eric Biggers <[email protected]>
Signed-off-by: Mimi Zohar <[email protected]>
---
Documentation/security/IMA-templates.rst | 3 +--
security/integrity/ima/ima_template_lib.c | 8 +++++---
2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/Documentation/security/IMA-templates.rst b/Documentation/security/IMA-templates.rst
index 1a91d92950a7..cab97f49971d 100644
--- a/Documentation/security/IMA-templates.rst
+++ b/Documentation/security/IMA-templates.rst
@@ -66,8 +66,7 @@ descriptors by adding their identifier to the format string
calculated with the SHA1 or MD5 hash algorithm;
- 'n': the name of the event (i.e. the file name), with size up to 255 bytes;
- 'd-ng': the digest of the event, calculated with an arbitrary hash
- algorithm (field format: [<hash algo>:]digest, where the digest
- prefix is shown only if the hash algorithm is not SHA1 or MD5);
+ algorithm (field format: <hash algo>:digest);
- 'd-modsig': the digest of the event without the appended modsig;
- 'n-ng': the name of the event, without size limitations;
- 'sig': the file signature, or the EVM portable signature if the file
diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
index 7155d17a3b75..e9d65f6fe2ae 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -271,9 +271,11 @@ static int ima_eventdigest_init_common(const u8 *digest, u32 digestsize,
/*
* digest formats:
* - DATA_FMT_DIGEST: digest
- * - DATA_FMT_DIGEST_WITH_ALGO: [<hash algo>] + ':' + '\0' + digest,
- * where <hash algo> is provided if the hash algorithm is not
- * SHA1 or MD5
+ * - DATA_FMT_DIGEST_WITH_ALGO: <hash algo> + ':' + '\0' + digest,
+ *
+ * where 'DATA_FMT_DIGEST' is the original digest format ('d')
+ * with a hash size limitation of 20 bytes,
+ * where <hash algo> is the hash_algo_name[] string.
*/
u8 buffer[CRYPTO_MAX_ALG_NAME + 2 + IMA_MAX_DIGEST_SIZE] = { 0 };
enum data_formats fmt = DATA_FMT_DIGEST;
--
2.27.0

2022-04-30 03:38:05

by Mimi Zohar

[permalink] [raw]
Subject: [PATCH v8 7/7] fsverity: update the documentation

Update the fsverity documentation related to IMA signature support.

Signed-off-by: Mimi Zohar <[email protected]>
---
Documentation/filesystems/fsverity.rst | 35 +++++++++++++++++---------
1 file changed, 23 insertions(+), 12 deletions(-)

diff --git a/Documentation/filesystems/fsverity.rst b/Documentation/filesystems/fsverity.rst
index 8cc536d08f51..b7d42fd65e9d 100644
--- a/Documentation/filesystems/fsverity.rst
+++ b/Documentation/filesystems/fsverity.rst
@@ -70,12 +70,23 @@ must live on a read-write filesystem because they are independently
updated and potentially user-installed, so dm-verity cannot be used.

The base fs-verity feature is a hashing mechanism only; actually
-authenticating the files is up to userspace. However, to meet some
-users' needs, fs-verity optionally supports a simple signature
-verification mechanism where users can configure the kernel to require
-that all fs-verity files be signed by a key loaded into a keyring; see
-`Built-in signature verification`_. Support for fs-verity file hashes
-in IMA (Integrity Measurement Architecture) policies is also planned.
+authenticating the files may be done by:
+
+* Userspace-only
+
+* Builtin signature verification + userspace policy
+
+ fs-verity optionally supports a simple signature verification
+ mechanism where users can configure the kernel to require that
+ all fs-verity files be signed by a key loaded into a keyring;
+ see `Built-in signature verification`_.
+
+* Integrity Measurement Architecture (IMA)
+
+ IMA supports including fs-verity file digests and signatures in the
+ IMA measurement list and verifying fs-verity based file signatures
+ stored as security.ima xattrs, based on policy.
+

User API
========
@@ -653,12 +664,12 @@ weren't already directly answered in other parts of this document.
hashed and what to do with those hashes, such as log them,
authenticate them, or add them to a measurement list.

- IMA is planned to support the fs-verity hashing mechanism as an
- alternative to doing full file hashes, for people who want the
- performance and security benefits of the Merkle tree based hash.
- But it doesn't make sense to force all uses of fs-verity to be
- through IMA. As a standalone filesystem feature, fs-verity
- already meets many users' needs, and it's testable like other
+ IMA supports the fs-verity hashing mechanism as an alternative
+ to full file hashes, for those who want the performance and
+ security benefits of the Merkle tree based hash. However, it
+ doesn't make sense to force all uses of fs-verity to be through
+ IMA. fs-verity already meets many users' needs even as a
+ standalone filesystem feature, and it's testable like other
filesystem features e.g. with xfstests.

:Q: Isn't fs-verity useless because the attacker can just modify the
--
2.27.0

2022-04-30 14:52:24

by Stefan Berger

[permalink] [raw]
Subject: Re: [PATCH v8 1/7] ima: fix 'd-ng' comments and documentation



On 4/29/22 07:25, Mimi Zohar wrote:
> Initially the 'd-ng' template field did not prefix the digest with either
> "md5" or "sha1" hash algorithms. Prior to being upstreamed this changed,
> but the comments and documentation were not updated. Fix the comments
> and documentation.
>
> Fixes: 4d7aeee73f53 ("ima: define new template ima-ng and template fields d-ng and n-ng")
> Reported-by: Eric Biggers <[email protected]>
> Signed-off-by: Mimi Zohar <[email protected]>
> ---
> Documentation/security/IMA-templates.rst | 3 +--
> security/integrity/ima/ima_template_lib.c | 8 +++++---
> 2 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/security/IMA-templates.rst b/Documentation/security/IMA-templates.rst
> index 1a91d92950a7..cab97f49971d 100644
> --- a/Documentation/security/IMA-templates.rst
> +++ b/Documentation/security/IMA-templates.rst
> @@ -66,8 +66,7 @@ descriptors by adding their identifier to the format string
> calculated with the SHA1 or MD5 hash algorithm;
> - 'n': the name of the event (i.e. the file name), with size up to 255 bytes;
> - 'd-ng': the digest of the event, calculated with an arbitrary hash
> - algorithm (field format: [<hash algo>:]digest, where the digest
> - prefix is shown only if the hash algorithm is not SHA1 or MD5);

That seemed to be true for 'd'

> + algorithm (field format: <hash algo>:digest);
> - 'd-modsig': the digest of the event without the appended modsig;
> - 'n-ng': the name of the event, without size limitations;
> - 'sig': the file signature, or the EVM portable signature if the file
> diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
> index 7155d17a3b75..e9d65f6fe2ae 100644
> --- a/security/integrity/ima/ima_template_lib.c
> +++ b/security/integrity/ima/ima_template_lib.c
> @@ -271,9 +271,11 @@ static int ima_eventdigest_init_common(const u8 *digest, u32 digestsize,
> /*
> * digest formats:
> * - DATA_FMT_DIGEST: digest
> - * - DATA_FMT_DIGEST_WITH_ALGO: [<hash algo>] + ':' + '\0' + digest,
> - * where <hash algo> is provided if the hash algorithm is not
> - * SHA1 or MD5
> + * - DATA_FMT_DIGEST_WITH_ALGO: <hash algo> + ':' + '\0' + digest, > + *
> + * where 'DATA_FMT_DIGEST' is the original digest format ('d')
> + * with a hash size limitation of 20 bytes,
> + * where <hash algo> is the hash_algo_name[] string.
> */
> u8 buffer[CRYPTO_MAX_ALG_NAME + 2 + IMA_MAX_DIGEST_SIZE] = { 0 };
> enum data_formats fmt = DATA_FMT_DIGEST;

Reviewed-by: Stefan Berger <[email protected]>

2022-04-30 17:30:03

by Mimi Zohar

[permalink] [raw]
Subject: [PATCH v8 5/7] ima: permit fsverity's file digests in the IMA measurement list

Permit fsverity's file digest (a hash of struct fsverity_descriptor) to
be included in the IMA measurement list, based on the new measurement
policy rule 'digest_type=verity' option.

To differentiate between a regular IMA file hash from an fsverity's
file digest, use the new d-ngv2 format field included in the ima-ngv2
template.

The following policy rule requires fsverity file digests and specifies
the new 'ima-ngv2' template, which contains the new 'd-ngv2' field. The
policy rule may be constrained, for example based on a fsuuid or LSM
label.

measure func=FILE_CHECK digest_type=verity template=ima-ngv2

Signed-off-by: Mimi Zohar <[email protected]>
---
Documentation/ABI/testing/ima_policy | 14 ++++++-
Documentation/security/IMA-templates.rst | 2 +-
security/integrity/ima/ima_api.c | 49 +++++++++++++++++++++--
security/integrity/ima/ima_main.c | 2 +-
security/integrity/ima/ima_policy.c | 38 +++++++++++++++++-
security/integrity/ima/ima_template_lib.c | 10 +++--
security/integrity/integrity.h | 1 +
7 files changed, 105 insertions(+), 11 deletions(-)

diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index 839fab811b18..0a8caed393e3 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -27,8 +27,9 @@ Description:
[fowner=] [fgroup=]]
lsm: [[subj_user=] [subj_role=] [subj_type=]
[obj_user=] [obj_role=] [obj_type=]]
- option: [[appraise_type=]] [template=] [permit_directio]
- [appraise_flag=] [appraise_algos=] [keyrings=]
+ option: [digest_type=] [template=] [permit_directio]
+ [appraise_type=] [appraise_flag=]
+ [appraise_algos=] [keyrings=]
base:
func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK]
[FIRMWARE_CHECK]
@@ -51,6 +52,9 @@ Description:
appraise_flag:= [check_blacklist]
Currently, blacklist check is only for files signed with appended
signature.
+ digest_type:= verity
+ Require fs-verity's file digest instead of the
+ regular IMA file hash.
keyrings:= list of keyrings
(eg, .builtin_trusted_keys|.ima). Only valid
when action is "measure" and func is KEY_CHECK.
@@ -149,3 +153,9 @@ Description:
security.ima xattr of a file:

appraise func=SETXATTR_CHECK appraise_algos=sha256,sha384,sha512
+
+ Example of a 'measure' rule requiring fs-verity's digests
+ with indication of type of digest in the measurement list.
+
+ measure func=FILE_CHECK digest_type=verity \
+ template=ima-ngv2
diff --git a/Documentation/security/IMA-templates.rst b/Documentation/security/IMA-templates.rst
index eafc4e34f890..09b5fac38195 100644
--- a/Documentation/security/IMA-templates.rst
+++ b/Documentation/security/IMA-templates.rst
@@ -67,7 +67,7 @@ descriptors by adding their identifier to the format string
- 'n': the name of the event (i.e. the file name), with size up to 255 bytes;
- 'd-ng': the digest of the event, calculated with an arbitrary hash
algorithm (field format: <hash algo>:digest);
- - 'd-ngv2': same as d-ng, but prefixed with the "ima" digest type
+ - 'd-ngv2': same as d-ng, but prefixed with the "ima" or "verity" digest type
(field format: <digest type>:<hash algo>:digest);
- 'd-modsig': the digest of the event without the appended modsig;
- 'n-ng': the name of the event, without size limitations;
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index c6805af46211..d64ec031b1b4 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -14,6 +14,7 @@
#include <linux/xattr.h>
#include <linux/evm.h>
#include <linux/iversion.h>
+#include <linux/fsverity.h>

#include "ima.h"

@@ -200,6 +201,34 @@ int ima_get_action(struct user_namespace *mnt_userns, struct inode *inode,
allowed_algos);
}

+static int ima_get_verity_digest(struct integrity_iint_cache *iint,
+ struct ima_max_digest_data *hash)
+{
+ enum hash_algo verity_alg;
+ int ret;
+
+ /*
+ * On failure, 'measure' policy rules will result in a file data
+ * hash containing 0's.
+ */
+ ret = fsverity_get_digest(iint->inode, hash->digest, &verity_alg);
+ if (ret) {
+ memset(hash->digest, 0, sizeof(hash->digest));
+ return ret;
+ }
+
+ /*
+ * Unlike in the case of actually calculating the file hash, in
+ * the fsverity case regardless of the hash algorithm, return
+ * the verity digest to be included in the measurement list. A
+ * mismatch between the verity algorithm and the xattr signature
+ * algorithm, if one exists, will be detected later.
+ */
+ hash->hdr.algo = verity_alg;
+ hash->hdr.length = hash_digest_size[verity_alg];
+ return 0;
+}
+
/*
* ima_collect_measurement - collect file measurement
*
@@ -242,16 +271,30 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
*/
i_version = inode_query_iversion(inode);
hash.hdr.algo = algo;
+ hash.hdr.length = hash_digest_size[algo];

/* Initialize hash digest to 0's in case of failure */
memset(&hash.digest, 0, sizeof(hash.digest));

- if (buf)
+ if (iint->flags & IMA_VERITY_REQUIRED) {
+ result = ima_get_verity_digest(iint, &hash);
+ switch (result) {
+ case 0:
+ break;
+ case -ENODATA:
+ audit_cause = "no-verity-digest";
+ break;
+ default:
+ audit_cause = "invalid-verity-digest";
+ break;
+ }
+ } else if (buf) {
result = ima_calc_buffer_hash(buf, size, &hash.hdr);
- else
+ } else {
result = ima_calc_file_hash(file, &hash.hdr);
+ }

- if (result && result != -EBADF && result != -EINVAL)
+ if (result == -ENOMEM)
goto out;

length = sizeof(hash.hdr) + hash.hdr.length;
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 1aebf63ad7a6..040b03ddc1c7 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -335,7 +335,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
hash_algo = ima_get_hash_algo(xattr_value, xattr_len);

rc = ima_collect_measurement(iint, file, buf, size, hash_algo, modsig);
- if (rc != 0 && rc != -EBADF && rc != -EINVAL)
+ if (rc == -ENOMEM)
goto out_locked;

if (!pathbuf) /* ima_rdwr_violation possibly pre-fetched */
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index eea6e92500b8..390a8faa77f9 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -1023,6 +1023,7 @@ enum policy_opt {
Opt_fowner_gt, Opt_fgroup_gt,
Opt_uid_lt, Opt_euid_lt, Opt_gid_lt, Opt_egid_lt,
Opt_fowner_lt, Opt_fgroup_lt,
+ Opt_digest_type,
Opt_appraise_type, Opt_appraise_flag, Opt_appraise_algos,
Opt_permit_directio, Opt_pcr, Opt_template, Opt_keyrings,
Opt_label, Opt_err
@@ -1065,6 +1066,7 @@ static const match_table_t policy_tokens = {
{Opt_egid_lt, "egid<%s"},
{Opt_fowner_lt, "fowner<%s"},
{Opt_fgroup_lt, "fgroup<%s"},
+ {Opt_digest_type, "digest_type=%s"},
{Opt_appraise_type, "appraise_type=%s"},
{Opt_appraise_flag, "appraise_flag=%s"},
{Opt_appraise_algos, "appraise_algos=%s"},
@@ -1172,6 +1174,21 @@ static void check_template_modsig(const struct ima_template_desc *template)
#undef MSG
}

+/*
+ * Warn if the template does not contain the given field.
+ */
+static void check_template_field(const struct ima_template_desc *template,
+ const char *field, const char *msg)
+{
+ int i;
+
+ for (i = 0; i < template->num_fields; i++)
+ if (!strcmp(template->fields[i]->field_id, field))
+ return;
+
+ pr_notice_once("%s", msg);
+}
+
static bool ima_validate_rule(struct ima_rule_entry *entry)
{
/* Ensure that the action is set and is compatible with the flags */
@@ -1214,7 +1231,8 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
IMA_INMASK | IMA_EUID | IMA_PCR |
IMA_FSNAME | IMA_GID | IMA_EGID |
IMA_FGROUP | IMA_DIGSIG_REQUIRED |
- IMA_PERMIT_DIRECTIO | IMA_VALIDATE_ALGOS))
+ IMA_PERMIT_DIRECTIO | IMA_VALIDATE_ALGOS |
+ IMA_VERITY_REQUIRED))
return false;

break;
@@ -1707,6 +1725,13 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
LSM_SUBJ_TYPE,
AUDIT_SUBJ_TYPE);
break;
+ case Opt_digest_type:
+ ima_log_string(ab, "digest_type", args[0].from);
+ if ((strcmp(args[0].from, "verity")) == 0)
+ entry->flags |= IMA_VERITY_REQUIRED;
+ else
+ result = -EINVAL;
+ break;
case Opt_appraise_type:
ima_log_string(ab, "appraise_type", args[0].from);
if ((strcmp(args[0].from, "imasig")) == 0)
@@ -1797,6 +1822,15 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
check_template_modsig(template_desc);
}

+ /* d-ngv2 template field recommended for unsigned fs-verity digests */
+ if (!result && entry->action == MEASURE &&
+ entry->flags & IMA_VERITY_REQUIRED) {
+ template_desc = entry->template ? entry->template :
+ ima_template_desc_current();
+ check_template_field(template_desc, "d-ngv2",
+ "verity rules should include d-ngv2");
+ }
+
audit_log_format(ab, "res=%d", !result);
audit_log_end(ab);
return result;
@@ -2154,6 +2188,8 @@ int ima_policy_show(struct seq_file *m, void *v)
else
seq_puts(m, "appraise_type=imasig ");
}
+ if (entry->flags & IMA_VERITY_REQUIRED)
+ seq_puts(m, "digest_type=verity ");
if (entry->flags & IMA_CHECK_BLACKLIST)
seq_puts(m, "appraise_flag=check_blacklist ");
if (entry->flags & IMA_PERMIT_DIRECTIO)
diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
index ff82e699149c..2ebcf6cd92b8 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -32,12 +32,14 @@ enum data_formats {

enum digest_type {
DIGEST_TYPE_IMA,
+ DIGEST_TYPE_VERITY,
DIGEST_TYPE__LAST
};

-#define DIGEST_TYPE_NAME_LEN_MAX 4 /* including NULL */
+#define DIGEST_TYPE_NAME_LEN_MAX 7 /* including NULL */
static const char * const digest_type_name[DIGEST_TYPE__LAST] = {
- [DIGEST_TYPE_IMA] = "ima"
+ [DIGEST_TYPE_IMA] = "ima",
+ [DIGEST_TYPE_VERITY] = "verity"
};

static int ima_write_template_field_data(const void *data, const u32 datalen,
@@ -297,7 +299,7 @@ static int ima_eventdigest_init_common(const u8 *digest, u32 digestsize,
*
* where 'DATA_FMT_DIGEST' is the original digest format ('d')
* with a hash size limitation of 20 bytes,
- * where <digest type> is "ima",
+ * where <digest type> is either "ima" or "verity",
* where <hash algo> is the hash_algo_name[] string.
*/
u8 buffer[DIGEST_TYPE_NAME_LEN_MAX + CRYPTO_MAX_ALG_NAME + 2 +
@@ -435,6 +437,8 @@ int ima_eventdigest_ngv2_init(struct ima_event_data *event_data,
cur_digestsize = event_data->iint->ima_hash->length;

hash_algo = event_data->iint->ima_hash->algo;
+ if (event_data->iint->flags & IMA_VERITY_REQUIRED)
+ digest_type = DIGEST_TYPE_VERITY;
out:
return ima_eventdigest_init_common(cur_digest, cur_digestsize,
digest_type, hash_algo,
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 3510e413ea17..04e2b99cd912 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -40,6 +40,7 @@
#define IMA_FAIL_UNVERIFIABLE_SIGS 0x10000000
#define IMA_MODSIG_ALLOWED 0x20000000
#define IMA_CHECK_BLACKLIST 0x40000000
+#define IMA_VERITY_REQUIRED 0x80000000

#define IMA_DO_MASK (IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \
IMA_HASH | IMA_APPRAISE_SUBMASK)
--
2.27.0

2022-04-30 20:52:07

by Mimi Zohar

[permalink] [raw]
Subject: [PATCH v8 4/7] ima: define a new template field named 'd-ngv2' and templates

In preparation to differentiate between unsigned regular IMA file
hashes and fs-verity's file digests in the IMA measurement list,
define a new template field named 'd-ngv2'.

Also define two new templates named 'ima-ngv2' and 'ima-sigv2', which
include the new 'd-ngv2' field.

Signed-off-by: Mimi Zohar <[email protected]>
---
.../admin-guide/kernel-parameters.txt | 3 +-
Documentation/security/IMA-templates.rst | 4 +
security/integrity/ima/ima_template.c | 4 +
security/integrity/ima/ima_template_lib.c | 79 ++++++++++++++++---
security/integrity/ima/ima_template_lib.h | 4 +
5 files changed, 82 insertions(+), 12 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 3f1cc5e317ed..5e866be89f5d 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1903,7 +1903,8 @@

ima_template= [IMA]
Select one of defined IMA measurements template formats.
- Formats: { "ima" | "ima-ng" | "ima-sig" }
+ Formats: { "ima" | "ima-ng" | "ima-ngv2" | "ima-sig" |
+ "ima-sigv2" }
Default: "ima-ng"

ima_template_fmt=
diff --git a/Documentation/security/IMA-templates.rst b/Documentation/security/IMA-templates.rst
index cab97f49971d..eafc4e34f890 100644
--- a/Documentation/security/IMA-templates.rst
+++ b/Documentation/security/IMA-templates.rst
@@ -67,6 +67,8 @@ descriptors by adding their identifier to the format string
- 'n': the name of the event (i.e. the file name), with size up to 255 bytes;
- 'd-ng': the digest of the event, calculated with an arbitrary hash
algorithm (field format: <hash algo>:digest);
+ - 'd-ngv2': same as d-ng, but prefixed with the "ima" digest type
+ (field format: <digest type>:<hash algo>:digest);
- 'd-modsig': the digest of the event without the appended modsig;
- 'n-ng': the name of the event, without size limitations;
- 'sig': the file signature, or the EVM portable signature if the file
@@ -87,7 +89,9 @@ Below, there is the list of defined template descriptors:

- "ima": its format is ``d|n``;
- "ima-ng" (default): its format is ``d-ng|n-ng``;
+ - "ima-ngv2": its format is ``d-ngv2|n-ng``;
- "ima-sig": its format is ``d-ng|n-ng|sig``;
+ - "ima-sigv2": its format is ``d-ngv2|n-ng|sig``;
- "ima-buf": its format is ``d-ng|n-ng|buf``;
- "ima-modsig": its format is ``d-ng|n-ng|sig|d-modsig|modsig``;
- "evm-sig": its format is ``d-ng|n-ng|evmsig|xattrnames|xattrlengths|xattrvalues|iuid|igid|imode``;
diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index db1ad6d7a57f..c25079faa208 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -20,6 +20,8 @@ static struct ima_template_desc builtin_templates[] = {
{.name = IMA_TEMPLATE_IMA_NAME, .fmt = IMA_TEMPLATE_IMA_FMT},
{.name = "ima-ng", .fmt = "d-ng|n-ng"},
{.name = "ima-sig", .fmt = "d-ng|n-ng|sig"},
+ {.name = "ima-ngv2", .fmt = "d-ngv2|n-ng"},
+ {.name = "ima-sigv2", .fmt = "d-ngv2|n-ng|sig"},
{.name = "ima-buf", .fmt = "d-ng|n-ng|buf"},
{.name = "ima-modsig", .fmt = "d-ng|n-ng|sig|d-modsig|modsig"},
{.name = "evm-sig",
@@ -38,6 +40,8 @@ static const struct ima_template_field supported_fields[] = {
.field_show = ima_show_template_string},
{.field_id = "d-ng", .field_init = ima_eventdigest_ng_init,
.field_show = ima_show_template_digest_ng},
+ {.field_id = "d-ngv2", .field_init = ima_eventdigest_ngv2_init,
+ .field_show = ima_show_template_digest_ngv2},
{.field_id = "n-ng", .field_init = ima_eventname_ng_init,
.field_show = ima_show_template_string},
{.field_id = "sig", .field_init = ima_eventsig_init,
diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
index 4b6706f864d4..ff82e699149c 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -24,11 +24,22 @@ static bool ima_template_hash_algo_allowed(u8 algo)
enum data_formats {
DATA_FMT_DIGEST = 0,
DATA_FMT_DIGEST_WITH_ALGO,
+ DATA_FMT_DIGEST_WITH_TYPE_AND_ALGO,
DATA_FMT_STRING,
DATA_FMT_HEX,
DATA_FMT_UINT
};

+enum digest_type {
+ DIGEST_TYPE_IMA,
+ DIGEST_TYPE__LAST
+};
+
+#define DIGEST_TYPE_NAME_LEN_MAX 4 /* including NULL */
+static const char * const digest_type_name[DIGEST_TYPE__LAST] = {
+ [DIGEST_TYPE_IMA] = "ima"
+};
+
static int ima_write_template_field_data(const void *data, const u32 datalen,
enum data_formats datafmt,
struct ima_field_data *field_data)
@@ -72,8 +83,9 @@ static void ima_show_template_data_ascii(struct seq_file *m,
u32 buflen = field_data->len;

switch (datafmt) {
+ case DATA_FMT_DIGEST_WITH_TYPE_AND_ALGO:
case DATA_FMT_DIGEST_WITH_ALGO:
- buf_ptr = strnchr(field_data->data, buflen, ':');
+ buf_ptr = strrchr(field_data->data, ':');
if (buf_ptr != field_data->data)
seq_printf(m, "%s", field_data->data);

@@ -178,6 +190,14 @@ void ima_show_template_digest_ng(struct seq_file *m, enum ima_show_type show,
field_data);
}

+void ima_show_template_digest_ngv2(struct seq_file *m, enum ima_show_type show,
+ struct ima_field_data *field_data)
+{
+ ima_show_template_field_data(m, show,
+ DATA_FMT_DIGEST_WITH_TYPE_AND_ALGO,
+ field_data);
+}
+
void ima_show_template_string(struct seq_file *m, enum ima_show_type show,
struct ima_field_data *field_data)
{
@@ -265,28 +285,38 @@ int ima_parse_buf(void *bufstartp, void *bufendp, void **bufcurp,
}

static int ima_eventdigest_init_common(const u8 *digest, u32 digestsize,
- u8 hash_algo,
+ u8 digest_type, u8 hash_algo,
struct ima_field_data *field_data)
{
/*
* digest formats:
* - DATA_FMT_DIGEST: digest
* - DATA_FMT_DIGEST_WITH_ALGO: <hash algo> + ':' + '\0' + digest,
+ * - DATA_FMT_DIGEST_WITH_TYPE_AND_ALGO:
+ * <digest type> + ':' + <hash algo> + ':' + '\0' + digest,
*
* where 'DATA_FMT_DIGEST' is the original digest format ('d')
* with a hash size limitation of 20 bytes,
+ * where <digest type> is "ima",
* where <hash algo> is the hash_algo_name[] string.
*/
- u8 buffer[CRYPTO_MAX_ALG_NAME + 2 + IMA_MAX_DIGEST_SIZE] = { 0 };
+ u8 buffer[DIGEST_TYPE_NAME_LEN_MAX + CRYPTO_MAX_ALG_NAME + 2 +
+ IMA_MAX_DIGEST_SIZE] = { 0 };
enum data_formats fmt = DATA_FMT_DIGEST;
u32 offset = 0;

- if (hash_algo < HASH_ALGO__LAST) {
+ if (digest_type < DIGEST_TYPE__LAST && hash_algo < HASH_ALGO__LAST) {
+ fmt = DATA_FMT_DIGEST_WITH_TYPE_AND_ALGO;
+ offset += 1 + sprintf(buffer, "%*s:%*s:",
+ (int)strlen(digest_type_name[digest_type]),
+ digest_type_name[digest_type],
+ (int)strlen(hash_algo_name[hash_algo]),
+ hash_algo_name[hash_algo]);
+ } else if (hash_algo < HASH_ALGO__LAST) {
fmt = DATA_FMT_DIGEST_WITH_ALGO;
- offset += snprintf(buffer, CRYPTO_MAX_ALG_NAME + 1, "%s",
- hash_algo_name[hash_algo]);
- buffer[offset] = ':';
- offset += 2;
+ offset += 1 + sprintf(buffer, "%*s:",
+ (int)strlen(hash_algo_name[hash_algo]),
+ hash_algo_name[hash_algo]);
}

if (digest)
@@ -361,7 +391,8 @@ int ima_eventdigest_init(struct ima_event_data *event_data,
cur_digestsize = hash.hdr.length;
out:
return ima_eventdigest_init_common(cur_digest, cur_digestsize,
- HASH_ALGO__LAST, field_data);
+ DIGEST_TYPE__LAST, HASH_ALGO__LAST,
+ field_data);
}

/*
@@ -382,7 +413,32 @@ int ima_eventdigest_ng_init(struct ima_event_data *event_data,
hash_algo = event_data->iint->ima_hash->algo;
out:
return ima_eventdigest_init_common(cur_digest, cur_digestsize,
- hash_algo, field_data);
+ DIGEST_TYPE__LAST, hash_algo,
+ field_data);
+}
+
+/*
+ * This function writes the digest of an event (without size limit),
+ * prefixed with both the digest type and hash algorithm.
+ */
+int ima_eventdigest_ngv2_init(struct ima_event_data *event_data,
+ struct ima_field_data *field_data)
+{
+ u8 *cur_digest = NULL, hash_algo = ima_hash_algo;
+ u32 cur_digestsize = 0;
+ u8 digest_type = DIGEST_TYPE_IMA;
+
+ if (event_data->violation) /* recording a violation. */
+ goto out;
+
+ cur_digest = event_data->iint->ima_hash->digest;
+ cur_digestsize = event_data->iint->ima_hash->length;
+
+ hash_algo = event_data->iint->ima_hash->algo;
+out:
+ return ima_eventdigest_init_common(cur_digest, cur_digestsize,
+ digest_type, hash_algo,
+ field_data);
}

/*
@@ -417,7 +473,8 @@ int ima_eventdigest_modsig_init(struct ima_event_data *event_data,
}

return ima_eventdigest_init_common(cur_digest, cur_digestsize,
- hash_algo, field_data);
+ DIGEST_TYPE__LAST, hash_algo,
+ field_data);
}

static int ima_eventname_init_common(struct ima_event_data *event_data,
diff --git a/security/integrity/ima/ima_template_lib.h b/security/integrity/ima/ima_template_lib.h
index c71f1de95753..9f7c335f304f 100644
--- a/security/integrity/ima/ima_template_lib.h
+++ b/security/integrity/ima/ima_template_lib.h
@@ -21,6 +21,8 @@ void ima_show_template_digest(struct seq_file *m, enum ima_show_type show,
struct ima_field_data *field_data);
void ima_show_template_digest_ng(struct seq_file *m, enum ima_show_type show,
struct ima_field_data *field_data);
+void ima_show_template_digest_ngv2(struct seq_file *m, enum ima_show_type show,
+ struct ima_field_data *field_data);
void ima_show_template_string(struct seq_file *m, enum ima_show_type show,
struct ima_field_data *field_data);
void ima_show_template_sig(struct seq_file *m, enum ima_show_type show,
@@ -38,6 +40,8 @@ int ima_eventname_init(struct ima_event_data *event_data,
struct ima_field_data *field_data);
int ima_eventdigest_ng_init(struct ima_event_data *event_data,
struct ima_field_data *field_data);
+int ima_eventdigest_ngv2_init(struct ima_event_data *event_data,
+ struct ima_field_data *field_data);
int ima_eventdigest_modsig_init(struct ima_event_data *event_data,
struct ima_field_data *field_data);
int ima_eventname_ng_init(struct ima_event_data *event_data,
--
2.27.0

2022-05-01 09:02:52

by Stefan Berger

[permalink] [raw]
Subject: Re: [PATCH v8 4/7] ima: define a new template field named 'd-ngv2' and templates



On 4/29/22 07:25, Mimi Zohar wrote:
> In preparation to differentiate between unsigned regular IMA file
> hashes and fs-verity's file digests in the IMA measurement list,
> define a new template field named 'd-ngv2'.
>
> Also define two new templates named 'ima-ngv2' and 'ima-sigv2', which
> include the new 'd-ngv2' field.
>
> Signed-off-by: Mimi Zohar <[email protected]>
> ---
> .../admin-guide/kernel-parameters.txt | 3 +-
> Documentation/security/IMA-templates.rst | 4 +
> security/integrity/ima/ima_template.c | 4 +
> security/integrity/ima/ima_template_lib.c | 79 ++++++++++++++++---
> security/integrity/ima/ima_template_lib.h | 4 +
> 5 files changed, 82 insertions(+), 12 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 3f1cc5e317ed..5e866be89f5d 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -1903,7 +1903,8 @@
>
> ima_template= [IMA]
> Select one of defined IMA measurements template formats.
> - Formats: { "ima" | "ima-ng" | "ima-sig" }
> + Formats: { "ima" | "ima-ng" | "ima-ngv2" | "ima-sig" |
> + "ima-sigv2" }
> Default: "ima-ng"
>
> ima_template_fmt=
> diff --git a/Documentation/security/IMA-templates.rst b/Documentation/security/IMA-templates.rst
> index cab97f49971d..eafc4e34f890 100644
> --- a/Documentation/security/IMA-templates.rst
> +++ b/Documentation/security/IMA-templates.rst
> @@ -67,6 +67,8 @@ descriptors by adding their identifier to the format string
> - 'n': the name of the event (i.e. the file name), with size up to 255 bytes;
> - 'd-ng': the digest of the event, calculated with an arbitrary hash
> algorithm (field format: <hash algo>:digest);
> + - 'd-ngv2': same as d-ng, but prefixed with the "ima" digest type
> + (field format: <digest type>:<hash algo>:digest);
> - 'd-modsig': the digest of the event without the appended modsig;
> - 'n-ng': the name of the event, without size limitations;
> - 'sig': the file signature, or the EVM portable signature if the file
> @@ -87,7 +89,9 @@ Below, there is the list of defined template descriptors:
>
> - "ima": its format is ``d|n``;
> - "ima-ng" (default): its format is ``d-ng|n-ng``;
> + - "ima-ngv2": its format is ``d-ngv2|n-ng``;
> - "ima-sig": its format is ``d-ng|n-ng|sig``;
> + - "ima-sigv2": its format is ``d-ngv2|n-ng|sig``;
> - "ima-buf": its format is ``d-ng|n-ng|buf``;
> - "ima-modsig": its format is ``d-ng|n-ng|sig|d-modsig|modsig``;
> - "evm-sig": its format is ``d-ng|n-ng|evmsig|xattrnames|xattrlengths|xattrvalues|iuid|igid|imode``;
> diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
> index db1ad6d7a57f..c25079faa208 100644
> --- a/security/integrity/ima/ima_template.c
> +++ b/security/integrity/ima/ima_template.c
> @@ -20,6 +20,8 @@ static struct ima_template_desc builtin_templates[] = {
> {.name = IMA_TEMPLATE_IMA_NAME, .fmt = IMA_TEMPLATE_IMA_FMT},
> {.name = "ima-ng", .fmt = "d-ng|n-ng"},
> {.name = "ima-sig", .fmt = "d-ng|n-ng|sig"},
> + {.name = "ima-ngv2", .fmt = "d-ngv2|n-ng"},
> + {.name = "ima-sigv2", .fmt = "d-ngv2|n-ng|sig"},
> {.name = "ima-buf", .fmt = "d-ng|n-ng|buf"},
> {.name = "ima-modsig", .fmt = "d-ng|n-ng|sig|d-modsig|modsig"},
> {.name = "evm-sig",
> @@ -38,6 +40,8 @@ static const struct ima_template_field supported_fields[] = {
> .field_show = ima_show_template_string},
> {.field_id = "d-ng", .field_init = ima_eventdigest_ng_init,
> .field_show = ima_show_template_digest_ng},
> + {.field_id = "d-ngv2", .field_init = ima_eventdigest_ngv2_init,
> + .field_show = ima_show_template_digest_ngv2},
> {.field_id = "n-ng", .field_init = ima_eventname_ng_init,
> .field_show = ima_show_template_string},
> {.field_id = "sig", .field_init = ima_eventsig_init,
> diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
> index 4b6706f864d4..ff82e699149c 100644
> --- a/security/integrity/ima/ima_template_lib.c
> +++ b/security/integrity/ima/ima_template_lib.c
> @@ -24,11 +24,22 @@ static bool ima_template_hash_algo_allowed(u8 algo)
> enum data_formats {
> DATA_FMT_DIGEST = 0,
> DATA_FMT_DIGEST_WITH_ALGO,
> + DATA_FMT_DIGEST_WITH_TYPE_AND_ALGO,
> DATA_FMT_STRING,
> DATA_FMT_HEX,
> DATA_FMT_UINT
> };
>
> +enum digest_type {
> + DIGEST_TYPE_IMA,
> + DIGEST_TYPE__LAST
> +};
> +
> +#define DIGEST_TYPE_NAME_LEN_MAX 4 /* including NULL */

You probably mean 'NUL' ('\0') here:
https://man7.org/linux/man-pages/man7/ascii.7.html

> +static const char * const digest_type_name[DIGEST_TYPE__LAST] = {
> + [DIGEST_TYPE_IMA] = "ima"
> +};
> +
> static int ima_write_template_field_data(const void *data, const u32 datalen,
> enum data_formats datafmt,
> struct ima_field_data *field_data)
> @@ -72,8 +83,9 @@ static void ima_show_template_data_ascii(struct seq_file *m,
> u32 buflen = field_data->len;
>
> switch (datafmt) {
> + case DATA_FMT_DIGEST_WITH_TYPE_AND_ALGO:
> case DATA_FMT_DIGEST_WITH_ALGO:
> - buf_ptr = strnchr(field_data->data, buflen, ':');
> + buf_ptr = strrchr(field_data->data, ':');
> if (buf_ptr != field_data->data)
> seq_printf(m, "%s", field_data->data);
>
> @@ -178,6 +190,14 @@ void ima_show_template_digest_ng(struct seq_file *m, enum ima_show_type show,
> field_data);
> }
>
> +void ima_show_template_digest_ngv2(struct seq_file *m, enum ima_show_type show,
> + struct ima_field_data *field_data)
> +{
> + ima_show_template_field_data(m, show,
> + DATA_FMT_DIGEST_WITH_TYPE_AND_ALGO,
> + field_data);
> +}
> +
> void ima_show_template_string(struct seq_file *m, enum ima_show_type show,
> struct ima_field_data *field_data)
> {
> @@ -265,28 +285,38 @@ int ima_parse_buf(void *bufstartp, void *bufendp, void **bufcurp,
> }
>
> static int ima_eventdigest_init_common(const u8 *digest, u32 digestsize,
> - u8 hash_algo,
> + u8 digest_type, u8 hash_algo,
> struct ima_field_data *field_data)
> {
> /*
> * digest formats:
> * - DATA_FMT_DIGEST: digest
> * - DATA_FMT_DIGEST_WITH_ALGO: <hash algo> + ':' + '\0' + digest,
> + * - DATA_FMT_DIGEST_WITH_TYPE_AND_ALGO:
> + * <digest type> + ':' + <hash algo> + ':' + '\0' + digest,
> *
> * where 'DATA_FMT_DIGEST' is the original digest format ('d')
> * with a hash size limitation of 20 bytes,
> + * where <digest type> is "ima",
> * where <hash algo> is the hash_algo_name[] string.
> */
> - u8 buffer[CRYPTO_MAX_ALG_NAME + 2 + IMA_MAX_DIGEST_SIZE] = { 0 };
> + u8 buffer[DIGEST_TYPE_NAME_LEN_MAX + CRYPTO_MAX_ALG_NAME + 2 +
> + IMA_MAX_DIGEST_SIZE] = { 0 };
> enum data_formats fmt = DATA_FMT_DIGEST;
> u32 offset = 0;
>
> - if (hash_algo < HASH_ALGO__LAST) {
> + if (digest_type < DIGEST_TYPE__LAST && hash_algo < HASH_ALGO__LAST) {
> + fmt = DATA_FMT_DIGEST_WITH_TYPE_AND_ALGO;
> + offset += 1 + sprintf(buffer, "%*s:%*s:",
> + (int)strlen(digest_type_name[digest_type]),
> + digest_type_name[digest_type],
> + (int)strlen(hash_algo_name[hash_algo]),
> + hash_algo_name[hash_algo]);

'%*s' seems to be for right-alignment but only makes sense if the length
indicator is different than then following string. sprintf(buffer,
"|%*s|",5,"test") prints | test|. Otherwise it seems to behave like
plain '%s' in this case... ?

2022-05-02 04:12:52

by Stefan Berger

[permalink] [raw]
Subject: Re: [PATCH v8 7/7] fsverity: update the documentation



On 4/29/22 07:26, Mimi Zohar wrote:
> Update the fsverity documentation related to IMA signature support.
>
> Signed-off-by: Mimi Zohar <[email protected]>
> ---
> Documentation/filesystems/fsverity.rst | 35 +++++++++++++++++---------
> 1 file changed, 23 insertions(+), 12 deletions(-)
>
> diff --git a/Documentation/filesystems/fsverity.rst b/Documentation/filesystems/fsverity.rst
> index 8cc536d08f51..b7d42fd65e9d 100644
> --- a/Documentation/filesystems/fsverity.rst
> +++ b/Documentation/filesystems/fsverity.rst
> @@ -70,12 +70,23 @@ must live on a read-write filesystem because they are independently
> updated and potentially user-installed, so dm-verity cannot be used.
>
> The base fs-verity feature is a hashing mechanism only; actually
> -authenticating the files is up to userspace. However, to meet some
> -users' needs, fs-verity optionally supports a simple signature
> -verification mechanism where users can configure the kernel to require
> -that all fs-verity files be signed by a key loaded into a keyring; see
> -`Built-in signature verification`_. Support for fs-verity file hashes
> -in IMA (Integrity Measurement Architecture) policies is also planned.
> +authenticating the files may be done by:
> +
> +* Userspace-only
> +
> +* Builtin signature verification + userspace policy
> +
> + fs-verity optionally supports a simple signature verification
> + mechanism where users can configure the kernel to require that
> + all fs-verity files be signed by a key loaded into a keyring;
> + see `Built-in signature verification`_.
> +
> +* Integrity Measurement Architecture (IMA)
> +
> + IMA supports including fs-verity file digests and signatures in the
> + IMA measurement list and verifying fs-verity based file signatures
> + stored as security.ima xattrs, based on policy.
> +
>
> User API
> ========
> @@ -653,12 +664,12 @@ weren't already directly answered in other parts of this document.
> hashed and what to do with those hashes, such as log them,
> authenticate them, or add them to a measurement list.
>
> - IMA is planned to support the fs-verity hashing mechanism as an
> - alternative to doing full file hashes, for people who want the
> - performance and security benefits of the Merkle tree based hash.
> - But it doesn't make sense to force all uses of fs-verity to be
> - through IMA. As a standalone filesystem feature, fs-verity
> - already meets many users' needs, and it's testable like other
> + IMA supports the fs-verity hashing mechanism as an alternative
> + to full file hashes, for those who want the performance and
> + security benefits of the Merkle tree based hash. However, it
> + doesn't make sense to force all uses of fs-verity to be through
> + IMA. fs-verity already meets many users' needs even as a
> + standalone filesystem feature, and it's testable like other
> filesystem features e.g. with xfstests.
>
> :Q: Isn't fs-verity useless because the attacker can just modify the

Acked-by: Stefan Berger <[email protected]>

2022-05-02 11:21:53

by Mimi Zohar

[permalink] [raw]
Subject: [PATCH v8 2/7] ima: use IMA default hash algorithm for integrity violations

Integrity file violations - ToM/ToU, open writers - are recorded in the IMA
measurement list, containing 0x00's in both the template data and file data
hash fields, but 0xFF's are actually extended into TPM PCRs. Although the
original 'ima' template data field ('d') is limited to 20 bytes, the 'd-ng'
template digest field is not.

The violation file data hash template field ('d-ng') is unnecessarily hard
coded to SHA1. Instead of simply replacing the hard coded SHA1 hash
algorithm with a larger hash algorithm, use the hash algorithm as defined
in "ima_hash_algo". ima_hash_algo is set to either the Kconfig IMA default
hash algorithm or as defined on the boot command line (ima_hash=).

Including a non-SHA1 file data hash algorithm in the 'd-ng' field of
violations is a cosmetic change. The template data hash field, which is
extended into the TPM PCRs, is not affected by this change and should not
affect attestation of the IMA measurement list.

Signed-off-by: Mimi Zohar <[email protected]>
---
security/integrity/ima/ima_template_lib.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
index e9d65f6fe2ae..4b6706f864d4 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -370,7 +370,7 @@ int ima_eventdigest_init(struct ima_event_data *event_data,
int ima_eventdigest_ng_init(struct ima_event_data *event_data,
struct ima_field_data *field_data)
{
- u8 *cur_digest = NULL, hash_algo = HASH_ALGO_SHA1;
+ u8 *cur_digest = NULL, hash_algo = ima_hash_algo;
u32 cur_digestsize = 0;

if (event_data->violation) /* recording a violation. */
--
2.27.0

2022-05-02 15:51:40

by Stefan Berger

[permalink] [raw]
Subject: Re: [PATCH v8 2/7] ima: use IMA default hash algorithm for integrity violations



On 4/29/22 07:25, Mimi Zohar wrote:
> Integrity file violations - ToM/ToU, open writers - are recorded in the IMA
> measurement list, containing 0x00's in both the template data and file data
> hash fields, but 0xFF's are actually extended into TPM PCRs. Although the
> original 'ima' template data field ('d') is limited to 20 bytes, the 'd-ng'
> template digest field is not.
>
> The violation file data hash template field ('d-ng') is unnecessarily hard
> coded to SHA1. Instead of simply replacing the hard coded SHA1 hash
> algorithm with a larger hash algorithm, use the hash algorithm as defined
> in "ima_hash_algo". ima_hash_algo is set to either the Kconfig IMA default
> hash algorithm or as defined on the boot command line (ima_hash=).
>
> Including a non-SHA1 file data hash algorithm in the 'd-ng' field of
> violations is a cosmetic change. The template data hash field, which is
> extended into the TPM PCRs, is not affected by this change and should not
> affect attestation of the IMA measurement list.
>
> Signed-off-by: Mimi Zohar <[email protected]>

The effect seems to be that violations that have previously looked like
this

10 0000000000000000000000000000000000000000 ima-ng
sha1:0000000000000000000000000000000000000000 /var/log/audit/audit.log

now look like this:

10 0000000000000000000000000000000000000000 ima-ng
sha256:0000000000000000000000000000000000000000 /var/log/audit/audit.log

evmctl [1.3.2] still works fine:

# evmctl ima_measurement --ignore-violations
/sys/kernel/security/ima/binary_runtime_measurements
Matched per TPM bank calculated digest(s).


Tested-by: Stefan Berger <[email protected]>


> ---
> security/integrity/ima/ima_template_lib.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
> index e9d65f6fe2ae..4b6706f864d4 100644
> --- a/security/integrity/ima/ima_template_lib.c
> +++ b/security/integrity/ima/ima_template_lib.c
> @@ -370,7 +370,7 @@ int ima_eventdigest_init(struct ima_event_data *event_data,
> int ima_eventdigest_ng_init(struct ima_event_data *event_data,
> struct ima_field_data *field_data)
> {
> - u8 *cur_digest = NULL, hash_algo = HASH_ALGO_SHA1;
> + u8 *cur_digest = NULL, hash_algo = ima_hash_algo;
> u32 cur_digestsize = 0;
>
> if (event_data->violation) /* recording a violation. */

2022-05-02 23:06:39

by Stefan Berger

[permalink] [raw]
Subject: Re: [PATCH v8 5/7] ima: permit fsverity's file digests in the IMA measurement list



On 4/29/22 07:25, Mimi Zohar wrote:
> Permit fsverity's file digest (a hash of struct fsverity_descriptor) to
> be included in the IMA measurement list, based on the new measurement
> policy rule 'digest_type=verity' option.
>
> To differentiate between a regular IMA file hash from an fsverity's
> file digest, use the new d-ngv2 format field included in the ima-ngv2
> template.
>
> The following policy rule requires fsverity file digests and specifies
> the new 'ima-ngv2' template, which contains the new 'd-ngv2' field. The
> policy rule may be constrained, for example based on a fsuuid or LSM
> label.
>
> measure func=FILE_CHECK digest_type=verity template=ima-ngv2
>
> Signed-off-by: Mimi Zohar <[email protected]>
> ---
> Documentation/ABI/testing/ima_policy | 14 ++++++-
> Documentation/security/IMA-templates.rst | 2 +-
> security/integrity/ima/ima_api.c | 49 +++++++++++++++++++++--
> security/integrity/ima/ima_main.c | 2 +-
> security/integrity/ima/ima_policy.c | 38 +++++++++++++++++-
> security/integrity/ima/ima_template_lib.c | 10 +++--
> security/integrity/integrity.h | 1 +
> 7 files changed, 105 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
> index 839fab811b18..0a8caed393e3 100644
> --- a/Documentation/ABI/testing/ima_policy
> +++ b/Documentation/ABI/testing/ima_policy
> @@ -27,8 +27,9 @@ Description:
> [fowner=] [fgroup=]]
> lsm: [[subj_user=] [subj_role=] [subj_type=]
> [obj_user=] [obj_role=] [obj_type=]]
> - option: [[appraise_type=]] [template=] [permit_directio]
> - [appraise_flag=] [appraise_algos=] [keyrings=]
> + option: [digest_type=] [template=] [permit_directio]
> + [appraise_type=] [appraise_flag=]
> + [appraise_algos=] [keyrings=]
> base:
> func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK]
> [FIRMWARE_CHECK]
> @@ -51,6 +52,9 @@ Description:
> appraise_flag:= [check_blacklist]
> Currently, blacklist check is only for files signed with appended
> signature.
> + digest_type:= verity
> + Require fs-verity's file digest instead of the
> + regular IMA file hash.
> keyrings:= list of keyrings
> (eg, .builtin_trusted_keys|.ima). Only valid
> when action is "measure" and func is KEY_CHECK.
> @@ -149,3 +153,9 @@ Description:
> security.ima xattr of a file:
>
> appraise func=SETXATTR_CHECK appraise_algos=sha256,sha384,sha512
> +
> + Example of a 'measure' rule requiring fs-verity's digests
> + with indication of type of digest in the measurement list.
> +
> + measure func=FILE_CHECK digest_type=verity \
> + template=ima-ngv2
> diff --git a/Documentation/security/IMA-templates.rst b/Documentation/security/IMA-templates.rst
> index eafc4e34f890..09b5fac38195 100644
> --- a/Documentation/security/IMA-templates.rst
> +++ b/Documentation/security/IMA-templates.rst
> @@ -67,7 +67,7 @@ descriptors by adding their identifier to the format string
> - 'n': the name of the event (i.e. the file name), with size up to 255 bytes;
> - 'd-ng': the digest of the event, calculated with an arbitrary hash
> algorithm (field format: <hash algo>:digest);
> - - 'd-ngv2': same as d-ng, but prefixed with the "ima" digest type
> + - 'd-ngv2': same as d-ng, but prefixed with the "ima" or "verity" digest type
> (field format: <digest type>:<hash algo>:digest);
> - 'd-modsig': the digest of the event without the appended modsig;
> - 'n-ng': the name of the event, without size limitations;
> diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> index c6805af46211..d64ec031b1b4 100644
> --- a/security/integrity/ima/ima_api.c
> +++ b/security/integrity/ima/ima_api.c
> @@ -14,6 +14,7 @@
> #include <linux/xattr.h>
> #include <linux/evm.h>
> #include <linux/iversion.h>
> +#include <linux/fsverity.h>
>
> #include "ima.h"
>
> @@ -200,6 +201,34 @@ int ima_get_action(struct user_namespace *mnt_userns, struct inode *inode,
> allowed_algos);
> }
>
> +static int ima_get_verity_digest(struct integrity_iint_cache *iint,
> + struct ima_max_digest_data *hash)
> +{
> + enum hash_algo verity_alg;
> + int ret;
> +
> + /*
> + * On failure, 'measure' policy rules will result in a file data
> + * hash containing 0's.
> + */
> + ret = fsverity_get_digest(iint->inode, hash->digest, &verity_alg);
> + if (ret) {
> + memset(hash->digest, 0, sizeof(hash->digest));
> + return ret;
> + }
> +
> + /*
> + * Unlike in the case of actually calculating the file hash, in
> + * the fsverity case regardless of the hash algorithm, return
> + * the verity digest to be included in the measurement list. A
> + * mismatch between the verity algorithm and the xattr signature
> + * algorithm, if one exists, will be detected later.
> + */
> + hash->hdr.algo = verity_alg;
> + hash->hdr.length = hash_digest_size[verity_alg];
> + return 0;
> +}
> +
> /*
> * ima_collect_measurement - collect file measurement
> *
> @@ -242,16 +271,30 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
> */
> i_version = inode_query_iversion(inode);
> hash.hdr.algo = algo;
> + hash.hdr.length = hash_digest_size[algo];
>
> /* Initialize hash digest to 0's in case of failure */
> memset(&hash.digest, 0, sizeof(hash.digest));

You seem to be doing this above as well in ima_get_verity_digest(). I
guess the above one could go.

>
> - if (buf)
> + if (iint->flags & IMA_VERITY_REQUIRED) {
> + result = ima_get_verity_digest(iint, &hash);
> + switch (result) {
> + case 0:
> + break;
> + case -ENODATA:
> + audit_cause = "no-verity-digest";
> + break;
> + default:
> + audit_cause = "invalid-verity-digest";
> + break;
> + }
> + } else if (buf) {
> result = ima_calc_buffer_hash(buf, size, &hash.hdr);
> - else
> + } else {
> result = ima_calc_file_hash(file, &hash.hdr);
> + }
>
> - if (result && result != -EBADF && result != -EINVAL)
> + if (result == -ENOMEM)
> goto out;
>
> length = sizeof(hash.hdr) + hash.hdr.length;
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 1aebf63ad7a6..040b03ddc1c7 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -335,7 +335,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
> hash_algo = ima_get_hash_algo(xattr_value, xattr_len);
>
> rc = ima_collect_measurement(iint, file, buf, size, hash_algo, modsig);
> - if (rc != 0 && rc != -EBADF && rc != -EINVAL)
> + if (rc == -ENOMEM)
> goto out_locked;
>
> if (!pathbuf) /* ima_rdwr_violation possibly pre-fetched */
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index eea6e92500b8..390a8faa77f9 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -1023,6 +1023,7 @@ enum policy_opt {
> Opt_fowner_gt, Opt_fgroup_gt,
> Opt_uid_lt, Opt_euid_lt, Opt_gid_lt, Opt_egid_lt,
> Opt_fowner_lt, Opt_fgroup_lt,
> + Opt_digest_type,
> Opt_appraise_type, Opt_appraise_flag, Opt_appraise_algos,
> Opt_permit_directio, Opt_pcr, Opt_template, Opt_keyrings,
> Opt_label, Opt_err
> @@ -1065,6 +1066,7 @@ static const match_table_t policy_tokens = {
> {Opt_egid_lt, "egid<%s"},
> {Opt_fowner_lt, "fowner<%s"},
> {Opt_fgroup_lt, "fgroup<%s"},
> + {Opt_digest_type, "digest_type=%s"},
> {Opt_appraise_type, "appraise_type=%s"},
> {Opt_appraise_flag, "appraise_flag=%s"},
> {Opt_appraise_algos, "appraise_algos=%s"},
> @@ -1172,6 +1174,21 @@ static void check_template_modsig(const struct ima_template_desc *template)
> #undef MSG
> }
>
> +/*
> + * Warn if the template does not contain the given field.
> + */
> +static void check_template_field(const struct ima_template_desc *template,
> + const char *field, const char *msg)
> +{
> + int i;
> +
> + for (i = 0; i < template->num_fields; i++)
> + if (!strcmp(template->fields[i]->field_id, field))
> + return;
> +
> + pr_notice_once("%s", msg);
> +}
> +
> static bool ima_validate_rule(struct ima_rule_entry *entry)
> {
> /* Ensure that the action is set and is compatible with the flags */
> @@ -1214,7 +1231,8 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
> IMA_INMASK | IMA_EUID | IMA_PCR |
> IMA_FSNAME | IMA_GID | IMA_EGID |
> IMA_FGROUP | IMA_DIGSIG_REQUIRED |
> - IMA_PERMIT_DIRECTIO | IMA_VALIDATE_ALGOS))
> + IMA_PERMIT_DIRECTIO | IMA_VALIDATE_ALGOS |
> + IMA_VERITY_REQUIRED))
> return false;
>
> break;
> @@ -1707,6 +1725,13 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
> LSM_SUBJ_TYPE,
> AUDIT_SUBJ_TYPE);
> break;
> + case Opt_digest_type:
> + ima_log_string(ab, "digest_type", args[0].from);
> + if ((strcmp(args[0].from, "verity")) == 0)
> + entry->flags |= IMA_VERITY_REQUIRED;
> + else
> + result = -EINVAL;
> + break;
> case Opt_appraise_type:
> ima_log_string(ab, "appraise_type", args[0].from);
> if ((strcmp(args[0].from, "imasig")) == 0)
> @@ -1797,6 +1822,15 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
> check_template_modsig(template_desc);
> }
>
> + /* d-ngv2 template field recommended for unsigned fs-verity digests */
> + if (!result && entry->action == MEASURE &&
> + entry->flags & IMA_VERITY_REQUIRED) {
> + template_desc = entry->template ? entry->template :
> + ima_template_desc_current();
> + check_template_field(template_desc, "d-ngv2",
> + "verity rules should include d-ngv2");
> + }
> +
> audit_log_format(ab, "res=%d", !result);
> audit_log_end(ab);
> return result;
> @@ -2154,6 +2188,8 @@ int ima_policy_show(struct seq_file *m, void *v)
> else
> seq_puts(m, "appraise_type=imasig ");
> }
> + if (entry->flags & IMA_VERITY_REQUIRED)
> + seq_puts(m, "digest_type=verity ");
> if (entry->flags & IMA_CHECK_BLACKLIST)
> seq_puts(m, "appraise_flag=check_blacklist ");
> if (entry->flags & IMA_PERMIT_DIRECTIO)
> diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
> index ff82e699149c..2ebcf6cd92b8 100644
> --- a/security/integrity/ima/ima_template_lib.c
> +++ b/security/integrity/ima/ima_template_lib.c
> @@ -32,12 +32,14 @@ enum data_formats {
>
> enum digest_type {
> DIGEST_TYPE_IMA,
> + DIGEST_TYPE_VERITY,
> DIGEST_TYPE__LAST
> };
>
> -#define DIGEST_TYPE_NAME_LEN_MAX 4 /* including NULL */
> +#define DIGEST_TYPE_NAME_LEN_MAX 7 /* including NULL */
> static const char * const digest_type_name[DIGEST_TYPE__LAST] = {
> - [DIGEST_TYPE_IMA] = "ima"
> + [DIGEST_TYPE_IMA] = "ima",
> + [DIGEST_TYPE_VERITY] = "verity"
> };
>
> static int ima_write_template_field_data(const void *data, const u32 datalen,
> @@ -297,7 +299,7 @@ static int ima_eventdigest_init_common(const u8 *digest, u32 digestsize,
> *
> * where 'DATA_FMT_DIGEST' is the original digest format ('d')
> * with a hash size limitation of 20 bytes,
> - * where <digest type> is "ima",
> + * where <digest type> is either "ima" or "verity",
> * where <hash algo> is the hash_algo_name[] string.
> */
> u8 buffer[DIGEST_TYPE_NAME_LEN_MAX + CRYPTO_MAX_ALG_NAME + 2 +
> @@ -435,6 +437,8 @@ int ima_eventdigest_ngv2_init(struct ima_event_data *event_data,
> cur_digestsize = event_data->iint->ima_hash->length;
>
> hash_algo = event_data->iint->ima_hash->algo;
> + if (event_data->iint->flags & IMA_VERITY_REQUIRED)
> + digest_type = DIGEST_TYPE_VERITY;
> out:
> return ima_eventdigest_init_common(cur_digest, cur_digestsize,
> digest_type, hash_algo,
> diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
> index 3510e413ea17..04e2b99cd912 100644
> --- a/security/integrity/integrity.h
> +++ b/security/integrity/integrity.h
> @@ -40,6 +40,7 @@
> #define IMA_FAIL_UNVERIFIABLE_SIGS 0x10000000
> #define IMA_MODSIG_ALLOWED 0x20000000
> #define IMA_CHECK_BLACKLIST 0x40000000
> +#define IMA_VERITY_REQUIRED 0x80000000
>
> #define IMA_DO_MASK (IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \
> IMA_HASH | IMA_APPRAISE_SUBMASK)

Acked-by: Stefan Berger <[email protected]>

2022-05-02 23:09:43

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH v8 1/7] ima: fix 'd-ng' comments and documentation

On Fri, 2022-04-29 at 12:44 -0400, Stefan Berger wrote:
>
> On 4/29/22 07:25, Mimi Zohar wrote:
> > Initially the 'd-ng' template field did not prefix the digest with either
> > "md5" or "sha1" hash algorithms. Prior to being upstreamed this changed,
> > but the comments and documentation were not updated. Fix the comments
> > and documentation.
> >
> > Fixes: 4d7aeee73f53 ("ima: define new template ima-ng and template fields d-ng and n-ng")
> > Reported-by: Eric Biggers <[email protected]>
> > Signed-off-by: Mimi Zohar <[email protected]>
> > ---
> > Documentation/security/IMA-templates.rst | 3 +--
> > security/integrity/ima/ima_template_lib.c | 8 +++++---
> > 2 files changed, 6 insertions(+), 5 deletions(-)
> >
> > diff --git a/Documentation/security/IMA-templates.rst b/Documentation/security/IMA-templates.rst
> > index 1a91d92950a7..cab97f49971d 100644
> > --- a/Documentation/security/IMA-templates.rst
> > +++ b/Documentation/security/IMA-templates.rst
> > @@ -66,8 +66,7 @@ descriptors by adding their identifier to the format string
> > calculated with the SHA1 or MD5 hash algorithm;
> > - 'n': the name of the event (i.e. the file name), with size up to 255 bytes;
> > - 'd-ng': the digest of the event, calculated with an arbitrary hash
> > - algorithm (field format: [<hash algo>:]digest, where the digest
> > - prefix is shown only if the hash algorithm is not SHA1 or MD5);
>
> That seemed to be true for 'd'

And initially for d-ng before it was upstreamed.

>
> > + algorithm (field format: <hash algo>:digest);
> > - 'd-modsig': the digest of the event without the appended modsig;
> > - 'n-ng': the name of the event, without size limitations;
> > - 'sig': the file signature, or the EVM portable signature if the file
> > diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
> > index 7155d17a3b75..e9d65f6fe2ae 100644
> > --- a/security/integrity/ima/ima_template_lib.c
> > +++ b/security/integrity/ima/ima_template_lib.c
> > @@ -271,9 +271,11 @@ static int ima_eventdigest_init_common(const u8 *digest, u32 digestsize,
> > /*
> > * digest formats:
> > * - DATA_FMT_DIGEST: digest
> > - * - DATA_FMT_DIGEST_WITH_ALGO: [<hash algo>] + ':' + '\0' + digest,
> > - * where <hash algo> is provided if the hash algorithm is not
> > - * SHA1 or MD5
> > + * - DATA_FMT_DIGEST_WITH_ALGO: <hash algo> + ':' + '\0' + digest, > + *
> > + * where 'DATA_FMT_DIGEST' is the original digest format ('d')
> > + * with a hash size limitation of 20 bytes,
> > + * where <hash algo> is the hash_algo_name[] string.
> > */
> > u8 buffer[CRYPTO_MAX_ALG_NAME + 2 + IMA_MAX_DIGEST_SIZE] = { 0 };
> > enum data_formats fmt = DATA_FMT_DIGEST;
>
> Reviewed-by: Stefan Berger <[email protected]>

Thank you, Stefan, for this and the other tags!

Mimi


2022-05-02 23:42:44

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH v8 4/7] ima: define a new template field named 'd-ngv2' and templates

On Fri, 2022-04-29 at 11:09 -0400, Stefan Berger wrote:
> > - if (hash_algo < HASH_ALGO__LAST) {
> > + if (digest_type < DIGEST_TYPE__LAST && hash_algo < HASH_ALGO__LAST) {
> > + fmt = DATA_FMT_DIGEST_WITH_TYPE_AND_ALGO;
> > + offset += 1 + sprintf(buffer, "%*s:%*s:",
> > + (int)strlen(digest_type_name[digest_type]),
> > + digest_type_name[digest_type],
> > + (int)strlen(hash_algo_name[hash_algo]),
> > + hash_algo_name[hash_algo]);
>
> '%*s' seems to be for right-alignment but only makes sense if the length
> indicator is different than then following string. sprintf(buffer,
> "|%*s|",5,"test") prints | test|. Otherwise it seems to behave like
> plain '%s' in this case... ?

Re-testing now, it works properly without the length.

thanks,

Mimi