Repository navigation
fix(policy,network): separate named and hostless destination classes — mixed scopes validate and match correctly - #5198
Conversation
…selectors A hostless allowed_ips endpoint and a named endpoint authorize different destination classes: the hostless endpoint matches raw-IP connections in its ranges, the named endpoint matches requests to its hostname. The two never compete for the same request's connection or request-pipeline config, so find_endpoint_ambiguities now skips pairs where exactly one side is hostless. This mirrors the existing plain_l4_endpoint_does_not_compete_with_l7_endpoint_metadata guarantee — an IP-scoped selector could never qualify as plain L4 because its destination selector is the allowed_ips list itself. Hostless-vs-hostless pairs still compete (both match the same raw-IP connection class), so differing allowed_ips between two hostless endpoints remains ambiguous. This unblocks the common red-team scope shape of named targets plus CIDR ranges in one policy, which the current check rejects even though the runtime match sets are disjoint. Signed-off-by: PurpleCHOIms <31537001+PurpleCHOIms@users.noreply.github.com>
…ions A hostless allowed_ips endpoint is a destination selector: it authorizes connections addressed by IP literal in its ranges. Under the previous matcher it also admitted every DNS-mediated hostname on its ports and constrained their pinned resolutions to its ranges, which made a policy mixing named endpoints with CIDR scopes deny every named host that resolves outside the range — the mixed-scope shape the ambiguity validator change (previous commit) was meant to unlock. The matcher now requires the destination to be an IP literal (dotted quad or anything carrying a colon); DNS-mediated names are governed by named endpoints, consistent with the named-vs-hostless class separation. The in-range check still happens in Rust after the pin. Tests updated: raw-IP admission, named rejection, multi-port on raw-IP. Signed-off-by: PurpleCHOIms <31537001+PurpleCHOIms@users.noreply.github.com>
…data matching endpoint_matches_request carried its own hostless matcher (port-only), so the metadata/allowed_ips extraction path still attached a CIDR endpoint's ranges to every DNS-mediated hostname sharing its ports. The policy-DNS pin validation then intersected the named endpoint's resolutions with the CIDR and produced an empty allowlist — mixed named+CIDR policies denied every named host resolving outside the range (live: example.com transparent_tcp_destination_denied). The hostless branch now requires a raw-IP destination, mirroring the admission matcher and the validator class separation. Signed-off-by: PurpleCHOIms <31537001+PurpleCHOIms@users.noreply.github.com>
|
Thank you for your submission! We ask that you sign our Developer Certificate of Origin before we can accept your contribution. You can sign the DCO by adding a comment below using this text: I have read the DCO document and I hereby sign the DCO. You can retrigger this bot by commenting recheck in this Pull Request. Posted by the DCO Assistant Lite bot. |
|
Thank you for your interest in contributing to OpenShell, @PurpleCHOIms. This project uses a vouch system for first-time contributors. Before submitting a pull request, you need to be vouched by a maintainer. To get vouched:
See CONTRIBUTING.md for details. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 82b6260a1c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if left.endpoint.host.is_empty() != right.endpoint.host.is_empty() { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
For non-TCP proxy endpoints, a nonempty host can still be an IP literal, so an endpoint such as host: 10.10.0.5 is not disjoint from a hostless endpoint on the same port. Both the exact-host and hostless branches of endpoint_matches_request match that raw-IP request, but this new shortcut permits them to disagree on tls, allowed_ips, and L7 metadata. The proxy's query_tls_mode and query_allowed_ips still take the first matching config, so a previously rejected ambiguous policy can now activate and apply the wrong TLS mode or deny an explicitly allowed IP using the other endpoint's CIDR. Preserve the overlap checks for host selectors that can match IP literals.
Useful? React with 👍 / 👎.
| object.get(endpoint, "host", "") == "" | ||
| count(object.get(endpoint, "allowed_ips", [])) > 0 | ||
| endpoint.ports[_] == network.port | ||
| raw_ip_destination(network.host) |
There was a problem hiding this comment.
For DNS-named private services, the installable policy-generation skill still recommends an allowed_ips-only endpoint because it permits “any domain on the port” (skills/generate-sandbox-policy/SKILL.md:359), and skills/generate-sandbox-policy/examples.md:646-668 generates exactly that policy. This new guard makes those policies deny hostname requests even when their resolved addresses fall within the CIDR, so following the shipped skill now produces non-working access policies. Update the skill and example, and document the raw-IP-only limitation in the published policy documentation, alongside this behavior change; the AGENTS.md maintenance guidance requires reviewing the affected skills.
AGENTS.md reference: AGENTS.md:L225-L225
Useful? React with 👍 / 👎.
Summary
Mixed named + hostless (CIDR) endpoint scopes — the common real-world shape for RoE-style policies (named targets plus IP/CIDR ranges) — were rejected by the ambiguity validator and, once validated on a relaxed build, matched incorrectly by the runtime: named traffic inherited the hostless endpoint's
allowed_ipsinterpretation and was kernel-denied. This change separates the two destination classes end to end: validator, admission matcher, and metadata matching.Related Issue
Closes #5193
Changes
crates/openshell-policy(validator) —find_endpoint_ambiguitiesno longer treats a named and a hostless endpoint as competing when they share ports: they authorize disjoint destination classes (DNS-mediated named requests vs raw-IP connections), mirroring the existing plain-L4 vs L7 guarantee. Hostless-vs-hostless competition is preserved — differingallowed_ipsbetween two hostless endpoints on shared ports is still rejected.crates/openshell-supervisor-network(approval matcher) — the hostless matcher insandbox-policy.regonow requires a raw-IP destination. Named connections resolve via policy DNS and match named endpoints only; the hostless endpoint owns the raw-IP class exclusively.crates/openshell-supervisor-network(metadata matching) —endpoint_matches_request's hostless branch (metadata/allowed_ipsextraction) gets the same raw-IP-only scoping. This was the live-visible failure path: the port-only hostless matcher attached a CIDR endpoint's ranges to every DNS-mediated hostname sharing its ports, and pin validation intersected the named endpoint's resolutions with the CIDR, producing an empty allowlist. IP-shaped destinations are detected without regex (octet/colon shape checks) so the Rego engine stays compatible.Release Notes
Policies mixing named targets with hostless
allowed_ips(IP/CIDR) endpoints on shared ports now validate and enforce correctly in a single policy: named connections follow their named endpoints, raw-IP connections follow the hostless endpoints.Testing
cargo test -p openshell-policy— 298 passed, 0 failed (295 existing unchanged + 3 new: named+hostless pairs are not ambiguous; named metadata conflicts do not spill into hostless pairs; hostless-vs-hostless differingallowed_ipsstill rejected).cargo test -p openshell-supervisor-network --lib— 1,580 passed, 0 failed (3 former-semantics fixtures updated to the new matcher semantics).example.com+10.10.0.0/24+ OSINT named target) creates born-confined withPhase: Ready; in-scopehttps://example.com→ HTTP 200 (the case that failed before the matcher fix); OSINT named target → HTTP 200; raw-IP inside the range (10.10.0.5) → forwarded (connect timeout against a dead target, not kernel-denied); raw-IP outside the range (10.10.1.5), out-of-scope hostname, and IMDS (169.254.169.254) → kernel DENIED with OCSF audit events.Checklist