Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Issue 315828 Workaround F5 ClientHello length bug.
Starred by 3 users Project Member Reported by agl@chromium.org, Nov 6 2013 Back to list
Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Feb 2014
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug



Sign in to add a comment
F5 tell us that we need only ensure that a ClientHello record is not between 256 and 511 bytes long (inclusive). We can do that by adding an extension to pad the ClientHello when it would otherwise fall into that range.

This is a tracking bug for landing that change.
 
Comment 1 by wtc@chromium.org, Nov 6 2013
Cc: wtc@chromium.org
I've used the cipher_suites list and the elliptic_curves extension to
test padding before. The cipher_suites list gives us more flexibility.

A new extension specifically for padding is a good idea, but I'm a
little worried that some firewall devices might block it as an unknown
extension.
Comment 2 by agl@chromium.org, Nov 6 2013
> some firewall devices might block it as an unknown extension.

But we've not hit this for NPN, ALPN or ChannelID extensions.
Labels: -Pri-2 Pri-1 M-31 Merge-Requested
I realize this is late in the game for M31 merge, but this fixes an issue which causes Chrome to time out when connecting to some sites fronted by F5 Big IP devices.
Project Member Comment 5 by bugdroid1@chromium.org, Nov 12 2013
------------------------------------------------------------------------
r234619 | agl@chromium.org | 2013-11-12T20:16:18.926542Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/net/third_party/nss/ssl/sslimpl.h?r1=234619&r2=234618&pathrev=234619
   M http://src.chromium.org/viewvc/chrome/trunk/src/net/third_party/nss/ssl/ssl3con.c?r1=234619&r2=234618&pathrev=234619
   M http://src.chromium.org/viewvc/chrome/trunk/src/net/third_party/nss/ssl/ssl3ext.c?r1=234619&r2=234618&pathrev=234619
   M http://src.chromium.org/viewvc/chrome/trunk/src/net/third_party/nss/patches/paddingextension.patch?r1=234619&r2=234618&pathrev=234619

net: don't add padding extension for SSLv3.

This is a no-op change because our SSLv3 handshakes aren't long enough to
trigger the padding extension. But, if they were, the padding extension would
break them.

BUG= 315828 

Review URL: https://codereview.chromium.org/66553007
------------------------------------------------------------------------
Project Member Comment 6 by bugdroid1@chromium.org, Nov 13 2013
------------------------------------------------------------------------
r234850 | agl@chromium.org | 2013-11-13T18:08:41.974304Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/net/third_party/nss/ssl/ssl3ext.c?r1=234850&r2=234849&pathrev=234850
   M http://src.chromium.org/viewvc/chrome/trunk/src/net/third_party/nss/patches/applypatches.sh?r1=234850&r2=234849&pathrev=234850
   M http://src.chromium.org/viewvc/chrome/trunk/src/net/third_party/nss/README.chromium?r1=234850&r2=234849&pathrev=234850
   A http://src.chromium.org/viewvc/chrome/trunk/src/net/third_party/nss/patches/paddingextensionall.patch?r1=234850&r2=234849&pathrev=234850

net: add padding extension to all handshakes.

This, temporary, change adds the padding extension to all handshakes to check
whether we encounter any servers with problems.

BUG= 315828 

Review URL: https://codereview.chromium.org/62443004
------------------------------------------------------------------------
Comment 7 by laforge@google.com, Nov 13 2013
Did we regress over M-30?
I'm guessing this is way to late to go into M-31 - and I don't believe this is a regression relative to M-30.

agl+wtc: Do you think we should merge this to M32 or should we just wait for this to go to M33? Is there UMA to get an estimate of how many connections this would impact?
Comment 9 by wtc@chromium.org, Nov 18 2013
In Chrome 31, the TLS ClientHello size should decrease by four bytes
(we gained four AES-GCM cipher suites but lost six HMAC-SHA256 cipher
suites). So this problem could only get slightly better in Chrome 31
than Chrome 30. I think we should not merge this fix to Chrome 31
because the fix may have its own side effect. Are we getting a lot of
reports of this problem?

I think the fix should be merged to Chrome 32. We also need the
OpenSSL version of the fix, which Dr. Steve Henson has implemented.
Labels: -M-31 M-32
Comment 11 by kareng@google.com, Nov 19 2013
m32 is really far down in its path, is there a reason this can't wait to m33?
Comment 12 by kareng@google.com, Nov 21 2013
wtc and i talked about this and we agreed we'll let it bake a bit.
Comment 13 by agl@chromium.org, Nov 25 2013
Labels: -M-32 -Merge-Requested
Pulling Merge-Request label after talking to Karen. M32 is a weird release and I don't think we will be breaking anymore more with M32 than M31 so I guess this has to wait until M33.
Project Member Comment 14 by bugdroid1@chromium.org, Dec 3 2013
------------------------------------------------------------------------
r238460 | wtc@chromium.org | 2013-12-03T21:32:21.979825Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/net/third_party/nss/patches/paddingextension.patch?r1=238460&r2=238459&pathrev=238460
   M http://src.chromium.org/viewvc/chrome/trunk/src/net/third_party/nss/ssl/ssl3ext.c?r1=238460&r2=238459&pathrev=238460

Fill the TLS padding extension with zero bytes instead of spaces.

R=agl@chromium.org
BUG= 315828 
TEST=none

Review URL: https://codereview.chromium.org/99253006
------------------------------------------------------------------------
Comment 15 by agl@chromium.org, Feb 20 2014
Status: Fixed
Comment 16 by wtc@chromium.org, Feb 20 2014
Labels: M-33
Project Member Comment 17 by bugdroid1@chromium.org, Feb 20 2014
------------------------------------------------------------------------
r252421 | agl@chromium.org | 2014-02-20T23:52:01.744737Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/net/third_party/nss/ssl/ssl3ext.c?r1=252421&r2=252420&pathrev=252421
   M http://src.chromium.org/viewvc/chrome/trunk/src/net/third_party/nss/patches/applypatches.sh?r1=252421&r2=252420&pathrev=252421
   M http://src.chromium.org/viewvc/chrome/trunk/src/net/third_party/nss/README.chromium?r1=252421&r2=252420&pathrev=252421
   D http://src.chromium.org/viewvc/chrome/trunk/src/net/third_party/nss/patches/paddingextensionall.patch?r1=252421&r2=252420&pathrev=252421

net: don't always add TLS padding.

In order to flush out any problems with padding, Chrome has always been
adding it, even when the ClientHello was small enough not to need it.

Since that change is in Chrome 33 (and the Chrome 34 branch), it's time
to remove it.

BUG= 315828 

Review URL: https://codereview.chromium.org/171713011
------------------------------------------------------------------------
Project Member Comment 18 by bugdroid1@chromium.org, Feb 27 2014
Labels: merge-merged-1847
------------------------------------------------------------------------
r253838 | agl@chromium.org | 2014-02-27T16:17:00.743845Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/1847/src/net/third_party/nss/patches/applypatches.sh?r1=253838&r2=253837&pathrev=253838
   M http://src.chromium.org/viewvc/chrome/branches/1847/src/net/third_party/nss/README.chromium?r1=253838&r2=253837&pathrev=253838
   D http://src.chromium.org/viewvc/chrome/branches/1847/src/net/third_party/nss/patches/paddingextensionall.patch?r1=253838&r2=253837&pathrev=253838
   M http://src.chromium.org/viewvc/chrome/branches/1847/src/net/third_party/nss/ssl/ssl3ext.c?r1=253838&r2=253837&pathrev=253838

Merge 252421 "net: don't always add TLS padding."

> net: don't always add TLS padding.
> 
> In order to flush out any problems with padding, Chrome has always been
> adding it, even when the ClientHello was small enough not to need it.
> 
> Since that change is in Chrome 33 (and the Chrome 34 branch), it's time
> to remove it.
> 
> BUG= 315828 
> 
> Review URL: https://codereview.chromium.org/171713011

TBR=agl@chromium.org

Review URL: https://codereview.chromium.org/180973006
------------------------------------------------------------------------
Cc: srsridhar@chromium.org
Labels: Needs-Feedback
Could you please let us know if QA can verify the fix. If so please provide us with the steps to verify the same.
Sign in to add a comment