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

Use plausible TLS messages #3753

Merged
merged 3 commits into from Jun 30, 2020
Merged

Use plausible TLS messages #3753

merged 3 commits into from Jun 30, 2020

Conversation

martinthomson
Copy link
Member

These messages should be plausible based on the requirements of draft-29. They are taken from a real implementation, then scrubbed to make sense. Mostly this is just TLS options that don't make a whole lot of sense in QUIC that I removed. I also scrubbed out disable_active_migration, because I don't think that belongs in the spec.

The ServerHello was OK, but the framing we were using was well out of date. I fixed that up too.

Hopefully we don't change too much more before this is over, because this process is a little tedious. The first commit includes the manual breakdown I did of the messages.

Here is the breakdown of the ClientHello:

```
060040f1 = CRYPTO offset 0, length 241
010000ed = TLS Handshake length 237
0303 = version: TLS 1.2
ebf8fa56f12939b9584a3896472ec40bb863cfd3e86804fe3a47f06a2b69484c = random
00 = legacy_session_id
0004 13011302 = cipher_suites: TLS_AES_128_GCM_SHA256, TLS_AES_256_GCM_SHA384
0100 = legacy_compression_methods: none
00e0 = extensions length 194
0000 0010 000e 00 000b 6578616d706c652e636f6d = server name: example.com
ff01 0001 00 = renegotiation info
000a 0008 0006 001d00170018 = supported groups: 25519, P-256, P-384
0010 0007 000504616c706e = alpn: h3-28
0005 0005 0100000000 = certificate status
0033 0026 0024001d00209370b2c9caa47fbabaf4559fedba753de171fa71f50f1ce15d43e994ec74d748 = key share: 25519
002b 0003 020304 = supported versions: TLS 1.3
000d 0010 000e 0403050306030203080408050806 = signature algorithms: some irrelevant stuff here
002d 0002 0101 = psk modes: psk+dh
001c 0002 4001 = record size limit: max
ffa5 0032 = QUIC transport parameters extension
04 08 ffffffffffffffff = initial_max_data 2^62-1
05 04 8000ffff = initial_max_stream_data_bidi_local 2^16-1
07 04 8000ffff = initial_max_stream_data_uni 2^16-1
08 01 10 = initial_max_streams_bidi 16
01 04 80007530 = max_idle_timeout 30s
09 01 10 = initial_max_streams_uni 16
0f 08 8394c8f03e515708 = initial_source_connection_id
06 04 8000ffff = initial_max_stream_data_bidi_remote 2^16-1
```

The ServerHello was OK, but the framing wasn't.
@martinthomson martinthomson added editorial An issue that does not affect the design of the protocol; does not require consensus. -tls labels Jun 10, 2020
@martinthomson
Copy link
Member Author

Does this mean that Jana has checked that this is a valid ClientHello?

@janaiyengar
Copy link
Contributor

janaiyengar commented Jun 16, 2020

No, it does not unfortunately. My approval is probably useless. I'll see if I can verify this tomorrow.

@martinthomson
Copy link
Member Author

Well, not completely :) I caught one error by feeding this to our implementation, but the fact is that you might find it hard to validate this fully. For instance, our implementation doesn't really process the transport parameters until later. It was perfectly happy to process a ClientHello that had no QUIC extension at all.

@martinthomson martinthomson merged commit 610d213 into master Jun 30, 2020
@martinthomson martinthomson deleted the fix-chello branch June 30, 2020 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-tls editorial An issue that does not affect the design of the protocol; does not require consensus.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants