Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

upgrade main #4457

Merged
merged 11 commits into from
Aug 13, 2023
18 changes: 16 additions & 2 deletions authorize/authorize.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,12 @@ func newPolicyEvaluator(opts *config.Options, store *store.Store) (*evaluator.Ev
ctx, span := trace.StartSpan(ctx, "authorize.newPolicyEvaluator")
defer span.End()

clientCA, err := opts.GetClientCA()
clientCA, err := opts.DownstreamMTLS.GetCA()
if err != nil {
return nil, fmt.Errorf("authorize: invalid client CA: %w", err)
}

clientCRL, err := opts.GetClientCRL()
clientCRL, err := opts.DownstreamMTLS.GetCRL()
if err != nil {
return nil, fmt.Errorf("authorize: invalid client CRL: %w", err)
}
Expand All @@ -114,10 +114,24 @@ func newPolicyEvaluator(opts *config.Options, store *store.Store) (*evaluator.Ev
return nil, fmt.Errorf("authorize: invalid signing key: %w", err)
}

// It is important to add an invalid_client_certificate rule even when the
// mTLS enforcement behavior is set to reject connections at the listener
// level, because of the per-route TLSDownstreamClientCA setting.
addDefaultClientCertificateRule :=
opts.DownstreamMTLS.GetEnforcement() != config.MTLSEnforcementPolicy

clientCertConstraints, err := evaluator.ClientCertConstraintsFromConfig(&opts.DownstreamMTLS)
if err != nil {
return nil, fmt.Errorf(
"authorize: internal error: couldn't build client cert constraints: %w", err)
}

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()),
Expand Down
17 changes: 17 additions & 0 deletions authorize/evaluator/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ type evaluatorConfig struct {
policies []config.Policy
clientCA []byte
clientCRL []byte
addDefaultClientCertificateRule bool
clientCertConstraints ClientCertConstraints
signingKey []byte
authenticateURL string
googleCloudServerlessAuthenticationServiceAccount string
Expand Down Expand Up @@ -46,6 +48,21 @@ func WithClientCRL(clientCRL []byte) Option {
}
}

// WithAddDefaultClientCertificateRule sets whether to add a default
// invalid_client_certificate deny rule to all policies.
func WithAddDefaultClientCertificateRule(addDefaultClientCertificateRule bool) Option {
return func(cfg *evaluatorConfig) {
cfg.addDefaultClientCertificateRule = addDefaultClientCertificateRule
}
}

// 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) {
Expand Down
22 changes: 12 additions & 10 deletions authorize/evaluator/evaluator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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 {
Expand All @@ -122,8 +124,8 @@ func New(ctx context.Context, store *store.Store, options ...Option) (*Evaluator
if err != nil {
return nil, fmt.Errorf("authorize: error computing policy route id: %w", err)
}
clientCA, _ := e.getClientCA(&configPolicy)
policyEvaluator, err := NewPolicyEvaluator(ctx, store, &configPolicy, clientCA)
policyEvaluator, err :=
NewPolicyEvaluator(ctx, store, &configPolicy, cfg.addDefaultClientCertificateRule)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -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)
}
Expand All @@ -225,7 +227,7 @@ func (e *Evaluator) evaluatePolicy(ctx context.Context, req *Request) (*PolicyRe
}

func (e *Evaluator) evaluateHeaders(ctx context.Context, req *Request) (*HeadersResponse, error) {
headersReq := NewHeadersRequestFromPolicy(req.Policy, req.HTTP.Hostname)
headersReq := NewHeadersRequestFromPolicy(req.Policy, req.HTTP)
headersReq.Session = req.Session
res, err := e.headersEvaluators.Evaluate(ctx, headersReq)
if err != nil {
Expand Down
50 changes: 49 additions & 1 deletion authorize/evaluator/evaluator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,18 @@ func TestEvaluator(t *testing.T) {
AllowedUsers: []string{"a@example.com"},
TLSDownstreamClientCA: base64.StdEncoding.EncodeToString([]byte(testCA)),
},
{
To: config.WeightedURLs{{URL: *mustParseURL("https://to12.example.com")}},
AllowedUsers: []string{"a@example.com"},
Policy: &config.PPLPolicy{
Policy: &parser.Policy{
Rules: []parser.Rule{{
Action: parser.ActionDeny,
Or: []parser.Criterion{{Name: "invalid_client_certificate"}},
}},
},
},
},
}
options := []Option{
WithAuthenticateURL("https://authn.example.com"),
Expand All @@ -129,7 +141,8 @@ func TestEvaluator(t *testing.T) {
t.Run("client certificate (default CA)", func(t *testing.T) {
// Clone the existing options and add a default client CA.
options := append([]Option(nil), options...)
options = append(options, WithClientCA([]byte(testCA)))
options = append(options, WithClientCA([]byte(testCA)),
WithAddDefaultClientCertificateRule(true))
t.Run("missing", func(t *testing.T) {
res, err := eval(t, options, nil, &Request{
Policy: &policies[0],
Expand Down Expand Up @@ -159,6 +172,9 @@ func TestEvaluator(t *testing.T) {
})
})
t.Run("client certificate (per-policy CA)", func(t *testing.T) {
// Clone existing options and add the default client certificate rule.
options := append([]Option(nil), options...)
options = append(options, WithAddDefaultClientCertificateRule(true))
t.Run("missing", func(t *testing.T) {
res, err := eval(t, options, nil, &Request{
Policy: &policies[10],
Expand Down Expand Up @@ -190,6 +206,38 @@ func TestEvaluator(t *testing.T) {
assert.False(t, res.Deny.Value)
})
})
t.Run("explicit client certificate rule", func(t *testing.T) {
// Clone the existing options and add a default client CA (but no
// default deny rule).
options := append([]Option(nil), options...)
options = append(options, WithClientCA([]byte(testCA)))
t.Run("invalid but allowed", func(t *testing.T) {
res, err := eval(t, options, nil, &Request{
Policy: &policies[0], // no explicit deny rule
HTTP: RequestHTTP{
ClientCertificate: ClientCertificateInfo{
Presented: true,
Leaf: testUntrustedCert,
},
},
})
require.NoError(t, err)
assert.False(t, res.Deny.Value)
})
t.Run("invalid", func(t *testing.T) {
res, err := eval(t, options, nil, &Request{
Policy: &policies[11], // policy has explicit deny rule
HTTP: RequestHTTP{
ClientCertificate: ClientCertificateInfo{
Presented: true,
Leaf: testUntrustedCert,
},
},
})
require.NoError(t, err)
assert.Equal(t, NewRuleResult(true, criteria.ReasonInvalidClientCertificate), res.Deny)
})
})
t.Run("identity_headers", func(t *testing.T) {
t.Run("kubernetes", func(t *testing.T) {
res, err := eval(t, options, []proto.Message{
Expand Down