From 9102e10399805b644bb03fed3d5db053c92c700e Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Wed, 23 Mar 2022 11:47:20 -0700 Subject: [PATCH] authz: protect various endpoints (#790) --- nexus/src/authn/mod.rs | 11 +++ nexus/src/authz/api_resources.rs | 54 +++++++++++- nexus/src/authz/mod.rs | 1 + nexus/src/authz/omicron.polar | 33 +++++-- nexus/src/authz/oso_generic.rs | 2 + nexus/src/db/datastore.rs | 41 ++++++--- .../db/fixed_data/role_assignment_builtin.rs | 11 +++ nexus/src/db/fixed_data/role_builtin.rs | 6 ++ nexus/src/db/fixed_data/user_builtin.rs | 11 +++ nexus/src/external_api/http_entrypoints.rs | 28 ++++-- nexus/src/internal_api/http_entrypoints.rs | 4 +- nexus/src/nexus.rs | 86 +++++++++++++++---- nexus/test-utils/src/lib.rs | 2 +- nexus/tests/integration_tests/basic.rs | 4 +- nexus/tests/integration_tests/endpoints.rs | 76 +++++++++++++++- .../tests/integration_tests/roles_builtin.rs | 1 + nexus/tests/integration_tests/timeseries.rs | 6 +- nexus/tests/integration_tests/unauthorized.rs | 7 +- nexus/tests/integration_tests/updates.rs | 36 ++++---- .../tests/integration_tests/users_builtin.rs | 2 + .../output/uncovered-authz-endpoints.txt | 8 -- 21 files changed, 350 insertions(+), 80 deletions(-) diff --git a/nexus/src/authn/mod.rs b/nexus/src/authn/mod.rs index c5eb280a56..be7bc1e16a 100644 --- a/nexus/src/authn/mod.rs +++ b/nexus/src/authn/mod.rs @@ -29,6 +29,7 @@ pub mod saga; pub use crate::db::fixed_data::user_builtin::USER_DB_INIT; pub use crate::db::fixed_data::user_builtin::USER_INTERNAL_API; +pub use crate::db::fixed_data::user_builtin::USER_INTERNAL_READ; pub use crate::db::fixed_data::user_builtin::USER_SAGA_RECOVERY; pub use crate::db::fixed_data::user_builtin::USER_TEST_PRIVILEGED; pub use crate::db::fixed_data::user_builtin::USER_TEST_UNPRIVILEGED; @@ -96,6 +97,11 @@ impl Context { Context::context_for_actor(USER_SAGA_RECOVERY.id) } + /// Returns an authenticated context for use by internal resource allocation + pub fn internal_read() -> Context { + Context::context_for_actor(USER_INTERNAL_READ.id) + } + /// Returns an authenticated context for Nexus-startup database /// initialization pub fn internal_db_init() -> Context { @@ -127,6 +133,7 @@ mod test { use super::Context; use super::USER_DB_INIT; use super::USER_INTERNAL_API; + use super::USER_INTERNAL_READ; use super::USER_SAGA_RECOVERY; use super::USER_TEST_PRIVILEGED; @@ -143,6 +150,10 @@ mod test { let actor = authn.actor().unwrap(); assert_eq!(actor.0, USER_TEST_PRIVILEGED.id); + let authn = Context::internal_read(); + let actor = authn.actor().unwrap(); + assert_eq!(actor.0, USER_INTERNAL_READ.id); + let authn = Context::internal_db_init(); let actor = authn.actor().unwrap(); assert_eq!(actor.0, USER_DB_INIT.id); diff --git a/nexus/src/authz/api_resources.rs b/nexus/src/authz/api_resources.rs index 36ff91aabb..64d12ea218 100644 --- a/nexus/src/authz/api_resources.rs +++ b/nexus/src/authz/api_resources.rs @@ -135,7 +135,8 @@ impl Fleet { } /// Returns an authz resource representing some other kind of child (e.g., - /// a built-in user, built-in role, etc. -- but _not_ an Organization) + /// a built-in user, built-in role, etc. -- but _not_ an Organization or + /// Sled) /// /// Aside from Organizations (which you create with /// [`Fleet::organization()`] instead), all instances of all types of Fleet @@ -149,6 +150,11 @@ impl Fleet { ) -> FleetChild { FleetChild { resource_type, lookup_type } } + + /// Returns an authz resource representing a Sled + pub fn sled(&self, sled_id: Uuid, lookup_type: LookupType) -> Sled { + Sled { sled_id, lookup_type } + } } impl Eq for Fleet {} @@ -255,6 +261,52 @@ impl ApiResourceError for FleetChild { } } +/// Represents a Sled for authz purposes +/// +/// This object is used for authorization checks on such resources by passing +/// this as the `resource` argument to +/// [`crate::context::OpContext::authorize()`]. You construct one of these +/// using [`Fleet::sled()`]. +#[derive(Clone, Debug)] +pub struct Sled { + sled_id: Uuid, + lookup_type: LookupType, +} + +impl Sled { + pub fn id(&self) -> Uuid { + self.sled_id + } +} + +impl oso::PolarClass for Sled { + fn get_polar_class_builder() -> oso::ClassBuilder { + oso::Class::builder() + .add_method( + "has_role", + // Roles are not supported on Sleds today. + |_: &Sled, _: AuthenticatedActor, _: String| false, + ) + .add_attribute_getter("fleet", |_: &Sled| FLEET) + } +} + +impl ApiResource for Sled { + fn db_resource(&self) -> Option<(ResourceType, Uuid)> { + None + } + + fn parent(&self) -> Option<&dyn AuthorizedResource> { + Some(&FLEET) + } +} + +impl ApiResourceError for Sled { + fn not_found(&self) -> Error { + self.lookup_type.clone().into_not_found(ResourceType::Sled) + } +} + /// Represents a [`crate::db::model::Organization`] for authz purposes /// /// This object is used for authorization checks on an Organization by passing diff --git a/nexus/src/authz/mod.rs b/nexus/src/authz/mod.rs index f47551b9e5..21092f6d9a 100644 --- a/nexus/src/authz/mod.rs +++ b/nexus/src/authz/mod.rs @@ -171,6 +171,7 @@ pub use api_resources::NetworkInterface; pub use api_resources::Organization; pub use api_resources::Project; pub use api_resources::RouterRoute; +pub use api_resources::Sled; pub use api_resources::Vpc; pub use api_resources::VpcRouter; pub use api_resources::VpcSubnet; diff --git a/nexus/src/authz/omicron.polar b/nexus/src/authz/omicron.polar index ab6383cea3..f79cf95b57 100644 --- a/nexus/src/authz/omicron.polar +++ b/nexus/src/authz/omicron.polar @@ -75,6 +75,7 @@ has_role(actor: AuthenticatedActor, "init", _resource: Database) # # - fleet.admin (superuser for the whole system) # - fleet.collaborator (can create and own orgs) +# - fleet.viewer (can read fleet-wide data) # - organization.admin (complete control over an organization) # - organization.collaborator (can create, modify, and delete projects) # - project.admin (complete control over a project) @@ -94,14 +95,17 @@ resource Fleet { "create_child", ]; - roles = [ "admin", "collaborator" ]; + roles = [ "admin", "collaborator", "viewer" ]; + + # Fleet viewers can view Fleet-wide data + "list_children" if "viewer"; + "read" if "viewer"; # Fleet collaborators can create Organizations and see fleet-wide # information, including Organizations that they don't have permissions # on. (They cannot list projects within those organizations, however.) # They cannot modify fleet-wide information. - "list_children" if "collaborator"; - "read" if "collaborator"; + "viewer" if "collaborator"; "create_child" if "collaborator"; # Fleet administrators are whole-system superusers. @@ -185,7 +189,7 @@ resource ProjectChild { } # Similarly, we use a generic resource to represent every kind of fleet-wide -# resource that's not part of the Organization/Project hierarchy. +# resource that's not part of the Organization/Project hierarchy and not a Sled. resource FleetChild { permissions = [ "list_children", @@ -195,8 +199,23 @@ resource FleetChild { ]; relations = { parent_fleet: Fleet }; - "list_children" if "admin" on "parent_fleet"; - "read" if "admin" on "parent_fleet"; + "list_children" if "viewer" on "parent_fleet"; + "read" if "viewer" on "parent_fleet"; + "modify" if "admin" on "parent_fleet"; + "create_child" if "admin" on "parent_fleet"; +} + +resource Sled { + permissions = [ + "list_children", + "modify", + "read", + "create_child", + ]; + + relations = { parent_fleet: Fleet }; + "list_children" if "viewer" on "parent_fleet"; + "read" if "viewer" on "parent_fleet"; "modify" if "admin" on "parent_fleet"; "create_child" if "admin" on "parent_fleet"; } @@ -210,6 +229,8 @@ has_relation(project: Project, "parent_project", project_child: ProjectChild) if project_child.project = project; has_relation(fleet: Fleet, "parent_fleet", fleet_child: FleetChild) if fleet_child.fleet = fleet; +has_relation(fleet: Fleet, "parent_fleet", sled: Sled) + if sled.fleet = fleet; # Define role relationships has_role(actor: AuthenticatedActor, role: String, resource: Resource) diff --git a/nexus/src/authz/oso_generic.rs b/nexus/src/authz/oso_generic.rs index 4af2e4cc37..3346c3466f 100644 --- a/nexus/src/authz/oso_generic.rs +++ b/nexus/src/authz/oso_generic.rs @@ -11,6 +11,7 @@ use super::api_resources::FleetChild; use super::api_resources::Organization; use super::api_resources::Project; use super::api_resources::ProjectChild; +use super::api_resources::Sled; use super::context::AuthorizedResource; use super::roles::RoleSet; use super::Authz; @@ -48,6 +49,7 @@ pub fn make_omicron_oso() -> Result { Project::get_polar_class(), ProjectChild::get_polar_class(), FleetChild::get_polar_class(), + Sled::get_polar_class(), ]; for c in classes { oso.register_class(c).context("registering class")?; diff --git a/nexus/src/db/datastore.rs b/nexus/src/db/datastore.rs index 08050cab5c..8aabb87ea5 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -158,30 +158,34 @@ impl DataStore { pub async fn sled_list( &self, + opctx: &OpContext, pagparams: &DataPageParams<'_, Uuid>, ) -> ListResultVec { + opctx.authorize(authz::Action::Read, &authz::FLEET).await?; use db::schema::sled::dsl; paginated(dsl::sled, dsl::id, pagparams) .select(Sled::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 sled_fetch(&self, id: Uuid) -> LookupResult { + pub async fn sled_fetch( + &self, + opctx: &OpContext, + authz_sled: &authz::Sled, + ) -> LookupResult { + opctx.authorize(authz::Action::Read, authz_sled).await?; use db::schema::sled::dsl; dsl::sled - .filter(dsl::id.eq(id)) + .filter(dsl::id.eq(authz_sled.id())) .select(Sled::as_select()) - .first_async(self.pool()) + .first_async(self.pool_authorized(opctx).await?) .await .map_err(|e| { public_error_from_diesel_pool( e, - ErrorHandler::NotFoundByLookup( - ResourceType::Sled, - LookupType::ById(id), - ), + ErrorHandler::NotFoundByResource(authz_sled), ) }) } @@ -3153,6 +3157,7 @@ impl DataStore { // Note: "db_init" is also a builtin user, but that one by necessity // is created with the database. &*authn::USER_INTERNAL_API, + &*authn::USER_INTERNAL_READ, &*authn::USER_SAGA_RECOVERY, &*authn::USER_TEST_PRIVILEGED, &*authn::USER_TEST_UNPRIVILEGED, @@ -3338,8 +3343,11 @@ impl DataStore { pub async fn update_available_artifact_upsert( &self, + opctx: &OpContext, artifact: UpdateAvailableArtifact, ) -> CreateResult { + opctx.authorize(authz::Action::Modify, &authz::FLEET).await?; + use db::schema::update_available_artifact::dsl; diesel::insert_into(dsl::update_available_artifact) .values(artifact.clone()) @@ -3347,21 +3355,25 @@ impl DataStore { .do_update() .set(artifact.clone()) .returning(UpdateAvailableArtifact::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::Server)) } pub async fn update_available_artifact_hard_delete_outdated( &self, + opctx: &OpContext, current_targets_role_version: i64, ) -> DeleteResult { - // We use the `targets_role_version` column in the table to delete any old rows, keeping - // the table in sync with the current copy of artifacts.json. + opctx.authorize(authz::Action::Modify, &authz::FLEET).await?; + + // We use the `targets_role_version` column in the table to delete any + // old rows, keeping the table in sync with the current copy of + // artifacts.json. use db::schema::update_available_artifact::dsl; diesel::delete(dsl::update_available_artifact) .filter(dsl::targets_role_version.lt(current_targets_role_version)) - .execute_async(self.pool()) + .execute_async(self.pool_authorized(opctx).await?) .await .map(|_rows_deleted| ()) .map_err(|e| { @@ -3374,8 +3386,11 @@ impl DataStore { pub async fn update_available_artifact_fetch( &self, + opctx: &OpContext, artifact: &UpdateArtifact, ) -> LookupResult { + opctx.authorize(authz::Action::Read, &authz::FLEET).await?; + use db::schema::update_available_artifact::dsl; dsl::update_available_artifact .filter( @@ -3385,7 +3400,7 @@ impl DataStore { .and(dsl::kind.eq(UpdateArtifactKind(artifact.kind))), ) .select(UpdateAvailableArtifact::as_select()) - .first_async(self.pool()) + .first_async(self.pool_authorized(opctx).await?) .await .map_err(|e| { Error::internal_error(&format!( diff --git a/nexus/src/db/fixed_data/role_assignment_builtin.rs b/nexus/src/db/fixed_data/role_assignment_builtin.rs index 3f2d4d9281..1814e513ef 100644 --- a/nexus/src/db/fixed_data/role_assignment_builtin.rs +++ b/nexus/src/db/fixed_data/role_assignment_builtin.rs @@ -31,5 +31,16 @@ lazy_static! { *FLEET_ID, role_builtin::FLEET_ADMIN.role_name, ), + + // The "internal-read" user gets the "viewer" role on the sole Fleet. + // This will grant them the ability to read various control plane + // data (like the list of sleds), which is in turn used to talk to + // sleds or allocate resources. + RoleAssignmentBuiltin::new( + user_builtin::USER_INTERNAL_READ.id, + role_builtin::FLEET_VIEWER.resource_type, + *FLEET_ID, + role_builtin::FLEET_VIEWER.role_name, + ), ]; } diff --git a/nexus/src/db/fixed_data/role_builtin.rs b/nexus/src/db/fixed_data/role_builtin.rs index bee076b3ad..c07ac64522 100644 --- a/nexus/src/db/fixed_data/role_builtin.rs +++ b/nexus/src/db/fixed_data/role_builtin.rs @@ -19,8 +19,14 @@ lazy_static! { role_name: "admin", description: "Fleet Administrator", }; + pub static ref FLEET_VIEWER: RoleBuiltinConfig = RoleBuiltinConfig { + resource_type: api::external::ResourceType::Fleet, + role_name: "viewer", + description: "Fleet Viewer", + }; pub static ref BUILTIN_ROLES: Vec = vec![ FLEET_ADMIN.clone(), + FLEET_VIEWER.clone(), RoleBuiltinConfig { resource_type: api::external::ResourceType::Fleet, role_name: "collaborator", diff --git a/nexus/src/db/fixed_data/user_builtin.rs b/nexus/src/db/fixed_data/user_builtin.rs index ebe65a295a..5320fe1c0d 100644 --- a/nexus/src/db/fixed_data/user_builtin.rs +++ b/nexus/src/db/fixed_data/user_builtin.rs @@ -47,6 +47,15 @@ lazy_static! { "used by Nexus when handling internal API requests", ); + /// Internal user used by Nexus to read privileged control plane data + pub static ref USER_INTERNAL_READ: UserBuiltinConfig = + UserBuiltinConfig::new_static( + // "4ead" looks like "read" + "001de000-05e4-4000-8000-000000004ead", + "internal-read", + "used by Nexus to read privileged control plane data", + ); + /// Internal user used by Nexus when recovering sagas pub static ref USER_SAGA_RECOVERY: UserBuiltinConfig = UserBuiltinConfig::new_static( @@ -83,6 +92,7 @@ mod test { use super::super::assert_valid_uuid; use super::USER_DB_INIT; use super::USER_INTERNAL_API; + use super::USER_INTERNAL_READ; use super::USER_SAGA_RECOVERY; use super::USER_TEST_PRIVILEGED; use super::USER_TEST_UNPRIVILEGED; @@ -91,6 +101,7 @@ mod test { fn test_builtin_user_ids_are_valid() { assert_valid_uuid(&USER_DB_INIT.id); assert_valid_uuid(&USER_INTERNAL_API.id); + assert_valid_uuid(&USER_INTERNAL_READ.id); assert_valid_uuid(&USER_SAGA_RECOVERY.id); assert_valid_uuid(&USER_TEST_PRIVILEGED.id); assert_valid_uuid(&USER_TEST_UNPRIVILEGED.id); diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 7585845622..48440bd932 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -2123,8 +2123,10 @@ async fn hardware_racks_get( let nexus = &apictx.nexus; let query = query_params.into_inner(); let handler = async { - let rack_stream = - nexus.racks_list(&data_page_params_for(&rqctx, &query)?).await?; + let opctx = OpContext::for_external_api(&rqctx).await?; + let rack_stream = nexus + .racks_list(&opctx, &data_page_params_for(&rqctx, &query)?) + .await?; let view_list = to_list::(rack_stream).await; Ok(HttpResponseOk(ScanById::results_page(&query, view_list)?)) }; @@ -2152,7 +2154,8 @@ async fn hardware_racks_get_rack( let nexus = &apictx.nexus; let path = path_params.into_inner(); let handler = async { - let rack_info = nexus.rack_lookup(&path.rack_id).await?; + let opctx = OpContext::for_external_api(&rqctx).await?; + let rack_info = nexus.rack_lookup(&opctx, &path.rack_id).await?; Ok(HttpResponseOk(rack_info.into())) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await @@ -2174,8 +2177,9 @@ async fn hardware_sleds_get( let nexus = &apictx.nexus; let query = query_params.into_inner(); let handler = async { + let opctx = OpContext::for_external_api(&rqctx).await?; let sleds = nexus - .sleds_list(&data_page_params_for(&rqctx, &query)?) + .sleds_list(&opctx, &data_page_params_for(&rqctx, &query)?) .await? .into_iter() .map(|s| s.into()) @@ -2206,7 +2210,8 @@ async fn hardware_sleds_get_sled( let nexus = &apictx.nexus; let path = path_params.into_inner(); let handler = async { - let sled_info = nexus.sled_lookup(&path.sled_id).await?; + let opctx = OpContext::for_external_api(&rqctx).await?; + let sled_info = nexus.sled_lookup(&opctx, &path.sled_id).await?; Ok(HttpResponseOk(sled_info.into())) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await @@ -2226,7 +2231,8 @@ async fn updates_refresh( let apictx = rqctx.context(); let nexus = &apictx.nexus; let handler = async { - nexus.updates_refresh_metadata().await?; + let opctx = OpContext::for_external_api(&rqctx).await?; + nexus.updates_refresh_metadata(&opctx).await?; Ok(HttpResponseUpdatedNoContent()) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await @@ -2249,7 +2255,8 @@ async fn sagas_get( let query = query_params.into_inner(); let pagparams = data_page_params_for(&rqctx, &query)?; let handler = async { - let saga_stream = nexus.sagas_list(&pagparams).await?; + let opctx = OpContext::for_external_api(&rqctx).await?; + let saga_stream = nexus.sagas_list(&opctx, &pagparams).await?; let view_list = to_list(saga_stream).await; Ok(HttpResponseOk(ScanById::results_page(&query, view_list)?)) }; @@ -2276,7 +2283,8 @@ async fn sagas_get_saga( let nexus = &apictx.nexus; let path = path_params.into_inner(); let handler = async { - let saga = nexus.saga_get(path.saga_id).await?; + let opctx = OpContext::for_external_api(&rqctx).await?; + let saga = nexus.saga_get(&opctx, path.saga_id).await?; Ok(HttpResponseOk(saga)) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await @@ -2357,7 +2365,9 @@ async fn timeseries_schema_get( let query = query_params.into_inner(); let limit = rqctx.page_limit(&query)?; let handler = async { - Ok(HttpResponseOk(nexus.timeseries_schema_list(&query, limit).await?)) + let opctx = OpContext::for_external_api(&rqctx).await?; + let list = nexus.timeseries_schema_list(&opctx, &query, limit).await?; + Ok(HttpResponseOk(list)) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await } diff --git a/nexus/src/internal_api/http_entrypoints.rs b/nexus/src/internal_api/http_entrypoints.rs index d3e3ec4f9c..4df80916d7 100644 --- a/nexus/src/internal_api/http_entrypoints.rs +++ b/nexus/src/internal_api/http_entrypoints.rs @@ -272,8 +272,10 @@ async fn cpapi_artifact_download( ) -> Result, HttpError> { let context = request_context.context(); let nexus = &context.nexus; + let opctx = OpContext::for_internal_api(&request_context).await; // TODO: return 404 if the error we get here says that the record isn't found - let body = nexus.download_artifact(path_params.into_inner()).await?; + let body = + nexus.download_artifact(&opctx, path_params.into_inner()).await?; Ok(Response::builder().status(StatusCode::OK).body(body.into())?) } diff --git a/nexus/src/nexus.rs b/nexus/src/nexus.rs index 96131ff6e9..fc57e56fb0 100644 --- a/nexus/src/nexus.rs +++ b/nexus/src/nexus.rs @@ -143,6 +143,9 @@ pub struct Nexus { /// Contents of the trusted root role for the TUF repository. updates_config: Option, + + /// Operational context used for Instance allocation + opctx_alloc: OpContext, } // TODO Is it possible to make some of these operations more generic? A @@ -204,6 +207,12 @@ impl Nexus { populate_status, timeseries_client, updates_config: config.updates.clone(), + opctx_alloc: OpContext::for_background( + log.new(o!("component" => "InstanceAllocator")), + Arc::clone(&authz), + authn::Context::internal_read(), + Arc::clone(&db_datastore), + ), }; // TODO-cleanup all the extra Arcs here seems wrong @@ -806,6 +815,13 @@ impl Nexus { // TODO-design This interface should not exist. See // SagaContext::alloc_server(). pub async fn sled_allocate(&self) -> Result { + // We need an OpContext to query the database. Normally we'd use + // one from the current operation, usually a saga action or API call. + // In this case, though, the caller may not have permissions to access + // the sleds in the system. We're really doing this as Nexus itself, + // operating on behalf of the caller. + let opctx = &self.opctx_alloc; + // TODO: replace this with a real allocation policy. // // This implementation always assigns the first sled (by ID order). @@ -814,7 +830,7 @@ impl Nexus { direction: dropshot::PaginationOrder::Ascending, limit: std::num::NonZeroU32::new(1).unwrap(), }; - let sleds = self.db_datastore.sled_list(&pagparams).await?; + let sleds = self.db_datastore.sled_list(&opctx, &pagparams).await?; sleds .first() @@ -1052,7 +1068,7 @@ impl Nexus { // Franky, returning an "Arc" here without a connection pool is a little // silly; it's not actually used if each client connection exists as a // one-shot. - let sled = self.sled_lookup(id).await?; + let sled = self.sled_lookup(&self.opctx_alloc, id).await?; let log = self.log.new(o!("SledAgent" => id.clone().to_string())); let dur = std::time::Duration::from_secs(60); @@ -2479,8 +2495,11 @@ impl Nexus { pub async fn racks_list( &self, + opctx: &OpContext, pagparams: &DataPageParams<'_, Uuid>, ) -> ListResult { + opctx.authorize(authz::Action::Read, &authz::FLEET).await?; + if let Some(marker) = pagparams.marker { if *marker >= self.rack_id { return Ok(futures::stream::empty().boxed()); @@ -2492,8 +2511,13 @@ impl Nexus { pub async fn rack_lookup( &self, + opctx: &OpContext, rack_id: &Uuid, ) -> LookupResult { + let authz_rack = authz::FLEET + .child_generic(ResourceType::Rack, LookupType::ById(*rack_id)); + opctx.authorize(authz::Action::Read, &authz_rack).await?; + if *rack_id == self.rack_id { Ok(self.as_rack()) } else { @@ -2505,22 +2529,27 @@ impl Nexus { pub async fn sleds_list( &self, + opctx: &OpContext, pagparams: &DataPageParams<'_, Uuid>, ) -> ListResultVec { - self.db_datastore.sled_list(pagparams).await + self.db_datastore.sled_list(&opctx, pagparams).await } pub async fn sled_lookup( &self, + opctx: &OpContext, sled_id: &Uuid, ) -> LookupResult { - self.db_datastore.sled_fetch(*sled_id).await + let authz_sled = + authz::FLEET.sled(*sled_id, LookupType::ById(*sled_id)); + self.db_datastore.sled_fetch(&opctx, &authz_sled).await } // Sagas pub async fn sagas_list( &self, + opctx: &OpContext, pagparams: &DataPageParams<'_, Uuid>, ) -> ListResult { // The endpoint we're serving only supports `ScanById`, which only @@ -2528,6 +2557,7 @@ impl Nexus { bail_unless!( pagparams.direction == dropshot::PaginationOrder::Ascending ); + opctx.authorize(authz::Action::Read, &authz::FLEET).await?; let marker = pagparams.marker.map(|s| SagaId::from(*s)); let saga_list = self .sec_client @@ -2539,7 +2569,12 @@ impl Nexus { Ok(futures::stream::iter(saga_list).boxed()) } - pub async fn saga_get(&self, id: Uuid) -> LookupResult { + pub async fn saga_get( + &self, + opctx: &OpContext, + id: Uuid, + ) -> LookupResult { + opctx.authorize(authz::Action::Read, &authz::FLEET).await?; self.sec_client .saga_get(steno::SagaId::from(id)) .await @@ -2706,9 +2741,11 @@ impl Nexus { /// List existing timeseries schema. pub async fn timeseries_schema_list( &self, + opctx: &OpContext, pag_params: &TimeseriesSchemaPaginationParams, limit: NonZeroU32, ) -> Result, Error> { + opctx.authorize(authz::Action::Read, &authz::FLEET).await?; self.timeseries_client .timeseries_schema_list(&pag_params.page, limit) .await @@ -2798,7 +2835,12 @@ impl Nexus { }) } - pub async fn updates_refresh_metadata(&self) -> Result<(), Error> { + pub async fn updates_refresh_metadata( + &self, + opctx: &OpContext, + ) -> Result<(), Error> { + opctx.authorize(authz::Action::Modify, &authz::FLEET).await?; + let updates_config = self.updates_config.as_ref().ok_or_else(|| { Error::InvalidRequest { message: "updates system not configured".into(), @@ -2826,33 +2868,39 @@ impl Nexus { internal_message: format!("error trying to refresh updates: {}", e), })?; - // FIXME: if we hit an error in any of these database calls, the available artifact table - // will be out of sync with the current artifacts.json. can we do a transaction or - // something? + // FIXME: if we hit an error in any of these database calls, the + // available artifact table will be out of sync with the current + // artifacts.json. can we do a transaction or something? let mut current_version = None; for artifact in &artifacts { current_version = Some(artifact.targets_role_version); self.db_datastore - .update_available_artifact_upsert(artifact.clone()) + .update_available_artifact_upsert(&opctx, artifact.clone()) .await?; } // ensure table is in sync with current copy of artifacts.json if let Some(current_version) = current_version { self.db_datastore - .update_available_artifact_hard_delete_outdated(current_version) + .update_available_artifact_hard_delete_outdated( + &opctx, + current_version, + ) .await?; } // demo-grade update logic: tell all sleds to apply all artifacts for sled in self .db_datastore - .sled_list(&DataPageParams { - marker: None, - direction: PaginationOrder::Ascending, - limit: NonZeroU32::new(100).unwrap(), - }) + .sled_list( + &opctx, + &DataPageParams { + marker: None, + direction: PaginationOrder::Ascending, + limit: NonZeroU32::new(100).unwrap(), + }, + ) .await? { let client = self.sled_client(&sled.id()).await?; @@ -2881,6 +2929,7 @@ impl Nexus { /// Downloads a file from within [`BASE_ARTIFACT_DIR`]. pub async fn download_artifact( &self, + opctx: &OpContext, artifact: UpdateArtifact, ) -> Result, Error> { let mut base_url = @@ -2891,10 +2940,11 @@ impl Nexus { base_url.push('/'); } - // We cache the artifact based on its checksum, so fetch that from the database. + // We cache the artifact based on its checksum, so fetch that from the + // database. let artifact_entry = self .db_datastore - .update_available_artifact_fetch(&artifact) + .update_available_artifact_fetch(opctx, &artifact) .await?; let filename = format!( "{}.{}.{}-{}", diff --git a/nexus/test-utils/src/lib.rs b/nexus/test-utils/src/lib.rs index 090f98ba41..00789986fd 100644 --- a/nexus/test-utils/src/lib.rs +++ b/nexus/test-utils/src/lib.rs @@ -27,7 +27,7 @@ pub mod http_testing; pub mod resource_helpers; pub const SLED_AGENT_UUID: &str = "b6d65341-167c-41df-9b5c-41cded99c229"; -const RACK_UUID: &str = "c19a698f-c6f9-4a17-ae30-20d711b8f7dc"; +pub const RACK_UUID: &str = "c19a698f-c6f9-4a17-ae30-20d711b8f7dc"; pub const OXIMETER_UUID: &str = "39e6175b-4df2-4730-b11d-cbc1e60a2e78"; pub const PRODUCER_UUID: &str = "a6458b7d-87c3-4483-be96-854d814c20de"; diff --git a/nexus/tests/integration_tests/basic.rs b/nexus/tests/integration_tests/basic.rs index a0fb62d396..11089d0613 100644 --- a/nexus/tests/integration_tests/basic.rs +++ b/nexus/tests/integration_tests/basic.rs @@ -7,11 +7,11 @@ //! This file defines a very basic set of tests against the API. //! TODO-coverage add test for racks, sleds -use dropshot::test_util::objects_list_page; use dropshot::test_util::ClientTestContext; use dropshot::HttpErrorResponseBody; use http::method::Method; use http::StatusCode; +use nexus_test_utils::resource_helpers::objects_list_page_authz; use nexus_test_utils::resource_helpers::project_get; use omicron_common::api::external::IdentityMetadataCreateParams; use omicron_common::api::external::IdentityMetadataUpdateParams; @@ -634,5 +634,5 @@ async fn projects_list( } async fn sleds_list(client: &ClientTestContext, sleds_url: &str) -> Vec { - objects_list_page::(client, sleds_url).await.items + objects_list_page_authz::(client, sleds_url).await.items } diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs index 6af67d4f04..0d0fcc87ee 100644 --- a/nexus/tests/integration_tests/endpoints.rs +++ b/nexus/tests/integration_tests/endpoints.rs @@ -9,6 +9,8 @@ use http::method::Method; use lazy_static::lazy_static; +use nexus_test_utils::RACK_UUID; +use nexus_test_utils::SLED_AGENT_UUID; use omicron_common::api::external::ByteCount; use omicron_common::api::external::IdentityMetadataCreateParams; use omicron_common::api::external::IdentityMetadataUpdateParams; @@ -26,6 +28,11 @@ use std::net::IpAddr; use std::net::Ipv4Addr; lazy_static! { + pub static ref HARDWARE_RACK_URL: String = + format!("/hardware/racks/{}", RACK_UUID); + pub static ref HARDWARE_SLED_URL: String = + format!("/hardware/sleds/{}", SLED_AGENT_UUID); + // Organization used for testing pub static ref DEMO_ORG_NAME: Name = "demo-org".parse().unwrap(); pub static ref DEMO_ORG_URL: String = @@ -224,6 +231,12 @@ pub enum AllowedMethod { Delete, /// HTTP "GET" method Get, + /// HTTP "GET" method, but where we cannot statically define a URL that will + /// work (so the test runner should not expect to get a 200). This should + /// be uncommon. In most cases, resources are identified either by names + /// that we define here or uuids that we control in the test suite (e.g., + /// the rack and sled uuids). + GetNonexistent, /// HTTP "POST" method, with sample input (which should be valid input for /// this endpoint) Post(serde_json::Value), @@ -238,6 +251,7 @@ impl AllowedMethod { match self { AllowedMethod::Delete => &Method::DELETE, AllowedMethod::Get => &Method::GET, + AllowedMethod::GetNonexistent => &Method::GET, AllowedMethod::Post(_) => &Method::POST, AllowedMethod::Put(_) => &Method::PUT, } @@ -249,7 +263,9 @@ impl AllowedMethod { /// If this returns `None`, the request body should be empty. pub fn body(&self) -> Option<&serde_json::Value> { match self { - AllowedMethod::Delete | AllowedMethod::Get => None, + AllowedMethod::Delete + | AllowedMethod::Get + | AllowedMethod::GetNonexistent => None, AllowedMethod::Post(body) => Some(&body), AllowedMethod::Put(body) => Some(&body), } @@ -590,5 +606,63 @@ lazy_static! { visibility: Visibility::Protected, allowed_methods: vec![AllowedMethod::Get], }, + + /* Hardware */ + + VerifyEndpoint { + url: "/hardware/racks", + visibility: Visibility::Public, + allowed_methods: vec![AllowedMethod::Get], + }, + + VerifyEndpoint { + url: &*HARDWARE_RACK_URL, + visibility: Visibility::Protected, + allowed_methods: vec![AllowedMethod::Get], + }, + + VerifyEndpoint { + url: "/hardware/sleds", + visibility: Visibility::Public, + allowed_methods: vec![AllowedMethod::Get], + }, + + VerifyEndpoint { + url: &*HARDWARE_SLED_URL, + visibility: Visibility::Protected, + allowed_methods: vec![AllowedMethod::Get], + }, + + /* Sagas */ + + VerifyEndpoint { + url: "/sagas", + visibility: Visibility::Public, + allowed_methods: vec![AllowedMethod::Get], + }, + + VerifyEndpoint { + url: "/sagas/48a1b8c8-fc1c-6fea-9de9-fdeb8dda7823", + visibility: Visibility::Public, + allowed_methods: vec![AllowedMethod::GetNonexistent], + }, + + /* Timeseries schema */ + + VerifyEndpoint { + url: "/timeseries/schema", + visibility: Visibility::Public, + allowed_methods: vec![AllowedMethod::Get], + }, + + /* Updates */ + + VerifyEndpoint { + url: "/updates/refresh", + visibility: Visibility::Public, + allowed_methods: vec![AllowedMethod::Post( + serde_json::Value::Null + )], + }, ]; } diff --git a/nexus/tests/integration_tests/roles_builtin.rs b/nexus/tests/integration_tests/roles_builtin.rs index 78cb1efdb2..dff7946f1f 100644 --- a/nexus/tests/integration_tests/roles_builtin.rs +++ b/nexus/tests/integration_tests/roles_builtin.rs @@ -30,6 +30,7 @@ async fn test_roles_builtin(cptestctx: &ControlPlaneTestContext) { let expected = [ ("fleet.admin", "Fleet Administrator"), ("fleet.collaborator", "Fleet Collaborator"), + ("fleet.viewer", "Fleet Viewer"), ("organization.admin", "Organization Administrator"), ("organization.collaborator", "Organization Collaborator"), ("project.admin", "Project Administrator"), diff --git a/nexus/tests/integration_tests/timeseries.rs b/nexus/tests/integration_tests/timeseries.rs index aa5a285547..6e02d5f7a6 100644 --- a/nexus/tests/integration_tests/timeseries.rs +++ b/nexus/tests/integration_tests/timeseries.rs @@ -2,7 +2,7 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -use dropshot::test_util::objects_list_page; +use nexus_test_utils::resource_helpers::objects_list_page_authz; use nexus_test_utils::ControlPlaneTestContext; use nexus_test_utils_macros::nexus_test; use omicron_test_utils::dev::poll::{wait_for_condition, CondCheckError}; @@ -18,7 +18,7 @@ async fn test_timeseries_schema(context: &ControlPlaneTestContext) { const POLL_DURATION: Duration = Duration::from_secs(10); let page = wait_for_condition( || async { - let page = objects_list_page::( + let page = objects_list_page_authz::( client, "/timeseries/schema", ) @@ -44,7 +44,7 @@ async fn test_timeseries_schema(context: &ControlPlaneTestContext) { "/timeseries/schema?page_token={}", page.next_page.as_ref().unwrap() ); - let page = objects_list_page::(client, &url).await; + let page = objects_list_page_authz::(client, &url).await; assert!( page.next_page.is_none(), "Expected exactly one page of timeseries schema" diff --git a/nexus/tests/integration_tests/unauthorized.rs b/nexus/tests/integration_tests/unauthorized.rs index 8f6f774511..0c0aacdad9 100644 --- a/nexus/tests/integration_tests/unauthorized.rs +++ b/nexus/tests/integration_tests/unauthorized.rs @@ -213,7 +213,7 @@ async fn verify_endpoint( let get_allowed = endpoint .allowed_methods .iter() - .any(|allowed| allowed.http_method() == Method::GET); + .any(|allowed| matches!(allowed, AllowedMethod::Get)); let resource_before: Option = if get_allowed { info!(log, "test: privileged GET"); Some( @@ -366,7 +366,10 @@ fn verify_response(response: &TestResponse) { StatusCode::NOT_FOUND => { assert_eq!(error.error_code.unwrap(), "ObjectNotFound"); assert!(error.message.starts_with("not found: ")); - assert!(error.message.contains(" with name \"")); + assert!( + error.message.contains(" with name \"") + || error.message.contains(" with id \"") + ); assert!(error.message.ends_with("\"")); } StatusCode::METHOD_NOT_ALLOWED => { diff --git a/nexus/tests/integration_tests/updates.rs b/nexus/tests/integration_tests/updates.rs index 9147200c9c..ce7243de90 100644 --- a/nexus/tests/integration_tests/updates.rs +++ b/nexus/tests/integration_tests/updates.rs @@ -15,6 +15,7 @@ use dropshot::{ }; use http::{Method, Response, StatusCode}; use hyper::Body; +use nexus_test_utils::http_testing::{AuthnMode, NexusRequest, RequestBuilder}; use nexus_test_utils::{load_test_config, test_setup, test_setup_with_config}; use omicron_common::api::internal::nexus::UpdateArtifactKind; use omicron_nexus::config::UpdatesConfig; @@ -76,20 +77,22 @@ async fn test_update_end_to_end() { // - download and verify the repo // - return 204 Non Content // - tells sled agent to do the thing - client - .make_request_no_body( - Method::POST, - "/updates/refresh", - StatusCode::NO_CONTENT, - ) - .await - .unwrap(); + NexusRequest::new( + RequestBuilder::new(client, Method::POST, "/updates/refresh") + .expect_status(Some(StatusCode::NO_CONTENT)), + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap(); // check sled agent did the thing assert_eq!( std::fs::read("/var/tmp/zones/cockroachdb").unwrap(), TARGET_CONTENTS ); + + cptestctx.teardown().await; } // =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= @@ -275,13 +278,16 @@ async fn test_download_with_dots_fails() { let filename = "hey/can/you/look/../../../../up/the/directory/tree"; let artifact_get_url = format!("/artifacts/{}", filename); - client - .make_request_error( - Method::GET, - &artifact_get_url, - StatusCode::BAD_REQUEST, - ) - .await; + NexusRequest::expect_failure( + client, + StatusCode::BAD_REQUEST, + Method::GET, + &artifact_get_url, + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap(); cptestctx.teardown().await; } diff --git a/nexus/tests/integration_tests/users_builtin.rs b/nexus/tests/integration_tests/users_builtin.rs index 7796d60ad3..47be370593 100644 --- a/nexus/tests/integration_tests/users_builtin.rs +++ b/nexus/tests/integration_tests/users_builtin.rs @@ -29,6 +29,8 @@ async fn test_users_builtin(cptestctx: &ControlPlaneTestContext) { assert_eq!(u.identity.id, authn::USER_DB_INIT.id); let u = users.remove(&authn::USER_INTERNAL_API.name.to_string()).unwrap(); assert_eq!(u.identity.id, authn::USER_INTERNAL_API.id); + let u = users.remove(&authn::USER_INTERNAL_READ.name.to_string()).unwrap(); + assert_eq!(u.identity.id, authn::USER_INTERNAL_READ.id); let u = users.remove(&authn::USER_SAGA_RECOVERY.name.to_string()).unwrap(); assert_eq!(u.identity.id, authn::USER_SAGA_RECOVERY.id); let u = diff --git a/nexus/tests/output/uncovered-authz-endpoints.txt b/nexus/tests/output/uncovered-authz-endpoints.txt index 426954b0f6..244106258b 100644 --- a/nexus/tests/output/uncovered-authz-endpoints.txt +++ b/nexus/tests/output/uncovered-authz-endpoints.txt @@ -1,22 +1,14 @@ API endpoints with no coverage in authz tests: instance_network_interfaces_delete_interface (delete "/organizations/{organization_name}/projects/{project_name}/instances/{instance_name}/network-interfaces/{interface_name}") project_snapshots_delete_snapshot (delete "/organizations/{organization_name}/projects/{project_name}/snapshots/{snapshot_name}") -hardware_racks_get (get "/hardware/racks") -hardware_racks_get_rack (get "/hardware/racks/{rack_id}") -hardware_sleds_get (get "/hardware/sleds") -hardware_sleds_get_sled (get "/hardware/sleds/{sled_id}") instance_disks_get (get "/organizations/{organization_name}/projects/{project_name}/instances/{instance_name}/disks") instance_network_interfaces_get (get "/organizations/{organization_name}/projects/{project_name}/instances/{instance_name}/network-interfaces") instance_network_interfaces_get_interface (get "/organizations/{organization_name}/projects/{project_name}/instances/{instance_name}/network-interfaces/{interface_name}") project_snapshots_get (get "/organizations/{organization_name}/projects/{project_name}/snapshots") project_snapshots_get_snapshot (get "/organizations/{organization_name}/projects/{project_name}/snapshots/{snapshot_name}") subnet_network_interfaces_get (get "/organizations/{organization_name}/projects/{project_name}/vpcs/{vpc_name}/subnets/{subnet_name}/network-interfaces") -sagas_get (get "/sagas") -sagas_get_saga (get "/sagas/{saga_id}") session_me (get "/session/me") -timeseries_schema_get (get "/timeseries/schema") spoof_login (post "/login") logout (post "/logout") instance_network_interfaces_post (post "/organizations/{organization_name}/projects/{project_name}/instances/{instance_name}/network-interfaces") project_snapshots_post (post "/organizations/{organization_name}/projects/{project_name}/snapshots") -updates_refresh (post "/updates/refresh")