Mask packet numbers with a per-connection-ID key #1043

martinthomson · 2018-01-09T06:22:09Z

This changes the key schedule so that connection ID is part of the key
derivation. Secrets are generally unchanged, except that the handshake secret
is now fixed (we integrate the connection ID via key derivation).

Packet numbers now start from zero, with an additive masking value that is
derived in the same way that packet protection keys are. Also, the type.

Closes #1048, #1034, #850, #990, #311.

This changes the key schedule so that connection ID is part of the key derivation. Secrets are generally unchanged, except that the handshake secret is now fixed (we integrate the connection ID via key derivation). Packet numbers now start from zero, with an additive masking value that is derived in the same way that packet protection keys are. Closes #1034, #850.

mikkelfj · 2018-01-09T08:09:27Z

draft-ietf-quic-transport.md

+connection ID provided by the server.  Using a new connection ID will produce a
+new packet protection key and IV.  New values for obscuring header fields (see
+{{header-obscuring}}) ensure that packet numbers can't be used to link activity
+on paths that use different connection IDs.



Is this optional? @ianswett suggested elsewhere that a packet might be dropped if it receives a packet with a tuple that does not match the connection ID. If so, a new connection ID must be used when transmitting on a new path. OTOH, but I am not sure about the implications with NAT rebinding.

It can't be optional if it is to be interoperable.

I'm not sure which point Ian was making. If you lose connection ID and the address tuple changes, you are lost, but either of those can be enough to recover. The address tuple is only sufficient if the server isn't routing based on connection ID though, and we expect that to be quite common.

I think it was in relation to two separate connections during handshake in long form, so it might not actually apply here.

mikkelfj · 2018-01-09T08:13:33Z

draft-ietf-quic-transport.md

+Negotiation packets don't contain a packet number and Retry packets include the
+packet number from the Initial packet it responds to.  Rather than relying on
+ACK frames, these packets are implicitly acknowledged by the next Initial packet
+sent by the client.



Not exclusive to this, but is there text to discuss DoS and amplification attacks related to triggering multiple VN and retry packets? One could imagine slowly triggering a lot of retry packets that are never responded to and though the server backs off, it might effectively send a flood to one target.

We've discussed this previously. Retry and Version Negotiation are necessarily smaller than the Initial packet that triggers them. It would therefore be more efficient for the attacker to send its packets directly toward the victim.

marten-seemann · 2018-01-09T09:37:46Z

draft-ietf-quic-transport.md

-for sending MUST increase by at least one after sending any packet, unless
-otherwise specified (see {{initial-packet-number}}).
+for sending starts at zero for the first packet set and MUST increase by one
+after sending a packet.


As discussed in #1030, random packet number gaps still need to be permitted to defend against the optimistic ACK attack.
If we want to change the defense against this attack, I'd prefer to discuss this in a separate PR.

That was an error. I wish that we had a better defense against optimistic ACK attacks though.

MikeBishop

Looks reasonable; minor nits.

MikeBishop · 2018-01-09T18:00:41Z

draft-ietf-quic-tls.md

+struct {
+   uint16 length = Length;
+   opaque label<6..255> = "QUIC " + Label;
+   uint8 connectionId<0>;


Is this included just to emphasize that it's zero-length, or does it actually include a byte representing the zero-length array?

Hmm, I meant to check up on this, and now that I have, I don't actually know how <0> would be encoded.

MikeBishop · 2018-01-09T18:05:15Z

draft-ietf-quic-tls.md

+been sent, 0-RTT keys can be generated and installed for writing, if 0-RTT is
+available.  Finally, the TLS state machine reports completion of the handshake,
+the 1-RTT keys can be generated and installed for writing.  Additionally, new
+keys are installed each time that the connection ID changes.


Does this need to mention TLS key updates?

Key updates in TLS are mentioned elsewhere, I think. We don't need to use them.

MikeBishop · 2018-01-09T18:06:44Z

draft-ietf-quic-transport.md

-The packet number and connection ID fields echo the corresponding fields from
-the triggering client packet.  This allows a client to verify that the server
-received its packet.
+The connection ID field echoes the corresponding fields from the triggering


MikeBishop · 2018-01-09T18:07:13Z

draft-ietf-quic-transport.md

-received its packet.
+The connection ID field echoes the corresponding fields from the triggering
+client packet.  This allows a client to correlate a Retry with the Initial
+packet that it sends that the server received its packet.


This sentence doesn't parse.

It's also wrong :/

MikeBishop · 2018-01-09T18:09:43Z

draft-ietf-quic-transport.md

+value is increased for each subsequent packet sent by the server as described in
+{{packet-numbers}}.  The client increments the packet number from its previous
+packet by one for each packet that it sends (which might be an
+Initial, 0-RTT Protected, or Handshake packet).


I know this isn't new text, but this statement about the client is redundant -- the client's behavior is already described at line 586. (In fact, it might be worth consolidating them and simply describing that the first packet sent by each party has a packet number of zero and is incremented with each packet.)

Trimmed down.

MikeBishop · 2018-01-09T18:12:00Z

draft-ietf-quic-transport.md

+numbers still increase monotonically as long as the connection ID remains
+constant.  However, these values are different for client and server, and they
+change when a connection ID changes, together ensuring that flows with different
+connection IDs are not linkable based on the value of these fields.


Strong statement; maybe we stick to making it more difficult to link?

I believe that this is true as qualified. Based on the values of these fields alone there isn't linkability: they are effectively random. There are ways to mess this up (and multipath will definitely risk doing that).

ianswett · 2018-01-09T21:02:11Z

I need some more time to think about this, but is there a discussion which led to this solution I missed?

Also, this PR is large. Is it possible to make it more focused?

… TLS how to encode

Closes #1048, #1034, #850, #990, #311.

This is an almost purely editorial change. It consolidates the description of QHKDF-Expand and corrects a bunch of minor things related to key derivation and expansion. This is the parts of #1043 that I wanted to keep. The only non-editorial change is a change to the handshake salt used to derive handshake keys. That should keep the fresh. Closes #1043.

QHKDF-Update was specific to PR quicwg#1043 (not merged) and included the Connection ID in the info structure, but the actual merged PR (quicwg#1077) does not use this, nor does it define QHKDF-Update.

martinthomson added -transport -tls labels Jan 9, 2018

mikkelfj reviewed Jan 9, 2018

View reviewed changes

mikkelfj mentioned this pull request Jan 9, 2018

Neccessity and viability of hidden connection migration linkage #990

Closed

marten-seemann suggested changes Jan 9, 2018

View reviewed changes

MikeBishop reviewed Jan 9, 2018

View reviewed changes

martinthomson added 2 commits January 10, 2018 13:55

At least one

f341608

connectionIdLength rather than an empty vector, which is unclear from…

e451864

… TLS how to encode

britram mentioned this pull request Jan 10, 2018

varint packet numbers #989

Closed

Mike's editorial comments

483a59d

Closes #1048, #1034, #850, #990, #311.

martinthomson force-pushed the pnadd branch from 75606ba to 483a59d Compare January 15, 2018 23:43

Let's make the connection ID thing more consistent

c13e788

This was referenced Jan 29, 2018

Refactor key schedule #1077

Merged

Packet number encryption #1079

Merged

martinthomson closed this in #1077 Mar 1, 2018

Lekensteyn mentioned this pull request Mar 18, 2018

tls: use QHKDF-Expand instead of QHKDF-Update #1228

Merged

martinthomson deleted the pnadd branch May 29, 2018 02:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mask packet numbers with a per-connection-ID key #1043

Mask packet numbers with a per-connection-ID key #1043

martinthomson commented Jan 9, 2018 •

edited

Loading

mikkelfj Jan 9, 2018 •

edited

Loading

martinthomson Jan 9, 2018

mikkelfj Jan 9, 2018

mikkelfj Jan 9, 2018

martinthomson Jan 9, 2018

marten-seemann Jan 9, 2018

martinthomson Jan 10, 2018

MikeBishop left a comment

MikeBishop Jan 9, 2018

martinthomson Jan 10, 2018

MikeBishop Jan 9, 2018

martinthomson Jan 10, 2018

MikeBishop Jan 9, 2018

MikeBishop Jan 9, 2018

martinthomson Jan 10, 2018

MikeBishop Jan 9, 2018

martinthomson Jan 10, 2018

MikeBishop Jan 9, 2018

martinthomson Jan 10, 2018

ianswett commented Jan 9, 2018

Mask packet numbers with a per-connection-ID key #1043

Mask packet numbers with a per-connection-ID key #1043

Conversation

martinthomson commented Jan 9, 2018 • edited Loading

mikkelfj Jan 9, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MikeBishop left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ianswett commented Jan 9, 2018

martinthomson commented Jan 9, 2018 •

edited

Loading

mikkelfj Jan 9, 2018 •

edited

Loading