Fix FIP creation being able to access IP Pools in other silos

nexus/db-queries/src/db/datastore/external_ip.rs

@@ -147,22 +147,34 @@ impl DataStore {
     ) -> CreateResult<ExternalIp> {
         let ip_id = Uuid::new_v4();
 
-        let pool_id = match params.pool {
-            Some(NameOrId::Name(name)) => {
-                LookupPath::new(opctx, self)
-                    .ip_pool_name(&Name(name))
-                    .fetch_for(authz::Action::Read)
-                    .await?
-                    .1
-            }
-            Some(NameOrId::Id(id)) => {
-                LookupPath::new(opctx, self)
-                    .ip_pool_id(id)
-                    .fetch_for(authz::Action::Read)
-                    .await?
-                    .1
+        // See `allocate_instance_ephemeral_ip`: we're replicating
+        // its strucutre to prevent cross-silo pool access.
+        let pool_id = if let Some(name_or_id) = params.pool {
+            let (.., authz_pool, pool) = match name_or_id {
+                NameOrId::Name(name) => {
+                    LookupPath::new(opctx, self)
+                        .ip_pool_name(&Name(name))
+                        .fetch_for(authz::Action::CreateChild)
+                        .await?
+                }
+                NameOrId::Id(id) => {
+                    LookupPath::new(opctx, self)
+                        .ip_pool_id(id)
+                        .fetch_for(authz::Action::CreateChild)
+                        .await?
+                }
+            };
+
+            let authz_silo_id = opctx.authn.silo_required()?.id();
+            if let Some(pool_silo_id) = pool.silo_id {
+                if pool_silo_id != authz_silo_id {
+                    return Err(authz_pool.not_found());
+                }
             }
-            None => self.ip_pools_fetch_default(opctx).await?,
+
+            pool
+        } else {
+            self.ip_pools_fetch_default(opctx).await?
         }
         .id();
 

nexus/src/app/sagas/disk_create.rs

@@ -854,7 +854,7 @@ pub(crate) mod test {
     const PROJECT_NAME: &str = "springfield-squidport";
 
     async fn create_org_and_project(client: &ClientTestContext) -> Uuid {
-        create_ip_pool(&client, "p0", None).await;
+        create_ip_pool(&client, "p0", None, None).await;
         let project = create_project(client, PROJECT_NAME).await;
         project.identity.id
     }

nexus/src/app/sagas/disk_delete.rs

@@ -202,7 +202,7 @@ pub(crate) mod test {
     const PROJECT_NAME: &str = "springfield-squidport";
 
     async fn create_org_and_project(client: &ClientTestContext) -> Uuid {
-        create_ip_pool(&client, "p0", None).await;
+        create_ip_pool(&client, "p0", None, None).await;
         let project = create_project(client, PROJECT_NAME).await;
         project.identity.id
     }

nexus/src/app/sagas/snapshot_create.rs

@@ -1786,7 +1786,7 @@ mod test {
     const INSTANCE_NAME: &str = "base-instance";
 
     async fn create_org_project_and_disk(client: &ClientTestContext) -> Uuid {
-        create_ip_pool(&client, "p0", None).await;
+        create_ip_pool(&client, "p0", None, None).await;
         create_project(client, PROJECT_NAME).await;
         create_disk(client, PROJECT_NAME, DISK_NAME).await.identity.id
     }

nexus/test-utils/src/resource_helpers.rs

@@ -134,6 +134,7 @@ pub async fn create_ip_pool(
     client: &ClientTestContext,
     pool_name: &str,
     ip_range: Option<IpRange>,
+    silo: Option<Uuid>,
 ) -> (IpPool, IpPoolRange) {
     let pool = object_create(
         client,
@@ -143,7 +144,7 @@ pub async fn create_ip_pool(
                 name: pool_name.parse().unwrap(),
                 description: String::from("an ip pool"),
             },
-            silo: None,
+            silo: silo.map(|id| NameOrId::Id(id)),
             is_default: false,
         },
     )

nexus/tests/integration_tests/external_ips.rs

@@ -19,22 +19,26 @@ use nexus_test_utils::resource_helpers::create_floating_ip;
 use nexus_test_utils::resource_helpers::create_instance_with;
 use nexus_test_utils::resource_helpers::create_ip_pool;
 use nexus_test_utils::resource_helpers::create_project;
+use nexus_test_utils::resource_helpers::create_silo;
 use nexus_test_utils::resource_helpers::populate_ip_pool;
 use nexus_test_utils_macros::nexus_test;
 use nexus_types::external_api::params;
+use nexus_types::external_api::shared;
 use nexus_types::external_api::views::FloatingIp;
 use omicron_common::address::IpRange;
 use omicron_common::address::Ipv4Range;
 use omicron_common::api::external::IdentityMetadataCreateParams;
 use omicron_common::api::external::Instance;
+use omicron_common::api::external::NameOrId;
 use uuid::Uuid;
 
 type ControlPlaneTestContext =
     nexus_test_utils::ControlPlaneTestContext<omicron_nexus::Server>;
 
 const PROJECT_NAME: &str = "rootbeer-float";
 
-const FIP_NAMES: &[&str] = &["vanilla", "chocolate", "strawberry", "pistachio"];
+const FIP_NAMES: &[&str] =
+    &["vanilla", "chocolate", "strawberry", "pistachio", "caramel"];
 
 pub fn get_floating_ips_url(project_name: &str) -> String {
     format!("/v1/floating-ips?project={project_name}")
@@ -107,7 +111,7 @@ async fn test_floating_ip_create(cptestctx: &ControlPlaneTestContext) {
         Ipv4Range::new(Ipv4Addr::new(10, 1, 0, 1), Ipv4Addr::new(10, 1, 0, 5))
             .unwrap(),
     );
-    create_ip_pool(&client, "other-pool", Some(other_pool_range)).await;
+    create_ip_pool(&client, "other-pool", Some(other_pool_range), None).await;
 
     let project = create_project(client, PROJECT_NAME).await;
 
@@ -142,7 +146,7 @@ async fn test_floating_ip_create(cptestctx: &ControlPlaneTestContext) {
     assert_eq!(fip.instance_id, None);
     assert_eq!(fip.ip, ip_addr);
 
-    // Create with no chosen IP from named pool.
+    // Create with no chosen IP from fleet-scoped named pool.
     let fip_name = FIP_NAMES[2];
     let fip = create_floating_ip(
         client,
@@ -157,7 +161,7 @@ async fn test_floating_ip_create(cptestctx: &ControlPlaneTestContext) {
     assert_eq!(fip.instance_id, None);
     assert_eq!(fip.ip, IpAddr::from(Ipv4Addr::new(10, 1, 0, 1)));
 
-    // Create with chosen IP from named pool.
+    // Create with chosen IP from fleet-scoped named pool.
     let fip_name = FIP_NAMES[3];
     let ip_addr = "10.1.0.5".parse().unwrap();
     let fip = create_floating_ip(
@@ -174,6 +178,65 @@ async fn test_floating_ip_create(cptestctx: &ControlPlaneTestContext) {
     assert_eq!(fip.ip, ip_addr);
 }
 
+#[nexus_test]
+async fn test_floating_ip_create_fails_in_other_silo_pool(
+    cptestctx: &ControlPlaneTestContext,
+) {
+    let client = &cptestctx.external_client;
+
+    populate_ip_pool(&client, "default", None).await;
+
+    let project = create_project(client, PROJECT_NAME).await;
+
+    // Create other silo and pool linked to that silo
+    let other_silo = create_silo(
+        &client,
+        "not-my-silo",
+        true,
+        shared::SiloIdentityMode::SamlJit,
+    )
+    .await;
+    let other_pool_range = IpRange::V4(
+        Ipv4Range::new(Ipv4Addr::new(10, 2, 0, 1), Ipv4Addr::new(10, 2, 0, 5))
+            .unwrap(),
+    );
+    create_ip_pool(
+        &client,
+        "external-silo-pool",
+        Some(other_pool_range),
+        Some(other_silo.identity.id),
+    )
+    .await;
+
+    let fip_name = FIP_NAMES[4];
+
+    // creating a floating IP should fail with a 404 as if the specified pool
+    // does not exist
+    let url =
+        format!("/v1/floating-ips?project={}", project.identity.name.as_str());
+    let body = params::FloatingIpCreate {
+        identity: IdentityMetadataCreateParams {
+            name: fip_name.parse().unwrap(),
+            description: String::from("a floating ip"),
+        },
+        address: None,
+        pool: Some(NameOrId::Name("external-silo-pool".parse().unwrap())),
+    };
+
+    let error = NexusRequest::new(
+        RequestBuilder::new(client, Method::POST, &url)
+            .body(Some(&body))
+            .expect_status(Some(StatusCode::NOT_FOUND)),
+    )
+    .authn_as(AuthnMode::PrivilegedUser)
+    .execute_and_parse_unwrap::<HttpErrorResponseBody>()
+    .await;
+    assert_eq!(
+        error.message,
+        "not found: ip-pool with name \"external-silo-pool\""
+    );
+}
+
 #[nexus_test]
 async fn test_floating_ip_create_ip_in_use(
     cptestctx: &ControlPlaneTestContext,

nexus/tests/integration_tests/instances.rs

@@ -3563,7 +3563,7 @@ async fn test_instance_ephemeral_ip_from_correct_pool(
         .unwrap(),
     );
     populate_ip_pool(&client, "default", Some(default_pool_range)).await;
-    create_ip_pool(&client, "other-pool", Some(other_pool_range)).await;
+    create_ip_pool(&client, "other-pool", Some(other_pool_range), None).await;
 
     // Create an instance with pool name blank, expect IP from default pool
     create_instance_with_pool(client, "default-pool-inst", None).await;