Hi

I after reading the specification a second time and started on a JavaScript
implementation things are starting to make sense and I like it. Maybe a bit
long (feels like a mountain to climb) before starting to work with it.
However well done Jim and all others that has contributed. I like the
combination of prose and CDDL, it is great to have the same thing described
in two ways.

If it´s not to late I would like to contribute with a set of comments.

== General comments
* Is it possible to link to tables it would be great not having to search
form them e.g. in section 3.1. where Common COSE Headers Parameters
referencing Table 2.

* Encryption section, 5, was harder to read then Signing, section 4, and
Mac, section 6. It felt a more mixed up, but I don’t have any concert
suggestions that would make it much better.

* Is this specification registering a whole new set of algorithms? and
registries? for CWT we only add mappings but keep the attributes, this is
also much of the work done in the ACE framework, but when I read here it
looks like you have new registries for everything. would it not be better
to just have mappings here.

* Without going into a deeper analysis it looks like section 7 to 9, and to
some extent 10 and 11 are doing the same thing as JWA (RFC7518). Would it
be possible to not duplicate things from RFC7518. I think this reflects in
the new IANA registries setup too. I think it would be good if we could try
to reuse more of the work gone into JOSE e.g. maybe only defining mappings
for the algorithm identifiers to CBOR labels/keys..


== Section specific comments and questions

=== 16.4.  COSE Algorithms Registry

“The initial contents of the registry can be found in Table 10,
   Table 9, Table 11, Table 5, Table 7, Table 8, Table 15, Table 16,
   Table 17, and Table 18.  The specification column for all rows in
   that table should be this document.”
* It would be great with links to the different sections (see general
comment about table links).


=== 1.1.  Design changes from JOSE
“MACed messages have the ability to use the same set of recipient
      algorithms as enveloped messages for obtaining the MAC
      authentication key.”
* Why is this called out for MACed messages, is this not true for signed
too?


=== 1.3.  CBOR Grammar
“We therefore describe the CBOR structures in prose.”
* I would prefer not to use ‘we’ and write something like “The CBOR
structures are therefore describe in prose.”

“The collected CDDL can be extracted from the XML version of this
   document via the following XPath expression below.  (Depending on the
   XPath evaluator one is using, it may be necessary to deal with >
   as an entity.)”
* Is the XML available for ready RFCs? I have only found it for drafts, if
not this comment might not be very helpful.


=== 1.4.  CBOR Related Terminology
“In COSE, we use strings, negative integers and unsigned
   integers as map keys.”
* Feels like mix of terms with unsigned and negative integers I think it
should be signed and unsigned or positive and negative integers.

“The integers are used for compactness of
   encoding and easy comparison.”
* A motivation for strings would be good too since the existence of the
option requires implementors to handle both (not crash if a string comes).
If I remember correctly the reason was was of development/debugging.


=== 3.  Header Parameters
“Two buckets are provided for each layer:”
* This is the first time I see the term layer used, maybe an explanation in
the terms section

“(With the exception of the COSE_Sign structure, the
   only data that needs to cross layers is the cryptographic key.)”
* Is the parentheses really required, I would just drop them.

“Labels in each of the maps MUST be unique.  When processing messages,
   if a label appears multiple times, the message MUST be rejected as
   malformed.  Applications SHOULD perform the same checks that the same
   label does not occur in both the protected and unprotected headers.
   If the message is not rejected as malformed, attributes MUST be
   obtained from the protected bucket before they are obtained from the
   unprotected bucket.”
* Does this mean that I can have algorithm in both protected and
unprotected bucket but that the protected one takes presence? Could we not
just forbid duplication over both buckets?


=== 3.1.  Common COSE Headers Parameters
* Why is some of the headers shortened (alg, crit) but not all (content
type)? I would prefer if we could map as close to JOSE as possible and I
would also like to avoid spaces in the names because then I can very easily
do JSON input to my implementation and have it do the transformation of
labels from strings to integers. When reading more I see that this applies
to more sections.

“This parameter MUST be present in the
      COSE_Signature, COSE_Sign1, COSE_Encrypt, COSE_Encrypt0, COSE_Mac,
      and COSE_Mac0 structures”
* Should it not be COSE_Sign and not COSE_Signature?


“(The algorithm range is -1 to -65536; the higher
         end is for more optional algorithm specific items.)”
* How does this relate to “Integer labels in the range -1 to -255 can be
omitted”?
* With the “higher end” are you referring to -1? it seem strange to have
the more optional parameters in that end to me


“As the IV is authenticated by the
      encryption process, it SHOULD be placed in the unprotected header
      bucket.”
* Is there a good reason for this SHOULD? why is it better to put it in the
unprotected header? if I could I would put all my headers in the protected
and not have to bother with the unprotected part. I would prefer the
phrasing under Partial IV to be “As the IV is authenticated by the
encryption process, this value can be placed in the unprotected header
bucket”

“COSE_Sign, COSE_Sign1, COSE_Signature, COSE_Encrypt,
      COSE_recipient, COSE_Encrypt0, COSE_Mac and COSE_Mac0”
* In table 2 it says that the counter signature is one or many
COSE_Signature, so it does not seem to match this statement?
* When is a counter signature an encryption?


=== 4.4.  Signing and Verification Process
“The protected attributes from the body structure encoded in a
       bstr type.  If there are no protected attributes, a bstr of
       length zero is used.”
* Or a empty map CBOR encoded, right?


=== 5.1.  Enveloped COSE Structure
* This might have been up before, but has it been considered to swap place
of ciphertext and recipients so that not the full message has to be
received before the CEK can be retrieved and maybe the ciphertext can be
decrypted as the message is received.
* I cannot se any language on the key format in the recipients structure is
is always a COSE key? should the content type header be used to describe
the key format? or is it implicitly known by the application context?


=== 5.3.  Encryption Algorithm for AEAD algorithms
=== 5.4.  Encryption algorithm for AE algorithms
* Capital ‘A’ on algorithm or a small ‘a’ in section 5.3 headline

* Heading “5.4.  Encryption algorithm for AE algorithms” and “5.3.
Encryption Algorithm for AEAD algorithms” is confusing compared to the
equivalent sections in mac and signing. I would like theses sections to
call out that it is how to encrypt and decrypt.

=== 7.  Key Objects
* The CDDL example looks different, in my opinion worse, compared to the
other earlier examples.

* Are there any examples, I found no reference here.


=== 7.1.  COSE Key Common Parameters
“If this parameter is present in the key structure,
      the application MUST verify that this algorithm matches the
      algorithm for which the key is being used.”
* This is not reflected in Signing, Encryption and Mac section, maybe it
would be good to add something there too, if the relation to COSE_Key
should be close.


=== 16.  IANA Considerations
Why is the major type included in the registry? Is it not enough with the
specification reference and having the definition there. In my opinion the
main goal of the registration is to avoid label collisions and and not to
provide more then basic information.


=== 17.2.  COSE Testing Library
I would prefer test vectors in the specification compared to this github
link.
_______________________________________________
COSE mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/cose

Reply via email to