Use 90 day test certs in temporal shard window

.github/workflows/golangci-lint.yml

@@ -8,6 +8,8 @@ on:
   pull_request:
     branches:
       - '*'
+  # Allows you to run this workflow manually from the Actions tab
+  workflow_dispatch:
 permissions:
   contents: read
   pull-requests: read
@@ -16,11 +18,11 @@ jobs:
     name: lint
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/setup-go@v2
-      - uses: actions/checkout@v2
+      - uses: actions/setup-go@v4
+      - uses: actions/checkout@v3
       - name: golangci-lint
-        uses: golangci/golangci-lint-action@v2
+        uses: golangci/golangci-lint-action@v3
         with:
-          version: v1.45.2
+          version: latest
           args: --timeout 9m
           only-new-issues: true

.github/workflows/test.yml

@@ -5,7 +5,8 @@ on:
     branches: [ main ]
   pull_request:
     branches: [ main ]
-
+  # Allows you to run this workflow manually from the Actions tab
+  workflow_dispatch:
 jobs:
   test:
     runs-on: ubuntu-latest
@@ -15,7 +16,7 @@ jobs:
     - name: Set up Go
       uses: actions/setup-go@v3
       with:
-        go-version: 1.17
+        go-version: 1.20.2
 
     - name: Install cover
       run: go install golang.org/x/tools/cmd/cover@latest

.golangci.yaml

@@ -12,3 +12,12 @@ linters:
     - stylecheck
     - unconvert
     - unused 
+linters-settings:
+  stylecheck:
+    # ST1005: Incorrectly formatted error string
+    checks: ["all", "-ST1005"]
+  gosec:
+    excludes:
+      # TODO(#133): Identify, fix, and remove violations of most of these rules
+      - G112 # Potential Slowloris Attack because ReadHeaderTimeout is not configured in the http.Server (gosec)
+      - G404 # Use of weak random number generator (math/rand instead of crypto/rand)

monitor/sth_fetcher.go

@@ -203,16 +203,21 @@ func (f *sthFetcher) observeSTH() {
 // firstSTH and secondSTH. If the two STHs don't verify then the
 // `sth_inconsistencies` prometheus counter is incremented with a label
 // indicating the category of inconsistency and an error is logged with
-// `logErrorf`. Presently there are three possible categories of STH consistency
-// failure:
-// 1. "equal-treesize-inequal-hash" - the two STHs are the same tree size but
-//    have different root hashes.
-// 2. "failed-to-get-proof" - the monitor encountered an error getting
-//    a consistency proof between the two STHs from the log.
-// 3. "failed-to-verify-proof" - the monitor returned a proof of consistency
-//    between the two STHs that did not verify.
-// When the monitor fetches a consistency proof from the log it publishes the
-// latency of the operation to the `sth_proof_latency` prometheus histogram.
+// `logErrorf`. Presently there are three possible categories of STH
+// consistency failure:
+//
+// 1. "equal-treesize-inequal-hash" - the two STHs are the same tree size
+// but have different root hashes.
+//
+// 2. "failed-to-get-proof" - the monitor encountered an error getting a
+// consistency proof between the two STHs from the log.
+//
+// 3. "failed-to-verify-proof" - the monitor returned a proof of
+// consistency between the two STHs that did not verify.
+//
+// When the monitor fetches a consistency proof from the log it publishes
+// the latency of the operation to the `sth_proof_latency` prometheus
+// histogram.
 func (f *sthFetcher) verifySTHConsistency(firstSTH, secondSTH *ct.SignedTreeHead) {
 	if firstSTH == nil || secondSTH == nil {
 		f.logErrorf("firstSTH or secondSTH was nil")

pki/certs.go

@@ -101,23 +101,26 @@ type CertificatePair struct {
 	Cert    *x509.Certificate
 }
 
-// IssueTestCertificate uses the monitor's certIssuer and certIssuerKey to generate
-// a precertificate and a matching final leaf-certificate that can be submitted
-// to a log. The certificate's subject common name will be a random subdomain
-// based on the certificate serial under the provided baseDomain (or
-// defaultTestCertDomain domain if baseDomain is empty).
+// IssueTestCertificate uses the monitor's certIssuer and certIssuerKey to
+// generate a precertificate and a matching final leaf-certificate that
+// can be submitted to a log. The certificate's subject common name will
+// be a random subdomain based on the certificate serial under the
+// provided baseDomain (or defaultTestCertDomain domain if baseDomain is
+// empty).
 //
-// If windowStart is nil the certificate NotBefore will be set to the current
-// time based on the provided clock. If windowStart is not nil then the
-// certificate NotBefore will be set to the windowStart plus one day.
-
-// If windowEnd is nil the certificate NotAfter will be set to 90 days after the
-// current time based on the provided clock. If windowEnd is not nil then the
-// certificate NotAfter will be set to the windowEnd minus one day.
+// If windowStart is nil the certificate NotBefore will be set to the
+// current time based on the provided clock.
+//
+// If windowEnd is nil the certificate NotAfter will be set to 90 days
+// after the current time based on the provided clock.
+//
+// If windowStart and windowEnd are not nil then issue a 90 day
+// certificate that falls in the window.
 //
-// This function creates certificates that will be submitted to public logs and
-// so while they are not issued by a trusted root  we try to avoid cablint
-// errors to avoid requiring log monitors special-case our submissions.
+// This function creates certificates that will be submitted to public
+// logs and so while they are not issued by a trusted root  we try to
+// avoid cablint errors to avoid requiring log monitors special-case our
+// submissions.
 func IssueTestCertificate(
 	baseDomain string,
 	issuerKey *ecdsa.PrivateKey,
@@ -141,16 +144,21 @@ func IssueTestCertificate(
 		return CertificatePair{}, err
 	}
 
-	earliest := clk.Now()
-	latest := earliest.AddDate(0, 0, 90)
+	// validityPeriod is 90 days minus 1 second, because RFC 5280 counts
+	// the final second as inclusive and golang counts it as exclusive.
+	validityPeriod := 90*24*time.Hour - time.Second
 
-	if windowStart != nil {
-		earliest = *windowStart
-		earliest.AddDate(0, 0, 1)
-	}
+	notBefore := clk.Now()
+	notAfter := notBefore.Add(validityPeriod)
+
+	// If `notAfter` generated from the system clock doesn't fall within the
+	// temporal shard window then we need to adjust the certificate validity
+	// to fall within the window.
 	if windowEnd != nil {
-		latest = *windowEnd
-		latest.AddDate(0, 0, -1)
+		if notAfter.Before(*windowStart) || notAfter.After(*windowEnd) {
+			notAfter = windowEnd.Add(-1 * time.Hour)
+			notBefore = notAfter.Add(-validityPeriod)
+		}
 	}
 
 	domain := hex.EncodeToString(serial.Bytes()[:5]) + baseDomain
@@ -162,8 +170,8 @@ func IssueTestCertificate(
 			},
 			DNSNames:              []string{domain},
 			SerialNumber:          serial,
-			NotBefore:             earliest,
-			NotAfter:              latest.AddDate(0, 0, -1),
+			NotBefore:             notBefore,
+			NotAfter:              notAfter,
 			KeyUsage:              x509.KeyUsageDigitalSignature,
 			ExtKeyUsage:           []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth},
 			BasicConstraintsValid: true,

pki/certs_test.go

@@ -165,7 +165,7 @@ func TestIssueTestCertificate(t *testing.T) {
 	}
 }
 
-func TestIssueTestCertificateWindow(t *testing.T) {
+func TestIssueTestCertificateWindowNil(t *testing.T) {
 	issuerKey, _ := RandKey()
 	issuerCert := &x509.Certificate{}
 	clk := clock.New()
@@ -185,12 +185,13 @@ func TestIssueTestCertificateWindow(t *testing.T) {
 	}
 
 	shortFormat := func(t time.Time) string {
-		return t.Format("2006-01-02")
+		return t.Format("2006-01-02 15:04")
 	}
 
 	// Check that the precert notbefore/notafter match defaults
-	now := shortFormat(clk.Now())
-	defaultNotAfter := shortFormat(clk.Now().AddDate(0, 0, 89))
+	now := shortFormat(clk.Now().UTC())
+	validityPeriod := 90*24*time.Hour - time.Second
+	defaultNotAfter := shortFormat(clk.Now().UTC().Add(validityPeriod))
 	notBefore := shortFormat(certPair.PreCert.NotBefore)
 	notAfter := shortFormat(certPair.PreCert.NotAfter)
 	if notBefore != now {
@@ -213,48 +214,95 @@ func TestIssueTestCertificateWindow(t *testing.T) {
 		t.Errorf("cert notAfter was %q, expected %q",
 			notAfter, defaultNotAfter)
 	}
+}
 
-	windowStart, _ := time.Parse(time.RFC3339, "2000-01-01T00:00:00Z")
-	windowEnd, _ := time.Parse(time.RFC3339, "2001-01-01T00:00:00Z")
-
-	// Issue a cert pair with specific WindowStart and WindowEnd
-	certPair, err = IssueTestCertificate("", issuerKey, issuerCert, clk, &windowStart, &windowEnd)
-	if err != nil {
-		t.Fatalf("unexpected error from IssueTestCertificate: %s", err.Error())
-	}
+// TestIssueTestCertificateWindowNotNil tests cases where the certificate
+// is being generated for temporal shards and ensures the certificates are
+// generated within the temporal window.
+func TestIssueTestCertificateWindowNotNil(t *testing.T) {
+	issuerKey, _ := RandKey()
+	issuerCert := &x509.Certificate{}
+	clk := clock.New()
+	validityPeriod := 90*24*time.Hour - time.Second
 
-	if certPair.PreCert == nil {
-		t.Fatalf("unexpected nil PreCert in CertPair returned from IssueTestCertificate")
-	}
+	// set window for a shard that is in its active temporal window
+	currentWindowStart := clk.Now().AddDate(0, 0, -30)
+	currentWindowEnd := clk.Now().AddDate(0, 0, 180)
 
-	if certPair.Cert == nil {
-		t.Fatalf("unexpected nil Cert in CertPair returned from IssueTestCertificate")
-	}
+	// set window for a shard that is in the past
+	pastWindowStart, _ := time.Parse(time.RFC3339, "2000-01-01T00:00:00Z")
+	pastWindowEnd, _ := time.Parse(time.RFC3339, "2001-01-01T00:00:00Z")
 
-	expectedStartDate := shortFormat(windowStart)
-	expectedEndDate := shortFormat(windowEnd.AddDate(0, 0, -1))
+	// set window for a shard that is in the future
+	futureWindowStart, _ := time.Parse(time.RFC3339, "2100-01-01T00:00:00Z")
+	futureWindowEnd, _ := time.Parse(time.RFC3339, "2101-01-01T00:00:00Z")
 
-	// Check the precert notbefore/notafter match expected
-	notBefore = shortFormat(certPair.PreCert.NotBefore)
-	notAfter = shortFormat(certPair.PreCert.NotAfter)
-	if notBefore != expectedStartDate {
-		t.Errorf("preCert notBefore was %q, expected %q",
-			notBefore, expectedStartDate)
-	}
-	if notAfter != expectedEndDate {
-		t.Errorf("preCert notAfter was %q, expected %q",
-			notAfter, expectedEndDate)
+	type testCase struct {
+		name        string
+		windowStart time.Time
+		windowEnd   time.Time
 	}
-	// Check that the cert notbefore/notafter match expected
-	notBefore = shortFormat(certPair.Cert.NotBefore)
-	notAfter = shortFormat(certPair.Cert.NotAfter)
-	if notBefore != expectedStartDate {
-		t.Errorf("cert notBefore was %q, expected %q",
-			notBefore, expectedStartDate)
+	testCases := []testCase{
+		{
+			name:        "current temporal shard",
+			windowStart: currentWindowStart,
+			windowEnd:   currentWindowEnd,
+		},
+		{
+			name:        "past temporal shard",
+			windowStart: pastWindowStart,
+			windowEnd:   pastWindowEnd,
+		},
+		{
+			name:        "future temporal shard",
+			windowStart: futureWindowStart,
+			windowEnd:   futureWindowEnd,
+		},
 	}
-	if notAfter != expectedEndDate {
-		t.Errorf("cert notAfter was %q, expected %q",
-			notAfter, expectedEndDate)
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			// Issue a cert pair with specific WindowStart and WindowEnd
+			certPair, err := IssueTestCertificate("", issuerKey, issuerCert, clk, &tc.windowStart, &tc.windowEnd)
+			if err != nil {
+				t.Fatalf("unexpected error from IssueTestCertificate: %s", err.Error())
+			}
+
+			if certPair.PreCert == nil {
+				t.Fatalf("unexpected nil PreCert in CertPair returned from IssueTestCertificate")
+			}
+
+			if certPair.Cert == nil {
+				t.Fatalf("unexpected nil Cert in CertPair returned from IssueTestCertificate")
+			}
+
+			shortFormat := func(t time.Time) string {
+				return t.Format("2006-01-02 15:04:05")
+			}
+
+			// Check the precert notbefore/notafter match expected
+			notAfter := shortFormat(certPair.PreCert.NotAfter)
+			expectedNotAfter := shortFormat(certPair.PreCert.NotBefore.Add(validityPeriod))
+			if notAfter != expectedNotAfter {
+				t.Errorf("preCert notAfter was %q, expected %q",
+					notAfter, expectedNotAfter)
+			}
+			if certPair.PreCert.NotAfter.Before(tc.windowStart) || certPair.PreCert.NotAfter.After(tc.windowEnd) {
+				t.Errorf("preCert notAfter was %q, expected to be between %q and %q",
+					notAfter, tc.windowStart, tc.windowEnd)
+			}
+
+			// Check that the cert notbefore/notafter match expected
+			notAfter = shortFormat(certPair.Cert.NotAfter)
+			expectedNotAfter = shortFormat(certPair.Cert.NotBefore.Add(validityPeriod))
+			if notAfter != expectedNotAfter {
+				t.Errorf("Cert notAfter was %q, expected %q",
+					notAfter, expectedNotAfter)
+			}
+			if certPair.Cert.NotAfter.Before(tc.windowStart) || certPair.Cert.NotAfter.After(tc.windowEnd) {
+				t.Errorf("cert notAfter was %q, expected to be between %q and %q",
+					notAfter, tc.windowStart, tc.windowEnd)
+			}
+		})
 	}
 }
 

test/cttestsrv/log.go

@@ -11,7 +11,6 @@ import (
 	"time"
 
 	ct "github.com/google/certificate-transparency-go"
-	"github.com/google/certificate-transparency-go/tls"
 	cttls "github.com/google/certificate-transparency-go/tls"
 	ctfe "github.com/google/certificate-transparency-go/trillian/ctfe"
 	"github.com/google/certificate-transparency-go/trillian/util"
@@ -99,7 +98,7 @@ func makeTree(name string, _ *ecdsa.PrivateKey) (*testTree, error) {
 
 	// initialize the tree with an empty STH
 	if err := initSTH(tt); err != nil {
-		return nil, fmt.Errorf("initalizing STH: %w", err)
+		return nil, fmt.Errorf("initializing STH: %w", err)
 	}
 
 	return tt, nil
@@ -111,14 +110,14 @@ func initSTH(tt *testTree) error {
 
 	// init the new tree by signing a STH for the empty root
 	slr := types.LogRoot{
-		Version: tls.Enum(trillian.LogRootFormat_LOG_ROOT_FORMAT_V1),
+		Version: cttls.Enum(trillian.LogRootFormat_LOG_ROOT_FORMAT_V1),
 		V1: &types.LogRootV1{
 			TreeSize:       0,
 			RootHash:       emptyRootHash[:],
 			TimestampNanos: uint64(timeSource.Now().UnixNano()),
 		},
 	}
-	slrBytes, err := tls.Marshal(slr)
+	slrBytes, err := cttls.Marshal(slr)
 	if err != nil {
 		return err
 	}
@@ -239,7 +238,7 @@ func (tl *testLog) getSTH() (*ct.SignedTreeHead, error) {
 	}
 
 	var slr ct.SignedTreeHead
-	_, err = tls.Unmarshal(signedLogRoot.LogRoot, &slr)
+	_, err = cttls.Unmarshal(signedLogRoot.LogRoot, &slr)
 	if err != nil {
 		return nil, err
 	}