linux-integrity.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 0/2] EVM: add some debugging info
@ 2021-06-03 15:18 Mimi Zohar
  2021-06-03 15:18 ` [RFC PATCH 1/2] ima: differentiate between EVM failures in the audit log Mimi Zohar
  2021-06-03 15:18 ` [RFC PATCH 2/2] evm: output EVM digest calculation info needed for debugging Mimi Zohar
  0 siblings, 2 replies; 7+ messages in thread
From: Mimi Zohar @ 2021-06-03 15:18 UTC (permalink / raw)
  To: linux-integrity; +Cc: Roberto Sassu, Stefan Berger, Russell Coker, Mimi Zohar

With the recent EVM portable & immutable signature usability improvements
and the ability of including the EVM portable signatures in the IMA
measurement list, adding some debugging information - security xattrs and
file metadata (misc info) - would be useful.

Suggestons on how to improve this patch set would be much appreciated.

thanks,

Mimi

Mimi Zohar (2):
  ima: differentiate between EVM failures in the audit log
  evm: output EVM digest calculation info needed for debugging

 security/integrity/evm/evm.h          |  1 +
 security/integrity/evm/evm_crypto.c   |  7 +++++++
 security/integrity/evm/evm_main.c     | 19 +++++++++++++++++++
 security/integrity/ima/ima_appraise.c |  3 ++-
 4 files changed, 29 insertions(+), 1 deletion(-)

-- 
2.27.0

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [RFC PATCH 1/2] ima: differentiate between EVM failures in the audit log
  2021-06-03 15:18 [RFC PATCH 0/2] EVM: add some debugging info Mimi Zohar
@ 2021-06-03 15:18 ` Mimi Zohar
  2021-06-04  6:37   ` Roberto Sassu
  2021-06-03 15:18 ` [RFC PATCH 2/2] evm: output EVM digest calculation info needed for debugging Mimi Zohar
  1 sibling, 1 reply; 7+ messages in thread
From: Mimi Zohar @ 2021-06-03 15:18 UTC (permalink / raw)
  To: linux-integrity; +Cc: Roberto Sassu, Stefan Berger, Russell Coker, Mimi Zohar

Differenatiate between an invalid EVM portable signature failure
from other EVM HMAC/signature failures.

Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 security/integrity/ima/ima_appraise.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 940695e7b535..ef9dcfce45d4 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -422,7 +422,8 @@ int ima_appraise_measurement(enum ima_hooks func,
 		goto out;
 	case INTEGRITY_FAIL_IMMUTABLE:
 		set_bit(IMA_DIGSIG, &iint->atomic_flags);
-		fallthrough;
+		cause = "invalid-fail-immutable";
+		goto out;
 	case INTEGRITY_FAIL:		/* Invalid HMAC/signature. */
 		cause = "invalid-HMAC";
 		goto out;
-- 
2.27.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [RFC PATCH 2/2] evm: output EVM digest calculation info needed for debugging
  2021-06-03 15:18 [RFC PATCH 0/2] EVM: add some debugging info Mimi Zohar
  2021-06-03 15:18 ` [RFC PATCH 1/2] ima: differentiate between EVM failures in the audit log Mimi Zohar
@ 2021-06-03 15:18 ` Mimi Zohar
  2021-06-03 16:34   ` nramas
  1 sibling, 1 reply; 7+ messages in thread
From: Mimi Zohar @ 2021-06-03 15:18 UTC (permalink / raw)
  To: linux-integrity; +Cc: Roberto Sassu, Stefan Berger, Russell Coker, Mimi Zohar

Convert and output the binary data used in calculating the EVM digest
to a hexadecimal string.

Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 security/integrity/evm/evm.h        |  1 +
 security/integrity/evm/evm_crypto.c |  7 +++++++
 security/integrity/evm/evm_main.c   | 19 +++++++++++++++++++
 3 files changed, 27 insertions(+)

diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
index 0d44f41d16f8..3d2d2da8fa97 100644
--- a/security/integrity/evm/evm.h
+++ b/security/integrity/evm/evm.h
@@ -64,5 +64,6 @@ int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name,
 int evm_init_hmac(struct inode *inode, const struct xattr *xattr,
 		  char *hmac_val);
 int evm_init_secfs(void);
+void evm_bin2hex_print(const char *prefix, const void *src, size_t count);
 
 #endif
diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index 1628e2ca9862..7601aa29c6d3 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -175,6 +175,8 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
 	    type != EVM_XATTR_PORTABLE_DIGSIG)
 		crypto_shash_update(desc, (u8 *)&inode->i_sb->s_uuid, UUID_SIZE);
 	crypto_shash_final(desc, digest);
+
+	evm_bin2hex_print("hmac_misc", &hmac_misc, sizeof(struct h_misc));
 }
 
 /*
@@ -230,6 +232,9 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
 					     req_xattr_value_len);
 			if (is_ima)
 				ima_present = true;
+
+			evm_bin2hex_print(req_xattr_name, req_xattr_value,
+				      req_xattr_value_len);
 			continue;
 		}
 		size = vfs_getxattr_alloc(&init_user_ns, dentry, xattr->name,
@@ -246,6 +251,8 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
 		crypto_shash_update(desc, (const u8 *)xattr_value, xattr_size);
 		if (is_ima)
 			ima_present = true;
+
+		evm_bin2hex_print(xattr->name, xattr_value, xattr_size);
 	}
 	hmac_add_misc(desc, inode, type, data->digest);
 
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 2c226e634ae9..03d963fe2e67 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -155,6 +155,23 @@ static int evm_find_protected_xattrs(struct dentry *dentry)
 	return count;
 }
 
+void evm_bin2hex_print(const char *prefix, const void *src, size_t count)
+{
+#ifdef DEBUG
+	char *asciihex, *p;
+
+	p = asciihex = kmalloc(count * 2 + 1, GFP_KERNEL);
+	if (!asciihex)
+		return;
+
+	p = bin2hex(p, src, count);
+	*p = 0;
+	printk("%s: (%lu) %.*s\n", prefix, count, (int) count * 2, asciihex);
+
+	kfree(asciihex);
+#endif
+}
+
 /*
  * evm_verify_hmac - calculate and compare the HMAC with the EVM xattr
  *
@@ -272,6 +289,8 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry,
 		else
 			evm_status = INTEGRITY_FAIL;
 	}
+
+	evm_bin2hex_print("evm digest", digest.digest, digest.hdr.length);
 out:
 	if (iint)
 		iint->evm_status = evm_status;
-- 
2.27.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH 2/2] evm: output EVM digest calculation info needed for debugging
  2021-06-03 15:18 ` [RFC PATCH 2/2] evm: output EVM digest calculation info needed for debugging Mimi Zohar
@ 2021-06-03 16:34   ` nramas
  2021-06-03 16:55     ` Mimi Zohar
  0 siblings, 1 reply; 7+ messages in thread
From: nramas @ 2021-06-03 16:34 UTC (permalink / raw)
  To: Mimi Zohar, linux-integrity; +Cc: Roberto Sassu, Stefan Berger, Russell Coker

On Thu, 2021-06-03 at 11:18 -0400, Mimi Zohar wrote:

Hi Mimi,

> Convert and output the binary data used in calculating the EVM digest
> to a hexadecimal string.
> 
> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> ---
>  security/integrity/evm/evm.h        |  1 +
>  security/integrity/evm/evm_crypto.c |  7 +++++++
>  security/integrity/evm/evm_main.c   | 19 +++++++++++++++++++
>  3 files changed, 27 insertions(+)
> 
> diff --git a/security/integrity/evm/evm.h
> b/security/integrity/evm/evm.h
> index 0d44f41d16f8..3d2d2da8fa97 100644
> --- a/security/integrity/evm/evm.h
> +++ b/security/integrity/evm/evm.h
> @@ -64,5 +64,6 @@ int evm_calc_hash(struct dentry *dentry, const char
> *req_xattr_name,
>  int (struct inode *inode, const struct xattr *xattr,
>  		  char *hmac_val);
>  int evm_init_secfs(void);
> +void evm_bin2hex_print(const char *prefix, const void *src, size_t
> count);
>  

For evm_bin2hex_print() can we could do the following in evm.h?

#ifdef DEBUG
void evm_bin2hex_print(const char *prefix, const void *src, size_t
count);
#else
void evm_bin2hex_print(const char *prefix, const void *src, size_t
count) {}
#endif /* DEBUG */

> void evm_bin2hex_print(const char *prefix, const void *src, size_t
> count);


>  #endif
> diff --git a/security/integrity/evm/evm_crypto.c
> b/security/integrity/evm/evm_crypto.c
> index 1628e2ca9862..7601aa29c6d3 100644
> --- a/security/integrity/evm/evm_crypto.c
> +++ b/security/integrity/evm/evm_crypto.c
> @@ -175,6 +175,8 @@ static void hmac_add_misc(struct shash_desc
> *desc, struct inode *inode,
>  	    type != EVM_XATTR_PORTABLE_DIGSIG)
>  		crypto_shash_update(desc, (u8 *)&inode->i_sb->s_uuid,
> UUID_SIZE);
>  	crypto_shash_final(desc, digest);
> +
> +	evm_bin2hex_print("hmac_misc", &hmac_misc, sizeof(struct
> h_misc));
>  }
>  
>  /*
> @@ -230,6 +232,9 @@ static int evm_calc_hmac_or_hash(struct dentry
> *dentry,
>  					     req_xattr_value_len);
>  			if (is_ima)
>  				ima_present = true;
> +
> +			evm_bin2hex_print(req_xattr_name,
> req_xattr_value,
> +				      req_xattr_value_len);
>  			continue;
>  		}
>  		size = vfs_getxattr_alloc(&init_user_ns, dentry, xattr-
> >name,
> @@ -246,6 +251,8 @@ static int evm_calc_hmac_or_hash(struct dentry
> *dentry,
>  		crypto_shash_update(desc, (const u8 *)xattr_value,
> xattr_size);
>  		if (is_ima)
>  			ima_present = true;
> +
> +		evm_bin2hex_print(xattr->name, xattr_value,
> xattr_size);
>  	}
>  	hmac_add_misc(desc, inode, type, data->digest);
>  
> diff --git a/security/integrity/evm/evm_main.c
> b/security/integrity/evm/evm_main.c
> index 2c226e634ae9..03d963fe2e67 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -155,6 +155,23 @@ static int evm_find_protected_xattrs(struct
> dentry *dentry)
>  	return count;
>  }
>  
> +void evm_bin2hex_print(const char *prefix, const void *src, size_t
> count)
> +{
> +#ifdef DEBUG
> +	char *asciihex, *p;
> +
> +	p = asciihex = kmalloc(count * 2 + 1, GFP_KERNEL);
> +	if (!asciihex)
> +		return;
> +
> +	p = bin2hex(p, src, count);
> +	*p = 0;
> +	printk("%s: (%lu) %.*s\n", prefix, count, (int) count * 2,
> asciihex);

Prefix the message with, say, "evm:".

thanks,
 -lakshmi

> +
> +	kfree(asciihex);
> +#endif
> +}
> +
>  /*
>   * evm_verify_hmac - calculate and compare the HMAC with the EVM
> xattr
>   *
> @@ -272,6 +289,8 @@ static enum integrity_status
> evm_verify_hmac(struct dentry *dentry,
>  		else
>  			evm_status = INTEGRITY_FAIL;
>  	}
> +
> +	evm_bin2hex_print("evm digest", digest.digest,
> digest.hdr.length);
>  out:
>  	if (iint)
>  		iint->evm_status = evm_status;


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH 2/2] evm: output EVM digest calculation info needed for debugging
  2021-06-03 16:34   ` nramas
@ 2021-06-03 16:55     ` Mimi Zohar
  2021-06-03 17:48       ` nramas
  0 siblings, 1 reply; 7+ messages in thread
From: Mimi Zohar @ 2021-06-03 16:55 UTC (permalink / raw)
  To: nramas, linux-integrity; +Cc: Roberto Sassu, Stefan Berger, Russell Coker

Hi Lakshmi,

On Thu, 2021-06-03 at 09:34 -0700, nramas wrote:
> > +void evm_bin2hex_print(const char *prefix, const void *src, size_t
> > count);
> >  
> 
> For evm_bin2hex_print() can we could do the following in evm.h?
> 
> #ifdef DEBUG
> void evm_bin2hex_print(const char *prefix, const void *src, size_t
> count);
> #else
> void evm_bin2hex_print(const char *prefix, const void *src, size_t
> count) {}
> #endif /* DEBUG */

Yes, if we decide that it needs to be based on DEBUG, this would be the
proper way of doing it.  However, since there's nothing really private
here, it's just displaying the security xattrs and other file metadata,
should enabling/disabling the debugging be runtime configurable?   Kind
of like how print_hex_dump() relies on loglevel.  Or should it be more
granular?

thanks,

Mimi


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH 2/2] evm: output EVM digest calculation info needed for debugging
  2021-06-03 16:55     ` Mimi Zohar
@ 2021-06-03 17:48       ` nramas
  0 siblings, 0 replies; 7+ messages in thread
From: nramas @ 2021-06-03 17:48 UTC (permalink / raw)
  To: Mimi Zohar, linux-integrity; +Cc: Roberto Sassu, Stefan Berger, Russell Coker

On Thu, 2021-06-03 at 12:55 -0400, Mimi Zohar wrote:
> 
> On Thu, 2021-06-03 at 09:34 -0700, nramas wrote:
> > > +void evm_bin2hex_print(const char *prefix, const void *src,
> > > size_t
> > > count);
> > >  
> > 
> > For evm_bin2hex_print() can we could do the following in evm.h?
> > 
> > #ifdef DEBUG
> > void evm_bin2hex_print(const char *prefix, const void *src, size_t
> > count);
> > #else
> > void evm_bin2hex_print(const char *prefix, const void *src, size_t
> > count) {}
> > #endif /* DEBUG */
> 
> Yes, if we decide that it needs to be based on DEBUG, this would be
> the
> proper way of doing it.  However, since there's nothing really
> private
> here, it's just displaying the security xattrs and other file
> metadata,
> should enabling/disabling the debugging be runtime
> configurable?   Kind
> of like how print_hex_dump() relies on loglevel.  Or should it be
> more
> granular?
> 

I read "Documentation/admin-guide/dynamic-debug-howto.rst". I think
dynamically enabling/disabling, like how print_hex_dump() does, would
be better for evm_bin2hex_print() as well.

thanks,
 -lakshmi
 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [RFC PATCH 1/2] ima: differentiate between EVM failures in the audit log
  2021-06-03 15:18 ` [RFC PATCH 1/2] ima: differentiate between EVM failures in the audit log Mimi Zohar
@ 2021-06-04  6:37   ` Roberto Sassu
  0 siblings, 0 replies; 7+ messages in thread
From: Roberto Sassu @ 2021-06-04  6:37 UTC (permalink / raw)
  To: Mimi Zohar, linux-integrity; +Cc: Stefan Berger, Russell Coker

> From: Mimi Zohar [mailto:zohar@linux.ibm.com]
> Sent: Thursday, June 3, 2021 5:18 PM
> Differenatiate between an invalid EVM portable signature failure

Typo.

> from other EVM HMAC/signature failures.
> 
> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>

Thanks, looks good.

Reviewed-by: Roberto Sassu <roberto.sassu@huawei.com>

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Li Jian, Shi Yanli

> ---
>  security/integrity/ima/ima_appraise.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/security/integrity/ima/ima_appraise.c
> b/security/integrity/ima/ima_appraise.c
> index 940695e7b535..ef9dcfce45d4 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -422,7 +422,8 @@ int ima_appraise_measurement(enum ima_hooks
> func,
>  		goto out;
>  	case INTEGRITY_FAIL_IMMUTABLE:
>  		set_bit(IMA_DIGSIG, &iint->atomic_flags);
> -		fallthrough;
> +		cause = "invalid-fail-immutable";
> +		goto out;
>  	case INTEGRITY_FAIL:		/* Invalid HMAC/signature. */
>  		cause = "invalid-HMAC";
>  		goto out;
> --
> 2.27.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-06-04  6:37 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-03 15:18 [RFC PATCH 0/2] EVM: add some debugging info Mimi Zohar
2021-06-03 15:18 ` [RFC PATCH 1/2] ima: differentiate between EVM failures in the audit log Mimi Zohar
2021-06-04  6:37   ` Roberto Sassu
2021-06-03 15:18 ` [RFC PATCH 2/2] evm: output EVM digest calculation info needed for debugging Mimi Zohar
2021-06-03 16:34   ` nramas
2021-06-03 16:55     ` Mimi Zohar
2021-06-03 17:48       ` nramas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).