Subject: [Last-Call] Secdir last call review of draft-ietf-quic-manageability-14

From: Barry Leiba via Datatracker <[email-masked]>
Date: Tue, 01 Feb 2022 10:05:04 -0800

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

---

From: "Brian Trammell (IETF)" <[email-masked]>
Date: Wed, 9 Feb 2022 15:45:58 +0100

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 <[email-masked]> wrote:
> 
> Reviewer: Barry Leiba
> Review result: Has Nits
>