convert functions to methods

Author: phbnf

internal/scti/chain_validation.go

@@ -149,7 +149,7 @@ func isPrecertificate(cert *x509.Certificate) (bool, error) {
 // supplied in the chain. Then applies the RFC requirement that the path must involve all
 // the submitted chain in the order of submission.
 // TODO(phboneff): make this a method func([][]byte) ([]*x509.Certificate, error)
-func validateChain(rawChain [][]byte, validationOpts ChainValidationOpts) ([]*x509.Certificate, error) {
+func (opts ChainValidationOpts) validateChain(rawChain [][]byte) ([]*x509.Certificate, error) {
 	if len(rawChain) == 0 {
 		return nil, errors.New("empty certificate chain")
 	}
@@ -172,8 +172,8 @@ func validateChain(rawChain [][]byte, validationOpts ChainValidationOpts) ([]*x5
 		}
 	}
 
-	naStart := validationOpts.notAfterStart
-	naLimit := validationOpts.notAfterLimit
+	naStart := opts.notAfterStart
+	naLimit := opts.notAfterLimit
 	cert := chain[0]
 
 	// Check whether the expiry date of the cert is within the acceptable range.
@@ -184,24 +184,24 @@ func validateChain(rawChain [][]byte, validationOpts ChainValidationOpts) ([]*x5
 		return nil, fmt.Errorf("certificate NotAfter (%v) >= %v", cert.NotAfter, *naLimit)
 	}
 
-	now := validationOpts.currentTime
+	now := opts.currentTime
 	if now.IsZero() {
 		now = time.Now()
 	}
 	expired := now.After(cert.NotAfter)
-	if validationOpts.rejectExpired && expired {
+	if opts.rejectExpired && expired {
 		return nil, errors.New("rejecting expired certificate")
 	}
-	if validationOpts.rejectUnexpired && !expired {
+	if opts.rejectUnexpired && !expired {
 		return nil, errors.New("rejecting unexpired certificate")
 	}
 
 	// Check for unwanted extension types, if required.
 	// TODO(al): Refactor CertValidationOpts c'tor to a builder pattern and
 	// pre-calc this in there
-	if len(validationOpts.rejectExtIds) != 0 {
+	if len(opts.rejectExtIds) != 0 {
 		badIDs := make(map[string]bool)
-		for _, id := range validationOpts.rejectExtIds {
+		for _, id := range opts.rejectExtIds {
 			badIDs[id.String()] = true
 		}
 		for idx, ext := range cert.Extensions {
@@ -214,9 +214,9 @@ func validateChain(rawChain [][]byte, validationOpts ChainValidationOpts) ([]*x5
 
 	// TODO(al): Refactor CertValidationOpts c'tor to a builder pattern and
 	// pre-calc this in there too.
-	if len(validationOpts.extKeyUsages) > 0 {
+	if len(opts.extKeyUsages) > 0 {
 		acceptEKUs := make(map[x509.ExtKeyUsage]bool)
-		for _, eku := range validationOpts.extKeyUsages {
+		for _, eku := range opts.extKeyUsages {
 			acceptEKUs[eku] = true
 		}
 		good := false
@@ -227,7 +227,7 @@ func validateChain(rawChain [][]byte, validationOpts ChainValidationOpts) ([]*x5
 			}
 		}
 		if !good {
-			return nil, fmt.Errorf("rejecting certificate without EKU in %v", validationOpts.extKeyUsages)
+			return nil, fmt.Errorf("rejecting certificate without EKU in %v", opts.extKeyUsages)
 		}
 	}
 
@@ -237,9 +237,9 @@ func validateChain(rawChain [][]byte, validationOpts ChainValidationOpts) ([]*x5
 	//  - allow certificate without policing them since this is not CT's responsibility
 	// See /internal/lax509/README.md for further information.
 	verifyOpts := lax509.VerifyOptions{
-		Roots:         validationOpts.trustedRoots.CertPool(),
+		Roots:         opts.trustedRoots.CertPool(),
 		Intermediates: intermediatePool.CertPool(),
-		KeyUsages:     validationOpts.extKeyUsages,
+		KeyUsages:     opts.extKeyUsages,
 	}
 
 	verifiedChains, err := lax509.Verify(cert, verifyOpts)
@@ -266,9 +266,9 @@ func validateChain(rawChain [][]byte, validationOpts ChainValidationOpts) ([]*x5
 // verifyAddChain is used by add-chain and add-pre-chain. It does the checks that the supplied
 // cert is of the correct type and chains to a trusted root.
 // TODO(phbnf): add tests
-func verifyAddChain(log *log, req rfc6962.AddChainRequest, expectingPrecert bool) ([]*x509.Certificate, error) {
+func (opts ChainValidationOpts) verifyAddChain(req rfc6962.AddChainRequest, expectingPrecert bool) ([]*x509.Certificate, error) {
 	// We already checked that the chain is not empty so can move on to verification
-	validPath, err := validateChain(req.Chain, log.chainValidationOpts)
+	validPath, err := opts.validateChain(req.Chain)
 	if err != nil {
 		// We rejected it because the cert failed checks or we could not find a path to a root etc.
 		// Lots of possible causes for errors
@@ -283,9 +283,9 @@ func verifyAddChain(log *log, req rfc6962.AddChainRequest, expectingPrecert bool
 	// The type of the leaf must match the one the handler expects
 	if isPrecert != expectingPrecert {
 		if expectingPrecert {
-			klog.Warningf("%s: Cert (or precert with invalid CT ext) submitted as precert chain: %q", log.origin, req.Chain)
+			klog.Warningf("Cert (or precert with invalid CT ext) submitted as precert chain: %q", req.Chain)
 		} else {
-			klog.Warningf("%s: Precert (or cert with invalid CT ext) submitted as cert chain: %q", log.origin, req.Chain)
+			klog.Warningf("Precert (or cert with invalid CT ext) submitted as cert chain: %q", req.Chain)
 		}
 		return nil, fmt.Errorf("cert / precert mismatch: %T", expectingPrecert)
 	}

internal/scti/chain_validation_test.go

@@ -254,7 +254,7 @@ func TestValidateChain(t *testing.T) {
 	if !fakeCARoots.AppendCertsFromPEM([]byte(testdata.RealPrecertIntermediatePEM)) {
 		t.Fatal("failed to load real intermediate")
 	}
-	validateOpts := ChainValidationOpts{
+	opts := ChainValidationOpts{
 		trustedRoots: fakeCARoots,
 	}
 
@@ -403,11 +403,11 @@ func TestValidateChain(t *testing.T) {
 	}
 	for _, test := range tests {
 		t.Run(test.desc, func(t *testing.T) {
-			validateOpts := validateOpts
+			opts := opts
 			if test.modifyOpts != nil {
-				test.modifyOpts(&validateOpts)
+				test.modifyOpts(&opts)
 			}
-			gotPath, err := validateChain(test.chain, validateOpts)
+			gotPath, err := opts.validateChain(test.chain)
 			if err != nil {
 				if !test.wantErr {
 					t.Errorf("ValidateChain()=%v,%v; want _,nil", gotPath, err)
@@ -433,7 +433,7 @@ func TestNotAfterRange(t *testing.T) {
 	if !fakeCARoots.AppendCertsFromPEM([]byte(testdata.FakeCACertPEM)) {
 		t.Fatal("failed to load fake root")
 	}
-	validateOpts := ChainValidationOpts{
+	opts := ChainValidationOpts{
 		trustedRoots:  fakeCARoots,
 		rejectExpired: false,
 	}
@@ -473,12 +473,12 @@ func TestNotAfterRange(t *testing.T) {
 	for _, test := range tests {
 		t.Run(test.desc, func(t *testing.T) {
 			if !test.notAfterStart.IsZero() {
-				validateOpts.notAfterStart = &test.notAfterStart
+				opts.notAfterStart = &test.notAfterStart
 			}
 			if !test.notAfterLimit.IsZero() {
-				validateOpts.notAfterLimit = &test.notAfterLimit
+				opts.notAfterLimit = &test.notAfterLimit
 			}
-			gotPath, err := validateChain(test.chain, validateOpts)
+			gotPath, err := opts.validateChain(test.chain)
 			if err != nil {
 				if !test.wantErr {
 					t.Errorf("ValidateChain()=%v,%v; want _,nil", gotPath, err)
@@ -500,7 +500,7 @@ func TestRejectExpiredUnexpired(t *testing.T) {
 	}
 	// Validity period: May 13, 2016 - Jul 12, 2019.
 	chain := pemsToDERChain(t, []string{testdata.LeafSignedByFakeIntermediateCertPEM, testdata.FakeIntermediateCertPEM})
-	validateOpts := ChainValidationOpts{
+	opts := ChainValidationOpts{
 		trustedRoots: fakeCARoots,
 		extKeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
 	}
@@ -587,10 +587,10 @@ func TestRejectExpiredUnexpired(t *testing.T) {
 		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
-			validateOpts.currentTime = tc.now
-			validateOpts.rejectExpired = tc.rejectExpired
-			validateOpts.rejectUnexpired = tc.rejectUnexpired
-			_, err := validateChain(chain, validateOpts)
+			opts.currentTime = tc.now
+			opts.rejectExpired = tc.rejectExpired
+			opts.rejectUnexpired = tc.rejectUnexpired
+			_, err := opts.validateChain(chain)
 			if err != nil {
 				if len(tc.wantErr) == 0 {
 					t.Errorf("ValidateChain()=_,%v; want _,nil", err)
@@ -692,7 +692,7 @@ func TestPreIssuedCert(t *testing.T) {
 				trustedRoots: roots,
 				extKeyUsages: tc.eku,
 			}
-			chain, err := validateChain(rawChain, opts)
+			chain, err := opts.validateChain(rawChain)
 			if err != nil {
 				t.Fatalf("failed to ValidateChain: %v", err)
 			}

internal/scti/handlers.go

@@ -265,7 +265,7 @@ func addChainInternal(ctx context.Context, opts *HandlerOptions, log *log, w htt
 	for _, der := range addChainReq.Chain {
 		opts.RequestLog.addDERToChain(ctx, der)
 	}
-	chain, err := verifyAddChain(log, addChainReq, isPrecert)
+	chain, err := log.chainValidationOpts.verifyAddChain(addChainReq, isPrecert)
 	if err != nil {
 		return http.StatusBadRequest, fmt.Errorf("failed to verify add-chain contents: %s", err)
 	}