validate the path with PathSeparatedPrefix? #1044

qicz · 2023-02-15T03:33:19Z

current gateway supports config match path with PathSeparatedPrefix, but does not validate in the gateway, when config path with invalid value like end with /, the gateway and envoy recv the dynamic route will occur error.

HTTPRoute config

apiVersion: gateway.networking.k8s.io/v1beta1
kind: HTTPRoute
metadata:
  name: backendxx
spec:
  parentRefs:
    - name: eg
  rules:
    - backendRefs:
        - group: ""
          kind: Service
          name: backend
          port: 3000
          weight: 1
      matches:
        - path:
            type: PathPrefix
            value: /backend/

gateway log

2023-02-15 13:29:15 2023-02-15T05:29:15.921Z    INFO    cache/logrwrapper.go:29 Got a new request, response_nonce 2, nodeID envoy-default-eg-64656661-7db8b55db6-gttln, node_version v1.26.0    {"runner": "xds-server"}
2023-02-15 13:29:15 2023-02-15T05:29:15.921Z    INFO    cache/logrwrapper.go:29 handling v3 xDS resource request, response_nonce 2, nodeID envoy-default-eg-64656661-7db8b55db6-gttln, node_version v1.26.0, resource_names_subscribe [], resource_names_unsubscribe [], type_url type.googleapis.com/envoy.config.route.v3.RouteConfiguration, errorCode 13, errorMessage Proto constraint validation failed (RouteConfigurationValidationError.VirtualHosts[0]: embedded message failed validation | caused by VirtualHostValidationError.Routes[0]: embedded message failed validation | caused by RouteValidationError.Match: embedded message failed validation | caused by RouteMatchValidationError.PathSeparatedPrefix: value does not match regex pattern "^[^?#]+[^?#/]$"): name: "default-eg-http"
2023-02-15 13:29:15 virtual_hosts {
2023-02-15 13:29:15   name: "default-eg-http"
2023-02-15 13:29:15   domains: "*"
2023-02-15 13:29:15   routes {
2023-02-15 13:29:15     match {
2023-02-15 13:29:15       path_separated_prefix: "/backend/"
2023-02-15 13:29:15     }
2023-02-15 13:29:15     route {
2023-02-15 13:29:15       cluster: "default-backendxx-rule-0-match-0-*"
2023-02-15 13:29:15     }
2023-02-15 13:29:15   }
2023-02-15 13:29:15 }
2023-02-15 13:29:15     {"runner": "xds-server"}

envoy log

2023-02-15 13:29:15 [2023-02-15 05:29:15.920][1][warning][config] [source/common/config/new_delta_subscription_state.cc:288] delta config for type.googleapis.com/envoy.config.route.v3.RouteConfiguration rejected: Proto constraint validation failed (RouteConfigurationValidationError.VirtualHosts[0]: embedded message failed validation | caused by VirtualHostValidationError.Routes[0]: embedded message failed validation | caused by RouteValidationError.Match: embedded message failed validation | caused by RouteMatchValidationError.PathSeparatedPrefix: value does not match regex pattern "^[^?#]+[^?#/]$"): name: "default-eg-http"
2023-02-15 13:29:15 virtual_hosts {
2023-02-15 13:29:15   name: "default-eg-http"
2023-02-15 13:29:15   domains: "*"
2023-02-15 13:29:15   routes {
2023-02-15 13:29:15     match {
2023-02-15 13:29:15       path_separated_prefix: "/backend/"
2023-02-15 13:29:15     }
2023-02-15 13:29:15     route {
2023-02-15 13:29:15       cluster: "default-backendxx-rule-0-match-0-*"
2023-02-15 13:29:15     }
2023-02-15 13:29:15   }
2023-02-15 13:29:15 }
2023-02-15 13:29:15 
2023-02-15 13:29:15 [2023-02-15 05:29:15.920][1][warning][config] [source/common/config/grpc_subscription_impl.cc:128] gRPC config for type.googleapis.com/envoy.config.route.v3.RouteConfiguration rejected: Proto constraint validation failed (RouteConfigurationValidationError.VirtualHosts[0]: embedded message failed validation | caused by VirtualHostValidationError.Routes[0]: embedded message failed validation | caused by RouteValidationError.Match: embedded message failed validation | caused by RouteMatchValidationError.PathSeparatedPrefix: value does not match regex pattern "^[^?#]+[^?#/]$"): name: "default-eg-http"
2023-02-15 13:29:15 virtual_hosts {
2023-02-15 13:29:15   name: "default-eg-http"
2023-02-15 13:29:15   domains: "*"
2023-02-15 13:29:15   routes {
2023-02-15 13:29:15     match {
2023-02-15 13:29:15       path_separated_prefix: "/backend/"
2023-02-15 13:29:15     }
2023-02-15 13:29:15     route {
2023-02-15 13:29:15       cluster: "default-backendxx-rule-0-match-0-*"
2023-02-15 13:29:15     }
2023-02-15 13:29:15   }
2023-02-15 13:29:15 }
2023-02-15 13:29:15

but the route is accepted

apiVersion: gateway.networking.k8s.io/v1beta1
kind: HTTPRoute
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"gateway.networking.k8s.io/v1beta1","kind":"HTTPRoute","metadata":{"annotations":{},"name":"backendxx","namespace":"default"},"spec":{"parentRefs":[{"name":"eg"}],"rules":[{"backendRefs":[{"group":"","kind":"Service","name":"backend","port":3000,"weight":1}],"matches":[{"path":{"type":"PathPrefix","value":"/backend/"}}]}]}}
  creationTimestamp: "2023-02-15T05:28:38Z"
  generation: 2
  name: backendxx
  namespace: default
  resourceVersion: "209201"
  uid: abffbeb8-e808-46ca-80ad-0dad38802d65
spec:
  parentRefs:
  - group: gateway.networking.k8s.io
    kind: Gateway
    name: eg
  rules:
  - backendRefs:
    - group: ""
      kind: Service
      name: backend
      port: 3000
      weight: 1
    matches:
    - path:
        type: PathPrefix
        value: /backend/
status:
  parents:
  - conditions:
    - lastTransitionTime: "2023-02-15T05:29:15Z"
      message: Route is accepted
      observedGeneration: 2
      reason: Accepted
      status: "True"
      type: Accepted
    controllerName: gateway.envoyproxy.io/gatewayclass-controller
    parentRef:
      group: gateway.networking.k8s.io
      kind: Gateway
      name: eg

I think that we should validate the PathSeparatedPrefix with an invalid path value and reject the route.

The text was updated successfully, but these errors were encountered:

arkodg · 2023-02-15T03:34:39Z

@qicz can you please attach a config that causes the issue ?

qicz · 2023-02-15T05:33:51Z

@qicz can you please attach a config that causes the issue ?

sorry, I am busy with others that moment. the config is appended.

qicz · 2023-02-15T13:10:44Z

other idea, when the path.value end with '/' use the RouteMatch_Prefix like follow:

if *pathMatch.Prefix == "/" {
	outMatch.PathSpecifier = &routev3.RouteMatch_Prefix{
		Prefix: "/",
	}
} else if strings.HasSuffix(*pathMatch.Prefix, "/") {
	outMatch.PathSpecifier = &routev3.RouteMatch_Prefix{
		Prefix: *pathMatch.Prefix,
	}
} else {
	outMatch.PathSpecifier = &routev3.RouteMatch_PathSeparatedPrefix{
		PathSeparatedPrefix: *pathMatch.Prefix,
	}
}

optimized

if strings.HasSuffix(*pathMatch.Prefix, "/") {
	outMatch.PathSpecifier = &routev3.RouteMatch_Prefix{
		Prefix: *pathMatch.Prefix,
	}
} else {
	outMatch.PathSpecifier = &routev3.RouteMatch_PathSeparatedPrefix{
		PathSeparatedPrefix: *pathMatch.Prefix,
	}
}

arkodg · 2023-02-15T17:11:04Z

awesome thanks for root causing the issue, the fix LGTM
@skriss is something similar done in contour ?

adding a help-wanted label, hoping someone from the community can incorporate the above patch and add test cases

qicz · 2023-02-16T05:39:36Z

another case @arkodg

type RouteMatch_PathSeparatedPrefix struct {
	// If specified, the route is a path-separated prefix rule meaning that the
	// ``:path`` header (without the query string) must either exactly match the
	// ``path_separated_prefix`` or have it as a prefix, followed by ``/``
	//
	// For example, ``/api/dev`` would match
	// ``/api/dev``, ``/api/dev/``, ``/api/dev/v1``, and ``/api/dev?param=true``
	// but would not match ``/api/developer``
	//
	// Expect the value to not contain ``?`` or ``#`` and not to end in ``/``
	PathSeparatedPrefix string `protobuf:"bytes,14,opt,name=path_separated_prefix,json=pathSeparatedPrefix,proto3,oneof"`
}

when the httproute config with urlrewrite like follows:

apiVersion: gateway.networking.k8s.io/v1beta1
kind: HTTPRoute
metadata:
  name: backend
spec:
  parentRefs:
    - name: eg
  rules:
    - backendRefs:
        - group: ""
          kind: Service
          name: backend
          port: 3000
          weight: 1
      filters:
        - type: URLRewrite
          urlRewrite:
            path:
              replacePrefixMatch: /
              type: ReplacePrefixMatch
      matches:
        - path:
            type: PathPrefix
            value: /om/test

the upstream recv double /

envoy doc

qicz · 2023-02-17T01:33:13Z

another case @arkodg

type RouteMatch_PathSeparatedPrefix struct {
	// If specified, the route is a path-separated prefix rule meaning that the
	// ``:path`` header (without the query string) must either exactly match the
	// ``path_separated_prefix`` or have it as a prefix, followed by ``/``
	//
	// For example, ``/api/dev`` would match
	// ``/api/dev``, ``/api/dev/``, ``/api/dev/v1``, and ``/api/dev?param=true``
	// but would not match ``/api/developer``
	//
	// Expect the value to not contain ``?`` or ``#`` and not to end in ``/``
	PathSeparatedPrefix string `protobuf:"bytes,14,opt,name=path_separated_prefix,json=pathSeparatedPrefix,proto3,oneof"`
}

when the httproute config with urlrewrite like follows:

apiVersion: gateway.networking.k8s.io/v1beta1
kind: HTTPRoute
metadata:
  name: backend
spec:
  parentRefs:
    - name: eg
  rules:
    - backendRefs:
        - group: ""
          kind: Service
          name: backend
          port: 3000
          weight: 1
      filters:
        - type: URLRewrite
          urlRewrite:
            path:
              replacePrefixMatch: /
              type: ReplacePrefixMatch
      matches:
        - path:
            type: PathPrefix
            value: /om/test

the upstream recv double /

envoy doc

I configured the envoy.yaml in envoy like following the case exists the same.

maybe this is a normal state? but some applications will occur errors

2023-02-15 08:44:22.966 ERROR 6 --- [nio-8080-exec-8] o.a.c.c.C.[.[.[/].[dispatcherServlet]    : Servlet.service() for servlet [dispatcherServlet] in context with path [] threw exception

org.springframework.security.web.firewall.RequestRejectedException: The request was rejected because the URL contained a potentially malicious String "//"
        at org.springframework.security.web.firewall.StrictHttpFirewall.rejectedBlocklistedUrls(StrictHttpFirewall.java:535)
        at org.springframework.security.web.firewall.StrictHttpFirewall.getFirewalledRequest(StrictHttpFirewall.java:505)
        at org.springframework.security.web.FilterChainProxy.doFilterInternal(FilterChainProxy.java:206)
        at org.springframework.security.web.FilterChainProxy.doFilter(FilterChainProxy.java:186)
        at org.springframework.web.filter.DelegatingFilterProxy.invokeDelegate(DelegatingFilterProxy.java:354)
        at org.springframework.web.filter.DelegatingFilterProxy.doFilter(DelegatingFilterProxy.java:267)
        at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:189)
        at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:162)
        at org.springframework.web.filter.RequestContextFilter.doFilterInternal(RequestContextFilter.java:100)
        at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:117)
        at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:189)
        at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:162)
        at org.springframework.web.filter.FormContentFilter.doFilterInternal(FormContentFilter.java:93)
        at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:117)
        at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:189)
        at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:162)
        at org.springframework.session.web.http.SessionRepositoryFilter.doFilterInternal(SessionRepositoryFilter.java:142)
        at org.springframework.session.web.http.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:82)
        at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:189)
        at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:162)
        at org.springframework.boot.actuate.metrics.web.servlet.WebMvcMetricsFilter.doFilterInternal(WebMvcMetricsFilter.java:96)
        at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:117)
        at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:189)
        at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:162)
        at org.springframework.web.filter.CharacterEncodingFilter.doFilterInternal(CharacterEncodingFilter.java:201)
        at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:117)
        at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:189)
        at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:162)
        at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:197)
        at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:97)
        at org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:541)
        at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:135)
        at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:92)
        at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:78)
        at org.apache.catalina.valves.RemoteIpValve.invoke(RemoteIpValve.java:769)
        at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:360)
        at org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:399)
        at org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:65)
        at org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:890)
        at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1789)
        at org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49)
        at org.apache.tomcat.util.threads.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1191)
        at org.apache.tomcat.util.threads.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:659)
        at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)

kflynn · 2023-03-09T18:02:22Z

Hey @qicz, this looks like excellent work! Is there something we did that stopped you from opening a PR? It feels like you literally did everything to just PR your fix, we'd love to be able to give you a shoutout for this... 🙂

qicz · 2023-03-12T15:15:27Z

fixed by #1125 ，close this issue

cc @kflynn

arkodg added good first issue Good for newcomers help wanted Extra attention is needed area/translator Issues related to Gateway's translation service, e.g. translating Gateway APIs into the IR. labels Feb 15, 2023

arkodg added this to the 0.4.0-rc.1 milestone Feb 15, 2023

arkodg added the kind/bug Something isn't working label Feb 15, 2023

This was referenced Mar 10, 2023

fix PathSeparatedPrefix & optimize the prefix ends with / logic. #1125

Merged

the upstream receive double / envoyproxy/envoy#26024

Closed

qicz closed this as completed Mar 12, 2023

qicz mentioned this issue Aug 2, 2024

upstream get unwanted / #3989

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

validate the path with PathSeparatedPrefix? #1044

validate the path with PathSeparatedPrefix? #1044

qicz commented Feb 15, 2023 •

edited

Loading

arkodg commented Feb 15, 2023

qicz commented Feb 15, 2023 •

edited

Loading

qicz commented Feb 15, 2023 •

edited

Loading

arkodg commented Feb 15, 2023

qicz commented Feb 16, 2023 •

edited

Loading

qicz commented Feb 17, 2023 •

edited

Loading

kflynn commented Mar 9, 2023

qicz commented Mar 12, 2023 •

edited

Loading

validate the path with PathSeparatedPrefix? #1044

validate the path with PathSeparatedPrefix? #1044

Comments

qicz commented Feb 15, 2023 • edited Loading

arkodg commented Feb 15, 2023

qicz commented Feb 15, 2023 • edited Loading

qicz commented Feb 15, 2023 • edited Loading

arkodg commented Feb 15, 2023

qicz commented Feb 16, 2023 • edited Loading

qicz commented Feb 17, 2023 • edited Loading

kflynn commented Mar 9, 2023

qicz commented Mar 12, 2023 • edited Loading

qicz commented Feb 15, 2023 •

edited

Loading

qicz commented Feb 15, 2023 •

edited

Loading

qicz commented Feb 15, 2023 •

edited

Loading

qicz commented Feb 16, 2023 •

edited

Loading

qicz commented Feb 17, 2023 •

edited

Loading

qicz commented Mar 12, 2023 •

edited

Loading