Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deny-all SecurityPolicy can return 403 or 404, depending on HTTPRoutes #4678

Open
dghubble opened this issue Nov 8, 2024 · 5 comments · May be fixed by #4900
Open

Deny-all SecurityPolicy can return 403 or 404, depending on HTTPRoutes #4678

dghubble opened this issue Nov 8, 2024 · 5 comments · May be fixed by #4900
Assignees
Labels
area/api API-related issues stale
Milestone

Comments

@dghubble
Copy link

dghubble commented Nov 8, 2024

Description:

What issue is being seen? Describe what should be happening instead of
the bug, for example: Envoy should not crash, the expected value isn't
returned, etc.

Using a SecurityPolicy to allow-list source IPs, envoy only returns a 403/forbidden when a HTTP/HTTPS request matches an HTTPRoute. If a request does not match an HTTPRoute, envoy returns a 404.

apiVersion: gateway.envoyproxy.io/v1alpha1
kind: SecurityPolicy
metadata:
  name: allowlist-source-ips
spec:
  targetRef:
    group: gateway.networking.k8s.io
    kind: Gateway
    name: gateway
  authorization:
    defaultAction: Deny
    rules:
      - action: Allow
        principal:
          clientCIDRs:
            - fakeIP/32

Repro steps:

Include sample requests, environment, etc. All data and inputs
required to reproduce the bug.

Query a hostname or the envoy proxy directly, from an IP that should be forbidden. Say a Gateway has some IP EnvoyIP. And the SecurityPolicy should deny our client IP always (e.g. make the rules list empty for repro).

curl --resolve matching.hostname:443:EnvoyIP https://matching.hostname
RBAC: access denied

Good, that above is expected. However, if you make a query that does not match a route, you'll get a 404 instead.

curl EnvoyIP
curl --resolve nonmatch.hostname:443:EnvoyIP https://nonmatch.hostname
HTTP/1.1 404 Not Found
date: Fri, 08 Nov 2024 03:29:16 GMT
transfer-encoding: chunked

Our expectation was that all traffic from a disallowed IP would receive a 403. Its undesired, because external parties have visibility into what hostnames/domains are served by a particular Gateway.

Basically this is an ordering question: Which comes first the 404 logic or the rbac forbidden?

Note: If there are privacy concerns, sanitize the data prior to
sharing.

Environment:

Include the environment like gateway version, envoy version and so on.

v1.2.1

Logs:

Include the access logs and the Envoy logs.

@dghubble dghubble added the triage label Nov 8, 2024
@arkodg
Copy link
Contributor

arkodg commented Nov 8, 2024

hey @dghubble the responseOverride feature can help standardize the responses

docs are WIP #4668, should be in soon

@arkodg
Copy link
Contributor

arkodg commented Nov 8, 2024

just noticed that responseOverride doesnt support modifying the statusCode in its current form

type CustomResponse struct {
, should be easy to enhance

as a workaround until statusCode is implemented in responseOverride, what you can do is add a pathPrefix match of / that returns a 403 using directResponse https://gateway.envoyproxy.io/docs/tasks/traffic/direct-response/#testing-direct-response. The pathPrefix of / will have the lowest match precedence, so anything that doesnt match your defined rules, will match this catch-all

@arkodg
Copy link
Contributor

arkodg commented Nov 8, 2024

Basically this is an ordering question: Which comes first the 404 logic or the rbac forbidden?

Envoy Gateway does support filter ordering https://gateway.envoyproxy.io/docs/tasks/operations/customize-envoyproxy/#customize-filter-order but the router filter must be the terminal/last filter (mandated by envoy proxy) so that won't be helpful here

@arkodg arkodg added this to the v1.3.0-rc.1 milestone Nov 22, 2024
@arkodg
Copy link
Contributor

arkodg commented Nov 22, 2024

keeping this issue open to implement updating statusCode within responseOverride

@arkodg arkodg self-assigned this Nov 22, 2024
@arkodg arkodg added area/api API-related issues and removed triage labels Nov 22, 2024
arkodg added a commit to arkodg/gateway that referenced this issue Dec 12, 2024
arkodg added a commit to arkodg/gateway that referenced this issue Dec 12, 2024
Copy link

This issue has been automatically marked as stale because it has not had activity in the last 30 days.

@github-actions github-actions bot added the stale label Dec 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api API-related issues stale
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants