authorize: add "client-certificate-required" reason (#4389)

Add a new reason "client-certificate-required" that will be returned by
the invalid_client_certificate criterion in the case that no client
certificate was provided. Determine this using the new 'presented' field
populated from the Envoy metadata.

authorize/check_response.go

@@ -34,7 +34,7 @@ func (a *Authorize) handleResult(
 ) (*envoy_service_auth_v3.CheckResponse, error) {
 	// If a client certificate is required, but the client did not provide a
 	// valid certificate, deny right away. Do not redirect to authenticate.
-	if result.Deny.Reasons.Has(criteria.ReasonInvalidClientCertificate) {
+	if invalidClientCertReason(result.Deny.Reasons) {
 		return a.handleResultDenied(ctx, in, request, result, result.Deny.Reasons)
 	}
 
@@ -93,7 +93,7 @@ func (a *Authorize) handleResultDenied(
 	case reasons.Has(criteria.ReasonRouteNotFound):
 		denyStatusCode = http.StatusNotFound
 		denyStatusText = httputil.DetailsText(http.StatusNotFound)
-	case reasons.Has(criteria.ReasonInvalidClientCertificate):
+	case invalidClientCertReason(reasons):
 		denyStatusCode = httputil.StatusInvalidClientCertificate
 		denyStatusText = httputil.DetailsText(httputil.StatusInvalidClientCertificate)
 	}
@@ -101,6 +101,11 @@ func (a *Authorize) handleResultDenied(
 	return a.deniedResponse(ctx, in, denyStatusCode, denyStatusText, nil)
 }
 
+func invalidClientCertReason(reasons criteria.Reasons) bool {
+	return reasons.Has(criteria.ReasonClientCertificateRequired) ||
+		reasons.Has(criteria.ReasonInvalidClientCertificate)
+}
+
 func (a *Authorize) okResponse(headers http.Header) *envoy_service_auth_v3.CheckResponse {
 	var requestHeaders []*envoy_config_core_v3.HeaderValueOption
 	for k, vs := range headers {

authorize/check_response_test.go

@@ -89,7 +89,7 @@ func TestAuthorize_handleResult(t *testing.T) {
 		})
 	})
 	t.Run("invalid-client-certificate", func(t *testing.T) {
-		// Even if the user is unauthenticated, if a client certificate was required and no valid
+		// Even if the user is unauthenticated, if a client certificate was required and an invalid
 		// certificate was provided, access should be denied (no login redirect).
 		res, err := a.handleResult(context.Background(),
 			&envoy_service_auth_v3.CheckRequest{},
@@ -101,6 +101,19 @@ func TestAuthorize_handleResult(t *testing.T) {
 		assert.NoError(t, err)
 		assert.Equal(t, 495, int(res.GetDeniedResponse().GetStatus().GetCode()))
 	})
+	t.Run("client-certificate-required", func(t *testing.T) {
+		// Likewise, if a client certificate was required and no certificate
+		// was presented, access should be denied (no login redirect).
+		res, err := a.handleResult(context.Background(),
+			&envoy_service_auth_v3.CheckRequest{},
+			&evaluator.Request{},
+			&evaluator.Result{
+				Allow: evaluator.NewRuleResult(false, criteria.ReasonUserUnauthenticated),
+				Deny:  evaluator.NewRuleResult(true, criteria.ReasonClientCertificateRequired),
+			})
+		assert.NoError(t, err)
+		assert.Equal(t, 495, int(res.GetDeniedResponse().GetStatus().GetCode()))
+	})
 }
 
 func TestAuthorize_okResponse(t *testing.T) {

authorize/evaluator/evaluator_test.go

@@ -131,9 +131,19 @@ func TestEvaluator(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("missing", func(t *testing.T) {
+			res, err := eval(t, options, nil, &Request{
+				Policy: &policies[0],
+			})
+			require.NoError(t, err)
+			assert.Equal(t, NewRuleResult(true, criteria.ReasonClientCertificateRequired), res.Deny)
+		})
 		t.Run("invalid", func(t *testing.T) {
 			res, err := eval(t, options, nil, &Request{
 				Policy: &policies[0],
+				HTTP: RequestHTTP{
+					ClientCertificate: ClientCertificateInfo{Presented: true},
+				},
 			})
 			require.NoError(t, err)
 			assert.Equal(t, NewRuleResult(true, criteria.ReasonInvalidClientCertificate), res.Deny)
@@ -150,11 +160,35 @@ func TestEvaluator(t *testing.T) {
 		})
 	})
 	t.Run("client certificate (per-policy CA)", func(t *testing.T) {
-		t.Run("invalid", func(t *testing.T) {
+		t.Run("missing", func(t *testing.T) {
 			res, err := eval(t, options, nil, &Request{
 				Policy: &policies[10],
 			})
 			require.NoError(t, err)
+			assert.Equal(t, NewRuleResult(true, criteria.ReasonClientCertificateRequired), res.Deny)
+		})
+		t.Run("invalid (Envoy)", func(t *testing.T) {
+			res, err := eval(t, options, nil, &Request{
+				Policy: &policies[10],
+				HTTP: RequestHTTP{
+					ClientCertificate: ClientCertificateInfo{Presented: true},
+				},
+			})
+			require.NoError(t, err)
+			assert.Equal(t, NewRuleResult(true, criteria.ReasonInvalidClientCertificate), res.Deny)
+		})
+		t.Run("invalid (authorize)", func(t *testing.T) {
+			res, err := eval(t, options, nil, &Request{
+				Policy: &policies[10],
+				HTTP: RequestHTTP{
+					ClientCertificate: ClientCertificateInfo{
+						Presented: true,
+						Validated: true,
+						Leaf:      testUnsignedCert,
+					},
+				},
+			})
+			require.NoError(t, err)
 			assert.Equal(t, NewRuleResult(true, criteria.ReasonInvalidClientCertificate), res.Deny)
 		})
 		t.Run("valid", func(t *testing.T) {

authorize/evaluator/policy_evaluator_test.go

@@ -118,7 +118,7 @@ func TestPolicyEvaluator(t *testing.T) {
 			Traces: []contextutil.PolicyEvaluationTrace{{Allow: true}},
 		}, output)
 	})
-	t.Run("invalid cert", func(t *testing.T) {
+	t.Run("no cert", func(t *testing.T) {
 		output, err := eval(t,
 			p1,
 			[]proto.Message{s1, u1, s2, u2},
@@ -129,6 +129,27 @@ func TestPolicyEvaluator(t *testing.T) {
 				IsValidClientCertificate: false,
 			})
 		require.NoError(t, err)
+		assert.Equal(t, &PolicyResponse{
+			Allow:  NewRuleResult(true, criteria.ReasonEmailOK),
+			Deny:   NewRuleResult(true, criteria.ReasonClientCertificateRequired),
+			Traces: []contextutil.PolicyEvaluationTrace{{Allow: true, Deny: true}},
+		}, output)
+	})
+	t.Run("invalid cert", 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",
+					ClientCertificate: ClientCertificateInfo{Presented: true},
+				},
+				Session: RequestSession{ID: "s1"},
+
+				IsValidClientCertificate: false,
+			})
+		require.NoError(t, err)
 		assert.Equal(t, &PolicyResponse{
 			Allow:  NewRuleResult(true, criteria.ReasonEmailOK),
 			Deny:   NewRuleResult(true, criteria.ReasonInvalidClientCertificate),
@@ -241,7 +262,7 @@ func TestPolicyEvaluator(t *testing.T) {
 			require.NoError(t, err)
 			assert.Equal(t, &PolicyResponse{
 				Allow:  NewRuleResult(false),
-				Deny:   NewRuleResult(true, criteria.ReasonAccept, criteria.ReasonInvalidClientCertificate),
+				Deny:   NewRuleResult(true, criteria.ReasonAccept, criteria.ReasonClientCertificateRequired),
 				Traces: []contextutil.PolicyEvaluationTrace{{Deny: true}, {ID: "p1", Deny: true}},
 			}, output)
 		})

pkg/policy/criteria/criteria_test.go

@@ -29,17 +29,22 @@ var testingNow = time.Date(2021, 5, 11, 13, 43, 0, 0, time.Local)
 
 type (
 	Input struct {
-		HTTP    InputHTTP    `json:"http"`
+		HTTP                     InputHTTP    `json:"http"`
-		Session InputSession `json:"session"`
+		Session                  InputSession `json:"session"`
+		IsValidClientCertificate bool         `json:"is_valid_client_certificate"`
 	}
 	InputHTTP struct {
-		Method  string              `json:"method"`
+		Method            string                `json:"method"`
-		Path    string              `json:"path"`
+		Path              string                `json:"path"`
-		Headers map[string][]string `json:"headers"`
+		Headers           map[string][]string   `json:"headers"`
+		ClientCertificate ClientCertificateInfo `json:"client_certificate"`
 	}
 	InputSession struct {
 		ID string `json:"id"`
 	}
+	ClientCertificateInfo struct {
+		Presented bool `json:"presented"`
+	}
 )
 
 func generateRegoFromYAML(raw string) (string, error) {

pkg/policy/criteria/invalid_client_certificate.go

@@ -7,9 +7,14 @@ import (
 	"github.com/pomerium/pomerium/pkg/policy/parser"
 )
 
-var invalidClientCertificateBody = ast.Body{
+var validClientCertificateBody = ast.Body{
 	ast.MustParseExpr(`is_boolean(input.is_valid_client_certificate)`),
-	ast.MustParseExpr(`not input.is_valid_client_certificate`),
+	ast.MustParseExpr(`input.is_valid_client_certificate`),
+}
+
+var noClientCertificateBody = ast.Body{
+	ast.MustParseExpr(`is_boolean(input.http.client_certificate.presented)`),
+	ast.MustParseExpr(`not input.http.client_certificate.presented`),
 }
 
 type invalidClientCertificateCriterion struct {
@@ -25,10 +30,26 @@ func (invalidClientCertificateCriterion) Name() string {
 }
 
 func (c invalidClientCertificateCriterion) GenerateRule(_ string, _ parser.Value) (*ast.Rule, []*ast.Rule, error) {
-	rule := NewCriterionRule(c.g, c.Name(),
+	r1 := c.g.NewRule(c.Name())
-		ReasonInvalidClientCertificate, ReasonValidClientCertificate,
+	r1.Head.Value = NewCriterionTerm(false, ReasonValidClientCertificate)
-		invalidClientCertificateBody)
+	r1.Body = validClientCertificateBody
-	return rule, nil, nil
+
+	r2 := &ast.Rule{
+		Head: &ast.Head{
+			Value: NewCriterionTerm(true, ReasonClientCertificateRequired),
+		},
+		Body: noClientCertificateBody,
+	}
+	r1.Else = r2
+
+	r3 := &ast.Rule{
+		Head: &ast.Head{
+			Value: NewCriterionTerm(true, ReasonInvalidClientCertificate),
+		},
+	}
+	r2.Else = r3
+
+	return r1, nil, nil
 }
 
 // InvalidClientCertificate returns a Criterion which returns true if the

pkg/policy/criteria/invalid_client_certificate_test.go

@@ -0,0 +1,60 @@
+package criteria
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestInvalidClientCertificate(t *testing.T) {
+	t.Parallel()
+
+	cases := []struct {
+		label    string
+		input    Input
+		expected A
+	}{
+		{
+			"not presented",
+			Input{},
+			A{true, A{ReasonClientCertificateRequired}, M{}},
+		},
+		{
+			"invalid",
+			Input{
+				HTTP: InputHTTP{
+					ClientCertificate: ClientCertificateInfo{Presented: true},
+				},
+			},
+			A{true, A{ReasonInvalidClientCertificate}, M{}},
+		},
+		{
+			"valid",
+			Input{
+				HTTP: InputHTTP{
+					ClientCertificate: ClientCertificateInfo{Presented: true},
+				},
+				IsValidClientCertificate: true,
+			},
+			A{false, A{ReasonValidClientCertificate}, M{}},
+		},
+	}
+
+	const policy = `
+deny:
+  or:
+    - invalid_client_certificate: true`
+
+	for i := range cases {
+		c := cases[i]
+		t.Run(c.label, func(t *testing.T) {
+			t.Parallel()
+
+			res, err := evaluate(t, policy, []dataBrokerRecord{}, c.input)
+			require.NoError(t, err)
+			assert.Equal(t, A{false, A{}}, res["allow"])
+			assert.Equal(t, c.expected, res["deny"])
+		})
+	}
+}

pkg/policy/criteria/reasons.go

@@ -7,31 +7,32 @@ type Reason string
 
 // Well-known reasons.
 const (
-	ReasonAccept                   = "accept"
+	ReasonAccept                    = "accept"
-	ReasonClaimOK                  = "claim-ok"
+	ReasonClaimOK                   = "claim-ok"
-	ReasonClaimUnauthorized        = "claim-unauthorized"
+	ReasonClaimUnauthorized         = "claim-unauthorized"
-	ReasonCORSRequest              = "cors-request"
+	ReasonClientCertificateRequired = "client-certificate-required"
-	ReasonDeviceOK                 = "device-ok"
+	ReasonCORSRequest               = "cors-request"
-	ReasonDeviceUnauthenticated    = "device-unauthenticated"
+	ReasonDeviceOK                  = "device-ok"
-	ReasonDeviceUnauthorized       = "device-unauthorized"
+	ReasonDeviceUnauthenticated     = "device-unauthenticated"
-	ReasonDomainOK                 = "domain-ok"
+	ReasonDeviceUnauthorized        = "device-unauthorized"
-	ReasonDomainUnauthorized       = "domain-unauthorized"
+	ReasonDomainOK                  = "domain-ok"
-	ReasonEmailOK                  = "email-ok"
+	ReasonDomainUnauthorized        = "domain-unauthorized"
-	ReasonEmailUnauthorized        = "email-unauthorized"
+	ReasonEmailOK                   = "email-ok"
-	ReasonHTTPMethodOK             = "http-method-ok"
+	ReasonEmailUnauthorized         = "email-unauthorized"
-	ReasonHTTPMethodUnauthorized   = "http-method-unauthorized"
+	ReasonHTTPMethodOK              = "http-method-ok"
-	ReasonHTTPPathOK               = "http-path-ok"
+	ReasonHTTPMethodUnauthorized    = "http-method-unauthorized"
-	ReasonHTTPPathUnauthorized     = "http-path-unauthorized"
+	ReasonHTTPPathOK                = "http-path-ok"
-	ReasonInvalidClientCertificate = "invalid-client-certificate"
+	ReasonHTTPPathUnauthorized      = "http-path-unauthorized"
-	ReasonNonCORSRequest           = "non-cors-request"
+	ReasonInvalidClientCertificate  = "invalid-client-certificate"
-	ReasonNonPomeriumRoute         = "non-pomerium-route"
+	ReasonNonCORSRequest            = "non-cors-request"
-	ReasonPomeriumRoute            = "pomerium-route"
+	ReasonNonPomeriumRoute          = "non-pomerium-route"
-	ReasonReject                   = "reject"
+	ReasonPomeriumRoute             = "pomerium-route"
-	ReasonRouteNotFound            = "route-not-found"
+	ReasonReject                    = "reject"
-	ReasonUserOK                   = "user-ok"
+	ReasonRouteNotFound             = "route-not-found"
-	ReasonUserUnauthenticated      = "user-unauthenticated" // user needs to log in
+	ReasonUserOK                    = "user-ok"
-	ReasonUserUnauthorized         = "user-unauthorized"    // user does not have access
+	ReasonUserUnauthenticated       = "user-unauthenticated" // user needs to log in
-	ReasonValidClientCertificate   = "valid-client-certificate"
+	ReasonUserUnauthorized          = "user-unauthorized"    // user does not have access
+	ReasonValidClientCertificate    = "valid-client-certificate"
 )
 
 // Reasons is a collection of reasons.