Repository navigation
feat(minimald-rpc,minimald,minimal2): policy types, GetSessionPolicy RPC, and session policy CLI #521
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
feat(minimald-rpc,minimald,minimal2): policy types, GetSessionPolicy RPC, and session policy CLI #521
Changes from all commits
21ab668
0826c6b
33e5144
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -276,3 +276,163 @@ impl OneshotSshRpc for SessionAbort { | |
| type Request<'a> = sessions::wire::request::Abort; | ||
| type Response = Errorable<()>; | ||
| } | ||
|
|
||
| // --------------------------------------------------------------------------- | ||
| // Networking policy types (Unit 2: egress, ingress, dynamic port mapping). | ||
| // --------------------------------------------------------------------------- | ||
|
|
||
| /// A single static ingress port mapping for an `OwnIp` PTask. | ||
| #[non_exhaustive] | ||
| #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] | ||
| pub struct PortMapping { | ||
| /// Host-side port that forwards inbound connections into the PTask. | ||
| pub external_port: u16, | ||
| /// PTask-side port that receives forwarded connections. | ||
| pub internal_port: u16, | ||
| /// Transport protocol for this mapping. | ||
| pub proto: IpProto, | ||
| } | ||
|
|
||
| /// Effective egress policy for an `OwnIp` PTask. | ||
| /// | ||
| /// Each field is `None` to mean allow-all for that dimension. Absent `egress` | ||
| /// config on a session is equivalent to all-`None` (allow-all). | ||
| #[non_exhaustive] | ||
| #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Default)] | ||
| pub struct EgressPolicy { | ||
| /// Allowed destination CIDR prefixes; `None` means allow-all subnets. | ||
| pub allow_subnets: Option<Vec<String>>, | ||
| /// Allowed destination DNS hostnames; `None` means allow-all hosts. | ||
| pub allow_dns_hosts: Option<Vec<String>>, | ||
| /// Allowed IP protocols; `None` means allow all protocols. | ||
| pub allow_protocols: Option<Vec<IpProto>>, | ||
| } | ||
|
|
||
| /// Effective ingress policy for an `OwnIp` PTask. | ||
| /// | ||
| /// Default (empty `port_mappings`, no `dynamic_allowed_range`) is deny-all-external. | ||
| #[non_exhaustive] | ||
| #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Default)] | ||
| pub struct IngressPolicy { | ||
| /// Static port mappings applied at PTask launch. | ||
| pub port_mappings: Vec<PortMapping>, | ||
| /// Inclusive port range within which dynamic port-mapping requests are | ||
| /// accepted; `None` means dynamic mapping is disallowed. | ||
| pub dynamic_allowed_range: Option<(u16, u16)>, | ||
| } | ||
|
|
||
| /// The effective networking policy for a named session, as returned by | ||
| /// [`GetSessionPolicy`]. | ||
| #[non_exhaustive] | ||
| #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] | ||
| pub struct SessionPolicy { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. LOW · correctness
#[non_exhaustive]
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
pub struct SessionPolicy { |
||
| /// Egress policy; `None` when the session is not `OwnIp` or no explicit | ||
| /// egress config is present (allow-all). | ||
| pub egress: Option<EgressPolicy>, | ||
| /// Ingress policy; `None` when the session is not `OwnIp`. | ||
| pub ingress: Option<IngressPolicy>, | ||
| } | ||
|
|
||
| impl SessionPolicy { | ||
| pub fn new(egress: Option<EgressPolicy>, ingress: Option<IngressPolicy>) -> Self { | ||
| Self { egress, ingress } | ||
| } | ||
| } | ||
|
|
||
| /// An RPC to read the effective networking policy for a session (R2.6). | ||
| pub struct GetSessionPolicy; | ||
|
|
||
| /// Request for the [`GetSessionPolicy`] RPC. | ||
| #[derive(Debug, Clone, Serialize, Deserialize)] | ||
| #[serde(rename_all = "snake_case")] | ||
| pub enum GetSessionPolicyRequest { | ||
| Name(String), | ||
| Id(SessionId), | ||
| } | ||
|
|
||
| impl OneshotSshRpc for GetSessionPolicy { | ||
| const NAME: &'static str = constcat::concat!(RPC_SUBSYSTEM_PREFIX, "GetSessionPolicy"); | ||
| type Request<'a> = GetSessionPolicyRequest; | ||
| type Response = Errorable<SessionPolicy>; | ||
| } | ||
|
|
||
| /// An RPC for a process inside a PTask to request a dynamic ingress port | ||
| /// mapping at runtime (R2.4). | ||
| pub struct DynamicPortMap; | ||
|
|
||
| /// Request for the [`DynamicPortMap`] RPC. | ||
| #[non_exhaustive] | ||
| #[derive(Debug, Clone, Serialize, Deserialize)] | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. LOW — correctness
For consistency with the other new structs in this PR that correctly carry #[non_exhaustive]
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct DynamicPortMapRequest { |
||
| pub struct DynamicPortMapRequest { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. LOW · correctness
#[non_exhaustive]
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct DynamicPortMapRequest { |
||
| pub id: SessionId, | ||
| pub external_port: u16, | ||
| pub internal_port: u16, | ||
| pub proto: IpProto, | ||
| } | ||
|
|
||
| impl DynamicPortMapRequest { | ||
| pub fn new(id: SessionId, external_port: u16, internal_port: u16, proto: IpProto) -> Self { | ||
| Self { | ||
| id, | ||
| external_port, | ||
| internal_port, | ||
| proto, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// Response for the [`DynamicPortMap`] RPC. | ||
| #[non_exhaustive] | ||
| #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. LOW · correctness
#[non_exhaustive]
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
pub struct DynamicPortMapResponse; |
||
| pub struct DynamicPortMapResponse; | ||
|
|
||
| impl OneshotSshRpc for DynamicPortMap { | ||
| const NAME: &'static str = constcat::concat!(RPC_SUBSYSTEM_PREFIX, "DynamicPortMap"); | ||
| type Request<'a> = DynamicPortMapRequest; | ||
| type Response = Errorable<DynamicPortMapResponse>; | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
|
|
||
| #[test] | ||
| fn policy_types_are_present_and_serializable() { | ||
| // PortMapping construction and round-trip | ||
| let mapping = PortMapping { | ||
| external_port: 8080, | ||
| internal_port: 80, | ||
| proto: IpProto::Tcp, | ||
| }; | ||
| let json = serde_json::to_string(&mapping).unwrap(); | ||
| let rt: PortMapping = serde_json::from_str(&json).unwrap(); | ||
| assert_eq!(rt, mapping); | ||
|
|
||
| // IngressPolicy default and round-trip | ||
| let ingress = IngressPolicy { | ||
| port_mappings: vec![mapping], | ||
| dynamic_allowed_range: Some((10000, 20000)), | ||
| }; | ||
| let json = serde_json::to_string(&ingress).unwrap(); | ||
| let rt: IngressPolicy = serde_json::from_str(&json).unwrap(); | ||
| assert_eq!(rt, ingress); | ||
|
|
||
| // EgressPolicy default serializes without error | ||
| let egress = EgressPolicy::default(); | ||
| let json = serde_json::to_string(&egress).unwrap(); | ||
| let _: EgressPolicy = serde_json::from_str(&json).unwrap(); | ||
|
|
||
| // SessionPolicy with null egress and default ingress matches expected CLI output | ||
| let policy = SessionPolicy { | ||
| egress: None, | ||
| ingress: Some(IngressPolicy::default()), | ||
| }; | ||
| let json = serde_json::to_string(&policy).unwrap(); | ||
| assert!(json.contains("\"egress\":null"), "got: {json}"); | ||
| assert!(json.contains("\"port_mappings\":[]"), "got: {json}"); | ||
| assert!( | ||
| json.contains("\"dynamic_allowed_range\":null"), | ||
| "got: {json}" | ||
| ); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MEDIUM — correctness
SessionPolicyis missing#[non_exhaustive]while all three sibling types added in this same PR —PortMapping(line 285),EgressPolicy(line 300), andIngressPolicy(line 314) — ca 10BC8 rry it. The workspace coding standard (docs/rust-coding-standards.md) requires#[non_exhaustive]on public enums/structs that may grow.SessionPolicyis a public RPC response type that is almost certain to grow as the networking stack matures (e.g. anetwork_modefield, DNS policy, or firewall zone could be added in Units 3–4). Without#[non_exhaustive], adding any field is a semver-breaking change tominimald-rpc.Suggested fix: add
#[non_exhaustive]on the line immediately above this#[derive]: