Extend ICMP allow to Redirect and Port Unreachable

Redirect came out of our earlier discussion, port unreachable should
be useful in the limit too.

Author: FelixMcFelix

common/src/api/external/mod.rs

@@ -3389,9 +3389,10 @@ pub enum BfdMode {
 )]
 pub struct ServiceIcmpConfig {
     /// When enabled, Nexus is able to receive ICMP Destination Unreachable
-    /// (type 4, fragmentation needed) and Time Exceeded messages. These
-    /// enable Nexus to perform Path MTU discovery and better cope with
-    /// fragmentation issues. Otherwise all ICMP traffic will be dropped.
+    /// 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,
 }
 

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

@@ -105,8 +105,13 @@ pub static NEXUS_ICMP_FW_RULE: LazyLock<VpcFirewallRuleUpdate> =
                 VpcFirewallRuleProtocol::Icmp(Some(VpcFirewallIcmpFilter {
                     // Type 3 -- Destination Unreachable
                     icmp_type: 3,
-                    // Code 4 -- Fragmentation needed
-                    code: Some(4.into()),
+                    // 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

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 {

openapi/nexus.json

@@ -22845,7 +22845,7 @@
         "type": "object",
         "properties": {
           "enabled": {
-            "description": "When enabled, Nexus is able to receive ICMP Destination Unreachable (type 4, fragmentation needed) and Time Exceeded messages. These enable Nexus to perform Path MTU discovery and better cope with fragmentation issues. Otherwise all ICMP traffic will be dropped.",
+            "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"
           }
         },

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

@@ -16,8 +16,13 @@ SELECT
   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['icmp:3,4','icmp:11'],
+  -- 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'