Change default SSL version to nil to use TLS version negotiation. #186
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
|
Hiroshi Nakamura » httpclient #97 SUCCESS |
|
Hiroshi Nakamura » httpclient #98 SUCCESS |
Old default SSLv3 disables version negotiation and does not connect to TSL-only servers. This was really problematic when httpclient was not used directly but by third-party libraries. This commit changes the default version to nil that will not set a SSL version on context creation. This way client and server can negotiation the SSL version and choose the most secure available. SSLv2 is still disabled in the cipher suite setting with "!SSLv2".
|
Hiroshi Nakamura » httpclient #99 SUCCESS |
|
Just to note, this would be a breaking change for some folks expecting to be set... like me ;) . Although, I'm probably the minority. |
|
It still can be set manual but setting it by default for everyone is essential a protocol downgrade attack ;) So this PR is just what you contributed over there: An option to force specific SSL version and an empty default value. It is a breaking change but for now any library using httpclient is just unusable for me. |
|
Actually, Excon did not have a means for setting SSL version, so we added it, but the default SSL version was kept to its original value, thus ensuring it was not breaking. HTTPClient can set SSL version, whats changing is the default. In a perfect world the libraries you have issue with that use HTTPClient would modify their API to expose more of the client allowing you to set the SSL version. I've often wondered why it's not more common to expose http clients or their features/settings in libraries that use them. In the end its up to the maintainer, just voicing a concern that may not have been heard otherwise. |
|
I'm usually arguing against exposing low level settings or operations due to responsibilities and usual lack of knowledge especially when working with any kind of encryption. I expect a HTTP client library to just work using the best possible settings, like protocol versions and ciphers available, without me configuring everything due to this reasons:
I'd expect a crypto library be written and regularly updated by security experts knowing what secure settings are. I see no reason for duplicating settings and defaults as that's just more maintance work and less security. Longer response times in cases of broken ciphers included. Same goes for using own CA cert stores. Maybe adding an option to set global defaults would be nice, so that one can configure HTTPClient in his own app to use more stricter or looser settings independent of any third-party library. Only independent tools (command line tools etc.) would still need to provide configuration options for SSL. That way libraries could use HTTPClient without configuring them as HTTPClient would use sane defaults or external specified global settings. |
nahi
pushed a commit
that referenced
this pull request
May 21, 2014
Change default SSL version to nil to use TLS version negotiation.
|
Sorry for late response, and thanks for the discussion. This PR (and #191, #204) is completely correct (and I was wrong using SSLv3 instead of SSLv23.) I merged both this PR (#186) and #204 which also allows :auto in addition to nil. I didn't merge #191 but it just means other PR includes #191. @JoshMcKin, it's my understanding that HTTPClient (and underlying OpenSSL) negotiates SSL version and it wouldn't break excon. It allows to set ssl_version explicitly including nil value, but not require setting it. I want to provide workaround for compatibility if needed. Do I understanding this issue correctly? Let me know if the new release could break excon. |
|
Hi @nahi, The work around already exists; I would have the set SSLv3 manually if OpenSSL was not picking it up. Perhaps a comment in the release notes stating the default change just in case there is a larger audience affected. Thanks, |
jsonn
pushed a commit
to jsonn/pkgsrc
that referenced
this pull request
Jan 21, 2015
(with post 2.6.0 fix: bin/httpclient one-liner broken)
## Changes
### Changes in 2.6.0
This release includes internal CookieManager implementation change. It
involves compatibility layer but for the case your library depends on internal
implementation it also provides a way to restore the implementation. See below
for more details.
* Changes
* feat: use http-cookie if available for better Cookies spec compliance.
Instead of WebAgent 0.6.2 that is not maintained over 10 years. To omit
maintaining that library use http-cookie for better spec compliance and
healthy development.
This introduces following incompatibility from existing cookies
implementation.
* Expired cookies are not saved. With the old implementation expired
cookies are saved in file and not be sent to the server. With the new
implementation the expired cookies are not saved to the file and not
be sent to the server.
* Cookie#domain returns dot-less domain for domain cookies. Instead,
Cookie#dot_domain returns with dot.
http-cookie is used by default if available but you can restore original
CookieManager behavior by loading 'httpclient/webagent-cookie' feature
before 'httpclient' like this;
```ruby
require 'httpclient/webagent-cookie'
require 'httpclient'
```
The new implementation dumps warnings to help you migrate to http-cookie.
Please follow the suggestion to avoid future compatibility.
```ruby
e.g.
WebAgent::Cookie is deprecated and will be replaced with HTTP::Cookie in the near future. Please use Cookie#origin= instead of Cookie#url= for the replacement.
Cookie#domain returns dot-less domain name now. Use Cookie#dot_domain if you need "." at the beginning.
CookieManager#find is deprecated and will be removed in near future. Use HTTP::Cookie.cookie_value(CookieManager#cookies) instead
```
* feat: Message#previous to get responses in negotiation
HTTP::Message#previous keeps previous response in negotiation. For
redirection, authorization negotiation and retry from custom filter.
Closes #234.
* feat: Add JSONClient
JSONClient auto-converts Hash <-> JSON in request and response.
* For POST or PUT request, convert Hash body to JSON String with
'application/json; charset=utf-8' header.
* For response, convert JSON String to Hash when content-type is
'(application|text)/(x-)?json'
This commit include bin/jsonclient that works as same as bin/httpclient
not with HTTPClient but with JSONClient.
* feat: Add download command
```
% httpclient download http://host/path > file
```
* Bug fixes
* fix: duplicated query params by follow_redirect
When the original request has query and the server returns redirection
response with Location, HTTPClient wrongly adds query to the new URI. In
such case the Location header could include query part;
```
e.g.
http://originalhost/api/call?limit=10
-> Location: http://otherhost/api/call?limit=10
```
HTTPClient should just hit the new location '/api/call?limit=10' not
'/api/call?limit=10&limit=10'. Closes #236.
* fix: NTLM & Basic dual auth
When a server returns two or more WWW-Authenticate headers and the first
one is NTLM, say WWW-Authenticate: NTLM and WWW-Authenticate: Basic in
this order, HTTPClient sent Basic Authorization header after finishing
NTLM auth negotiation.
NTLM auth is a connection authentication scheme so HTTPClient deleted
the internal auth negotiation state so that NTLM authenticator does not
do anything after the negotiation has completed. In such case, for the
subsequent requests, NTLM authenticator does nothing but Basic
authenticator sends Basic Authorization header to the server that is
already negotiated via NTLM authenticator. This can cause authentication
failure.
This commit changes the internal state handling not to delete the state
but introduce :done state. NTLM authenticator returns :skip for the
request to the server that auth negotiation has completed. WWWAuth skips
other authenticator to avoid above issue. Closes #157.
* fix: transplant IO positions to new request in negotiation
In authorization negotiation HTTP::Message for request is generated for
each request, of course, but HTTPClient did not care the IO position
recorded in the previous requests in the subsequent requests. Closes #130.
* fix: avoid inconsistent Content-Length and actual body
If lengths of all posted arguments are known HTTPClient sends
'Content-Length' as a sum length of all arguments. But the length of
actual body was wrong because it read as much as possible regardless of
what IO#size returned. So if the file is getting bigger while HTTPClient
is processing a request the request has inconsistent Content-Length and
body.
This bug is found, and the fix is proposed both by @Teshootub7. Thank
you very much for patient trouble shooting! Fixes #117.
* fix: KeepAliveDisconnected race condition
As details explained in #84, current HTTPClient's KeepAliveDisconnected
handling has a race condition bug that allows a client to have
invalidated connection two or more times. This could be a cause of #185.
To avoid this, make HTTPClient acquire new connection for retry of
KeepAliveDisconnected. Closes #84. Closes #185.
### Changes in 2.5.3
This release includes behavior changes of POST and PUT requests that has
nil as a body. See changes below. Emtpty String as a body is not affected.
* Changes
* Update cacert. "Certificate data from Mozilla as of: Tue Oct 28 22:03:58 2014"
-> Reverted in 2.5.3.3 because it caused unexpected SSLError. See
nahi/httpclient#230
* Allow no content POST and PUT.
Previously POST or PUT with :body => nil meant that 'POST or PUT with 0
length entity body'. But sometimes you need to POST or PUT actually no
content which should not have Content-Type nor Content-Length.
It could be incompatible change for user who POST/PUT-ed with empty body
but it should be rare, actually WEBrick cannot handle such 'no content'
POST and PUT. #128.
* Add default_header property.
:default_header is for providing default headers Hash that all HTTP
requests should have, such as custom 'Authorization' header in API. You
can override :default_header with :header Hash parameter in HTTP request
methods.
* raise if redirect res does not have Location header. #155.
* Bug fixes
* Avoid NPE by a cookie without domain=.
The root cause is still uncertain though. Closes #123
* Suppress verify_callback warning.
Because OpenSSL can try multiple certificate chains and some of it can
fail, and one of them succeeds. For that case warning is irrelevant.
Let it warn only in $DEBUG mode. #221.
### Changes in 2.5.2
Oct 29, 2014 - version 2.5.2
* Changes
* Add :force_basic_auth config - #166, #179, #181.
Generally HTTP client must send Authorization header after it gets 401
error from server from security reason. But in some situation (e.g.
API client) you might want to send Authorization from the beginning.
You can turn on/off force_basic_auth flag for sending Authorization
header from the beginning. (Of cource, if a request URI matches with
the URI you set in set_auth method)
Syntax:
```ruby
HTTPClient.new(:force_basic_auth => true)
# or
c = HTTPClient.new
c.force_basic_auth = true
```
* Add :base_url to HTTPClient configuration.
Passing path to get, post, etc. is recognized as a request to
:base_url + uri. If you pass full URL :base_url is ignored.
```ruby
api = HTTPClient.new(:base_url => 'https://api.example.com/v1')
api.get("/users.json") # => Get https://api.example.com/v1/users.json
api.get("https://localhost/path") # => https://localhost/path
```
### Changes in 2.5.1
Oct 19, 2014 - version 2.5.1
* Changes
* Allow to specify :query in POST, PUT, DELETE and OPTIONS requests.
Closes #83.
* Allow to specify :body in OPTIONS request. Closes #136.
### Changes in 2.5.0
Oct 17, 2014 - version 2.5.0
**IMPORTANT CHANGES**
This version changes (again) default SSL options to help
BEAST/CRIME/POODLE Attack prevension.
* Disabled SSLv3 in favor of POODLE Attack prevention.
* Enabled 1/n-1 fragment in favor of BEAST Attack prevention.
* No TLS compression in favor of CRIME Attack prevention.
You can restore the previous SSL configuration like this;
```ruby
client = HTTPClient.new
client.ssl_config.ssl_version = :SSLv23
client.ssl_config.options = OpenSSL::SSL::OP_ALL | OpenSSL::SSL::OP_NO_SSLv2
```
* Changes
* Change default SSL options. See above.
* Keep cause error of KeepAliveDisconnected. It allows caller to
investigate the cause of KeepAliveDisconnected.
### Changes in 2.4.0
Jun 8, 2014 - version 2.4.0
**IMPORTANT CHANGES**
This version changes default SSL version to :auto (same as nil) to use SSL/TLS
version negotiation. Former versions use SSLv3 as default that does not connect
via TLS. This change makes underlying OpenSSL library decide which SSL/TLS
version to use but SSLv2 is disabled.
This change makes your secure connection safer but if you see SSL connection
failure with this version try specifying SSL version to use SSLv3 like;
```
client = HTTPClient.new
client.ssl_config.ssl_version = :SSLv3
```
* Bug fixes
* Avoid unnecessary connection retries for OAuth error.
[#203](nahi/httpclient#203)
* Make authentication drivers Thread-safe. Note that HTTPClient instance is
Thread-safe for authentication state update but it shares authentication
state across threads by design. If you don't want to share authentication
state, such as for using different authentication username/password pair
per thread, create HTTPClient instance for each Thread.
[#200](nahi/httpclient#200)
* Avoid chunked String recycle in callback block.
[#193](nahi/httpclient#193)
* Do not send empty 'oauth_token' in signed request for compatibility.
[#188](nahi/httpclient#188)
* Ignore negative Content-Length header from server.
[#175](nahi/httpclient#175)
* Fix incorrect use of absolute URL for HTTPS proxy requests.
[#168](nahi/httpclient#168)
* Handle UTF characters in chunked bodies.
[#167](nahi/httpclient#167)
* A new cookie never be accepted if an HTTPClient has the same expired cookie.
[#154](nahi/httpclient#154)
* Allow spaces in NO_PROXY environment like; "hosta, hostb"
[#141](nahi/httpclient#141)
* Avoid HttpClient::Message::Body#dump causes Encoding::CompatibilityError.
[#140](nahi/httpclient#140)
* Changes
* Change default SSL version to :auto to use version negotiation.
[#186](nahi/httpclient#186),
[#204](nahi/httpclient#204)
* Allow to pass client private key passphrase in SSLConfig.
[#201](nahi/httpclient#201)
* Convert README to markdown syntax
[#198](nahi/httpclient#198)
* Update default CA certificates: change the source from JDK's to Firefox's.
The file is downloaded from
https://raw.githubusercontent.com/bagder/ca-bundle/master/ca-bundle.crt
(Certificate data from Mozilla as of: Tue Apr 22 08:29:31 2014)
[#195](nahi/httpclient#195)
* Callback block can be defined as to get 2 arguments to retrieve the
response object.
[#194](nahi/httpclient#194)
* Remove [] from given address for IPv6 compat.
[#176](nahi/httpclient#176)
* Update API endpoints to those of Twitter REST API v1.1.
[#150](nahi/httpclient#150)
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.
By default httpclient sets SSLContext version to SSLv3 and therefore disables TLS version negotiation. That means every library using httpclient that does not unset SSL version always uses only SSLv3 and cannot connect to a server that only supports TLSv1.0+.
This also leads to the problem that if httpclient was not used directly but by a third-party library it will suffer the same problem without any way to change this outside patching the library's usage of httpclient.
Error when connecting to a TLS-only server using default settings:
If no SSL version is given to SSLContext it will correctly negotiate the version with the server and (in my case) will use TLSv1.2. Additional allowing the client to negotiate the version with the server would allow the client to use the best possible available protocol version. SSLv2 would still be not allowed as it is disabled in the cipher suite list (
!SSLv2).This PR changes the default value to
nil. If SSL version is set tonilit will not be passed to the SSLContext. I've also updated the corresponding documentation.