Re: [GNAP] draft-ietf-gnap-core-protocol-00 feedback

Hi

Some comments marked with FI.

Fabien

Le mar. 20 oct. 2020 à 05:27, Dick Hardt <dick.hardt@gmail.com> a écrit :

> *Previous Feedback*
> The following items were discussed in the design team, but did not make it
> into the draft for WG discussion. A concern I have with many of the editor
> notes (which I point out) is that they are heavily biased by Justin's
> vision and misrepresent the options.
>

FI : I'm sure we can get past hard feelings and focus on the core of the
concerns. The editor's notes do their best to point where discussion should
happen. If some of them need further clarification, we can certainly do
that.

>
> *Abstract:* "information passed directly to the software" can mean pretty
> much anything. The charter has "user identifiers, and identity assertions."
>
> *1. *Same point as in Abstract around "information".
>
> *1.1* Roles:
> Add in roles for operators of AS and RC.
> Show trust framework between legal entities (user, RO, client operator, AS
> operator)
> Differentiate between legal entities having a relationship or HCI and the
> actual protocol roles (RC, AS, RS)
> What is the difference between roles and parties? Can we avoid the term
> parties? RQ is both a party and a role.
> Alternative names:
> client for RC
> user for RQ
>

FI : here we should have a more general discussion on the terminology work
that was started within the group. The point was that we could then replace
in the document.

Personal preference :
- we had already a quite extensive discussion on why "client" can be
problematic. I liked your "Grant Client" (GC) better.
- end-user is clearer for me

>
> *1.2* Elements:
> defined access token without using "credential"
> Grant as both a noun (what is being requested) and a verb
> Grant is already is a noun being used as an adjective in "Grant
> Negotiation ..." and "Grant Response"
> "Subject Information" is better than just "information" -- but is still
> extremely broad
> The subject information should be about the user, not the RO. For example,
> in an enterprise use case where resource access and identity claims are
> being requested, the RO is not the user, but could be an administrator. The
> "subject information" is going to be about the user of the client, not the
> administrator.
>

FI : Justin has asked for feedback on this, that we didn't have much time
to give. I agree with your comments.

>
> *1.3.3* step 7 - what is a refreshed credential? We should refer to the
> defined terms.
>

> *2.* title should be Grant Request to match with Grant Response in (3),
> and use of "Grant Request" in 2.7, 2.9, 5, 5.4, & 5.5
>
> *2.1.2:* "a RC MAY send a string known to the AS or RS"
> why "or"? Explain.
>
> *2.2* "sub_ids"
> are sub_ids a good idea, a best practice? despite warnings in the
> protocol, developers will use an email identifier as a communication
> identifier, or worse, a verified identifier.
> GNAP is defining a query language here for subject identifiers.
>

FI : it makes sense to ask this question. There's pro and cons. Pro = it's
useful for the client to be able to reuse some knowledge from an AS it
trusts / Con = despite all we say, people will indeed misuse this
possibility.

>
> *2.3 *Identifying a client via URL
> Mobile Apps and SPA can identify themselves with an HTTPS url that only
> that client can receive redirects to (universal link in iOS, android app
> link). The client passes the general identifier for the client (class_id?)
> and the AS ensures the redirect URI belongs to that client. A parameter is
> passed on the redirect to the client that the client presents to the AS to
> prove it is an instance of that client. The AS may then release access and
> claims to the client. The AS may also provide an instance identifier for
> the client so that instance of the client can uniquely identify itself.
>
> The editor note that instance_id is analogous to the client_id prevents an
> instance of the client in the preceding paragraph from uniquely identifying
> itself, and the name is misleading.
>
> Additionally, pre-registered identifiers (class_id which should be
> equivalent to client_id for confidential clients) and dynamic identifiers
> (instance_id) have different operational and infrastructure requirements.
> pre-registered identifiers should be read only for the AS vs read / write
> for the AS. There may be millions of dynamic identifier for one
> pre-registered identifier (mobile app, SPA)
>
> *2.3.1 *Do we need to support symmetric keys? Most OAuth clients support
> a secret, not a key.
>

FI : it's difficult to assume people with use public key cryptography for
everything. Seems to me the text is clear enough on what this implies.

>
> *2.3.2*
> what are the supported public key formats?
> What if the client is able to use different proofing mechanisms? How does
> it negotiate with the AS?
>
>
> *2.4.1* identifying the user
> this identifier would be useful if it had properties that other opaque
> identifiers did not have: being globally unique. A reason developers have
> used the email in ID Tokens was that it was a globally unique string in
> contrast to the tuple of "iss" and "sub" in the ID Token which was much
> more complicated to add and work with in a DB. Otherwise, we are creating
> yet another user identifier.
>

FI : OK but how would you make this a global identifier? Seems close to
what DIF Keri is looking for (although I find the way they're doing it is
overly/unnecessarily complex). Would that replace sub_ids?

>
> *2.5* interaction capabilities / modes (section 3.3 as well)
> While the document has renamed "interaction capabilities" to "interaction
> modes", but the client is still declaring capabilities, but in 3.3 there
> are conflicting statements
>
> "If the RC has indicated a capability to interact with the RO in its
> request (Section 2.5)"
>
> The AS MUST NOT respond with any interaction mode that the RC did not
> indicate in its request.
>
> using the same 4 interaction modes in the client request would make it
> easier for the client to say which modes it supports, and the AS to respond
> with which modes are allowed. It also allows all parameters required for a
> mode to be in the mode. For example the URL for a double redirection, vs a
> redirection to an information page after a user_code or decoupled redirect.
>
> *2.5* declaring RC capabilities
> Editor's note on what this is supposed to do, and why it can't be done by
> just adding the properties to the request.
>
> *2.7 *perhaps this could be done by updating  an existing Grant Request.
> This seems overly complicated.
>
> *2.8* Perhaps this should be in 2.2 where we are requesting information
> about the user?
> Given that solving the use cases solved by OIDC is in conflict with the
> editor note that OIDF should be doing the integration with GNAP. The
> current model is confusing as requests are in one object, and responses are
> in a different object. We don't have to combine the userinfo endpoint
> request with the id_token request just because that was convenient for
> OIDC. The editorial comments make thi seem much messier than it is.
>

FI : the goal hasn't changed here. We still integrate with other projects
such as OIDC.

>
> *3.* The URI can be stable, and the access token is potentially
> superfluous. As the client is authenticating with the same key in all
> subsequent requests, rotation of the URI or access token may be
> superfluous. Having an access token for the AS seems that is used while
> authenticating vs the typical access token for an RS seems very confusing
> to a developer.
>

FI : why would that be confusing?

>
> Additionally, putting the access token for calls to the AS in the HTTP
> Authorization header precludes using the HTTP Authorization header for
> client authentication in 8.
>

FI : that's one of the choices we need to make. But that doesn't mean what
is proposed is worse, it's just a different design with different
tradeoffs. This requires a complete discussion on section 8 (which already
occurred in the small group, but Justin will be able to comment).

>
> *4.4.2 *This functionality looks like a WebHook, and perhaps belongs in
> the API between the client and the AS rather than an interaction that
> involves the user. Also, this functionality provides no protection to
> session fixation. The interaction reference has no value.
>
> *4.4.3 *While it is good to see an editor's note that a unique
> callback URL could be used, the statement "but it would not prevent an
> attacker from injecting an unrelated interaction reference into this
> channel." is misleading. This would only happen if the client did not
> ensure it is the same user, as that would be linked to the correct URL.
> Similarly, the interaction reference does not provide protection if the
> client does not ensure it is the same user.
> Using a unique callback URL would be much simpler, and appears to provide
> the same protection as the interaction reference and the hashing.
>

FI : having to rely on the client only to ensure it is the same user is
precisely what you want to avoid. Hence the hashing.

>
> *5. *
> Continuing a Grant Request (which in (2) is called an Access Request)
> Is "continuing the right word"?
> We are either "validating" the request (5.1), polling the request (5.2),
> modifying the request (5.3), reading the request (5.4), or cancelling the
> request (5.5).
>

FI : continuation is broad enough to contain these different cases, I don't
see that as a problem.

>
> Include editorial discussion of replacing a request, replacing part of a
> request, and adding to a request, and if we should have semantics for all 3
> mechanisms.
>

> *8*
> While some binding mechanisms work the same for the client calling the AS
> as the RS, the requirements are different. When the client calls the AS,
> the client is authenticating. When the client is calling the RS, the client
> is proving it is authorized to access the resource. The RS does not need to
> know the identity of the client -- just that it is the client that the AS
> gave the access token to.
>

> *8.2 Attached JWS*
> Why not a JWT?
> This is the ONLY self contained mechanism. All the other ones require all,
> or most of the HTTP headers and the body to be kept together.
>
> The editor note
>
> "Alternatively, we could add all these fields to the body of the request,
> but then it gets awkward for non-body requests like GET/DELETE."
>
> It is not awkward. A mechanism is described later of having the JWS as an
> HTTP header. Something JWS was designed for.
>
> The editor note
>
> "A downside to this method is that it requires the content type to be
> something other than application/json, and it doesn't work against an RS
> without additional profiling since it takes over the request body - plus we
> have to specify different delivery locations for a GET vs. a POST, for
> example."
>
> Not being application/json is a feature as the AS can detect the signature
> mechanism. Per above, the requirements for the AS and RS are different. The
> client can use the same mechanism of signing with an empty body and placing
> the JWS in an HTTP header just as it does below for requests with no body.
> The RS is proving it was given the access token by the AS, and binding that
> to time, URI, and method.
>
>
> The editor note
>
> "Additionally it is potentially fragile like a detached JWS since a
> multi-tier system could parse the payload and pass the parsed payload
> downstream with potential transformations. We might want to remove this in
> favor of general-purpose HTTP signing, or at least provide guidance on its
> use."
>
> is very misleading. The whole point is to pass the JWS between
> components so that each can independently verify the request. ALL the other
> mechanisms have the fragility described as different components are going
> to pass around the JSON -- and even if they pass around the detached
> signature information and other headers, a component could transform the
> JSON in parsing and stringifying the request.
>
> The straightforward way to use JOSE from a developers point of view would
> be:
>
> - use JWT
> - include the key in the header
> - have "iat", the method, and the URI in the payload
> - put the JWT in the HTTP Authorization header for methods without a body
>
> This is how I wrote it in my implementation. It was very straightforward.
>
> Only the last point conflicts with GNAP as now written -- all the other
> aspects could be how it is done for JWT.
>
>
> *Additional feedback*
> the document name is redundant - protocol is in it twice
> (gnap = Grant Negotiation and Authorization Protocol)-core-protocol
> "draft-ietf-gnap-core" would be sufficient
>
> 1.2 Elements
> Key - "A cryptographic element binding a request to the holder of the key."
> It is actually *any* holder of the key.
>

FI : yes. Which is always the case in cryptography, unless you have some
hardware component to protect this. This can later be enclosed in a threat
model section.

>
> 2.2 Editor's note "you'd need a full identity protocol and not just this"
> implies that GNAP is not an identity protocol despite it supporting the
> OpenId Connect use cases.
>
> 3.5 Do we need this section? The only thing with "handle" in the name is
> user_handle. The access token is not really a handle.
>
> ᐧ
> --
> TXAuth mailing list
> TXAuth@ietf.org
> https://www.ietf.org/mailman/listinfo/txauth
>