Re: Secdir last call review of draft-ietf-quic-manageability-14

["Brian Trammell (IETF)" <[e-mail]>]

hi Barry,

Many thanks for the review! We've incorporated the editorial changes in https://github.com/quicwg/ops-drafts/pull/450

Cheers,

Brian

> On 1 Feb 2022, at 19:05, Barry Leiba via Datatracker <[e-mail]> wrote:
> 
> Reviewer: Barry Leiba
> Review result: Has Nits
> 
> Thanks for a clear, well-written document that was very easy to read.  From a
> security point of view, there’s quite a bit of explanation about how encryption
> is negotiated, the different contexts before and after handshakes, and the
> like, and how all that makes any tampering discoverable.  I appreciate having
> that explanation, and it all looks solid to me.
> 
> As I went through it, I thought about whether QUIC-INVARIANTS, RFC 8999, should
> be a normative reference: it’s cited several times, and in places where it
> seems that the information might be critical to fully understanding this
> document.  As I looked back and forth between this document and that one, I
> decided that it’s correctly classified as informative (the normative
> information is in QUIC-TRANSPORT), but I can see how another reviewer might
> fall on the other side of that.  Just something to be aware of.
> 
> I only have a few minor editorial suggestions:
> 
> — Section 2.4 —
> 
>   The content of Initial packets is encrypted using Initial Secrets,
>   which are derived from a per-version constant and the client's
>   destination connection ID; they are therefore observable by any on-
>   path device that knows the per-version constant and considered
>   visible in this illustration.  The content of QUIC Handshake packets
>   are encrypted using keys established during the initial handshake
>   exchange, and are therefore not visible.
> 
> I found this a little hard to read, as I initially attached “considered
> visible” to the on-path device and thought the word “is” was missing.  I now
> realize that it’s meant to connect to “they”, but then *that* makes me realize
> that “they” is wrong because it’s supposed to refer to the bit that’s
> encrypted, which is “the content of Initial packets”.  While “packets” is
> plural, “the content” is singular and is used singularly above (“is
> encrypted”).  Whoo.
> 
> I suggest splitting it into two sentences, rather than using the semicolon,
> handling it this way, and making sure to refer to the content as singular:
> 
> NEW
>   The content of Initial packets is encrypted using Initial Secrets,
>   which are derived from a per-version constant and the client's
>   destination connection ID. That content is therefore observable by
>   any on-path device that knows the per-version constant and is
>   considered visible in this illustration.  The content of QUIC
>   Handshake packets is encrypted using keys established during the
>   initial handshake exchange, and is therefore not visible.
> END
> 
> — Section 2.6 —
> 
>   This
>   allows rebinding of a connection after one of the endpoints
>   experienced an address change - usually the client.
> 
> Nit, to take or leave: “usually the client” is, strictly speaking, misplaced:
> “This allows rebinding of a connection after one of the endpoints - usually the
> client - has experienced an address change.”
> 
> — Section 3.4.1 —
> 
>   Further,
>   the client's Initial packet(s) may contain other frames, so the first
>   bytes of each frame need to be checked to identify the frame type,
>   and if needed skipped over it.
> 
> The last phrase isn’t well formed grammatically.  Are you talking about
> identifying frame types and skipping over frames that you’re not interested in?
> If so, maybe this works?:
> 
> NEW
>   Further,
>   the client's Initial packet(s) may contain other frames, so the first
>   bytes of each frame need to be checked to identify the frame type and
>   determine which frames can be skipped over.
> END
> 
>