Subject: [Ace] Shepard review for draft-ietf-ace-oauth-authz

From: Jim Schaad <[email-masked]>
Date: Tue, 29 Jan 2019 22:01:52 -0800

1. Update the reference from RFC 5246 to RFC 8446 in all locations

Items that don't appear to be resolved:

* Section 3.1 - Refresh Token - I don't think that refresh tokens are going
to be strings because binary is more efficient.  
	 Unless you are going to say that this is not OAuth 2.0, then a
refresh token is still a text string.

*  I don't see any text that is addressing this.

> On 22/10/2018 21:09, Jim Schaad wrote:
> > * Section 5.8.2 - If the RS is going to do introspection, can it send
some
> > type of "Server Busy - try again in xxx" while it does the introspection
> > rather than just doing an ack of the request and possibly waiting a long
> > time?
> 
> This is probably transport protocol specific, and we were asked not to 
> link the framework to a specific protocol, thus I don't know if we can 
> provide guidance here.

I think it would be okay to say something like "some transport protocols
may provide a way to indicate that the server is busy and the client should
retry after an interval; this type of status update would be appropriate
while the server is waiting for an introspection response".  Which does
provide guidance, but in a non-normative fashion that does not require or
prohibit any given transport protocol.

*  I don't see anything that I think addresses this issue.  I would expect
it to be a security consideration

Comments on protection of CWT/Token:  I am wondering if there needs to be
any comments on how a CWT is going to be protected.  While it is ok to use
either a symmetric key or a direct key agreement operation for a single
recipient without forcing a signature operation to occur.  If the token is
going to be targeted a single audience hosted on multiple RSs then a
signature operation would be required for the purposes of authentication.  

****** IANA Section Issues

1.  None of the new registries appear to have any guidance for the DEs to
use when approving items.

2.  The title of the registry "ACE Authorization Server Information" is not
really descriptive of what is in the registry.   It makes sense in the text
but not as a stand along title.  Something along the lines of "ACE
Authorization Server Request Creation Hints" seems to be closer to a solid
identification.

3.  The mapping registries should use the OAuth registry name as a prefix.
Thus "OAuth Access Token Types" and "OAuth Access Token Type CBOR Mappings".

4.  What is the initial registrations that need to occur for the "ACE
Profile" registry.  If there are none then this needs to be stated.

5. For the CBOR Web Token Claims - how many of these should have the JWT
Claim name filled in?  It would seem that all of them should.  If not a
comment about this is needed.

Jim

---

From: Ludwig Seitz <[email-masked]>
Date: Wed, 30 Jan 2019 09:37:45 +0100

Thank you Jim,

I'll upload a new version as soon as we have resolved my questions below.

/Ludwig

> *  I don't see any text that is addressing this.

That text just describes how it is in OAuth 2.0 (where refresh tokens 
are strings), since we didn't see the need to specify the use of refresh 
tokens in ACE, we didn't mention them further. If we had we would 
certainly have defined them to be binary.

> it to be a security consideration

There is text in the draft according to the suggestion above in section 
5.8.1 "The Authorization Information Endpoint". Should that text be 
moved to security considerations?

> signature operation would be required for the purposes of authentication.

Right. I'll add some security considerations on that.

> use when approving items.

Is it acceptable to add a single guidance section for all of the new 
registries, or does it need to be separate for each of them?

> Authorization Server Request Creation Hints" seems to be closer to a solid
Would "ACE Authorization Server Discovery Hints" be better?

> Thus "OAuth Access Token Types" and "OAuth Access Token Type CBOR Mappings".
Done. Actually quite some changes were required to align all of the 
mappings sections with the OAuth registry names.

> Profile" registry.  If there are none then this needs to be stated.
It's initially empty, since draft-ieft-ace-oauth-authz doesn't define a 
profile. However the DTLS and OSCORE profile will provide the two 
initial entries. I added a sentence to that effect in the IANA section.

> comment about this is needed.
Fixed.

/Ludwig

-- 
Ludwig Seitz, PhD
Security Lab, RISE
Phone +46(0)70-349 92 51

---

From: Benjamin Kaduk <[email-masked]>
Date: Wed, 30 Jan 2019 10:21:56 -0600

> registries, or does it need to be separate for each of them?

That's fine.

-Ben

---

From: Jim Schaad <[email-masked]>
Date: Wed, 30 Jan 2019 10:23:26 -0800

> -----Original Message-----
> From: Ludwig Seitz <[email-masked]>
> Sent: Wednesday, January 30, 2019 12:38 AM
> To: Jim Schaad <[email-masked]>; draft-ietf-ace-oauth-
> [email-masked]
> Cc: [email-masked]
> Subject: Re: Shepard review for draft-ietf-ace-oauth-authz
> 
> Thank you Jim,
> 
> > * Section 3.1 - Refresh Token - I don't think that refresh tokens are
> > going to be strings because binary is more efficient.
> That text just describes how it is in OAuth 2.0 (where refresh tokens are
> strings), since we didn't see the need to specify the use of refresh tokens in
> ACE, we didn't mention them further. If we had we would certainly have
> defined them to be binary.

That would be fine, but you actually do define a CBOR mapping tag for refresh tokens in the body of the text and define it as binary.

> moved to security considerations?

No, I just missed the text.  It just took me three times reading through to find it.

> registries, or does it need to be separate for each of them?

As long as the guidance is comment this is fine.  That is what I did for all of the COSE registries

> Would "ACE Authorization Server Discovery Hints" be better?

I thought about that, but it does not really cover the idea of having the nonce value there or the possibility of later adding things like - ok you should use this audience or this scope or some other similar thing.

> initial entries. I added a sentence to that effect in the IANA section.

Yea, I thought that was the case since the registrations are actually done in the profile documents.  I just wanted to make sure that IANA realized that this was initially going to be an empty registry.

Jim

---

From: Ludwig Seitz <[email-masked]>
Date: Thu, 31 Jan 2019 10:20:23 +0100

On 30/01/2019 19:23, Jim Schaad wrote:
> 
> 
>> -----Original Message----- From: Ludwig Seitz <[email-masked]> 
>> Sent: Wednesday, January 30, 2019 12:38 AM To: Jim Schaad
>> <[email-masked]>; draft-ietf-ace-oauth- [email-masked] Cc:
>> [email-masked] Subject: Re: Shepard review for
>> draft-ietf-ace-oauth-authz
>> 
>> Thank you Jim,
>> 
>>> * Section 3.1 - Refresh Token - I don't think that refresh tokens
>>> are going to be strings because binary is more efficient. Unless
>>> you are going to say that this is not OAuth 2.0, then a refresh
>>> token is still a text string.
>>> 
>> That text just describes how it is in OAuth 2.0 (where refresh
>> tokens are strings), since we didn't see the need to specify the
>> use of refresh tokens in ACE, we didn't mention them further. If we
>> had we would certainly have defined them to be binary.
> 
> That would be fine, but you actually do define a CBOR mapping tag for
> refresh tokens in the body of the text and define it as binary.
> 
Right,
(background: I did define a mapping for all OAuth parameters and 
re-mapped all that were Strings to binary. That does not necessarily 
mean the have a use case currently in ACE.)

in order to resolve this I will add a sentence in the description of the 
refresh token, saying that we define them to be binary here.

>>> 1.  None of the new registries appear to have any guidance for
>>> the DEs to use when approving items.
>> 
>> Is it acceptable to add a single guidance section for all of the
>> new registries, or does it need to be separate for each of them?
> 

I'll copy liberally from your example then (and a bit from the CWT RFC). 
Hope you don't mind.

>>> 
>>> 2.  The title of the registry "ACE Authorization Server
>>> Information" is not really descriptive of what is in the
>>> registry.   It makes sense in the text but not as a stand along
>>> title.  Something along the lines of "ACE Authorization Server
>>> Request Creation Hints" seems to be closer to a solid 
>>> identification.
>>> 
> I thought about that, but it does not really cover the idea of having
> the nonce value there or the possibility of later adding things like
> - ok you should use this audience or this scope or some other similar
> thing.
> 

I see. I'll use the somewhat clunky but descriptive "ACE Authorization 
Server Request Creation Hints".

I've also taken the liberty of adding audience (req_aud) and scope as 
optional parameters in the AS Request Creation Hints message, in order 
to justify its name.

While resolving issues from the DTLS profile the authors have noticed
two elements that need to be added to the framework:

1.) A definition of "Authorization Information"
"The information an RS uses to determine wether a request is authorized, 
including the claims of applicable access tokens."

2.) Adding the "kid" parameter to the AS Request Creation Hints, so that 
a client can request a token with the same pop key when it has an 
existing security association, but the token has expired.
The procedure is currently defined in the DTLS profile, but it applies 
to any other profile as well and should therefore be in the framework.

/Ludwig

-- 
Ludwig Seitz, PhD
Security Lab, RISE
Phone +46(0)70-349 92 51

---

From: Ludwig Seitz <[email-masked]>
Date: Thu, 31 Jan 2019 10:40:57 +0100

Hello,

we have an unresolved review comment by Steffi that got lost in the 
holiday season:

https://mailarchive.ietf.org/arch/msg/ace/CBTkVUBzYrfC55zH3_UJDngiy9U
https://mailarchive.ietf.org/arch/msg/ace/NrQWetugoy0TWp9eg3lwtSictc8

The issue is the following (my words):

The AS provides the client with key material used by the RS. This can 
either be a common symmetric pop-key, or an asymmetric key used by the 
RS to authenticate towards the client.

Since there is (currently) no metadata associated to those keys, the 
client has no way of knowing if these keys are still valid. This may 
lead to situations where the client sends requests containing sensitive 
information to the RS using a key that is expired and possibly in the 
hands of an attacker, or accepts responses from the RS that are no 
properly protected and could possibly have been forged by an attacker.

The options to resolve this that I currently see are this:

1. If the client has no additional data it MUST assume that the key is 
valid as long as the access token together with which it received that 
key. Since the access token is opaque to the client, the client MUST now 
determine how long the token is valid:

Option 1.1 The client is provisioned in advance with a default validity 
time for tokens issued by the AS. This could be done when the client is 
registered at the AS.

Option 1.2 The AS informs the client using the "expires_in" parameter in 
the Access Information.

This means that we need to implement a check whether the client knows a 
default validity, and if that is not the case reject an access token 
that does not come together with an "expires_in" parameter.

2. We can define a new parameter that informs the client specifically 
about the validity of the keys the RS uses, if that differs from the 
validity of the token. Note that this is a realistic use case, since the 
RS might use an asymmetric key for authentication that is valid for a 
significantly longer period than some access token.

I would need some feed-back from the group to proceed here.

/Ludwig

-- 
Ludwig Seitz, PhD
Security Lab, RISE
Phone +46(0)70-349 92 51

---

From: Jim Schaad <[email-masked]>
Date: Thu, 31 Jan 2019 08:44:16 -0800

> Sent: Thursday, January 31, 2019 1:20 AM
> >>> are going to be strings because binary is more efficient. Unless you
> >>> are going to say that this is not OAuth 2.0, then a refresh token is
> >>> still a text string.
> >>>
> >> are strings), since we didn't see the need to specify the use of
> >> refresh tokens in ACE, we didn't mention them further. If we had we
> >> would certainly have defined them to be binary.
> >
> (background: I did define a mapping for all OAuth parameters and re-
> mapped all that were Strings to binary. That does not necessarily mean the
> have a use case currently in ACE.)
> 
> in order to resolve this I will add a sentence in the description of the refresh
> token, saying that we define them to be binary here.
> 
> 

I never really object to copying things that work.  Just think for a bit f anything is missing.  I will probably be doing some expanding of them in the move forward to Full Standard.

> optional parameters in the AS Request Creation Hints message, in order

I have always thought that those two items should be there anyway.  

Jim