cryptutil: update CRL parsing (#4454)

Move the parseCRLs() method from package 'authorize/evaluator' to
'pkg/cryptutil', replacing the existing DecodeCRL() method. This method
will parse all CRLs found in the PEM input, rather than just the first.

(This removes our usage of the deprecated method x509.ParseDERCRL.)

Update this method to return an error if there is non-PEM data found in
the input, to satisfy the existing test that raw DER-encoded CRLs are
not permitted.

Delete the CRLFromBase64() and CRLFromFile() methods, as these are no
longer used.

authorize/evaluator/functions.go

@@ -11,6 +11,7 @@ import (
 	lru "github.com/hashicorp/golang-lru/v2"
 
 	"github.com/pomerium/pomerium/internal/log"
+	"github.com/pomerium/pomerium/pkg/cryptutil"
 )
 
 // ClientCertConstraints contains additional constraints to validate when
@@ -61,7 +62,7 @@ func isValidClientCertificate(
 		return false, err
 	}
 
-	crls, err := parseCRLs([]byte(crl))
+	crls, err := cryptutil.ParseCRLs([]byte(crl))
 	if err != nil {
 		return false, err
 	}
@@ -168,22 +169,3 @@ func parseCertificate(pemStr string) (*x509.Certificate, error) {
 	}
 	return x509.ParseCertificate(block.Bytes)
 }
-
-func parseCRLs(crl []byte) (map[string]*x509.RevocationList, error) {
-	m := make(map[string]*x509.RevocationList)
-	for {
-		var block *pem.Block
-		block, crl = pem.Decode(crl)
-		if block == nil {
-			return m, nil
-		}
-		if block.Type != "X509 CRL" {
-			continue
-		}
-		l, err := x509.ParseRevocationList(block.Bytes)
-		if err != nil {
-			return nil, err
-		}
-		m[string(l.RawIssuer)] = l
-	}
-}

config/mtls.go

@@ -128,12 +128,9 @@ func (s *DownstreamMTLSSettings) validate() error {
 	crl, err := s.GetCRL()
 	if err != nil {
 		return err
-	}
+	} else if _, err := cryptutil.ParseCRLs(crl); err != nil {
-	if len(crl) > 0 {
-		if _, err := cryptutil.DecodeCRL(crl); err != nil {
 		return fmt.Errorf("CRL: %w", err)
 	}
-	}
 
 	switch s.Enforcement {
 	case "",

config/mtls_test.go

@@ -131,7 +131,7 @@ func TestDownstreamMTLSSettingsValidate(t *testing.T) {
 		{"both CRL and CRL file", DownstreamMTLSSettings{CRL: "CRL", CRLFile: "CRLFile"},
 			"cannot set both crl and crl_file"},
 		{"bad CRL", DownstreamMTLSSettings{CRL: "dGhpc2lzZmluZQo="},
-			"CRL: cryptutil: invalid crl, no X509 CRL block found"},
+			"CRL: cryptutil: non-PEM data in CRL bundle"},
 		{"bad CRL file", DownstreamMTLSSettings{CRLFile: "-"},
 			"CRL file: open -: no such file or directory"},
 		{"bad enforcement mode", DownstreamMTLSSettings{Enforcement: "whatever"},

pkg/cryptutil/certificates.go

@@ -4,7 +4,6 @@ import (
 	"crypto/ecdsa"
 	"crypto/tls"
 	"crypto/x509"
-	"crypto/x509/pkix"
 	"encoding/base64"
 	"encoding/pem"
 	"errors"
@@ -16,7 +15,6 @@ import (
 )
 
 const (
-	crlPemType      = "X509 CRL"
 	maxCertFileSize = 1 << 16
 )
 
@@ -41,44 +39,29 @@ func CertificateFromFile(certFile, keyFile string) (*tls.Certificate, error) {
 	return &cert, err
 }
 
-// CRLFromBase64 parses a certificate revocation list from a base64 encoded blob.
+// ParseCRLs parses PEM-encoded certificate revocation lists, returning a map
-func CRLFromBase64(rawCRL string) (*pkix.CertificateList, error) {
+// of the parsed CRLs keyed by the raw issuer name.
-	bs, err := base64.StdEncoding.DecodeString(rawCRL)
+func ParseCRLs(crl []byte) (map[string]*x509.RevocationList, error) {
-	if err != nil {
+	m := make(map[string]*x509.RevocationList)
-		return nil, fmt.Errorf("cryptutil: failed to decode base64 crl: %w", err)
+	for {
-	}
-	return DecodeCRL(bs)
-}
-
-// CRLFromFile parses a certificate revocation list from a file.
-func CRLFromFile(fileName string) (*pkix.CertificateList, error) {
-	bs, err := os.ReadFile(fileName)
-	if err != nil {
-		return nil, fmt.Errorf("cryptutil: failed to read crl file (%s): %w", fileName, err)
-	}
-	return DecodeCRL(bs)
-}
-
-// DecodeCRL decodes a PEM-encoded certificate revocation list.
-func DecodeCRL(encodedCRL []byte) (*pkix.CertificateList, error) {
-	data := encodedCRL
-	for len(data) > 0 {
 		var block *pem.Block
-		block, data = pem.Decode(data)
+		block, crl = pem.Decode(crl)
 		if block == nil {
-			break
+			if len(crl) > 0 {
+				return nil, errors.New("cryptutil: non-PEM data in CRL bundle")
 			}
-
+			return m, nil
-		if block.Type == crlPemType {
+		}
-			lst, err := x509.ParseDERCRL(block.Bytes)
+		if block.Type != "X509 CRL" {
+			continue
+		}
+		l, err := x509.ParseRevocationList(block.Bytes)
 		if err != nil {
 			return nil, fmt.Errorf("cryptutil: failed to parse crl: %w", err)
 		}
-			return lst, nil
+		m[string(l.RawIssuer)] = l
 	}
 }
-	return nil, fmt.Errorf("cryptutil: invalid crl, no %s block found", crlPemType)
-}
 
 // DecodePublicKey decodes a PEM-encoded ECDSA public key.
 func DecodePublicKey(encodedKey []byte) (*ecdsa.PublicKey, error) {

pkg/cryptutil/certificates_test.go

@@ -106,29 +106,18 @@ func TestCertificateFromFile(t *testing.T) {
 	_ = listener
 }
 
-func TestCRLFromBase64(t *testing.T) {
+func TestParseCRLs(t *testing.T) {
-	bs := base64.StdEncoding.EncodeToString([]byte(pemCRL))
+	crls, err := ParseCRLs([]byte(pemCRL))
-	lst, err := CRLFromBase64(bs)
 	assert.NoError(t, err)
-	assert.NotNil(t, lst)
+	assert.Equal(t, 1, len(crls))
-}
+	crl := crls["0l1\x1a0\x18\x06\x03U\x04\n\x13\x11RSA Security Inc.1\x1e0\x1c\x06\x03U\x04\x03\x13\x15RSA Public Root CA v11.0,\x06\t*\x86H\x86\xf7\r\x01\t\x01\x16\x1frsakeonrootsign@rsasecurity.com"]
-
+	assert.NotNil(t, crl)
-func TestCRLFromFile(t *testing.T) {
-	lst, err := CRLFromFile("testdata/example-crl.pem")
-	assert.NoError(t, err)
-	assert.NotNil(t, lst)
-}
-
-func TestDecodeCRL(t *testing.T) {
-	lst, err := DecodeCRL([]byte(pemCRL))
-	assert.NoError(t, err)
-	assert.NotNil(t, lst)
 
 	t.Run("der", func(t *testing.T) {
 		bs, _ := base64.StdEncoding.DecodeString(derCRLBase64)
-		lst, err := DecodeCRL(bs)
+		crls, err := ParseCRLs(bs)
 		assert.Error(t, err, "should not allow DER encoded CRL")
-		assert.Nil(t, lst)
+		assert.Nil(t, crls)
 	})
 }
 

pkg/cryptutil/testdata/example-crl.pem

@@ -1,13 +0,0 @@
------BEGIN X509 CRL-----
-MIIB9jCCAV8CAQEwDQYJKoZIhvcNAQEFBQAwbDEaMBgGA1UEChMRUlNBIFNlY3Vy
-aXR5IEluYy4xHjAcBgNVBAMTFVJTQSBQdWJsaWMgUm9vdCBDQSB2MTEuMCwGCSqG
-SIb3DQEJARYfcnNha2VvbnJvb3RzaWduQHJzYXNlY3VyaXR5LmNvbRcNMTEwMjIz
-MTkyODMwWhcNMTEwODIyMTkyODMwWjCBjDBKAhEArDqoh9FHJHXT7OPguun4+BcN
-MDkxMTAyMTQyNzA5WjAmMAoGA1UdFQQDCgEJMBgGA1UdGAQRGA8yMDA5MTEwMjE0
-MjQ1NVowPgIRALGznZ095PB5aAOLPg57fMMXDTAyMTAyMzE0NTAxNFowGjAYBgNV
-HRgEERgPMjAwMjEwMjMxNDUwMTRaoDAwLjAfBgNVHSMEGDAWgBT1TDF6UQM/LNeL
-l5lvqHGQq3g9mzALBgNVHRQEBAICAIQwDQYJKoZIhvcNAQEFBQADgYEAFU5As6Mz
-q5PRsifaobQPGh1aJLyC+Ms5Agc0bWyA3GAdxur5SpPZeRWCBjiP/MEHBWJClBHP
-GRcq5yId3EjDkaEyxRa+i67LzvhI6c29Ee6K9pSYwji/7RUhmmnPrXtTxlL0lrLr
-mQQJ6xhDRa5G3QA4CmUdsHNvbrzgmCYpvVE=
------END X509 CRL-----