Re: [Ace] Gen-ART Last Call review of draft-ietf-ace-dtls-authorize-12

[Paul Kyzivat]

Jim,

Sorry - I forgot to respond sooner.

Please see comments inline.

Thanks,
Paul

On 7/19/20 6:29 PM, Jim Schaad wrote:




-Original Message-
From: Paul Kyzivat 
Sent: Sunday, July 19, 2020 1:24 PM
To: draft-ietf-ace-dtls-authorize@ietf.org
Cc: General Area Review Team 
Subject: Gen-ART Last Call review of draft-ietf-ace-dtls-authorize-12

I am the assigned Gen-ART reviewer for this draft. The General Area Review
Team (Gen-ART) reviews all IETF documents being processed by the IESG for
the IETF Chair.  Please treat these comments just like any other last call
comments.

For more information, please see the FAQ at

.

Document: draft-ietf-ace-dtls-authorize-12
Reviewer: Paul Kyzivat
Review Date: 2020-07-19
IETF LC End Date: 2020-07-20
IESG Telechat date: ?

Summary:

This draft is on the right track but has open issues, described in the

review.


General:

TBD

Issues:

Major: 2
Minor: 1
Nits:  1

1) MAJOR: Management of token storage in RS

There seems to be an expectation that when the client uploads an access

token

that the RS will retain it until the client attempts to establish a DTLS
association. This seems to require some sort of management of token

lifetime

in the RS. The only discussion I can find of this issue is the following

in section 7:


 ... A similar issue exists with the
 unprotected authorization information endpoint where the resource
 server needs to keep valid access tokens until their expiry.
 Adversaries can fill up the constrained resource server's internal
 storage for a very long time with interjected or otherwise retrieved
 valid access tokens.

This seems to imply a normative requirement to keep tokens until their

expiry.

But I find no supporting normative requirements about this. And, this

section

only presents it as a DoS attack, rather than a potential problem with

valid

usage.

ISTM that there is an implied requirement that the RS have the capacity to
store one access token for every PoP key of every authorized client.
If so, that ought to be stated. If not, then some other way of bounding

storage

ought to be discussed.


In section 5.8.1 of draft-ietf-ace-oauth-authz is the sentence "The RS MUST
be prepared to store at least one access token for future use."   When this
was put in, this is exactly what we were discussing.  There is no
requirement that an RS needs to store two access tokens for future use.  I
think this means that there is a strongly bounded requirement on storage.


When I read that, it seems that the stated requirement is to store one 
token *in total*, yet the context implies that it ought to be one token 
*per* client.


The procedures defined aren't going to work unless there is one per client.


Authors - It might be worthwhile to re-iterate this requirement in both of
the profile documents.



2) MAJOR: Missing normative language

I found several places where the text seems to suggest required behavior

but

fails to do so using normative language:

* In section 3.3:

 ... Instead of
 providing the keying material in the access token, the authorization
 server includes the key identifier in the "kid" parameter, see
 Figure 7.  This key identifier enables the resource server to
 calculate the symmetric key used for the communication with the
 client using the key derivation key and a KDF to be defined by the
 application, for example HKDF-SHA-256.  The key identifier picked by
 the authorization server needs to be unique for each access token
 where a unique symmetric key is required.
 ...
 Use of a unique (per resource server) "kid" and the use of a key
 derivation IKM that is unique per authorization server/resource
 server pair as specified above will ensure that the derived key is
 not shared across multiple clients.

The uniqueness seems to be a requirement. Perhaps "needs to be unique"
should be "MUST be unique". (And something similar for the IKM.)

* Also in section 3.3:

 All CBOR data types are encoded in CBOR using preferred serialization
 and deterministic encoding as specified in Section 4 of
 [I-D.ietf-cbor-7049bis].  This implies in particular that the "type"
 and "L" components use the minimum length encoding.  The content of
 the "access_token" field is treated as opaque data for the purpose of
 key derivation.

IIUC the type of serialization and encoding is a requirement. Will need

some

rewording to make it so.

* In section 3.3.1:

 ... To
 be consistent with the recommendations in [RFC7252] a client is
 expected to offer at least the ciphersuite TLS_PSK_WITH_AES_128_CCM_8
 [RFC6655] to the resource server.

I think "is expected" should be "MUST".


The rule would be MUST implement not MUST offer.  A client could offer a
completely different ciphersuite that is consistently used in the group and
that would be fine.

Re: [Ace] Gen-ART Last Call review of draft-ietf-ace-dtls-authorize-12

[Jim Schaad]

> -Original Message-
> From: Paul Kyzivat 
> Sent: Sunday, July 19, 2020 1:24 PM
> To: draft-ietf-ace-dtls-authorize@ietf.org
> Cc: General Area Review Team 
> Subject: Gen-ART Last Call review of draft-ietf-ace-dtls-authorize-12
> 
> I am the assigned Gen-ART reviewer for this draft. The General Area Review
> Team (Gen-ART) reviews all IETF documents being processed by the IESG for
> the IETF Chair.  Please treat these comments just like any other last call
> comments.
> 
> For more information, please see the FAQ at
> 
> .
> 
> Document: draft-ietf-ace-dtls-authorize-12
> Reviewer: Paul Kyzivat
> Review Date: 2020-07-19
> IETF LC End Date: 2020-07-20
> IESG Telechat date: ?
> 
> Summary:
> 
> This draft is on the right track but has open issues, described in the
review.
> 
> General:
> 
> TBD
> 
> Issues:
> 
> Major: 2
> Minor: 1
> Nits:  1
> 
> 1) MAJOR: Management of token storage in RS
> 
> There seems to be an expectation that when the client uploads an access
token
> that the RS will retain it until the client attempts to establish a DTLS
> association. This seems to require some sort of management of token
lifetime
> in the RS. The only discussion I can find of this issue is the following
in section 7:
> 
> ... A similar issue exists with the
> unprotected authorization information endpoint where the resource
> server needs to keep valid access tokens until their expiry.
> Adversaries can fill up the constrained resource server's internal
> storage for a very long time with interjected or otherwise retrieved
> valid access tokens.
> 
> This seems to imply a normative requirement to keep tokens until their
expiry.
> But I find no supporting normative requirements about this. And, this
section
> only presents it as a DoS attack, rather than a potential problem with
valid
> usage.
> 
> ISTM that there is an implied requirement that the RS have the capacity to
> store one access token for every PoP key of every authorized client.
> If so, that ought to be stated. If not, then some other way of bounding
storage
> ought to be discussed.

In section 5.8.1 of draft-ietf-ace-oauth-authz is the sentence "The RS MUST
be prepared to store at least one access token for future use."   When this
was put in, this is exactly what we were discussing.  There is no
requirement that an RS needs to store two access tokens for future use.  I
think this means that there is a strongly bounded requirement on storage.

Authors - It might be worthwhile to re-iterate this requirement in both of
the profile documents.

> 
> 2) MAJOR: Missing normative language
> 
> I found several places where the text seems to suggest required behavior
but
> fails to do so using normative language:
> 
> * In section 3.3:
> 
> ... Instead of
> providing the keying material in the access token, the authorization
> server includes the key identifier in the "kid" parameter, see
> Figure 7.  This key identifier enables the resource server to
> calculate the symmetric key used for the communication with the
> client using the key derivation key and a KDF to be defined by the
> application, for example HKDF-SHA-256.  The key identifier picked by
> the authorization server needs to be unique for each access token
> where a unique symmetric key is required.
> ...
> Use of a unique (per resource server) "kid" and the use of a key
> derivation IKM that is unique per authorization server/resource
> server pair as specified above will ensure that the derived key is
> not shared across multiple clients.
> 
> The uniqueness seems to be a requirement. Perhaps "needs to be unique"
> should be "MUST be unique". (And something similar for the IKM.)
> 
> * Also in section 3.3:
> 
> All CBOR data types are encoded in CBOR using preferred serialization
> and deterministic encoding as specified in Section 4 of
> [I-D.ietf-cbor-7049bis].  This implies in particular that the "type"
> and "L" components use the minimum length encoding.  The content of
> the "access_token" field is treated as opaque data for the purpose of
> key derivation.
> 
> IIUC the type of serialization and encoding is a requirement. Will need
some
> rewording to make it so.
> 
> * In section 3.3.1:
> 
> ... To
> be consistent with the recommendations in [RFC7252] a client is
> expected to offer at least the ciphersuite TLS_PSK_WITH_AES_128_CCM_8
> [RFC6655] to the resource server.
> 
> I think "is expected" should be "MUST".

The rule would be MUST implement not MUST offer.  A client could offer a
completely different ciphersuite that is consistently used in the group and
that would be fine.  

> 
> * Also in section 3.3.1:
> 
> ... This
> specification assumes that the access token is a PoP token as
> described in [I-D.ietf-ace-oauth-authz] unless specifically stated
> otherwise.
> 
> I think