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
<https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.
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.
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".
* 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 "assumes ... unless" should be "MUST ... unless".
* Also in section 3.3.1:
... New access tokens issued by the
authorization server are supposed to replace previously issued access
tokens for the respective client.
Is this normative? Should "are supposed to" be "MUST"?
3) MINOR: Insufficient specification
(I'm waffling whether this is minor or major.)
There are a couple of places where what seem to be requirements are
stated too vaguely to be implemented consistently:
* In the previously mentioned paragraph in 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.
The "unless specifically stated otherwise" is too vague to be normative.
How would the alternative be indicated? Is this an escape hatch for
future extensions? If so, it needs more work to make that clear and to
open a path for that future work.
* Also in section 3.3.1:
... The resource server therefore must
have a common understanding with the authorization server how access
tokens are ordered.
The last statement ("must have a common understanding") is mysterious.
IIUC section 4 is covering the same topic in a less mysterious way.
4) NIT: Subsection organization
Both 3.2 and 3.3 share a common structure:
* The section begins with discussion of the interaction between the
client and the AS.
* it is followed by a subsection discussing the interaction between the
client and the RS.
It is odd to have a section with a single subsection. And the structure
isn't easily discerned from the TOC.
I suggest it would be clearer if each of these sections had *two*
subsections, one covering the AS interactions and the other the RS
interactions. IOW, put the material covering the AS interactions into a
new subsection.
_______________________________________________
Gen-art mailing list
Gen-art@ietf.org
https://www.ietf.org/mailman/listinfo/gen-art