Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

openssl3 integration: workaround for new EVP_Cipher return code #3466

Merged
merged 3 commits into from
Aug 31, 2022

Conversation

toidiu
Copy link
Contributor

@toidiu toidiu commented Aug 24, 2022

Description of changes:

In openssl3 EVP_Cipher checks if a provider has been set and then introduces different functionality:

  • before: returns 1 on success or 0 on failure
  • now: returns the amount of encrypted / decrypted bytes, or -1 on failure

So based on the source code, openssl3 now checks if there is a provider (cipher→prov) set on the ctx and based on that triggers the new behavior.

Fix1 (recommended):
A solution is to use EVP_EncryptUpdate and EVP_DecryptUpdate as is done in the other cipher implemenations: code.

pros:

  • avoids the new return code behavior change
  • consolidates cipher impl across the codebase

Fix2:
Since in ossl3 we load the "default" and "legacy" providers, we can assume and handle the new return code behavior.

This however introduces the need for ifdef which makes the code more complicated to reason about.

Testing:

Locally unit test passes in ossl3 and also ossl2 locally.


-------libcrypto
    libcrypto.so.3 => /home/ubuntu/projects/rsync/s2n-tls/test-deps/openssl-3.0/lib/libcrypto.so.3 (0x00007fe95a89e000)
-------libcrypto

~/projects/rsync/s2n-tls/tests/unit ~/projects/rsync/s2n-tls
Running /home/ubuntu/projects/rsync/s2n-tls/tests/unit/s2n_aes_sha_composite_test.c ... PASSED    2297077 tests
Running /home/ubuntu/projects/rsync/s2n-tls/tests/unit/s2n_certificate_test.c ... PASSED        392 tests
Running /home/ubuntu/projects/rsync/s2n-tls/tests/unit/s2n_client_hello_test.c ... PASSED        708 tests
Running /home/ubuntu/projects/rsync/s2n-tls/tests/unit/s2n_extension_type_test.c ... PASSED        306 tests
Running /home/ubuntu/projects/rsync/s2n-tls/tests/unit/s2n_mutual_auth_test.c ... PASSED        634 tests
Running /home/ubuntu/projects/rsync/s2n-tls/tests/unit/s2n_optional_client_auth_test.c ... PASSED        774 tests
Running /home/ubuntu/projects/rsync/s2n-tls/tests/unit/s2n_record_size_test.c ... PASSED        183 tests
Running /home/ubuntu/projects/rsync/s2n-tls/tests/unit/s2n_self_talk_offload_signing_test.c ... PASSED         59 tests
Running /home/ubuntu/projects/rsync/s2n-tls/tests/unit/s2n_handshake_test.c ... PASSED       2961 tests
Running /home/ubuntu/projects/rsync/s2n-tls/tests/unit/s2n_self_talk_nonblocking_test.c ... PASSED        271 tests

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@github-actions github-actions bot added the s2n-core team label Aug 24, 2022
@toidiu toidiu changed the title Ak openssl3 evp cipher openssl3 integration: workaround for new EVP_Cipher return code Aug 24, 2022
@toidiu toidiu mentioned this pull request Aug 24, 2022
10 tasks
@toidiu toidiu marked this pull request as ready for review August 25, 2022 00:26
@toidiu toidiu requested a review from a team as a code owner August 25, 2022 00:27
@toidiu toidiu requested review from lrstewart and maddeleine August 25, 2022 00:28
@toidiu toidiu requested a review from franklee26 August 25, 2022 20:57
@toidiu toidiu force-pushed the ak-openssl3_evp_cipher branch from 087808c to b4abd23 Compare August 25, 2022 20:58
@toidiu toidiu force-pushed the ak-openssl3_evp_cipher branch 2 times, most recently from 10df1fc to 5b21395 Compare August 30, 2022 21:57
@toidiu toidiu requested a review from lrstewart August 30, 2022 21:58
@toidiu toidiu enabled auto-merge (squash) August 30, 2022 22:29
@toidiu toidiu force-pushed the ak-openssl3_evp_cipher branch from 5b21395 to 58e6930 Compare August 30, 2022 23:35
@toidiu toidiu merged commit 1209208 into aws:main Aug 31, 2022
@toidiu toidiu deleted the ak-openssl3_evp_cipher branch August 31, 2022 00:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants