* [PATCH v3 2/7] ima-evm-utils: Define hash and sig buffer sizes and add asserts
2018-12-03 3:35 [PATCH v3 1/7] ima-evm-utils: Fix hash buffer overflow in verify_evm and hmac_evm Vitaly Chikunov
@ 2018-12-03 3:35 ` Vitaly Chikunov
2018-12-03 3:35 ` [PATCH v3 3/7] ima-evm-utils: Define the '--xattr-user' option for testing Vitaly Chikunov
` (5 subsequent siblings)
6 siblings, 0 replies; 18+ messages in thread
From: Vitaly Chikunov @ 2018-12-03 3:35 UTC (permalink / raw)
To: Mimi Zohar, Dmitry Kasatkin, linux-integrity
To prevent hash and sig buffers size mismatch, define their maximum
sizes and add sanity checking asserts.
Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
---
Changes since v1:
- New patch.
Changes since v2:
- Add single comment line to sign_evm.
src/evmctl.c | 35 ++++++++++++++++++++++-------------
src/imaevm.h | 3 +++
src/libimaevm.c | 4 +++-
3 files changed, 28 insertions(+), 14 deletions(-)
diff --git a/src/evmctl.c b/src/evmctl.c
index f8035da..f019a67 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -505,15 +505,17 @@ static int calc_evm_hash(const char *file, unsigned char *hash)
static int sign_evm(const char *file, const char *key)
{
- unsigned char hash[64];
- unsigned char sig[1024];
+ unsigned char hash[MAX_DIGEST_SIZE];
+ unsigned char sig[MAX_SIGNATURE_SIZE];
int len, err;
len = calc_evm_hash(file, hash);
+ assert(len <= sizeof(hash));
if (len <= 1)
return len;
len = sign_hash(params.hash_algo, hash, len, key, NULL, sig + 1);
+ assert(len < sizeof(sig));
if (len <= 1)
return len;
@@ -543,7 +545,7 @@ static int sign_evm(const char *file, const char *key)
static int hash_ima(const char *file)
{
- unsigned char hash[66]; /* MAX hash size + 2 */
+ unsigned char hash[MAX_DIGEST_SIZE + 2]; /* +2 byte xattr header */
int len, err, offset;
int algo = get_hash_algo(params.hash_algo);
@@ -557,6 +559,7 @@ static int hash_ima(const char *file)
}
len = ima_calc_hash(file, hash + offset);
+ assert(len + offset <= sizeof(hash));
if (len <= 1)
return len;
@@ -581,15 +584,17 @@ static int hash_ima(const char *file)
static int sign_ima(const char *file, const char *key)
{
- unsigned char hash[64];
- unsigned char sig[1024];
+ unsigned char hash[MAX_DIGEST_SIZE];
+ unsigned char sig[MAX_SIGNATURE_SIZE];
int len, err;
len = ima_calc_hash(file, hash);
+ assert(len <= sizeof(hash));
if (len <= 1)
return len;
len = sign_hash(params.hash_algo, hash, len, key, NULL, sig + 1);
+ assert(len < sizeof(sig));
if (len <= 1)
return len;
@@ -695,8 +700,8 @@ static int cmd_sign_hash(struct command *cmd)
int hashlen = 0;
size_t line_len;
ssize_t len;
- unsigned char hash[64];
- unsigned char sig[1024] = "\x03";
+ unsigned char hash[MAX_DIGEST_SIZE];
+ unsigned char sig[MAX_SIGNATURE_SIZE] = "\x03";
int siglen;
key = params.keyfile ? : "/etc/keys/privkey_evm.pem";
@@ -711,9 +716,11 @@ static int cmd_sign_hash(struct command *cmd)
token = strpbrk(line, ", \t");
hashlen = token ? token - line : strlen(line);
- hex2bin(hash, line, hashlen);
+ assert(hashlen / 2 <= sizeof(hash));
+ hex2bin(hash, line, hashlen / 2);
siglen = sign_hash(params.hash_algo, hash, hashlen/2,
key, NULL, sig + 1);
+ assert(siglen < sizeof(sig));
if (siglen <= 1)
return siglen;
@@ -761,8 +768,8 @@ static int cmd_sign_evm(struct command *cmd)
static int verify_evm(const char *file)
{
- unsigned char hash[64];
- unsigned char sig[1024];
+ unsigned char hash[MAX_DIGEST_SIZE];
+ unsigned char sig[MAX_SIGNATURE_SIZE];
int mdlen;
int len;
@@ -804,12 +811,13 @@ static int cmd_verify_evm(struct command *cmd)
static int verify_ima(const char *file)
{
- unsigned char sig[1024];
+ unsigned char sig[MAX_SIGNATURE_SIZE];
int len;
if (sigfile) {
void *tmp = file2bin(file, "sig", &len);
+ assert(len <= sizeof(sig));
memcpy(sig, tmp, len);
free(tmp);
} else {
@@ -1138,8 +1146,8 @@ out:
static int hmac_evm(const char *file, const char *key)
{
- unsigned char hash[64];
- unsigned char sig[1024];
+ unsigned char hash[MAX_DIGEST_SIZE];
+ unsigned char sig[MAX_SIGNATURE_SIZE];
int len, err;
len = calc_evm_hmac(file, key, hash);
@@ -1149,6 +1157,7 @@ static int hmac_evm(const char *file, const char *key)
log_info("hmac: ");
log_dump(hash, len);
+ assert(len < sizeof(sig));
memcpy(sig + 1, hash, len);
if (xattr) {
diff --git a/src/imaevm.h b/src/imaevm.h
index 1bafaad..2ebe7e7 100644
--- a/src/imaevm.h
+++ b/src/imaevm.h
@@ -75,6 +75,9 @@
#define DATA_SIZE 4096
#define SHA1_HASH_LEN 20
+#define MAX_DIGEST_SIZE 64
+#define MAX_SIGNATURE_SIZE 1024
+
#define __packed __attribute__((packed))
enum evm_ima_xattr_type {
diff --git a/src/libimaevm.c b/src/libimaevm.c
index 6fa0ed4..80b61a2 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -49,6 +49,7 @@
#include <dirent.h>
#include <string.h>
#include <stdio.h>
+#include <assert.h>
#include <openssl/pem.h>
#include <openssl/evp.h>
@@ -590,7 +591,7 @@ int verify_hash(const char *file, const unsigned char *hash, int size, unsigned
int ima_verify_signature(const char *file, unsigned char *sig, int siglen,
unsigned char *digest, int digestlen)
{
- unsigned char hash[64];
+ unsigned char hash[MAX_DIGEST_SIZE];
int hashlen, sig_hash_algo;
if (sig[0] != 0x03) {
@@ -614,6 +615,7 @@ int ima_verify_signature(const char *file, unsigned char *sig, int siglen,
return verify_hash(file, digest, digestlen, sig + 1, siglen - 1);
hashlen = ima_calc_hash(file, hash);
+ assert(hashlen <= sizeof(hash));
if (hashlen <= 1)
return hashlen;
--
2.11.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 3/7] ima-evm-utils: Define the '--xattr-user' option for testing
2018-12-03 3:35 [PATCH v3 1/7] ima-evm-utils: Fix hash buffer overflow in verify_evm and hmac_evm Vitaly Chikunov
2018-12-03 3:35 ` [PATCH v3 2/7] ima-evm-utils: Define hash and sig buffer sizes and add asserts Vitaly Chikunov
@ 2018-12-03 3:35 ` Vitaly Chikunov
2018-12-03 3:35 ` [PATCH v3 4/7] ima-evm-utils: Allow using Streebog hash function Vitaly Chikunov
` (4 subsequent siblings)
6 siblings, 0 replies; 18+ messages in thread
From: Vitaly Chikunov @ 2018-12-03 3:35 UTC (permalink / raw)
To: Mimi Zohar, Dmitry Kasatkin, linux-integrity
The IMA/EVM attributes are currently stored in the "security" namespace,
which requires root privileges. Storing the ima/evm attributes in the
"user" namespace, instead of the "security" namespace, would be useful
for debugging and testing purposes, and because "--sigfile" does not
work for evm signatures.
Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
---
Changes since v1:
- No code changes. Only the description is reworded.
Changes since v2:
- Update README.
README | 1 +
src/evmctl.c | 33 +++++++++++++++++++++------------
src/libimaevm.c | 2 +-
3 files changed, 23 insertions(+), 13 deletions(-)
diff --git a/README b/README
index 4805564..05cc2ff 100644
--- a/README
+++ b/README
@@ -44,6 +44,7 @@ OPTIONS
-s, --imasig make IMA signature
-d, --imahash make IMA hash
-f, --sigfile store IMA signature in .sig file instead of xattr
+ --xattr-user store xattrs in user namespace (for testing purposes)
--rsa use RSA key type and signing scheme v1
-k, --key path to signing key (default: /etc/keys/{privkey,pubkey}_evm.pem)
-o, --portable generate portable EVM signatures
diff --git a/src/evmctl.c b/src/evmctl.c
index f019a67..f4df027 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -145,6 +145,9 @@ static int find(const char *path, int dts, find_cb_t func);
struct command cmds[];
static void print_usage(struct command *cmd);
+static const char *xattr_ima = "security.ima";
+static const char *xattr_evm = "security.evm";
+
static int bin2file(const char *file, const char *ext, const unsigned char *data, int len)
{
FILE *fp;
@@ -533,7 +536,7 @@ static int sign_evm(const char *file, const char *key)
dump(sig, len);
if (xattr) {
- err = lsetxattr(file, "security.evm", sig, len, 0);
+ err = lsetxattr(file, xattr_evm, sig, len, 0);
if (err < 0) {
log_err("setxattr failed: %s\n", file);
return err;
@@ -572,7 +575,7 @@ static int hash_ima(const char *file)
dump(hash, len);
if (xattr) {
- err = lsetxattr(file, "security.ima", hash, len, 0);
+ err = lsetxattr(file, xattr_ima, hash, len, 0);
if (err < 0) {
log_err("setxattr failed: %s\n", file);
return err;
@@ -609,7 +612,7 @@ static int sign_ima(const char *file, const char *key)
bin2file(file, "sig", sig, len);
if (xattr) {
- err = lsetxattr(file, "security.ima", sig, len, 0);
+ err = lsetxattr(file, xattr_ima, sig, len, 0);
if (err < 0) {
log_err("setxattr failed: %s\n", file);
return err;
@@ -778,14 +781,14 @@ static int verify_evm(const char *file)
if (mdlen <= 1)
return mdlen;
- len = lgetxattr(file, "security.evm", sig, sizeof(sig));
+ len = lgetxattr(file, xattr_evm, sig, sizeof(sig));
if (len < 0) {
log_err("getxattr failed: %s\n", file);
return len;
}
if (sig[0] != 0x03) {
- log_err("security.evm has no signature\n");
+ log_err("%s has no signature\n", xattr_evm);
return -1;
}
@@ -821,7 +824,7 @@ static int verify_ima(const char *file)
memcpy(sig, tmp, len);
free(tmp);
} else {
- len = lgetxattr(file, "security.ima", sig, sizeof(sig));
+ len = lgetxattr(file, xattr_ima, sig, sizeof(sig));
if (len < 0) {
log_err("getxattr failed: %s\n", file);
return len;
@@ -964,7 +967,7 @@ static int setxattr_ima(const char *file, char *sig_file)
if (!sig)
return 0;
- err = lsetxattr(file, "security.ima", sig, len, 0);
+ err = lsetxattr(file, xattr_ima, sig, len, 0);
if (err < 0)
log_err("setxattr failed: %s\n", file);
free(sig);
@@ -1162,7 +1165,7 @@ static int hmac_evm(const char *file, const char *key)
if (xattr) {
sig[0] = EVM_XATTR_HMAC;
- err = lsetxattr(file, "security.evm", sig, len + 1, 0);
+ err = lsetxattr(file, xattr_evm, sig, len + 1, 0);
if (err < 0) {
log_err("setxattr failed: %s\n", file);
return err;
@@ -1218,9 +1221,9 @@ static int ima_fix(const char *path)
}
for (; size > 0; len++, size -= len, list += len) {
len = strlen(list);
- if (!strcmp(list, "security.ima"))
+ if (!strcmp(list, xattr_ima))
ima = 1;
- else if (!strcmp(list, "security.evm"))
+ else if (!strcmp(list, xattr_evm))
evm = 1;
}
if (ima && evm)
@@ -1297,8 +1300,8 @@ static int cmd_ima_fix(struct command *cmd)
static int ima_clear(const char *path)
{
log_info("%s\n", path);
- lremovexattr(path, "security.ima");
- lremovexattr(path, "security.evm");
+ lremovexattr(path, xattr_ima);
+ lremovexattr(path, xattr_evm);
return 0;
}
@@ -1654,6 +1657,7 @@ static void usage(void)
" -s, --imasig make IMA signature\n"
" -d, --imahash make IMA hash\n"
" -f, --sigfile store IMA signature in .sig file instead of xattr\n"
+ " --xattr-user store xattrs in user namespace (for testing purposes)\n"
" --rsa use RSA key type and signing scheme v1\n"
" -k, --key path to signing key (default: /etc/keys/{privkey,pubkey}_evm.pem)\n"
" -o, --portable generate portable EVM signatures\n"
@@ -1728,6 +1732,7 @@ static struct option opts[] = {
{"selinux", 1, 0, 136},
{"caps", 2, 0, 137},
{"list", 0, 0, 138},
+ {"xattr-user", 0, 0, 140},
{}
};
@@ -1879,6 +1884,10 @@ int main(int argc, char *argv[])
case 138:
measurement_list = 1;
break;
+ case 140: /* --xattr-user */
+ xattr_ima = "user.ima";
+ xattr_evm = "user.evm";
+ break;
case '?':
exit(1);
break;
diff --git a/src/libimaevm.c b/src/libimaevm.c
index 80b61a2..34501ca 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -595,7 +595,7 @@ int ima_verify_signature(const char *file, unsigned char *sig, int siglen,
int hashlen, sig_hash_algo;
if (sig[0] != 0x03) {
- log_err("security.ima has no signature\n");
+ log_err("xattr ima has no signature\n");
return -1;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 4/7] ima-evm-utils: Allow using Streebog hash function
2018-12-03 3:35 [PATCH v3 1/7] ima-evm-utils: Fix hash buffer overflow in verify_evm and hmac_evm Vitaly Chikunov
2018-12-03 3:35 ` [PATCH v3 2/7] ima-evm-utils: Define hash and sig buffer sizes and add asserts Vitaly Chikunov
2018-12-03 3:35 ` [PATCH v3 3/7] ima-evm-utils: Define the '--xattr-user' option for testing Vitaly Chikunov
@ 2018-12-03 3:35 ` Vitaly Chikunov
2018-12-03 3:35 ` [PATCH v3 5/7] ima-evm-utils: Preload OpenSSL engine via '--engine' option Vitaly Chikunov
` (3 subsequent siblings)
6 siblings, 0 replies; 18+ messages in thread
From: Vitaly Chikunov @ 2018-12-03 3:35 UTC (permalink / raw)
To: Mimi Zohar, Dmitry Kasatkin, linux-integrity
This patch will allow using GOST algorithms from OpenSSL's
gost-engine[1] via config extension (which is the usual way).
[1] https://github.com/gost-engine/engine
Full usage example:
1. Install the gost-engine package for your distro, this could be
libengine-gost-openssl1.1, openssl-gost-engine, or openssl-engines.
2. Edit openssl.cnf appropriately. Reference INSTALL.md of gost-engine
for the detailed instructions.
3. Then GOST algorithms should work:
$ cp /dev/null a
$ openssl dgst -streebog256 a
md_gost12_256(a)= 3f539a213e97c802cc229d474c6aa32a825a360b2a933a949fd925208d9ce1bb
$ evmctl -v ima_hash -a streebog256 --xattr-user a
hash: 04123f539a213e97c802cc229d474c6aa32a825a360b2a933a949fd925208d9ce1bb
$ getfattr -d -m. -ehex a
# file: a
user.ima=0x04123f539a213e97c802cc229d474c6aa32a825a360b2a933a949fd925208d9ce1bb
Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
---
Changes since v1:
- "--engine" option is removed into separate patch.
Changes since v2:
- Add usage example to description.
src/evmctl.c | 6 +++---
src/imaevm.h | 13 +++++++++++++
src/libimaevm.c | 15 +++++++++++----
3 files changed, 27 insertions(+), 7 deletions(-)
diff --git a/src/evmctl.c b/src/evmctl.c
index f4df027..1f6dad5 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -388,7 +388,7 @@ static int calc_evm_hash(const char *file, unsigned char *hash)
md = EVP_get_digestbyname(params.hash_algo);
if (!md) {
- log_err("EVP_get_digestbyname() failed\n");
+ log_err("EVP_get_digestbyname(%s) failed\n", params.hash_algo);
return 1;
}
@@ -1064,7 +1064,7 @@ static int calc_evm_hmac(const char *file, const char *keyfile, unsigned char *h
md = EVP_get_digestbyname(params.hash_algo);
if (!md) {
- log_err("EVP_get_digestbyname() failed\n");
+ log_err("EVP_get_digestbyname(%s) failed\n", params.hash_algo);
goto out;
}
@@ -1653,7 +1653,7 @@ static void usage(void)
printf(
"\n"
- " -a, --hashalgo sha1 (default), sha224, sha256, sha384, sha512\n"
+ " -a, --hashalgo sha1 (default), sha224, sha256, sha384, sha512, streebog256, streebog512\n"
" -s, --imasig make IMA signature\n"
" -d, --imahash make IMA hash\n"
" -f, --sigfile store IMA signature in .sig file instead of xattr\n"
diff --git a/src/imaevm.h b/src/imaevm.h
index 2ebe7e7..c81bf21 100644
--- a/src/imaevm.h
+++ b/src/imaevm.h
@@ -152,6 +152,7 @@ struct signature_hdr {
char mpi[0];
} __packed;
+/* reflect enum hash_algo from include/uapi/linux/hash_info.h */
enum pkey_hash_algo {
PKEY_HASH_MD4,
PKEY_HASH_MD5,
@@ -161,6 +162,18 @@ enum pkey_hash_algo {
PKEY_HASH_SHA384,
PKEY_HASH_SHA512,
PKEY_HASH_SHA224,
+ PKEY_HASH_RIPE_MD_128,
+ PKEY_HASH_RIPE_MD_256,
+ PKEY_HASH_RIPE_MD_320,
+ PKEY_HASH_WP_256,
+ PKEY_HASH_WP_384,
+ PKEY_HASH_WP_512,
+ PKEY_HASH_TGR_128,
+ PKEY_HASH_TGR_160,
+ PKEY_HASH_TGR_192,
+ PKEY_HASH_SM3_256,
+ PKEY_HASH_STREEBOG_256,
+ PKEY_HASH_STREEBOG_512,
PKEY_HASH__LAST
};
diff --git a/src/libimaevm.c b/src/libimaevm.c
index 34501ca..7b2b62c 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -51,6 +51,7 @@
#include <stdio.h>
#include <assert.h>
+#include <openssl/crypto.h>
#include <openssl/pem.h>
#include <openssl/evp.h>
#include <openssl/x509.h>
@@ -67,6 +68,8 @@ const char *const pkey_hash_algo[PKEY_HASH__LAST] = {
[PKEY_HASH_SHA384] = "sha384",
[PKEY_HASH_SHA512] = "sha512",
[PKEY_HASH_SHA224] = "sha224",
+ [PKEY_HASH_STREEBOG_256] = "streebog256",
+ [PKEY_HASH_STREEBOG_512] = "streebog512",
};
/*
@@ -291,7 +294,7 @@ int ima_calc_hash(const char *file, uint8_t *hash)
md = EVP_get_digestbyname(params.hash_algo);
if (!md) {
- log_err("EVP_get_digestbyname() failed\n");
+ log_err("EVP_get_digestbyname(%s) failed\n", params.hash_algo);
return 1;
}
@@ -509,14 +512,16 @@ int verify_hash_v2(const char *file, const unsigned char *hash, int size,
asn1 = &RSA_ASN1_templates[hdr->hash_algo];
if (len < asn1->size || memcmp(out, asn1->data, asn1->size)) {
- log_err("%s: verification failed: %d\n", file, err);
+ log_err("%s: verification failed: %d (asn1 mismatch)\n",
+ file, err);
return -1;
}
len -= asn1->size;
if (len != size || memcmp(out + asn1->size, hash, len)) {
- log_err("%s: verification failed: %d\n", file, err);
+ log_err("%s: verification failed: %d (digest mismatch)\n",
+ file, err);
return -1;
}
@@ -528,7 +533,8 @@ int get_hash_algo(const char *algo)
int i;
for (i = 0; i < PKEY_HASH__LAST; i++)
- if (!strcmp(algo, pkey_hash_algo[i]))
+ if (pkey_hash_algo[i] &&
+ !strcmp(algo, pkey_hash_algo[i]))
return i;
return PKEY_HASH_SHA1;
@@ -901,5 +907,6 @@ int sign_hash(const char *hashalgo, const unsigned char *hash, int size, const c
static void libinit()
{
OpenSSL_add_all_algorithms();
+ OPENSSL_add_all_algorithms_conf();
ERR_load_crypto_strings();
}
--
2.11.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 5/7] ima-evm-utils: Preload OpenSSL engine via '--engine' option
2018-12-03 3:35 [PATCH v3 1/7] ima-evm-utils: Fix hash buffer overflow in verify_evm and hmac_evm Vitaly Chikunov
` (2 preceding siblings ...)
2018-12-03 3:35 ` [PATCH v3 4/7] ima-evm-utils: Allow using Streebog hash function Vitaly Chikunov
@ 2018-12-03 3:35 ` Vitaly Chikunov
2018-12-03 3:35 ` [PATCH v3 6/7] ima-evm-utils: Extract digest algorithms from hash_info.h Vitaly Chikunov
` (2 subsequent siblings)
6 siblings, 0 replies; 18+ messages in thread
From: Vitaly Chikunov @ 2018-12-03 3:35 UTC (permalink / raw)
To: Mimi Zohar, Dmitry Kasatkin, linux-integrity
Another method of using GOST algorithms (and cryptographic accelerators)
is via direct preloading of appropriate engine using '--engine' option.
For the gost-engine it should be '--engine gost'.
Usage example:
1. Install gost-engine appropriately. (No need to edit openssl.cnf).
2. Then GOST algorithms should work:
# cp /dev/null a
# evmctl -v ima_hash --engine gost -a streebog256 a
hash: 04123f539a213e97c802cc229d474c6aa32a825a360b2a933a949fd925208d9ce1bb
Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
---
Changes since v1:
- Code split from prevously combined patch.
- More verbose OpenSSL error message.
Changes since v2:
- Update README.
- Add example engine name to the --help.
- Add usage example to description.
README | 1 +
src/evmctl.c | 23 +++++++++++++++++++++++
2 files changed, 24 insertions(+)
diff --git a/README b/README
index 05cc2ff..3603ae8 100644
--- a/README
+++ b/README
@@ -58,6 +58,7 @@ OPTIONS
--smack use extra SMACK xattrs for EVM
--m32 force EVM hmac/signature for 32 bit target system
--m64 force EVM hmac/signature for 64 bit target system
+ --engine e preload OpenSSL engine e (such as: gost)
-v increase verbosity level
-h, --help display this help and exit
diff --git a/src/evmctl.c b/src/evmctl.c
index 1f6dad5..0459798 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -62,6 +62,7 @@
#include <openssl/hmac.h>
#include <openssl/err.h>
#include <openssl/rsa.h>
+#include <openssl/engine.h>
#ifndef XATTR_APPAARMOR_SUFFIX
#define XATTR_APPARMOR_SUFFIX "apparmor"
@@ -1680,6 +1681,7 @@ static void usage(void)
" --selinux use custom Selinux label for EVM\n"
" --caps use custom Capabilities for EVM(unspecified: from FS, empty: do not use)\n"
" --list measurement list verification\n"
+ " --engine e preload OpenSSL engine e (such as: gost)\n"
" -v increase verbosity level\n"
" -h, --help display this help and exit\n"
"\n");
@@ -1732,6 +1734,7 @@ static struct option opts[] = {
{"selinux", 1, 0, 136},
{"caps", 2, 0, 137},
{"list", 0, 0, 138},
+ {"engine", 1, 0, 139},
{"xattr-user", 0, 0, 140},
{}
@@ -1774,6 +1777,7 @@ static char *get_password(void)
int main(int argc, char *argv[])
{
int err = 0, c, lind;
+ ENGINE *eng = NULL;
g_argv = argv;
g_argc = argc;
@@ -1884,6 +1888,18 @@ int main(int argc, char *argv[])
case 138:
measurement_list = 1;
break;
+ case 139: /* --engine e */
+ eng = ENGINE_by_id(optarg);
+ if (!eng) {
+ log_err("engine %s isn't available\n", optarg);
+ ERR_print_errors_fp(stderr);
+ } else if (!ENGINE_init(eng)) {
+ log_err("engine %s init failed\n", optarg);
+ ERR_print_errors_fp(stderr);
+ ENGINE_free(eng);
+ eng = NULL;
+ }
+ break;
case 140: /* --xattr-user */
xattr_ima = "user.ima";
xattr_evm = "user.evm";
@@ -1914,6 +1930,13 @@ int main(int argc, char *argv[])
}
}
+ if (eng) {
+ ENGINE_finish(eng);
+ ENGINE_free(eng);
+#if OPENSSL_API_COMPAT < 0x10100000L
+ ENGINE_cleanup();
+#endif
+ }
ERR_free_strings();
EVP_cleanup();
BIO_free(NULL);
--
2.11.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 6/7] ima-evm-utils: Extract digest algorithms from hash_info.h
2018-12-03 3:35 [PATCH v3 1/7] ima-evm-utils: Fix hash buffer overflow in verify_evm and hmac_evm Vitaly Chikunov
` (3 preceding siblings ...)
2018-12-03 3:35 ` [PATCH v3 5/7] ima-evm-utils: Preload OpenSSL engine via '--engine' option Vitaly Chikunov
@ 2018-12-03 3:35 ` Vitaly Chikunov
2018-12-03 3:35 ` [PATCH v3 7/7] ima-evm-utils: Try to load digest by its alias Vitaly Chikunov
2018-12-03 13:03 ` [PATCH v3 1/7] ima-evm-utils: Fix hash buffer overflow in verify_evm and hmac_evm Mimi Zohar
6 siblings, 0 replies; 18+ messages in thread
From: Vitaly Chikunov @ 2018-12-03 3:35 UTC (permalink / raw)
To: Mimi Zohar, Dmitry Kasatkin, linux-integrity
If configured with "--with-kernel-headers=PATH" try to extract hash
algorithms from "hash_info.h" from the kernel source tree or
kernel-headers package located in the specified path. (Otherwise, it
will be tried to get from the installed kernel.)
This also introduces two algorithm lists, one is built-in and another is
from the kernel source. (They should never contain conflicting algorithm
IDs by their append-only nature.) If the digest is not found in the
built-in list it will be searched in the list from kernel's
"hash_info.h".
This patch will allow evmctl to be just recompiled to work with digest
algorithms introduced in the newer kernels.
Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
---
Changes since v1:
- New patch.
Changes since v2:
- Update commit description.
- Update description of --with-kernel-headers.
configure.ac | 6 ++++++
src/Makefile.am | 6 ++++++
src/hash_info.gen | 43 +++++++++++++++++++++++++++++++++++++++++++
src/libimaevm.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
4 files changed, 98 insertions(+), 1 deletion(-)
create mode 100755 src/hash_info.gen
diff --git a/configure.ac b/configure.ac
index a5b4288..60f3684 100644
--- a/configure.ac
+++ b/configure.ac
@@ -27,12 +27,18 @@ AC_HEADER_STDC
PKG_CHECK_MODULES(OPENSSL, [ openssl >= 0.9.8 ])
AC_SUBST(OPENSSL_CFLAGS)
AC_SUBST(OPENSSL_LIBS)
+AC_SUBST(KERNEL_HEADERS)
AC_CHECK_HEADER(unistd.h)
AC_CHECK_HEADERS(openssl/conf.h)
AC_CHECK_HEADERS(sys/xattr.h, , [AC_MSG_ERROR([sys/xattr.h header not found. You need the c-library development package.])])
AC_CHECK_HEADERS(keyutils.h, , [AC_MSG_ERROR([keyutils.h header not found. You need the libkeyutils development package.])])
+AC_ARG_WITH(kernel_headers, [AS_HELP_STRING([--with-kernel-headers=PATH],
+ [specifies the Linux kernel-headers package location or kernel root directory you want to use])],
+ [KERNEL_HEADERS="$withval"],
+ [KERNEL_HEADERS=/lib/modules/$(uname -r)/source])
+
#debug support - yes for a while
PKG_ARG_ENABLE(debug, "yes", DEBUG, [Enable Debug support])
if test $pkg_cv_enable_debug = yes; then
diff --git a/src/Makefile.am b/src/Makefile.am
index deb18fb..d74fc6f 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -9,6 +9,11 @@ libimaevm_la_LIBADD = $(OPENSSL_LIBS)
include_HEADERS = imaevm.h
+nodist_libimaevm_la_SOURCES = hash_info.h
+BUILT_SOURCES = hash_info.h
+hash_info.h: Makefile
+ ./hash_info.gen $(KERNEL_HEADERS) >$@
+
bin_PROGRAMS = evmctl
evmctl_SOURCES = evmctl.c
@@ -18,5 +23,6 @@ evmctl_LDADD = $(OPENSSL_LIBS) -lkeyutils libimaevm.la
INCLUDES = -I$(top_srcdir) -include config.h
+CLEANFILES = hash_info.h
DISTCLEANFILES = @DISTCLEANFILES@
diff --git a/src/hash_info.gen b/src/hash_info.gen
new file mode 100755
index 0000000..60fc750
--- /dev/null
+++ b/src/hash_info.gen
@@ -0,0 +1,43 @@
+#!/bin/sh
+#
+# Generate hash_info.h from kernel headers
+#
+# Copyright (C) 2018 <vt@altlinux.org>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+
+KERNEL_HEADERS=$1
+HASH_INFO_H=uapi/linux/hash_info.h
+HASH_INFO=$KERNEL_HEADERS/include/$HASH_INFO_H
+
+# Allow to specify kernel-headers past include/
+if [ ! -e $HASH_INFO ]; then
+ HASH_INFO2=$KERNEL_HEADERS/$HASH_INFO_H
+ if [ -e $HASH_INFO2 ]; then
+ HASH_INFO=$HASH_INFO2
+ fi
+fi
+
+if [ ! -e $HASH_INFO ]; then
+ echo "/* $HASH_INFO is not found */"
+ HASH_INFO=/dev/null
+else
+ echo "/* $HASH_INFO is found */"
+fi
+
+echo "enum hash_algo {"
+grep HASH_ALGO_.*, $HASH_INFO
+printf "\tHASH_ALGO__LAST\n"
+echo "};"
+
+echo "const char *const hash_algo_name[HASH_ALGO__LAST] = {"
+sed -n 's/HASH_ALGO_\(.*\),/[HASH_ALGO_\1] = "\L\1\E",/p' $HASH_INFO
+echo "};"
diff --git a/src/libimaevm.c b/src/libimaevm.c
index 7b2b62c..cb4721b 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -50,6 +50,7 @@
#include <string.h>
#include <stdio.h>
#include <assert.h>
+#include <ctype.h>
#include <openssl/crypto.h>
#include <openssl/pem.h>
@@ -58,6 +59,7 @@
#include <openssl/err.h>
#include "imaevm.h"
+#include "hash_info.h"
const char *const pkey_hash_algo[PKEY_HASH__LAST] = {
[PKEY_HASH_MD4] = "md4",
@@ -153,6 +155,17 @@ void dump(const void *ptr, int len)
do_dump(stdout, ptr, len, true);
}
+const char *get_hash_algo_by_id(int algo)
+{
+ if (algo < PKEY_HASH__LAST)
+ return pkey_hash_algo[algo];
+ if (algo < HASH_ALGO__LAST)
+ return hash_algo_name[algo];
+
+ log_err("digest %d not found\n", algo);
+ return "unknown";
+}
+
int get_filesize(const char *filename)
{
struct stat stats;
@@ -528,15 +541,44 @@ int verify_hash_v2(const char *file, const unsigned char *hash, int size,
return 0;
}
+/* compare algo names case insensitively and ignoring separators */
+static int algocmp(const char *a, const char *b)
+{
+ while (*a && *b) {
+ int cha, chb;
+
+ cha = tolower((unsigned char)*a++);
+ if (!isalnum(cha))
+ continue;
+ chb = tolower((unsigned char)*b++);
+ if (!isalnum(chb)) {
+ a--;
+ continue;
+ }
+ if (cha != chb)
+ return -1;
+ }
+ return *a || *b;
+}
+
int get_hash_algo(const char *algo)
{
int i;
+ /* first iterate over builtin algorithms */
for (i = 0; i < PKEY_HASH__LAST; i++)
if (pkey_hash_algo[i] &&
!strcmp(algo, pkey_hash_algo[i]))
return i;
+ /* iterate over algorithms provided by kernel-headers */
+ for (i = 0; i < HASH_ALGO__LAST; i++) {
+ if (hash_algo_name[i] &&
+ !algocmp(algo, hash_algo_name[i]))
+ return i;
+ }
+
+ log_info("digest %s not found, fall back to sha1\n", algo);
return PKEY_HASH_SHA1;
}
@@ -611,7 +653,7 @@ int ima_verify_signature(const char *file, unsigned char *sig, int siglen,
return -1;
}
/* Use hash algorithm as retrieved from signature */
- params.hash_algo = pkey_hash_algo[sig_hash_algo];
+ params.hash_algo = get_hash_algo_by_id(sig_hash_algo);
/*
* Validate the signature based on the digest included in the
--
2.11.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 7/7] ima-evm-utils: Try to load digest by its alias
2018-12-03 3:35 [PATCH v3 1/7] ima-evm-utils: Fix hash buffer overflow in verify_evm and hmac_evm Vitaly Chikunov
` (4 preceding siblings ...)
2018-12-03 3:35 ` [PATCH v3 6/7] ima-evm-utils: Extract digest algorithms from hash_info.h Vitaly Chikunov
@ 2018-12-03 3:35 ` Vitaly Chikunov
2019-02-11 17:38 ` Mimi Zohar
2018-12-03 13:03 ` [PATCH v3 1/7] ima-evm-utils: Fix hash buffer overflow in verify_evm and hmac_evm Mimi Zohar
6 siblings, 1 reply; 18+ messages in thread
From: Vitaly Chikunov @ 2018-12-03 3:35 UTC (permalink / raw)
To: Mimi Zohar, Dmitry Kasatkin, linux-integrity
For compatibility with older OpenSSL try to load digest by its alias if
load by its proper name is failed.
This is configured in pkey_hash_algo by mentioning loadable alias first in the
comma separated list of algo names.
Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
---
Changes since v1:
- New patch.
Changes since v2:
- No changes.
src/imaevm.h | 1 +
src/libimaevm.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++----
2 files changed, 52 insertions(+), 4 deletions(-)
diff --git a/src/imaevm.h b/src/imaevm.h
index c81bf21..795966a 100644
--- a/src/imaevm.h
+++ b/src/imaevm.h
@@ -75,6 +75,7 @@
#define DATA_SIZE 4096
#define SHA1_HASH_LEN 20
+#define MAX_DIGEST_NAME 32
#define MAX_DIGEST_SIZE 64
#define MAX_SIGNATURE_SIZE 1024
diff --git a/src/libimaevm.c b/src/libimaevm.c
index cb4721b..7501303 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -41,6 +41,7 @@
/* should we use logger instead for library? */
#define USE_FPRINTF
+#define _GNU_SOURCE
#include <sys/types.h>
#include <sys/param.h>
#include <sys/stat.h>
@@ -56,6 +57,7 @@
#include <openssl/pem.h>
#include <openssl/evp.h>
#include <openssl/x509.h>
+#include <openssl/engine.h>
#include <openssl/err.h>
#include "imaevm.h"
@@ -70,8 +72,8 @@ const char *const pkey_hash_algo[PKEY_HASH__LAST] = {
[PKEY_HASH_SHA384] = "sha384",
[PKEY_HASH_SHA512] = "sha512",
[PKEY_HASH_SHA224] = "sha224",
- [PKEY_HASH_STREEBOG_256] = "streebog256",
- [PKEY_HASH_STREEBOG_512] = "streebog512",
+ [PKEY_HASH_STREEBOG_256] = "md_gost12_256,streebog256",
+ [PKEY_HASH_STREEBOG_512] = "md_gost12_512,streebog512",
};
/*
@@ -284,6 +286,35 @@ static int add_dev_hash(struct stat *st, EVP_MD_CTX *ctx)
return !EVP_DigestUpdate(ctx, &dev, sizeof(dev));
}
+/* Call EVP_get_digestbyname with provided name and with alias,
+ * which is first name in the comma separated list of names
+ * in pkey_hash_algo.
+ */
+static const EVP_MD *get_digestbyname(const char *name)
+{
+ const EVP_MD *md;
+
+ md = EVP_get_digestbyname(params.hash_algo);
+ if (!md) {
+ char alias[MAX_DIGEST_NAME];
+ int len;
+ int algo_id;
+ const char *algo_alias;
+
+ /* try to get algo by its alias */
+ algo_id = get_hash_algo(params.hash_algo);
+ algo_alias = get_hash_algo_by_id(algo_id);
+ len = strchrnul(algo_alias, ',') - algo_alias;
+ if (len < sizeof(alias)) {
+ memcpy(alias, algo_alias, len);
+ alias[len] = '\0';
+ if (strcmp(params.hash_algo, alias))
+ md = EVP_get_digestbyname(alias);
+ }
+ }
+ return md;
+}
+
int ima_calc_hash(const char *file, uint8_t *hash)
{
const EVP_MD *md;
@@ -305,7 +336,7 @@ int ima_calc_hash(const char *file, uint8_t *hash)
return err;
}
- md = EVP_get_digestbyname(params.hash_algo);
+ md = get_digestbyname(params.hash_algo);
if (!md) {
log_err("EVP_get_digestbyname(%s) failed\n", params.hash_algo);
return 1;
@@ -561,6 +592,22 @@ static int algocmp(const char *a, const char *b)
return *a || *b;
}
+static int strmatch(const char *needle, const char *haystack)
+{
+ int len = strlen(needle);
+ const char *p = haystack;
+ const char *t;
+
+ for (t = strchrnul(p, ','); *p; p = t, t = strchrnul(p, ',')) {
+ if (t - p == len &&
+ !strncmp(needle, p, len))
+ return 0;
+ if (!*t++)
+ break;
+ }
+ return 1;
+}
+
int get_hash_algo(const char *algo)
{
int i;
@@ -568,7 +615,7 @@ int get_hash_algo(const char *algo)
/* first iterate over builtin algorithms */
for (i = 0; i < PKEY_HASH__LAST; i++)
if (pkey_hash_algo[i] &&
- !strcmp(algo, pkey_hash_algo[i]))
+ !strmatch(algo, pkey_hash_algo[i]))
return i;
/* iterate over algorithms provided by kernel-headers */
--
2.11.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v3 7/7] ima-evm-utils: Try to load digest by its alias
2018-12-03 3:35 ` [PATCH v3 7/7] ima-evm-utils: Try to load digest by its alias Vitaly Chikunov
@ 2019-02-11 17:38 ` Mimi Zohar
2019-02-11 17:52 ` Vitaly Chikunov
0 siblings, 1 reply; 18+ messages in thread
From: Mimi Zohar @ 2019-02-11 17:38 UTC (permalink / raw)
To: Vitaly Chikunov, Mimi Zohar, Dmitry Kasatkin, linux-integrity
Hi Vitaly,
On Mon, 2018-12-03 at 06:35 +0300, Vitaly Chikunov wrote:
> For compatibility with older OpenSSL try to load digest by its alias if
> load by its proper name is failed.
>
> This is configured in pkey_hash_algo by mentioning loadable alias first in the
> comma separated list of algo names.
After this patch, I can not verify the signature. It's failing to
find the hash algorithm.
evmctl ima_sign -k <private key> -p -a streebog256 ./foo.txt --xattr-user
evmctl ima_verify -k <public key> ./foo.txt --xattr-user
Mimi
>
> Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
> ---
> Changes since v1:
> - New patch.
> Changes since v2:
> - No changes.
>
> src/imaevm.h | 1 +
> src/libimaevm.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++----
> 2 files changed, 52 insertions(+), 4 deletions(-)
>
> diff --git a/src/imaevm.h b/src/imaevm.h
> index c81bf21..795966a 100644
> --- a/src/imaevm.h
> +++ b/src/imaevm.h
> @@ -75,6 +75,7 @@
> #define DATA_SIZE 4096
> #define SHA1_HASH_LEN 20
>
> +#define MAX_DIGEST_NAME 32
> #define MAX_DIGEST_SIZE 64
> #define MAX_SIGNATURE_SIZE 1024
>
> diff --git a/src/libimaevm.c b/src/libimaevm.c
> index cb4721b..7501303 100644
> --- a/src/libimaevm.c
> +++ b/src/libimaevm.c
> @@ -41,6 +41,7 @@
> /* should we use logger instead for library? */
> #define USE_FPRINTF
>
> +#define _GNU_SOURCE
> #include <sys/types.h>
> #include <sys/param.h>
> #include <sys/stat.h>
> @@ -56,6 +57,7 @@
> #include <openssl/pem.h>
> #include <openssl/evp.h>
> #include <openssl/x509.h>
> +#include <openssl/engine.h>
> #include <openssl/err.h>
>
> #include "imaevm.h"
> @@ -70,8 +72,8 @@ const char *const pkey_hash_algo[PKEY_HASH__LAST] = {
> [PKEY_HASH_SHA384] = "sha384",
> [PKEY_HASH_SHA512] = "sha512",
> [PKEY_HASH_SHA224] = "sha224",
> - [PKEY_HASH_STREEBOG_256] = "streebog256",
> - [PKEY_HASH_STREEBOG_512] = "streebog512",
> + [PKEY_HASH_STREEBOG_256] = "md_gost12_256,streebog256",
> + [PKEY_HASH_STREEBOG_512] = "md_gost12_512,streebog512",
> };
>
> /*
> @@ -284,6 +286,35 @@ static int add_dev_hash(struct stat *st, EVP_MD_CTX *ctx)
> return !EVP_DigestUpdate(ctx, &dev, sizeof(dev));
> }
>
> +/* Call EVP_get_digestbyname with provided name and with alias,
> + * which is first name in the comma separated list of names
> + * in pkey_hash_algo.
> + */
> +static const EVP_MD *get_digestbyname(const char *name)
> +{
> + const EVP_MD *md;
> +
> + md = EVP_get_digestbyname(params.hash_algo);
> + if (!md) {
> + char alias[MAX_DIGEST_NAME];
> + int len;
> + int algo_id;
> + const char *algo_alias;
> +
> + /* try to get algo by its alias */
> + algo_id = get_hash_algo(params.hash_algo);
> + algo_alias = get_hash_algo_by_id(algo_id);
> + len = strchrnul(algo_alias, ',') - algo_alias;
> + if (len < sizeof(alias)) {
> + memcpy(alias, algo_alias, len);
> + alias[len] = '\0';
> + if (strcmp(params.hash_algo, alias))
> + md = EVP_get_digestbyname(alias);
> + }
> + }
> + return md;
> +}
> +
> int ima_calc_hash(const char *file, uint8_t *hash)
> {
> const EVP_MD *md;
> @@ -305,7 +336,7 @@ int ima_calc_hash(const char *file, uint8_t *hash)
> return err;
> }
>
> - md = EVP_get_digestbyname(params.hash_algo);
> + md = get_digestbyname(params.hash_algo);
> if (!md) {
> log_err("EVP_get_digestbyname(%s) failed\n", params.hash_algo);
> return 1;
> @@ -561,6 +592,22 @@ static int algocmp(const char *a, const char *b)
> return *a || *b;
> }
>
> +static int strmatch(const char *needle, const char *haystack)
> +{
> + int len = strlen(needle);
> + const char *p = haystack;
> + const char *t;
> +
> + for (t = strchrnul(p, ','); *p; p = t, t = strchrnul(p, ',')) {
> + if (t - p == len &&
> + !strncmp(needle, p, len))
> + return 0;
> + if (!*t++)
> + break;
> + }
> + return 1;
> +}
> +
> int get_hash_algo(const char *algo)
> {
> int i;
> @@ -568,7 +615,7 @@ int get_hash_algo(const char *algo)
> /* first iterate over builtin algorithms */
> for (i = 0; i < PKEY_HASH__LAST; i++)
> if (pkey_hash_algo[i] &&
> - !strcmp(algo, pkey_hash_algo[i]))
> + !strmatch(algo, pkey_hash_algo[i]))
> return i;
>
> /* iterate over algorithms provided by kernel-headers */
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 7/7] ima-evm-utils: Try to load digest by its alias
2019-02-11 17:38 ` Mimi Zohar
@ 2019-02-11 17:52 ` Vitaly Chikunov
2019-02-11 17:59 ` Mimi Zohar
0 siblings, 1 reply; 18+ messages in thread
From: Vitaly Chikunov @ 2019-02-11 17:52 UTC (permalink / raw)
To: Mimi Zohar; +Cc: Mimi Zohar, Dmitry Kasatkin, linux-integrity
Mimi,
On Mon, Feb 11, 2019 at 12:38:58PM -0500, Mimi Zohar wrote:
> On Mon, 2018-12-03 at 06:35 +0300, Vitaly Chikunov wrote:
> > For compatibility with older OpenSSL try to load digest by its alias if
> > load by its proper name is failed.
> >
> > This is configured in pkey_hash_algo by mentioning loadable alias first in the
> > comma separated list of algo names.
>
> After this patch, I can not verify the signature. It's failing to
> find the hash algorithm.
Yes, it's fixed in "ima-evm-utils: convert sign v2 from RSA to EVP_PKEY
API". I didn't consider it a problem since streebog256 should not work
for sign/verify anyway (since RSA should not support it). That EVP_PKEY
patch which adds ability to sign/verify for Streebog also fixes above
problem.
> evmctl ima_sign -k <private key> -p -a streebog256 ./foo.txt --xattr-user
> evmctl ima_verify -k <public key> ./foo.txt --xattr-user
>
> Mimi
>
> >
> > Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
> > ---
> > Changes since v1:
> > - New patch.
> > Changes since v2:
> > - No changes.
> >
> > src/imaevm.h | 1 +
> > src/libimaevm.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++----
> > 2 files changed, 52 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/imaevm.h b/src/imaevm.h
> > index c81bf21..795966a 100644
> > --- a/src/imaevm.h
> > +++ b/src/imaevm.h
> > @@ -75,6 +75,7 @@
> > #define DATA_SIZE 4096
> > #define SHA1_HASH_LEN 20
> >
> > +#define MAX_DIGEST_NAME 32
> > #define MAX_DIGEST_SIZE 64
> > #define MAX_SIGNATURE_SIZE 1024
> >
> > diff --git a/src/libimaevm.c b/src/libimaevm.c
> > index cb4721b..7501303 100644
> > --- a/src/libimaevm.c
> > +++ b/src/libimaevm.c
> > @@ -41,6 +41,7 @@
> > /* should we use logger instead for library? */
> > #define USE_FPRINTF
> >
> > +#define _GNU_SOURCE
> > #include <sys/types.h>
> > #include <sys/param.h>
> > #include <sys/stat.h>
> > @@ -56,6 +57,7 @@
> > #include <openssl/pem.h>
> > #include <openssl/evp.h>
> > #include <openssl/x509.h>
> > +#include <openssl/engine.h>
> > #include <openssl/err.h>
> >
> > #include "imaevm.h"
> > @@ -70,8 +72,8 @@ const char *const pkey_hash_algo[PKEY_HASH__LAST] = {
> > [PKEY_HASH_SHA384] = "sha384",
> > [PKEY_HASH_SHA512] = "sha512",
> > [PKEY_HASH_SHA224] = "sha224",
> > - [PKEY_HASH_STREEBOG_256] = "streebog256",
> > - [PKEY_HASH_STREEBOG_512] = "streebog512",
> > + [PKEY_HASH_STREEBOG_256] = "md_gost12_256,streebog256",
> > + [PKEY_HASH_STREEBOG_512] = "md_gost12_512,streebog512",
> > };
> >
> > /*
> > @@ -284,6 +286,35 @@ static int add_dev_hash(struct stat *st, EVP_MD_CTX *ctx)
> > return !EVP_DigestUpdate(ctx, &dev, sizeof(dev));
> > }
> >
> > +/* Call EVP_get_digestbyname with provided name and with alias,
> > + * which is first name in the comma separated list of names
> > + * in pkey_hash_algo.
> > + */
> > +static const EVP_MD *get_digestbyname(const char *name)
> > +{
> > + const EVP_MD *md;
> > +
> > + md = EVP_get_digestbyname(params.hash_algo);
> > + if (!md) {
> > + char alias[MAX_DIGEST_NAME];
> > + int len;
> > + int algo_id;
> > + const char *algo_alias;
> > +
> > + /* try to get algo by its alias */
> > + algo_id = get_hash_algo(params.hash_algo);
> > + algo_alias = get_hash_algo_by_id(algo_id);
> > + len = strchrnul(algo_alias, ',') - algo_alias;
> > + if (len < sizeof(alias)) {
> > + memcpy(alias, algo_alias, len);
> > + alias[len] = '\0';
> > + if (strcmp(params.hash_algo, alias))
> > + md = EVP_get_digestbyname(alias);
> > + }
> > + }
> > + return md;
> > +}
> > +
> > int ima_calc_hash(const char *file, uint8_t *hash)
> > {
> > const EVP_MD *md;
> > @@ -305,7 +336,7 @@ int ima_calc_hash(const char *file, uint8_t *hash)
> > return err;
> > }
> >
> > - md = EVP_get_digestbyname(params.hash_algo);
> > + md = get_digestbyname(params.hash_algo);
> > if (!md) {
> > log_err("EVP_get_digestbyname(%s) failed\n", params.hash_algo);
> > return 1;
> > @@ -561,6 +592,22 @@ static int algocmp(const char *a, const char *b)
> > return *a || *b;
> > }
> >
> > +static int strmatch(const char *needle, const char *haystack)
> > +{
> > + int len = strlen(needle);
> > + const char *p = haystack;
> > + const char *t;
> > +
> > + for (t = strchrnul(p, ','); *p; p = t, t = strchrnul(p, ',')) {
> > + if (t - p == len &&
> > + !strncmp(needle, p, len))
> > + return 0;
> > + if (!*t++)
> > + break;
> > + }
> > + return 1;
> > +}
> > +
> > int get_hash_algo(const char *algo)
> > {
> > int i;
> > @@ -568,7 +615,7 @@ int get_hash_algo(const char *algo)
> > /* first iterate over builtin algorithms */
> > for (i = 0; i < PKEY_HASH__LAST; i++)
> > if (pkey_hash_algo[i] &&
> > - !strcmp(algo, pkey_hash_algo[i]))
> > + !strmatch(algo, pkey_hash_algo[i]))
> > return i;
> >
> > /* iterate over algorithms provided by kernel-headers */
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 7/7] ima-evm-utils: Try to load digest by its alias
2019-02-11 17:52 ` Vitaly Chikunov
@ 2019-02-11 17:59 ` Mimi Zohar
2019-02-11 18:13 ` Vitaly Chikunov
0 siblings, 1 reply; 18+ messages in thread
From: Mimi Zohar @ 2019-02-11 17:59 UTC (permalink / raw)
To: Vitaly Chikunov; +Cc: Mimi Zohar, Dmitry Kasatkin, linux-integrity
On Mon, 2019-02-11 at 20:52 +0300, Vitaly Chikunov wrote:
> Mimi,
>
> On Mon, Feb 11, 2019 at 12:38:58PM -0500, Mimi Zohar wrote:
> > On Mon, 2018-12-03 at 06:35 +0300, Vitaly Chikunov wrote:
> > > For compatibility with older OpenSSL try to load digest by its alias if
> > > load by its proper name is failed.
> > >
> > > This is configured in pkey_hash_algo by mentioning loadable alias first in the
> > > comma separated list of algo names.
> >
> > After this patch, I can not verify the signature. It's failing to
> > find the hash algorithm.
>
> Yes, it's fixed in "ima-evm-utils: convert sign v2 from RSA to EVP_PKEY
> API". I didn't consider it a problem since streebog256 should not work
> for sign/verify anyway (since RSA should not support it). That EVP_PKEY
> patch which adds ability to sign/verify for Streebog also fixes above
> problem.
I've been having second thoughts about this patch in general, as it
made the hash algorithm comparison unnecessarily more complex for the
simple case. As there hasn't been a new ima-evm-utils release with
patch, perhaps we should simple remove/revert it?
Mimi
>
> > evmctl ima_sign -k <private key> -p -a streebog256 ./foo.txt --xattr-user
> > evmctl ima_verify -k <public key> ./foo.txt --xattr-user
> >
> > Mimi
> >
> > >
> > > Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
> > > ---
> > > Changes since v1:
> > > - New patch.
> > > Changes since v2:
> > > - No changes.
> > >
> > > src/imaevm.h | 1 +
> > > src/libimaevm.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++----
> > > 2 files changed, 52 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/src/imaevm.h b/src/imaevm.h
> > > index c81bf21..795966a 100644
> > > --- a/src/imaevm.h
> > > +++ b/src/imaevm.h
> > > @@ -75,6 +75,7 @@
> > > #define DATA_SIZE 4096
> > > #define SHA1_HASH_LEN 20
> > >
> > > +#define MAX_DIGEST_NAME 32
> > > #define MAX_DIGEST_SIZE 64
> > > #define MAX_SIGNATURE_SIZE 1024
> > >
> > > diff --git a/src/libimaevm.c b/src/libimaevm.c
> > > index cb4721b..7501303 100644
> > > --- a/src/libimaevm.c
> > > +++ b/src/libimaevm.c
> > > @@ -41,6 +41,7 @@
> > > /* should we use logger instead for library? */
> > > #define USE_FPRINTF
> > >
> > > +#define _GNU_SOURCE
> > > #include <sys/types.h>
> > > #include <sys/param.h>
> > > #include <sys/stat.h>
> > > @@ -56,6 +57,7 @@
> > > #include <openssl/pem.h>
> > > #include <openssl/evp.h>
> > > #include <openssl/x509.h>
> > > +#include <openssl/engine.h>
> > > #include <openssl/err.h>
> > >
> > > #include "imaevm.h"
> > > @@ -70,8 +72,8 @@ const char *const pkey_hash_algo[PKEY_HASH__LAST] = {
> > > [PKEY_HASH_SHA384] = "sha384",
> > > [PKEY_HASH_SHA512] = "sha512",
> > > [PKEY_HASH_SHA224] = "sha224",
> > > - [PKEY_HASH_STREEBOG_256] = "streebog256",
> > > - [PKEY_HASH_STREEBOG_512] = "streebog512",
> > > + [PKEY_HASH_STREEBOG_256] = "md_gost12_256,streebog256",
> > > + [PKEY_HASH_STREEBOG_512] = "md_gost12_512,streebog512",
> > > };
> > >
> > > /*
> > > @@ -284,6 +286,35 @@ static int add_dev_hash(struct stat *st, EVP_MD_CTX *ctx)
> > > return !EVP_DigestUpdate(ctx, &dev, sizeof(dev));
> > > }
> > >
> > > +/* Call EVP_get_digestbyname with provided name and with alias,
> > > + * which is first name in the comma separated list of names
> > > + * in pkey_hash_algo.
> > > + */
> > > +static const EVP_MD *get_digestbyname(const char *name)
> > > +{
> > > + const EVP_MD *md;
> > > +
> > > + md = EVP_get_digestbyname(params.hash_algo);
> > > + if (!md) {
> > > + char alias[MAX_DIGEST_NAME];
> > > + int len;
> > > + int algo_id;
> > > + const char *algo_alias;
> > > +
> > > + /* try to get algo by its alias */
> > > + algo_id = get_hash_algo(params.hash_algo);
> > > + algo_alias = get_hash_algo_by_id(algo_id);
> > > + len = strchrnul(algo_alias, ',') - algo_alias;
> > > + if (len < sizeof(alias)) {
> > > + memcpy(alias, algo_alias, len);
> > > + alias[len] = '\0';
> > > + if (strcmp(params.hash_algo, alias))
> > > + md = EVP_get_digestbyname(alias);
> > > + }
> > > + }
> > > + return md;
> > > +}
> > > +
> > > int ima_calc_hash(const char *file, uint8_t *hash)
> > > {
> > > const EVP_MD *md;
> > > @@ -305,7 +336,7 @@ int ima_calc_hash(const char *file, uint8_t *hash)
> > > return err;
> > > }
> > >
> > > - md = EVP_get_digestbyname(params.hash_algo);
> > > + md = get_digestbyname(params.hash_algo);
> > > if (!md) {
> > > log_err("EVP_get_digestbyname(%s) failed\n", params.hash_algo);
> > > return 1;
> > > @@ -561,6 +592,22 @@ static int algocmp(const char *a, const char *b)
> > > return *a || *b;
> > > }
> > >
> > > +static int strmatch(const char *needle, const char *haystack)
> > > +{
> > > + int len = strlen(needle);
> > > + const char *p = haystack;
> > > + const char *t;
> > > +
> > > + for (t = strchrnul(p, ','); *p; p = t, t = strchrnul(p, ',')) {
> > > + if (t - p == len &&
> > > + !strncmp(needle, p, len))
> > > + return 0;
> > > + if (!*t++)
> > > + break;
> > > + }
> > > + return 1;
> > > +}
> > > +
> > > int get_hash_algo(const char *algo)
> > > {
> > > int i;
> > > @@ -568,7 +615,7 @@ int get_hash_algo(const char *algo)
> > > /* first iterate over builtin algorithms */
> > > for (i = 0; i < PKEY_HASH__LAST; i++)
> > > if (pkey_hash_algo[i] &&
> > > - !strcmp(algo, pkey_hash_algo[i]))
> > > + !strmatch(algo, pkey_hash_algo[i]))
> > > return i;
> > >
> > > /* iterate over algorithms provided by kernel-headers */
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 7/7] ima-evm-utils: Try to load digest by its alias
2019-02-11 17:59 ` Mimi Zohar
@ 2019-02-11 18:13 ` Vitaly Chikunov
2019-02-11 18:21 ` Vitaly Chikunov
0 siblings, 1 reply; 18+ messages in thread
From: Vitaly Chikunov @ 2019-02-11 18:13 UTC (permalink / raw)
To: Mimi Zohar; +Cc: Mimi Zohar, Dmitry Kasatkin, linux-integrity
Mimi,
On Mon, Feb 11, 2019 at 12:59:12PM -0500, Mimi Zohar wrote:
> On Mon, 2019-02-11 at 20:52 +0300, Vitaly Chikunov wrote:
> > On Mon, Feb 11, 2019 at 12:38:58PM -0500, Mimi Zohar wrote:
> > > On Mon, 2018-12-03 at 06:35 +0300, Vitaly Chikunov wrote:
> > > > For compatibility with older OpenSSL try to load digest by its alias if
> > > > load by its proper name is failed.
> > > >
> > > > This is configured in pkey_hash_algo by mentioning loadable alias first in the
> > > > comma separated list of algo names.
> > >
> > > After this patch, I can not verify the signature. It's failing to
> > > find the hash algorithm.
> >
> > Yes, it's fixed in "ima-evm-utils: convert sign v2 from RSA to EVP_PKEY
> > API". I didn't consider it a problem since streebog256 should not work
> > for sign/verify anyway (since RSA should not support it). That EVP_PKEY
> > patch which adds ability to sign/verify for Streebog also fixes above
> > problem.
>
> I've been having second thoughts about this patch in general, as it
> made the hash algorithm comparison unnecessarily more complex for the
> simple case. As there hasn't been a new ima-evm-utils release with
> patch, perhaps we should simple remove/revert it?
It was only for compatibility with older openssl/gost-engine, where
"streebog256" name is not defined yet. If you don't want to allow that
users to use Streebog feel free to revert it.
I was not found this mistake at the time, since Streebog should only be
used in ima_hash and not in ima_sign/verify (which requires RSA support
which I intentionally didn't add, planning to add EC-RDSA support later),
so that code path was not tested.
>
> Mimi
>
> >
> > > evmctl ima_sign -k <private key> -p -a streebog256 ./foo.txt --xattr-user
> > > evmctl ima_verify -k <public key> ./foo.txt --xattr-user
> > >
> > > Mimi
> > >
> > > >
> > > > Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
> > > > ---
> > > > Changes since v1:
> > > > - New patch.
> > > > Changes since v2:
> > > > - No changes.
> > > >
> > > > src/imaevm.h | 1 +
> > > > src/libimaevm.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++----
> > > > 2 files changed, 52 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/src/imaevm.h b/src/imaevm.h
> > > > index c81bf21..795966a 100644
> > > > --- a/src/imaevm.h
> > > > +++ b/src/imaevm.h
> > > > @@ -75,6 +75,7 @@
> > > > #define DATA_SIZE 4096
> > > > #define SHA1_HASH_LEN 20
> > > >
> > > > +#define MAX_DIGEST_NAME 32
> > > > #define MAX_DIGEST_SIZE 64
> > > > #define MAX_SIGNATURE_SIZE 1024
> > > >
> > > > diff --git a/src/libimaevm.c b/src/libimaevm.c
> > > > index cb4721b..7501303 100644
> > > > --- a/src/libimaevm.c
> > > > +++ b/src/libimaevm.c
> > > > @@ -41,6 +41,7 @@
> > > > /* should we use logger instead for library? */
> > > > #define USE_FPRINTF
> > > >
> > > > +#define _GNU_SOURCE
> > > > #include <sys/types.h>
> > > > #include <sys/param.h>
> > > > #include <sys/stat.h>
> > > > @@ -56,6 +57,7 @@
> > > > #include <openssl/pem.h>
> > > > #include <openssl/evp.h>
> > > > #include <openssl/x509.h>
> > > > +#include <openssl/engine.h>
> > > > #include <openssl/err.h>
> > > >
> > > > #include "imaevm.h"
> > > > @@ -70,8 +72,8 @@ const char *const pkey_hash_algo[PKEY_HASH__LAST] = {
> > > > [PKEY_HASH_SHA384] = "sha384",
> > > > [PKEY_HASH_SHA512] = "sha512",
> > > > [PKEY_HASH_SHA224] = "sha224",
> > > > - [PKEY_HASH_STREEBOG_256] = "streebog256",
> > > > - [PKEY_HASH_STREEBOG_512] = "streebog512",
> > > > + [PKEY_HASH_STREEBOG_256] = "md_gost12_256,streebog256",
> > > > + [PKEY_HASH_STREEBOG_512] = "md_gost12_512,streebog512",
> > > > };
> > > >
> > > > /*
> > > > @@ -284,6 +286,35 @@ static int add_dev_hash(struct stat *st, EVP_MD_CTX *ctx)
> > > > return !EVP_DigestUpdate(ctx, &dev, sizeof(dev));
> > > > }
> > > >
> > > > +/* Call EVP_get_digestbyname with provided name and with alias,
> > > > + * which is first name in the comma separated list of names
> > > > + * in pkey_hash_algo.
> > > > + */
> > > > +static const EVP_MD *get_digestbyname(const char *name)
> > > > +{
> > > > + const EVP_MD *md;
> > > > +
> > > > + md = EVP_get_digestbyname(params.hash_algo);
> > > > + if (!md) {
> > > > + char alias[MAX_DIGEST_NAME];
> > > > + int len;
> > > > + int algo_id;
> > > > + const char *algo_alias;
> > > > +
> > > > + /* try to get algo by its alias */
> > > > + algo_id = get_hash_algo(params.hash_algo);
> > > > + algo_alias = get_hash_algo_by_id(algo_id);
> > > > + len = strchrnul(algo_alias, ',') - algo_alias;
> > > > + if (len < sizeof(alias)) {
> > > > + memcpy(alias, algo_alias, len);
> > > > + alias[len] = '\0';
> > > > + if (strcmp(params.hash_algo, alias))
> > > > + md = EVP_get_digestbyname(alias);
> > > > + }
> > > > + }
> > > > + return md;
> > > > +}
> > > > +
> > > > int ima_calc_hash(const char *file, uint8_t *hash)
> > > > {
> > > > const EVP_MD *md;
> > > > @@ -305,7 +336,7 @@ int ima_calc_hash(const char *file, uint8_t *hash)
> > > > return err;
> > > > }
> > > >
> > > > - md = EVP_get_digestbyname(params.hash_algo);
> > > > + md = get_digestbyname(params.hash_algo);
> > > > if (!md) {
> > > > log_err("EVP_get_digestbyname(%s) failed\n", params.hash_algo);
> > > > return 1;
> > > > @@ -561,6 +592,22 @@ static int algocmp(const char *a, const char *b)
> > > > return *a || *b;
> > > > }
> > > >
> > > > +static int strmatch(const char *needle, const char *haystack)
> > > > +{
> > > > + int len = strlen(needle);
> > > > + const char *p = haystack;
> > > > + const char *t;
> > > > +
> > > > + for (t = strchrnul(p, ','); *p; p = t, t = strchrnul(p, ',')) {
> > > > + if (t - p == len &&
> > > > + !strncmp(needle, p, len))
> > > > + return 0;
> > > > + if (!*t++)
> > > > + break;
> > > > + }
> > > > + return 1;
> > > > +}
> > > > +
> > > > int get_hash_algo(const char *algo)
> > > > {
> > > > int i;
> > > > @@ -568,7 +615,7 @@ int get_hash_algo(const char *algo)
> > > > /* first iterate over builtin algorithms */
> > > > for (i = 0; i < PKEY_HASH__LAST; i++)
> > > > if (pkey_hash_algo[i] &&
> > > > - !strcmp(algo, pkey_hash_algo[i]))
> > > > + !strmatch(algo, pkey_hash_algo[i]))
> > > > return i;
> > > >
> > > > /* iterate over algorithms provided by kernel-headers */
> >
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 7/7] ima-evm-utils: Try to load digest by its alias
2019-02-11 18:13 ` Vitaly Chikunov
@ 2019-02-11 18:21 ` Vitaly Chikunov
2019-02-11 19:26 ` Vitaly Chikunov
0 siblings, 1 reply; 18+ messages in thread
From: Vitaly Chikunov @ 2019-02-11 18:21 UTC (permalink / raw)
To: Mimi Zohar; +Cc: Mimi Zohar, Dmitry Kasatkin, linux-integrity
On Mon, Feb 11, 2019 at 09:13:00PM +0300, Vitaly Chikunov wrote:
> Mimi,
>
> On Mon, Feb 11, 2019 at 12:59:12PM -0500, Mimi Zohar wrote:
> > On Mon, 2019-02-11 at 20:52 +0300, Vitaly Chikunov wrote:
> > > On Mon, Feb 11, 2019 at 12:38:58PM -0500, Mimi Zohar wrote:
> > > > On Mon, 2018-12-03 at 06:35 +0300, Vitaly Chikunov wrote:
> > > > > For compatibility with older OpenSSL try to load digest by its alias if
> > > > > load by its proper name is failed.
> > > > >
> > > > > This is configured in pkey_hash_algo by mentioning loadable alias first in the
> > > > > comma separated list of algo names.
> > > >
> > > > After this patch, I can not verify the signature. It's failing to
> > > > find the hash algorithm.
> > >
> > > Yes, it's fixed in "ima-evm-utils: convert sign v2 from RSA to EVP_PKEY
> > > API". I didn't consider it a problem since streebog256 should not work
> > > for sign/verify anyway (since RSA should not support it). That EVP_PKEY
> > > patch which adds ability to sign/verify for Streebog also fixes above
> > > problem.
> >
> > I've been having second thoughts about this patch in general, as it
> > made the hash algorithm comparison unnecessarily more complex for the
> > simple case. As there hasn't been a new ima-evm-utils release with
> > patch, perhaps we should simple remove/revert it?
>
> It was only for compatibility with older openssl/gost-engine, where
> "streebog256" name is not defined yet. If you don't want to allow that
> users to use Streebog feel free to revert it.
>
> I was not found this mistake at the time, since Streebog should only be
> used in ima_hash and not in ima_sign/verify (which requires RSA support
> which I intentionally didn't add, planning to add EC-RDSA support later),
> so that code path was not tested.
This copy-paste of the fix form "ima-evm-utils: convert sign v2 from RSA to EVP_PKEY API":
@@ -159,8 +102,12 @@ void dump(const void *ptr, int len)
const char *get_hash_algo_by_id(int algo)
{
- if (algo < PKEY_HASH__LAST)
- return pkey_hash_algo[algo];
+ if (algo < PKEY_HASH__LAST) {
+ const char *name = pkey_hash_algo[algo];
+ const char *last = strrchr(name, ',');
+
+ return last ? last + 1 : name;
+ }
if (algo < HASH_ALGO__LAST)
return hash_algo_name[algo];
>
> >
> > Mimi
> >
> > >
> > > > evmctl ima_sign -k <private key> -p -a streebog256 ./foo.txt --xattr-user
> > > > evmctl ima_verify -k <public key> ./foo.txt --xattr-user
> > > >
> > > > Mimi
> > > >
> > > > >
> > > > > Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
> > > > > ---
> > > > > Changes since v1:
> > > > > - New patch.
> > > > > Changes since v2:
> > > > > - No changes.
> > > > >
> > > > > src/imaevm.h | 1 +
> > > > > src/libimaevm.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++----
> > > > > 2 files changed, 52 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/src/imaevm.h b/src/imaevm.h
> > > > > index c81bf21..795966a 100644
> > > > > --- a/src/imaevm.h
> > > > > +++ b/src/imaevm.h
> > > > > @@ -75,6 +75,7 @@
> > > > > #define DATA_SIZE 4096
> > > > > #define SHA1_HASH_LEN 20
> > > > >
> > > > > +#define MAX_DIGEST_NAME 32
> > > > > #define MAX_DIGEST_SIZE 64
> > > > > #define MAX_SIGNATURE_SIZE 1024
> > > > >
> > > > > diff --git a/src/libimaevm.c b/src/libimaevm.c
> > > > > index cb4721b..7501303 100644
> > > > > --- a/src/libimaevm.c
> > > > > +++ b/src/libimaevm.c
> > > > > @@ -41,6 +41,7 @@
> > > > > /* should we use logger instead for library? */
> > > > > #define USE_FPRINTF
> > > > >
> > > > > +#define _GNU_SOURCE
> > > > > #include <sys/types.h>
> > > > > #include <sys/param.h>
> > > > > #include <sys/stat.h>
> > > > > @@ -56,6 +57,7 @@
> > > > > #include <openssl/pem.h>
> > > > > #include <openssl/evp.h>
> > > > > #include <openssl/x509.h>
> > > > > +#include <openssl/engine.h>
> > > > > #include <openssl/err.h>
> > > > >
> > > > > #include "imaevm.h"
> > > > > @@ -70,8 +72,8 @@ const char *const pkey_hash_algo[PKEY_HASH__LAST] = {
> > > > > [PKEY_HASH_SHA384] = "sha384",
> > > > > [PKEY_HASH_SHA512] = "sha512",
> > > > > [PKEY_HASH_SHA224] = "sha224",
> > > > > - [PKEY_HASH_STREEBOG_256] = "streebog256",
> > > > > - [PKEY_HASH_STREEBOG_512] = "streebog512",
> > > > > + [PKEY_HASH_STREEBOG_256] = "md_gost12_256,streebog256",
> > > > > + [PKEY_HASH_STREEBOG_512] = "md_gost12_512,streebog512",
> > > > > };
> > > > >
> > > > > /*
> > > > > @@ -284,6 +286,35 @@ static int add_dev_hash(struct stat *st, EVP_MD_CTX *ctx)
> > > > > return !EVP_DigestUpdate(ctx, &dev, sizeof(dev));
> > > > > }
> > > > >
> > > > > +/* Call EVP_get_digestbyname with provided name and with alias,
> > > > > + * which is first name in the comma separated list of names
> > > > > + * in pkey_hash_algo.
> > > > > + */
> > > > > +static const EVP_MD *get_digestbyname(const char *name)
> > > > > +{
> > > > > + const EVP_MD *md;
> > > > > +
> > > > > + md = EVP_get_digestbyname(params.hash_algo);
> > > > > + if (!md) {
> > > > > + char alias[MAX_DIGEST_NAME];
> > > > > + int len;
> > > > > + int algo_id;
> > > > > + const char *algo_alias;
> > > > > +
> > > > > + /* try to get algo by its alias */
> > > > > + algo_id = get_hash_algo(params.hash_algo);
> > > > > + algo_alias = get_hash_algo_by_id(algo_id);
> > > > > + len = strchrnul(algo_alias, ',') - algo_alias;
> > > > > + if (len < sizeof(alias)) {
> > > > > + memcpy(alias, algo_alias, len);
> > > > > + alias[len] = '\0';
> > > > > + if (strcmp(params.hash_algo, alias))
> > > > > + md = EVP_get_digestbyname(alias);
> > > > > + }
> > > > > + }
> > > > > + return md;
> > > > > +}
> > > > > +
> > > > > int ima_calc_hash(const char *file, uint8_t *hash)
> > > > > {
> > > > > const EVP_MD *md;
> > > > > @@ -305,7 +336,7 @@ int ima_calc_hash(const char *file, uint8_t *hash)
> > > > > return err;
> > > > > }
> > > > >
> > > > > - md = EVP_get_digestbyname(params.hash_algo);
> > > > > + md = get_digestbyname(params.hash_algo);
> > > > > if (!md) {
> > > > > log_err("EVP_get_digestbyname(%s) failed\n", params.hash_algo);
> > > > > return 1;
> > > > > @@ -561,6 +592,22 @@ static int algocmp(const char *a, const char *b)
> > > > > return *a || *b;
> > > > > }
> > > > >
> > > > > +static int strmatch(const char *needle, const char *haystack)
> > > > > +{
> > > > > + int len = strlen(needle);
> > > > > + const char *p = haystack;
> > > > > + const char *t;
> > > > > +
> > > > > + for (t = strchrnul(p, ','); *p; p = t, t = strchrnul(p, ',')) {
> > > > > + if (t - p == len &&
> > > > > + !strncmp(needle, p, len))
> > > > > + return 0;
> > > > > + if (!*t++)
> > > > > + break;
> > > > > + }
> > > > > + return 1;
> > > > > +}
> > > > > +
> > > > > int get_hash_algo(const char *algo)
> > > > > {
> > > > > int i;
> > > > > @@ -568,7 +615,7 @@ int get_hash_algo(const char *algo)
> > > > > /* first iterate over builtin algorithms */
> > > > > for (i = 0; i < PKEY_HASH__LAST; i++)
> > > > > if (pkey_hash_algo[i] &&
> > > > > - !strcmp(algo, pkey_hash_algo[i]))
> > > > > + !strmatch(algo, pkey_hash_algo[i]))
> > > > > return i;
> > > > >
> > > > > /* iterate over algorithms provided by kernel-headers */
> > >
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 7/7] ima-evm-utils: Try to load digest by its alias
2019-02-11 18:21 ` Vitaly Chikunov
@ 2019-02-11 19:26 ` Vitaly Chikunov
2019-02-11 20:21 ` Mimi Zohar
0 siblings, 1 reply; 18+ messages in thread
From: Vitaly Chikunov @ 2019-02-11 19:26 UTC (permalink / raw)
To: Mimi Zohar; +Cc: Mimi Zohar, Dmitry Kasatkin, linux-integrity
Mimi,
On Mon, Feb 11, 2019 at 09:21:15PM +0300, Vitaly Chikunov wrote:
> On Mon, Feb 11, 2019 at 09:13:00PM +0300, Vitaly Chikunov wrote:
> > On Mon, Feb 11, 2019 at 12:59:12PM -0500, Mimi Zohar wrote:
> > > On Mon, 2019-02-11 at 20:52 +0300, Vitaly Chikunov wrote:
> > > > On Mon, Feb 11, 2019 at 12:38:58PM -0500, Mimi Zohar wrote:
> > > > > On Mon, 2018-12-03 at 06:35 +0300, Vitaly Chikunov wrote:
> > > > > > For compatibility with older OpenSSL try to load digest by its alias if
> > > > > > load by its proper name is failed.
> > > > > >
> > > > > > This is configured in pkey_hash_algo by mentioning loadable alias first in the
> > > > > > comma separated list of algo names.
> > > > >
> > > > > After this patch, I can not verify the signature. It's failing to
> > > > > find the hash algorithm.
> > > >
> > > > Yes, it's fixed in "ima-evm-utils: convert sign v2 from RSA to EVP_PKEY
> > > > API". I didn't consider it a problem since streebog256 should not work
> > > > for sign/verify anyway (since RSA should not support it). That EVP_PKEY
> > > > patch which adds ability to sign/verify for Streebog also fixes above
> > > > problem.
> > >
> > > I've been having second thoughts about this patch in general, as it
> > > made the hash algorithm comparison unnecessarily more complex for the
> > > simple case. As there hasn't been a new ima-evm-utils release with
> > > patch, perhaps we should simple remove/revert it?
> >
> > It was only for compatibility with older openssl/gost-engine, where
> > "streebog256" name is not defined yet. If you don't want to allow that
> > users to use Streebog feel free to revert it.
Or you may suggest simpler approach.
Basically, we need to resole both text strings into the same PKEY_HASH_
id, allow any of the string pass into EVP_get_digestbyname, and resolve
PKEY_HASH_ id back into the correct hash name. Optionally, allow user to
specify new hash name on older openssl/gost-engine. This is all
implemented by that patch, it was not just overly complicated "hash
algorithm comparison".
My wish is we retain support of older openssl/gost-engine.
Thanks,
> >
> > I was not found this mistake at the time, since Streebog should only be
> > used in ima_hash and not in ima_sign/verify (which requires RSA support
> > which I intentionally didn't add, planning to add EC-RDSA support later),
> > so that code path was not tested.
>
> This copy-paste of the fix form "ima-evm-utils: convert sign v2 from RSA to EVP_PKEY API":
>
> @@ -159,8 +102,12 @@ void dump(const void *ptr, int len)
>
> const char *get_hash_algo_by_id(int algo)
> {
> - if (algo < PKEY_HASH__LAST)
> - return pkey_hash_algo[algo];
> + if (algo < PKEY_HASH__LAST) {
> + const char *name = pkey_hash_algo[algo];
> + const char *last = strrchr(name, ',');
> +
> + return last ? last + 1 : name;
> + }
> if (algo < HASH_ALGO__LAST)
> return hash_algo_name[algo];
>
> >
> > >
> > > Mimi
> > >
> > > >
> > > > > evmctl ima_sign -k <private key> -p -a streebog256 ./foo.txt --xattr-user
> > > > > evmctl ima_verify -k <public key> ./foo.txt --xattr-user
> > > > >
> > > > > Mimi
> > > > >
> > > > > >
> > > > > > Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
> > > > > > ---
> > > > > > Changes since v1:
> > > > > > - New patch.
> > > > > > Changes since v2:
> > > > > > - No changes.
> > > > > >
> > > > > > src/imaevm.h | 1 +
> > > > > > src/libimaevm.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++----
> > > > > > 2 files changed, 52 insertions(+), 4 deletions(-)
> > > > > >
> > > > > > diff --git a/src/imaevm.h b/src/imaevm.h
> > > > > > index c81bf21..795966a 100644
> > > > > > --- a/src/imaevm.h
> > > > > > +++ b/src/imaevm.h
> > > > > > @@ -75,6 +75,7 @@
> > > > > > #define DATA_SIZE 4096
> > > > > > #define SHA1_HASH_LEN 20
> > > > > >
> > > > > > +#define MAX_DIGEST_NAME 32
> > > > > > #define MAX_DIGEST_SIZE 64
> > > > > > #define MAX_SIGNATURE_SIZE 1024
> > > > > >
> > > > > > diff --git a/src/libimaevm.c b/src/libimaevm.c
> > > > > > index cb4721b..7501303 100644
> > > > > > --- a/src/libimaevm.c
> > > > > > +++ b/src/libimaevm.c
> > > > > > @@ -41,6 +41,7 @@
> > > > > > /* should we use logger instead for library? */
> > > > > > #define USE_FPRINTF
> > > > > >
> > > > > > +#define _GNU_SOURCE
> > > > > > #include <sys/types.h>
> > > > > > #include <sys/param.h>
> > > > > > #include <sys/stat.h>
> > > > > > @@ -56,6 +57,7 @@
> > > > > > #include <openssl/pem.h>
> > > > > > #include <openssl/evp.h>
> > > > > > #include <openssl/x509.h>
> > > > > > +#include <openssl/engine.h>
> > > > > > #include <openssl/err.h>
> > > > > >
> > > > > > #include "imaevm.h"
> > > > > > @@ -70,8 +72,8 @@ const char *const pkey_hash_algo[PKEY_HASH__LAST] = {
> > > > > > [PKEY_HASH_SHA384] = "sha384",
> > > > > > [PKEY_HASH_SHA512] = "sha512",
> > > > > > [PKEY_HASH_SHA224] = "sha224",
> > > > > > - [PKEY_HASH_STREEBOG_256] = "streebog256",
> > > > > > - [PKEY_HASH_STREEBOG_512] = "streebog512",
> > > > > > + [PKEY_HASH_STREEBOG_256] = "md_gost12_256,streebog256",
> > > > > > + [PKEY_HASH_STREEBOG_512] = "md_gost12_512,streebog512",
> > > > > > };
> > > > > >
> > > > > > /*
> > > > > > @@ -284,6 +286,35 @@ static int add_dev_hash(struct stat *st, EVP_MD_CTX *ctx)
> > > > > > return !EVP_DigestUpdate(ctx, &dev, sizeof(dev));
> > > > > > }
> > > > > >
> > > > > > +/* Call EVP_get_digestbyname with provided name and with alias,
> > > > > > + * which is first name in the comma separated list of names
> > > > > > + * in pkey_hash_algo.
> > > > > > + */
> > > > > > +static const EVP_MD *get_digestbyname(const char *name)
> > > > > > +{
> > > > > > + const EVP_MD *md;
> > > > > > +
> > > > > > + md = EVP_get_digestbyname(params.hash_algo);
> > > > > > + if (!md) {
> > > > > > + char alias[MAX_DIGEST_NAME];
> > > > > > + int len;
> > > > > > + int algo_id;
> > > > > > + const char *algo_alias;
> > > > > > +
> > > > > > + /* try to get algo by its alias */
> > > > > > + algo_id = get_hash_algo(params.hash_algo);
> > > > > > + algo_alias = get_hash_algo_by_id(algo_id);
> > > > > > + len = strchrnul(algo_alias, ',') - algo_alias;
> > > > > > + if (len < sizeof(alias)) {
> > > > > > + memcpy(alias, algo_alias, len);
> > > > > > + alias[len] = '\0';
> > > > > > + if (strcmp(params.hash_algo, alias))
> > > > > > + md = EVP_get_digestbyname(alias);
> > > > > > + }
> > > > > > + }
> > > > > > + return md;
> > > > > > +}
> > > > > > +
> > > > > > int ima_calc_hash(const char *file, uint8_t *hash)
> > > > > > {
> > > > > > const EVP_MD *md;
> > > > > > @@ -305,7 +336,7 @@ int ima_calc_hash(const char *file, uint8_t *hash)
> > > > > > return err;
> > > > > > }
> > > > > >
> > > > > > - md = EVP_get_digestbyname(params.hash_algo);
> > > > > > + md = get_digestbyname(params.hash_algo);
> > > > > > if (!md) {
> > > > > > log_err("EVP_get_digestbyname(%s) failed\n", params.hash_algo);
> > > > > > return 1;
> > > > > > @@ -561,6 +592,22 @@ static int algocmp(const char *a, const char *b)
> > > > > > return *a || *b;
> > > > > > }
> > > > > >
> > > > > > +static int strmatch(const char *needle, const char *haystack)
> > > > > > +{
> > > > > > + int len = strlen(needle);
> > > > > > + const char *p = haystack;
> > > > > > + const char *t;
> > > > > > +
> > > > > > + for (t = strchrnul(p, ','); *p; p = t, t = strchrnul(p, ',')) {
> > > > > > + if (t - p == len &&
> > > > > > + !strncmp(needle, p, len))
> > > > > > + return 0;
> > > > > > + if (!*t++)
> > > > > > + break;
> > > > > > + }
> > > > > > + return 1;
> > > > > > +}
> > > > > > +
> > > > > > int get_hash_algo(const char *algo)
> > > > > > {
> > > > > > int i;
> > > > > > @@ -568,7 +615,7 @@ int get_hash_algo(const char *algo)
> > > > > > /* first iterate over builtin algorithms */
> > > > > > for (i = 0; i < PKEY_HASH__LAST; i++)
> > > > > > if (pkey_hash_algo[i] &&
> > > > > > - !strcmp(algo, pkey_hash_algo[i]))
> > > > > > + !strmatch(algo, pkey_hash_algo[i]))
> > > > > > return i;
> > > > > >
> > > > > > /* iterate over algorithms provided by kernel-headers */
> > > >
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 7/7] ima-evm-utils: Try to load digest by its alias
2019-02-11 19:26 ` Vitaly Chikunov
@ 2019-02-11 20:21 ` Mimi Zohar
2019-02-11 20:37 ` Vitaly Chikunov
0 siblings, 1 reply; 18+ messages in thread
From: Mimi Zohar @ 2019-02-11 20:21 UTC (permalink / raw)
To: Vitaly Chikunov; +Cc: Mimi Zohar, Dmitry Kasatkin, linux-integrity
On Mon, 2019-02-11 at 22:26 +0300, Vitaly Chikunov wrote:
> Mimi,
>
> On Mon, Feb 11, 2019 at 09:21:15PM +0300, Vitaly Chikunov wrote:
> > On Mon, Feb 11, 2019 at 09:13:00PM +0300, Vitaly Chikunov wrote:
> > > On Mon, Feb 11, 2019 at 12:59:12PM -0500, Mimi Zohar wrote:
> > > > On Mon, 2019-02-11 at 20:52 +0300, Vitaly Chikunov wrote:
> > > > > On Mon, Feb 11, 2019 at 12:38:58PM -0500, Mimi Zohar wrote:
> > > > > > On Mon, 2018-12-03 at 06:35 +0300, Vitaly Chikunov wrote:
> > > > > > > For compatibility with older OpenSSL try to load digest by its alias if
> > > > > > > load by its proper name is failed.
> > > > > > >
> > > > > > > This is configured in pkey_hash_algo by mentioning loadable alias first in the
> > > > > > > comma separated list of algo names.
> > > > > >
> > > > > > After this patch, I can not verify the signature. It's failing to
> > > > > > find the hash algorithm.
> > > > >
> > > > > Yes, it's fixed in "ima-evm-utils: convert sign v2 from RSA to EVP_PKEY
> > > > > API". I didn't consider it a problem since streebog256 should not work
> > > > > for sign/verify anyway (since RSA should not support it). That EVP_PKEY
> > > > > patch which adds ability to sign/verify for Streebog also fixes above
> > > > > problem.
> > > >
> > > > I've been having second thoughts about this patch in general, as it
> > > > made the hash algorithm comparison unnecessarily more complex for the
> > > > simple case. As there hasn't been a new ima-evm-utils release with
> > > > patch, perhaps we should simple remove/revert it?
> > >
> > > It was only for compatibility with older openssl/gost-engine, where
> > > "streebog256" name is not defined yet. If you don't want to allow that
> > > users to use Streebog feel free to revert it.
>
> Or you may suggest simpler approach.
>
> Basically, we need to resole both text strings into the same PKEY_HASH_
> id, allow any of the string pass into EVP_get_digestbyname, and resolve
> PKEY_HASH_ id back into the correct hash name. Optionally, allow user to
> specify new hash name on older openssl/gost-engine. This is all
> implemented by that patch, it was not just overly complicated "hash
> algorithm comparison".
>
> My wish is we retain support of older openssl/gost-engine.
The following seems to fix the problem, but instead of adding it in
strmatch(), as below, I would add it before the strmatch() call. Then
strmatch is only called as needed.
diff --git a/src/libimaevm.c b/src/libimaevm.c
index d9ffa13befb0..7901215da655 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -598,6 +598,9 @@ static int strmatch(const char *needle, const char *haystack)
const char *p = haystack;
const char *t;
+ if (strcmp(needle, haystack) == 0)
+ return 0;
+
for (t = strchrnul(p, ','); *p; p = t, t = strchrnul(p, ',')) {
if (t - p == len &&
!strncmp(needle, p, len))
Mimi
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v3 7/7] ima-evm-utils: Try to load digest by its alias
2019-02-11 20:21 ` Mimi Zohar
@ 2019-02-11 20:37 ` Vitaly Chikunov
2019-02-12 15:41 ` Mimi Zohar
0 siblings, 1 reply; 18+ messages in thread
From: Vitaly Chikunov @ 2019-02-11 20:37 UTC (permalink / raw)
To: Mimi Zohar; +Cc: Mimi Zohar, Dmitry Kasatkin, linux-integrity
Mimi,
On Mon, Feb 11, 2019 at 03:21:03PM -0500, Mimi Zohar wrote:
> On Mon, 2019-02-11 at 22:26 +0300, Vitaly Chikunov wrote:
> > On Mon, Feb 11, 2019 at 09:21:15PM +0300, Vitaly Chikunov wrote:
> > > On Mon, Feb 11, 2019 at 09:13:00PM +0300, Vitaly Chikunov wrote:
> > > > On Mon, Feb 11, 2019 at 12:59:12PM -0500, Mimi Zohar wrote:
> > > > > On Mon, 2019-02-11 at 20:52 +0300, Vitaly Chikunov wrote:
> > > > > > On Mon, Feb 11, 2019 at 12:38:58PM -0500, Mimi Zohar wrote:
> > > > > > > On Mon, 2018-12-03 at 06:35 +0300, Vitaly Chikunov wrote:
> > > > > > > > For compatibility with older OpenSSL try to load digest by its alias if
> > > > > > > > load by its proper name is failed.
> > > > > > > >
> > > > > > > > This is configured in pkey_hash_algo by mentioning loadable alias first in the
> > > > > > > > comma separated list of algo names.
> > > > > > >
> > > > > > > After this patch, I can not verify the signature. It's failing to
> > > > > > > find the hash algorithm.
> > > > > >
> > > > > > Yes, it's fixed in "ima-evm-utils: convert sign v2 from RSA to EVP_PKEY
> > > > > > API". I didn't consider it a problem since streebog256 should not work
> > > > > > for sign/verify anyway (since RSA should not support it). That EVP_PKEY
> > > > > > patch which adds ability to sign/verify for Streebog also fixes above
> > > > > > problem.
> > > > >
> > > > > I've been having second thoughts about this patch in general, as it
> > > > > made the hash algorithm comparison unnecessarily more complex for the
> > > > > simple case. As there hasn't been a new ima-evm-utils release with
> > > > > patch, perhaps we should simple remove/revert it?
> > > >
> > > > It was only for compatibility with older openssl/gost-engine, where
> > > > "streebog256" name is not defined yet. If you don't want to allow that
> > > > users to use Streebog feel free to revert it.
> >
> > Or you may suggest simpler approach.
> >
> > Basically, we need to resole both text strings into the same PKEY_HASH_
> > id, allow any of the string pass into EVP_get_digestbyname, and resolve
> > PKEY_HASH_ id back into the correct hash name. Optionally, allow user to
> > specify new hash name on older openssl/gost-engine. This is all
> > implemented by that patch, it was not just overly complicated "hash
> > algorithm comparison".
> >
> > My wish is we retain support of older openssl/gost-engine.
>
> The following seems to fix the problem, but instead of adding it in
> strmatch(), as below, I would add it before the strmatch() call. Then
> strmatch is only called as needed.
IC. I will try to conceptually simplify the code and prepare another
patch. Probably, I will split pkey_hash_algop[] into two arrays, simple
one where strcmp() in the loop is used, and complex one for hash names
with aliases. I will try to replace strmatch() with just simple string
array loop.
>
> diff --git a/src/libimaevm.c b/src/libimaevm.c
> index d9ffa13befb0..7901215da655 100644
> --- a/src/libimaevm.c
> +++ b/src/libimaevm.c
> @@ -598,6 +598,9 @@ static int strmatch(const char *needle, const char *haystack)
> const char *p = haystack;
> const char *t;
>
> + if (strcmp(needle, haystack) == 0)
> + return 0;
> +
> for (t = strchrnul(p, ','); *p; p = t, t = strchrnul(p, ',')) {
> if (t - p == len &&
> !strncmp(needle, p, len))
>
> Mimi
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 7/7] ima-evm-utils: Try to load digest by its alias
2019-02-11 20:37 ` Vitaly Chikunov
@ 2019-02-12 15:41 ` Mimi Zohar
2019-02-12 17:07 ` Vitaly Chikunov
0 siblings, 1 reply; 18+ messages in thread
From: Mimi Zohar @ 2019-02-12 15:41 UTC (permalink / raw)
To: Vitaly Chikunov; +Cc: Mimi Zohar, Dmitry Kasatkin, linux-integrity
On Mon, 2019-02-11 at 23:37 +0300, Vitaly Chikunov wrote:
> Mimi,
>
> On Mon, Feb 11, 2019 at 03:21:03PM -0500, Mimi Zohar wrote:
> > On Mon, 2019-02-11 at 22:26 +0300, Vitaly Chikunov wrote:
> > > On Mon, Feb 11, 2019 at 09:21:15PM +0300, Vitaly Chikunov wrote:
> > > > On Mon, Feb 11, 2019 at 09:13:00PM +0300, Vitaly Chikunov wrote:
> > > > > On Mon, Feb 11, 2019 at 12:59:12PM -0500, Mimi Zohar wrote:
> > > > > > On Mon, 2019-02-11 at 20:52 +0300, Vitaly Chikunov wrote:
> > > > > > > On Mon, Feb 11, 2019 at 12:38:58PM -0500, Mimi Zohar wrote:
> > > > > > > > On Mon, 2018-12-03 at 06:35 +0300, Vitaly Chikunov wrote:
> > > > > > > > > For compatibility with older OpenSSL try to load digest by its alias if
> > > > > > > > > load by its proper name is failed.
> > > > > > > > >
> > > > > > > > > This is configured in pkey_hash_algo by mentioning loadable alias first in the
> > > > > > > > > comma separated list of algo names.
> > > > > > > >
> > > > > > > > After this patch, I can not verify the signature. It's failing to
> > > > > > > > find the hash algorithm.
> > > > > > >
> > > > > > > Yes, it's fixed in "ima-evm-utils: convert sign v2 from RSA to EVP_PKEY
> > > > > > > API". I didn't consider it a problem since streebog256 should not work
> > > > > > > for sign/verify anyway (since RSA should not support it). That EVP_PKEY
> > > > > > > patch which adds ability to sign/verify for Streebog also fixes above
> > > > > > > problem.
> > > > > >
> > > > > > I've been having second thoughts about this patch in general, as it
> > > > > > made the hash algorithm comparison unnecessarily more complex for the
> > > > > > simple case. As there hasn't been a new ima-evm-utils release with
> > > > > > patch, perhaps we should simple remove/revert it?
> > > > >
> > > > > It was only for compatibility with older openssl/gost-engine, where
> > > > > "streebog256" name is not defined yet. If you don't want to allow that
> > > > > users to use Streebog feel free to revert it.
> > >
> > > Or you may suggest simpler approach.
> > >
> > > Basically, we need to resole both text strings into the same PKEY_HASH_
> > > id, allow any of the string pass into EVP_get_digestbyname, and resolve
> > > PKEY_HASH_ id back into the correct hash name. Optionally, allow user to
> > > specify new hash name on older openssl/gost-engine. This is all
> > > implemented by that patch, it was not just overly complicated "hash
> > > algorithm comparison".
> > >
> > > My wish is we retain support of older openssl/gost-engine.
> >
> > The following seems to fix the problem, but instead of adding it in
> > strmatch(), as below, I would add it before the strmatch() call. Then
> > strmatch is only called as needed.
>
> IC.
The patch below isn't addressing old vs. new naming, but fixes
"ima_verify". Adding the following line, results in "strmatch: algo
md_gost12_256,streebog256". It's trying to match the entire string,
but fails without the strcmp().
+ log_info("strmatch: algo %s \n", needle);
Mimi
> I will try to conceptually simplify the code and prepare another
> patch. Probably, I will split pkey_hash_algop[] into two arrays, simple
> one where strcmp() in the loop is used, and complex one for hash names
> with aliases. I will try to replace strmatch() with just simple string
> array loop.
>
> >
> > diff --git a/src/libimaevm.c b/src/libimaevm.c
> > index d9ffa13befb0..7901215da655 100644
> > --- a/src/libimaevm.c
> > +++ b/src/libimaevm.c
> > @@ -598,6 +598,9 @@ static int strmatch(const char *needle, const char *haystack)
> > const char *p = haystack;
> > const char *t;
> >
> > + if (strcmp(needle, haystack) == 0)
> > + return 0;
> > +
> > for (t = strchrnul(p, ','); *p; p = t, t = strchrnul(p, ',')) {
> > if (t - p == len &&
> > !strncmp(needle, p, len))
> >
> > Mimi
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 7/7] ima-evm-utils: Try to load digest by its alias
2019-02-12 15:41 ` Mimi Zohar
@ 2019-02-12 17:07 ` Vitaly Chikunov
0 siblings, 0 replies; 18+ messages in thread
From: Vitaly Chikunov @ 2019-02-12 17:07 UTC (permalink / raw)
To: Mimi Zohar; +Cc: Mimi Zohar, Dmitry Kasatkin, linux-integrity
On Tue, Feb 12, 2019 at 10:41:46AM -0500, Mimi Zohar wrote:
> On Mon, 2019-02-11 at 23:37 +0300, Vitaly Chikunov wrote:
> > Mimi,
> >
> > On Mon, Feb 11, 2019 at 03:21:03PM -0500, Mimi Zohar wrote:
> > > On Mon, 2019-02-11 at 22:26 +0300, Vitaly Chikunov wrote:
> > > > On Mon, Feb 11, 2019 at 09:21:15PM +0300, Vitaly Chikunov wrote:
> > > > > On Mon, Feb 11, 2019 at 09:13:00PM +0300, Vitaly Chikunov wrote:
> > > > > > On Mon, Feb 11, 2019 at 12:59:12PM -0500, Mimi Zohar wrote:
> > > > > > > On Mon, 2019-02-11 at 20:52 +0300, Vitaly Chikunov wrote:
> > > > > > > > On Mon, Feb 11, 2019 at 12:38:58PM -0500, Mimi Zohar wrote:
> > > > > > > > > On Mon, 2018-12-03 at 06:35 +0300, Vitaly Chikunov wrote:
> > > > > > > > > > For compatibility with older OpenSSL try to load digest by its alias if
> > > > > > > > > > load by its proper name is failed.
> > > > > > > > > >
> > > > > > > > > > This is configured in pkey_hash_algo by mentioning loadable alias first in the
> > > > > > > > > > comma separated list of algo names.
> > > > > > > > >
> > > > > > > > > After this patch, I can not verify the signature. It's failing to
> > > > > > > > > find the hash algorithm.
> > > > > > > >
> > > > > > > > Yes, it's fixed in "ima-evm-utils: convert sign v2 from RSA to EVP_PKEY
> > > > > > > > API". I didn't consider it a problem since streebog256 should not work
> > > > > > > > for sign/verify anyway (since RSA should not support it). That EVP_PKEY
> > > > > > > > patch which adds ability to sign/verify for Streebog also fixes above
> > > > > > > > problem.
> > > > > > >
> > > > > > > I've been having second thoughts about this patch in general, as it
> > > > > > > made the hash algorithm comparison unnecessarily more complex for the
> > > > > > > simple case. As there hasn't been a new ima-evm-utils release with
> > > > > > > patch, perhaps we should simple remove/revert it?
> > > > > >
> > > > > > It was only for compatibility with older openssl/gost-engine, where
> > > > > > "streebog256" name is not defined yet. If you don't want to allow that
> > > > > > users to use Streebog feel free to revert it.
> > > >
> > > > Or you may suggest simpler approach.
> > > >
> > > > Basically, we need to resole both text strings into the same PKEY_HASH_
> > > > id, allow any of the string pass into EVP_get_digestbyname, and resolve
> > > > PKEY_HASH_ id back into the correct hash name. Optionally, allow user to
> > > > specify new hash name on older openssl/gost-engine. This is all
> > > > implemented by that patch, it was not just overly complicated "hash
> > > > algorithm comparison".
> > > >
> > > > My wish is we retain support of older openssl/gost-engine.
> > >
> > > The following seems to fix the problem, but instead of adding it in
> > > strmatch(), as below, I would add it before the strmatch() call. Then
> > > strmatch is only called as needed.
> >
> > IC.
>
> The patch below isn't addressing old vs. new naming, but fixes
What patch?
> "ima_verify". Adding the following line, results in "strmatch: algo
> md_gost12_256,streebog256". It's trying to match the entire string,
This problem was fixed in "ima-evm-utils: convert sign v2 from RSA to
EVP_PKEY API". This wasn't problem because Streebog should not be
used in sign/verify (which was using RSA). When sign/verify ability for
Streebog is added in "ima-evm-utils: convert sign v2 from RSA to EVP_PKEY
API" sign/verify problem is fixed in the same patch.
But, since I sent new patch which removes strmatch this becomes
somewhat irrelevant.
> but fails without the strcmp().
>
> + log_info("strmatch: algo %s \n", needle);
>
> Mimi
>
>
> > I will try to conceptually simplify the code and prepare another
> > patch. Probably, I will split pkey_hash_algop[] into two arrays, simple
> > one where strcmp() in the loop is used, and complex one for hash names
> > with aliases. I will try to replace strmatch() with just simple string
> > array loop.
> >
> > >
> > > diff --git a/src/libimaevm.c b/src/libimaevm.c
> > > index d9ffa13befb0..7901215da655 100644
> > > --- a/src/libimaevm.c
> > > +++ b/src/libimaevm.c
> > > @@ -598,6 +598,9 @@ static int strmatch(const char *needle, const char *haystack)
> > > const char *p = haystack;
> > > const char *t;
> > >
> > > + if (strcmp(needle, haystack) == 0)
> > > + return 0;
> > > +
> > > for (t = strchrnul(p, ','); *p; p = t, t = strchrnul(p, ',')) {
> > > if (t - p == len &&
> > > !strncmp(needle, p, len))
> > >
> > > Mimi
> >
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/7] ima-evm-utils: Fix hash buffer overflow in verify_evm and hmac_evm
2018-12-03 3:35 [PATCH v3 1/7] ima-evm-utils: Fix hash buffer overflow in verify_evm and hmac_evm Vitaly Chikunov
` (5 preceding siblings ...)
2018-12-03 3:35 ` [PATCH v3 7/7] ima-evm-utils: Try to load digest by its alias Vitaly Chikunov
@ 2018-12-03 13:03 ` Mimi Zohar
6 siblings, 0 replies; 18+ messages in thread
From: Mimi Zohar @ 2018-12-03 13:03 UTC (permalink / raw)
To: Vitaly Chikunov, Mimi Zohar, Dmitry Kasatkin, linux-integrity
On Mon, 2018-12-03 at 06:35 +0300, Vitaly Chikunov wrote:
> Commit ae1319eeabd6 ("Remove hardcoding of SHA1 in EVM signatures")
> introduces overflow of 20 byte buffer on the stack while calculating
> hash. Also, invalid hash length is passed to the underlying verification
> function in verify_evm. This prevents any non-SHA1 hashes from being
> properly validated using evmctl.
>
> Fixes: ae1319eeabd6 ("Remove hardcoding of SHA1 in EVM signatures")
> Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
Thank you! This patch set has now been applied and is in master.
Mimi
> ---
> Changes since v1:
> - Fix similar issue in hmac_evm
> Changes since v2:
> - No changes.
>
> src/evmctl.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/src/evmctl.c b/src/evmctl.c
> index 1b46d58..f8035da 100644
> --- a/src/evmctl.c
> +++ b/src/evmctl.c
> @@ -55,6 +55,7 @@
> #include <keyutils.h>
> #include <ctype.h>
> #include <termios.h>
> +#include <assert.h>
>
> #include <openssl/sha.h>
> #include <openssl/pem.h>
> @@ -760,13 +761,15 @@ static int cmd_sign_evm(struct command *cmd)
>
> static int verify_evm(const char *file)
> {
> - unsigned char hash[20];
> + unsigned char hash[64];
> unsigned char sig[1024];
> + int mdlen;
> int len;
>
> - len = calc_evm_hash(file, hash);
> - if (len <= 1)
> - return len;
> + mdlen = calc_evm_hash(file, hash);
> + assert(mdlen <= sizeof(hash));
> + if (mdlen <= 1)
> + return mdlen;
>
> len = lgetxattr(file, "security.evm", sig, sizeof(sig));
> if (len < 0) {
> @@ -779,7 +782,7 @@ static int verify_evm(const char *file)
> return -1;
> }
>
> - return verify_hash(file, hash, sizeof(hash), sig + 1, len - 1);
> + return verify_hash(file, hash, mdlen, sig + 1, len - 1);
> }
>
> static int cmd_verify_evm(struct command *cmd)
> @@ -1135,11 +1138,12 @@ out:
>
> static int hmac_evm(const char *file, const char *key)
> {
> - unsigned char hash[20];
> + unsigned char hash[64];
> unsigned char sig[1024];
> int len, err;
>
> len = calc_evm_hmac(file, key, hash);
> + assert(len <= sizeof(hash));
> if (len <= 1)
> return len;
>
^ permalink raw reply [flat|nested] 18+ messages in thread