Investigate admin role and admin-level operations

[paulhowardarm]

Summary

A use case has been identified where it would be valuable to have an admin-level role within Parsec, such that a subset of API opcodes could be classed as admin-only operations and only be permitted when the calling user can prove itself to be one of a limited number of admin-level clients.

Scope

The intention here is not necessarily to create a full Role-Based Access Control (RBAC) mechanism for Parsec. The use case is quite straightforward: a small number of housekeeping operations can only be called by an admin user. All other operations can be called by any user. So there are only two levels (admin and regular), and it is permissible to hard-code the operations that fall into each level, rather than allow that to be configurable.

It is expected that admin users would be identified within service config, relative to whatever application identity mechanism is in effect (for example it could be a system UID or a SPIFFE identity).

For future-proofing, it might be prudent to consider a config design that scales to a more general-purpose RBAC system in the future, but this is not a strict requirement.

Admin users would be expected to call the service on the same socket/endpoint as regular users. There is no requirement at this stage to have a privileged endpoint.

Definition of Done

This issue can be considered done when we have a design proposal (perhaps as a PR against the parsec-book) and a corresponding set of broken-down GitHub issues for the required engineering tasks to provide the implementation, including a threat model update. The implementation work must include the tasks needed to fully test and prove the configuration and enforcement for an admin-level user. It does not need to include any admin-specific operations, however it might be difficult to do an e2e test without at least one such operation for demonstration purposes. This operation can either be one that has been identified as being useful later, or just something very minimal like an AdminPing (with the same semantics as Ping except that it is only permitted when called by an admin client).

[hug-dev]

Similarly to the other question I asked in #293, then specifying the admin in the configuration file with just a string might not be enough, because multiple authentication methods can have the same application name.
I propose that under our authenticator section of config.toml, we add an admin field for each authenticator type. This field would be optional so that there could be:

• no admin at all in the service
• only a few admins selected only for some authentication methods. That also leaves the possibility of allowing an admin only for stronger authentication methods (SPIFFE/UID as opposed to direct authentication).

So that one client would have admin privilege in the service if both the authentication type and the application name are correct.

[ionut-arm]

For future-proofing, it might be prudent to consider a config design that scales to a more general-purpose RBAC system in the future, but this is not a strict requirement.

I propose that under our authenticator section of config.toml, we add an admin field for each authenticator type. This field would be optional so that there could be:

Do you think it's a good idea to try and make that admin field a list of objects, instead of a list of strings (i.e. identifiers)? As in, something like

admin = [ { name = "admin_1" }, { name = "admin_2" } ]

[hug-dev]

I would agree with allowing multiple admins. What is the advantage of using:

admins = [ { name = "admin_1" }, { name = "admin_2" } ]

instead of

admins = [ "admin_1", "admin_2" ]

[ionut-arm]

What I quoted from Paul's proposal - it allows us to add functionality in the future to those entries, without breaking compatibility with older versions. Say you want to set what calls an admin can make:

admin = [ { name = "admin_1" }, { name = "admin_2", allowed_calls = ["ListClients"] } ]

[hug-dev]

Ah I see, yes I think that would be a clever choice!

[ionut-arm]

Alternatively, maybe we should do what we do for KIM and create a separate list of admins and their "properties", and then for each authenticator we only add the name as you suggested above

[hug-dev]

Alternatively, maybe we should do what we do for KIM and create a separate list of admins and their "properties", and then for each authenticator we only add the name as you suggested above

How would that look like? Would that then allow mutliple authenticators to use the same admin names?

[ionut-arm]

[[admin]]
name = "admin_1"
role = "DeleteAllTheStuff"

[[admin]]
name = "admin_2"
role = "JustCheckOnThings"

[[authenticator]]
#...
admins = [ "admin_1" ]

[[authenticator]]
#...
admins = ["admin_1", "admin_2"]

For KIM we had to invent a name to reference from other parts of the config, but for admins it's a natural property

[hug-dev]

I see. That could be an option but I would prefer what we said before first for simplicity in the service builder and then for the fact that the name field could be authenticator-dependant depending on how the authentication field is parsed. For example for SPIFFE, it's a string but for UID it could be a number. Only direct authentication really (with the authenticator that we have) could share the same application name as the other ones.

[hug-dev]

Design proposal

Here are my thoughts on how the implementation could look like. Please give your opinion 😃

In the config file

We add under the [authenticator] section the admins field:

admins = [ { name = "admin_1" }, { name = "admin_2" } ]

The type of name is authenticator-dependant but would map to application names that are considered as admin for that authenticator: a SPIFFE ID, a UID, a string (for direct auth).

In the service builder

When building the authenticator structure (implementing Authenticate), we store the information of the admins application name in the authenticator structure.

In the front-end handler

The front-end handler calls the authenticate method on the authenticator to then dispatch the request with the good application name (or none). Things here stay the same but the ApplicationName structure would be enhanced with a boolean field indicating if it is an admin application name or not (is_admin). It's the authenticator during the authenticate call that would set that boolean or not if the application name is in the list of the admin ones.

In the back-end handler

Before calling the appropriate Provide method, for those that require admin privilege, is_admin is called to check if the application has privilege or not. If not a AuthenticationError response code is returned (or should we create a new one?).

Testing

We could use the multitenancy test and use one of the two clients as an admin. The admin one must execute ListClients successfully while the other one should fail with the appropriate error. This for all authenticators supported. The infrasctruture is almost already there for that, expect that ListClients will have to be implemented first.

Documentation

• In the config.toml file we would explain the new field
• In the operations page we would add a paragraph about admins operations and in each of them, add the information that it is an admin operation.
• If we add a new status code, add it here as well
• Updates to the threat model (see next section)

Threat model updates

• A9 for Spoofing and Tampering need to be updated with the new idea that someone malicious could set itself as the admin. More assets are then impacted because all clients keys can be destroyed. O-6 cover the right mitigation.
• A1 Spoofing threats are aggravated if the authentication stolen is the admin one: in that case not only admin keys are at risks but everyone's. I propose that we add a notice in the AS1 section noting down that the admin's authentication token is particularly sensitive as it impacts all.
• TOCTOU attacks: the admins list can not change between the time the is_admin call is made and the time keys are destroyed because a change of configuration necessitates a reload of the service which rebuilds all components (and wait for all curent operations to finish).

Split in issues

• Issue in Parsec to add the admin logic
• Issue in Parsec to add the test once ListClients is done
• Issue in the book to add the documentation + threat model changes

[ionut-arm]

The type of name is authenticator-dependant but would map to application names that are considered as admin for that authenticator: a SPIFFE ID, a UID, a string (for direct auth).

I assume you mean the "meaning" of the name is authenticator-dependent, not the type of the field.

When building the authenticator structure (implementing Authenticate), we store the information of the admins application name in the authenticator structure.

Is that just the name or the whole structure that contains name = ...?

I wish we could somehow stop non-admin app ids from being used for admin calls without having to do an explicit if 🤔

[hug-dev]

I assume you mean the "meaning" of the name is authenticator-dependent, not the type of the field.

Well I also meant the type, for example the SPIFFE ID is a string while the UID could be a u32? Could also be a String I was thinking it might be more efficient to compare integers than strings together but maybe it's easier to maintain if they are all the same type.

Is that just the name or the whole structure that contains name = ...?

I guess only the names for now, the other things might not belong in the authenticator maybe 🤔

[ionut-arm]

If not a AuthenticationError response code is returned (or should we create a new one?).

I think a new error would be better, though I don't think it makes that much of a difference.

A1 Spoofing threats are aggravated if the authentication stolen is the admin one: in that case not only admin keys are at risks but everyone's. I propose that we add a notice in the AS1 section noting down that the admin's authentication token is particularly sensitive as it impacts all.

At least it only allows deletion of keys, not exposure/usage!

for example the SPIFFE ID is a string while the UID could be a u32?

Yeah, that makes sense!

[hug-dev]

Issues created:

• Implement admin logic #308
• Add a test for admin operations #309
• Add a new ResponseStatus for admin operations parsec-interface-rs#93
• Update with the admin feature parsec-book#78