code review 851041: crypto/tls: good defaults

crypto/tls: good defaults

[rsc, 2010-04-05 20:31:42]

Hello agl1 (cc: golang-dev@googlegroups.com),

I'd like you to review this change.

[rsc, 2010-04-05 20:35:33]

Hi Adam.  I'm trying to test the certificate checking code
by inserting /dev/null at the beginning of the list of certificate
files in this CL and verifying that a TLS connection to
gmail gets rejected (because it doesn't use one of
the 0 valid root CAs described by /dev/null).

When I run the program below I get "unexpected EOF"
instead of seeing the error propagate through.  I was hoping
for something more like "invalid certificate for gmail.com".

When a certificate is invalid, handshake_client.go does
    h.controlChan <- ConnectionState{Error: e}
processControlMsg doesn't seem to have a case for
ConnectionState, yet I don't see the print from the default case.

Any ideas?

Thanks.
Russ

package main

import (
	"crypto/tls"
	"io"
	"log"
	"os"
)

func main() {
	c, err := tls.Dial("tcp", "", "www.google.com:https")
	if err != nil {
		log.Exit(err)
	}
	
	c.Write([]byte("GET / HTTP/1.0\r\nHost: www.google.com\r\n\r\n"))
	_, err = io.Copy(os.Stdout, c)
	log.Exit(err)
}

[agl1, 2010-04-05 20:44:58]

LGTM

http://codereview.appspot.com/851041/diff/5001/6001
File src/pkg/crypto/tls/common.go (right):

http://codereview.appspot.com/851041/diff/5001/6001#newcode153
src/pkg/crypto/tls/common.go:153: "/usr/share/curl/curl-ca-bundle.crt", // OS X
Also /etc/pki/tls/cert.pem for RedHat based systems.

[agl1, 2010-04-05 20:48:14]

On Mon, Apr 5, 2010 at 4:35 PM, Russ Cox <rsc@golang.org> wrote:
> When a certificate is invalid, handshake_client.go does
>    h.controlChan <- ConnectionState{Error: e}
> processControlMsg doesn't seem to have a case for
> ConnectionState, yet I don't see the print from the default case.

Yes, the error handling basically doesn't work. The upper layers of
the TLS stuff start to get pretty threadbare I'm afraid because I
never finished them.

Now that we have goinstall it might be time to remove crypto/tls from
the main repo. Although it's mostly there, it's still only really demo
quality at the moment. What do you think?


AGL

[rsc, 2010-04-05 20:51:44]

> Now that we have goinstall it might be time to remove crypto/tls from
> the main repo. Although it's mostly there, it's still only really demo
> quality at the moment. What do you think?

I think the question is where it would be if finished.
I think it belongs in the main distribution, so that it
is there by default and can be used by package http
for https requests, etc.

Russ

[agl1, 2010-04-05 20:55:03]

On Mon, Apr 5, 2010 at 4:51 PM, Russ Cox <rsc@golang.org> wrote:
> I think the question is where it would be if finished.
> I think it belongs in the main distribution, so that it
> is there by default and can be used by package http
> for https requests, etc.

Ok. In which case I'll see about finding a day this week to work on it.


AGL

[rsc, 2010-04-05 21:38:05]

*** Submitted as http://code.google.com/p/go/source/detail?r=1adcc7290d6f ***

crypto/tls: good defaults

R=agl1
CC=golang-dev
http://codereview.appspot.com/851041