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 s2n_shutdown + failed recv bug #4350

Merged
merged 4 commits into from
Jan 18, 2024
Merged

Fix s2n_shutdown + failed recv bug #4350

merged 4 commits into from
Jan 18, 2024

Conversation

lrstewart
Copy link
Contributor

@lrstewart lrstewart commented Jan 10, 2024

Resolved issues:

Related to #3976

Description of changes:

I'm working on cleaning up the flaky blinding tests in the tokio bindings. They were mysteriously not failing with the correct expected error. I discovered that because the bindings tests trigger blinding by receiving a malformed record, they were causing s2n_shutdown to try to read with a dirty header_in stuffer. That led to some very strange errors.

I've fixed the problem by cleaning up header_in/in before every read instead of after every read. That's what s2n_shutdown used to do, and I'm not sure why I changed it :(

Testing:

Added a C unit test, fixed the original failing s2n-tls-tokio test

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 Jan 10, 2024
@lrstewart lrstewart marked this pull request as ready for review January 10, 2024 04:54
@lrstewart lrstewart reopened this Jan 17, 2024
Co-authored-by: maddeleine <[email protected]>
@lrstewart lrstewart enabled auto-merge (squash) January 18, 2024 22:37
@lrstewart lrstewart merged commit d946c2d into aws:main Jan 18, 2024
30 checks passed
@lrstewart lrstewart deleted the blinding_c branch January 19, 2024 01:00
jmayclin pushed a commit to jmayclin/s2n-tls that referenced this pull request Jan 20, 2024
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