| Message ID | 1563287417-31780-2-git-send-email-zohar@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] ima_evm_utils: erroneous "verification failed: 0 (invalid padding)" message | expand |
Mimi, On Tue, Jul 16, 2019 at 10:30:17AM -0400, Mimi Zohar wrote: > Unlike the user provided list of public keys, we don't know which > default public key file to use until verify_hash(). As a result, the > "Failed to open keyfile" message may be repeated multiple times. > > Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> > --- > src/libimaevm.c | 33 ++++++++++++++++++++++++++++++++- > 1 file changed, 32 insertions(+), 1 deletion(-) > > diff --git a/src/libimaevm.c b/src/libimaevm.c > index 472ab53c7b42..793643331f4b 100644 > --- a/src/libimaevm.c > +++ b/src/libimaevm.c > @@ -296,18 +296,49 @@ err: > return err; > } > > +/* > + * Keep track of missing keyfile names. > + * > + * Return 1 for found, return 0 for not found. > + */ > +static int lookup_keyfile_name(const char *keyfile_name) > +{ > + struct keyfile_name_entry { > + struct keyfile_name_entry *next; > + char name[]; > + } *entry; > + static struct keyfile_name_entry *keyfile_names = NULL; > + > + for (entry = keyfile_names; entry != NULL; entry = entry->next) { > + if (strcmp(entry->name, keyfile_name) == 0) > + return 1; > + } > + > + entry = malloc(sizeof(struct keyfile_name_entry) + > + strlen(keyfile_name) + 1); > + if (entry) { > + strcpy(entry->name, keyfile_name); > + entry->next = keyfile_names; > + keyfile_names = entry; > + } > + return 0; > +} > + > EVP_PKEY *read_pub_pkey(const char *keyfile, int x509) > { > FILE *fp; > X509 *crt = NULL; > EVP_PKEY *pkey = NULL; > + int found; > > if (!keyfile) > return NULL; > > fp = fopen(keyfile, "r"); > if (!fp) { > - log_err("Failed to open keyfile: %s\n", keyfile); > + found = lookup_keyfile_name(keyfile); > + if (!found) > + log_err("Failed to open keyfile: %s\n", keyfile); > return NULL; Now filename list is decoupled from keys themselves. Also we have key list creation in init_public_keys(). Maybe we should just always call init_public_keys for verify operations? Thanks, > } > > -- > 2.7.5
On Wed, 2019-07-17 at 00:49 +0300, Vitaly Chikunov wrote: > Mimi, > > On Tue, Jul 16, 2019 at 10:30:17AM -0400, Mimi Zohar wrote: > > Unlike the user provided list of public keys, we don't know which > > default public key file to use until verify_hash(). As a result, the > > "Failed to open keyfile" message may be repeated multiple times. > > > > Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> > > --- > > src/libimaevm.c | 33 ++++++++++++++++++++++++++++++++- > > 1 file changed, 32 insertions(+), 1 deletion(-) > > > > diff --git a/src/libimaevm.c b/src/libimaevm.c > > index 472ab53c7b42..793643331f4b 100644 > > --- a/src/libimaevm.c > > +++ b/src/libimaevm.c > > @@ -296,18 +296,49 @@ err: > > return err; > > } > > > > +/* > > + * Keep track of missing keyfile names. > > + * > > + * Return 1 for found, return 0 for not found. > > + */ > > +static int lookup_keyfile_name(const char *keyfile_name) > > +{ > > + struct keyfile_name_entry { > > + struct keyfile_name_entry *next; > > + char name[]; > > + } *entry; > > + static struct keyfile_name_entry *keyfile_names = NULL; > > + > > + for (entry = keyfile_names; entry != NULL; entry = entry->next) { > > + if (strcmp(entry->name, keyfile_name) == 0) > > + return 1; > > + } > > + > > + entry = malloc(sizeof(struct keyfile_name_entry) + > > + strlen(keyfile_name) + 1); > > + if (entry) { > > + strcpy(entry->name, keyfile_name); > > + entry->next = keyfile_names; > > + keyfile_names = entry; > > + } > > + return 0; > > +} > > + > > EVP_PKEY *read_pub_pkey(const char *keyfile, int x509) > > { > > FILE *fp; > > X509 *crt = NULL; > > EVP_PKEY *pkey = NULL; > > + int found; > > > > if (!keyfile) > > return NULL; > > > > fp = fopen(keyfile, "r"); > > if (!fp) { > > - log_err("Failed to open keyfile: %s\n", keyfile); > > + found = lookup_keyfile_name(keyfile); > > + if (!found) > > + log_err("Failed to open keyfile: %s\n", keyfile); > > return NULL; > > > Now filename list is decoupled from keys themselves. Also we have key > list creation in init_public_keys(). Maybe we should just always call > init_public_keys for verify operations? Initially, I tried that. The code snippet, below, would be called once. The problem is that only during verify_hash() do we know which default public key file to use. if (params.keyfile) params.keyfile = <default key file> init_public_keys(params.keyfile); Mimi > > Thanks, > > > } > > > > -- > > 2.7.5 >
Vitaly, On Wed, 2019-07-17 at 00:49 +0300, Vitaly Chikunov wrote: > > EVP_PKEY *read_pub_pkey(const char *keyfile, int x509) > > { > > FILE *fp; > > X509 *crt = NULL; > > EVP_PKEY *pkey = NULL; > > + int found; > > > > if (!keyfile) > > return NULL; > > > > fp = fopen(keyfile, "r"); > > if (!fp) { > > - log_err("Failed to open keyfile: %s\n", keyfile); > > + found = lookup_keyfile_name(keyfile); > > + if (!found) > > + log_err("Failed to open keyfile: %s\n", keyfile); > > return NULL; > > > Now filename list is decoupled from keys themselves. Also we have key > list creation in init_public_keys(). Maybe we should just always call > init_public_keys for verify operations? With v2 of "ima_evm_utils: erroneous "verification failed: 0 (invalid padding)" message", I was able to drop this patch. Mimi
diff --git a/src/libimaevm.c b/src/libimaevm.c index 472ab53c7b42..793643331f4b 100644 --- a/src/libimaevm.c +++ b/src/libimaevm.c @@ -296,18 +296,49 @@ err: return err; } +/* + * Keep track of missing keyfile names. + * + * Return 1 for found, return 0 for not found. + */ +static int lookup_keyfile_name(const char *keyfile_name) +{ + struct keyfile_name_entry { + struct keyfile_name_entry *next; + char name[]; + } *entry; + static struct keyfile_name_entry *keyfile_names = NULL; + + for (entry = keyfile_names; entry != NULL; entry = entry->next) { + if (strcmp(entry->name, keyfile_name) == 0) + return 1; + } + + entry = malloc(sizeof(struct keyfile_name_entry) + + strlen(keyfile_name) + 1); + if (entry) { + strcpy(entry->name, keyfile_name); + entry->next = keyfile_names; + keyfile_names = entry; + } + return 0; +} + EVP_PKEY *read_pub_pkey(const char *keyfile, int x509) { FILE *fp; X509 *crt = NULL; EVP_PKEY *pkey = NULL; + int found; if (!keyfile) return NULL; fp = fopen(keyfile, "r"); if (!fp) { - log_err("Failed to open keyfile: %s\n", keyfile); + found = lookup_keyfile_name(keyfile); + if (!found) + log_err("Failed to open keyfile: %s\n", keyfile); return NULL; }
Unlike the user provided list of public keys, we don't know which default public key file to use until verify_hash(). As a result, the "Failed to open keyfile" message may be repeated multiple times. Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> --- src/libimaevm.c | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-)