Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(3228)

Issue 10762044: code review 10762044: crypto/tls: implement TLS 1.2. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 10 months ago by agl1
Modified:
10 years, 9 months ago
Reviewers:
rsc
CC:
golang-dev, rsc
Visibility:
Public.

Description

crypto/tls: implement TLS 1.2. This does not include AES-GCM yet. Also, it assumes that the handshake and certificate signature hash are always SHA-256, which is true of the ciphersuites that we currently support.

Patch Set 1 #

Patch Set 2 : diff -r b43eb215afd3 https://code.google.com/p/go/ #

Patch Set 3 : diff -r b43eb215afd3 https://code.google.com/p/go/ #

Patch Set 4 : diff -r b43eb215afd3 https://code.google.com/p/go/ #

Total comments: 6

Patch Set 5 : diff -r e334347476c4 https://code.google.com/p/go/ #

Patch Set 6 : diff -r e334347476c4 https://code.google.com/p/go/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1141 lines, -93 lines) Patch
M src/pkg/crypto/tls/cipher_suites.go View 1 2 chunks +6 lines, -4 lines 0 comments Download
M src/pkg/crypto/tls/common.go View 1 4 chunks +34 lines, -7 lines 0 comments Download
M src/pkg/crypto/tls/conn.go View 1 2 2 chunks +6 lines, -2 lines 0 comments Download
M src/pkg/crypto/tls/handshake_client.go View 1 7 chunks +11 lines, -10 lines 0 comments Download
M src/pkg/crypto/tls/handshake_client_test.go View 1 2 chunks +271 lines, -0 lines 0 comments Download
M src/pkg/crypto/tls/handshake_messages.go View 1 2 3 4 16 chunks +137 lines, -11 lines 0 comments Download
M src/pkg/crypto/tls/handshake_messages_test.go View 1 1 chunk +3 lines, -0 lines 0 comments Download
M src/pkg/crypto/tls/handshake_server.go View 1 4 chunks +7 lines, -6 lines 0 comments Download
M src/pkg/crypto/tls/handshake_server_test.go View 1 2 3 4 4 chunks +537 lines, -1 line 0 comments Download
M src/pkg/crypto/tls/key_agreement.go View 1 5 chunks +42 lines, -6 lines 0 comments Download
M src/pkg/crypto/tls/prf.go View 1 2 3 4 7 chunks +87 lines, -46 lines 0 comments Download

Messages

Total messages: 3
agl1
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go/
10 years, 10 months ago (2013-06-28 20:14:58 UTC) #1
rsc
LGTM https://codereview.appspot.com/10762044/diff/6002/src/pkg/crypto/tls/handshake_messages.go File src/pkg/crypto/tls/handshake_messages.go (right): https://codereview.appspot.com/10762044/diff/6002/src/pkg/crypto/tls/handshake_messages.go#newcode997 src/pkg/crypto/tls/handshake_messages.go:997: y[0] = uint8(len(m.signatureAndHashes) >> 7) This is kind ...
10 years, 9 months ago (2013-07-02 01:47:52 UTC) #2
agl1
10 years, 9 months ago (2013-07-03 00:00:49 UTC) #3
*** Submitted as https://code.google.com/p/go/source/detail?r=44b8d2e5ebc6 ***

crypto/tls: implement TLS 1.2.

This does not include AES-GCM yet. Also, it assumes that the handshake and
certificate signature hash are always SHA-256, which is true of the ciphersuites
that we currently support.

R=golang-dev, rsc
CC=golang-dev
https://codereview.appspot.com/10762044

https://codereview.appspot.com/10762044/diff/6002/src/pkg/crypto/tls/handshak...
File src/pkg/crypto/tls/handshake_messages.go (right):

https://codereview.appspot.com/10762044/diff/6002/src/pkg/crypto/tls/handshak...
src/pkg/crypto/tls/handshake_messages.go:997: y[0] =
uint8(len(m.signatureAndHashes) >> 7)
On 2013/07/02 01:47:53, rsc wrote:
> This is kind of opaque.
> Appears to be
> 
> n := 2*len(m.signatureAndHashes)
> y[0] = uint(n>>8)
> y[1] = uint(n)
> 

Done.

https://codereview.appspot.com/10762044/diff/6002/src/pkg/crypto/tls/prf.go
File src/pkg/crypto/tls/prf.go (right):

https://codereview.appspot.com/10762044/diff/6002/src/pkg/crypto/tls/prf.go#n...
src/pkg/crypto/tls/prf.go:67: // pRF12 implements the TLS 1.2 pseudo-random
function, as defined in RFC 5246, section 5.
On 2013/07/02 01:47:53, rsc wrote:
> fwiw you are inconsistent about prf vs pRF. prf seems better

Done.

https://codereview.appspot.com/10762044/diff/6002/src/pkg/crypto/tls/prf.go#n...
src/pkg/crypto/tls/prf.go:170: if version == VersionTLS12 {
On 2013/07/02 01:47:53, rsc wrote:
> Do you want == or >=?
> Same question a few times below too.

Good point. >= is probably better on the assumption that TLS 1.3 probably looks
more like 1.2 than anything else.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b