[lamps] AD review of draft-ietf-lamps-cmp-algorithms-07

[Roman Danyliw <[e-mail]>]

Hi!

I did an AD review of draft-ietf-lamps-cmp-algorithms-07 with my comments below.  Thanks for this work to evolve CMP.

** Section 2, 3, etc.  Editorial. When listing the places where digest and signature algorithm identifies are found, consider if enumerating them as a bulleted list might be easier to read.  For example:

OLD
   Digest algorithm identifiers are located in the hashAlg field of
   OOBCertHash, the owf field of Challenge, PBMParameter, CertStatus,
   and DHBMParameter, and the digestAlgorithms field of SignedData and
   the digestAlgorithm field of SignerInfo.

NEW
Digest algorithm identifiers are located in:
* hashAlg field of OOBCertHash, 
* owf field of Challenge, PBMParameter, CertStatus, and DHBMParameter 
* digestAlgorithms field of SignedData
* digestAlgorithm field of SignerInfo.

** Section 2.
Note: Specific conventions are needed for CertStatus content in
   certConf messages when confirming certificates where the
   AlgorithmIdentifier of the certificate signature does not clearly
   imply a specific hash algorithm.  In such cases the hash algorithm to
   use to build certHash should be specified, e.g., as done in
   Section 2.1 and Section 2.2 for certificates signed using EdDSA.

I'm confirming my understanding of the text.  Is the specific convention being described here the second sentence (i.e., explicitly specifying the hash algorithm)?  I ask because "conventions" was plural suggesting that there was more than one.

** Section 3.  Editorial.  s/Section 7.2,RFC/Section 7.2, RFC/

** Section 3.1.  Editorial.
OLD
The algorithm identifiers for RSASAA-PSS signatures used with SHA2
   message digest algorithms is identified by the following OID:

NEW
The algorithm identifier for RSASAA-PSS signatures used with SHA2
   message digest algorithms is identified by the following OID:

** Section 3.3.  Editorial. s/signature algorithm Ed25519 MUST/signature algorithm Ed25519 that MUST/

** Section 4.1.

Key agreement algorithm identifiers are located in the EnvelopedData
   RecipientInfos KeyAgreeRecipientInfo keyEncryptionAlgorithm fields.

-- "keyEncryptionAlgorithm" is the type, the field name is "KeyEncryptionAlgorithmIdentifier" per Section 6.2.1 of RFC5652.  It should likely read "... KeyAgreeRecipientInfo KeyEncryptionAlgorithmIdentifier".

-- why is "fields" plural? Isn't the cardinality here a single field?

-- I'm mentioning it here but it applies to the other uses, using spaces to convey the nested CMS structure is confusing.

** Section 4.1. Checking my understanding, is it accurate that both the key agreement algorithm identifiers and the key encryption algorithm identifiers are going to the same place in EnvelopedData?

** Section 4.1.
   Wrapped content-encryption keys are located in the EnvelopedData
   RecipientInfos KeyAgreeRecipientInfo RecipientEncryptedKeys
   encryptedKey field.
  
-- "encryptedKey" is also a type.  It should likely be "EncryptedKey" per Section 6.2.2 of RFC5652.

-- My ASN.1 reading always needs help.  Should it be "... RecipientEncryptedKeys RecipientEncryptedKey EncryptedKey" or "RecipientEncryptedKeys EncryptedKey"?  Same thing would apply to "RecipientInfos" and "RecipientInfo". 

** Section 4.2.  
Key transport algorithm identifiers are located in the EnvelopedData
   RecipientInfos KeyTransRecipientInfo keyEncryptionAlgorithm field.

Similar to Section 4.1, keyEncryptionAlgorithm is the type.  It likely should be KeyEncryptionAlgorithmIdentifier per Section 6.2.1 of RFC5652

** Section 4.2
   Key transport encrypted content-encryption keys are located in the
   EnvelopedData RecipientInfos KeyTransRecipientInfo encryptedKey
   Field.

Similar to Section 4.1, encryptedKey is the type.  It likely should be EncryptedKey per Section 6.2.1 of RFC5652.

** Section 4.3.  As before, s/keyEncryptionAlgorithm/KeyEncryptionAlgorithmIdentifier/

** Section 4.3.  As before, s/encryptedKey/EncryptedKey/

** Section 4.4. As before, s/keyDerivationAlgorithm/KeyDerivationAlgorithmIdentifier/

** Section 5.  Editorial.  s/Section 7,RFC/Section 7, RFC/

** Section 5.  As before, s/ contentEncryptionAlgorithm /ContentEncryptionAlgorithmIdentifier/

** Section 6. Editorial.  s/Section 7,RFC/Section 7, RFC/

** Section 6.1.1. Editorial. s/andAlgorithm/and Algorithm/

** Section 7.  I would have benefit from a bit more clarity on the direction provided in this section. 

-- "The following criteria will help implementers choose appropriate algorithms for managing certificates"

The bulleted list seems like only part of the criteria.  As a non-exhaustive list, additional considerations might be the capabilities of the end-point (e.g., does the target deployment have hardware acceleration for some, are some algorithms simply out of reach due to available compute or expected software support); or perhaps local policy or compliance requirements?

-- "The cryptographic strength of the algorithm with the lowest security."

It seems like some words are missing here.  Is the intent here to guide the selection towards the algorithm that provides the minimum amount of strength relative to the needed security requirements?

** Section 7.1.  The text in this section seems it should explicit say that it is updating RFC4210.

OLD
The following table contains definitions of algorithms used within

NEW
The following table updates the definitions of algorithms used within

** Section 7.1.  If an algorithm is listed in the "Change from 4210" column, should it be considered the equivalent of being added to the "other" column (per Appendix D.2 of RFC4210?  If so, please be explicit about that.

** Section 7.1.  Table 1.

-- What does it mean for PasswordBasedMac to be in both the Mandatory and Change-from-4210 column for the MSG_MAC_ALG row?  It was already the MTI in RFC4210.  Same question and situation with D-H for the PROT_ENC_ALG row.

-- Should 3-DES be considered "other" in MSG_MAC_ALG and PROT_SYM_ALG?

-- Editorial.  In PROT_SYM_ALG and SYM_PENC_ALG, the "3-DES (3-key-EDE), CBC Mode" should read "3-DES (3-key-EDE, CBC mode)" per the text in RFC4210.

** Section 9.  Can a reference be added for the theoretical weakness in PasswordBasedMac?

** Section 9.  Editorially, the first paragraph ("RFC 4210 Appendix D.2 ...) and the two paragraphs of "In Section 7 ..." and "To keep the list ...", seem to be saying the same thing.  Recommend a merge.

** Section 9.
In such systems the weakened algorithms should
   be disabled from further use.  

Can this document make stronger recommendations about deprecating certain algorithms such as 3-DES SHA-1 with normative language rather than keeping it in the "other" category in the profiles?

** Normative References.  [I-D.ietf-lamps-lightweight-cmp-profile] should be a normative reference given the use of normative language around it in Section 7.2.

Regards,
Roman