authz: protect various endpoints #790

davepacheco · 2022-03-18T23:39:00Z

This started as a small PR to protect a handful of miscellaneous unprotected endpoints (in categories that were too small for their own PR). But it grew much larger than expected. If it would be helpful for me to break this up, I can do that.

I annotated the changes with some context to help sort through them. Key ones:

Added a new "internal-read" user for reading control plane data from Nexus
Added new "fleet viewer" role for the new user
Added authz::Sled
Added protection for the following endpoints:

hardware_racks_get
hardware_racks_get_rack
hardware_sleds_get
hardware_sleds_get_sled
sagas_get
sagas_get_saga
timeseries_schema_get
updates_refresh

davepacheco · 2022-03-18T23:54:40Z

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;


The changes in this file are all part of adding a new "internal-read" user (described later).

davepacheco · 2022-03-18T23:55:56Z

nexus/src/authz/api_resources.rs

+/// [`crate::context::OpContext::authorize()`].  You construct one of these
+/// using [`Fleet::sled()`].
+#[derive(Clone, Debug)]
+pub struct Sled {


I wanted to keep things simple and put sled permissions under read permission on the fleet, but that doesn't trigger the correct 404 behavior when you ask for a Sled that doesn't exist (because it would say the fleet doesn't exist). So then I wanted to use authz::FleetChild, but that doesn't include an id (and can't, because it's used for built-in roles that don't have one). So I created a real new struct for Sled.

Note that I want to refactor all the authz API resources to be more type-safe, but that'll be a bigger (future) change.

davepacheco · 2022-03-18T23:56:55Z

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)


I added this new "Fleet Viewer" role for the new "internal-read" user (described later).

davepacheco · 2022-03-18T23:58:12Z

nexus/src/db/datastore.rs

        artifact: UpdateAvailableArtifact,
    ) -> CreateResult<UpdateAvailableArtifact> {
+        opctx.authorize(authz::Action::Modify, &authz::FLEET).await?;


@iliana here's one of the couple of authz checks added for updates. I started as restrictive as possible: you basically have to have a near-superuser privilege on the fleet-level ("Fleet Collaborator" or "Fleet Admin") to have this permission. I imagine we'll want to rethink this but I hope this will work for now.

davepacheco · 2022-03-18T23:58:21Z

nexus/src/db/datastore.rs

        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?;


davepacheco · 2022-03-18T23:58:29Z

nexus/src/db/datastore.rs

        artifact: &UpdateArtifact,
    ) -> LookupResult<UpdateAvailableArtifact> {
+        opctx.authorize(authz::Action::Read, &authz::FLEET).await?;


davepacheco · 2022-03-18T23:59:00Z

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


Added a new built-in user called "internal-read". This is used by Nexus when it needs to read control plane data that an ordinary user wouldn't have access to. For example, when a user provisions an Instance (or really any time we need to make a request to a Sled Agent), we need to look up information about the Sled in the database. That's now protected by authz. But the end user making the request probably doesn't have permission to read information about sleds. Instead, Nexus uses this new "internal-read" user.

davepacheco · 2022-03-19T00:00:28Z

nexus/src/nexus.rs

        pagparams: &DataPageParams<'_, Uuid>,
    ) -> ListResult<db::model::Rack> {
+        opctx.authorize(authz::Action::Read, &authz::FLEET).await?;


Racks are faked-up -- not in the database -- so their access check is right in nexus.rs.

nexus/src/nexus.rs

davepacheco · 2022-03-19T00:01:50Z

nexus/src/nexus.rs

+        &self,
+        opctx: &OpContext,
+    ) -> Result<(), Error> {
+        opctx.authorize(authz::Action::Modify, &authz::FLEET).await?;


CC @iliana I put this check here to avoid querying the tuf repo unless the user is authorized. The database queries are separately authorized in datastore.rs.

I think this works for now. In the future, this code path will be called within Nexus when automatic updates are enabled (perhaps once every few hours, or some other regular cadence). What might this look like when we start doing that?

For that, we could create an OpContext when Nexus starts up specifically for use to do automatic updates, similar to what we do for saga recovery:

omicron/nexus/src/nexus.rs

Lines 211 to 216 in c66d2f6

let opctx = OpContext::for_background(

log.new(o!("component" => "SagaRecoverer")),

Arc::clone(&authz),

authn::Context::internal_saga_recovery(),

Arc::clone(&db_datastore),

);

This uses OpContext::for_background:

omicron/nexus/src/context.rs

Line 349 in c66d2f6

pub fn for_background(

We could create a separate built-in user for that context that would only have whatever privileges are needed for it. Then whenever we did any of these operations, we'd use that background OpContext that's associated with that user.

iliana · 2022-03-19T02:21:39Z

nexus/src/nexus.rs

+        &self,
+        opctx: &OpContext,
+    ) -> Result<(), Error> {
+        opctx.authorize(authz::Action::Modify, &authz::FLEET).await?;


I think this works for now. In the future, this code path will be called within Nexus when automatic updates are enabled (perhaps once every few hours, or some other regular cadence). What might this look like when we start doing that?

davepacheco · 2022-03-23T18:03:53Z

Thanks for the reviews!

davepacheco added 3 commits March 18, 2022 15:34

authz: protect a bunch of endpoints

be53402

more work

3c14179

more fixes

e9f0886

davepacheco commented Mar 19, 2022

View reviewed changes

davepacheco requested review from iliana and bnaecker March 19, 2022 00:04

davepacheco marked this pull request as ready for review March 19, 2022 00:04

iliana approved these changes Mar 19, 2022

View reviewed changes

davepacheco mentioned this pull request Mar 22, 2022

tracking issue for authz work #419

Closed

71 tasks

Merge branch 'main' into authz-misc

e94381e

davepacheco enabled auto-merge (squash) March 23, 2022 18:03

davepacheco merged commit 9102e10 into main Mar 23, 2022

davepacheco deleted the authz-misc branch March 23, 2022 18:47

iliana mentioned this pull request Mar 25, 2022

/updates/refresh should check authZ #761

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

authz: protect various endpoints #790

authz: protect various endpoints #790

davepacheco commented Mar 18, 2022 •

edited

Loading

davepacheco Mar 18, 2022

davepacheco Mar 18, 2022

davepacheco Mar 18, 2022

davepacheco Mar 18, 2022

davepacheco Mar 18, 2022

davepacheco Mar 18, 2022

davepacheco Mar 18, 2022

davepacheco Mar 19, 2022

davepacheco Mar 19, 2022

iliana Mar 19, 2022

davepacheco Mar 19, 2022

iliana Mar 19, 2022

davepacheco commented Mar 23, 2022

	let opctx = OpContext::for_background(
	log.new(o!("component" => "SagaRecoverer")),
	Arc::clone(&authz),
	authn::Context::internal_saga_recovery(),
	Arc::clone(&db_datastore),
	);

authz: protect various endpoints #790

authz: protect various endpoints #790

Conversation

davepacheco commented Mar 18, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davepacheco commented Mar 23, 2022

davepacheco commented Mar 18, 2022 •

edited

Loading