feat(daemon): use new identities system to authorize API requests

[benhoyt]

This adds the code to actually use the new state-based identities for API authorization and authentication (part of spec OP043).

The PR updates the AccessCheckers to actually use the UserState value, and the daemon's userFromRequest to actually look up the user/identity. It also updates the notices code to use the UserState value where applicable.

If there is a named identity (a non-nil UserState), that is used, even if the "default UID-based auth" would let them in. So named identities trump the default ("uid==0 || uid==daemon_uid" means admin; any uid means read). For example, if you have a user bob: {access: untrusted, local: {user-id: 42}} and are making a request with UID 42, it'll only work for untrusted/open endpoints (/v1/health and /v1/system-info), not the usual of any read API.

[hpidcock]

Let's do this one last.

[IronCore864]

I roughly viewed the code (in those smaller PRs in Ben's forked repo, thus didn't leave any comments), and tested this feature. In summary, the feature works as expected, but I find a few places that could be improved, see details below. Here are the tests I ran:

pebble identities

• permissions: requires only user access, which is expected.
• output: text/json/yaml all worked correctly.

pebble add-identities

• permission: needs admin access, user access doesn't work, as expected.
• user already exists: got an error "identities already exist", as expected.
• invalid input YAML: got an error: "cannot unmarshal identities: yaml: unmarshal errors", as expected.
• missing "access": got an error 'invalid access "", must be "admin", "read", or "untrusted"'. As expected, but invalid access "" seems not clear, it would be nicer if it said something like 'invalid value "" for key access in the YAML file' or something to that effect; simply "invalid access" sounds like I don't have the permission to do so.
• wrong value for "access": similar error as above, but better: 'invalid "access": cannot decode request body: invalid access "test", must be "admin", "read", or "untrusted"'. The behaviour is as expected, and the error message is better.
• missing type ("local"): got error 'identity must have at least one type ("local")', as expected.
• missing user-id: got error 'local identity must specify user-id', as expected.
• existing uid: got error 'error: identities "alice" and "bob" cannot both have user ID 1000', as expected.

pebble identity

• non-existing identity: pebble identity foo got error 'cannot find identity "foo"', as expected.
• more than one parameter: /pebble identity bob foo got error "too many arguments for command", as expected.

pebble update-identities

• invalid input: got error "cannot unmarshal identities", as expected.
• repeated updates: OK.
• existing uid: got error 'identities "alice" and "bob" cannot both have user ID 1000', as expected.

pebble remove-identities

• remove using exactly the same YAML that the user was created: got error 'identity "alice" must be null when removing'. This error message is not very helpful, what does it mean by "must be null"? Delete the user from the OS? Does the user have something that must be deleted first? Also, it's slight annoying because users have to edit the YAML file they used to create the user in the first place. It would be nice if we can run add/udpdate and remove with the same input file, just like kubectl create/apply and delete.

State File

Inspected, content as expected.

Other Small Things

• It would be nice if the help command can show a sample YAML input, telling users the structure and the mandatory fields.
• pebble remove-identities requires YAML input, but given user names and UIDs are both unique, it would be nice if we can do pebble remove-identities --uid 1000 or simply pebble remove-identities bob.
• There is no pebble replace-identities CLI command, but it seems this is available by looking at the code, at least it's possible to do so via API.
• It seems the "untrusted" identity isn't useful, all operations require either read access or admin access. Also, from a user's perspective, why would I add a user as untrusted in Pebble so that they can do nothing? If the user don't add it as untrusted, they can't do anything anyway.

[benhoyt]

@IronCore864 Thanks a lot for your manual testing and feedback. Responding to your suggestions:

missing "access" ... As expected, but invalid access "" seems not clear

I've added a more specific error for the "" case, and clarified the other cases slightly -- thanks.

remove using exactly the same YAML that the user was created: got error 'identity "alice" must be null when removing'. This error message is not very helpful, what does it mean by "must be null"? Delete the user from the OS? Does the user have something that must be deleted first?

I've clarified the error message slightly.

Also, it's slight annoying because users have to edit the YAML file they used to create the user in the first place. It would be nice if we can run add/udpdate and remove with the same input file, just like kubectl create/apply and delete.

This is by design (per spec review discussion), so it's not easy to accidentally remove a user. I can see that it's a bit annoying, but I think annoying is better than an accidental removal. Also, it's consistent with the pebble update-identities --replace remove operation (must be null). If this design turns out to be a mistake, we can always make pebble remove-identities more permissive (but we can't go the other way).

It would be nice if the help command can show a sample YAML input, telling users the structure and the mandatory fields.

I've added some basic examples to the help text. I'm not going to specify full details of the YAML structure (that will be in our Pebble docs!) but hopefully these basic examples help. They also help clarify the remove-identities null thing.

pebble remove-identities requires YAML input, but given user names and UIDs are both unique, it would be nice if we can do pebble remove-identities --uid 1000 or simply pebble remove-identities bob.

In the spec review, we discussed a future extension (for both remove-identities and add/update-identities) that would allow you to remove identities (or add/update a single identity for add-identities/update-identities) using command line parameters. However, we felt that the YAML version was enough for a v1, so we're leaving that for future work. I think if we did this we'd allow pebble remove-identities bob alice.

There is no pebble replace-identities CLI command, but it seems this is available by looking at the code, at least it's possible to do so via API.

That's pebble update-identities --replace .... In spec review we decided to make it a single CLI command with the --replace option.

It seems the "untrusted" identity isn't useful, all operations require either read access or admin access. Also, from a user's perspective, why would I add a user as untrusted in Pebble so that they can do nothing? If the user don't add it as untrusted, they can't do anything anyway.

You're right: it's not really useful. However, we allow "untrusted" as one of the access constants for completeness. Also, I think it may be useful for other (future) authentication types, though I haven't fully thought that one through.

[hpidcock]

@benhoyt I think I want to review this one in isolation after the rest have been merged.

[nishitm]

the changes look good from the security point of view