Summary: Has enough positions to pass.
Thanks for the easy to read diff on a bis document, and for addressing the SecDir feedback. ** Section 8. Per “For an algorithm, the first element should always be a key type value, but the items that are specific to a key should not be included in the algorithm capabilities” and later “For a key, the first element should also be a key type value” is there a reason for not making these “should” into normative MUSTs? ** Section 8. I would have benefited from more text to understand how to parse a capabilities field. IMO, it would have been helpful to say that the first element of the array maps to the Value column of the COSE Key Type registry. The new Capabilities column of that corresponding entry describes the semantics of the second array element. ** Editorial Nits: -- The text in the header notes this document is standards track. However, the datatracker and shepherd write-up note that it is informational. I suspect it should be fixed in this document to be informational. -- Abstract. Please remove the explicit references from abstract (as they are not permitted to be there) -- Abstraction. Editorial. The sentence “In this specification the conventions …” is incomplete. -- Section 1. Editorial. “Additional algorithms beyond what are in this document are defined elsewhere”. IMO, this sentence doesn’t appear to add anything. Recommend removal. -- Section 8. Per ”There is a presumption in the way that this is laid out is that the algorithm identifier itself is not needed to be a part of this as it is specified in a different location”, I had trouble following this sentence from the previous one – what is the double reference to “this” here? -- Section 8.3. Typo. s/it is encodes/, they encode/ -- Section 10.2. Typo. s/rquested/requested/
Thank you for addressing my Discuss point. I guess I was expecting a little more text as well as the [HKDF] reference in Section 5.1, though I confess I am not entirely sure what I was expecting such text to actually say...
As others have pointed out, the Abstract needs fixing. I'm surprised none of my colleagues thought the document status and downrefs problem isn't worthy of a DISCUSS, because it's important to get right. But as one of the newbies of the group I'll go along with them and merely pile on by mentioning it again. And lo, a bunch of editorial nits: Section 1: * "... small in terms of messages transport and implementation size ..." -- s/messages/message/ Section 1.4: * Missing period at the end of the last sentence. Section 2: * "Part Section 9.1 of ..." -- remove "Part" Section 2.1: * "... collisions of this value leads to ..." -- s/leads/lead/ * "(2 coordinate elliptic curve)" -- suggest "two" instead of "2" OLD: Other documents can define it to work with other curves and points in the future. NEW: Future documents may extend support to include other curves and points. Section 2.1.1: * "... truncate a hash function down ..." -- "down" feels redundant here Section 2.2.1: * "... for this reason, they should not be used with the other algorithm." -- I don't understand what this is saying. Section 3: * "Part Section 9.2 of ..." -- remove "Part" * "... such as MD5 has decreased over time; the security ..." -- s/;/,/ Section 3.2: * Where is "IV" defined? Section 3.2.1: * "Cipher Block Chaining (CBC) mode, if the same key ..." -- maybe add "In" at the front? Section 4: * Errant "Part" at the front again. Section 5: * ... and again. Section 5.1: * I had to look up what a "bstr" is. Is that definition assumed to be imported from somewhere? Section 6: * "Part" again. Section 8: * I don't understand the second paragraph of this section. Section 10.2: * "... this registration, if this is ..." -- "If" should start a new sentence. * "... then the DE should be ..." -- s/DE/Designated Expert/ Section 11: * "One area that has been starting to get exposure is doing traffic ..." -- the word "doing" feels out of place here
(1) I am concerned -- confused may be a better word -- about the status of this document for several reasons: (a) The header on this document still says that it is intended to remain in the Standards Track -- but the datatracker says that is should be Informational. This is simply a nit. (b) Except for a note when the publication was requested , I didn't find any other discussion in the mail archive. Was the status discussed in the WG? The Shepherd writeup  does say that the status "marks the state of consensus at the time of publication, and allows for the flexibility to deprecate and obsolete in the future." Except for potentially a higher bar when updating an Internet Standard, the process is the same... (c) The fact that this document resulted from the split of rfc8152 confuses me even more: the "other half" (rfc8152bis-struct) is moving on as an Internet Standard and it includes a Normative reference to this document. Note that the Normative reference makes sense, but the Informational status of this document doesn't...at least to me. Even though we can use DownRefs, it seems unnecessary to "downgrade" this part of the document and end up with a downref to an Informational document... This is a non-blocking comment...I simply don't understand.  https://mailarchive.ietf.org/arch/msg/cose/tVDVZtfBhfYsKiqT0kCtkGoL_2U/  https://datatracker.ietf.org/doc/draft-ietf-cose-rfc8152bis-algs/shepherdwriteup/ (2) §10.1/§10.2: The references should be changed from rfc8152 to this document. (3) §10.2 (Changes to "COSE Algorithms" registry) IANA is requested to create a new column in the "COSE Algorithms" registry. The new column is to be labeled "Capabilities". The new column is populated with "[kty]" for all current, non-provisional, registrations. It is expected that the documents which define those algorithms will be expanded to include this registration, if this is not done then the DE should be consulted before final registration for this document is done. I am not sure what is the expectation here; a new column is added and all the entries are populated with "[kty]" -- so far so good. What I don't understand is the part about other "documents...will be expanded to include this registration". Does that mean that the other documents need to be updated? What should the DE do if the work is not completed? I am even more confused because this document doesn't seem to take an action related to that new column for the algorithms defined here, and the new row (in this same section) doesn't include the Capabilities column. (4) §10.2: "Note to IANA: There is an action in [I-D.ietf-cose-rfc8152bis-struct] which also modifies data in the reference column." I didn't see that action in the other document. (5) I assume that this document (and not -struct) should also update the COSE Elliptic Curves registry.
Thank you for the work on this document. To be honest, I had only time to quickly browse through: I am trusting the security AD for the security considerations. Not really important but I wonder why the security considerations are spread through all the document (sections 3.1.1, 3.2.1, ...). It obviously makes the text clearer and easier to read but may I suggest to rename those sections in something more specific (to avoid the comparaison with the well-know security considerations section of all I-D -- that is Section 11 in this document) : e.g., "Section 4.2.1 Security Considerations of AES CCM". I noted that this is used in "6.2.1. Security Considerations for AES-KW" already, just be consistent ;-) Hope this helps -éric
Hi, I've only reviewed the diff against RFC8152. The vast majority of this document seemed to have only minimal changes and looked fine from what I could see. A few minor comments that may help improve the document: Abstract Concise Binary Object Representation (CBOR) is a data format designed for small code size and small message size. There is a need for the ability to have basic security services defined for this data format. This document defines the CBOR Object Signing and Encryption (COSE) protocol. This specification describes how to create and process signatures, message authentication codes, and encryption using CBOR for serialization. COSE additionally describes how to represent cryptographic keys using CBOR. In this specification the conventions for the use of a number of cryptographic algorithms with COSE. First sentence of the second paragraph doesn't quite scan, and I wasn't convinced that the first paragraph is necessarily correct/accurate after the document split (in particular the "This document" and "This specification" bits). Section 7.2 x: This contains the public key. The byte string contains the public key as defined by the algorithm. (For X25591, internally it is a little-endian integer.) d: This contains the private key. One minor thing that I noted, is that you define that the public key is defined by the algorithm, but don't say the same thing for the private key. If you change this, then you might want to check other references to "private key" as well. 8. COSE Capabilities The concept is being pulled forward and defined now for COSE. Perhaps rephrase this to: "This document defines the same concept for COSE." 8. COSE Capabilities The algorithm identifier is not part of the capabilities data as it should already be part of message structure. There is a presumption in the way that this is laid out is that the algorithm identifier itself is not needed to be a part of this as it is specified in a different location. I found the second sentence somewhat unclear and could probably be wordsmithed, in particular does "this" refer to the capabilities data or the message structure. 8. COSE Capabilities Two different types of capabilities are defined: capabilities for algorithms and capabilities for key structures. Once defined by registration with IANA, the list of capabilities is immutable. If it is later found that a new capability is needed for a key type or an algorithm, it will require that a new code point be assigned to deal with that. As a general rule, the capabilities are going to map to algorithm-specific header parameters or key parameters, but they do not need to do so. An example of this is the HSS-LMS key capabilities defined below where the hash algorithm used is included. Defining the capabilities list as immutable seemed like a misnomer to me because a new IANA registration would mutate the list. Perhaps this sentence, and the one following it, could be removed? The capability structure is an array of values, the order being dependent on the specific algorithm or key. For an algorithm, the first element should always be a key type value, but the items that are specific to a key should not be included in the algorithm capabilities. This means that if one wishes to enumerate all of the capabilities for a device which implements ECDH, it requires multiple pairs of capability structures (algorithm, key) to deal with the different key types and curves that are supported. For a key, the first element should also be a key type value. While this means that this value will be duplicated if both an algorithm and key capability are used, the key type is needed in order to understand the rest of the values. Should you be using RFC 2119 language here? And should " key should not be included" be "MUST NOT"? For the ECDH case, does that mean that the "algorithm" entry will be duplicated with different "key" entries? If so, would it help clarify to explicitly state this? Thanks, Rob