3pp-mirror/pomerium
1
0
Fork 0
mirror of https://github.com/pomerium/pomerium.git synced 2025-08-04 09:19:39 +02:00
Code Issues Projects Releases Packages Wiki Activity

authorize: omit client cert rule when not needed (#4386)

Browse source 
Currently we always add an invalid_client_certificate deny rule to all
PPL policies. Instead, let's add this rule only when a client CA is
configured. This way, if a user is not using client certificates at all,
they won't see any reason strings related to client certificates in the
authorize logs.

Change the "valid-client-certificate-or-none-required" reason string to
just "valid-client-certificate" accordingly.

Pass the main Evaluator config to NewPolicyEvaluator so that we can
determine whether there is a client CA configured or not. Extract the
existing default deny rule to a separate method. Add unit tests
exercising the new behavior.
This commit is contained in:
Kenneth Jenkins 2023-07-24 15:27:57 -07:00 • committed by GitHub
parent 219296a875
commit 4698e4661a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 166 additions and 103 deletions
Download patch file Download diff file Expand all files Collapse all files

7
authorize/evaluator/evaluator.go
View file

@ -119,6 +119,8 @@ func New(ctx context.Context, store *store.Store, options ...Option) (*Evaluator
return nil, err
}
e.clientCA = cfg.clientCA
e.policyEvaluators = make(map[uint64]*PolicyEvaluator)
for i := range cfg.policies {
configPolicy := cfg.policies[i]
@ -126,15 +128,14 @@ 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)
}
policyEvaluator, err := NewPolicyEvaluator(ctx, store, &configPolicy)
clientCA, _ := e.getClientCA(&configPolicy)
policyEvaluator, err := NewPolicyEvaluator(ctx, store, &configPolicy, clientCA)
if err != nil {
return nil, err
}
e.policyEvaluators[id] = policyEvaluator
}
e.clientCA = cfg.clientCA
return e, nil
}

92
authorize/evaluator/evaluator_test.go
View file

@ -2,6 +2,7 @@ package evaluator
import (
"context"
"encoding/base64"
"net/http"
"net/url"
"testing"
@ -109,10 +110,14 @@ func TestEvaluator(t *testing.T) {
},
},
},
{
To: config.WeightedURLs{{URL: *mustParseURL("https://to11.example.com")}},
AllowedUsers: []string{"a@example.com"},
TLSDownstreamClientCA: base64.StdEncoding.EncodeToString([]byte(testCA)),
},
}
options := []Option{
WithAuthenticateURL("https://authn.example.com"),
WithClientCA([]byte(testCA)),
WithPolicies(policies),
}
@ -122,7 +127,10 @@ func TestEvaluator(t *testing.T) {
Leaf: testValidCert,
}
t.Run("client certificate", func(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)))
t.Run("invalid", func(t *testing.T) {
res, err := eval(t, options, nil, &Request{
Policy: &policies[0],
@ -141,6 +149,25 @@ func TestEvaluator(t *testing.T) {
assert.False(t, res.Deny.Value)
})
})
t.Run("client certificate (per-policy CA)", func(t *testing.T) {
t.Run("invalid", func(t *testing.T) {
res, err := eval(t, options, nil, &Request{
Policy: &policies[10],
})
require.NoError(t, err)
assert.Equal(t, NewRuleResult(true, criteria.ReasonInvalidClientCertificate), res.Deny)
})
t.Run("valid", func(t *testing.T) {
res, err := eval(t, options, nil, &Request{
Policy: &policies[10],
HTTP: RequestHTTP{
ClientCertificate: validCertInfo,
},
})
require.NoError(t, err)
assert.False(t, res.Deny.Value)
})
})
t.Run("identity_headers", func(t *testing.T) {
t.Run("kubernetes", func(t *testing.T) {
res, err := eval(t, options, []proto.Message{
@ -158,9 +185,8 @@ func TestEvaluator(t *testing.T) {
ID: "session1",
},
HTTP: RequestHTTP{
Method: http.MethodGet,
URL: "https://from.example.com",
ClientCertificate: validCertInfo,
Method: http.MethodGet,
URL: "https://from.example.com",
},
})
require.NoError(t, err)
@ -183,9 +209,8 @@ func TestEvaluator(t *testing.T) {
ID: "session1",
},
HTTP: RequestHTTP{
Method: http.MethodGet,
URL: "https://from.example.com",
ClientCertificate: validCertInfo,
Method: http.MethodGet,
URL: "https://from.example.com",
},
})
require.NoError(t, err)
@ -210,9 +235,8 @@ func TestEvaluator(t *testing.T) {
ID: "session1",
},
HTTP: RequestHTTP{
Method: http.MethodGet,
URL: "https://from.example.com",
ClientCertificate: validCertInfo,
Method: http.MethodGet,
URL: "https://from.example.com",
},
})
require.NoError(t, err)
@ -234,9 +258,8 @@ func TestEvaluator(t *testing.T) {
ID: "session1",
},
HTTP: RequestHTTP{
Method: http.MethodGet,
URL: "https://from.example.com",
ClientCertificate: validCertInfo,
Method: http.MethodGet,
URL: "https://from.example.com",
},
})
require.NoError(t, err)
@ -258,9 +281,8 @@ func TestEvaluator(t *testing.T) {
ID: "session1",
},
HTTP: RequestHTTP{
Method: http.MethodGet,
URL: "https://from.example.com",
ClientCertificate: validCertInfo,
Method: http.MethodGet,
URL: "https://from.example.com",
},
})
require.NoError(t, err)
@ -289,9 +311,8 @@ func TestEvaluator(t *testing.T) {
ID: "session2",
},
HTTP: RequestHTTP{
Method: http.MethodGet,
URL: "https://from.example.com",
ClientCertificate: validCertInfo,
Method: http.MethodGet,
URL: "https://from.example.com",
},
})
require.NoError(t, err)
@ -314,9 +335,8 @@ func TestEvaluator(t *testing.T) {
ID: "session1",
},
HTTP: RequestHTTP{
Method: http.MethodGet,
URL: "https://from.example.com",
ClientCertificate: validCertInfo,
Method: http.MethodGet,
URL: "https://from.example.com",
},
})
require.NoError(t, err)
@ -338,9 +358,8 @@ func TestEvaluator(t *testing.T) {
ID: "session1",
},
HTTP: RequestHTTP{
Method: http.MethodGet,
URL: "https://from.example.com",
ClientCertificate: validCertInfo,
Method: http.MethodGet,
URL: "https://from.example.com",
},
})
require.NoError(t, err)
@ -367,9 +386,8 @@ func TestEvaluator(t *testing.T) {
ID: "session1",
},
HTTP: RequestHTTP{
Method: http.MethodGet,
URL: "https://from.example.com",
ClientCertificate: validCertInfo,
Method: http.MethodGet,
URL: "https://from.example.com",
},
})
require.NoError(t, err)
@ -390,9 +408,8 @@ func TestEvaluator(t *testing.T) {
ID: "session1",
},
HTTP: RequestHTTP{
Method: http.MethodGet,
URL: "https://from.example.com",
ClientCertificate: validCertInfo,
Method: http.MethodGet,
URL: "https://from.example.com",
},
})
require.NoError(t, err)
@ -427,10 +444,9 @@ func TestEvaluator(t *testing.T) {
ID: "session1",
},
HTTP: RequestHTTP{
Method: http.MethodGet,
URL: "https://from.example.com",
ClientCertificate: validCertInfo,
Headers: tc.src,
Method: http.MethodGet,
URL: "https://from.example.com",
Headers: tc.src,
},
})
if assert.NoError(t, err) {
@ -445,7 +461,7 @@ func TestEvaluator(t *testing.T) {
http.MethodGet,
*mustParseURL("https://from.example.com/"),
nil,
validCertInfo,
ClientCertificateInfo{},
"",
),
})
@ -459,7 +475,7 @@ func TestEvaluator(t *testing.T) {
"POST",
*mustParseURL("https://from.example.com/test"),
nil,
validCertInfo,
ClientCertificateInfo{},
"",
),
})

7
authorize/evaluator/policy_evaluator.go
View file

@ -107,11 +107,16 @@ type PolicyEvaluator struct {
}
// NewPolicyEvaluator creates a new PolicyEvaluator.
func NewPolicyEvaluator(ctx context.Context, store *store.Store, configPolicy *config.Policy) (*PolicyEvaluator, error) {
func NewPolicyEvaluator(
ctx context.Context, store *store.Store, configPolicy *config.Policy, clientCA string,
) (*PolicyEvaluator, error) {
e := new(PolicyEvaluator)
// generate the base rego script for the policy
ppl := configPolicy.ToPPL()
if clientCA != "" {
ppl.AddDefaultClientCertificateRule()
}
base, err := policy.GenerateRegoFromPolicy(ppl)
if err != nil {
return nil, err

53
authorize/evaluator/policy_evaluator_test.go
View file

@ -32,13 +32,15 @@ func TestPolicyEvaluator(t *testing.T) {
privateJWK, err := cryptutil.PrivateJWKFromBytes(encodedSigningKey)
require.NoError(t, err)
var clientCA string
eval := func(t *testing.T, policy *config.Policy, data []proto.Message, input *PolicyRequest) (*PolicyResponse, error) {
ctx := context.Background()
ctx = storage.WithQuerier(ctx, storage.NewStaticQuerier(data...))
store := store.New()
store.UpdateJWTClaimHeaders(config.NewJWTClaimHeaders("email", "groups", "user", "CUSTOM_KEY"))
store.UpdateSigningKey(privateJWK)
e, err := NewPolicyEvaluator(ctx, store, policy)
e, err := NewPolicyEvaluator(ctx, store, policy, clientCA)
require.NoError(t, err)
return e.Evaluate(ctx, input)
}
@ -66,6 +68,40 @@ func TestPolicyEvaluator(t *testing.T) {
}
t.Run("allowed", func(t *testing.T) {
output, err := eval(t,
p1,
[]proto.Message{s1, u1, s2, u2},
&PolicyRequest{
HTTP: RequestHTTP{Method: http.MethodGet, URL: "https://from.example.com/path"},
Session: RequestSession{ID: "s1"},
})
require.NoError(t, err)
assert.Equal(t, &PolicyResponse{
Allow: NewRuleResult(true, criteria.ReasonEmailOK),
Deny: NewRuleResult(false),
Traces: []contextutil.PolicyEvaluationTrace{{Allow: true}},
}, output)
})
t.Run("forbidden", func(t *testing.T) {
output, err := eval(t,
p1,
[]proto.Message{s1, u1, s2, u2},
&PolicyRequest{
HTTP: RequestHTTP{Method: http.MethodGet, URL: "https://from.example.com/path"},
Session: RequestSession{ID: "s2"},
})
require.NoError(t, err)
assert.Equal(t, &PolicyResponse{
Allow: NewRuleResult(false, criteria.ReasonEmailUnauthorized, criteria.ReasonUserUnauthorized),
Deny: NewRuleResult(false),
Traces: []contextutil.PolicyEvaluationTrace{{}},
}, output)
})
// Enable client certificate validation.
clientCA = "---FAKE CA CERTIFICATE---"
t.Run("allowed with cert", func(t *testing.T) {
output, err := eval(t,
p1,
[]proto.Message{s1, u1, s2, u2},
@ -78,7 +114,7 @@ func TestPolicyEvaluator(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, &PolicyResponse{
Allow: NewRuleResult(true, criteria.ReasonEmailOK),
Deny: NewRuleResult(false, criteria.ReasonValidClientCertificateOrNoneRequired),
Deny: NewRuleResult(false, criteria.ReasonValidClientCertificate),
Traces: []contextutil.PolicyEvaluationTrace{{Allow: true}},
}, output)
})
@ -99,7 +135,7 @@ func TestPolicyEvaluator(t *testing.T) {
Traces: []contextutil.PolicyEvaluationTrace{{Allow: true, Deny: true}},
}, output)
})
t.Run("forbidden", func(t *testing.T) {
t.Run("forbidden with cert", func(t *testing.T) {
output, err := eval(t,
p1,
[]proto.Message{s1, u1, s2, u2},
@ -112,10 +148,11 @@ func TestPolicyEvaluator(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, &PolicyResponse{
Allow: NewRuleResult(false, criteria.ReasonEmailUnauthorized, criteria.ReasonUserUnauthorized),
Deny: NewRuleResult(false, criteria.ReasonValidClientCertificateOrNoneRequired),
Deny: NewRuleResult(false, criteria.ReasonValidClientCertificate),
Traces: []contextutil.PolicyEvaluationTrace{{}},
}, output)
})
t.Run("ppl", func(t *testing.T) {
t.Run("allow", func(t *testing.T) {
rego, err := policy.GenerateRegoFromReader(strings.NewReader(`
@ -143,7 +180,7 @@ func TestPolicyEvaluator(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, &PolicyResponse{
Allow: NewRuleResult(true, criteria.ReasonAccept),
Deny: NewRuleResult(false, criteria.ReasonValidClientCertificateOrNoneRequired),
Deny: NewRuleResult(false, criteria.ReasonValidClientCertificate),
Traces: []contextutil.PolicyEvaluationTrace{{}, {ID: "p1", Allow: true}},
}, output)
})
@ -243,7 +280,7 @@ func TestPolicyEvaluator(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, &PolicyResponse{
Allow: NewRuleResult(true),
Deny: NewRuleResult(false, criteria.ReasonValidClientCertificateOrNoneRequired),
Deny: NewRuleResult(false, criteria.ReasonValidClientCertificate),
Traces: []contextutil.PolicyEvaluationTrace{{}, {ID: "p1", Allow: true}},
}, output)
})
@ -266,7 +303,7 @@ func TestPolicyEvaluator(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, &PolicyResponse{
Allow: NewRuleResult(true, criteria.ReasonEmailOK),
Deny: NewRuleResult(false, criteria.ReasonValidClientCertificateOrNoneRequired),
Deny: NewRuleResult(false, criteria.ReasonValidClientCertificate),
Traces: []contextutil.PolicyEvaluationTrace{{Allow: true}},
}, output)
})
@ -290,7 +327,7 @@ func TestPolicyEvaluator(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, &PolicyResponse{
Allow: NewRuleResult(false, criteria.ReasonUserUnauthenticated),
Deny: NewRuleResult(false, criteria.ReasonValidClientCertificateOrNoneRequired),
Deny: NewRuleResult(false, criteria.ReasonValidClientCertificate),
Traces: []contextutil.PolicyEvaluationTrace{{Allow: false}},
}, output)
})