[Curdle] AD Review: draft-ietf-curdle-cms-ecdh-new-curves-04.txt

[Eric Rescorla <[e-mail]>]

OVERALL
I see a bunch of quasi-normative text here, e.g.,

   [CURVES].  Those other curves are not deprecated, but support for
   curve25519 and curve448 is encouraged.

Can you use RFC 2119 language here?


TECHNICAL
S 2.
   X25519 is described in Section 6.1 of [CURVES], and X448 is described
   in Section 6.2 of [CURVES].  Since curve25519 and curve448 have
   cofactors of 8 and 4, respectively, an input point of small order
   will eliminate any contribution from the other party’s private key.
   As described in Section 7 of [CURVES], implementations SHOULD detect
   this situation by checking for the all-zero output.

Why are you not requiring this check? SSH and TLS both do.


   The ECC-CMS-SharedInfo keyInfo field contains the object identifier
   of the key-encryption algorithm and associated parameters.  This
   algorithm will be used to wrap the content-encryption key.  For
   example, the AES Key Wrap algorithm [AESKW] does not need parameters,
   so the algorithm identifier parameters are absent.

It's important that this be clear. Is it required to be absent? Must
I check?


   The ECC-CMS-SharedInfo entityUInfo field optionally contains
   additional keying material supplied by the sending agent.  Note that
   [CMS] requires implementations to accept a KeyAgreeRecipientInfo
   SEQUENCE that includes the ukm field.  If the ukm field is present,
   the ukm is placed in the entityUInfo field.  The ukm value need not
   be longer than the key-encryption key that will be produced by the
   KDF.

Need not? Please clarify what the purpose is here. It seems like
it's to generate a unique KEK. In that case, the security bounds
are what, uniqueness?


S 2.2.
  if ukm is provided, then salt = ukm, else salt = zero

zero in this case is "all zeros"? In any case, HKDF already
knows how to deal with salt not provided, to that would be
easier to specify.


S 8.
Don't forget to fill in these values. I assume you will do
so once IANA registers your code points?

S 9.
This section is kinda diffident about whether the sender's
ephemeral is truly ephemeral. Is this a MUST? If so, please
say so.

Appendix:
How many people have checked this ASN.1 module?


EDITORIAL
S 2.
Please put KEK in parentheses in your first use.

S 3.
It would be easier to put this above the key derivation stage.

Please provide some sort of reference or cross-reference for
"kari"

   KeyAgreeRecipientInfo ukm is optional.  Note that [CMS] requires
   implementations to accept a KeyAgreeRecipientInfo SEQUENCE that
   includes the ukm field.  If present, the ukm is placed in the
   entityUInfo field of the ECC-CMS-SharedInfo as input to the KDF.  The
   ukm value need not be longer than the key-encryption key produced by
   the KDF.

This seems to be duplicated.


S 5.
This also seems to be largely duplicated. Can you refactor out
the common stuff.

-Ekr