Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Disable SSLv2 and SSLv3 in CurlHandler#6672

Merged
stephentoub merged 2 commits intodotnet:masterfrom
stephentoub:disable_ssl
Mar 5, 2016
Merged

Disable SSLv2 and SSLv3 in CurlHandler#6672
stephentoub merged 2 commits intodotnet:masterfrom
stephentoub:disable_ssl

Conversation

@stephentoub
Copy link
Member

Forces CurlHandler to not use SSLv2 or SSLv3 even if they're available. SSLv2 has been disabled by default in libcurl since 7.18.1, and SSLv3 since 7.39.0, but this ensures they're disabled even before those versions.

Fixes https://github.com/dotnet/corefx/issues/6668
cc: @bartonjs, @CIPop, @davidsh, @kapilash, @joelverhagen

@blowdart
Copy link
Contributor

blowdart commented Mar 4, 2016

👏 That made my day


internal static void SetSslOptions(EasyRequest easy, ClientCertificateOption clientCertOption)
{
easy.SetCurlOption(Interop.Http.CURLoption.CURLOPT_SSLVERSION, (long)Interop.Http.CurlSslVersion.CURL_SSLVERSION_TLSv1); // disable SSLv2/SSLv3
Copy link
Member

Choose a reason for hiding this comment

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

I went to look this up in the documentation to confirm that this wasn't going to lock it to 1.0.

Might be worth a comment that says that this value enables all TLS 1.x versions and disables SSLv2 and SSLv3.

Copy link
Contributor

Choose a reason for hiding this comment

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

I went to look this up in the documentation to confirm that this wasn't going to lock it to 1.0.

I had the same thought.

Copy link
Member Author

Choose a reason for hiding this comment

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

Tweaked the comment that was there.

@bartonjs
Copy link
Member

bartonjs commented Mar 4, 2016

LGTM, aside from maybe adding one comment.

(Oh, no, what about the 0.01% (1/10000) of sites that still use SSLv3? There, pretended to defend them.)

@justinvp
Copy link
Contributor

justinvp commented Mar 4, 2016

👍

@davidsh
Copy link
Contributor

davidsh commented Mar 4, 2016

LGTM

stephentoub added a commit that referenced this pull request Mar 5, 2016
Disable SSLv2 and SSLv3 in CurlHandler
@stephentoub stephentoub merged commit 27d02a5 into dotnet:master Mar 5, 2016
@stephentoub stephentoub deleted the disable_ssl branch March 5, 2016 01:34
@karelz karelz modified the milestone: 1.0.0-rtm Dec 3, 2016
@karelz karelz added the os-linux Linux OS (any supported distro) label Mar 8, 2017
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
Disable SSLv2 and SSLv3 in CurlHandler

Commit migrated from dotnet/corefx@27d02a5
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Net os-linux Linux OS (any supported distro)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants