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

["Brockhaus, Hendrik" <[e-mail]>]

Roman

Many thanks for your feedback.

I am confused regarding your comments on field names vs. type names.
If you look for example at the definition of KeyAgreeRecipientInfo (RFC 5652 Section 6.2.2):
      KeyAgreeRecipientInfo ::= SEQUENCE {
        version CMSVersion,  -- always set to 3
        originator [0] EXPLICIT OriginatorIdentifierOrKey,
        ukm [1] EXPLICIT UserKeyingMaterial OPTIONAL,
        keyEncryptionAlgorithm KeyEncryptionAlgorithmIdentifier,
        recipientEncryptedKeys RecipientEncryptedKeys }
My reading is that keyEncryptionAlgorithm is the field name and KeyEncryptionAlgorithmIdentifierits the type. Am I wrong?

Please find further comments below inline.
Feedback on you comment regarding Section 7 and Section 9 will come next week.

> -----Ursprüngliche Nachricht-----
> Von: Spasm <[e-mail]> Im Auftrag von Roman Danyliw
> 
> 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.

I like this proposal and will update the draft accordingly. I also change the paragraph on digest value locations accordingly. I did the same in other sections, where appropriate.

> 
> ** 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.

Good point. Doth this proposal solved your issue?

Old:
   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 3.3 for certificates signed using EdDSA.

New:
   Note: When confirming certificates where the AlgorithmIdentifier
   of the certificate signature does not clearly imply a specific hash
   algorithm and no specific hash algorithm is defined for usage with
   this signature algorithm, as done in Section 3.3 for EdDSA, the hash
   algorithm needs to indicated in the hashAlgs field of the certConf
   message as specified in CMP Updates Section 2.9
   [I-D.ietf-lamps-cmp-updates].

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

Yes, thanks

> 
> ** 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:

I will update the draft accordingly

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

Yes, thanks

> 
> ** 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".

Section 4.1 addresses key agreement. Therefore RFC 5652 Section 6.2.2 applies.

RFC 5652 Section 6.2.2 defines:
      KeyAgreeRecipientInfo ::= SEQUENCE {
        version CMSVersion,  -- always set to 3
        originator [0] EXPLICIT OriginatorIdentifierOrKey,
        ukm [1] EXPLICIT UserKeyingMaterial OPTIONAL,
        keyEncryptionAlgorithm KeyEncryptionAlgorithmIdentifier,
        recipientEncryptedKeys RecipientEncryptedKeys }

Isn't keyEncryptionAlgorithm the field name?

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

Yes, it should be "field".

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

I propose to use '-' instead of a space in case such structure is required. While switching to the bulleted lists I removed the nested structure.

> 
> ** 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?

Thanks for spotting this. This points at a confusion on my side. This needs to be changed in Lightweight CMP Profile Section 4.1.6.1 as well.

Looking at RFC 5753 Section 3.1.1 I understand now, that the key agreement algorithm OID is located in keyEncryptionAlgorithm and the key-wrap algorithm OID is located in the parameters field of the key agreement algorithm OID, e.g., a PARAMS field (see RFC 5753 A.3).

RFC 5753 Section 3.1.1
   -  keyEncryptionAlgorithm MUST contain the object identifier of the
      key-encryption algorithm, which in this case is a key agreement
      algorithm (see Section 7.1.4).  The parameters field contains
      KeyWrapAlgorithm.  The KeyWrapAlgorithm is the algorithm
      identifier that indicates the symmetric encryption algorithm used
      to encrypt the content-encryption key (CEK) with the key-
      encryption key (KEK) and any associated parameters (see Section
      7.1.5).  Algorithm requirements are found in Section 8.

RFC 5753 Appendix A.2
kaa-dhSinglePass-stdDH-sha224kdf-scheme KEY-AGREE ::= {
  IDENTIFIER dhSinglePass-stdDH-sha224kdf-scheme
  PARAMS TYPE KeyWrapAlgorithm ARE required
  UKM -- TYPE unencoded data -- ARE preferredPresent
  SMIME-CAPS cap-kaa-dhSinglePass-stdDH-sha224kdf-scheme
}

> 
> ** 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.

RFC 5652 Section 6.2.2 defines
      RecipientEncryptedKey ::= SEQUENCE {
        rid KeyAgreeRecipientIdentifier,
        encryptedKey EncryptedKey }

Isn't encryptedKey the field name?

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

I propose updating Section 4.1 as follows:
Old:
   Key agreement algorithm identifiers are located in the EnvelopedData
   RecipientInfos KeyAgreeRecipientInfo keyEncryptionAlgorithm fields.

   Key encryption algorithm identifiers are located in the EnvelopedData
   RecipientInfos KeyAgreeRecipientInfo keyEncryptionAlgorithm field.

   Wrapped content-encryption keys are located in the EnvelopedData
   RecipientInfos KeyAgreeRecipientInfo RecipientEncryptedKeys
   encryptedKey field.

New:
Key agreement algorithm identifiers are located in:
* keyEncryptionAlgorithm field of KeyAgreeRecipientInfo

Key wrap algorithm identifiers are located in:
* KeyWrapAlgorithm parameters within keyEncryptionAlgorithm field of KeyAgreeRecipientInfo

Wrapped content-encryption keys are located in:
* encryptedKey field of RecipientEncryptedKeys

> 
> ** 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

RFC 5652 Section 6.2.1 defines:
      KeyTransRecipientInfo ::= SEQUENCE {
        version CMSVersion,  -- always set to 0 or 2
        rid RecipientIdentifier,
        keyEncryptionAlgorithm KeyEncryptionAlgorithmIdentifier,
        encryptedKey EncryptedKey }

Isn't keyEncryptionAlgorithm the field name?

> 
> ** 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.

RFC 5652 Section 6.2.1 defines:
      KeyTransRecipientInfo ::= SEQUENCE {
        version CMSVersion,  -- always set to 0 or 2
        rid RecipientIdentifier,
        keyEncryptionAlgorithm KeyEncryptionAlgorithmIdentifier,
        encryptedKey EncryptedKey }

Isn't encryptedKey the field name?

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

See above

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

See above

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

RFC 5652 Section 6.2.4 defines:
      PasswordRecipientInfo ::= SEQUENCE {
        version CMSVersion,   -- Always set to 0
        keyDerivationAlgorithm [0] KeyDerivationAlgorithmIdentifier
                                     OPTIONAL,
        keyEncryptionAlgorithm KeyEncryptionAlgorithmIdentifier,
        encryptedKey EncryptedKey }

Isn't keyDerivationAlgorithm the field name?

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

Yes, thanks

> 
> ** Section 5.  As before, s/ contentEncryptionAlgorithm
> /ContentEncryptionAlgorithmIdentifier/
> 
> ** Section 6. Editorial.  s/Section 7,RFC/Section 7, RFC/

Yes, thanks

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

Yes, thanks

> 
> ** 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.

I will move the reference up


Hendrik