Move network interface authz to the data store

nexus/src/authz/api_resources.rs

@@ -515,3 +515,4 @@ pub type Disk = ProjectChild;
 pub type Instance = ProjectChild;
 pub type Vpc = ProjectChild;
 pub type VpcSubnet = ProjectChild;
+pub type NetworkInterface = ProjectChild;

nexus/src/authz/mod.rs

@@ -167,6 +167,7 @@ pub use api_resources::Disk;
 pub use api_resources::Fleet;
 pub use api_resources::FleetChild;
 pub use api_resources::Instance;
+pub use api_resources::NetworkInterface;
 pub use api_resources::Organization;
 pub use api_resources::Project;
 pub use api_resources::Vpc;

nexus/src/db/datastore.rs

@@ -1676,8 +1676,29 @@ impl DataStore {
     }
 
     // Network interfaces
+
+    /// Create a network interface attached to the provided instance.
     pub async fn instance_create_network_interface(
         &self,
+        opctx: &OpContext,
+        authz_subnet: &authz::VpcSubnet,
+        authz_instance: &authz::Instance,
+        interface: IncompleteNetworkInterface,
+    ) -> Result<NetworkInterface, NetworkInterfaceError> {
+        opctx
+            .authorize(authz::Action::CreateChild, authz_instance)
+            .await
+            .map_err(NetworkInterfaceError::External)?;
+        opctx
+            .authorize(authz::Action::CreateChild, authz_subnet)
+            .await
+            .map_err(NetworkInterfaceError::External)?;
+        self.instance_create_network_interface_raw(&opctx, interface).await
+    }
+
+    pub(super) async fn instance_create_network_interface_raw(
+        &self,
+        opctx: &OpContext,
         interface: IncompleteNetworkInterface,
     ) -> Result<NetworkInterface, NetworkInterfaceError> {
         use db::schema::network_interface::dsl;
@@ -1688,7 +1709,11 @@ impl DataStore {
         diesel::insert_into(dsl::network_interface)
             .values(query)
             .returning(NetworkInterface::as_returning())
-            .get_result_async(self.pool())
+            .get_result_async(
+                self.pool_authorized(opctx)
+                    .await
+                    .map_err(NetworkInterfaceError::External)?,
+            )
             .await
             .map_err(|e| NetworkInterfaceError::from_pool(e, &interface))
     }
@@ -1720,109 +1745,124 @@ impl DataStore {
         Ok(())
     }
 
-    pub async fn instance_delete_network_interface(
+    /// Fetches a `NetworkInterface` from the database and returns both the
+    /// database row and an [`authz::NetworkInterface`] for doing authz checks.
+    ///
+    /// See [`DataStore::organization_lookup_noauthz()`] for intended use cases
+    /// and caveats.
+    // TODO-security See the note on organization_lookup_noauthz().
+    async fn network_interface_lookup_noauthz(
         &self,
-        interface_id: &Uuid,
-    ) -> DeleteResult {
+        authz_instance: &authz::Instance,
+        interface_name: &Name,
+    ) -> LookupResult<(authz::NetworkInterface, NetworkInterface)> {
         use db::schema::network_interface::dsl;
-        let now = Utc::now();
-        let result = diesel::update(dsl::network_interface)
-            .filter(dsl::id.eq(*interface_id))
+        dsl::network_interface
             .filter(dsl::time_deleted.is_null())
-            .set((dsl::time_deleted.eq(now),))
-            .check_if_exists::<db::model::NetworkInterface>(*interface_id)
-            .execute_and_check(self.pool())
+            .filter(dsl::instance_id.eq(authz_instance.id()))
+            .filter(dsl::name.eq(interface_name.clone()))
+            .select(NetworkInterface::as_select())
+            .first_async(self.pool())
             .await
             .map_err(|e| {
                 public_error_from_diesel_pool(
                     e,
                     ErrorHandler::NotFoundByLookup(
                         ResourceType::NetworkInterface,
-                        LookupType::ById(*interface_id),
+                        LookupType::ByName(interface_name.as_str().to_owned()),
                     ),
                 )
-            })?;
-        match result.status {
-            UpdateStatus::Updated => Ok(()),
-            UpdateStatus::NotUpdatedButExists => {
-                let interface = &result.found;
-                if interface.time_deleted().is_some() {
-                    // Already deleted
-                    Ok(())
-                } else {
-                    Err(Error::internal_error(&format!(
-                        "failed to delete network interface: {}",
-                        interface_id
-                    )))
-                }
-            }
-        }
+            })
+            .map(|d| {
+                (
+                    authz_instance.child_generic(
+                        ResourceType::NetworkInterface,
+                        d.id(),
+                        LookupType::from(&interface_name.0),
+                    ),
+                    d,
+                )
+            })
     }
 
-    pub async fn subnet_lookup_network_interface(
+    /// Lookup a `NetworkInterface` by name and return the full database record,
+    /// along with an [`authz::NetworkInterface`] for subsequent authorization
+    /// checks.
+    pub async fn network_interface_fetch(
         &self,
-        subnet_id: &Uuid,
+        opctx: &OpContext,
+        authz_instance: &authz::Instance,
+        name: &Name,
+    ) -> LookupResult<(authz::NetworkInterface, NetworkInterface)> {
+        let (authz_interface, db_interface) =
+            self.network_interface_lookup_noauthz(authz_instance, name).await?;
+        opctx.authorize(authz::Action::Read, &authz_interface).await?;
+        Ok((authz_interface, db_interface))
+    }
+
+    /// Delete a `NetworkInterface` attached to a provided instance.
+    pub async fn instance_delete_network_interface(
+        &self,
+        opctx: &OpContext,
+        authz_instance: &authz::Instance,
         interface_name: &Name,
-    ) -> LookupResult<db::model::NetworkInterface> {
-        use db::schema::network_interface::dsl;
+    ) -> DeleteResult {
+        let (authz_interface, _) = self
+            .network_interface_fetch(opctx, &authz_instance, interface_name)
+            .await?;
+        opctx.authorize(authz::Action::Delete, &authz_interface).await?;
 
-        dsl::network_interface
-            .filter(dsl::subnet_id.eq(*subnet_id))
+        use db::schema::network_interface::dsl;
+        let now = Utc::now();
+        let interface_id = authz_interface.id();
+        diesel::update(dsl::network_interface)
+            .filter(dsl::id.eq(interface_id))
             .filter(dsl::time_deleted.is_null())
-            .filter(dsl::name.eq(interface_name.clone()))
-            .select(db::model::NetworkInterface::as_select())
-            .get_result_async(self.pool())
+            .set((dsl::time_deleted.eq(now),))
+            .execute_async(self.pool_authorized(opctx).await?)
             .await
             .map_err(|e| {
                 public_error_from_diesel_pool(
                     e,
                     ErrorHandler::NotFoundByLookup(
                         ResourceType::NetworkInterface,
-                        LookupType::ByName(interface_name.to_string()),
+                        LookupType::ById(interface_id),
                     ),
                 )
-            })
+            })?;
+        Ok(())
     }
 
     /// List network interfaces associated with a given instance.
     pub async fn instance_list_network_interfaces(
         &self,
-        instance_id: &Uuid,
+        opctx: &OpContext,
+        authz_instance: &authz::Instance,
         pagparams: &DataPageParams<'_, Name>,
     ) -> ListResultVec<NetworkInterface> {
+        opctx.authorize(authz::Action::ListChildren, authz_instance).await?;
+
         use db::schema::network_interface::dsl;
         paginated(dsl::network_interface, dsl::name, &pagparams)
             .filter(dsl::time_deleted.is_null())
-            .filter(dsl::instance_id.eq(*instance_id))
+            .filter(dsl::instance_id.eq(authz_instance.id()))
             .select(NetworkInterface::as_select())
-            .load_async::<NetworkInterface>(self.pool())
+            .load_async::<NetworkInterface>(self.pool_authorized(opctx).await?)
             .await
             .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server))
     }
 
     /// Get a network interface by name attached to an instance
     pub async fn instance_lookup_network_interface(
         &self,
-        instance_id: &Uuid,
+        opctx: &OpContext,
+        authz_instance: &authz::Instance,
         interface_name: &Name,
     ) -> LookupResult<NetworkInterface> {
-        use db::schema::network_interface::dsl;
-        dsl::network_interface
-            .filter(dsl::instance_id.eq(*instance_id))
-            .filter(dsl::name.eq(interface_name.clone()))
-            .filter(dsl::time_deleted.is_null())
-            .select(NetworkInterface::as_select())
-            .get_result_async(self.pool())
-            .await
-            .map_err(|e| {
-                public_error_from_diesel_pool(
-                    e,
-                    ErrorHandler::NotFoundByLookup(
-                        ResourceType::NetworkInterface,
-                        LookupType::ByName(interface_name.to_string()),
-                    ),
-                )
-            })
+        Ok(self
+            .network_interface_fetch(opctx, &authz_instance, interface_name)
+            .await?
+            .1)
     }
 
     // Create a record for a new Oximeter instance

nexus/src/db/subnet_allocation.rs

@@ -1134,6 +1134,7 @@ impl QueryFragment<Pg> for InsertNetworkInterfaceQueryValues {
 mod test {
     use super::NetworkInterfaceError;
     use super::SubnetError;
+    use crate::context::OpContext;
     use crate::db::model::{
         self, IncompleteNetworkInterface, NetworkInterface, VpcSubnet,
     };
@@ -1298,6 +1299,7 @@ mod test {
         let pool = Arc::new(crate::db::Pool::new(&cfg));
         let db_datastore =
             Arc::new(crate::db::DataStore::new(Arc::clone(&pool)));
+        let opctx = OpContext::for_tests(log.new(o!()), db_datastore.clone());
 
         // Two test VpcSubnets, in different VPCs. The IPv4 range has space for
         // 16 addresses, less the 6 that are reserved.
@@ -1352,7 +1354,7 @@ mod test {
         )
         .unwrap();
         let inserted_interface = db_datastore
-            .instance_create_network_interface(interface.clone())
+            .instance_create_network_interface_raw(&opctx, interface.clone())
             .await
             .expect("Failed to insert interface with known-good IP address");
         assert_interfaces_eq(&interface, &inserted_interface);
@@ -1380,7 +1382,7 @@ mod test {
         )
         .unwrap();
         let inserted_interface = db_datastore
-            .instance_create_network_interface(interface.clone())
+            .instance_create_network_interface_raw(&opctx, interface.clone())
             .await
             .expect("Failed to insert interface with known-good IP address");
         assert_interfaces_eq(&interface, &inserted_interface);
@@ -1404,8 +1406,9 @@ mod test {
             Some(requested_ip),
         )
         .unwrap();
-        let result =
-            db_datastore.instance_create_network_interface(interface).await;
+        let result = db_datastore
+            .instance_create_network_interface_raw(&opctx, interface)
+            .await;
         assert!(
             matches!(
                 result,
@@ -1429,8 +1432,9 @@ mod test {
             None,
         )
         .unwrap();
-        let result =
-            db_datastore.instance_create_network_interface(interface).await;
+        let result = db_datastore
+            .instance_create_network_interface_raw(&opctx, interface)
+            .await;
         assert!(
             matches!(
                 result,
@@ -1455,8 +1459,9 @@ mod test {
                 addr,
             )
             .unwrap();
-            let result =
-                db_datastore.instance_create_network_interface(interface).await;
+            let result = db_datastore
+                .instance_create_network_interface_raw(&opctx, interface)
+                .await;
             assert!(
                 matches!(result, Err(NetworkInterfaceError::InstanceSpansMultipleVpcs(_))),
                 "Attaching an interface to an instance which already has one in a different VPC should fail"
@@ -1481,8 +1486,9 @@ mod test {
                 None,
             )
             .unwrap();
-            let result =
-                db_datastore.instance_create_network_interface(interface).await;
+            let result = db_datastore
+                .instance_create_network_interface_raw(&opctx, interface)
+                .await;
             assert!(
                 result.is_ok(),
                 "We should be able to allocate 8 more interfaces successfully",
@@ -1501,8 +1507,9 @@ mod test {
             None,
         )
         .unwrap();
-        let result =
-            db_datastore.instance_create_network_interface(interface).await;
+        let result = db_datastore
+            .instance_create_network_interface_raw(&opctx, interface)
+            .await;
         assert!(
             matches!(
                 result,
@@ -1528,8 +1535,9 @@ mod test {
                 None,
             )
             .unwrap();
-            let result =
-                db_datastore.instance_create_network_interface(interface).await;
+            let result = db_datastore
+                .instance_create_network_interface_raw(&opctx, interface)
+                .await;
             assert!(
                 result.is_ok(),
                 concat!(
@@ -1577,6 +1585,7 @@ mod test {
         let pool = Arc::new(crate::db::Pool::new(&cfg));
         let db_datastore =
             Arc::new(crate::db::DataStore::new(Arc::clone(&pool)));
+        let opctx = OpContext::for_tests(log.new(o!()), db_datastore.clone());
         let ipv4_block = Ipv4Net("172.30.0.0/28".parse().unwrap());
         let ipv6_block = Ipv6Net("fd12:3456:7890::/64".parse().unwrap());
         let subnet_name = "subnet-a".to_string().try_into().unwrap();
@@ -1610,13 +1619,13 @@ mod test {
         )
         .unwrap();
         let inserted_interface = db_datastore
-            .instance_create_network_interface(interface.clone())
+            .instance_create_network_interface_raw(&opctx, interface.clone())
             .await
             .expect("Failed to insert interface with known-good IP address");
 
         // Attempt to insert the exact same record again.
         let result = db_datastore
-            .instance_create_network_interface(interface.clone())
+            .instance_create_network_interface_raw(&opctx, interface.clone())
             .await;
         if let Err(NetworkInterfaceError::DuplicatePrimaryKey(key)) = result {
             assert_eq!(key, inserted_interface.identity.id);