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 <noreply@xxxxxxxx> 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 > > -- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
