diff options
author | Tomas Mraz <tomas@openssl.org> | 2022-04-12 17:58:23 +0200 |
---|---|---|
committer | Tomas Mraz <tomas@openssl.org> | 2022-04-19 14:07:21 +0200 |
commit | 4f675d8c600bfde652aff28cb10c2d16be11fa65 (patch) | |
tree | 7a1101614f1f02551f8d56f86924da58fc865886 /providers | |
parent | 8a4c3ed751cf256a0702fa87417e4bbf429c4562 (diff) |
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)
Diffstat (limited to 'providers')
-rw-r--r-- | providers/implementations/macs/cmac_prov.c | 3 | ||||
-rw-r--r-- | providers/implementations/macs/gmac_prov.c | 25 | ||||
-rw-r--r-- | providers/implementations/macs/hmac_prov.c | 26 | ||||
-rw-r--r-- | providers/implementations/macs/poly1305_prov.c | 6 | ||||
-rw-r--r-- | providers/implementations/macs/siphash_prov.c | 12 |
5 files changed, 39 insertions, 33 deletions
diff --git a/providers/implementations/macs/cmac_prov.c b/providers/implementations/macs/cmac_prov.c index b44f13b5fa..5b38273d34 100644 --- 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 89904fc89d..589d29b54f 100644 --- 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 78c4924a38..297971af42 100644 --- 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 5a09926551..ad67216d2a 100644 --- 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 0c374bd861..2a291d7f45 100644 --- 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); } |