[Cfrg] review of draft-irtf-cfrg-hpke-05

Stephen Farrell <stephen.farrell@cs.tcd.ie> Mon, 10 August 2020 17:22 UTC

Return-Path: <stephen.farrell@cs.tcd.ie>
X-Original-To: cfrg@ietfa.amsl.com
Delivered-To: cfrg@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 4878A3A0B4F; Mon, 10 Aug 2020 10:22:29 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.999
X-Spam-Level:
X-Spam-Status: No, score=-1.999 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1024-bit key) header.d=cs.tcd.ie
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id MSI8nfWHwS4G; Mon, 10 Aug 2020 10:22:27 -0700 (PDT)
Received: from mercury.scss.tcd.ie (mercury.scss.tcd.ie [134.226.56.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 0EB213A0B56; Mon, 10 Aug 2020 10:22:26 -0700 (PDT)
Received: from localhost (localhost [127.0.0.1]) by mercury.scss.tcd.ie (Postfix) with ESMTP id 0B3A4BE64; Mon, 10 Aug 2020 18:22:24 +0100 (IST)
X-Virus-Scanned: Debian amavisd-new at scss.tcd.ie
Received: from mercury.scss.tcd.ie ([127.0.0.1]) by localhost (mercury.scss.tcd.ie [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 8v-0bBsjHj0S; Mon, 10 Aug 2020 18:22:22 +0100 (IST)
Received: from [10.244.2.119] (95-45-153-252-dynamic.agg2.phb.bdt-fng.eircom.net [95.45.153.252]) by mercury.scss.tcd.ie (Postfix) with ESMTPSA id F3EABBE53; Mon, 10 Aug 2020 18:22:21 +0100 (IST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=cs.tcd.ie; s=mail; t=1597080142; bh=K4oP27P2p/TmEnItQUjb8sSAReCpZGkpbBpAPiIHFj8=; h=To:From:Subject:Date:From; b=CscxFPzznmaDEsFpBuGIBD/qIDB5oaQPLcqbs7Q3kwkTJKotafOT5QzLT8HqZBvTl od6TUPFxnHKOOM9D5DEN9/ndjBdfwCXZxez0CygXX2BgKKVnbpuWtY8JlsBnUX0KJy 1YKxYjpCdzjJFN8In8KfiBvoc5TkPGVKfqm6Pfrk=
To: "crypto-panel@irtf.org" <crypto-panel@irtf.org>, "cfrg@irtf.org" <Cfrg@irtf.org>
From: Stephen Farrell <stephen.farrell@cs.tcd.ie>
Autocrypt: addr=stephen.farrell@cs.tcd.ie; prefer-encrypt=mutual; keydata= mQINBFo9UDIBEADUH4ZPcUnX5WWRWO4kEkHea5Y5eEvZjSwe/YA+G0nrTuOU9nemCP5PMvmh 5Cg8gBTyWyN4Z2+O25p9Tja5zUb+vPMWYvOtokRrp46yhFZOmiS5b6kTq0IqYzsEv5HI58S+ QtaFq978CRa4xH9Gi9u4yzUmT03QNIGDXE37honcAM4MOEtEgvw4fVhVWJuyy3w//0F2tzKr EMjmL5VGuD/Q9+G/7abuXiYNNd9ZFjv4625AUWwy+pAh4EKzS1FE7BOZp9daMu9MUQmDqtZU bUv0Q+DnQAB/4tNncejJPz0p2z3MWCp5iSwHiQvytYgatMp34a50l6CWqa13n6vY8VcPlIqO Vz+7L+WiVfxLbeVqBwV+4uL9to9zLF9IyUvl94lCxpscR2kgRgpM6A5LylRDkR6E0oudFnJg b097ZaNyuY1ETghVB5Uir1GCYChs8NUNumTHXiOkuzk+Gs4DAHx/a78YxBolKHi+esLH8r2k 4LyM2lp5FmBKjG7cGcpBGmWavACYEa7rwAadg4uBx9SHMV5i33vDXQUZcmW0vslQ2Is02NMK 7uB7E7HlVE1IM1zNkVTYYGkKreU8DVQu8qNOtPVE/CdaCJ/pbXoYeHz2B1Nvbl9tlyWxn5Xi HzFPJleXc0ksb9SkJokAfwTSZzTxeQPER8la5lsEEPbU/cDTcwARAQABtDJTdGVwaGVuIEZh cnJlbGwgKDIwMTcpIDxzdGVwaGVuLmZhcnJlbGxAY3MudGNkLmllPokCQAQTAQgAKgIbAwUJ CZQmAAULCQgHAgYVCAkKCwIEFgIDAQIeAQIXgAUCWj6jdwIZAQAKCRBasvrxexcr6o7QD/9m x9DPJetmW794RXmNTrbTJ44zc/tJbcLdRBh0KBn9OW/EaAqjDmgNJeCMyJTKr1ywaps8HGUN hLEVkc14NUpgi4/Zkrbi3DmTp25OHj6wXBS5qVMyVynTMEIjOfeFFyxG+48od+Xn7qg6LT7G rHeNf+z/r0v9+8eZ1Ip63kshQDGhhpmRMKu4Ws9ZvTW2ACXkkTFaSGYJj3yIP4R6IgwBYGMz DXFX6nS4LA1s3pcPNxOgrvCyb60AiJZTLcOk/rRrpZtXB1XQc23ZZmrlTkl2HaThL6w3YKdi Ti1NbuMeOxZqtXcUshII45sANm4HuWNTiRh93Bn5bN6ddjgsaXEZBKUBuUaPBl7gQiQJcAlS 3MmGgVS4ZoX8+VaPGpXdQVFyBMRFlOKOC5XJESt7wY0RE2C8PFm+5eywSO/P1fkl9whkMgml 3OEuIQiP2ehRt/HVLMHkoM9CPQ7t6UwdrXrvX+vBZykav8x9U9M6KTgfsXytxUl6Vx5lPMLi 2/Jrsz6Mzh/IVZa3xjhq1OLFSI/tT2ji4FkJDQbO+yYUDhcuqfakDmtWLMxecZsY6O58A/95 8Qni6Xeq+Nh7zJ7wNcQOMoDGj+24di2TX1cKLzdDMWFaWzlNP5dB5VMwS9Wqj1Z6TzKjGjru q8soqohwb2CK9B3wzFg0Bs1iBI+2RuFnxLkCDQRaPVAyARAA+g3R0HzGr/Dl34Y07XqGqzq5 SU0nXIu9u8Ynsxj7gR5qb3HgUWYEWrHW2jHOByXnvkffucf5yzwrsvw8Q8iI8CFHiTYHPpey 4yPVn6R0w/FOMcY70eTIu/k6EEFDlDbs09DtKcrsT9bmN0XoRxITlXwWTufYqUnmS+YkAuk+ TLCtUin7OdaS2uU6Ata3PLQSeM2ZsUQMmYmHPwB9rmf+q2I005AJ9Q1SPQ2KNg/8xOGxo13S VuaSqYRQdpV93RuCOzg4vuXtR+gP0KQrus/P2ZCEPvU9cXF/2MIhXgOz207lv3iE2zGyNXld /n8spvWk+0bH5Zqd9Wcba/rGcBhmX9NKKDARZqjkv/zVEP1X97w1HsNYeUFNcg2lk9zQKb4v l1jx/Uz8ukzH2QNhU4R39dbF/4AwWuSVkGW6bTxHJqGs6YimbfdQqxTzmqFwz3JP0OtXX5q/ 6D4pHwcmJwEiDNzsBLl6skPSQ0Xyq3pua/qAP8MVm+YxCxJQITqZ8qjDLzoe7s9X6FLLC/DA L9kxl5saVSfDbuI3usH/emdtn0NA9/M7nfgih92zD92sl1yQXHT6BDa8xW1j+RU4P+E0wyd7 zgB2UeYgrp2IIcfG+xX2uFG5MJQ/nYfBoiALb0+dQHNHDtFnNGY3Oe8z1M9c5aDG3/s29QbJ +w7hEKKo9YMAEQEAAYkCJQQYAQgADwUCWj1QMgIbDAUJCZQmAAAKCRBasvrxexcr6qwvD/9b Rek3kfN8Q+jGrKl8qwY8HC5s4mhdDJZI/JP2FImf5J2+d5/e8UJ4fcsT79E0/FqX3Z9wZr6h sofPqLh1/YzDsYkZDHTYSGrlWGP/I5kXwUmFnBZHzM3WGrL3S7ZmCYMdudhykxXXjq7M6Do1 oxM8JofrXGtwBTLv5wfvvygJouVCVe87Ge7mCeY5vey1eUi4zSSF1zPpR6gg64w2g4TXM5qt SwkZVOv1g475LsGlYWRuJV8TA67yp1zJI7HkNqCo8KyHX0DPOh9c+Sd9ZX4aqKfqH9HIpnCL AYEgj7vofeix7gM3kQQmwynqq32bQGQBrKJEYp2vfeO30VsVx4dzuuiC5lyjUccVmw5D72J0 FlGrfEm0kw6D1qwyBg0SAMqamKN6XDdjhNAtXIaoA2UMZK/vZGGUKbqTgDdk0fnzOyb2zvXK CiPFKqIPAqKaDHg0JHdGI3KpQdRNLLzgx083EqEc6IAwWA6jSz+6lZDV6XDgF0lYqAYIkg3+ 6OUXUv6plMlwSHquiOc/MQXHfgUP5//Ra5JuiuyCj954FD+MBKIj8eWROfnzyEnBplVHGSDI ZLzL3pvV14dcsoajdeIH45i8DxnVm64BvEFHtLNlnliMrLOrk4shfmWyUqNlzilXN2BTFVFH 4MrnagFdcFnWYp1JPh96ZKjiqBwMv/H0kw==
Message-ID: <4025d64f-9d7d-5474-b3ce-d2829d3a0df1@cs.tcd.ie>
Date: Mon, 10 Aug 2020 18:22:21 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0
MIME-Version: 1.0
Content-Type: multipart/signed; micalg="pgp-sha512"; protocol="application/pgp-signature"; boundary="rwrFbMv07K0BW3tWKQ5DmskVL8TOuPiX2"
Archived-At: <https://mailarchive.ietf.org/arch/msg/cfrg/7zhOHPFkCyZC00xLZnsEBT3o6ZU>
Subject: [Cfrg] review of draft-irtf-cfrg-hpke-05
X-BeenThere: cfrg@irtf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Crypto Forum Research Group <cfrg.irtf.org>
List-Unsubscribe: <https://www.irtf.org/mailman/options/cfrg>, <mailto:cfrg-request@irtf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/cfrg/>
List-Post: <mailto:cfrg@irtf.org>
List-Help: <mailto:cfrg-request@irtf.org?subject=help>
List-Subscribe: <https://www.irtf.org/mailman/listinfo/cfrg>, <mailto:cfrg-request@irtf.org?subject=subscribe>
X-List-Received-Date: Mon, 10 Aug 2020 17:22:29 -0000

Hiya,

I implemented draft -02 and will be doing -05 shortly.  I
didn't yet do that so haven't verified the test vectors.
Will follow up if any issues arise.

Overall this is ready to proceed IMO. If my entire review
were ignored all would still be ok.  I do have one thing I
think the RG might want to discuss, a few minor comments
and some very ignorable nits.

Cheers,
S.


The thing to ponder relates to the IANA considerations.
Why not add a "recommended" column a la TLS.  The RG can
hand over responsibility to some DEs appointed by the IESG
and call for the same setup as TLS.  (I.e. other than
the initial values recommended == yes requires IETF
standards track, otherwise spec required.) If we don't
do that then applications using HPKE will always each
need to say which suites are MUSTs, leading to IMO mostly
pointless variation and possibly worsening interop if
libraries implement disjoint sets of suites.

Minor comments:

- Is random() needed? it only seems to be used in the
context of "DeriveKeyPair(random(Nsk))"

- Do we really need to s/HPKE-05/RFCXXXX/ later? Why not
just change to "HPKE-first-rfc" once the RG are done with
the document? (There can be delays @ IRSG and subsequently
that I'd prefer not have to affect interop.)

- p7 & elsewhere: refers to "HPKE-05 " - that space seems
like a bad idea.  I missed it until I got to p27.  And
it's not consistently present and the text is ambiguous as
to whether the replacement ought be "RFCXXXX"
or "RFCXXXX "

- It strikes me as odd to not have any identifier for the
public key being used but to have one for the PSK being
used. If allowing the application to handle that for
public keys works, why won't that work for PSKs?

- DeriveKeyPair() for NIST curves requires the HPKE code
to know the order of the various NIST curves. That seems
like the kind of thing where bugs might arise that
wouldn't be noticed much. I also don't think many crypto
APIs would provide a ``get_order_as_integer(curve_id)``
function, so using the wrong value would seem not
unlikely. I'd say maybe add the values to use here
somewhere for each curve. If not in 7.1.2 then put in a
forward reference. Perhaps also say what can happen if the
wrong value for "order" is used.

- p29: I'm not sure why section 8.8 is useful here. I think
it'd be better deleted TBH - it might muddy the definition
of "signature" for some people and doesn't seem to add much
as-is.

nits:

- p4: PGP and fiveG don't depend on HPKE, whereas msl and
esni do. Maybe worth saying that.

- p5: I2OSP and OS2IP aren't expanded here - be no harm to
do so

- p6: Open as the opposite of Seal, doesn't seem like the
best choice - there are too many functions called open()
in too many contexts. Unseal would be better IMO.

- p5: DeriveKeyPair was added after discussion in github
(and maybe on the MLS list) but I don't recall any
disussion of that on the CFRG list at all. Was there any?
Adding this seems fine to me if MLS wants it and the
definition is also almost fine (see above) so this is just
a process nit.

- p5 & elsewhere: "fixed-length" is used in various places
where it's not quite true - the various lengths are fixed
only after you pick a ciphersuite - so people's code has
to support different sizes (if they support >1 suite)