Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(210)

Issue 240633006: Move signature_algorithm extension to the end in NSS. (Closed)

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

Description

Move signature_algorithm extension to the end in NSS. WebSphere Application Server 7.0 appears to be intolerant of an empty extension at the end. To that end, also ensure we never send an empty padding extension. BUG=363583 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=267396

Patch Set 1 #

Patch Set 2 : Update nss/patches/ #

Total comments: 2

Patch Set 3 : Match other patches #

Total comments: 3

Patch Set 4 : Include name of server in comment #

Total comments: 8

Patch Set 5 : README.chromium #

Patch Set 6 : Trailing comma #

Patch Set 7 : Rebase and use signature_algorithms instead. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+51 lines, -5 lines) Patch
M net/third_party/nss/README.chromium View 1 2 3 4 5 6 1 chunk +5 lines, -0 lines 0 comments Download
M net/third_party/nss/patches/applypatches.sh View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
A net/third_party/nss/patches/reorderextensions.patch View 1 2 3 4 5 6 1 chunk +34 lines, -0 lines 0 comments Download
M net/third_party/nss/ssl/ssl3ext.c View 1 2 3 4 5 6 2 chunks +10 lines, -5 lines 0 comments Download

Messages

Total messages: 18 (0 generated)
davidben
6 years, 8 months ago (2014-04-18 15:53:25 UTC) #1
Ryan Sleevi
You need to update the patches/ dir (and README.chromium). Please also consider filing a bug ...
6 years, 8 months ago (2014-04-18 18:45:41 UTC) #2
davidben
On 2014/04/18 18:45:41, Ryan Sleevi wrote: > You need to update the patches/ dir (and ...
6 years, 8 months ago (2014-04-21 16:38:49 UTC) #3
Ryan Sleevi
On 2014/04/21 16:38:49, David Benjamin wrote: > On 2014/04/18 18:45:41, Ryan Sleevi wrote: > > ...
6 years, 8 months ago (2014-04-21 18:37:20 UTC) #4
Ryan Sleevi
LGTM, mod patch nit. https://codereview.chromium.org/240633006/diff/20001/net/third_party/nss/patches/applypatches.sh File net/third_party/nss/patches/applypatches.sh (right): https://codereview.chromium.org/240633006/diff/20001/net/third_party/nss/patches/applypatches.sh#newcode52 net/third_party/nss/patches/applypatches.sh:52: patch -p5 < $patches_dir/reorderextensions.patch Can ...
6 years, 8 months ago (2014-04-21 18:38:24 UTC) #5
davidben
Poked at NSS upstream. Their order is server_name renegotiation_info elliptic_curve ec_point_formats session_ticket next_proto_nego app_layer_protocol use_srtp ...
6 years, 8 months ago (2014-04-21 20:00:15 UTC) #6
wtc
Patch set 3 LGTM. Please add an entry about this patch to README.chromium. Note: we ...
6 years, 8 months ago (2014-04-22 21:50:49 UTC) #7
davidben
https://codereview.chromium.org/240633006/diff/40001/net/third_party/nss/ssl/ssl3ext.c File net/third_party/nss/ssl/ssl3ext.c (right): https://codereview.chromium.org/240633006/diff/40001/net/third_party/nss/ssl/ssl3ext.c#newcode297 net/third_party/nss/ssl/ssl3ext.c:297: /* Some servers are intolerant to the last extension ...
6 years, 7 months ago (2014-04-28 19:52:26 UTC) #8
wtc
Patch set 4 LGTM. https://codereview.chromium.org/240633006/diff/50001/net/third_party/nss/patches/applypatches.sh File net/third_party/nss/patches/applypatches.sh (right): https://codereview.chromium.org/240633006/diff/50001/net/third_party/nss/patches/applypatches.sh#newcode52 net/third_party/nss/patches/applypatches.sh:52: patch -p4 < $patches_dir/reorderextensions.patch Please ...
6 years, 7 months ago (2014-04-28 23:18:07 UTC) #9
davidben
https://codereview.chromium.org/240633006/diff/50001/net/third_party/nss/patches/applypatches.sh File net/third_party/nss/patches/applypatches.sh (right): https://codereview.chromium.org/240633006/diff/50001/net/third_party/nss/patches/applypatches.sh#newcode52 net/third_party/nss/patches/applypatches.sh:52: patch -p4 < $patches_dir/reorderextensions.patch On 2014/04/28 23:18:07, wtc wrote: ...
6 years, 7 months ago (2014-04-28 23:35:02 UTC) #10
wtc
https://codereview.chromium.org/240633006/diff/50001/net/third_party/nss/ssl/ssl3ext.c File net/third_party/nss/ssl/ssl3ext.c (right): https://codereview.chromium.org/240633006/diff/50001/net/third_party/nss/ssl/ssl3ext.c#newcode302 net/third_party/nss/ssl/ssl3ext.c:302: { ssl_ec_point_formats_xtn, &ssl3_SendSupportedPointFormatsXtn }, On 2014/04/28 23:35:02, David Benjamin ...
6 years, 7 months ago (2014-04-28 23:38:33 UTC) #11
davidben
https://codereview.chromium.org/240633006/diff/50001/net/third_party/nss/ssl/ssl3ext.c File net/third_party/nss/ssl/ssl3ext.c (right): https://codereview.chromium.org/240633006/diff/50001/net/third_party/nss/ssl/ssl3ext.c#newcode302 net/third_party/nss/ssl/ssl3ext.c:302: { ssl_ec_point_formats_xtn, &ssl3_SendSupportedPointFormatsXtn }, On 2014/04/28 23:38:33, wtc wrote: ...
6 years, 7 months ago (2014-04-28 23:50:37 UTC) #12
wtc
https://codereview.chromium.org/240633006/diff/50001/net/third_party/nss/ssl/ssl3ext.c File net/third_party/nss/ssl/ssl3ext.c (right): https://codereview.chromium.org/240633006/diff/50001/net/third_party/nss/ssl/ssl3ext.c#newcode302 net/third_party/nss/ssl/ssl3ext.c:302: { ssl_ec_point_formats_xtn, &ssl3_SendSupportedPointFormatsXtn }, On 2014/04/28 23:50:38, David Benjamin ...
6 years, 7 months ago (2014-04-29 12:16:42 UTC) #13
davidben
https://codereview.chromium.org/240633006/diff/50001/net/third_party/nss/ssl/ssl3ext.c File net/third_party/nss/ssl/ssl3ext.c (right): https://codereview.chromium.org/240633006/diff/50001/net/third_party/nss/ssl/ssl3ext.c#newcode302 net/third_party/nss/ssl/ssl3ext.c:302: { ssl_ec_point_formats_xtn, &ssl3_SendSupportedPointFormatsXtn }, On 2014/04/29 12:16:42, wtc wrote: ...
6 years, 7 months ago (2014-04-30 20:49:36 UTC) #14
wtc
Patch set 7 LGTM.
6 years, 7 months ago (2014-04-30 21:07:06 UTC) #15
davidben
The CQ bit was checked by davidben@chromium.org
6 years, 7 months ago (2014-04-30 21:38:47 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/davidben@chromium.org/240633006/110001
6 years, 7 months ago (2014-04-30 21:39:34 UTC) #17
commit-bot: I haz the power
6 years, 7 months ago (2014-05-01 00:39:15 UTC) #18
Message was sent while issue was closed.
Change committed as 267396

Powered by Google App Engine
This is Rietveld 408576698