* [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.