Subject: Re: [Ace] FW: WGLC comments on draft-ietf-ace-dtls-authorize

From: Olaf Bergmann <[email-masked]>
Date: Mon, 05 Nov 2018 16:58:55 +0100

Jim,

Thank you for this very useful review. Please find our comments inline:

Jim Schaad <[email-masked]> writes:

> Section 3 - Am I just missing where you talk about introspection or is it
> missing from the text?

The role of token introspection for authorization is discussed in
draft-ietf-ace-oauth-authz which is referenced here as the governing
mechanism. The DTLS profile is completely agnostic in this regard as you
may or may not use token introspection on access tokens retrieved via
the DTLS profile.

> Section 3.2 - looks like you missed a "cnf" in the first paragraph - should
> be req_cnf

Fixed.

> Section 3.3 - Figure 4 - Where is the 'alg' parameter defined at that level?

See next comment.

> Section 3.3 - I am always bothered by the fact that PSK should really be PSS
> at this point.  The secret value is no longer a key and thus does not
> necessarily have a length.  There is also a problem of trying to decide what
> the length of this value would be based on the algorithm.  If the client
> offers TLS_PSK_WITH_AES_128_CCM_8 and TLS_PSK_WITH_AES_256_CCM_8  (I may
> have gotten these wrong but the intent should be understandable) then what
> length is the PSK supposed to be?

I think what you are saying is that for the shared secret (k) in the
COSE_Key structure in Fig. 4, the AS needs to tell C what to do with
that shared secret? This was the intention of the alg parameter (which
has a not-so-useful value in this example).

> Section 3.3 - Figure 5 - Is this defining a new set of entries for cnf or is
> there a missing layer someplace?

This is indeed a new set of entries. An internal discussion between the
draft authors led to the opinion that we cannot use a COSE_Key structure
in the cnf structure as this would have different semantics as intended
here. We came to the conclusion that for key derivation, listing the
required fields directly in the cnf structure is the cleanest solution.

> Section 3.3 - Should some type of key identifier also be defined so that the
> entire token is not always sent on every DTLS setup?  Also so that the key
> can be referenced on a second request to the AS.

How about adding the following to the paragraph on the cnf in the access
information at the end of the second paragraph in Section 3.3:

OLD:
  "AS adds a cnf parameter to the access information carrying a COSE_Key
  object that informs the client about the symmetric key that is to be
  used between C and the resource server."

NEW:
  "AS adds a cnf parameter to the access information carrying a COSE_Key
  object that informs the client about the symmetric key that is to be
  used between C and the resource server. To enable dynamic updates of
  the access token, the AS SHOULD add a kid field with a random
  identifier for the symmetric key that is conveyed by the cnf object."

> Section 3.4 - Is it worth while to make it explicit when the DTLS session is
> required to be shutdown and that a simple authorization error is not one of
> them?   Looks like it should be - these errors SHOULD NOT cause the DTLS
> connection to be closed.

How about this:

  "Unauthorized requests that have been received over a DTLS session
  should be treated as non-fatal by the RS, i.e., the DTLS session
  should be kept alive until the associated access token has expired."

> Section 4 - The requirement that the "kid" be in a previously issued token
> would have problems with this if you were to use the "kid" of an RPK which
> can be constant rather than being changed on each newly created token.

To solve this we could require that the COSE_Key structure for RS's RPK
in the AS-to-Client response must also have a kid. Section 4 does not
prevent the kid to be constant.

> Section 4 - The term "must associate" is slightly ambiguous - does this mean
> add to (augment) or replace the previous token.

How about:

OLD:
  "A resource server MUST associate the updated authorization
   information with any existing DTLS session that is identified by this
   key identifier.

NEW:
  "A resource server MUST replace the authorization information of any
  existing DTLS session that is identified by this key identifier with
  the updated authorization information."

(Effectively allowing only one access token per session at a time.)

> Section 4 - Should a secondary access token have a later expiration date
> that the one that contains the actual key value?

I suppose yes but this should be solved in general by the framework.

> Section 5 - this seems to be a very strange place to define the new
> parameter.

True. How about moving this parameter to Section 4?

> Section 8 - you need to put your kid in the IANA considerations as well

Fixed.

> Section 9.1 - Having tilicoa-tls-dos-handshake as a normative reference may
> cause problems unless this is on a publication track.

You are right. After speaking to the main author, we came to the
conclusion that we should remove this reference entirely as
draft-tilocoa-tls-dos-handshake will not be pursued any more.

> Section 9.3 - Can we get these URLs converted over to normal references?  

This section somehow is generated from the XML by the submission tool
(in the gh-pages version [1] you have clickable links in-place which is
very convenient for editing). Once the framework document's structure is
stable, we will convert them back to normal references.

[1] https://ace-wg.github.io/ace-dtls-profile/

Grüße
Olaf