Issue 21887: Audit all ciphers to ensure dupctx method is implemented by nhorman · Pull Request #21933 · openssl/openssl

nhorman · 2023-09-01T18:21:15Z

It was noted that several cipher implementations were missing their dupctx method, resulting in an inability to use EVP_CIPHER_CTX_dup/copy, which was then ignored because the cipher tests just assume that a failure there was a lack of support, and ignored them. Update the tests to fail on a missing dupctx method, and implement said method in all those that are missing it

Fixes #21887

Checklist

tests are added or updated

hlandau · 2023-09-04T06:51:02Z

Needs rebase

nhorman · 2023-09-04T12:01:31Z

rebase complete

test/evp_test.c

providers/implementations/ciphers/cipher_aes_cbc_hmac_sha.c

test/evp_test.c

t8m

Just nits

providers/implementations/ciphers/cipher_aes_ccm.c

providers/implementations/ciphers/cipher_aes_gcm_siv.c

providers/implementations/ciphers/cipher_aria_gcm.c

providers/implementations/ciphers/cipher_sm4_ccm.c

test/evp_test.c

paulidale

Found an oopsy.

providers/implementations/ciphers/cipher_aes_cbc_hmac_sha.c

providers/implementations/ciphers/cipher_aes_wrp.c

There should be no reason that a cipher can't be duplicated Fixes openssl#21887

Add dupctx method support to to ciphers implemented with IMPLEMENT_aead_cipher This includes: aes-<kbits>-gcm aria-<kbits>-ccm aria-<kbits>-gcm sm4-<kibs>-gcm Fixes openssl#21887

create a dupctx method for aes_WRAP implementations of all sizes Fixes openssl#21887

This cipher family has a dupctx function, but was failing because it was attempting to memdup a field only if it was null Fix the conditional check to get it working again Fixes openssl#21887

Pretty straightforward, just clone the requested context, no pointers to fixup Fixes #21887 Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl/openssl#21933) Signed-off-by: fly2x <fly2x@hitls.org>

There should be no reason that a cipher can't be duplicated Fixes #21887 Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl/openssl#21933) (cherry picked from commit 39d857b) Signed-off-by: fly2x <fly2x@hitls.org>

Add dupctx method support to to ciphers implemented with IMPLEMENT_aead_cipher This includes: aes-<kbits>-gcm aria-<kbits>-ccm aria-<kbits>-gcm sm4-<kibs>-gcm Fixes #21887 Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl/openssl#21933) (cherry picked from commit 0239fb3) Signed-off-by: fly2x <fly2x@hitls.org>

create a dupctx method for aes_WRAP implementations of all sizes Fixes #21887 Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl/openssl#21933) (cherry picked from commit 2c021e7) Signed-off-by: fly2x <fly2x@hitls.org>

Same as chacha20 in the last commit, just clone the ctx and its underlying tlsmac array if its allocated Fixes #21887 Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl/openssl#21933) (cherry picked from commit df93b3c) Signed-off-by: fly2x <fly2x@hitls.org>

Pretty straightforward, just clone the requested context, no pointers to fixup Fixes #21887 Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl/openssl#21933) (cherry picked from commit 123c858) Signed-off-by: fly2x <fly2x@hitls.org>

alex · 2023-12-09T01:19:28Z

It looks like this was backported to 3.1, then reverted in #22081, but never re-applied. What needs to happen to get this + the fix for the regression re-applied to 3.1?

t8m · 2023-12-12T09:09:46Z

I am not sure it is a good idea to backport this. It is not a bug fix in a strict sense.

alex · 2023-12-12T11:53:30Z

It is a regression compared to the 1.x series, where dup ctx worked on these ciphers.

t8m · 2023-12-12T13:05:36Z

Hmm... true, I forgot about that. @nhorman would you please create a backport PRs? I think they need to be separate for 3.1 and 3.0 branches if I am not mistaken. Also please do not forget to add all the later fixes that were applied after the initial PR.

nhorman · 2023-12-12T13:14:08Z

#23018
#23019

Opened to track backporting work

There should be no reason that a cipher can't be duplicated Fixes openssl#21887 Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#21933)

Add dupctx method support to to ciphers implemented with IMPLEMENT_aead_cipher This includes: aes-<kbits>-gcm aria-<kbits>-ccm aria-<kbits>-gcm Fixes openssl#21887 Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#21933)

create a dupctx method for aes_WRAP implementations of all sizes Fixes openssl#21887 Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#21933)

Same as chacha20 in the last commit, just clone the ctx and its underlying tlsmac array if its allocated Fixes openssl#21887 Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#21933)

Pretty straightforward, just clone the requested context, no pointers to fixup Fixes openssl#21887 Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#21933)