From 9d99d1515578192a12b1960564621f06525b21eb Mon Sep 17 00:00:00 2001 From: Sunjay Bhatia <5337253+sunjayBhatia@users.noreply.github.com> Date: Thu, 12 Oct 2023 15:29:48 -0400 Subject: [PATCH 1/3] Add configurability for HTTP requests per IO cycle (#5827) An additional mitigation to CVE-2023-44487 available in Envoy 1.27.1. This change allows configuring the http.max_requests_per_io_cycle Envoy runtime setting via Contour configuration to allow administrators of Contour to prevent abusive connections from starving resources from others. The default is left as the existing behavior, that is no limit, so as not to impact existing valid traffic. See the Envoy release notes for more information: https://www.envoyproxy.io/docs/envoy/v1.27.1/version_history/v1.27/v1.27.1 Signed-off-by: Sunjay Bhatia --- apis/projectcontour/v1alpha1/contourconfig.go | 10 +++ .../v1alpha1/zz_generated.deepcopy.go | 5 ++ .../unreleased/5827-sunjayBhatia-minor.md | 16 ++++ cmd/contour/serve.go | 4 +- cmd/contour/servecontext.go | 1 + cmd/contour/servecontext_test.go | 10 +++ examples/contour/01-crds.yaml | 23 ++++++ examples/render/contour-deployment.yaml | 23 ++++++ .../render/contour-gateway-provisioner.yaml | 23 ++++++ examples/render/contour-gateway.yaml | 23 ++++++ examples/render/contour.yaml | 23 ++++++ internal/envoy/v3/runtime.go | 12 ++- internal/envoy/v3/runtime_test.go | 41 +++++++--- internal/xdscache/v3/runtime.go | 30 +++++-- internal/xdscache/v3/runtime_test.go | 78 ++++++++++++++----- internal/xdscache/v3/server_test.go | 2 +- pkg/config/parameters.go | 11 +++ pkg/config/parameters_test.go | 16 ++++ .../docs/main/config/api-reference.html | 17 ++++ site/content/docs/main/configuration.md | 1 + 20 files changed, 329 insertions(+), 40 deletions(-) create mode 100644 changelogs/unreleased/5827-sunjayBhatia-minor.md diff --git a/apis/projectcontour/v1alpha1/contourconfig.go b/apis/projectcontour/v1alpha1/contourconfig.go index 5b6eb1ad8bd..e0ec26c597b 100644 --- a/apis/projectcontour/v1alpha1/contourconfig.go +++ b/apis/projectcontour/v1alpha1/contourconfig.go @@ -361,6 +361,16 @@ type EnvoyListenerConfig struct { // TLS holds various configurable Envoy TLS listener values. // +optional TLS *EnvoyTLS `json:"tls,omitempty"` + + // Defines the limit on number of HTTP requests that Envoy will process from a single + // connection in a single I/O cycle. Requests over this limit are processed in subsequent + // I/O cycles. Can be used as a mitigation for CVE-2023-44487 when abusive traffic is + // detected. Configures the http.max_requests_per_io_cycle Envoy runtime setting. The default + // value when this is not set is no limit. + // + // +kubebuilder:validation:Minimum=1 + // +optional + MaxRequestsPerIOCycle *uint32 `json:"maxRequestsPerIOCycle"` } // EnvoyTLS describes tls parameters for Envoy listneners. diff --git a/apis/projectcontour/v1alpha1/zz_generated.deepcopy.go b/apis/projectcontour/v1alpha1/zz_generated.deepcopy.go index 7ee5df65a55..2cd0cc98331 100644 --- a/apis/projectcontour/v1alpha1/zz_generated.deepcopy.go +++ b/apis/projectcontour/v1alpha1/zz_generated.deepcopy.go @@ -526,6 +526,11 @@ func (in *EnvoyListenerConfig) DeepCopyInto(out *EnvoyListenerConfig) { *out = new(EnvoyTLS) (*in).DeepCopyInto(*out) } + if in.MaxRequestsPerIOCycle != nil { + in, out := &in.MaxRequestsPerIOCycle, &out.MaxRequestsPerIOCycle + *out = new(uint32) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new EnvoyListenerConfig. diff --git a/changelogs/unreleased/5827-sunjayBhatia-minor.md b/changelogs/unreleased/5827-sunjayBhatia-minor.md new file mode 100644 index 00000000000..93010ffb61a --- /dev/null +++ b/changelogs/unreleased/5827-sunjayBhatia-minor.md @@ -0,0 +1,16 @@ +## Max HTTP requests per IO cycle is configurable as an additional mitigation for HTTP/2 CVE-2023-44487 + +Envoy v1.27.1 mitigates CVE-2023-44487 with some default runtime settings, however the `http.max_requests_per_io_cycle` does not have a default value. +This change allows configuring this runtime setting via Contour configuration to allow administrators of Contour to prevent abusive connections from starving resources from other valid connections. +The default is left as the existing behavior (no limit) so as not to impact existing valid traffic. + +The Contour ConfigMap can be modified similar to the following (and Contour restarted) to set this value: + +``` +listener: + max-requests-per-io-cycle: 10 +``` + +(Note this can be used in addition to the existing Listener configuration field `listener.max-requests-per-connection` which is used primarily for HTTP/1.1 connections and is an approximate limit for HTTP/2) + +See the [Envoy release notes](https://www.envoyproxy.io/docs/envoy/v1.27.1/version_history/v1.27/v1.27.1) for more details. diff --git a/cmd/contour/serve.go b/cmd/contour/serve.go index 179719f358b..7dd02c3529b 100644 --- a/cmd/contour/serve.go +++ b/cmd/contour/serve.go @@ -383,7 +383,9 @@ func (s *Server) doServe() error { &xdscache_v3.RouteCache{}, &xdscache_v3.ClusterCache{}, endpointHandler, - &xdscache_v3.RuntimeCache{}, + xdscache_v3.NewRuntimeCache(xdscache_v3.ConfigurableRuntimeSettings{ + MaxRequestsPerIOCycle: contourConfiguration.Envoy.Listener.MaxRequestsPerIOCycle, + }), } // snapshotHandler is used to produce new snapshots when the internal state changes for any xDS resource. diff --git a/cmd/contour/servecontext.go b/cmd/contour/servecontext.go index dab3bcd267e..f9576c58469 100644 --- a/cmd/contour/servecontext.go +++ b/cmd/contour/servecontext.go @@ -454,6 +454,7 @@ func (ctx *serveContext) convertToContourConfigurationSpec() contour_api_v1alpha DisableMergeSlashes: &ctx.Config.DisableMergeSlashes, ServerHeaderTransformation: serverHeaderTransformation, ConnectionBalancer: ctx.Config.Listener.ConnectionBalancer, + MaxRequestsPerIOCycle: ctx.Config.Listener.MaxRequestsPerIOCycle, TLS: &contour_api_v1alpha1.EnvoyTLS{ MinimumProtocolVersion: ctx.Config.TLS.MinimumProtocolVersion, CipherSuites: cipherSuites, diff --git a/cmd/contour/servecontext_test.go b/cmd/contour/servecontext_test.go index 4d5583caa2b..3aaf2458538 100644 --- a/cmd/contour/servecontext_test.go +++ b/cmd/contour/servecontext_test.go @@ -699,6 +699,16 @@ func TestConvertServeContext(t *testing.T) { return cfg }, }, + "envoy listener settings": { + getServeContext: func(ctx *serveContext) *serveContext { + ctx.Config.Listener.MaxRequestsPerIOCycle = ref.To(uint32(10)) + return ctx + }, + getContourConfiguration: func(cfg contour_api_v1alpha1.ContourConfigurationSpec) contour_api_v1alpha1.ContourConfigurationSpec { + cfg.Envoy.Listener.MaxRequestsPerIOCycle = ref.To(uint32(10)) + return cfg + }, + }, } for name, tc := range cases { diff --git a/examples/contour/01-crds.yaml b/examples/contour/01-crds.yaml index e497e155a22..33da15a8e19 100644 --- a/examples/contour/01-crds.yaml +++ b/examples/contour/01-crds.yaml @@ -180,6 +180,17 @@ spec: slashes from request URL paths. \n Contour's default is false." type: boolean + maxRequestsPerIOCycle: + description: Defines the limit on number of HTTP requests + that Envoy will process from a single connection in a single + I/O cycle. Requests over this limit are processed in subsequent + I/O cycles. Can be used as a mitigation for CVE-2023-44487 + when abusive traffic is detected. Configures the http.max_requests_per_io_cycle + Envoy runtime setting. The default value when this is not + set is no limit. + format: int32 + minimum: 1 + type: integer serverHeaderTransformation: description: "Defines the action to be applied to the Server header on the response path. When configured as overwrite, @@ -3197,6 +3208,18 @@ spec: duplicate slashes from request URL paths. \n Contour's default is false." type: boolean + maxRequestsPerIOCycle: + description: Defines the limit on number of HTTP requests + that Envoy will process from a single connection in + a single I/O cycle. Requests over this limit are processed + in subsequent I/O cycles. Can be used as a mitigation + for CVE-2023-44487 when abusive traffic is detected. + Configures the http.max_requests_per_io_cycle Envoy + runtime setting. The default value when this is not + set is no limit. + format: int32 + minimum: 1 + type: integer serverHeaderTransformation: description: "Defines the action to be applied to the Server header on the response path. When configured diff --git a/examples/render/contour-deployment.yaml b/examples/render/contour-deployment.yaml index f15fb261eb2..b39fe9466ca 100644 --- a/examples/render/contour-deployment.yaml +++ b/examples/render/contour-deployment.yaml @@ -393,6 +393,17 @@ spec: slashes from request URL paths. \n Contour's default is false." type: boolean + maxRequestsPerIOCycle: + description: Defines the limit on number of HTTP requests + that Envoy will process from a single connection in a single + I/O cycle. Requests over this limit are processed in subsequent + I/O cycles. Can be used as a mitigation for CVE-2023-44487 + when abusive traffic is detected. Configures the http.max_requests_per_io_cycle + Envoy runtime setting. The default value when this is not + set is no limit. + format: int32 + minimum: 1 + type: integer serverHeaderTransformation: description: "Defines the action to be applied to the Server header on the response path. When configured as overwrite, @@ -3410,6 +3421,18 @@ spec: duplicate slashes from request URL paths. \n Contour's default is false." type: boolean + maxRequestsPerIOCycle: + description: Defines the limit on number of HTTP requests + that Envoy will process from a single connection in + a single I/O cycle. Requests over this limit are processed + in subsequent I/O cycles. Can be used as a mitigation + for CVE-2023-44487 when abusive traffic is detected. + Configures the http.max_requests_per_io_cycle Envoy + runtime setting. The default value when this is not + set is no limit. + format: int32 + minimum: 1 + type: integer serverHeaderTransformation: description: "Defines the action to be applied to the Server header on the response path. When configured diff --git a/examples/render/contour-gateway-provisioner.yaml b/examples/render/contour-gateway-provisioner.yaml index 756411676a1..e3390a39334 100644 --- a/examples/render/contour-gateway-provisioner.yaml +++ b/examples/render/contour-gateway-provisioner.yaml @@ -194,6 +194,17 @@ spec: slashes from request URL paths. \n Contour's default is false." type: boolean + maxRequestsPerIOCycle: + description: Defines the limit on number of HTTP requests + that Envoy will process from a single connection in a single + I/O cycle. Requests over this limit are processed in subsequent + I/O cycles. Can be used as a mitigation for CVE-2023-44487 + when abusive traffic is detected. Configures the http.max_requests_per_io_cycle + Envoy runtime setting. The default value when this is not + set is no limit. + format: int32 + minimum: 1 + type: integer serverHeaderTransformation: description: "Defines the action to be applied to the Server header on the response path. When configured as overwrite, @@ -3211,6 +3222,18 @@ spec: duplicate slashes from request URL paths. \n Contour's default is false." type: boolean + maxRequestsPerIOCycle: + description: Defines the limit on number of HTTP requests + that Envoy will process from a single connection in + a single I/O cycle. Requests over this limit are processed + in subsequent I/O cycles. Can be used as a mitigation + for CVE-2023-44487 when abusive traffic is detected. + Configures the http.max_requests_per_io_cycle Envoy + runtime setting. The default value when this is not + set is no limit. + format: int32 + minimum: 1 + type: integer serverHeaderTransformation: description: "Defines the action to be applied to the Server header on the response path. When configured diff --git a/examples/render/contour-gateway.yaml b/examples/render/contour-gateway.yaml index eefdec6b033..47feb9d51a0 100644 --- a/examples/render/contour-gateway.yaml +++ b/examples/render/contour-gateway.yaml @@ -399,6 +399,17 @@ spec: slashes from request URL paths. \n Contour's default is false." type: boolean + maxRequestsPerIOCycle: + description: Defines the limit on number of HTTP requests + that Envoy will process from a single connection in a single + I/O cycle. Requests over this limit are processed in subsequent + I/O cycles. Can be used as a mitigation for CVE-2023-44487 + when abusive traffic is detected. Configures the http.max_requests_per_io_cycle + Envoy runtime setting. The default value when this is not + set is no limit. + format: int32 + minimum: 1 + type: integer serverHeaderTransformation: description: "Defines the action to be applied to the Server header on the response path. When configured as overwrite, @@ -3416,6 +3427,18 @@ spec: duplicate slashes from request URL paths. \n Contour's default is false." type: boolean + maxRequestsPerIOCycle: + description: Defines the limit on number of HTTP requests + that Envoy will process from a single connection in + a single I/O cycle. Requests over this limit are processed + in subsequent I/O cycles. Can be used as a mitigation + for CVE-2023-44487 when abusive traffic is detected. + Configures the http.max_requests_per_io_cycle Envoy + runtime setting. The default value when this is not + set is no limit. + format: int32 + minimum: 1 + type: integer serverHeaderTransformation: description: "Defines the action to be applied to the Server header on the response path. When configured diff --git a/examples/render/contour.yaml b/examples/render/contour.yaml index 7b256cdc5f1..9ff7295f938 100644 --- a/examples/render/contour.yaml +++ b/examples/render/contour.yaml @@ -393,6 +393,17 @@ spec: slashes from request URL paths. \n Contour's default is false." type: boolean + maxRequestsPerIOCycle: + description: Defines the limit on number of HTTP requests + that Envoy will process from a single connection in a single + I/O cycle. Requests over this limit are processed in subsequent + I/O cycles. Can be used as a mitigation for CVE-2023-44487 + when abusive traffic is detected. Configures the http.max_requests_per_io_cycle + Envoy runtime setting. The default value when this is not + set is no limit. + format: int32 + minimum: 1 + type: integer serverHeaderTransformation: description: "Defines the action to be applied to the Server header on the response path. When configured as overwrite, @@ -3410,6 +3421,18 @@ spec: duplicate slashes from request URL paths. \n Contour's default is false." type: boolean + maxRequestsPerIOCycle: + description: Defines the limit on number of HTTP requests + that Envoy will process from a single connection in + a single I/O cycle. Requests over this limit are processed + in subsequent I/O cycles. Can be used as a mitigation + for CVE-2023-44487 when abusive traffic is detected. + Configures the http.max_requests_per_io_cycle Envoy + runtime setting. The default value when this is not + set is no limit. + format: int32 + minimum: 1 + type: integer serverHeaderTransformation: description: "Defines the action to be applied to the Server header on the response path. When configured diff --git a/internal/envoy/v3/runtime.go b/internal/envoy/v3/runtime.go index 8e2a121e06b..135eddab56b 100644 --- a/internal/envoy/v3/runtime.go +++ b/internal/envoy/v3/runtime.go @@ -24,11 +24,15 @@ const ( maxRegexProgramSizeWarn = 1000 ) -func RuntimeLayers() []*envoy_service_runtime_v3.Runtime { +func RuntimeLayers(configurableRuntimeFields map[string]*structpb.Value) []*envoy_service_runtime_v3.Runtime { + baseLayer := baseRuntimeLayer() + for k, v := range configurableRuntimeFields { + baseLayer.Fields[k] = v + } return []*envoy_service_runtime_v3.Runtime{ { Name: DynamicRuntimeLayerName, - Layer: baseRuntimeLayer(), + Layer: baseLayer, }, } } @@ -36,8 +40,8 @@ func RuntimeLayers() []*envoy_service_runtime_v3.Runtime { func baseRuntimeLayer() *structpb.Struct { return &structpb.Struct{ Fields: map[string]*structpb.Value{ - "re2.max_program_size.error_level": {Kind: &structpb.Value_NumberValue{NumberValue: maxRegexProgramSizeError}}, - "re2.max_program_size.warn_level": {Kind: &structpb.Value_NumberValue{NumberValue: maxRegexProgramSizeWarn}}, + "re2.max_program_size.error_level": structpb.NewNumberValue(maxRegexProgramSizeError), + "re2.max_program_size.warn_level": structpb.NewNumberValue(maxRegexProgramSizeWarn), }, } } diff --git a/internal/envoy/v3/runtime_test.go b/internal/envoy/v3/runtime_test.go index a0ca20d3cf0..9e84d136b65 100644 --- a/internal/envoy/v3/runtime_test.go +++ b/internal/envoy/v3/runtime_test.go @@ -22,15 +22,38 @@ import ( ) func TestRuntimeLayers(t *testing.T) { - require.Equal(t, []*envoy_service_runtime_v3.Runtime{ - { - Name: "dynamic", - Layer: &structpb.Struct{ - Fields: map[string]*structpb.Value{ - "re2.max_program_size.error_level": {Kind: &structpb.Value_NumberValue{NumberValue: 1 << 20}}, - "re2.max_program_size.warn_level": {Kind: &structpb.Value_NumberValue{NumberValue: 1000}}, - }, + testCases := map[string]struct { + configurableFields map[string]*structpb.Value + }{ + "nil configurable fields": {}, + "empty configurable fields": { + configurableFields: map[string]*structpb.Value{}, + }, + "some configurable fields": { + configurableFields: map[string]*structpb.Value{ + "some.value1": structpb.NewBoolValue(true), + "some.value2": structpb.NewNumberValue(1000), }, }, - }, RuntimeLayers()) + } + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + expectedFields := map[string]*structpb.Value{ + "re2.max_program_size.error_level": structpb.NewNumberValue(1 << 20), + "re2.max_program_size.warn_level": structpb.NewNumberValue(1000), + } + for k, v := range tc.configurableFields { + expectedFields[k] = v + } + layers := RuntimeLayers(tc.configurableFields) + require.Equal(t, []*envoy_service_runtime_v3.Runtime{ + { + Name: "dynamic", + Layer: &structpb.Struct{ + Fields: expectedFields, + }, + }, + }, layers) + }) + } } diff --git a/internal/xdscache/v3/runtime.go b/internal/xdscache/v3/runtime.go index a163350804f..b4124bfa217 100644 --- a/internal/xdscache/v3/runtime.go +++ b/internal/xdscache/v3/runtime.go @@ -20,30 +20,46 @@ import ( envoy_v3 "github.com/projectcontour/contour/internal/envoy/v3" "github.com/projectcontour/contour/internal/protobuf" "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/types/known/structpb" ) +type ConfigurableRuntimeSettings struct { + MaxRequestsPerIOCycle *uint32 +} + // RuntimeCache manages the contents of the gRPC RTDS cache. -type RuntimeCache struct { +type runtimeCache struct { contour.Cond + runtimeKV map[string]*structpb.Value +} + +// NewRuntimeCache builds a RuntimeCache with the provided runtime +// settings that will be set in the runtime layer configured by Contour. +func NewRuntimeCache(runtimeSettings ConfigurableRuntimeSettings) *runtimeCache { + runtimeKV := make(map[string]*structpb.Value) + if runtimeSettings.MaxRequestsPerIOCycle != nil && *runtimeSettings.MaxRequestsPerIOCycle > 0 { + runtimeKV["http.max_requests_per_io_cycle"] = structpb.NewNumberValue(float64(*runtimeSettings.MaxRequestsPerIOCycle)) + } + return &runtimeCache{runtimeKV: runtimeKV} } // Contents returns all Runtime layers. -func (c *RuntimeCache) Contents() []proto.Message { - return protobuf.AsMessages(envoy_v3.RuntimeLayers()) +func (c *runtimeCache) Contents() []proto.Message { + return protobuf.AsMessages(envoy_v3.RuntimeLayers(c.runtimeKV)) } // Query returns only the "dynamic" layer if requested, otherwise empty. -func (c *RuntimeCache) Query(names []string) []proto.Message { +func (c *runtimeCache) Query(names []string) []proto.Message { for _, name := range names { if name == envoy_v3.DynamicRuntimeLayerName { - return protobuf.AsMessages(envoy_v3.RuntimeLayers()) + return protobuf.AsMessages(envoy_v3.RuntimeLayers(c.runtimeKV)) } } return []proto.Message{} } -func (*RuntimeCache) TypeURL() string { return resource.RuntimeType } +func (*runtimeCache) TypeURL() string { return resource.RuntimeType } -func (c *RuntimeCache) OnChange(root *dag.DAG) { +func (c *runtimeCache) OnChange(root *dag.DAG) { // DAG changes do not affect runtime layers at the moment. } diff --git a/internal/xdscache/v3/runtime_test.go b/internal/xdscache/v3/runtime_test.go index 71793f0c40d..b7aa25815d2 100644 --- a/internal/xdscache/v3/runtime_test.go +++ b/internal/xdscache/v3/runtime_test.go @@ -18,16 +18,72 @@ import ( envoy_service_runtime_v3 "github.com/envoyproxy/go-control-plane/envoy/service/runtime/v3" "github.com/projectcontour/contour/internal/protobuf" + "github.com/projectcontour/contour/internal/ref" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/known/structpb" ) func TestRuntimeCacheContents(t *testing.T) { - rc := &RuntimeCache{} - protobuf.ExpectEqual(t, runtimeLayers(), rc.Contents()) + testCases := map[string]struct { + runtimeSettings ConfigurableRuntimeSettings + additionalFields map[string]*structpb.Value + }{ + "no values set": { + runtimeSettings: ConfigurableRuntimeSettings{}, + }, + "http max requests per io cycle set": { + runtimeSettings: ConfigurableRuntimeSettings{ + MaxRequestsPerIOCycle: ref.To(uint32(1)), + }, + additionalFields: map[string]*structpb.Value{ + "http.max_requests_per_io_cycle": structpb.NewNumberValue(1), + }, + }, + "http max requests per io cycle set invalid": { + runtimeSettings: ConfigurableRuntimeSettings{ + MaxRequestsPerIOCycle: ref.To(uint32(0)), + }, + }, + "http max requests per io cycle set nil": { + runtimeSettings: ConfigurableRuntimeSettings{ + MaxRequestsPerIOCycle: nil, + }, + }, + } + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + rc := NewRuntimeCache(tc.runtimeSettings) + fields := map[string]*structpb.Value{ + "re2.max_program_size.error_level": structpb.NewNumberValue(1 << 20), + "re2.max_program_size.warn_level": structpb.NewNumberValue(1000), + } + for k, v := range tc.additionalFields { + fields[k] = v + } + protobuf.ExpectEqual(t, []proto.Message{ + &envoy_service_runtime_v3.Runtime{ + Name: "dynamic", + Layer: &structpb.Struct{ + Fields: fields, + }, + }, + }, rc.Contents()) + }) + } } func TestRuntimeCacheQuery(t *testing.T) { + baseRuntimeLayers := []proto.Message{ + &envoy_service_runtime_v3.Runtime{ + Name: "dynamic", + Layer: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + "re2.max_program_size.error_level": structpb.NewNumberValue(1 << 20), + "re2.max_program_size.warn_level": structpb.NewNumberValue(1000), + }, + }, + }, + } testCases := map[string]struct { names []string expected []proto.Message @@ -38,7 +94,7 @@ func TestRuntimeCacheQuery(t *testing.T) { }, "names include dynamic": { names: []string{"foo", "dynamic", "bar"}, - expected: runtimeLayers(), + expected: baseRuntimeLayers, }, "names excludes dynamic": { names: []string{"foo", "bar", "baz"}, @@ -47,22 +103,8 @@ func TestRuntimeCacheQuery(t *testing.T) { } for name, tc := range testCases { t.Run(name, func(t *testing.T) { - rc := &RuntimeCache{} + rc := NewRuntimeCache(ConfigurableRuntimeSettings{}) protobuf.ExpectEqual(t, tc.expected, rc.Query(tc.names)) }) } } - -func runtimeLayers() []proto.Message { - return []proto.Message{ - &envoy_service_runtime_v3.Runtime{ - Name: "dynamic", - Layer: &structpb.Struct{ - Fields: map[string]*structpb.Value{ - "re2.max_program_size.error_level": {Kind: &structpb.Value_NumberValue{NumberValue: 1 << 20}}, - "re2.max_program_size.warn_level": {Kind: &structpb.Value_NumberValue{NumberValue: 1000}}, - }, - }, - }, - } -} diff --git a/internal/xdscache/v3/server_test.go b/internal/xdscache/v3/server_test.go index 0acc20e90f2..22437a86573 100644 --- a/internal/xdscache/v3/server_test.go +++ b/internal/xdscache/v3/server_test.go @@ -210,7 +210,7 @@ func TestGRPC(t *testing.T) { &RouteCache{}, &ClusterCache{}, et, - &RuntimeCache{}, + NewRuntimeCache(ConfigurableRuntimeSettings{}), } eh = contour.NewEventHandler(contour.EventHandlerConfig{ diff --git a/pkg/config/parameters.go b/pkg/config/parameters.go index ed9026803dd..a04a2b4c941 100644 --- a/pkg/config/parameters.go +++ b/pkg/config/parameters.go @@ -436,6 +436,13 @@ type ListenerParameters struct { // See https://www.envoyproxy.io/docs/envoy/latest/api-v2/api/v2/listener.proto#envoy-api-msg-listener-connectionbalanceconfig // for more information. ConnectionBalancer string `yaml:"connection-balancer"` + + // Defines the limit on number of HTTP requests that Envoy will process from a single + // connection in a single I/O cycle. Requests over this limit are processed in subsequent + // I/O cycles. Can be used as a mitigation for CVE-2023-44487 when abusive traffic is + // detected. Configures the http.max_requests_per_io_cycle Envoy runtime setting. The default + // value when this is not set is no limit. + MaxRequestsPerIOCycle *uint32 `yaml:"max-requests-per-io-cycle"` } func (p *ListenerParameters) Validate() error { @@ -446,6 +453,10 @@ func (p *ListenerParameters) Validate() error { if p.ConnectionBalancer != "" && p.ConnectionBalancer != "exact" { return fmt.Errorf("invalid listener connection balancer value %q, only 'exact' connection balancing is supported for now", p.ConnectionBalancer) } + + if p.MaxRequestsPerIOCycle != nil && *p.MaxRequestsPerIOCycle < 1 { + return fmt.Errorf("invalid max connections per IO cycle value %q set on listener, minimum value is 1", *p.MaxRequestsPerIOCycle) + } return nil } diff --git a/pkg/config/parameters_test.go b/pkg/config/parameters_test.go index d136eea3467..871207e7804 100644 --- a/pkg/config/parameters_test.go +++ b/pkg/config/parameters_test.go @@ -18,6 +18,7 @@ import ( "strings" "testing" + "github.com/projectcontour/contour/internal/ref" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gopkg.in/yaml.v3" @@ -417,6 +418,13 @@ network: num-trusted-hops: 1 admin-port: 9001 `) + + check(func(t *testing.T, conf *Parameters) { + assert.Equal(t, ref.To(uint32(1)), conf.Listener.MaxRequestsPerIOCycle) + }, ` +listener: + max-requests-per-io-cycle: 1 +`) } func TestMetricsParametersValidation(t *testing.T) { @@ -491,4 +499,12 @@ func TestListenerValidation(t *testing.T) { ConnectionBalancer: "invalid", } require.Error(t, l.Validate()) + l = &ListenerParameters{ + MaxRequestsPerIOCycle: ref.To(uint32(1)), + } + require.NoError(t, l.Validate()) + l = &ListenerParameters{ + MaxRequestsPerIOCycle: ref.To(uint32(0)), + } + require.Error(t, l.Validate()) } diff --git a/site/content/docs/main/config/api-reference.html b/site/content/docs/main/config/api-reference.html index be449e43201..edbd5c79505 100644 --- a/site/content/docs/main/config/api-reference.html +++ b/site/content/docs/main/config/api-reference.html @@ -5979,6 +5979,23 @@

EnvoyListenerConfig

TLS holds various configurable Envoy TLS listener values.

+ + +maxRequestsPerIOCycle +
+ +uint32 + + + +(Optional) +

Defines the limit on number of HTTP requests that Envoy will process from a single +connection in a single I/O cycle. Requests over this limit are processed in subsequent +I/O cycles. Can be used as a mitigation for CVE-2023-44487 when abusive traffic is +detected. Configures the http.max_requests_per_io_cycle Envoy runtime setting. The default +value when this is not set is no limit.

+ +

EnvoyLogging diff --git a/site/content/docs/main/configuration.md b/site/content/docs/main/configuration.md index 9fd9c77ff3d..8660031462b 100644 --- a/site/content/docs/main/configuration.md +++ b/site/content/docs/main/configuration.md @@ -185,6 +185,7 @@ The listener configuration block can be used to configure various parameters for | Field Name | Type | Default | Description | | ------------------- | ------ | ------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | connection-balancer | string | `""` | This field specifies the listener connection balancer. If the value is `exact`, the listener will use the exact connection balancer to balance connections between threads in a single Envoy process. See [the Envoy documentation][14] for more information. | +| max-requests-per-io-cycle | int | none | Defines the limit on number of HTTP requests that Envoy will process from a single connection in a single I/O cycle. Requests over this limit are processed in subsequent I/O cycles. Can be used as a mitigation for CVE-2023-44487 when abusive traffic is detected. Configures the `http.max_requests_per_io_cycle` Envoy runtime setting. The default value when this is not set is no limit. | ### Server Configuration From c78c69bb68c3ae24be1d4287ff3c13725dd57877 Mon Sep 17 00:00:00 2001 From: Sunjay Bhatia <5337253+sunjayBhatia@users.noreply.github.com> Date: Fri, 13 Oct 2023 15:58:36 -0400 Subject: [PATCH 2/3] HTTP/2 max concurrent streams can be configured (#5850) Adds a global Listener configuration field for admins to be able to protect their installations of Contour/Envoy with a limit. Default is no limit to ensure existing behavior is not impacted for valid traffic. This field can be used for tuning resource usage or mitigated DOS attacks like in CVE-2023-44487. Also fixes omitempty tags on MaxRequestsPerIOCycle field. Fixes: #5846 Signed-off-by: Sunjay Bhatia --- apis/projectcontour/v1alpha1/contourconfig.go | 13 +- .../v1alpha1/zz_generated.deepcopy.go | 5 + .../unreleased/5850-sunjayBhatia-minor.md | 11 ++ cmd/contour/serve.go | 1 + cmd/contour/servecontext.go | 1 + cmd/contour/servecontext_test.go | 2 + examples/contour/01-crds.yaml | 24 +++ examples/render/contour-deployment.yaml | 24 +++ .../render/contour-gateway-provisioner.yaml | 24 +++ examples/render/contour-gateway.yaml | 24 +++ examples/render/contour.yaml | 24 +++ .../contourconfiguration_test.go | 1 + internal/envoy/v3/listener.go | 12 ++ internal/envoy/v3/listener_test.go | 56 +++++++ internal/xdscache/v3/listener.go | 6 + internal/xdscache/v3/listener_test.go | 137 ++++++++++++++++++ pkg/config/parameters.go | 15 +- pkg/config/parameters_test.go | 14 ++ .../docs/main/config/api-reference.html | 18 +++ site/content/docs/main/configuration.md | 1 + 20 files changed, 411 insertions(+), 2 deletions(-) create mode 100644 changelogs/unreleased/5850-sunjayBhatia-minor.md diff --git a/apis/projectcontour/v1alpha1/contourconfig.go b/apis/projectcontour/v1alpha1/contourconfig.go index e0ec26c597b..9143243c6f4 100644 --- a/apis/projectcontour/v1alpha1/contourconfig.go +++ b/apis/projectcontour/v1alpha1/contourconfig.go @@ -370,7 +370,18 @@ type EnvoyListenerConfig struct { // // +kubebuilder:validation:Minimum=1 // +optional - MaxRequestsPerIOCycle *uint32 `json:"maxRequestsPerIOCycle"` + MaxRequestsPerIOCycle *uint32 `json:"maxRequestsPerIOCycle,omitempty"` + + // Defines the value for SETTINGS_MAX_CONCURRENT_STREAMS Envoy will advertise in the + // SETTINGS frame in HTTP/2 connections and the limit for concurrent streams allowed + // for a peer on a single HTTP/2 connection. It is recommended to not set this lower + // than 100 but this field can be used to bound resource usage by HTTP/2 connections + // and mitigate attacks like CVE-2023-44487. The default value when this is not set is + // unlimited. + // + // +kubebuilder:validation:Minimum=1 + // +optional + HTTP2MaxConcurrentStreams *uint32 `json:"httpMaxConcurrentStreams,omitempty"` } // EnvoyTLS describes tls parameters for Envoy listneners. diff --git a/apis/projectcontour/v1alpha1/zz_generated.deepcopy.go b/apis/projectcontour/v1alpha1/zz_generated.deepcopy.go index 2cd0cc98331..5a849fbdab5 100644 --- a/apis/projectcontour/v1alpha1/zz_generated.deepcopy.go +++ b/apis/projectcontour/v1alpha1/zz_generated.deepcopy.go @@ -531,6 +531,11 @@ func (in *EnvoyListenerConfig) DeepCopyInto(out *EnvoyListenerConfig) { *out = new(uint32) **out = **in } + if in.HTTP2MaxConcurrentStreams != nil { + in, out := &in.HTTP2MaxConcurrentStreams, &out.HTTP2MaxConcurrentStreams + *out = new(uint32) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new EnvoyListenerConfig. diff --git a/changelogs/unreleased/5850-sunjayBhatia-minor.md b/changelogs/unreleased/5850-sunjayBhatia-minor.md new file mode 100644 index 00000000000..32e38a6c494 --- /dev/null +++ b/changelogs/unreleased/5850-sunjayBhatia-minor.md @@ -0,0 +1,11 @@ +## HTTP/2 max concurrent streams is configurable + +This field can be used to limit the number of concurrent streams Envoy will allow on a single connection from a downstream peer. +It can be used to tune resource usage and as a mitigation for DOS attacks arising from vulnerabilities like CVE-2023-44487. + +The Contour ConfigMap can be modified similar to the following (and Contour restarted) to set this value: + +``` +listener: + http2-max-concurrent-streams: 50 +``` diff --git a/cmd/contour/serve.go b/cmd/contour/serve.go index 7dd02c3529b..8a77be24e10 100644 --- a/cmd/contour/serve.go +++ b/cmd/contour/serve.go @@ -365,6 +365,7 @@ func (s *Server) doServe() error { ServerHeaderTransformation: contourConfiguration.Envoy.Listener.ServerHeaderTransformation, XffNumTrustedHops: *contourConfiguration.Envoy.Network.XffNumTrustedHops, ConnectionBalancer: contourConfiguration.Envoy.Listener.ConnectionBalancer, + HTTP2MaxConcurrentStreams: contourConfiguration.Envoy.Listener.HTTP2MaxConcurrentStreams, } if listenerConfig.RateLimitConfig, err = s.setupRateLimitService(contourConfiguration); err != nil { diff --git a/cmd/contour/servecontext.go b/cmd/contour/servecontext.go index f9576c58469..7e45fe05fef 100644 --- a/cmd/contour/servecontext.go +++ b/cmd/contour/servecontext.go @@ -455,6 +455,7 @@ func (ctx *serveContext) convertToContourConfigurationSpec() contour_api_v1alpha ServerHeaderTransformation: serverHeaderTransformation, ConnectionBalancer: ctx.Config.Listener.ConnectionBalancer, MaxRequestsPerIOCycle: ctx.Config.Listener.MaxRequestsPerIOCycle, + HTTP2MaxConcurrentStreams: ctx.Config.Listener.HTTP2MaxConcurrentStreams, TLS: &contour_api_v1alpha1.EnvoyTLS{ MinimumProtocolVersion: ctx.Config.TLS.MinimumProtocolVersion, CipherSuites: cipherSuites, diff --git a/cmd/contour/servecontext_test.go b/cmd/contour/servecontext_test.go index 3aaf2458538..6f31c542298 100644 --- a/cmd/contour/servecontext_test.go +++ b/cmd/contour/servecontext_test.go @@ -702,10 +702,12 @@ func TestConvertServeContext(t *testing.T) { "envoy listener settings": { getServeContext: func(ctx *serveContext) *serveContext { ctx.Config.Listener.MaxRequestsPerIOCycle = ref.To(uint32(10)) + ctx.Config.Listener.HTTP2MaxConcurrentStreams = ref.To(uint32(30)) return ctx }, getContourConfiguration: func(cfg contour_api_v1alpha1.ContourConfigurationSpec) contour_api_v1alpha1.ContourConfigurationSpec { cfg.Envoy.Listener.MaxRequestsPerIOCycle = ref.To(uint32(10)) + cfg.Envoy.Listener.HTTP2MaxConcurrentStreams = ref.To(uint32(30)) return cfg }, }, diff --git a/examples/contour/01-crds.yaml b/examples/contour/01-crds.yaml index 33da15a8e19..3599bd5e8a9 100644 --- a/examples/contour/01-crds.yaml +++ b/examples/contour/01-crds.yaml @@ -180,6 +180,18 @@ spec: slashes from request URL paths. \n Contour's default is false." type: boolean + httpMaxConcurrentStreams: + description: Defines the value for SETTINGS_MAX_CONCURRENT_STREAMS + Envoy will advertise in the SETTINGS frame in HTTP/2 connections + and the limit for concurrent streams allowed for a peer + on a single HTTP/2 connection. It is recommended to not + set this lower than 100 but this field can be used to bound + resource usage by HTTP/2 connections and mitigate attacks + like CVE-2023-44487. The default value when this is not + set is unlimited. + format: int32 + minimum: 1 + type: integer maxRequestsPerIOCycle: description: Defines the limit on number of HTTP requests that Envoy will process from a single connection in a single @@ -3208,6 +3220,18 @@ spec: duplicate slashes from request URL paths. \n Contour's default is false." type: boolean + httpMaxConcurrentStreams: + description: Defines the value for SETTINGS_MAX_CONCURRENT_STREAMS + Envoy will advertise in the SETTINGS frame in HTTP/2 + connections and the limit for concurrent streams allowed + for a peer on a single HTTP/2 connection. It is recommended + to not set this lower than 100 but this field can be + used to bound resource usage by HTTP/2 connections and + mitigate attacks like CVE-2023-44487. The default value + when this is not set is unlimited. + format: int32 + minimum: 1 + type: integer maxRequestsPerIOCycle: description: Defines the limit on number of HTTP requests that Envoy will process from a single connection in diff --git a/examples/render/contour-deployment.yaml b/examples/render/contour-deployment.yaml index b39fe9466ca..cfacba99330 100644 --- a/examples/render/contour-deployment.yaml +++ b/examples/render/contour-deployment.yaml @@ -393,6 +393,18 @@ spec: slashes from request URL paths. \n Contour's default is false." type: boolean + httpMaxConcurrentStreams: + description: Defines the value for SETTINGS_MAX_CONCURRENT_STREAMS + Envoy will advertise in the SETTINGS frame in HTTP/2 connections + and the limit for concurrent streams allowed for a peer + on a single HTTP/2 connection. It is recommended to not + set this lower than 100 but this field can be used to bound + resource usage by HTTP/2 connections and mitigate attacks + like CVE-2023-44487. The default value when this is not + set is unlimited. + format: int32 + minimum: 1 + type: integer maxRequestsPerIOCycle: description: Defines the limit on number of HTTP requests that Envoy will process from a single connection in a single @@ -3421,6 +3433,18 @@ spec: duplicate slashes from request URL paths. \n Contour's default is false." type: boolean + httpMaxConcurrentStreams: + description: Defines the value for SETTINGS_MAX_CONCURRENT_STREAMS + Envoy will advertise in the SETTINGS frame in HTTP/2 + connections and the limit for concurrent streams allowed + for a peer on a single HTTP/2 connection. It is recommended + to not set this lower than 100 but this field can be + used to bound resource usage by HTTP/2 connections and + mitigate attacks like CVE-2023-44487. The default value + when this is not set is unlimited. + format: int32 + minimum: 1 + type: integer maxRequestsPerIOCycle: description: Defines the limit on number of HTTP requests that Envoy will process from a single connection in diff --git a/examples/render/contour-gateway-provisioner.yaml b/examples/render/contour-gateway-provisioner.yaml index e3390a39334..3696a4be417 100644 --- a/examples/render/contour-gateway-provisioner.yaml +++ b/examples/render/contour-gateway-provisioner.yaml @@ -194,6 +194,18 @@ spec: slashes from request URL paths. \n Contour's default is false." type: boolean + httpMaxConcurrentStreams: + description: Defines the value for SETTINGS_MAX_CONCURRENT_STREAMS + Envoy will advertise in the SETTINGS frame in HTTP/2 connections + and the limit for concurrent streams allowed for a peer + on a single HTTP/2 connection. It is recommended to not + set this lower than 100 but this field can be used to bound + resource usage by HTTP/2 connections and mitigate attacks + like CVE-2023-44487. The default value when this is not + set is unlimited. + format: int32 + minimum: 1 + type: integer maxRequestsPerIOCycle: description: Defines the limit on number of HTTP requests that Envoy will process from a single connection in a single @@ -3222,6 +3234,18 @@ spec: duplicate slashes from request URL paths. \n Contour's default is false." type: boolean + httpMaxConcurrentStreams: + description: Defines the value for SETTINGS_MAX_CONCURRENT_STREAMS + Envoy will advertise in the SETTINGS frame in HTTP/2 + connections and the limit for concurrent streams allowed + for a peer on a single HTTP/2 connection. It is recommended + to not set this lower than 100 but this field can be + used to bound resource usage by HTTP/2 connections and + mitigate attacks like CVE-2023-44487. The default value + when this is not set is unlimited. + format: int32 + minimum: 1 + type: integer maxRequestsPerIOCycle: description: Defines the limit on number of HTTP requests that Envoy will process from a single connection in diff --git a/examples/render/contour-gateway.yaml b/examples/render/contour-gateway.yaml index 47feb9d51a0..02b7696109a 100644 --- a/examples/render/contour-gateway.yaml +++ b/examples/render/contour-gateway.yaml @@ -399,6 +399,18 @@ spec: slashes from request URL paths. \n Contour's default is false." type: boolean + httpMaxConcurrentStreams: + description: Defines the value for SETTINGS_MAX_CONCURRENT_STREAMS + Envoy will advertise in the SETTINGS frame in HTTP/2 connections + and the limit for concurrent streams allowed for a peer + on a single HTTP/2 connection. It is recommended to not + set this lower than 100 but this field can be used to bound + resource usage by HTTP/2 connections and mitigate attacks + like CVE-2023-44487. The default value when this is not + set is unlimited. + format: int32 + minimum: 1 + type: integer maxRequestsPerIOCycle: description: Defines the limit on number of HTTP requests that Envoy will process from a single connection in a single @@ -3427,6 +3439,18 @@ spec: duplicate slashes from request URL paths. \n Contour's default is false." type: boolean + httpMaxConcurrentStreams: + description: Defines the value for SETTINGS_MAX_CONCURRENT_STREAMS + Envoy will advertise in the SETTINGS frame in HTTP/2 + connections and the limit for concurrent streams allowed + for a peer on a single HTTP/2 connection. It is recommended + to not set this lower than 100 but this field can be + used to bound resource usage by HTTP/2 connections and + mitigate attacks like CVE-2023-44487. The default value + when this is not set is unlimited. + format: int32 + minimum: 1 + type: integer maxRequestsPerIOCycle: description: Defines the limit on number of HTTP requests that Envoy will process from a single connection in diff --git a/examples/render/contour.yaml b/examples/render/contour.yaml index 9ff7295f938..f9546dcb5b8 100644 --- a/examples/render/contour.yaml +++ b/examples/render/contour.yaml @@ -393,6 +393,18 @@ spec: slashes from request URL paths. \n Contour's default is false." type: boolean + httpMaxConcurrentStreams: + description: Defines the value for SETTINGS_MAX_CONCURRENT_STREAMS + Envoy will advertise in the SETTINGS frame in HTTP/2 connections + and the limit for concurrent streams allowed for a peer + on a single HTTP/2 connection. It is recommended to not + set this lower than 100 but this field can be used to bound + resource usage by HTTP/2 connections and mitigate attacks + like CVE-2023-44487. The default value when this is not + set is unlimited. + format: int32 + minimum: 1 + type: integer maxRequestsPerIOCycle: description: Defines the limit on number of HTTP requests that Envoy will process from a single connection in a single @@ -3421,6 +3433,18 @@ spec: duplicate slashes from request URL paths. \n Contour's default is false." type: boolean + httpMaxConcurrentStreams: + description: Defines the value for SETTINGS_MAX_CONCURRENT_STREAMS + Envoy will advertise in the SETTINGS frame in HTTP/2 + connections and the limit for concurrent streams allowed + for a peer on a single HTTP/2 connection. It is recommended + to not set this lower than 100 but this field can be + used to bound resource usage by HTTP/2 connections and + mitigate attacks like CVE-2023-44487. The default value + when this is not set is unlimited. + format: int32 + minimum: 1 + type: integer maxRequestsPerIOCycle: description: Defines the limit on number of HTTP requests that Envoy will process from a single connection in diff --git a/internal/contourconfig/contourconfiguration_test.go b/internal/contourconfig/contourconfiguration_test.go index 408ad4abace..6e527085ae1 100644 --- a/internal/contourconfig/contourconfiguration_test.go +++ b/internal/contourconfig/contourconfiguration_test.go @@ -55,6 +55,7 @@ func TestOverlayOnDefaults(t *testing.T) { UseProxyProto: ref.To(true), DisableAllowChunkedLength: ref.To(true), DisableMergeSlashes: ref.To(true), + HTTP2MaxConcurrentStreams: ref.To(uint32(10)), ServerHeaderTransformation: contour_api_v1alpha1.PassThroughServerHeader, ConnectionBalancer: "yesplease", TLS: &contour_api_v1alpha1.EnvoyTLS{ diff --git a/internal/envoy/v3/listener.go b/internal/envoy/v3/listener.go index 18ab8da13a0..897a3d200dd 100644 --- a/internal/envoy/v3/listener.go +++ b/internal/envoy/v3/listener.go @@ -165,6 +165,7 @@ type httpConnectionManagerBuilder struct { serverHeaderTransformation http.HttpConnectionManager_ServerHeaderTransformation forwardClientCertificate *dag.ClientCertificateDetails numTrustedHops uint32 + http2MaxConcurrentStreams *uint32 } // RouteConfigName sets the name of the RDS element that contains @@ -265,6 +266,11 @@ func (b *httpConnectionManagerBuilder) NumTrustedHops(num uint32) *httpConnectio return b } +func (b *httpConnectionManagerBuilder) HTTP2MaxConcurrentStreams(http2MaxConcurrentStreams *uint32) *httpConnectionManagerBuilder { + b.http2MaxConcurrentStreams = http2MaxConcurrentStreams + return b +} + func (b *httpConnectionManagerBuilder) DefaultFilters() *httpConnectionManagerBuilder { // Add a default set of ordered http filters. @@ -507,6 +513,12 @@ func (b *httpConnectionManagerBuilder) Get() *envoy_listener_v3.Filter { } } + if b.http2MaxConcurrentStreams != nil { + cm.Http2ProtocolOptions = &envoy_core_v3.Http2ProtocolOptions{ + MaxConcurrentStreams: wrapperspb.UInt32(*b.http2MaxConcurrentStreams), + } + } + return &envoy_listener_v3.Filter{ Name: wellknown.HTTPConnectionManager, ConfigType: &envoy_listener_v3.Filter_TypedConfig{ diff --git a/internal/envoy/v3/listener_test.go b/internal/envoy/v3/listener_test.go index 441e3427d20..c9a75553206 100644 --- a/internal/envoy/v3/listener_test.go +++ b/internal/envoy/v3/listener_test.go @@ -38,6 +38,7 @@ import ( "github.com/projectcontour/contour/internal/dag" "github.com/projectcontour/contour/internal/envoy" "github.com/projectcontour/contour/internal/protobuf" + "github.com/projectcontour/contour/internal/ref" "github.com/projectcontour/contour/internal/timeout" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -607,6 +608,7 @@ func TestHTTPConnectionManager(t *testing.T) { serverHeaderTranformation v1alpha1.ServerHeaderTransformationType forwardClientCertificate *dag.ClientCertificateDetails xffNumTrustedHops uint32 + http2MaxConcurrentStreams *uint32 want *envoy_listener_v3.Filter }{ "default": { @@ -1344,6 +1346,59 @@ func TestHTTPConnectionManager(t *testing.T) { }, }, }, + "http2MaxConcurrentStreams set": { + routename: "default/kuard", + accesslogger: FileAccessLogEnvoy("/dev/stdout", "", nil, v1alpha1.LogLevelInfo), + http2MaxConcurrentStreams: ref.To(uint32(50)), + want: &envoy_listener_v3.Filter{ + Name: wellknown.HTTPConnectionManager, + ConfigType: &envoy_listener_v3.Filter_TypedConfig{ + TypedConfig: protobuf.MustMarshalAny(&http.HttpConnectionManager{ + StatPrefix: "default/kuard", + RouteSpecifier: &http.HttpConnectionManager_Rds{ + Rds: &http.Rds{ + RouteConfigName: "default/kuard", + ConfigSource: &envoy_core_v3.ConfigSource{ + ResourceApiVersion: envoy_core_v3.ApiVersion_V3, + ConfigSourceSpecifier: &envoy_core_v3.ConfigSource_ApiConfigSource{ + ApiConfigSource: &envoy_core_v3.ApiConfigSource{ + ApiType: envoy_core_v3.ApiConfigSource_GRPC, + TransportApiVersion: envoy_core_v3.ApiVersion_V3, + GrpcServices: []*envoy_core_v3.GrpcService{{ + TargetSpecifier: &envoy_core_v3.GrpcService_EnvoyGrpc_{ + EnvoyGrpc: &envoy_core_v3.GrpcService_EnvoyGrpc{ + ClusterName: "contour", + Authority: "contour", + }, + }, + }}, + }, + }, + }, + }, + }, + HttpFilters: defaultHTTPFilters, + HttpProtocolOptions: &envoy_core_v3.Http1ProtocolOptions{ + // Enable support for HTTP/1.0 requests that carry + // a Host: header. See #537. + AcceptHttp_10: true, + }, + CommonHttpProtocolOptions: &envoy_core_v3.HttpProtocolOptions{}, + Http2ProtocolOptions: &envoy_core_v3.Http2ProtocolOptions{ + MaxConcurrentStreams: wrapperspb.UInt32(50), + }, + AccessLog: FileAccessLogEnvoy("/dev/stdout", "", nil, v1alpha1.LogLevelInfo), + UseRemoteAddress: wrapperspb.Bool(true), + NormalizePath: wrapperspb.Bool(true), + StripPortMode: &http.HttpConnectionManager_StripAnyHostPort{ + StripAnyHostPort: true, + }, + PreserveExternalRequestId: true, + MergeSlashes: false, + }), + }, + }, + }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { @@ -1362,6 +1417,7 @@ func TestHTTPConnectionManager(t *testing.T) { ServerHeaderTransformation(tc.serverHeaderTranformation). NumTrustedHops(tc.xffNumTrustedHops). ForwardClientCertificate(tc.forwardClientCertificate). + HTTP2MaxConcurrentStreams(tc.http2MaxConcurrentStreams). DefaultFilters(). Get() diff --git a/internal/xdscache/v3/listener.go b/internal/xdscache/v3/listener.go index ddb096edb14..bf3458bff93 100644 --- a/internal/xdscache/v3/listener.go +++ b/internal/xdscache/v3/listener.go @@ -137,6 +137,9 @@ type ListenerConfig struct { // If no configuration is specified, Envoy will not attempt to balance active connections between worker threads // If specified, the listener will use the exact connection balancer. ConnectionBalancer string + + HTTP2MaxConcurrentStreams *uint32 + // RateLimitConfig optionally configures the global Rate Limit Service to be // used. RateLimitConfig *RateLimitConfig @@ -394,6 +397,7 @@ func (c *ListenerCache) OnChange(root *dag.DAG) { MergeSlashes(cfg.MergeSlashes). ServerHeaderTransformation(cfg.ServerHeaderTransformation). NumTrustedHops(cfg.XffNumTrustedHops). + HTTP2MaxConcurrentStreams(cfg.HTTP2MaxConcurrentStreams). AddFilter(envoy_v3.GlobalRateLimitFilter(envoyGlobalRateLimitConfig(cfg.RateLimitConfig))). Get() @@ -457,6 +461,7 @@ func (c *ListenerCache) OnChange(root *dag.DAG) { NumTrustedHops(cfg.XffNumTrustedHops). AddFilter(envoy_v3.GlobalRateLimitFilter(envoyGlobalRateLimitConfig(cfg.RateLimitConfig))). ForwardClientCertificate(forwardClientCertificate). + HTTP2MaxConcurrentStreams(cfg.HTTP2MaxConcurrentStreams). Get() filters = envoy_v3.Filters(cm) @@ -524,6 +529,7 @@ func (c *ListenerCache) OnChange(root *dag.DAG) { NumTrustedHops(cfg.XffNumTrustedHops). AddFilter(envoy_v3.GlobalRateLimitFilter(envoyGlobalRateLimitConfig(cfg.RateLimitConfig))). ForwardClientCertificate(forwardClientCertificate). + HTTP2MaxConcurrentStreams(cfg.HTTP2MaxConcurrentStreams). Get() // Default filter chain diff --git a/internal/xdscache/v3/listener_test.go b/internal/xdscache/v3/listener_test.go index ddab6b5213e..e593f3e599b 100644 --- a/internal/xdscache/v3/listener_test.go +++ b/internal/xdscache/v3/listener_test.go @@ -32,6 +32,7 @@ import ( envoy_v3 "github.com/projectcontour/contour/internal/envoy/v3" "github.com/projectcontour/contour/internal/k8s" "github.com/projectcontour/contour/internal/protobuf" + "github.com/projectcontour/contour/internal/ref" "github.com/projectcontour/contour/internal/timeout" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/known/durationpb" @@ -3478,6 +3479,142 @@ func TestListenerVisit(t *testing.T) { SocketOptions: envoy_v3.TCPKeepaliveSocketOptions(), }), }, + "httpproxy with HTTP2MaxConcurrentStreams set in listener config": { + ListenerConfig: ListenerConfig{ + HTTP2MaxConcurrentStreams: ref.To(uint32(100)), + }, + objs: []any{ + &contour_api_v1.HTTPProxy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "simple", + Namespace: "default", + }, + Spec: contour_api_v1.HTTPProxySpec{ + VirtualHost: &contour_api_v1.VirtualHost{ + Fqdn: "www.example.com", + }, + Routes: []contour_api_v1.Route{{ + Conditions: []contour_api_v1.MatchCondition{{ + Prefix: "/", + }}, + Services: []contour_api_v1.Service{{ + Name: "backend", + Port: 80, + }}, + }}, + }, + }, + &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "backend", + Namespace: "default", + }, + Spec: v1.ServiceSpec{ + Ports: []v1.ServicePort{{ + Name: "http", + Protocol: "TCP", + Port: 80, + }}, + }, + }, + }, + want: listenermap(&envoy_listener_v3.Listener{ + Name: ENVOY_HTTP_LISTENER, + Address: envoy_v3.SocketAddress("0.0.0.0", 8080), + FilterChains: envoy_v3.FilterChains( + envoy_v3.HTTPConnectionManagerBuilder(). + RouteConfigName(ENVOY_HTTP_LISTENER). + MetricsPrefix(ENVOY_HTTP_LISTENER). + AccessLoggers(envoy_v3.FileAccessLogEnvoy(DEFAULT_HTTP_ACCESS_LOG, "", nil, v1alpha1.LogLevelInfo)). + DefaultFilters(). + HTTP2MaxConcurrentStreams(ref.To(uint32(100))). + Get(), + ), + SocketOptions: envoy_v3.TCPKeepaliveSocketOptions(), + }), + }, + "httpsproxy with HTTP2MaxConcurrentStreams set in listener config": { + ListenerConfig: ListenerConfig{ + HTTP2MaxConcurrentStreams: ref.To(uint32(101)), + }, + objs: []any{ + &contour_api_v1.HTTPProxy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "simple", + Namespace: "default", + }, + Spec: contour_api_v1.HTTPProxySpec{ + VirtualHost: &contour_api_v1.VirtualHost{ + Fqdn: "www.example.com", + TLS: &contour_api_v1.TLS{ + SecretName: "secret", + }, + }, + Routes: []contour_api_v1.Route{{ + Services: []contour_api_v1.Service{{ + Name: "backend", + Port: 80, + }}, + }}, + }, + }, + &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "secret", + Namespace: "default", + }, + Type: "kubernetes.io/tls", + Data: secretdata(CERTIFICATE, RSA_PRIVATE_KEY), + }, + &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "backend", + Namespace: "default", + }, + Spec: v1.ServiceSpec{ + Ports: []v1.ServicePort{{ + Name: "http", + Protocol: "TCP", + Port: 80, + }}, + }, + }, + }, + want: listenermap(&envoy_listener_v3.Listener{ + Name: ENVOY_HTTP_LISTENER, + Address: envoy_v3.SocketAddress("0.0.0.0", 8080), + FilterChains: envoy_v3.FilterChains(envoy_v3.HTTPConnectionManagerBuilder(). + RouteConfigName(ENVOY_HTTP_LISTENER). + MetricsPrefix(ENVOY_HTTP_LISTENER). + AccessLoggers(envoy_v3.FileAccessLogEnvoy(DEFAULT_HTTP_ACCESS_LOG, "", nil, v1alpha1.LogLevelInfo)). + DefaultFilters(). + HTTP2MaxConcurrentStreams(ref.To(uint32(101))). + Get(), + ), + SocketOptions: envoy_v3.TCPKeepaliveSocketOptions(), + }, &envoy_listener_v3.Listener{ + Name: ENVOY_HTTPS_LISTENER, + Address: envoy_v3.SocketAddress("0.0.0.0", 8443), + FilterChains: []*envoy_listener_v3.FilterChain{{ + FilterChainMatch: &envoy_listener_v3.FilterChainMatch{ + ServerNames: []string{"www.example.com"}, + }, + TransportSocket: transportSocket("secret", envoy_tls_v3.TlsParameters_TLSv1_2, nil, "h2", "http/1.1"), + Filters: envoy_v3.Filters(envoy_v3.HTTPConnectionManagerBuilder(). + AddFilter(envoy_v3.FilterMisdirectedRequests("www.example.com")). + DefaultFilters(). + MetricsPrefix(ENVOY_HTTPS_LISTENER). + RouteConfigName(path.Join("https", "www.example.com")). + AccessLoggers(envoy_v3.FileAccessLogEnvoy(DEFAULT_HTTP_ACCESS_LOG, "", nil, v1alpha1.LogLevelInfo)). + HTTP2MaxConcurrentStreams(ref.To(uint32(101))). + Get()), + }}, + ListenerFilters: envoy_v3.ListenerFilters( + envoy_v3.TLSInspector(), + ), + SocketOptions: envoy_v3.TCPKeepaliveSocketOptions(), + }), + }, } for name, tc := range tests { diff --git a/pkg/config/parameters.go b/pkg/config/parameters.go index a04a2b4c941..a4de1480fac 100644 --- a/pkg/config/parameters.go +++ b/pkg/config/parameters.go @@ -442,7 +442,15 @@ type ListenerParameters struct { // I/O cycles. Can be used as a mitigation for CVE-2023-44487 when abusive traffic is // detected. Configures the http.max_requests_per_io_cycle Envoy runtime setting. The default // value when this is not set is no limit. - MaxRequestsPerIOCycle *uint32 `yaml:"max-requests-per-io-cycle"` + MaxRequestsPerIOCycle *uint32 `yaml:"max-requests-per-io-cycle,omitempty"` + + // Defines the value for SETTINGS_MAX_CONCURRENT_STREAMS Envoy will advertise in the + // SETTINGS frame in HTTP/2 connections and the limit for concurrent streams allowed + // for a peer on a single HTTP/2 connection. It is recommended to not set this lower + // than 100 but this field can be used to bound resource usage by HTTP/2 connections + // and mitigate attacks like CVE-2023-44487. The default value when this is not set is + // unlimited. + HTTP2MaxConcurrentStreams *uint32 `yaml:"http2-max-concurrent-streams,omitempty"` } func (p *ListenerParameters) Validate() error { @@ -457,6 +465,11 @@ func (p *ListenerParameters) Validate() error { if p.MaxRequestsPerIOCycle != nil && *p.MaxRequestsPerIOCycle < 1 { return fmt.Errorf("invalid max connections per IO cycle value %q set on listener, minimum value is 1", *p.MaxRequestsPerIOCycle) } + + if p.HTTP2MaxConcurrentStreams != nil && *p.HTTP2MaxConcurrentStreams < 1 { + return fmt.Errorf("invalid max HTTP/2 concurrent streams value %q set on listener, minimum value is 1", *p.HTTP2MaxConcurrentStreams) + } + return nil } diff --git a/pkg/config/parameters_test.go b/pkg/config/parameters_test.go index 871207e7804..b025ea85253 100644 --- a/pkg/config/parameters_test.go +++ b/pkg/config/parameters_test.go @@ -419,6 +419,13 @@ network: admin-port: 9001 `) + check(func(t *testing.T, conf *Parameters) { + assert.Equal(t, ref.To(uint32(10)), conf.Listener.HTTP2MaxConcurrentStreams) + }, ` +listener: + http2-max-concurrent-streams: 10 +`) + check(func(t *testing.T, conf *Parameters) { assert.Equal(t, ref.To(uint32(1)), conf.Listener.MaxRequestsPerIOCycle) }, ` @@ -507,4 +514,11 @@ func TestListenerValidation(t *testing.T) { MaxRequestsPerIOCycle: ref.To(uint32(0)), } require.Error(t, l.Validate()) + l = &ListenerParameters{ + HTTP2MaxConcurrentStreams: ref.To(uint32(1)), + } + require.NoError(t, l.Validate()) + l = &ListenerParameters{ + HTTP2MaxConcurrentStreams: ref.To(uint32(0)), + } } diff --git a/site/content/docs/main/config/api-reference.html b/site/content/docs/main/config/api-reference.html index edbd5c79505..5dce95a3059 100644 --- a/site/content/docs/main/config/api-reference.html +++ b/site/content/docs/main/config/api-reference.html @@ -5996,6 +5996,24 @@

EnvoyListenerConfig value when this is not set is no limit.

+ + +httpMaxConcurrentStreams +
+ +uint32 + + + +(Optional) +

Defines the value for SETTINGS_MAX_CONCURRENT_STREAMS Envoy will advertise in the +SETTINGS frame in HTTP/2 connections and the limit for concurrent streams allowed +for a peer on a single HTTP/2 connection. It is recommended to not set this lower +than 100 but this field can be used to bound resource usage by HTTP/2 connections +and mitigate attacks like CVE-2023-44487. The default value when this is not set is +unlimited.

+ +

EnvoyLogging diff --git a/site/content/docs/main/configuration.md b/site/content/docs/main/configuration.md index 8660031462b..26c7595fe82 100644 --- a/site/content/docs/main/configuration.md +++ b/site/content/docs/main/configuration.md @@ -186,6 +186,7 @@ The listener configuration block can be used to configure various parameters for | ------------------- | ------ | ------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | connection-balancer | string | `""` | This field specifies the listener connection balancer. If the value is `exact`, the listener will use the exact connection balancer to balance connections between threads in a single Envoy process. See [the Envoy documentation][14] for more information. | | max-requests-per-io-cycle | int | none | Defines the limit on number of HTTP requests that Envoy will process from a single connection in a single I/O cycle. Requests over this limit are processed in subsequent I/O cycles. Can be used as a mitigation for CVE-2023-44487 when abusive traffic is detected. Configures the `http.max_requests_per_io_cycle` Envoy runtime setting. The default value when this is not set is no limit. | +| http2-max-concurrent-streams | int | none | Defines the value for SETTINGS_MAX_CONCURRENT_STREAMS Envoy will advertise in the SETTINGS frame in HTTP/2 connections and the limit for concurrent streams allowed for a peer on a single HTTP/2 connection. It is recommended to not set this lower than 100 but this field can be used to bound resource usage by HTTP/2 connections and mitigate attacks like CVE-2023-44487. The default value when this is not set is unlimited. | ### Server Configuration From 357a4d7e8d9916f26fc22296d3781b97e21ea7ed Mon Sep 17 00:00:00 2001 From: Sunjay Bhatia Date: Mon, 16 Oct 2023 16:29:51 +0000 Subject: [PATCH 3/3] lint fix Signed-off-by: Sunjay Bhatia --- internal/xdscache/v3/runtime.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/xdscache/v3/runtime.go b/internal/xdscache/v3/runtime.go index b4124bfa217..6c26f7257fb 100644 --- a/internal/xdscache/v3/runtime.go +++ b/internal/xdscache/v3/runtime.go @@ -28,28 +28,28 @@ type ConfigurableRuntimeSettings struct { } // RuntimeCache manages the contents of the gRPC RTDS cache. -type runtimeCache struct { +type RuntimeCache struct { contour.Cond runtimeKV map[string]*structpb.Value } // NewRuntimeCache builds a RuntimeCache with the provided runtime // settings that will be set in the runtime layer configured by Contour. -func NewRuntimeCache(runtimeSettings ConfigurableRuntimeSettings) *runtimeCache { +func NewRuntimeCache(runtimeSettings ConfigurableRuntimeSettings) *RuntimeCache { runtimeKV := make(map[string]*structpb.Value) if runtimeSettings.MaxRequestsPerIOCycle != nil && *runtimeSettings.MaxRequestsPerIOCycle > 0 { runtimeKV["http.max_requests_per_io_cycle"] = structpb.NewNumberValue(float64(*runtimeSettings.MaxRequestsPerIOCycle)) } - return &runtimeCache{runtimeKV: runtimeKV} + return &RuntimeCache{runtimeKV: runtimeKV} } // Contents returns all Runtime layers. -func (c *runtimeCache) Contents() []proto.Message { +func (c *RuntimeCache) Contents() []proto.Message { return protobuf.AsMessages(envoy_v3.RuntimeLayers(c.runtimeKV)) } // Query returns only the "dynamic" layer if requested, otherwise empty. -func (c *runtimeCache) Query(names []string) []proto.Message { +func (c *RuntimeCache) Query(names []string) []proto.Message { for _, name := range names { if name == envoy_v3.DynamicRuntimeLayerName { return protobuf.AsMessages(envoy_v3.RuntimeLayers(c.runtimeKV)) @@ -58,8 +58,8 @@ func (c *runtimeCache) Query(names []string) []proto.Message { return []proto.Message{} } -func (*runtimeCache) TypeURL() string { return resource.RuntimeType } +func (*RuntimeCache) TypeURL() string { return resource.RuntimeType } -func (c *runtimeCache) OnChange(root *dag.DAG) { +func (c *RuntimeCache) OnChange(root *dag.DAG) { // DAG changes do not affect runtime layers at the moment. }