Re: [Last-Call] Artart last call review of draft-ietf-quic-version-negotiation-10

[David Schinazi <[e-mail]>]

Hi Tim, thank you for your review.
Responses inline.
David

On Tue, Oct 4, 2022 at 2:31 PM Tim Bray via Datatracker <[e-mail]>
wrote:

> Reviewer: Tim Bray
> Review result: Ready with Issues
>
> This is a clear, well-written document, with one exception noted below.
> Note
> that my understanding of QUIC is quite shallow and it's possible I missed
> something in here that is obviously wrong or dangerous if you have a deeper
> understanding of QUIC.  I think the following are all nits except perhaps
> the
> extremely thin state of the Security Considerations section.
>
> Section 2.3 paragraph 4, this is arguably part of the definition of what
> "Compatibility" means: that such a document exists with a description of
> the
> server-to-client signaling mechanism.  Thus, the definition of
> "Compatible" at
> the start of section 2.2 is arguably incomplete: 'A is said to be
> "compatible"
> with B if it is possible to take a first flight of packets from version A
> and
> convert it into a first flight of packets from version B.'
>

Could you say more about the definition being incomplete please?
More specifically, what do you think is missing from the definition?

Same para: "Any set of mutually compatible versions SHOULD use the same
> mechanism." Um, are mutually compatible versions necessarily organized into
> sets which are disjoint? That wasn't obvious to me from the narrative and,
> if
> so, maybe worth saying.
>

You're right, this wasn't clear. Version compatibility is not symmetric, so
you can
visualize it as a directed graph but not as a set. I've removed the word
set:
https://github.com/quicwg/version-negotiation/commit/a8e8c29999262d391017ef8abc60afa4bcf9c818

Section 4 paragraph 5 is really hard to understand for this
> non-QUIC-expert.
> An example of the situation where the client might (invalidly) make a
> choice
> incompatible with its knowledge of what the server supports would be
> useful.
>

That's fair. I couldn't easily find a way to make this clearer, I'll think
about it some more.

Section 5, last para, "Note that this opens connections to version
> downgrades"
> - do you mean "opportunities for" or "risk of"? "connections to" is
> awkward.
>

Fair enough, that wasn't well worded. I've rewritten it:
https://github.com/quicwg/version-negotiation/commit/37ee05cf65bee54aaeda834aa71f115fca9324a4
    Note that, during the update window, connections are vulnerable to
downgrade
    attacks for partially-deployed versions. This is because a client cannot
    distinguish such a downgrade attack from legitimate exchanges with both
updated
    and non-updated server instances.

Section 7.1, send para: "If a future document wishes to define compatibility
> between two versions that support retry, that document MUST specify…" Is
> an RFC
> allowed to impose MUST constraints on future RFCs? Not a rhetorical
> question,
> just never seen anything like this before. (Also 7.3)
>

Yes, I think this is common practice as far as I know. Future documents can
however
remove the requirement, but that generally requires an Updates tag.

Section 7.2. Sounds reasonable, but some motivation might be nice.
>

It would, but I don't think that research will happen in our publication
time frame.

Section 9, Security Considerations, seems very short for such a foundational
> piece of protocol.  I would have hoped to see some discussion of threat
> models.
>  (There seems to be good attention to security issues in the body of the
> document.)
>

The document defines and mitigates version downgrade attacks, the main
concern
in any version negotiation, but that's in the body of the document. I don't
think moving
text to make the security considerations longer would improve clarity.