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