Find a new home for some functions (#245)

* move entryFromChain to x509util

# Conflicts:
#	internal/hammer/loadtest/workers.go

# Conflicts:
#	internal/scti/handlers.go

* move verifyAddChain

* convert functions to methods

Author: phbnf

internal/hammer/loadtest/workers.go

@@ -16,7 +16,6 @@ package loadtest
 
 import (
 	"context"
-	"crypto/sha256"
 	"crypto/x509"
 	"encoding/base64"
 	"encoding/json"
@@ -32,7 +31,6 @@ import (
 	"github.com/transparency-dev/static-ct/internal/types/rfc6962"
 	"github.com/transparency-dev/static-ct/internal/x509util"
 	"github.com/transparency-dev/trillian-tessera/api/layout"
-	"github.com/transparency-dev/trillian-tessera/ctonly"
 	"k8s.io/klog/v2"
 )
 
@@ -362,7 +360,7 @@ func (v *MMDVerifier) Run(ctx context.Context) {
 			v.errChan <- fmt.Errorf("failed to parse certificates: %v", err)
 			continue
 		}
-		entry, err := entryFromChain(certs, false, leafMMD.timestamp)
+		entry, err := x509util.EntryFromChain(certs, false, leafMMD.timestamp)
 		if err != nil {
 			v.errChan <- fmt.Errorf("failed to create entry from chain: %v", err)
 			continue
@@ -384,78 +382,3 @@ func (v *MMDVerifier) Kill() {
 		v.cancel()
 	}
 }
-
-// entryFromChain generates an Entry from a chain and timestamp.
-// Copied from certificate-transparency-go/serialization.go and internal/scti/handlers.go.
-// TODO(phboneff): move in a different file maybe?
-func entryFromChain(chain []*x509.Certificate, isPrecert bool, timestamp uint64) (*ctonly.Entry, error) {
-	leaf := ctonly.Entry{
-		IsPrecert: isPrecert,
-		Timestamp: timestamp,
-	}
-
-	if len(chain) > 1 {
-		issuersChain := make([][32]byte, len(chain)-1)
-		for i, c := range chain[1:] {
-			issuersChain[i] = sha256.Sum256(c.Raw)
-		}
-		leaf.FingerprintsChain = issuersChain
-	}
-
-	if !isPrecert {
-		leaf.Certificate = chain[0].Raw
-		return &leaf, nil
-	}
-
-	// Pre-certs are more complicated. First, parse the leaf pre-cert and its
-	// putative issuer.
-	if len(chain) < 2 {
-		return nil, fmt.Errorf("no issuer cert available for precert leaf building")
-	}
-	issuer := chain[1]
-	cert := chain[0]
-
-	var preIssuer *x509.Certificate
-	if isPreIssuer(issuer) {
-		// Replace the cert's issuance information with details from the pre-issuer.
-		preIssuer = issuer
-
-		// The issuer of the pre-cert is not going to be the issuer of the final
-		// cert.  Change to use the final issuer's key hash.
-		if len(chain) < 3 {
-			return nil, fmt.Errorf("no issuer cert available for pre-issuer")
-		}
-		issuer = chain[2]
-	}
-
-	// Next, post-process the DER-encoded TBSCertificate, to remove the CT poison
-	// extension and possibly update the issuer field.
-	defangedTBS, err := x509util.BuildPrecertTBS(cert.RawTBSCertificate, preIssuer)
-	if err != nil {
-		return nil, fmt.Errorf("failed to remove poison extension: %v", err)
-	}
-
-	leaf.Precertificate = cert.Raw
-	// TODO(phboneff): do we need this?
-	// leaf.PrecertSigningCert = issuer.Raw
-	leaf.Certificate = defangedTBS
-
-	issuerKeyHash := sha256.Sum256(issuer.RawSubjectPublicKeyInfo)
-	leaf.IssuerKeyHash = issuerKeyHash[:]
-	return &leaf, nil
-}
-
-// isPreIssuer indicates whether a certificate is a pre-cert issuer with the specific
-// certificate transparency extended key usage.
-// Copied from certificate-transparency-go/serialization.go and internal/scti/handlers.go.
-// TODO(phboneff): unify these.
-func isPreIssuer(cert *x509.Certificate) bool {
-	// Look for the extension in the Extensions field and not ExtKeyUsage
-	// since crypto/x509 does not recognize this extension as an ExtKeyUsage.
-	for _, ext := range cert.Extensions {
-		if rfc6962.OIDExtKeyUsageCertificateTransparency.Equal(ext.Id) {
-			return true
-		}
-	}
-	return false
-}

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)
@@ -263,6 +263,36 @@ func validateChain(rawChain [][]byte, validationOpts ChainValidationOpts) ([]*x5
 	return nil, errors.New("no RFC compliant path to root found when trying to validate chain")
 }
 
+// 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 (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 := 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
+		return nil, fmt.Errorf("chain failed to verify: %s", err)
+	}
+
+	isPrecert, err := isPrecertificate(validPath[0])
+	if err != nil {
+		return nil, fmt.Errorf("precert test failed: %s", err)
+	}
+
+	// The type of the leaf must match the one the handler expects
+	if isPrecert != expectingPrecert {
+		if expectingPrecert {
+			klog.Warningf("Cert (or precert with invalid CT ext) submitted as precert chain: %q", req.Chain)
+		} else {
+			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)
+	}
+
+	return validPath, nil
+}
+
 func chainsEquivalent(inChain []*x509.Certificate, verifiedChain []*x509.Certificate) bool {
 	// The verified chain includes a root, but the input chain may or may not include a
 	// root (RFC 6962 s4.1/ s4.2 "the last [certificate] is either the root certificate

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)
 			}