Skip to content

[HttpClient] Add option crypto_method to set the minimum TLS version and make it default to v1.2 #50274

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

Merged
merged 1 commit into from
May 12, 2023

Conversation

nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented May 9, 2023

Q A
Branch? 6.3
Bug fix? no
New feature? yes
Deprecations? no
Tickets -
License MIT
Doc PR -

Idea borrowed from async-aws/aws#1402 by @GrahamCampbell

Note that Firefox/Chrome disabled support for TLS < 1.2 in 2020 and TLSv1.2 is available since 2008.

@symfony symfony deleted a comment from carsonbot May 9, 2023
@nicolas-grekas nicolas-grekas force-pushed the hc-ssl-options branch 6 times, most recently from 368b912 to 38f0aa1 Compare May 9, 2023 14:46
@stof
Copy link
Member

stof commented May 9, 2023

I don't see where you actually configure the min TLS version of the amp/socket TLS context in this PR. Is this actually implemented in this PR ?
Side note, the default in amp/socket is TLS 1.2, so the current behavior for what is supported in symfony/http-client depends on the client being used.

@@ -141,6 +142,7 @@ private function getClient(array $options): array
$options['local_cert'] && $context = $context->withCertificate(new Certificate($options['local_cert'], $options['local_pk']));
$options['ciphers'] && $context = $context->withCiphers($options['ciphers']);
$options['capture_peer_cert_chain'] && $context = $context->withPeerCapturing();
$options['crypto_method'] && $context = $context->withMinimumVersion($options['crypto_method']);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stof this is the line you're looking for

@nicolas-grekas nicolas-grekas changed the title [HttpClient] Add option crypto_method to set the minimum SSL version and make it default to TLSv1.2 [HttpClient] Add option crypto_method to set the minimum TLS/SSL version and make it default to TLSv1.2 May 9, 2023
@nicolas-grekas nicolas-grekas changed the title [HttpClient] Add option crypto_method to set the minimum TLS/SSL version and make it default to TLSv1.2 [HttpClient] Add option crypto_method to set the minimum SSL/TLS version and make it default to TLSv1.2 May 9, 2023
@nicolas-grekas
Copy link
Member Author

PR updated to target 6.3 in case we'd like to merge it now.

Status: needs review

@nicolas-grekas nicolas-grekas changed the title [HttpClient] Add option crypto_method to set the minimum SSL/TLS version and make it default to TLSv1.2 [HttpClient] Add option crypto_method to set the minimum TLS version and make it default to TLSv1.2 May 10, 2023
@nicolas-grekas nicolas-grekas changed the title [HttpClient] Add option crypto_method to set the minimum TLS version and make it default to TLSv1.2 [HttpClient] Add option crypto_method to set the minimum TLS version and make it default to v1.2 May 10, 2023
@fabpot
Copy link
Member

fabpot commented May 12, 2023

Thank you @nicolas-grekas.

@GrahamCampbell
Copy link
Contributor

FYI, this has now similarly been implemented in Guzzle 7.6.0, though without the change to the default, not just because I feel it is arguably breaking, but also because Guzzle tends to have people using much older technology including versions of curl that may not even have the needed constant defined.

nicolas-grekas added a commit that referenced this pull request Sep 28, 2024
…(HypeMC)

This PR was merged into the 6.4 branch.

Discussion
----------

[HttpClient] Add `crypto_method` to scoped client options

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | -
| License       | MIT

While working on something else I've noticed that the `crypto_method` option wasn't added for scoped clients in #50274. I'm not sure if this was intentional or not, but as far as I can tell, there's no reason for the option to not be there, so I'm guessing it was an oversight.

Commits
-------

e274ee4 [HttpClient] Add `crypto_method` to scoped client options
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants