Re: [QUIC] Early implementation feedback on draft-hamilton-quic-transport-protocol-00

[Jana Iyengar <[e-mail]>]

Hi Lars,

Thanks again for your implementation effort and detailed feedback, and
apologies for taking a while to respond. I'm happy to iterate, and I expect
that my turnaround will be much faster (unlike this time :-))

On Fri, Sep 23, 2016 at 2:47 AM, Eggert, Lars <[e-mail]> wrote:

> Hi,
>
> I've started to do a little QUIC implementation exercise, to see if
> the current set of documents is sufficient to allow independent
> implementation. As can be expected, they're not, but I wanted to post
> some feedback based on what I encountered at this very, very, VERY
> early phase, so the documents can be improved.
>

Thank you! This is what we're hoping for as well. Thank you for being a
willing early implementer.


> I'm purposefully avoiding to rely at documentation other than the IDs.
> I won't look at the Chromium code, and I'm only looking at the specs
> on Google Docs or wireshark outputs in order to more accurately
> describe what's currently missing in the IDs. However, I run my
> implementation against the Chromium quic_server and quic_client
> implementations, in order to see if they interoperate.
>

I anticipate some trouble in getting it to work with quic_client and
quic_server if you're just building from the spec, since these binaries use
the QUIC crypto handshake
<https://docs.google.com/document/d/1g5nIXAIkN_Y-7XJW5K45IblHd_L2f5LTaDUDwvZ5L6g/edit>,
and the spec currently does not specify one.


> (I may open source this pure C implementation at some point; at the
> moment, it's too embarrassing to do so.)
>
> I began by reading draft-hamilton-quic-transport-protocol-00, in order
> to implement the initial handshake. The ID is reasonably long and
> complete-looking. Sections 1, 2 and 3 are reasonable in terms of
> content.
>
> Section 4 (Connection Establishment) is difficult to make sense of
> without first reading Section 9 (Packet Types and Formats), because
> Section 4 refers to header fields, flags and other concepts that are
> only defined in Section 9. The content seems arbitrarily split between
> those sections, e.g., Section 4.1 attempts to specify version
> negotiation, but Section 9.2.1 describes the version negotiation
> packet format. One needs to constantly flip back and forth, which is
> annoying. I'd recommend reordering the content here.
>

I wasn't sure how to organize this content, so it's currently mechanisms in
early sections and wire formats later. From your experience it seems like
it may be better organized if the the wire format of the various packets
and frames were alongside the mechanisms. Does that sound like a good rule
of thumb? Given the size of the draft, that may be better, so I'm happy to
do it.

One thing that threw me was that Section 9.1 states "all QUIC packets
> on the wire begin with a public header" and then has a diagram of what
> that header looks like, only to immediately follow it in Section 9.2
> with special packets that don't fully include that public header.
> That's really confusing. Is there a way to describe the public header
> that would include the format of the special packets? Is there a way
> to make those special packets use the public header? They're so rarely
> sent that deviating to save a few bytes doesn't seem to be a smart
> tradeoff.
>

I think there may be a way to separate the common parts out. Specifically,
the flags and the connection ID are really the "common header", but the
reason for having these three packet types is because they differ in the
parts that are not encrypted (The parts that are encrypted should be
specified, and I'll add that.) Perhaps one way to describe them would be to
say:
- Common Header = Flags + (optional) Connection ID
- Regular Packets = Common Header + (optional) Version + Packet Number +
Frames
- Version Negotiation Packet = Common Header (Version Negotiation bit set)
+ Version List
- Public Reset Packet = Common Header (Reset bit set) + Optional tag-values

I realized that we don't describe the format for the optional tag-values in
the spec, but they're potentially useful to help authenticate the Reset.
That's entirely optional at the moment, but really the Public Reset packet
is identified by the bit in the flags and the connection is identified by
the Connection ID.


> Also, the document makes no mention of what endianness the protocol
> fields are supposed to be encoded in.
>

Good point -- I've added a sentence to the Packet Types and Formats saying
"All values are little-endian unless otherwise noted." This sentence will
likely move as the doc gets restructured, but that is the first place a
packet format is currently being described.

The ID mentions QUIC version "Q025", so my code attempts to negotiate
> this version with the Chromium quic_server, but that offers "Q036" and
> indicates support for "Q035Q034Q033Q032" as well. The negotiation
> response from the server does not conform to what's in the ID, since
> it has the "diversification nonce" flag set (0x04), which is not
> specified in Section 9.2.1. It also seems to use only 12 bytes of the
> fixed 32 bytes that the nonce should occupy to indicate support for
> other versions, according to Section 9.1.
>
> Also, that "diversification nonce" is not referred to anywhere else in
> the ID other than Section 9.1. It remains fully unclear what it is.
>

Good catch. The diversification nonce was a need for QUIC crypto, and it
won't be required for TLS or for the general format. I'll remove mentions
of it from the document.


> I change my code to negotiate version "Q036", and am trying to
> figure out which frame types I am supposed to include in the initial
> packet. The various subsections of Section 4.2 list some (and say that
> some are required), but there is not enough information here for me to
> determine which to include and what the initial values should be.
>

So, this initial packet will be a CHLO, which is a crypto handshake packet.
Basically, this is a Regular QUIC Packet, with the crypto handshake bits
being sent as a STREAM frame on stream ID 1. It's clear to me now that
having a detailed description of a handshake packet at this point in the
text is probably helpful. If the reorganization that I suggested earlier
makes sense, this may be around where I'd introduce the Regular Packet and
the STREAM frame format.

So I'm cheating, and am looking at a wireshark dump of the initial
> packet that the Chromium quic_client sends to my server. I notice
> there is a "Message Authentication Hash" following the public header
> that is not mentioned at all in the ID. There is some brief text about
> "AEAD data" in Section 9.3, but nothing that would let me understand
> how that hash is computed. A quick glance at draft-thomson-quic-tls
> doesn't resolve this either. Googling around, I find that older
> versions of the spec on Google Docs have a short paragraph on "Initial
> Packet Null Encryption" that talks about using a FNV1a­128 hash, but
> even the current version on Google Docs doesn't have that paragraph
> anymore. There's clearly some content missing here.
>

Right -- this is the hash that is used by the AEAD algo used by QUIC's
current packet crypto. This may change with TLS, and may be a different
hash, but the location of the hash is right after the public header. This
isn't documtned in the spec as you note, so I'll add some text about this
along with the revision of the Packet Formats.

And that's where I'm currently at, after a few hours of coding. Hope
> this is useful.
>

Yes, extremely useful. Thanks again for your effort and feedback -- I'll
try to incorporate as much as I can before the next rev.

- jana


> Lars
> _______________________________________________
> QUIC mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/quic
>