diff mbox series

[v5,4/8] ima: define a new template field 'd-type' and a new template 'ima-ngv2'

Message ID 20220211214310.119257-5-zohar@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series ima: support fs-verity digests and signatures | expand

Commit Message

Mimi Zohar Feb. 11, 2022, 9:43 p.m. UTC
In preparation to differentiate between regular IMA file hashes and
fs-verity's file digests, define a new template field named 'd-type'.
Define a new template named 'ima-ngv2', which includes the new 'd-type'
field.

Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 security/integrity/ima/ima_template.c     |  3 +++
 security/integrity/ima/ima_template_lib.c | 13 +++++++++++++
 security/integrity/ima/ima_template_lib.h |  2 ++
 3 files changed, 18 insertions(+)

Comments

Stefan Berger Feb. 14, 2022, 8:38 p.m. UTC | #1
On 2/11/22 16:43, Mimi Zohar wrote:
> In preparation to differentiate between regular IMA file hashes and
> fs-verity's file digests, define a new template field named 'd-type'.
> Define a new template named 'ima-ngv2', which includes the new 'd-type'
> field.
>
> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> ---
>   security/integrity/ima/ima_template.c     |  3 +++
>   security/integrity/ima/ima_template_lib.c | 13 +++++++++++++
>   security/integrity/ima/ima_template_lib.h |  2 ++
>   3 files changed, 18 insertions(+)
>
> diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
> index db1ad6d7a57f..b321342e5bee 100644
> --- a/security/integrity/ima/ima_template.c
> +++ b/security/integrity/ima/ima_template.c
> @@ -19,6 +19,7 @@ enum header_fields { HDR_PCR, HDR_DIGEST, HDR_TEMPLATE_NAME,
>   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-ngv2", .fmt = "d-ng|n-ng|d-type"},
>   	{.name = "ima-sig", .fmt = "d-ng|n-ng|sig"},
>   	{.name = "ima-buf", .fmt = "d-ng|n-ng|buf"},
>   	{.name = "ima-modsig", .fmt = "d-ng|n-ng|sig|d-modsig|modsig"},
> @@ -40,6 +41,8 @@ static const struct ima_template_field supported_fields[] = {
>   	 .field_show = ima_show_template_digest_ng},
>   	{.field_id = "n-ng", .field_init = ima_eventname_ng_init,
>   	 .field_show = ima_show_template_string},
> +	{.field_id = "d-type", .field_init = ima_eventdigest_type_init,
> +	 .field_show = ima_show_template_string},
>   	{.field_id = "sig", .field_init = ima_eventsig_init,
>   	 .field_show = ima_show_template_sig},
>   	{.field_id = "buf", .field_init = ima_eventbuf_init,
> diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
> index 7155d17a3b75..a213579e825e 100644
> --- a/security/integrity/ima/ima_template_lib.c
> +++ b/security/integrity/ima/ima_template_lib.c
> @@ -383,6 +383,19 @@ int ima_eventdigest_ng_init(struct ima_event_data *event_data,
>   					   hash_algo, field_data);
>   }
>   
> +/*
> + * This function writes the digest type of an event.
> + */
> +int ima_eventdigest_type_init(struct ima_event_data *event_data,
> +			      struct ima_field_data *field_data)
> +{
> +	static const char * const digest_type[] = {"ima"};

This array makes sense with 6/8.

Acked-by: Stefan Berger <stefanb@linux.ibm.com>


> +
> +	return ima_write_template_field_data(digest_type[0],
> +					     strlen(digest_type[0]),
> +					     DATA_FMT_STRING, field_data);
> +}
> +
>   /*
>    * This function writes the digest of the file which is expected to match the
>    * digest contained in the file's appended signature.
> diff --git a/security/integrity/ima/ima_template_lib.h b/security/integrity/ima/ima_template_lib.h
> index c71f1de95753..539a5e354925 100644
> --- a/security/integrity/ima/ima_template_lib.h
> +++ b/security/integrity/ima/ima_template_lib.h
> @@ -38,6 +38,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_type_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,
Eric Biggers Feb. 24, 2022, 12:32 a.m. UTC | #2
On Fri, Feb 11, 2022 at 04:43:06PM -0500, Mimi Zohar wrote:
> In preparation to differentiate between regular IMA file hashes and
> fs-verity's file digests, define a new template field named 'd-type'.
> Define a new template named 'ima-ngv2', which includes the new 'd-type'
> field.
> 
> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> ---
>  security/integrity/ima/ima_template.c     |  3 +++
>  security/integrity/ima/ima_template_lib.c | 13 +++++++++++++
>  security/integrity/ima/ima_template_lib.h |  2 ++
>  3 files changed, 18 insertions(+)
> 
> diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
> index db1ad6d7a57f..b321342e5bee 100644
> --- a/security/integrity/ima/ima_template.c
> +++ b/security/integrity/ima/ima_template.c
> @@ -19,6 +19,7 @@ enum header_fields { HDR_PCR, HDR_DIGEST, HDR_TEMPLATE_NAME,
>  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-ngv2", .fmt = "d-ng|n-ng|d-type"},
>  	{.name = "ima-sig", .fmt = "d-ng|n-ng|sig"},
>  	{.name = "ima-buf", .fmt = "d-ng|n-ng|buf"},
>  	{.name = "ima-modsig", .fmt = "d-ng|n-ng|sig|d-modsig|modsig"},
> @@ -40,6 +41,8 @@ static const struct ima_template_field supported_fields[] = {
>  	 .field_show = ima_show_template_digest_ng},
>  	{.field_id = "n-ng", .field_init = ima_eventname_ng_init,
>  	 .field_show = ima_show_template_string},
> +	{.field_id = "d-type", .field_init = ima_eventdigest_type_init,
> +	 .field_show = ima_show_template_string},
>  	{.field_id = "sig", .field_init = ima_eventsig_init,
>  	 .field_show = ima_show_template_sig},
>  	{.field_id = "buf", .field_init = ima_eventbuf_init,

I notice that the "d-ng" field already contains both the hash algorithm and the
hash itself, in the form <algorithm>:<hash>.  Wouldn't it make more sense to
define a "d-ngv2" field that contains <type>:<algorithm>:<hash>?  After all,
both the type and algorithm are required to interpret the hash.

Or in other words, what about the hash type is different from the hash algorithm
that would result in them needing different handling here?

- Eric
Mimi Zohar Feb. 24, 2022, 4:16 p.m. UTC | #3
On Wed, 2022-02-23 at 16:32 -0800, Eric Biggers wrote:
> On Fri, Feb 11, 2022 at 04:43:06PM -0500, Mimi Zohar wrote:
> > In preparation to differentiate between regular IMA file hashes and
> > fs-verity's file digests, define a new template field named 'd-type'.
> > Define a new template named 'ima-ngv2', which includes the new 'd-type'
> > field.
> > 
> > Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> > ---
> >  security/integrity/ima/ima_template.c     |  3 +++
> >  security/integrity/ima/ima_template_lib.c | 13 +++++++++++++
> >  security/integrity/ima/ima_template_lib.h |  2 ++
> >  3 files changed, 18 insertions(+)
> > 
> > diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
> > index db1ad6d7a57f..b321342e5bee 100644
> > --- a/security/integrity/ima/ima_template.c
> > +++ b/security/integrity/ima/ima_template.c
> > @@ -19,6 +19,7 @@ enum header_fields { HDR_PCR, HDR_DIGEST, HDR_TEMPLATE_NAME,
> >  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-ngv2", .fmt = "d-ng|n-ng|d-type"},
> >  	{.name = "ima-sig", .fmt = "d-ng|n-ng|sig"},
> >  	{.name = "ima-buf", .fmt = "d-ng|n-ng|buf"},
> >  	{.name = "ima-modsig", .fmt = "d-ng|n-ng|sig|d-modsig|modsig"},
> > @@ -40,6 +41,8 @@ static const struct ima_template_field supported_fields[] = {
> >  	 .field_show = ima_show_template_digest_ng},
> >  	{.field_id = "n-ng", .field_init = ima_eventname_ng_init,
> >  	 .field_show = ima_show_template_string},
> > +	{.field_id = "d-type", .field_init = ima_eventdigest_type_init,
> > +	 .field_show = ima_show_template_string},
> >  	{.field_id = "sig", .field_init = ima_eventsig_init,
> >  	 .field_show = ima_show_template_sig},
> >  	{.field_id = "buf", .field_init = ima_eventbuf_init,
> 
> I notice that the "d-ng" field already contains both the hash algorithm and the
> hash itself, in the form <algorithm>:<hash>.  Wouldn't it make more sense to
> define a "d-ngv2" field that contains <type>:<algorithm>:<hash>?  After all,
> both the type and algorithm are required to interpret the hash.
> 
> Or in other words, what about the hash type is different from the hash algorithm
> that would result in them needing different handling here?

Thanks, Eric.  I really like your suggestion.  Currently, type is
defined as either "ima" or "verity".   Are you ok with prefixing each
record with these strings?
Eric Biggers Feb. 24, 2022, 6:46 p.m. UTC | #4
On Thu, Feb 24, 2022 at 11:16:51AM -0500, Mimi Zohar wrote:
> On Wed, 2022-02-23 at 16:32 -0800, Eric Biggers wrote:
> > On Fri, Feb 11, 2022 at 04:43:06PM -0500, Mimi Zohar wrote:
> > > In preparation to differentiate between regular IMA file hashes and
> > > fs-verity's file digests, define a new template field named 'd-type'.
> > > Define a new template named 'ima-ngv2', which includes the new 'd-type'
> > > field.
> > > 
> > > Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> > > ---
> > >  security/integrity/ima/ima_template.c     |  3 +++
> > >  security/integrity/ima/ima_template_lib.c | 13 +++++++++++++
> > >  security/integrity/ima/ima_template_lib.h |  2 ++
> > >  3 files changed, 18 insertions(+)
> > > 
> > > diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
> > > index db1ad6d7a57f..b321342e5bee 100644
> > > --- a/security/integrity/ima/ima_template.c
> > > +++ b/security/integrity/ima/ima_template.c
> > > @@ -19,6 +19,7 @@ enum header_fields { HDR_PCR, HDR_DIGEST, HDR_TEMPLATE_NAME,
> > >  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-ngv2", .fmt = "d-ng|n-ng|d-type"},
> > >  	{.name = "ima-sig", .fmt = "d-ng|n-ng|sig"},
> > >  	{.name = "ima-buf", .fmt = "d-ng|n-ng|buf"},
> > >  	{.name = "ima-modsig", .fmt = "d-ng|n-ng|sig|d-modsig|modsig"},
> > > @@ -40,6 +41,8 @@ static const struct ima_template_field supported_fields[] = {
> > >  	 .field_show = ima_show_template_digest_ng},
> > >  	{.field_id = "n-ng", .field_init = ima_eventname_ng_init,
> > >  	 .field_show = ima_show_template_string},
> > > +	{.field_id = "d-type", .field_init = ima_eventdigest_type_init,
> > > +	 .field_show = ima_show_template_string},
> > >  	{.field_id = "sig", .field_init = ima_eventsig_init,
> > >  	 .field_show = ima_show_template_sig},
> > >  	{.field_id = "buf", .field_init = ima_eventbuf_init,
> > 
> > I notice that the "d-ng" field already contains both the hash algorithm and the
> > hash itself, in the form <algorithm>:<hash>.  Wouldn't it make more sense to
> > define a "d-ngv2" field that contains <type>:<algorithm>:<hash>?  After all,
> > both the type and algorithm are required to interpret the hash.
> > 
> > Or in other words, what about the hash type is different from the hash algorithm
> > that would result in them needing different handling here?
> 
> Thanks, Eric.  I really like your suggestion.  Currently, type is
> defined as either "ima" or "verity".   Are you ok with prefixing each
> record with these strings?
> 

As I mentioned before I think "full_hash" would be more descriptive than "ima".
(All of this is part of IMA.)  It's up to you though.

- Eric
Mimi Zohar Feb. 25, 2022, 8:01 p.m. UTC | #5
On Thu, 2022-02-24 at 10:46 -0800, Eric Biggers wrote:
> > Thanks, Eric.  I really like your suggestion.  Currently, type is
> > defined as either "ima" or "verity".   Are you ok with prefixing each
> > record with these strings?
> > 
> 
> As I mentioned before I think "full_hash" would be more descriptive than "ima".
> (All of this is part of IMA.)  It's up to you though.

Sorry, to me that doesn't sense.  Either we use "full_hash" and
"Merkle-tree", or "ima" and "verity".
diff mbox series

Patch

diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index db1ad6d7a57f..b321342e5bee 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -19,6 +19,7 @@  enum header_fields { HDR_PCR, HDR_DIGEST, HDR_TEMPLATE_NAME,
 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-ngv2", .fmt = "d-ng|n-ng|d-type"},
 	{.name = "ima-sig", .fmt = "d-ng|n-ng|sig"},
 	{.name = "ima-buf", .fmt = "d-ng|n-ng|buf"},
 	{.name = "ima-modsig", .fmt = "d-ng|n-ng|sig|d-modsig|modsig"},
@@ -40,6 +41,8 @@  static const struct ima_template_field supported_fields[] = {
 	 .field_show = ima_show_template_digest_ng},
 	{.field_id = "n-ng", .field_init = ima_eventname_ng_init,
 	 .field_show = ima_show_template_string},
+	{.field_id = "d-type", .field_init = ima_eventdigest_type_init,
+	 .field_show = ima_show_template_string},
 	{.field_id = "sig", .field_init = ima_eventsig_init,
 	 .field_show = ima_show_template_sig},
 	{.field_id = "buf", .field_init = ima_eventbuf_init,
diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
index 7155d17a3b75..a213579e825e 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -383,6 +383,19 @@  int ima_eventdigest_ng_init(struct ima_event_data *event_data,
 					   hash_algo, field_data);
 }
 
+/*
+ * This function writes the digest type of an event.
+ */
+int ima_eventdigest_type_init(struct ima_event_data *event_data,
+			      struct ima_field_data *field_data)
+{
+	static const char * const digest_type[] = {"ima"};
+
+	return ima_write_template_field_data(digest_type[0],
+					     strlen(digest_type[0]),
+					     DATA_FMT_STRING, field_data);
+}
+
 /*
  * This function writes the digest of the file which is expected to match the
  * digest contained in the file's appended signature.
diff --git a/security/integrity/ima/ima_template_lib.h b/security/integrity/ima/ima_template_lib.h
index c71f1de95753..539a5e354925 100644
--- a/security/integrity/ima/ima_template_lib.h
+++ b/security/integrity/ima/ima_template_lib.h
@@ -38,6 +38,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_type_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,