authz: protect VPC Subnet endpoints

nexus/src/authz/api_resources.rs

@@ -363,7 +363,7 @@ impl Project {
         lookup_type: LookupType,
     ) -> ProjectChild {
         ProjectChild {
-            parent: self.clone(),
+            parent: ProjectChildKind::Direct(self.clone()),
             resource_type,
             resource_id,
             lookup_type,
@@ -425,19 +425,50 @@ impl ApiResourceError for Project {
 /// using [`Project::child_generic()`].
 #[derive(Clone, Debug)]
 pub struct ProjectChild {
-    parent: Project,
+    parent: ProjectChildKind,
     resource_type: ResourceType,
     resource_id: Uuid,
     lookup_type: LookupType,
 }
 
+#[derive(Clone, Debug)]
+enum ProjectChildKind {
+    Direct(Project),
+    Indirect(Box<ProjectChild>),
+}
+
 impl ProjectChild {
     pub fn id(&self) -> Uuid {
         self.resource_id
     }
 
     pub fn project(&self) -> &Project {
-        &self.parent
+        match &self.parent {
+            ProjectChildKind::Direct(p) => p,
+            ProjectChildKind::Indirect(p) => p.project(),
+        }
+    }
+
+    /// Returns an authz resource representing a child of this Project child.
+    ///
+    /// This is currently only used for children of Vpc, which include
+    /// VpcSubnets.
+    // TODO-cleanup It would be more type-safe to have a more explicit resource
+    // hierarchy -- i.e., Project -> Vpc -> VpcSubnet.  However, it would also
+    // mean a bunch more boilerplate and it'd be more confusing: you'd have
+    // Projects with children Vpc _or_ ProjectChild.
+    pub fn child_generic(
+        &self,
+        resource_type: ResourceType,
+        resource_id: Uuid,
+        lookup_type: LookupType,
+    ) -> ProjectChild {
+        ProjectChild {
+            parent: ProjectChildKind::Indirect(Box::new(self.clone())),
+            resource_type,
+            resource_id,
+            lookup_type,
+        }
     }
 }
 
@@ -455,7 +486,7 @@ impl oso::PolarClass for ProjectChild {
                 },
             )
             .add_attribute_getter("project", |pr: &ProjectChild| {
-                pr.parent.clone()
+                pr.project().clone()
             })
     }
 }
@@ -467,7 +498,10 @@ impl ApiResource for ProjectChild {
     }
 
     fn parent(&self) -> Option<&dyn AuthorizedResource> {
-        Some(&self.parent)
+        match &self.parent {
+            ProjectChildKind::Direct(p) => Some(p),
+            ProjectChildKind::Indirect(p) => Some(p.as_ref()),
+        }
     }
 }
 
@@ -480,3 +514,4 @@ impl ApiResourceError for ProjectChild {
 pub type Disk = ProjectChild;
 pub type Instance = ProjectChild;
 pub type Vpc = ProjectChild;
+pub type VpcSubnet = ProjectChild;

nexus/src/authz/mod.rs

@@ -171,6 +171,7 @@ pub use api_resources::Instance;
 pub use api_resources::Organization;
 pub use api_resources::Project;
 pub use api_resources::Vpc;
+pub use api_resources::VpcSubnet;
 pub use api_resources::FLEET;
 
 mod context;

nexus/src/db/datastore.rs

@@ -2207,7 +2207,7 @@ impl DataStore {
         opctx: &OpContext,
         authz_project: &authz::Project,
         vpc: Vpc,
-    ) -> Result<Vpc, Error> {
+    ) -> Result<(authz::Vpc, Vpc), Error> {
         use db::schema::vpc::dsl;
 
         assert_eq!(authz_project.id(), vpc.project_id);
@@ -2226,7 +2226,14 @@ impl DataStore {
                     ErrorHandler::Conflict(ResourceType::Vpc, name.as_str()),
                 )
             })?;
-        Ok(vpc)
+        Ok((
+            authz_project.child_generic(
+                ResourceType::Vpc,
+                vpc.id(),
+                LookupType::ByName(vpc.name().to_string()),
+            ),
+            vpc,
+        ))
     }
 
     pub async fn project_update_vpc(
@@ -2418,33 +2425,40 @@ impl DataStore {
 
     pub async fn vpc_list_subnets(
         &self,
-        vpc_id: &Uuid,
+        opctx: &OpContext,
+        authz_vpc: &authz::Vpc,
         pagparams: &DataPageParams<'_, Name>,
     ) -> ListResultVec<VpcSubnet> {
-        use db::schema::vpc_subnet::dsl;
+        opctx.authorize(authz::Action::ListChildren, authz_vpc).await?;
 
+        use db::schema::vpc_subnet::dsl;
         paginated(dsl::vpc_subnet, dsl::name, &pagparams)
             .filter(dsl::time_deleted.is_null())
-            .filter(dsl::vpc_id.eq(*vpc_id))
+            .filter(dsl::vpc_id.eq(authz_vpc.id()))
             .select(VpcSubnet::as_select())
-            .load_async(self.pool())
+            .load_async(self.pool_authorized(opctx).await?)
             .await
             .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server))
     }
 
-    pub async fn vpc_subnet_fetch_by_name(
+    /// Fetches a VpcSubnet from the database and returns both the database row
+    /// and an [`authz::VpcSubnet`] 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 vpc_subnet_lookup_noauthz(
         &self,
-        vpc_id: &Uuid,
+        authz_vpc: &authz::Vpc,
         subnet_name: &Name,
-    ) -> LookupResult<VpcSubnet> {
+    ) -> LookupResult<(authz::VpcSubnet, VpcSubnet)> {
         use db::schema::vpc_subnet::dsl;
-
         dsl::vpc_subnet
             .filter(dsl::time_deleted.is_null())
-            .filter(dsl::vpc_id.eq(*vpc_id))
+            .filter(dsl::vpc_id.eq(authz_vpc.id()))
             .filter(dsl::name.eq(subnet_name.clone()))
             .select(VpcSubnet::as_select())
-            .get_result_async(self.pool())
+            .first_async(self.pool())
             .await
             .map_err(|e| {
                 public_error_from_diesel_pool(
@@ -2455,10 +2469,70 @@ impl DataStore {
                     ),
                 )
             })
+            .map(|d| {
+                (
+                    authz_vpc.child_generic(
+                        ResourceType::VpcSubnet,
+                        d.id(),
+                        LookupType::from(&subnet_name.0),
+                    ),
+                    d,
+                )
+            })
+    }
+
+    /// Look up the id for a VpcSubnet based on its name
+    ///
+    /// Returns an [`authz::VpcSubnet`] (which makes the id available).
+    ///
+    /// Like the other "lookup_by_path()" functions, this function does no authz
+    /// checks.
+    pub async fn vpc_subnet_lookup_by_path(
+        &self,
+        organization_name: &Name,
+        project_name: &Name,
+        vpc_name: &Name,
+        subnet_name: &Name,
+    ) -> LookupResult<authz::Vpc> {
+        let authz_vpc = self
+            .vpc_lookup_by_path(organization_name, project_name, vpc_name)
+            .await?;
+        self.vpc_subnet_lookup_noauthz(&authz_vpc, subnet_name)
+            .await
+            .map(|(v, _)| v)
+    }
+
+    /// Lookup a VpcSubnet by name and return the full database record, along
+    /// with an [`authz::VpcSubnet`] for subsequent authorization checks
+    pub async fn vpc_subnet_fetch(
+        &self,
+        opctx: &OpContext,
+        authz_vpc: &authz::Vpc,
+        name: &Name,
+    ) -> LookupResult<(authz::VpcSubnet, VpcSubnet)> {
+        let (authz_vpc_subnet, db_vpc_subnet) =
+            self.vpc_subnet_lookup_noauthz(authz_vpc, name).await?;
+        opctx.authorize(authz::Action::Read, &authz_vpc_subnet).await?;
+        Ok((authz_vpc_subnet, db_vpc_subnet))
     }
 
     /// Insert a VPC Subnet, checking for unique IP address ranges.
     pub async fn vpc_create_subnet(
+        &self,
+        opctx: &OpContext,
+        authz_vpc: &authz::Vpc,
+        subnet: VpcSubnet,
+    ) -> Result<VpcSubnet, SubnetError> {
+        opctx
+            .authorize(authz::Action::CreateChild, authz_vpc)
+            .await
+            .map_err(SubnetError::External)?;
+        assert_eq!(authz_vpc.id(), subnet.vpc_id);
+
+        self.vpc_create_subnet_raw(subnet).await
+    }
+
+    pub(super) async fn vpc_create_subnet_raw(
         &self,
         subnet: VpcSubnet,
     ) -> Result<VpcSubnet, SubnetError> {
@@ -2488,66 +2562,71 @@ impl DataStore {
             })
     }
 
-    pub async fn vpc_delete_subnet(&self, subnet_id: &Uuid) -> DeleteResult {
-        use db::schema::vpc_subnet::dsl;
+    pub async fn vpc_delete_subnet(
+        &self,
+        opctx: &OpContext,
+        authz_subnet: &authz::VpcSubnet,
+    ) -> DeleteResult {
+        opctx.authorize(authz::Action::Delete, authz_subnet).await?;
 
+        use db::schema::vpc_subnet::dsl;
         let now = Utc::now();
         diesel::update(dsl::vpc_subnet)
             .filter(dsl::time_deleted.is_null())
-            .filter(dsl::id.eq(*subnet_id))
+            .filter(dsl::id.eq(authz_subnet.id()))
             .set(dsl::time_deleted.eq(now))
             .returning(VpcSubnet::as_returning())
-            .get_result_async(self.pool())
+            .get_result_async(self.pool_authorized(opctx).await?)
             .await
             .map_err(|e| {
                 public_error_from_diesel_pool(
                     e,
-                    ErrorHandler::NotFoundByLookup(
-                        ResourceType::VpcSubnet,
-                        LookupType::ById(*subnet_id),
-                    ),
+                    ErrorHandler::NotFoundByResource(authz_subnet),
                 )
             })?;
         Ok(())
     }
 
     pub async fn vpc_update_subnet(
         &self,
-        subnet_id: &Uuid,
+        opctx: &OpContext,
+        authz_subnet: &authz::VpcSubnet,
         updates: VpcSubnetUpdate,
-    ) -> Result<(), Error> {
-        use db::schema::vpc_subnet::dsl;
+    ) -> UpdateResult<VpcSubnet> {
+        opctx.authorize(authz::Action::Modify, authz_subnet).await?;
 
+        use db::schema::vpc_subnet::dsl;
         diesel::update(dsl::vpc_subnet)
             .filter(dsl::time_deleted.is_null())
-            .filter(dsl::id.eq(*subnet_id))
+            .filter(dsl::id.eq(authz_subnet.id()))
             .set(updates)
-            .execute_async(self.pool())
+            .returning(VpcSubnet::as_returning())
+            .get_result_async(self.pool_authorized(opctx).await?)
             .await
             .map_err(|e| {
                 public_error_from_diesel_pool(
                     e,
-                    ErrorHandler::NotFoundByLookup(
-                        ResourceType::VpcSubnet,
-                        LookupType::ById(*subnet_id),
-                    ),
+                    ErrorHandler::NotFoundByResource(authz_subnet),
                 )
-            })?;
-        Ok(())
+            })
     }
 
     pub async fn subnet_list_network_interfaces(
         &self,
-        subnet_id: &Uuid,
+        opctx: &OpContext,
+        authz_subnet: &authz::VpcSubnet,
         pagparams: &DataPageParams<'_, Name>,
     ) -> ListResultVec<NetworkInterface> {
-        use db::schema::network_interface::dsl;
+        opctx.authorize(authz::Action::ListChildren, authz_subnet).await?;
 
+        use db::schema::network_interface::dsl;
         paginated(dsl::network_interface, dsl::name, pagparams)
             .filter(dsl::time_deleted.is_null())
-            .filter(dsl::subnet_id.eq(*subnet_id))
+            .filter(dsl::subnet_id.eq(authz_subnet.id()))
             .select(NetworkInterface::as_select())
-            .load_async::<db::model::NetworkInterface>(self.pool())
+            .load_async::<db::model::NetworkInterface>(
+                self.pool_authorized(opctx).await?,
+            )
             .await
             .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server))
     }