From bd63c2ef27820b96f56d6537af0c8e6fe3c605cb Mon Sep 17 00:00:00 2001 From: Magnus Jungsbluth Date: Wed, 16 Aug 2023 10:18:10 +0200 Subject: [PATCH] OPA: authorization based on request body Signed-off-by: Magnus Jungsbluth --- config/config.go | 3 + docs/reference/filters.md | 8 +- filters/openpolicyagent/evaluation.go | 2 +- .../internal/envoy/envoyplugin.go | 6 +- .../internal/envoy/skipperadapter.go | 3 +- .../opaauthorizerequest.go | 17 ++++- .../opaauthorizerequest_test.go | 74 ++++++++++++++++++- .../opaserveresponse/opaserveresponse.go | 10 ++- filters/openpolicyagent/openpolicyagent.go | 55 +++++++++++++- filters/openpolicyagent/response.go | 8 +- skipper.go | 3 +- 11 files changed, 176 insertions(+), 13 deletions(-) diff --git a/config/config.go b/config/config.go index 53231eb4f0..855466d78d 100644 --- a/config/config.go +++ b/config/config.go @@ -279,6 +279,7 @@ type Config struct { OpenPolicyAgentConfigTemplate string `yaml:"open-policy-agent-config-template"` OpenPolicyAgentEnvoyMetadata string `yaml:"open-policy-agent-envoy-metadata"` OpenPolicyAgentCleanerInterval time.Duration `yaml:"open-policy-agent-cleaner-interval"` + OpenPolicyAgentMaxBodySize int `yaml:"open-policy-agent-max-body-size"` } const ( @@ -493,6 +494,7 @@ func NewConfig() *Config { flag.StringVar(&cfg.OpenPolicyAgentConfigTemplate, "open-policy-agent-config-template", "", "file containing a template for an Open Policy Agent configuration file that is interpolated for each OPA filter instance") flag.StringVar(&cfg.OpenPolicyAgentEnvoyMetadata, "open-policy-agent-envoy-metadata", "", "JSON file containing meta-data passed as input for compatibility with Envoy policies in the format") flag.DurationVar(&cfg.OpenPolicyAgentCleanerInterval, "open-policy-agent-cleaner-interval", openpolicyagent.DefaultCleanIdlePeriod, "JSON file containing meta-data passed as input for compatibility with Envoy policies in the format") + flag.IntVar(&cfg.OpenPolicyAgentMaxBodySize, "open-policy-agent-max-body-size", openpolicyagent.DefaultMaxBodySize, "Maximum number of bytes from the body that are passed as input to the policy") // TLS client certs flag.StringVar(&cfg.ClientKeyFile, "client-tls-key", "", "TLS Key file for backend connections, multiple keys may be given comma separated - the order must match the certs") @@ -894,6 +896,7 @@ func (c *Config) ToOptions() skipper.Options { OpenPolicyAgentConfigTemplate: c.OpenPolicyAgentConfigTemplate, OpenPolicyAgentEnvoyMetadata: c.OpenPolicyAgentEnvoyMetadata, OpenPolicyAgentCleanerInterval: c.OpenPolicyAgentCleanerInterval, + OpenPolicyAgentMaxBodySize: c.OpenPolicyAgentMaxBodySize, } for _, rcci := range c.CloneRoute { eskipClone := eskip.NewClone(rcci.Reg, rcci.Repl) diff --git a/docs/reference/filters.md b/docs/reference/filters.md index 3ef8cfa4a8..c8ef32a018 100644 --- a/docs/reference/filters.md +++ b/docs/reference/filters.md @@ -1796,7 +1796,13 @@ The difference is that if the decision in (3) is equivalent to false, the respon Headers both to the upstream and the downstream service can be manipulated the same way this works for [Envoy external authorization](https://www.openpolicyagent.org/docs/latest/envoy-primer/#example-policy-with-additional-controls) -This allows both to add and remove unwanted headers in allow/deny cases. +This allows both to add and remove unwanted headers in allow/deny cases. + +*Authorize based on request body* + +Requests can also be authorized based on the request body the same way that is supported with the [Open Policy Agent Envoy plugin](https://www.openpolicyagent.org/docs/latest/envoy-primer/#example-input), look for the input attribute `parsed_body` in the upstream documentation. + +The body is parsed up to a maximum size that can be configured via the `-open-policy-agent-max-body-size` command line argument. #### opaServeResponse diff --git a/filters/openpolicyagent/evaluation.go b/filters/openpolicyagent/evaluation.go index cc8ed0042c..aebac29fe1 100644 --- a/filters/openpolicyagent/evaluation.go +++ b/filters/openpolicyagent/evaluation.go @@ -41,7 +41,7 @@ func (opa *OpenPolicyAgentInstance) Eval(ctx context.Context, req *ext_authz_v3. } logger := opa.manager.Logger().WithFields(map[string]interface{}{"decision-id": result.DecisionID}) - input, err = envoyauth.RequestToInput(req, logger, nil, true) + input, err = envoyauth.RequestToInput(req, logger, nil, opa.EnvoyPluginConfig().SkipRequestBodyParse) if err != nil { return nil, fmt.Errorf("failed to convert request to input: %w", err) } diff --git a/filters/openpolicyagent/internal/envoy/envoyplugin.go b/filters/openpolicyagent/internal/envoy/envoyplugin.go index a0a88a940f..6e143ae7e4 100644 --- a/filters/openpolicyagent/internal/envoy/envoyplugin.go +++ b/filters/openpolicyagent/internal/envoy/envoyplugin.go @@ -48,8 +48,10 @@ func (p *Plugin) Reconfigure(ctx context.Context, config interface{}) { // PluginConfig represents the plugin configuration. type PluginConfig struct { - Path string `json:"path"` - DryRun bool `json:"dry-run"` + Path string `json:"path"` + DryRun bool `json:"dry-run"` + SkipRequestBodyParse bool `json:"skip-request-body-parse"` + ParsedQuery ast.Body } diff --git a/filters/openpolicyagent/internal/envoy/skipperadapter.go b/filters/openpolicyagent/internal/envoy/skipperadapter.go index 918d9099d7..9f8f0acae9 100644 --- a/filters/openpolicyagent/internal/envoy/skipperadapter.go +++ b/filters/openpolicyagent/internal/envoy/skipperadapter.go @@ -8,7 +8,7 @@ import ( ext_authz_v3 "github.com/envoyproxy/go-control-plane/envoy/service/auth/v3" ) -func AdaptToExtAuthRequest(req *http.Request, metadata *ext_authz_v3_core.Metadata, contextExtensions map[string]string) *ext_authz_v3.CheckRequest { +func AdaptToExtAuthRequest(req *http.Request, metadata *ext_authz_v3_core.Metadata, contextExtensions map[string]string, rawBody []byte) *ext_authz_v3.CheckRequest { headers := make(map[string]string, len(req.Header)) for h, vv := range req.Header { @@ -25,6 +25,7 @@ func AdaptToExtAuthRequest(req *http.Request, metadata *ext_authz_v3_core.Metada Method: req.Method, Path: req.URL.Path, Headers: headers, + RawBody: rawBody, }, }, ContextExtensions: contextExtensions, diff --git a/filters/openpolicyagent/opaauthorizerequest/opaauthorizerequest.go b/filters/openpolicyagent/opaauthorizerequest/opaauthorizerequest.go index b3747d5d5b..399a1e9d89 100644 --- a/filters/openpolicyagent/opaauthorizerequest/opaauthorizerequest.go +++ b/filters/openpolicyagent/opaauthorizerequest/opaauthorizerequest.go @@ -1,6 +1,8 @@ package opaauthorizerequest import ( + "encoding/json" + "errors" "net/http" "time" @@ -86,12 +88,25 @@ func (f *opaAuthorizeRequestFilter) Request(fc filters.FilterContext) { span, ctx := f.opa.StartSpanFromFilterContext(fc) defer span.Finish() - authzreq := envoy.AdaptToExtAuthRequest(req, f.opa.InstanceConfig().GetEnvoyMetadata(), f.envoyContextExtensions) + body, rawBodyBytes, err := f.opa.ExtractHttpBodyOptionally(req) + if err != nil { + f.opa.HandleInvalidDecisionError(fc, span, nil, err, !f.opa.EnvoyPluginConfig().DryRun) + return + } + req.Body = body + + authzreq := envoy.AdaptToExtAuthRequest(req, f.opa.InstanceConfig().GetEnvoyMetadata(), f.envoyContextExtensions, rawBodyBytes) start := time.Now() result, err := f.opa.Eval(ctx, authzreq) fc.Metrics().MeasureSince(f.opa.MetricsKey("eval_time"), start) + var jsonErr *json.SyntaxError + if errors.As(err, &jsonErr) { + f.opa.HandleEvaluationError(fc, span, result, err, !f.opa.EnvoyPluginConfig().DryRun, http.StatusBadRequest) + return + } + if err != nil { f.opa.HandleInvalidDecisionError(fc, span, result, err, !f.opa.EnvoyPluginConfig().DryRun) return diff --git a/filters/openpolicyagent/opaauthorizerequest/opaauthorizerequest_test.go b/filters/openpolicyagent/opaauthorizerequest/opaauthorizerequest_test.go index 41c3169259..9d8e98ed5a 100644 --- a/filters/openpolicyagent/opaauthorizerequest/opaauthorizerequest_test.go +++ b/filters/openpolicyagent/opaauthorizerequest/opaauthorizerequest_test.go @@ -5,6 +5,7 @@ import ( "io" "net/http" "net/http/httptest" + "strings" "testing" opasdktest "github.com/open-policy-agent/opa/sdk/test" @@ -22,6 +23,9 @@ func TestAuthorizeRequestFilter(t *testing.T) { bundleName string regoQuery string requestPath string + requestMethod string + requestHeaders http.Header + body string contextExtensions string expectedBody string expectedHeaders http.Header @@ -34,6 +38,7 @@ func TestAuthorizeRequestFilter(t *testing.T) { bundleName: "somebundle.tar.gz", regoQuery: "envoy/authz/allow", requestPath: "/allow", + requestMethod: "GET", contextExtensions: "", expectedStatus: http.StatusOK, expectedBody: "Welcome!", @@ -46,6 +51,7 @@ func TestAuthorizeRequestFilter(t *testing.T) { bundleName: "somebundle.tar.gz", regoQuery: "envoy/authz/allow_context_extensions", requestPath: "/allow", + requestMethod: "GET", contextExtensions: "com.mycompany.myprop: myvalue", expectedStatus: http.StatusOK, expectedBody: "Welcome!", @@ -58,6 +64,7 @@ func TestAuthorizeRequestFilter(t *testing.T) { bundleName: "somebundle.tar.gz", regoQuery: "envoy/authz/allow", requestPath: "/forbidden", + requestMethod: "GET", contextExtensions: "", expectedStatus: http.StatusForbidden, expectedHeaders: make(http.Header), @@ -69,6 +76,7 @@ func TestAuthorizeRequestFilter(t *testing.T) { bundleName: "somebundle.tar.gz", regoQuery: "envoy/authz/allow_object", requestPath: "/allow/structured", + requestMethod: "GET", contextExtensions: "", expectedStatus: http.StatusOK, expectedBody: "Welcome!", @@ -81,6 +89,7 @@ func TestAuthorizeRequestFilter(t *testing.T) { bundleName: "somebundle.tar.gz", regoQuery: "envoy/authz/allow_object", requestPath: "/forbidden", + requestMethod: "GET", contextExtensions: "", expectedStatus: http.StatusUnauthorized, expectedHeaders: map[string][]string{"X-Ext-Auth-Allow": {"no"}}, @@ -93,6 +102,7 @@ func TestAuthorizeRequestFilter(t *testing.T) { bundleName: "somebundle.tar.gz", regoQuery: "envoy/authz/invalid_path", requestPath: "/allow", + requestMethod: "GET", contextExtensions: "", expectedStatus: http.StatusInternalServerError, expectedBody: "", @@ -105,6 +115,7 @@ func TestAuthorizeRequestFilter(t *testing.T) { bundleName: "somebundle.tar.gz", regoQuery: "envoy/authz/allow_wrong_type", requestPath: "/allow", + requestMethod: "GET", contextExtensions: "", expectedStatus: http.StatusInternalServerError, expectedBody: "", @@ -117,6 +128,7 @@ func TestAuthorizeRequestFilter(t *testing.T) { bundleName: "somebundle.tar.gz", regoQuery: "envoy/authz/allow_object_invalid_headers_to_remove", requestPath: "/allow", + requestMethod: "GET", contextExtensions: "", expectedStatus: http.StatusInternalServerError, expectedBody: "", @@ -129,6 +141,7 @@ func TestAuthorizeRequestFilter(t *testing.T) { bundleName: "somebundle.tar.gz", regoQuery: "envoy/authz/allow_object_invalid_headers", requestPath: "/allow", + requestMethod: "GET", contextExtensions: "", expectedStatus: http.StatusInternalServerError, expectedBody: "", @@ -136,6 +149,48 @@ func TestAuthorizeRequestFilter(t *testing.T) { backendHeaders: make(http.Header), removeHeaders: make(http.Header), }, + { + msg: "Allow With Body", + bundleName: "somebundle.tar.gz", + regoQuery: "envoy/authz/allow_body", + requestMethod: "POST", + body: `{ "target_id" : "123456" }`, + requestHeaders: map[string][]string{"content-type": {"application/json"}}, + requestPath: "/allow_body", + expectedStatus: http.StatusOK, + expectedBody: "Welcome!", + expectedHeaders: make(http.Header), + backendHeaders: make(http.Header), + removeHeaders: make(http.Header), + }, + { + msg: "Forbidden With Body", + bundleName: "somebundle.tar.gz", + regoQuery: "envoy/authz/allow_body", + requestMethod: "POST", + body: `{ "target_id" : "wrong id" }`, + requestHeaders: map[string][]string{"content-type": {"application/json"}}, + requestPath: "/allow_body", + expectedStatus: http.StatusForbidden, + expectedBody: "", + expectedHeaders: make(http.Header), + backendHeaders: make(http.Header), + removeHeaders: make(http.Header), + }, + { + msg: "Broken Body", + bundleName: "somebundle.tar.gz", + regoQuery: "envoy/authz/allow_body", + requestMethod: "POST", + body: `{ "target_id" / "wrong id" }`, + requestHeaders: map[string][]string{"content-type": {"application/json"}}, + requestPath: "/allow_body", + expectedStatus: http.StatusBadRequest, + expectedBody: "", + expectedHeaders: make(http.Header), + backendHeaders: make(http.Header), + removeHeaders: make(http.Header), + }, } { t.Run(ti.msg, func(t *testing.T) { t.Logf("Running test for %v", ti) @@ -143,6 +198,12 @@ func TestAuthorizeRequestFilter(t *testing.T) { w.Write([]byte("Welcome!")) assert.True(t, isHeadersPresent(t, ti.backendHeaders, r.Header), "Enriched request header is absent.") assert.True(t, isHeadersAbsent(t, ti.removeHeaders, r.Header), "Unwanted HTTP Headers present.") + + body, err := io.ReadAll(r.Body) + if err != nil { + t.Fatal(err) + } + assert.Equal(t, ti.body, string(body)) })) opaControlPlane := opasdktest.MustNewServer( @@ -192,6 +253,12 @@ func TestAuthorizeRequestFilter(t *testing.T) { "allowed": true, "headers": "bogus string instead of object" } + + default allow_body = false + + allow_body { + input.parsed_body.target_id == "123456" + } `, }), ) @@ -225,12 +292,17 @@ func TestAuthorizeRequestFilter(t *testing.T) { proxy := proxytest.New(fr, r...) - req, err := http.NewRequest("GET", proxy.URL+ti.requestPath, nil) + req, err := http.NewRequest(ti.requestMethod, proxy.URL+ti.requestPath, strings.NewReader(ti.body)) for name, values := range ti.removeHeaders { for _, value := range values { req.Header.Add(name, value) //adding the headers to validate removal. } } + for name, values := range ti.requestHeaders { + for _, value := range values { + req.Header.Add(name, value) + } + } assert.NoError(t, err) diff --git a/filters/openpolicyagent/opaserveresponse/opaserveresponse.go b/filters/openpolicyagent/opaserveresponse/opaserveresponse.go index 6134fa0d74..50feb45e6c 100644 --- a/filters/openpolicyagent/opaserveresponse/opaserveresponse.go +++ b/filters/openpolicyagent/opaserveresponse/opaserveresponse.go @@ -83,7 +83,15 @@ func (f *opaServeResponseFilter) Request(fc filters.FilterContext) { span, ctx := f.opa.StartSpanFromFilterContext(fc) defer span.Finish() - authzreq := envoy.AdaptToExtAuthRequest(fc.Request(), f.opa.InstanceConfig().GetEnvoyMetadata(), f.envoyContextExtensions) + req := fc.Request() + body, rawBodyBytes, err := f.opa.ExtractHttpBodyOptionally(req) + if err != nil { + f.opa.ServeInvalidDecisionError(fc, span, nil, err) + return + } + req.Body = body + + authzreq := envoy.AdaptToExtAuthRequest(fc.Request(), f.opa.InstanceConfig().GetEnvoyMetadata(), f.envoyContextExtensions, rawBodyBytes) start := time.Now() result, err := f.opa.Eval(ctx, authzreq) diff --git a/filters/openpolicyagent/openpolicyagent.go b/filters/openpolicyagent/openpolicyagent.go index 56aa581131..ba824625f8 100644 --- a/filters/openpolicyagent/openpolicyagent.go +++ b/filters/openpolicyagent/openpolicyagent.go @@ -1,9 +1,12 @@ package openpolicyagent import ( + "bufio" "bytes" "context" "fmt" + "io" + "net/http" "os" "strings" "sync" @@ -37,6 +40,7 @@ const ( defaultReuseDuration = 30 * time.Second defaultShutdownGracePeriod = 30 * time.Second DefaultCleanIdlePeriod = 10 * time.Second + DefaultMaxBodySize = 128 * 1024 * 1024 ) type OpenPolicyAgentRegistry struct { @@ -55,6 +59,7 @@ type OpenPolicyAgentRegistry struct { quit chan struct{} reuseDuration time.Duration cleanInterval time.Duration + maxBodyBytes int } type OpenPolicyAgentFilter interface { @@ -68,6 +73,13 @@ func WithReuseDuration(duration time.Duration) func(*OpenPolicyAgentRegistry) er } } +func WithMaxBodyBytes(n int) func(*OpenPolicyAgentRegistry) error { + return func(cfg *OpenPolicyAgentRegistry) error { + cfg.maxBodyBytes = n + return nil + } +} + func WithCleanInterval(interval time.Duration) func(*OpenPolicyAgentRegistry) error { return func(cfg *OpenPolicyAgentRegistry) error { cfg.cleanInterval = interval @@ -82,6 +94,7 @@ func NewOpenPolicyAgentRegistry(opts ...func(*OpenPolicyAgentRegistry) error) *O instances: make(map[string]*OpenPolicyAgentInstance), lastused: make(map[*OpenPolicyAgentInstance]time.Time), quit: make(chan struct{}), + maxBodyBytes: DefaultMaxBodySize, } for _, opt := range opts { @@ -282,7 +295,7 @@ func (registry *OpenPolicyAgentRegistry) newOpenPolicyAgentInstance(bundleName s return nil, err } - engine, err := New(inmem.New(), configBytes, config, filterName, bundleName) + engine, err := New(inmem.New(), configBytes, config, filterName, bundleName, registry.maxBodyBytes) if err != nil { return nil, err } @@ -306,6 +319,7 @@ type OpenPolicyAgentInstance struct { instanceConfig OpenPolicyAgentInstanceConfig opaConfig *config.Config bundleName string + maxBodyBytes int preparedQuery *rego.PreparedEvalQuery preparedQueryDoOnce *sync.Once interQueryBuiltinCache iCache.InterQueryCache @@ -343,7 +357,7 @@ func interpolateConfigTemplate(configTemplate []byte, bundleName string) ([]byte } // New returns a new OPA object. -func New(store storage.Store, configBytes []byte, instanceConfig OpenPolicyAgentInstanceConfig, filterName string, bundleName string) (*OpenPolicyAgentInstance, error) { +func New(store storage.Store, configBytes []byte, instanceConfig OpenPolicyAgentInstanceConfig, filterName string, bundleName string, maxBodyBytes int) (*OpenPolicyAgentInstance, error) { id := uuid.New().String() opaConfig, err := config.ParseConfig(configBytes, id) @@ -372,6 +386,7 @@ func New(store storage.Store, configBytes []byte, instanceConfig OpenPolicyAgent manager: manager, opaConfig: opaConfig, bundleName: bundleName, + maxBodyBytes: maxBodyBytes, preparedQueryDoOnce: new(sync.Once), interQueryBuiltinCache: iCache.NewInterQueryCache(manager.InterQueryBuiltinCacheConfig()), @@ -470,6 +485,42 @@ func (opa *OpenPolicyAgentInstance) MetricsKey(key string) string { return key + "." + opa.bundleName } +type bufferedReadCloser struct { + reader *bufio.Reader + closer io.ReadCloser +} + +func (r *bufferedReadCloser) Read(p []byte) (n int, err error) { + return r.reader.Read(p) +} + +func (r *bufferedReadCloser) Close() error { + return r.closer.Close() +} + +func (opa *OpenPolicyAgentInstance) ExtractHttpBodyOptionally(req *http.Request) (io.ReadCloser, []byte, error) { + body := req.Body + var rawBody []byte + if body != nil && !opa.EnvoyPluginConfig().SkipRequestBodyParse && opa.maxBodyBytes > 0 { + bufferedReader := bufio.NewReaderSize(body, opa.maxBodyBytes) + wrapper := &bufferedReadCloser{ + closer: req.Body, + reader: bufferedReader, + } + + var err error + rawBody, err = bufferedReader.Peek(opa.maxBodyBytes) + + if err != io.EOF { + return wrapper, nil, fmt.Errorf("cannot read http body: %w", err) + } + + return wrapper, rawBody, nil + } + + return req.Body, nil, nil +} + // ParsedQuery is an implementation of the envoyauth.EvalContext interface func (opa *OpenPolicyAgentInstance) ParsedQuery() ast.Body { return opa.EnvoyPluginConfig().ParsedQuery diff --git a/filters/openpolicyagent/response.go b/filters/openpolicyagent/response.go index df613b1716..7a35459bd1 100644 --- a/filters/openpolicyagent/response.go +++ b/filters/openpolicyagent/response.go @@ -16,6 +16,10 @@ func (opa *OpenPolicyAgentInstance) ServeInvalidDecisionError(fc filters.FilterC } func (opa *OpenPolicyAgentInstance) HandleInvalidDecisionError(fc filters.FilterContext, span opentracing.Span, result *envoyauth.EvalResult, err error, serve bool) { + opa.HandleEvaluationError(fc, span, result, err, serve, http.StatusInternalServerError) +} + +func (opa *OpenPolicyAgentInstance) HandleEvaluationError(fc filters.FilterContext, span opentracing.Span, result *envoyauth.EvalResult, err error, serve bool, status int) { fc.Metrics().IncCounter(opa.MetricsKey("decision.err")) span.SetTag("error", true) @@ -39,12 +43,12 @@ func (opa *OpenPolicyAgentInstance) HandleInvalidDecisionError(fc filters.Filter opa.Logger().WithFields(map[string]interface{}{ "err": err, - }).Info("Rejecting request because of an invalid decision") + }).Info("Rejecting request because of an error") } if serve { resp := http.Response{} - resp.StatusCode = http.StatusInternalServerError + resp.StatusCode = status fc.Serve(&resp) } diff --git a/skipper.go b/skipper.go index 1f4af8f91a..3a60c915f6 100644 --- a/skipper.go +++ b/skipper.go @@ -900,6 +900,7 @@ type Options struct { OpenPolicyAgentConfigTemplate string OpenPolicyAgentEnvoyMetadata string OpenPolicyAgentCleanerInterval time.Duration + OpenPolicyAgentMaxBodySize int } func (o *Options) KubernetesDataClientOptions() kubernetes.Options { @@ -1763,7 +1764,7 @@ func run(o Options, sig chan os.Signal, idleConnsCH chan struct{}) error { var opaRegistry *openpolicyagent.OpenPolicyAgentRegistry if o.EnableOpenPolicyAgent { - opaRegistry = openpolicyagent.NewOpenPolicyAgentRegistry(openpolicyagent.WithCleanInterval(o.OpenPolicyAgentCleanerInterval)) + opaRegistry := openpolicyagent.NewOpenPolicyAgentRegistry(openpolicyagent.WithMaxBodyBytes(o.OpenPolicyAgentMaxBodySize), openpolicyagent.WithCleanInterval(o.OpenPolicyAgentCleanerInterval)) defer opaRegistry.Close() opts := make([]func(*openpolicyagent.OpenPolicyAgentInstanceConfig) error, 0)