Skip to content
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

crypto/tls: Failed handshake should not send any data to client #8720

Closed
AudriusButkevicius opened this issue Sep 13, 2014 · 3 comments
Closed

crypto/tls: Failed handshake should not send any data to client #8720

AudriusButkevicius opened this issue Sep 13, 2014 · 3 comments
Milestone

Comments

@AudriusButkevicius
Copy link
Contributor

@AudriusButkevicius AudriusButkevicius commented Sep 13, 2014

What does 'go version' print?

go version go1.3 windows/amd64

What steps reproduce the problem?

Snippet: http://play.golang.org/p/eu_6CStY3k

1. Run the snippet above
2. Browse to http://localhost:8181

What happened?

Depending on platform or browser different things might happen.
If the browser thinks the data looks binary, it usually starts a download.
If the data looks plain-text'ish, it outputs bytes to the browser.

What should have happened instead?

The connection should have been closed without any data sent, since it doesn't get past
the TLS handshake due to protocol mismatch.

Or alternatively, a feature specific to net/http using TLS could downgrade the protocol
to HTTP and deal with it in some way.

Please provide any additional information below.

The culprit is the following line:
https://code.google.com/p/go/source/browse/src/crypto/tls/conn.go#582

If commented out, it produces the expected "No data received" error on Chrome.
@AudriusButkevicius

This comment has been minimized.

Copy link
Contributor Author

@AudriusButkevicius AudriusButkevicius commented Sep 13, 2014

Comment 1:

After some discussion on IRC, some of us believe that until the handshake has been
completed, and it was well established that both parties use TLS, we shouldn't send back
alerts, as we are not guaranteed that the other party knows how to read them.
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Sep 13, 2014

Comment 2:

Labels changed: added repo-main, release-none.

@bradfitz bradfitz removed the new label Dec 18, 2014
@rsc rsc added this to the Unplanned milestone Apr 10, 2015
@rsc rsc removed release-none labels Apr 10, 2015
@FiloSottile

This comment has been minimized.

Copy link
Member

@FiloSottile FiloSottile commented Feb 10, 2018

Fixed by 2a8c81f amongst others.

@golang golang locked and limited conversation to collaborators Feb 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.