Clean CertValidationOptions (#94)

* move CertValidation to cert_checker.go * move cert_checker.go to chain_validation.go * s/certValidationOptions/chainValidationOptions
transparency-dev · Jan 22, 2025 · c9c70e9 · c9c70e9
1 parent da8ae85
commit c9c70e9
Show file tree

Hide file tree

Showing 5 changed files with 47 additions and 49 deletions.
diff --git a/cert_checker.go → chain_validation.go b/cert_checker.go → chain_validation.go
@@ -29,6 +29,30 @@ var (
 	ErrNoRFCCompliantPathFound = errors.New("no RFC compliant path to root found when trying to validate chain")
 )
 
+// ChainValidationOpts contains various parameters for certificate chain validation
+type ChainValidationOpts struct {
+	// trustedRoots is a pool of certificates that defines the roots the CT log will accept
+	trustedRoots *x509util.PEMCertPool
+	// currentTime is the time used for checking a certificate's validity period
+	// against. If it's zero then time.Now() is used. Only for testing.
+	currentTime time.Time
+	// rejectExpired indicates that expired certificates will be rejected.
+	rejectExpired bool
+	// rejectUnexpired indicates that certificates that are currently valid or not yet valid will be rejected.
+	rejectUnexpired bool
+	// notAfterStart is the earliest notAfter date which will be accepted.
+	// nil means no lower bound on the accepted range.
+	notAfterStart *time.Time
+	// notAfterLimit defines the cut off point of notAfter dates - only notAfter
+	// dates strictly *before* notAfterLimit will be accepted.
+	// nil means no upper bound on the accepted range.
+	notAfterLimit *time.Time
+	// extKeyUsages contains the list of EKUs to use during chain verification
+	extKeyUsages []x509.ExtKeyUsage
+	// rejectExtIds contains a list of X.509 extension IDs to reject during chain verification.
+	rejectExtIds []asn1.ObjectIdentifier
+}
+
 // isPrecertificate tests if a certificate is a pre-certificate as defined in CT.
 // An error is returned if the CT extension is present but is not ASN.1 NULL as defined
 // by the spec.
@@ -51,7 +75,7 @@ func isPrecertificate(cert *x509.Certificate) (bool, error) {
 // end entity certificate in the chain to a trusted root cert, possibly using the intermediates
 // supplied in the chain. Then applies the RFC requirement that the path must involve all
 // the submitted chain in the order of submission.
-func validateChain(rawChain [][]byte, validationOpts CertValidationOpts) ([]*x509.Certificate, error) {
+func validateChain(rawChain [][]byte, validationOpts ChainValidationOpts) ([]*x509.Certificate, error) {
 	// First make sure the certs parse as X.509
 	chain := make([]*x509.Certificate, 0, len(rawChain))
 	intermediatePool := x509util.NewPEMCertPool()

diff --git a/cert_checker_test.go → chain_validation_test.go b/cert_checker_test.go → chain_validation_test.go
@@ -114,7 +114,7 @@ func TestValidateChain(t *testing.T) {
 	if !fakeCARoots.AppendCertsFromPEM([]byte(testdata.RealPrecertIntermediatePEM)) {
 		t.Fatal("failed to load real intermediate")
 	}
-	validateOpts := CertValidationOpts{
+	validateOpts := ChainValidationOpts{
 		trustedRoots: fakeCARoots,
 	}
 
@@ -123,7 +123,7 @@ func TestValidateChain(t *testing.T) {
 		chain       [][]byte
 		wantErr     bool
 		wantPathLen int
-		modifyOpts  func(v *CertValidationOpts)
+		modifyOpts  func(v *ChainValidationOpts)
 	}{
 		{
 			desc:    "missing-intermediate-cert",
@@ -183,7 +183,7 @@ func TestValidateChain(t *testing.T) {
 		{
 			desc:  "reject-non-existent-ext-id",
 			chain: pemsToDERChain(t, []string{testdata.LeafSignedByFakeIntermediateCertPEM, testdata.FakeIntermediateCertPEM}),
-			modifyOpts: func(v *CertValidationOpts) {
+			modifyOpts: func(v *ChainValidationOpts) {
 				// reject SubjectKeyIdentifier extension
 				v.rejectExtIds = []asn1.ObjectIdentifier{[]int{99, 99, 99, 99}}
 			},
@@ -192,7 +192,7 @@ func TestValidateChain(t *testing.T) {
 		{
 			desc:  "reject-non-existent-ext-id-precert",
 			chain: pemsToDERChain(t, []string{testdata.PrecertPEMValid}),
-			modifyOpts: func(v *CertValidationOpts) {
+			modifyOpts: func(v *ChainValidationOpts) {
 				// reject SubjectKeyIdentifier extension
 				v.rejectExtIds = []asn1.ObjectIdentifier{[]int{99, 99, 99, 99}}
 			},
@@ -202,7 +202,7 @@ func TestValidateChain(t *testing.T) {
 			desc:    "reject-ext-id",
 			chain:   pemsToDERChain(t, []string{testdata.LeafSignedByFakeIntermediateCertPEM, testdata.FakeIntermediateCertPEM}),
 			wantErr: true,
-			modifyOpts: func(v *CertValidationOpts) {
+			modifyOpts: func(v *ChainValidationOpts) {
 				// reject SubjectKeyIdentifier extension
 				v.rejectExtIds = []asn1.ObjectIdentifier{[]int{2, 5, 29, 14}}
 			},
@@ -211,7 +211,7 @@ func TestValidateChain(t *testing.T) {
 			desc:    "reject-ext-id-precert",
 			chain:   pemsToDERChain(t, []string{testdata.PrecertPEMValid}),
 			wantErr: true,
-			modifyOpts: func(v *CertValidationOpts) {
+			modifyOpts: func(v *ChainValidationOpts) {
 				// reject SubjectKeyIdentifier extension
 				v.rejectExtIds = []asn1.ObjectIdentifier{[]int{2, 5, 29, 14}}
 			},
@@ -220,7 +220,7 @@ func TestValidateChain(t *testing.T) {
 			desc:    "reject-eku-not-present-in-cert",
 			chain:   pemsToDERChain(t, []string{testdata.LeafSignedByFakeIntermediateCertPEM, testdata.FakeIntermediateCertPEM}),
 			wantErr: true,
-			modifyOpts: func(v *CertValidationOpts) {
+			modifyOpts: func(v *ChainValidationOpts) {
 				// reject cert without ExtKeyUsageEmailProtection
 				v.extKeyUsages = []x509.ExtKeyUsage{x509.ExtKeyUsageEmailProtection}
 			},
@@ -229,15 +229,15 @@ func TestValidateChain(t *testing.T) {
 			desc:        "allow-eku-present-in-cert",
 			chain:       pemsToDERChain(t, []string{testdata.LeafSignedByFakeIntermediateCertPEM, testdata.FakeIntermediateCertPEM}),
 			wantPathLen: 3,
-			modifyOpts: func(v *CertValidationOpts) {
+			modifyOpts: func(v *ChainValidationOpts) {
 				v.extKeyUsages = []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}
 			},
 		},
 		{
 			desc:    "reject-eku-not-present-in-precert",
 			chain:   pemsToDERChain(t, []string{testdata.RealPrecertWithEKUPEM}),
 			wantErr: true,
-			modifyOpts: func(v *CertValidationOpts) {
+			modifyOpts: func(v *ChainValidationOpts) {
 				// reject cert without ExtKeyUsageEmailProtection
 				v.extKeyUsages = []x509.ExtKeyUsage{x509.ExtKeyUsageEmailProtection}
 			},
@@ -246,7 +246,7 @@ func TestValidateChain(t *testing.T) {
 			desc:        "allow-eku-present-in-precert",
 			chain:       pemsToDERChain(t, []string{testdata.RealPrecertWithEKUPEM}),
 			wantPathLen: 2,
-			modifyOpts: func(v *CertValidationOpts) {
+			modifyOpts: func(v *ChainValidationOpts) {
 				v.extKeyUsages = []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}
 			},
 		},
@@ -283,7 +283,7 @@ func TestNotAfterRange(t *testing.T) {
 	if !fakeCARoots.AppendCertsFromPEM([]byte(testdata.FakeCACertPEM)) {
 		t.Fatal("failed to load fake root")
 	}
-	validateOpts := CertValidationOpts{
+	validateOpts := ChainValidationOpts{
 		trustedRoots:  fakeCARoots,
 		rejectExpired: false,
 	}
@@ -350,7 +350,7 @@ func TestRejectExpiredUnexpired(t *testing.T) {
 	}
 	// Validity period: May 13, 2016 - Jul 12, 2019.
 	chain := pemsToDERChain(t, []string{testdata.LeafSignedByFakeIntermediateCertPEM, testdata.FakeIntermediateCertPEM})
-	validateOpts := CertValidationOpts{
+	validateOpts := ChainValidationOpts{
 		trustedRoots: fakeCARoots,
 		extKeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
 	}
@@ -527,7 +527,7 @@ func TestCMPreIssuedCert(t *testing.T) {
 		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
-			opts := CertValidationOpts{
+			opts := ChainValidationOpts{
 				trustedRoots: cmRoot,
 				extKeyUsages: tc.eku,
 			}

diff --git a/ctlog.go b/ctlog.go
@@ -75,9 +75,9 @@ type log struct {
 	origin string
 	// signSCT Signs SCTs.
 	signSCT signSCT
-	// certValidationOpts contains various parameters for certificate chain
+	// chainValidationOpts contains various parameters for certificate chain
 	// validation.
-	certValidationOpts CertValidationOpts
+	chainValidationOpts ChainValidationOpts
 	// storage stores certificate data.
 	storage Storage
 }
@@ -108,7 +108,7 @@ func NewLog(ctx context.Context, origin string, signer crypto.Signer, cfg ChainV
 	if err != nil {
 		return nil, fmt.Errorf("invalid cert validation config: %v", err)
 	}
-	log.certValidationOpts = *vlc
+	log.chainValidationOpts = *vlc
 
 	cpSigner, err := NewCpSigner(signer, origin, ts)
 	if err != nil {
@@ -126,7 +126,7 @@ func NewLog(ctx context.Context, origin string, signer crypto.Signer, cfg ChainV
 
 // newCertValidationOpts checks that a chain validation config is valid,
 // parses it, and loads resources to validate chains.
-func newCertValidationOpts(cfg ChainValidationConfig) (*CertValidationOpts, error) {
+func newCertValidationOpts(cfg ChainValidationConfig) (*ChainValidationOpts, error) {
 	// Load the trusted roots.
 	if cfg.RootsPEMFile == "" {
 		return nil, errors.New("empty rootsPemFile")
@@ -145,7 +145,7 @@ func newCertValidationOpts(cfg ChainValidationConfig) (*CertValidationOpts, erro
 		return nil, fmt.Errorf("'Not After' limit %q before start %q", cfg.NotAfterLimit.Format(time.RFC3339), cfg.NotAfterStart.Format(time.RFC3339))
 	}
 
-	validationOpts := CertValidationOpts{
+	validationOpts := ChainValidationOpts{
 		trustedRoots:    roots,
 		rejectExpired:   cfg.RejectExpired,
 		rejectUnexpired: cfg.RejectUnexpired,

diff --git a/handlers.go b/handlers.go
@@ -28,10 +28,8 @@ import (
 	"sync"
 	"time"
 
-	"github.com/google/certificate-transparency-go/asn1"
 	"github.com/google/certificate-transparency-go/tls"
 	"github.com/google/certificate-transparency-go/x509"
-	"github.com/google/certificate-transparency-go/x509util"
 	"github.com/google/trillian/monitoring"
 	"github.com/transparency-dev/static-ct/modules/dedup"
 	tessera "github.com/transparency-dev/trillian-tessera"
@@ -151,30 +149,6 @@ func (a AppHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	}
 }
 
-// CertValidationOpts contains various parameters for certificate chain validation
-type CertValidationOpts struct {
-	// trustedRoots is a pool of certificates that defines the roots the CT log will accept
-	trustedRoots *x509util.PEMCertPool
-	// currentTime is the time used for checking a certificate's validity period
-	// against. If it's zero then time.Now() is used. Only for testing.
-	currentTime time.Time
-	// rejectExpired indicates that expired certificates will be rejected.
-	rejectExpired bool
-	// rejectUnexpired indicates that certificates that are currently valid or not yet valid will be rejected.
-	rejectUnexpired bool
-	// notAfterStart is the earliest notAfter date which will be accepted.
-	// nil means no lower bound on the accepted range.
-	notAfterStart *time.Time
-	// notAfterLimit defines the cut off point of notAfter dates - only notAfter
-	// dates strictly *before* notAfterLimit will be accepted.
-	// nil means no upper bound on the accepted range.
-	notAfterLimit *time.Time
-	// extKeyUsages contains the list of EKUs to use during chain verification
-	extKeyUsages []x509.ExtKeyUsage
-	// rejectExtIds contains a list of X.509 extension IDs to reject during chain verification.
-	rejectExtIds []asn1.ObjectIdentifier
-}
-
 // logInfo holds information for a specific log instance.
 type logInfo struct {
 	// Origin identifies the log, as per https://c2sp.org/static-ct-api
@@ -190,7 +164,7 @@ type logInfo struct {
 	// deadline is a timeout for HTTP requests.
 	deadline time.Duration
 	// validationOpts contains the certificate chain validation parameters.
-	validationOpts CertValidationOpts
+	validationOpts ChainValidationOpts
 	// storage stores log data.
 	storage Storage
 	// signSCT signs SCTs.
@@ -215,7 +189,7 @@ type HandlerOptions struct {
 // newLogInfo creates a new instance of logInfo.
 func newLogInfo(
 	hOpts HandlerOptions,
-	validationOpts CertValidationOpts,
+	validationOpts ChainValidationOpts,
 	signSCT signSCT,
 	timeSource TimeSource,
 	storage Storage,
@@ -240,7 +214,7 @@ func newLogInfo(
 
 func NewPathHandlers(opts *HandlerOptions, log *log) PathHandlers {
 	// TODO(phboneff): simplify signature of newLogInfo
-	logInfo := newLogInfo(*opts, log.certValidationOpts, log.signSCT, opts.TimeSource, log.storage, log.origin)
+	logInfo := newLogInfo(*opts, log.chainValidationOpts, log.signSCT, opts.TimeSource, log.storage, log.origin)
 	return logInfo.Handlers(log.origin)
 }
 

diff --git a/handlers_test.go b/handlers_test.go
@@ -71,7 +71,7 @@ func setupTest(t *testing.T, pemRoots []string, signer crypto.Signer) handlerTes
 	}
 
 	info.storage = mockstorage.NewMockStorage(info.mockCtrl)
-	vOpts := CertValidationOpts{
+	vOpts := ChainValidationOpts{
 		trustedRoots:  info.roots,
 		rejectExpired: false,
 	}