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

[David Schinazi <[e-mail]>]

Thanks Tim, responses inline.
David


On Thu, Oct 6, 2022 at 11:59 AM Tim Bray <[e-mail]> wrote:

> A couple of small notes inline below.
>
> I have to say that I remain concerned about the thin-ness of section 9.
> I'm not on the IESG but if I were, and a draft spec for a pretty central
> piece of Internet infrastructure came in without threat analysis or
> in-depth security discussion, I would find it very difficult to be happy.
> Your call.
>

QUIC is a pretty central piece of Internet infrastructure, but this
document describes a very small portion. At the end of the day, this
document defines an algorithm whose single purpose is to take two lists of
supported versions as inputs and produce one version as output. The
security risk there is downgrade attacks, more on that below.

On Wed, Oct 5, 2022 at 3:46 PM David Schinazi <[e-mail]>
> wrote:
>
>
>> 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?
>>
>
> Well,  it's self-evidently incomplete because in 2.3 it's clear that for
> versions to be compatible, a document needs to exist and you say useful
> things about what that document has to contain. So I think the spec would
> be cleaner if you mentioned the document up there in 2.2.
>

Isn't that covered by this paragraph in s2.2? It uses "otherwise specified"
to refer to the required document.

    When a new version of QUIC is defined, it is assumed to not be
compatible
    with any other version unless otherwise specified. Similarly, no other
version
    is compatible with the new version unless otherwise specified.
Implementations
    MUST NOT assume compatibility between versions unless explicitly
specified.

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.
>>
>
> Suggestion: Add a couple of examples.  I'm a huge fan of examples in
> specifications. I often find that when I have to write them, it forces me
> to clarify my thinking and has regularly led to improvements in the
> explanatory text.
>

Thanks for the nudge. I've added examples:
https://github.com/quicwg/version-negotiation/commit/01c8b63a9f7585531c9d3779b66338c9502a3fd9

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.
>>
>
> Okay, that's useful.  If you believe that the major threat scenario is
> downgrade attacks, and that there aren't any others that are worth
> addressing in this RFC, it would be useful to say so.
>

I'm not sure there's much benefit in saying that - doesn't it amount to
saying "we protected against what we knew about"? There's never an absolute
guarantee when it comes to security.

Is it a fair assumption that anyone who's going to read this will
> understand what a "downgrade attack" is, so there's no benefit from
> describing the scenario?
>

Oh right my bad, the definition had been written but not added yet, it's
currently a PR:
https://github.com/quicwg/version-negotiation/pull/127/files