[release/v1.3] release v1.3.3 cherry-pick

internal/gatewayapi/backendtrafficpolicy.go

@@ -933,19 +933,37 @@
 	}, nil
 }
 
+func checkResponseBodySize(b *string) error {
+	// Make this configurable in the future
+	// https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route.proto.html#max_direct_response_body_size_bytes
+	maxDirectResponseSize := 4096
+	lenB := len(*b)
+	if lenB > maxDirectResponseSize {
+		return fmt.Errorf("response.body size %d greater than the max size %d", lenB, maxDirectResponseSize)
+	}
+
+	return nil
+}
+
 func getCustomResponseBody(body *egv1a1.CustomResponseBody, resources *resource.Resources, policyNs string) (*string, error) {
 	if body != nil && body.Type != nil && *body.Type == egv1a1.ResponseValueTypeValueRef {
 		cm := resources.GetConfigMap(policyNs, string(body.ValueRef.Name))
 		if cm != nil {
 			b, dataOk := cm.Data["response.body"]
 			switch {
 			case dataOk:
+				if err := checkResponseBodySize(&b); err != nil {
+					return nil, err
+				}
 				return &b, nil
 			case len(cm.Data) > 0: // Fallback to the first key if response.body is not found
 				for _, value := range cm.Data {
 					b = value
 					break
 				}
+				if err := checkResponseBodySize(&b); err != nil {
+					return nil, err
+				}
 				return &b, nil
 			default:
 				return nil, fmt.Errorf("can't find the key response.body in the referenced configmap %s", body.ValueRef.Name)
@@ -955,6 +973,9 @@
 			return nil, fmt.Errorf("can't find the referenced configmap %s", body.ValueRef.Name)
 		}
 	} else if body != nil && body.Inline != nil {
+		if err := checkResponseBodySize(body.Inline); err != nil {
+			return nil, err
+		}
 		return body.Inline, nil
 	}
 

internal/gatewayapi/clienttrafficpolicy.go

@@ -948,7 +948,12 @@ func translateEarlyRequestHeaders(headerModifier *gwapiv1.HTTPHeaderFilter) ([]i
 			}
 			// Per Gateway API specification on HTTPHeaderName, : and / are invalid characters in header names
 			if strings.ContainsAny(string(addHeader.Name), "/:") {
-				errs = errors.Join(errs, fmt.Errorf("EarlyRequestHeaders Filter cannot set headers with a '/' or ':' character in them. Header: %q", string(addHeader.Name)))
+				errs = errors.Join(errs, fmt.Errorf("EarlyRequestHeaders cannot add a header with a '/' or ':' character in them. Header: '%q'", string(addHeader.Name)))
+				continue
+			}
+			// Gateway API specification allows only valid value as defined by RFC 7230
+			if !HeaderValueRegexp.MatchString(addHeader.Value) {
+				errs = errors.Join(errs, fmt.Errorf("EarlyRequestHeaders cannot add a header with an invalid value. Header: '%q'", string(addHeader.Name)))
 				continue
 			}
 			// Check if the header is a duplicate
@@ -988,7 +993,12 @@ func translateEarlyRequestHeaders(headerModifier *gwapiv1.HTTPHeaderFilter) ([]i
 			}
 			// Per Gateway API specification on HTTPHeaderName, : and / are invalid characters in header names
 			if strings.ContainsAny(string(setHeader.Name), "/:") {
-				errs = errors.Join(errs, fmt.Errorf("EarlyRequestHeaders cannot set headers with a '/' or ':' character in them. Header: '%s'", string(setHeader.Name)))
+				errs = errors.Join(errs, fmt.Errorf("EarlyRequestHeaders cannot set a header with a '/' or ':' character in them. Header: '%q'", string(setHeader.Name)))
+				continue
+			}
+			// Gateway API specification allows only valid value as defined by RFC 7230
+			if !HeaderValueRegexp.MatchString(setHeader.Value) {
+				errs = errors.Join(errs, fmt.Errorf("EarlyRequestHeaders cannot set a header with an invalid value. Header: '%q'", string(setHeader.Name)))
 				continue
 			}
 

internal/gatewayapi/filters.go

@@ -62,6 +62,9 @@ type HTTPFilterIR struct {
 	ExtensionRefs []*ir.UnstructuredRef
 }
 
+// Header value pattern according to RFC 7230
+var HeaderValueRegexp = regexp.MustCompile(`^[!-~]+([\t ]?[!-~]+)*$`)
+
 // ProcessHTTPFilters translates gateway api http filters to IRs.
 func (t *Translator) ProcessHTTPFilters(parentRef *RouteParentContext,
 	route RouteContext,
@@ -370,6 +373,7 @@ func (t *Translator) processRequestHeaderModifierFilter(
 			emptyFilterConfig = false
 		}
 		for _, addHeader := range headersToAdd {
+
 			emptyFilterConfig = false
 			if addHeader.Name == "" {
 				updateRouteStatusForFilter(
@@ -378,16 +382,27 @@ func (t *Translator) processRequestHeaderModifierFilter(
 				// try to process the rest of the headers and produce a valid config.
 				continue
 			}
+
 			if !isModifiableHeader(string(addHeader.Name)) {
 				updateRouteStatusForFilter(
 					filterContext,
 					fmt.Sprintf(
-						"Header: %q. The RequestHeaderModifier filter cannot set the Host header or headers with a '/' "+
+						"Header: %q. The RequestHeaderModifier filter cannot add the Host header or headers with a '/' "+
 							"or ':' character in them. To modify the Host header use the URLRewrite or the HTTPRouteFilter filter.",
 						string(addHeader.Name)),
 				)
 				continue
 			}
+
+			if !HeaderValueRegexp.MatchString(addHeader.Value) {
+				updateRouteStatusForFilter(
+					filterContext,
+					fmt.Sprintf(
+						"Header: %q. RequestHeaderModifier Filter cannot add a header with an invalid value.",
+						string(addHeader.Name)))
+				continue
+			}
+
 			// Check if the header is a duplicate
 			headerKey := string(addHeader.Name)
 			canAddHeader := true
@@ -437,6 +452,15 @@ func (t *Translator) processRequestHeaderModifierFilter(
 				continue
 			}
 
+			if !HeaderValueRegexp.MatchString(setHeader.Value) {
+				updateRouteStatusForFilter(
+					filterContext,
+					fmt.Sprintf(
+						"Header: %q. RequestHeaderModifier Filter cannot set a header with an invalid value.",
+						string(setHeader.Name)))
+				continue
+			}
+
 			// Check if the header to be set has already been configured
 			headerKey := string(setHeader.Name)
 			canAddHeader := true
@@ -550,6 +574,7 @@ func (t *Translator) processResponseHeaderModifierFilter(
 				// try to process the rest of the headers and produce a valid config.
 				continue
 			}
+
 			if !isModifiableHeader(string(addHeader.Name)) {
 				updateRouteStatusForFilter(
 					filterContext,
@@ -559,6 +584,16 @@ func (t *Translator) processResponseHeaderModifierFilter(
 						string(addHeader.Name)))
 				continue
 			}
+
+			if !HeaderValueRegexp.MatchString(addHeader.Value) {
+				updateRouteStatusForFilter(
+					filterContext,
+					fmt.Sprintf(
+						"Header: %q. ResponseHeaderModifier Filter cannot add a header with an invalid value.",
+						string(addHeader.Name)))
+				continue
+			}
+
 			// Check if the header is a duplicate
 			headerKey := string(addHeader.Name)
 			canAddHeader := true
@@ -607,6 +642,15 @@ func (t *Translator) processResponseHeaderModifierFilter(
 				continue
 			}
 
+			if !HeaderValueRegexp.MatchString(setHeader.Value) {
+				updateRouteStatusForFilter(
+					filterContext,
+					fmt.Sprintf(
+						"Header: %q. ResponseHeaderModifier Filter cannot set a header with an invalid value.",
+						string(setHeader.Name)))
+				continue
+			}
+
 			// Check if the header to be set has already been configured
 			headerKey := string(setHeader.Name)
 			canAddHeader := true

internal/gatewayapi/sort.go

@@ -63,6 +63,7 @@
 	// Equal case
 
 	// 3. Sort based on the number of Header matches.
+	// When the number is same, sort based on number of Exact Header matches.
 	hCountI := len(x[i].HeaderMatches)
 	hCountJ := len(x[j].HeaderMatches)
 	if hCountI < hCountJ {
@@ -71,12 +72,31 @@
 	if hCountI > hCountJ {
 		return false
 	}
+
+	hExtNumberI := numberOfExactMatches(x[i].HeaderMatches)
+	hExtNumberJ := numberOfExactMatches(x[j].HeaderMatches)
+	if hExtNumberI < hExtNumberJ {
+		return true
+	}
+	if hExtNumberI > hExtNumberJ {
+		return false
+	}
 	// Equal case
 
 	// 4. Sort based on the number of Query param matches.
+	// When the number is same, sort based on number of Exact Query param matches.
 	qCountI := len(x[i].QueryParamMatches)
 	qCountJ := len(x[j].QueryParamMatches)
-	return qCountI < qCountJ
+	if qCountI < qCountJ {
+		return true
+	}
+	if qCountI > qCountJ {
+		return false
+	}
+
+	qExtNumberI := numberOfExactMatches(x[i].QueryParamMatches)
+	qExtNumberJ := numberOfExactMatches(x[j].QueryParamMatches)
+	return qExtNumberI < qExtNumberJ
 }
 
 // sortXdsIR sorts the xdsIR based on the match precedence
@@ -107,3 +127,13 @@
 	}
 	return 0
 }
+
+func numberOfExactMatches(stringMatches []*ir.StringMatch) int {
+	var cnt int
+	for _, stringMatch := range stringMatches {
+		if stringMatch != nil && stringMatch.Exact != nil {
+			cnt++
+		}
+	}
+	return cnt
+}

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

@@ -13,13 +13,18 @@ clientTrafficPolicies:
         add:
         - name: ""
           value: "empty"
-        - name: "invalid"
+        - name: "Example/Header/1"
           value: ":/"
+        - name: "example-header-2"
+          value: |
+            multi-line-header-value
         set:
         - name: ""
           value: "empty"
-        - name: "invalid"
+        - name: "Example:Header:3"
           value: ":/"
+        - name: "example-header-4"
+          value: "  invalid"
         remove:
         - ""
     targetRef:

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

@@ -11,15 +11,20 @@ clientTrafficPolicies:
         add:
         - name: ""
           value: empty
-        - name: invalid
+        - name: Example/Header/1
           value: :/
+        - name: example-header-2
+          value: |
+            multi-line-header-value
         remove:
         - ""
         set:
         - name: ""
           value: empty
-        - name: invalid
+        - name: Example:Header:3
           value: :/
+        - name: example-header-4
+          value: '  invalid'
       enableEnvoyHeaders: true
       preserveXRequestID: true
       withUnderscoresAction: Allow
@@ -38,8 +43,13 @@ clientTrafficPolicies:
       - lastTransitionTime: null
         message: |-
           Headers: EarlyRequestHeaders cannot add a header with an empty name
+          EarlyRequestHeaders cannot add a header with a '/' or ':' character in them. Header: '"Example/Header/1"'
+          EarlyRequestHeaders cannot add a header with an invalid value. Header: '"example-header-2"'
           EarlyRequestHeaders cannot set a header with an empty name
-          EarlyRequestHeaders cannot remove a header with an empty name.
+          EarlyRequestHeaders cannot set a header with a '/' or ':' character in them. Header: '"Example:Header:3"'
+          EarlyRequestHeaders cannot set a header with an invalid value. Header: '"example-header-4"'
+          EarlyRequestHeaders cannot remove a header with an empty name
+          EarlyRequestHeaders did not provide valid configuration to add/set/remove any headers.
         reason: Invalid
         status: "False"
         type: Accepted

internal/gatewayapi/testdata/httproute-with-direct-response.in.yaml

@@ -49,7 +49,7 @@ httpRoutes:
 - apiVersion: gateway.networking.k8s.io/v1
   kind: HTTPRoute
   metadata:
-    name: direct-response-with-errors
+    name: direct-response-with-value-not-found
     namespace: default
   spec:
     parentRefs:
@@ -67,6 +67,27 @@ httpRoutes:
           group: gateway.envoyproxy.io
           kind: HTTPRouteFilter
           name: direct-response-value-ref-not-found
+- apiVersion: gateway.networking.k8s.io/v1
+  kind: HTTPRoute
+  metadata:
+    name: direct-response-too-long
+    namespace: default
+  spec:
+    parentRefs:
+    - name: gateway-1
+      namespace: envoy-gateway
+      sectionName: http
+    rules:
+    - matches:
+      - path:
+          type: PathPrefix
+          value: /too-long
+      filters:
+      - type: ExtensionRef
+        extensionRef:
+          group: gateway.envoyproxy.io
+          kind: HTTPRouteFilter
+          name: direct-response-too-long
 configMaps:
 - apiVersion: v1
   kind: ConfigMap
@@ -117,3 +138,14 @@ httpFilters:
           group: ""
           kind: ConfigMap
           name: value-ref-response
+- apiVersion: gateway.envoyproxy.io/v1alpha1
+  kind: HTTPRouteFilter
+  metadata:
+    name: direct-response-too-long
+    namespace: default
+  spec:
+    directResponse:
+      contentType: text/plain
+      body:
+        type: Inline
+        inline: "-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------"