Comment 1:

-> agl for triage.
If agl thinks it is worth doing please send us the patches using the code review process
described at http://golang.org/doc/contribute.html.
Thanks.

Owner changed to @agl.

Comment 2 by ian.ragsdale:

Will do as soon as I get the nod from agl

Comment 3:

This has previously been a good method for educating people not to use crypto/tls for
serious things. Since the whole crypto stack, from ModExp upwards, is basically
unreviewed, I want to encourage people to put Go behind OpenSSL or something similar
once they get to the point of having client diversity like this.
So I would actually like to not support SSLv2 compat handshakes; not just because it
highlights poor clients (OpenSSL fixed this behaviour quite a while ago), but because of
that more subtle reason.

Labels changed: added priority-later, removed priority-triage.

Comment 4 by ian.ragsdale:

I see your point, but I can't easily put it behind OpenSSL - I'm working on a SMTP
server that should support STARTTLS, and this makes most of the clients that I've tried
with not work out of the box.  I'd like this service to be usable by as many as possible
without complicated SSL configuration.
I'm not aware of any existing SMTP proxy that I can use like Nginx or HAProxy to be a
SMTP front-end, only store & forward type SMTP servers, which I'd rather not use.
I understand that the TLS implementation isn't as well-reviewed as OpenSSL, but what
incentive do people have to do that as long as the answer is always to put it behind
OpenSSL?
Anyway, I understand if the answer is no, it's a limited use case that will eventually
be a non-issue, it just seemed worthwhile to improve interop in the short term.  I can
just keep on using my patched version. :)

Comment 5:

You make a fair point about SMTP: you don't have a lot of options there and
SMTP/STARTTLS clients are a real mess because nobody cares much about them.
I am on the fence about this and could be persuaded if it was causing a bunch of people
pain. But it sounds like it's not a big problem for you, and having less code is nice.
So far this has caused pleasingly little trouble for people, at least judging by the
reports that I've seen, so I'm tempted to keep things as they are. But we can leave this
bug open in case there are others who want this and it turns out that I'm misjudging the
cost/benefit.

Comment 6 by ian.ragsdale:

Makes sense to me.  I just want to add that the error returned from the TLS package is
"local error: unexpected message", and the OpenSSL error happens in
SSL23_GET_SERVER_HELLO, so that if someone else runs into this error they might
hopefully find this page and the patch.

Comment 7 by vadim.spivak:

I just hit the same issue after debugging it for quiet some time..

Comment 8:

vadim: whatever the merits of the bug, having people waste time debugging isn't good.
I've sent out a change for review to return a more specific error in this case.
http://golang.org/cl/6474055

Comment 9:

This issue was updated by revision 0a115d7.

Return a better error message in this situation.
R=golang-dev, r
CC=golang-dev
http://golang.org/cl/6474055

Comment 10 by vspivak@rbcon.com:

I tried implementing an OpenSSL wrapper for GO - https://github.com/vadims/sslconn
However, it's not very pretty due to the non blocking IO needed to multiplex OpenSSL
connections and Go's blocking Readers/Writers. 
Is there any way you could reconsider merging Ian's patch?

Comment 11 by ian.ragsdale:

The reason given for not merging my patch is that they don't consider the existing TLS
code production ready, not because of any known defects, but because it hasn't received
the kind of code review warranted for a cryptographic library.
A more interesting question (and perhaps a bug for this one to depend on) is what level
of review WOULD be sufficient for this code to be considered production ready. 
Identifying that as a need could be the start of a good project for someone, perhaps a
summer of code project, or something that could be sponsored by one of the companies out
there using Go in production.

Comment 12 by vspivak@rbcon.com:

The problem is that developers who use crypto/tls don't know that it's not production
ready, specifically on the client side where compatibility is less important. 
It's only apparent when you run into these corner cases.

Comment 13:

Labels changed: added priority-someday, removed priority-later.

Status changed to LongTerm.

Comment 14 by justinjosephsmith:

Any update here? It would be great to see some sort of list that indicates what has gone
through a security review. After reading this thread I'm not sure how much of "crypto"
has been reviewed for use "in a real app". In lieu of more information, the only viable
option appears to be relying on OpenSSL for all crypto.

Comment 15:

justinjosephsmith: the quality of the crypto code is on a par with the rest of the Go
standard library. The issue with crypto is that there's more scope for subtle mistakes
to be a real problem.
I've received one (non-security) bug from someone competent who reviewed the code, but
it certainly hasn't undergone the kind of review that OpenSSL has. (That makes it the
same as the vast majority of crypto code: GnuTLS, GnuPG etc.)
On the other hand, the security advisories for OpenSSL are fairly numerous
http://openssl.org/news/ and there are more in the pipeline.

Comment 16 by ian.ragsdale:

It seems like we have a bit of a chicken/egg problem.  If we discourage the use of the
crypto code there isn't much incentive to review it. 
Perhaps it makes sense to allow the functionality but have a big warning about using it?
It could require a specific configuration option, like the one for disabling certificate
checks in the client.
tls.Config{InsecureAllowV2Clients: true}

Comment 17 by justinjosephsmith:

This makes sense. I misunderstood an earlier statement. Thanks for the response.

Comment 18 by tomfite:

Thanks for posting the patch, I found this thread after trying to track down SSLv2
errors with a Go server application that I'm working on.
I've attempted to merge the patch into Go v1.1.1 and I'm not having much luck as the
crypto/tls library has changed quite a bit since this patch was written. By chance has
anyone attempted to make this patch work with newer versions of the library?

Comment 19 by erwan.legrand:

Here is a new version of the patch for the Go 1.1 branch.
Before you use this, please read Adam Langley's comments above: the Go TLS stack is not
ready for production. For all we know, it might leak your private key. You have been
warned.

Attachments:

1. Allow-SSLv2-ClientHello-go1.1.patch (5309 bytes)

Comment 20:

Labels changed: added repo-main.

Comment 21:

Adding Release=None to all Priority=Someday bugs.

Labels changed: added release-none.

I have the same problem, also SMTP related - I'd rather avoid manually patching Go source (since it would mean other developers doing the same)

If the concern is discouraging the use of crypto/tls, could the patch be modified so it has a feature flag (disabled by default), forcing developers to read the documentation (or code) which could explain the risks. I think that would be safer than expecting developers to patch the Go source, which may be a necessity for some.

If crypto/tls really isn't "production ready", the documentation should make it much clearer - I only found this bug after trying to connect to the Go server using openssl s_client!

Considering that SSLv3 is broken (https://disablessl3.com/, http://googleonlinesecurity.blogspot.com/2014/10/this-poodle-bites-exploiting-ssl-30.html) nobody should be using SSLv3 these days, much less SSLv2.

I'm tempted to close this bug without action. @agl?

bradfitz: this is about ancient clients that wrap an SSLv3/TLS handshake in SSLv2 form in order to be compatible with SSLv2 servers. IE on XP was a big source of this and we've made that mostly moot by disabling SSLv3 by default on trunk. You can also get the same thing with s_client if you don't update OpenSSL, which is similarly a bad idea.

Since this deprecation is so old (1996) that it can vote now, I think it's ok that it's a little bit of work to support it.

I've tweaked erwan.legrand's Go 1.1 patch to support Go 1.4. I applied it to my std lib, and the dreaded failed requests and tls: unsupported SSLv2 handshake received error messages appear to be gone! (See below.)

Having a 95% solution in Go was painful, and after trying many other options (e.g., re-implementing much of my proxy app in another language with a more mature TLS library, using a not-programmable-enough Python proxy, and almost starting from scratch and using Nginx + Lua), this finally appears to be a fully working solution -- and 100% in Go!

To reiterate previous security warnings:

Before you use this, please read Adam Langley's comments above: the Go TLS stack is not ready for production. For all we know, it might leak your private key. You have been
warned.

To download and use this patch, then recompile the Go standard library on Linux, run something like...

cd ~
wget https://gist.githubusercontent.com/elimisteve/e3fea232c11fa08146d6/raw/e2457232b63a8f0433326874621565475cdb2524/Allow-SSLv2-ClientHello-go1.4.patch
cd go/src  # cd to wherever your Go source is located
patch -p0 < ~/Allow-SSLv2-ClientHello-go1.4.patch
./all.bash  # recompile

Thank you, all!

For some reason I kept accidently using an old Go runtime after recompiling. If you've used the above patch and recompiled, and you want to make sure your go binary is patched, you can run

grep 'tls: unsupported SSLv2 handshake received' `which go`

to see if this SSLv2 error from the standard library, which should NOT be in your patched go, is present nonetheless. And if you run go build myprogram.go, afterward you can run grep 'tls: unsupported SSLv2 handshake received' myprogram to make sure that the Go runtime that has been linked into your myprogram binary is the patched one.

I just got bitten by this one.

The problem (at least for me) is less ancient clients that actually support SSLv2, and more: the Java 6 default behavior, when making an SSL connection, is to send SSLv2 Hello (it doesn't actually support SSLv2, but does send the Hello--see http://www.oracle.com/technetwork/java/javase/documentation/cve-2014-3566-2342133.html ).

Now, sure, Java 6 is also pretty old, but it's in really wide use in the wild, especially in the context of big horrible Enterprisey apps (which is where this got me). And in many cases it's not going to be feasible to modify the Java properties of the connecting applications to only do TLS.

Six months ago I would have had more sympathy for "Go's SSL implementation is not production-ready." These days I'm pretty sure OpenSSL's SSL implementation isn't production-ready either.

I rebased on 1.5.2/fixed/polished/optimized that patch a bit and successfully ran it at pretty high load. It's here: crypto/tls: support SSLv2 compatibility handshakes. I wouldn't use it if I were you, but if you're desperate, it might not blow up.

I'm tempted to close this bug without action. @agl?

That sounds like a good idea.
https://drownattack.com/

Hah, indeed. I've wanted to close this bug for a long time, but that's a great excuse.

I'm not against closing this, but DROWN is not relevant.

This ticket is about supporting an unfortunate compatibility trick, not the entire protocol, so it would not introduce the vulnerability.

My thought process was that if people finally kill off SSLv2, this compatibility feature is no longer relevant.

If more anecdata helps, I ran into this within the last couple of months with a third-party service doing HTTPS requests to our servers from Java 6 code (as mentioned elsewhere in this thread, the Java 6 SSL stack will do an SSLv2 Client Hello by default, although it is possible to work around explicitly).

I, too, ran into this SSLv2 issue from a Java client, which was trying to connecting to an HTTPS proxy I wrote in Go.

Yeah. As far as I know, the primary place this shows up is The Very Important Service That Only Runs On Java 6 And No One Understands It Enough To Rewrite It And There Is No Way We Are Going To Crack It Open To Change The SSL Settings Are You Kidding? Which, on my more cynical days, I think describes about 90% of Enterprise Software.

Okay, I can reopen this. I was being too hopeful.

@FiloSottile, the tree is open. Can you formally send https://gist.github.com/FiloSottile/8e0fe1a3523126e8c581? It doesn't look too invasive. We can guard it behind a flag (make it opt-in) for more paranoia.

Done https://go-review.googlesource.com/20094

Considering that ian.ragsdale, erwan.legrand and @elimisteve contributed to earlier versions of the patch, should we chase their CLAs?

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

I looked at the sequence of patches. The one in the Gist has only trivial changes from the original by Ian Ragsdale, so I think the only CLA we absolutely need is his.

@iragsdale, could you complete a contributor CLA (https://golang.org/doc/contribute#copyright) so that we can put the SSLv2 compatibility code into Go's standard crypto/tls?

If @ErwanLegrand and @elimisteve want to complete CLAs as well I'm happy to credit them too, but the followup changes appear to have been quite minor.

Thanks everyone.

@agl replied on https://golang.org/cl/20094 :

I don't believe that we should land this. It goes against the general trend of dropping support for old SSL versions which keep causing problems.

I am glad that we have established that Java 6 is the holdout. I looked around on the internet and found what appears to be Oracle's official advice on SSL settings as of January 2015.

Under the text "For the following use cases Oracle recommends", it says that client-side uses should set the protocols to exclude the SSLv2 hello, for example:

sslSocket.setEnabledProtocols(new String[] {"TLSv1"});
sslEngine.setEnabledProtocols(new String[] {"TLSv1"});

So the Java clients that are still sending SSLv2Hello are not following Oracle's advice.

Based on @agl's Go server-side advice and Oracle's Java client-side advice, I think it is reasonable for Go not to support SSLv2 compatibility handshakes for Java clients.

@iragsdale, @ErwanLegrand, @elimisteve, sorry for the false alarm; no CLA needed after all.

Thanks everyone.