Mimi, On Tue, Jul 16, 2019 at 10:30:16AM -0400, Mimi Zohar wrote: > When keys are not provided, the default key is used to verify the file > signature, resulting in this erroneous message. Before using the default > key to verify the file signature, verify the keyid is correct. 1. What is default key when keys are not provided? 2. I don't see keyid verification in the patch. 3. Now we have so complicated keyfile handling. > > Signed-off-by: Mimi Zohar <zohar@xxxxxxxxxxxxx> > --- > src/libimaevm.c | 19 ++++++++++--------- > 1 file changed, 10 insertions(+), 9 deletions(-) > > diff --git a/src/libimaevm.c b/src/libimaevm.c > index ae487f9fe36c..472ab53c7b42 100644 > --- a/src/libimaevm.c > +++ b/src/libimaevm.c > @@ -302,6 +302,9 @@ EVP_PKEY *read_pub_pkey(const char *keyfile, int x509) > X509 *crt = NULL; > EVP_PKEY *pkey = NULL; > > + if (!keyfile) > + return NULL; > + > fp = fopen(keyfile, "r"); > if (!fp) { > log_err("Failed to open keyfile: %s\n", keyfile); > @@ -569,27 +572,25 @@ static int get_hash_algo_from_sig(unsigned char *sig) > int verify_hash(const char *file, const unsigned char *hash, int size, unsigned char *sig, > int siglen) > { > - const char *key; > - int x509; > + const char *key = NULL; > verify_hash_fn_t verify_hash; > > /* Get signature type from sig header */ > if (sig[0] == DIGSIG_VERSION_1) { > verify_hash = verify_hash_v1; > + > /* Read pubkey from RSA key */ > - x509 = 0; > + if (!params.keyfile) > + key = "/etc/keys/pubkey_evm.pem"; > } else if (sig[0] == DIGSIG_VERSION_2) { > verify_hash = verify_hash_v2; > + > /* Read pubkey from x509 cert */ > - x509 = 1; > + if (!params.keyfile) > + init_public_keys("/etc/keys/x509_evm.der"); Also, in "ima-evm-utils: Preload public keys for ima_verify" I call init_public_keys in cmd_verify_ima() which calls this verify_hash(). So there will be double calling of init_public_keys(). ps. Btw, I think we should remove this verify_hash_fn_t indirect call trick and replace it with two normal calls in each if branch. verify_hash() calling verify_hash() obscures cscope, and with direct calls it will be easier to parse. I may send patch for this. Thanks, > } else > return -1; > > - /* Determine what key to use for verification*/ > - key = params.keyfile ? : x509 ? > - "/etc/keys/x509_evm.der" : > - "/etc/keys/pubkey_evm.pem"; > - > return verify_hash(file, hash, size, sig, siglen, key); > } > > -- > 2.7.5