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

[Roman Danyliw <[e-mail]>]

Hi Hendrik!

> -----Original Message-----
> From: Brockhaus, Hendrik <[e-mail]>
> Sent: Wednesday, October 27, 2021 2:26 AM
> To: Roman Danyliw <[e-mail]>
> Cc: LAMPS WG <[e-mail]>
> Subject: AW: AD review of draft-ietf-lamps-cmp-algorithms-07
> 
> Roman
> 
> Many thanks for your valuable and detailed review.
> We will come back to each of your comments in detail later.

Sounds like a plan.

> > ** 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:
> I like you suggestion using a bulleted list for the algorithm usage.
> 
> > -- I'm mentioning it here but it applies to the other uses, using
> > spaces to convey the nested CMS structure is confusing.
> I was thinking about a better delimiter to convey nested structures. What do
> you think about '-' or '/'?

I have a weak preference for "-" but can live with anything that isn't a space.

Thanks,
Roman

> Hendrik
> 
> > Von: Spasm <[e-mail]> Im Auftrag von Roman Danyliw
> > Gesendet: Dienstag, 26. Oktober 2021 23:49
> >
> > 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
> >
> >
> > _______________________________________________
> > Spasm mailing list
> > [e-mail]
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.
> > ietf
> >
> .org%2Fmailman%2Flistinfo%2Fspasm&amp;data=04%7C01%7Chendrik.brockh
> a
> >
> us%40siemens.com%7C2394a26fac8b4a6b67b508d998ca7af2%7C38ae3bcd95
> 7
> >
> 94fd4addab42e1495d55a%7C1%7C0%7C637708817695901864%7CUnknown
> %7
> >
> CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLC
> J
> >
> XVCI6Mn0%3D%7C1000&amp;sdata=5aRCZDwb1xqkuvlnErpd2w07Bktd3LjpA7
> IA
> > xZClxLc%3D&amp;reserved=0