Migrate away from ctgo x509, asn1 and tls packages

ctlog.go

@@ -17,15 +17,15 @@
 import (
 	"context"
 	"crypto"
+	"crypto/x509"
+	"encoding/asn1"
 	"errors"
 	"fmt"
 	"net/http"
 	"strings"
 	"time"
 
-	"github.com/google/certificate-transparency-go/asn1"
-	"github.com/google/certificate-transparency-go/x509"
 	"github.com/transparency-dev/static-ct/internal/scti"
 	"github.com/transparency-dev/static-ct/internal/x509util"
 	"github.com/transparency-dev/static-ct/storage"
 )

internal/scti/chain_validation.go

@@ -16,14 +16,15 @@
 
 import (
 	"bytes"
+	"crypto/x509"
+	"encoding/asn1"
 	"errors"
 	"fmt"
 	"strconv"
 	"strings"
 	"time"
 
-	"github.com/google/certificate-transparency-go/asn1"
-	"github.com/google/certificate-transparency-go/x509"
+	"github.com/transparency-dev/static-ct/internal/types"
 	"github.com/transparency-dev/static-ct/internal/x509util"
 	"k8s.io/klog/v2"
 )
@@ -127,7 +128,7 @@
 // by the spec.
 func isPrecertificate(cert *x509.Certificate) (bool, error) {
 	for _, ext := range cert.Extensions {
-		if x509.OIDExtensionCTPoison.Equal(ext.Id) {
+		if types.OIDExtensionCTPoison.Equal(ext.Id) {
 			if !ext.Critical || !bytes.Equal(asn1.NullBytes, ext.Value) {
 				return false, fmt.Errorf("CT poison ext is not critical or invalid: %v", ext)
 			}
@@ -139,6 +140,66 @@
 	return false, nil
 }
 
+// getLaxVerifiedChain returns a verified certificate chain, allowing for specific
+// errors that are commonly raised with certificates submitted to CT logs.
+//
+// Allowed x509 errors:
+//
+//   - UnhandledCriticalExtension: Precertificates have the poison extension
+//     which the Go library code does not recognize; also the Go library code
+//     does not support the standard PolicyConstraints extension (which is
+//     required to be marked critical, RFC 5280 s4.2.1.11)
+//   - Expired: CT logs should be able to log expired certificates.
+//   - IncompatibleUsage: Pre-issued precertificates have the Certificate
+//     Transparency EKU, which intermediates don't have. Also some leaves have
+//     unknown EKUs that should not be bounced just because the intermediate
+//     does not also have them (cf.  https://github.com/golang/go/issues/24590)
+//     so disable EKU checks inside the x509 library, but we've already done our
+//     own check on the leaf above.
+//   - NoValidChains: Do no enforce policy validation.
+//   - TooManyIntermediates: path length checks get confused by the presence of
+//     an additional pre-issuer intermediate.
+//   - CANotAuthorizedForThisName: allow to log all certificates, even if they
+//     have been isued by a CA trhat is not auhotized to issue certs for a
+//     given domain.
+//
+// TODO(phboneff): this doesn't work because, as it should, cert.Verify()
+// does not return a chain when it raises an error.
+func getLaxVerifiedChain(cert *x509.Certificate, opts x509.VerifyOptions) ([][]*x509.Certificate, error) {
+	chains, err := cert.Verify(opts)
+	switch err.(type) {
+	// TODO(phboneff): check if we could make the x509 library aware of the CT
+	// poison.
+	// TODO(phboneff): re-evaluate whether PolicyConstraints is still an issue.
+	case x509.UnhandledCriticalExtension:
+		return chains, nil
+	case x509.CertificateInvalidError:
+		if e, ok := err.(x509.CertificateInvalidError); ok {
+			switch e.Reason {
+			// TODO(phboneff): if need be, change time to make sure that the cert is
+			// never considered as expired.
+			// TODO(phboneff): see if TooManyIntermediates handling could be improved
+			// upstream.
+			// TODO(phboneff): see if it's necessary to log certs for which
+			// CANotAuthorizedForThisName is raised. If browsers all check this
+			// as well, then there is no need to log these certs.
+			case x509.Expired, x509.TooManyIntermediates, x509.CANotAuthorizedForThisName:
+				return chains, nil
+			// TODO(phboneff): check if we can remove these two exceptions.
+			// NoValidChains was not a thing back when x509 was forked in ctgo.
+			// New CT logs should all filter incoming certs with EKU, and
+			// https://github.com/golang/go/issues/24590 has been updated,
+			// so we should be able to remove IncompatibleUsage as well.
+			case x509.IncompatibleUsage, x509.NoValidChains:
+				return chains, nil
+			default:
+				return chains, err
+			}
+		}
+	}
+	return chains, err
+}
+
 // validateChain takes the certificate chain as it was parsed from a JSON request. Ensures all
 // elements in the chain decode as X.509 certificates. Ensures that there is a valid path from the
 // end entity certificate in the chain to a trusted root cert, possibly using the intermediates
@@ -152,8 +213,8 @@
 
 	for i, certBytes := range rawChain {
 		cert, err := x509.ParseCertificate(certBytes)
-		if x509.IsFatal(err) {
-			return nil, err
+		if err != nil {
+			return nil, fmt.Errorf("x509.ParseCertificate(): %v", err)
 		}
 
 		chain = append(chain, cert)
@@ -223,32 +284,16 @@
 		}
 	}
 
-	// We can now do the verification.  Use fairly lax options for verification, as
+	// We can now do the verification. Use fairly lax options for verification, as
 	// CT is intended to observe certificates rather than police them.
 	verifyOpts := x509.VerifyOptions{
-		Roots:             validationOpts.trustedRoots.CertPool(),
-		CurrentTime:       now,
-		Intermediates:     intermediatePool.CertPool(),
-		DisableTimeChecks: true,
-		// Precertificates have the poison extension; also the Go library code does not
-		// support the standard PolicyConstraints extension (which is required to be marked
-		// critical, RFC 5280 s4.2.1.11), so never check unhandled critical extensions.
-		DisableCriticalExtensionChecks: true,
-		// Pre-issued precertificates have the Certificate Transparency EKU; also some
-		// leaves have unknown EKUs that should not be bounced just because the intermediate
-		// does not also have them (cf. https://github.com/golang/go/issues/24590) so
-		// disable EKU checks inside the x509 library, but we've already done our own check
-		// on the leaf above.
-		DisableEKUChecks: true,
-		// Path length checks get confused by the presence of an additional
-		// pre-issuer intermediate, so disable them.
-		DisablePathLenChecks:        true,
-		DisableNameConstraintChecks: true,
-		DisableNameChecks:           false,
-		KeyUsages:                   validationOpts.extKeyUsages,
+		Roots:         validationOpts.trustedRoots.CertPool(),
+		CurrentTime:   now,
+		Intermediates: intermediatePool.CertPool(),
+		KeyUsages:     validationOpts.extKeyUsages,
 	}
 
-	verifiedChains, err := cert.Verify(verifyOpts)
+	verifiedChains, err := getLaxVerifiedChain(cert, verifyOpts)
 	if err != nil {
 		return nil, err
 	}

internal/scti/chain_validation_test.go

@@ -15,16 +15,18 @@
 package scti
 
 import (
+	"crypto/md5"
+	"crypto/x509"
+	"crypto/x509/pkix"
+	"encoding/asn1"
 	"encoding/base64"
 	"encoding/pem"
 	"strings"
 	"testing"
 	"time"
 
-	"github.com/google/certificate-transparency-go/asn1"
-	"github.com/google/certificate-transparency-go/x509"
-	"github.com/google/certificate-transparency-go/x509/pkix"
 	"github.com/transparency-dev/static-ct/internal/testdata"
+	"github.com/transparency-dev/static-ct/internal/types"
 	"github.com/transparency-dev/static-ct/internal/x509util"
 )
 
@@ -35,13 +37,13 @@ func wipeExtensions(cert *x509.Certificate) *x509.Certificate {
 
 func makePoisonNonCritical(cert *x509.Certificate) *x509.Certificate {
 	// Invalid as a pre-cert because poison extension needs to be marked as critical.
-	cert.Extensions = []pkix.Extension{{Id: x509.OIDExtensionCTPoison, Critical: false, Value: asn1.NullBytes}}
+	cert.Extensions = []pkix.Extension{{Id: types.OIDExtensionCTPoison, Critical: false, Value: asn1.NullBytes}}
 	return cert
 }
 
 func makePoisonNonNull(cert *x509.Certificate) *x509.Certificate {
 	// Invalid as a pre-cert because poison extension is not ASN.1 NULL value.
-	cert.Extensions = []pkix.Extension{{Id: x509.OIDExtensionCTPoison, Critical: false, Value: []byte{0x42, 0x42, 0x42}}}
+	cert.Extensions = []pkix.Extension{{Id: types.OIDExtensionCTPoison, Critical: false, Value: []byte{0x42, 0x42, 0x42}}}
 	return cert
 }
 
@@ -271,7 +273,7 @@ func TestValidateChain(t *testing.T) {
 			if len(gotPath) != test.wantPathLen {
 				t.Errorf("|ValidateChain()|=%d; want %d", len(gotPath), test.wantPathLen)
 				for _, c := range gotPath {
-					t.Logf("Subject: %s Issuer: %s", x509util.NameToString(c.Subject), x509util.NameToString(c.Issuer))
+					t.Logf("Subject: %s Issuer: %s", c.Subject, c.Issuer)
 				}
 			}
 		})
@@ -475,8 +477,8 @@ func pemToCert(t *testing.T, pemData string) *x509.Certificate {
 	}
 
 	cert, err := x509.ParseCertificate(bytes.Bytes)
-	if x509.IsFatal(err) {
-		t.Fatal(err)
+	if err != nil {
+		t.Fatalf("x509.ParseCertificate(): %v", err)
 	}
 
 	return cert
@@ -536,7 +538,7 @@ func TestCMPreIssuedCert(t *testing.T) {
 				t.Fatalf("failed to ValidateChain: %v", err)
 			}
 			for i, c := range chain {
-				t.Logf("chain[%d] = \n%s", i, x509util.CertificateToString(c))
+				t.Logf("chain[%d] = \n%s", i, md5.Sum(c.Raw))
 			}
 		})
 	}

internal/scti/ctlog.go

@@ -4,10 +4,10 @@ import (
 	"context"
 	"crypto"
 	"crypto/ecdsa"
+	"crypto/x509"
 	"errors"
 	"fmt"
 
-	"github.com/google/certificate-transparency-go/x509"
 	"github.com/transparency-dev/static-ct/internal/types"
 	"github.com/transparency-dev/static-ct/modules/dedup"
 	"github.com/transparency-dev/static-ct/storage"

internal/scti/handlers.go

@@ -17,6 +17,7 @@ package scti
 import (
 	"context"
 	"crypto/sha256"
+	"crypto/x509"
 	"encoding/base64"
 	"encoding/json"
 	"errors"
@@ -29,10 +30,10 @@ import (
 	"time"
 
 	"github.com/google/certificate-transparency-go/tls"
-	"github.com/google/certificate-transparency-go/x509"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promauto"
 	"github.com/transparency-dev/static-ct/internal/types"
+	"github.com/transparency-dev/static-ct/internal/x509util"
 	"github.com/transparency-dev/static-ct/modules/dedup"
 	tessera "github.com/transparency-dev/trillian-tessera"
 	"github.com/transparency-dev/trillian-tessera/ctonly"
@@ -491,7 +492,7 @@ func entryFromChain(chain []*x509.Certificate, isPrecert bool, timestamp uint64)
 
 	// Next, post-process the DER-encoded TBSCertificate, to remove the CT poison
 	// extension and possibly update the issuer field.
-	defangedTBS, err := x509.BuildPrecertTBS(cert.RawTBSCertificate, preIssuer)
+	defangedTBS, err := x509util.BuildPrecertTBS(cert.RawTBSCertificate, preIssuer)
 	if err != nil {
 		return nil, fmt.Errorf("failed to remove poison extension: %v", err)
 	}
@@ -508,10 +509,9 @@ func entryFromChain(chain []*x509.Certificate, isPrecert bool, timestamp uint64)
 
 // isPreIssuer indicates whether a certificate is a pre-cert issuer with the specific
 // certificate transparency extended key usage.
-// copied form certificate-transparency-go/serialization.go
-func isPreIssuer(issuer *x509.Certificate) bool {
-	for _, eku := range issuer.ExtKeyUsage {
-		if eku == x509.ExtKeyUsageCertificateTransparency {
+func isPreIssuer(cert *x509.Certificate) bool {
+	for _, ext := range cert.Extensions {
+		if types.OIDExtKeyUsageCertificateTransparency.Equal(ext.Id) {
 			return true
 		}
 	}

internal/scti/handlers_test.go

@@ -19,6 +19,7 @@ import (
 	"bytes"
 	"context"
 	"crypto"
+	"crypto/x509"
 	"encoding/hex"
 	"encoding/json"
 	"fmt"
@@ -30,7 +31,6 @@ import (
 	"time"
 
 	"github.com/golang/mock/gomock"
-	"github.com/google/certificate-transparency-go/x509"
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/transparency-dev/static-ct/internal/testdata"

internal/scti/requestlog.go

@@ -16,11 +16,10 @@ package scti
 
 import (
 	"context"
+	"crypto/x509"
 	"encoding/hex"
 	"time"
 
-	"github.com/google/certificate-transparency-go/x509"
-	"github.com/google/certificate-transparency-go/x509util"
 	"k8s.io/klog/v2"
 )
 
@@ -86,8 +85,8 @@ func (dlr *DefaultRequestLog) addDERToChain(_ context.Context, d []byte) {
 // certificate that is part of a submitted chain.
 func (dlr *DefaultRequestLog) addCertToChain(_ context.Context, cert *x509.Certificate) {
 	klog.V(vLevel).Infof("RL: Cert: Sub: %s Iss: %s notBef: %s notAft: %s",
-		x509util.NameToString(cert.Subject),
-		x509util.NameToString(cert.Issuer),
+		cert.Subject,
+		cert.Issuer,
 		cert.NotBefore.Format(time.RFC1123Z),
 		cert.NotAfter.Format(time.RFC1123Z))
 }

internal/scti/signatures.go

@@ -18,12 +18,12 @@ import (
 	"crypto"
 	"crypto/rand"
 	"crypto/sha256"
+	"crypto/x509"
 	"encoding/binary"
 	"fmt"
 	"time"
 
 	"github.com/google/certificate-transparency-go/tls"
-	"github.com/google/certificate-transparency-go/x509"
 	tfl "github.com/transparency-dev/formats/log"
 	"github.com/transparency-dev/static-ct/internal/types"
 	"golang.org/x/mod/sumdb/note"

internal/scti/signatures_test.go

@@ -18,13 +18,13 @@ import (
 	"bytes"
 	"crypto"
 	"crypto/sha256"
+	"crypto/x509"
 	"encoding/hex"
 	"encoding/pem"
 	"testing"
 	"time"
 
 	"github.com/google/certificate-transparency-go/tls"
-	"github.com/google/certificate-transparency-go/x509"
 	"github.com/kylelemons/godebug/pretty"
 	"github.com/transparency-dev/static-ct/internal/testdata"
 	"github.com/transparency-dev/static-ct/internal/types"
@@ -245,7 +245,7 @@ func TestSerializeV1STHSignatureKAT(t *testing.T) {
 
 func TestBuildV1MerkleTreeLeafForCert(t *testing.T) {
 	cert, err := x509util.CertificateFromPEM([]byte(testdata.LeafSignedByFakeIntermediateCertPEM))
-	if x509.IsFatal(err) {
+	if err != nil {
 		t.Fatalf("failed to set up test cert: %v", err)
 	}
 
@@ -308,7 +308,7 @@ func TestBuildV1MerkleTreeLeafForCert(t *testing.T) {
 
 func TestSignV1SCTForPrecertificate(t *testing.T) {
 	cert, err := x509util.CertificateFromPEM([]byte(testdata.PrecertPEMValid))
-	if x509.IsFatal(err) {
+	if err != nil {
 		t.Fatalf("failed to set up test precert: %v", err)
 	}
 
@@ -367,7 +367,7 @@ func TestSignV1SCTForPrecertificate(t *testing.T) {
 	if got, want := keyHash[:], leaf.TimestampedEntry.PrecertEntry.IssuerKeyHash[:]; !bytes.Equal(got, want) {
 		t.Fatalf("Issuer key hash bytes mismatch, got %v, expected %v", got, want)
 	}
-	defangedTBS, _ := x509.RemoveCTPoison(cert.RawTBSCertificate)
+	defangedTBS, _ := x509util.RemoveCTPoison(cert.RawTBSCertificate)
 	if got, want := leaf.TimestampedEntry.PrecertEntry.TBSCertificate, defangedTBS; !bytes.Equal(got, want) {
 		t.Fatalf("TBS cert mismatch, got %v, expected %v", got, want)
 	}