Add TLS1.3 support #169
Open
Add TLS1.3 support #169
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
tomato42
reviewed
Dec 12, 2018
| @@ -31,6 +31,7 @@ else | |||
| case "$(uname -s)" in | |||
| Darwin) | |||
| opensslbin_name="openssl-darwin64" | |||
| openssl_legacy_bin_name="openssl-darwin64" | |||
There was a problem hiding this comment.
Yeah, until I build a recent openssl for darwin.
|
Any objection to merge this? (other than travisci being stupid) |
|
I haven't really gone through this, I should have some time to do it this weekend |
tomato42
requested changes
Jan 13, 2019
| @@ -126,6 +128,16 @@ SHORTCIPHERSUITE=( | |||
| join_array_by_char ':' "${SHORTCIPHERSUITE[@]}" | |||
| SHORTCIPHERSUITESTRING="$joined_array" | |||
|
|
|||
| # TLS 1.3 is different from other versions of the protocol and | |||
| # ciphersuites must be passed to openssl explicitely | |||
| if [[ $line =~ New,\ ]]; then | ||
| local match=($line) | ||
| current_protocol="${match[1]%%,}" |
There was a problem hiding this comment.
don't older versions print SSL3/TLS1 here?
| @@ -447,7 +478,7 @@ parse_openssl_output() { | |||
| fi | |||
|
|
|||
| # extract used protocol | |||
| if [[ $line =~ ^Protocol\ + ]]; then | |||
| if [[ $line =~ Protocol\ + ]]; then | |||
| @@ -1439,8 +1550,8 @@ test_tls_tolerance() { | |||
| # v2, small but with TLS1.1 as max version | |||
| # | |||
| ratelimit | |||
| verbose "Testing fallback with ${sslcommand[*]} -no_tls1_2" | |||
| local tmp=$(echo Q | "${sslcommand[@]}" -no_tls1_2 2>/dev/null) | |||
| verbose "Testing fallback with ${sslcommand[*]} -no_tls1_3 -no_tls1_2" | |||
There was a problem hiding this comment.
if we are adding that unconditionally, then we need to check for version of OpenSSL (if provided by user) and abort if it's not 1.1.1
|
oh, and maybe try to rebase on top of master? |
ap-wtioit
added a commit
to ap-wtioit/cipherscan
that referenced
this pull request
Dec 12, 2022
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.
I finally decided to take the easiest possible route and create a new openssl binary based off 1.1.1, and rename the previous binary as legacy. This will allow us to keep support for old ciphers/protocols and track the latest features at the same time.
Since TLS1.3 is fairly different from previous versions, I created a separate function to scan for it. We can't use the usual "discard previous used cipher" algorithm with it, as OpenSSL doesn't support the
!flag in the-ciphersuitesparameter, but the logic isn't that different otherwise.The refactoring caused a few issues that I think are now all fixed, but we should still test heavily before merging this patch.