mirror of
https://github.com/NVIDIA/OpenShell.git
synced 2026-10-16 05:20:22 +08:00
John T. Myers
c960d480fd
* fix(sandbox): canonicalize HTTP request-targets before L7 policy evaluation
The L7 REST proxy evaluated OPA path rules against the raw request-target
and forwarded the raw header block byte-for-byte to the upstream. The
upstream normalized `..`, `%2e%2e`, and `//` before dispatching, so a
compromised agent inside the sandbox could bypass any path-based allow
rule by prefixing the blocked path with an allowed one (e.g.
`GET /public/../secret` matches `/public/**` at the proxy but the
upstream serves `/secret`). The inference-routing detection had the same
normalization gap via `normalize_inference_path`, which only stripped
scheme+authority and preserved dot-segments verbatim.
- Add `crates/openshell-sandbox/src/l7/path.rs` with
`canonicalize_request_target`. Percent-decodes (with uppercase hex
re-emission), resolves `.` / `..` segments per RFC 3986 5.2.4,
collapses doubled slashes, strips trailing `;params`, rejects
fragments / raw non-ASCII / control bytes / encoded slashes (unless
explicitly enabled per-endpoint), and returns a `rewritten` flag so
callers know when to rewrite the outbound request line.
- Wire the canonicalizer into `rest.rs::parse_http_request`. The
canonical path is the `target` on `L7Request` that OPA sees, and
when the canonical form differs from the raw input the request line
is rebuilt in `raw_header` so the upstream dispatches on the exact
bytes the policy evaluated.
- Canonicalize the forward (plain HTTP proxy) path at
`proxy.rs`'s second L7 evaluation site. A non-canonical request-target
is rejected with 400 Bad Request and an OCSF `NetworkActivity` Fail
event rather than silently passed through.
- Replace the old `normalize_inference_path` body with a call to the
canonicalizer. On canonicalization error the raw path is returned
(so inference detection simply misses and the normal forward path
handles and rejects).
- Document the invariant in `sandbox-policy.rego`: `input.request.path`
is pre-canonicalized so rules must not attempt defensive matching
against `..` or `%2e%2e`.
- Architecture doc (`architecture/sandbox.md`) lists the new module.
- 24 new unit tests cover dot segments, percent-encoded dot segments,
double-slash collapse, encoded-slash reject/opt-in, null/control byte
rejection, legitimate percent-encoded bytes round-tripping, mixed-case
percent normalization, fragment rejection, absolute-form stripping,
empty / length-bounded / non-ASCII handling, and regression payloads
for the specific bypasses above.
Tracks OS-99.
* fix(sandbox): close forward-proxy parser-differential and wire allow_encoded_slash
Addresses two review findings on the L7 path canonicalization change.
1. `handle_forward_proxy` previously evaluated OPA on the canonical path
but wrote the raw path to the upstream via `rewrite_forward_request`,
leaving the parser-differential open for plain-HTTP forward-proxy
traffic even though it was closed for the L7 tunnel path. `path` is
now reassigned to the canonical form inside the L7 block before the
outbound write, so the bytes dispatched to the upstream match what
OPA evaluated. Covered by a new regression test against
`rewrite_forward_request`.
2. `allow_encoded_slash` is now wired through the YAML policy schema,
the proto `NetworkEndpoint` message, the Rego endpoint config, and
the `L7EndpointConfig` → `RestProvider` canonicalization options.
Operators can opt in per-endpoint for upstreams like GitLab that
embed `%2F` in namespaced resource paths; the default remains
strict. The `RestProvider` gains a constructor
`RestProvider::with_options` so `relay_rest` constructs a provider
with per-endpoint options while `relay_passthrough_with_credentials`
retains strict defaults.
Integration tests:
- `parse_http_request_canonicalizes_target_and_rewrites_raw_header`
drives a non-canonical request through the parser and asserts both
`L7Request.target` (what OPA evaluates) and `raw_header` (what the
upstream receives) are canonical.
- `parse_http_request_canonicalization_preserves_query_string`,
`parse_http_request_leaves_canonical_input_byte_for_byte`,
`parse_http_request_rejects_traversal_above_root`,
`parse_http_request_preserves_http_10_version_on_rewrite`.
- `parse_http_request_accepts_encoded_slash_when_endpoint_opts_in` /
`_rejects_encoded_slash_by_default` verify the
`allow_encoded_slash` gate.
- `test_rewrite_forward_request_uses_canonical_path_on_the_wire`
asserts the fix to the forward-proxy flow.
- `parse_l7_config_allow_encoded_slash_{defaults_false,opt_in}` verify
the YAML/Rego wiring.