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

[Russ Housley <[e-mail]>]

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 <[e-mail]> wrote:
> 
> 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].

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