Packet number encryption #1079

martinthomson · 2018-01-29T06:01:01Z

This is a little rough, but I think that the bones are here. I'm not all that proud of my analysis of the security of this hack, but I couldn't think of a way of disclaiming its value without also having it seem like we were throwing unlinkability for connection migrations under the bus.

I know that we discussed removing short packet types in favor of putting the lengths under encryption. I will do that separately, just to keep the size of this changeset to something more manageable.

Closes Packet Number Gap and Connection Probing #1174
Closes Mask packet numbers with a per-connection-ID key #1043
Closes Packet number rework #1048
Closes The packet number gap specification needs work #1034
Closes Remove initial packet number randomization #850
Closes Neccessity and viability of hidden connection migration linkage #990
Closes Largest Acknowledged can't be 1 or 2 bytes #734

Enables #598

mikkelfj · 2018-01-29T10:11:48Z

draft-ietf-quic-tls.md

+packet numbers MUST be free from side-channels that would reveal the packet
+number or its encoded size.
+
+
 # Error Codes {#errors}


I am not convinced this timing attack protection is worthwhile. There is no argument on how an attacker might do the timing. There is no immediately observable reply, and an ACK would not happen just because the packet was guessed. It could affect ACK of a valid packet and a powerful attack could delay valid packets to measure timing. But this must be balanced against the value of guessing the packet number. The value can lead to tracking a migration, but so can tons of other fingerprints.

In conclusion, requiring that a full AEAD validation is performed on essentially all invalid packet headers seems to be unreasonable. However, if it is important, I would MUCH rather prefer a short 16-bit header validation tag that can quickly reject any invalid header in case of a flood attack.

EDIT: of course a short header tag would also be subject to timing attacks.

I'll second this. You can't tell from the network that a packet has been discarded, and opening yourself up to the attack of being forced to decrypt something you already know is either junk or useless doesn't seem worth it.

I knew that this would raise eyebrows :)

Early discard doesn't give any better protection against denial of service. The space of valid packet numbers is >= 50% of the total expressible space, which - with the type outside of protection as it currently is - can be 2^32. That number doesn't go down with guesses. That means that an attacker can always force the receiver to do a full AEAD. Given that, the cost of full side-channel resistance is marginal.

As for observing the timing of a discarded packet, I agree that it is hard. Of course, there is always the next packet.

It is also about being able to reject retransmissions of valid packets early, if you suddenly get a burst of such packets. Not sure how likely that is.

mikkelfj · 2018-01-29T10:21:01Z

draft-ietf-quic-tls.md

+assumed to be its largest possible length (4 octets).  Thus, for a short header,
+the sampled ciphertext starts at either octet 5 when the connection ID is
+omitted, or octet 13 when the connection is present.
+


If packet number protection (for good reasons) is only applied after handshake, this prevents connection migration from looking like a new handshake unless there are special rules for packet numbering during a handshake. Hence, perhaps protetected packet numbers should have a separate numbering space.

As it stands now, connection migration in no way looks like a new connection, because no long form handshake packets are used on the new path.

As it stands now,

I was thinking a bit ahead - considering the open issue(s?) on the topic.

Perhaps a client-initiated PATH_CHALLENGE/PATH_RESPONSE should be long header packets? But that's totally orthogonal to this PR, I think. So long as the packet number is obscured in both header types, an observer can't tell whether a connection is restarting from zero or not.

The initial handshake needs to negotiate encryption and hence cannot encrypt those packets.

Perhaps the packet numbers ought to be encrypted with a version specific non-negotiably algorithm, but that hurts devices that doesn't speak AES natively. This is why I suggest each handshake and migration sequence uses unencrypted, from 0, packet numbers, unique to each connection ID.

The biggest problem for having migration appear to be a new connection is that the packets won't decrypt with the handshake keys. This doesn't change any of that. @mikkelfj separately suggested that we use 0-RTT, and that could work, but it's a pretty big lift.

Yes that is true - in 0-RTT the packet numbers could be encrypted.

BTW: if 0-RTT doesn't actively take measures to avoid fingerprinting, that would be an excellent way to track users. This somewhat relates to the replay thing.

MikeBishop

Looks fine.

MikeBishop · 2018-01-29T18:59:17Z

draft-ietf-quic-tls.md

+plaintext, adding only an authentication tag.  Therefore, this document assumes
+that each bit of sampled AEAD output contains one bit of entropy and that an
+attacker is unable to reduce this without knowledge of the key.  Based on this
+assumption, the odds of two samples of ciphertext approach the birthday bound


It feels like you're missing a word here, but I'm not sure what it should be.

MikeBishop · 2018-01-29T19:02:07Z

draft-ietf-quic-tls.md

+  protection algorithm might be used.
+
+To prevent an attacker from modifying packet numbers, values of packet numbers
+are transitively authenticated using the packet number protection.  A falsified


I thought it was authenticated using the AD from the packet protection, not the packet number protection?

Yeah, you are right. Got used to typing packet number protection.

MikeBishop · 2018-01-29T19:04:39Z

draft-ietf-quic-tls.md

+packet numbers MUST be free from side-channels that would reveal the packet
+number or its encoded size.
+
+
 # Error Codes {#errors}


I'll second this. You can't tell from the network that a packet has been discarded, and opening yourself up to the attack of being forced to decrypt something you already know is either junk or useless doesn't seem worth it.

MikeBishop · 2018-01-29T19:05:20Z

draft-ietf-quic-transport.md

-number.  All subsequent packets contain a packet number that is incremented by
-one, see ({{packet-numbers}}).
+The first Initial packet that is sent by a client contains a packet number of 0.
+All subsequent packets contain a packet number that is incremented by one, see


I think elsewhere we've said "at least one" in order to allow for skipping the occasional packet number.

MikeBishop · 2018-01-29T19:07:46Z

draft-ietf-quic-transport.md

@@ -401,7 +401,8 @@ Version:
 Packet Number:

 : Octets 13 to 16 contain the packet number.  {{packet-numbers}} describes the
-  use of packet numbers.
+  use of packet numbers.  Packet numbers are protected separate to the packet


You use "separately from" on 679, which feels more natural. (Again, possible dialect difference.) Same on line 486-487.

Agreed, separately from feels more natural

MikeBishop · 2018-01-29T19:09:28Z

draft-ietf-quic-transport.md

@@ -709,8 +714,8 @@ packets MUST use connection ID selected by the client.
 The packet number is an integer in the range 0 to 2^62-1. The value is used in
 determining the cryptographic nonce for packet encryption.  Each endpoint
 maintains a separate packet number for sending and receiving.  The packet number
-for sending MUST increase by at least one after sending any packet, unless
-otherwise specified (see {{initial-packet-number}}).
+for sending MUST starts at zero for the first packet set and MUST increase by at


s/starts/start/

and s/set/sent

Wow, epic fail :)

ianswett

Q: If we encode the packet number length inside the encryption, which seems nice, does the decrypter have to assume the longest possible packet number length, then check the first byte after decryption to determine the actual length?

ianswett · 2018-01-29T20:54:25Z

draft-ietf-quic-transport.md

@@ -401,7 +401,8 @@ Version:
 Packet Number:

 : Octets 13 to 16 contain the packet number.  {{packet-numbers}} describes the
-  use of packet numbers.
+  use of packet numbers.  Packet numbers are protected separate to the packet


Agreed, separately from feels more natural

ianswett · 2018-01-29T20:59:18Z

draft-ietf-quic-transport.md

@@ -709,8 +714,8 @@ packets MUST use connection ID selected by the client.
 The packet number is an integer in the range 0 to 2^62-1. The value is used in
 determining the cryptographic nonce for packet encryption.  Each endpoint
 maintains a separate packet number for sending and receiving.  The packet number
-for sending MUST increase by at least one after sending any packet, unless
-otherwise specified (see {{initial-packet-number}}).
+for sending MUST starts at zero for the first packet set and MUST increase by at


and s/set/sent

ianswett · 2018-01-29T21:08:20Z

draft-ietf-quic-tls.md

@@ -866,6 +867,85 @@ the connection that is hosted on stream 0.  This sequence number is not visible
 to QUIC.


+## Packet Number Protection {#pn-encrypt}


If we move to varint packet numbers inside encryption, determining the length of the varint with this approach seems not completely obvious. If we stick with this approach instead of the overlapping crypto operations, it would be good to add small section on removing packet number protection.

Q: If we encode the packet number length inside the encryption, which seems nice, does the decrypter have to assume the longest possible packet number length, then check the first byte after decryption to determine the actual length?

Since the encryption is XOR based, it suffice to decrypt the first byte with XOR to decide the length, or, just the first two bits, actually. In praxis it might be simpler/faster to XOR with the maximum length, then read as necessary. Escpecially if the packet is guaranteed to be long enought and when fast unaligned 8 byte reads are available.

What @mikkelfj says. Say we choose an encoding with a 4 octet maximum length, then you copy 4 octets out, XOR those, then decode from that buffer. You then know the length and can discard the excess.

How about something like:

Set a barrier at the start of the chosen ciphertext input.

If the barrier is further than 8 bytes from the beginning of the packet number, XOR 8 bytes from beginning of packet number,

If the barrier is closer than that, XOR up to the barrier.

So this isn't missed by people reviewing, I've taken this suggestion. That way we can make stronger assertions about birthday bounds.

martinthomson

Thanks for the quick response.

martinthomson · 2018-01-29T22:46:56Z

draft-ietf-quic-tls.md

@@ -866,6 +867,85 @@ the connection that is hosted on stream 0.  This sequence number is not visible
 to QUIC.


+## Packet Number Protection {#pn-encrypt}


What @mikkelfj says. Say we choose an encoding with a 4 octet maximum length, then you copy 4 octets out, XOR those, then decode from that buffer. You then know the length and can discard the excess.

martinthomson · 2018-01-29T22:48:55Z

draft-ietf-quic-tls.md

+assumed to be its largest possible length (4 octets).  Thus, for a short header,
+the sampled ciphertext starts at either octet 5 when the connection ID is
+omitted, or octet 13 when the connection is present.
+


The biggest problem for having migration appear to be a new connection is that the packets won't decrypt with the handshake keys. This doesn't change any of that. @mikkelfj separately suggested that we use 0-RTT, and that could work, but it's a pretty big lift.

martinthomson · 2018-01-29T22:51:06Z

draft-ietf-quic-tls.md

+  protection algorithm might be used.
+
+To prevent an attacker from modifying packet numbers, values of packet numbers
+are transitively authenticated using the packet number protection.  A falsified


Yeah, you are right. Got used to typing packet number protection.

martinthomson · 2018-01-29T22:52:14Z

draft-ietf-quic-transport.md

@@ -709,8 +714,8 @@ packets MUST use connection ID selected by the client.
 The packet number is an integer in the range 0 to 2^62-1. The value is used in
 determining the cryptographic nonce for packet encryption.  Each endpoint
 maintains a separate packet number for sending and receiving.  The packet number
-for sending MUST increase by at least one after sending any packet, unless
-otherwise specified (see {{initial-packet-number}}).
+for sending MUST starts at zero for the first packet set and MUST increase by at


Wow, epic fail :)

kazuho · 2018-01-30T04:45:06Z

draft-ietf-quic-tls.md

+packet protection algorithm.  This might happen if a packet with a short header
+contains minimal data and uses a packet number encoding that is shorter than 4
+octets.  Additional zero octets are added to the end of the sequence to reach
+the required amount of data.


It might make sense to use the last N bytes of the packet as IV.

By doing so, I think that we can remove the zero-padding rule introduced in this paragraph, since it is guaranteed (at least for the algorithms being defined here) that the length of the AEAD tag is longer than that required for the IV.

I considered this, and it does have that advantage. However, I have been told that reading that far ahead is problematic for some implementations, performance-wise.

The odds of actually having a short packet are tiny. For a 1 octet packet number, you need to have <3 octets of payload. For a 2 octet packet number, then you need <2 octets of payload. And right now only PADDING is one octet. PING will eventually go back to 1 octet as well. That's for a 2^-56 bound. At 2^-60, STREAM, BLOCKED, and STREAM_ID_BLOCKED can be 2 octets. MAX_DATA also, if you believe that <64 octets is ever something you would see on the wire.

Instead of zero padding, the input could sampled from protected packet content including cipher text and, for short cipher text, also the authentication tag.

But

I don't like that it is not possible to do AEAD verification in-place without modifying the packet content. Thus AEAD should cover the encrypted packet number, not the plain text packet number. However, then it does not work to use the cipher nor the tag as input.

@martinthomson Thank you for elaborating the issue behind the design decision. That makes sense to me.

@mikkelfj

Instead of zero padding, the input could sampled from protected packet content including cipher text and, for short cipher text, also the authentication tag.

The issue is that it is impossible to tell where the ciphertext or the authentication tag begins, assuming that the length of the packet number field will become part of the protected packet number.

Consider the case where we have protected packet number (1 octet), ciphertext (1 octet), tag (16 octets).

The length of the packet number is encoded within the protected packet number. We need to find entropy from the octets that are guaranteed to be not part of the protected packet number.

The text in the PR proposes to take from the fifth octet of the encrypted triplet (i.e. protected packet number, ciphertext, tag). That would mean that the minimum length of the entropy would be 14 bytes. Hence zero padding.

I don't like that it is not possible to do AEAD verification in-place without modifying the packet content.

My assumption was that we would nevertheless be required to modify the packet header before feeding it as AAD to AEAD, assuming that we would either have greasing (in the packet header) or at least one spin bit. Is that no longer the case?

The issue is that it is impossible to tell where the ciphertext or the authentication tag begins

I see. But I don't think it would be a big deal to expose the length bits in clear text. But then again, zero padding is also fine.

My assumption was that we would nevertheless be required to modify the packet header

I would assume you could apply AEAD after spin bits etc.. However, packet numbers are different because they are the IV for the AEAD itself and because their encryption is derived from the same AEAD.

See my "barrier" comment above. The only requirement is that one does not re-encrypt the bytes used for the IV, because that would lead to trouble. So, set a barrier before these bytes, and never XOR bytes after that barrier.

I'm not sure what your proposal is there. What I have described takes at least 14 octets always. I think that's enough, even with the birthday bound on 112 bits being pretty low: with AES you have to re-key long before you hit that many packets, and I'd say that you'd want to do the same with ChaCha even if it remains cryptographically sound for far longer.

What I have in mind is the following:

Assume that after encryption but before packet number encryption the packet is composed of encrypted payload (ccc...ccc), packet number (pp), and other header bytes (h)

hhhhhhppppccccccc...ccc

After packet number encryption the "p" bytes will be encrypted (P). Some of the leading bytes in the ciphertext may also be re-encrypted (C):

hhhhhhPPPPCCCCccc...ccc

This only works if the receiver can find out whether the "cipher text input" starts, without knowing the exact length of the packet encoding. At that point it only knows the position of the first packet number byte, and the length of the packet. I would like a way to specify the length of the "PN encryption" and the offset of the ciphertext sample without requiring the receiver to exactly know the packet number length.

My proposition is to set the "offset" value to MIN(pn_start + 8, packet_length-16), and to encrypt the bytes between "pn_start" and "offset".

I was about to propose something similar to @huitema but then thought zero padding might do just as well.

However, I think it is better to take the tail content when the default start overreach because then you can always consume the content directly without moving it to a separate buffer. It's a quite simple solution.

huitema · 2018-01-31T04:10:49Z

draft-ietf-quic-tls.md

+
+Use of the same key and nonce for encryption for encryption can weaken
+encryption.  For the schemes described, protecting two packet numbers with the
+same key and nonce would reveal the packet number.  For packet number protection


for encryption twice?

Should I say two "different" packet numbers? Basically, this is the standard admonition against reuse of key and nonce.

huitema · 2018-01-31T19:50:46Z

draft-ietf-quic-tls.md

+This algorithm samples 16 octets from the packet ciphertext.  This input is used
+as the AES initialization vector (IV).  This value is input to AES that is keyed
+using the current packet protection key.
+


So is this 16 or 14?

rpaulo · 2018-02-01T00:50:23Z

draft-ietf-quic-tls.md

+property that AEAD algorithms do not guarantee.  Therefore, no strong assurances
+about the general security of this mechanism can be proven.
+
+Use of the same key and nonce for encryption for encryption can weaken


"for encryption" twice

chris-wood · 2018-02-01T15:56:06Z

draft-ietf-quic-tls.md

+
+~~~
+encoded = EncodePacketNumber(packet_number)
+counter = DecodeLE(sample[0..3])


Missing sample definition, or is it just assumed to be the same as with AES?

chris-wood · 2018-02-01T15:58:50Z

draft-ietf-quic-tls.md

+attacker is unable to reduce this without knowledge of the key.  Based on this
+assumption, the odds of two samples of ciphertext being identical approach the
+birthday bound for the size of the sample (that is, two to the negative power of
+half the number of sampled bits).


Possible simplification of these three paragraphs:

Assuming all AEAD functions used are reducible to PRFs, an (encoded) packet number p is encrypted as:

p XOR PRF(pn_key, m),

where m is an L-bit string sampled from a protected packet’s ciphertext. This construction is secure
against chosen plaintext attacks (IND-CPA) if the distribution of m is uniform over {0,1}^L {{KatzLindell}}.
Since AEAD schemes are semantically secure, we assume each bit of sampled ciphertext contains one bit
of entropy. Thus, two ciphertexts will collide, possibly leading to nonce reuse, with probability bounded by
2^(-L/2), i.e., the birthday bound. Use of the same key and nonce more than once can weaken the
guarantees provided by this protection. For the schemes described, protecting two different packet
numbers with the same key and nonce reveals the exclusive OR of those packet numbers, which might be
used to compromise confidentiality.

KatzLindell: Katz, Jonathan, and Yehuda Lindell. Introduction to modern cryptography. CRC press, 2014.

I think that's the right formulation: we assume that the AEAD is a PRF. It's not guaranteed to be by the contract, but it's probably true for the AEADs here. Thanks for the text. I'll probably want to make the assumption much more prominent though. Also, the general form here is using AES or ChaCha as a PRF, so that means that we can factor out the cipher-specific parts.

I'll see what I can do.

huitema · 2018-02-01T21:19:51Z

draft-ietf-quic-tls.md

+algorithms for AEAD_AES_128_GCM, AEAD_AES_128_CCM, AEAD_AES_256_GCM,
+AEAD_AES_256_CCM (all AES AEADs are defined in {{!RFC5116}}), and
+AEAD_CHACHA20_POLY1305 ({{!CHACHA=RFC7539}}).
+


I find this whole section hard to understand and hard to implement. I would like to see much more uniformity between the various constructs. I get the following:

We are going to use the same encryption algorithm for PN as the encryption algorithm part of AEAD.

There will be a specific key for PN encryption, different from the key used for data encryption.

The PN encryption will use a 16 byte sample of the packet's ciphertext as nonce.

Our AEAD specification tells us that the nonce is obtained by combining a preset IV with a sequence number. In our case, we will use the first 4 bytes of the sample as a 32 bit sequence number, and the last 12 bytes of the sample as IV, zero padded to the length required by the selected algorithm if necessary.

Conceptually, AEAD algorithm encode an N byte field by XOR it with a crypto-graphically generated stream of data, plus a 16 byte MAC. When doing PN encryption, we only use the first N bytes of the output, and do not compute or check the MAC. That is, PN encryption uses the "XOR" subset of the AEAD algorithm.

Using only the XOR part of the AEAD algorithm requires specific optimizations depending on the specific AEAD algorithm. These optimizations are presented in the following sections for AES and CHACHA20.

One way to test that a PN encryption implementation is correct is to compute the AEAD encryption of the PN with the selected IV and sequence number, and verify that the AEAD algorithm provides for the same initial bytes as the PN encryption implementation.

Do we agree on that?

You can consider this an application of the AEAD, but I think that would be an error. I prefer to think of this as using the same primitives that the AEAD uses. That is, we build AEAD_AES_128_GCM using AES, and so we use AES here.

There are commonalities in the design, and I suspect that if you abuse your AEAD interface hard enough, you can get the AEAD to produce the right bits, but that's probably inefficient (you don't need the MAC, but the AEAD will calculate one) and not a general solution (there is no guarantee that the AEAD puts the MAC at the end, or doesn't do other post-processing that would might this design).

The reason for that is part of what we sample from the ciphertext of the packet is fed into the counter part and most AEADs don't let you tweak that (all AEADs I've seen have an internal counter that starts from 0 for each invocation). We need that counter because the 96-bit nonce doesn't have enough entropy. 128 bits ensures that we don't have to worry about collisions, because we can only send 2^62 packets.

So 1 - shared primitives, not same, 2 - agreed, 3 - this will depend on AEAD, unfortunately, 4 - that's internal AEAD details, 5 - that's AEAD-specific, 6 - see point about shared primitives, 7 - this might be true for the current AEADs if you can abuse them enough, but it's not generally true.

I think Kazuho has a plausible generalization with his CTR mode API. Maybe we should find a way to document that.

We can't know that there is a CTR mode for the underlying primitive. I'm happy to include some advice along the lines of "for AES, this is just CTR mode", or a hint that in general CTR mode is a good design, but any further generalization is a presumption that might make it harder to deploy a new cipher.

I can understand @martinthomson's point that if a stream cipher (or block cipher in CTR mode) exists beneath an AEAD is an implementation detail. Hence we need a mapping.

I also agree with @huitema that it would help implementors if the AES algorithm was explained as CTR mode. That would help people come up with an implementation that shares the code between the cipher suites.

The CCM mode has a 16 byte header that includes the nonce - it won't be encrypted unless it is taken care of explicitly for that mode and if it isn't encrypted, encrypting the packer number is pointless.

@mikkelfj: I believe you have misunderstood how CCM is used in TLS (and presumably in QUIC).

The inputs to CCM encryption are:

A nonce N

The key

The plaintext

The associated data A

The inputs to CCM decryption are:

A nonce N

The key

The ciphertext

The associated data A

In some versions of CCM, you explicitly render N on the wire, but in TLS 1.3, you would not do this, but rather (as in AES-GCM), derive it from the PN. There is then no need to render the nonce on the wire, and it behaves as GCM does.

That is reassuring to hear. I originally found an older RFC 3610, but I then checked TLS 1.3 draft 23 https://tools.ietf.org/html/draft-ietf-tls-tls13-23#appendix-B.4 which referred to RFC 5116 https://tools.ietf.org/html/rfc5116#section-5.3 which in turn referred via https://tools.ietf.org/html/rfc5116#ref-CCM to NIST https://csrc.nist.gov/publications/detail/sp/800-38c/final appendix A, which lists said header with NONCE - but it wasn't very clearly formulated, so I (apparently incorrectly) assumed it was similar to https://tools.ietf.org/html/rfc3610 except for fixed parameters chosen in the header (as I would have expected).

If I very carefully read https://tools.ietf.org/html/draft-ietf-tls-tls13-23#section-5.3, I can see the nonce is implicit.

Is this resolved now? Does the text below address @huitema and @kazuho 's concerns?

I believe so.

britram · 2018-02-02T12:42:22Z

The section "Receiving Protected Packets" needs to be updated as part of this PR; at least

Failure to unprotect a packet does not necessarily indicate the existence of a
protocol error in a peer or an attack. The truncated packet number encoding
used in QUIC can cause packet numbers to be decoded incorrectly if they are
delayed significantly.

no longer holds.

I'd also suggest adding a more in-depth outline of what a receiver should do with a given packet to this section -- I went through the exercise of doing this myself, and it alleviated many of my concerns about the complexity of this approach.

martinthomson · 2018-02-02T22:18:14Z

@britram, that statement is still true.

I've noted the suggestion. Monday.

mikkelfj · 2018-03-26T13:28:52Z

I mentioned a variation of this design on the mailing list but thought it better to make a note here as well.

The packet number is removed from the header and the AEAD protected content. This is safe because the packet number is the IV for AEAD.
The last 16 octets of the AEAD message (typically the full tag) is AES encrypted using a traffic key derived for this purpose - but the tag is stored as is, we only need the encrypted value temporarily.
The encrypted tag value is truncated to the length of the packet number and the packet number is encrypted by XOR'ing with this value.
The encrypted packet number is appended to the packet content after the AEAD tag.
The lenght of the packet number must be known. It can be fixed length, the length can be stored in the packet header, or it could be variable length with the last octet remainting some unencrypted bits.

The packet number is not useful without the tag and the tag is not useful without the packet number so it makes sense to store them together, typically in a single cache line. This also speaks for keeping the packet number length encoded last if not fixed length.

This approach avoids any modification to the AEAD packet content which can therefore be decoded without modification which is both good for hardware offload and is cache and synchronization friendly for multi-processor systems.

The approach does not consume more space than an unencrypted packet number.
Due to the linear dependency of packet number encryption there is an estimated latency of 24ns on current hardware because AES cannot take advantage of parallel operations. However, if there are multiple queued packets it might be possible to have the packet number decrypted efficiently by feeding the AES pipeline from multiple packets and then sending the result to the relevant receiving thread, all without additional bus traffic cause by buffer modifications.

mikkelfj · 2018-03-26T13:43:05Z

The above modification helps CPU cache but it does not help hardware that wants to decrypt while receiving data before the entire packet has been seen.

This can be achieved by moving the encrypted packet number before the AEAD content, and therefore before the packet header. The packet number would use the first encrypted bytes of the packet rather than the AEAD tag to feed its encryption.

This is not as clean, but it would allow processing to start before the full packet has been seen.
Additionally, if a 1 octet checksum is added to the packet number, the packet number decryption could very quickly reject random packet content. This addtional benefit could pay for the addtional encryption overhead in DoS rejection.

MikeBishop · 2018-03-27T22:02:12Z

Note that #1077 has landed, so this can be retargeted/rebased onto master.

kazuho · 2018-04-23T08:00:30Z

@mikkelfj

I'm not sure what you mean by stream cipher in the PN context? You cannot have the next PN depend on the previous due to packet loss.

Because PN leak is a privacy concern, you need to guarantee that PN checksum is secure in cryptographical sense. Using several octets of a block cipher for the purpose (as explained in #1079 (comment)) is secure in sense that the probability of collision depends on the block cipher.

The downside of using a block cipher is that so far none of the proposal using block ciphers provide crypto-agility. The only proposal that has agility is the one proposed in this PR. However, it relies on a stream cipher (i.e. AES-CTR or Chacha20).

The issue with stream ciphers is that it is extremely fragile to side-channel attacks as @ad-l has pointed out. I can think of several ways to shuffle the bits in a secure and somewhat efficient way (e.g. determine the bit order using the output of the stream cipher and use bmi2 instruction for shuffling), but I do not think that it would be easy enough for us to define as a standard.

As to a possible checksum - see my earlier comment in this PR - essentially any reasonable multiplication + shift could do:
#1079 (comment)

As stated above, the checksum needs to be cryptographically secure, assuming that we want to prevent side channel attacks. I do not think the proposal meets that goal.

mikkelfj · 2018-04-23T09:31:15Z

As stated above, the checksum needs to be cryptographically secure, assuming that we want to prevent side channel attacks. I do not think the proposal meets that goal.

I don't think it needs to be that strong, but PN's then need to be segregated per path. But otherwise yes, I share those concerns, I also mentioned similar concerns in my comment. However, side channel attacks are near impossible to avoid to some degree when you need to have DoS protection.

This is a little rough, but I think that the bones are here. I know that we discussed removing short packet types in favor of putting the lengths under encryption. I will do that separately, just to keep the size of this changeset to something more manageable.

This is pretty onerous, so I'd like to see if people think that this is acceptable. The cost is that all packets need to have their AEAD run the whole way through. The gain is that there will be no side-channels for attackers to exploit.

dtikhonov · 2018-04-25T12:51:30Z

draft-ietf-quic-transport.md

@@ -346,7 +346,7 @@ Packet Number:

 : The Packet Number is a 32-bit field that follows the two connection IDs.
  {{packet-numbers}} describes the use of packet numbers.  Packet numbers are
-  protected separate to the packet payload.
+  protected separately to the packet payload.


I think this should read "Packet numbers are protected separately from the packet payload."

martinthomson · 2018-04-30T03:41:44Z

@kazuho

As far as I understand, any PNE scheme without accompanying checksum is fragile to side-channel attacks, because the probability of the receiver dropping the packet without decrypting the payload is proportional to the value of the decrypted PN field. An attacker that has access to side-channel information can use that information to guess the value of the PN field.

The PR mandates that receivers not do that (though I concede that that could easily turn into a 6919 "MUST (BUT WE KNOW YOU WON'T)"). In short, the checksum you are looking for is the integrity check that is integrated into the AEAD.

janaiyengar

This is really close. A few comments.

janaiyengar · 2018-05-03T01:11:09Z

draft-ietf-quic-tls.md

+encryption algorithm.
+
+For packets with a long header, the ciphertext starting
+immediately after the packet number is used (that is, octet 17 onwards).


+1. This needs to be fixed.

janaiyengar · 2018-05-03T01:12:16Z

draft-ietf-quic-tls.md

@@ -859,6 +865,11 @@ used for QUIC packet protection is AEAD that is negotiated for use with the TLS
 connection.  For example, if TLS is using the TLS_AES_128_GCM_SHA256, the
 AEAD_AES_128_GCM function is used.

+QUIC packets are protected prior to applying packet number encryption
+({{pn-encrypt}}).  Thus, the unprotected packet number is part of the AAD.  When


s/AAD/AEAD/

AAD is correct, but it's not clear and it's the first use of AAD, so I have reworded to use consistent terminology.

janaiyengar · 2018-05-03T01:18:39Z

draft-ietf-quic-tls.md

+algorithms for AEAD_AES_128_GCM, AEAD_AES_128_CCM, AEAD_AES_256_GCM,
+AEAD_AES_256_CCM (all AES AEADs are defined in {{!RFC5116}}), and
+AEAD_CHACHA20_POLY1305 ({{!CHACHA=RFC7539}}).
+


Is this resolved now? Does the text below address @huitema and @kazuho 's concerns?

janaiyengar · 2018-05-03T01:22:00Z

draft-ietf-quic-transport.md

@@ -345,7 +345,8 @@ Payload Length:
 Packet Number:

 : The Packet Number is a 32-bit field that follows the two connection IDs.
-  {{packet-numbers}} describes the use of packet numbers.
+  {{packet-numbers}} describes the use of packet numbers.  Packet numbers are
+  protected separately to the packet payload.


s/to/from/. Or is this because of Englishes?

janaiyengar · 2018-05-03T01:23:07Z

draft-ietf-quic-transport.md

@@ -452,7 +453,8 @@ Destination Connection ID:
 Packet Number:

 : The length of the packet number field depends on the packet type.  This field
-  can be 1, 2 or 4 octets long depending on the short packet type.
+  can be 1, 2 or 4 octets long depending on the short packet type.  Packet
+  numbers are protected separate to the packet payload.


ditto. Also, "separately".

janaiyengar · 2018-05-03T01:35:51Z

draft-ietf-quic-transport.md

+subsequently protected as normal. \[\[Editor's Note: This isn't ideal, because
+it creates a "cheat" where the client assumes a value.  That's a problem, so I'm
+tempted to suggest that this include any value less than 2^30 so that normal
+processing works - and can be properly exercised.]]


This also makes a client that uses a 0-length CID vulnerable to injection of an arbitrary Retry packet.

The 0-length connection ID is only possible after the client has received a Retry, after which it will discard any Retry packets that it receives. So I don't think so.

Right, of course.

janaiyengar · 2018-05-03T01:37:15Z

draft-ietf-quic-transport.md

-The packet number field contains a packet number, which increases with each
-packet sent, see {{packet-numbers}} for details.
+The packet number field contains a packet number, which is protected separately
+from the rest of the packet (see {{QUIC-TLS}} for details).  The underlying


You're using "separately from" here. I would make it the same in other places.

Reworded all of these to emphasize additional confidentiality protection, rather than separateness, which is a tiny bit misleading. That also neatly avoids the to/from/than problems I seem to have.

nibanks · 2018-05-03T02:51:10Z

draft-ietf-quic-tls.md

+packet number is used.
+
+For packets with a short header, the packet number length is not known before
+decryption, so it is assumed to be the smaller of the maximum possible packet


I'm a bit confused. Why is the packet number length not known before hand? Doesn't the short packet header still indicate the length?

This is in preparation for going to varint packet numbers. It would need a complete rewrite if this assumption were lifted.

But this PR doesn't change the short header to use varint packet numbers... unless I missed something. Assuming I didn't, it looks like one of three things need to happen then:

An additional PR that changes short headers to varint packet numbers needs to be merged first, and then this rebased on that.

This PR should also change the short headers to varint packet numbers.

This PR should be written to the current format of short header packet numbers.

If we are going to do it, I don't see why we don't just do (2) right now. Do you expect that to be a huge change?

Given where this PR is right now, given that the chairs have green-lighted this, and given that we're unicorning, I recommend landing this PR as is. Nick, can you open an issue to track this editorial issue, so that it doesn't get dropped?

I can open an issue, no problem. But we have waited and discussed all this time to ensure we get a good solution. Can't we take a bit longer just to iron out the details and get a complete solution merged all at once? It should be trivial enough to change the short header packet number encoding to varints.

I have fixed the incorrect statement, but left the design unchanged.

martinthomson added -transport -tls labels Jan 29, 2018

martinthomson requested a review from ekr January 29, 2018 06:01

mikkelfj reviewed Jan 29, 2018

View reviewed changes

MikeBishop reviewed Jan 29, 2018

View reviewed changes

ianswett reviewed Jan 29, 2018

View reviewed changes

martinthomson commented Jan 29, 2018

View reviewed changes

kazuho reviewed Jan 30, 2018

View reviewed changes

huitema reviewed Jan 31, 2018

View reviewed changes

rpaulo reviewed Feb 1, 2018

View reviewed changes

chris-wood reviewed Feb 1, 2018

View reviewed changes

huitema reviewed Feb 1, 2018

View reviewed changes

huitema mentioned this pull request Feb 5, 2018

Should path migration mimic an initial connection? #1096

Closed

mikkelfj mentioned this pull request Feb 10, 2018

Segmented Packet Numbers #1105

Closed

ianswett mentioned this pull request Mar 13, 2018

GREASE the packet type octet #311

Closed

martinthomson force-pushed the pn-encrypt branch from 894e0fe to 3386bf7 Compare March 14, 2018 13:31

martinthomson mentioned this pull request Mar 14, 2018

varint packet numbers #989

Closed

ianswett mentioned this pull request Mar 14, 2018

The packet number gap specification needs work #1034

Closed

martinthomson changed the base branch from key-schedule-cleanup to master April 4, 2018 05:46

martinthomson mentioned this pull request Apr 6, 2018

Packet protection keys not CID-based #1267

Closed

MikeBishop mentioned this pull request Apr 9, 2018

HoLB from NCID frames #1280

Closed

mikkelfj mentioned this pull request Apr 9, 2018

define compound packets #1262

Merged

kazuho mentioned this pull request Apr 24, 2018

Preventing KEY_PHASE bit from being used as a tool to correlate CIDs #1322

Closed

martinthomson added 15 commits April 25, 2018 12:21

Constant time

41196ef

This is pretty onerous, so I'd like to see if people think that this is acceptable. The cost is that all packets need to have their AEAD run the whole way through. The gain is that there will be no side-channels for attackers to exploit.

That was a little unclear on a re-read

1d6b979

That was a little unclear on a re-read, again

16b663a

More tweaking (I have to stop this)

1ea1ae5

Some fairly obvious errors

2325725

Don't mess with ChaCha20, use it directly

86f262d

Fix off-by-one error

1aec4b0

Two *different* packet numbers

a5a6df0

Remove repetition

e641b72

Take and rework suggestions by @cwood

3791020

Just use AES-CTR

0f3c970

Make the order of operations clearer.

0c54fbf

Revert unnecessary change

f163e2a

Restore some of the endpoint-agnosticism that got lost in rebasing

d6d9837

martinthomson force-pushed the pn-encrypt branch from d7cd4ad to d6d9837 Compare April 25, 2018 10:31

Grammar boobo

a1c4d41

dtikhonov reviewed Apr 25, 2018

View reviewed changes

marten-seemann mentioned this pull request May 2, 2018

implement header encryption quic-go/quic-go#1335

Closed

janaiyengar reviewed May 3, 2018

View reviewed changes

Addressing nits from final review

8e7540f

nibanks reviewed May 3, 2018

View reviewed changes

janaiyengar approved these changes May 3, 2018

View reviewed changes

Remove confusion without changing the design

947db7f

martinthomson merged commit f4b5283 into master May 3, 2018

martinthomson deleted the pn-encrypt branch May 3, 2018 04:26

		@@ -866,6 +867,85 @@ the connection that is hosted on stream 0. This sequence number is not visible
		to QUIC.


		## Packet Number Protection {#pn-encrypt}

Packet number encryption #1079

Packet number encryption #1079

Conversation

martinthomson commented Jan 29, 2018 • edited by MikeBishop Loading

mikkelfj Jan 29, 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

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

ianswett 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

martinthomson 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

kazuho Jan 30, 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

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

huitema Feb 1, 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

mikkelfj Feb 21, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

britram commented Feb 2, 2018

martinthomson commented Feb 2, 2018

mikkelfj commented Mar 26, 2018 • edited Loading

mikkelfj commented Mar 26, 2018 • edited Loading

MikeBishop commented Mar 27, 2018

kazuho commented Apr 23, 2018

mikkelfj commented Apr 23, 2018

Choose a reason for hiding this comment

martinthomson commented Jan 29, 2018 •

edited by MikeBishop

Loading

mikkelfj Jan 29, 2018 •

edited

Loading

kazuho Jan 30, 2018 •

edited

Loading

huitema Feb 1, 2018 •

edited

Loading

mikkelfj Feb 21, 2018 •

edited

Loading

mikkelfj commented Mar 26, 2018 •

edited

Loading

mikkelfj commented Mar 26, 2018 •

edited

Loading