2022-03-18 19:23:04

by Mimi Zohar

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

Permit fsverity's file digest (a hash of struct fsverity_digest) 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 | 9 ++++++
Documentation/security/IMA-templates.rst | 8 +++++
security/integrity/ima/ima_api.c | 38 +++++++++++++++++++++--
security/integrity/ima/ima_policy.c | 38 ++++++++++++++++++++++-
security/integrity/ima/ima_template_lib.c | 4 ++-
security/integrity/integrity.h | 3 +-
6 files changed, 95 insertions(+), 5 deletions(-)

diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index 839fab811b18..2e0c501ce9c8 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -51,6 +51,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 +152,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 1a91d92950a7..2d4789dc7750 100644
--- a/Documentation/security/IMA-templates.rst
+++ b/Documentation/security/IMA-templates.rst
@@ -68,6 +68,9 @@ descriptors by adding their identifier to the format string
- '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);
+ - 'd-ngv2': same as d-ng, but prefixed with the digest type.
+ field format: [<digest type>:<hash algo>:]digest,
+ where the digest type is either "ima" or "verity".
- '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
@@ -106,3 +109,8 @@ currently the following methods are supported:
the ``ima_template=`` parameter;
- register a new template descriptor with custom format through the kernel
command line parameter ``ima_template_fmt=``.
+
+
+References
+==========
+[1] Documentation/filesystems/fsverity.rst
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index c6805af46211..525b13916745 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,23 @@ 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)
+{
+ u8 verity_digest[FS_VERITY_MAX_DIGEST_SIZE];
+ enum hash_algo verity_alg;
+ int ret;
+
+ ret = fsverity_get_digest(iint->inode, verity_digest, &verity_alg);
+ if (ret)
+ return ret;
+ if (hash->hdr.algo != verity_alg)
+ return -EINVAL;
+ hash->hdr.length = hash_digest_size[verity_alg];
+ memcpy(hash->hdr.digest, verity_digest, hash->hdr.length);
+ return 0;
+}
+
/*
* ima_collect_measurement - collect file measurement
*
@@ -242,14 +260,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 (buf) {
result = ima_calc_buffer_hash(buf, size, &hash.hdr);
- else
+ } else 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";
+ result = -EINVAL;
+ break;
+ case -EINVAL:
+ default:
+ audit_cause = "invalid-verity-digest";
+ break;
+ }
+ } else {
result = ima_calc_file_hash(file, &hash.hdr);
+ }

if (result && result != -EBADF && result != -EINVAL)
goto out;
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index a0f3775cbd82..c6b0454b2e25 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -1024,6 +1024,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
@@ -1066,6 +1067,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"},
@@ -1173,6 +1175,21 @@ static void check_template_modsig(const struct ima_template_desc *template)
#undef MSG
}

+/*
+ * Make sure the policy rule and template format are in sync.
+ */
+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 */
@@ -1215,7 +1232,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;
@@ -1708,6 +1726,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)
@@ -1798,6 +1823,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;
@@ -2155,6 +2189,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 bd95864a5f6f..0cff6658a4c2 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -31,7 +31,7 @@ enum data_formats {
};

#define DIGEST_TYPE_MAXLEN 16 /* including NULL */
-static const char * const digest_type_name[] = {"ima"};
+static const char * const digest_type_name[] = {"ima", "verity"};
static int digest_type_size = ARRAY_SIZE(digest_type_name);

static int ima_write_template_field_data(const void *data, const u32 datalen,
@@ -430,6 +430,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 = 1;
out:
return ima_eventdigest_init_common(cur_digest, cur_digestsize,
digest_type, hash_algo, field_data);
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index daf49894fd7d..d42a01903f08 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -32,7 +32,7 @@
#define IMA_HASHED 0x00000200

/* iint policy rule cache flags */
-#define IMA_NONACTION_FLAGS 0xff000000
+#define IMA_NONACTION_FLAGS 0xff800000
#define IMA_DIGSIG_REQUIRED 0x01000000
#define IMA_PERMIT_DIRECTIO 0x02000000
#define IMA_NEW_FILE 0x04000000
@@ -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-03-21 21:11:31

by Stefan Berger

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



On 3/18/22 14:21, Mimi Zohar wrote:
> Permit fsverity's file digest (a hash of struct fsverity_digest) 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 | 9 ++++++
> Documentation/security/IMA-templates.rst | 8 +++++
> security/integrity/ima/ima_api.c | 38 +++++++++++++++++++++--
> security/integrity/ima/ima_policy.c | 38 ++++++++++++++++++++++-
> security/integrity/ima/ima_template_lib.c | 4 ++-
> security/integrity/integrity.h | 3 +-
> 6 files changed, 95 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
> index 839fab811b18..2e0c501ce9c8 100644
> --- a/Documentation/ABI/testing/ima_policy
> +++ b/Documentation/ABI/testing/ima_policy
> @@ -51,6 +51,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 +152,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 1a91d92950a7..2d4789dc7750 100644
> --- a/Documentation/security/IMA-templates.rst
> +++ b/Documentation/security/IMA-templates.rst
> @@ -68,6 +68,9 @@ descriptors by adding their identifier to the format string
> - '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);
> + - 'd-ngv2': same as d-ng, but prefixed with the digest type.
> + field format: [<digest type>:<hash algo>:]digest,
> + where the digest type is either "ima" or "verity".
> - '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
> @@ -106,3 +109,8 @@ currently the following methods are supported:
> the ``ima_template=`` parameter;
> - register a new template descriptor with custom format through the kernel
> command line parameter ``ima_template_fmt=``.
> +
> +
> +References
> +==========
> +[1] Documentation/filesystems/fsverity.rst
> diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> index c6805af46211..525b13916745 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,23 @@ 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)
> +{
> + u8 verity_digest[FS_VERITY_MAX_DIGEST_SIZE];
> + enum hash_algo verity_alg;
> + int ret;
> +
> + ret = fsverity_get_digest(iint->inode, verity_digest, &verity_alg);
> + if (ret)
> + return ret;
> + if (hash->hdr.algo != verity_alg)
> + return -EINVAL;
> + hash->hdr.length = hash_digest_size[verity_alg];
> + memcpy(hash->hdr.digest, verity_digest, hash->hdr.length);

Could you not pass hash->hdr.digest into fsverity_get_digest() and avoid
the copying here?

> + return 0;
> +}
> +
> /*
> * ima_collect_measurement - collect file measurement
> *
> @@ -242,14 +260,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 (buf) {
> result = ima_calc_buffer_hash(buf, size, &hash.hdr);
> - else
> + } else 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";
> + result = -EINVAL;
> + break;
> + case -EINVAL:
> + default:
> + audit_cause = "invalid-verity-digest";
> + break;
> + }
> + } else {
> result = ima_calc_file_hash(file, &hash.hdr);
> + }
>
> if (result && result != -EBADF && result != -EINVAL)
> goto out;
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index a0f3775cbd82..c6b0454b2e25 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -1024,6 +1024,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
> @@ -1066,6 +1067,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"},
> @@ -1173,6 +1175,21 @@ static void check_template_modsig(const struct ima_template_desc *template)
> #undef MSG
> }
>
> +/*
> + * Make sure the policy rule and template format are in sync.
If they are not in sync I need to update my policy rule?

> + */
> +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 */
> @@ -1215,7 +1232,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;
> @@ -1708,6 +1726,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)
> @@ -1798,6 +1823,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;
> @@ -2155,6 +2189,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 bd95864a5f6f..0cff6658a4c2 100644
> --- a/security/integrity/ima/ima_template_lib.c
> +++ b/security/integrity/ima/ima_template_lib.c
> @@ -31,7 +31,7 @@ enum data_formats {
> };
>
> #define DIGEST_TYPE_MAXLEN 16 /* including NULL */
> -static const char * const digest_type_name[] = {"ima"};
> +static const char * const digest_type_name[] = {"ima", "verity"};
> static int digest_type_size = ARRAY_SIZE(digest_type_name);

if this static needs to exist at all, and I dn't think it should, it
should probably be called digest_type_array_size. Though I would just
use ARRAY_SIZE() where needed.

>
> static int ima_write_template_field_data(const void *data, const u32 datalen,
> @@ -430,6 +430,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 = 1;
> out:
> return ima_eventdigest_init_common(cur_digest, cur_digestsize,
> digest_type, hash_algo, field_data);
> diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
> index daf49894fd7d..d42a01903f08 100644
> --- a/security/integrity/integrity.h
> +++ b/security/integrity/integrity.h
> @@ -32,7 +32,7 @@
> #define IMA_HASHED 0x00000200
>
> /* iint policy rule cache flags */
> -#define IMA_NONACTION_FLAGS 0xff000000
> +#define IMA_NONACTION_FLAGS 0xff800000
> #define IMA_DIGSIG_REQUIRED 0x01000000
> #define IMA_PERMIT_DIRECTIO 0x02000000
> #define IMA_NEW_FILE 0x04000000
> @@ -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)

2022-03-21 21:52:52

by Mimi Zohar

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

On Mon, 2022-03-21 at 08:59 -0400, Stefan Berger wrote:

> > +/*
> > + * Make sure the policy rule and template format are in sync.
> If they are not in sync I need to update my policy rule?

It doesn't prevent loading the policy, if they're not in sync, but
simply issues a warning.

>
> > + */
> > +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 */
> > @@ -1215,7 +1232,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;
> > @@ -1708,6 +1726,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)
> > @@ -1798,6 +1823,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;
> > @@ -2155,6 +2189,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 bd95864a5f6f..0cff6658a4c2 100644
> > --- a/security/integrity/ima/ima_template_lib.c
> > +++ b/security/integrity/ima/ima_template_lib.c
> > @@ -31,7 +31,7 @@ enum data_formats {
> > };
> >
> > #define DIGEST_TYPE_MAXLEN 16 /* including NULL */
> > -static const char * const digest_type_name[] = {"ima"};
> > +static const char * const digest_type_name[] = {"ima", "verity"};
> > static int digest_type_size = ARRAY_SIZE(digest_type_name);
>
> if this static needs to exist at all, and I dn't think it should, it
> should probably be called digest_type_array_size. Though I would just
> use ARRAY_SIZE() where needed.

Ok.

thanks,

Mimi