Skip to content

Add support for ICMPv6 protocol filter in firewall rules #10101

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bnaecker merged 3 commits into from
Mar 24, 2026
Merged

Add support for ICMPv6 protocol filter in firewall rules #10101

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a541bfd
Add support for ICMPv6 protocol filter in firewall rules
bnaecker Mar 19, 2026
052ccbb
Review feedback: use explicit methods rather than Display / FromStr f…
bnaecker Mar 24, 2026
ec83d5c
Merge branch 'main' into ben/icmpv6-firewall-support
bnaecker Mar 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions clients/sled-agent-client/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@ impl From<omicron_common::api::external::VpcFirewallRuleProtocol>
Tcp => Self::Tcp,
Udp => Self::Udp,
Icmp(v) => Self::Icmp(v),
Icmp6(v) => Self::Icmp6(v),
}
}
}
Expand Down
114 changes: 64 additions & 50 deletions common/src/api/external/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1877,16 +1877,35 @@ pub enum VpcFirewallRuleProtocol {
Tcp,
Udp,
Icmp(Option<VpcFirewallIcmpFilter>),
// TODO: IPv6 not supported by instances.
// Icmpv6(Option<VpcFirewallIcmpFilter>),
Icmp6(Option<VpcFirewallIcmpFilter>),
// TODO: OPTE does not yet permit further L4 protocols. (opte#609)
// Other(u16),
}

impl FromStr for VpcFirewallRuleProtocol {
type Err = Error;
impl VpcFirewallRuleProtocol {
/// Returns a string representation of this protocol filter suitable for
/// use as an API string or in the database.
///
/// This is the inverse of `from_api_string`.
pub fn to_api_string(&self) -> String {
match self {
VpcFirewallRuleProtocol::Tcp => "tcp".to_string(),
VpcFirewallRuleProtocol::Udp => "udp".to_string(),
VpcFirewallRuleProtocol::Icmp(None) => "icmp".to_string(),
VpcFirewallRuleProtocol::Icmp(Some(v)) => {
format!("icmp:{}", v.to_api_string())
}
VpcFirewallRuleProtocol::Icmp6(None) => "icmp6".to_string(),
VpcFirewallRuleProtocol::Icmp6(Some(v)) => {
format!("icmp6:{}", v.to_api_string())
}
}
}

fn from_str(proto: &str) -> Result<Self, Self::Err> {
/// Parses a protocol filter from the API string format.
///
/// This is the inverse of `to_api_string`.
pub fn from_api_string(proto: &str) -> Result<Self, Error> {
let (ty_str, content_str) = match proto.split_once(':') {
None => (proto, None),
Some((lhs, rhs)) => (lhs, Some(rhs)),
Expand All @@ -1898,9 +1917,15 @@ impl FromStr for VpcFirewallRuleProtocol {
(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, Some(rhs)) if lhs.eq_ignore_ascii_case("icmp") => Ok(
Self::Icmp(Some(VpcFirewallIcmpFilter::from_api_string(rhs)?)),
),
(lhs, None) if lhs.eq_ignore_ascii_case("icmp6") => {
Ok(Self::Icmp6(None))
}
(lhs, Some(rhs)) if lhs.eq_ignore_ascii_case("icmp6") => Ok(
Self::Icmp6(Some(VpcFirewallIcmpFilter::from_api_string(rhs)?)),
),
(lhs, None) => Err(Error::invalid_value(
"vpc_firewall_rule_protocol",
format!("unrecognized protocol: {lhs}"),
Expand All @@ -1915,45 +1940,28 @@ impl FromStr for VpcFirewallRuleProtocol {
}
}

impl TryFrom<String> for VpcFirewallRuleProtocol {
type Error = <VpcFirewallRuleProtocol as FromStr>::Err;

fn try_from(proto: String) -> Result<Self, Self::Error> {
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<IcmpParamRange>,
}

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}")?;
impl VpcFirewallIcmpFilter {
/// Returns a string representation of this ICMP filter suitable for use
/// as part of an API string or in the database.
///
/// This is the inverse of `from_api_string`.
pub fn to_api_string(&self) -> String {
match self.code {
None => self.icmp_type.to_string(),
Some(code) => format!("{},{code}", self.icmp_type),
}
Ok(())
}
}

impl FromStr for VpcFirewallIcmpFilter {
type Err = Error;

fn from_str(filter: &str) -> Result<Self, Self::Err> {
/// Parses an ICMP filter from the API string format.
///
/// This is the inverse of `to_api_string`.
pub fn from_api_string(filter: &str) -> Result<Self, Error> {
let (ty_str, code_str) = match filter.split_once(',') {
None => (filter, None),
Some((lhs, rhs)) => (lhs, Some(rhs)),
Expand Down Expand Up @@ -3927,72 +3935,78 @@ mod test {
}

#[test]
fn test_firewall_rule_proto_filter_parse() {
assert_eq!(VpcFirewallRuleProtocol::Tcp, "tcp".parse().unwrap());
assert_eq!(VpcFirewallRuleProtocol::Udp, "udp".parse().unwrap());
fn test_firewall_rule_proto_filter_from_api_string() {
assert_eq!(
VpcFirewallRuleProtocol::Tcp,
VpcFirewallRuleProtocol::from_api_string("tcp").unwrap()
);
assert_eq!(
VpcFirewallRuleProtocol::Udp,
VpcFirewallRuleProtocol::from_api_string("udp").unwrap()
);

assert_eq!(
VpcFirewallRuleProtocol::Icmp(None),
"icmp".parse().unwrap()
VpcFirewallRuleProtocol::from_api_string("icmp").unwrap()
);
assert_eq!(
VpcFirewallRuleProtocol::Icmp(Some(VpcFirewallIcmpFilter {
icmp_type: 4,
code: None
})),
"icmp:4".parse().unwrap()
VpcFirewallRuleProtocol::from_api_string("icmp:4").unwrap()
);
assert_eq!(
VpcFirewallRuleProtocol::Icmp(Some(VpcFirewallIcmpFilter {
icmp_type: 60,
code: Some(0.into())
})),
"icmp:60,0".parse().unwrap()
VpcFirewallRuleProtocol::from_api_string("icmp:60,0").unwrap()
);
assert_eq!(
VpcFirewallRuleProtocol::Icmp(Some(VpcFirewallIcmpFilter {
icmp_type: 60,
code: Some((0..=10).try_into().unwrap())
})),
"icmp:60,0-10".parse().unwrap()
VpcFirewallRuleProtocol::from_api_string("icmp:60,0-10").unwrap()
);
assert_eq!(
"icmp:".parse::<VpcFirewallRuleProtocol>(),
VpcFirewallRuleProtocol::from_api_string("icmp:"),
Err(Error::invalid_value(
"icmp_type",
"\"\" unparsable for type: cannot parse integer from empty string"
))
);
assert_eq!(
"icmp:20-30".parse::<VpcFirewallRuleProtocol>(),
VpcFirewallRuleProtocol::from_api_string("icmp:20-30"),
Err(Error::invalid_value(
"icmp_type",
"\"20-30\" unparsable for type: invalid digit found in string"
))
);
assert_eq!(
"icmp:10,".parse::<VpcFirewallRuleProtocol>(),
VpcFirewallRuleProtocol::from_api_string("icmp:10,"),
Err(Error::invalid_value(
"code",
"\"\" unparsable for type: cannot parse integer from empty string"
))
);
assert_eq!(
"icmp:257,".parse::<VpcFirewallRuleProtocol>(),
VpcFirewallRuleProtocol::from_api_string("icmp:257,"),
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::<VpcFirewallRuleProtocol>(),
VpcFirewallRuleProtocol::from_api_string("icmp:0,1000-1001"),
Err(Error::invalid_value(
"code",
"\"1000\" unparsable for type: number too large to fit in target type"
))
);
assert_eq!(
"icmp:0,30-".parse::<VpcFirewallRuleProtocol>(),
VpcFirewallRuleProtocol::from_api_string("icmp:0,30-"),
Err(Error::invalid_value("code", "range has no end value"))
);
}
Expand Down
8 changes: 8 additions & 0 deletions illumos-utils/src/opte/firewall_rules.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,14 @@ impl FromVpcFirewallRule for ResolvedVpcFirewallRule {
}
}))
}
VpcFirewallRuleProtocol::Icmp6(v) => {
ProtoFilter::Icmpv6(v.map(|v| {
oxide_vpc::api::IcmpFilter {
ty: v.icmp_type,
codes: v.code.map(Into::into),
}
}))
}
})
.collect(),
_ => vec![ProtoFilter::Any],
Expand Down
6 changes: 3 additions & 3 deletions nexus/db-model/src/vpc_firewall_rule.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,14 @@ NewtypeFrom! { () pub struct VpcFirewallRuleProtocol(external::VpcFirewallRulePr
NewtypeDeref! { () pub struct VpcFirewallRuleProtocol(external::VpcFirewallRuleProtocol); }

impl DatabaseString for VpcFirewallRuleProtocol {
type Error = <external::VpcFirewallRuleProtocol as FromStr>::Err;
type Error = external::Error;

fn to_database_string(&self) -> Cow<'_, str> {
self.0.to_string().into()
self.0.to_api_string().into()
}

fn from_database_string(s: &str) -> Result<Self, Self::Error> {
s.parse::<external::VpcFirewallRuleProtocol>().map(Self)
external::VpcFirewallRuleProtocol::from_api_string(s).map(Self)
}
}

Expand Down
46 changes: 45 additions & 1 deletion nexus/external-api/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ use openapiv3::OpenAPI;
mod v2025_11_20_00_local;
mod v2026_01_01_00_local;
mod v2026_01_30_00_local;
mod v2026_03_24_00_local;

api_versions!([
// API versions are in the format YYYY_MM_DD_NN.0.0, defined below as
Expand Down Expand Up @@ -78,6 +79,7 @@ api_versions!([
// | date-based version should be at the top of the list.
// v
// (next_yyyy_mm_dd_nn, IDENT),
(2026_03_24_00, ADD_ICMPV6_FIREWALL_SUPPORT),
(2026_03_23_00, RENAME_PREFIX_LEN),
(2026_03_14_00, MULTICAST_DROP_MVLAN),
(2026_03_12_00, CAPITALIZE_DESCRIPTIONS),
Expand Down Expand Up @@ -6109,12 +6111,31 @@ pub trait NexusExternalApi {
method = GET,
path = "/v1/vpc-firewall-rules",
tags = ["vpcs"],
versions = VERSION_ADD_ICMPV6_FIREWALL_SUPPORT..,
}]
async fn vpc_firewall_rules_view(
rqctx: RequestContext<Self::Context>,
query_params: Query<latest::vpc::VpcSelector>,
) -> Result<HttpResponseOk<VpcFirewallRules>, HttpError>;

/// List firewall rules
#[endpoint {
operation_id = "vpc_firewall_rules_view",
method = GET,
path = "/v1/vpc-firewall-rules",
tags = ["vpcs"],
versions = ..VERSION_ADD_ICMPV6_FIREWALL_SUPPORT,
}]
async fn vpc_firewall_rules_view_v2026_03_24_00(
rqctx: RequestContext<Self::Context>,
query_params: Query<latest::vpc::VpcSelector>,
) -> Result<HttpResponseOk<v2026_03_24_00_local::VpcFirewallRules>, HttpError>
{
Self::vpc_firewall_rules_view(rqctx, query_params).await.and_then(
|resp| resp.try_map(TryInto::try_into).map_err(HttpError::from),
)
}

// Note: the limits in the below comment come from the firewall rules model
// file, nexus/db-model/src/vpc_firewall_rule.rs.

Expand All @@ -6136,13 +6157,36 @@ pub trait NexusExternalApi {
method = PUT,
path = "/v1/vpc-firewall-rules",
tags = ["vpcs"],
versions = VERSION_ADD_ICMPV6_FIREWALL_SUPPORT..,
}]
async fn vpc_firewall_rules_update(
rqctx: RequestContext<Self::Context>,
query_params: Query<latest::vpc::VpcSelector>,
router_params: TypedBody<VpcFirewallRuleUpdateParams>,
update: TypedBody<VpcFirewallRuleUpdateParams>,
) -> Result<HttpResponseOk<VpcFirewallRules>, HttpError>;

/// Replace firewall rules
#[endpoint {
operation_id = "vpc_firewall_rules_update",
method = PUT,
path = "/v1/vpc-firewall-rules",
tags = ["vpcs"],
versions = ..VERSION_ADD_ICMPV6_FIREWALL_SUPPORT,
}]
async fn vpc_firewall_rules_update_v2026_03_24_00(
rqctx: RequestContext<Self::Context>,
query_params: Query<latest::vpc::VpcSelector>,
update: TypedBody<v2026_03_24_00_local::VpcFirewallRuleUpdateParams>,
) -> Result<HttpResponseOk<v2026_03_24_00_local::VpcFirewallRules>, HttpError>
{
let body = update.map(Into::into);
Self::vpc_firewall_rules_update(rqctx, query_params, body)
.await
.and_then(|resp| {
resp.try_map(TryInto::try_into).map_err(HttpError::from)
})
}

// VPC Routers

/// List routers
Expand Down
Loading
Loading