Reject certificates that are valid for too long.

Reject certificates that are valid for too long.

This is in conformance with the CA/Browser Forum Baseline Requirements for
certificate issuance.

This CL is adapted from a diff provided by sigbjorn@opera.com. Thanks!

BUG=119211
TBR=abarth

Committed: https://crrev.com/15faa5904d553ebff1d72969ab1b7631d7d48d78
Cr-Commit-Position: refs/heads/master@{#303286}

[palmer, 2013-08-01 19:03:53]

Trying again.

abarth for OWNERS in chrome/browser/ssl.

We'll also need a cert_verify_proc_unittest; coming up.

[Ryan Sleevi, 2013-08-01 19:05:01]

Comments from the bug pasted here:

I think we'll want to be careful about this, pending discussions in the CA/B
Forum.

I think reasonable steps to take are similar to what we've discussed for other
BR issues:
- UMA to see how frequent users are experiencing such certs
- Backend analysis to see how prevalent such certs are 'in the wild'

If the numbers indicate a 'reasonable' amount (TBD), then we do a two-phase
approach:
- Downgrade the 'security' indicator for the site (eg: mixed content)
- Show an interstitial or warning page.

I think the latter two points are fairly consistent with how Yngve and Opera
have dealt with other SSL related issues (eg: revocation checking), and provides
a reasonable balance between warning fatigue for users.

I think we'll also only want to be applying this check for certificates from
well-known roots (aka public CAs), since internal/enterprise CAs are not
'expected' to conform to the BRs - even if they are best practice.

[abarth-chromium, 2013-08-01 20:57:26]

Someone else should review this change.  I'd welcome a CL to remove me from
browser/ssl/OWNERS.

[sigbjorn, 2013-08-03 17:52:54]

lgtm

https://codereview.chromium.org/20628006/diff/1/chrome/app/generated_resource...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/20628006/diff/1/chrome/app/generated_resource...
chrome/app/generated_resources.grd:3528: +        Internet standards bodies have
settled on a maximum validity period of 60 months. It is believed that for
certificates with validity periods longer than that, the certification authority
that issued the certificate cannot ensure that the certificate holder is still
the entity they issued the certificate to.
Suggestion:
Internet standards bodies have found that for security reasons, server
certificates should have limited lifespans. This certificate has been issued in
violation of these standards, and might not be safe.

Rationale:
10 years, 60 months, 39 months - the same message will be used for all, so don't
mention the time limit.
There are a bunch of reasons why certificates shouldn't have longer lifespans,
but the main point is that this certificate has been issued contrary to the BRs,
and therefore is suspect.

[Ryan Sleevi, 2013-08-16 02:39:47]

Just to circle back on this - we haven't abandoned it, sigbjorn. We're in the
process of finalizing policy regarding such certs. Thanks for your patience with
this CL, and hopefully we'll be able to move forward on this issue soon.

[palmer, 2013-08-19 17:46:39]

As Ryan says,

https://cabforum.org/pipermail/public/2013-August/002135.html

we'll land this in early 2014.

[Ryan Sleevi, 2013-08-19 17:57:50]

https://codereview.chromium.org/20628006/diff/1/net/cert/cert_verify_proc.cc
File net/cert/cert_verify_proc.cc (right):

https://codereview.chromium.org/20628006/diff/1/net/cert/cert_verify_proc.cc#...
net/cert/cert_verify_proc.cc:353: ++month_diff;
Definitely should add unittests for this logic.

My biggest concern here is about the clamping behaviour on systems where time_t
is 32-bit, and valid_start()/valid_expiry() get off into the woods.

https://codereview.chromium.org/20628006/diff/1/net/cert/cert_verify_proc.cc#...
net/cert/cert_verify_proc.cc:360: base::Time::FromString("1 Jul 2019",
&Jul2019);
Style: Palmer, can you hardcode these and use
base::Time::FromInternal(GG_INT64_C(constant_here));?

Avoid the need for additional parsing.

https://codereview.chromium.org/20628006/diff/1/net/cert/cert_verify_proc.h
File net/cert/cert_verify_proc.h (right):

https://codereview.chromium.org/20628006/diff/1/net/cert/cert_verify_proc.h#n...
net/cert/cert_verify_proc.h:109: // For certificates issued after 1 April 2015:
39 months.
This is not correct. After 1 April 2015, it IS permissible to be > 39 months,
but <= 60 months, provided you meet the criteria.

While I support this change in spirit, I'm not sure what ill-effects will be
seen from the 39-month (if any), so I think we need to be careful.

https://codereview.chromium.org/20628006/diff/1/net/cert/cert_verify_proc.h#n...
net/cert/cert_verify_proc.h:113: // years after the BRs (i.e. by July 2019).
comment nit: rephrase this part without the pronouns, as "we" is unclear here.

// For certificates issued before the BRs took effect, there were no
// guidelines, but clamp them at a maximum of 10 year validity, with
// the requirement they expire within 7 years after the effective
// date of the BRs (i.e. by 1 July 2019).

https://codereview.chromium.org/20628006/diff/1/net/cert/cert_verify_proc.h#n...
net/cert/cert_verify_proc.h:114: static bool HasTooLongValidity(const
X509Certificate& cert);
There's no need to make this a static function, as opposed to within an unnamed
namespace, since this is used (internally) by the CertVerifyProc, and isn't
being unittested.

[wtc, 2013-08-21 00:48:04]

On 2013/08/03 17:52:54, sigbjorn wrote:
>
> Suggestion:
> Internet standards bodies have found that for security reasons, server
> certificates should have limited lifespans. This certificate has been issued
in
> violation of these standards, and might not be safe.

This seems fine to me. I suggest using "validity periods" instead of
"lifespans".

[palmer, 2013-08-21 01:26:24]

https://codereview.chromium.org/20628006/diff/1/chrome/app/generated_resource...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/20628006/diff/1/chrome/app/generated_resource...
chrome/app/generated_resources.grd:3528: +        Internet standards bodies have
settled on a maximum validity period of 60 months. It is believed that for
certificates with validity periods longer than that, the certification authority
that issued the certificate cannot ensure that the certificate holder is still
the entity they issued the certificate to.
On 2013/08/03 17:52:54, sigbjorn wrote:
> Suggestion:
> Internet standards bodies have found that for security reasons, server
> certificates should have limited lifespans. This certificate has been issued
in
> violation of these standards, and might not be safe.
> 
> Rationale:
> 10 years, 60 months, 39 months - the same message will be used for all, so
don't
> mention the time limit.
> There are a bunch of reasons why certificates shouldn't have longer lifespans,
> but the main point is that this certificate has been issued contrary to the
BRs,
> and therefore is suspect.

Done.

https://codereview.chromium.org/20628006/diff/1/net/cert/cert_verify_proc.h
File net/cert/cert_verify_proc.h (right):

https://codereview.chromium.org/20628006/diff/1/net/cert/cert_verify_proc.h#n...
net/cert/cert_verify_proc.h:109: // For certificates issued after 1 April 2015:
39 months.
> This is not correct. After 1 April 2015, it IS permissible to be > 39 months,
> but <= 60 months, provided you meet the criteria.
> 
> While I support this change in spirit, I'm not sure what ill-effects will be
> seen from the 39-month (if any), so I think we need to be careful.

Shall we just go with 60 months then?

https://codereview.chromium.org/20628006/diff/1/net/cert/cert_verify_proc.h#n...
net/cert/cert_verify_proc.h:113: // years after the BRs (i.e. by July 2019).
On 2013/08/19 17:57:50, Ryan Sleevi wrote:
> comment nit: rephrase this part without the pronouns, as "we" is unclear here.
> 
> // For certificates issued before the BRs took effect, there were no
> // guidelines, but clamp them at a maximum of 10 year validity, with
> // the requirement they expire within 7 years after the effective
> // date of the BRs (i.e. by 1 July 2019).

Done. Your fixation is odd. :)

https://codereview.chromium.org/20628006/diff/1/net/cert/cert_verify_proc.h#n...
net/cert/cert_verify_proc.h:114: static bool HasTooLongValidity(const
X509Certificate& cert);
On 2013/08/19 17:57:50, Ryan Sleevi wrote:
> There's no need to make this a static function, as opposed to within an
unnamed
> namespace, since this is used (internally) by the CertVerifyProc, and isn't
> being unittested.

I see it as being like |IsHostnameNonUnique| and so on, and perhaps should be
unit-tested?

[Ryan Sleevi, 2013-08-21 20:07:41]

https://codereview.chromium.org/20628006/diff/1/net/cert/cert_verify_proc.h
File net/cert/cert_verify_proc.h (right):

https://codereview.chromium.org/20628006/diff/1/net/cert/cert_verify_proc.h#n...
net/cert/cert_verify_proc.h:109: // For certificates issued after 1 April 2015:
39 months.
On 2013/08/21 01:26:25, Chromium Palmer wrote:
> > This is not correct. After 1 April 2015, it IS permissible to be > 39
months,
> > but <= 60 months, provided you meet the criteria.
> > 
> > While I support this change in spirit, I'm not sure what ill-effects will be
> > seen from the 39-month (if any), so I think we need to be careful.
> 
> Shall we just go with 60 months then?

I'm fine landing this as the plan of record, and going back on that if
necessary.

I don't think we can safely histogram this either. We can just work to publicize
it as "It's acceptable for CAs to do so for legacy hardware/clients, but if
you're wanting to talk to Chrome, 39 months". This doesn't mean CAs are
violating the BRs though - just that Chrome's policy is security.

May affect the interstitial warnings.

https://codereview.chromium.org/20628006/diff/1/net/cert/cert_verify_proc.h#n...
net/cert/cert_verify_proc.h:114: static bool HasTooLongValidity(const
X509Certificate& cert);
On 2013/08/21 01:26:25, Chromium Palmer wrote:
> On 2013/08/19 17:57:50, Ryan Sleevi wrote:
> > There's no need to make this a static function, as opposed to within an
> unnamed
> > namespace, since this is used (internally) by the CertVerifyProc, and isn't
> > being unittested.
> 
> I see it as being like |IsHostnameNonUnique| and so on, and perhaps should be
> unit-tested?

Glad you just volunteered to write unit tests ;) That was the goal :)

[palmer, 2013-08-21 22:24:15]

https://codereview.chromium.org/20628006/diff/1/net/cert/cert_verify_proc.cc
File net/cert/cert_verify_proc.cc (right):

https://codereview.chromium.org/20628006/diff/1/net/cert/cert_verify_proc.cc#...
net/cert/cert_verify_proc.cc:360: base::Time::FromString("1 Jul 2019",
&Jul2019);
On 2013/08/19 17:57:50, Ryan Sleevi wrote:
> Style: Palmer, can you hardcode these and use
> base::Time::FromInternal(GG_INT64_C(constant_here));?
> 
> Avoid the need for additional parsing.

Done.

https://codereview.chromium.org/20628006/diff/1/net/cert/cert_verify_proc.h
File net/cert/cert_verify_proc.h (right):

https://codereview.chromium.org/20628006/diff/1/net/cert/cert_verify_proc.h#n...
net/cert/cert_verify_proc.h:109: // For certificates issued after 1 April 2015:
39 months.
Can you suggest accurate wording for the comment?

https://codereview.chromium.org/20628006/diff/1/net/cert/cert_verify_proc.h#n...
net/cert/cert_verify_proc.h:114: static bool HasTooLongValidity(const
X509Certificate& cert);
On 2013/08/21 20:07:41, Ryan Sleevi wrote:
> On 2013/08/21 01:26:25, Chromium Palmer wrote:
> > On 2013/08/19 17:57:50, Ryan Sleevi wrote:
> > > There's no need to make this a static function, as opposed to within an
> > unnamed
> > > namespace, since this is used (internally) by the CertVerifyProc, and
isn't
> > > being unittested.
> > 
> > I see it as being like |IsHostnameNonUnique| and so on, and perhaps should
be
> > unit-tested?
> 
> Glad you just volunteered to write unit tests ;) That was the goal :)

Done.

[palmer, 2014-01-30 20:04:51]

Is it time to land this? Assuming the bots are green.

[palmer, 2014-10-17 01:16:43]

Soo... this CL is long in the tooth... rsleevi, should I resuscitate it, or is
there some alternate plan that is going to happen soon?

[Ryan Sleevi, 2014-10-27 22:07:39]

Sorry about the 'slight' delays on this x_x

You're gonna blow up all the tests (since they have too long validity), but the
"is issued by known root" should be sufficient to mitigate this.

https://codereview.chromium.org/20628006/diff/129001/net/cert/cert_verify_pro...
File net/cert/cert_verify_proc.cc (right):

https://codereview.chromium.org/20628006/diff/129001/net/cert/cert_verify_pro...
net/cert/cert_verify_proc.cc:277: if (HasTooLongValidity(*cert)) {
Remind me why we aren't checking is_issued_by_known_root here?

Creating an internal cert for 30 years is pretty common and not a Bad Thing
(tm), since it's local policy.

https://codereview.chromium.org/20628006/diff/129001/net/cert/cert_verify_pro...
net/cert/cert_verify_proc.cc:548: cert.valid_expiry().UTCExplode(&expiry);
Note that both of these can fail for some certificates (e.g. assume hostile
2038-bug exploiting certs)

Also, shouldn't you be checking that expiry > start, since we may be calling
this method for certs where it's not true? (Nothing is guaranteed in the
pre/post conditions that this is a 'valid' cert)

[palmer, 2014-10-28 00:05:53]

https://codereview.chromium.org/20628006/diff/129001/net/cert/cert_verify_pro...
File net/cert/cert_verify_proc.cc (right):

https://codereview.chromium.org/20628006/diff/129001/net/cert/cert_verify_pro...
net/cert/cert_verify_proc.cc:277: if (HasTooLongValidity(*cert)) {
On 2014/10/27 22:07:39, Ryan Sleevi wrote:
> Remind me why we aren't checking is_issued_by_known_root here?
> 
> Creating an internal cert for 30 years is pretty common and not a Bad Thing
> (tm), since it's local policy.

Done.

https://codereview.chromium.org/20628006/diff/129001/net/cert/cert_verify_pro...
net/cert/cert_verify_proc.cc:548: cert.valid_expiry().UTCExplode(&expiry);
On 2014/10/27 22:07:39, Ryan Sleevi wrote:
> Note that both of these can fail for some certificates (e.g. assume hostile
> 2038-bug exploiting certs)
> 
> Also, shouldn't you be checking that expiry > start, since we may be calling
> this method for certs where it's not true? (Nothing is guaranteed in the
> pre/post conditions that this is a 'valid' cert)

Done for a first pass. My immediate goal is to re-build this CL (it's so old it
doesn't apply cleanly anymore). See what you think of the up-coming patchset.

[Ryan Sleevi, 2014-10-29 22:22:07]

https://codereview.chromium.org/20628006/diff/229001/net/base/net_error_list.h
File net/base/net_error_list.h (right):

https://codereview.chromium.org/20628006/diff/229001/net/base/net_error_list....
net/base/net_error_list.h:448: NET_ERROR(CERT_TOO_LONG_VALIDITY, -213)
CERT_VALIDITY_TOO_LONG ?

https://codereview.chromium.org/20628006/diff/229001/net/cert/cert_verify_pro...
File net/cert/cert_verify_proc.cc (right):

https://codereview.chromium.org/20628006/diff/229001/net/cert/cert_verify_pro...
net/cert/cert_verify_proc.cc:281: if (!verify_result->is_issued_by_known_root &&
HasTooLongValidity(*cert)) {
This is wrong.

if (verify_result->is_issued_by_known_root ...)

[This will also make your test fail, which is right, because the test is wrong]

https://codereview.chromium.org/20628006/diff/229001/net/cert/cert_verify_pro...
net/cert/cert_verify_proc.cc:642: exploded_start.year * 12 -
exploded_start.month;
I still hate math and years, but also hate multiplying ints by 12. Even if they
shouldn't exceed 9999.

if ((exploded_expiry.year - exploded_start.year) > 10)
  return true;
int month_diff =
    (exploded_expiry.year - exploded_start.year) * 12 +
    (exploded_expiry.month - exploded_start.month);

2014/01 - 2014/03 = 0 * 12 + (3 - 1) = 2
2014/01 - 2015/01 = 1 * 12 + (1 - 1) = 12
2014/12 - 2015/01 = 1 * 12 + (1 - 12) = 1
2014/01 - 2015/03 = 1 * 12 + (3 - 1) = 14

Pretty sure the math works...

(If you disagree, well, your indents are still wrong at least. git cl format
that)

https://codereview.chromium.org/20628006/diff/229001/net/cert/cert_verify_pro...
net/cert/cert_verify_proc.cc:648: GG_INT64_C(1427871600));
GG_ is deprecated. Long live the <stdint.h> macros of INT64_C

https://codereview.chromium.org/20628006/diff/229001/net/data/ssl/certificate...
File net/data/ssl/certificates/README (right):

https://codereview.chromium.org/20628006/diff/229001/net/data/ssl/certificate...
net/data/ssl/certificates/README:257: TODO(palmer): Encapsulate this in a
script, like the others.
generate-test-certs.sh is where these free-standing ones go.

[palmer, 2014-10-30 01:23:16]

A not-yet-ready patchset is on the way.

https://codereview.chromium.org/20628006/diff/229001/net/base/net_error_list.h
File net/base/net_error_list.h (right):

https://codereview.chromium.org/20628006/diff/229001/net/base/net_error_list....
net/base/net_error_list.h:448: NET_ERROR(CERT_TOO_LONG_VALIDITY, -213)
> CERT_VALIDITY_TOO_LONG ?

Done throughout.

https://codereview.chromium.org/20628006/diff/229001/net/cert/cert_verify_pro...
File net/cert/cert_verify_proc.cc (right):

https://codereview.chromium.org/20628006/diff/229001/net/cert/cert_verify_pro...
net/cert/cert_verify_proc.cc:281: if (!verify_result->is_issued_by_known_root &&
HasTooLongValidity(*cert)) {
On 2014/10/29 22:22:07, Ryan Sleevi wrote:
> This is wrong.
> 
> if (verify_result->is_issued_by_known_root ...)
> 
> [This will also make your test fail, which is right, because the test is
wrong]

Done.

https://codereview.chromium.org/20628006/diff/229001/net/cert/cert_verify_pro...
net/cert/cert_verify_proc.cc:642: exploded_start.year * 12 -
exploded_start.month;
On 2014/10/29 22:22:07, Ryan Sleevi wrote:
> I still hate math and years, but also hate multiplying ints by 12. Even if
they
> shouldn't exceed 9999.
> 
> if ((exploded_expiry.year - exploded_start.year) > 10)
>   return true;
> int month_diff =
>     (exploded_expiry.year - exploded_start.year) * 12 +
>     (exploded_expiry.month - exploded_start.month);
> 
> 2014/01 - 2014/03 = 0 * 12 + (3 - 1) = 2
> 2014/01 - 2015/01 = 1 * 12 + (1 - 1) = 12
> 2014/12 - 2015/01 = 1 * 12 + (1 - 12) = 1
> 2014/01 - 2015/03 = 1 * 12 + (3 - 1) = 14
> 
> Pretty sure the math works...
> 
> (If you disagree, well, your indents are still wrong at least. git cl format
> that)

Done.

https://codereview.chromium.org/20628006/diff/229001/net/cert/cert_verify_pro...
net/cert/cert_verify_proc.cc:648: GG_INT64_C(1427871600));
On 2014/10/29 22:22:07, Ryan Sleevi wrote:
> GG_ is deprecated. Long live the <stdint.h> macros of INT64_C

Done.

https://codereview.chromium.org/20628006/diff/229001/net/data/ssl/certificate...
File net/data/ssl/certificates/README (right):

https://codereview.chromium.org/20628006/diff/229001/net/data/ssl/certificate...
net/data/ssl/certificates/README:257: TODO(palmer): Encapsulate this in a
script, like the others.
On 2014/10/29 22:22:07, Ryan Sleevi wrote:
> generate-test-certs.sh is where these free-standing ones go.

Done.

[palmer, 2014-10-30 20:55:51]

OK, here is some more testing.

One problem: CertVerifyProcTest.IntranetHostsRejected fails because ok_cert.pem
is valid for too long. If I replace it with a new ok_cert.pem with a legal
validity period, a bunch of other tests fail because the new cert is not valid.
But:

~/chromium/src $ diff net/data/ssl/certificates/ok_cert.pem
net/data/ssl/certificates/ok_cert.pem.new
[key material change...]
36,37c36,37
<             Not Before: Aug 14 03:05:29 2014 GMT
<             Not After : Aug 11 03:05:29 2024 GMT
---
>             Not Before: Oct 30 01:08:46 2014 GMT
>             Not After : Jul 26 01:08:46 2017 GMT
43,60c43,60
[signature change...]

Any clues? Its specific key or signature or something is pinned or statically
encoded somewhere?

I see e.g.:

const TestCertMetaData kCert2 = {
    "ok_cert.pem", "cert:6C9DFD2CFA9885C71BE6DE0EA0CF962AC8F9131B"};

in net/http/disk_based_cert_cache_unittest.cc.

[Ryan Sleevi, 2014-10-30 22:29:33]

On 2014/10/30 20:55:51, Chromium Palmer wrote:
> OK, here is some more testing.
> 
> One problem: CertVerifyProcTest.IntranetHostsRejected fails because
ok_cert.pem
> is valid for too long. If I replace it with a new ok_cert.pem with a legal
> validity period, a bunch of other tests fail because the new cert is not
valid.

Yes, ok_cert.pem is hardcoded all over the place (HSTS especially)

Don't replace ok_cert.pem. Just change IntranetHostsRejected to a new test cert
(and slap it in generate-test-certs.sh)

[palmer, 2014-10-31 20:05:23]

> Don't replace ok_cert.pem. Just change IntranetHostsRejected to a new test
cert
> (and slap it in generate-test-certs.sh)

Done.

[Ryan Sleevi, 2014-11-04 21:48:27]

LGTM mod several nits that should be addressed/explained.

https://codereview.chromium.org/20628006/diff/289001/net/data/ssl/certificate...
File net/data/ssl/certificates/61_months_after_2012_07.pem (right):

https://codereview.chromium.org/20628006/diff/289001/net/data/ssl/certificate...
net/data/ssl/certificates/61_months_after_2012_07.pem:10: Subject:
CN=xn--wgv71a119e.com
Punycode CNs may be overkill.

https://codereview.chromium.org/20628006/diff/289001/net/data/ssl/certificate...
File net/data/ssl/certificates/README (right):

https://codereview.chromium.org/20628006/diff/289001/net/data/ssl/certificate...
net/data/ssl/certificates/README:153: Certs to test that the maximum validity
durations set by the Baseline
the CA/Browser Forum Baseline Requirements

https://codereview.chromium.org/20628006/diff/289001/net/data/ssl/scripts/gen...
File net/data/ssl/scripts/generate-test-certs.sh (right):

https://codereview.chromium.org/20628006/diff/289001/net/data/ssl/scripts/gen...
net/data/ssl/scripts/generate-test-certs.sh:70: -days 1000 \
Why was this change necessary? Unrelated? Why didn't you change line 80?

Seems like this was unnecessary given your new code to avoid ok_cert

https://codereview.chromium.org/20628006/diff/289001/net/data/ssl/scripts/gen...
net/data/ssl/scripts/generate-test-certs.sh:129: ## Reject intranet hosts
Reject intranet hostnames in "publicly" trusted certs

https://codereview.chromium.org/20628006/diff/289001/net/data/ssl/scripts/gen...
net/data/ssl/scripts/generate-test-certs.sh:143: -days $((365 * 11)) \
ಠ_ಠ

KISS. This is /bin/sh style we're dealing with.

https://codereview.chromium.org/20628006/diff/289001/net/data/ssl/scripts/gen...
net/data/ssl/scripts/generate-test-certs.sh:165: -days $((30 * 61)) \
Ditto

[palmer, 2014-11-07 19:16:14]

https://codereview.chromium.org/20628006/diff/289001/net/data/ssl/certificate...
File net/data/ssl/certificates/61_months_after_2012_07.pem (right):

https://codereview.chromium.org/20628006/diff/289001/net/data/ssl/certificate...
net/data/ssl/certificates/61_months_after_2012_07.pem:10: Subject:
CN=xn--wgv71a119e.com
> Punycode CNs may be overkill. 

It doesn't hurt anything; and we might as well normalize the concept since it's
not going away.

https://codereview.chromium.org/20628006/diff/289001/net/data/ssl/certificate...
File net/data/ssl/certificates/README (right):

https://codereview.chromium.org/20628006/diff/289001/net/data/ssl/certificate...
net/data/ssl/certificates/README:153: Certs to test that the maximum validity
durations set by the Baseline
On 2014/11/04 21:48:27, Ryan Sleevi wrote:
> the CA/Browser Forum Baseline Requirements

Done.

https://codereview.chromium.org/20628006/diff/289001/net/data/ssl/scripts/gen...
File net/data/ssl/scripts/generate-test-certs.sh (right):

https://codereview.chromium.org/20628006/diff/289001/net/data/ssl/scripts/gen...
net/data/ssl/scripts/generate-test-certs.sh:70: -days 1000 \
> Seems like this was unnecessary given your new code to avoid ok_cert

Yeah, reverted. I think it was from when I was trying to get ok_cert to work for
me.

https://codereview.chromium.org/20628006/diff/289001/net/data/ssl/scripts/gen...
net/data/ssl/scripts/generate-test-certs.sh:129: ## Reject intranet hosts
On 2014/11/04 21:48:27, Ryan Sleevi wrote:
> Reject intranet hostnames in "publicly" trusted certs

Done.

https://codereview.chromium.org/20628006/diff/289001/net/data/ssl/scripts/gen...
net/data/ssl/scripts/generate-test-certs.sh:143: -days $((365 * 11)) \
On 2014/11/04 21:48:27, Ryan Sleevi wrote:
> ಠ_ಠ
> 
> KISS. This is /bin/sh style we're dealing with.

Done.

https://codereview.chromium.org/20628006/diff/289001/net/data/ssl/scripts/gen...
net/data/ssl/scripts/generate-test-certs.sh:165: -days $((30 * 61)) \
Done and on line 131 too.

[palmer, 2014-11-07 19:17:17]

The CQ bit was checked by palmer@chromium.org

[commit-bot: I haz the power, 2014-11-07 19:18:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/20628006/309001

[commit-bot: I haz the power, 2014-11-07 19:25:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-11-07 19:25:28]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[palmer, 2014-11-07 19:36:29]

The CQ bit was checked by palmer@chromium.org

[commit-bot: I haz the power, 2014-11-07 19:38:28]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/20628006/309001

[commit-bot: I haz the power, 2014-11-07 20:32:24]

Committed patchset #10 (id:309001)

[commit-bot: I haz the power, 2014-11-07 20:32:57]

Patchset 10 (id:??) landed as
https://crrev.com/15faa5904d553ebff1d72969ab1b7631d7d48d78
Cr-Commit-Position: refs/heads/master@{#303286}

[twocsies, 2014-11-09 12:12:58]

Looks like this change caused some errors. Until April 2015 there can be 60
month certs. Many sites are now throwing the NET::ERR_CERT_VALIDITY_TOO_LONG
including my university's site. 

The sites I checked all recently obtained 60 month certs, good until July 2018.
For example:
https://stuadmin.flinders.edu.au
https://www.systemshock.org

"Your connection is not private

Attackers might be trying to steal your information from
stuadmin.flinders.edu.au (for example, passwords, messages, or credit cards).

Back to safetyAdvanced
NET::ERR_CERT_VALIDITY_TOO_LONG"

[levente.tamas, 2014-11-10 12:35:31]

Why doesn't it change this in the FUTURE so that issuers have time to react and
reissue certs. We bought 10+ certs this year all for 5 year and they became
unusable. Also to make matters worse if HSTS was enabled then there is no bypass
making the site completely unreachable with a message:


You cannot visit XXXXX right now because the website uses HSTS. Network errors
and attacks are usually temporary, so this page will probably work later.

NET::ERR_CERT_VALIDITY_TOO_LONG

[haavardm, 2014-11-10 16:20:00]

haavardm@opera.com changed reviewers:
+ haavardm@opera.com

[haavardm, 2014-11-10 16:20:01]

https://codereview.chromium.org/20628006/diff/309001/net/cert/cert_verify_pro...
File net/cert/cert_verify_proc.cc (right):

https://codereview.chromium.org/20628006/diff/309001/net/cert/cert_verify_pro...
net/cert/cert_verify_proc.cc:656: if (start >= time_2015_04_01)
The 39 month check kicks in since the hardcoded dates are set in seconds, while
start is in microseconds since the epoch. Adding 6 zeros to the hardcoded dates 
should fix the issue.