Skip to content

Commit 5e7df65

Browse files
gavinkflamgavinkflam
authored
fix: add validation for header values (envoyproxy#5933)
Signed-off-by: Gavin Lam <gavin.oss@tutamail.com>
1 parent ef50718 commit 5e7df65

13 files changed

+451
-81
lines changed

‎internal/gatewayapi/clienttrafficpolicy.go‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -952,7 +952,12 @@ func translateEarlyRequestHeaders(headerModifier *gwapiv1.HTTPHeaderFilter) ([]i
952952
}
953953
// Per Gateway API specification on HTTPHeaderName, : and / are invalid characters in header names
954954
if strings.ContainsAny(string(addHeader.Name), "/:") {
955-
errs = errors.Join(errs, fmt.Errorf("EarlyRequestHeaders Filter cannot set headers with a '/' or ':' character in them. Header: %q", string(addHeader.Name)))
955+
errs = errors.Join(errs, fmt.Errorf("EarlyRequestHeaders cannot add a header with a '/' or ':' character in them. Header: '%q'", string(addHeader.Name)))
956+
continue
957+
}
958+
// Gateway API specification allows only valid value as defined by RFC 7230
959+
if !HeaderValueRegexp.MatchString(addHeader.Value) {
960+
errs = errors.Join(errs, fmt.Errorf("EarlyRequestHeaders cannot add a header with an invalid value. Header: '%q'", string(addHeader.Name)))
956961
continue
957962
}
958963
// Check if the header is a duplicate
@@ -992,7 +997,12 @@ func translateEarlyRequestHeaders(headerModifier *gwapiv1.HTTPHeaderFilter) ([]i
992997
}
993998
// Per Gateway API specification on HTTPHeaderName, : and / are invalid characters in header names
994999
if strings.ContainsAny(string(setHeader.Name), "/:") {
995-
errs = errors.Join(errs, fmt.Errorf("EarlyRequestHeaders cannot set headers with a '/' or ':' character in them. Header: '%s'", string(setHeader.Name)))
1000+
errs = errors.Join(errs, fmt.Errorf("EarlyRequestHeaders cannot set a header with a '/' or ':' character in them. Header: '%q'", string(setHeader.Name)))
1001+
continue
1002+
}
1003+
// Gateway API specification allows only valid value as defined by RFC 7230
1004+
if !HeaderValueRegexp.MatchString(setHeader.Value) {
1005+
errs = errors.Join(errs, fmt.Errorf("EarlyRequestHeaders cannot set a header with an invalid value. Header: '%q'", string(setHeader.Name)))
9961006
continue
9971007
}
9981008

‎internal/gatewayapi/filters.go‎

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,9 @@ type HTTPFilterIR struct {
6666
ExtensionRefs []*ir.UnstructuredRef
6767
}
6868

69+
// Header value pattern according to RFC 7230
70+
var HeaderValueRegexp = regexp.MustCompile(`^[!-~]+([\t ]?[!-~]+)*$`)
71+
6972
// ProcessHTTPFilters translates gateway api http filters to IRs.
7073
func (t *Translator) ProcessHTTPFilters(parentRef *RouteParentContext,
7174
route RouteContext,
@@ -376,6 +379,7 @@ func (t *Translator) processRequestHeaderModifierFilter(
376379
emptyFilterConfig = false
377380
}
378381
for _, addHeader := range headersToAdd {
382+
379383
emptyFilterConfig = false
380384
if addHeader.Name == "" {
381385
updateRouteStatusForFilter(
@@ -384,16 +388,27 @@ func (t *Translator) processRequestHeaderModifierFilter(
384388
// try to process the rest of the headers and produce a valid config.
385389
continue
386390
}
391+
387392
if !isModifiableHeader(string(addHeader.Name)) {
388393
updateRouteStatusForFilter(
389394
filterContext,
390395
fmt.Sprintf(
391-
"Header: %q. The RequestHeaderModifier filter cannot set the Host header or headers with a '/' "+
396+
"Header: %q. The RequestHeaderModifier filter cannot add the Host header or headers with a '/' "+
392397
"or ':' character in them. To modify the Host header use the URLRewrite or the HTTPRouteFilter filter.",
393398
string(addHeader.Name)),
394399
)
395400
continue
396401
}
402+
403+
if !HeaderValueRegexp.MatchString(addHeader.Value) {
404+
updateRouteStatusForFilter(
405+
filterContext,
406+
fmt.Sprintf(
407+
"Header: %q. RequestHeaderModifier Filter cannot add a header with an invalid value.",
408+
string(addHeader.Name)))
409+
continue
410+
}
411+
397412
// Check if the header is a duplicate
398413
headerKey := string(addHeader.Name)
399414
canAddHeader := true
@@ -443,6 +458,15 @@ func (t *Translator) processRequestHeaderModifierFilter(
443458
continue
444459
}
445460

461+
if !HeaderValueRegexp.MatchString(setHeader.Value) {
462+
updateRouteStatusForFilter(
463+
filterContext,
464+
fmt.Sprintf(
465+
"Header: %q. RequestHeaderModifier Filter cannot set a header with an invalid value.",
466+
string(setHeader.Name)))
467+
continue
468+
}
469+
446470
// Check if the header to be set has already been configured
447471
headerKey := string(setHeader.Name)
448472
canAddHeader := true
@@ -556,6 +580,7 @@ func (t *Translator) processResponseHeaderModifierFilter(
556580
// try to process the rest of the headers and produce a valid config.
557581
continue
558582
}
583+
559584
if !isModifiableHeader(string(addHeader.Name)) {
560585
updateRouteStatusForFilter(
561586
filterContext,
@@ -565,6 +590,16 @@ func (t *Translator) processResponseHeaderModifierFilter(
565590
string(addHeader.Name)))
566591
continue
567592
}
593+
594+
if !HeaderValueRegexp.MatchString(addHeader.Value) {
595+
updateRouteStatusForFilter(
596+
filterContext,
597+
fmt.Sprintf(
598+
"Header: %q. ResponseHeaderModifier Filter cannot add a header with an invalid value.",
599+
string(addHeader.Name)))
600+
continue
601+
}
602+
568603
// Check if the header is a duplicate
569604
headerKey := string(addHeader.Name)
570605
canAddHeader := true
@@ -613,6 +648,15 @@ func (t *Translator) processResponseHeaderModifierFilter(
613648
continue
614649
}
615650

651+
if !HeaderValueRegexp.MatchString(setHeader.Value) {
652+
updateRouteStatusForFilter(
653+
filterContext,
654+
fmt.Sprintf(
655+
"Header: %q. ResponseHeaderModifier Filter cannot set a header with an invalid value.",
656+
string(setHeader.Name)))
657+
continue
658+
}
659+
616660
// Check if the header to be set has already been configured
617661
headerKey := string(setHeader.Name)
618662
canAddHeader := true

‎internal/gatewayapi/testdata/clienttrafficpolicy-headers-error.in.yaml‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,18 @@ clientTrafficPolicies:
1313
add:
1414
- name: ""
1515
value: "empty"
16-
- name: "invalid"
16+
- name: "Example/Header/1"
1717
value: ":/"
18+
- name: "example-header-2"
19+
value: |
20+
multi-line-header-value
1821
set:
1922
- name: ""
2023
value: "empty"
21-
- name: "invalid"
24+
- name: "Example:Header:3"
2225
value: ":/"
26+
- name: "example-header-4"
27+
value: " invalid"
2328
remove:
2429
- ""
2530
targetRef:

‎internal/gatewayapi/testdata/clienttrafficpolicy-headers-error.out.yaml‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,20 @@ clientTrafficPolicies:
1111
add:
1212
- name: ""
1313
value: empty
14-
- name: invalid
14+
- name: Example/Header/1
1515
value: :/
16+
- name: example-header-2
17+
value: |
18+
multi-line-header-value
1619
remove:
1720
- ""
1821
set:
1922
- name: ""
2023
value: empty
21-
- name: invalid
24+
- name: Example:Header:3
2225
value: :/
26+
- name: example-header-4
27+
value: ' invalid'
2328
enableEnvoyHeaders: true
2429
preserveXRequestID: true
2530
withUnderscoresAction: Allow
@@ -38,8 +43,13 @@ clientTrafficPolicies:
3843
- lastTransitionTime: null
3944
message: |-
4045
Headers: EarlyRequestHeaders cannot add a header with an empty name
46+
EarlyRequestHeaders cannot add a header with a '/' or ':' character in them. Header: '"Example/Header/1"'
47+
EarlyRequestHeaders cannot add a header with an invalid value. Header: '"example-header-2"'
4148
EarlyRequestHeaders cannot set a header with an empty name
42-
EarlyRequestHeaders cannot remove a header with an empty name.
49+
EarlyRequestHeaders cannot set a header with a '/' or ':' character in them. Header: '"Example:Header:3"'
50+
EarlyRequestHeaders cannot set a header with an invalid value. Header: '"example-header-4"'
51+
EarlyRequestHeaders cannot remove a header with an empty name
52+
EarlyRequestHeaders did not provide valid configuration to add/set/remove any headers.
4353
reason: Invalid
4454
status: "False"
4555
type: Accepted

‎internal/gatewayapi/testdata/httproute-with-header-filter-empty-header-values.in.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ httpRoutes:
3939
requestHeaderModifier:
4040
set:
4141
- name: "example-header-1"
42-
value: ""
42+
value: "dummy"
4343
add:
4444
- name: "example-header-2"
4545
value: ""

‎internal/gatewayapi/testdata/httproute-with-header-filter-empty-header-values.out.yaml‎

Lines changed: 5 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ httpRoutes:
6565
value: ""
6666
set:
6767
- name: example-header-1
68-
value: ""
68+
value: dummy
6969
type: RequestHeaderModifier
7070
matches:
7171
- path:
@@ -74,9 +74,10 @@ httpRoutes:
7474
parents:
7575
- conditions:
7676
- lastTransitionTime: null
77-
message: Route is accepted
78-
reason: Accepted
79-
status: "True"
77+
message: 'Header: "example-header-2". RequestHeaderModifier Filter cannot
78+
add a header with an invalid value.'
79+
reason: UnsupportedValue
80+
status: "False"
8081
type: Accepted
8182
- lastTransitionTime: null
8283
message: Resolved all the Object references for the Route
@@ -125,37 +126,6 @@ xdsIR:
125126
escapedSlashesAction: UnescapeAndRedirect
126127
mergeSlashes: true
127128
port: 10080
128-
routes:
129-
- addRequestHeaders:
130-
- append: true
131-
name: example-header-2
132-
value:
133-
- ""
134-
- append: false
135-
name: example-header-1
136-
value:
137-
- ""
138-
destination:
139-
name: httproute/default/httproute-1/rule/0
140-
settings:
141-
- addressType: IP
142-
endpoints:
143-
- host: 7.7.7.7
144-
port: 8080
145-
name: httproute/default/httproute-1/rule/0/backend/0
146-
protocol: HTTP
147-
weight: 1
148-
hostname: gateway.envoyproxy.io
149-
isHTTP2: false
150-
metadata:
151-
kind: HTTPRoute
152-
name: httproute-1
153-
namespace: default
154-
name: httproute/default/httproute-1/rule/0/match/0/gateway_envoyproxy_io
155-
pathMatch:
156-
distinct: false
157-
name: ""
158-
prefix: /
159129
readyListener:
160130
address: 0.0.0.0
161131
ipFamily: IPv4

‎internal/gatewayapi/testdata/httproute-with-header-filter-invalid-header-values.in.yaml‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
gateways:
2+
- apiVersion: gateway.networking.k8s.io/v1
3+
kind: Gateway
4+
metadata:
5+
namespace: envoy-gateway
6+
name: gateway-1
7+
spec:
8+
gatewayClassName: envoy-gateway-class
9+
listeners:
10+
- name: http
11+
protocol: HTTP
12+
port: 80
13+
hostname: "*.envoyproxy.io"
14+
allowedRoutes:
15+
namespaces:
16+
from: All
17+
httpRoutes:
18+
- apiVersion: gateway.networking.k8s.io/v1
19+
kind: HTTPRoute
20+
metadata:
21+
namespace: default
22+
name: httproute-1
23+
spec:
24+
hostnames:
25+
- gateway.envoyproxy.io
26+
parentRefs:
27+
- namespace: envoy-gateway
28+
name: gateway-1
29+
sectionName: http
30+
rules:
31+
- matches:
32+
- path:
33+
value: "/"
34+
backendRefs:
35+
- name: service-1
36+
port: 8080
37+
filters:
38+
- type: RequestHeaderModifier
39+
requestHeaderModifier:
40+
set:
41+
- name: "example-header-1"
42+
value: |
43+
multi-line-header-value
44+
add:
45+
- name: "example-header-2"
46+
value: "dummy"
47+

0 commit comments

Comments
 (0)