Af/optional min tls version#1037
Conversation
… for property and initialization testing.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…rlsessionConfiguration.tlsMinimumSupportedProtocolVersion = .TLSv12 instead of .TLSv10/.TLSv11, TLS 1.0 and 1.1 have been deprecated.
…SucceedsWithAllTLSVersions
jbelkins
left a comment
jbelkins
left a comment
There was a problem hiding this comment.
Please remove all of the .xcscheme files from this PR. I recommend converting them to local Xcode config instead of shared:
https://developer.apple.com/documentation/xcode/customizing-the-build-schemes-for-a-project#Overview
(Uncheck "Shared" in the Manage Schemes dialog for each scheme file.)
| case .tls10, | ||
| .tls11: | ||
| // Enforce a secure minimum; do not allow TLS 1.0 or 1.1, they have been deprecated. | ||
| urlsessionConfiguration.tlsMinimumSupportedProtocolVersion = .TLSv12 |
There was a problem hiding this comment.
Why is the minimum TLS 1.2 enforced on the URLSession-based client but not on the CRT-based client?
Also, we should provide some sort of notice to the customer that their TLS setting is not supported when they choose a version before TLS 1.2. Logging would probably be the best choice.
There was a problem hiding this comment.
Adding the Logger.error now, as for the enforcement difference between URLSession and CRT, my reasoning was that URLSession uses a security protocol version (from Apple I suspect) that already enforces it for macOS 12.0 so your systems platform should be able to trust but if you're using a macOS before 12.0 then this should stop the user as an added safety measure. CRT on the other hand does not set any minimum because it trust your OS set TLS.
| case tls10 = "TLSv1.0" | ||
| case tls11 = "TLSv1.1" | ||
| case tls12 = "TLSv1.2" | ||
| case tls13 = "TLSv1.3" |
There was a problem hiding this comment.
nit: use underscore in place of the period in case names, i.e. .tls1_2 instead of .tls12
There was a problem hiding this comment.
I corrected the case names.
| xcuserdata | ||
| **/xcshareddata | ||
| **/*.xcshareddata | ||
| xcshareddata |
There was a problem hiding this comment.
Please remove these 3 lines from .gitignore, we actually do, in some cases, want to commit files in Xcode shared data (Not routinely, but in specific situations.)
|
@AntAmazonian The cause of your linux failures appears to be that the CRT library has to be initialized prior to use. Try calling this in the setup function for the crashing test: See |
…TLS min values are enforced, and added a setup() for CRTClientTLSOptionsTests.
3 participants
Add a new option to set TLS version.
Issue #
awslabs/aws-sdk-swift#567
Description of changes
Allow customers to set minimum TLS version.
Scope
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.