Re: [Cfrg] comments on draft-irtf-cfrg-hpke

[Christopher Wood <[e-mail]>]

Hi Riad,

Thanks for the feedback! Please see inline below for followup comments.

On Tue, Jun 2, 2020, at 1:08 PM, Riad S. Wahby wrote:
> 
> - Section 3: encode_big_endian seems to be the same as I2OSP from RFC8017.
>   Would it make sense to reference the existing primitive instead, or at
>   least to note that the two are identical? 

Good point. We should probably use I2OSP, as that's what other CFRG documents use:

   https://github.com/cfrg/draft-irtf-cfrg-hpke/issues/104

> - Section 4: the definitions of AuthEncap and AuthDecap contain words to the
>   effect, 'the KEM shared secret key is known only to the holder of the
>   private key "skS".' It would be more accurate to say , 'the KEM shared
>   secret key was generated by the holder of the private key "skS"'.

That's a fine clarification:

   https://github.com/cfrg/draft-irtf-cfrg-hpke/issues/105

> - Section 4 and 4.1: it's not totally obvious to me that this is necessary,
>   but might it make sense to note that Unmarshal can fail?

Yes, definitely, that's a great observation:

   https://github.com/cfrg/draft-irtf-cfrg-hpke/issues/106

> - Section 4.1: this may be paranoia, but it would be slightly nicer to include
>   the DH group name in the label arguments of LabeledExtract and LabeledExpand
>   to ensure that invocations from different DHKEM instantiations are orthogonal.

That seems reasonable to me. We went through a couple iterations on domain separation, and it would be a shame to have this lingering:

   https://github.com/cfrg/draft-irtf-cfrg-hpke/issues/108

> - Section 5.1.3: it would be nice to include a reference or citation for
>   unknown key share attacks.

I concur. I'll fold it in an issue addressing editorial changes:

   https://github.com/cfrg/draft-irtf-cfrg-hpke/issues/107

> - Section 5.2: is there a reason to put the word "amortize" in quotes? 
>
> - Section 7.1.2: it might be worth mentioning here that [keyagreement] also
>   includes checking that the public key is not the identity point.

Agreed. I'll fold these into #107.

> - Section 7.1.2: is there a reason to recommend either checking for a nonzero 
>   scalar or checking for a non-identity DH output? Checking the latter covers
>   the former and also covers the check from my prior comment. Moreover, it is
>   not clear to me that checking the scalar is useful for the recipient, since
>   this is essentially just checking that their long-term secret is nonzero.

We could certainly clarify what's required here. This check was meant to be before senders and receivers invoke the DH() function. It could be easier to just check the output. I'll address this in #107. 

> - Section 8.1: the sentence "In particular, the KDFs and DH groups..." might
>   want to clarify that this statement is true only when these primitives are
>   used as specified. The concern is that HKDF is only indifferentiable under
>   some restrictions on salt length (for reasons noted in Section 8.3).

Agreed. I'll fold this into #107 as well.
   
> - Section 8.4: it is not quite true that using a PSK longer than Nh bytes 
>   has no effect on security, since in practice there is no guarantee that
>   each byte of PSK contributes 8 bits of entropy. Making this distinction
>   seems worthwhile, lest people decide to use only Nh bytes with lots of
>   structure (e.g., ASCII text).
>
>   (I am aware that the next paragraph discusses low-entropy passwords, 
>   but I am talking about an intermediate regime. For example, consider 
>   the case of a diceware passphrase: each word contributes only 10--15
>   bits of entropy, but is ~6 bytes long. Using 12 words is certainly
>   better than using 6, even though 6 words already has length greater
>   than Nh for SHA-256.)

Hmm, fair point. We can clarify (in #107).

> - Section 8.4: would it make sense to point to PBKDF / bcrypt / scrypt here?
>   Or is the idea to discourage use of low-entropy passwords entirely?  (But
>   one might worry that folks will use low-entropy passwords anyway...)

I don't see why not! I'll add the reference as part of #107.

Thanks again for the thoughtful feedback and careful review. I may ask you to review these changes before they land. :-)

Best,
Chris