Update to draft-ietf-tls-esni-13. Later CLs will clean up the ClientHello construction a bit (draft-12 avoids computing ClientHelloOuter twice). I suspect the transcript handling on the client can also be simpler, but I'll see what's convenient after I've changed how ClientHelloOuter is constructed. Changes of note between draft-10 and draft-13: - There is now an ECH confirmation signal in both HRR and SH. We don't actually make much use of this in our client right now, but it resolves a bunch of weird issues around HRR, including edge cases if HRR applies to one ClientHello but not the other. - The confirmation signal no longer depends on key_share and PSK, so we don't have to work around a weird ordering issue. - ech_is_inner is now folded into the main encrypted_client_hello code point. This works better with some stuff around HRR. - Padding is moved from the padding extension, computed with ClientHelloInner, to something we fill in afterwards. This makes it easier to pad up the whole thing to a multiple of 32. I've accordingly updated to the latest recommended padding construction, and updated the GREASE logic to match. - ech_outer_extensions is much easier to process because the order is required to be consistent. We were doing that anyway, and now a simple linear scan works. - ClientHelloOuterAAD now uses an all zero placeholder payload of the same length. This lets us simplify the server code, but, for now, I've kept the client code the same. I'll follow this up with a CL to avoid computing ClientHelloOuter twice. - ClientHelloOuterAAD is allowed to contain a placeholder PSK. I haven't filled that in and will do it in a follow-up CL. Bug: 275 Change-Id: I7464345125c53968b2fe692f9268e392120fc2eb Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48912 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc index 76f88c7..60d820b 100644 --- a/ssl/ssl_test.cc +++ b/ssl/ssl_test.cc
@@ -1675,8 +1675,8 @@ return false; } } - if (!CBB_add_u16(&contents, params.max_name_len) || - !CBB_add_u16_length_prefixed(&contents, &child) || + if (!CBB_add_u8(&contents, params.max_name_len) || + !CBB_add_u8_length_prefixed(&contents, &child) || !CBB_add_bytes( &child, reinterpret_cast<const uint8_t *>(params.public_name.data()), params.public_name.size()) || @@ -1735,9 +1735,9 @@ static const uint8_t kECHConfig[] = { // version - 0xfe, 0x0a, + 0xfe, 0x0d, // length - 0x00, 0x43, + 0x00, 0x41, // contents.config_id 0x01, // contents.kem_id @@ -1749,10 +1749,10 @@ // contents.cipher_suites 0x00, 0x08, 0x00, 0x01, 0x00, 0x01, 0x00, 0x01, 0x00, 0x03, // contents.maximum_name_length - 0x00, 0x10, + 0x10, // contents.public_name - 0x00, 0x0e, 0x70, 0x75, 0x62, 0x6c, 0x69, 0x63, 0x2e, 0x65, 0x78, 0x61, - 0x6d, 0x70, 0x6c, 0x65, + 0x0e, 0x70, 0x75, 0x62, 0x6c, 0x69, 0x63, 0x2e, 0x65, 0x78, 0x61, 0x6d, + 0x70, 0x6c, 0x65, // contents.extensions 0x00, 0x00}; uint8_t *ech_config; @@ -2069,20 +2069,26 @@ EXPECT_EQ(client_hello_len, client_hello_len_baseline); EXPECT_EQ(ech_len, ech_len_baseline); - size_t client_hello_len_129, ech_len_129; - ASSERT_TRUE(GetECHLength(ctx.get(), &client_hello_len_129, &ech_len_129, 128, - std::string(129, 'a').c_str())); - // The padding calculation should not pad beyond the maximum. - EXPECT_GT(ech_len_129, ech_len_baseline); + // Name lengths above the maximum do not get named-based padding, but the + // overall input is padded to a multiple of 32. + size_t client_hello_len_baseline2, ech_len_baseline2; + ASSERT_TRUE(GetECHLength(ctx.get(), &client_hello_len_baseline2, + &ech_len_baseline2, 128, + std::string(128 + 32, 'a').c_str())); + EXPECT_EQ(ech_len_baseline2, ech_len_baseline + 32); + // The ClientHello lengths may match if we are still under the threshold for + // padding extension. + EXPECT_GE(client_hello_len_baseline2, client_hello_len_baseline); - // If the SNI exceeds the maximum name length, we apply some generic padding, - // so close name lengths still match. - for (size_t name_len : {129, 130, 131, 132}) { + for (size_t name_len = 128 + 1; name_len < 128 + 32; name_len++) { SCOPED_TRACE(name_len); ASSERT_TRUE(GetECHLength(ctx.get(), &client_hello_len, &ech_len, 128, std::string(name_len, 'a').c_str())); - EXPECT_EQ(client_hello_len, client_hello_len_129); - EXPECT_EQ(ech_len, ech_len_129); + EXPECT_TRUE(ech_len == ech_len_baseline || ech_len == ech_len_baseline2) + << ech_len; + EXPECT_TRUE(client_hello_len == client_hello_len_baseline || + client_hello_len == client_hello_len_baseline2) + << client_hello_len; } }