Subject: [lamps] AD Review of draft-ietf-lamps-cms-update-alg-id-protect-02

From: Roman Danyliw <[email-masked]>
Date: Mon, 27 Jul 2020 19:03:16 +0000

Hi!

I conducted an AD review of draft-ietf-lamps-cms-update-alg-id-protect-02.  Thanks for the work on this document to harden CMS.  Below is my feedback.  As it is editorial in nature, it can be handled with any other IETF LC feedback.

** Section 3.  Section title. Editorial. s/Require use the same hash algorithm/Required use of the same hash algorithm/

** Section 3.  Editorial.  Recommend consistent use of either signedAttrs or signedAttributes (i.e., choose either the data structure name or the variable name)
-- Section 3.1 says "If signedAttrs are present in SignerInfo ..."

-- Section 3.2 says "When the signedAttrs field is present ..."

-- Section 3.3.says "If the SignedData signerInfo includes signedAttributes ..." and "... included in the signedAttributes of the SignedData signerInfo"

** Section 3.1.  Editorial.  Simplify the guidance.

OLD
If signedAttrs are present in the SignerInfo, then the same digest
      algorithm MUST be used to compute the digest of the SignedData
      encapContentInfo eContent, which is carried in the message-digest
      attribute, and to compute the digest of the DER-encoded SET OF
      signed attributes, which is passed to the signature algorithm.
NEW
If signedAttrs are present in the SignerInfo, then the same digest algorithm MUST be used to compute both the digest of the SignedData encapContentInfo eContent, which is carried in the message-digest attribute, and the digest of the DER-encoded SET OF signed attributes, which is passed to the signature algorithm.

** Section 3.2. Typo. s/the the/the/

** Section 3.3.  Typo. s/the the/the/

** Section 3.4.  Significant editorial.  s/might lead to compatibility/might lead to incompatibility/

** Section 6.  Per "There is not currently protection mechanism ..." and "Likewise there is not currently a protection mechanism ...", it's true that these are a gap, but would that age well?  Perhaps just say:

OLD
There is not currently protection mechanism for
   the algorithm identifiers used in the enveloped-data, digested-data,
   or encrypted-data content types.  Likewise there is not currently a
   protection mechanism for the algorithm identifiers used in the
   authenticated-enveloped-data content type defined in [RFC5083].

NEW

It provides no protection mechanism for the algorithm identifiers used in the enveloped-data, digested-data, or encrypted-data content types.  Likewise, The CMSAlgorithmProtection attribute provides no protection for the algorithm identifiers used in the authenticated-enveloped-data content type defined in [RFC5083].

Regards,
Roman

---

From: Russ Housley <[email-masked]>
Date: Mon, 27 Jul 2020 16:18:28 -0400

Roman:

Thanks for the careful review and editorial suggestions.  I have taken acre of all of these in my edit buffer.

In your final comment (below), I came up with a slightly different wording.

> On Jul 27, 2020, at 3:03 PM, Roman Danyliw <[email-masked]> wrote:
> 
> Hi!
> 

The paragraph now reads:

   The CMSAlgorithmProtection attribute [RFC6211] offers protection for
   the algorithm identifiers used in the signed-data and authenticated-
   data content types.  However, no protection is provided for the
   algorithm identifiers in the enveloped-data, digested-data, or
   encrypted-data content types.  Likewise, The CMSAlgorithmProtection
   attribute provides no protection for the algorithm identifiers used
   in the authenticated-enveloped-data content type defined in
   [RFC5083].

Russ