Re: [COSE] 🔔 WGLC of draft-ietf-cose-webauthn-algorithms

[Mike Jones <[e-mail]>]

Thanks for your review, Jim.  Responses are inline, prefixed by “Mike>”.

                                                       -- Mike

From: Jim Schaad <[e-mail]>
Sent: Tuesday, September 17, 2019 6:46 PM
To: 'cose' <[e-mail]>
Cc: [e-mail]
Subject: RE: [COSE] 🔔 WGLC of draft-ietf-cose-webauthn-algorithms

I start this review by copying forward all of my comments on draft-jones-cose-additional-algorithms-00

1.  I was surprised that there are two algorithms still in the WebAuthn document that are not included here.  Is that intentional and what is the plan forward for them? This one was addressed



2.  The title of the document is not descriptive in terms of a standalone sentence on what this document is doing.  It should be more specific on the algorithms.  The new title is fine except for the fact that WebAuthn is not expanded to anything.



3.  Abstract:  A list of the algorithms being processed in this document would be reasonable.  Based on the current text, the abstract would need to be re-written as soon as the algorithms are registered because they would then be registered and thus the specification is not necessary.  It would also be slightly odd in the event that the CTAP or WebAuthn protocols are updated to add new algorithms as they would not be in this document.  (See issue #1.)  No Change



Mike> Understood.  I’ll reword so that the text remains correct after the IANA registrations have been performed.



4.  Section 1:  See abstract comment.  No change



Mike> Ditto



5.  Section 2: The recommended column needs to be added to the table.  No change



Mike> I assume the column you’re talking about is the one requested in the next comment.



6.  Section 2:  The discussion on what is and is not recommended and why belongs in this section not in the security considerations.  No change



Mike> OK, I can add an “Implementation Requirements” column, paralleling the treatment in https://tools.ietf.org/html/rfc7518#section-3.1.



7.  Section 2: You are missing some checks that should be done on signing and verification.  Look at the standard text at the end of sections in RFC 8152 for a pattern.  You might also want to add a check on lengths of keys both for a maximum and a minimum length.  No change



Mike> Key size considerations for the RSA algorithms are already covered in https://tools.ietf.org/html/draft-ietf-cose-webauthn-algorithms-01#section-5.1.  For ES256K, I can say that the X and Y values for the public key must both be exactly 256 bits.  What are the specific checks you wanted added?  I couldn’t tell which sections of RFC 8152 you were referring to.  Also, note that between draft-jones-00 and draft-ietf-01, the descriptions of the use of hash functions were beefed up, in response to other comments received.



8.  Section 3:  This should not be the first use of JOSE and COSE and therefore the expansions here make no sense.  That should be in the introduction.  No change



Mike> I’ll look into expanding abbreviations on first uses and not on subsequent uses.



9. Section 3.1:  I have only rarely seen this curve as P-256K, it is almost always referred to as secp256k1.  I would ask that this be changed in the COSE representation.  I am agnostic about what should be in the JOSE representation.  Fixed



10. Section 3.1:  I don't understand the reason for the last paragraph in the section.  It seems to me that this should just refer to all of the items defined for EC2 in COSE.  No Change



Mike> OK



11. Section 3.1:  Is there a recommendation on using compressed vs non-compressed points?  No change



Mike> I will state that uncompressed point representations are to be used for JWKs, as a compressed representation for key type “EC” is not defined.  And I will state that the uncompressed must also be used for CWKs, as I don’t know that the Galois y² = x³ + 7 finite field form used with secp256k1 is amenable to compression.  (However, if the CRFG or other experts tell me that it is, I will also allow for the compressed representation with COSE.)



12.  Section 3.1:  You need to talk about if this is going to be IETF recommended or not.   (And why)  No change



Mike> https://tools.ietf.org/html/draft-ietf-cose-webauthn-algorithms-01#section-4.1 has it being recommended.  I will describe why in the next version, as requested.



13.  Section 3.2:  I don't understand what this section is doing.  If you are really defining a new signature algorithm, then that should be noted as part of the requirements in section 3.1 on the key.  If you are not defining a new signature algorithm then this section makes zero sense.  (You are not saying what the hash algorithm is.)  Both options are reasonable.  This is partly dealt with:



Mike> This is intentionally exactly parallel to RFC 7518, which defines the “ES256” algorithm identifier to be used with the “P-256” curve.  In this case, it is defining the “ES256K” algorithm identifier to be used with the “secp256k1” curve.



  1.  Step 1 is incorrect in terms of COSE, it is not the payload that is signed.

Mike> I’ll revise the description here to align with correct COSE terminology.


  1.  The new signature algorithm is being defined without motivation.  There should be text describing why this is ES256K and not ES256 (ECDSA with SHA-256).  I do note that there may be different text being described here for JOSE than for COSE.

Mike> “ES256” is defined by RFC 7518 to be always used with the P-256 curve.  A different algorithm identifier is therefore required to use with secp256k1.  This is by design.  And the use of identifiers is being kept intentionally parallel between JOSE and COSE.


  1.  Please include text related to deterministic ECDSA in this text.

Mike> What do you want this text to say?  I’m reluctant to use the text at https://tools.ietf.org/html/rfc8152#section-8.1, which says that “implementations SHOLUD use a deterministic algorithm”, which is misleading, in that it implies that there are many such algorithms that could be used.  In fact, exactly one is being specified.


  1.  Text similar to the end of section 2.1 in draft-ietf-cose-rfc8152bis-algs should be included to nail things down tighter


Mike> Are you talking about the text starting “When using a COSE key for this algorithm, the following checks are made:”?  Are you requesting those same checks?



14. Section 3.2:  If this is a new signature algorithm then it should say what curves it operates with.  Else there should be an addition of the algorithms that are legal in section 3.1 Done, but see comment 13



15.  Section 4.1 - The correct term is 'provisional' not 'temporary'. Not of sufficient importance to worry about.  However you can just use the provisional numbers without any comment or TBD given they are assigned.



16. Section 5.4 - So if somebody changes the crv value, is there any way to tell the difference?  If so then that should be described here.  No change


Mike> The way that you can tell that the “crv” value has been changed is that “crv” parameter will have a different value.  That’s so simple as to be practically tautological and not worth explicitly saying.  I’m guessing you want me to say something else, but I don’t know what it is.



17.  Section 5.3 - This text seems to be self-contradictory.  If you MUST NOT use it, then how can you then use it?  How does an implementation know when it is or is not acceptable practice to use this?  No change


Mike> I will add the description agreed to in the on-list conversation with Neil Madden.

New Items:

18.  FIDO2 not expanded on first use in abstract and introduction.   Ditto for WebAuthn, COSE,


Mike> Will do

19.  Abstract does not refer to JSON – should it?


Mike> I’ll look into editorially where to best introduce JSON.

20.  Should be pointing to the current drafts and not to RFC 8152 in this document.


Mike> I’ll plan to update this specification once the bis versions are RFCs.  But there’s no need to take a normative dependence on the new versions, as this spec uses nothing new being introduced by them.

21.  Given that the algorithms in section 2 are already defined in JOSE, there should be text that this section only applies to COSE so there is no confusion.


Mike> Sure

Jim


From: COSE <[e-mail]<mailto:[e-mail]>> On Behalf Of ivaylo petrov
Sent: Tuesday, September 17, 2019 7:31 AM
To: cose <[e-mail]<mailto:[e-mail]>>
Subject: [COSE] 🔔 WGLC of draft-ietf-cose-webauthn-algorithms

Dear all,

This message starts the Working Group Last Call on the draft-ietf-cose-webauthn-algorithms [1].

The working group last call will run for **two weeks**, ending on
October 1, 2019.

Please review and send any comments or feedback to the working group. Even if your feedback is "this is ready", please let us know.

Thank you,

- Matthew and Ivaylo
COSE Chairs

[1]: https://datatracker.ietf.org/doc/draft-ietf-cose-webauthn-algorithms/<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdatatracker.ietf.org%2Fdoc%2Fdraft-ietf-cose-webauthn-algorithms%2F&data=02%7C01%7CMichael.Jones%40microsoft.com%7Cb65306e054df4a68cc4b08d73bda0c93%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C1%7C637043680013925141&sdata=MpyFQf0Mdxd6PEXtNUJJLJBjSDWGacfLnCIEsMBTqhg%3D&reserved=0>