Prevent failures on 2 subsequent warning alerts if S2N_ALERT_IGNORE_WARNINGS is set #2213
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zz85
approved these changes
Aug 7, 2020
Codecov Report
@@ Coverage Diff @@
## main #2213 +/- ##
=======================================
Coverage ? 83.54%
=======================================
Files ? 249
Lines ? 16611
Branches ? 0
=======================================
Hits ? 13877
Misses ? 2734
Partials ? 0 |
baldwinmatt
approved these changes
Aug 8, 2020
…ARNINGS is set Previously when mode was set to S2N_ALERT_IGNORE_WARNINGS and warning was ignored, we did not clean up the conn->alert_in buffer, so when we received a second warning we would fail the connection with S2N_ERR_ALERT_PRESENT at the beginning of s2n_process_alert_fragment function.
xonatius
force-pushed
the
fix-2-warn-alerts-handling
branch
from
ac7c3f1
to
72b2ca0
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of changes:
Previously when mode was set to
S2N_ALERT_IGNORE_WARNINGSand warning was ignored, we did not clean up theconn->alert_inbuffer, so when we received a second warning we would fail the connection withS2N_ERR_ALERT_PRESENTat the beginning ofs2n_process_alert_fragmentfunction.Call-outs:
One drawback is that we won't be able to fetch all warning alerts we received from the peer through
s2n_connection_get_alertand only the fatal ones will get there. However, I think we can tackle that separately by introducing a different api to get multiple alerts.Testing:
Extended existing unit test to send 2 alerts and check that connection still works in
S2N_ALERT_IGNORE_WARNINGSmode. The test fails without the change.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.