Computation of validity period is incorrect

[aarongable]

RFC 5280, Section 4.1.2.5, says:

The validity period for a certificate is the period of time from
notBefore through notAfter, inclusive.

This means that a certificate with a notBefore timestamp of 2021-01-01 01:01:01 GMT and a notAfter timestamp of 2021-01-01 02:01:01 GMT actually has a "validity period" of 1 hour and one second.

The Baseline Requirements, Section 6.3.2, say:

For the purpose of calculations, a day is measured as 86,400 seconds. Any amount of
time greater than this, including fractional seconds and/or leap seconds, shall represent
an additional day. For this reason, Subscriber Certificates SHOULD NOT be issued for
the maximum permissible time by default, in order to account for such adjustments.

When zlint calculates the validity period of a certificate (in the apple, cabf_br, and ev versions of the 825_days and 39_months lints) it does so using this computation:

zlint/v3/lints/cabf_br/lint_sub_cert_valid_time_longer_than_825_days.go

Line 45 in 7e75dc3

if c.NotBefore.AddDate(0, 0, 825).Before(c.NotAfter) {

In other words, it adds 825 days to the notBefore, and checks to see if that date falls before the notAfter. This does not account for the maximum validity period being inclusive of the time represented by the notAfter timestamp.

I have created a pair of tests. The first one uses a certificate with a lifetime of exactly 825 days, inclusive, and passes:

func TestSubCertValidTimeExactly825DaysInclusive(t *testing.T) {
	// This certificate has a notBefore of XXX and a notAfter of YYY, giving it
	// a validity period (calculated inclusive of both endpoints, as per RFC5280)
	// of exactly 825 days (71280000 seconds).
	inputPath := "subCertExactly825DaysInclusive.pem"
	expected := lint.Pass
	out := test.TestLint("e_sub_cert_valid_time_longer_than_825_days", inputPath)
	if out.Status != expected {
		t.Errorf("%s: expected %s, got %s", inputPath, expected, out.Status)
	}
}

The second one uses a certificate with a validity period of 825 days and one second, which should be rejected, but the test fails as zlint sees no problem with this certificate:

func TestSubCertValidTimeExactly825DaysExclusive(t *testing.T) {
	// This certificate has a notBefore of XXX and a notAfter of YYY, giving it
	// a validity period (calculated inclusive of both endpoints, as per RFC5280)
	// of exactly 825 days and one second (71280001 seconds).
	inputPath := "subCertExactly825DaysExclusive.pem"
	expected := lint.Error
	out := test.TestLint("e_sub_cert_valid_time_longer_than_825_days", inputPath)
	if out.Status != expected {
		t.Errorf("%s: expected %s, got %s", inputPath, expected, out.Status)
	}
}

You can see both tests here: master...aarongable:validity-period-tests

Unfortunately, fixing this is not as trivial as simply offsetting the calculation by one second. Per RFC 5280, the notBefore and notAfter timestamps may be encoded either as UTCTime or as GenralizedTime (with requirements on which is used based on the date being represented). Unfortunately, UTCTime allows timestamp granularity of either seconds or minutes. Meaning that a fully correct implementation of this must determine the granularity of the timestamp and perform the inclusive validity time computation accordingly.

[aarongable]

(FYI, I'm happy to tackle fixing this, just wanted to get all the details down in a bug before opening PRs.)

[sleevi]

@aarongable I believe this is similar to the Chrome issue you filed, namely https://bugs.chromium.org/p/chromium/issues/detail?id=1217920

The context being that there were existing lints that used the old BR definitions. The new lints that were added for 398 day certs - also part of SC31 - use the definition that accompanied that ballot