[QUIC] Review of draft-hamilton-quic-transport-protocol-01

[Kyle Rose <[e-mail]>]

In general, the spec is pretty clear and easy to follow. The most confusing
bit is that it's not entirely clear from this spec which things are
encrypted/authenticated and when. Maybe an additional state diagram
detailing encryption/authentication keys at various stages of connection
establishment would help.


3.7. I know there will be much discussion of the connection ID as a
tracking cookie. I'm willing to bet there's some reduction from public key
cryptography to any scheme for sending omega(1)
cryptographically-independent identifying tokens (e.g., table indices)
based on O(1) shared data, so until PKC is cheap enough, we're probably
stuck with either issuing enough connection IDs that we can be assured of a
unique ID per packet, or living with some degree of trackability across
client address changes.

4.2. Packet number is authenticated but not encrypted? Is there a good
reason not to encrypt it to the degree possible? (I.e., with a traffic key
derived from the best shared secret we know at any particular point in
connection establishment.) The same goes for ACKs. See also: section 11.1.

4.2.1. See the discussion in the "Packet number recovery" thread. Slight
addendum: about 10 seconds after hitting "send" on my message, I realized
that one of the doom scenarios I posited (K*2^n) couldn't happen given some
of the earlier assumptions I made about how the sender must choose packet
numbers, i.e. that such large packet numbers couldn't be used without
largest acked advancing.

5.1. Is it possible for the mandatory version negotiation response behavior
to be an amplification vector for spoofed packets?

5.2.1.3: How is the client to know that it doesn't need a connection ID?
What if there's an SNAT close to the server? A synthetic example would be a
lab environment that needs to use all public IP space, and so NATs
connections from external IPs to a specific gateway IP.

5.2.3: Is it intended for the STK to be folded into the TLS session ticket?
I haven't gotten to Martin's draft yet.

5.2.3: "The crypto handshake MUST ensure that the final negotiated key is
distinct for every connection between two endpoints.": trivially
unachievable if both ends cooperate. Maybe want to say something like "The
crypto handshake MUST ensure that the final negotiated key is distinct for
every connection between two endpoints if at least one endpoint desires
this property."

5.2.3: TLS hasn't had to deal with amplification attacks of this sort
because of the TCP 3WHS. I'm not sure compression is enough here, but I
also don't know if there's a good universal mitigation to the spoofing
problem.

6.2: I would move the "It is recommended for the sender to send the most
recent largest acked packet [number]...." out of the normative protocol
description. I would also explain why this is the recommended strategy,
because it isn't obvious to me why you'd want to do this or why this
wouldn't result in spurious retransmits from slightly out-of-order delivery.

6.2: I would mostly eliminate opportunistic ACK attacks by authenticating
all ACKs to the degree possible (as suggested above). See also: section
11.1.

6.2: "The two 'LL/MM' bits encode..." A normative mapping from two bits to
{1,2,4,6} is required IMO.

6.2: Num Blocks: So, the value is actually number of additional ACK blocks
minus 1?

6.2: "...between the largest acked and the first packet whose timestamp is
being reported." By "first" you mean "latest"? It seems they must go in
reverse order, so I think "first" might be confusing terminology here.

6.2: Is there ever a situation in which it might be valuable not to include
packets in the explicit ACK list if they are also included in the timestamp
section? If the packets in the timestamp section are sparse, then
introducing gaps would make the ACK frame strictly larger, but if every
packet appears in both places it could reduce the size of the ACK frame
somewhat. I'm willing to concede that this may be an optimization that is
more complicated than it is worth.

6.5: Is there a situation in which a BLOCKED frame could inform the
recipient that the sender missed a WINDOW_UPDATE frame? This would
presumably fix itself eventually via retransmission, anyway.

7: "A sender SHOULD minimize per-packet bandwidth and computational costs
by bundling as many frames as possible within a QUIC packet." This directly
conflicts with the earlier recommendation "An implementation is therefore
advised to bundle as few streams as necessary in outgoing packets without
losing transmission efficiency to underfilled packets." Heuristics around
frame packing would be helpful here: surely, for streams A and B you never
want to send packets A_1||B_1 and A_2||B_2 when you could send A_1||A_2 and
B_1||B_2 with the same latency, but whether you want to send A and B in
separate packets or as A||B depends on some function of overhead and loss.

7: "...a receiver MUST NOT generate an ACK in response to every packet
containing only ACK frames *or padding*".

7: Is the goal of sending an ACK-of-ACKs just to update largest ACKed?

8: One potential problem with QUIC streams as a message abstraction is that
the stream ID space is pretty limited at 2^32.

8.1: I think "recv FIN" on the transition from "half closed (remote)" to
"closed" and "send FIN" on the transition from "half closed (local)" to
"closed" are reversed. Should there also be a transition from "idle" to
"closed" for "recv RST"?

8.1.6: Given the requirement to send a RST immediately after receiving an
unsolicited RST, should there also be a "remote reset" state whenever "recv
RST" occurs first, followed immediately by "send RST" and a transition to
"closed"? Or does this just complicate the diagram for no good reason?

8.4: Is triggered delivery prohibited? I.e., is it out-of-spec for a sender
to send a series of packets containing the bytes 1-65535 of a message and
then trigger instantaneous application delivery of this blob by sending
byte 0 in the last packet? Surely this can happen by accident, but doing it
intentionally is somewhat different.

9.1.1: Should a RST with a lower offset than highest data offset already
seen for a stream result in a connection error? Otherwise, the sender and
receiver could have inconsistent views of the flow control state.

9.1.4: "...it is generally considered best to not let the sender go into
quiescence if avoidable." Should this be "an active sender"?

11.1: I again feel like there's no reason to allow unauthenticated ACKs
when authentication is possible.


Nits: (Maybe will be PRs if this ends up in Github)

5.2. "crypto stream": forward reference?

6.1: I would move things like the "Implementation note" out of the
normative protocol description into some other section describing
recommendations/best practices for optimization.

6.9. "QuicErrorCode": forward reference?


Kyle