Renames, more tests, more docs

Signed-off-by: Magnus Jungsbluth <[email protected]>

config/config.go

@@ -278,12 +278,12 @@ type Config struct {
 	LuaModules *listFlag `yaml:"lua-modules"`
 	LuaSources *listFlag `yaml:"lua-sources"`
 
-	EnableOpenPolicyAgent             bool          `yaml:"enable-open-policy-agent"`
-	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"`
-	OpenPolicyAgentMaxRequestBodySize int64         `yaml:"open-policy-agent-max-request-body-size"`
-	OpenPolicyAgentMaxTotalBodySize   int64         `yaml:"open-policy-agent-max-total-body-size"`
+	EnableOpenPolicyAgent               bool          `yaml:"enable-open-policy-agent"`
+	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"`
+	OpenPolicyAgentMaxRequestBodySize   int64         `yaml:"open-policy-agent-max-request-body-size"`
+	OpenPolicyAgentMaxMemoryBodyParsing int64         `yaml:"open-policy-agent-max-memory-body-parsing"`
 }
 
 const (
@@ -502,7 +502,7 @@ func NewConfig() *Config {
 	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.Int64Var(&cfg.OpenPolicyAgentMaxRequestBodySize, "open-policy-agent-max-request-body-size", openpolicyagent.DefaultMaxRequestBodySize, "Maximum number of bytes from a http request body that are passed as input to the policy")
-	flag.Int64Var(&cfg.OpenPolicyAgentMaxTotalBodySize, "open-policy-agent-max-total-body-size", openpolicyagent.DefaultMaxTotalBodySize, "Total number of bytes used to parse http request bodies across all requests. Once the limit is met, requests will be rejected.")
+	flag.Int64Var(&cfg.OpenPolicyAgentMaxMemoryBodyParsing, "open-policy-agent-max-memory-body-parsing", openpolicyagent.DefaultMaxMemoryBodyParsing, "Total number of bytes used to parse http request bodies across all requests. Once the limit is met, requests will be rejected.")
 
 	// 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")
@@ -903,12 +903,12 @@ func (c *Config) ToOptions() skipper.Options {
 		LuaModules: c.LuaModules.values,
 		LuaSources: c.LuaSources.values,
 
-		EnableOpenPolicyAgent:             c.EnableOpenPolicyAgent,
-		OpenPolicyAgentConfigTemplate:     c.OpenPolicyAgentConfigTemplate,
-		OpenPolicyAgentEnvoyMetadata:      c.OpenPolicyAgentEnvoyMetadata,
-		OpenPolicyAgentCleanerInterval:    c.OpenPolicyAgentCleanerInterval,
-		OpenPolicyAgentMaxRequestBodySize: c.OpenPolicyAgentMaxRequestBodySize,
-		OpenPolicyAgentMaxTotalBodySize:   c.OpenPolicyAgentMaxTotalBodySize,
+		EnableOpenPolicyAgent:               c.EnableOpenPolicyAgent,
+		OpenPolicyAgentConfigTemplate:       c.OpenPolicyAgentConfigTemplate,
+		OpenPolicyAgentEnvoyMetadata:        c.OpenPolicyAgentEnvoyMetadata,
+		OpenPolicyAgentCleanerInterval:      c.OpenPolicyAgentCleanerInterval,
+		OpenPolicyAgentMaxRequestBodySize:   c.OpenPolicyAgentMaxRequestBodySize,
+		OpenPolicyAgentMaxMemoryBodyParsing: c.OpenPolicyAgentMaxMemoryBodyParsing,
 	}
 	for _, rcci := range c.CloneRoute {
 		eskipClone := eskip.NewClone(rcci.Reg, rcci.Repl)

config/config_test.go

@@ -163,7 +163,7 @@ func defaultConfig() *Config {
 		LuaSources:                              commaListFlag(),
 		OpenPolicyAgentCleanerInterval:          openpolicyagent.DefaultCleanIdlePeriod,
 		OpenPolicyAgentMaxRequestBodySize:       openpolicyagent.DefaultMaxRequestBodySize,
-		OpenPolicyAgentMaxTotalBodySize:         openpolicyagent.DefaultMaxTotalBodySize,
+		OpenPolicyAgentMaxMemoryBodyParsing:     openpolicyagent.DefaultMaxMemoryBodyParsing,
 	}
 }
 

docs/reference/filters.md

@@ -1804,7 +1804,7 @@ Requests can also be authorized based on the request body the same way that is s
 
 This filter has the same parameters that the `opaAuthorizeRequest` filter has.
 
-The body is parsed up to a maximum size with a default of 1MB that can be configured via the `-open-policy-agent-max-request-body-size` command line argument. To avoid OOM errors due to too many authorized body requests, another flag `-open-policy-agent-max-total-body-size` controls how much memory can be used across all requests with a default of 100MB. If  in-flight requests that use body authorization exceed that limit, incoming requests that use the body will be rejected with an internal server error. 
+A request's body is parsed up to a maximum size with a default of 1MB that can be configured via the `-open-policy-agent-max-request-body-size` command line argument. To avoid OOM errors due to too many concurrent authorized body requests, another flag `-open-policy-agent-max-memory-body-parsing` controls how much memory can be used across all requests with a default of 100MB. If in-flight requests that use body authorization exceed that limit, incoming requests that use the body will be rejected with an internal server error. The number of concurrent requests is <max-memory-body-parsing> / min(avg(<request content-length>), <max-request-body-size>), so if requests on average have 100KB and the maximum memory is set to 100MB, on average 1024 authorized requests can be processed concurrently. 
 
 The filter also honors the `skip-request-body-parse` of the corresponding [configuration](https://www.openpolicyagent.org/docs/latest/envoy-introduction/#configuration) that the OPA plugin uses. 
 
@@ -1857,7 +1857,7 @@ If you want to serve requests directly from an Open Policy Agent policy that use
 
 This filter has the same parameters that the `opaServeResponse` filter has.
 
-The body is parsed up to a maximum size with a default of 1MB that can be configured via the `-open-policy-agent-max-request-body-size` command line argument. To avoid OOM errors due to too many authorized body requests, another flag `-open-policy-agent-max-total-body-size` controls how much memory can be used across all requests with a default of 100MB. If  in-flight requests that use body authorization exceed that limit, incoming requests that use the body will be rejected with an internal server error. 
+A request's body is parsed up to a maximum size with a default of 1MB that can be configured via the `-open-policy-agent-max-request-body-size` command line argument. To avoid OOM errors due to too many concurrent authorized body requests, another flag `-open-policy-agent-max-memory-body-parsing` controls how much memory can be used across all requests with a default of 100MB. If  in-flight requests that use body authorization exceed that limit, incoming requests that use the body will be rejected with an internal server error. The number of concurrent requests is <max-memory-body-parsing> / min(avg(<request content-length>), <max-request-body-size>), so if requests on average have 100KB and the maximum memory is set to 100MB, on average 1024 authorized requests can be processed concurrently.
 
 The filter also honors the `skip-request-body-parse` of the corresponding [configuration](https://www.openpolicyagent.org/docs/latest/envoy-introduction/#configuration) that the OPA plugin uses. 
 

filters/openpolicyagent/opaauthorizerequest/opaauthorizerequest_test.go

@@ -190,6 +190,20 @@ func TestAuthorizeRequestFilter(t *testing.T) {
 			backendHeaders:  make(http.Header),
 			removeHeaders:   make(http.Header),
 		},
+		{
+			msg:             "GET against body protected endpoint",
+			filterName:      "opaAuthorizeRequestWithBody",
+			bundleName:      "somebundle.tar.gz",
+			regoQuery:       "envoy/authz/allow_body",
+			requestMethod:   "GET",
+			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",
 			filterName:      "opaAuthorizeRequestWithBody",
@@ -328,7 +342,12 @@ func TestAuthorizeRequestFilter(t *testing.T) {
 
 			proxy := proxytest.New(fr, r...)
 
-			req, err := http.NewRequest(ti.requestMethod, proxy.URL+ti.requestPath, strings.NewReader(ti.body))
+			var bodyReader io.Reader
+			if ti.body != "" {
+				bodyReader = strings.NewReader(ti.body)
+			}
+
+			req, err := http.NewRequest(ti.requestMethod, proxy.URL+ti.requestPath, bodyReader)
 			for name, values := range ti.removeHeaders {
 				for _, value := range values {
 					req.Header.Add(name, value) //adding the headers to validate removal.

filters/openpolicyagent/openpolicyagent.go

@@ -39,12 +39,12 @@ import (
 )
 
 const (
-	DefaultCleanIdlePeriod     = 10 * time.Second
-	DefaultMaxRequestBodySize  = 1 << 20 // 1 MB
-	DefaultMaxTotalBodySize    = 100 * DefaultMaxRequestBodySize
-	defaultReuseDuration       = 30 * time.Second
-	defaultShutdownGracePeriod = 30 * time.Second
-	defaultBodyBufferSize      = 8192 * 1024
+	DefaultCleanIdlePeriod      = 10 * time.Second
+	DefaultMaxRequestBodySize   = 1 << 20 // 1 MB
+	DefaultMaxMemoryBodyParsing = 100 * DefaultMaxRequestBodySize
+	defaultReuseDuration        = 30 * time.Second
+	defaultShutdownGracePeriod  = 30 * time.Second
+	defaultBodyBufferSize       = 8192 * 1024
 )
 
 type OpenPolicyAgentRegistry struct {
@@ -64,9 +64,9 @@ type OpenPolicyAgentRegistry struct {
 	reuseDuration time.Duration
 	cleanInterval time.Duration
 
-	totalBodySizeSem    *semaphore.Weighted
-	maxRequestBodyBytes int64
-	bodyReadBufferSize  int64
+	maxMemoryBodyParsingSem *semaphore.Weighted
+	maxRequestBodyBytes     int64
+	bodyReadBufferSize      int64
 }
 
 type OpenPolicyAgentFilter interface {
@@ -87,9 +87,9 @@ func WithMaxRequestBodyBytes(n int64) func(*OpenPolicyAgentRegistry) error {
 	}
 }
 
-func WithMaxTotalBodyBytes(n int64) func(*OpenPolicyAgentRegistry) error {
+func WithMaxMemoryBodyParsing(n int64) func(*OpenPolicyAgentRegistry) error {
 	return func(cfg *OpenPolicyAgentRegistry) error {
-		cfg.totalBodySizeSem = semaphore.NewWeighted(n)
+		cfg.maxMemoryBodyParsingSem = semaphore.NewWeighted(n)
 		return nil
 	}
 }
@@ -115,16 +115,16 @@ func NewOpenPolicyAgentRegistry(opts ...func(*OpenPolicyAgentRegistry) error) *O
 		instances:           make(map[string]*OpenPolicyAgentInstance),
 		lastused:            make(map[*OpenPolicyAgentInstance]time.Time),
 		quit:                make(chan struct{}),
-		maxRequestBodyBytes: DefaultMaxTotalBodySize,
+		maxRequestBodyBytes: DefaultMaxMemoryBodyParsing,
 		bodyReadBufferSize:  defaultBodyBufferSize,
 	}
 
 	for _, opt := range opts {
 		opt(registry)
 	}
 
-	if registry.totalBodySizeSem == nil {
-		registry.totalBodySizeSem = semaphore.NewWeighted(DefaultMaxTotalBodySize)
+	if registry.maxMemoryBodyParsingSem == nil {
+		registry.maxMemoryBodyParsingSem = semaphore.NewWeighted(DefaultMaxMemoryBodyParsing)
 	}
 
 	go registry.startCleanerDaemon()
@@ -616,12 +616,12 @@ func (opa *OpenPolicyAgentInstance) ExtractHttpBodyOptionally(req *http.Request)
 				wrapper := newBufferedBodyReader(req.Body, opa.maxBodyBytes, opa.bodyReadBufferSize)
 
 				requestedBodyBytes := bodyUpperBound(req.ContentLength, opa.maxBodyBytes)
-				if !opa.registry.totalBodySizeSem.TryAcquire(requestedBodyBytes) {
+				if !opa.registry.maxMemoryBodyParsingSem.TryAcquire(requestedBodyBytes) {
 					return req.Body, nil, func() {}, ErrTotalBodyBytesExceeded
 				}
 
 				rawBody, err := wrapper.fillBuffer(req.ContentLength)
-				return wrapper, rawBody, func() { opa.registry.totalBodySizeSem.Release(requestedBodyBytes) }, err
+				return wrapper, rawBody, func() { opa.registry.maxMemoryBodyParsingSem.Release(requestedBodyBytes) }, err
 			}
 		}
 	}

filters/openpolicyagent/openpolicyagent_test.go

@@ -431,7 +431,7 @@ func TestBodyExtractionExhausingTotalBytes(t *testing.T) {
 
 	registry := NewOpenPolicyAgentRegistry(WithMaxRequestBodyBytes(21),
 		WithReadBodyBufferSize(21),
-		WithMaxTotalBodyBytes(40))
+		WithMaxMemoryBodyParsing(40))
 
 	cfg, err := NewOpenPolicyAgentConfig(WithConfigTemplate(config))
 	assert.NoError(t, err)
@@ -466,6 +466,69 @@ func TestBodyExtractionExhausingTotalBytes(t *testing.T) {
 	}
 
 	_, _, f3, err := inst.ExtractHttpBodyOptionally(&req3)
-	defer f3()
+	f3()
 	assert.NoError(t, err)
 }
+
+func TestBodyExtractionEmptyBody(t *testing.T) {
+
+	_, config := mockControlPlane()
+
+	registry := NewOpenPolicyAgentRegistry(WithMaxRequestBodyBytes(21),
+		WithReadBodyBufferSize(21),
+		WithMaxMemoryBodyParsing(40))
+
+	cfg, err := NewOpenPolicyAgentConfig(WithConfigTemplate(config))
+	assert.NoError(t, err)
+
+	inst, err := registry.NewOpenPolicyAgentInstance("use_body", *cfg, "testfilter")
+	assert.NoError(t, err)
+
+	req1 := http.Request{
+		ContentLength: 0,
+		Body:          nil,
+	}
+
+	body, bodybytes, f1, err := inst.ExtractHttpBodyOptionally(&req1)
+	assert.NoError(t, err)
+	assert.Nil(t, body)
+	assert.Nil(t, bodybytes)
+
+	f1()
+}
+
+func TestBodyExtractionUnknownBody(t *testing.T) {
+
+	_, config := mockControlPlane()
+
+	registry := NewOpenPolicyAgentRegistry(WithMaxRequestBodyBytes(21),
+		WithReadBodyBufferSize(21),
+		WithMaxMemoryBodyParsing(21))
+
+	cfg, err := NewOpenPolicyAgentConfig(WithConfigTemplate(config))
+	assert.NoError(t, err)
+
+	inst, err := registry.NewOpenPolicyAgentInstance("use_body", *cfg, "testfilter")
+	assert.NoError(t, err)
+
+	req1 := http.Request{
+		ContentLength: -1,
+		Body:          io.NopCloser(bytes.NewReader([]byte(`{ "welcome": "world" }`))),
+	}
+
+	_, _, f1, err := inst.ExtractHttpBodyOptionally(&req1)
+	assert.NoError(t, err)
+
+	req2 := http.Request{
+		ContentLength: 3,
+		Body:          io.NopCloser(bytes.NewReader([]byte(`{ }`))),
+	}
+
+	_, _, f2, err := inst.ExtractHttpBodyOptionally(&req2)
+	if assert.Error(t, err) {
+		assert.Equal(t, ErrTotalBodyBytesExceeded, err)
+	}
+
+	f1()
+	f2()
+}

skipper.go

@@ -912,12 +912,12 @@ type Options struct {
 	// filters.
 	LuaSources []string
 
-	EnableOpenPolicyAgent             bool
-	OpenPolicyAgentConfigTemplate     string
-	OpenPolicyAgentEnvoyMetadata      string
-	OpenPolicyAgentCleanerInterval    time.Duration
-	OpenPolicyAgentMaxRequestBodySize int64
-	OpenPolicyAgentMaxTotalBodySize   int64
+	EnableOpenPolicyAgent               bool
+	OpenPolicyAgentConfigTemplate       string
+	OpenPolicyAgentEnvoyMetadata        string
+	OpenPolicyAgentCleanerInterval      time.Duration
+	OpenPolicyAgentMaxRequestBodySize   int64
+	OpenPolicyAgentMaxMemoryBodyParsing int64
 }
 
 func (o *Options) KubernetesDataClientOptions() kubernetes.Options {
@@ -1798,7 +1798,7 @@ func run(o Options, sig chan os.Signal, idleConnsCH chan struct{}) error {
 	if o.EnableOpenPolicyAgent {
 		opaRegistry := openpolicyagent.NewOpenPolicyAgentRegistry(
 			openpolicyagent.WithMaxRequestBodyBytes(o.OpenPolicyAgentMaxRequestBodySize),
-			openpolicyagent.WithMaxTotalBodyBytes(o.OpenPolicyAgentMaxTotalBodySize),
+			openpolicyagent.WithMaxMemoryBodyParsing(o.OpenPolicyAgentMaxMemoryBodyParsing),
 			openpolicyagent.WithCleanInterval(o.OpenPolicyAgentCleanerInterval))
 		defer opaRegistry.Close()