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

[Eric Rescorla <[e-mail]>]

On Sat, May 6, 2017 at 2:27 PM, Russ Housley <[e-mail]> wrote:

> Eric:
>
> > 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?
>
> The text says that other curves are not deprecated, but it is encouraging
> support for X25519 and X448.  We know that there are some communities that
> are not ready to embrace the CFRG curves over the NIST curves.  So, I’d
> rather drop the second half of the sentence than change it to RFC 2119
> language.
>

OK.


> 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.
>
> RFC 7748 [CURVES] says:
>
>    Protocol designers using Diffie-Hellman over the curves defined in
>    this document must not assume "contributory behaviour".  Specially,
>    contributory behaviour means that both parties' private keys
>    contribute to the resulting shared key.  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.  This situation can be detected by checking for the all-
>    zero output, which implementations MAY do, as specified in Section 6.
>    However, a large number of existing implementations do not do this.
>
> We upgraded the MAY to a SHOULD.  We are being told that some
> implementations will not perform this check, so it seemed wrong to go all
> the way to MUST.
>

I'd like to push on this some, because I'm having trouble seeing why
different IETF protocols have different needs. When you say "you are being
told" is that an S/MIME specific point or merely the the text above?




>
> >    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?
>
> I suggest this wording:
>
>    … There is no security benefit to using a ukm value that is
>    longer than the key-encryption key that will be produced by
>    the KDF.
>

Hmm... I believe that this statement is true, but it also seems to be
incomplete. I may be reasoning about this incorrectly, but it seems
to me that the minimal security requirement is that the UKM be
unique, but that can be achieved with a value much smaller than
the KEK. For instance, it seems like if you have a 256-bit KEK,
then you would still be OK with a randomly-generated 128-bit
UKM. And if we're concerned about random collisions, then the
usefulness bound is actually min(|KEK|, |hash compression function size|).



> > 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.
>
> Section 2 already explains that the originator uses an ephemeral key pair.
>

Yes, but it doesn't have any normative language (see above on this topic).


> Appendix:
> > How many people have checked this ASN.1 module?
>
> At least two people have compiled it with different toolsets.
>
> > EDITORIAL
> > S 2.
> > Please put KEK in parentheses in your first use.
>
> In most places, it is spelled out.  I added (KEK) to the first place, and
> left it in the other places.


SGTM.



>
> > S 3.
> > It would be easier to put this above the key derivation stage.
>
> I followed the outline used in other specifications.  I figures that the
> implementers were fine with the previous outline.
>
> > Please provide some sort of reference or cross-reference for
> > “kari"
>
> The kari is defined in [CMS] as one of the choices in RecipientInfo.  I
> think that is clear from the sentence.
>

Yeah, I didn't find it super-clear. How about quoting it.

-Ekr


>
> >    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.
>
> Yes.  I rewrote the text in Section 3.2 to point back to the earlier text.
>
> > S 5.
> > This also seems to be largely duplicated. Can you refactor out
> > the common stuff.
>
> Because the content type is different, I think I already reduced it to the
> minimum.
>
> Russ
>
>