diff --git a/authorize/authorize.go b/authorize/authorize.go index 7e932ee9f..c648c121a 100644 --- a/authorize/authorize.go +++ b/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()), diff --git a/authorize/evaluator/config.go b/authorize/evaluator/config.go index 8f3733b3b..7cf080c06 100644 --- a/authorize/evaluator/config.go +++ b/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) { diff --git a/authorize/evaluator/evaluator.go b/authorize/evaluator/evaluator.go index 141561d97..4dc073d6e 100644 --- a/authorize/evaluator/evaluator.go +++ b/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) } diff --git a/authorize/evaluator/functions.go b/authorize/evaluator/functions.go index fa0ea75d3..d5858dc31 100644 --- a/authorize/evaluator/functions.go +++ b/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 diff --git a/authorize/evaluator/functions_test.go b/authorize/evaluator/functions_test.go index ea3b271f6..c43b91157 100644 --- a/authorize/evaluator/functions_test.go +++ b/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") }) diff --git a/config/envoyconfig/listeners.go b/config/envoyconfig/listeners.go index 75c0ca428..e05caebd0 100644 --- a/config/envoyconfig/listeners.go +++ b/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 { diff --git a/config/envoyconfig/listeners_test.go b/config/envoyconfig/listeners_test.go index 09540dbfa..8703817ba 100644 --- a/config/envoyconfig/listeners_test.go +++ b/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, diff --git a/config/mtls.go b/config/mtls.go index 20619362c..e4886f887 100644 --- a/config/mtls.go +++ b/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 diff --git a/config/mtls_test.go b/config/mtls_test.go index da2eb5f5f..1f4cc7f98 100644 --- a/config/mtls_test.go +++ b/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()