Feedback: allow limited ICMP into external DNS zones.

Author: FelixMcFelix

nexus/db-fixed-data/src/vpc_firewall_rule.rs

@@ -77,13 +77,12 @@ 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 zone that packets have been explicitly dropped. This is a key part in
-/// enabling path MTU discovery, such as when customers are accessing Necus
+/// 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.
 ///
-/// Note that we currently rely on this being exactly one rule to implement the
-/// system-level enable/disable endpoint. See `nexus/networking/src/firewall_rules.rs`
-/// for more details.
+/// 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<VpcFirewallRuleUpdate> =
     LazyLock::new(|| VpcFirewallRuleUpdate {
         name: NEXUS_ICMP_FW_RULE_NAME.parse().unwrap(),
@@ -92,9 +91,14 @@ pub static NEXUS_ICMP_FW_RULE: LazyLock<VpcFirewallRuleUpdate> =
                 .to_string(),
         status: VpcFirewallRuleStatus::Enabled,
         direction: VpcFirewallRuleDirection::Inbound,
-        targets: vec![VpcFirewallRuleTarget::Subnet(
-            super::vpc_subnet::NEXUS_VPC_SUBNET.name().clone(),
-        )],
+        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![

nexus/tests/integration_tests/schema.rs

@@ -2429,7 +2429,6 @@ fn after_151_0_0<'a>(ctx: &'a MigrationContext<'a>) -> BoxFuture<'a, ()> {
     })
 }
 
-
 // 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.

schema/crdb/vpc-firewall-icmp/up08.sql

@@ -4,16 +4,21 @@
 INSERT INTO omicron.public.vpc_firewall_rule (
   id,
   name, description,
-  time_created, time_modified, vpc_id, status, direction,
-  targets, filter_protocols, action, priority
+  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', 'inbound',
+  NOW(), NOW(), vpc.id, 'enabled',
+  -- Apply to the Nexus and External DNS zones...
+  ARRAY['subnet:nexus','subnet:external-dns'],
   -- Allow inbound ICMP Destination Unreachable (Too Large) and Time Exceeded
-  ARRAY['subnet:nexus'], ARRAY['icmp:3,4','icmp:11'], 'allow', 65534
+  ARRAY['icmp:3,4','icmp:11'],
+  'inbound', 'allow', 65534
 FROM omicron.public.vpc
   WHERE vpc.id = '001de000-074c-4000-8000-000000000000'
     AND vpc.name = 'oxide-services'