code review 6188061: tls: added AES256 ciphers

tls: added AES256 ciphers

Removed SHA256 ciphers per reviewer feedback....

[b, 2012-05-10 20:35:33]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go/

[rsc, 2012-05-10 23:45:28]

+agl

[agl1, 2012-05-11 00:34:32]

On Thu, May 10, 2012 at 4:35 PM,  <b@b3k.us> wrote:
> Description:
> tls: added AES256 and SHA256 ciphers

SHA256 ciphers are TLS 1.2 only as they alter the PRF. However, Go
only supports TLS 1.0 so, unless I'm missing something critical, I
don't see how this patch can possible work I'm afraid.


Cheers

AGL

[b, 2012-05-11 00:40:50]

Agreed, and the first version I was going to submit did not include the SHA256
ciphers.  I tested against OpenSSL 1.0.1 with s_server -tls1 and it worked. 
Happy to remove those and resubmit.


b

On May 10, 2012, at 5:34 PM, Adam Langley <agl@golang.org> wrote:

> On Thu, May 10, 2012 at 4:35 PM,  <b@b3k.us> wrote:
>> Description:
>> tls: added AES256 and SHA256 ciphers
> 
> SHA256 ciphers are TLS 1.2 only as they alter the PRF. However, Go
> only supports TLS 1.0 so, unless I'm missing something critical, I
> don't see how this patch can possible work I'm afraid.
> 
> 
> Cheers
> 
> AGL

[agl1, 2012-05-11 00:49:41]

On Thu, May 10, 2012 at 8:40 PM, Benjamin Black <b@b3k.us> wrote:
> Agreed, and the first version I was going to submit did not include the SHA256
ciphers.  I tested against OpenSSL 1.0.1 with s_server -tls1 and it worked.
 Happy to remove those and resubmit.

There was a bug in OpenSSL 1.0.1 that I fixed in 1.0.1b where it would
negotiate TLS 1.2 ciphers with TLS 1.0. I suspect that's how it
managed to work.

If you just include the AES256 flavours then it'll be a nice, and very
small, patch.


Cheers

AGL

[b, 2012-05-11 01:18:44]

Makes sense.  Updated and sent.

b

On May 10, 2012, at 5:49 PM, Adam Langley <agl@golang.org> wrote:

> On Thu, May 10, 2012 at 8:40 PM, Benjamin Black <b@b3k.us> wrote:
>> Agreed, and the first version I was going to submit did not include the
SHA256 ciphers.  I tested against OpenSSL 1.0.1 with s_server -tls1 and it
worked.  Happy to remove those and resubmit.
> 
> There was a bug in OpenSSL 1.0.1 that I fixed in 1.0.1b where it would
> negotiate TLS 1.2 ciphers with TLS 1.0. I suspect that's how it
> managed to work.
> 
> If you just include the AES256 flavours then it'll be a nice, and very
> small, patch.
> 
> 
> Cheers
> 
> AGL

[agl1, 2012-05-11 14:28:59]

I'm afraid that I don't see the updated patch.

[b, 2012-05-12 19:45:16]

hg change 6188061 does not indicate an error or success and hg submit
errors out because I am not yet listed in CONTRIBUTORS.

On Fri, May 11, 2012 at 7:28 AM,  <agl@golang.org> wrote:
> I'm afraid that I don't see the updated patch.
>
> http://codereview.appspot.com/6188061/

[b, 2012-05-17 23:48:30]

Hello golang-dev@googlegroups.com, rsc@golang.org, agl@golang.org (cc:
golang-dev@googlegroups.com),

Please take another look.

[b, 2012-05-17 23:50:00]

hg mail is what I should've been using, of course.  Minimal change
updated per feedback.

On Sat, May 12, 2012 at 12:44 PM, Benjamin Black <b@b3k.us> wrote:
> hg change 6188061 does not indicate an error or success and hg submit
> errors out because I am not yet listed in CONTRIBUTORS.
>
> On Fri, May 11, 2012 at 7:28 AM,  <agl@golang.org> wrote:
>> I'm afraid that I don't see the updated patch.
>>
>> http://codereview.appspot.com/6188061/

[agl1, 2012-05-18 01:13:25]

LGTM. Will land tomorrow.

[agl1, 2012-05-18 15:07:14]

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

tls: add AES256 ciphers

R=golang-dev, rsc, agl
CC=golang-dev
http://codereview.appspot.com/6188061

Committer: Adam Langley <agl@golang.org>