Hello bradfitz@golang.org (cc: golang-codereviews@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go
Brad: please patch this in and see whether it works for you. I'll be at the IETF next week anyway.
https://codereview.appspot.com/108710046/diff/40001/src/pkg/crypto/tls/handsh... File src/pkg/crypto/tls/handshake_client_test.go (right): https://codereview.appspot.com/108710046/diff/40001/src/pkg/crypto/tls/handsh... src/pkg/crypto/tls/handshake_client_test.go:53: // ConnectionState of the resulting connection. It returns false if the returns an error, not false https://codereview.appspot.com/108710046/diff/40001/src/pkg/crypto/tls/handsh... File src/pkg/crypto/tls/handshake_messages.go (right): https://codereview.appspot.com/108710046/diff/40001/src/pkg/crypto/tls/handsh... src/pkg/crypto/tls/handshake_messages.go:263: copy(z[1:], []byte(s)) Use s... instead of []byte alloc?
https://codereview.appspot.com/108710046/diff/40001/src/pkg/crypto/tls/handsh... File src/pkg/crypto/tls/handshake_client_test.go (right): https://codereview.appspot.com/108710046/diff/40001/src/pkg/crypto/tls/handsh... src/pkg/crypto/tls/handshake_client_test.go:53: // ConnectionState of the resulting connection. It returns false if the On 2014/07/20 18:56:48, bradfitz wrote: > returns an error, not false Done. https://codereview.appspot.com/108710046/diff/40001/src/pkg/crypto/tls/handsh... File src/pkg/crypto/tls/handshake_messages.go (right): https://codereview.appspot.com/108710046/diff/40001/src/pkg/crypto/tls/handsh... src/pkg/crypto/tls/handshake_messages.go:263: copy(z[1:], []byte(s)) On 2014/07/20 18:56:48, bradfitz wrote: > Use s... instead of []byte alloc? Done.
rfc7301, yay! pls add "Fixes issue 6736." line to the cl descr. https://codereview.appspot.com/108710046/diff/60001/src/pkg/crypto/tls/handsh... File src/pkg/crypto/tls/handshake_client.go (right): https://codereview.appspot.com/108710046/diff/60001/src/pkg/crypto/tls/handsh... src/pkg/crypto/tls/handshake_client.go:44: } also need to check the entire protocol_name_list length (2<<16-1)? https://codereview.appspot.com/108710046/diff/60001/src/pkg/crypto/tls/handsh... File src/pkg/crypto/tls/handshake_server.go (right): https://codereview.appspot.com/108710046/diff/60001/src/pkg/crypto/tls/handsh... src/pkg/crypto/tls/handshake_server.go:168: if selectedProto, fallback := mutualProtocol(hs.clientHello.alpnProtocols, c.config.NextProtos); !fallback { i think we should display a no_application_protocol(120) alert to the client here.
https://codereview.appspot.com/108710046/diff/60001/src/pkg/crypto/tls/handsh... File src/pkg/crypto/tls/handshake_client.go (right): https://codereview.appspot.com/108710046/diff/60001/src/pkg/crypto/tls/handsh... src/pkg/crypto/tls/handshake_client.go:44: } On 2014/07/30 06:52:46, mikio wrote: > also need to check the entire protocol_name_list length (2<<16-1)? Done. https://codereview.appspot.com/108710046/diff/60001/src/pkg/crypto/tls/handsh... File src/pkg/crypto/tls/handshake_server.go (right): https://codereview.appspot.com/108710046/diff/60001/src/pkg/crypto/tls/handsh... src/pkg/crypto/tls/handshake_server.go:168: if selectedProto, fallback := mutualProtocol(hs.clientHello.alpnProtocols, c.config.NextProtos); !fallback { On 2014/07/30 06:52:46, mikio wrote: > i think we should display a no_application_protocol(120) alert to the client > here. This isn't, in practice, what other TLS stacks do, even though the RFC suggests it.
LGTM
*** Submitted as https://code.google.com/p/go/source/detail?r=71dc1b4815f2 *** crypto/tls: add ALPN support. Fixes issue 6736. LGTM=mikioh.mikioh R=bradfitz, mikioh.mikioh CC=golang-codereviews https://codereview.appspot.com/108710046
