All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] ima-evm-utils: Some cleanups and bugfixes
@ 2021-04-19 15:01 Stefan Berger
  2021-04-19 15:01 ` [PATCH 1/6] libimaevm: Properly check for error returned by EVP_DigestUpdate Stefan Berger
                   ` (6 more replies)
  0 siblings, 7 replies; 9+ messages in thread
From: Stefan Berger @ 2021-04-19 15:01 UTC (permalink / raw)
  To: linux-integrity; +Cc: zohar, dmitry.kasatkin, 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


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

* [PATCH 1/6] libimaevm: Properly check for error returned by EVP_DigestUpdate
  2021-04-19 15:01 [PATCH 0/6] ima-evm-utils: Some cleanups and bugfixes Stefan Berger
@ 2021-04-19 15:01 ` Stefan Berger
  2021-04-19 15:01 ` [PATCH 2/6] libimaevm: Remove unused off variable Stefan Berger
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Stefan Berger @ 2021-04-19 15:01 UTC (permalink / raw)
  To: linux-integrity; +Cc: zohar, dmitry.kasatkin, 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


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

* [PATCH 2/6] libimaevm: Remove unused off variable
  2021-04-19 15:01 [PATCH 0/6] ima-evm-utils: Some cleanups and bugfixes Stefan Berger
  2021-04-19 15:01 ` [PATCH 1/6] libimaevm: Properly check for error returned by EVP_DigestUpdate Stefan Berger
@ 2021-04-19 15:01 ` Stefan Berger
  2021-04-19 15:01 ` [PATCH 3/6] libimaevm: Rename variable returned from readlink to len Stefan Berger
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Stefan Berger @ 2021-04-19 15:01 UTC (permalink / raw)
  To: linux-integrity; +Cc: zohar, dmitry.kasatkin, 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


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

* [PATCH 3/6] libimaevm: Rename variable returned from readlink to len
  2021-04-19 15:01 [PATCH 0/6] ima-evm-utils: Some cleanups and bugfixes Stefan Berger
  2021-04-19 15:01 ` [PATCH 1/6] libimaevm: Properly check for error returned by EVP_DigestUpdate Stefan Berger
  2021-04-19 15:01 ` [PATCH 2/6] libimaevm: Remove unused off variable Stefan Berger
@ 2021-04-19 15:01 ` Stefan Berger
  2021-04-19 15:22   ` Stefan Berger
  2021-04-19 15:01 ` [PATCH 4/6] libimaevm: Rename variable from cr to newline Stefan Berger
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 9+ messages in thread
From: Stefan Berger @ 2021-04-19 15:01 UTC (permalink / raw)
  To: linux-integrity; +Cc: zohar, dmitry.kasatkin, 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


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

* [PATCH 4/6] libimaevm: Rename variable from cr to newline
  2021-04-19 15:01 [PATCH 0/6] ima-evm-utils: Some cleanups and bugfixes Stefan Berger
                   ` (2 preceding siblings ...)
  2021-04-19 15:01 ` [PATCH 3/6] libimaevm: Rename variable returned from readlink to len Stefan Berger
@ 2021-04-19 15:01 ` Stefan Berger
  2021-04-19 15:01 ` [PATCH 5/6] libimaevm: Report unsupported filetype using log_err Stefan Berger
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Stefan Berger @ 2021-04-19 15:01 UTC (permalink / raw)
  To: linux-integrity; +Cc: zohar, dmitry.kasatkin, 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


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

* [PATCH 5/6] libimaevm: Report unsupported filetype using log_err
  2021-04-19 15:01 [PATCH 0/6] ima-evm-utils: Some cleanups and bugfixes Stefan Berger
                   ` (3 preceding siblings ...)
  2021-04-19 15:01 ` [PATCH 4/6] libimaevm: Rename variable from cr to newline Stefan Berger
@ 2021-04-19 15:01 ` Stefan Berger
  2021-04-19 15:01 ` [PATCH 6/6] libimaevm: Use function parameter algo for name of hash Stefan Berger
  2021-06-25 20:42 ` [PATCH 0/6] ima-evm-utils: Some cleanups and bugfixes Mimi Zohar
  6 siblings, 0 replies; 9+ messages in thread
From: Stefan Berger @ 2021-04-19 15:01 UTC (permalink / raw)
  To: linux-integrity; +Cc: zohar, dmitry.kasatkin, 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


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

* [PATCH 6/6] libimaevm: Use function parameter algo for name of hash
  2021-04-19 15:01 [PATCH 0/6] ima-evm-utils: Some cleanups and bugfixes Stefan Berger
                   ` (4 preceding siblings ...)
  2021-04-19 15:01 ` [PATCH 5/6] libimaevm: Report unsupported filetype using log_err Stefan Berger
@ 2021-04-19 15:01 ` Stefan Berger
  2021-06-25 20:42 ` [PATCH 0/6] ima-evm-utils: Some cleanups and bugfixes Mimi Zohar
  6 siblings, 0 replies; 9+ messages in thread
From: Stefan Berger @ 2021-04-19 15:01 UTC (permalink / raw)
  To: linux-integrity; +Cc: zohar, dmitry.kasatkin, 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


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

* Re: [PATCH 3/6] libimaevm: Rename variable returned from readlink to len
  2021-04-19 15:01 ` [PATCH 3/6] libimaevm: Rename variable returned from readlink to len Stefan Berger
@ 2021-04-19 15:22   ` Stefan Berger
  0 siblings, 0 replies; 9+ messages in thread
From: Stefan Berger @ 2021-04-19 15:22 UTC (permalink / raw)
  To: linux-integrity; +Cc: zohar, dmitry.kasatkin


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



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

* Re: [PATCH 0/6] ima-evm-utils: Some cleanups and bugfixes
  2021-04-19 15:01 [PATCH 0/6] ima-evm-utils: Some cleanups and bugfixes Stefan Berger
                   ` (5 preceding siblings ...)
  2021-04-19 15:01 ` [PATCH 6/6] libimaevm: Use function parameter algo for name of hash Stefan Berger
@ 2021-06-25 20:42 ` Mimi Zohar
  6 siblings, 0 replies; 9+ messages in thread
From: Mimi Zohar @ 2021-06-25 20:42 UTC (permalink / raw)
  To: Stefan Berger, linux-integrity; +Cc: dmitry.kasatkin

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


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

end of thread, other threads:[~2021-06-25 20:42 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-19 15:01 [PATCH 0/6] ima-evm-utils: Some cleanups and bugfixes Stefan Berger
2021-04-19 15:01 ` [PATCH 1/6] libimaevm: Properly check for error returned by EVP_DigestUpdate Stefan Berger
2021-04-19 15:01 ` [PATCH 2/6] libimaevm: Remove unused off variable Stefan Berger
2021-04-19 15:01 ` [PATCH 3/6] libimaevm: Rename variable returned from readlink to len Stefan Berger
2021-04-19 15:22   ` Stefan Berger
2021-04-19 15:01 ` [PATCH 4/6] libimaevm: Rename variable from cr to newline Stefan Berger
2021-04-19 15:01 ` [PATCH 5/6] libimaevm: Report unsupported filetype using log_err Stefan Berger
2021-04-19 15:01 ` [PATCH 6/6] libimaevm: Use function parameter algo for name of hash Stefan Berger
2021-06-25 20:42 ` [PATCH 0/6] ima-evm-utils: Some cleanups and bugfixes Mimi Zohar

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.