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)

(cherry picked from commit c9ddc5af5199909d196ee80ccd7abcff2eb42a34)

diff --git a/providers/implementations/macs/cmac_prov.c b/providers/implementations/macs/cmac_prov.c
index b44f13b5faec3694213de8019ad319b3077bb781..5b38273d341f13ca9f838668851b38867a955f22 100644 (file)
--- a/providers/implementations/macs/cmac_prov.c
+++ b/providers/implementations/macs/cmac_prov.c
@@ -122,7 +122,8 @@ static int cmac_init(void *vmacctx, const unsigned char *key,
         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 89904fc89d40785dfa409387627f4254399c93fe..589d29b54fd546bebd70167d03ce8c29bceaeb7f 100644 (file)
--- a/providers/implementations/macs/gmac_prov.c
+++ b/providers/implementations/macs/gmac_prov.c
@@ -120,7 +120,7 @@ static int gmac_init(void *vmacctx, const unsigned char *key,
         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 @@ static int gmac_set_ctx_params(void *vmacctx, const OSSL_PARAM params[])
 
     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 78c4924a38c086fccd56b3c6f22733525086cec1..297971af422cedf6e0c87948488e8f909fcaee06 100644 (file)
--- a/providers/implementations/macs/hmac_prov.c
+++ b/providers/implementations/macs/hmac_prov.c
@@ -152,7 +152,7 @@ static int hmac_setkey(struct hmac_data_st *macctx,
 {
     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 @@ static int hmac_init(void *vmacctx, const unsigned char *key,
     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 @@ static int hmac_set_ctx_params(void *vmacctx, const OSSL_PARAM params[])
     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)
-            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 */))
+        if (!hmac_setkey(macctx, p->data, p->data_size))
             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 5a0992655187479273ce3f248ab2f5b6d4ca9fd8..ad67216d2a4b4be000636258c46a6174f125a97d 100644 (file)
--- a/providers/implementations/macs/poly1305_prov.c
+++ b/providers/implementations/macs/poly1305_prov.c
@@ -37,6 +37,7 @@ static OSSL_FUNC_mac_final_fn poly1305_final;
 
 struct poly1305_data_st {
     void *provctx;
+    int updated;
     POLY1305 poly1305;           /* Poly1305 data */
 };
 
@@ -98,7 +99,8 @@ static int poly1305_init(void *vmacctx, const unsigned char *key,
         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 @@ static int poly1305_update(void *vmacctx, const unsigned char *data,
 {
     struct poly1305_data_st *ctx = vmacctx;
 
+    ctx->updated = 1;
     if (datalen == 0)
         return 1;
 
@@ -121,6 +124,7 @@ static int poly1305_final(void *vmacctx, unsigned char *out, size_t *outl,
 
     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 0c374bd86192386452a5c27b45c889775e8355d2..2a291d7f45ccaff1423acc14f1005ffc950ebead 100644 (file)
--- a/providers/implementations/macs/siphash_prov.c
+++ b/providers/implementations/macs/siphash_prov.c
@@ -39,6 +39,7 @@ static OSSL_FUNC_mac_final_fn siphash_final;
 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 size_t siphash_size(void *vmacctx)
 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 @@ static int siphash_init(void *vmacctx, const unsigned char *key, size_t keylen,
     /* 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);
 }