fix: allow b64 decoding using libcrypto for sidechannel resistance #5103
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
* fix commenting style
* update is_base64_char to use addition instaed of bitwise or
* use bitwise xor. This seems less likely to get optimized away
lrstewart
reviewed
Feb 10, 2025
goatgoose
reviewed
Feb 11, 2025
tests/cbmc/proofs/s2n_stuffer_read_base64/s2n_stuffer_read_base64_harness.c
Outdated
Show resolved
Hide resolved
tests/cbmc/proofs/s2n_stuffer_read_base64/s2n_stuffer_read_base64_harness.c
Show resolved
Hide resolved
Co-authored-by: Sam Clark <[email protected]>
* add comment about side-channel resistant decoding * remove debug bound that I added when fighting with CBMC
lrstewart
reviewed
Feb 11, 2025
* cite docs on each quote * use stuffer_wipe_n to clear the null terminator * use base64_group convention
* add known value tests for b64 reading and writing related to padding * copy paste old implementation for more in depth is-char testing
goatgoose
approved these changes
Feb 12, 2025
lrstewart
approved these changes
Feb 12, 2025
* move comment * add posix ensure
* use posix ensure gte
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
This PR gets rid of s2n-tls' b64 implementation and uses the libcrypto's implementation instead. If customers link against a libcrypto with sidechannel-resistant b64 decoding such as AWS-LC, then s2n-tls will also use this sidechannel-resistant implementation.
Additionally,
s2n_is_base64_charno longer performs a table lookup.Credit
We would like to thank researchers Zhiyuan Zhang and Gilles Barthe at 'The Max Planck Institute for Security and Privacy' for identifying and responsibly disclosing this issue to AWS. Security-related questions or concerns can be brought to our attention via [email protected] and reports can be submitted through our Vulnerability Disclosure Program via https://hackerone.com/aws_vdp.
Testing
All existing unit tests should pass. The CBMC test for is_base_64 should also pass.
I also spot checked the generated assembly of the new
s2n_is_base64_charto ensure that there is minimal branching: here