fix(supervisor-network): canonicalize dot-segments before policy evaluation - #2699
Open
alangou wants to merge 3 commits into
Open
fix(supervisor-network): canonicalize dot-segments before policy evaluation#2699alangou wants to merge 3 commits into
alangou wants to merge 3 commits into
Conversation
…olution Signed-off-by: Adrien Langou <alangou@nvidia.com>
…endpoint Signed-off-by: Adrien Langou <alangou@nvidia.com>
alangou
requested review from
a team,
derekwaynecarr,
maxamillion and
mrunalp
as code owners
August 11, 2026 12:07
|
Label |
johntmyers
reviewed
Aug 11, 2026
alangou
enabled auto-merge
August 13, 2026 13:53
johntmyers
reviewed
Aug 13, 2026
| if !config.allow_encoded_slash | ||
| && crate::l7::path::canonical_path_has_encoded_slash(&req.target) | ||
| { | ||
| crate::l7::rest::RestProvider::default() |
Collaborator
There was a problem hiding this comment.
Are we emitting a OCSF denial event before the 403 rejection? Should be able to use emit_parse_rejection before sending the denial.
Signed-off-by: Adrien Langou <alangou@nvidia.com>
alangou
force-pushed
the
fix/l7-path-canonicalization
branch
from
August 14, 2026 12:07
68c1e01 to
fa5c600
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The L7 request-target canonicalizer could return a path that still contained a
./..segment. Since that canonical path is simultaneously the OPA policy input and the bytes forwarded upstream, a residual dot-segment let a request escape the path prefix its policy allowed. This PR fixes the ordering that produced it, adds a defense-in-depth guard, and scopes theallow_encoded_slashopt-in to the endpoint that actually matched.Related Issue
No issue required: security fix. Per SECURITY.md
Changes
fix(supervisor-network): strip path parameters before dot-segment resolutioncanonicalize_request_targetstripped;paramsafter resolving dot-segments. A dot segment carrying a path parameter was therefore not byte-equal to..when the traversal guard inspected it, and reverted to a bare..during reconstruction. Stripping now runs first../..surviving canonicalization is rejected with a newCanonicalizeError::ResidualDotSegment. This also covers a dot-segment held inside a segment by the%2Fsentinel on endpoints that opted intoallow_encoded_slash.strip_path_parametersand reused it inbuild_canonical_path.fix(supervisor-network): scope allow_encoded_slash to the matched L7 endpointallow_encoded_slashwith.any()across every config sharing ahost:port, because canonicalization runs before the matching config is known. One endpoint opting in loosened path parsing for every other endpoint on that host — contrary to the per-endpoint opt-in that the passthrough path documents.l7/relay.rsandproxy.rs). A surviving%2Fis rejected when that config did not opt in. The rejection mirrors the parser's own response, so a correctly-scoped request is indistinguishable from one rejected at parse time.canonical_path_has_encoded_slashhelper. The check is exact:build_canonical_pathemits a literal%2Fonly for the sentinel and percent-encodes any other%as%25.Testing
mise run cigreen (lint + compile + tests). 31 tests inl7::path, all 25 pre-existing ones unchanged and passing.Unit tests for the ordering fix:
;on the dot segment (..;,..;x,..;jsessionid=…,.;), percent-encoded forms (%2e%2e;,..%3B,..%3b), chained traversal hitting the root guard, and..behind a%2Fsentinel under the opt-in.An invariant test asserting no canonical output ever contains a dot-segment.
An exactness test for the encoded-slash helper, including
%252F, which decodes to the three characters%,2,Fand must not be mistaken for a sentinel.Relay integration tests driving the real
relay_with_route_selectionwith two endpoints on onehost:port— one opted intoallow_encoded_slash, one not — asserting the non-opted-in endpoint rejects%2Fwithout reaching upstream, and that the opted-in endpoint still forwards an encoded slug verbatim.mise run pre-commitpassesUnit tests added/updated
E2E tests added/updated (if applicable)
Checklist
Notes for reviewers
..;-style targets, and%2Fon an endpoint that did not itself opt in. Both were only reachable through non-canonical input, but operators relying on the accidental cross-endpoint%2Fleniency will see 4xx where they previously saw traffic pass.%2Fin an early segment can still change which config matches (/admin%2Fxdoes not match a/admin/**subtree). That fails closed — no match means denial — and fixing it means revisiting route matching itself.proxy::tests::forward_handler_preserves_ssrf_response_and_denial_stagefailed once during this work and passed on re-run. It is unreachable from these changes (the SSRF denial returns well before L7 config selection), but it looks intermittent and probably deserves its own issue.