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

[Fabien Imbault <[e-mail]>]

Hi Dick,

Regarding the identifier : I agree in principle, but "globally unique" is
anything but trivial.

Fabien

Le mer. 21 oct. 2020 à 19:36, Dick Hardt <[e-mail]> a écrit :

> Hi Fabien
>
> There *could* be value in GNAP creating yet-another-user-identifier if the
> identifier was guaranteed to be globally unique -- a property not held by
> "sub" in an ID Token.
>
> Using the SecEvent subject identifier draft for a client to share which
> user the client *thinks* it has is useful -- that is what the draft was
> intended for. In my opinion, querying for identifiers independent of what
> is already defined in OpenID and other standards is a path for developer
> misery.
>
> I don't think the hash provides any extra value if the redirect URL
> provided by the client to the AS is unique to the transaction. It has no
> value in a push back to the client, or a decoupled interaction -- both of
> which are susceptible to session fixation / phishing attacks.
>
> ᐧ
>
> On Wed, Oct 21, 2020 at 1:16 AM Fabien Imbault <[e-mail]>
> wrote:
>
>> Thanks Dick, it clarifies a few points. In the core of your email, I
>> focus on 2 items : sub_ids and hash.
>>
>> Beyond that we will also need a further description of the
>> possibilities/tradeoffs in section 8, to clarify for everyone (not included
>> here, but will be useful).
>>
>> There are quite a few places where practical implementations could help
>> us decide, beyond theorical arguments.
>>
>> Fabien
>>
>> Le mar. 20 oct. 2020 à 19:45, Dick Hardt <[e-mail]> a écrit :
>>
>>> Responses inline with sections without comments deleted ...
>>>
>>> On Tue, Oct 20, 2020 at 2:40 AM Fabien Imbault <[e-mail]>
>>> wrote:
>>>
>>>> Hi
>>>>
>>>> Some comments marked with FI.
>>>>
>>>> Fabien
>>>>
>>>> Le mar. 20 oct. 2020 à 05:27, Dick Hardt <[e-mail]> 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.
>>>>
>>>
>>> While I am disappointed that the design team did not decide on the best
>>> features or XYZ and XAuth, and instead started with the XYZ doc, I was not
>>> trying to express any hard feelings!
>>>
>>> Reading the draft, Justin clearly has a bias to using HTTP Signing
>>> rather than a self contained JOSE token.
>>> His comments (as noted below) on 8.2 misrepresent, and do not call out
>>> the advantage over all the other mechanisms of being self contained.
>>>
>>> Except for at the bottom, these are points for discussion that I brought
>>> up in the design team that did not make it into the draft for discussion.
>>>
>>>
>>>>> *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.
>>>>
>>>
>>> We are requiring HTTPS -- so the client has support for asymmetric
>>> crypto. My point is that the editor notes should bring up this question,
>>> which I asked in the design team.
>>>
>>
>>>
>>>>
>>>>> *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).
>>>>
>>>
>>> Lots of options. No requirement to specify how the identifier is ensured
>>> to be unique as it should be opaque to the client. It could be a URI that
>>> starts with the AS URI, or a UUID.
>>>
>>> My point is that a note on this option should be in the draft.
>>>
>>
>> FI : so the minimal requirement on which we agree is to have an opaque
>> identifier. So typically this would be unique for the AS and transmitted
>> through a user_handle (already in the spec). The client could make its own
>> local mapping if it already knows something about the user.
>>
>> To know more about the user, it is possible to :
>> - use a verifiable identity layer such as OIDC or equivalent
>> - (TBD) use sub_ids, based on the information already locally available
>> at the AS (with the limitations mentioned previously) ; or maybe we could
>> use an alternative/complementary design. For instance a more explicit json
>> on what is transmitted, or carry a correlation ID given by the client, or
>> etc.
>>
>>
>>
>>>
>>>
>>>> Would that replace sub_ids?
>>>>
>>>
>>> yes
>>>
>>>
>>>>
>>>>> *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?
>>>>
>>>
>>> Because developers don't use access tokens when accessing an OAuth AS or
>>> an OIDC OP?
>>>
>>
>>>
>>>>
>>>>> 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).
>>>>
>>>
>>> Agreed. I had asked for this to be called out as a discussion point in
>>> the editor notes. It was not.
>>>
>>>
>>>>
>>>>> *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.
>>>>
>>>
>>> The hashing does not protect against session fixation if the client does
>>> not ensure it has the same user before and after the redirect.
>>>
>>> For example, if the user has a decoupled interaction such as scanning a
>>> QR code on their PC with their phone, the client cannot ensure it is the
>>> same user coming back. An attacker can trick a user into clicking on the
>>> decoupled URL that the attacker obtained from the AS. The hashing does not
>>> protect against this.
>>>
>>
>> FI : good point. We certainly need to handle that case correctly. We
>> should note that to make sure we don't forget.
>> That said, this comes as an additional requirement for some decoupled
>> interaction modes, not as a replacement of the entire scheme.
>>
>>>
>>> /Dick
>>> ᐧ
>>>
>>