From 6387ec6d492caffa4c9bc137f1cb6c171366c7c7 Mon Sep 17 00:00:00 2001 From: Daniel Kubec Date: Sun, 28 Sep 2025 00:24:18 +0200 Subject: [PATCH] Fix EVP_DecryptFinal_ex() for ChaCha20-Poly1305. When using the ChaCha20-Poly1305 algorithm, the final interface returns success without setting the authentication tag, whereas the AES-GCM algorithm correctly returns failure in such cases. Fixes #28137 Reviewed-by: Shane Lontis Reviewed-by: Paul Dale Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/28683) --- .../ciphers/cipher_chacha20_poly1305.c.in | 10 ++- test/evp_extra_test.c | 90 ++++++++++++++++++- 2 files changed, 95 insertions(+), 5 deletions(-) diff --git a/providers/implementations/ciphers/cipher_chacha20_poly1305.c.in b/providers/implementations/ciphers/cipher_chacha20_poly1305.c.in index 8a474395dd2..bfb54d7f841 100644 --- a/providers/implementations/ciphers/cipher_chacha20_poly1305.c.in +++ b/providers/implementations/ciphers/cipher_chacha20_poly1305.c.in @@ -330,14 +330,18 @@ static int chacha20_poly1305_cipher(void *vctx, unsigned char *out, static int chacha20_poly1305_final(void *vctx, unsigned char *out, size_t *outl, size_t outsize) { - PROV_CIPHER_CTX *ctx = (PROV_CIPHER_CTX *)vctx; + PROV_CHACHA20_POLY1305_CTX *ctx = (PROV_CHACHA20_POLY1305_CTX *)vctx; PROV_CIPHER_HW_CHACHA20_POLY1305 *hw = - (PROV_CIPHER_HW_CHACHA20_POLY1305 *)ctx->hw; + (PROV_CIPHER_HW_CHACHA20_POLY1305 *)ctx->base.hw; if (!ossl_prov_is_running()) return 0; - if (hw->aead_cipher(ctx, out, outl, NULL, 0) <= 0) + /* The tag must be set before actually decrypting data */ + if (!ctx->base.enc && ctx->tag_len == 0) + return 0; + + if (hw->aead_cipher((PROV_CIPHER_CTX *)ctx, out, outl, NULL, 0) <= 0) return 0; *outl = 0; diff --git a/test/evp_extra_test.c b/test/evp_extra_test.c index 4a39ed243c3..ac1b62c4fa2 100644 --- a/test/evp_extra_test.c +++ b/test/evp_extra_test.c @@ -4066,10 +4066,16 @@ static int test_decrypt_null_chunks(void) unsigned char msg[] = "It was the best of times, it was the worst of times"; unsigned char ciphertext[80]; unsigned char plaintext[80]; + unsigned char tag[16]; /* We initialise tmp to a non zero value on purpose */ int ctlen, ptlen, tmp = 99; int ret = 0; const int enc_offset = 10, dec_offset = 20; + OSSL_PARAM params[2]; + + params[0] = OSSL_PARAM_construct_octet_string(OSSL_CIPHER_PARAM_AEAD_TAG, + tag, sizeof(tag)); + params[1] = OSSL_PARAM_construct_end(); if (!TEST_ptr(cipher = EVP_CIPHER_fetch(testctx, "ChaCha20-Poly1305", testpropq)) || !TEST_ptr(ctx = EVP_CIPHER_CTX_new()) @@ -4086,12 +4092,13 @@ static int test_decrypt_null_chunks(void) sizeof(msg) - enc_offset)) || !TEST_int_eq(ctlen += tmp, sizeof(msg)) || !TEST_true(EVP_EncryptFinal(ctx, ciphertext + ctlen, &tmp)) - || !TEST_int_eq(tmp, 0)) + || !TEST_int_eq(tmp, 0) + || !TEST_true(EVP_CIPHER_CTX_get_params(ctx, params))) goto err; /* Deliberately initialise tmp to a non zero value */ tmp = 99; - if (!TEST_true(EVP_DecryptInit_ex(ctx, cipher, NULL, key, iv)) + if (!TEST_true(EVP_DecryptInit_ex2(ctx, cipher, key, iv, params)) || !TEST_true(EVP_DecryptUpdate(ctx, plaintext, &ptlen, ciphertext, dec_offset)) /* @@ -5257,6 +5264,83 @@ static int test_evp_updated_iv(int idx) return testresult; } +typedef struct { + const char *cipher; +} EVP_FINAL_NO_TAG_TEST_st; + +static const EVP_FINAL_NO_TAG_TEST_st evp_final_no_tag[] = { + { + "chacha20-poly1305" + }, + { + "aes-256-gcm" + } +}; + +static int test_evp_final_no_tag(int idx) +{ + const EVP_FINAL_NO_TAG_TEST_st *t = &evp_final_no_tag[idx]; + EVP_CIPHER_CTX *ctx = NULL; + EVP_CIPHER *cipher = NULL; + unsigned char tag[16]; + unsigned char data[5] = {1, 1, 1, 1, 1}; + uint32_t data_len = 5; + unsigned char ctext[1024], plaintext[1024]; + int ctext_len = 0, len = 0, testresult = 0; + unsigned char key[] = { + 0xc9, 0xee, 0xa3, 0x0c, 0x1c, 0x59, 0x0c, 0x8b, 0xd8, 0xbb, 0xa1, 0x1c, + 0xbc, 0x3a, 0x56, 0xe7, 0xb7, 0xe1, 0x9f, 0xfd, 0x3b, 0x4a, 0xa3, 0xd5, + 0xc4, 0xdc, 0x2e, 0x62, 0xe6, 0x75, 0x15, 0x5c + }; + unsigned char iv[16] = { + 0x03, 0x2d, 0x79, 0xef, 0xed, 0x2e, 0xad, 0x3e, 0x0b, 0xdc, 0x8f, 0x57, + 0x0d, 0x0e, 0x0f, 0x10 + }; + + if ((cipher = EVP_CIPHER_fetch(testctx, t->cipher, testpropq)) == NULL) { + TEST_info("cipher %s not supported, skipping", t->cipher); + goto ok; + } + + if (!TEST_ptr(ctx = EVP_CIPHER_CTX_new())) + goto err; + if (!TEST_true(EVP_EncryptInit_ex(ctx, cipher, NULL, key, iv))) + goto err; + if (!TEST_true(EVP_EncryptUpdate(ctx, ctext, &len, data, data_len))) + goto err; + ctext_len = len; + if (!TEST_true(EVP_EncryptFinal_ex(ctx, ctext + len, &len))) + goto err; + + ctext_len += len; + if (!TEST_true(EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_AEAD_GET_TAG, 16, tag))) + goto err; + EVP_CIPHER_CTX_free(ctx); + + if (!TEST_ptr(ctx = EVP_CIPHER_CTX_new())) + goto err; + if (!TEST_true(EVP_DecryptInit_ex(ctx, cipher, NULL, key, iv))) + goto err; + if (!TEST_true(EVP_DecryptUpdate(ctx, plaintext, &len, ctext, ctext_len))) + goto err; + if (!TEST_mem_eq(plaintext, 5, data, 5)) + goto err; + + /* + * The tag must be set before decrypting the data; here we expect failure + * for each of the defined ciphers. + */ + if (!TEST_false(EVP_DecryptFinal_ex(ctx, ctext + len, &len))) + goto err; + +ok: + testresult = 1; +err: + EVP_CIPHER_CTX_free(ctx); + EVP_CIPHER_free(cipher); + return testresult; +} + typedef struct { const unsigned char *iv1; const unsigned char *iv2; @@ -6923,6 +7007,8 @@ int setup_tests(void) ADD_ALL_TESTS(test_evp_reinit_seq, OSSL_NELEM(evp_reinit_tests)); ADD_ALL_TESTS(test_gcm_reinit, OSSL_NELEM(gcm_reinit_tests)); ADD_ALL_TESTS(test_evp_updated_iv, OSSL_NELEM(evp_updated_iv_tests)); + ADD_ALL_TESTS(test_evp_final_no_tag, OSSL_NELEM(evp_final_no_tag)); + ADD_ALL_TESTS(test_ivlen_change, OSSL_NELEM(ivlen_change_ciphers)); if (OSSL_NELEM(keylen_change_ciphers) - 1 > 0) ADD_ALL_TESTS(test_keylen_change, OSSL_NELEM(keylen_change_ciphers) - 1); -- 2.47.3