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.

Sign up for GitHub

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

Jump to bottom

net/http: newConn() copies the tls.ConnectionState before TLS handshake is completed #2081

Closed
gopherbot opened this issue Jul 18, 2011 · 6 comments
Closed

net/http: newConn() copies the tls.ConnectionState before TLS handshake is completed #2081

gopherbot opened this issue Jul 18, 2011 · 6 comments
Labels
FrozenDueToAge

Comments

@gopherbot
Copy link

by crest@tzi.de:

I tried to use X.509 client certificates to control access to a HTTP server. I
discovered that req.TLS in the request passed to a HandlerFunc called by a http.Server
is invalid.

What steps will reproduce the problem?
1. Fetch the project from github.com (https://github.com/Crest/gresec)
2. Create a CA certificate in cacert.pem and a keypair in key.pem,cert.pem.
3. cat key.pem cert.pem > both.pem for usage by curl
4. (gomake && ./gresec) &
5. curl http://127.0.0.1:8080

What is the expected output?
With the patch applied it will print a valid tls.ConnectionState.

What do you see instead?
With release-branch.r58 it gresec will print the invalid tls.ConnectionState cached by
http.newConn().

Which compiler are you using (5g, 6g, 8g, gccgo)?
6g

Which operating system are you using?
FreeBSD 8.2

Which revision are you using?  (hg identify)
1b38d90eebcd+ (release-branch.r58) release/release.r58

Please provide any additional information below.
The attached patch is just a first try by a go noob. It increases the overhead per HTTP
request by at least one new() and copy.
@adg
Copy link
Contributor

adg commented Jul 18, 2011

Comment 1:

"With the patch applied" what patch?

@gopherbot
Copy link
Author

Comment 2 by crest@tzi.de:

:-!

Attachments:

  1. connection_state.patch (535 bytes)

@adg
Copy link
Contributor

adg commented Jul 19, 2011

Comment 3:

Owner changed to @bradfitz.

@robpike
Copy link
Contributor

robpike commented Jul 20, 2011

Comment 4:

Status changed to Accepted.

@bradfitz
Copy link
Contributor

Comment 5:

Assigning to Adam. I'm not sure I understand why this is necessary. The TLS peer state
changes throughout the connection?

Owner changed to @agl.

@agl
Copy link
Contributor

agl commented Aug 1, 2011

Comment 6:

Works for me.
See the test program that I used, attached. I had to delete the cipher suites
restriction because my curl can't do ECDHE.
$ curl -E both.pem -k https://127.0.0.1:10443/
results in this being printed:
&tls.ConnectionState{HandshakeComplete:true, CipherSuite:0x2f, NegotiatedProtocol:"",
NegotiatedProtocolIsMutual:true,
PeerCertificates:[]*x509.Certificate{(*x509.Certificate)(0xf84010e900)},
VerifiedChains:[][]*x509.Certificate{}}
The peer certificate is in there. However, note that client-side auth doesn't current
verify against RootCAs. Any certificate chain will be included in PeerCertificates.

Status changed to WorkingAsIntended.

Attachments:

  1. serve.go (1331 bytes)

@mikioh mikioh changed the title http.newConn() copies the tls.ConnectionState before TLS handshake is completed net/http: newConn() copies the tls.ConnectionState before TLS handshake is completed Jan 14, 2015
@golang golang locked and limited conversation to collaborators Jun 24, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@bradfitz @agl @robpike @adg @gopherbot