From b8e5622809d3b3f61c4a615e51f5a8fd492ee23f Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Thu, 10 Sep 2020 14:46:41 +0100 Subject: [PATCH] Don't send -1 as the length of the hmac key The dgst app was using an undocumented behaviour in the EVP_PKEY_new_raw_private_key() function when setting a key length for a MAC. The old EVP_PKEY to MAC bridge, probably by accident, converts a -1 length to a strlen() call, by virtue of the fact that it eventually calls ASN1_STRING_set() which has this feature. As noted above this is undocumented, and unexpected since the len parameter to EVP_PKEY_new_raw_private_key() is an unsigned value (size_t). In the old bridge it was later (silently) cast to an int, and therefore the original -1 value was restored. This only works because sizeof(int) <= sizeof(size_t). If we ever run on a platform where sizeof(int) > sizeof(size_t) then it would have failed. The behaviour also doesn't hold for EVP_PKEY_new_raw_private_key() in general - only when the old MAC bridge was in use. Rather than restore the original behaviour I think it is best to simply fix the dgst app to not assume it exists. We should not bake in this backwards and inconsistent behaviour. Fixes #12837 Reviewed-by: Dmitry Belyavskiy (Merged from https://github.com/openssl/openssl/pull/12850) --- apps/dgst.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/dgst.c b/apps/dgst.c index 0bbde71d4b6..7fc7da1e53a 100644 --- a/apps/dgst.c +++ b/apps/dgst.c @@ -319,7 +319,8 @@ int dgst_main(int argc, char **argv) if (hmac_key != NULL) { sigkey = EVP_PKEY_new_raw_private_key(EVP_PKEY_HMAC, impl, - (unsigned char *)hmac_key, -1); + (unsigned char *)hmac_key, + strlen(hmac_key)); if (sigkey == NULL) goto end; } -- 2.47.2