fix(supervisor-network): check the canonical target for encoded slashes

Signed-off-by: Adrien Langou <alangou@nvidia.com>

Author: alangou

crates/openshell-supervisor-network/src/l7/relay.rs

@@ -569,14 +569,20 @@ where
         // on this host:port. Re-check it against the config that actually
         // matched: the opt-in is per-endpoint, and one endpoint enabling it
         // must not loosen parsing for the others.
+        // Check `req.target`, not `route_target`: redaction percent-decodes any
+        // segment holding a credential placeholder and re-inserts the redacted
+        // form without re-encoding, so a `%2F` sharing that segment becomes a
+        // literal `/` and would escape this check.
         if !config.allow_encoded_slash
-            && crate::l7::path::canonical_path_has_encoded_slash(&route_target)
+            && crate::l7::path::canonical_path_has_encoded_slash(&req.target)
         {
+            let detail = "request-target contains an encoded '/' (%2F) which is not allowed on this endpoint";
+            emit_parse_rejection(ctx, detail, engine_type_for_protocol(config.protocol));
             crate::l7::rest::RestProvider::default()
                 .deny_with_redacted_target(
                     &req,
                     &ctx.policy_name,
-                    "request-target contains an encoded '/' (%2F) which is not allowed on this endpoint",
+                    detail,
                     client,
                     None,
                     Some(crate::l7::rest::DenyResponseContext::from_l7_context(ctx)),
@@ -6184,7 +6190,11 @@ network_policies:
             .clone_engine_for_tunnel(engine.current_generation())
             .unwrap();
         let configs = encoded_slash_scoping_configs();
-        let ctx = encoded_slash_scoping_ctx();
+        let (activity_tx, mut activity_rx) = tokio::sync::mpsc::channel(1);
+        let ctx = L7EvalContext {
+            activity_tx: Some(activity_tx),
+            ..encoded_slash_scoping_ctx()
+        };
 
         let (mut app, mut relay_client) = tokio::io::duplex(8192);
         let (mut relay_upstream, mut upstream) = tokio::io::duplex(8192);
@@ -6217,6 +6227,85 @@ network_policies:
             "denial must name the encoded-slash reason: {response}"
         );
 
+        let mut upstream_bytes = [0u8; 16];
+        let result = tokio::time::timeout(
+            std::time::Duration::from_millis(100),
+            upstream.read(&mut upstream_bytes),
+        )
+        .await;
+        assert!(
+            matches!(result, Err(_) | Ok(Ok(0))),
+            "request must not reach upstream"
+        );
+        let activity = tokio::time::timeout(std::time::Duration::from_secs(1), activity_rx.recv())
+            .await
+            .expect("parse rejection activity should be emitted")
+            .expect("activity channel should remain open");
+        assert!(activity.denied);
+        assert_eq!(activity.deny_group, "l7_parse_rejection");
+
+        drop(app);
+        tokio::time::timeout(std::time::Duration::from_secs(1), relay)
+            .await
+            .expect("relay should finish")
+            .unwrap()
+            .unwrap();
+    }
+
+    /// Credential redaction percent-decodes any segment holding a placeholder
+    /// and re-inserts the redacted form without re-encoding it. A `%2F` sharing
+    /// that segment therefore becomes a literal `/` in the redacted target, so
+    /// the scoping check must read the canonical target rather than the
+    /// redacted one — otherwise a placeholder is enough to smuggle an encoded
+    /// slash past an endpoint that never opted in.
+    #[tokio::test]
+    async fn route_selected_encoded_slash_check_survives_credential_redaction() {
+        let engine = OpaEngine::from_strings(TEST_POLICY, ENCODED_SLASH_SCOPING_POLICY).unwrap();
+        let tunnel_engine = engine
+            .clone_engine_for_tunnel(engine.current_generation())
+            .unwrap();
+        let configs = encoded_slash_scoping_configs();
+        let (child_env, resolver) = SecretResolver::from_provider_env(
+            std::iter::once(("TOKEN".to_string(), "real-token".to_string())).collect(),
+        );
+        let placeholder = child_env.get("TOKEN").expect("placeholder env").clone();
+        let ctx = L7EvalContext {
+            secret_resolver: resolver.map(Arc::new),
+            ..encoded_slash_scoping_ctx()
+        };
+
+        let (mut app, mut relay_client) = tokio::io::duplex(8192);
+        let (mut relay_upstream, mut upstream) = tokio::io::duplex(8192);
+        let relay = tokio::spawn(async move {
+            relay_with_route_selection(
+                &configs,
+                tunnel_engine,
+                &mut relay_client,
+                &mut relay_upstream,
+                &ctx,
+            )
+            .await
+        });
+
+        // Placeholder and encoded slash in the same segment, on the endpoint
+        // that did NOT opt into encoded slashes.
+        let request = format!(
+            "GET /admin/{placeholder}%2Fx HTTP/1.1\r\nHost: gateway.example.test\r\nConnection: close\r\n\r\n"
+        );
+        app.write_all(request.as_bytes()).await.unwrap();
+
+        let mut response = [0u8; 1024];
+        let n = tokio::time::timeout(std::time::Duration::from_secs(1), app.read(&mut response))
+            .await
+            .expect("denial should reach client")
+            .unwrap();
+        let response = String::from_utf8_lossy(&response[..n]);
+        assert!(response.contains("403 Forbidden"), "{response}");
+        assert!(
+            response.contains("not allowed on this endpoint"),
+            "redaction must not hide the encoded slash: {response}"
+        );
+
         let mut upstream_bytes = [0u8; 16];
         let result = tokio::time::timeout(
             std::time::Duration::from_millis(100),

crates/openshell-supervisor-network/src/proxy.rs

@@ -59,6 +59,8 @@ const TUNNEL_PROTOCOL_PEEK_POLL: std::time::Duration = std::time::Duration::from
 const TUNNEL_PROTOCOL_PEEK_POLL: std::time::Duration = std::time::Duration::from_millis(1);
 const INFERENCE_LOCAL_HOST: &str = "inference.local";
 const INFERENCE_LOCAL_PORT: u16 = 443;
+const FORWARD_ENCODED_SLASH_REJECTION_DETAIL: &str =
+    "request-target contains an encoded '/' (%2F) which is not allowed on this endpoint";
 #[cfg(target_os = "linux")]
 const SIDECAR_SUPERVISOR_TOPOLOGY: &str = "sidecar";
 
@@ -908,6 +910,41 @@ fn build_forward_parse_error_ocsf_event(path: &str) -> openshell_ocsf::OcsfEvent
         .build()
 }
 
+#[allow(clippy::too_many_arguments)]
+fn build_forward_l7_parse_rejection_ocsf_event(
+    peer_addr: SocketAddr,
+    method: &str,
+    host: &str,
+    port: u16,
+    path: &str,
+    binary: &str,
+    pid: &str,
+    ancestors: &str,
+    cmdline: &str,
+    policy: &str,
+    detail: &str,
+) -> openshell_ocsf::OcsfEvent {
+    HttpActivityBuilder::new(openshell_ocsf::ctx::ctx())
+        .activity(ActivityId::Other)
+        .action(ActionId::Denied)
+        .disposition(DispositionId::Blocked)
+        .severity(SeverityId::Medium)
+        .status(StatusId::Failure)
+        .http_request(HttpRequest::new(
+            method,
+            OcsfUrl::new("http", host, path, port),
+        ))
+        .dst_endpoint(Endpoint::from_domain(host, port))
+        .src_endpoint(Endpoint::from_ip(peer_addr.ip(), peer_addr.port()))
+        .actor_process(Process::from_bypass(binary, pid, ancestors).with_cmd_line(cmdline))
+        .firewall_rule(policy, "l7")
+        .message(format!(
+            "FORWARD_L7 denied non-canonical request-target for {method} {host}:{port}{path}"
+        ))
+        .status_detail(detail)
+        .build()
+}
+
 #[allow(clippy::too_many_arguments)]
 fn build_forward_policy_deny_ocsf_event(
     peer_addr: SocketAddr,
@@ -4458,16 +4495,19 @@ async fn handle_forward_proxy(
         if !l7_config.config.allow_encoded_slash
             && crate::l7::path::canonical_path_has_encoded_slash(&path)
         {
-            let event = NetworkActivityBuilder::new(openshell_ocsf::ctx::ctx())
-                .activity(ActivityId::Fail)
-                .severity(SeverityId::Medium)
-                .status(StatusId::Failure)
-                .dst_endpoint(Endpoint::from_domain(&host_lc, port))
-                .message(
-                    "FORWARD rejecting non-canonical request-target: request-target contains an encoded '/' (%2F) which is not allowed on this endpoint".to_string(),
-                )
-                .build();
-            ocsf_emit!(event);
+            ocsf_emit!(build_forward_l7_parse_rejection_ocsf_event(
+                workload_addr,
+                method,
+                &host_lc,
+                port,
+                &telemetry_path,
+                &binary_str,
+                &pid_str,
+                &ancestors_str,
+                &cmdline_str,
+                policy_str,
+                FORWARD_ENCODED_SLASH_REJECTION_DETAIL,
+            ));
             emit_activity_simple(activity_tx, true, "forward_parse_rejection");
             respond(
                 client,
@@ -5645,6 +5685,42 @@ network_policies: {}
         assert_eq!(json["disposition"], "Blocked");
     }
 
+    #[test]
+    fn forward_l7_parse_rejection_ocsf_includes_denial_context() {
+        let event = build_forward_l7_parse_rejection_ocsf_event(
+            "127.0.0.1:45123".parse().unwrap(),
+            "GET",
+            "api.example.com",
+            80,
+            "/admin/x%2Fy",
+            "/usr/bin/curl",
+            "42",
+            "/usr/bin/bash",
+            "curl http://api.example.com/admin/x%2Fy",
+            "allow_api",
+            FORWARD_ENCODED_SLASH_REJECTION_DETAIL,
+        );
+        let json = event.to_json().unwrap();
+
+        assert_eq!(json["class_name"], "HTTP Activity");
+        assert_eq!(json["activity_name"], "Other");
+        assert_eq!(json["action"], "Denied");
+        assert_eq!(json["disposition"], "Blocked");
+        assert_eq!(json["severity"], "Medium");
+        assert_eq!(json["status"], "Failure");
+        assert_eq!(json["http_request"]["http_method"], "GET");
+        assert_eq!(json["http_request"]["url"]["path"], "/admin/x%2Fy");
+        assert_eq!(json["dst_endpoint"]["domain"], "api.example.com");
+        assert_eq!(json["dst_endpoint"]["port"], 80);
+        assert_eq!(json["actor"]["process"]["name"], "/usr/bin/curl");
+        assert_eq!(json["firewall_rule"]["name"], "allow_api");
+        assert_eq!(json["firewall_rule"]["type"], "l7");
+        assert_eq!(
+            json["status_detail"],
+            FORWARD_ENCODED_SLASH_REJECTION_DETAIL
+        );
+    }
+
     #[test]
     fn forward_ocsf_events_omit_queries_and_credential_key_names() {
         let peer = "127.0.0.1:45123".parse().unwrap();