I have updated my review results to: Ready


On Wed, Feb 27, 2019 at 7:29 AM Paul Wouters <paul@nohats.ca> wrote:
>
> On Tue, 26 Feb 2019, Christopher Wood wrote:
>
> > We just posted a revision to this document incorporating updates based
> > on your comments [1]. (Thanks again for your careful review!) If you
> > have time to check the diffs, we would greatly appreciate any more
> > feedback you may have.
>
> In general, the changes look good to me, although I still have
> reservations about some of the toy protocols mentioned which just gives
> them too much credibility. Thanks for adding openvpn. I see you did not
> add openconnect/anyconnect, and kept in curvecp/minimalt. Not my
> preference but if that's what the WG wants, I'm okay with it.
>
> Note that the latest openvpn version now uses AES-GCM per default, so
> perhaps you can excorsise blowfish from the document because Bruce
> said not to use it like over a decade ago. If you do mention blowfish,
> I think it should come with a big disclaimer to ensure people don't
> think IETF belives it's okay to use. And I don't think we need to
> point people to blowfish in the reference section.
> (see https://openvpn.net/community-downloads/)

Good to know. Consider blowfish gone.

> Just a few remaining questionts/comments left:
>
> >>>         WireGuard is designed to be entirely stateless, modulo the
> >>> CryptoKey
> >>>         routing table,
> >>>
> >>> Wouldn't you be able to say the same about the ESP SPD/SAD table? I'm
> >>> not sure
> >>> how different the ESP/wireguard statelessness is on a scale or truly
> >>> stateless
> >>> to NFS
>
> I'm still a bit concerned about the "designed to be entirely stateless".
> As I said before, that's more of a marketing gimmick than an actual
> technical property. Every VPN like protocol is stateless except for the
> state it needs (a rule database, counters, crypto keys). I would suggest
> removing this entire sentence:
>
>     WireGuard is designed to be entirely stateless, modulo the CryptoKey
>     routing table, which has size linear with the number of trusted
>     peers.

Good suggestion! I'll take it for the next update.

>
> >>> You do not mention mobility or session resumption for WireGuard. Since
> >>> it is
> >>> all about static inner IP addresses over arbitrary outer addresses, it
> >>> has
> >>> mobility. And I guess the 1RTT means it has session resumption?
>
> You did not add these to the wireguard feature list.

Whoops -- missed this. I'll add mobility (they call it roaming),
though there is no session resumption, if I understand correctly. I'll
work with Jason afterwards to make sure this section is accurate.

>
> >>> 5.1:
> >>>
> >>> Listed as mandatory feature is:
> >>>
> >>>    o  Public-key certificate based authentication.
> >>>
> >>> Yet you have mentioned that neither WireGuard or CurveCP can do
> >>> authentication
> >>> based on certificates?
> >>
> >> Indeed. This should read, “Public-key (raw or certificate) based
> >> authentication.”
>
> It seems you decided to completely remove the mandatory requirement?
> What that on purpose or by accident?

Intentional! We felt that unilateral responder authentication was more
fitting here.

Thanks again,
Chris