diff --git a/.github/buildomat/jobs/deploy.sh b/.github/buildomat/jobs/deploy.sh index 52c29fe342b..4c670925f54 100755 --- a/.github/buildomat/jobs/deploy.sh +++ b/.github/buildomat/jobs/deploy.sh @@ -2,7 +2,7 @@ #: #: name = "helios / deploy" #: variety = "basic" -#: target = "lab-2.0-opte-0.36" +#: target = "lab-2.0-opte-0.37" #: output_rules = [ #: "%/var/svc/log/oxide-*.log*", #: "%/zone/oxz_*/root/var/svc/log/oxide-*.log*", diff --git a/Cargo.lock b/Cargo.lock index 1bea60531d8..f742c5c3da6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2311,7 +2311,7 @@ dependencies = [ [[package]] name = "ddm-admin-client" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/maghemite?rev=760b4b547e301a31d4dcb92ba97aabdb2a3e0cba#760b4b547e301a31d4dcb92ba97aabdb2a3e0cba" +source = "git+https://github.com/oxidecomputer/maghemite?rev=638c897d5ed1e5d3f2de0c1cb9dfaa4d77a35dea#638c897d5ed1e5d3f2de0c1cb9dfaa4d77a35dea" dependencies = [ "oxnet", "percent-encoding", @@ -4738,7 +4738,7 @@ dependencies = [ [[package]] name = "illumos-sys-hdrs" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/opte?rev=f5560fae02ad3fc349fabc6454c321143199ca9e#f5560fae02ad3fc349fabc6454c321143199ca9e" +source = "git+https://github.com/oxidecomputer/opte?rev=3f2dfe36f156b486e60e7a08263ad6227be1e969#3f2dfe36f156b486e60e7a08263ad6227be1e969" dependencies = [ "bitflags 2.9.0", ] @@ -5317,7 +5317,7 @@ dependencies = [ [[package]] name = "kstat-macro" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/opte?rev=f5560fae02ad3fc349fabc6454c321143199ca9e#f5560fae02ad3fc349fabc6454c321143199ca9e" +source = "git+https://github.com/oxidecomputer/opte?rev=3f2dfe36f156b486e60e7a08263ad6227be1e969#3f2dfe36f156b486e60e7a08263ad6227be1e969" dependencies = [ "quote", "syn 2.0.104", @@ -5882,7 +5882,7 @@ dependencies = [ [[package]] name = "mg-admin-client" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/maghemite?rev=760b4b547e301a31d4dcb92ba97aabdb2a3e0cba#760b4b547e301a31d4dcb92ba97aabdb2a3e0cba" +source = "git+https://github.com/oxidecomputer/maghemite?rev=638c897d5ed1e5d3f2de0c1cb9dfaa4d77a35dea#638c897d5ed1e5d3f2de0c1cb9dfaa4d77a35dea" dependencies = [ "anyhow", "chrono", @@ -8489,7 +8489,7 @@ dependencies = [ [[package]] name = "opte" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/opte?rev=f5560fae02ad3fc349fabc6454c321143199ca9e#f5560fae02ad3fc349fabc6454c321143199ca9e" +source = "git+https://github.com/oxidecomputer/opte?rev=3f2dfe36f156b486e60e7a08263ad6227be1e969#3f2dfe36f156b486e60e7a08263ad6227be1e969" dependencies = [ "bitflags 2.9.0", "dyn-clone", @@ -8501,12 +8501,13 @@ dependencies = [ "serde", "tabwriter", "version_check", + "zerocopy 0.8.26", ] [[package]] name = "opte-api" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/opte?rev=f5560fae02ad3fc349fabc6454c321143199ca9e#f5560fae02ad3fc349fabc6454c321143199ca9e" +source = "git+https://github.com/oxidecomputer/opte?rev=3f2dfe36f156b486e60e7a08263ad6227be1e969#3f2dfe36f156b486e60e7a08263ad6227be1e969" dependencies = [ "illumos-sys-hdrs", "ingot", @@ -8519,7 +8520,7 @@ dependencies = [ [[package]] name = "opte-ioctl" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/opte?rev=f5560fae02ad3fc349fabc6454c321143199ca9e#f5560fae02ad3fc349fabc6454c321143199ca9e" +source = "git+https://github.com/oxidecomputer/opte?rev=3f2dfe36f156b486e60e7a08263ad6227be1e969#3f2dfe36f156b486e60e7a08263ad6227be1e969" dependencies = [ "libc", "libnet", @@ -8613,7 +8614,7 @@ dependencies = [ [[package]] name = "oxide-vpc" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/opte?rev=f5560fae02ad3fc349fabc6454c321143199ca9e#f5560fae02ad3fc349fabc6454c321143199ca9e" +source = "git+https://github.com/oxidecomputer/opte?rev=3f2dfe36f156b486e60e7a08263ad6227be1e969#3f2dfe36f156b486e60e7a08263ad6227be1e969" dependencies = [ "cfg-if", "illumos-sys-hdrs", diff --git a/Cargo.toml b/Cargo.toml index 7e509f93005..18791a573c2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -511,8 +511,8 @@ lldp_protocol = { git = "https://github.com/oxidecomputer/lldp", package = "prot macaddr = { version = "1.0.1", features = ["serde_std"] } maplit = "1.0.2" newtype_derive = "0.1.6" -mg-admin-client = { git = "https://github.com/oxidecomputer/maghemite", rev = "760b4b547e301a31d4dcb92ba97aabdb2a3e0cba" } -ddm-admin-client = { git = "https://github.com/oxidecomputer/maghemite", rev = "760b4b547e301a31d4dcb92ba97aabdb2a3e0cba" } +mg-admin-client = { git = "https://github.com/oxidecomputer/maghemite", rev = "638c897d5ed1e5d3f2de0c1cb9dfaa4d77a35dea" } +ddm-admin-client = { git = "https://github.com/oxidecomputer/maghemite", rev = "638c897d5ed1e5d3f2de0c1cb9dfaa4d77a35dea" } multimap = "0.10.1" nexus-auth = { path = "nexus/auth" } nexus-background-task-interface = { path = "nexus/background-task-interface" } @@ -568,7 +568,7 @@ omicron-workspace-hack = "0.1.0" omicron-zone-package = "0.12.2" oxide-client = { path = "clients/oxide-client" } oxide-tokio-rt = "0.1.1" -oxide-vpc = { git = "https://github.com/oxidecomputer/opte", rev = "f5560fae02ad3fc349fabc6454c321143199ca9e", features = [ "api", "std" ] } +oxide-vpc = { git = "https://github.com/oxidecomputer/opte", rev = "3f2dfe36f156b486e60e7a08263ad6227be1e969", features = [ "api", "std" ] } oxlog = { path = "dev-tools/oxlog" } oxnet = "0.1.2" once_cell = "1.21.3" @@ -578,7 +578,7 @@ openapiv3 = "2.2.0" # must match samael's crate! openssl = "0.10" openssl-sys = "0.9" -opte-ioctl = { git = "https://github.com/oxidecomputer/opte", rev = "f5560fae02ad3fc349fabc6454c321143199ca9e" } +opte-ioctl = { git = "https://github.com/oxidecomputer/opte", rev = "3f2dfe36f156b486e60e7a08263ad6227be1e969" } oso = "0.27" owo-colors = "4.2.2" oximeter = { path = "oximeter/oximeter" } diff --git a/clients/sled-agent-client/src/lib.rs b/clients/sled-agent-client/src/lib.rs index 5eb8c3ecafe..d63c80fff5d 100644 --- a/clients/sled-agent-client/src/lib.rs +++ b/clients/sled-agent-client/src/lib.rs @@ -90,6 +90,7 @@ progenitor::generate_api!( TypedUuidForSupportBundleKind = omicron_uuid_kinds::SupportBundleUuid, TypedUuidForZpoolKind = omicron_uuid_kinds::ZpoolUuid, Vni = omicron_common::api::external::Vni, + VpcFirewallIcmpFilter = omicron_common::api::external::VpcFirewallIcmpFilter, ZpoolKind = omicron_common::zpool_name::ZpoolKind, ZpoolName = omicron_common::zpool_name::ZpoolName, } @@ -308,7 +309,7 @@ impl From match s { Tcp => Self::Tcp, Udp => Self::Udp, - Icmp => Self::Icmp, + Icmp(v) => Self::Icmp(v), } } } diff --git a/common/src/api/external/mod.rs b/common/src/api/external/mod.rs index c7c7c6092c3..e5a11c6a034 100644 --- a/common/src/api/external/mod.rs +++ b/common/src/api/external/mod.rs @@ -42,8 +42,10 @@ use std::fmt::Formatter; use std::fmt::Result as FormatResult; use std::net::IpAddr; use std::net::Ipv4Addr; +use std::num::ParseIntError; use std::num::{NonZeroU16, NonZeroU32}; use std::ops::Deref; +use std::ops::RangeInclusive; use std::str::FromStr; use tufaceous_artifact::ArtifactHash; use uuid::Uuid; @@ -1842,11 +1844,129 @@ pub struct VpcFirewallRuleFilter { /// The protocols that may be specified in a firewall rule's filter #[derive(Clone, Copy, Debug, PartialEq, Deserialize, Serialize, JsonSchema)] -#[serde(rename_all = "UPPERCASE")] +#[serde(rename_all = "snake_case")] +#[serde(tag = "type", content = "value")] pub enum VpcFirewallRuleProtocol { Tcp, Udp, - Icmp, + Icmp(Option), + // TODO: IPv6 not supported by instances. + // Icmpv6(Option), + // TODO: OPTE does not yet permit further L4 protocols. (opte#609) + // Other(u16), +} + +impl FromStr for VpcFirewallRuleProtocol { + type Err = Error; + + fn from_str(proto: &str) -> Result { + let (ty_str, content_str) = match proto.split_once(':') { + None => (proto, None), + Some((lhs, rhs)) => (lhs, Some(rhs)), + }; + + match (ty_str, content_str) { + (lhs, None) if lhs.eq_ignore_ascii_case("tcp") => Ok(Self::Tcp), + (lhs, None) if lhs.eq_ignore_ascii_case("udp") => Ok(Self::Udp), + (lhs, None) if lhs.eq_ignore_ascii_case("icmp") => { + Ok(Self::Icmp(None)) + } + (lhs, Some(rhs)) if lhs.eq_ignore_ascii_case("icmp") => { + Ok(Self::Icmp(Some(rhs.parse()?))) + } + (lhs, None) => Err(Error::invalid_value( + "vpc_firewall_rule_protocol", + format!("unrecognized protocol: {lhs}"), + )), + (lhs, Some(rhs)) => Err(Error::invalid_value( + "vpc_firewall_rule_protocol", + format!( + "cannot specify extra filters ({rhs}) for protocol \"{lhs}\"" + ), + )), + } + } +} + +impl TryFrom for VpcFirewallRuleProtocol { + type Error = ::Err; + + fn try_from(proto: String) -> Result { + proto.parse() + } +} + +impl Display for VpcFirewallRuleProtocol { + fn fmt(&self, f: &mut Formatter<'_>) -> FormatResult { + match self { + VpcFirewallRuleProtocol::Tcp => write!(f, "tcp"), + VpcFirewallRuleProtocol::Udp => write!(f, "udp"), + VpcFirewallRuleProtocol::Icmp(None) => write!(f, "icmp"), + VpcFirewallRuleProtocol::Icmp(Some(v)) => write!(f, "icmp:{v}"), + } + } +} + +#[derive(Clone, Copy, Debug, PartialEq, Deserialize, Serialize, JsonSchema)] +pub struct VpcFirewallIcmpFilter { + pub icmp_type: u8, + pub code: Option, +} + +impl Display for VpcFirewallIcmpFilter { + fn fmt(&self, f: &mut Formatter<'_>) -> FormatResult { + write!(f, "{}", self.icmp_type)?; + if let Some(code) = self.code { + write!(f, ",{code}")?; + } + Ok(()) + } +} + +impl FromStr for VpcFirewallIcmpFilter { + type Err = Error; + + fn from_str(filter: &str) -> Result { + let (ty_str, code_str) = match filter.split_once(',') { + None => (filter, None), + Some((lhs, rhs)) => (lhs, Some(rhs)), + }; + + Ok(Self { + icmp_type: ty_str.parse::().map_err(|e| { + Error::invalid_value( + "icmp_type", + format!("{ty_str:?} unparsable for type: {e}"), + ) + })?, + code: code_str + .map(|v| { + v.parse::().map_err(|e| { + Error::invalid_value("code", e.to_string()) + }) + }) + .transpose()?, + }) + } +} + +impl From for IcmpParamRange { + fn from(value: u8) -> Self { + Self { first: value, last: value } + } +} + +impl From> for IcmpParamRange { + fn from(value: RangeInclusive) -> Self { + let (first, last) = value.into_inner(); + Self { first, last } + } +} + +impl From for RangeInclusive { + fn from(value: IcmpParamRange) -> Self { + value.first..=value.last + } } #[derive(Clone, Copy, Debug, Deserialize, Serialize, PartialEq, JsonSchema)] @@ -1976,26 +2096,26 @@ pub struct L4PortRange { } impl FromStr for L4PortRange { - type Err = String; + type Err = L4PortRangeError; fn from_str(range: &str) -> Result { - const INVALID_PORT_NUMBER_MSG: &str = "invalid port number"; - match range.split_once('-') { None => { let port = range .parse::() - .map_err(|_| INVALID_PORT_NUMBER_MSG.to_string())? + .map_err(|e| L4PortRangeError::Value(range.into(), e))? .into(); Ok(L4PortRange { first: port, last: port }) } + Some(("", _)) => Err(L4PortRangeError::MissingStart), + Some((_, "")) => Err(L4PortRangeError::MissingEnd), Some((left, right)) => { let first = left .parse::() - .map_err(|_| INVALID_PORT_NUMBER_MSG.to_string())? + .map_err(|e| L4PortRangeError::Value(left.into(), e))? .into(); let last = right .parse::() - .map_err(|_| INVALID_PORT_NUMBER_MSG.to_string())? + .map_err(|e| L4PortRangeError::Value(right.into(), e))? .into(); Ok(L4PortRange { first, last }) } @@ -2003,6 +2123,22 @@ impl FromStr for L4PortRange { } } +#[derive(Clone, Debug, Eq, PartialEq, thiserror::Error)] +pub enum L4PortRangeError { + #[error("range has no start value")] + MissingStart, + #[error("range has no end value")] + MissingEnd, + #[error("{0:?} unparsable for type: {1}")] + Value(String, ParseIntError), +} + +impl From for Error { + fn from(value: L4PortRangeError) -> Self { + Error::invalid_value("l4_port_range", value.to_string()) + } +} + impl TryFrom for L4PortRange { type Error = ::Err; @@ -2055,6 +2191,117 @@ impl JsonSchema for L4PortRange { } } +impl From for L4PortRange { + fn from(value: L4Port) -> Self { + Self { first: value, last: value } + } +} + +impl From> for L4PortRange { + fn from(value: RangeInclusive) -> Self { + let (first, last) = value.into_inner(); + Self { first, last } + } +} + +/// A range of ICMP(v6) types or codes. This range is inclusive on both ends. +#[derive( + Clone, Copy, Debug, DeserializeFromStr, SerializeDisplay, PartialEq, +)] +pub struct IcmpParamRange { + /// The first number in the range + pub first: u8, + /// The last number in the range + pub last: u8, +} + +impl FromStr for IcmpParamRange { + type Err = IcmpParamRangeError; + fn from_str(range: &str) -> Result { + match range.split_once('-') { + None => { + let param = range + .parse::() + .map_err(|e| IcmpParamRangeError::Value(range.into(), e))?; + Ok(IcmpParamRange { first: param, last: param }) + } + Some(("", _)) => Err(IcmpParamRangeError::MissingStart), + Some((_, "")) => Err(IcmpParamRangeError::MissingEnd), + Some((left, right)) => { + let first = left + .parse::() + .map_err(|e| IcmpParamRangeError::Value(left.into(), e))?; + let last = right + .parse::() + .map_err(|e| IcmpParamRangeError::Value(right.into(), e))?; + Ok(IcmpParamRange { first, last }) + } + } + } +} + +#[derive(Clone, Debug, Eq, PartialEq, thiserror::Error)] +pub enum IcmpParamRangeError { + #[error("range has no start value")] + MissingStart, + #[error("range has no end value")] + MissingEnd, + #[error("{0:?} unparsable for type: {1}")] + Value(String, ParseIntError), +} + +impl TryFrom for IcmpParamRange { + type Error = ::Err; + + fn try_from(range: String) -> Result { + range.parse() + } +} + +impl Display for IcmpParamRange { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + if self.first == self.last { + write!(f, "{}", self.first) + } else { + write!(f, "{}-{}", self.first, self.last) + } + } +} + +impl JsonSchema for IcmpParamRange { + fn schema_name() -> String { + "IcmpParamRange".to_string() + } + + fn json_schema( + _: &mut schemars::gen::SchemaGenerator, + ) -> schemars::schema::Schema { + schemars::schema::SchemaObject { + metadata: Some(Box::new(schemars::schema::Metadata { + title: Some("A range of ICMP(v6) types or codes".to_string()), + description: Some( + "An inclusive-inclusive range of ICMP(v6) types or codes. \ + The second value may be omitted to represent a single parameter." + .to_string(), + ), + examples: vec!["3".into(), "20-120".into()], + ..Default::default() + })), + instance_type: Some( + schemars::schema::InstanceType::String.into() + ), + string: Some(Box::new(schemars::schema::StringValidation { + max_length: Some(7), // 3 digits for each value and the dash + min_length: Some(1), + pattern: Some( + r#"^[0-9]{1,3}(-[0-9]{1,3})?$"#.to_string(), + ), + })), + ..Default::default() + }.into() + } +} + /// The `MacAddr` represents a Media Access Control (MAC) address, used to uniquely identify /// hardware devices on a network. // NOTE: We're using the `macaddr` crate for the internal representation. But as with the `ipnet`, @@ -3127,6 +3374,28 @@ pub enum BfdMode { MultiHop, } +/// Configuration of inbound ICMP allowed by API services. +#[derive( + Clone, + Copy, + Debug, + Deserialize, + Serialize, + JsonSchema, + PartialEq, + Eq, + Ord, + PartialOrd, +)] +pub struct ServiceIcmpConfig { + /// When enabled, Nexus is able to receive ICMP Destination Unreachable + /// type 3 (port unreachable) and type 4 (fragmentation needed), + /// Redirect, and Time Exceeded messages. These enable Nexus to perform Path + /// MTU discovery and better cope with fragmentation issues. Otherwise all + /// inbound ICMP traffic will be dropped. + pub enabled: bool, +} + /// A description of an uploaded TUF repository. #[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize, JsonSchema)] pub struct TufRepoDescription { @@ -3322,6 +3591,7 @@ mod test { use super::Generation; use super::RouteDestination; use super::RouteTarget; + use super::VpcFirewallIcmpFilter; use super::VpcFirewallRuleHostFilter; use super::VpcFirewallRuleTarget; use super::{ @@ -3664,32 +3934,54 @@ mod test { ); assert_eq!( - L4PortRange::try_from("".to_string()), - Err("invalid port number".to_string()) + L4PortRange::try_from("".to_string()).map_err(Into::into), + Err(Error::invalid_value( + "l4_port_range", + "\"\" unparsable for type: cannot parse integer from empty string" + )) ); assert_eq!( - L4PortRange::try_from("65536".to_string()), - Err("invalid port number".to_string()) + L4PortRange::try_from("65536".to_string()).map_err(Into::into), + Err(Error::invalid_value( + "l4_port_range", + "\"65536\" unparsable for type: number too large to fit in target type" + )) ); assert_eq!( - L4PortRange::try_from("65535-65536".to_string()), - Err("invalid port number".to_string()) + L4PortRange::try_from("65535-65536".to_string()) + .map_err(Into::into), + Err(Error::invalid_value( + "l4_port_range", + "\"65536\" unparsable for type: number too large to fit in target type" + )) ); assert_eq!( - L4PortRange::try_from("0x23".to_string()), - Err("invalid port number".to_string()) + L4PortRange::try_from("0x23".to_string()).map_err(Into::into), + Err(Error::invalid_value( + "l4_port_range", + "\"0x23\" unparsable for type: invalid digit found in string" + )) ); assert_eq!( - L4PortRange::try_from("0".to_string()), - Err("invalid port number".to_string()) + L4PortRange::try_from("0".to_string()).map_err(Into::into), + Err(Error::invalid_value( + "l4_port_range", + "\"0\" unparsable for type: number would be zero for non-zero type" + )) ); assert_eq!( - L4PortRange::try_from("0-20".to_string()), - Err("invalid port number".to_string()) + L4PortRange::try_from("0-20".to_string()).map_err(Into::into), + Err(Error::invalid_value( + "l4_port_range", + "\"0\" unparsable for type: number would be zero for non-zero type" + )) ); assert_eq!( - L4PortRange::try_from("-20".to_string()), - Err("invalid port number".to_string()) + L4PortRange::try_from("-20".to_string()).map_err(Into::into), + Err(Error::invalid_value( + "l4_port_range", + "range has no start value" + )) ); } @@ -3729,7 +4021,7 @@ mod test { "status": "disabled", "direction": "outbound", "targets": [ { "type": "vpc", "value": "default" } ], - "filters": {"ports": [ "22-25", "27" ], "protocols": [ "UDP" ]}, + "filters": {"ports": [ "22-25", "27" ], "protocols": [ { "type": "udp" } ]}, "action": "deny", "priority": 65533, "description": "second rule" @@ -3920,6 +4212,77 @@ mod test { assert!("foo".parse::().is_err()); } + #[test] + fn test_firewall_rule_proto_filter_parse() { + assert_eq!(VpcFirewallRuleProtocol::Tcp, "tcp".parse().unwrap()); + assert_eq!(VpcFirewallRuleProtocol::Udp, "udp".parse().unwrap()); + + assert_eq!( + VpcFirewallRuleProtocol::Icmp(None), + "icmp".parse().unwrap() + ); + assert_eq!( + VpcFirewallRuleProtocol::Icmp(Some(VpcFirewallIcmpFilter { + icmp_type: 4, + code: None + })), + "icmp:4".parse().unwrap() + ); + assert_eq!( + VpcFirewallRuleProtocol::Icmp(Some(VpcFirewallIcmpFilter { + icmp_type: 60, + code: Some(0.into()) + })), + "icmp:60,0".parse().unwrap() + ); + assert_eq!( + VpcFirewallRuleProtocol::Icmp(Some(VpcFirewallIcmpFilter { + icmp_type: 60, + code: Some((0..=10).into()) + })), + "icmp:60,0-10".parse().unwrap() + ); + assert_eq!( + "icmp:".parse::(), + Err(Error::invalid_value( + "icmp_type", + "\"\" unparsable for type: cannot parse integer from empty string" + )) + ); + assert_eq!( + "icmp:20-30".parse::(), + Err(Error::invalid_value( + "icmp_type", + "\"20-30\" unparsable for type: invalid digit found in string" + )) + ); + assert_eq!( + "icmp:10,".parse::(), + Err(Error::invalid_value( + "code", + "\"\" unparsable for type: cannot parse integer from empty string" + )) + ); + assert_eq!( + "icmp:257,".parse::(), + Err(Error::invalid_value( + "icmp_type", + "\"257\" unparsable for type: number too large to fit in target type" + )) + ); + assert_eq!( + "icmp:0,1000-1001".parse::(), + Err(Error::invalid_value( + "code", + "\"1000\" unparsable for type: number too large to fit in target type" + )) + ); + assert_eq!( + "icmp:0,30-".parse::(), + Err(Error::invalid_value("code", "range has no end value")) + ); + } + #[test] fn test_digest() { // No prefix diff --git a/illumos-utils/src/opte/firewall_rules.rs b/illumos-utils/src/opte/firewall_rules.rs index 26ab4d6218f..948808832a6 100644 --- a/illumos-utils/src/opte/firewall_rules.rs +++ b/illumos-utils/src/opte/firewall_rules.rs @@ -21,7 +21,6 @@ use oxide_vpc::api::FirewallRule; use oxide_vpc::api::IpAddr; use oxide_vpc::api::Ports; use oxide_vpc::api::ProtoFilter; -use oxide_vpc::api::Protocol; use oxnet::IpNet; trait FromVpcFirewallRule { @@ -100,12 +99,17 @@ impl FromVpcFirewallRule for ResolvedVpcFirewallRule { match self.filter_protocols { Some(ref protos) if !protos.is_empty() => protos .iter() - .map(|proto| { - ProtoFilter::Proto(match proto { - VpcFirewallRuleProtocol::Tcp => Protocol::TCP, - VpcFirewallRuleProtocol::Udp => Protocol::UDP, - VpcFirewallRuleProtocol::Icmp => Protocol::ICMP, - }) + .map(|proto| match proto { + VpcFirewallRuleProtocol::Tcp => ProtoFilter::Tcp, + VpcFirewallRuleProtocol::Udp => ProtoFilter::Udp, + VpcFirewallRuleProtocol::Icmp(v) => { + ProtoFilter::Icmp(v.map(|v| { + oxide_vpc::api::IcmpFilter { + ty: v.icmp_type, + codes: v.code.map(Into::into), + } + })) + } }) .collect(), _ => vec![ProtoFilter::Any], @@ -151,10 +155,19 @@ pub fn opte_firewall_rules( direction, filters: { let mut filters = Filters::new(); + + // Port assignments are incompatible with non + // TCP/UDP protocols. + if matches!( + proto, + ProtoFilter::Tcp | ProtoFilter::Udp + ) { + filters.set_ports(ports.clone()); + } + filters .set_hosts(*hosts) - .set_protocol(*proto) - .set_ports(ports.clone()); + .set_protocol(proto.clone()); filters }, }) diff --git a/nexus/db-fixed-data/src/vpc_firewall_rule.rs b/nexus/db-fixed-data/src/vpc_firewall_rule.rs index d59e1c556e6..ccd0fc2d539 100644 --- a/nexus/db-fixed-data/src/vpc_firewall_rule.rs +++ b/nexus/db-fixed-data/src/vpc_firewall_rule.rs @@ -4,9 +4,10 @@ use nexus_types::identity::Resource; use omicron_common::api::external::{ - L4PortRange, VpcFirewallRuleAction, VpcFirewallRuleDirection, - VpcFirewallRuleFilter, VpcFirewallRulePriority, VpcFirewallRuleProtocol, - VpcFirewallRuleStatus, VpcFirewallRuleTarget, VpcFirewallRuleUpdate, + L4PortRange, VpcFirewallIcmpFilter, VpcFirewallRuleAction, + VpcFirewallRuleDirection, VpcFirewallRuleFilter, VpcFirewallRulePriority, + VpcFirewallRuleProtocol, VpcFirewallRuleStatus, VpcFirewallRuleTarget, + VpcFirewallRuleUpdate, }; use std::sync::LazyLock; @@ -69,3 +70,57 @@ pub static NEXUS_VPC_FW_RULE: LazyLock = action: VpcFirewallRuleAction::Allow, priority: VpcFirewallRulePriority(65534), }); + +/// The name for the built-in VPC firewall rule for Nexus. +pub const NEXUS_ICMP_FW_RULE_NAME: &str = "nexus-icmp"; + +/// Built-in VPC firewall rule for Nexus. +/// +/// This rule allows *arbitrary forwarding nodes* on the network to inform the +/// Nexus/DNS zones that packets have been explicitly dropped. This is a key part +/// in enabling path MTU discovery, such as when customers are accessing Nexus +/// over a VPN. +/// +/// We currently rely on this being exactly one rule to implement the system-level +/// enable/disable endpoint. See `nexus/networking/src/firewall_rules.rs`. +pub static NEXUS_ICMP_FW_RULE: LazyLock = + LazyLock::new(|| VpcFirewallRuleUpdate { + name: NEXUS_ICMP_FW_RULE_NAME.parse().unwrap(), + description: + "allow typical inbound ICMP error codes for outbound flows" + .to_string(), + status: VpcFirewallRuleStatus::Enabled, + direction: VpcFirewallRuleDirection::Inbound, + targets: vec![ + VpcFirewallRuleTarget::Subnet( + super::vpc_subnet::NEXUS_VPC_SUBNET.name().clone(), + ), + VpcFirewallRuleTarget::Subnet( + super::vpc_subnet::DNS_VPC_SUBNET.name().clone(), + ), + ], + filters: VpcFirewallRuleFilter { + hosts: None, + protocols: Some(vec![ + VpcFirewallRuleProtocol::Icmp(Some(VpcFirewallIcmpFilter { + // Type 3 -- Destination Unreachable + icmp_type: 3, + // Codes 3,4 -- Port Unreachable, Fragmentation needed + code: Some((3..=4).into()), + })), + VpcFirewallRuleProtocol::Icmp(Some(VpcFirewallIcmpFilter { + // Type 5 -- Redirect + icmp_type: 5, + code: None, + })), + VpcFirewallRuleProtocol::Icmp(Some(VpcFirewallIcmpFilter { + // Type 11 -- Time Exceeded + icmp_type: 11, + code: None, + })), + ]), + ports: None, + }, + action: VpcFirewallRuleAction::Allow, + priority: VpcFirewallRulePriority(65534), + }); diff --git a/nexus/db-model/src/schema_versions.rs b/nexus/db-model/src/schema_versions.rs index 20f01cee5fa..e2f6ed0088f 100644 --- a/nexus/db-model/src/schema_versions.rs +++ b/nexus/db-model/src/schema_versions.rs @@ -16,7 +16,7 @@ use std::{collections::BTreeMap, sync::LazyLock}; /// /// This must be updated when you change the database schema. Refer to /// schema/crdb/README.adoc in the root of this repository for details. -pub const SCHEMA_VERSION: Version = Version::new(154, 0, 0); +pub const SCHEMA_VERSION: Version = Version::new(155, 0, 0); /// List of all past database schema versions, in *reverse* order /// @@ -28,6 +28,7 @@ static KNOWN_VERSIONS: LazyLock> = LazyLock::new(|| { // | leaving the first copy as an example for the next person. // v // KnownVersion::new(next_int, "unique-dirname-with-the-sql-files"), + KnownVersion::new(155, "vpc-firewall-icmp"), KnownVersion::new(154, "add-pending-mgs-updates"), KnownVersion::new(153, "chicken-switches"), KnownVersion::new(152, "ereports"), diff --git a/nexus/db-model/src/vpc_firewall_rule.rs b/nexus/db-model/src/vpc_firewall_rule.rs index b59c5d1481c..f8183c036d4 100644 --- a/nexus/db-model/src/vpc_firewall_rule.rs +++ b/nexus/db-model/src/vpc_firewall_rule.rs @@ -58,19 +58,29 @@ impl_enum_wrapper!( NewtypeFrom! { () pub struct VpcFirewallRuleAction(external::VpcFirewallRuleAction); } NewtypeDeref! { () pub struct VpcFirewallRuleAction(external::VpcFirewallRuleAction); } -impl_enum_wrapper!( - VpcFirewallRuleProtocolEnum: - - #[derive(Clone, Debug, AsExpression, FromSqlRow, Serialize, Deserialize)] - pub struct VpcFirewallRuleProtocol(pub external::VpcFirewallRuleProtocol); - - Tcp => b"TCP" - Udp => b"UDP" - Icmp => b"ICMP" -); +/// Newtype wrapper around [`external::VpcFirewallRuleProtocol`] so we can derive +/// diesel traits for it +#[derive(Clone, Debug, AsExpression, FromSqlRow, Serialize, Deserialize)] +#[diesel(sql_type = sql_types::Text)] +#[repr(transparent)] +pub struct VpcFirewallRuleProtocol(pub external::VpcFirewallRuleProtocol); NewtypeFrom! { () pub struct VpcFirewallRuleProtocol(external::VpcFirewallRuleProtocol); } NewtypeDeref! { () pub struct VpcFirewallRuleProtocol(external::VpcFirewallRuleProtocol); } +impl DatabaseString for VpcFirewallRuleProtocol { + type Error = ::Err; + + fn to_database_string(&self) -> Cow { + self.0.to_string().into() + } + + fn from_database_string(s: &str) -> Result { + s.parse::().map(Self) + } +} + +impl_from_sql_text!(VpcFirewallRuleProtocol); + /// Newtype wrapper around [`external::VpcFirewallRuleTarget`] so we can derive /// diesel traits for it #[derive(Clone, Debug, AsExpression, FromSqlRow, Serialize, Deserialize)] diff --git a/nexus/db-queries/src/db/datastore/vpc.rs b/nexus/db-queries/src/db/datastore/vpc.rs index c35ae5f76ab..d54496c35d1 100644 --- a/nexus/db-queries/src/db/datastore/vpc.rs +++ b/nexus/db-queries/src/db/datastore/vpc.rs @@ -55,6 +55,7 @@ use nexus_db_fixed_data::vpc::SERVICES_INTERNET_GATEWAY_DEFAULT_ROUTE_V4; use nexus_db_fixed_data::vpc::SERVICES_INTERNET_GATEWAY_DEFAULT_ROUTE_V6; use nexus_db_fixed_data::vpc::SERVICES_INTERNET_GATEWAY_ID; use nexus_db_fixed_data::vpc::SERVICES_VPC_ID; +use nexus_db_fixed_data::vpc_firewall_rule::NEXUS_ICMP_FW_RULE_NAME; use nexus_db_lookup::DbConnection; use nexus_db_model::DbBpZoneDisposition; use nexus_db_model::ExternalIp; @@ -76,8 +77,10 @@ use omicron_common::api::external::ResourceType; use omicron_common::api::external::RouteDestination; use omicron_common::api::external::RouteTarget; use omicron_common::api::external::RouterRouteKind as ExternalRouteKind; +use omicron_common::api::external::ServiceIcmpConfig; use omicron_common::api::external::UpdateResult; use omicron_common::api::external::Vni as ExternalVni; +use omicron_common::api::external::VpcFirewallRuleStatus; use omicron_common::api::external::http_pagination::PaginatedBy; use omicron_common::api::internal::shared::InternetGatewayRouterTarget; use omicron_common::api::internal::shared::ResolvedVpcRoute; @@ -235,6 +238,7 @@ impl DataStore { opctx: &OpContext, ) -> Result<(), Error> { use nexus_db_fixed_data::vpc_firewall_rule::DNS_VPC_FW_RULE; + use nexus_db_fixed_data::vpc_firewall_rule::NEXUS_ICMP_FW_RULE; use nexus_db_fixed_data::vpc_firewall_rule::NEXUS_VPC_FW_RULE; debug!(opctx.log, "attempting to create built-in VPC firewall rules"); @@ -273,6 +277,15 @@ impl DataStore { fw_rules.insert(NEXUS_VPC_FW_RULE.name.clone(), rule); } + if !fw_rules.contains_key(&NEXUS_ICMP_FW_RULE.name) { + let rule = VpcFirewallRule::new( + Uuid::new_v4(), + *SERVICES_VPC_ID, + &NEXUS_ICMP_FW_RULE, + )?; + fw_rules.insert(NEXUS_ICMP_FW_RULE.name.clone(), rule); + } + let rules = fw_rules .into_values() .map(|mut rule| { @@ -740,6 +753,80 @@ impl DataStore { }) } + pub async fn nexus_inbound_icmp_view( + &self, + opctx: &OpContext, + ) -> Result { + opctx.authorize(authz::Action::Read, &authz::FLEET).await?; + use nexus_db_schema::schema::vpc_firewall_rule::dsl; + + let conn = self.pool_connection_authorized(opctx).await?; + + let rule = dsl::vpc_firewall_rule + .filter(dsl::time_deleted.is_null()) + .filter(dsl::vpc_id.eq(*SERVICES_VPC_ID)) + .filter(dsl::name.eq(NEXUS_ICMP_FW_RULE_NAME)) + .limit(1) + .select(VpcFirewallRule::as_select()) + .get_result_async::(&*conn) + .await + .optional() + .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?; + + if let Some(rule) = rule { + Ok(ServiceIcmpConfig { + enabled: rule.status.0 == VpcFirewallRuleStatus::Enabled, + }) + } else { + Err(Error::internal_error(&format!( + "services VPC is missing the builtin firewall rule \ + {NEXUS_ICMP_FW_RULE_NAME}" + ))) + } + } + + pub async fn nexus_inbound_icmp_update( + &self, + opctx: &OpContext, + config: ServiceIcmpConfig, + ) -> Result { + opctx.authorize(authz::Action::Modify, &authz::FLEET).await?; + use nexus_db_schema::schema::vpc_firewall_rule::dsl; + + let ServiceIcmpConfig { enabled } = config; + + let conn = self.pool_connection_authorized(opctx).await?; + + let status = nexus_db_model::VpcFirewallRuleStatus(if enabled { + VpcFirewallRuleStatus::Enabled + } else { + VpcFirewallRuleStatus::Disabled + }); + + let now = Utc::now(); + let rule = diesel::update(dsl::vpc_firewall_rule) + .filter(dsl::time_deleted.is_null()) + .filter(dsl::vpc_id.eq(*SERVICES_VPC_ID)) + .filter(dsl::name.eq(NEXUS_ICMP_FW_RULE_NAME)) + .set((dsl::time_modified.eq(now), dsl::status.eq(status))) + .returning(VpcFirewallRule::as_returning()) + .get_result_async(&*conn) + .await + .optional() + .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?; + + if let Some(rule) = rule { + Ok(ServiceIcmpConfig { + enabled: rule.status.0 == VpcFirewallRuleStatus::Enabled, + }) + } else { + Err(Error::internal_error(&format!( + "services VPC is missing the builtin firewall rule \ + {NEXUS_ICMP_FW_RULE_NAME}" + ))) + } + } + /// Return the list of `Sled`s hosting instances or control plane services /// with network interfaces on the provided VPC. pub async fn vpc_resolve_to_sleds( diff --git a/nexus/db-schema/src/enums.rs b/nexus/db-schema/src/enums.rs index f75d40beb3d..e4391e3be70 100644 --- a/nexus/db-schema/src/enums.rs +++ b/nexus/db-schema/src/enums.rs @@ -88,7 +88,6 @@ define_enums! { VolumeResourceUsageTypeEnum => "volume_resource_usage_type", VpcFirewallRuleActionEnum => "vpc_firewall_rule_action", VpcFirewallRuleDirectionEnum => "vpc_firewall_rule_direction", - VpcFirewallRuleProtocolEnum => "vpc_firewall_rule_protocol", VpcFirewallRuleStatusEnum => "vpc_firewall_rule_status", VpcRouterKindEnum => "vpc_router_kind", WebhookDeliveryAttemptResultEnum => "webhook_delivery_attempt_result", diff --git a/nexus/db-schema/src/schema.rs b/nexus/db-schema/src/schema.rs index 385178b9e7e..f5599acb2a0 100644 --- a/nexus/db-schema/src/schema.rs +++ b/nexus/db-schema/src/schema.rs @@ -1307,9 +1307,9 @@ table! { targets -> Array, filter_hosts -> Nullable>, filter_ports -> Nullable>, - filter_protocols -> Nullable>, action -> crate::enums::VpcFirewallRuleActionEnum, priority -> Int4, + filter_protocols -> Nullable>, } } diff --git a/nexus/defaults/src/lib.rs b/nexus/defaults/src/lib.rs index 73c090a4d1b..6f6afe171f5 100644 --- a/nexus/defaults/src/lib.rs +++ b/nexus/defaults/src/lib.rs @@ -5,6 +5,18 @@ //! Default values for data in the Nexus API, when not provided explicitly in a request. use omicron_common::api::external; +use omicron_common::api::external::L4Port; +use omicron_common::api::external::Name; +use omicron_common::api::external::VpcFirewallRuleAction; +use omicron_common::api::external::VpcFirewallRuleDirection; +use omicron_common::api::external::VpcFirewallRuleFilter; +use omicron_common::api::external::VpcFirewallRuleHostFilter; +use omicron_common::api::external::VpcFirewallRulePriority; +use omicron_common::api::external::VpcFirewallRuleProtocol; +use omicron_common::api::external::VpcFirewallRuleStatus; +use omicron_common::api::external::VpcFirewallRuleTarget; +use omicron_common::api::external::VpcFirewallRuleUpdate; +use omicron_common::api::external::VpcFirewallRuleUpdateParams; use oxnet::Ipv4Net; use oxnet::Ipv6Net; use std::net::Ipv4Addr; @@ -21,44 +33,63 @@ pub const DEFAULT_PRIMARY_NIC_NAME: &str = "net0"; pub static DEFAULT_VPC_SUBNET_IPV4_BLOCK: LazyLock = LazyLock::new(|| Ipv4Net::new(Ipv4Addr::new(172, 30, 0, 0), 22).unwrap()); -pub static DEFAULT_FIREWALL_RULES: LazyLock< - external::VpcFirewallRuleUpdateParams, -> = LazyLock::new(|| { - serde_json::from_str(r#"{ - "rules": [ - { - "name": "allow-internal-inbound", - "status": "enabled", - "direction": "inbound", - "targets": [ { "type": "vpc", "value": "default" } ], - "filters": { "hosts": [ { "type": "vpc", "value": "default" } ] }, - "action": "allow", - "priority": 65534, - "description": "allow inbound traffic to all instances within the VPC if originated within the VPC" - }, - { - "name": "allow-ssh", - "status": "enabled", - "direction": "inbound", - "targets": [ { "type": "vpc", "value": "default" } ], - "filters": { "ports": [ "22" ], "protocols": [ "TCP" ] }, - "action": "allow", - "priority": 65534, - "description": "allow inbound TCP connections on port 22 from anywhere" - }, - { - "name": "allow-icmp", - "status": "enabled", - "direction": "inbound", - "targets": [ { "type": "vpc", "value": "default" } ], - "filters": { "protocols": [ "ICMP" ] }, - "action": "allow", - "priority": 65534, - "description": "allow inbound ICMP traffic from anywhere" - } - ] - }"#).unwrap() -}); +pub static DEFAULT_FIREWALL_RULES: LazyLock = + LazyLock::new(|| { + let default: Name = "default".parse().unwrap(); + let targets = vec![VpcFirewallRuleTarget::Vpc(default.clone())]; + VpcFirewallRuleUpdateParams { + rules: vec![ + VpcFirewallRuleUpdate { + name: "allow-internal-inbound".parse().unwrap(), + description: + "allow inbound traffic to all instances within the VPC if originated within the VPC" + .to_string(), + status: VpcFirewallRuleStatus::Enabled, + direction: VpcFirewallRuleDirection::Inbound, + targets: targets.clone(), + filters: VpcFirewallRuleFilter { + hosts: Some(vec![VpcFirewallRuleHostFilter::Vpc(default)]), + protocols: None, + ports: None, + }, + action: VpcFirewallRuleAction::Allow, + priority: VpcFirewallRulePriority(65534), + }, + VpcFirewallRuleUpdate { + name: "allow-ssh".parse().unwrap(), + description: + "allow inbound TCP connections on port 22 from anywhere" + .to_string(), + status: VpcFirewallRuleStatus::Enabled, + direction: VpcFirewallRuleDirection::Inbound, + targets: targets.clone(), + filters: VpcFirewallRuleFilter { + hosts: None, + protocols: Some(vec![VpcFirewallRuleProtocol::Tcp]), + ports: Some(vec![L4Port::try_from(22u16).unwrap().into()]), + }, + action: VpcFirewallRuleAction::Allow, + priority: VpcFirewallRulePriority(65534), + }, + VpcFirewallRuleUpdate { + name: "allow-icmp".parse().unwrap(), + description: + "allow inbound ICMP traffic from anywhere" + .to_string(), + status: VpcFirewallRuleStatus::Enabled, + direction: VpcFirewallRuleDirection::Inbound, + targets, + filters: VpcFirewallRuleFilter { + hosts: None, + protocols: Some(vec![VpcFirewallRuleProtocol::Icmp(None)]), + ports: None, + }, + action: VpcFirewallRuleAction::Allow, + priority: VpcFirewallRulePriority(65534), + }, + ] + } + }); /// Generate a random VPC IPv6 prefix, in the range `fd00::/48`. pub fn random_vpc_ipv6_prefix() -> Result { diff --git a/nexus/external-api/output/nexus_tags.txt b/nexus/external-api/output/nexus_tags.txt index fcaa92f442a..8e520a5276c 100644 --- a/nexus/external-api/output/nexus_tags.txt +++ b/nexus/external-api/output/nexus_tags.txt @@ -252,6 +252,8 @@ networking_bgp_exported GET /v1/system/networking/bgp-expo networking_bgp_imported_routes_ipv4 GET /v1/system/networking/bgp-routes-ipv4 networking_bgp_message_history GET /v1/system/networking/bgp-message-history networking_bgp_status GET /v1/system/networking/bgp-status +networking_inbound_icmp_update PUT /v1/system/networking/inbound-icmp +networking_inbound_icmp_view GET /v1/system/networking/inbound-icmp networking_loopback_address_create POST /v1/system/networking/loopback-address networking_loopback_address_delete DELETE /v1/system/networking/loopback-address/{rack_id}/{switch_location}/{address}/{subnet_mask} networking_loopback_address_list GET /v1/system/networking/loopback-address diff --git a/nexus/external-api/src/lib.rs b/nexus/external-api/src/lib.rs index 897e9c71c45..bc15efc8141 100644 --- a/nexus/external-api/src/lib.rs +++ b/nexus/external-api/src/lib.rs @@ -2007,6 +2007,27 @@ pub trait NexusExternalApi { params: TypedBody, ) -> Result, HttpError>; + /// Return whether API services can receive limited ICMP traffic + #[endpoint { + method = GET, + path = "/v1/system/networking/inbound-icmp", + tags = ["system/networking"], + }] + async fn networking_inbound_icmp_view( + rqctx: RequestContext, + ) -> Result, HttpError>; + + /// Set whether API services can receive limited ICMP traffic + #[endpoint { + method = PUT, + path = "/v1/system/networking/inbound-icmp", + tags = ["system/networking"], + }] + async fn networking_inbound_icmp_update( + rqctx: RequestContext, + params: TypedBody, + ) -> Result; + // Images /// List images diff --git a/nexus/networking/src/firewall_rules.rs b/nexus/networking/src/firewall_rules.rs index 5dba6d3bfbf..95fbb921d2f 100644 --- a/nexus/networking/src/firewall_rules.rs +++ b/nexus/networking/src/firewall_rules.rs @@ -316,6 +316,11 @@ pub async fn resolve_firewall_rules_for_sled_agent( // `nexus_db_queries::fixed_data::vpc_firewall_rule` for those // rules.) If those rules change to include any filter hosts, this // logic needs to change as well. + // + // Note that inbound ICMP is not currently governed by this filter, + // as error-type ICMP messages can arrive from any host which a + // Nexus/DNS zone reaches out to *or* a gateway/router on the path to + // that destination. (None, Some(allowed_ips)) => { if allowlist_applies_to_firewall_rule(rule) { match allowed_ips { diff --git a/nexus/src/app/vpc.rs b/nexus/src/app/vpc.rs index 23a03bb3574..32582f0d05b 100644 --- a/nexus/src/app/vpc.rs +++ b/nexus/src/app/vpc.rs @@ -23,6 +23,7 @@ use omicron_common::api::external::ListResultVec; use omicron_common::api::external::LookupResult; use omicron_common::api::external::LookupType; use omicron_common::api::external::NameOrId; +use omicron_common::api::external::ServiceIcmpConfig; use omicron_common::api::external::UpdateResult; use omicron_common::api::external::VpcFirewallRuleUpdateParams; use omicron_common::api::external::http_pagination::PaginatedBy; @@ -272,4 +273,58 @@ impl super::Nexus { ) .await } + + pub async fn nexus_firewall_inbound_icmp_view( + &self, + opctx: &OpContext, + ) -> Result { + self.datastore().nexus_inbound_icmp_view(opctx).await + } + + pub async fn nexus_firewall_inbound_icmp_update( + &self, + opctx: &OpContext, + config: ServiceIcmpConfig, + ) -> Result { + let out = + self.datastore().nexus_inbound_icmp_update(opctx, config).await?; + + // Notify the sled-agents of the updated firewall rules. + // + // This code comes directly from `Nexus::allow_list_upsert`, where + // there is substantial commentary on the impact of changing the logger, + // actor, etc. + info!( + opctx.log, + "updated user-facing services ICMP status, switching to \ + internal opcontext to plumb rules to sled-agents"; + "icmp-allowed" => ?config.enabled, + ); + let new_opctx = self.opctx_for_internal_api(); + match nexus_networking::plumb_service_firewall_rules( + self.datastore(), + &new_opctx, + &[], + &new_opctx, + &new_opctx.log, + ) + .await + { + Ok(_) => { + info!(self.log, "plumbed updated ICMP status to sled-agents"); + Ok(out) + } + Err(e) => { + let message = "Failed to plumb ICMP status as firewall rules \ + to relevant sled agents. The request must be retried for them \ + to take effect."; + error!( + self.log, + "failed to update sled-agents with new ICMP status"; + "error" => format!("{message}: {e:?}"), + ); + Err(Error::unavail(message)) + } + } + } } diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index ba587836c7e..9f31c067505 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -80,6 +80,7 @@ use omicron_common::api::external::NameOrId; use omicron_common::api::external::Probe; use omicron_common::api::external::RouterRoute; use omicron_common::api::external::RouterRouteKind; +use omicron_common::api::external::ServiceIcmpConfig; use omicron_common::api::external::SwitchPort; use omicron_common::api::external::SwitchPortSettings; use omicron_common::api::external::SwitchPortSettingsIdentity; @@ -4303,6 +4304,50 @@ impl NexusExternalApi for NexusExternalApiImpl { .await } + async fn networking_inbound_icmp_view( + rqctx: RequestContext, + ) -> Result, HttpError> { + let apictx = rqctx.context(); + let handler = async { + let nexus = &apictx.context.nexus; + let opctx = + crate::context::op_context_for_external_api(&rqctx).await?; + nexus + .nexus_firewall_inbound_icmp_view(&opctx) + .await + .map(HttpResponseOk) + .map_err(HttpError::from) + }; + apictx + .context + .external_latencies + .instrument_dropshot_handler(&rqctx, handler) + .await + } + + async fn networking_inbound_icmp_update( + rqctx: RequestContext, + params: TypedBody, + ) -> Result { + let apictx = rqctx.context(); + let handler = async { + let nexus = &apictx.context.nexus; + let params = params.into_inner(); + let opctx = + crate::context::op_context_for_external_api(&rqctx).await?; + nexus + .nexus_firewall_inbound_icmp_update(&opctx, params) + .await + .map(|_| HttpResponseUpdatedNoContent()) + .map_err(HttpError::from) + }; + apictx + .context + .external_latencies + .instrument_dropshot_handler(&rqctx, handler) + .await + } + // Images async fn image_list( diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs index 6d6aea75d0c..944b37aeede 100644 --- a/nexus/tests/integration_tests/endpoints.rs +++ b/nexus/tests/integration_tests/endpoints.rs @@ -37,6 +37,7 @@ use omicron_common::api::external::NameOrId; use omicron_common::api::external::Nullable; use omicron_common::api::external::RouteDestination; use omicron_common::api::external::RouteTarget; +use omicron_common::api::external::ServiceIcmpConfig; use omicron_common::api::external::UserId; use omicron_common::api::external::VpcFirewallRuleUpdateParams; use omicron_test_utils::certificates::CertificateChain; @@ -1244,6 +1245,12 @@ pub static DEMO_WEBHOOK_SECRET_CREATE: LazyLock = secret: "TRUSTNO1".to_string(), }); +pub static DEMO_INBOUND_ICMP_URL: &'static str = + "/v1/system/networking/inbound-icmp"; + +pub static DEMO_INBOUND_ICMP_UPDATE: LazyLock = + LazyLock::new(|| ServiceIcmpConfig { enabled: true }); + /// Describes an API endpoint to be verified by the "unauthorized" test /// /// These structs are also used to check whether we're covering all endpoints in @@ -2862,6 +2869,19 @@ pub static VERIFY_ENDPOINTS: LazyLock> = ), ], }, + // User-facing services inbound ICMP allow/block + VerifyEndpoint { + url: DEMO_INBOUND_ICMP_URL, + visibility: Visibility::Public, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![ + AllowedMethod::Get, + AllowedMethod::Put( + serde_json::to_value(&*DEMO_INBOUND_ICMP_UPDATE) + .unwrap(), + ), + ], + }, // Alerts VerifyEndpoint { url: &WEBHOOK_RECEIVERS_URL, diff --git a/nexus/tests/integration_tests/schema.rs b/nexus/tests/integration_tests/schema.rs index 5f1215ec5f4..dda6195ee11 100644 --- a/nexus/tests/integration_tests/schema.rs +++ b/nexus/tests/integration_tests/schema.rs @@ -918,6 +918,12 @@ const VOLUME2: Uuid = Uuid::from_u128(0x2222566f_5c3d_4647_83b0_8f3515da7be1); const VOLUME3: Uuid = Uuid::from_u128(0x3333566f_5c3d_4647_83b0_8f3515da7be1); const VOLUME4: Uuid = Uuid::from_u128(0x4444566f_5c3d_4647_83b0_8f3515da7be1); +// "566C" -> "VPC". See above on "V". +const VPC: Uuid = Uuid::from_u128(0x1111566c_5c3d_4647_83b0_8f3515da7be1); + +// "7213" -> Firewall "Rule" +const FW_RULE: Uuid = Uuid::from_u128(0x11117213_5c3d_4647_83b0_8f3515da7be1); + fn before_23_0_0<'a>(ctx: &'a MigrationContext<'a>) -> BoxFuture<'a, ()> { Box::pin(async move { // Create two silos @@ -2395,6 +2401,110 @@ fn after_151_0_0<'a>(ctx: &'a MigrationContext<'a>) -> BoxFuture<'a, ()> { }) } +fn before_155_0_0<'a>(ctx: &'a MigrationContext<'a>) -> BoxFuture<'a, ()> { + Box::pin(async { + use nexus_db_queries::db::fixed_data::project::*; + use nexus_db_queries::db::fixed_data::vpc::*; + use omicron_common::address::SERVICE_VPC_IPV6_PREFIX; + + // First, create the oxide-services vpc. The migration will only add + // a new rule if it detects the presence of *this* VPC entry (i.e., + // RSS has run before). + let svc_vpc = *SERVICES_VPC_ID; + let svc_proj = *SERVICES_PROJECT_ID; + let svc_router = *SERVICES_VPC_ROUTER_ID; + let svc_vpc_name = &SERVICES_DB_NAME; + ctx.client + .batch_execute(&format!( + "INSERT INTO vpc ( + id, name, description, time_created, time_modified, + project_id, system_router_id, dns_name, + vni, ipv6_prefix, firewall_gen, subnet_gen + ) + VALUES ( + '{svc_vpc}', '{svc_vpc_name}', '', now(), now(), + '{svc_proj}', '{svc_router}', '{svc_vpc_name}', + 100, '{}', 0, 0 + );", + *SERVICE_VPC_IPV6_PREFIX + )) + .await + .expect("failed to create services vpc record"); + + // Second, create a firewall rule in a dummied-out VPC to validate + // that we correctly convert from ENUM[] to STRING[]. + ctx.client + .batch_execute(&format!( + "INSERT INTO vpc_firewall_rule ( + id, + name, description, + time_created, time_modified, vpc_id, status, direction, + targets, filter_protocols, + action, priority + ) + VALUES ( + '{FW_RULE}', + 'test-fw', '', + now(), now(), '{VPC}', 'enabled', 'outbound', + ARRAY['vpc:fiction'], ARRAY['ICMP', 'TCP', 'UDP'], + 'allow', 1234 + );" + )) + .await + .expect("failed to create firewall rule"); + }) +} + +fn after_155_0_0<'a>(ctx: &'a MigrationContext<'a>) -> BoxFuture<'a, ()> { + Box::pin(async { + use nexus_db_queries::db::fixed_data::vpc::*; + // Firstly -- has the new firewall rule been added? + let rows = ctx + .client + .query( + &format!( + "SELECT id, description + FROM vpc_firewall_rule + WHERE name = 'nexus-icmp' and vpc_id = '{}'", + *SERVICES_VPC_ID + ), + &[], + ) + .await + .expect("failed to load instance auto-restart policies"); + let records = process_rows(&rows); + + assert_eq!(records.len(), 1); + + // Secondly, have FW_RULE's filter_protocols been correctly stringified? + let rows = ctx + .client + .query( + &format!( + "SELECT filter_protocols + FROM vpc_firewall_rule + WHERE id = '{FW_RULE}'" + ), + &[], + ) + .await + .expect("failed to load instance auto-restart policies"); + let records = process_rows(&rows); + assert_eq!(records.len(), 1); + assert_eq!( + records[0].values, + vec![ColumnValue::new( + "filter_protocols", + AnySqlType::TextArray(vec![ + "icmp".into(), + "tcp".into(), + "udp".into(), + ]) + )], + ); + }) +} + // Lazily initializes all migration checks. The combination of Rust function // pointers and async makes defining a static table fairly painful, so we're // using lazy initialization instead. @@ -2471,6 +2581,10 @@ fn get_migration_checks() -> BTreeMap { Version::new(151, 0, 0), DataMigrationFns::new().before(before_151_0_0).after(after_151_0_0), ); + map.insert( + Version::new(155, 0, 0), + DataMigrationFns::new().before(before_155_0_0).after(after_155_0_0), + ); map } diff --git a/nexus/tests/integration_tests/vpc_firewall.rs b/nexus/tests/integration_tests/vpc_firewall.rs index 83bc5599ba8..91280d8d95e 100644 --- a/nexus/tests/integration_tests/vpc_firewall.rs +++ b/nexus/tests/integration_tests/vpc_firewall.rs @@ -4,20 +4,28 @@ use http::StatusCode; use http::method::Method; -use nexus_test_utils::http_testing::{AuthnMode, NexusRequest}; +use nexus_auth::authn; +use nexus_auth::context::OpContext; +use nexus_db_lookup::LookupPath; +use nexus_db_queries::db::fixed_data::vpc_firewall_rule::NEXUS_ICMP_FW_RULE_NAME; +use nexus_db_queries::db::{self, DataStore}; +use nexus_networking::vpc_list_firewall_rules; +use nexus_test_utils::http_testing::{AuthnMode, NexusRequest, RequestBuilder}; use nexus_test_utils::resource_helpers::{ create_project, create_vpc, object_get, object_put, object_put_error, }; use nexus_test_utils_macros::nexus_test; use nexus_types::external_api::views::Vpc; use omicron_common::api::external::{ - IdentityMetadata, L4Port, L4PortRange, VpcFirewallRule, + IdentityMetadata, L4Port, L4PortRange, ServiceIcmpConfig, VpcFirewallRule, VpcFirewallRuleAction, VpcFirewallRuleDirection, VpcFirewallRuleFilter, VpcFirewallRuleHostFilter, VpcFirewallRulePriority, VpcFirewallRuleProtocol, VpcFirewallRuleStatus, VpcFirewallRuleTarget, VpcFirewallRuleUpdate, VpcFirewallRuleUpdateParams, VpcFirewallRules, }; +use omicron_nexus::Nexus; use std::convert::TryFrom; +use std::sync::Arc; use uuid::Uuid; type ControlPlaneTestContext = @@ -90,7 +98,7 @@ async fn test_vpc_firewall(cptestctx: &ControlPlaneTestContext) { filters: VpcFirewallRuleFilter { hosts: None, ports: None, - protocols: Some(vec![VpcFirewallRuleProtocol::Icmp]), + protocols: Some(vec![VpcFirewallRuleProtocol::Icmp(None)]), }, direction: VpcFirewallRuleDirection::Inbound, priority: VpcFirewallRulePriority(10), @@ -191,7 +199,7 @@ fn is_default_firewall_rules( )], filters: VpcFirewallRuleFilter { hosts: None, - protocols: Some(vec![VpcFirewallRuleProtocol::Icmp]), + protocols: Some(vec![VpcFirewallRuleProtocol::Icmp(None)]), ports: None, }, action: VpcFirewallRuleAction::Allow, @@ -582,3 +590,84 @@ async fn test_firewall_rules_max_lengths(cptestctx: &ControlPlaneTestContext) { ) ); } + +async fn icmp_rule_is_enabled( + enabled: bool, + datastore: &DataStore, + nexus: &Nexus, + opctx: &OpContext, +) -> bool { + // (Partly) reimplementing opctx_for_internal_api. + // This is necessary to *reach* the services VPC. + let opctx = OpContext::for_background( + opctx.log.clone(), + Arc::clone(nexus.authz()), + authn::Context::internal_api(), + Arc::clone(nexus.datastore()) as Arc, + ); + let svcs_vpc = LookupPath::new(&opctx, datastore) + .vpc_id(*db::fixed_data::vpc::SERVICES_VPC_ID); + + let fw_rules = + vpc_list_firewall_rules(&datastore, &opctx, &svcs_vpc).await.unwrap(); + let the_rule = fw_rules + .iter() + .find(|v| v.identity.name.as_str() == NEXUS_ICMP_FW_RULE_NAME) + .unwrap(); + + the_rule.status.0 + == if enabled { + VpcFirewallRuleStatus::Enabled + } else { + VpcFirewallRuleStatus::Disabled + } +} + +#[nexus_test] +async fn test_nexus_firewall_icmp(cptestctx: &ControlPlaneTestContext) { + let client = &cptestctx.external_client; + let apictx = &cptestctx.server.server_context(); + let nexus = &apictx.nexus; + let datastore = nexus.datastore(); + let opctx = + OpContext::for_tests(cptestctx.logctx.log.new(o!()), datastore.clone()); + + const NEXUS_ICMP_URL: &str = "/v1/system/networking/inbound-icmp"; + + // ICMP access should be enabled by default. + let icmp_allowed: ServiceIcmpConfig = + NexusRequest::object_get(client, NEXUS_ICMP_URL) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap() + .parsed_body() + .unwrap(); + + assert!(icmp_allowed.enabled); + assert!(icmp_rule_is_enabled(true, datastore, nexus, &opctx).await); + + // Disabling this should propagate to the rule. + NexusRequest::new( + RequestBuilder::new(client, http::Method::PUT, NEXUS_ICMP_URL) + .body(Some(&ServiceIcmpConfig { enabled: false })) + .expect_status(Some(http::StatusCode::NO_CONTENT)), + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap(); + assert!(icmp_rule_is_enabled(false, datastore, nexus, &opctx).await); + + // Now, re-anable the rule. + NexusRequest::new( + RequestBuilder::new(client, http::Method::PUT, NEXUS_ICMP_URL) + .body(Some(&ServiceIcmpConfig { enabled: true })) + .expect_status(Some(http::StatusCode::NO_CONTENT)), + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap(); + assert!(icmp_rule_is_enabled(true, datastore, nexus, &opctx).await); +} diff --git a/openapi/nexus.json b/openapi/nexus.json index 5306f4417e5..216744207d5 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -9842,6 +9842,61 @@ } } }, + "/v1/system/networking/inbound-icmp": { + "get": { + "tags": [ + "system/networking" + ], + "summary": "Return whether API services can receive limited ICMP traffic", + "operationId": "networking_inbound_icmp_view", + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ServiceIcmpConfig" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + }, + "put": { + "tags": [ + "system/networking" + ], + "summary": "Set whether API services can receive limited ICMP traffic", + "operationId": "networking_inbound_icmp_update", + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ServiceIcmpConfig" + } + } + }, + "required": true + }, + "responses": { + "204": { + "description": "resource updated" + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, "/v1/system/networking/loopback-address": { "get": { "tags": [ @@ -18990,6 +19045,15 @@ "minLength": 1, "maxLength": 253 }, + "IcmpParamRange": { + "example": "3", + "title": "A range of ICMP(v6) types or codes", + "description": "An inclusive-inclusive range of ICMP(v6) types or codes. The second value may be omitted to represent a single parameter.", + "type": "string", + "pattern": "^[0-9]{1,3}(-[0-9]{1,3})?$", + "minLength": 1, + "maxLength": 7 + }, "IdentityProvider": { "description": "View of an Identity Provider", "type": "object", @@ -22776,6 +22840,19 @@ "technical_contact_email" ] }, + "ServiceIcmpConfig": { + "description": "Configuration of inbound ICMP allowed by API services.", + "type": "object", + "properties": { + "enabled": { + "description": "When enabled, Nexus is able to receive ICMP Destination Unreachable type 3 (port unreachable) and type 4 (fragmentation needed), Redirect, and Time Exceeded messages. These enable Nexus to perform Path MTU discovery and better cope with fragmentation issues. Otherwise all inbound ICMP traffic will be dropped.", + "type": "boolean" + } + }, + "required": [ + "enabled" + ] + }, "ServiceUsingCertificate": { "description": "The service intended to use this certificate.", "oneOf": [ @@ -25850,6 +25927,27 @@ "name" ] }, + "VpcFirewallIcmpFilter": { + "type": "object", + "properties": { + "code": { + "nullable": true, + "allOf": [ + { + "$ref": "#/components/schemas/IcmpParamRange" + } + ] + }, + "icmp_type": { + "type": "integer", + "format": "uint8", + "minimum": 0 + } + }, + "required": [ + "icmp_type" + ] + }, "VpcFirewallRule": { "description": "A single rule in a VPC firewall", "type": "object", @@ -26097,11 +26195,58 @@ }, "VpcFirewallRuleProtocol": { "description": "The protocols that may be specified in a firewall rule's filter", - "type": "string", - "enum": [ - "TCP", - "UDP", - "ICMP" + "oneOf": [ + { + "type": "object", + "properties": { + "type": { + "type": "string", + "enum": [ + "tcp" + ] + } + }, + "required": [ + "type" + ] + }, + { + "type": "object", + "properties": { + "type": { + "type": "string", + "enum": [ + "udp" + ] + } + }, + "required": [ + "type" + ] + }, + { + "type": "object", + "properties": { + "type": { + "type": "string", + "enum": [ + "icmp" + ] + }, + "value": { + "nullable": true, + "allOf": [ + { + "$ref": "#/components/schemas/VpcFirewallIcmpFilter" + } + ] + } + }, + "required": [ + "type", + "value" + ] + } ] }, "VpcFirewallRuleStatus": { diff --git a/openapi/sled-agent.json b/openapi/sled-agent.json index 0dfd23b535c..97a9fc3c6a6 100644 --- a/openapi/sled-agent.json +++ b/openapi/sled-agent.json @@ -4588,6 +4588,15 @@ ], "additionalProperties": false }, + "IcmpParamRange": { + "example": "3", + "title": "A range of ICMP(v6) types or codes", + "description": "An inclusive-inclusive range of ICMP(v6) types or codes. The second value may be omitted to represent a single parameter.", + "type": "string", + "pattern": "^[0-9]{1,3}(-[0-9]{1,3})?$", + "minLength": 1, + "maxLength": 7 + }, "IdMapDatasetConfig": { "type": "object", "additionalProperties": { @@ -7687,6 +7696,27 @@ "format": "uint32", "minimum": 0 }, + "VpcFirewallIcmpFilter": { + "type": "object", + "properties": { + "code": { + "nullable": true, + "allOf": [ + { + "$ref": "#/components/schemas/IcmpParamRange" + } + ] + }, + "icmp_type": { + "type": "integer", + "format": "uint8", + "minimum": 0 + } + }, + "required": [ + "icmp_type" + ] + }, "VpcFirewallRuleAction": { "type": "string", "enum": [ @@ -7703,11 +7733,58 @@ }, "VpcFirewallRuleProtocol": { "description": "The protocols that may be specified in a firewall rule's filter", - "type": "string", - "enum": [ - "TCP", - "UDP", - "ICMP" + "oneOf": [ + { + "type": "object", + "properties": { + "type": { + "type": "string", + "enum": [ + "tcp" + ] + } + }, + "required": [ + "type" + ] + }, + { + "type": "object", + "properties": { + "type": { + "type": "string", + "enum": [ + "udp" + ] + } + }, + "required": [ + "type" + ] + }, + { + "type": "object", + "properties": { + "type": { + "type": "string", + "enum": [ + "icmp" + ] + }, + "value": { + "nullable": true, + "allOf": [ + { + "$ref": "#/components/schemas/VpcFirewallIcmpFilter" + } + ] + } + }, + "required": [ + "type", + "value" + ] + } ] }, "VpcFirewallRuleStatus": { diff --git a/package-manifest.toml b/package-manifest.toml index 88a764d66d9..60c78f2ce1e 100644 --- a/package-manifest.toml +++ b/package-manifest.toml @@ -638,10 +638,10 @@ source.repo = "maghemite" # `tools/maghemite_openapi_version`. Failing to do so will cause a failure when # building `ddm-admin-client` (which will instruct you to update # `tools/maghemite_openapi_version`). -source.commit = "760b4b547e301a31d4dcb92ba97aabdb2a3e0cba" +source.commit = "638c897d5ed1e5d3f2de0c1cb9dfaa4d77a35dea" # The SHA256 digest is automatically posted to: # https://buildomat.eng.oxide.computer/public/file/oxidecomputer/maghemite/image//mg-ddm-gz.sha256.txt -source.sha256 = "181f5e4605b9c9f2395cb5044d523e2ce0b66420036f6ec67595993ff7b0d7af" +source.sha256 = "9a528e6af530c8a5dcd180a41e17ebb1adb05e336550c1ac2134acca1d6557ab" output.type = "tarball" [package.mg-ddm] @@ -654,10 +654,10 @@ source.repo = "maghemite" # `tools/maghemite_openapi_version`. Failing to do so will cause a failure when # building `ddm-admin-client` (which will instruct you to update # `tools/maghemite_openapi_version`). -source.commit = "760b4b547e301a31d4dcb92ba97aabdb2a3e0cba" +source.commit = "638c897d5ed1e5d3f2de0c1cb9dfaa4d77a35dea" # The SHA256 digest is automatically posted to: # https://buildomat.eng.oxide.computer/public/file/oxidecomputer/maghemite/image//mg-ddm.sha256.txt -source.sha256 = "3d5a3d0ed9eadbe599eefd38844b040bd151d98274ac1b60562f4ff0e42e3d83" +source.sha256 = "275bf1088c548784e6a5eea7dd91db1878be1c44696a89f506b2a0515a5138ef" output.type = "zone" output.intermediate_only = true @@ -669,10 +669,10 @@ source.repo = "maghemite" # `tools/maghemite_openapi_version`. Failing to do so will cause a failure when # building `ddm-admin-client` (which will instruct you to update # `tools/maghemite_openapi_version`). -source.commit = "760b4b547e301a31d4dcb92ba97aabdb2a3e0cba" +source.commit = "638c897d5ed1e5d3f2de0c1cb9dfaa4d77a35dea" # The SHA256 digest is automatically posted to: # https://buildomat.eng.oxide.computer/public/file/oxidecomputer/maghemite/image//mgd.sha256.txt -source.sha256 = "5f87a6bd81e5bc83e4755dba31a0df75883888b00596fb70a5c5200dfedbdddd" +source.sha256 = "6451cdc1473e555676bc33eb3e94f96205a2d839120318b3dfd4600232f95626" output.type = "zone" output.intermediate_only = true diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql index 86fc3fbb115..8806c09c695 100644 --- a/schema/crdb/dbinit.sql +++ b/schema/crdb/dbinit.sql @@ -1868,12 +1868,6 @@ CREATE TYPE IF NOT EXISTS omicron.public.vpc_firewall_rule_action AS ENUM ( 'deny' ); -CREATE TYPE IF NOT EXISTS omicron.public.vpc_firewall_rule_protocol AS ENUM ( - 'TCP', - 'UDP', - 'ICMP' -); - CREATE TABLE IF NOT EXISTS omicron.public.vpc_firewall_rule ( /* Identity metadata (resource) */ id UUID PRIMARY KEY, @@ -1893,9 +1887,9 @@ CREATE TABLE IF NOT EXISTS omicron.public.vpc_firewall_rule ( /* Also an array of targets */ filter_hosts STRING(128)[], filter_ports STRING(11)[], - filter_protocols omicron.public.vpc_firewall_rule_protocol[], action omicron.public.vpc_firewall_rule_action NOT NULL, - priority INT4 CHECK (priority BETWEEN 0 AND 65535) NOT NULL + priority INT4 CHECK (priority BETWEEN 0 AND 65535) NOT NULL, + filter_protocols STRING(32)[] ); CREATE UNIQUE INDEX IF NOT EXISTS lookup_firewall_by_vpc ON omicron.public.vpc_firewall_rule ( @@ -6116,7 +6110,7 @@ INSERT INTO omicron.public.db_metadata ( version, target_version ) VALUES - (TRUE, NOW(), NOW(), '154.0.0', NULL) + (TRUE, NOW(), NOW(), '155.0.0', NULL) ON CONFLICT DO NOTHING; COMMIT; diff --git a/schema/crdb/vpc-firewall-icmp/README.adoc b/schema/crdb/vpc-firewall-icmp/README.adoc new file mode 100644 index 00000000000..71a0957b54b --- /dev/null +++ b/schema/crdb/vpc-firewall-icmp/README.adoc @@ -0,0 +1,16 @@ +# Overview + +This schema change alters the definition of a firewall rule's protocol filters from an `ENUM[]` to a `STRING[]`. +The reason we're going through this is to be able to express more complex filters on protocol families, such as type/code restrictions on ICMP(v6) flows. +This also allows us, in future, to let customers express arbitrary L4 protocols (once OPTE allows them to be sent/received). + +However, as with many other well-intentioned schema type changes, we need to go through a bit of a circuitous route to get there since a) `ALTER COLUMN xxx TYPE` is experimental, and b) we cannot idempotently rename a column: + +. Add a new `filter_protocols_2` column with the correct type. (`up01.sql`) +. Populate this with the lower case contents of `filter_protocols`, then drop that column. (`up02.sql`, `up03.sql`) +. Recreate `filter_protocols` with the new target type. Copy `filter_protocols_2` into it for all rows. (`up04.sql`, `up05.sql`) +. Drop `filter_columns_2`. (`up06.sql`) +. Drop the now unused `vpc_firewall_rule_protocol` enum type. (`up07.sql`) + +Finally, we add in a new builtin firewall rule to the services VPC to enable Path MTU discovery in Nexus (`up08.sql`). +The presence of this rule is replied upon by the `/v1/system/networking/inbound-icmp` endpoint to enable/disable this access. diff --git a/schema/crdb/vpc-firewall-icmp/up01.sql b/schema/crdb/vpc-firewall-icmp/up01.sql new file mode 100644 index 00000000000..a83680b5ed2 --- /dev/null +++ b/schema/crdb/vpc-firewall-icmp/up01.sql @@ -0,0 +1,10 @@ +-- We need to support more specific filters on given protocol families +-- (e.g., optional ICMP code/type specifiers). Move to have Nexus +-- parse/deparse these. This also opens the door for matches by IpProtocol +-- ID (numeric) in future. +-- +-- However, ALTER COLUMN type from ENUM[] to STRING[] is 'experimental', given +-- the error messages. So we'll be roundtripping through a new column. + +ALTER TABLE omicron.public.vpc_firewall_rule + ADD COLUMN IF NOT EXISTS filter_protocols_2 STRING(32)[]; diff --git a/schema/crdb/vpc-firewall-icmp/up02.sql b/schema/crdb/vpc-firewall-icmp/up02.sql new file mode 100644 index 00000000000..4bd1ccef500 --- /dev/null +++ b/schema/crdb/vpc-firewall-icmp/up02.sql @@ -0,0 +1,7 @@ +SET LOCAL disallow_full_table_scans = off; + +-- Convert to lower-case strings. Existing filters like "ICMP" +-- will still be interpreted as 'all ICMP'. +UPDATE omicron.public.vpc_firewall_rule +SET + filter_protocols_2 = lower(filter_protocols::STRING)::STRING(32)[]; diff --git a/schema/crdb/vpc-firewall-icmp/up03.sql b/schema/crdb/vpc-firewall-icmp/up03.sql new file mode 100644 index 00000000000..e25cdc0c228 --- /dev/null +++ b/schema/crdb/vpc-firewall-icmp/up03.sql @@ -0,0 +1,2 @@ +ALTER TABLE omicron.public.vpc_firewall_rule + DROP COLUMN IF EXISTS filter_protocols; diff --git a/schema/crdb/vpc-firewall-icmp/up04.sql b/schema/crdb/vpc-firewall-icmp/up04.sql new file mode 100644 index 00000000000..189a4dc3ce7 --- /dev/null +++ b/schema/crdb/vpc-firewall-icmp/up04.sql @@ -0,0 +1,2 @@ +ALTER TABLE omicron.public.vpc_firewall_rule + ADD COLUMN IF NOT EXISTS filter_protocols STRING(32)[]; diff --git a/schema/crdb/vpc-firewall-icmp/up05.sql b/schema/crdb/vpc-firewall-icmp/up05.sql new file mode 100644 index 00000000000..4542874ef20 --- /dev/null +++ b/schema/crdb/vpc-firewall-icmp/up05.sql @@ -0,0 +1,5 @@ +SET LOCAL disallow_full_table_scans = off; + +UPDATE omicron.public.vpc_firewall_rule +SET + filter_protocols = filter_protocols_2; diff --git a/schema/crdb/vpc-firewall-icmp/up06.sql b/schema/crdb/vpc-firewall-icmp/up06.sql new file mode 100644 index 00000000000..bf42ef80166 --- /dev/null +++ b/schema/crdb/vpc-firewall-icmp/up06.sql @@ -0,0 +1,2 @@ +ALTER TABLE omicron.public.vpc_firewall_rule + DROP COLUMN IF EXISTS filter_protocols_2; diff --git a/schema/crdb/vpc-firewall-icmp/up07.sql b/schema/crdb/vpc-firewall-icmp/up07.sql new file mode 100644 index 00000000000..24f1b68b45a --- /dev/null +++ b/schema/crdb/vpc-firewall-icmp/up07.sql @@ -0,0 +1 @@ +DROP TYPE IF EXISTS omicron.public.vpc_firewall_rule_protocol; diff --git a/schema/crdb/vpc-firewall-icmp/up08.sql b/schema/crdb/vpc-firewall-icmp/up08.sql new file mode 100644 index 00000000000..79e713468ad --- /dev/null +++ b/schema/crdb/vpc-firewall-icmp/up08.sql @@ -0,0 +1,30 @@ +-- Add a well-known rule to services VPC to allow limited forms +-- of ICMP traffic. Inserting this rule is conditional on the +-- 'oxide-services' VPC existing. +INSERT INTO omicron.public.vpc_firewall_rule ( + id, + name, description, + time_created, time_modified, vpc_id, status, + targets, + filter_protocols, + direction, action, priority +) +SELECT + gen_random_uuid(), + -- Hardcoded name/description, see nexus/db-fixed-data/src/vpc_firewall_rule.rs. + 'nexus-icmp', 'allow typical inbound ICMP error codes for outbound flows', + NOW(), NOW(), vpc.id, 'enabled', + -- Apply to the Nexus and External DNS zones... + ARRAY['subnet:nexus','subnet:external-dns'], + -- Allow inbound ICMP: + -- * Destination Unreachable + -- * Port Unreachable + -- * Fragmentation Needed + -- * Redirect + -- * Time Exceeded + ARRAY['icmp:3,3-4','icmp:5','icmp:11'], + 'inbound', 'allow', 65534 +FROM omicron.public.vpc + WHERE vpc.id = '001de000-074c-4000-8000-000000000000' + AND vpc.name = 'oxide-services' +ON CONFLICT DO NOTHING; diff --git a/tools/maghemite_ddm_openapi_version b/tools/maghemite_ddm_openapi_version index 85c82fbfa4f..ba3584bd99e 100644 --- a/tools/maghemite_ddm_openapi_version +++ b/tools/maghemite_ddm_openapi_version @@ -1,2 +1,2 @@ -COMMIT="760b4b547e301a31d4dcb92ba97aabdb2a3e0cba" +COMMIT="638c897d5ed1e5d3f2de0c1cb9dfaa4d77a35dea" SHA2="9146aaf60a52ecd138139708e4019e4496f330fb81a2c5a7a70cd3436a6a1318" diff --git a/tools/maghemite_mg_openapi_version b/tools/maghemite_mg_openapi_version index f7a163a1162..a21dd3b71f3 100644 --- a/tools/maghemite_mg_openapi_version +++ b/tools/maghemite_mg_openapi_version @@ -1,2 +1,2 @@ -COMMIT="760b4b547e301a31d4dcb92ba97aabdb2a3e0cba" +COMMIT="638c897d5ed1e5d3f2de0c1cb9dfaa4d77a35dea" SHA2="7af1675e2e93e395185f8d3676db972db0123714c4c5640608f3e3570f3ce3a8" diff --git a/tools/maghemite_mgd_checksums b/tools/maghemite_mgd_checksums index 8b04688f313..89303c1d36d 100644 --- a/tools/maghemite_mgd_checksums +++ b/tools/maghemite_mgd_checksums @@ -1,2 +1,2 @@ -CIDL_SHA256="5f87a6bd81e5bc83e4755dba31a0df75883888b00596fb70a5c5200dfedbdddd" -MGD_LINUX_SHA256="961583be3ab75f086824776ba0b861ebd0e38c2b7a3dd2579cda66e4a9c923fd" \ No newline at end of file +CIDL_SHA256="6451cdc1473e555676bc33eb3e94f96205a2d839120318b3dfd4600232f95626" +MGD_LINUX_SHA256="7f0490149777538f6c7d4b074a6ed5c973f07f96bdf74e672f93b17062d8ffc1" \ No newline at end of file diff --git a/tools/opte_version b/tools/opte_version index 06101fb0ca4..b0a9ceb5cb6 100644 --- a/tools/opte_version +++ b/tools/opte_version @@ -1 +1 @@ -0.36.374 +0.37.381