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

[Justin Richer <[e-mail]>]

On Oct 19, 2020, at 11:26 PM, Dick Hardt <[e-mail]> wrote:
> 
> 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.
> 

I do not think this is an accurate representation of either the design team process or the resulting document. The point of the editor’s notes were to lay out options and capture discussion, and not just to present my own limited view of the world. I do not personally agree with all of the statements in every editor’s note, nor do I personally think that all the options presented are equal — but even so, acting as editor for the design team, I sought to capture those options as best as I could, and add commentary and context where possible. The design team provided feedback to the document and to the editor’s notes, and while they aren’t perfect, I do not think that they are biased as you claim.

Even so, like Fabien, I do hope we can move past this somehow lingering bitterness and the working group can build a good protocol.

Most of the feedback I’m giving here has been brought up before, either on the public list, in the design team calls, or in design team emails. 

Again, apologies for the lengthy response email. As I mentioned in the other thread, I personally think items like the list here are better captured as GitHub issues, and I’d like to see us move to that mechanism for capturing things soon, if we can.

> Abstract: "information passed directly to the software" can mean pretty much anything. The charter has "user identifiers, and identity assertions." 

The charter, as you know, is guidance for the group and not dictate. Even so, the charter itself includes this line which is what the abstract text in question was based off of:

	- Optimized inclusion of additional information (including identifiers and 
	identity assertions) through the delegation process 

The charter specifically does not limit what the protocol could be used for, only what this WG will define for its use. Therefore I was attempting to capture the broader scope of possibility in the abstract.

> 
> 1. Same point as in Abstract around "information".
> 
> 1.1 Roles: 
> Add in roles for operators of AS and RC. 

What roles do these play in the protocol itself? They may exist in deployments, but the roles should be defined in terms of the protocol.

> Show trust framework between legal entities (user, RO, client operator, AS operator)

Does this working group really want to get into the definition of legal relationships? There are such a wide variety of possible deployments and trust models, most of which don’t affect the nature of the protocol itself, that I’m not sure we can narrow that down.

> Differentiate between legal entities having a relationship or HCI and the actual protocol roles (RC, AS, RS)

The current draft does not define any legal entities, only protocol roles (including human roles like RO and RQ).

> What is the difference between roles and parties? Can we avoid the term parties? RQ is both a party and a role.

RQ is a role and is defined as such — only in the name “Requesting Party” (which as noted was taken from UMA2) does “party” occur. The name of this entity is up for discussion, still, and this is a reasonable argument for moving away from “Requesting Party”.

“Parties” are traditionally entities that fill a role. I did try to avoid the use of “party” where its use would be inaccurate or ambiguous, though I’m sure this could be cleaned up if you have specific instances of such language.

> Alternative names:
> client for RC
> user for RQ

These are already listed with the terms themselves.

> 
> 1.2 Elements:
> defined access token without using "credential"

This is largely modeled on the OAuth 2 definition of access token. Can you expand on your issue with the term “credential” here? Do you have an alternate definition to propose?

> 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"

Grant as a noun is problematic in several ways, not the least of which is that it conflicts with how “grant” is both used (“a credential given to the client to trade for a token”) and understood (“the kind of action the client has to take”) in OAuth 2. It also leads to confusing semantic satitation when used in context with the verb form. 

If we follow the plain definitions outside of OAuth2, the “grant” is the result of the request, but what we tend to talk about is the ongoing state of the request. I would love for us to have a better term for this artifact — I had previously termed this element “the transaction”. 

> "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.

I agree this is where things need to be made more explicit, especially when stretched across different use cases. The “normal” use case where the RQ and RO are the same person collapses this fine, but not in all cases. Could an RQ request “subject information” about another user (the RO)? This isn’t dissimilar to the CIBA use case of a call-center “login". Ultimately, I think we need to nail down which “subject” this returned information is about.

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

It’s specifically the URI and token values within the “continue” element that are returned from the AS, so this text can be reworded. 

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

Thanks, terminology can be made more consistent throughout the document, for sure.

> 
> 2.1.2: "a RC MAY send a string known to the AS or RS"
> why "or"? Explain.
> 

This logic is based on how OAuth 2 scopes are used in many places.

If the AS knows about the string, it can enforce policies on what an RO can approve or an RC can request. It’s really common to have these kinds of policies. If the AS doesn’t know about the string, the AS can still provide the request to the RO for approval and convey the approved string information to an RS downstream for enforcement.

If the RS knows about the string, it can enforce policies on what the token is good for, assuming the string is conveyed to the RS as-is. Alternatively, the RS could be naive about the requested string and instead rely on the AS to translate the string into something specific for the RS to consume. 

This is not an exclusive “or”, of course. If both the AS and RS know about the string, that also works fine.

> 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. 

We know from years of experience with OIDC and OAuth 2 that client developers want to have an identifier they can correlate to other information within their applications, and this is a feature to address that. 

> 
> 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.

The conflict only occurs if you want to call the class_id the client_id. In practice, to serve this kind of client, the AS would register a class_id and the developer would include that class_id with every initial request. 

> 
> 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)

Thus why a class_id and an instance_id are different, and we no longer have “client_id” that confuses the concepts. 

I think it would be worthwhile to add an explicit “class_id” field, but as I recall in the design team we didn’t quite settle on that.

What are you proposing should change?

> 
> 2.3.1 Do we need to support symmetric keys? Most OAuth clients support a secret, not a key.

I believe we should support symmetric keys (but not client secrets). I also believe that we can safely do so using the client instance reference, as laid out by the discussion in the section.

The problems with symmetric crypto largely come from key distribution and not the actual cryptography. If the keys are pre-placed or securely delivered or derived out-of-band, symmetric crypto will fit fine in this space.

> 
> 2.3.2
> what are the supported public key formats?

What’s listed in the document are currently JWK and PEM, these should be expandable to other formats (CWK for example).

> What if the client is able to use different proofing mechanisms? How does it negotiate with the AS?

Inline negotiation of proofing mechanisms is dangerous at best, since the RC needs to protect the negotiation request. 

Most clients are likely to have one method that they always do, and it will be based on the key material that they have access to, but the discovery mechanisms in the draft would allow a more capable client of dynamically selecting a mechanism. 

Yaron raised the point of what is Mandatory to Implement, and I agree that this is something that should be.

> 
> 
> 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.

This identifier is very narrowly focused, and is used for the RC to communicate its notion of “the current user” back to the AS specifically. With that focus, it doesn’t need to be globally unique or even recognizable to the RC, just a string assigned by the AS. There are other subject identifiers that can fulfill a “globally unique” role instead. Globally unique identifiers for users are discussed in section 3.4 and linked from several sections, as was suggested during design team conversations. Section 3.5 (that defines where this string might come from) discusses that the user_handle string talked about here could potentially be replaced by a more general mechanism instead of this narrowly-focused one, which could in turn have the aspects you are discussing.

> 
> 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. 

These are not in conflict — the intended interpretation is that IF the AS responds to an interaction then it does it in a particular way, not that it MUST respond to an interaction it doesn’t know how to handle. The rest of the sentence quoted above states in full (emphasis added):

	If the RC has indicated a capability to interact with the RO in its request <https://www.ietf.org/archive/id/draft-ietf-gnap-core-protocol-00.html#request-interact> (Section 2.5 <https://www.ietf.org/archive/id/draft-ietf-gnap-core-protocol-00.html#request-interact>), and the AS has determined that interaction is both supported and necessary, the AS responds to the RC with any of the following values in the interact field of the response.

In addition, all of the subsections in 3.3 have text like the following (emphasis added):

	If the RC indicates that it can redirect to an arbitrary URL <https://www.ietf.org/archive/id/draft-ietf-gnap-core-protocol-00.html#request-interact-redirect> (Section 2.5.1 <https://www.ietf.org/archive/id/draft-ietf-gnap-core-protocol-00.html#request-interact-redirect>) and the AS supports this mode for the RC's request,

So the RC declares what it can do, and the AS responds to any parts that:

	- the AS supports
	- the AS determines are appropriate for what’s being requested (resources and subject info)
	- the AS allows for that specific RC instance
	- the AS allows for any identified RO in the request

The combination of these together tell the RC exactly what it is allowed to do.

Do you have a suggestion on how to make that more clear?

> 
> 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.

As has been discussed at length, what you describe here is the same negotiation model as in the draft. The only difference is whether the post-interaction method is bundled as a property of the engagement method, which is discussed in detail in the notes along with two different proposals of how it could be done.

> 
> 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.

This is a proposal for an inline advanced discovery mechanism. I’m personally not a fan of keeping it if we don’t have a concrete use for it, but at this stage it’s important to have options like this listed.

> 
> 2.7 perhaps this could be done by updating  an existing Grant Request. This seems overly complicated.

Updating an existing request is a different use case, much like refreshing an access token is different from deriving an access token through token exchange. 

> 
> 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.
> 

I agree that it’s not ideal to combine the userinfo and id_token elements, but that’s how OIDC defined this query language, and an important aspect of this query language is that the top-level element of the query object defines the “target” of the requested claims. We could ignore this part of the spec, but if we are to use the OIDC query language, we shouldn’t be re-defining how it works, which is what splitting up the components would do. 

This stance is based on having implemented OIDC claims processing on several platforms. If we were to break up the claims query language, it would be difficult to re-use any existing parsing and processing mechanisms. It would also not work for extensions to the OIDC claims query language, such as one that’s proposed for VC’s: https://mattrglobal.github.io/oidc-client-bound-assertions-spec/ <https://mattrglobal.github.io/oidc-client-bound-assertions-spec/>

I would rather see us focus on general purpose mechanisms for requesting resources, subject identifiers, and bound assertions. 

> 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.

The key here is that this is a :bound: access token, just one that is bound to the client’s key in this case. It’s exactly the same mechanism that the RC would use to call the RS, and that separation of concerns is a key feature of this architecture. If we are expecting GNAP developers to be confused by being asked to present an access token, then I don’t think we have much hope. :)

> 
> 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.

The purpose of the key presentation is not just for client authentication, but about binding the message to a particular key holder. This is what allows the same mechanisms to be used across different elements of the protocol instead of having to invent new things for each piece.

> 
> 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.

It’s functionality that is triggered post-interaction, just like a post-interaction redirect to a callback URL (or the static page option proposed in the notes).

> 
> 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.
> 

I believe the unique URLs and reference/hash mechanism solve different but related problems. This is an area for discussion, but I don’t see a reason to :not: have cryptographic protection on front channel mechanisms. OAuth 2 started without it, but OIDC added them back in (with hybrid mode) and now JARM is defining a general-purpose way to do that.

> 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).

I believe “continue” is a wide enough net to cover all of these, as it’s action on a previously-started request. Do you have a suggestion for a better encompassing term?

> 
> 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.

This seems to be covered in the discussion on updating the request and constraints on the atomicity of each portion of the request. 

> 
> 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.

None of the mechanisms require the RS to know the identity of the RC. The AS can verify the RC instance’s identity based on its key, if the AS knows the key and has it associated to an instance. But I’d also add that the AS doesn’t need to know the identity of the RC either, in dynamic cases, since the key is just being introduced. In those cases, as you state above, the “authentication” of the client is a more complex process involving class identifiers (as well as device posture and the like). This is an important discussion for the WG to have as it gets at our model of what a “client” is on several dimensions.

Could you please expand on how you see using the same set of binding mechanisms across the protocol would cause problems for AS, RS, or RC developers? 

> 
> 8.2 Attached JWS
> Why not a JWT?

This is not a token, therefore JWT is not an appropriate technology. JWS is appropriate as it provides a signature mechanism, which is what’s being discussed here.

> 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.

That’s correct, are you suggesting a change, or that this should be called out more explicitly?

> 
> 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.

I think it’s incredibly awkward for the signature layer to mess around with the payload of the HTTP message itself. It breaks the separation of these layers, and now the signature layers need to know about the content and format of the HTTP message.

For instance, you’d have to have the client put the signature parameters (such as HTTP method and URI) either in the body of the request OR in a special body that’s used only in body-less requests, depending on what you’re doing. This is on top of putting the signed element in a different place when you’re done with it. 

> 
> 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 note is accurate, it gets in the way of the RS’s input. This signature wouldn’t work on an XML-based API, for example. And what of an empty POST request? An API would now need to look for either an empty body OR a JSON body. Or take a JSON based API, but one that’s implemented with a strictly typed parser. Not only would you need to pre-process the JOSE in order to extract the JSON (which I’ve done in my implementation of this section, and it’s not pretty), but now you’d also need to remove the additional fields that cover the HTTP elements.

The detached JWS method described in 8.1 would work in all of these cases — but with the developer doing the same thing every time.

> 
> 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.
> 

Right, what I’m saying is that your specific use case has some clear assumptions about the way integrity is processed within the system. All of those need to be enumerated very carefully.

> 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.

Additionally, the proposal laid out here has a significant anti-pattern in including both the HTTP method and the HTTP URI in the HTTP entity body itself. Doing so is not only redundant, but it causes more error conditions that the application layer would need to check against. Requiring these in all requests, and not just for the JOSE requests, would also put a burden on other signature mechanisms that don’t need it. HTTP Message Signatures, DPoP signatures, and (the historical) OAuth PoP signatures all have mechanisms for including the HTTP elements in the signature outside of the message body. Mutual TLS binding does not need to extend the HTTP message at all in order to cover all aspects under the signature. Why should all of these mechanisms put HTTP components in the body?

I understand that your own use case requires the incoming message to be fully protected as a single unit, and the functionality in 8.2 enables this, as we have discussed. Can you please explain how 8.2 does not address your use case? I understand that it’s not how you built it, but I think it’s important to understand what this different approach would make impossible or difficult with what you’re trying to solve.

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

The document name was suggested by the chairs.

> 1.2 Elements
> Key - "A cryptographic element binding a request to the holder of the key."
> It is actually any holder of the key. 

Yes, thanks.

> 
> 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. 

GNAP supporting some of OIDC’s use cases does not make it a full identity protocol, and there was significant pushback in GNAP trying to define a full identity protocol with user schemas, authenticator classes, assertion formats, authentication event information, and all of that. There was middle ground in defining a few simple elements of the OIDC use case, such as identifying the current user. 

> 
> 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. 

Yes, I believe it provides necessary functionality. This section is for providing a dynamic identifier that the RC can use for itself, and one that it can use for the RQ in future requests. This section does mention access tokens except in the example.

 — Justin