Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(3)

Issue 21696002: Implement the AES GCM cipher suites for TLS. (Closed)

Created:
6 years, 11 months ago by wtc
Modified:
6 years, 11 months ago
Reviewers:
agl, Ryan Sleevi
CC:
chromium-reviews, cbentzel+watch_chromium.org
Visibility:
Public.

Description

Implement the AES GCM cipher suites for TLS. The AES GCM cipher suites are disabled in DTLS. This will be fixed soon. Disable the HMAC-SHA256 cipher suites so that our ClientHello doesn't become too big. Patch by Adam Langley. R=agl@chromium.org,rsleevi@chromium.org BUG=255241 TEST=none Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=217716

Patch Set 1 #

Patch Set 2 : Look up PK11_Encrypt and PK11_Decrypt at run time on Linux. Minor cleanup. #

Patch Set 3 : Also update the ecdhe_ecdsa_suites and ecdhe_rsa_suites arrays #

Total comments: 6

Patch Set 4 : Address AGL comments #

Total comments: 3

Patch Set 5 : First complete review by wtc #

Patch Set 6 : Fix a grammatical error in a comment #

Total comments: 16

Patch Set 7 : Add PKCS11 bypass mode support, add workaround for lack of DTLS support, add patch files #

Patch Set 8 : Resolve merge conflicts in README.chromium and applypatches.sh. Fix &seq_num in ssl3_AESGCMBypass. #

Patch Set 9 : Add a ssl3_DisableGCMSuites call to ssl3_HandleClientHello #

Patch Set 10 : Add a TODO to README.chromium to remove cbc.patch #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1925 lines, -229 lines) Patch
M net/socket/nss_ssl_util.cc View 1 2 3 4 5 6 1 chunk +3 lines, -2 lines 0 comments Download
M net/third_party/nss/README.chromium View 1 2 3 4 5 6 7 8 9 2 chunks +10 lines, -0 lines 0 comments Download
A net/third_party/nss/patches/aesgcm.patch View 1 2 3 4 5 6 7 1 chunk +1179 lines, -0 lines 0 comments Download
A net/third_party/nss/patches/aesgcmchromium.patch View 1 2 3 4 5 6 7 8 1 chunk +117 lines, -0 lines 0 comments Download
M net/third_party/nss/patches/applypatches.sh View 1 2 3 4 5 6 7 1 chunk +4 lines, -0 lines 0 comments Download
M net/third_party/nss/ssl/dtlscon.c View 1 2 3 4 5 6 1 chunk +7 lines, -0 lines 0 comments Download
M net/third_party/nss/ssl/ssl3con.c View 1 2 3 4 5 6 7 8 24 chunks +535 lines, -204 lines 0 comments Download
M net/third_party/nss/ssl/ssl3ecc.c View 1 2 3 chunks +4 lines, -0 lines 0 comments Download
M net/third_party/nss/ssl/sslenum.c View 1 2 3 4 5 6 1 chunk +8 lines, -0 lines 0 comments Download
M net/third_party/nss/ssl/sslimpl.h View 1 2 3 4 8 chunks +33 lines, -20 lines 0 comments Download
M net/third_party/nss/ssl/sslinfo.c View 1 2 3 4 5 chunks +9 lines, -1 line 0 comments Download
M net/third_party/nss/ssl/sslproto.h View 1 2 3 4 2 chunks +9 lines, -0 lines 0 comments Download
M net/third_party/nss/ssl/sslsock.c View 1 2 3 4 3 chunks +4 lines, -0 lines 0 comments Download
M net/third_party/nss/ssl/sslt.h View 1 2 3 4 1 chunk +3 lines, -2 lines 0 comments Download

Messages

Total messages: 16 (0 generated)
wtc
rsleevi: you can ignore this CL for now. agl: please take a quick look at ...
6 years, 11 months ago (2013-08-02 02:31:16 UTC) #1
agl
https://codereview.chromium.org/21696002/diff/12005/net/third_party/nss/ssl/ssl3con.c File net/third_party/nss/ssl/ssl3con.c (right): https://codereview.chromium.org/21696002/diff/12005/net/third_party/nss/ssl/ssl3con.c#newcode1801 net/third_party/nss/ssl/ssl3con.c:1801: ssl3_CheckGCMSupport(void) This function is called CheckGCMSupport, but it doesn't ...
6 years, 11 months ago (2013-08-02 14:59:50 UTC) #2
wtc
agl: thank you for your comments. Please take a look at patch set 4. https://codereview.chromium.org/21696002/diff/12005/net/third_party/nss/ssl/ssl3con.c ...
6 years, 11 months ago (2013-08-03 01:02:14 UTC) #3
agl
lgtm
6 years, 11 months ago (2013-08-05 16:53:59 UTC) #4
Ryan Sleevi
wtc: Is this ready for review? I took a quick scan, and it looks fine, ...
6 years, 11 months ago (2013-08-05 22:21:42 UTC) #5
wtc
rsleevi: thanks for the comments. No, this is not yet ready for a deep dive. ...
6 years, 11 months ago (2013-08-05 22:39:18 UTC) #6
wtc
https://codereview.chromium.org/21696002/diff/41001/net/third_party/nss/ssl/ssl3con.c File net/third_party/nss/ssl/ssl3con.c (right): https://codereview.chromium.org/21696002/diff/41001/net/third_party/nss/ssl/ssl3con.c#newcode1803 net/third_party/nss/ssl/ssl3con.c:1803: #ifdef LINUX On 2013/08/05 22:21:42, Ryan Sleevi wrote: > ...
6 years, 11 months ago (2013-08-05 23:07:28 UTC) #7
wtc
This CL is ready for a careful review. Patch set 1 is agl's original NSS ...
6 years, 11 months ago (2013-08-10 01:22:59 UTC) #8
agl
LGTM with nits. https://codereview.chromium.org/21696002/diff/65001/net/third_party/nss/ssl/ssl3con.c File net/third_party/nss/ssl/ssl3con.c (right): https://codereview.chromium.org/21696002/diff/65001/net/third_party/nss/ssl/ssl3con.c#newcode1896 net/third_party/nss/ssl/ssl3con.c:1896: memcpy(nonce + 4, &seq_num, explicitNonceLen); This ...
6 years, 11 months ago (2013-08-13 12:17:12 UTC) #9
Ryan Sleevi
Forgot to publish draft reviews, but I see Adam made the same remark, so publishing ...
6 years, 11 months ago (2013-08-13 21:05:45 UTC) #10
Ryan Sleevi
LGTM. Just adding a note as a reminder. https://codereview.chromium.org/21696002/diff/65001/net/third_party/nss/ssl/ssl3con.c File net/third_party/nss/ssl/ssl3con.c (right): https://codereview.chromium.org/21696002/diff/65001/net/third_party/nss/ssl/ssl3con.c#newcode2357 net/third_party/nss/ssl/ssl3con.c:2357: #define ...
6 years, 11 months ago (2013-08-13 21:34:46 UTC) #11
wtc
agl,rsleevi: thank you for the review. I responded to all of your comments. Please review ...
6 years, 11 months ago (2013-08-14 01:57:02 UTC) #12
wtc
rsleevi: I am going to commit patch set 10 now. It would be nice if ...
6 years, 11 months ago (2013-08-14 22:05:42 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/wtc@chromium.org/21696002/164001
6 years, 11 months ago (2013-08-14 22:06:56 UTC) #14
Ryan Sleevi
lgtm
6 years, 11 months ago (2013-08-14 22:26:48 UTC) #15
commit-bot: I haz the power
6 years, 11 months ago (2013-08-15 00:51:35 UTC) #16
Message was sent while issue was closed.
Change committed as 217716

Powered by Google App Engine
This is Rietveld 408576698