[PATCH 0/6] ima-evm-utils: Some cleanups and bugfixes

[PATCH 0/6] ima-evm-utils: Some cleanups and bugfixes

[Stefan Berger]

This PR contains some cleanups and bugfixes for libimaevm of ima-evm-utils.

   Stefan

Stefan Berger (6):
  libimaevm: Properly check for error returned by EVP_DigestUpdate
  libimaevm: Remove unused off variable
  libimaevm: Rename variable returned from readlink to len
  libimaevm: Rename variable from cr to newline
  libimaevm: Report unsupported filetype using log_err
  libimaevm: Use function parameter algo for name of hash

 src/libimaevm.c | 37 +++++++++++++++++--------------------
 1 file changed, 17 insertions(+), 20 deletions(-)

-- 
2.30.2

[PATCH 1/6] libimaevm: Properly check for error returned by EVP_DigestUpdate

[Stefan Berger]

The error checking in add_dir_hash was wrong. EVP_DigestUpdate returns 1
on success and 0 on error, so we cannot just accumulate it using or'ing.

From the man page:
       EVP_DigestInit_ex(), EVP_DigestUpdate(), EVP_DigestFinal_ex()
           Returns 1 for success and 0 for failure.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 src/libimaevm.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/src/libimaevm.c b/src/libimaevm.c
index fa6c278..d8fc0d4 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -179,7 +179,6 @@ out:
 
 static int add_dir_hash(const char *file, EVP_MD_CTX *ctx)
 {
-	int err;
 	struct dirent *de;
 	DIR *dir;
 	unsigned long long ino, off;
@@ -198,11 +197,10 @@ static int add_dir_hash(const char *file, EVP_MD_CTX *ctx)
 		type = de->d_type;
 		log_debug("entry: %s, ino: %llu, type: %u, off: %llu, reclen: %hu\n",
 			  de->d_name, ino, type, off, de->d_reclen);
-		err = EVP_DigestUpdate(ctx, de->d_name, strlen(de->d_name));
-		/*err |= EVP_DigestUpdate(ctx, &off, sizeof(off));*/
-		err |= EVP_DigestUpdate(ctx, &ino, sizeof(ino));
-		err |= EVP_DigestUpdate(ctx, &type, sizeof(type));
-		if (!err) {
+		if (EVP_DigestUpdate(ctx, de->d_name, strlen(de->d_name)) != 1 ||
+		    /* EVP_DigestUpdate(ctx, &off, sizeof(off)) != 1 || */
+		    EVP_DigestUpdate(ctx, &ino, sizeof(ino)) != 1||
+		    EVP_DigestUpdate(ctx, &type, sizeof(type)) != 1) {
 			log_err("EVP_DigestUpdate() failed\n");
 			output_openssl_errors();
 			result = 1;
-- 
2.30.2

[PATCH 2/6] libimaevm: Remove unused off variable

[Stefan Berger]

The 'off' variable was unused in add_dir_hash(), so remove it.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 src/libimaevm.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/src/libimaevm.c b/src/libimaevm.c
index d8fc0d4..0137884 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -181,7 +181,7 @@ static int add_dir_hash(const char *file, EVP_MD_CTX *ctx)
 {
 	struct dirent *de;
 	DIR *dir;
-	unsigned long long ino, off;
+	unsigned long long ino;
 	unsigned int type;
 	int result = 0;
 
@@ -193,12 +193,10 @@ static int add_dir_hash(const char *file, EVP_MD_CTX *ctx)
 
 	while ((de = readdir(dir))) {
 		ino = de->d_ino;
-		off = de->d_off;
 		type = de->d_type;
-		log_debug("entry: %s, ino: %llu, type: %u, off: %llu, reclen: %hu\n",
-			  de->d_name, ino, type, off, de->d_reclen);
+		log_debug("entry: %s, ino: %llu, type: %u, reclen: %hu\n",
+			  de->d_name, ino, type, de->d_reclen);
 		if (EVP_DigestUpdate(ctx, de->d_name, strlen(de->d_name)) != 1 ||
-		    /* EVP_DigestUpdate(ctx, &off, sizeof(off)) != 1 || */
 		    EVP_DigestUpdate(ctx, &ino, sizeof(ino)) != 1||
 		    EVP_DigestUpdate(ctx, &type, sizeof(type)) != 1) {
 			log_err("EVP_DigestUpdate() failed\n");
-- 
2.30.2

[PATCH 3/6] libimaevm: Rename variable returned from readlink to len

[Stefan Berger]

The variable returned from readlink is a length indicator of the
number of bytes placed into a buffer, not only an error. Leave
a note in the code that a zero-length link is also treated as an
error, besdies the usual -1.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 src/libimaevm.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/libimaevm.c b/src/libimaevm.c
index 0137884..9a6739b 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -213,15 +213,16 @@ static int add_dir_hash(const char *file, EVP_MD_CTX *ctx)
 
 static int add_link_hash(const char *path, EVP_MD_CTX *ctx)
 {
-	int err;
+	int len;
 	char buf[1024];
 
-	err = readlink(path, buf, sizeof(buf));
-	if (err <= 0)
+	len = readlink(path, buf, sizeof(buf));
+	/* 0-length links are also an error */
+	if (len <= 0)
 		return -1;
 
-	log_info("link: %s -> %.*s\n", path, err, buf);
-	return !EVP_DigestUpdate(ctx, buf, err);
+	log_info("link: %s -> %.*s\n", path, len, buf);
+	return !EVP_DigestUpdate(ctx, buf, len);
 }
 
 static int add_dev_hash(struct stat *st, EVP_MD_CTX *ctx)
-- 
2.30.2

Re: [PATCH 3/6] libimaevm: Rename variable returned from readlink to len

[Stefan Berger]

On 4/19/21 11:01 AM, Stefan Berger wrote:
> The variable returned from readlink is a length indicator of the
> number of bytes placed into a buffer, not only an error. Leave
> a note in the code that a zero-length link is also treated as an
> error, besdies the usual -1.

Is link signing supported by IMA in the kernel? Maybe I missed something 
when looking at the code in the Linux kernel, but I could not find that 
it was supported. What about directory signing and socket/device file 
signing?


    Stefan

[PATCH 4/6] libimaevm: Rename variable from cr to newline

[Stefan Berger]

Rename function variable from cr (carriage return, '\r') to
newline, because this is what it is.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 src/libimaevm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/libimaevm.c b/src/libimaevm.c
index 9a6739b..b40b6d8 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -90,14 +90,14 @@ struct libimaevm_params imaevm_params = {
 
 static void __attribute__ ((constructor)) libinit(void);
 
-void imaevm_do_hexdump(FILE *fp, const void *ptr, int len, bool cr)
+void imaevm_do_hexdump(FILE *fp, const void *ptr, int len, bool newline)
 {
 	int i;
 	uint8_t *data = (uint8_t *) ptr;
 
 	for (i = 0; i < len; i++)
 		fprintf(fp, "%02x", data[i]);
-	if (cr)
+	if (newline)
 		fprintf(fp, "\n");
 }
 
-- 
2.30.2

[PATCH 5/6] libimaevm: Report unsupported filetype using log_err

[Stefan Berger]

There's no errno set at this point so that using log_errno would
display something useful. Instead use log_error().

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 src/libimaevm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/libimaevm.c b/src/libimaevm.c
index b40b6d8..06f1063 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -286,7 +286,7 @@ int ima_calc_hash(const char *file, uint8_t *hash)
 		err = add_dev_hash(&st, pctx);
 		break;
 	default:
-		log_errno("Unsupported file type");
+		log_err("Unsupported file type (0x%x)", st.st_mode & S_IFMT);
 		err = -1;
 		goto err;
 	}
-- 
2.30.2

[PATCH 6/6] libimaevm: Use function parameter algo for name of hash

[Stefan Berger]

Instead of using the global variable imaevm_params.hash_algo as the
hash algo to use, use the algo parameter passed into the function.
Existing code in this function already uses 'algo' for writing the
has into the header:

        hdr->hash_algo = imaevm_get_hash_algo(algo);

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 src/libimaevm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/libimaevm.c b/src/libimaevm.c
index 06f1063..2856270 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -913,7 +913,7 @@ static int sign_hash_v2(const char *algo, const unsigned char *hash,
 		return -1;
 	}
 
-	log_info("hash(%s): ", imaevm_params.hash_algo);
+	log_info("hash(%s): ", algo);
 	log_dump(hash, size);
 
 	pkey = read_priv_pkey(keyfile, imaevm_params.keypass);
@@ -939,7 +939,7 @@ static int sign_hash_v2(const char *algo, const unsigned char *hash,
 	if (!EVP_PKEY_sign_init(ctx))
 		goto err;
 	st = "EVP_get_digestbyname";
-	if (!(md = EVP_get_digestbyname(imaevm_params.hash_algo)))
+	if (!(md = EVP_get_digestbyname(algo)))
 		goto err;
 	st = "EVP_PKEY_CTX_set_signature_md";
 	if (!EVP_PKEY_CTX_set_signature_md(ctx, md))
-- 
2.30.2

Re: [PATCH 0/6] ima-evm-utils: Some cleanups and bugfixes

[Mimi Zohar]

Hi Stefan,

On Mon, 2021-04-19 at 11:01 -0400, Stefan Berger wrote:
> This PR contains some cleanups and bugfixes for libimaevm of ima-evm-utils.

In the future, for ima-evm-utils patches, please prefix all patches
with "ima-evm-utils", not just the cover letter (e.g. --subject-
prefix="PATCH ima-evm-utils").

This patch set is now in next-testing.

thanks,

Mimi