Re: [Ace] Genart last call review of draft-ietf-ace-oauth-params-06

[Ludwig Seitz <[e-mail]>]

Hello Elwyn,

I have now submitted -09 to fix the minor issues and nits, which I
forgot in my -08.

Comments inline.


Regards,

Ludwig


On 2019-12-14 23:46, Elwyn Davies via Datatracker wrote:

> Minor issues:
> ss3.1, 3.2 and 4.1:  The COSE_Key type 'EC' used in several kty fields is not
> defined.  I assume it should be 'EC2'.
>
Fixed

> ss3.1, 3.2 and 4.1:  Does it matter that the definitions of the x and y
> parameters in an EC2 key are given as 'h' encodings in RFC8152 but are given as
> 'b64' in this document?  I am very much not an expert here.
>
All changed to 'h' encoding

> s6: This section starts with 'If CBOR is used...': The main ACE draft
> draft-ietf-ace-oauth-authz is apparently intended to cover both JSON and CBOR
> encodings of payloads, although CBOR is recommended.  This is not made explicit
> in this addition to that specification and the use of CBOR diagnostic
> representation and the prominence of COSE_Key items could make it appear up
> until s6 that this specification is intended just for CBOR encoding.  A few
> words at the beginning would clarify the dual alternatives.
>
Added a paragraph in the introduction to clarify this.


> Nits/editorial comments:
> General: s/e.g./e.g.,/ (3 places)
>
Fixed

> Abstract, 2nd sentence: s/whishes/wishes/
>
Fixed
> Abstract: Need to expand AS and RS.
>
> s2:  RS, AS and (probably) various other terms are defined in RFC 6749 and need
> to be expanded on  first use.  Adding something like the para from
> draft-ietf-ace-oauth-authz would solve this (except for the abstract).
>
I added a sentence in the terminology section to clarify this. However
note that it already said (in -06):

    Readers are assumed to be familiar with the terminology from
    [I-D.ietf-ace-oauth-authz].

Which included the terms AS and RS.

>
> s3:  This section needs a reference to RFC 8152 for the specification of the
> CWT COSE_Key item that is used extensively.

Done

>
> s3/s4: Some introductory text for each section is desirable.
>
Done

> s3.1, para 2 (definition of req_cnf):: Possibly add a note that the
> recommendation against symmetric keys implies currently kty being 'Symmetric'.
> Will it ever be anything else?
>
Done

> s3.1:  The text in s3.2 of draft-ietf-ace-cwt-proof-of-possession-03 contans
> the following
>
>     The COSE_Key MUST contain the required key members for a COSE_Key of that
>     key type and MAY contain other COSE_Key members, including the "kid" (Key
>     ID) member.
>
>     The "COSE_Key" member MAY also be used for a COSE_Key representing a
>     symmetric key, provided that the CWT is encrypted so that the key is not
>     revealed to unintended parties. The means of encrypting a CWT is explained
>     in [RFC8392]. If the CWT is not encrypted, the symmetric key MUST be
>     encrypted as described in Section 3.3.
>
> These riders probably apply to all the subsectons of s3 and to s4.1 and could
> be included in the currently empty main section text.
>

Here I disagree. The text explicitly refers to
draft-ietf-ace-cwt-proof-of-possession, saying that the contents of the
'cnf', 'req_cnf' and 'rs_cnf' parameters use the syntax of the 'cnf'
claim from section 3.1 of draft-ietf-ace-cwt-proof-of-possession.
The requirements in section 3.2 draft-ietf-ace-cwt-proof-of-possession
follow from the use of the definitions in 3.1.

I don't see the value of reiterating such a long text from that document
here, when an explicit reference is already given.


> s4.1, rs_cnf - DTLS-RPK: This term needs a reference (RFC 7250). Also all other
> uses do not hyphenate and RPK needs to be expanded.
>     s/DTLS-RPK handshake/DTLS Raw Public Key (RPK) handshake [RFC7250]/
Done