fix(cert-create): fix key loading logic
When key_load() attempts to load the key from a file and it fails to
open this file, the 'err_code' output argument is set to
'KEY_ERR_OPEN' error code. However, it is incorrectly overwritten
later on with 'KEY_ERR_NONE' or 'KEY_ERR_LOAD'.
The latter case messes up with the key creation scenario. The
'KEY_ERR_LOAD' error leads the tool to exit, when it should attempt to
create the said key file if invoked with the --new-keys/-n option.
Note that, to complicate matters further, which of 'KEY_ERR_OPEN' or
'KEY_ERR_NONE' values is returned by key_load() depends on the version
of OpenSSL in use:
- If using v3+, KEY_ERROR_LOAD is returned.
- If using <v3, KEY_ERROR_NONE is returned as a result of the key
pair container being initialized by key_new().
This patch fixes this bug and also takes the opportunity to refactor
key_load() implementation to (hopefully) make it more straight-forward
and easier to reason about.
Fixes: 616b3ce27d9a "feat(cert-create): add pkcs11 engine support"
Signed-off-by: Sandrine Bailleux <sandrine.bailleux@arm.com>
Reported-by: Wenchen Tan <xtaens@qq.com>
Change-Id: Ia78ff442e04c5ff98e6ced8d26becbd817a8ccb7
diff --git a/tools/cert_create/include/key.h b/tools/cert_create/include/key.h
index 56f1c21..e0ecdae 100644
--- a/tools/cert_create/include/key.h
+++ b/tools/cert_create/include/key.h
@@ -74,7 +74,7 @@
int key_new(key_t *key);
#endif
int key_create(key_t *key, int type, int key_bits);
-int key_load(key_t *key, unsigned int *err_code);
+unsigned int key_load(key_t *key);
int key_store(key_t *key);
void key_cleanup(void);
diff --git a/tools/cert_create/src/key.c b/tools/cert_create/src/key.c
index 14c8e18..04214aa 100644
--- a/tools/cert_create/src/key.c
+++ b/tools/cert_create/src/key.c
@@ -239,38 +239,34 @@
}
-int key_load(key_t *key, unsigned int *err_code)
+unsigned int key_load(key_t *key)
{
- FILE *fp;
+ if (key->fn == NULL) {
+ VERBOSE("Key not specified\n");
+ return KEY_ERR_FILENAME;
+ }
- if (key->fn) {
- if (!strncmp(key->fn, "pkcs11:", 7)) {
- /* Load key through pkcs11 */
- key->key = key_load_pkcs11(key->fn);
- } else {
- /* Load key from file */
- fp = fopen(key->fn, "r");
- if (fp) {
- key->key = PEM_read_PrivateKey(fp, NULL, NULL, NULL);
- fclose(fp);
- } else {
- WARN("Cannot open file %s\n", key->fn);
- *err_code = KEY_ERR_OPEN;
- }
- }
- if (key->key) {
- *err_code = KEY_ERR_NONE;
- return 1;
- } else {
- ERROR("Cannot load key from %s\n", key->fn);
- *err_code = KEY_ERR_LOAD;
- }
+ if (strncmp(key->fn, "pkcs11:", 7) == 0) {
+ /* Load key through pkcs11 */
+ key->key = key_load_pkcs11(key->fn);
} else {
- VERBOSE("Key not specified\n");
- *err_code = KEY_ERR_FILENAME;
+ /* Load key from file */
+ FILE *fp = fopen(key->fn, "r");
+ if (fp == NULL) {
+ WARN("Cannot open file %s\n", key->fn);
+ return KEY_ERR_OPEN;
+ }
+
+ key->key = PEM_read_PrivateKey(fp, NULL, NULL, NULL);
+ fclose(fp);
+ }
+
+ if (key->key == NULL) {
+ ERROR("Cannot load key from %s\n", key->fn);
+ return KEY_ERR_LOAD;
}
- return 0;
+ return KEY_ERR_NONE;
}
int key_store(key_t *key)
diff --git a/tools/cert_create/src/main.c b/tools/cert_create/src/main.c
index 2ab6bcf..f10a768 100644
--- a/tools/cert_create/src/main.c
+++ b/tools/cert_create/src/main.c
@@ -441,7 +441,8 @@
#endif
/* First try to load the key from disk */
- if (key_load(&keys[i], &err_code)) {
+ err_code = key_load(&keys[i]);
+ if (err_code == KEY_ERR_NONE) {
/* Key loaded successfully */
continue;
}