Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Packet number encryption #1079

Merged
merged 18 commits into from
May 3, 2018
Merged

Packet number encryption #1079

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
23018df
Initial cut at packet number encryption
martinthomson Jan 29, 2018
41196ef
Constant time
martinthomson Jan 29, 2018
1d6b979
That was a little unclear on a re-read
martinthomson Jan 29, 2018
16b663a
That was a little unclear on a re-read, again
martinthomson Jan 29, 2018
1ea1ae5
More tweaking (I have to stop this)
martinthomson Jan 29, 2018
2325725
Some fairly obvious errors
martinthomson Jan 29, 2018
86f262d
Don't mess with ChaCha20, use it directly
martinthomson Jan 30, 2018
1aec4b0
Fix off-by-one error
martinthomson Jan 30, 2018
a5a6df0
Two *different* packet numbers
martinthomson Jan 31, 2018
e641b72
Remove repetition
martinthomson Feb 1, 2018
3791020
Take and rework suggestions by @cwood
martinthomson Feb 2, 2018
0f3c970
Just use AES-CTR
martinthomson Feb 2, 2018
0c54fbf
Make the order of operations clearer.
martinthomson Feb 7, 2018
f163e2a
Revert unnecessary change
martinthomson Apr 4, 2018
d6d9837
Restore some of the endpoint-agnosticism that got lost in rebasing
martinthomson Apr 25, 2018
a1c4d41
Grammar boobo
martinthomson Apr 25, 2018
8e7540f
Addressing nits from final review
martinthomson May 3, 2018
947db7f
Remove confusion without changing the design
martinthomson May 3, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions draft-ietf-quic-tls.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -866,9 +866,9 @@ 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
removing packet protection, an endpoint first removes the protection from the
packet number.
({{pn-encrypt}}). The unprotected packet number is part of the associated data
(A). When removing packet protection, an endpoint first removes the protection
from the packet number.

All QUIC packets other than Version Negotiation and Stateless Reset packets are
protected with an AEAD algorithm {{!AEAD}}. Prior to establishing a shared
Expand Down Expand Up @@ -941,8 +941,8 @@ Packet number protection is applied after packet protection is applied (see
{{aead}}). The ciphertext of the packet is sampled and used as input to an
encryption algorithm.

For packets with a long header, the ciphertext starting
immediately after the packet number is used (that is, octet 17 onwards).
For packets with a long header, the ciphertext starting immediately after the
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. An additional PR that changes short headers to varint packet numbers needs to be merged first, and then this rebased on that.
  2. This PR should also change the short headers to varint packet numbers.
  3. 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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Expand Down
16 changes: 9 additions & 7 deletions draft-ietf-quic-transport.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,8 +345,9 @@ 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 are
protected separately to the packet payload.
Packet numbers are not encrypted as part of packet protection, but instead
have additional confidentiality protection. {{packet-numbers}} describes the
use of packet numbers.


Payload:
Expand Down Expand Up @@ -454,7 +455,8 @@ 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. Packet
numbers are protected separate to the packet payload.
numbers are not encrypted as part of packet protection, but instead have
additional confidentiality protection.

Protected Payload:

Expand Down Expand Up @@ -696,10 +698,10 @@ continue to use the same Destination Connection ID for 0-RTT packets, see

The version field for protected packets is the current QUIC version.

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
packet number increases with each packet sent, see {{packet-numbers}} for
details.
The packet number field contains a packet number, which has additional
confidentiality protection that is applied after packet protection is applied
(see {{QUIC-TLS}} for details). The underlying packet number increases with
each packet sent, see {{packet-numbers}} for details.

The payload is protected using authenticated encryption. {{QUIC-TLS}} describes
packet protection in detail. After decryption, the plaintext consists of a
Expand Down