CBOR Object Signing and Encryption (COSE): Initial Algorithms
draft-ietf-cose-rfc8152bis-algs-12

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

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 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

(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 [1], I didn't find 
    any other discussion in the mail archive.  Was the status discussed in 
    the WG?

    The Shepherd writeup [2] 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.

[1] https://mailarchive.ietf.org/arch/msg/cose/tVDVZtfBhfYsKiqT0kCtkGoL_2U/
[2] 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 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 everyone else has pointed out, the header needs to be fixed to indicate this is Informational, not Standards Track.

Section 1.
s/messages transport/message transport
s/of the Javascript/of Javascript

Sec 1.3
In the definitions of “AE” and “AEAD”, I don’t understand the functional difference between authentication of “plaintext contents” (AE) and authentication of “non-encrypted data” (AEAD). AFAICT AE isn’t actually used in the document, so it might be easiest to simply delete it.

Sec 1.5. Replace the URL with a reference.

I actually read this whole document but got pretty lost by the end, not being an expert in this area.

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