Compute validity periods inclusive of notAfter second (#5494)

In the CA, compute the notAfter timestamp such that the cert is actually
valid for the intended duration, not for one second longer. In the
Issuance library, compute the validity period by including the full
length of the final second indicated by the notAfter date when
determining if the certificate request matches our profile. Update tests
and config files to match.

Fixes #5473

ca/ca.go

@@ -386,10 +386,10 @@ func (ca *certificateAuthorityImpl) generateSerialNumberAndValidity() (*big.Int,
 	serialBigInt := big.NewInt(0)
 	serialBigInt = serialBigInt.SetBytes(serialBytes)
 
-	notBefore := ca.clk.Now().Add(-1 * ca.backdate)
+	notBefore := ca.clk.Now().Add(-ca.backdate)
 	validity := validity{
 		NotBefore: notBefore,
-		NotAfter:  notBefore.Add(ca.validityPeriod),
+		NotAfter:  notBefore.Add(ca.validityPeriod - time.Second),
 	}
 
 	return serialBigInt, validity, nil

ca/ca_test.go

@@ -434,7 +434,7 @@ func issueCertificateSubTestIssuePrecertificate(t *testing.T, i *TestCertificate
 
 func issueCertificateSubTestValidityUsesCAClock(t *testing.T, i *TestCertificateIssuance) {
 	test.AssertEquals(t, i.cert.NotBefore, i.ca.clk.Now().Add(-1*i.ca.backdate))
-	test.AssertEquals(t, i.cert.NotAfter, i.cert.NotBefore.Add(i.ca.validityPeriod))
+	test.AssertEquals(t, i.cert.NotAfter.Add(time.Second).Sub(i.cert.NotBefore), i.ca.validityPeriod)
 }
 
 // Test issuing when multiple issuers are present.

issuance/issuance.go

@@ -314,7 +314,9 @@ func (p *Profile) requestValid(clk clock.Clock, req *IssuanceRequest) error {
 		return errors.New("common name cannot be included")
 	}
 
-	validity := req.NotAfter.Sub(req.NotBefore)
+	// The validity period is calculated inclusive of the whole second represented
+	// by the notAfter timestamp.
+	validity := req.NotAfter.Add(time.Second).Sub(req.NotBefore)
 	if validity <= 0 {
 		return errors.New("NotAfter must be after NotBefore")
 	}

issuance/issuance_test.go

@@ -267,10 +267,23 @@ func TestRequestValid(t *testing.T) {
 			request: &IssuanceRequest{
 				PublicKey: &ecdsa.PublicKey{},
 				NotBefore: fc.Now(),
-				NotAfter:  fc.Now().Add(time.Hour),
+				NotAfter:  fc.Now().Add(time.Hour - time.Second),
 			},
 			expectedError: "validity period is more than the maximum allowed period (1h0m0s>1m0s)",
 		},
+		{
+			name: "validity larger than max due to inclusivity",
+			profile: &Profile{
+				useForECDSALeaves: true,
+				maxValidity:       time.Hour,
+			},
+			request: &IssuanceRequest{
+				PublicKey: &ecdsa.PublicKey{},
+				NotBefore: fc.Now(),
+				NotAfter:  fc.Now().Add(time.Hour),
+			},
+			expectedError: "validity period is more than the maximum allowed period (1h0m1s>1h0m0s)",
+		},
 		{
 			name: "validity backdated more than max",
 			profile: &Profile{
@@ -536,7 +549,7 @@ func TestIssue(t *testing.T) {
 				CommonName: "example.com",
 				DNSNames:   []string{"example.com"},
 				NotBefore:  fc.Now(),
-				NotAfter:   fc.Now().Add(time.Hour),
+				NotAfter:   fc.Now().Add(time.Hour - time.Second),
 			})
 			test.AssertNotError(t, err, "Issue failed")
 			cert, err := x509.ParseCertificate(certBytes)
@@ -569,7 +582,7 @@ func TestIssueRSA(t *testing.T) {
 		Serial:    []byte{1, 2, 3, 4, 5, 6, 7, 8},
 		DNSNames:  []string{"example.com"},
 		NotBefore: fc.Now(),
-		NotAfter:  fc.Now().Add(time.Hour),
+		NotAfter:  fc.Now().Add(time.Hour - time.Second),
 	})
 	test.AssertNotError(t, err, "Issue failed")
 	cert, err := x509.ParseCertificate(certBytes)
@@ -599,7 +612,7 @@ func TestIssueCTPoison(t *testing.T) {
 		DNSNames:        []string{"example.com"},
 		IncludeCTPoison: true,
 		NotBefore:       fc.Now(),
-		NotAfter:        fc.Now().Add(time.Hour),
+		NotAfter:        fc.Now().Add(time.Hour - time.Second),
 	})
 	test.AssertNotError(t, err, "Issue failed")
 	cert, err := x509.ParseCertificate(certBytes)
@@ -631,7 +644,7 @@ func TestIssueSCTList(t *testing.T) {
 			{},
 		},
 		NotBefore: fc.Now(),
-		NotAfter:  fc.Now().Add(time.Hour),
+		NotAfter:  fc.Now().Add(time.Hour - time.Second),
 	})
 	test.AssertNotError(t, err, "Issue failed")
 	cert, err := x509.ParseCertificate(certBytes)
@@ -664,7 +677,7 @@ func TestIssueMustStaple(t *testing.T) {
 		DNSNames:          []string{"example.com"},
 		IncludeMustStaple: true,
 		NotBefore:         fc.Now(),
-		NotAfter:          fc.Now().Add(time.Hour),
+		NotAfter:          fc.Now().Add(time.Hour - time.Second),
 	})
 	test.AssertNotError(t, err, "Issue failed")
 	cert, err := x509.ParseCertificate(certBytes)
@@ -690,7 +703,7 @@ func TestIssueBadLint(t *testing.T) {
 		Serial:    []byte{1, 2, 3, 4, 5, 6, 7, 8},
 		DNSNames:  []string{"example.com"},
 		NotBefore: fc.Now(),
-		NotAfter:  fc.Now().Add(time.Hour),
+		NotAfter:  fc.Now().Add(time.Hour - time.Second),
 	})
 	test.AssertError(t, err, "Issue didn't fail")
 	test.AssertEquals(t, err.Error(), "tbsCertificate linting failed: failed lints: w_ct_sct_policy_count_unsatisfied")

test/config-next/ca-a.json

@@ -48,7 +48,7 @@
             ]
           }
         ],
-        "maxValidityPeriod": "7775999s",
+        "maxValidityPeriod": "7776000s",
         "maxValidityBackdate": "1h5m"
       },
       "issuers": [
@@ -79,7 +79,7 @@
       ],
       "ignoredLints": ["n_subject_common_name_included"]
     },
-    "expiry": "7775999s",
+    "expiry": "7776000s",
     "backdate": "1h",
     "serialPrefix": 255,
     "maxNames": 100,

test/config-next/ca-b.json

@@ -48,7 +48,7 @@
             ]
           }
         ],
-        "maxValidityPeriod": "7775999s",
+        "maxValidityPeriod": "7776000s",
         "maxValidityBackdate": "1h5m"
       },
       "issuers": [
@@ -79,7 +79,7 @@
       ],
       "ignoredLints": ["n_subject_common_name_included"]
     },
-    "expiry": "7775999s",
+    "expiry": "7776000s",
     "backdate": "1h",
     "serialPrefix": 255,
     "maxNames": 100,

test/config-next/cert-checker.json

@@ -5,7 +5,7 @@
       "maxOpenConns": 10
     },
     "hostnamePolicyFile": "test/hostname-policy.yaml",
-    "acceptableValidityPeriods": [7775999, 7776000],
+    "acceptableValidityPeriods": [7776000],
     "ignoredLints": [
       "n_subject_common_name_included"
     ]

test/config/cert-checker.json

@@ -5,6 +5,7 @@
       "maxOpenConns": 10
     },
     "hostnamePolicyFile": "test/hostname-policy.yaml",
+    "acceptableValidityPeriods": [7775999, 7776000],
     "ignoredLints": [
       "n_subject_common_name_included"
     ]