Re: [Ace] AD review of draft-ietf-ace-oauth-authz-24

[Jim Schaad <[e-mail]>]

I have trimmed down to the issues that I wanted to respond to.

Jim


-----Original Message-----
From: Ludwig Seitz <[e-mail]> 
Sent: Tuesday, October 15, 2019 7:08 AM
To: Benjamin Kaduk <[e-mail]>; [e-mail]
Cc: [e-mail]
Subject: Re: AD review of draft-ietf-ace-oauth-authz-24

Hello Ben,

thank you for your thorough review.

I have taken the liberty to add numbers to your comments in order to 
refer to them in a easier way.

I have fixed 93 your 113 and there are 20 left where I am asking for 
clarifications. These are:

  6.), 12.), 16.), 19.), 34.), 39.), 41.), 45.), 52.), 57.), 58.), 65.), 
66.), 68.), 76.), 78.), 79.), 88.), 99.), 111.)

Note that 39.) Requires input from OAuth experts. Hannes?

If you whish to inspect the changes made on other review issues, please 
consult the commits here:
https://github.com/ace-wg/ace-oauth/commits/master


Detailed comments below.

/Ludwig

> 
> Hi all,
> 
> The length of this review notwithstanding, this document is generally in
> good shape -- there's a bunch of localized items to tighten up, and we
> can flesh out the security considerations some more, but nothing too
> drastic should be needed.
> 


> 12.)
>       A refresh token in OAuth 2.0 is a string representing the
>       authorization granted to the client by the resource owner.  The
>       string is usually opaque to the client.  The token denotes an
> 
> Apparently I'd forgotten that this couldn't be binary.

[LS] From RFC 6749, section 1.5: "A refresh token is a string ..."
No qualifiers such as "binary" so I interpret this to mean a text-string.
Do you want us to add some clarification?

[JLS] I think we may want to change this at some point in the future, but I don't know how much people are going to use refresh tokens so I don't think I would worry about it.

> 32.)
> Section 5.6.1
> 
>    The client sends a POST request to the token endpoint at the AS.  The
>    profile MUST specify how the communication is protected.  The content
> 
> In the previous section we said that maybe even other transports than
> coaps or https would be possible; are we limited to those that have POST
> verbs?
> Also, a similar comment as above about what attributes the protection
> entails seems to apply.

[LS] This will need a major rephrasing of the text.
I see two options here:

1.) We rewrite all parts to use a neutral language in general and specify
POST/GET etc. for transports that have these verbs.

2.) We state in the beginning that transports that do not use RESTful verbs
should use the best equivalent.

Option 1. would get a bit cluncky, while option 2. might be a bit confusing
Do you have a specific preference?

[JLS] I would suggest that this could fall under the punt to the new transport that does not have the same as these verbs in it to explain how they would map.  

> 39.)
>    Refresh tokens are typically not stored as securely as proof-of-
>    possession keys in requesting clients.  Proof-of-possession based
>    refresh token requests MUST NOT request different proof-of-possession
>    keys or different audiences in token requests.  Refresh token
>    requests can only use to request access tokens bound to the same
>    proof-of-possession key and the same audience as access tokens issued
>    in the initial token request.
> 
> This is perhaps something of a philosophical question, but if a refresh
> token is only usable at the token endpoint, in some sense its audience
> is definitionally the AS.  So there's a little bit of a mismatch in
> treating it as having the audience value that the access tokens issued
> from it will have.  I don't know the background for audience-restriced
> refresh tokens in regular OAuth 2.0, though, so hopefully someone can
> educate me.

[LS] I'm equally confused. I suggest that Hannes or one of the other OAuth
experts give us a hint on that one.

[JLS]  My understanding is that the refresh token is completely opaque to anybody but the AS so how it is stored is not as critical.  The client then does a normal request with that as a parameter.  The refresh token allows for the AS to avoid doing some of the authorization steps and potentially re-use the same key for the new token.  As such, the newly requested token needs to headed to the same audience as the original POP token.  This is the reason for needing to keep audience with it on the client.  

> 41.)
>    token_type:
>       This parameter is OPTIONAL, as opposed to 'required' in [RFC6749].
>       By default implementations of this framework SHOULD assume that
>       the token_type is "pop".  If a specific use case requires another
>       token_type (e.g., "Bearer") to be used then this parameter is
>       REQUIRED.
> 
> When we are "weakening" the formal requirements of the parent spec like
> this, we should be careful about what happens when someone is trying to
> use the ACE stuff but ends up needing to go over HTTPS like traditional
> OAuth -- do they have to fall back to the OAuth required behavior in
> that case or are we trying to preempt that as well?

[LS] The use case here is a constrained connection between client and AS
(e.g. due to a constrained client), where we try to save the bytes for
sending a token_type by defining a default. I would guess that a HTTPS based
connection would not have such constraints and would therefore be able 
to send
the token_type = "pop" as required by RFC6749. Should we explain this 
somewhere
and describe the cases where RFC6749 behaviour is warranted?

[JLS] Given that it would be identified as being application/ace+json, I don't think there is a problem with having the default value.  The fact that this is ACE is identifiable from the media-type of the message sent over HTTPS.

> 52.)
> Section 5.7.4
> 
> "scope" is showing up with the key 9 a lot; I have mixed feelings about
> having the CBOR map key value be aliased across different contexts (but
> it's probably too late to change without disruption anyway).

[LS] Scope is used in 4 places:

I. AS Request Creation Hints
II. /token request and response parameter
III. CWT claim
IV. /introspection response parameter

I claim that in all 4 cases we expect the same content with the same
formatting and semantics:  I. tells the client what to put into II.
which in turn informs the AS about the requested claim value in III.
and IV. is supposed to reflect that claim value. I therefore don't feel
hesitant to use the same abbreviation for all four cases.
Is that acceptable or do you whish us to perform any action on this?

[JLS]  I could have wished you said something sooner.  I kept trying to argue that there was no need to be consistent on the mapping, but that is how it is.  I will note that sections 8.6, 8.9, 8.11 as well as 8.13 are all separate registries for doing the mapping.  This means that while today there is consistency across all of the registries, there is no guarantee that this will be true going forward.


-- 
Ludwig Seitz, PhD
Security Lab, RISE
Phone [phone]