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

[Lucas Pardue <[e-mail]>]

Hi Ben,

Thanks for the review. I've created GitHub issue(s) to track each comment
on the QUIC WG repository, see the URL(s) in line.

On Wed, Jan 20, 2021 at 11:38 PM Benjamin Kaduk via Datatracker <
[e-mail]> wrote:

> 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.
>

https://github.com/quicwg/base-drafts/issues/4791


> 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.
>

https://github.com/quicwg/base-drafts/issues/4792


> 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.
>

https://github.com/quicwg/base-drafts/issues/4793


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

https://github.com/quicwg/base-drafts/issues/4794


> 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).
>

https://github.com/quicwg/base-drafts/issues/4795


> 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).
>

https://github.com/quicwg/base-drafts/issues/4796


Cheers
Lucas
On behalf of QUIC WG Chairs