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

From: Renzo Navas <[email-masked]>
Date: Fri, 8 Jul 2016 17:04:43 +0200

Hi ace wg,
Hi Ludwig, Göran, et al.

Thank you first for this magnificent document (and appropriate
acknowledge to the previous work on ace framework -acre, dcaf, etc-).

I found the document very clear and well written (the figures and
examples are a big plus); and me coming from a oauth complete
ignorance, I can say that I found on the document the necessary
pointers to understand the oauth framework, the pop token and
architecture, jwt, etc.: excellent introductory work.
I show also my possitive feedbak on the new "client_token" concept ,
and I believe is a useful/relevant use case; and this provides an
elegant an easy solution.

I should have sent this review one (two?) week ago as promised on the
interim meeting, I'm sorry I did not.
I believe my comments are a bit messy, excuse me in advance. I hope
you can get some positive input from my comments that can help to make
the document better -and feel free to ask me again if some comment is
not clear-:

----------------------------------------------------------------
Review of https://tools.ietf.org/html/draft-ietf-ace-oauth-authz-02
---------------------------------------------------------------

4. Protocol Interactions
"The consent of the resource owner, for giving a client access to a
protected resource, can be pre-configured authorization policies or
dynamically at the time when the request is sent."

"when the request is sent": The request we are talking is the Resource
Request (Client already has the access token) or the token request? I
believe is Resource Request.
would it be good to explicit where this authz. policies are
pre-configured (at the AS? were enforced before at the token request
and are now embedded on the Access token; or at the RS?);
and how the consent can be dynamically determined (will involve
interaction with the Res Owner : RS to RO; RS to AS to RO ?).
All this is out of scope of the draft, however I believe making
explicit at least where the authz policies are pre-conf can be a good
idea, just a suggestion.
(My understanding is that they are at the AS; and were enforced at the
token request; if introspection is used, can be enforced again at the
resource request).

4. Messages Flows description: A-F:
Would it be too redundant to state which security properties needs
each messages, or it will be clearer? (A,B,D,E,F encrypted+integrity
protected. C does not)
This is just a question/suggestion; each message sec. requ. are stated
in other parts of the draft (early on section 4, and on section 5 and
6...).

5.  Framework
Proof-of-Possession
FROM           " i.e. that the authenticated token holder is bound to
the token."
to Something like " i.e. that the token holder can prove being a
holder of the key bound to the token"

5.  Framework
"In OAuth 2.0 the communication with the Token and the Introspection
resources at the AS (...) This framework RECOMMENDS to use CoAP instead and
RECOMMENDS the use of the following alternative instead of Uri-query
parameters: The sender (client or RS) encodes the parameters of its
request as a CBOR map and submits that map as the payload of the POST
request.  The Content-format MUST be "application/cbor" in that case."

If we use plain cbor on top of coap this means we are using dtls  to
protect the messages.

If we are protecting the messages with cose, should we not use the
Content-Format: "application/cose;cose-type=cose-encrypt" ?

All the examples of the doc use appplication/cbor; however, can be
useful to mention somewhere (here?) that if cose is used the content
format is "application/cose"?

----------------------
6.  The 'Token' Resource
"Communication between the client and the token resource at the AS
MUST be integrity protected and encrypted.  Furthermore AS and client
MUST perform mutual authentication. Profiles of this framework are
expected to specify how authentication and communication security is
implemented."

Is it worthy (.. is it accurate?) to express that the Token Request,
from C to AS might not be source-authenticated, and the AS -in that
case- can authenticate the request ("a posteriori") with the
client_id/client_secret?
The buzz in my head came because authentication of the client can be
done at dtls/cose level (that, for me, its not a must), and also with
the client credentials inside the request.
I think that the description on the text is generic enough so any
interpretation of how to authenticate the C towards AS, is OK.
In any case I send to you this (confusing) thoughts I had when I first
read this sentence. Authentication can be done at communication level
with the PSK C-AS , and also with the client_secret (maybe this is an
"application authentication"...).

6.  The 'Token' Resource
"The figures of this section uses CBOR diagnostic notation without the
integer abbreviations for the parameters or their values for better
readability."
(already did this comment) is worth mentioning that if cose is used,
the coap messages content-type are application/cose, and we show the
cbor plaintext payload?

-------------------

6.1.  Client-to-AS Request
Figure 3: Example request for an access token bound to an asymmetric key.
"grant_type" : "token"

I have not found the definition of grant_type "token" , should not be
"client_credentials"? (
https://tools.ietf.org/html/rfc6749#section-4.4.2 )

------------
6.2.  AS-to-Client Response
Figure 5: Example AS response with an access token bound to a symmetric key.
"{ "access_token" : b64'SlAV32hkKG ...
      (...),
     "token_type" : "pop",
     "alg" : "HS256",
     "cnf" : {
       "COSE_Key" : { ...."

how does the "alg" field of the token response relates to the
"key_alg" from the COSE_Key object if present, they must match , or
cose_key alg must be a super-set of this "alg"?
In 6.4.2 this "alg" is defined, Now that I read the passage
attentively, this "alg" field in the token response is only used for
the initial PoP; then the key can be used with other algorithms (e.g.
to encrypt with aes-ccm). OK.
I left this comment to show the confusion that produced the "alg"
field at two different levels, maybe its worthy to briefly tell the
difference/purpose of each one in contrast to each other.
------------

6.4.4.  Confirmation

"COSE_Encrypted  In this case the 'cnf' parameter contains an
encrypted symmetriic key destined for the client."
(typo "symmetriic").
Is it needed to encrypt the symmetric key inside cnf?
Was not assumed that the communication between AS and Client was
already protected (the whole response was integrity protected and
encrypted).
Or it was a general statement, and this is a particular case? In any
case, fields outside cnf should be at least integrity protected.

GENERAL OBSERVATION (already did on section 4): Can be a good idea to
specify for each message which kind of security properties
(confidentiality, integrity/data authentication)'
we need and at what level/layer (e.g: transport layer/application
layer, Cose_key field needs confidentiality but the rest only
integrity, etc.)?

-----------

7.  The 'Introspect' Resource
" Finally the AS SHOULD to verify that the RS"
remove/"to" ?
------------

7.4.  Client Token

Personally I believe this is a very relevant use case.
This "client token" solution is elegant, and easy to understand on the
context on the rest of the framework.

------------

8.2.  Token Expiration
"RS verifies these by comparing them to values from its internal clock
as defined in [RFC7519].
In this case the RS must have a real time chip (RTC) or some other ..."
Shall it say "real time clock (RTC)"?
Its worthy to be more clear and say that the internal clock should
reflect the current date/time, or at least be synchronized with the
issuer-of-the-token's clock (the AS)?

----------------------

Appendix C.  Deployment Examples

Comment: I found it VERY useful to see the real exchange of messages.

C.1.  Local Token Validation
" The token includes the claim "aif" with the authorized
  access that an owner of the temperature device can enjoy.  The
  'aif' claim, issued by the AS, informs the RS that the owner of
  the token, that can prove the possession of a key is authorized to"

The first sentence should not say: ".. that an owner of the of the
token, that can prove the possession of a key can enjoy" instead of
the "owner of the temperature device" ?

C.1. Local Token Validation

About "AIF" claim

should not reference
https://tools.ietf.org/html/draft-bormann-core-ace-aif-03 somewhere?

"aif" claim is only mentioned on appendix C, what happened on the rest
of the document? (is not a valid claim?)
In cbor web token
(https://tools.ietf.org/html/draft-ietf-ace-cbor-web-token-00#appendix-A.3
) , the only reference of aif is on Appendix 3!
shall we give more visibility for the aif claim on the main body of the doc?

--------------------------------------------

C.2.  Introspection Aided Token Validation

"Figure 23: Request and Response Payload for C offline"
"COSE_Key" : {
         "kid" : b64'c29tZSBwdWJsaWMga2V5IGlk',
         "kty" : "oct",
         "alg" : "HS256",
"

* Should not be: "kty": "Symmetric " ?

* "alg", supossedly this is key_alg that specifies "Key usage
restriction to this algorithm".
"HS256", I found a reference on json web token, and says "indicates
that this token is signed using HMAC-SHA256."
Should not be put on "alg" the algorithm that will be used on the
DTLS-PSK performed later? e.g.: "AES-CCM-16-64-128"?
------------------------------

"E: The AS provides the introspection response containing
 (...). (...) allows the RS to verify the client's proof of
possession in step F.
After receiving message E, the RS responds to the client's POST in
step C with Code 2.01 Created."

See "Figure 24: Token Introspection for C offline"

"step C with Code 2.01 Created", Is this not the "Step F"?

-------------
Both comments coming from "Figure 25: Request and Response Payload for
Introspection:"

"Request-Payload:
...
  "client_id" : "FrontDoor",
  "client_secret" : "ytrewq"
"

This is introspection request from RS to AS.
How did the RS got the client_id and the client_secret? (previous,
message from C to RS is not protected)

"Response-Payload:
...
"cnf" : {
       "kid" : b64'...'}
"
Should not "cnf" need to have the whole "COSE_Key" (including "k")?
On the text it was not assumed that RS and C already shared a Key (if
they did, at some point after original C-AS request, AS must have
provisioned it to RS -this can be assumed-).
In the general case AS will have to transport the key to RS on the
introspection response I think

--------------------------------

Saludos,
Renzo

---

From: Ludwig Seitz <[email-masked]>
Date: Mon, 18 Jul 2016 18:21:05 +0200

> Hi Ludwig, Göran, et al.
~snip~

Thank you Renzo for taking the time to do this review. Please find my 
comments inline.

Regards,

Ludwig

Sorry this is a case of bad word-choice by me. The idea is of course 
that the AS determines the consent of the RO at the time of the access 
token request (i.e. C -> AS). This can be either done by asking the RO 
or by using authorization policies defined by the RO previously at the 
AS. I'm adding an issue about this.

> in other parts of the draft (early on section 4, and on section 5 and

This could depend on the profiles, e.g. message B could need to be 
encrypted (if a symmetric PoP key is transferred) or just integrity 
protected (if a asymmetric PoP key is used). Therefore I'd rather not 
make these kind of statements here.

> holder of the key bound to the token"

Sounds good.

> format is "application/cose"?

Correct, if we add other security wrappers around the data we may need 
to change the Content-format. I'll add an issue about this.

You are right, we should clarify these options ... creating an issue

> the coap messages content-type are application/cose, and we show the

Yes that was the intention of these examples. I'll check if that needs 
clarification.

Good catch! I think that was just an error.

The intention is of course that 'alg' is used to identify the initial 
PoP procedure, this may include generating further keys for 
communication security. Obviously the key_alg parameter, if present, 
should identify the same algorithm as the alg parameter.

I'll add an issue to clarify this.

> encrypted symmetriic key destined for the client."

Noted.

> Is it needed to encrypt the symmetric key inside cnf?

If the channel is not secure this would be necessary. Otherwise you can 
just use a COSE_Key inside 'cnf'

> case, fields outside cnf should be at least integrity protected.

It might be the case that this is not really needed. I'll go through the 
use cases and check. This comes from the fact that we copied the 
properties of the 'cnf' parameter form RFC7800, which includes an 
encrypted key.

> layer, Cose_key field needs confidentiality but the rest only
I think that depends on the specific contents of the message, so I'm not 
sure if it can be said generally for each message.

Noted

> context on the rest of the framework.

I'll clarify this.

Sorry all the examples in the appendices are not up to date, we intended 
to remove the 'aif' claim from this document for the time being and 
missed this section.

> shall we give more visibility for the aif claim on the main body of the doc?

See comment above.

I'll check that.

> DTLS-PSK performed later? e.g.: "AES-CCM-16-64-128"?

This is actually not quite clear in the COSE draft. Table 3 suggests the 
parameter should be called 'alg' not 'key_alg'. I'll ask for 
clarification by Jim Schaad.

I think this example needs to be updated as well, it seems broken.

Yes this figure is not aligned with the steps in figure 1. I'll fix that.

Good catch, all these examples in the appendices need to be looked over 
I think.

> introspection response I think
You are right there as well.

-- 
Ludwig Seitz, PhD
SICS Swedish ICT AB
Ideon Science Park
Building Beta 2
Scheelevägen 17
SE-223 70 Lund

Phone +46(0)70-349 92 51
http://www.sics.se