Avoid undefined behavior of provided macs on EVP_MAC reinitialization
When the context is reinitialized, i.e. the same key should be used
we must properly reinitialize the underlying implementation.
However in POLY1305 case it does not make sense as this special MAC
should not reuse keys. We fail with this provided implementation
when reinitialization happens.
Fixes #17811
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/18100)
diff --git a/providers/implementations/macs/cmac_prov.c b/providers/implementations/macs/cmac_prov.c
index b44f13b..5b38273 100644
--- a/providers/implementations/macs/cmac_prov.c
+++ b/providers/implementations/macs/cmac_prov.c
@@ -122,7 +122,8 @@
return 0;
if (key != NULL)
return cmac_setkey(macctx, key, keylen);
- return 1;
+ /* Reinitialize the CMAC context */
+ return CMAC_Init(macctx->ctx, NULL, 0, NULL, NULL);
}
static int cmac_update(void *vmacctx, const unsigned char *data,
diff --git a/providers/implementations/macs/gmac_prov.c b/providers/implementations/macs/gmac_prov.c
index 89904fc..589d29b 100644
--- a/providers/implementations/macs/gmac_prov.c
+++ b/providers/implementations/macs/gmac_prov.c
@@ -120,7 +120,7 @@
return 0;
if (key != NULL)
return gmac_setkey(macctx, key, keylen);
- return 1;
+ return EVP_EncryptInit_ex(macctx->ctx, NULL, NULL, NULL, NULL);
}
static int gmac_update(void *vmacctx, const unsigned char *data,
@@ -209,19 +209,22 @@
if (params == NULL)
return 1;
- if (ctx == NULL
- || !ossl_prov_cipher_load_from_params(&macctx->cipher, params, provctx))
+ if (ctx == NULL)
return 0;
- if (EVP_CIPHER_get_mode(ossl_prov_cipher_cipher(&macctx->cipher))
- != EVP_CIPH_GCM_MODE) {
- ERR_raise(ERR_LIB_PROV, PROV_R_INVALID_MODE);
- return 0;
+ if ((p = OSSL_PARAM_locate_const(params, OSSL_MAC_PARAM_CIPHER)) != NULL) {
+ if (!ossl_prov_cipher_load_from_params(&macctx->cipher, params, provctx))
+ return 0;
+ if (EVP_CIPHER_get_mode(ossl_prov_cipher_cipher(&macctx->cipher))
+ != EVP_CIPH_GCM_MODE) {
+ ERR_raise(ERR_LIB_PROV, PROV_R_INVALID_MODE);
+ return 0;
+ }
+ if (!EVP_EncryptInit_ex(ctx, ossl_prov_cipher_cipher(&macctx->cipher),
+ ossl_prov_cipher_engine(&macctx->cipher), NULL,
+ NULL))
+ return 0;
}
- if (!EVP_EncryptInit_ex(ctx, ossl_prov_cipher_cipher(&macctx->cipher),
- ossl_prov_cipher_engine(&macctx->cipher), NULL,
- NULL))
- return 0;
if ((p = OSSL_PARAM_locate_const(params, OSSL_MAC_PARAM_KEY)) != NULL)
if (p->data_type != OSSL_PARAM_OCTET_STRING
diff --git a/providers/implementations/macs/hmac_prov.c b/providers/implementations/macs/hmac_prov.c
index 78c4924..297971a 100644
--- a/providers/implementations/macs/hmac_prov.c
+++ b/providers/implementations/macs/hmac_prov.c
@@ -152,7 +152,7 @@
{
const EVP_MD *digest;
- if (macctx->keylen > 0)
+ if (macctx->key != NULL)
OPENSSL_secure_clear_free(macctx->key, macctx->keylen);
/* Keep a copy of the key in case we need it for TLS HMAC */
macctx->key = OPENSSL_secure_malloc(keylen > 0 ? keylen : 1);
@@ -177,9 +177,11 @@
if (!ossl_prov_is_running() || !hmac_set_ctx_params(macctx, params))
return 0;
- if (key != NULL && !hmac_setkey(macctx, key, keylen))
- return 0;
- return 1;
+ if (key != NULL)
+ return hmac_setkey(macctx, key, keylen);
+
+ /* Just reinit the HMAC context */
+ return HMAC_Init_ex(macctx->ctx, NULL, 0, NULL, NULL);
}
static int hmac_update(void *vmacctx, const unsigned char *data,
@@ -325,22 +327,10 @@
if ((p = OSSL_PARAM_locate_const(params, OSSL_MAC_PARAM_KEY)) != NULL) {
if (p->data_type != OSSL_PARAM_OCTET_STRING)
return 0;
-
- if (macctx->keylen > 0)
- OPENSSL_secure_clear_free(macctx->key, macctx->keylen);
- /* Keep a copy of the key if we need it for TLS HMAC */
- macctx->key = OPENSSL_secure_malloc(p->data_size > 0 ? p->data_size : 1);
- if (macctx->key == NULL)
+ if (!hmac_setkey(macctx, p->data, p->data_size))
return 0;
- memcpy(macctx->key, p->data, p->data_size);
- macctx->keylen = p->data_size;
-
- if (!HMAC_Init_ex(macctx->ctx, p->data, p->data_size,
- ossl_prov_digest_md(&macctx->digest),
- NULL /* ENGINE */))
- return 0;
-
}
+
if ((p = OSSL_PARAM_locate_const(params,
OSSL_MAC_PARAM_TLS_DATA_SIZE)) != NULL) {
if (!OSSL_PARAM_get_size_t(p, &macctx->tls_data_size))
diff --git a/providers/implementations/macs/poly1305_prov.c b/providers/implementations/macs/poly1305_prov.c
index 5a09926..ad67216 100644
--- a/providers/implementations/macs/poly1305_prov.c
+++ b/providers/implementations/macs/poly1305_prov.c
@@ -37,6 +37,7 @@
struct poly1305_data_st {
void *provctx;
+ int updated;
POLY1305 poly1305; /* Poly1305 data */
};
@@ -98,7 +99,8 @@
return 0;
if (key != NULL)
return poly1305_setkey(ctx, key, keylen);
- return 1;
+ /* no reinitialization of context with the same key is allowed */
+ return ctx->updated == 0;
}
static int poly1305_update(void *vmacctx, const unsigned char *data,
@@ -106,6 +108,7 @@
{
struct poly1305_data_st *ctx = vmacctx;
+ ctx->updated = 1;
if (datalen == 0)
return 1;
@@ -121,6 +124,7 @@
if (!ossl_prov_is_running())
return 0;
+ ctx->updated = 1;
Poly1305_Final(&ctx->poly1305, out);
*outl = poly1305_size();
return 1;
diff --git a/providers/implementations/macs/siphash_prov.c b/providers/implementations/macs/siphash_prov.c
index 0c374bd..2a291d7 100644
--- a/providers/implementations/macs/siphash_prov.c
+++ b/providers/implementations/macs/siphash_prov.c
@@ -39,6 +39,7 @@
struct siphash_data_st {
void *provctx;
SIPHASH siphash; /* Siphash data */
+ SIPHASH sipcopy; /* Siphash data copy for reinitialization */
unsigned int crounds, drounds;
};
@@ -94,9 +95,14 @@
static int siphash_setkey(struct siphash_data_st *ctx,
const unsigned char *key, size_t keylen)
{
+ int ret;
+
if (keylen != SIPHASH_KEY_SIZE)
return 0;
- return SipHash_Init(&ctx->siphash, key, crounds(ctx), drounds(ctx));
+ ret = SipHash_Init(&ctx->siphash, key, crounds(ctx), drounds(ctx));
+ if (ret)
+ ctx->sipcopy = ctx->siphash;
+ return ret;
}
static int siphash_init(void *vmacctx, const unsigned char *key, size_t keylen,
@@ -109,8 +115,10 @@
/* Without a key, there is not much to do here,
* The actual initialization happens through controls.
*/
- if (key == NULL)
+ if (key == NULL) {
+ ctx->siphash = ctx->sipcopy;
return 1;
+ }
return siphash_setkey(ctx, key, keylen);
}
