[pve-devel] [RFC 1/23] API schema: add 'notoken' property

[Fabian Grünbichler]

On October 22, 2019 1:41 pm, Tim Marx wrote:
> Joining a little late into this, but I would vote for an option where we inform the client that this endpoint needs some sort of re-authentication to be accessible, similar to what Thomas already proposed. I discussed this a little bit with Thomas (off-list), but for me the only remaining difference between the api token and the ticket is the fixed expiration, which brings up why we would need both internally, but we didn't find a consent on this for now and as it does not add any immediate value I just mentioned it here for documenting purpose. During our discussion we came up with the idea to inform the user e.g. via mail when a certain action like a ticket creation is performed to mitigate scenarios where a malicious app asks a user for their password (e.g. because the endpoint needs more than a token) and uses it to generate a ticket in the background with all the user privileges while the user still thinks the app has only the restricted subset from the actual api token.

what we tell the client is a bit orthogonal to how we decide that we 
want to tell the client something ;) but yes, having a schema property 
makes it easy to get unified handling without much extra overhead, 
compared to manually checking and raising a specific exception/...

tickets and tokens have a bit different properties..

tickets:
- generated at specific time
- valid for a fixed, short period starting at generation
- verifiable as www-data (RSA signature check only needs public key)
- renewable
- stored client-side only, no state on server
- stored/transferred (by default) in/via cookie, thus susceptible to CSRF
- intended usage: interactive sessions, like our GUI
- no revocation possible without invalidating all tickets for the whole cluster

API tokens:
- creation time irrevelant
- valid for long period (optional expiration timestamp!)
- verifiable as root only (access to token shadow file[1])
- not renewable using just token (as long as token API is marked notoken)
- stored client-side and server-side, with full state on server
- transferred via header, storage up to client (no CSRF)
- intended usage: non-interactive API clients, integration into 
  devops/CI/...
- revocation of individual tokens

I'd keep it simple for API tokens 'v1'. we don't need to throw out 
tickets altogether just because we have tokens now - they serve 
different purposes. for those usage scenarios that I have in mind, it's 
perfectly fine to be limited to almost all actions instead of all 
actions.

API methods that are marked notoken are like that for a reason. e.g., 
it's perfectly okay to require a regular, full user to login and 
configure a token before you paste that token into your ansible vault or 
wherever ;) that's a one-time setup step, and not something that you 
need on an on-going basis. if we ever have demand for managing users or 
tokens while authenticated with a token, we can introduce a privilege 
for that and adapt the 'notoken' check to honor it.

I am not sure what you mean by "need both internally" - we pass it along 
when proxying requests, but currently we don't use the token internally 
anywhere ;) I can see some future use-cases (but those depend on other 
functionality that is not yet there) where we could benefit from tokens 
for cross-cluster communication, but the current stuff (cluster join) is 
a one-off thing that is perfectly fine with interactive login/ticket. 

maybe you could expand a bit what you discussed? notifications on ticket 
creation would only be possible for 'new' tickets (those created with 
user+password, not with an existing ticket), and that does not really 
solve the issue at hand I guess.

1: but see comment on patch regarding how to make this usable for 
regular pveproxy as well

>> Thomas Lamprecht <t.lamprecht at proxmox.com> hat am 17. Oktober 2019 17:21 geschrieben:
>> 
>>  
>> On 10/17/19 4:59 PM, Fabian Grünbichler wrote:
>> > sure - I don't care about the name at all, and a positively-named option 
>> > with a default of 1 is also okay :)
>> > 
>> > I am not sure whether we even really need such a schema option - we 
>> > could also just have a helper in PVE::RPCEnvironment or 
>> > PVE::AccessControl and call that directly at the start of the API 
>> > methods. in the end, it probably comes down to how many such API methods 
>> > there are.
>> 
>> I'd rather have it encoded in the schema, so API docs are annotated,
>> also the schematic way to define such things is nicer, if possible.
>> 
>> Only issue would be if we have to guard API endpoints depending on
>> the parameters or their value, but we could naturally still have both
>> like now with the permission checks.
>> 
>> 
>> _______________________________________________
>> pve-devel mailing list
>> pve-devel at pve.proxmox.com
>> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>