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.

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

Adjust lengths to match cTLS-04 #8

Merged
merged 5 commits into from Nov 12, 2021
Merged

Adjust lengths to match cTLS-04 #8

merged 5 commits into from Nov 12, 2021

Conversation

bemasc
Copy link
Owner

@bemasc bemasc commented Oct 25, 2021

cTLS-04 made profile_id a single byte, and got rid of varint encoding (although this appears to have resulted in some confusion about how the fragment field is represented).

@bemasc bemasc requested a review from cjpatton October 25, 2021 15:03
@bemasc bemasc requested a review from cjpatton October 25, 2021 18:18
1. Set `tweak = "client hs" + profile_id` if sent by the client, or `"server hs" + profile_id` if sent by the server.
2. Replace the message with `STPRP-Encipher(key, tweak, message)`.
3. Fragment the message if necessary, ensuring at least 16 bytes of message in each fragment.
4. Change the `content_type` of the final fragment to `ctls_handshake_end(TBD)`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this accomplishes the goal, but I think the ideal way to spell it is as a layer of logic that is applied after generating records in the usual way but before writing them to the socket. In other words, ideally we could say: "The client (resp. server) constructs the sequence of records as usual. Before writing the records to the wire, they are transformed using the following algorithm: ..."

Then the algorithm would apply two transformations:

  1. First, for each contiguous sequence of CTLSPlaintext records, concatenate the sequence of fragments in the order in which they appear into a message. Next, partition STPRP-Encipher(key, tweak, message) into fragments and replace the plaintext fragments with the enciphered fragments.
  2. Second, transform each record header as you already describe.

The first transformation is perhaps more generally than we need, but the upside is that it's fairly easy to describe. I also don't think it's necessary to introduce a new content type to denote the final fragment, since the transformation applies to contiguous sequence of plaintext records.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I'm not convinced that TLS/cTLS has any concept of "contiguous sequence of CTLSPlaintext records". For example, a TLS library might hand back individual records to a callback, one at a time, without any indication of whether two records are for the same message. Conversely, a TLS library might return a "flight" of records that actually contain multiple distinct messages.

More concretely, I'm pretty sure we need the final-fragment indicator, because the recipient has no way to know where the "contiguous sequence" ends. Without this flag, the only way to decipher a plaintext record would be to keep accumulating fragments and re-applying STPRP-Decipher() to the concatenation until the output is a valid message. This is O(N^2) in the number of fragments, and isn't strictly guaranteed to be free of false positives.

Notably, DTLS doesn't have this problem, because it actually provides a header for this purpose, but TLS/TCP relies on the plaintext messages being self-delimiting.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not convinced that TLS/cTLS has any concept of "contiguous sequence of CTLSPlaintext records". For example, a TLS library might hand back individual records to a callback, one at a time, without any indication of whether two records are for the same message. Conversely, a TLS library might return a "flight" of records that actually contain multiple distinct messages.

Hmm, yeah both of those seem plausible. My understanding comes from the following from Section 5.1 of the 1.3 spec:

  • Handshake messages MUST NOT be interleaved with other record
    types. That is, if a handshake message is split over two or more
    records, there MUST NOT be any other records between them.

From this statement I infer that the records that the records pertaining to the ClientHello must be contiguous. Two things are possible, however. First, TLS implementations might ignore this "MUST NOT". Second, cTLS picks different record layer rules that makes this point irrelevant.

More concretely, I'm pretty sure we need the final-fragment indicator, because the recipient has no way to know where the "contiguous sequence" ends.

Ok, I see the problem. Let's focus on the client's first flight. This flight of records contains just one handshake message, the ClientHello. Assuming it's true that handshake records aren't interleaved with records of other types, it follows that there is just one sequence of handshake records in the first, and it's contiguous. My (incorrect) idea is that to decipher the message, the server first deciphers each of the record header, starting with the first handshake record in the sequence and ending with the first non-handshake record. Before processing the next record, it assembles the fragments and deciphers the handshake message. The problem is that the non-handshake record never shows up, since a handshake record was, in all likelihood, the last record the client sent.

Your solution is the right way to go.

@bemasc bemasc requested a review from cjpatton October 28, 2021 17:20
Copy link
Collaborator

@cjpatton cjpatton left a comment

Choose a reason for hiding this comment

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

LGTM. The one downside I see is that the encoding isn't completely agnostic to the size of handshake record fragments. There should be an alternative that is (or at least makes weaker assumptions), though it would be more complicated. I think we should opt to keep things simple for now. (Plus, as a practical matter, it seems super unlikely that handshake record fragments would be smaller than 16 bytes!)

1. Set `tweak = "client hs" + profile_id` if sent by the client, or `"server hs" + profile_id` if sent by the server.
2. Replace the message with `STPRP-Encipher(key, tweak, message)`.
3. Fragment the message if necessary, ensuring at least 16 bytes of message in each fragment.
4. Change the `content_type` of the final fragment to `ctls_handshake_end(TBD)`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not convinced that TLS/cTLS has any concept of "contiguous sequence of CTLSPlaintext records". For example, a TLS library might hand back individual records to a callback, one at a time, without any indication of whether two records are for the same message. Conversely, a TLS library might return a "flight" of records that actually contain multiple distinct messages.

Hmm, yeah both of those seem plausible. My understanding comes from the following from Section 5.1 of the 1.3 spec:

  • Handshake messages MUST NOT be interleaved with other record
    types. That is, if a handshake message is split over two or more
    records, there MUST NOT be any other records between them.

From this statement I infer that the records that the records pertaining to the ClientHello must be contiguous. Two things are possible, however. First, TLS implementations might ignore this "MUST NOT". Second, cTLS picks different record layer rules that makes this point irrelevant.

More concretely, I'm pretty sure we need the final-fragment indicator, because the recipient has no way to know where the "contiguous sequence" ends.

Ok, I see the problem. Let's focus on the client's first flight. This flight of records contains just one handshake message, the ClientHello. Assuming it's true that handshake records aren't interleaved with records of other types, it follows that there is just one sequence of handshake records in the first, and it's contiguous. My (incorrect) idea is that to decipher the message, the server first deciphers each of the record header, starting with the first handshake record in the sequence and ending with the first non-handshake record. Before processing the next record, it assembles the fragments and deciphers the handshake message. The problem is that the non-handshake record never shows up, since a handshake record was, in all likelihood, the last record the client sent.

Your solution is the right way to go.


Note: This procedure assumes that handshake messages are at least 16 bytes long. This condition is automatically true in most configurations.
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: The procedure doesn't "assume" this, since step 3 requires it.

Copy link
Owner Author

Choose a reason for hiding this comment

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

OK, changed to "requires"

draft-cpbs-pseudorandom-ctls.md Outdated Show resolved Hide resolved
Benjamin M. Schwartz and others added 2 commits November 12, 2021 14:38
Co-authored-by: Christopher Patton <cpatton@cloudflare.com>
@bemasc bemasc merged commit 61c7ad4 into main Nov 12, 2021
@bemasc bemasc deleted the bemasc-lengths branch November 12, 2021 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants