From 93ba27230e5a62323d90a75a3ae7e3cdb4a4d6d5 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Wed, 3 Aug 2022 17:13:47 -0600 Subject: [PATCH 01/35] Don't restrict IPv4 addresses to private subnets Needed to use Ipv4Net for firewall rule targets. --- common/src/api/external/mod.rs | 26 ++------------------------ openapi/nexus.json | 2 +- openapi/sled-agent.json | 2 +- 3 files changed, 4 insertions(+), 26 deletions(-) diff --git a/common/src/api/external/mod.rs b/common/src/api/external/mod.rs index 3dd75e9bc52..460cce4ba40 100644 --- a/common/src/api/external/mod.rs +++ b/common/src/api/external/mod.rs @@ -998,33 +998,11 @@ impl JsonSchema for Ipv4Net { })), instance_type: Some(schemars::schema::InstanceType::String.into()), string: Some(Box::new(schemars::schema::StringValidation { - // Addresses must be from an RFC 1918 private address space pattern: Some( concat!( - r#"^("#, - // 10.0.0.0/8 (10.0.0.0 .. 10.255.255.255) - r#"10\."#, + r#"^(([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])\.){3}"#, r#"([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])"#, - r#"\."#, - r#"([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])"#, - r#"\."#, - r#"([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])"#, - r#"\/([8-9]|1[0-9]|2[0-9]|3[0-2])|"#, - // 172.16.0.0/12 (172.16.0.0 .. 172.31.255.255) - r#"172\."#, - r#"(1[6-9]|2[0-9]|3[0-1])"#, - r#"\."#, - r#"([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])"#, - r#"\."#, - r#"([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])"#, - r#"\/(1[2-9]|2[0-9]|3[0-2])|"#, - // 192.168.0.0/16 (192.168.0.0 .. 192.168.255.255) - r#"192\.168\."#, - r#"([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])"#, - r#"\."#, - r#"([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])"#, - r#"\/(1[6-9]|2[0-9]|3[0-2])"#, - r#")$"#, + r#"/([8-9]|1[0-9]|2[0-9]|3[0-2])$"#, ) .to_string(), ), diff --git a/openapi/nexus.json b/openapi/nexus.json index 9fd18064b5f..7c3176b3fb9 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -9200,7 +9200,7 @@ "title": "An IPv4 subnet", "description": "An IPv4 subnet, including prefix and subnet mask", "type": "string", - "pattern": "^(10\\.([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])\\.([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])\\.([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])\\/([8-9]|1[0-9]|2[0-9]|3[0-2])|172\\.(1[6-9]|2[0-9]|3[0-1])\\.([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])\\.([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])\\/(1[2-9]|2[0-9]|3[0-2])|192\\.168\\.([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])\\.([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])\\/(1[6-9]|2[0-9]|3[0-2]))$" + "pattern": "^(([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])\\.){3}([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])/([8-9]|1[0-9]|2[0-9]|3[0-2])$" }, "Ipv4Range": { "description": "A non-decreasing IPv4 address range, inclusive of both ends.\n\nThe first address must be less than or equal to the last address.", diff --git a/openapi/sled-agent.json b/openapi/sled-agent.json index d39fd033954..7950ed20724 100644 --- a/openapi/sled-agent.json +++ b/openapi/sled-agent.json @@ -1174,7 +1174,7 @@ "title": "An IPv4 subnet", "description": "An IPv4 subnet, including prefix and subnet mask", "type": "string", - "pattern": "^(10\\.([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])\\.([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])\\.([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])\\/([8-9]|1[0-9]|2[0-9]|3[0-2])|172\\.(1[6-9]|2[0-9]|3[0-1])\\.([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])\\.([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])\\/(1[2-9]|2[0-9]|3[0-2])|192\\.168\\.([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])\\.([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])\\/(1[6-9]|2[0-9]|3[0-2]))$" + "pattern": "^(([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])\\.){3}([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])/([8-9]|1[0-9]|2[0-9]|3[0-2])$" }, "Ipv6Net": { "example": "fd12:3456::/64", From d0f2d7864c369b3b7ff43af00ed535c509c28384 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Mon, 1 Aug 2022 09:45:16 -0600 Subject: [PATCH 02/35] Start plumbing firewall rules through sled-agent --- sled-agent/src/http_entrypoints.rs | 28 ++++++ sled-agent/src/instance_manager.rs | 18 +++- sled-agent/src/opte/illumos/port_manager.rs | 98 +++++++++++++++++++ .../src/opte/non_illumos/port_manager.rs | 25 +++++ sled-agent/src/params.rs | 19 ++++ sled-agent/src/sim/http_entrypoints.rs | 25 ++++- sled-agent/src/sled_agent.rs | 12 ++- 7 files changed, 222 insertions(+), 3 deletions(-) diff --git a/sled-agent/src/http_entrypoints.rs b/sled-agent/src/http_entrypoints.rs index 9547405cc44..e2e8bae0cd2 100644 --- a/sled-agent/src/http_entrypoints.rs +++ b/sled-agent/src/http_entrypoints.rs @@ -7,6 +7,7 @@ use crate::params::{ DatasetEnsureBody, DiskEnsureBody, InstanceEnsureBody, InstanceSerialConsoleData, InstanceSerialConsoleRequest, ServiceEnsureBody, + VpcFirewallRulesEnsureBody, }; use crate::serial::ByteOffset; use dropshot::{ @@ -39,6 +40,7 @@ pub fn api() -> SledApiDescription { api.register(instance_serial_get)?; api.register(instance_issue_disk_snapshot_request)?; api.register(issue_disk_snapshot_request)?; + api.register(vpc_firewall_rules_put)?; Ok(()) } @@ -290,3 +292,29 @@ async fn issue_disk_snapshot_request( snapshot_id: body.snapshot_id, })) } + +/// Path parameters for VPC requests (sled agent API) +#[derive(Deserialize, JsonSchema)] +struct VpcPathParam { + vpc_id: Uuid, +} + +#[endpoint { + method = PUT, + path = "/vpc/{vpc_id}/firewall/rules", +}] +async fn vpc_firewall_rules_put( + rqctx: Arc>, + path_params: Path, + body: TypedBody, +) -> Result { + let sa = rqctx.context(); + let vpc_id = path_params.into_inner().vpc_id; + let body_args = body.into_inner(); + + sa.firewall_rules_ensure(vpc_id, &body_args.rules[..]) + .await + .map_err(Error::from)?; + + Ok(HttpResponseUpdatedNoContent()) +} diff --git a/sled-agent/src/instance_manager.rs b/sled-agent/src/instance_manager.rs index 733bc29c2f9..7e5a3ad7b7d 100644 --- a/sled-agent/src/instance_manager.rs +++ b/sled-agent/src/instance_manager.rs @@ -10,7 +10,7 @@ use crate::nexus::LazyNexusClient; use crate::opte::PortManager; use crate::params::{ InstanceHardware, InstanceMigrateParams, InstanceRuntimeStateRequested, - InstanceSerialConsoleData, + InstanceSerialConsoleData, VpcFirewallRule, }; use crate::serial::ByteOffset; use macaddr::MacAddr6; @@ -212,6 +212,22 @@ impl InstanceManager { .await .map_err(Error::from) } + + pub async fn firewall_rules_ensure( + &self, + rules: &[VpcFirewallRule], + ) -> Result<(), Error> { + // TODO-correctness: map from VPC to VNICs on instances. Right now + // we just update each port on every instance, which is totally wrong. + for port_name in self.inner.port_manager.port_names() { + info!( + &self.inner.log, + "Updating firewall rules for OPTE port {}", &port_name + ); + self.inner.port_manager.firewall_rules_ensure(port_name, rules)?; + } + Ok(()) + } } /// Represents membership of an instance in the [`InstanceManager`]. diff --git a/sled-agent/src/opte/illumos/port_manager.rs b/sled-agent/src/opte/illumos/port_manager.rs index 23eae2cd96e..ed3ea4ca328 100644 --- a/sled-agent/src/opte/illumos/port_manager.rs +++ b/sled-agent/src/opte/illumos/port_manager.rs @@ -14,15 +14,26 @@ use crate::opte::Port; use crate::opte::Vni; use crate::params::NetworkInterface; use crate::params::SourceNatConfig; +use crate::params::VpcFirewallRule; use ipnetwork::IpNetwork; use macaddr::MacAddr6; +use omicron_common::api::external::{ + IpNet, VpcFirewallRuleAction, VpcFirewallRuleDirection, + VpcFirewallRuleProtocol, VpcFirewallRuleStatus, +}; +use opte::api::Direction; use opte::api::IpCidr; use opte::api::Ipv4Cidr; use opte::api::Ipv4PrefixLen; use opte::api::MacAddr; +use opte::api::Protocol; use opte::oxide_vpc::api::AddRouterEntryIpv4Req; use opte::oxide_vpc::api::RouterTarget; use opte::oxide_vpc::api::SNatCfg; +use opte::oxide_vpc::api::SetFwRulesReq; +use opte::oxide_vpc::api::{ + Action, Address, Filters, FirewallRule, ProtoFilter, +}; use opte_ioctl::OpteHdl; use slog::debug; use slog::info; @@ -435,6 +446,93 @@ impl PortManager { ); Ok(port) } + + pub fn port_names(&self) -> Vec { + self.inner + .ports + .lock() + .unwrap() + .keys() + .map(|(_instance_id, port_name)| port_name.clone()) + .collect::>() + } + + pub fn firewall_rules_ensure( + &self, + port_name: String, + rules: &[VpcFirewallRule], + ) -> Result<(), Error> { + let hdl = OpteHdl::open(OpteHdl::DLD_CTL)?; + let rules = opte_firewall_rules(rules); + info!(self.inner.log, "OPTE firewall rules"; "rules" => ?&rules,); + hdl.set_fw_rules(&SetFwRulesReq { port_name, rules })?; + Ok(()) + } +} + +/// Translate from a slice of VPC firewall rules to a vector of OPTE rules. +fn opte_firewall_rules(rules: &[VpcFirewallRule]) -> Vec { + rules + .iter() + .filter_map(|rule| match rule.status { + VpcFirewallRuleStatus::Disabled => None, + VpcFirewallRuleStatus::Enabled => Some(FirewallRule { + direction: match rule.direction { + VpcFirewallRuleDirection::Inbound => Direction::In, + VpcFirewallRuleDirection::Outbound => Direction::Out, + }, + filters: { + let mut filters = Filters::new(); + filters + .set_hosts(match rule.filter_hosts { + None => Address::Any, + // TODO: handle multiple host filters + Some(ref hosts) if hosts.len() >0 => { + match hosts[0] { + IpNet::V4(net) /*if net.prefix() == 32*/ => { + Address::Ip(net.ip().into()) + } + /*IpNet::V4(net) => { + Address::Subnet(Ipv4Cidr::new( + net.ip().into(), + Ipv4PrefixLen::new(net.prefix()) + .unwrap(), + )) + }*/ + IpNet::V6(_net) => { + todo!("handle IPv6 host filters") + } + } + } + _ => Address::Any, + }) + .set_protocol(match rule.filter_protocols { + None => ProtoFilter::Any, + Some(ref protos) if protos.len() == 1 => { + match protos[0] { + VpcFirewallRuleProtocol::Tcp => { + ProtoFilter::Proto(Protocol::TCP) + } + VpcFirewallRuleProtocol::Udp => { + ProtoFilter::Proto(Protocol::UDP) + } + VpcFirewallRuleProtocol::Icmp => { + ProtoFilter::Proto(Protocol::ICMP) + } + } + } + _ => todo!("handle multiple protocol filters"), + }); // XXX: .set_ports + filters + }, + action: match rule.action { + VpcFirewallRuleAction::Allow => Action::Allow, + VpcFirewallRuleAction::Deny => Action::Deny, + }, + priority: rule.priority.0, + }), + }) + .collect::>() } #[derive(Clone)] diff --git a/sled-agent/src/opte/non_illumos/port_manager.rs b/sled-agent/src/opte/non_illumos/port_manager.rs index e2b1da07926..f929d7091d7 100644 --- a/sled-agent/src/opte/non_illumos/port_manager.rs +++ b/sled-agent/src/opte/non_illumos/port_manager.rs @@ -12,6 +12,7 @@ use crate::opte::Port; use crate::opte::Vni; use crate::params::NetworkInterface; use crate::params::SourceNatConfig; +use crate::params::VpcFirewallRule; use ipnetwork::IpNetwork; use macaddr::MacAddr6; use slog::debug; @@ -182,6 +183,30 @@ impl PortManager { ); Ok(port) } + + pub fn port_names(&self) -> Vec { + self.inner + .ports + .lock() + .unwrap() + .keys() + .map(|(_instance_id, port_name)| port_name.clone()) + .collect::>() + } + + pub fn firewall_rules_ensure( + &self, + port_name: String, + rules: &[VpcFirewallRule], + ) -> Result<(), Error> { + info!( + self.inner.log, + "Ignoring {} firewall rules for {}", + rules.len(), + &port_name + ); + Ok(()) + } } #[derive(Clone)] diff --git a/sled-agent/src/params.rs b/sled-agent/src/params.rs index a5a34bd9179..e0d9d2a4150 100644 --- a/sled-agent/src/params.rs +++ b/sled-agent/src/params.rs @@ -40,6 +40,25 @@ pub struct SourceNatConfig { pub last_port: u16, } +/// Update firewall rules for a VPC +#[derive(Clone, Debug, Serialize, Deserialize, JsonSchema)] +pub struct VpcFirewallRulesEnsureBody { + pub rules: Vec, +} + +/// VPC firewall rule after object name resolution has been performed by Nexus +#[derive(Clone, Debug, Serialize, Deserialize, JsonSchema)] +pub struct VpcFirewallRule { + pub status: external::VpcFirewallRuleStatus, + pub direction: external::VpcFirewallRuleDirection, + pub targets: Vec, + pub filter_hosts: Option>, + pub filter_ports: Option>, + pub filter_protocols: Option>, + pub action: external::VpcFirewallRuleAction, + pub priority: external::VpcFirewallRulePriority, +} + /// Used to request a Disk state change #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize, JsonSchema)] #[serde(rename_all = "lowercase", tag = "state", content = "instance")] diff --git a/sled-agent/src/sim/http_entrypoints.rs b/sled-agent/src/sim/http_entrypoints.rs index 88cbe889297..5202f4c795f 100644 --- a/sled-agent/src/sim/http_entrypoints.rs +++ b/sled-agent/src/sim/http_entrypoints.rs @@ -6,7 +6,7 @@ use crate::params::{ DiskEnsureBody, InstanceEnsureBody, InstanceSerialConsoleData, - InstanceSerialConsoleRequest, + InstanceSerialConsoleRequest, VpcFirewallRulesEnsureBody, }; use crate::serial::ByteOffset; use dropshot::endpoint; @@ -43,6 +43,7 @@ pub fn api() -> SledApiDescription { api.register(instance_serial_get)?; api.register(instance_issue_disk_snapshot_request)?; api.register(issue_disk_snapshot_request)?; + api.register(vpc_firewall_rules_put)?; Ok(()) } @@ -284,3 +285,25 @@ async fn issue_disk_snapshot_request( snapshot_id: body.snapshot_id, })) } + +/// Path parameters for VPC requests (sled agent API) +#[derive(Deserialize, JsonSchema)] +struct VpcPathParam { + vpc_id: Uuid, +} + +#[endpoint { + method = PUT, + path = "/vpc/{vpc_id}/firewall/rules", +}] +async fn vpc_firewall_rules_put( + rqctx: Arc>>, + path_params: Path, + body: TypedBody, +) -> Result { + let _sa = rqctx.context(); + let _vpc_id = path_params.into_inner().vpc_id; + let _body_args = body.into_inner(); + + Ok(HttpResponseUpdatedNoContent()) +} diff --git a/sled-agent/src/sled_agent.rs b/sled-agent/src/sled_agent.rs index adcd8b1d5c6..2f613eec89c 100644 --- a/sled-agent/src/sled_agent.rs +++ b/sled-agent/src/sled_agent.rs @@ -17,7 +17,7 @@ use crate::nexus::LazyNexusClient; use crate::params::{ DatasetKind, DiskStateRequested, InstanceHardware, InstanceMigrateParams, InstanceRuntimeStateRequested, InstanceSerialConsoleData, - ServiceEnsureBody, + ServiceEnsureBody, VpcFirewallRule, }; use crate::services::{self, ServiceManager}; use crate::storage_manager::StorageManager; @@ -431,6 +431,16 @@ impl SledAgent { // means. Currently unimplemented. todo!(); } + + pub async fn firewall_rules_ensure( + &self, + _vpc_id: Uuid, + rules: &[VpcFirewallRule], + ) -> Result<(), Error> { + // TODO-correctness: map from VPC to VNICs on instances. Right now + // we just update each port on every instance, which is totally wrong. + self.instances.firewall_rules_ensure(rules).await.map_err(Error::from) + } } // Delete all underlay addresses created directly over the etherstub VNIC used From 2909d15348e4926b71285ff1cf8334e61cbaddca Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Mon, 1 Aug 2022 11:50:59 -0600 Subject: [PATCH 03/35] Send firewall rules to sled-agent --- nexus/src/app/vpc.rs | 127 ++++++++++++++++++++++++++--- nexus/src/db/datastore/vpc.rs | 25 ++++++ openapi/sled-agent.json | 146 ++++++++++++++++++++++++++++++++++ sled-agent-client/src/lib.rs | 57 +++++++++++++ 4 files changed, 345 insertions(+), 10 deletions(-) diff --git a/nexus/src/app/vpc.rs b/nexus/src/app/vpc.rs index 9c6950abbaa..d0c14d70e18 100644 --- a/nexus/src/app/vpc.rs +++ b/nexus/src/app/vpc.rs @@ -7,6 +7,7 @@ use crate::authz; use crate::context::OpContext; use crate::db; +use crate::db::identity::{Asset, Resource}; use crate::db::lookup::LookupPath; use crate::db::model::Name; use crate::db::model::VpcRouterKind; @@ -17,6 +18,7 @@ use omicron_common::api::external; use omicron_common::api::external::CreateResult; use omicron_common::api::external::DataPageParams; use omicron_common::api::external::DeleteResult; +use omicron_common::api::external::Error; use omicron_common::api::external::IdentityMetadataCreateParams; use omicron_common::api::external::ListResultVec; use omicron_common::api::external::LookupResult; @@ -27,6 +29,8 @@ use omicron_common::api::external::RouterRouteCreateParams; use omicron_common::api::external::RouterRouteKind; use omicron_common::api::external::UpdateResult; use omicron_common::api::external::VpcFirewallRuleUpdateParams; + +use futures::future::join_all; use uuid::Uuid; impl super::Nexus { @@ -62,7 +66,7 @@ impl super::Nexus { )?; let (authz_vpc, db_vpc) = self .db_datastore - .project_create_vpc(opctx, &authz_project, vpc) + .project_create_vpc(opctx, &authz_project, vpc.clone()) .await?; // TODO: Ultimately when the VPC is created a system router w/ an @@ -171,8 +175,9 @@ impl super::Nexus { defaults::DEFAULT_FIREWALL_RULES.clone(), ); self.db_datastore - .vpc_update_firewall_rules(opctx, &authz_vpc, rules) + .vpc_update_firewall_rules(opctx, &authz_vpc, rules.clone()) .await?; + self.send_sled_agents_firewall_rules(&db_vpc, &rules).await?; Ok(db_vpc) } @@ -315,18 +320,120 @@ impl super::Nexus { vpc_name: &Name, params: &VpcFirewallRuleUpdateParams, ) -> UpdateResult> { - let (.., authz_vpc) = LookupPath::new(opctx, &self.db_datastore) - .organization_name(organization_name) - .project_name(project_name) - .vpc_name(vpc_name) - .lookup_for(authz::Action::Modify) - .await?; + let (.., authz_vpc, db_vpc) = + LookupPath::new(opctx, &self.db_datastore) + .organization_name(organization_name) + .project_name(project_name) + .vpc_name(vpc_name) + .fetch_for(authz::Action::Modify) + .await?; let rules = db::model::VpcFirewallRule::vec_from_params( authz_vpc.id(), params.clone(), ); - self.db_datastore + let rules = self + .db_datastore .vpc_update_firewall_rules(opctx, &authz_vpc, rules) - .await + .await?; + self.send_sled_agents_firewall_rules(&db_vpc, &rules).await?; + Ok(rules) + } + + async fn send_sled_agents_firewall_rules( + &self, + vpc: &db::model::Vpc, + rules: &[db::model::VpcFirewallRule], + ) -> Result<(), Error> { + info!(self.log, "sending firewall rules to sled agents"); + + let rules_for_sled = + self.resolve_firewall_rules_for_sled_agent(&vpc, rules).await?; + info!(self.log, "resolved {} rules for sleds", rules_for_sled.len()); + let sled_rules_request = + sled_agent_client::types::VpcFirewallRulesEnsureBody { + rules: rules_for_sled, + }; + + let vpc_to_sleds = + self.db_datastore.vpc_resolve_to_sleds(vpc.id()).await?; + info!(self.log, "resolved sleds for vpc {}", vpc.name(); "vpc_to_sled" => ?vpc_to_sleds); + let mut sled_requests = Vec::with_capacity(vpc_to_sleds.len()); + for sled in &vpc_to_sleds { + let sled_id = sled.id(); + let vpc_id = vpc.id(); + let sled_rules_request = sled_rules_request.clone(); + sled_requests.push(async move { + self.sled_client(&sled_id) + .await? + .vpc_firewall_rules_put(&vpc_id, &sled_rules_request) + .await + .map_err(|e| Error::internal_error(&e.to_string())) + }); + } + + let results = join_all(sled_requests).await; + // TODO-correctness: handle more than one failure in the sled-agent requests + for (sled, result) in vpc_to_sleds.iter().zip(results) { + if let Err(e) = result { + warn!(self.log, "failed to update firewall rules on sled agent"; + "sled_id" => %sled.id(), + "vpc_id" => %vpc.id(), + "error" => %e); + return Err(e); + } + } + info!( + self.log, + "updated firewall rules on {} sleds", + vpc_to_sleds.len() + ); + + Ok(()) + } + + async fn resolve_firewall_rules_for_sled_agent( + &self, + vpc: &db::model::Vpc, + rules: &[db::model::VpcFirewallRule], + ) -> Result, Error> { + Ok(rules + .iter() + .map(|rule| sled_agent_client::types::VpcFirewallRule { + status: rule.status.0.into(), + direction: rule.direction.0.into(), + targets: vec![], // XXX + filter_hosts: rule.filter_hosts.as_ref().map(|hosts| { + hosts + .iter() + .filter_map(|host| match host { + db::model::VpcFirewallRuleHostFilter( + external::VpcFirewallRuleHostFilter::Ip( + std::net::IpAddr::V4(addr), + ), + ) => Some(sled_agent_client::types::IpNet::V4( + external::Ipv4Net( + ipnetwork::Ipv4Network::new( + addr.clone(), + 32, + ) + .unwrap(), + ) + .into(), + )), + _ => None, // XXX + }) + .collect() + }), + filter_ports: rule + .filter_ports + .as_ref() + .map(|ports| ports.iter().map(|v| v.0.into()).collect()), + filter_protocols: rule.filter_protocols.as_ref().map( + |protocols| protocols.iter().map(|v| v.0.into()).collect(), + ), + action: rule.action.0.into(), + priority: rule.priority.0 .0, + }) + .collect::>()) } } diff --git a/nexus/src/db/datastore/vpc.rs b/nexus/src/db/datastore/vpc.rs index 946207f65a3..3afb0dd629f 100644 --- a/nexus/src/db/datastore/vpc.rs +++ b/nexus/src/db/datastore/vpc.rs @@ -21,6 +21,7 @@ use crate::db::model::Name; use crate::db::model::NetworkInterface; use crate::db::model::RouterRoute; use crate::db::model::RouterRouteUpdate; +use crate::db::model::Sled; use crate::db::model::Vpc; use crate::db::model::VpcFirewallRule; use crate::db::model::VpcRouter; @@ -318,6 +319,30 @@ impl DataStore { }) } + /// Return the list of `Sled`s hosting instances with network interfaces + /// on the provided VPC. + pub async fn vpc_resolve_to_sleds( + &self, + vpc_id: Uuid, + ) -> Result, Error> { + // Resolve each VNIC in the VPC to the Sled it's on, so we know which + // Sleds to notify when firewall rules change. + use db::schema::{instance, network_interface, sled}; + network_interface::table + .inner_join( + instance::table + .on(instance::id.eq(network_interface::instance_id)), + ) + .inner_join(sled::table.on(sled::id.eq(instance::active_server_id))) + .filter(network_interface::vpc_id.eq(vpc_id)) + .filter(network_interface::time_deleted.is_null()) + .filter(instance::time_deleted.is_null()) + .select(Sled::as_select()) + .get_results_async(self.pool()) + .await + .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server)) + } + pub async fn vpc_list_subnets( &self, opctx: &OpContext, diff --git a/openapi/sled-agent.json b/openapi/sled-agent.json index 7950ed20724..42a7590bc6e 100644 --- a/openapi/sled-agent.json +++ b/openapi/sled-agent.json @@ -352,6 +352,44 @@ } } } + }, + "/vpc/{vpc_id}/firewall/rules": { + "put": { + "operationId": "vpc_firewall_rules_put", + "parameters": [ + { + "in": "path", + "name": "vpc_id", + "required": true, + "schema": { + "type": "string", + "format": "uuid" + }, + "style": "simple" + } + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/VpcFirewallRulesEnsureBody" + } + } + }, + "required": true + }, + "responses": { + "204": { + "description": "resource updated" + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } } }, "components": { @@ -1183,6 +1221,15 @@ "type": "string", "pattern": "^([fF][dD])[0-9a-fA-F]{2}:(([0-9a-fA-F]{1,4}:){6}[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,6}:)\\/([1-9]|[1-9][0-9]|1[0-1][0-9]|12[0-8])$" }, + "L4PortRange": { + "example": "22", + "title": "A range of IP ports", + "description": "An inclusive-inclusive range of IP ports. The second port may be omitted to represent a single port", + "type": "string", + "pattern": "^[0-9]{1,5}(-[0-9]{1,5})?$", + "minLength": 1, + "maxLength": 11 + }, "MacAddr": { "example": "ff:ff:ff:ff:ff:ff", "title": "A MAC address", @@ -1572,6 +1619,105 @@ ] } ] + }, + "VpcFirewallRule": { + "description": "VPC firewall rule after object name resolution has been performed by Nexus", + "type": "object", + "properties": { + "action": { + "$ref": "#/components/schemas/VpcFirewallRuleAction" + }, + "direction": { + "$ref": "#/components/schemas/VpcFirewallRuleDirection" + }, + "filter_hosts": { + "nullable": true, + "type": "array", + "items": { + "$ref": "#/components/schemas/IpNet" + } + }, + "filter_ports": { + "nullable": true, + "type": "array", + "items": { + "$ref": "#/components/schemas/L4PortRange" + } + }, + "filter_protocols": { + "nullable": true, + "type": "array", + "items": { + "$ref": "#/components/schemas/VpcFirewallRuleProtocol" + } + }, + "priority": { + "type": "integer", + "format": "uint16", + "minimum": 0 + }, + "status": { + "$ref": "#/components/schemas/VpcFirewallRuleStatus" + }, + "targets": { + "type": "array", + "items": { + "$ref": "#/components/schemas/NetworkInterface" + } + } + }, + "required": [ + "action", + "direction", + "priority", + "status", + "targets" + ] + }, + "VpcFirewallRuleAction": { + "type": "string", + "enum": [ + "allow", + "deny" + ] + }, + "VpcFirewallRuleDirection": { + "type": "string", + "enum": [ + "inbound", + "outbound" + ] + }, + "VpcFirewallRuleProtocol": { + "description": "The protocols that may be specified in a firewall rule's filter", + "type": "string", + "enum": [ + "TCP", + "UDP", + "ICMP" + ] + }, + "VpcFirewallRuleStatus": { + "type": "string", + "enum": [ + "disabled", + "enabled" + ] + }, + "VpcFirewallRulesEnsureBody": { + "description": "Update firewall rules for a VPC", + "type": "object", + "properties": { + "rules": { + "type": "array", + "items": { + "$ref": "#/components/schemas/VpcFirewallRule" + } + } + }, + "required": [ + "rules" + ] } } } diff --git a/sled-agent-client/src/lib.rs b/sled-agent-client/src/lib.rs index 1d04733127c..2a50ebb3122 100644 --- a/sled-agent-client/src/lib.rs +++ b/sled-agent-client/src/lib.rs @@ -235,6 +235,12 @@ impl From for types::IpNet { } } +impl From for types::L4PortRange { + fn from(s: omicron_common::api::external::L4PortRange) -> Self { + Self::try_from(s.to_string()).unwrap_or_else(|e| panic!("{}: {}", s, e)) + } +} + impl From for types::UpdateArtifact { @@ -261,6 +267,57 @@ impl From } } +impl From + for types::VpcFirewallRuleAction +{ + fn from(s: omicron_common::api::external::VpcFirewallRuleAction) -> Self { + use omicron_common::api::external::VpcFirewallRuleAction::*; + match s { + Allow => Self::Allow, + Deny => Self::Deny, + } + } +} + +impl From + for types::VpcFirewallRuleDirection +{ + fn from( + s: omicron_common::api::external::VpcFirewallRuleDirection, + ) -> Self { + use omicron_common::api::external::VpcFirewallRuleDirection::*; + match s { + Inbound => Self::Inbound, + Outbound => Self::Outbound, + } + } +} + +impl From + for types::VpcFirewallRuleStatus +{ + fn from(s: omicron_common::api::external::VpcFirewallRuleStatus) -> Self { + use omicron_common::api::external::VpcFirewallRuleStatus::*; + match s { + Enabled => Self::Enabled, + Disabled => Self::Disabled, + } + } +} + +impl From + for types::VpcFirewallRuleProtocol +{ + fn from(s: omicron_common::api::external::VpcFirewallRuleProtocol) -> Self { + use omicron_common::api::external::VpcFirewallRuleProtocol::*; + match s { + Tcp => Self::Tcp, + Udp => Self::Udp, + Icmp => Self::Icmp, + } + } +} + /// Exposes additional [`Client`] interfaces for use by the test suite. These /// are bonus endpoints, not generated in the real client. #[async_trait] From 8715ba8b660dc20fd07cc7dcc46c52ca48807e80 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Mon, 8 Aug 2022 10:30:45 -0600 Subject: [PATCH 04/35] Plumb ports in firewall rules --- sled-agent/src/opte/illumos/port_manager.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/sled-agent/src/opte/illumos/port_manager.rs b/sled-agent/src/opte/illumos/port_manager.rs index ed3ea4ca328..69d2b9ed34b 100644 --- a/sled-agent/src/opte/illumos/port_manager.rs +++ b/sled-agent/src/opte/illumos/port_manager.rs @@ -32,7 +32,7 @@ use opte::oxide_vpc::api::RouterTarget; use opte::oxide_vpc::api::SNatCfg; use opte::oxide_vpc::api::SetFwRulesReq; use opte::oxide_vpc::api::{ - Action, Address, Filters, FirewallRule, ProtoFilter, + Action, Address, Filters, FirewallRule, Ports, ProtoFilter, }; use opte_ioctl::OpteHdl; use slog::debug; @@ -522,7 +522,16 @@ fn opte_firewall_rules(rules: &[VpcFirewallRule]) -> Vec { } } _ => todo!("handle multiple protocol filters"), - }); // XXX: .set_ports + }) + .set_ports(match rule.filter_ports { + None => Ports::Any, + Some(ref ports) => Ports::PortList( + ports.iter().flat_map(|range| { + (range.first.0.get()..=range.last.0.get()) + .collect::>() + }).collect::>() + ), + }); filters }, action: match rule.action { From 1c4d983642915c0fd734f8f728cab2d50964dc27 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Mon, 8 Aug 2022 13:23:59 -0600 Subject: [PATCH 05/35] Handle multiple hosts & protocols in firewall rules --- sled-agent/src/opte/illumos/port_manager.rs | 146 +++++++++++--------- 1 file changed, 83 insertions(+), 63 deletions(-) diff --git a/sled-agent/src/opte/illumos/port_manager.rs b/sled-agent/src/opte/illumos/port_manager.rs index 69d2b9ed34b..7da30f81e2e 100644 --- a/sled-agent/src/opte/illumos/port_manager.rs +++ b/sled-agent/src/opte/illumos/port_manager.rs @@ -463,7 +463,7 @@ impl PortManager { rules: &[VpcFirewallRule], ) -> Result<(), Error> { let hdl = OpteHdl::open(OpteHdl::DLD_CTL)?; - let rules = opte_firewall_rules(rules); + let rules = collect_firewall_rules(rules); info!(self.inner.log, "OPTE firewall rules"; "rules" => ?&rules,); hdl.set_fw_rules(&SetFwRulesReq { port_name, rules })?; Ok(()) @@ -471,76 +471,96 @@ impl PortManager { } /// Translate from a slice of VPC firewall rules to a vector of OPTE rules. -fn opte_firewall_rules(rules: &[VpcFirewallRule]) -> Vec { +/// OPTE rules can only encode a single host address and protocol, so we must +/// unroll rules with multiple hosts/protocols. +fn collect_firewall_rules(rules: &[VpcFirewallRule]) -> Vec { rules .iter() - .filter_map(|rule| match rule.status { - VpcFirewallRuleStatus::Disabled => None, - VpcFirewallRuleStatus::Enabled => Some(FirewallRule { - direction: match rule.direction { - VpcFirewallRuleDirection::Inbound => Direction::In, - VpcFirewallRuleDirection::Outbound => Direction::Out, + .filter(|rule| match rule.status { + VpcFirewallRuleStatus::Disabled => false, + VpcFirewallRuleStatus::Enabled => true, + }) + .map(|rule| { + let priority = rule.priority.0; + let action = match rule.action { + VpcFirewallRuleAction::Allow => Action::Allow, + VpcFirewallRuleAction::Deny => Action::Deny, + }; + let direction = match rule.direction { + VpcFirewallRuleDirection::Inbound => Direction::In, + VpcFirewallRuleDirection::Outbound => Direction::Out, + }; + let ports = match rule.filter_ports { + Some(ref ports) if ports.len() > 0 => Ports::PortList( + ports + .iter() + .flat_map(|range| { + (range.first.0.get()..=range.last.0.get()) + .collect::>() + }) + .collect::>(), + ), + _ => Ports::Any, + }; + let proto_filters = rule.filter_protocols.as_ref().map_or_else( + || vec![ProtoFilter::Any], + |protos| { + protos + .iter() + .map(|proto| { + ProtoFilter::Proto(match proto { + VpcFirewallRuleProtocol::Tcp => Protocol::TCP, + VpcFirewallRuleProtocol::Udp => Protocol::UDP, + VpcFirewallRuleProtocol::Icmp => Protocol::ICMP, + }) + }) + .collect::>() }, - filters: { - let mut filters = Filters::new(); - filters - .set_hosts(match rule.filter_hosts { - None => Address::Any, - // TODO: handle multiple host filters - Some(ref hosts) if hosts.len() >0 => { - match hosts[0] { - IpNet::V4(net) /*if net.prefix() == 32*/ => { - Address::Ip(net.ip().into()) - } - /*IpNet::V4(net) => { - Address::Subnet(Ipv4Cidr::new( - net.ip().into(), - Ipv4PrefixLen::new(net.prefix()) - .unwrap(), - )) - }*/ - IpNet::V6(_net) => { - todo!("handle IPv6 host filters") - } - } + ); + let host_filters = rule.filter_hosts.as_ref().map_or_else( + || vec![Address::Any], + |hosts| { + hosts + .iter() + .map(|host| match host { + IpNet::V4(net) if net.prefix() == 32 => { + Address::Ip(net.ip().into()) } - _ => Address::Any, - }) - .set_protocol(match rule.filter_protocols { - None => ProtoFilter::Any, - Some(ref protos) if protos.len() == 1 => { - match protos[0] { - VpcFirewallRuleProtocol::Tcp => { - ProtoFilter::Proto(Protocol::TCP) - } - VpcFirewallRuleProtocol::Udp => { - ProtoFilter::Proto(Protocol::UDP) - } - VpcFirewallRuleProtocol::Icmp => { - ProtoFilter::Proto(Protocol::ICMP) - } - } + IpNet::V4(net) => Address::Subnet(Ipv4Cidr::new( + net.ip().into(), + Ipv4PrefixLen::new(net.prefix()).unwrap(), + )), + IpNet::V6(_net) => { + todo!("IPv6 host filters") } - _ => todo!("handle multiple protocol filters"), }) - .set_ports(match rule.filter_ports { - None => Ports::Any, - Some(ref ports) => Ports::PortList( - ports.iter().flat_map(|range| { - (range.first.0.get()..=range.last.0.get()) - .collect::>() - }).collect::>() - ), - }); - filters - }, - action: match rule.action { - VpcFirewallRuleAction::Allow => Action::Allow, - VpcFirewallRuleAction::Deny => Action::Deny, + .collect::>() }, - priority: rule.priority.0, - }), + ); + proto_filters + .iter() + .map(|proto| { + host_filters + .iter() + .map(|hosts| FirewallRule { + priority, + action, + direction, + filters: { + let mut filters = Filters::new(); + filters + .set_hosts(hosts.clone()) + .set_protocol(proto.clone()) + .set_ports(ports.clone()); + filters + }, + }) + .collect::>() + }) + .collect::>>() }) + .flatten() + .flatten() .collect::>() } From 8fe8dfe7239b510875c169bbb9b856d50ccfdbff Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Mon, 15 Aug 2022 13:25:58 -0600 Subject: [PATCH 06/35] Handle Ipv4Net host filters --- nexus/src/app/vpc.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/nexus/src/app/vpc.rs b/nexus/src/app/vpc.rs index d0c14d70e18..b4c2b435ace 100644 --- a/nexus/src/app/vpc.rs +++ b/nexus/src/app/vpc.rs @@ -420,6 +420,15 @@ impl super::Nexus { ) .into(), )), + db::model::VpcFirewallRuleHostFilter( + external::VpcFirewallRuleHostFilter::IpNet( + external::IpNet::V4(external::Ipv4Net( + network, + )), + ), + ) => Some(sled_agent_client::types::IpNet::V4( + external::Ipv4Net(network.clone()).into(), + )), _ => None, // XXX }) .collect() From b15b4526051d3fba627aca4f5c319c9c12cfd445 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Fri, 19 Aug 2022 15:54:33 -0600 Subject: [PATCH 07/35] Resolve firewall targets and hosts --- Cargo.lock | 1 + nexus/db-model/src/name.rs | 1 + nexus/src/app/vpc.rs | 347 +++++++++++++++++--- nexus/src/db/datastore/network_interface.rs | 151 +++++++-- nexus/src/db/datastore/vpc.rs | 30 ++ sled-agent-client/Cargo.toml | 1 + sled-agent-client/src/lib.rs | 46 +++ 7 files changed, 489 insertions(+), 88 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d511f91c6c8..2f5a67e68af 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4930,6 +4930,7 @@ version = "0.1.0" dependencies = [ "async-trait", "chrono", + "ipnetwork", "omicron-common", "progenitor", "regress", diff --git a/nexus/db-model/src/name.rs b/nexus/db-model/src/name.rs index 158a4c545a1..96603530333 100644 --- a/nexus/db-model/src/name.rs +++ b/nexus/db-model/src/name.rs @@ -22,6 +22,7 @@ use serde::{Deserialize, Serialize}; AsExpression, FromSqlRow, Eq, + Hash, PartialEq, Ord, PartialOrd, diff --git a/nexus/src/app/vpc.rs b/nexus/src/app/vpc.rs index b4c2b435ace..dc225728df8 100644 --- a/nexus/src/app/vpc.rs +++ b/nexus/src/app/vpc.rs @@ -29,8 +29,11 @@ use omicron_common::api::external::RouterRouteCreateParams; use omicron_common::api::external::RouterRouteKind; use omicron_common::api::external::UpdateResult; use omicron_common::api::external::VpcFirewallRuleUpdateParams; +use sled_agent_client::types::NetworkInterface; use futures::future::join_all; +use ipnetwork::IpNetwork; +use std::collections::{HashMap, HashSet}; use uuid::Uuid; impl super::Nexus { @@ -177,7 +180,14 @@ impl super::Nexus { self.db_datastore .vpc_update_firewall_rules(opctx, &authz_vpc, rules.clone()) .await?; - self.send_sled_agents_firewall_rules(&db_vpc, &rules).await?; + self.send_sled_agents_firewall_rules( + opctx, + organization_name, + project_name, + &db_vpc, + &rules, + ) + .await?; Ok(db_vpc) } @@ -335,20 +345,35 @@ impl super::Nexus { .db_datastore .vpc_update_firewall_rules(opctx, &authz_vpc, rules) .await?; - self.send_sled_agents_firewall_rules(&db_vpc, &rules).await?; + self.send_sled_agents_firewall_rules( + opctx, + organization_name, + project_name, + &db_vpc, + &rules, + ) + .await?; Ok(rules) } async fn send_sled_agents_firewall_rules( &self, + opctx: &OpContext, + organization_name: &Name, + project_name: &Name, vpc: &db::model::Vpc, rules: &[db::model::VpcFirewallRule], ) -> Result<(), Error> { - info!(self.log, "sending firewall rules to sled agents"); - - let rules_for_sled = - self.resolve_firewall_rules_for_sled_agent(&vpc, rules).await?; - info!(self.log, "resolved {} rules for sleds", rules_for_sled.len()); + let rules_for_sled = self + .resolve_firewall_rules_for_sled_agent( + opctx, + organization_name, + project_name, + &vpc, + rules, + ) + .await?; + debug!(self.log, "resolved {} rules for sleds", rules_for_sled.len()); let sled_rules_request = sled_agent_client::types::VpcFirewallRulesEnsureBody { rules: rules_for_sled, @@ -356,7 +381,8 @@ impl super::Nexus { let vpc_to_sleds = self.db_datastore.vpc_resolve_to_sleds(vpc.id()).await?; - info!(self.log, "resolved sleds for vpc {}", vpc.name(); "vpc_to_sled" => ?vpc_to_sleds); + debug!(self.log, "resolved sleds for vpc {}", vpc.name(); "vpc_to_sled" => ?vpc_to_sleds); + let mut sled_requests = Vec::with_capacity(vpc_to_sleds.len()); for sled in &vpc_to_sleds { let sled_id = sled.id(); @@ -371,6 +397,7 @@ impl super::Nexus { }); } + debug!(self.log, "sending firewall rules to sled agents"); let results = join_all(sled_requests).await; // TODO-correctness: handle more than one failure in the sled-agent requests for (sled, result) in vpc_to_sleds.iter().zip(results) { @@ -393,56 +420,272 @@ impl super::Nexus { async fn resolve_firewall_rules_for_sled_agent( &self, + opctx: &OpContext, + organization_name: &Name, + project_name: &Name, vpc: &db::model::Vpc, rules: &[db::model::VpcFirewallRule], ) -> Result, Error> { - Ok(rules - .iter() - .map(|rule| sled_agent_client::types::VpcFirewallRule { + // Collect the names of instances, subnets, and VPCs that are either + // targets or host filters. We have to find the sleds for all the + // targets, and we'll need information about the IP addresses or + // subnets for things that are specified as host filters as well. + let mut instances: HashSet = HashSet::new(); + let mut subnets: HashSet = HashSet::new(); + let mut vpcs: HashSet = HashSet::new(); + for rule in rules { + for target in &rule.targets { + match &target.0 { + external::VpcFirewallRuleTarget::Instance(name) => { + instances.insert(name.clone().into()); + } + external::VpcFirewallRuleTarget::Subnet(name) => { + subnets.insert(name.clone().into()); + } + external::VpcFirewallRuleTarget::Vpc(name) => { + if name != vpc.name() { + return Err(Error::invalid_request( + "cross-VPC firewall target unsupported", + )); + } + vpcs.insert(name.clone().into()); + } + // We don't need to resolve anything for Ip(Net)s. + external::VpcFirewallRuleTarget::Ip(_) => (), + external::VpcFirewallRuleTarget::IpNet(_) => (), + } + } + + for host in rule.filter_hosts.iter().flatten() { + match &host.0 { + external::VpcFirewallRuleHostFilter::Instance(name) => { + instances.insert(name.clone().into()); + } + external::VpcFirewallRuleHostFilter::Subnet(name) => { + subnets.insert(name.clone().into()); + } + external::VpcFirewallRuleHostFilter::Vpc(name) => { + if name != vpc.name() { + return Err(Error::invalid_request( + "cross-VPC firewall host filter unsupported", + )); + } + vpcs.insert(name.clone().into()); + } + // We don't need to resolve anything for Ip(Net)s. + external::VpcFirewallRuleHostFilter::Ip(_) => (), + external::VpcFirewallRuleHostFilter::IpNet(_) => (), + } + } + } + + // Resolve named instances, VPCs, and subnets. + // TODO-correctness: It's possible the resolving queries produce + // inconsistent results due to concurrent changes. They should be + // transactional. + type NetMap = HashMap>; + type NicMap = HashMap>; + let no_networks: Vec = Vec::new(); + let no_interfaces: Vec = Vec::new(); + + let mut instance_interfaces: NicMap = HashMap::new(); + for instance_name in &instances { + let (.., authz_instance) = + LookupPath::new(opctx, &self.db_datastore) + .organization_name(organization_name) + .project_name(project_name) + .instance_name(instance_name) + .lookup_for(authz::Action::ListChildren) + .await?; + for iface in self + .db_datastore + .derive_guest_network_interface_info(opctx, &authz_instance) + .await? + { + instance_interfaces + .entry(instance_name.0.clone()) + .or_insert_with(Vec::new) + .push(iface); + } + } + + let mut vpc_interfaces: NicMap = HashMap::new(); + for vpc_name in &vpcs { + let (.., authz_vpc) = LookupPath::new(opctx, &self.db_datastore) + .organization_name(organization_name) + .project_name(project_name) + .vpc_name(vpc_name) + .lookup_for(authz::Action::ListChildren) + .await?; + for iface in self + .db_datastore + .derive_vpc_network_interface_info(opctx, &authz_vpc) + .await? + { + vpc_interfaces + .entry(vpc_name.0.clone()) + .or_insert_with(Vec::new) + .push(iface); + } + } + + let mut subnet_interfaces: NicMap = HashMap::new(); + for subnet_name in &subnets { + let (.., authz_subnet) = LookupPath::new(opctx, &self.db_datastore) + .organization_name(organization_name) + .project_name(project_name) + .vpc_name(&Name::from(vpc.name().clone())) + .vpc_subnet_name(subnet_name) + .lookup_for(authz::Action::ListChildren) + .await?; + for iface in self + .db_datastore + .derive_subnet_network_interface_info(opctx, &authz_subnet) + .await? + { + subnet_interfaces + .entry(subnet_name.0.clone()) + .or_insert_with(Vec::new) + .push(iface); + } + } + + let subnet_networks: NetMap = self + .db_datastore + .resolve_subnets_to_ips(vpc, subnets) + .await? + .into_iter() + .map(|(name, v)| (name.0, v)) + .collect(); + + debug!( + self.log, + "resolved names for firewall rules"; + "instance_interfaces" => ?instance_interfaces, + "vpc_interfaces" => ?vpc_interfaces, + "subnet_interfaces" => ?subnet_interfaces, + "subnet_networks" => ?subnet_networks, + ); + + // Compile resolved rules for the sled agents. + let mut sled_agent_rules = Vec::with_capacity(rules.len()); + for rule in rules { + // TODO: what is the correct behavior when a name is not found? + // Options: + // (1) Fail update request (though note this can still arise + // from things like instance deletion) + // (2) Allow update request, ignore this rule (but store it + // in case it becomes valid later). This is consistent + // with the semantics of the rules. Rules with bad + // references should likely at least be flagged to users. + // We currently adopt option (2), as this allows users to add + // firewall rules (including default rules) before instances + // and their interfaces are instantiated. + + let mut targets = Vec::with_capacity(rule.targets.len()); + for target in &rule.targets { + match &target.0 { + external::VpcFirewallRuleTarget::Vpc(name) => { + for interface in + vpc_interfaces.get(&name).unwrap_or(&no_interfaces) + { + targets.push(interface.clone().into()); + } + } + external::VpcFirewallRuleTarget::Subnet(name) => { + for interface in subnet_interfaces + .get(&name) + .unwrap_or(&no_interfaces) + { + targets.push(interface.clone().into()); + } + } + external::VpcFirewallRuleTarget::Instance(name) => { + for interface in instance_interfaces + .get(&name) + .unwrap_or(&no_interfaces) + { + targets.push(interface.clone().into()); + } + } + external::VpcFirewallRuleTarget::Ip(_addr) => todo!("target addr"), + external::VpcFirewallRuleTarget::IpNet(_net) => todo!("target net"), + } + } + + let filter_hosts = match &rule.filter_hosts { + None => None, + Some(hosts) => { + let mut host_addrs = Vec::with_capacity(hosts.len()); + for host in hosts { + match &host.0 { + external::VpcFirewallRuleHostFilter::Instance( + name, + ) => { + for interface in instance_interfaces + .get(&name) + .unwrap_or(&no_interfaces) + { + host_addrs.push(interface.ip.into()) + } + } + external::VpcFirewallRuleHostFilter::Subnet( + name, + ) => { + for subnet in subnet_networks + .get(&name) + .unwrap_or(&no_networks) + { + host_addrs.push(subnet.clone().into()); + } + } + external::VpcFirewallRuleHostFilter::Ip(addr) => { + host_addrs.push(addr.clone().into()) + } + external::VpcFirewallRuleHostFilter::IpNet(net) => { + host_addrs.push(net.clone().into()) + } + external::VpcFirewallRuleHostFilter::Vpc(name) => { + for interface in vpc_interfaces + .get(&name) + .unwrap_or(&no_interfaces) + { + host_addrs.push(interface.ip.into()) + } + } + } + } + Some(host_addrs) + } + }; + + let filter_ports = rule + .filter_ports + .as_ref() + .map(|ports| ports.iter().map(|v| v.0.into()).collect()); + + let filter_protocols = + rule.filter_protocols.as_ref().map(|protocols| { + protocols.iter().map(|v| v.0.into()).collect() + }); + + sled_agent_rules.push(sled_agent_client::types::VpcFirewallRule { status: rule.status.0.into(), direction: rule.direction.0.into(), - targets: vec![], // XXX - filter_hosts: rule.filter_hosts.as_ref().map(|hosts| { - hosts - .iter() - .filter_map(|host| match host { - db::model::VpcFirewallRuleHostFilter( - external::VpcFirewallRuleHostFilter::Ip( - std::net::IpAddr::V4(addr), - ), - ) => Some(sled_agent_client::types::IpNet::V4( - external::Ipv4Net( - ipnetwork::Ipv4Network::new( - addr.clone(), - 32, - ) - .unwrap(), - ) - .into(), - )), - db::model::VpcFirewallRuleHostFilter( - external::VpcFirewallRuleHostFilter::IpNet( - external::IpNet::V4(external::Ipv4Net( - network, - )), - ), - ) => Some(sled_agent_client::types::IpNet::V4( - external::Ipv4Net(network.clone()).into(), - )), - _ => None, // XXX - }) - .collect() - }), - filter_ports: rule - .filter_ports - .as_ref() - .map(|ports| ports.iter().map(|v| v.0.into()).collect()), - filter_protocols: rule.filter_protocols.as_ref().map( - |protocols| protocols.iter().map(|v| v.0.into()).collect(), - ), + targets, + filter_hosts, + filter_ports, + filter_protocols, action: rule.action.0.into(), priority: rule.priority.0 .0, - }) - .collect::>()) + }); + } + debug!( + self.log, + "resolved firewall rules for sled agents"; + "sled_agent_rules" => ?sled_agent_rules, + ); + + Ok(sled_agent_rules) } } diff --git a/nexus/src/db/datastore/network_interface.rs b/nexus/src/db/datastore/network_interface.rs index b341cb612fd..c71efa89f73 100644 --- a/nexus/src/db/datastore/network_interface.rs +++ b/nexus/src/db/datastore/network_interface.rs @@ -35,6 +35,39 @@ use omicron_common::api::external::ResourceType; use omicron_common::api::external::UpdateResult; use sled_agent_client::types as sled_client_types; +/// OPTE requires information that's currently split across the network +/// interface and VPC subnet tables. +#[derive(Debug, diesel::Queryable)] +struct NicInfo { + name: db::model::Name, + ip: ipnetwork::IpNetwork, + mac: db::model::MacAddr, + ipv4_block: db::model::Ipv4Net, + ipv6_block: db::model::Ipv6Net, + vni: db::model::Vni, + primary: bool, + slot: i16, +} + +impl From for sled_client_types::NetworkInterface { + fn from(nic: NicInfo) -> sled_client_types::NetworkInterface { + let ip_subnet = if nic.ip.is_ipv4() { + external::IpNet::V4(nic.ipv4_block.0) + } else { + external::IpNet::V6(nic.ipv6_block.0) + }; + sled_client_types::NetworkInterface { + name: sled_client_types::Name::from(&nic.name.0), + ip: nic.ip.ip(), + mac: sled_client_types::MacAddr::from(nic.mac.0), + subnet: sled_client_types::IpNet::from(ip_subnet), + vni: sled_client_types::Vni::from(nic.vni.0), + primary: nic.primary, + slot: u8::try_from(nic.slot).unwrap(), + } + } +} + impl DataStore { /// Create a network interface attached to the provided instance. pub async fn instance_create_network_interface( @@ -147,10 +180,6 @@ impl DataStore { /// Return the information about an instance's network interfaces required /// for the sled agent to instantiate them via OPTE. - /// - /// OPTE requires information that's currently split across the network - /// interface and VPC subnet tables. This query just joins those for each - /// NIC in the given instance. pub(crate) async fn derive_guest_network_interface_info( &self, opctx: &OpContext, @@ -162,38 +191,6 @@ impl DataStore { use db::schema::vpc; use db::schema::vpc_subnet; - // The record type for the results of the below JOIN query - #[derive(Debug, diesel::Queryable)] - struct NicInfo { - name: db::model::Name, - ip: ipnetwork::IpNetwork, - mac: db::model::MacAddr, - ipv4_block: db::model::Ipv4Net, - ipv6_block: db::model::Ipv6Net, - vni: db::model::Vni, - primary: bool, - slot: i16, - } - - impl From for sled_client_types::NetworkInterface { - fn from(nic: NicInfo) -> sled_client_types::NetworkInterface { - let ip_subnet = if nic.ip.is_ipv4() { - external::IpNet::V4(nic.ipv4_block.0) - } else { - external::IpNet::V6(nic.ipv6_block.0) - }; - sled_client_types::NetworkInterface { - name: sled_client_types::Name::from(&nic.name.0), - ip: nic.ip.ip(), - mac: sled_client_types::MacAddr::from(nic.mac.0), - subnet: sled_client_types::IpNet::from(ip_subnet), - vni: sled_client_types::Vni::from(nic.vni.0), - primary: nic.primary, - slot: u8::try_from(nic.slot).unwrap(), - } - } - } - let rows = network_interface::table .filter(network_interface::instance_id.eq(authz_instance.id())) .filter(network_interface::time_deleted.is_null()) @@ -227,6 +224,88 @@ impl DataStore { .collect()) } + /// Return information about all VNICs connected to a VPC required + /// for the sled agent to instantiate firewall rules via OPTE. + pub(crate) async fn derive_vpc_network_interface_info( + &self, + opctx: &OpContext, + authz_vpc: &authz::Vpc, + ) -> ListResultVec { + opctx.authorize(authz::Action::ListChildren, authz_vpc).await?; + + use db::schema::{network_interface, vpc, vpc_subnet}; + let rows = network_interface::table + .filter(network_interface::vpc_id.eq(authz_vpc.id())) + .filter(network_interface::time_deleted.is_null()) + .inner_join( + vpc_subnet::table + .on(network_interface::subnet_id.eq(vpc_subnet::id)), + ) + .inner_join(vpc::table.on(vpc_subnet::vpc_id.eq(vpc::id))) + .order_by(network_interface::slot) + // TODO-cleanup: See DRY comment above. + .select(( + network_interface::name, + network_interface::ip, + network_interface::mac, + vpc_subnet::ipv4_block, + vpc_subnet::ipv6_block, + vpc::vni, + network_interface::is_primary, + network_interface::slot, + )) + .get_results_async::(self.pool_authorized(opctx).await?) + .await + .map_err(|e| { + public_error_from_diesel_pool(e, ErrorHandler::Server) + })?; + Ok(rows + .into_iter() + .map(sled_client_types::NetworkInterface::from) + .collect()) + } + + /// Return information about all VNICs connected to a VpcSubnet required + /// for the sled agent to instantiate firewall rules via OPTE. + pub(crate) async fn derive_subnet_network_interface_info( + &self, + opctx: &OpContext, + authz_subnet: &authz::VpcSubnet, + ) -> ListResultVec { + opctx.authorize(authz::Action::ListChildren, authz_subnet).await?; + + use db::schema::{network_interface, vpc, vpc_subnet}; + let rows = network_interface::table + .filter(network_interface::subnet_id.eq(authz_subnet.id())) + .filter(network_interface::time_deleted.is_null()) + .inner_join( + vpc_subnet::table + .on(network_interface::subnet_id.eq(vpc_subnet::id)), + ) + .inner_join(vpc::table.on(vpc_subnet::vpc_id.eq(vpc::id))) + .order_by(network_interface::slot) + // TODO-cleanup: See DRY comment above. + .select(( + network_interface::name, + network_interface::ip, + network_interface::mac, + vpc_subnet::ipv4_block, + vpc_subnet::ipv6_block, + vpc::vni, + network_interface::is_primary, + network_interface::slot, + )) + .get_results_async::(self.pool_authorized(opctx).await?) + .await + .map_err(|e| { + public_error_from_diesel_pool(e, ErrorHandler::Server) + })?; + Ok(rows + .into_iter() + .map(sled_client_types::NetworkInterface::from) + .collect()) + } + /// List network interfaces associated with a given instance. pub async fn instance_list_network_interfaces( &self, diff --git a/nexus/src/db/datastore/vpc.rs b/nexus/src/db/datastore/vpc.rs index 3afb0dd629f..026f3e35033 100644 --- a/nexus/src/db/datastore/vpc.rs +++ b/nexus/src/db/datastore/vpc.rs @@ -45,6 +45,7 @@ use omicron_common::api::external::ListResultVec; use omicron_common::api::external::LookupType; use omicron_common::api::external::ResourceType; use omicron_common::api::external::UpdateResult; +use std::collections::HashMap; use uuid::Uuid; impl DataStore { @@ -700,4 +701,33 @@ impl DataStore { ) }) } + + /// Identify all subnets in use by each VpcSubnet + pub async fn resolve_subnets_to_ips>( + &self, + vpc: &Vpc, + subnet_names: T, + ) -> Result>, Error> { + use db::schema::vpc_subnet; + let subnets = vpc_subnet::table + .select(VpcSubnet::as_select()) + .filter(vpc_subnet::vpc_id.eq(vpc.id())) + .filter(vpc_subnet::name.eq_any(subnet_names)) + .filter(vpc_subnet::time_deleted.is_null()) + .get_results_async(self.pool()) + .await + .map_err(|e| { + public_error_from_diesel_pool(e, ErrorHandler::Server) + })?; + + let mut result = HashMap::with_capacity(subnets.len()); + for subnet in subnets { + let entry = result + .entry(subnet.name().clone().into()) + .or_insert_with(Vec::new); + entry.push(ipnetwork::IpNetwork::V4(subnet.ipv4_block.0 .0)); + entry.push(ipnetwork::IpNetwork::V6(subnet.ipv6_block.0 .0)); + } + Ok(result) + } } diff --git a/sled-agent-client/Cargo.toml b/sled-agent-client/Cargo.toml index f5e542a8d37..88ab0e31eaa 100644 --- a/sled-agent-client/Cargo.toml +++ b/sled-agent-client/Cargo.toml @@ -6,6 +6,7 @@ license = "MPL-2.0" [dependencies] async-trait = "0.1" +ipnetwork = "0.20" progenitor = { git = "https://github.com/oxidecomputer/progenitor" } regress = "0.4.1" reqwest = { version = "0.11", default-features = false, features = ["rustls-tls", "stream"] } diff --git a/sled-agent-client/src/lib.rs b/sled-agent-client/src/lib.rs index 2a50ebb3122..9f22257d7dc 100644 --- a/sled-agent-client/src/lib.rs +++ b/sled-agent-client/src/lib.rs @@ -235,6 +235,52 @@ impl From for types::IpNet { } } +impl From for types::Ipv4Net { + fn from(n: ipnetwork::Ipv4Network) -> Self { + Self::try_from(n.to_string()).unwrap_or_else(|e| panic!("{}: {}", n, e)) + } +} + +impl From for types::Ipv6Net { + fn from(n: ipnetwork::Ipv6Network) -> Self { + Self::try_from(n.to_string()).unwrap_or_else(|e| panic!("{}: {}", n, e)) + } +} + +impl From for types::IpNet { + fn from(n: ipnetwork::IpNetwork) -> Self { + use ipnetwork::IpNetwork; + match n { + IpNetwork::V4(v4) => Self::V4(v4.into()), + IpNetwork::V6(v6) => Self::V6(v6.into()), + } + } +} + +impl From for types::Ipv4Net { + fn from(n: std::net::Ipv4Addr) -> Self { + Self::try_from(format!("{n}/32")) + .unwrap_or_else(|e| panic!("{}: {}", n, e)) + } +} + +impl From for types::Ipv6Net { + fn from(n: std::net::Ipv6Addr) -> Self { + Self::try_from(format!("{n}/128")) + .unwrap_or_else(|e| panic!("{}: {}", n, e)) + } +} + +impl From for types::IpNet { + fn from(s: std::net::IpAddr) -> Self { + use std::net::IpAddr; + match s { + IpAddr::V4(v4) => Self::V4(v4.into()), + IpAddr::V6(v6) => Self::V6(v6.into()), + } + } +} + impl From for types::L4PortRange { fn from(s: omicron_common::api::external::L4PortRange) -> Self { Self::try_from(s.to_string()).unwrap_or_else(|e| panic!("{}: {}", s, e)) From c5944c5cf15c19ec01e068ca38d0fa3f041083e5 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Mon, 22 Aug 2022 12:38:40 -0600 Subject: [PATCH 08/35] Set firewall rules only on matching targets --- sled-agent/src/instance_manager.rs | 17 ++++---- sled-agent/src/opte/illumos/port_manager.rs | 42 +++++++++++-------- .../src/opte/non_illumos/port_manager.rs | 18 +------- sled-agent/src/sled_agent.rs | 2 - 4 files changed, 33 insertions(+), 46 deletions(-) diff --git a/sled-agent/src/instance_manager.rs b/sled-agent/src/instance_manager.rs index 7e5a3ad7b7d..baf317187df 100644 --- a/sled-agent/src/instance_manager.rs +++ b/sled-agent/src/instance_manager.rs @@ -217,15 +217,14 @@ impl InstanceManager { &self, rules: &[VpcFirewallRule], ) -> Result<(), Error> { - // TODO-correctness: map from VPC to VNICs on instances. Right now - // we just update each port on every instance, which is totally wrong. - for port_name in self.inner.port_manager.port_names() { - info!( - &self.inner.log, - "Updating firewall rules for OPTE port {}", &port_name - ); - self.inner.port_manager.firewall_rules_ensure(port_name, rules)?; - } + info!( + &self.inner.log, + "Ensuring VPC firewall rules"; + "rules" => ?&rules, + ); + self.inner + .port_manager + .firewall_rules_ensure(rules)?; Ok(()) } } diff --git a/sled-agent/src/opte/illumos/port_manager.rs b/sled-agent/src/opte/illumos/port_manager.rs index 7da30f81e2e..a1d177f82f1 100644 --- a/sled-agent/src/opte/illumos/port_manager.rs +++ b/sled-agent/src/opte/illumos/port_manager.rs @@ -447,39 +447,45 @@ impl PortManager { Ok(port) } - pub fn port_names(&self) -> Vec { - self.inner - .ports - .lock() - .unwrap() - .keys() - .map(|(_instance_id, port_name)| port_name.clone()) - .collect::>() - } - pub fn firewall_rules_ensure( &self, - port_name: String, rules: &[VpcFirewallRule], ) -> Result<(), Error> { let hdl = OpteHdl::open(OpteHdl::DLD_CTL)?; - let rules = collect_firewall_rules(rules); - info!(self.inner.log, "OPTE firewall rules"; "rules" => ?&rules,); - hdl.set_fw_rules(&SetFwRulesReq { port_name, rules })?; + for ((_, port_name), port) in self.inner.ports.lock().unwrap().iter() { + let rules = opte_firewall_rules(port, rules); + let port_name = port_name.clone(); + info!( + self.inner.log, + "Setting OPTE firewall rules"; + "port" => ?&port_name, + "rules" => ?&rules, + ); + hdl.set_fw_rules(&SetFwRulesReq { port_name, rules })?; + } Ok(()) } } -/// Translate from a slice of VPC firewall rules to a vector of OPTE rules. -/// OPTE rules can only encode a single host address and protocol, so we must -/// unroll rules with multiple hosts/protocols. -fn collect_firewall_rules(rules: &[VpcFirewallRule]) -> Vec { +/// Translate from a slice of VPC firewall rules to a vector of OPTE rules +/// that match the given port's MAC address. OPTE rules can only encode a +/// single host address and protocol, so we must unroll rules with multiple +/// hosts/protocols. +fn opte_firewall_rules( + port: &Port, + rules: &[VpcFirewallRule], +) -> Vec { rules .iter() .filter(|rule| match rule.status { VpcFirewallRuleStatus::Disabled => false, VpcFirewallRuleStatus::Enabled => true, }) + .filter(|rule| { + // TODO: no targets means apply everywhere, right? + rule.targets.is_empty() + || rule.targets.iter().any(|nic| nic.mac.0 == *port.mac()) + }) .map(|rule| { let priority = rule.priority.0; let action = match rule.action { diff --git a/sled-agent/src/opte/non_illumos/port_manager.rs b/sled-agent/src/opte/non_illumos/port_manager.rs index f929d7091d7..eb2d15fb82d 100644 --- a/sled-agent/src/opte/non_illumos/port_manager.rs +++ b/sled-agent/src/opte/non_illumos/port_manager.rs @@ -184,27 +184,11 @@ impl PortManager { Ok(port) } - pub fn port_names(&self) -> Vec { - self.inner - .ports - .lock() - .unwrap() - .keys() - .map(|(_instance_id, port_name)| port_name.clone()) - .collect::>() - } - pub fn firewall_rules_ensure( &self, - port_name: String, rules: &[VpcFirewallRule], ) -> Result<(), Error> { - info!( - self.inner.log, - "Ignoring {} firewall rules for {}", - rules.len(), - &port_name - ); + info!(self.inner.log, "Ignoring {} firewall rules", rules.len()); Ok(()) } } diff --git a/sled-agent/src/sled_agent.rs b/sled-agent/src/sled_agent.rs index 2f613eec89c..b98aab41836 100644 --- a/sled-agent/src/sled_agent.rs +++ b/sled-agent/src/sled_agent.rs @@ -437,8 +437,6 @@ impl SledAgent { _vpc_id: Uuid, rules: &[VpcFirewallRule], ) -> Result<(), Error> { - // TODO-correctness: map from VPC to VNICs on instances. Right now - // we just update each port on every instance, which is totally wrong. self.instances.firewall_rules_ensure(rules).await.map_err(Error::from) } } From a50ddf0e22fd5eaa7c8cc796c2e3d7f53ecff60e Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Mon, 22 Aug 2022 16:06:59 -0600 Subject: [PATCH 09/35] fmt --- nexus/src/app/vpc.rs | 8 ++++++-- sled-agent/src/instance_manager.rs | 4 +--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/nexus/src/app/vpc.rs b/nexus/src/app/vpc.rs index dc225728df8..14c983f57ed 100644 --- a/nexus/src/app/vpc.rs +++ b/nexus/src/app/vpc.rs @@ -608,8 +608,12 @@ impl super::Nexus { targets.push(interface.clone().into()); } } - external::VpcFirewallRuleTarget::Ip(_addr) => todo!("target addr"), - external::VpcFirewallRuleTarget::IpNet(_net) => todo!("target net"), + external::VpcFirewallRuleTarget::Ip(_addr) => { + todo!("target addr") + } + external::VpcFirewallRuleTarget::IpNet(_net) => { + todo!("target net") + } } } diff --git a/sled-agent/src/instance_manager.rs b/sled-agent/src/instance_manager.rs index baf317187df..53701e0474b 100644 --- a/sled-agent/src/instance_manager.rs +++ b/sled-agent/src/instance_manager.rs @@ -222,9 +222,7 @@ impl InstanceManager { "Ensuring VPC firewall rules"; "rules" => ?&rules, ); - self.inner - .port_manager - .firewall_rules_ensure(rules)?; + self.inner.port_manager.firewall_rules_ensure(rules)?; Ok(()) } } From 0156492b671e6d74c1daaa2fce8467aff14d0194 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Mon, 22 Aug 2022 16:19:33 -0600 Subject: [PATCH 10/35] clippy --- nexus/src/app/vpc.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/nexus/src/app/vpc.rs b/nexus/src/app/vpc.rs index 14c983f57ed..7f663d9762d 100644 --- a/nexus/src/app/vpc.rs +++ b/nexus/src/app/vpc.rs @@ -29,7 +29,7 @@ use omicron_common::api::external::RouterRouteCreateParams; use omicron_common::api::external::RouterRouteKind; use omicron_common::api::external::UpdateResult; use omicron_common::api::external::VpcFirewallRuleUpdateParams; -use sled_agent_client::types::NetworkInterface; +use sled_agent_client::types::{IpNet, NetworkInterface}; use futures::future::join_all; use ipnetwork::IpNetwork; @@ -589,7 +589,7 @@ impl super::Nexus { for interface in vpc_interfaces.get(&name).unwrap_or(&no_interfaces) { - targets.push(interface.clone().into()); + targets.push(interface.clone()); } } external::VpcFirewallRuleTarget::Subnet(name) => { @@ -597,7 +597,7 @@ impl super::Nexus { .get(&name) .unwrap_or(&no_interfaces) { - targets.push(interface.clone().into()); + targets.push(interface.clone()); } } external::VpcFirewallRuleTarget::Instance(name) => { @@ -605,7 +605,7 @@ impl super::Nexus { .get(&name) .unwrap_or(&no_interfaces) { - targets.push(interface.clone().into()); + targets.push(interface.clone()); } } external::VpcFirewallRuleTarget::Ip(_addr) => { @@ -630,7 +630,7 @@ impl super::Nexus { .get(&name) .unwrap_or(&no_interfaces) { - host_addrs.push(interface.ip.into()) + host_addrs.push(IpNet::from(interface.ip)) } } external::VpcFirewallRuleHostFilter::Subnet( @@ -640,21 +640,21 @@ impl super::Nexus { .get(&name) .unwrap_or(&no_networks) { - host_addrs.push(subnet.clone().into()); + host_addrs.push(IpNet::from(*subnet)); } } external::VpcFirewallRuleHostFilter::Ip(addr) => { - host_addrs.push(addr.clone().into()) + host_addrs.push(IpNet::from(*addr)) } external::VpcFirewallRuleHostFilter::IpNet(net) => { - host_addrs.push(net.clone().into()) + host_addrs.push(IpNet::from(*net)) } external::VpcFirewallRuleHostFilter::Vpc(name) => { for interface in vpc_interfaces .get(&name) .unwrap_or(&no_interfaces) { - host_addrs.push(interface.ip.into()) + host_addrs.push(IpNet::from(interface.ip)) } } } From 4241a7e6100b02fcdb4abf413265a26d6e083164 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Tue, 23 Aug 2022 11:42:38 -0600 Subject: [PATCH 11/35] Customize default firewall rules for non-default VPCs --- nexus/src/app/vpc.rs | 30 ++++++++++++++++++- nexus/tests/integration_tests/vpc_firewall.rs | 25 +++++++++------- 2 files changed, 43 insertions(+), 12 deletions(-) diff --git a/nexus/src/app/vpc.rs b/nexus/src/app/vpc.rs index 7f663d9762d..e0bc8d78847 100644 --- a/nexus/src/app/vpc.rs +++ b/nexus/src/app/vpc.rs @@ -173,10 +173,38 @@ impl super::Nexus { } SubnetError::External(e) => e, })?; - let rules = db::model::VpcFirewallRule::vec_from_params( + + // Customize the default firewall rules for this VPC. + let mut rules = db::model::VpcFirewallRule::vec_from_params( authz_vpc.id(), defaults::DEFAULT_FIREWALL_RULES.clone(), ); + for rule in rules.iter_mut() { + for target in rule.targets.iter_mut() { + match target.0 { + external::VpcFirewallRuleTarget::Vpc(ref mut name) + if name.as_str() == "default" => + { + *name = params.identity.name.clone() + } + _ => (), + } + if let Some(ref mut filter_hosts) = rule.filter_hosts { + for host in filter_hosts.iter_mut() { + match host.0 { + external::VpcFirewallRuleHostFilter::Vpc( + ref mut name, + ) if name.as_str() == "default" => { + *name = params.identity.name.clone() + } + _ => (), + } + } + } + } + } + debug!(self.log, "default firewall rules"; "rules" => ?&rules); + self.db_datastore .vpc_update_firewall_rules(opctx, &authz_vpc, rules.clone()) .await?; diff --git a/nexus/tests/integration_tests/vpc_firewall.rs b/nexus/tests/integration_tests/vpc_firewall.rs index e8242de4c23..d8eade9c5c9 100644 --- a/nexus/tests/integration_tests/vpc_firewall.rs +++ b/nexus/tests/integration_tests/vpc_firewall.rs @@ -46,7 +46,7 @@ async fn test_vpc_firewall(cptestctx: &ControlPlaneTestContext) { let default_vpc_firewall = format!("{}/firewall/rules", default_vpc_url); let rules = get_rules(client, &default_vpc_firewall).await; assert!(rules.iter().all(|r| r.vpc_id == default_vpc.identity.id)); - assert!(is_default_firewall_rules(&rules)); + assert!(is_default_firewall_rules("default", &rules)); // Create another VPC and make sure it gets the default rules. let other_vpc = "second-vpc"; @@ -55,7 +55,7 @@ async fn test_vpc_firewall(cptestctx: &ControlPlaneTestContext) { let vpc2 = create_vpc(&client, &org_name, &project_name, &other_vpc).await; let rules = get_rules(client, &other_vpc_firewall).await; assert!(rules.iter().all(|r| r.vpc_id == vpc2.identity.id)); - assert!(is_default_firewall_rules(&rules)); + assert!(is_default_firewall_rules(other_vpc, &rules)); // Modify one VPC's firewall let new_rules = vec![ @@ -106,21 +106,21 @@ async fn test_vpc_firewall(cptestctx: &ControlPlaneTestContext) { .parsed_body::() .unwrap() .rules; - assert!(!is_default_firewall_rules(&updated_rules)); + assert!(!is_default_firewall_rules("default", &updated_rules)); assert_eq!(updated_rules.len(), new_rules.len()); assert_eq!(updated_rules[0].identity.name, "allow-icmp"); assert_eq!(updated_rules[1].identity.name, "deny-all-incoming"); // Make sure the firewall is changed let rules = get_rules(client, &default_vpc_firewall).await; - assert!(!is_default_firewall_rules(&rules)); + assert!(!is_default_firewall_rules("default", &rules)); assert_eq!(rules.len(), new_rules.len()); assert_eq!(rules[0].identity.name, "allow-icmp"); assert_eq!(rules[1].identity.name, "deny-all-incoming"); // Make sure the other firewall is unchanged let rules = get_rules(client, &other_vpc_firewall).await; - assert!(is_default_firewall_rules(&rules)); + assert!(is_default_firewall_rules(other_vpc, &rules)); // DELETE is unsupported NexusRequest::expect_failure( @@ -177,7 +177,10 @@ async fn get_rules( .rules } -fn is_default_firewall_rules(rules: &Vec) -> bool { +fn is_default_firewall_rules( + vpc_name: &str, + rules: &Vec, +) -> bool { let default_rules = vec![ VpcFirewallRule { identity: IdentityMetadata { @@ -191,7 +194,7 @@ fn is_default_firewall_rules(rules: &Vec) -> bool { status: VpcFirewallRuleStatus::Enabled, direction: VpcFirewallRuleDirection::Inbound, targets: vec![VpcFirewallRuleTarget::Vpc( - "default".parse().unwrap(), + vpc_name.parse().unwrap(), )], filters: VpcFirewallRuleFilter { hosts: None, @@ -216,11 +219,11 @@ fn is_default_firewall_rules(rules: &Vec) -> bool { status: VpcFirewallRuleStatus::Enabled, direction: VpcFirewallRuleDirection::Inbound, targets: vec![VpcFirewallRuleTarget::Vpc( - "default".parse().unwrap(), + vpc_name.parse().unwrap(), )], filters: VpcFirewallRuleFilter { hosts: Some(vec![VpcFirewallRuleHostFilter::Vpc( - "default".parse().unwrap(), + vpc_name.parse().unwrap(), )]), protocols: None, ports: None, @@ -242,7 +245,7 @@ fn is_default_firewall_rules(rules: &Vec) -> bool { status: VpcFirewallRuleStatus::Enabled, direction: VpcFirewallRuleDirection::Inbound, targets: vec![VpcFirewallRuleTarget::Vpc( - "default".parse().unwrap(), + vpc_name.parse().unwrap(), )], filters: VpcFirewallRuleFilter { hosts: None, @@ -269,7 +272,7 @@ fn is_default_firewall_rules(rules: &Vec) -> bool { status: VpcFirewallRuleStatus::Enabled, direction: VpcFirewallRuleDirection::Inbound, targets: vec![VpcFirewallRuleTarget::Vpc( - "default".parse().unwrap(), + vpc_name.parse().unwrap(), )], filters: VpcFirewallRuleFilter { hosts: None, From 1dc02d3b0a600fe636bcaf2ccbb9a59e4942a964 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Wed, 7 Sep 2022 10:40:15 -0600 Subject: [PATCH 12/35] Factor out default firewall rule generation Also return an error on default rules we don't understand during the walk. --- nexus/src/app/vpc.rs | 83 +++++++++++++++++++++++++++----------------- 1 file changed, 52 insertions(+), 31 deletions(-) diff --git a/nexus/src/app/vpc.rs b/nexus/src/app/vpc.rs index e0bc8d78847..c2dd83aa5cb 100644 --- a/nexus/src/app/vpc.rs +++ b/nexus/src/app/vpc.rs @@ -174,37 +174,13 @@ impl super::Nexus { SubnetError::External(e) => e, })?; - // Customize the default firewall rules for this VPC. - let mut rules = db::model::VpcFirewallRule::vec_from_params( - authz_vpc.id(), - defaults::DEFAULT_FIREWALL_RULES.clone(), - ); - for rule in rules.iter_mut() { - for target in rule.targets.iter_mut() { - match target.0 { - external::VpcFirewallRuleTarget::Vpc(ref mut name) - if name.as_str() == "default" => - { - *name = params.identity.name.clone() - } - _ => (), - } - if let Some(ref mut filter_hosts) = rule.filter_hosts { - for host in filter_hosts.iter_mut() { - match host.0 { - external::VpcFirewallRuleHostFilter::Vpc( - ref mut name, - ) if name.as_str() == "default" => { - *name = params.identity.name.clone() - } - _ => (), - } - } - } - } - } - debug!(self.log, "default firewall rules"; "rules" => ?&rules); - + // Save and send the default firewall rules for the new VPC. + let rules = self + .default_firewall_rules_for_vpc( + authz_vpc.id(), + params.identity.name.clone().into(), + ) + .await?; self.db_datastore .vpc_update_firewall_rules(opctx, &authz_vpc, rules.clone()) .await?; @@ -216,6 +192,7 @@ impl super::Nexus { &rules, ) .await?; + Ok(db_vpc) } @@ -384,6 +361,50 @@ impl super::Nexus { Ok(rules) } + /// Customize the default firewall rules for a particular VPC. + async fn default_firewall_rules_for_vpc( + &self, + vpc_id: Uuid, + vpc_name: Name, + ) -> Result, Error> { + let mut rules = db::model::VpcFirewallRule::vec_from_params( + vpc_id, + defaults::DEFAULT_FIREWALL_RULES.clone(), + ); + for rule in rules.iter_mut() { + for target in rule.targets.iter_mut() { + match target.0 { + external::VpcFirewallRuleTarget::Vpc(ref mut name) + if name.as_str() == "default" => + { + *name = vpc_name.clone().into() + } + _ => { + return Err(external::Error::internal_error( + "unexpected target in default firewall rule", + )) + } + } + if let Some(ref mut filter_hosts) = rule.filter_hosts { + for host in filter_hosts.iter_mut() { + match host.0 { + external::VpcFirewallRuleHostFilter::Vpc( + ref mut name, + ) if name.as_str() == "default" => { + *name = vpc_name.clone().into() + } + _ => return Err(external::Error::internal_error( + "unexpected host filter in default firewall rule" + )), + } + } + } + } + } + debug!(self.log, "default firewall rules for vpc {}", vpc_name; "rules" => ?&rules); + Ok(rules) + } + async fn send_sled_agents_firewall_rules( &self, opctx: &OpContext, From 1fa97ab832011cc29bb1cffb690fbc7e8a35d806 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Thu, 8 Sep 2022 09:47:52 -0600 Subject: [PATCH 13/35] De-duplicate firewall rule target interfaces --- nexus/src/app/vpc.rs | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/nexus/src/app/vpc.rs b/nexus/src/app/vpc.rs index c2dd83aa5cb..39b4d008729 100644 --- a/nexus/src/app/vpc.rs +++ b/nexus/src/app/vpc.rs @@ -631,31 +631,39 @@ impl super::Nexus { // firewall rules (including default rules) before instances // and their interfaces are instantiated. + // Collect unique network interface targets. + // This would be easier if `NetworkInterface` were `Hash`, + // but that's not easy because it's a generated type. We + // use the pair (VNI, MAC) as a unique interface identifier. + let mut nics = HashSet::new(); let mut targets = Vec::with_capacity(rule.targets.len()); + let mut push_target_nic = |nic: &NetworkInterface| { + if nics.insert(((*nic.vni).clone(), (*nic.mac).clone())) { + targets.push(nic.clone()); + } + }; for target in &rule.targets { match &target.0 { external::VpcFirewallRuleTarget::Vpc(name) => { - for interface in - vpc_interfaces.get(&name).unwrap_or(&no_interfaces) - { - targets.push(interface.clone()); - } + vpc_interfaces + .get(&name) + .unwrap_or(&no_interfaces) + .iter() + .for_each(&mut push_target_nic); } external::VpcFirewallRuleTarget::Subnet(name) => { - for interface in subnet_interfaces + subnet_interfaces .get(&name) .unwrap_or(&no_interfaces) - { - targets.push(interface.clone()); - } + .iter() + .for_each(&mut push_target_nic); } external::VpcFirewallRuleTarget::Instance(name) => { - for interface in instance_interfaces + instance_interfaces .get(&name) .unwrap_or(&no_interfaces) - { - targets.push(interface.clone()); - } + .iter() + .for_each(&mut push_target_nic); } external::VpcFirewallRuleTarget::Ip(_addr) => { todo!("target addr") From 1994a43d8ca2fcb7d4dc33706da8baa3449ef652 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Thu, 8 Sep 2022 10:06:35 -0600 Subject: [PATCH 14/35] Match OPTE ports to NICs by (VNI, MAC) pair --- sled-agent/src/opte/illumos/port.rs | 8 ++++++-- sled-agent/src/opte/illumos/port_manager.rs | 9 ++++++--- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/sled-agent/src/opte/illumos/port.rs b/sled-agent/src/opte/illumos/port.rs index efc15984bc8..33c65a3c401 100644 --- a/sled-agent/src/opte/illumos/port.rs +++ b/sled-agent/src/opte/illumos/port.rs @@ -31,7 +31,7 @@ struct PortInner { // Emulated PCI slot for the guest NIC, passed to Propolis slot: u8, // Geneve VNI for the VPC - _vni: Vni, + vni: Vni, // IP address of the hosting sled _underlay_ip: Ipv6Addr, // The external IP address and port range provided for this port, to allow @@ -127,7 +127,7 @@ impl Port { _subnet: subnet, mac, slot, - _vni: vni, + vni: vni, _underlay_ip: underlay_ip, source_nat, external_ips, @@ -150,6 +150,10 @@ impl Port { &self.inner.mac } + pub fn vni(&self) -> &Vni { + &self.inner.vni + } + pub fn vnic_name(&self) -> &str { &self.inner.vnic } diff --git a/sled-agent/src/opte/illumos/port_manager.rs b/sled-agent/src/opte/illumos/port_manager.rs index a1d177f82f1..0311acc569a 100644 --- a/sled-agent/src/opte/illumos/port_manager.rs +++ b/sled-agent/src/opte/illumos/port_manager.rs @@ -482,9 +482,12 @@ fn opte_firewall_rules( VpcFirewallRuleStatus::Enabled => true, }) .filter(|rule| { - // TODO: no targets means apply everywhere, right? - rule.targets.is_empty() - || rule.targets.iter().any(|nic| nic.mac.0 == *port.mac()) + rule.targets.is_empty() // no targets means apply everywhere + || rule.targets.iter().any(|nic| { + // (VNI, MAC) is a unique identifier for the NIC. + u32::from(nic.vni) == u32::from(*port.vni()) + && nic.mac.0 == *port.mac() + }) }) .map(|rule| { let priority = rule.priority.0; From 578a2d9fbcbace5f9e4840c1673fa873fe666c00 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Thu, 8 Sep 2022 15:18:37 -0600 Subject: [PATCH 15/35] Select just the columns needed in resolve_subnets_to_ips --- nexus/src/db/datastore/vpc.rs | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/nexus/src/db/datastore/vpc.rs b/nexus/src/db/datastore/vpc.rs index 026f3e35033..3f9ddeff965 100644 --- a/nexus/src/db/datastore/vpc.rs +++ b/nexus/src/db/datastore/vpc.rs @@ -29,6 +29,7 @@ use crate::db::model::VpcRouterUpdate; use crate::db::model::VpcSubnet; use crate::db::model::VpcSubnetUpdate; use crate::db::model::VpcUpdate; +use crate::db::model::{Ipv4Net, Ipv6Net}; use crate::db::pagination::paginated; use crate::db::queries::vpc::InsertVpcQuery; use crate::db::queries::vpc_subnet::FilterConflictingVpcSubnetRangesQuery; @@ -37,6 +38,7 @@ use async_bb8_diesel::AsyncConnection; use async_bb8_diesel::AsyncRunQueryDsl; use chrono::Utc; use diesel::prelude::*; +use ipnetwork::IpNetwork; use omicron_common::api::external::CreateResult; use omicron_common::api::external::DataPageParams; use omicron_common::api::external::DeleteResult; @@ -707,14 +709,25 @@ impl DataStore { &self, vpc: &Vpc, subnet_names: T, - ) -> Result>, Error> { + ) -> Result>, Error> { + #[derive(diesel::Queryable)] + struct SubnetIps { + name: Name, + ipv4_block: Ipv4Net, + ipv6_block: Ipv6Net, + } + use db::schema::vpc_subnet; let subnets = vpc_subnet::table - .select(VpcSubnet::as_select()) .filter(vpc_subnet::vpc_id.eq(vpc.id())) .filter(vpc_subnet::name.eq_any(subnet_names)) .filter(vpc_subnet::time_deleted.is_null()) - .get_results_async(self.pool()) + .select(( + vpc_subnet::name, + vpc_subnet::ipv4_block, + vpc_subnet::ipv6_block, + )) + .get_results_async::(self.pool()) .await .map_err(|e| { public_error_from_diesel_pool(e, ErrorHandler::Server) @@ -722,11 +735,9 @@ impl DataStore { let mut result = HashMap::with_capacity(subnets.len()); for subnet in subnets { - let entry = result - .entry(subnet.name().clone().into()) - .or_insert_with(Vec::new); - entry.push(ipnetwork::IpNetwork::V4(subnet.ipv4_block.0 .0)); - entry.push(ipnetwork::IpNetwork::V6(subnet.ipv6_block.0 .0)); + let entry = result.entry(subnet.name).or_insert_with(Vec::new); + entry.push(IpNetwork::V4(subnet.ipv4_block.0 .0)); + entry.push(IpNetwork::V6(subnet.ipv6_block.0 .0)); } Ok(result) } From 2c70fd8d2e2277dfb4299d4959458f5ab2cd8b3d Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Thu, 8 Sep 2022 20:11:55 -0600 Subject: [PATCH 16/35] clippy --- nexus/src/app/vpc.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nexus/src/app/vpc.rs b/nexus/src/app/vpc.rs index 39b4d008729..d6df4c7f5e6 100644 --- a/nexus/src/app/vpc.rs +++ b/nexus/src/app/vpc.rs @@ -638,7 +638,7 @@ impl super::Nexus { let mut nics = HashSet::new(); let mut targets = Vec::with_capacity(rule.targets.len()); let mut push_target_nic = |nic: &NetworkInterface| { - if nics.insert(((*nic.vni).clone(), (*nic.mac).clone())) { + if nics.insert((*nic.vni, (*nic.mac).clone())) { targets.push(nic.clone()); } }; From 902e6a84632cdcf31f64820dba297b293c19706f Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Fri, 9 Sep 2022 17:56:56 -0600 Subject: [PATCH 17/35] Factor derive_*_network_interface_info routines --- nexus/src/db/datastore/network_interface.rs | 111 ++++++++------------ 1 file changed, 43 insertions(+), 68 deletions(-) diff --git a/nexus/src/db/datastore/network_interface.rs b/nexus/src/db/datastore/network_interface.rs index c71efa89f73..3cebcd16059 100644 --- a/nexus/src/db/datastore/network_interface.rs +++ b/nexus/src/db/datastore/network_interface.rs @@ -10,6 +10,7 @@ use crate::context::OpContext; use crate::db; use crate::db::collection_insert::AsyncInsertError; use crate::db::collection_insert::DatastoreCollection; +use crate::db::cte_utils::BoxedQuery; use crate::db::error::public_error_from_diesel_pool; use crate::db::error::ErrorHandler; use crate::db::error::TransactionError; @@ -178,21 +179,20 @@ impl DataStore { Ok(()) } - /// Return the information about an instance's network interfaces required - /// for the sled agent to instantiate them via OPTE. - pub(crate) async fn derive_guest_network_interface_info( + /// Return information about network interfaces required for the sled + /// agent to instantiate or modify them via OPTE. This function takes + /// a partially constructed query over the network interface table so + /// that we can use it for instances, VPCs, and subnets. + async fn derive_network_interface_info( &self, opctx: &OpContext, - authz_instance: &authz::Instance, + partial_query: BoxedQuery, ) -> ListResultVec { - opctx.authorize(authz::Action::ListChildren, authz_instance).await?; - use db::schema::network_interface; use db::schema::vpc; use db::schema::vpc_subnet; - let rows = network_interface::table - .filter(network_interface::instance_id.eq(authz_instance.id())) + let rows = partial_query .filter(network_interface::time_deleted.is_null()) .inner_join( vpc_subnet::table @@ -224,6 +224,25 @@ impl DataStore { .collect()) } + /// Return the information about an instance's network interfaces required + /// for the sled agent to instantiate them via OPTE. + pub(crate) async fn derive_guest_network_interface_info( + &self, + opctx: &OpContext, + authz_instance: &authz::Instance, + ) -> ListResultVec { + opctx.authorize(authz::Action::ListChildren, authz_instance).await?; + + use db::schema::network_interface; + self.derive_network_interface_info( + opctx, + network_interface::table + .filter(network_interface::instance_id.eq(authz_instance.id())) + .into_boxed(), + ) + .await + } + /// Return information about all VNICs connected to a VPC required /// for the sled agent to instantiate firewall rules via OPTE. pub(crate) async fn derive_vpc_network_interface_info( @@ -233,36 +252,14 @@ impl DataStore { ) -> ListResultVec { opctx.authorize(authz::Action::ListChildren, authz_vpc).await?; - use db::schema::{network_interface, vpc, vpc_subnet}; - let rows = network_interface::table - .filter(network_interface::vpc_id.eq(authz_vpc.id())) - .filter(network_interface::time_deleted.is_null()) - .inner_join( - vpc_subnet::table - .on(network_interface::subnet_id.eq(vpc_subnet::id)), - ) - .inner_join(vpc::table.on(vpc_subnet::vpc_id.eq(vpc::id))) - .order_by(network_interface::slot) - // TODO-cleanup: See DRY comment above. - .select(( - network_interface::name, - network_interface::ip, - network_interface::mac, - vpc_subnet::ipv4_block, - vpc_subnet::ipv6_block, - vpc::vni, - network_interface::is_primary, - network_interface::slot, - )) - .get_results_async::(self.pool_authorized(opctx).await?) - .await - .map_err(|e| { - public_error_from_diesel_pool(e, ErrorHandler::Server) - })?; - Ok(rows - .into_iter() - .map(sled_client_types::NetworkInterface::from) - .collect()) + use db::schema::network_interface; + self.derive_network_interface_info( + opctx, + network_interface::table + .filter(network_interface::vpc_id.eq(authz_vpc.id())) + .into_boxed(), + ) + .await } /// Return information about all VNICs connected to a VpcSubnet required @@ -274,36 +271,14 @@ impl DataStore { ) -> ListResultVec { opctx.authorize(authz::Action::ListChildren, authz_subnet).await?; - use db::schema::{network_interface, vpc, vpc_subnet}; - let rows = network_interface::table - .filter(network_interface::subnet_id.eq(authz_subnet.id())) - .filter(network_interface::time_deleted.is_null()) - .inner_join( - vpc_subnet::table - .on(network_interface::subnet_id.eq(vpc_subnet::id)), - ) - .inner_join(vpc::table.on(vpc_subnet::vpc_id.eq(vpc::id))) - .order_by(network_interface::slot) - // TODO-cleanup: See DRY comment above. - .select(( - network_interface::name, - network_interface::ip, - network_interface::mac, - vpc_subnet::ipv4_block, - vpc_subnet::ipv6_block, - vpc::vni, - network_interface::is_primary, - network_interface::slot, - )) - .get_results_async::(self.pool_authorized(opctx).await?) - .await - .map_err(|e| { - public_error_from_diesel_pool(e, ErrorHandler::Server) - })?; - Ok(rows - .into_iter() - .map(sled_client_types::NetworkInterface::from) - .collect()) + use db::schema::network_interface; + self.derive_network_interface_info( + opctx, + network_interface::table + .filter(network_interface::subnet_id.eq(authz_subnet.id())) + .into_boxed(), + ) + .await } /// List network interfaces associated with a given instance. From a1f17cf3460ad42dd50de80af39878406fe05094 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Fri, 23 Sep 2022 09:56:16 -0600 Subject: [PATCH 18/35] Send firewall rules to sled-agent on instance creation --- nexus/src/app/instance.rs | 28 +++++++++++++++++ nexus/src/app/sagas/instance_migrate.rs | 2 ++ nexus/src/app/vpc.rs | 41 +++++-------------------- nexus/src/db/datastore/vpc.rs | 26 ++++++++++++++++ openapi/sled-agent.json | 7 +++++ sled-agent/src/instance.rs | 1 + sled-agent/src/instance_manager.rs | 1 + sled-agent/src/params.rs | 1 + 8 files changed, 73 insertions(+), 34 deletions(-) diff --git a/nexus/src/app/instance.rs b/nexus/src/app/instance.rs index ed2417d8a3a..518e8d4bb99 100644 --- a/nexus/src/app/instance.rs +++ b/nexus/src/app/instance.rs @@ -30,6 +30,7 @@ use omicron_common::api::external::InternalContext; use omicron_common::api::external::ListResultVec; use omicron_common::api::external::LookupResult; use omicron_common::api::external::UpdateResult; +use omicron_common::api::external::Vni; use omicron_common::api::internal::nexus; use sled_agent_client::types::InstanceRuntimeStateMigrateParams; use sled_agent_client::types::InstanceRuntimeStateRequested; @@ -556,6 +557,32 @@ impl super::Nexus { let source_nat = SourceNatConfig::from(snat_ip.into_iter().next().unwrap()); + // Gather the firewall rules for the VPC this instance is in. + // The NIC info we gathered above doesn't have VPC information + // because the sled agent doesn't care about that directly, + // so we fetch it via the first interface's VNI. (It doesn't + // matter which one we use because all NICs must be in the + // same VPC; see the check in project_create_instance.) + let firewall_rules = if let Some(nic) = nics.first() { + let vni = Vni::try_from(nic.vni.0)?; + let vpc = self + .db_datastore + .resolve_vni_to_vpc(opctx, db::model::Vni(vni)) + .await?; + let (.., authz_vpc) = LookupPath::new(opctx, &self.db_datastore) + .vpc_id(vpc.id()) + .lookup_for(authz::Action::Read) + .await?; + let rules = self + .db_datastore + .vpc_list_firewall_rules(opctx, &authz_vpc) + .await?; + self.resolve_firewall_rules_for_sled_agent(opctx, &vpc, &rules) + .await? + } else { + vec![] + }; + // Gather the SSH public keys of the actor make the request so // that they may be injected into the new image via cloud-init. // TODO-security: this should be replaced with a lookup based on @@ -596,6 +623,7 @@ impl super::Nexus { nics, source_nat, external_ips, + firewall_rules, disks: disk_reqs, cloud_init_bytes: Some(base64::encode( db_instance.generate_cidata(&public_keys)?, diff --git a/nexus/src/app/sagas/instance_migrate.rs b/nexus/src/app/sagas/instance_migrate.rs index 373c1585f76..f07f0c1b19d 100644 --- a/nexus/src/app/sagas/instance_migrate.rs +++ b/nexus/src/app/sagas/instance_migrate.rs @@ -227,6 +227,8 @@ async fn sim_instance_migrate( nics: vec![], source_nat, external_ips, + // TODO: populate firewall rules + firewall_rules: vec![], // TODO: populate disks disks: vec![], // TODO: populate cloud init bytes diff --git a/nexus/src/app/vpc.rs b/nexus/src/app/vpc.rs index d6df4c7f5e6..a864970d5fa 100644 --- a/nexus/src/app/vpc.rs +++ b/nexus/src/app/vpc.rs @@ -184,14 +184,7 @@ impl super::Nexus { self.db_datastore .vpc_update_firewall_rules(opctx, &authz_vpc, rules.clone()) .await?; - self.send_sled_agents_firewall_rules( - opctx, - organization_name, - project_name, - &db_vpc, - &rules, - ) - .await?; + self.send_sled_agents_firewall_rules(opctx, &db_vpc, &rules).await?; Ok(db_vpc) } @@ -350,14 +343,7 @@ impl super::Nexus { .db_datastore .vpc_update_firewall_rules(opctx, &authz_vpc, rules) .await?; - self.send_sled_agents_firewall_rules( - opctx, - organization_name, - project_name, - &db_vpc, - &rules, - ) - .await?; + self.send_sled_agents_firewall_rules(opctx, &db_vpc, &rules).await?; Ok(rules) } @@ -408,19 +394,11 @@ impl super::Nexus { async fn send_sled_agents_firewall_rules( &self, opctx: &OpContext, - organization_name: &Name, - project_name: &Name, vpc: &db::model::Vpc, rules: &[db::model::VpcFirewallRule], ) -> Result<(), Error> { let rules_for_sled = self - .resolve_firewall_rules_for_sled_agent( - opctx, - organization_name, - project_name, - &vpc, - rules, - ) + .resolve_firewall_rules_for_sled_agent(opctx, &vpc, rules) .await?; debug!(self.log, "resolved {} rules for sleds", rules_for_sled.len()); let sled_rules_request = @@ -467,11 +445,9 @@ impl super::Nexus { Ok(()) } - async fn resolve_firewall_rules_for_sled_agent( + pub(crate) async fn resolve_firewall_rules_for_sled_agent( &self, opctx: &OpContext, - organization_name: &Name, - project_name: &Name, vpc: &db::model::Vpc, rules: &[db::model::VpcFirewallRule], ) -> Result, Error> { @@ -541,8 +517,7 @@ impl super::Nexus { for instance_name in &instances { let (.., authz_instance) = LookupPath::new(opctx, &self.db_datastore) - .organization_name(organization_name) - .project_name(project_name) + .project_id(vpc.project_id) .instance_name(instance_name) .lookup_for(authz::Action::ListChildren) .await?; @@ -561,8 +536,7 @@ impl super::Nexus { let mut vpc_interfaces: NicMap = HashMap::new(); for vpc_name in &vpcs { let (.., authz_vpc) = LookupPath::new(opctx, &self.db_datastore) - .organization_name(organization_name) - .project_name(project_name) + .project_id(vpc.project_id) .vpc_name(vpc_name) .lookup_for(authz::Action::ListChildren) .await?; @@ -581,8 +555,7 @@ impl super::Nexus { let mut subnet_interfaces: NicMap = HashMap::new(); for subnet_name in &subnets { let (.., authz_subnet) = LookupPath::new(opctx, &self.db_datastore) - .organization_name(organization_name) - .project_name(project_name) + .project_id(vpc.project_id) .vpc_name(&Name::from(vpc.name().clone())) .vpc_subnet_name(subnet_name) .lookup_for(authz::Action::ListChildren) diff --git a/nexus/src/db/datastore/vpc.rs b/nexus/src/db/datastore/vpc.rs index 7734f89c58f..960b844f915 100644 --- a/nexus/src/db/datastore/vpc.rs +++ b/nexus/src/db/datastore/vpc.rs @@ -21,6 +21,7 @@ use crate::db::model::NetworkInterface; use crate::db::model::RouterRoute; use crate::db::model::RouterRouteUpdate; use crate::db::model::Sled; +use crate::db::model::Vni; use crate::db::model::Vpc; use crate::db::model::VpcFirewallRule; use crate::db::model::VpcRouter; @@ -43,6 +44,7 @@ use omicron_common::api::external::DataPageParams; use omicron_common::api::external::DeleteResult; use omicron_common::api::external::Error; use omicron_common::api::external::ListResultVec; +use omicron_common::api::external::LookupResult; use omicron_common::api::external::LookupType; use omicron_common::api::external::ResourceType; use omicron_common::api::external::UpdateResult; @@ -741,4 +743,28 @@ impl DataStore { } Ok(result) } + + /// Look up a VPC by VNI. + pub async fn resolve_vni_to_vpc( + &self, + opctx: &OpContext, + vni: Vni, + ) -> LookupResult { + use db::schema::vpc::dsl; + dsl::vpc + .filter(dsl::vni.eq(vni)) + .filter(dsl::time_deleted.is_null()) + .select(Vpc::as_select()) + .get_result_async(self.pool_authorized(opctx).await?) + .await + .map_err(|e| { + public_error_from_diesel_pool( + e, + ErrorHandler::NotFoundByLookup( + ResourceType::Vpc, + LookupType::ByCompositeId("VNI".to_string()), + ), + ) + }) + } } diff --git a/openapi/sled-agent.json b/openapi/sled-agent.json index 42a7590bc6e..993a9c5529f 100644 --- a/openapi/sled-agent.json +++ b/openapi/sled-agent.json @@ -952,6 +952,12 @@ "format": "ip" } }, + "firewall_rules": { + "type": "array", + "items": { + "$ref": "#/components/schemas/VpcFirewallRule" + } + }, "nics": { "type": "array", "items": { @@ -968,6 +974,7 @@ "required": [ "disks", "external_ips", + "firewall_rules", "nics", "runtime", "source_nat" diff --git a/sled-agent/src/instance.rs b/sled-agent/src/instance.rs index e40d3e94b42..e1aa93d9120 100644 --- a/sled-agent/src/instance.rs +++ b/sled-agent/src/instance.rs @@ -896,6 +896,7 @@ mod test { last_port: 16_384, }, external_ips: vec![], + firewall_rules: vec![], disks: vec![], cloud_init_bytes: None, } diff --git a/sled-agent/src/instance_manager.rs b/sled-agent/src/instance_manager.rs index 53701e0474b..1d5f7420800 100644 --- a/sled-agent/src/instance_manager.rs +++ b/sled-agent/src/instance_manager.rs @@ -303,6 +303,7 @@ mod test { last_port: 1 << 14 - 1, }, external_ips: vec![], + firewall_rules: vec![], disks: vec![], cloud_init_bytes: None, } diff --git a/sled-agent/src/params.rs b/sled-agent/src/params.rs index 742d8f2c836..4daaa399082 100644 --- a/sled-agent/src/params.rs +++ b/sled-agent/src/params.rs @@ -101,6 +101,7 @@ pub struct InstanceHardware { /// Zero or more external IP addresses (either floating or ephemeral), /// provided to an instance to allow inbound connectivity. pub external_ips: Vec, + pub firewall_rules: Vec, pub disks: Vec, pub cloud_init_bytes: Option, } From 41f61ca60e1b497133865e4fd284e7d5061c492e Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Fri, 23 Sep 2022 13:34:37 -0600 Subject: [PATCH 19/35] Setup firewall rules on OPTE port creation --- sled-agent/src/instance.rs | 4 ++++ sled-agent/src/opte/illumos/port_manager.rs | 22 +++++++++++++++---- .../src/opte/non_illumos/port_manager.rs | 1 + 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/sled-agent/src/instance.rs b/sled-agent/src/instance.rs index e1aa93d9120..7482c5fa7f6 100644 --- a/sled-agent/src/instance.rs +++ b/sled-agent/src/instance.rs @@ -18,6 +18,7 @@ use crate::opte::PortManager; use crate::opte::PortTicket; use crate::params::NetworkInterface; use crate::params::SourceNatConfig; +use crate::params::VpcFirewallRule; use crate::params::{ InstanceHardware, InstanceMigrateParams, InstanceRuntimeStateRequested, InstanceSerialConsoleData, @@ -225,6 +226,7 @@ struct InstanceInner { requested_nics: Vec, source_nat: SourceNatConfig, external_ips: Vec, + firewall_rules: Vec, // Disk related properties requested_disks: Vec, @@ -492,6 +494,7 @@ impl Instance { requested_nics: initial.nics, source_nat: initial.source_nat, external_ips: initial.external_ips, + firewall_rules: initial.firewall_rules, requested_disks: initial.disks, cloud_init_bytes: initial.cloud_init_bytes, state: InstanceStates::new(initial.runtime), @@ -541,6 +544,7 @@ impl Instance { nic, snat, external_ips, + &inner.firewall_rules, )?; opte_ports.push(port); port_tickets.push(port_ticket); diff --git a/sled-agent/src/opte/illumos/port_manager.rs b/sled-agent/src/opte/illumos/port_manager.rs index be39ffa11a7..1c7e2f98eb2 100644 --- a/sled-agent/src/opte/illumos/port_manager.rs +++ b/sled-agent/src/opte/illumos/port_manager.rs @@ -184,6 +184,7 @@ impl PortManager { nic: &NetworkInterface, source_nat: Option, external_ips: Option>, + firewall_rules: &[VpcFirewallRule], ) -> Result<(Port, PortTicket), Error> { // TODO-completess: Remove IPv4 restrictions once OPTE supports virtual // IPv6 networks. @@ -299,6 +300,19 @@ impl PortManager { "port_name" => &port_name, ); + // Initialize firewall rules for the new port. + { + let port_name = port_name.clone(); + let rules = opte_firewall_rules(firewall_rules, &vni, &mac); + debug!( + self.inner.log, + "Setting firewall rules"; + "port_name" => ?&port_name, + "rules" => ?&rules, + ); + hdl.set_fw_rules(&SetFwRulesReq { port_name, rules })?; + } + // Create a VNIC on top of this device, to hook Viona into. // // Viona is the illumos MAC provider that implements the VIRTIO @@ -455,7 +469,7 @@ impl PortManager { ) -> Result<(), Error> { let hdl = OpteHdl::open(OpteHdl::DLD_CTL)?; for ((_, port_name), port) in self.inner.ports.lock().unwrap().iter() { - let rules = opte_firewall_rules(port, rules); + let rules = opte_firewall_rules(rules, port.vni(), port.mac()); let port_name = port_name.clone(); info!( self.inner.log, @@ -474,8 +488,9 @@ impl PortManager { /// single host address and protocol, so we must unroll rules with multiple /// hosts/protocols. fn opte_firewall_rules( - port: &Port, rules: &[VpcFirewallRule], + vni: &Vni, + mac: &MacAddr6, ) -> Vec { rules .iter() @@ -487,8 +502,7 @@ fn opte_firewall_rules( rule.targets.is_empty() // no targets means apply everywhere || rule.targets.iter().any(|nic| { // (VNI, MAC) is a unique identifier for the NIC. - u32::from(nic.vni) == u32::from(*port.vni()) - && nic.mac.0 == *port.mac() + u32::from(nic.vni) == u32::from(*vni) && nic.mac.0 == *mac }) }) .map(|rule| { diff --git a/sled-agent/src/opte/non_illumos/port_manager.rs b/sled-agent/src/opte/non_illumos/port_manager.rs index c5fde4e9af2..af615ed0a9b 100644 --- a/sled-agent/src/opte/non_illumos/port_manager.rs +++ b/sled-agent/src/opte/non_illumos/port_manager.rs @@ -112,6 +112,7 @@ impl PortManager { nic: &NetworkInterface, source_nat: Option, external_ips: Option>, + _firewall_rules: &[VpcFirewallRule], ) -> Result<(Port, PortTicket), Error> { // TODO-completess: Remove IPv4 restrictions once OPTE supports virtual // IPv6 networks. From 5e1d7a8c09d161ee255b5ea03da2b21c4ac914f6 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Wed, 28 Sep 2022 14:29:20 -0600 Subject: [PATCH 20/35] Ip(Net) targets for firewall rules --- nexus/src/app/vpc.rs | 37 ++++++++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/nexus/src/app/vpc.rs b/nexus/src/app/vpc.rs index a864970d5fa..49c94afff5b 100644 --- a/nexus/src/app/vpc.rs +++ b/nexus/src/app/vpc.rs @@ -475,9 +475,10 @@ impl super::Nexus { } vpcs.insert(name.clone().into()); } - // We don't need to resolve anything for Ip(Net)s. - external::VpcFirewallRuleTarget::Ip(_) => (), - external::VpcFirewallRuleTarget::IpNet(_) => (), + external::VpcFirewallRuleTarget::Ip(_) + | external::VpcFirewallRuleTarget::IpNet(_) => { + vpcs.insert(vpc.name().clone().into()); + } } } @@ -638,11 +639,33 @@ impl super::Nexus { .iter() .for_each(&mut push_target_nic); } - external::VpcFirewallRuleTarget::Ip(_addr) => { - todo!("target addr") + external::VpcFirewallRuleTarget::Ip(addr) => { + vpc_interfaces + .get(vpc.name()) + .unwrap_or(&no_interfaces) + .iter() + .filter(|nic| nic.ip == *addr) + .for_each(&mut push_target_nic); } - external::VpcFirewallRuleTarget::IpNet(_net) => { - todo!("target net") + external::VpcFirewallRuleTarget::IpNet(net) => { + vpc_interfaces + .get(vpc.name()) + .unwrap_or(&no_interfaces) + .iter() + .filter(|nic| { + use external::IpNet; + use std::net::IpAddr; + match (IpNet::from(*net), IpAddr::from(nic.ip)) { + (IpNet::V4(net), IpAddr::V4(ip)) => { + net.contains(ip) + } + (IpNet::V6(net), IpAddr::V6(ip)) => { + net.contains(ip) + } + (_, _) => false, + } + }) + .for_each(&mut push_target_nic); } } } From ab86911c6ea166f3c4e0355e94ab248ae1a2210d Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Wed, 28 Sep 2022 17:47:36 -0600 Subject: [PATCH 21/35] Skip firewall rules with unresolved target/host filters --- nexus/src/app/vpc.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/nexus/src/app/vpc.rs b/nexus/src/app/vpc.rs index 49c94afff5b..2ad6ee9b728 100644 --- a/nexus/src/app/vpc.rs +++ b/nexus/src/app/vpc.rs @@ -669,6 +669,10 @@ impl super::Nexus { } } } + if !rule.targets.is_empty() && targets.is_empty() { + // Target not found; skip this rule. + continue; + } let filter_hosts = match &rule.filter_hosts { None => None, @@ -712,6 +716,10 @@ impl super::Nexus { } } } + if !hosts.is_empty() && host_addrs.is_empty() { + // Filter host not found; skip this rule. + continue; + } Some(host_addrs) } }; From 3433bcdcfacce98d425c7fa6768820004d21a668 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Wed, 28 Sep 2022 18:13:35 -0600 Subject: [PATCH 22/35] fmt --- nexus/src/app/vpc.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/nexus/src/app/vpc.rs b/nexus/src/app/vpc.rs index 2ad6ee9b728..4c7f0aea5ef 100644 --- a/nexus/src/app/vpc.rs +++ b/nexus/src/app/vpc.rs @@ -655,7 +655,8 @@ impl super::Nexus { .filter(|nic| { use external::IpNet; use std::net::IpAddr; - match (IpNet::from(*net), IpAddr::from(nic.ip)) { + match (IpNet::from(*net), IpAddr::from(nic.ip)) + { (IpNet::V4(net), IpAddr::V4(ip)) => { net.contains(ip) } From cb0dc621a24a2fa7732baf9a61fe3e749bbc10ea Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Thu, 29 Sep 2022 11:11:07 -0600 Subject: [PATCH 23/35] clippy --- nexus/src/app/vpc.rs | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/nexus/src/app/vpc.rs b/nexus/src/app/vpc.rs index 4c7f0aea5ef..30672bbaa86 100644 --- a/nexus/src/app/vpc.rs +++ b/nexus/src/app/vpc.rs @@ -34,6 +34,7 @@ use sled_agent_client::types::{IpNet, NetworkInterface}; use futures::future::join_all; use ipnetwork::IpNetwork; use std::collections::{HashMap, HashSet}; +use std::net::IpAddr; use uuid::Uuid; impl super::Nexus { @@ -652,19 +653,14 @@ impl super::Nexus { .get(vpc.name()) .unwrap_or(&no_interfaces) .iter() - .filter(|nic| { - use external::IpNet; - use std::net::IpAddr; - match (IpNet::from(*net), IpAddr::from(nic.ip)) - { - (IpNet::V4(net), IpAddr::V4(ip)) => { - net.contains(ip) - } - (IpNet::V6(net), IpAddr::V6(ip)) => { - net.contains(ip) - } - (_, _) => false, + .filter(|nic| match (net, nic.ip) { + (external::IpNet::V4(net), IpAddr::V4(ip)) => { + net.contains(ip) + } + (external::IpNet::V6(net), IpAddr::V6(ip)) => { + net.contains(ip) } + (_, _) => false, }) .for_each(&mut push_target_nic); } From f0dc4ddb5b3a86cc31a2b19a500ad9152777fac6 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Tue, 4 Oct 2022 14:41:26 -0600 Subject: [PATCH 24/35] Link to tracking issue for missing instance migration info --- nexus/src/app/sagas/instance_migrate.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/nexus/src/app/sagas/instance_migrate.rs b/nexus/src/app/sagas/instance_migrate.rs index f07f0c1b19d..eac1c8e6e2d 100644 --- a/nexus/src/app/sagas/instance_migrate.rs +++ b/nexus/src/app/sagas/instance_migrate.rs @@ -221,6 +221,8 @@ async fn sim_instance_migrate( } let source_nat = SourceNatConfig::from(snat_ip.into_iter().next().unwrap()); + // The TODOs below are tracked in + // https://github.com/oxidecomputer/omicron/issues/1783 let instance_hardware = InstanceHardware { runtime: runtime.into(), // TODO: populate NICs From f3b64ae0a55003b25a45743cd64a750d9b8836af Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Tue, 4 Oct 2022 14:47:40 -0600 Subject: [PATCH 25/35] Ungroup imports --- nexus/src/app/vpc.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/nexus/src/app/vpc.rs b/nexus/src/app/vpc.rs index 30672bbaa86..b6896d42297 100644 --- a/nexus/src/app/vpc.rs +++ b/nexus/src/app/vpc.rs @@ -7,7 +7,8 @@ use crate::authz; use crate::context::OpContext; use crate::db; -use crate::db::identity::{Asset, Resource}; +use crate::db::identity::Asset; +use crate::db::identity::Resource; use crate::db::lookup::LookupPath; use crate::db::model::Name; use crate::db::model::VpcRouterKind; @@ -29,7 +30,8 @@ use omicron_common::api::external::RouterRouteCreateParams; use omicron_common::api::external::RouterRouteKind; use omicron_common::api::external::UpdateResult; use omicron_common::api::external::VpcFirewallRuleUpdateParams; -use sled_agent_client::types::{IpNet, NetworkInterface}; +use sled_agent_client::types::IpNet; +use sled_agent_client::types::NetworkInterface; use futures::future::join_all; use ipnetwork::IpNetwork; From 8ab92afbbcfaa70296bf7898c1cc33071ea22026 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Tue, 4 Oct 2022 14:48:38 -0600 Subject: [PATCH 26/35] Rename resolution function for clarity --- nexus/src/app/vpc.rs | 2 +- nexus/src/db/datastore/vpc.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/nexus/src/app/vpc.rs b/nexus/src/app/vpc.rs index b6896d42297..095b8ef54ec 100644 --- a/nexus/src/app/vpc.rs +++ b/nexus/src/app/vpc.rs @@ -578,7 +578,7 @@ impl super::Nexus { let subnet_networks: NetMap = self .db_datastore - .resolve_subnets_to_ips(vpc, subnets) + .resolve_vpc_subnets_to_ip_networks(vpc, subnets) .await? .into_iter() .map(|(name, v)| (name.0, v)) diff --git a/nexus/src/db/datastore/vpc.rs b/nexus/src/db/datastore/vpc.rs index 960b844f915..fbfdeab9cd3 100644 --- a/nexus/src/db/datastore/vpc.rs +++ b/nexus/src/db/datastore/vpc.rs @@ -707,7 +707,7 @@ impl DataStore { } /// Identify all subnets in use by each VpcSubnet - pub async fn resolve_subnets_to_ips>( + pub async fn resolve_vpc_subnets_to_ip_networks>( &self, vpc: &Vpc, subnet_names: T, From cbedb56b48c7131fd9d057c65772a7b513c2407b Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Tue, 4 Oct 2022 14:54:09 -0600 Subject: [PATCH 27/35] Use a B-tree instead of a hash map in resolution function --- nexus/src/db/datastore/vpc.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/nexus/src/db/datastore/vpc.rs b/nexus/src/db/datastore/vpc.rs index fbfdeab9cd3..fd640b59483 100644 --- a/nexus/src/db/datastore/vpc.rs +++ b/nexus/src/db/datastore/vpc.rs @@ -48,7 +48,7 @@ use omicron_common::api::external::LookupResult; use omicron_common::api::external::LookupType; use omicron_common::api::external::ResourceType; use omicron_common::api::external::UpdateResult; -use std::collections::HashMap; +use std::collections::BTreeMap; use uuid::Uuid; impl DataStore { @@ -711,7 +711,7 @@ impl DataStore { &self, vpc: &Vpc, subnet_names: T, - ) -> Result>, Error> { + ) -> Result>, Error> { #[derive(diesel::Queryable)] struct SubnetIps { name: Name, @@ -735,7 +735,7 @@ impl DataStore { public_error_from_diesel_pool(e, ErrorHandler::Server) })?; - let mut result = HashMap::with_capacity(subnets.len()); + let mut result = BTreeMap::new(); for subnet in subnets { let entry = result.entry(subnet.name).or_insert_with(Vec::new); entry.push(IpNetwork::V4(subnet.ipv4_block.0 .0)); From adc62ccc9ec734902f28fd01eba509494c05ca73 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Tue, 4 Oct 2022 15:07:32 -0600 Subject: [PATCH 28/35] Remove superfluous scope --- sled-agent/src/opte/illumos/port_manager.rs | 22 ++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/sled-agent/src/opte/illumos/port_manager.rs b/sled-agent/src/opte/illumos/port_manager.rs index 3ca2b063232..edc5bb1a7c2 100644 --- a/sled-agent/src/opte/illumos/port_manager.rs +++ b/sled-agent/src/opte/illumos/port_manager.rs @@ -314,17 +314,17 @@ impl PortManager { ); // Initialize firewall rules for the new port. - { - let port_name = port_name.clone(); - let rules = opte_firewall_rules(firewall_rules, &vni, &mac); - debug!( - self.inner.log, - "Setting firewall rules"; - "port_name" => ?&port_name, - "rules" => ?&rules, - ); - hdl.set_fw_rules(&SetFwRulesReq { port_name, rules })?; - } + let rules = opte_firewall_rules(firewall_rules, &vni, &mac); + debug!( + self.inner.log, + "Setting firewall rules"; + "port_name" => &port_name, + "rules" => ?&rules, + ); + hdl.set_fw_rules(&SetFwRulesReq { + port_name: port_name.clone(), + rules, + })?; // Create a VNIC on top of this device, to hook Viona into. // From 6048c143ddebfcc7dd1b968751b9959a884bf781 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Tue, 4 Oct 2022 15:13:42 -0600 Subject: [PATCH 29/35] fmt --- nexus/src/db/datastore/vpc.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/nexus/src/db/datastore/vpc.rs b/nexus/src/db/datastore/vpc.rs index fd640b59483..261f7bc99f8 100644 --- a/nexus/src/db/datastore/vpc.rs +++ b/nexus/src/db/datastore/vpc.rs @@ -707,7 +707,9 @@ impl DataStore { } /// Identify all subnets in use by each VpcSubnet - pub async fn resolve_vpc_subnets_to_ip_networks>( + pub async fn resolve_vpc_subnets_to_ip_networks< + T: IntoIterator, + >( &self, vpc: &Vpc, subnet_names: T, From 9869832a2f8d4e4f45012cbc41c35f37d601e5fc Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Wed, 5 Oct 2022 12:38:32 -0600 Subject: [PATCH 30/35] Refactor firewall rule conversion routine --- sled-agent/src/opte/illumos/firewall_rules.rs | 170 ++++++++++++++++++ sled-agent/src/opte/illumos/mod.rs | 2 + sled-agent/src/opte/illumos/port_manager.rs | 120 +------------ 3 files changed, 173 insertions(+), 119 deletions(-) create mode 100644 sled-agent/src/opte/illumos/firewall_rules.rs diff --git a/sled-agent/src/opte/illumos/firewall_rules.rs b/sled-agent/src/opte/illumos/firewall_rules.rs new file mode 100644 index 00000000000..13c53358a89 --- /dev/null +++ b/sled-agent/src/opte/illumos/firewall_rules.rs @@ -0,0 +1,170 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! Convert Omicron VPC firewall rules to OPTE firewall rules. + +use crate::opte::Vni; +use crate::params::VpcFirewallRule; +use macaddr::MacAddr6; +use omicron_common::api::external::IpNet; +use omicron_common::api::external::VpcFirewallRuleAction; +use omicron_common::api::external::VpcFirewallRuleDirection; +use omicron_common::api::external::VpcFirewallRuleProtocol; +use omicron_common::api::external::VpcFirewallRuleStatus; +use oxide_vpc::api::Action; +use oxide_vpc::api::Address; +use oxide_vpc::api::Direction; +use oxide_vpc::api::Filters; +use oxide_vpc::api::FirewallRule; +use oxide_vpc::api::Ipv4Cidr; +use oxide_vpc::api::Ipv4PrefixLen; +use oxide_vpc::api::Ports; +use oxide_vpc::api::ProtoFilter; +use oxide_vpc::api::Protocol; + +trait FromVpcFirewallRule { + fn action(self: &Self) -> Action; + fn direction(self: &Self) -> Direction; + fn disabled(self: &Self) -> bool; + fn hosts(self: &Self) -> Vec
; + fn ports(self: &Self) -> Ports; + fn priority(self: &Self) -> u16; + fn protos(self: &Self) -> Vec; +} + +impl FromVpcFirewallRule for VpcFirewallRule { + fn action(self: &Self) -> Action { + match self.action { + VpcFirewallRuleAction::Allow => Action::Allow, + VpcFirewallRuleAction::Deny => Action::Deny, + } + } + + fn direction(self: &Self) -> Direction { + match self.direction { + VpcFirewallRuleDirection::Inbound => Direction::In, + VpcFirewallRuleDirection::Outbound => Direction::Out, + } + } + + fn disabled(self: &Self) -> bool { + match self.status { + VpcFirewallRuleStatus::Disabled => false, + VpcFirewallRuleStatus::Enabled => true, + } + } + + fn hosts(self: &Self) -> Vec
{ + self.filter_hosts.as_ref().map_or_else( + || vec![Address::Any], + |hosts| { + hosts + .iter() + .map(|host| match host { + IpNet::V4(net) if net.prefix() == 32 => { + Address::Ip(net.ip().into()) + } + IpNet::V4(net) => Address::Subnet(Ipv4Cidr::new( + net.ip().into(), + Ipv4PrefixLen::new(net.prefix()).unwrap(), + )), + IpNet::V6(_net) => { + todo!("IPv6 host filters") + } + }) + .collect::>() + }, + ) + } + + fn ports(self: &Self) -> Ports { + match self.filter_ports { + Some(ref ports) if ports.len() > 0 => Ports::PortList( + ports + .iter() + .flat_map(|range| { + (range.first.0.get()..=range.last.0.get()) + .collect::>() + }) + .collect::>(), + ), + _ => Ports::Any, + } + } + + fn priority(self: &Self) -> u16 { + self.priority.0 + } + + fn protos(self: &Self) -> Vec { + self.filter_protocols.as_ref().map_or_else( + || vec![ProtoFilter::Any], + |protos| { + protos + .iter() + .map(|proto| { + ProtoFilter::Proto(match proto { + VpcFirewallRuleProtocol::Tcp => Protocol::TCP, + VpcFirewallRuleProtocol::Udp => Protocol::UDP, + VpcFirewallRuleProtocol::Icmp => Protocol::ICMP, + }) + }) + .collect::>() + }, + ) + } +} + +/// Translate from a slice of VPC firewall rules to a vector of OPTE rules +/// that match a given port's VNI and MAC address. OPTE rules can only encode +/// a single host address and protocol, so we must unroll rules with multiple +/// hosts/protocols. +pub fn opte_firewall_rules( + rules: &[VpcFirewallRule], + vni: &Vni, + mac: &MacAddr6, +) -> Vec { + rules + .iter() + .filter(|rule| rule.disabled()) + .filter(|rule| { + rule.targets.is_empty() // no targets means apply everywhere + || rule.targets.iter().any(|nic| { + // (VNI, MAC) is a unique identifier for the NIC. + u32::from(nic.vni) == u32::from(*vni) && nic.mac.0 == *mac + }) + }) + .map(|rule| { + let priority = rule.priority(); + let action = rule.action(); + let direction = rule.direction(); + let ports = rule.ports(); + let protos = rule.protos(); + let hosts = rule.hosts(); + protos + .iter() + .map(|proto| { + hosts + .iter() + .map(|hosts| FirewallRule { + priority, + action, + direction, + filters: { + let mut filters = Filters::new(); + filters + .set_hosts(hosts.clone()) + .set_protocol(proto.clone()) + .set_ports(ports.clone()); + filters + }, + }) + .collect::>() + }) + .collect::>>() + }) + .flatten() + .flatten() + .collect::>() +} diff --git a/sled-agent/src/opte/illumos/mod.rs b/sled-agent/src/opte/illumos/mod.rs index d5cf1201cd2..1aaadabb034 100644 --- a/sled-agent/src/opte/illumos/mod.rs +++ b/sled-agent/src/opte/illumos/mod.rs @@ -11,9 +11,11 @@ use slog::Logger; use std::fs; use std::path::Path; +mod firewall_rules; mod port; mod port_manager; +pub use firewall_rules::opte_firewall_rules; pub use port::Port; pub use port_manager::PortManager; pub use port_manager::PortTicket; diff --git a/sled-agent/src/opte/illumos/port_manager.rs b/sled-agent/src/opte/illumos/port_manager.rs index edc5bb1a7c2..9fa694f1aff 100644 --- a/sled-agent/src/opte/illumos/port_manager.rs +++ b/sled-agent/src/opte/illumos/port_manager.rs @@ -8,6 +8,7 @@ use crate::illumos::dladm::Dladm; use crate::illumos::dladm::PhysicalLink; use crate::illumos::dladm::VnicSource; use crate::opte::default_boundary_services; +use crate::opte::opte_firewall_rules; use crate::opte::Error; use crate::opte::Gateway; use crate::opte::Port; @@ -17,27 +18,14 @@ use crate::params::SourceNatConfig; use crate::params::VpcFirewallRule; use ipnetwork::IpNetwork; use macaddr::MacAddr6; -use omicron_common::api::external::IpNet; -use omicron_common::api::external::VpcFirewallRuleAction; -use omicron_common::api::external::VpcFirewallRuleDirection; -use omicron_common::api::external::VpcFirewallRuleProtocol; -use omicron_common::api::external::VpcFirewallRuleStatus; use opte_ioctl::OpteHdl; -use oxide_vpc::api::Action; use oxide_vpc::api::AddRouterEntryReq; -use oxide_vpc::api::Address; -use oxide_vpc::api::Direction; -use oxide_vpc::api::Filters; -use oxide_vpc::api::FirewallRule; use oxide_vpc::api::IpCfg; use oxide_vpc::api::IpCidr; use oxide_vpc::api::Ipv4Cfg; use oxide_vpc::api::Ipv4Cidr; use oxide_vpc::api::Ipv4PrefixLen; use oxide_vpc::api::MacAddr; -use oxide_vpc::api::Ports; -use oxide_vpc::api::ProtoFilter; -use oxide_vpc::api::Protocol; use oxide_vpc::api::RouterTarget; use oxide_vpc::api::SNat4Cfg; use oxide_vpc::api::SetFwRulesReq; @@ -497,112 +485,6 @@ impl PortManager { } } -/// Translate from a slice of VPC firewall rules to a vector of OPTE rules -/// that match the given port's MAC address. OPTE rules can only encode a -/// single host address and protocol, so we must unroll rules with multiple -/// hosts/protocols. -fn opte_firewall_rules( - rules: &[VpcFirewallRule], - vni: &Vni, - mac: &MacAddr6, -) -> Vec { - rules - .iter() - .filter(|rule| match rule.status { - VpcFirewallRuleStatus::Disabled => false, - VpcFirewallRuleStatus::Enabled => true, - }) - .filter(|rule| { - rule.targets.is_empty() // no targets means apply everywhere - || rule.targets.iter().any(|nic| { - // (VNI, MAC) is a unique identifier for the NIC. - u32::from(nic.vni) == u32::from(*vni) && nic.mac.0 == *mac - }) - }) - .map(|rule| { - let priority = rule.priority.0; - let action = match rule.action { - VpcFirewallRuleAction::Allow => Action::Allow, - VpcFirewallRuleAction::Deny => Action::Deny, - }; - let direction = match rule.direction { - VpcFirewallRuleDirection::Inbound => Direction::In, - VpcFirewallRuleDirection::Outbound => Direction::Out, - }; - let ports = match rule.filter_ports { - Some(ref ports) if ports.len() > 0 => Ports::PortList( - ports - .iter() - .flat_map(|range| { - (range.first.0.get()..=range.last.0.get()) - .collect::>() - }) - .collect::>(), - ), - _ => Ports::Any, - }; - let proto_filters = rule.filter_protocols.as_ref().map_or_else( - || vec![ProtoFilter::Any], - |protos| { - protos - .iter() - .map(|proto| { - ProtoFilter::Proto(match proto { - VpcFirewallRuleProtocol::Tcp => Protocol::TCP, - VpcFirewallRuleProtocol::Udp => Protocol::UDP, - VpcFirewallRuleProtocol::Icmp => Protocol::ICMP, - }) - }) - .collect::>() - }, - ); - let host_filters = rule.filter_hosts.as_ref().map_or_else( - || vec![Address::Any], - |hosts| { - hosts - .iter() - .map(|host| match host { - IpNet::V4(net) if net.prefix() == 32 => { - Address::Ip(net.ip().into()) - } - IpNet::V4(net) => Address::Subnet(Ipv4Cidr::new( - net.ip().into(), - Ipv4PrefixLen::new(net.prefix()).unwrap(), - )), - IpNet::V6(_net) => { - todo!("IPv6 host filters") - } - }) - .collect::>() - }, - ); - proto_filters - .iter() - .map(|proto| { - host_filters - .iter() - .map(|hosts| FirewallRule { - priority, - action, - direction, - filters: { - let mut filters = Filters::new(); - filters - .set_hosts(hosts.clone()) - .set_protocol(proto.clone()) - .set_ports(ports.clone()); - filters - }, - }) - .collect::>() - }) - .collect::>>() - }) - .flatten() - .flatten() - .collect::>() -} - pub struct PortTicket { id: Uuid, port_name: String, From f7ba1eb7b6eb0d0503188fba470e042985979845 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Wed, 5 Oct 2022 13:11:28 -0600 Subject: [PATCH 31/35] Elide needless clone --- nexus/src/app/vpc.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nexus/src/app/vpc.rs b/nexus/src/app/vpc.rs index 095b8ef54ec..941d58d469b 100644 --- a/nexus/src/app/vpc.rs +++ b/nexus/src/app/vpc.rs @@ -72,7 +72,7 @@ impl super::Nexus { )?; let (authz_vpc, db_vpc) = self .db_datastore - .project_create_vpc(opctx, &authz_project, vpc.clone()) + .project_create_vpc(opctx, &authz_project, vpc) .await?; // TODO: Ultimately when the VPC is created a system router w/ an From 171a62516b32feb50f4aa84a07bca77abd5e5bdf Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Wed, 5 Oct 2022 13:14:47 -0600 Subject: [PATCH 32/35] Clarify doc comment for `default_firewall_rules_for_vpc` --- nexus/src/app/vpc.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/nexus/src/app/vpc.rs b/nexus/src/app/vpc.rs index 941d58d469b..84c959c2b0e 100644 --- a/nexus/src/app/vpc.rs +++ b/nexus/src/app/vpc.rs @@ -350,7 +350,8 @@ impl super::Nexus { Ok(rules) } - /// Customize the default firewall rules for a particular VPC. + /// Customize the default firewall rules for a particular VPC + /// by replacing the name `default` with the VPC's actual name. async fn default_firewall_rules_for_vpc( &self, vpc_id: Uuid, From aae7d208f242bf9cfdb11fd176b0ce3bf0f6bb98 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Wed, 5 Oct 2022 13:21:42 -0600 Subject: [PATCH 33/35] Link to multiple-error-handling issue --- nexus/src/app/vpc.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/nexus/src/app/vpc.rs b/nexus/src/app/vpc.rs index 84c959c2b0e..a289f968c72 100644 --- a/nexus/src/app/vpc.rs +++ b/nexus/src/app/vpc.rs @@ -431,6 +431,7 @@ impl super::Nexus { debug!(self.log, "sending firewall rules to sled agents"); let results = join_all(sled_requests).await; // TODO-correctness: handle more than one failure in the sled-agent requests + // https://github.com/oxidecomputer/omicron/issues/1791 for (sled, result) in vpc_to_sleds.iter().zip(results) { if let Err(e) = result { warn!(self.log, "failed to update firewall rules on sled agent"; From 63e92bc751ce40a303b1e9044a50e30aeacea9d6 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Thu, 13 Oct 2022 09:36:55 -0600 Subject: [PATCH 34/35] SELECT DISTINCT sleds for firewall rule updates --- nexus/src/db/datastore/vpc.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/nexus/src/db/datastore/vpc.rs b/nexus/src/db/datastore/vpc.rs index 261f7bc99f8..82b210ab473 100644 --- a/nexus/src/db/datastore/vpc.rs +++ b/nexus/src/db/datastore/vpc.rs @@ -343,6 +343,7 @@ impl DataStore { .filter(network_interface::time_deleted.is_null()) .filter(instance::time_deleted.is_null()) .select(Sled::as_select()) + .distinct() .get_results_async(self.pool()) .await .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server)) From fd5c3e441bb61cae2400e0142b193eb9e48c57ac Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Thu, 13 Oct 2022 17:41:25 -0600 Subject: [PATCH 35/35] Don't fail on missing names in firewall rule resolution --- nexus/src/app/vpc.rs | 82 ++++++++++++++++++++++++-------------------- 1 file changed, 45 insertions(+), 37 deletions(-) diff --git a/nexus/src/app/vpc.rs b/nexus/src/app/vpc.rs index a289f968c72..121e73d05e3 100644 --- a/nexus/src/app/vpc.rs +++ b/nexus/src/app/vpc.rs @@ -521,60 +521,68 @@ impl super::Nexus { let mut instance_interfaces: NicMap = HashMap::new(); for instance_name in &instances { - let (.., authz_instance) = + if let Ok((.., authz_instance)) = LookupPath::new(opctx, &self.db_datastore) .project_id(vpc.project_id) .instance_name(instance_name) .lookup_for(authz::Action::ListChildren) - .await?; - for iface in self - .db_datastore - .derive_guest_network_interface_info(opctx, &authz_instance) - .await? + .await { - instance_interfaces - .entry(instance_name.0.clone()) - .or_insert_with(Vec::new) - .push(iface); + for iface in self + .db_datastore + .derive_guest_network_interface_info(opctx, &authz_instance) + .await? + { + instance_interfaces + .entry(instance_name.0.clone()) + .or_insert_with(Vec::new) + .push(iface); + } } } let mut vpc_interfaces: NicMap = HashMap::new(); for vpc_name in &vpcs { - let (.., authz_vpc) = LookupPath::new(opctx, &self.db_datastore) - .project_id(vpc.project_id) - .vpc_name(vpc_name) - .lookup_for(authz::Action::ListChildren) - .await?; - for iface in self - .db_datastore - .derive_vpc_network_interface_info(opctx, &authz_vpc) - .await? + if let Ok((.., authz_vpc)) = + LookupPath::new(opctx, &self.db_datastore) + .project_id(vpc.project_id) + .vpc_name(vpc_name) + .lookup_for(authz::Action::ListChildren) + .await { - vpc_interfaces - .entry(vpc_name.0.clone()) - .or_insert_with(Vec::new) - .push(iface); + for iface in self + .db_datastore + .derive_vpc_network_interface_info(opctx, &authz_vpc) + .await? + { + vpc_interfaces + .entry(vpc_name.0.clone()) + .or_insert_with(Vec::new) + .push(iface); + } } } let mut subnet_interfaces: NicMap = HashMap::new(); for subnet_name in &subnets { - let (.., authz_subnet) = LookupPath::new(opctx, &self.db_datastore) - .project_id(vpc.project_id) - .vpc_name(&Name::from(vpc.name().clone())) - .vpc_subnet_name(subnet_name) - .lookup_for(authz::Action::ListChildren) - .await?; - for iface in self - .db_datastore - .derive_subnet_network_interface_info(opctx, &authz_subnet) - .await? + if let Ok((.., authz_subnet)) = + LookupPath::new(opctx, &self.db_datastore) + .project_id(vpc.project_id) + .vpc_name(&Name::from(vpc.name().clone())) + .vpc_subnet_name(subnet_name) + .lookup_for(authz::Action::ListChildren) + .await { - subnet_interfaces - .entry(subnet_name.0.clone()) - .or_insert_with(Vec::new) - .push(iface); + for iface in self + .db_datastore + .derive_subnet_network_interface_info(opctx, &authz_subnet) + .await? + { + subnet_interfaces + .entry(subnet_name.0.clone()) + .or_insert_with(Vec::new) + .push(iface); + } } }