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

fix: init implicit iv for serialization feature #4572

Merged
merged 11 commits into from
May 30, 2024
Merged

Conversation

maddeleine
Copy link
Contributor

Resolved issues:

N/A

Description of changes:

The s2n-tls serialization feature doesn't work when s2n-tls is built with AWS-LC or boringssl. The reason is that they assume that the first sequence number to be seen after init is 0, which means that the nonce we provide can be considered the implicit IV. This implicit IV is used to ensure the monotonicity of sequence numbers in the future. Serialization breaks this assumption because after deserialization the sequence number may not be 0, which means that AWS-LC will store the wrong implicit IV.

Therefore this change does a throwaway encrypt call right after deserialization to allow AWS-LC to store the correct implicit IV.

Call-outs:

I also included a code change to close the connection after serializing, as we really don't want people to serialize and continue to use the connection.

Testing:

Increased the number of times we encrypt/decrypt after serialization in the 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 May 24, 2024
tls/s2n_connection_serialize.c Outdated Show resolved Hide resolved
tls/s2n_connection_serialize.c Outdated Show resolved Hide resolved
tls/s2n_connection_serialize.c Outdated Show resolved Hide resolved
tls/s2n_connection_serialize.c Outdated Show resolved Hide resolved
tls/s2n_connection_serialize.c Outdated Show resolved Hide resolved
@maddeleine maddeleine requested a review from lrstewart May 24, 2024 22:59
tls/s2n_connection_serialize.c Outdated Show resolved Hide resolved
tls/s2n_connection_serialize.c Outdated Show resolved Hide resolved
@maddeleine maddeleine requested a review from lrstewart May 28, 2024 21:42
crypto/s2n_evp.h Outdated Show resolved Hide resolved
tests/features/S2N_LIBCRYPTO_SUPPORTS_EVP_AEAD_TLS.c Outdated Show resolved Hide resolved
@maddeleine maddeleine changed the title fix: Throwaway encrypt for serialization fix: init implicit iv for serialization feature May 28, 2024
@maddeleine maddeleine requested a review from lrstewart May 29, 2024 17:14
tests/unit/s2n_connection_serialize_test.c Show resolved Hide resolved
tests/unit/s2n_connection_serialize_test.c Outdated Show resolved Hide resolved
tls/s2n_connection_serialize.c Show resolved Hide resolved
tls/s2n_connection_serialize.c Outdated Show resolved Hide resolved
tls/s2n_connection_serialize.c Outdated Show resolved Hide resolved
@maddeleine maddeleine enabled auto-merge (squash) May 30, 2024 00:06
@maddeleine maddeleine merged commit 76af4d0 into main May 30, 2024
36 checks passed
@maddeleine maddeleine deleted the throwaway_encrypt branch May 30, 2024 00:53
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.

3 participants