s2n-tls icon indicating copy to clipboard operation
s2n-tls copied to clipboard

Check failures with openssl 3.0.0

Open loqs opened this issue 4 years ago • 2 comments

Problem:

Check fails when s2n-tls 1.1.1 is built with openssl 3.0.0 with

  cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=/usr -DBUILD_SHARED_LIBS=ON -DUNSAFE_TREAT_WARNINGS_AS_ERRORS=OFF  -S . -B build

UNSAFE_TREAT_WARNINGS_AS_ERRORS=OFF is for https://github.com/aws/s2n-tls/issues/2989 and functions deprecated in openssl 3.0.0:

  • RSA_get0_key
  • EVP_PKEY_get0_RSA
  • EVP_PKEY_cmp

See test log https://gist.github.com/loqs/df14add57fcb773a71426f80a28537c8

There appear to be multiple distinct failures being triggered:

  1. 'Error encountered in /build/s2n-tls/src/s2n-tls-1.1.1/utils/s2n_safety.c:210' 1 - s2n_3des_test (Failed) 5 - s2n_aes_test (Failed)
  2. 'Error encountered in /build/s2n-tls/src/s2n-tls-1.1.1/crypto/s2n_composite_cipher_aes_sha.c:165' 4 - s2n_aes_sha_composite_test (Failed) 31 - s2n_client_hello_test (Failed) 69 - s2n_extension_type_test (Failed) 79 - s2n_handshake_test (Failed) 94 - s2n_mutual_auth_test (Failed) 96 - s2n_optional_client_auth_test (Failed) 114 - s2n_record_size_test (Failed) 135 - s2n_self_talk_offload_signing_test (Failed)
  3. 'Error encountered in /build/s2n-tls/src/s2n-tls-1.1.1/crypto/s2n_rsa_signing.c:220' 120 - s2n_rsa_pss_rsae_test (Failed)
  4. Test timeout 134 - s2n_self_talk_nonblocking_test (Timeout)

Solution:

  1. 'Error encountered in /build/s2n-tls/src/s2n-tls-1.1.1/utils/s2n_safety.c:210' I was able to fix by replacing use of EVP_CIPH_NO_PADDING in calls to EVP_CIPHER_CTX_set_padding with 0.

loqs avatar Oct 05 '21 22:10 loqs

Sorry for the late response; as you can see s2n-tls doesn't yet support OpenSSL 3.0.0, any help enumerating where the issues are is appreciated.

dougch avatar May 26 '22 00:05 dougch

Sorry for the late response; as you can see s2n-tls doesn't yet support OpenSSL 3.0.0, any help enumerating where the issues are is appreciated.

Issue 1 https://github.com/aws/s2n-tls/blob/664fef571e7051a4cce290e5dac5ac6919c375ee/crypto/s2n_composite_cipher_aes_sha.c#L165 https://github.com/aws/s2n-tls/blob/664fef571e7051a4cce290e5dac5ac6919c375ee/crypto/s2n_composite_cipher_aes_sha.c#L175 https://www.openssl.org/docs/man3.0/man3/EVP_Cipher.html Return Values

EVP_Cipher() returns the amount of encrypted / decrypted bytes, or -1 on failure if the flag EVP_CIPH_FLAG_CUSTOM_CIPHER is set for the cipher. EVP_Cipher() returns 1 on success or 0 on failure, if the flag EVP_CIPH_FLAG_CUSTOM_CIPHER is not set for the cipher.

With OpenSSL 3.0 EVP_CIPH_FLAG_CUSTOM_CIPHER is set for composite ciphers so the return value is the number of bytes processed while POSIX_GUARD_OSSL requires the return value to be 1. EVP_CIPHER_CTX_test_flags can be used to check for EVP_CIPH_FLAG_CUSTOM_CIPHER and adjust the expected return value and fix the related test failures. Change is backward compatible with OpenSSL 1.1 and 1.0.

Issue 2: crypto/s2n_cbc_cipher_aes.c four occurrences https://github.com/aws/s2n-tls/blob/fe8df74ddafb712d3141056107db7228b19b6ef7/crypto/s2n_cbc_cipher_aes.c#L65 https://github.com/aws/s2n-tls/blob/fe8df74ddafb712d3141056107db7228b19b6ef7/crypto/s2n_cbc_cipher_aes.c#L75 https://github.com/aws/s2n-tls/blob/fe8df74ddafb712d3141056107db7228b19b6ef7/crypto/s2n_cbc_cipher_aes.c#L85 https://github.com/aws/s2n-tls/blob/fe8df74ddafb712d3141056107db7228b19b6ef7/crypto/s2n_cbc_cipher_aes.c#L95 crypto/s2n_composite_cipher_aes_sha.c eight occurrences https://github.com/aws/s2n-tls/blob/664fef571e7051a4cce290e5dac5ac6919c375ee/crypto/s2n_composite_cipher_aes_sha.c#L203 https://github.com/aws/s2n-tls/blob/664fef571e7051a4cce290e5dac5ac6919c375ee/crypto/s2n_composite_cipher_aes_sha.c#L213 https://github.com/aws/s2n-tls/blob/664fef571e7051a4cce290e5dac5ac6919c375ee/crypto/s2n_composite_cipher_aes_sha.c#L223 https://github.com/aws/s2n-tls/blob/664fef571e7051a4cce290e5dac5ac6919c375ee/crypto/s2n_composite_cipher_aes_sha.c#L233 https://github.com/aws/s2n-tls/blob/664fef571e7051a4cce290e5dac5ac6919c375ee/crypto/s2n_composite_cipher_aes_sha.c#L243 https://github.com/aws/s2n-tls/blob/664fef571e7051a4cce290e5dac5ac6919c375ee/crypto/s2n_composite_cipher_aes_sha.c#L253 https://github.com/aws/s2n-tls/blob/664fef571e7051a4cce290e5dac5ac6919c375ee/crypto/s2n_composite_cipher_aes_sha.c#L263 https://github.com/aws/s2n-tls/blob/664fef571e7051a4cce290e5dac5ac6919c375ee/crypto/s2n_composite_cipher_aes_sha.c#L273 crypto/s2n_cbc_cipher_3des.c two occurences https://github.com/aws/s2n-tls/blob/fe8df74ddafb712d3141056107db7228b19b6ef7/crypto/s2n_cbc_cipher_3des.c#L60 https://github.com/aws/s2n-tls/blob/fe8df74ddafb712d3141056107db7228b19b6ef7/crypto/s2n_cbc_cipher_3des.c#L70 https://www.openssl.org/docs/manmaster/man3/EVP_CIPHER_CTX_set_padding.html

EVP_CIPHER_CTX_set_padding() Enables or disables padding. This function should be called after the context is set up for encryption or decryption with EVP_EncryptInit_ex2(), EVP_DecryptInit_ex2() or EVP_CipherInit_ex2(). By default encryption operations are padded using standard block padding and the padding is checked and removed when decrypting. If the pad parameter is zero then no padding is performed, the total amount of data encrypted or decrypted must then be a multiple of the block size or an error will occur. crypto/s2n_cbc_cipher_aes.c four occurrences

EVP_CIPHER_CTX_set_padding is called before the context is set up which under OpenSSL3.0 causes it to be ignored. EVP_CIPH_NO_PADDING is 0x100 and is meant to be used internally by the OpenSSL library. Moving EVP_CIPHER_CTX_set_padding calls to after context set up and replacing EVP_CIPH_NO_PADDING with 0 fixes related test failures. Change is backward compatible with OpenSSL 1.1 and 1.0.

Issue 3: https://github.com/aws/s2n-tls/blob/fe8df74ddafb712d3141056107db7228b19b6ef7/tests/unit/s2n_rsa_pss_rsae_test.c#L245 https://www.openssl.org/docs/manmaster/man7/migration_guide.html

Functions that return an internal key should be treated as read only

Functions such as EVP_PKEY_get0_RSA(3) behave slightly differently in OpenSSL 3.0. Previously they returned a pointer to the low-level key used internally by libcrypto. From OpenSSL 3.0 this key may now be held in a provider. Calling these functions will only return a handle on the internal key where the EVP_PKEY was constructed using this key in the first place, for example using a function or macro such as EVP_PKEY_assign_RSA(3), EVP_PKEY_set1_RSA(3), etc. Where the EVP_PKEY holds a provider managed key, then these functions now return a cached copy of the key. Changes to the internal provider key that take place after the first time the cached key is accessed will not be reflected back in the cached copy. Similarly any changes made to the cached copy by application code will not be reflected back in the internal provider key.

For the above reasons the keys returned from these functions should typically be treated as read-only. To emphasise this the value returned from EVP_PKEY_get0_RSA(3), EVP_PKEY_get0_DSA(3), EVP_PKEY_get0_EC_KEY(3) and EVP_PKEY_get0_DH(3) have been made const. This may break some existing code. Applications broken by this change should be modified. The preferred solution is to refactor the code to avoid the use of these deprecated functions. Failing this the code should be modified to use a const pointer instead. The EVP_PKEY_get1_RSA(3), EVP_PKEY_get1_DSA(3), EVP_PKEY_get1_EC_KEY(3) and EVP_PKEY_get1_DH(3) functions continue to return a non-const pointer to enable them to be "freed". However they should also be treated as read-only.

Which distills down to RSA_set0_key setting rsa_key does not set rsa_public_key.pkey.

EVP_PKEY_set1_RSA(rsa_public_key.pkey, rsa_key);

or something like

#include <openssl/param_build.h>
...
OSSL_PARAM_BLD *bld = OSSL_PARAM_BLD_new();
OSSL_PARAM_BLD_push_BN(bld, "n", n);
OSSL_PARAM_BLD_push_BN(bld, "e", e);
OSSL_PARAM_BLD_push_BN(bld, "d", d);
OSSL_PARAM * params = OSSL_PARAM_BLD_to_param(bld);
OSSL_PARAM_BLD_free(bld);
EVP_PKEY_CTX * ctx = EVP_PKEY_CTX_new_from_name(NULL, "RSA", NULL);
EVP_PKEY_fromdata_init(ctx);
EVP_PKEY_fromdata(ctx, &rsa_public_key.pkey, EVP_PKEY_KEYPAIR, params);
...
EVP_PKEY_CTX_free(ctx);
OSSL_PARAM_free(params);

Issue 4: OpenSSL3.0 split some ciphers into the legacy provider. Currently only affects rc4 https://github.com/aws/s2n-tls/issues/3293#issuecomment-1125591270 Currently I am ensuring the legacy provider is loaded before the test suite is run.

With those four issues resolved and warnings allowed all tests then pass.

loqs avatar Jun 11 '22 19:06 loqs

ossl3 support has been added: https://github.com/aws/s2n-tls/issues/3442

toidiu avatar Sep 12 '22 16:09 toidiu