diff mbox series

[RFC,2/2] evm: output EVM digest calculation info needed for debugging

Message ID 20210603151819.242247-3-zohar@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series EVM: add some debugging info | expand

Commit Message

Mimi Zohar June 3, 2021, 3:18 p.m. UTC
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(+)

Comments

Lakshmi Ramasubramanian June 3, 2021, 4:34 p.m. UTC | #1
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;
Mimi Zohar June 3, 2021, 4:55 p.m. UTC | #2
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
Lakshmi Ramasubramanian June 3, 2021, 5:48 p.m. UTC | #3
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
diff mbox series

Patch

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;