[COSE] AD Review of draft-ietf-cose-countersign-05

[Roman Danyliw <[e-mail]>]

Hi!

I performed an AD review of draft-ietf-cose-countersign-05.  Thanks for this document and the efforts of the WG, especially Russ and the chairs, who picked up this document after Jim Schaad 's untimely passing.  This document is another reminder of Jim's lasting and significant contribution to our community.

My feedback is as follows:

** Updating RFC8152.

-- Abstract.  The meta-data suggests that this document updates RFC8152, please note that in this section.

-- The text of this document doesn't seem to explicitly say what section of RFC8152 is being updated and in what way.  The closest is the guidance of the revised IANA table.  Typically, an explicit statement would be made.  Could something to the effect of "The countersignature approach described in Section 4.5 of RFC8152 is  deprecated" be added somewhere.

** Section 1.

   One of the standards that has
   come out of this process is "Concise Binary Object Representation
   (CBOR)" [RFC8949].  CBOR extended the data model of the JavaScript
   Object Notation (JSON) [STD90]

Editorial.  Since the text is referencing "standards" in the IETF maturity sense, either reference CBOR as STD94 or JSON as RFC8259 to be consistent.

** Section 1. s/IoT world/IoT user cases/

** Section 1.  Recommend being clearer on the motivation, especially since multiple document are being cited and a distinction is being made between the abstract property of the countersignature and the precise specification.

OLD
   During the process of advancing COSE to an Internet Standard, it was
   noticed the description of the security properties of
   countersignatures was incorrect for the COSE_Sign1 structure.  Since
   the security properties that were described, those of a true
   countersignature, were those that the working group desired, the
   decision was made to remove all of the countersignature text from
   [I-D.ietf-cose-rfc8152bis-struct] and create a new document to both
   deprecate the old countersignature algorithm and to define a new one
   with the desired security properties.

NEW
During the process of advancing COSE to an Internet Standard, it was noticed that while the description of the security properties of countersignatures was accurate, the associate specification of their implementation in Section 4.5 of RFC8152 [RFC8152] was incorrect for the COSE_Sign1 structure.  To remedy this situation, the working group decided to remove all of the countersignature text from [I-D.ietf-cose-rfc8152bis-struct] which was also updating [RFC8152] and create this, new document to both deprecate the previously specified countersignature algorithm and to define a revised one.

** Section 1

The new
   algorithm is more aggressive about the set of values included in the
   countersignature computation so that the cryptographic computed
   values is included.

Can this be restated.  I'm not sure what is means to be "more aggressive about the set of values ..."

** Section 1.  Typo. s/cryptographic computed/cryptographically computed/

** Section 1.3.

   Information which is part of the
   context can come from several different sources including: Protocol
   interactions, associated key structures, and program configuration

-- s/Protocol interactions/protocol interactions/

-- Is it "program configuration" or application configurations"?  [I-D.ietf-cose-rfc8152bis-struct] seems to favor application.

** Section 2.  Table 1.  What is the purpose of the blank "Value Registry" column?  I appreciate that this column is from the associated IANA registry, but the specifics are restated in Section 5.

** Section 3.  Editorial.  Be clearer on which "operations" are referenced.

OLD
It needs to be noted that the
   countersignature needs to be treated as a separate operation from the
   initial operation even if it is applied by the same user as is done
   in [I-D.ietf-core-oscore-groupcomm].

NEW
A countersignature needs to be treated as a distinct operation from the initial operation that created the security structures on which it is being applied even if both are being performed by the same user as is done in [I-D.ietf-core-oscore-groupcomm].

** Section 3.  Editorial. s/original version Section 4.5 of [RFC8152]/original version specified in Section 4.5 of [RFC8152]/

** Section 3.  Editorial.  s/a time point/a point in time/

** Section 3.
When done on a COSE_Mac or COSE_Mac0, the
   payload is included as well as the MAC value.  

Unlike the rest of the text, this sentence is describing what is signed not the semantics.  Is this operation like COSE_Signature ("normal countersign semantics) or COSE_Sign (where there is a parallel signature).

** Section 3.
It should be noted that there is a big difference
   between attesting to the encrypted data as opposed to attesting to
   the plaintext data.  

No disagreement.  I might have said s/big different/distinction/.  Isn't the distinction among three categories - on encrypted data (i.e., COSE_Encrypt, COSE_Encrypt0), on plaintext data (i.e., COSE_Sign), and on "cryptographic operation results" (i.e., COSE_Signature). It might be clearer to tie this caught directly the specific signing targets it implies.

** Section 3.1.  I believe that Section 8.1 of [I-D.ietf-cose-rfc8152bis-struct] is the more specific reference for signature algorithms with appendix.

** Section 3.2

   Abbreviated countersignatures were designed primarily to deal with
   the problem of encrypted group messaging, but where it is required to
   know who originated the message. The objective was to keep the
   countersignature as small as possible while still providing the
   needed security.    

Why does the history of the primitive matter?  Wouldn't it be better for this text simply to state what it is?  What is "needed security"

NEW
Abbreviated countersignatures only encode the signature value and rely on the shared application or protocol context to guide it's processing.

** Section 3.3.  Editorial.  Additional clarity.

OLD
process takes in countersignature target
   structure

NEW
process takes in countersignature target
   structure (i.e., COSE_Signature, COSE_Sign1, COSE_Sign, COSE_Mac, COSE_Mac0, COSE_Encrypt, or COSE_Encrypt0)

** Section 3.3.  A numbered, 6-item list of the contents of the Countersign_structure is provided.

-- Editorial.  For easier reading, provide the name of the actual field in the bulleted list for all 6 items.  For example:

OLD
2.  The protected attributes from the target structure encoded in a
       bstr type.  If there are no protected attributes, a zero-length
       byte string is used.

NEW
2.  body_protected: The protected attributes from the target structure encoded in a
       bstr type.  If there are no protected attributes, a zero-length
       byte string is used.

-- The text of #2 and #3 says this field is a bstr, but the CCDL fragment says its empty_or_serialized_map.

** Section 3.3.  Be clear that there are mutually exclusive choices.

OLD
   4.  Place the resulting signature value in the correct location.
       This is the 'signature' field of the COSE_Countersignature
       structure.  This is the value of the Countersignature0 attribute.

NEW
   4.  Place the resulting signature value in the correct location.
       This is the 'signature' field of the COSE_Countersignature 
       structure for full countersignatures (see Section 3.1).  This is the value of the Countersignature0 attribute for abbreviated countersignatures (see Section 3.2).

** Section 4.

In order to always regenerate the same byte string for the "to be
   signed" value, the core deterministic encoding rules defined in
   Section 4.2.1 of [RFC8949].  These rules match the ones laid out in
   Section 9 of [I-D.ietf-cose-rfc8152bis-struct].

-- Editorially, the first sentence doesn't parse as it is missing a verb.

NEW
In order to always regenerate the same byte string for the "to be
   signed" value, the core deterministic encoding rules defined in
   Section 4.2.1 of [RFC8949] MUST be followed

-- What is the guidance to implementers with the sentence?  Are they also supposed to following the restrictions in Section 9 of  [I-D.ietf-cose-rfc8152bis-struct]?

** Section 5.1.  Please explicitly state that this is a new entry in the "CBOR Tags" registry.

** Section 5.2. Table 2

-- Typo. s/vesion/version/

-- Why is the description for "counter signature version 2" = "v2 countersignature attribute", but for "Countersignature0 version 2" = "abbreviated ... version 2" - that is, why don't they both begin with "V2 xxx" (i.e., "V2 full counter signature" and "V2 abbreviated counter signature")?

* Section 6.  Why are the first 6 paragraph of this document, ~1.5 pages, cut-and-paste from draft-ietf-cose-rfc8152bis-struct-15?  That document is already with the RFCEd, please just cite the text and focus on the unique guidance here.

Thanks,
Roman