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

Fix https:// notifications if TLSvX is mandatory #223

Merged
merged 1 commit into from Jun 21, 2014

Conversation

Projects
None yet
2 participants
@dgeo

dgeo commented Jun 19, 2014

Nowadays servers tend to use TLSv1.[12] and SSLv3 will start to disappear
as only ie6 requires it…
In the post-receive hook SSLv3 is forced and notification fails
if server only accepts TLSv1.[12](eg: server gets 'A+' on https://www.ssllabs.com/ssltest/)

remote: Notifying Redmine about changes to this repository : 'project/repo' ...
remote: Redmine URL : 'https://myforge/githooks/post-receive/redmine/project'
remote: HTTP_ERROR : SSL_connect returned=1 errno=0 state=SSLv3 read server hello A: sslv3 alert handshake failure
remote: Error contacting Redmine about changes to this repo.

This change let ruby's ssl stack negociate protocol with the server (works for me™ with a TLS-only server)

Fix https:// notifications if TLSvX is mandatory
Nowadays servers tend to use TLSv1.[12] and SSLv3 will start to disappear
as only ie6 requires it…
In the post-receive hook SSLv3 is forced and notification fails
if server only accepts TLSv1.[12]
(eg: server gets 'A+' on https://www.ssllabs.com/ssltest/)

```
remote: Notifying Redmine about changes to this repository : 'project/repo' ...
remote: Redmine URL : 'https://myforge/githooks/post-receive/redmine/project'
remote: HTTP_ERROR : SSL_connect returned=1 errno=0 state=SSLv3 read server hello A: sslv3 alert handshake failure
remote: Error contacting Redmine about changes to this repo.
```
@@ -83,7 +83,6 @@ def run_http_query(git_config)
if url.scheme == 'https'
http.use_ssl = true
http.ssl_version = :SSLv3 if http.respond_to?(:ssl_version)

This comment has been minimized.

@dgeo

dgeo Jun 19, 2014

default seems to negociate SSL/TLS protocol

@dgeo

dgeo Jun 19, 2014

default seems to negociate SSL/TLS protocol

@n-rodriguez n-rodriguez self-assigned this Jun 20, 2014

@n-rodriguez

This comment has been minimized.

Show comment
Hide comment
@n-rodriguez

n-rodriguez Jun 20, 2014

Member

Hi! Thanks for your feedback!

I don't know if I can merge this patch now as it conflicts with #53.

Member

n-rodriguez commented Jun 20, 2014

Hi! Thanks for your feedback!

I don't know if I can merge this patch now as it conflicts with #53.

@dgeo

This comment has been minimized.

Show comment
Hide comment
@dgeo

dgeo Jun 20, 2014

I dont think sslv2 is still default...
And locking to sslv3 is not future-proof.
My server doesn't accept v2 nor v3, only tlsv1.x, ant it works without forcing anything...
Should be safe for most (in my opinion).
Openssl's way to deal with that is the opposite: you can just forbid some versions of the protocol.
My 2 cts

dgeo commented Jun 20, 2014

I dont think sslv2 is still default...
And locking to sslv3 is not future-proof.
My server doesn't accept v2 nor v3, only tlsv1.x, ant it works without forcing anything...
Should be safe for most (in my opinion).
Openssl's way to deal with that is the opposite: you can just forbid some versions of the protocol.
My 2 cts

@n-rodriguez n-rodriguez added the bugfix label Jun 21, 2014

n-rodriguez added a commit that referenced this pull request Jun 21, 2014

Merge pull request #223 from dgeo/devel
Fix https:// notifications if TLSvX is mandatory

@n-rodriguez n-rodriguez merged commit 2558df5 into jbox-web:devel Jun 21, 2014

1 check failed

continuous-integration/travis-ci The Travis CI build failed
Details
@dgeo

This comment has been minimized.

Show comment
Hide comment
@dgeo

dgeo Jun 24, 2014

thank you :)

dgeo commented Jun 24, 2014

thank you :)

n-rodriguez added a commit that referenced this pull request Sep 10, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment