Reduce the network timeout for NSS from 60 seconds to 15 seconds

Reduce the network timeout for NSS from 60 seconds to 15 seconds

BUG=143747
TEST=On Linux, Enable revocation checking via preferences.
  Run chrome with --v=1 from behind a captive portal/firewall that
  allows SSL access to a particular host, but does not allow OCSP
  or CRL fetches, and instead blackholes them (holds the TCP
  connection open). You should see "OCSP Timed out" after 15
  seconds from when the OCSP connection started.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=154105

[Ryan Sleevi, 2012-08-24 21:28:58]

wtc: PTAL.

I wonder whether this should be reduced to 10 or even 5 seconds, but for now, 15
to match parity with Windows' defaults.

[wtc, 2012-08-24 21:52:08]

Patch set 1 LGTM.

http://codereview.chromium.org/10875059/diff/1/net/ocsp/nss_ocsp.cc
File net/ocsp/nss_ocsp.cc (right):

http://codereview.chromium.org/10875059/diff/1/net/ocsp/nss_ocsp.cc#newcode552
net/ocsp/nss_ocsp.cc:552: // The default NSS value for OCSP timeouts is 60
seconds. Set this to be

Nit: omit this comment because it is already in front of
the kOcspTimeoutInSecs constant definition.

We should also change the default NSS OCSP timeout to a smaller
value. Rick Andrews and Scott Rea's Revocation document
does not recommend a specific timeout. I think 15 seconds
should be fine.

http://codereview.chromium.org/10875059/diff/1/net/ocsp/nss_ocsp.cc#newcode554
net/ocsp/nss_ocsp.cc:554: CERT_SetOCSPTimeout(kOcspTimeoutInSecs);

I verified that this function always returns SECSuccess.
So it is fine to not check the return value.

IMPORTANT: please verify this function affects libpkix.
I seem to remember libpkix has its own hardcoded timeout:

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/libpkix/...

http://mxr.mozilla.org/security/ident?i=PKIX_DEFAULT_COMM_TIMEOUT_SECONDS

If so, we'll need to cap the timeout inside OCSPCreate().

[Ryan Sleevi, 2012-08-24 21:58:24]

http://codereview.chromium.org/10875059/diff/1/net/ocsp/nss_ocsp.cc
File net/ocsp/nss_ocsp.cc (right):

http://codereview.chromium.org/10875059/diff/1/net/ocsp/nss_ocsp.cc#newcode552
net/ocsp/nss_ocsp.cc:552: // The default NSS value for OCSP timeouts is 60
seconds. Set this to be
On 2012/08/24 21:52:08, wtc wrote:
> 
> Nit: omit this comment because it is already in front of
> the kOcspTimeoutInSecs constant definition.
> 
> We should also change the default NSS OCSP timeout to a smaller
> value. Rick Andrews and Scott Rea's Revocation document
> does not recommend a specific timeout. I think 15 seconds
> should be fine.

They suggest (no less than) 3 seconds, but I think that's as unrealistic as the
premise that OCSP responders answer within 300ms.

http://codereview.chromium.org/10875059/diff/1/net/ocsp/nss_ocsp.cc#newcode554
net/ocsp/nss_ocsp.cc:554: CERT_SetOCSPTimeout(kOcspTimeoutInSecs);
On 2012/08/24 21:52:08, wtc wrote:
> 
> I verified that this function always returns SECSuccess.
> So it is fine to not check the return value.
> 
> IMPORTANT: please verify this function affects libpkix.
> I seem to remember libpkix has its own hardcoded timeout:
> 
>
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/libpkix/...
> 
> http://mxr.mozilla.org/security/ident?i=PKIX_DEFAULT_COMM_TIMEOUT_SECONDS
> 
> If so, we'll need to cap the timeout inside OCSPCreate().

Yes, I checked this function, at least couldn't find it being hit in debug.

The NSS timeout seconds only appears to affect AIA fetches. However, we should
also have those capped at 15 seconds, so that's reason enough to ensure that
OCSPCreate also adds a cap. I will do that.

[wtc, 2012-08-24 23:51:52]

Patch set 2 LGTM.

http://codereview.chromium.org/10875059/diff/6001/net/ocsp/nss_ocsp.cc
File net/ocsp/nss_ocsp.cc (right):

http://codereview.chromium.org/10875059/diff/6001/net/ocsp/nss_ocsp.cc#newcode47
net/ocsp/nss_ocsp.cc:47: // saner upper limit for OCSP/CRL/AIA fetches.

Nit: OCSP responder location is also listed in the AIA extension.
Perhaps change "AIA" to "CA issuer" or "CA certificate"?

[commit-bot: I haz the power, 2012-08-29 19:20:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rsleevi@chromium.org/10875059/9001

[commit-bot: I haz the power, 2012-08-29 23:58:02]

Try job failure for 10875059-9001 (retry) (retry) on win_rel for step
"browser_tests".
It's a second try, previously, step "browser_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2012-08-30 01:37:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rsleevi@chromium.org/10875059/9001

[commit-bot: I haz the power, 2012-08-30 04:47:27]

Try job failure for 10875059-9001 (retry) on win_rel for step "browser_tests".
It's a second try, previously, step "browser_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2012-08-30 04:57:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rsleevi@chromium.org/10875059/9001

[commit-bot: I haz the power, 2012-08-30 09:53:20]

Change committed as 154105