config: add support for max_verify_depth (#4452)

Add a new max_verify_depth option to the downstream_mtls settings group,
with a default value of 1 (to match the behavior of current Pomerium
releases).

Populate the corresponding setting within Envoy, and also implement a
depth check within isValidClientCertificate() in the authorize service.

authorize/authorize.go

@@ -120,11 +120,16 @@ func newPolicyEvaluator(opts *config.Options, store *store.Store) (*evaluator.Ev
 	addDefaultClientCertificateRule :=
 		opts.DownstreamMTLS.GetEnforcement() != config.MTLSEnforcementPolicy
 
+	clientCertConstraints := evaluator.ClientCertConstraints{
+		MaxVerifyDepth: opts.DownstreamMTLS.GetMaxVerifyDepth(),
+	}
+
 	return evaluator.New(ctx, store,
 		evaluator.WithPolicies(opts.GetAllPolicies()),
 		evaluator.WithClientCA(clientCA),
 		evaluator.WithAddDefaultClientCertificateRule(addDefaultClientCertificateRule),
 		evaluator.WithClientCRL(clientCRL),
+		evaluator.WithClientCertConstraints(clientCertConstraints),
 		evaluator.WithSigningKey(signingKey),
 		evaluator.WithAuthenticateURL(authenticateURL.String()),
 		evaluator.WithGoogleCloudServerlessAuthenticationServiceAccount(opts.GetGoogleCloudServerlessAuthenticationServiceAccount()),

authorize/evaluator/config.go

@@ -9,6 +9,7 @@ type evaluatorConfig struct {
 	clientCA                                          []byte
 	clientCRL                                         []byte
 	addDefaultClientCertificateRule                   bool
+	clientCertConstraints                             ClientCertConstraints
 	signingKey                                        []byte
 	authenticateURL                                   string
 	googleCloudServerlessAuthenticationServiceAccount string
@@ -55,6 +56,13 @@ func WithAddDefaultClientCertificateRule(addDefaultClientCertificateRule bool) O
 	}
 }
 
+// WithClientCertConstraints sets addition client certificate constraints.
+func WithClientCertConstraints(constraints ClientCertConstraints) Option {
+	return func(cfg *evaluatorConfig) {
+		cfg.clientCertConstraints = constraints
+	}
+}
+
 // WithSigningKey sets the signing key and algorithm in the config.
 func WithSigningKey(signingKey []byte) Option {
 	return func(cfg *evaluatorConfig) {

authorize/evaluator/evaluator.go

@@ -89,11 +89,12 @@ type Result struct {
 
 // An Evaluator evaluates policies.
 type Evaluator struct {
-	store             *store.Store
-	policyEvaluators  map[uint64]*PolicyEvaluator
-	headersEvaluators *HeadersEvaluator
-	clientCA          []byte
-	clientCRL         []byte
+	store                 *store.Store
+	policyEvaluators      map[uint64]*PolicyEvaluator
+	headersEvaluators     *HeadersEvaluator
+	clientCA              []byte
+	clientCRL             []byte
+	clientCertConstraints ClientCertConstraints
 }
 
 // New creates a new Evaluator.
@@ -114,6 +115,7 @@ func New(ctx context.Context, store *store.Store, options ...Option) (*Evaluator
 
 	e.clientCA = cfg.clientCA
 	e.clientCRL = cfg.clientCRL
+	e.clientCertConstraints = cfg.clientCertConstraints
 
 	e.policyEvaluators = make(map[uint64]*PolicyEvaluator)
 	for i := range cfg.policies {
@@ -211,8 +213,8 @@ func (e *Evaluator) evaluatePolicy(ctx context.Context, req *Request) (*PolicyRe
 		return nil, err
 	}
 
-	isValidClientCertificate, err :=
-		isValidClientCertificate(clientCA, string(e.clientCRL), req.HTTP.ClientCertificate)
+	isValidClientCertificate, err := isValidClientCertificate(
+		clientCA, string(e.clientCRL), req.HTTP.ClientCertificate, e.clientCertConstraints)
 	if err != nil {
 		return nil, fmt.Errorf("authorize: error validating client certificate: %w", err)
 	}

authorize/evaluator/functions.go

@@ -3,6 +3,7 @@ package evaluator
 import (
 	"context"
 	"crypto/x509"
+	"encoding/json"
 	"encoding/pem"
 	"errors"
 	"fmt"
@@ -12,9 +13,19 @@ import (
 	"github.com/pomerium/pomerium/internal/log"
 )
 
-var isValidClientCertificateCache, _ = lru.New2Q[[4]string, bool](100)
+// ClientCertConstraints contains additional constraints to validate when
+// verifying a client certificate.
+type ClientCertConstraints struct {
+	// MaxVerifyDepth is the maximum allowed certificate chain depth (not
+	// counting the leaf certificate). A value of 0 indicates no maximum.
+	MaxVerifyDepth uint32
+}
 
-func isValidClientCertificate(ca, crl string, certInfo ClientCertificateInfo) (bool, error) {
+var isValidClientCertificateCache, _ = lru.New2Q[[5]string, bool](100)
+
+func isValidClientCertificate(
+	ca, crl string, certInfo ClientCertificateInfo, constraints ClientCertConstraints,
+) (bool, error) {
 	// when ca is the empty string, client certificates are not required
 	if ca == "" {
 		return true, nil
@@ -27,7 +38,12 @@ func isValidClientCertificate(ca, crl string, certInfo ClientCertificateInfo) (b
 		return false, nil
 	}
 
-	cacheKey := [4]string{ca, crl, cert, intermediates}
+	constraintsJSON, err := json.Marshal(constraints)
+	if err != nil {
+		return false, fmt.Errorf("internal error: failed to serialize constraints: %w", err)
+	}
+
+	cacheKey := [5]string{ca, crl, cert, intermediates, string(constraintsJSON)}
 
 	value, ok := isValidClientCertificateCache.Get(cacheKey)
 	if ok {
@@ -50,7 +66,7 @@ func isValidClientCertificate(ca, crl string, certInfo ClientCertificateInfo) (b
 		return false, err
 	}
 
-	verifyErr := verifyClientCertificate(xcert, roots, intermediatesPool, crls)
+	verifyErr := verifyClientCertificate(xcert, roots, intermediatesPool, crls, constraints)
 	valid := verifyErr == nil
 
 	if verifyErr != nil {
@@ -67,6 +83,7 @@ func verifyClientCertificate(
 	roots *x509.CertPool,
 	intermediates *x509.CertPool,
 	crls map[string]*x509.RevocationList,
+	constraints ClientCertConstraints,
 ) error {
 	chains, err := cert.Verify(x509.VerifyOptions{
 		Roots:         roots,
@@ -77,10 +94,11 @@ func verifyClientCertificate(
 		return err
 	}
 
-	// At least one of the verified chains must also pass revocation checking.
+	// At least one of the verified chains must also pass revocation checking
+	// and satisfy any additional constraints.
 	err = errors.New("internal error: no verified chains")
 	for _, chain := range chains {
-		err = validateClientCertificateChain(chain, crls)
+		err = validateClientCertificateChain(chain, crls, constraints)
 		if err == nil {
 			return nil
 		}
@@ -94,7 +112,15 @@ func verifyClientCertificate(
 func validateClientCertificateChain(
 	chain []*x509.Certificate,
 	crls map[string]*x509.RevocationList,
+	constraints ClientCertConstraints,
 ) error {
+	if constraints.MaxVerifyDepth > 0 {
+		if d := uint32(len(chain) - 1); d > constraints.MaxVerifyDepth {
+			return fmt.Errorf("chain depth %d exceeds max_verify_depth %d",
+				d, constraints.MaxVerifyDepth)
+		}
+	}
+
 	// Consult CRLs for all CAs in the chain (that is, all certificates except
 	// for the first one). To match Envoy's behavior, if a CRL is provided for
 	// any CA in the chain, CRLs must be provided for all CAs in the chain (see

authorize/evaluator/functions_test.go

@@ -97,13 +97,15 @@ BVAnH/e8AiEAjy8cP1msG62BeDaAVU5NcU9RAXDw1Oz4HkpELXQWqK8=
 )
 
 func Test_isValidClientCertificate(t *testing.T) {
+	var noConstraints ClientCertConstraints
 	t.Run("no ca", func(t *testing.T) {
-		valid, err := isValidClientCertificate("", "", ClientCertificateInfo{Leaf: "WHATEVER!"})
+		valid, err := isValidClientCertificate(
+			"", "", ClientCertificateInfo{Leaf: "WHATEVER!"}, noConstraints)
 		assert.NoError(t, err, "should not return an error")
 		assert.True(t, valid, "should return true")
 	})
 	t.Run("no cert", func(t *testing.T) {
-		valid, err := isValidClientCertificate(testCA, "", ClientCertificateInfo{})
+		valid, err := isValidClientCertificate(testCA, "", ClientCertificateInfo{}, noConstraints)
 		assert.NoError(t, err, "should not return an error")
 		assert.False(t, valid, "should return false")
 	})
@@ -111,7 +113,7 @@ func Test_isValidClientCertificate(t *testing.T) {
 		valid, err := isValidClientCertificate(testCA, "", ClientCertificateInfo{
 			Presented: true,
 			Leaf:      testValidCert,
-		})
+		}, noConstraints)
 		assert.NoError(t, err, "should not return an error")
 		assert.True(t, valid, "should return true")
 	})
@@ -120,7 +122,7 @@ func Test_isValidClientCertificate(t *testing.T) {
 			Presented:     true,
 			Leaf:          testValidIntermediateCert,
 			Intermediates: testIntermediateCA,
-		})
+		}, noConstraints)
 		assert.NoError(t, err, "should not return an error")
 		assert.True(t, valid, "should return true")
 	})
@@ -129,7 +131,7 @@ func Test_isValidClientCertificate(t *testing.T) {
 			Presented:     true,
 			Leaf:          testValidIntermediateCert,
 			Intermediates: "",
-		})
+		}, noConstraints)
 		assert.NoError(t, err, "should not return an error")
 		assert.False(t, valid, "should return false")
 	})
@@ -137,7 +139,7 @@ func Test_isValidClientCertificate(t *testing.T) {
 		valid, err := isValidClientCertificate(testIntermediateCA, "", ClientCertificateInfo{
 			Presented: true,
 			Leaf:      testValidIntermediateCert,
-		})
+		}, noConstraints)
 		assert.NoError(t, err, "should not return an error")
 		assert.True(t, valid, "should return true")
 	})
@@ -145,7 +147,7 @@ func Test_isValidClientCertificate(t *testing.T) {
 		valid, err := isValidClientCertificate(testCA, "", ClientCertificateInfo{
 			Presented: true,
 			Leaf:      testUntrustedCert,
-		})
+		}, noConstraints)
 		assert.NoError(t, err, "should not return an error")
 		assert.False(t, valid, "should return false")
 	})
@@ -153,7 +155,7 @@ func Test_isValidClientCertificate(t *testing.T) {
 		valid, err := isValidClientCertificate(testCA, "", ClientCertificateInfo{
 			Presented: true,
 			Leaf:      "WHATEVER!",
-		})
+		}, noConstraints)
 		assert.Error(t, err, "should return an error")
 		assert.False(t, valid, "should return false")
 	})
@@ -164,11 +166,11 @@ func Test_isValidClientCertificate(t *testing.T) {
 		}
 
 		// The "revoked cert" should otherwise be valid (when no CRL is specified).
-		valid, err := isValidClientCertificate(testCA, "", revokedCertInfo)
+		valid, err := isValidClientCertificate(testCA, "", revokedCertInfo, noConstraints)
 		assert.NoError(t, err, "should not return an error")
 		assert.True(t, valid, "should return true")
 
-		valid, err = isValidClientCertificate(testCA, testCRL, revokedCertInfo)
+		valid, err = isValidClientCertificate(testCA, testCRL, revokedCertInfo, noConstraints)
 		assert.NoError(t, err, "should not return an error")
 		assert.False(t, valid, "should return false")
 
@@ -176,7 +178,7 @@ func Test_isValidClientCertificate(t *testing.T) {
 		valid, err = isValidClientCertificate(testCA, testCRL, ClientCertificateInfo{
 			Presented: true,
 			Leaf:      testValidCert,
-		})
+		}, noConstraints)
 		assert.NoError(t, err, "should not return an error")
 		assert.True(t, valid, "should return true")
 	})
@@ -186,7 +188,16 @@ func Test_isValidClientCertificate(t *testing.T) {
 			Presented:     true,
 			Leaf:          testValidIntermediateCert,
 			Intermediates: testIntermediateCA,
-		})
+		}, noConstraints)
+		assert.NoError(t, err, "should not return an error")
+		assert.False(t, valid, "should return false")
+	})
+	t.Run("chain too deep", func(t *testing.T) {
+		valid, err := isValidClientCertificate(testCA, "", ClientCertificateInfo{
+			Presented:     true,
+			Leaf:          testValidIntermediateCert,
+			Intermediates: testIntermediateCA,
+		}, ClientCertConstraints{MaxVerifyDepth: 1})
 		assert.NoError(t, err, "should not return an error")
 		assert.False(t, valid, "should return false")
 	})

config/envoyconfig/listeners.go

@@ -553,6 +553,10 @@ func (b *Builder) buildDownstreamValidationContext(
 		TrustedCa: b.filemgr.BytesDataSource("client-ca.pem", clientCA),
 	}
 
+	if d := cfg.Options.DownstreamMTLS.GetMaxVerifyDepth(); d > 0 {
+		vc.MaxVerifyDepth = wrapperspb.UInt32(d)
+	}
+
 	if cfg.Options.DownstreamMTLS.GetEnforcement() == config.MTLSEnforcementRejectConnection {
 		dtc.RequireClientCertificate = wrapperspb.Bool(true)
 	} else {

config/envoyconfig/listeners_test.go

@@ -118,6 +118,7 @@ func Test_buildDownstreamTLSContext(t *testing.T) {
 				},
 				"alpnProtocols": ["h2", "http/1.1"],
 				"validationContext": {
+					"maxVerifyDepth": 1,
 					"trustChainVerification": "ACCEPT_UNTRUSTED",
 					"trustedCa": {
 						"filename": "`+clientCAFileName+`"
@@ -149,6 +150,7 @@ func Test_buildDownstreamTLSContext(t *testing.T) {
 				},
 				"alpnProtocols": ["h2", "http/1.1"],
 				"validationContext": {
+					"maxVerifyDepth": 1,
 					"trustedCa": {
 						"filename": "`+clientCAFileName+`"
 					}
@@ -183,6 +185,7 @@ func Test_buildDownstreamTLSContext(t *testing.T) {
 				},
 				"alpnProtocols": ["h2", "http/1.1"],
 				"validationContext": {
+					"maxVerifyDepth": 1,
 					"trustChainVerification": "ACCEPT_UNTRUSTED",
 					"trustedCa": {
 						"filename": "`+clientCAFileName+`"
@@ -191,6 +194,38 @@ func Test_buildDownstreamTLSContext(t *testing.T) {
 			}
 		}`, downstreamTLSContext)
 	})
+	t.Run("client-ca-max-verify-depth", func(t *testing.T) {
+		var maxVerifyDepth uint32
+		config := &config.Config{Options: &config.Options{
+			DownstreamMTLS: config.DownstreamMTLSSettings{
+				MaxVerifyDepth: &maxVerifyDepth,
+				CA:             "VEVTVAo=", // "TEST\n"
+			},
+		}}
+
+		maxVerifyDepth = 10
+		downstreamTLSContext, err :=
+			b.buildDownstreamTLSContextMulti(context.Background(), config, nil)
+		require.NoError(t, err)
+		testutil.AssertProtoJSONEqual(t, `{
+			"maxVerifyDepth": 10,
+			"trustChainVerification": "ACCEPT_UNTRUSTED",
+			"trustedCa": {
+				"filename": "`+clientCAFileName+`"
+			}
+		}`, downstreamTLSContext.GetCommonTlsContext().GetValidationContext())
+
+		maxVerifyDepth = 0
+		downstreamTLSContext, err =
+			b.buildDownstreamTLSContextMulti(context.Background(), config, nil)
+		require.NoError(t, err)
+		testutil.AssertProtoJSONEqual(t, `{
+			"trustChainVerification": "ACCEPT_UNTRUSTED",
+			"trustedCa": {
+				"filename": "`+clientCAFileName+`"
+			}
+		}`, downstreamTLSContext.GetCommonTlsContext().GetValidationContext())
+	})
 	t.Run("http1", func(t *testing.T) {
 		downstreamTLSContext, err := b.buildDownstreamTLSContextMulti(context.Background(), &config.Config{Options: &config.Options{
 			Cert:      aExampleComCert,

config/mtls.go

@@ -54,6 +54,10 @@ type DownstreamMTLSSettings struct {
 	// Enforcement indicates the behavior applied to requests without a valid
 	// client certificate.
 	Enforcement MTLSEnforcement `mapstructure:"enforcement" yaml:"enforcement,omitempty"`
+
+	// MaxVerifyDepth is the maximum allowed depth of a certificate trust chain
+	// (not counting the leaf certificate). The value 0 indicates no maximum.
+	MaxVerifyDepth *uint32 `mapstructure:"max_verify_depth" yaml:"max_verify_depth,omitempty"`
 }
 
 // GetCA returns the certificate authority (or nil if unset).
@@ -102,6 +106,15 @@ func (s *DownstreamMTLSSettings) GetEnforcement() MTLSEnforcement {
 	return s.Enforcement
 }
 
+// GetMaxVerifyDepth returns the maximum certificate chain depth. The value 0
+// indicates no maximum.
+func (s *DownstreamMTLSSettings) GetMaxVerifyDepth() uint32 {
+	if s.MaxVerifyDepth == nil {
+		return 1
+	}
+	return *s.MaxVerifyDepth
+}
+
 func (s *DownstreamMTLSSettings) validate() error {
 	if _, err := s.GetCA(); err != nil {
 		return err

config/mtls_test.go

@@ -95,6 +95,24 @@ func TestDownstreamMTLSSettingsGetEnforcement(t *testing.T) {
 	}
 }
 
+func TestDownstreamMTLSSettingsGetMaxVerifyDepth(t *testing.T) {
+	t.Parallel()
+
+	// MaxVerifyDepth should default to 1 if not set explicitly.
+	var s DownstreamMTLSSettings
+	assert.Equal(t, uint32(1), s.GetMaxVerifyDepth())
+
+	var maxVerifyDepth uint32
+	s.MaxVerifyDepth = &maxVerifyDepth
+	assert.Equal(t, uint32(0), s.GetMaxVerifyDepth())
+
+	maxVerifyDepth = 1
+	assert.Equal(t, uint32(1), s.GetMaxVerifyDepth())
+
+	maxVerifyDepth = 1000
+	assert.Equal(t, uint32(1000), s.GetMaxVerifyDepth())
+}
+
 func TestDownstreamMTLSSettingsValidate(t *testing.T) {
 	t.Parallel()