crypto/tls: does not support renegotiation

rogpeppe commented 11 years ago

We would like to use net/http to talk to Microsoft Azure services, but
their servers force a TLS renegotiation, which crypto/tls
does not currently support.

bradfitz commented 11 years ago

Comment 1:

Owner changed to @agl.

Status changed to Accepted.

agl commented 11 years ago

Comment 2:

Really don't ever want to support this. TLS renegotiation is a huge mess.

rogpeppe commented 11 years ago

Comment 3:

> Really don't ever want to support this. TLS renegotiation is a huge mess.
That's a pity, although I do understand. It seems we're ending up using
http://godoc.org/github.com/andelf/go-curl which is a) a pretty messy package and b) a
cgo dependency we could do without.
Have you got a suggestion for a neater way around the issue, by any chance?

rsc commented 10 years ago

Comment 4:

Labels changed: added go1.3maybe.

rsc commented 10 years ago

Comment 5:

Labels changed: added release-none, removed go1.3maybe.

rsc commented 10 years ago

Comment 6:

Labels changed: added repo-main.

gopherbot commented 10 years ago

Comment 7 by otto.bretz:

Fixed by https://code.google.com/p/go/source/detail?r=f1e918132139?

mikioh commented 10 years ago

Comment 8:

This issue was updated by revision 779ef7bd132ae4971f07baf2df8eec508a45f60.

gopherbot commented 10 years ago

Comment 9 by salmodan:

I am creating a Packer plugin for the Microsoft Azure services but I am getting a "local
error: no renegotiation" error message when I make the client.Do(req)   after setting up
the transport to use my client certificate.  I am using Go version 1.2.1.  Am I
understanding this issue correctly in that 1.2.1 is not be able to to talk to Microsoft
Azure services?  Is my only choice to compile a new dev version of Go?  Are there any
newer binaries available?

agl commented 10 years ago

Comment 10:

salmodan: there is no published version of the code that does the weird renegotiation
dance that Azure needs. I wrote a patch for some folks who needed it, but it didn't make
Go 1.3. I do think, at this point, that supporting Azure is sufficiently compelling for
1.4 however.

Labels changed: added release-go1.4, removed priority-triage, release-none.

lukescott commented 10 years ago

Comment 11:

Is there a reason why this wasn't included in 1.3 when it was reviewed well before the
feature freeze? This occurs with FirstData's payment gateway API. Is there anything
wrong with the patch itself? Is there a better way to address this issue?

agl commented 10 years ago

Comment 12:

luke@webconnex.com: what patch do you think was reviewed before the freeze? I may have
missed one.
In general, sites that do this are mistaken and the solution is a hack which is why I've
resisted doing it. This form of authentication is also broken by triple-handshake
attacks[1] and the fix for that hasn't gone through the IETF yet.
It needs careful thought in order to minimise the amount of disruption to the code while
supporting these needs.
[1] https://secure-resumption.com/

gopherbot commented 10 years ago

Comment 13 by Hope2BelieveIn:

"I wrote a patch for some folks who needed it".
Can you please give a link to your patch?

gopherbot commented 10 years ago

Comment 14 by Hope2BelieveIn:

agl@golang.org: "I wrote a patch for some folks who needed it"
Can you please give a link to this patch?

rsc commented 10 years ago

Comment 15:

Too late for 1.4.

Labels changed: added release-go1.5, removed release-go1.4.

gopherbot commented 9 years ago

Comment 16 by thegroff:

Will it be too late for 1.5 too? #7 patch has been sitting there for a year. Is there a
problem with the patch?

minux commented 9 years ago

Comment 17:

what #7 mentioned is a commit that is in 1.4.
the reason this issue is not closed by that
commit is that we still need client support,
but the secure solution hasn't been made into
official RFC yet.
To summarize, there is no unreviewed pending
patch for this issue, and before the RFC come
out, we can't do anything about the client
side of the issue. (agl explained why he didn't
want to implement a workaround in #12)

rsc commented 9 years ago

https://github.com/MSOpenTech/azure-sdk-for-go includes a forked copy of net/http because they needed to be able to talk to Azure servers. Is it possible to support only the limited behavior of the Azure services without getting into the entire TLS renegotiation mess?

ahmetb commented 9 years ago

@rsc Thanks for bringing this up. We are still looking for a clean solution to handle client-side custom TLS renegotiation in the Azure Go SDK and get rid of the crypto/tls fork we have in our repo.

ottob commented 9 years ago

Too late for 1.5?

I ran into this again today with a new service we are using. It would be great if this could get resolved somehow. We're using go-curl now as rogpeppe suggested, but not having the cgo dependency would be super nice and ease cross compilation of our project.

bradfitz commented 9 years ago

This is still tagged with Milestone Go 1.5, but I don't know @agl's latest thoughts on this. Adam?

ottob commented 9 years ago

Would it be less invasive if it was made an option like InsecureSkipVerify: true? AllowRenegotiation: true?

ahmetb commented 9 years ago

@ottob that surely sounds less invasive and testable. +1 for InsecureSkipVerify. How do others feel? @agl

agl commented 9 years ago

Punting to 1.6 because it obviously didn't make 1.5.

In BoringSSL we're settling on removing all renegotiation support except for server-initiated (when explicitly allowed on the client connection) and disallowing concurrent application data flow with the handshake.

That does seem plausible for Go to do too, although it's quite a lot of complexity for a weird, Microsoft corner case. None the less, it'll likely happen.

athornton commented 9 years ago

+1 for AllowRenegotiation.

Like probably everyone else who wants the feature here, talking to Microsoft stuff (in addition to Azure, you need this for using certificate auth to WinRM) is the use case. And yeah, it's just a weird Microsoft corner case, but it is also a weird Microsoft corner case our customers are demanding. Like it or not, SCVMM and Azure (and, generally, WinRM) are part of the landscape, and I'm sure I'm not the only person whose software has to interoperate with them.

okdave commented 9 years ago

It would be really good to have this in 1.6 to support Azure. @agl do you think you'll be able to tackle it in this cycle? I would be willing to take it on, but I have next to no knowledge in this area.

vincepri commented 9 years ago

+1 for renegotiation support, we are also having trouble with postgres and redshift

ottob commented 8 years ago

I found this today: https://boringssl.googlesource.com/boringssl/+/2ae77d2784b4080afa65575fb101de76fc5cef52%5E!/

Is there any chance this might make it into 1.6?

bradfitz commented 8 years ago

I think it's @agl's choice whether he trusts that code enough to put it in the standard library at this point.

ghost commented 8 years ago

I'm also interested in TLS renegotiation. I think Vegeta's -insecure flag (Ignore invalid server TLS certificates) is dependant upon it.

rsc commented 8 years ago

Too late for Go 1.6, sorry.

Integralist commented 8 years ago

Hello from BBC. We're also very eager to see this make its way into 1.7 as all of our platforms are behind self-signed client cert auth and so renegotiation issues have cropped up a few times for us.

ghost commented 8 years ago

Is there a fork with this fix in as I also work for the BBC and this is holding me back?

ahmetb commented 8 years ago

@aidylewis yes there is an old fork here that has this feature. https://github.com/Azure/azure-sdk-for-go/tree/master/core/tls

ottob commented 8 years ago

Any chance of this happening in 1.7?

hoenirvili commented 8 years ago

It would be very nice to add the renegotiation feature. It would help to use the stdlib in development for https winrm support protocol package insted of some forks.

agl commented 8 years ago

https://go-review.googlesource.com/#/c/22475/ may address this bug, but the primary use case is HTTPS servers which do the Microsoft trick of renegotiating a connection to add client certificates and I don't have any.

Can any interested party here either point me at such a server (with or without a test certificate to authenticate with) or test that patch themselves?

gopherbot commented 8 years ago

CL https://golang.org/cl/22475 mentions this issue.

ahmetb commented 8 years ago

@agl reaching out to you via email to provide an endpoint and certs.

agl commented 8 years ago

@ahmetalpbalkan thank you!

ahmetb commented 8 years ago

@agl thanks for confirming the fix and thanks for the patch.

hoenirvili commented 8 years ago

So this is resolved?

rsc commented 8 years ago

There is a CL out that I expect to be in Go 1.7. This bug will be closed when the CL is committed to Git, but you'll have to wait for Go 1.7 to use it (unless you run from the Git master branch, which can be unstable at times and is not recommended for production code).

ottob commented 8 years ago

The patch works fine for me. Huge thanks to @agl!

hoenirvili commented 8 years ago

Thanks for clarifying @rsc, and thanks @agl again for the patch.

ghost commented 8 years ago

Has anyone tested this with the Go 1.7 RC?

bradfitz commented 8 years ago

@aidylewis, @ottob above said he had tested this.

ahmetb commented 8 years ago

Also verified this with go1.7rc6 in azure-sdk-for-go.

elimisteve commented 7 years ago

Just to be explicit to future readers: this feature has been merged into master, and you can use it by setting the Renegotiation field in your *tls.Config to tls.RenegotiateFreelyAsClient:

config := &tls.Config{
    Renegotiation: tls.RenegotiateFreelyAsClient,
}

Integralist commented 7 years ago

Thanks @elimisteve :-)

golang / go

crypto/tls: does not support renegotiation #5742