From: Stefan Berger <stefanb@linux.ibm.com>
To: Mimi Zohar <zohar@linux.ibm.com>, linux-integrity@vger.kernel.org
Cc: Eric Biggers <ebiggers@kernel.org>,
linux-fscrypt@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v6 2/5] ima: define a new template field named 'd-ngv2' and templates
Date: Mon, 21 Mar 2022 08:53:35 -0400 [thread overview]
Message-ID: <b19eca12-3ae1-a8a5-fcfd-a22b5ee9319c@linux.ibm.com> (raw)
In-Reply-To: <20220318182151.100847-3-zohar@linux.ibm.com>
On 3/18/22 14:21, 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 <zohar@linux.ibm.com>
> ---
> .../admin-guide/kernel-parameters.txt | 3 +-
> security/integrity/ima/ima_template.c | 4 ++
> security/integrity/ima/ima_template_lib.c | 71 ++++++++++++++++---
> security/integrity/ima/ima_template_lib.h | 4 ++
> 4 files changed, 72 insertions(+), 10 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index f5a27f067db9..47386ac10471 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -1876,7 +1876,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/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 7155d17a3b75..bd95864a5f6f 100644
> --- a/security/integrity/ima/ima_template_lib.c
> +++ b/security/integrity/ima/ima_template_lib.c
> @@ -24,11 +24,16 @@ 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
> };
>
> +#define DIGEST_TYPE_MAXLEN 16 /* including NULL */
> +static const char * const digest_type_name[] = {"ima"};
> +static int digest_type_size = ARRAY_SIZE(digest_type_name);
> +
> 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 +77,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 +184,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,26 +279,39 @@ 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 <hash type> is either "ima" or "verity",
> * where <hash algo> is provided if the hash algorithm is not
> * SHA1 or MD5
> */
> - u8 buffer[CRYPTO_MAX_ALG_NAME + 2 + IMA_MAX_DIGEST_SIZE] = { 0 };
> + u8 buffer[DIGEST_TYPE_MAXLEN + CRYPTO_MAX_ALG_NAME + 2 +
> + IMA_MAX_DIGEST_SIZE] = { 0 };
Should it not be DIGEST_TYPE_MAXLEN + 1 /* for ':' */ +
CRYPTO_MAX_ALG_NAME + ....
> enum data_formats fmt = DATA_FMT_DIGEST;
> u32 offset = 0;
>
> - if (hash_algo < HASH_ALGO__LAST) {
> - fmt = DATA_FMT_DIGEST_WITH_ALGO;
> - offset += snprintf(buffer, CRYPTO_MAX_ALG_NAME + 1, "%s",
> + if (digest_type < digest_type_size && hash_algo < HASH_ALGO__LAST) {
It's a bit difficult to see what the digest_type has to do with size...
Maybe digest_type should be a enum and the comparison here should be
digest_type != DIGEST_TYPE_NONE or something like it..
> + fmt = DATA_FMT_DIGEST_WITH_TYPE_AND_ALGO;
> + offset += snprintf(buffer, DIGEST_TYPE_MAXLEN +
> + CRYPTO_MAX_ALG_NAME + 1, "%*s:%s",
> + (int)strlen(digest_type_name[digest_type]),
> + digest_type_name[digest_type],
> hash_algo_name[hash_algo]);
> buffer[offset] = ':';
> offset += 2;
> + } 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;
> }
>
> if (digest)
> @@ -359,7 +386,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_size, HASH_ALGO__LAST,
> + field_data);
> }
>
> /*
> @@ -380,7 +408,31 @@ 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_size, hash_algo,
> + field_data);
> +}
> +
> +/*
> + * This function writes the digest of an event (without size limit),
> + * prefixed with both the hash type and algorithm.
> + */
> +int ima_eventdigest_ngv2_init(struct ima_event_data *event_data,
> + struct ima_field_data *field_data)
> +{
> + u8 *cur_digest = NULL, hash_algo = HASH_ALGO_SHA1;
> + u32 cur_digestsize = 0;
> + u8 digest_type = 0;
What does '0' mean? I think this should definitely be an enum or at
least #define.
> +
> + 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);
> }
>
> /*
> @@ -415,7 +467,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_size, 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,
next prev parent reply other threads:[~2022-03-21 12:53 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-18 18:21 [PATCH v6 0/5] ima: support fs-verity digests and signatures Mimi Zohar
2022-03-18 18:21 ` [PATCH v6 1/5] fs-verity: define a function to return the integrity protected file digest Mimi Zohar
2022-03-18 18:21 ` [PATCH v6 2/5] ima: define a new template field named 'd-ngv2' and templates Mimi Zohar
2022-03-21 12:53 ` Stefan Berger [this message]
2022-03-21 19:48 ` Mimi Zohar
2022-03-21 20:46 ` Stefan Berger
2022-03-18 18:21 ` [PATCH v6 3/5] ima: permit fsverity's file digests in the IMA measurement list Mimi Zohar
2022-03-21 12:59 ` Stefan Berger
2022-03-21 19:54 ` Mimi Zohar
2022-03-18 18:21 ` [PATCH v6 4/5] ima: support fs-verity file digest based version 3 signatures Mimi Zohar
2022-03-21 13:10 ` Stefan Berger
2022-03-25 12:31 ` Mimi Zohar
2022-03-25 13:49 ` Stefan Berger
2022-03-18 18:21 ` [PATCH v6 5/5] fsverity: update the documentation Mimi Zohar
2022-03-18 20:55 ` Stefan Berger
2022-03-21 12:55 ` Mimi Zohar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b19eca12-3ae1-a8a5-fcfd-a22b5ee9319c@linux.ibm.com \
--to=stefanb@linux.ibm.com \
--cc=ebiggers@kernel.org \
--cc=linux-fscrypt@vger.kernel.org \
--cc=linux-integrity@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=zohar@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox