On 11/19/23 11:50, Mimi Zohar wrote:
Instead of relying on the "imaevm_params.keypass" global variable, which
is not concurrency-safe, add keypass as a parameter to the static library
functions definitions. Update function callers.
To avoid library incompatablity, don't remove imaevm_params.keypass
variable.
Signed-off-by: Mimi Zohar <zohar@xxxxxxxxxxxxx>
---
src/evmctl.c | 9 +++++----
src/libimaevm.c | 17 ++++++++---------
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/src/evmctl.c b/src/evmctl.c
index b802eeb1bf15..6d6160159a1f 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -141,6 +141,7 @@ static bool evm_portable;
static bool veritysig;
static bool hwtpm;
static char *hash_algo;
+static char *keypass;
#define HMAC_FLAG_NO_UUID 0x0001
#define HMAC_FLAG_CAPS_SET 0x0002
@@ -3082,9 +3083,9 @@ int main(int argc, char *argv[])
break;
case 'p':
if (optarg)
- imaevm_params.keypass = optarg;
+ keypass = optarg;
else
- imaevm_params.keypass = get_password();
+ keypass = get_password();
break;
case 'f':
sigfile = 1;
@@ -3226,8 +3227,8 @@ int main(int argc, char *argv[])
}
}
- if (!imaevm_params.keypass)
- imaevm_params.keypass = getenv("EVMCTL_KEY_PASSWORD");
+ if (!keypass)
+ keypass = getenv("EVMCTL_KEY_PASSWORD");
if (imaevm_params.keyfile != NULL &&
imaevm_params.eng == NULL &&
The problem at this point in evmctl is that keypass is never passed to
anywhere. You have to pass it to sign_hash().
diff --git a/src/libimaevm.c b/src/libimaevm.c
index 18b6a6f27237..10ec847da08a 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -1124,7 +1124,8 @@ static int get_hash_algo_v1(const char *algo)
}
static int sign_hash_v1(const char *hashalgo, const unsigned char *hash,
- int size, const char *keyfile, unsigned char *sig)
+ int size, const char *keyfile, const char *keypass,
+ unsigned char *sig)
{
int len = -1, hashalgo_idx;
SHA_CTX ctx;
@@ -1158,7 +1159,7 @@ static int sign_hash_v1(const char *hashalgo, const unsigned char *hash,
log_info("hash(%s): ", hashalgo);
log_dump(hash, size);
- key = read_priv_key(keyfile, imaevm_params.keypass);
+ key = read_priv_key(keyfile, keypass);
if (!key)
return -1;
@@ -1211,7 +1212,8 @@ out:
* Return: -1 signing error, >0 length of signature
*/
static int sign_hash_v2(const char *algo, const unsigned char *hash,
- int size, const char *keyfile, unsigned char *sig)
+ int size, const char *keyfile, const char *keypass,
+ unsigned char *sig)
{
struct signature_v2_hdr *hdr;
int len = -1;
@@ -1246,7 +1248,7 @@ static int sign_hash_v2(const char *algo, const unsigned char *hash,
log_info("hash(%s): ", algo);
log_dump(hash, size);
- pkey = read_priv_pkey(keyfile, imaevm_params.keypass);
+ pkey = read_priv_pkey(keyfile, keypass);
if (!pkey)
return -1;
@@ -1316,14 +1318,11 @@ err:
int sign_hash(const char *hashalgo, const unsigned char *hash, int size, const char *keyfile, const char *keypass, unsigned char *sig)
{
- if (keypass)
- imaevm_params.keypass = keypass;
-
I hope all library callers, other than evmctl, passed the keypass in
already, otherwise they could have set the password via
imaevm_params.keypass global and passed in NULL and for them this will
be a behavioral change. Anyway, you have no other choice to get rid of
imaevm_params as much as possible, so I guess it's ok.
It looks like at this point imaevm_params.keypass doesn't have a single
user anymore so you could rename it to 'unused1' or so in the stucture
since it will have zero effect for anyone to set it.
if (imaevm_params.x509)
- return sign_hash_v2(hashalgo, hash, size, keyfile, sig);
+ return sign_hash_v2(hashalgo, hash, size, keyfile, keypass, sig);
#if CONFIG_SIGV1
else
- return sign_hash_v1(hashalgo, hash, size, keyfile, sig);
+ return sign_hash_v1(hashalgo, hash, size, keyfile, keypass, sig);
#endif
log_info("Signature version 1 deprecated.");
return -1;
