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 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 (5):
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 | 36 +++++-
.../admin-guide/kernel-parameters.txt | 3 +-
Documentation/filesystems/fsverity.rst | 22 ++--
Documentation/security/IMA-templates.rst | 12 +-
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 | 38 +++++-
security/integrity/ima/ima_appraise.c | 114 +++++++++++++++++-
security/integrity/ima/ima_policy.c | 68 ++++++++++-
security/integrity/ima/ima_template.c | 4 +
security/integrity/ima/ima_template_lib.c | 77 ++++++++++--
security/integrity/ima/ima_template_lib.h | 4 +
security/integrity/integrity.h | 26 +++-
16 files changed, 431 insertions(+), 45 deletions(-)
--
2.27.0
Update the fsverity documentation related to IMA signature support.
Signed-off-by: Mimi Zohar <[email protected]>
---
Documentation/filesystems/fsverity.rst | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/Documentation/filesystems/fsverity.rst b/Documentation/filesystems/fsverity.rst
index 1d831e3cbcb3..28a47488848e 100644
--- a/Documentation/filesystems/fsverity.rst
+++ b/Documentation/filesystems/fsverity.rst
@@ -74,8 +74,12 @@ 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.
+`Built-in signature verification`_.
+
+IMA supports including fs-verity file digests and signatures in the
+IMA (Integrity Measurement Architecture) measurement list and
+verifying fs-verity based file signatures stored as security.ima
+xattrs, based on policy.
User API
========
@@ -653,13 +657,13 @@ 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
- filesystem features e.g. with xfstests.
+ IMA supports 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 filesystem
+ features e.g. with xfstests.
:Q: Isn't fs-verity useless because the attacker can just modify the
hashes in the Merkle tree, which is stored on-disk?
--
2.27.0
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 +-
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 };
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) {
+ 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;
+
+ 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,
--
2.27.0
On Fri, 2022-03-18 at 16:55 -0400, Stefan Berger wrote:
>
> On 3/18/22 14:21, Mimi Zohar wrote:
> > Update the fsverity documentation related to IMA signature support.
> >
> > Signed-off-by: Mimi Zohar <[email protected]>
> > ---
> > Documentation/filesystems/fsverity.rst | 22 +++++++++++++---------
> > 1 file changed, 13 insertions(+), 9 deletions(-)
> >
> > diff --git a/Documentation/filesystems/fsverity.rst b/Documentation/filesystems/fsverity.rst
> > index 1d831e3cbcb3..28a47488848e 100644
> > --- a/Documentation/filesystems/fsverity.rst
> > +++ b/Documentation/filesystems/fsverity.rst
> > @@ -74,8 +74,12 @@ 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.
> > +`Built-in signature verification`_.
> > +
> > +IMA supports including fs-verity file digests and signatures in the
> > +IMA (Integrity Measurement Architecture) measurement list and
>
> The Integrity Measurement Architecture (IMA) supports including ...
>
> > +verifying fs-verity based file signatures stored as security.ima
> > +xattrs, based on policy.
> >
> > User API
> > ========
> > @@ -653,13 +657,13 @@ 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
> > - filesystem features e.g. with xfstests.
> > + IMA supports the fs-verity hashing mechanism as an alternative
> > + to doing full file hashes, for people who want the performance
>
> IMA supports the fs-verity hashing mechanism as an alternative to full
> file hashes for those who want the performance and security benefits ...
>
> > + 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
>
> However, it doesn't make sense ...
>
> > + IMA. As a standalone filesystem feature, fs-verity already meets
> > + many users' needs, and it's testable like other filesystem
> > + features e.g. with xfstests.
>
> Fs-verity already meets many user' needs even as a standalone filesystem
> feature and it is testable like other ...
>
> >
> > :Q: Isn't fs-verity useless because the attacker can just modify the
> > hashes in the Merkle tree, which is stored on-disk?
Thanks, Stefan, for the suggestions. I tried to minimize the changes
as much as possible. Based on another thread, the documentation should
be updated, but I'm not going to be presumptuous and make those
changes. Eric, should I drop this patch and let you update the fs-
verity documentation as you want?
--
thanks,
Mimi
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 <[email protected]>
> ---
> .../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,
On 3/18/22 14:21, Mimi Zohar wrote:
> Update the fsverity documentation related to IMA signature support.
>
> Signed-off-by: Mimi Zohar <[email protected]>
> ---
> Documentation/filesystems/fsverity.rst | 22 +++++++++++++---------
> 1 file changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/filesystems/fsverity.rst b/Documentation/filesystems/fsverity.rst
> index 1d831e3cbcb3..28a47488848e 100644
> --- a/Documentation/filesystems/fsverity.rst
> +++ b/Documentation/filesystems/fsverity.rst
> @@ -74,8 +74,12 @@ 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.
> +`Built-in signature verification`_.
> +
> +IMA supports including fs-verity file digests and signatures in the
> +IMA (Integrity Measurement Architecture) measurement list and
The Integrity Measurement Architecture (IMA) supports including ...
> +verifying fs-verity based file signatures stored as security.ima
> +xattrs, based on policy.
>
> User API
> ========
> @@ -653,13 +657,13 @@ 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
> - filesystem features e.g. with xfstests.
> + IMA supports the fs-verity hashing mechanism as an alternative
> + to doing full file hashes, for people who want the performance
IMA supports the fs-verity hashing mechanism as an alternative to full
file hashes for those who want the performance and security benefits ...
> + 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
However, it doesn't make sense ...
> + IMA. As a standalone filesystem feature, fs-verity already meets
> + many users' needs, and it's testable like other filesystem
> + features e.g. with xfstests.
Fs-verity already meets many user' needs even as a standalone filesystem
feature and it is testable like other ...
>
> :Q: Isn't fs-verity useless because the attacker can just modify the
> hashes in the Merkle tree, which is stored on-disk?
On Mon, 2022-03-21 at 08:53 -0400, Stefan Berger wrote:
> > /*
> > * 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 + ....
DIGEST_TYPE_MAXLEN is hard coded as 16.
>
> > 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..
digest_type_size is the size of the array. It's defined as "static int
digest_type_size = ARRAY_SIZE(digest_type_name);", with the first
element as "ima".
> > +
> > +/*
> > + * 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.
The first element of the array is "ima". Should I define two macros
similar to kernel_read_file_id and kernel_read_file_str for just two
strings?
thanks,
Mimi
On 3/21/22 15:48, Mimi Zohar wrote:
> On Mon, 2022-03-21 at 08:53 -0400, Stefan Berger wrote:
>>> +
>>> +/*
>>> + * 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.
>
> The first element of the array is "ima". Should I define two macros
> similar to kernel_read_file_id and kernel_read_file_str for just two
> strings?
I would introduce an enum like enum hash_algo:
https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/hash_info.h#L38
And an array like hash_algo_name:
https://elixir.bootlin.com/linux/latest/source/crypto/hash_info.c#L12
>
> thanks,
>
> Mimi
>