Proposed breaking change in crypto/tls for 1.3.

[Adam Langley]

crypto/tls has two functions for creating a client connection: Dial,
which most users are expected to use, and Client, which is the
lower-level API.

Dial does what you expect: it gives you a secure connection to the
host that you specify and the majority of users of crypto/tls appear
to work fine with it.

Client gives more control but needs more care. Specifically, if it
isn't given a server name in the tls.Config then it doesn't check that
the server's certificates match any hostname - because it doesn't have
one to check against. It's assumed that users of the low-level API
call VerifyHostname on the certificate themselves if they didn't
supply a hostname.

A review of the uses of Client both within Google and in a couple of
external libraries has shown that nearly all of them got this wrong.

To a fair extent, this is a documentation problem. Client simply says
that a nil Config uses the defaults and Config didn't document how
important ServerName was when using the that API. This is my screwup
which will be fixed in Go 1.3[1].

However, in light of the results of the review I now believe that this
API is too dangerous and so we propose changing tls.Conn.Handshake so
that it returns an error when neither ServerName nor
InsecureSkipVerify is given in the tls.Config.

This might break some existing code and that's not something that we
do lightly[2]. However, it appears the nearly all the code that will
break has a security problem and so we believe that this might be
justified.

In many cases, users of tls.Client were only using it because they
wanted to use it with net.DialTimeout. I'll be adding tls.DialTimeout
in time for Go 1.3 so that these cases can safely use the high-level
API.

[1] https://code.google.com/p/go/source/detail?r=50f617f47bbc1d0fe1372a596c3f0537d5f6fbf9
[2] http://golang.org/doc/go1compat


Cheers

AGL

[mic...@dynamine.net]

On Wednesday, February 19, 2014 9:08:20 AM UTC-8, agl wrote:

Client gives more control but needs more care. Specifically, if it
isn't given a server name in the tls.Config then it doesn't check that
the server's certificates match any hostname - because it doesn't have
one to check against. It's assumed that users of the low-level API
call VerifyHostname on the certificate themselves if they didn't

supply a hostname. ...

 

However, in light of the results of the review I now believe that this
API is too dangerous and so we propose changing tls.Conn.Handshake so
that it returns an error when neither ServerName nor
InsecureSkipVerify is given in the tls.Config.

As a default, I think this is fine, but I'd also like the option to verify that the presented certificate matches, but ignore a server hostname mismatch.  Better still would be to support a verify callback a la OpenSSL's SSL_CTX_set_cert_verify_callback(3), but I suspect that's a lot more work.

--Michael

[Adam Langley]

On Wed, Feb 19, 2014 at 3:01 PM, <mic...@dynamine.net> wrote:
> As a default, I think this is fine, but I'd also like the option to verify
> that the presented certificate matches, but ignore a server hostname
> mismatch.

I'm afraid that I don't understand you here. There are two steps to
verification: checking that the certificate is valid (i.e. it's signed
by a trusted root) and then checking that it's a certificate for the
intended destination.

The current behaviour of tls.Client without a ServerName is to check
only the first step. In the future, you'll need to give
InsecureSkipVerify in order to disable both checks and then do them
yourself. Thankfully it's not much code:

certs := conn.ConnectionState().PeerCertificates

opts := x509.VerifyOptions{
»·······DNSName: c.config.ServerName,
»·······Intermediates: x509.NewCertPool(),
}

for i, cert := range certs {
»·······if i == 0 {
»·······»·······continue
»·······}
»·······opts.Intermediates.AddCert(cert)
}

chains, err = certs[0].Verify(opts)


Cheers

AGL

[Michael S. Fischer]

On Wed, Feb 19, 2014 at 12:10 PM, Adam Langley <a...@golang.org> wrote:

On Wed, Feb 19, 2014 at 3:01 PM,  <mic...@dynamine.net> wrote:
> As a default, I think this is fine, but I'd also like the option to verify
> that the presented certificate matches, but ignore a server hostname
> mismatch.

I'm afraid that I don't understand you here. There are two steps to
verification: checking that the certificate is valid (i.e. it's signed
by a trusted root) and then checking that it's a certificate for the
intended destination.

Right.  What I'm saying is that sometimes I only want to disable the second check, not both (for testing, for example).

--Michael

[Mikio Hara]

On Thu, Feb 20, 2014 at 2:08 AM, Adam Langley <a...@golang.org> wrote:

> In many cases, users of tls.Client were only using it because they
> wanted to use it with net.DialTimeout. I'll be adding tls.DialTimeout
> in time for Go 1.3 so that these cases can safely use the high-level
> API.

Or selecting a data path by specifying a local address on multihomed node.
Making tls.Dialer{ net.Dialer } and tls.Dialer.Dial method might be better?

[Adam Langley]

On Wed, Feb 19, 2014 at 8:36 PM, Mikio Hara <mikioh...@gmail.com> wrote

> Or selecting a data path by specifying a local address on multihomed node.
> Making tls.Dialer{ net.Dialer } and tls.Dialer.Dial method might be better?

That's a very good point. It should probably take a *Dialer to avoid
repeating the same mistake.


Cheers

AGL

[Rob Pike]

Reminder: The deadline for feature changes in Go 1.3 is a week away.

-rob

[Raffaele Sena]

I have a use case where I want to leave everything as default, but want to change the list of ciphersuites (Config.CipherSuites). With this change will I have to go figure out how to get the correct ServerName ?

-- Raffaele

On Thu, Feb 20, 2014 at 10:41 AM, Rob Pike <r...@golang.org> wrote:

Reminder: The deadline for feature changes in Go 1.3 is a week away.

-rob

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[Adam Langley]

On Thu, Feb 20, 2014 at 3:12 PM, Raffaele Sena <raf...@gmail.com> wrote:
> I have a use case where I want to leave everything as default, but want to
> change the list of ciphersuites (Config.CipherSuites). With this change will
> I have to go figure out how to get the correct ServerName ?

Not with tls.Dial. That will continue to work fine.

If you are using tls.Client then you likely already needed to set
ServerName (unless you were checking yourself). After the proposed
change, it'll be an error not to set it unless InsecureSkipVerify is
set to indicate your intent to check yourself.


Cheers

AGL

[Mikio Hara]

I forgot to say: +1 for fixing tls.Conn.Handshake for making (mostly)
SNI work safely.
For the additional cooked client API, I'm fine either in go1.3 or go1.4.

# Look forward to see some proposals for adding ALPN, Heartbeat
extensions in go1.4. ;)