Benjamin Kaduk's No Objection on draft-ietf-quic-qpack-20: (with COMMENT)

[Benjamin Kaduk via Datatracker <[e-mail]>]

Benjamin Kaduk has entered the following ballot position for
draft-ietf-quic-qpack-20: No Objection

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-quic-qpack/



----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Thanks for another well-written document!

I put up some editorial suggestions at https://github.com/quicwg/base-drafts/pull/4789

Section 2.1.1.1

Figure 1 might benefit from an explicit indication of which direction is
higher (or lower) absolute index.

Section 7.1.2

The mitigation technique of segregating the dynamic table by entity
constructing the message seems to inherently require the encoder to have
direct knowledge of the entity on whose behalf it is constructing a
message.  For the other mitigation technique we present (of always using
string literals for sensitive values), we include the 'N' bit to allow
this attribute to be propagated through intermediaries.  However, I
think that in scenarios where multiple intermediaries are involved, in
the later steps in the intermediary chain the encoder will not
necessarily have knowledge of which entity created a given message, and
thus could inadvertently merge compression contexts that the original
encoder had specifically kept separate.  The preconditions necessary for
this to enable an attack seem rare, with one of the originating entities
having access to observe the transport layer in a location several hops
removed, so it doesn't really seem worth attempting to add a technical
mitigation.  It would probably be worth documenting the risk, though.

Section 8.1

(nit) In the prose we refer to the setting names with a "SETTINGS_" prefix.
Blindly adding that to the table looks like it would blow out the column
count for the plaintext output, though, so I didn't put that in my
editorial PR.

Appendix A

(nit) At least the plaintext output might benefit from a disclaimer
about line wraps in the 'Value' column being display artifacts.

Appendix B

I worked through the examples.  The presentation format is quite nice, and
I appreciate all the detailed breakdowns!

However, we show the dynamic table as being 1-indexed, but I'm pretty
sure the prose says it should be 0-indexed.  We do it consistently, at
least, and toss some extra '1's into the math to make the numbers work
out, but since the static table is by definition 0-indexed, it's a bit
weird to show the dynamic table as 1-indexed.

Additionally, I think that B.5 is an exception to the "we do it
consistently" -- while the 81... dynamic insert with relative index 1
does refer to the indicated custom-key field, that would be absolute
index 3 in the 1-indexed presentation we give (though it would be
absolute index 2 if 0-indexed, if I'm getting this right).

Appendix C

It might be worth a brief mention of the API contracts for (e.g.) the
encode*() functions.  The second argument of encodeInteger() as "the
byte value used to fill in the bits of the first byte that are not
consumed by the trailing N-prefix integer" is particularly hard to infer
(if it's even the correct inference).