Subject: [GNAP] User code, was Re: Review of draft-ietf-gnap-core-protocol-00 (first half)

From: Yaron Sheffer <[email-masked]>
Date: Mon, 19 Oct 2020 12:52:04 +0300

Hi Tom,

Thanks for your response. I would like to focus on one point.

The document does not explain what the user code solution tries to achieve, and I honestly don’t understand the exact use case(s) myself. If you could write up a paragraph, we could include it in the document for my own and others’ edification.

Regards,

                Yaron

From: Tom Jones <[email-masked]>
Date: Monday, October 19, 2020 at 00:38
To: Yaron Sheffer <[email-masked]>
Cc: GNAP Mailing List <[email-masked]>
Subject: Re: [GNAP] Review of draft-ietf-gnap-core-protocol-00 (first half)

then i guess we must address the entire email, which is not helpful.  Here are at least a few of my thoughts.

1.2, "key" is a too generic term in the context of a security protocol. Can we call it something more specific, maybe "holder identity key"?

>> that is not more specific. if you want to understand the source of the key then perhaps the entire jwk element needs to be augmented.

1.3.2: it is unclear to me what the goal of the user-code interaction is, in other words what is the threat model. If it's just "second factor, something you have", then an OTP MFA device would be simpler and offer the same security.

>> first of all, you are wrong in your assessment, second that is not all that the user-code interaction supplies.

2.1.2: I don't see the value of the notion of "expansion". It is sufficient that the string is understood by the AS.

>> I am already running into this as a problem with strings in OIDC. I believe we need a better way to expand strings into structures.  I proposed the use of 3part jose structures as a solution.

* 2.3: what's the benefit of including a URI (URL really) in "display"? Do we really want the user (RO) to click links provided by the RC?

>> I hope you are not proposing that users actually see any of this?  That would be grotesque.

* 2.4: IMO assertion validation is a MUST, and we should specify what we require for every assertion type we allow.

>> I plan on including assertions that do not need to be validated. I believe it is not up to the sender whether the receiver validates the assertion.

In general I find that many of your comments are not trivial and need deeper analysis.
Peace ..tom

On Sun, Oct 18, 2020 at 1:00 PM Yaron Sheffer <[email-masked]> wrote:

We haven’t discussed this process yet, and the default IETF process is, everything goes through the mailing list.

IMO small things can be quickly cleaned up (fixed or rejected), and we can open GH issues for bigger things.

Thanks,

                Yaron

From: Tom Jones <[email-masked]>
Date: Sunday, October 18, 2020 at 22:47
To: Yaron Sheffer <[email-masked]>
Cc: GNAP Mailing List <[email-masked]>
Subject: Re: [GNAP] Review of draft-ietf-gnap-core-protocol-00 (first half)

why are you doing this? Should these be issues now?
Peace ..tom

On Sun, Oct 18, 2020 at 12:44 PM Yaron Sheffer <[email-masked]> wrote:

Hi Justin, all,

The draft has undergone major changes since we started the design team, so I spent some time reviewing it. I only got as far as Sec. 3, and I fully intend to review the remainder of the document. But I figured since I have so many comments, I might as well send out what I have.

* General comment: we have dozens, maybe hundreds of variants/options here. I think we need to define a must-implement subset, or we will never reach interoperability.

* Abstract: the boilerplate text (MUST etc.) usually goes to the Introduction, not the Abstract.

* 1. "The requesting party operating the software" - I think these actions are typically performed by the resource owner, who is not necessarily the requesting party.

* RS, aka API: we are using the word "API" several times in a more generic sense (management API, identity API), so maybe replace with "RS, typically a protected API".

* 1.2, "key" is a too generic term in the context of a security protocol. Can we call it something more specific, maybe "holder identity key"?

* 1.3.1. "augmented with a hash of the security information" - this is too implementation-specific, suggest "is cryptographically bound to the security information". In general, step (7) is way too detailed, and verges on the normative.

* 1.3.2: it is unclear to me what the goal of the user-code interaction is, in other words what is the threat model. If it's just "second factor, something you have", then an OTP MFA device would be simpler and offer the same security.

* 1.3.5 (4): Typically the access token would not be sent once it is expired, right? Let's make the example more realistic.

* 2: "OpenID Connect claims request" - why not "identity claims request" (which just happens to be similar to the OpenID request)?

* Never liked the "dolphin" here... If this is listed under "actions", it should be a verb, e.g. "swim".

* 2.1: an array for a single AT vs. an object for multiple ATs? This is extremely non-intuitive and probably ugly to code. Also, why should (multiple) tokens be named if a single token doesn't have to, why not list them in an array instead?

* 2.1.1, "locations": are these really URIs or just URLs? On the other hand, if these are URIs, why do we need a separate "identifier"? There's too much semantic overlap between "type", "location" and "identifier" - specifying just "type" may be sufficient.

* 2.1.1: it's implied but should be spelled out that the semantics of this example is the cross product of permissions: I am requesting to read and write and "dolphin" both "metadata" and "images", and that applies to both "locations" - 12 different possible actions.

* 2.1.2: I don't see the value of the notion of "expansion". It is sufficient that the string is understood by the AS.

* 2.1.2, "in some situations the value is intended to be seen and understood be the RC developer" - shouldn't we require this value to be always fully documented by the AS owner, so that the RC knows what it is requesting?

* 2.1.4: why are flags represented as resources? The answer, "this is how OAuth 2 does it" is not great.

* "the each access tokens" -> each access token.

* I suggest to extend the editor's note to clarify what is the use case, so that the group can decide whether split tokens are indeed necessary.

* Why is key binding of the access token even optional?

* 2.2, "Subject identifiers requested by the RC serve only to identify the RO in the context of the AS and can't be used as communication channels by the RC" - this sounds a bit naive, people will do that anyway. So why is this a useful statement? (And similarly in 3.4)

* 2.3: what's the benefit of including a URI (URL really) in "display"? Do we really want the user (RO) to click links provided by the RC?

* 2.3: why are we sending a JWK *as well as* a cert? Are we checking that the cert contains the same public key as the cert?

* 2.3, " the AS MUST ensure that the key used to sign the request... is associated with the instance identifier" - can we be much more specific here, e.g. the instance_id MUST be byte-identical to the Common Name of the cert? This "association" is never clarified, either here or in Sec. 8.

* 2.3.2: allowing to send the key in multiple formats virtually invites security vulnerabilities. Why is this a good thing?

* 2.3.2: cert#256 is not a key, it's an identifier. Why do we include it here? Note that in Sec. 8.3 you are using the full certificate rather than the fingerprint.

* 2.3.2: why are we using raw PEM certificates instead of the JOSE representation?

* 2.3.3: For obvious security reasons, I suggest adding: The AS MAY choose to avoid displaying an arbitrary URI. RC developers must only assume that the "name" field will be displayed.

* 2.3.4, "created just for this request" -> created just for this series of requests.

* 2.3.4: I suggest referring to RCs that present unknown keys as "anonymous RC", and then in 2.4, say that the AS MUST NOT accept any assertions from an anonymous RC.

* 2.4: IMO assertion validation is a MUST, and we should specify what we require for every assertion type we allow.

* 2.4: moreover, when possible, the AS MUST match the assertions to the presented sub_id. It may be a hint, but you're not allowed to lie.

* 2.4, "If the identified RQ does not match the RO present at the AS" - I thought there are cases where the RQ is different from the RO. Also, I am wondering why this is a SHOULD, not a MUST.

* 2.4.1 (but probably applicable to any references): what is the lifetime of the reference? As an RC, how long can I assume the user reference is still valid?

* 2.5: "preferred_locales" is obviously not a mode of interaction. Why is it bundled here?

* 2.5: the way "callback" is described here is as a capability, not an interaction mode. The example only strengthens this view, "callback" is a modifier of the "redirect" mode.

* 2.5: "use code" -> user code.

* 2.5.1: this unnecessary polymorphism IMO just complicates implementations and prevents extensibility. Instead, I would suggest:

redirect: {}

redirect:{max_length: 255}

redirect:{max_length: 255, callback: {...}}

* 2.5.2: the "app" option is problematic, as you already note. The RC may not even know if a given URL will result in an application launching.

* 2.5.2: and shouldn't the RC include a list of supported applications? (Which would have lovely privacy implications.)

* 2.5.3: there's a parenthetical discussion on whether the URL is unique. But the nonce parameter implies that the "hash" parameter is unique per request, making this discussion moot.

* 2.5.3: even if we need a different kind of post-interaction "redirect", where the RC is not involved, there must be a way to ensure that the RC receives positive (cryptographically protected) confirmation that the authorization was successful. In other words, what we're discussing here is not an RC interaction mode, it is something else, and should probably have a separate protocol element to describe it.

* 2.5.3: In the example for "interact" as an array (which I support), the second "redirect" and "user_code" should be independent elements of the array, not members of an object. This assumes they are truly independent interaction modes.

* 2.5.3.1: what does "RQ is present on the request" mean?

* 2.5.3.2: typo: HTTP the request.

* 2.7: this seems to assume a short access token (which implies a bearer token). What if we want to use longer self-contained access tokens containing full keys? Should we have a separate "grant_id" value instead?

* 2.8: another problem with this stanza is that the RC needs to know a priority that the AS supports it. What happens if the AS doesn't?

* 2.8: I suggest to move this section into a separate I-D, or at least a separate appendix.

* 2.9: it makes sense to bundle extensions and the "capabilities" section into one.

* 2.9: The AS MUST ignore any unknown extension.

* 3: it's confusing that we're using access_token for two different things. Maybe we could change the RC/AS one to "as_access_token"?

* 3.1: MAY vary *by* request.

* 3.1: how is the client behavior "deterministic in all cases" - how does the RC know whether the AS allows continuation or only allows one request at a time?

* 3.2.1: the "value" of the access token is opaque in some cases (bearer token) but not in others.

* 3.2.1: The ASCII restriction on "value" is insufficient. Needs to be printable ASCII, 0x20<v<=0x7f. And I'm not sure if this doesn't leave some characters that still need to be escaped.

* 3.2.1: why is "expires_in" optional? Typically access tokens are not revoked, and the RC needs a way to manage their lifetime.

* 3.2.1: "resources" refers to this same section. Did you mean 2.1.2, or are we missing a subsection describing resources/rights?

* 3.2.1: "key": another polymorphism alert!

* 3.2.1: the example at the bottom of the section talks about a way to present the token, I believe this discussion is out of context here.

* 3.3: the AS knows now which interaction modes are available. Why not pick just one for its response, to simplify the protocol? As one example, the AS can then more easily decide on timeout behavior.

* 3.3.3: Suggest to add to the last paragraph: If the RC does not receive a callback until an RC-determined timeout occurs, the RC MUST act as if the entire request failed.

* 3.4: "updated_at": Unix time is nice, but for absolute time it's common to use ISO 8601 format.

* 3.6: YES, the "error" element is exclusive!

* 3.6: in SecEvent we tried to use RFC 7807 "problem details", I think it could work nicely here.

* 10.4: "indicating that GNAP" - sentence is broken.

* This "speculative" access is not useful if the response is only a MAY. Why would the RC try it if it is unlikely to provide useful information?

* 15: the BCP195 ref is broken - the author names are missing (yes I am one of them...).

* Appendix D: can we name the first scenario, maybe "Simple API Access"?

-- 
TXAuth mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/txauth

---

From: Fabien Imbault <[email-masked]>
Date: Mon, 19 Oct 2020 16:58:22 +0200

User code is an interaction mode that you use when an interaction via a
browser is not possible. For instance, a small device that can display a
code, so that the end-user can easily enter that code through a
predetermined endpoint (typically through a mobile phone).

On Mon, Oct 19, 2020 at 11:52 AM Yaron Sheffer <[email-masked]>
wrote:

> The document does not explain what the user code solution tries to
> achieve, and I honestly don’t understand the exact use case(s) myself. If
> you could write up a paragraph, we could include it in the document for my
> own and others’ edification.
>
>
>
> Regards,
>
>                 Yaron
>
>
>
> *From: *Tom Jones <[email-masked]>
> *Date: *Monday, October 19, 2020 at 00:38
> *To: *Yaron Sheffer <[email-masked]>
> *Cc: *GNAP Mailing List <[email-masked]>
> *Subject: *Re: [GNAP] Review of draft-ietf-gnap-core-protocol-00 (first
> half)
>
>
>
> 1.2, "key" is a too generic term in the context of a security protocol.
> Can we call it something more specific, maybe "holder identity key"?
>
> >> that is not more specific. if you want to understand the source of the
> key then perhaps the entire jwk element needs to be augmented.
>
> 1.3.2: it is unclear to me what the goal of the user-code interaction is,
> in other words what is the threat model. If it's just "second factor,
> something you have", then an OTP MFA device would be simpler and offer the
> same security.
>
> 2.1.2: I don't see the value of the notion of "expansion". It is
> sufficient that the string is understood by the AS.
>
> >> I am already running into this as a problem with strings in OIDC. I
> believe we need a better way to expand strings into structures.  I proposed
> the use of 3part jose structures as a solution.
>
> * 2.3: what's the benefit of including a URI (URL really) in "display"? Do
> we really want the user (RO) to click links provided by the RC?
>
> * 2.4: IMO assertion validation is a MUST, and we should specify what we
> require for every assertion type we allow.
>
> >> I plan on including assertions that do not need to be validated. I
> believe it is not up to the sender whether the receiver validates the
> assertion.
>
>
>
> We haven’t discussed this process yet, and the default IETF process is,
> everything goes through the mailing list.
>
>
>
> *From: *Tom Jones <[email-masked]>
> *Date: *Sunday, October 18, 2020 at 22:47
> *To: *Yaron Sheffer <[email-masked]>
> *Cc: *GNAP Mailing List <[email-masked]>
> *Subject: *Re: [GNAP] Review of draft-ietf-gnap-core-protocol-00 (first
> half)
>
>
>
> The draft has undergone major changes since we started the design team, so
> I spent some time reviewing it. I only got as far as Sec. 3, and I fully
> intend to review the remainder of the document. But I figured since I have
> so many comments, I might as well send out what I have.
>
>
>
> * General comment: we have dozens, maybe hundreds of variants/options
> here. I think we need to define a must-implement subset, or we will never
> reach interoperability.
>
> * 1. "The requesting party operating the software" - I think these actions
> are typically performed by the resource owner, who is not necessarily the
> requesting party.
>
> * RS, aka API: we are using the word "API" several times in a more generic
> sense (management API, identity API), so maybe replace with "RS, typically
> a protected API".
>
> * 1.2, "key" is a too generic term in the context of a security protocol.
> Can we call it something more specific, maybe "holder identity key"?
>
> * 1.3.1. "augmented with a hash of the security information" - this is too
> implementation-specific, suggest "is cryptographically bound to the
> security information". In general, step (7) is way too detailed, and verges
> on the normative.
>
> * 1.3.2: it is unclear to me what the goal of the user-code interaction
> is, in other words what is the threat model. If it's just "second factor,
> something you have", then an OTP MFA device would be simpler and offer the
> same security.
>
> * 1.3.5 (4): Typically the access token would not be sent once it is
> expired, right? Let's make the example more realistic.
>
> * 2: "OpenID Connect claims request" - why not "identity claims request"
> (which just happens to be similar to the OpenID request)?
>
> * 2.1: an array for a single AT vs. an object for multiple ATs? This is
> extremely non-intuitive and probably ugly to code. Also, why should
> (multiple) tokens be named if a single token doesn't have to, why not list
> them in an array instead?
>
> * 2.1.1, "locations": are these really URIs or just URLs? On the other
> hand, if these are URIs, why do we need a separate "identifier"? There's
> too much semantic overlap between "type", "location" and "identifier" -
> specifying just "type" may be sufficient.
>
> * 2.1.1: it's implied but should be spelled out that the semantics of this
> example is the cross product of permissions: I am requesting to read and
> write and "dolphin" both "metadata" and "images", and that applies to both
> "locations" - 12 different possible actions.
>
> * 2.1.2: I don't see the value of the notion of "expansion". It is
> sufficient that the string is understood by the AS.
>
> * 2.1.2, "in some situations the value is intended to be seen and
> understood be the RC developer" - shouldn't we require this value to be
> always fully documented by the AS owner, so that the RC knows what it is
> requesting?
>
> * I suggest to extend the editor's note to clarify what is the use case,
> so that the group can decide whether split tokens are indeed necessary.
>
> * 2.2, "Subject identifiers requested by the RC serve only to identify the
> RO in the context of the AS and can't be used as communication channels by
> the RC" - this sounds a bit naive, people will do that anyway. So why is
> this a useful statement? (And similarly in 3.4)
>
> * 2.3: what's the benefit of including a URI (URL really) in "display"? Do
> we really want the user (RO) to click links provided by the RC?
>
> * 2.3: why are we sending a JWK *as well as* a cert? Are we checking that
> the cert contains the same public key as the cert?
>
> * 2.3, " the AS MUST ensure that the key used to sign the request... is
> associated with the instance identifier" - can we be much more specific
> here, e.g. the instance_id MUST be byte-identical to the Common Name of the
> cert? This "association" is never clarified, either here or in Sec. 8.
>
> * 2.3.2: allowing to send the key in multiple formats virtually invites
> security vulnerabilities. Why is this a good thing?
>
> * 2.3.2: cert#256 is not a key, it's an identifier. Why do we include it
> here? Note that in Sec. 8.3 you are using the full certificate rather than
> the fingerprint.
>
> * 2.3.3: For obvious security reasons, I suggest adding: The AS MAY choose
> to avoid displaying an arbitrary URI. RC developers must only assume that
> the "name" field will be displayed.
>
> * 2.3.4: I suggest referring to RCs that present unknown keys as
> "anonymous RC", and then in 2.4, say that the AS MUST NOT accept any
> assertions from an anonymous RC.
>
> * 2.4: IMO assertion validation is a MUST, and we should specify what we
> require for every assertion type we allow.
>
> * 2.4: moreover, when possible, the AS MUST match the assertions to the
> presented sub_id. It may be a hint, but you're not allowed to lie.
>
> * 2.4, "If the identified RQ does not match the RO present at the AS" - I
> thought there are cases where the RQ is different from the RO. Also, I am
> wondering why this is a SHOULD, not a MUST.
>
> * 2.4.1 (but probably applicable to any references): what is the lifetime
> of the reference? As an RC, how long can I assume the user reference is
> still valid?
>
> * 2.5: the way "callback" is described here is as a capability, not an
> interaction mode. The example only strengthens this view, "callback" is a
> modifier of the "redirect" mode.
>
> * 2.5.1: this unnecessary polymorphism IMO just complicates
> implementations and prevents extensibility. Instead, I would suggest:
>
> redirect: {}
>
> * 2.5.2: the "app" option is problematic, as you already note. The RC may
> not even know if a given URL will result in an application launching.
>
> * 2.5.2: and shouldn't the RC include a list of supported applications?
> (Which would have lovely privacy implications.)
>
> * 2.5.3: there's a parenthetical discussion on whether the URL is unique.
> But the nonce parameter implies that the "hash" parameter is unique per
> request, making this discussion moot.
>
> * 2.5.3: even if we need a different kind of post-interaction "redirect",
> where the RC is not involved, there must be a way to ensure that the RC
> receives positive (cryptographically protected) confirmation that the
> authorization was successful. In other words, what we're discussing here is
> not an RC interaction mode, it is something else, and should probably have
> a separate protocol element to describe it.
>
> * 2.5.3: In the example for "interact" as an array (which I support), the
> second "redirect" and "user_code" should be independent elements of the
> array, not members of an object. This assumes they are truly independent
> interaction modes.
>
> * 2.7: this seems to assume a short access token (which implies a bearer
> token). What if we want to use longer self-contained access tokens
> containing full keys? Should we have a separate "grant_id" value instead?
>
> * 2.8: another problem with this stanza is that the RC needs to know a
> priority that the AS supports it. What happens if the AS doesn't?
>
> * 3: it's confusing that we're using access_token for two different
> things. Maybe we could change the RC/AS one to "as_access_token"?
>
> * 3.1: how is the client behavior "deterministic in all cases" - how does
> the RC know whether the AS allows continuation or only allows one request
> at a time?
>
> * 3.2.1: The ASCII restriction on "value" is insufficient. Needs to be
> printable ASCII, 0x20<v<=0x7f. And I'm not sure if this doesn't leave some
> characters that still need to be escaped.
>
> * 3.2.1: why is "expires_in" optional? Typically access tokens are not
> revoked, and the RC needs a way to manage their lifetime.
>
> * 3.2.1: "resources" refers to this same section. Did you mean 2.1.2, or
> are we missing a subsection describing resources/rights?
>
> * 3.2.1: the example at the bottom of the section talks about a way to
> present the token, I believe this discussion is out of context here.
>
> * 3.3: the AS knows now which interaction modes are available. Why not
> pick just one for its response, to simplify the protocol? As one example,
> the AS can then more easily decide on timeout behavior.
>
> * 3.3.3: Suggest to add to the last paragraph: If the RC does not receive
> a callback until an RC-determined timeout occurs, the RC MUST act as if the
> entire request failed.
>
> * This "speculative" access is not useful if the response is only a MAY.
> Why would the RC try it if it is unlikely to provide useful information?
>

---

From: Tom Jones <[email-masked]>
Date: Mon, 19 Oct 2020 10:46:02 -0700

Ok - several others have been asking as well.  I wrote up this page
https://tcwiki.azurewebsites.net/index.php?title=Apps_on_User_Devices

basically there are assertions from the programmer of the app, and from the
app itself that are passed in the id token (whatever) from the user device.
Note that this means that not all assertions are signed by the same key in
this case!
Nor are all the attributes about the subject. Some are just in support of
the level of assurance.
The big difference with FIDO U2F and II is that the keys are bound to the
user in the device, not in the RP.
Peace ..tom