Skip to content

Commit bfa6787

Browse files
kazuhokazuho
authored
Merge pull request #596 from h2o/topic/fix-grease-ech-0rtt
fix three client-side ECH bugs (grease 0-RTT, HRR/SH, NULL configs)
2 parents 4e5e357 + a83ce98 commit bfa6787

File tree

3 files changed

+236
-71
lines changed

3 files changed

+236
-71
lines changed

‎include/picotls.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1123,8 +1123,9 @@ typedef struct st_ptls_handshake_properties_t {
11231123
*/
11241124
struct {
11251125
/**
1126-
* Config offered by server e.g., by HTTPS RR. If config.base is non-NULL but config.len is zero, a grease ECH will
1127-
* be sent, assuming that X25519-SHA256 KEM and SHA256-AES-128-GCM HPKE cipher is available.
1126+
* An ECH config offered by server e.g., by HTTPS RR. If config.len is zero and .base is non-NULL, a grease ECH will
1127+
* be sent, assuming that X25519-SHA256 KEM and SHA256-AES-128-GCM HPKE cipher is available. If .base is also NULL,
1128+
* ECH will not be used at all, even if the context provided the ECH ciphers.
11281129
*/
11291130
ptls_iovec_t configs;
11301131
/**

‎lib/picotls.c‎

Lines changed: 87 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -180,9 +180,16 @@ struct st_decoded_ech_config_t {
180180
* Properties for ECH. Iff ECH is used and not rejected, `aead` is non-NULL.
181181
*/
182182
struct st_ptls_ech_t {
183-
uint8_t offered : 1;
184-
uint8_t offered_grease : 1;
185-
uint8_t accepted : 1;
183+
/**
184+
* ECH state for this connection. `OFFERED` and `ACCEPTED` are used on both client and server; `GREASE` is client-only (server
185+
* cannot distinguish GREASE from a config mismatch, both are simply ECH that fails to decrypt).
186+
*/
187+
enum en_ptls_ech_state_t {
188+
PTLS_ECH_STATE_NONE = 0,
189+
PTLS_ECH_STATE_OFFERED,
190+
PTLS_ECH_STATE_ACCEPTED,
191+
PTLS_ECH_STATE_GREASE
192+
} state;
186193
uint8_t config_id;
187194
ptls_hpke_kem_t *kem;
188195
ptls_hpke_cipher_suite_t *cipher;
@@ -2305,7 +2312,7 @@ static int encode_client_hello(ptls_context_t *ctx, ptls_buffer_t *sendbuf, enum
23052312
buffer_push_extension(sendbuf, PTLS_EXTENSION_TYPE_PRE_SHARED_KEY, {
23062313
ptls_buffer_push_block(sendbuf, 2, {
23072314
ptls_buffer_push_block(sendbuf, 2, {
2308-
if (mode == ENCODE_CH_MODE_OUTER) {
2315+
if (mode == ENCODE_CH_MODE_OUTER && ech->state != PTLS_ECH_STATE_GREASE) {
23092316
if ((ret = ptls_buffer_reserve(sendbuf, psk_identity.len)) != 0)
23102317
goto Exit;
23112318
ctx->random_bytes(sendbuf->base + sendbuf->off, psk_identity.len);
@@ -2315,7 +2322,7 @@ static int encode_client_hello(ptls_context_t *ctx, ptls_buffer_t *sendbuf, enum
23152322
}
23162323
});
23172324
uint32_t age;
2318-
if (mode == ENCODE_CH_MODE_OUTER) {
2325+
if (mode == ENCODE_CH_MODE_OUTER && ech->state != PTLS_ECH_STATE_GREASE) {
23192326
ctx->random_bytes(&age, sizeof(age));
23202327
} else {
23212328
age = obfuscated_ticket_age;
@@ -2341,6 +2348,28 @@ static int encode_client_hello(ptls_context_t *ctx, ptls_buffer_t *sendbuf, enum
23412348
return ret;
23422349
}
23432350

2351+
/**
2352+
* Feeds the CH message into the hash, computing the PSK binder if necessary. `binder_key` must be derived before calling this
2353+
* function.
2354+
*/
2355+
static int update_ch_hash_and_binder(ptls_key_schedule_t *ks, uint8_t *ch, size_t ch_start, size_t ch_end, int has_psk,
2356+
uint8_t *binder_key, int is_outer)
2357+
{
2358+
int ret = 0;
2359+
size_t hash_off = ch_start;
2360+
2361+
if (has_psk) {
2362+
size_t psk_binder_off = ch_end - (3 + ks->hashes[0].algo->digest_size);
2363+
ptls__key_schedule_update_hash(ks, ch + hash_off, psk_binder_off - hash_off, is_outer);
2364+
hash_off = psk_binder_off;
2365+
if ((ret = calc_verify_data(ch + psk_binder_off + 3, ks, binder_key)) != 0)
2366+
return ret;
2367+
}
2368+
ptls__key_schedule_update_hash(ks, ch + hash_off, ch_end - hash_off, is_outer);
2369+
2370+
return ret;
2371+
}
2372+
23442373
static int send_client_hello(ptls_t *tls, ptls_message_emitter_t *emitter, ptls_handshake_properties_t *properties,
23452374
ptls_iovec_t *cookie)
23462375
{
@@ -2351,7 +2380,7 @@ static int send_client_hello(ptls_t *tls, ptls_message_emitter_t *emitter, ptls_
23512380
} psk = {{NULL}};
23522381
uint32_t obfuscated_ticket_age = 0;
23532382
const char *sni_name = NULL;
2354-
size_t mess_start, msghash_off;
2383+
size_t mess_start;
23552384
uint8_t binder_key[PTLS_MAX_DIGEST_SIZE];
23562385
ptls_buffer_t encoded_ch_inner;
23572386
int ret, is_second_flight = tls->key_schedule != NULL;
@@ -2371,10 +2400,11 @@ static int send_client_hello(ptls_t *tls, ptls_message_emitter_t *emitter, ptls_
23712400
if ((ret = client_setup_ech(&tls->ech, &decoded, tls->ctx->random_bytes)) != 0)
23722401
goto Exit;
23732402
}
2374-
} else {
2375-
/* zero-length config indicates ECH greasing */
2403+
} else if (properties->client.ech.configs.base != NULL) {
2404+
/* zero-length config with non-NULL base indicates ECH greasing; NULL base means no ECH */
23762405
client_setup_ech_grease(&tls->ech, tls->ctx->random_bytes, tls->ctx->ech.client.kems, tls->ctx->ech.client.ciphers,
23772406
sni_name);
2407+
tls->ech.state = PTLS_ECH_STATE_GREASE;
23782408
}
23792409
}
23802410
}
@@ -2462,7 +2492,7 @@ static int send_client_hello(ptls_t *tls, ptls_message_emitter_t *emitter, ptls_
24622492
/* start generating CH */
24632493
if ((ret = emitter->begin_message(emitter)) != 0)
24642494
goto Exit;
2465-
mess_start = msghash_off = emitter->buf->off;
2495+
mess_start = emitter->buf->off;
24662496

24672497
/* generate true (inner) CH */
24682498
if ((ret = encode_client_hello(tls->ctx, emitter->buf, ENCODE_CH_MODE_INNER, is_second_flight, properties,
@@ -2474,15 +2504,12 @@ static int send_client_hello(ptls_t *tls, ptls_message_emitter_t *emitter, ptls_
24742504

24752505
/* update the message hash, filling in the PSK binder HMAC if necessary */
24762506
if (psk.secret.base != NULL) {
2477-
size_t psk_binder_off = emitter->buf->off - (3 + tls->key_schedule->hashes[0].algo->digest_size);
24782507
if ((ret = derive_secret_with_empty_digest(tls->key_schedule, binder_key, psk.label)) != 0)
24792508
goto Exit;
2480-
ptls__key_schedule_update_hash(tls->key_schedule, emitter->buf->base + msghash_off, psk_binder_off - msghash_off, 0);
2481-
msghash_off = psk_binder_off;
2482-
if ((ret = calc_verify_data(emitter->buf->base + psk_binder_off + 3, tls->key_schedule, binder_key)) != 0)
2483-
goto Exit;
24842509
}
2485-
ptls__key_schedule_update_hash(tls->key_schedule, emitter->buf->base + msghash_off, emitter->buf->off - msghash_off, 0);
2510+
if ((ret = update_ch_hash_and_binder(tls->key_schedule, emitter->buf->base, mess_start, emitter->buf->off,
2511+
psk.secret.base != NULL, binder_key, 0)) != 0)
2512+
goto Exit;
24862513

24872514
/* ECH */
24882515
if (tls->ech.aead != NULL) {
@@ -2541,14 +2568,26 @@ static int send_client_hello(ptls_t *tls, ptls_message_emitter_t *emitter, ptls_
25412568
memcpy(tls->ech.client.first_ech.base,
25422569
emitter->buf->base + ech_size_offset - outer_ech_header_size(tls->ech.client.enc.len), len);
25432570
tls->ech.client.first_ech.len = len;
2544-
if (properties->client.ech.configs.len != 0) {
2545-
tls->ech.offered = 1;
2546-
} else {
2547-
tls->ech.offered_grease = 1;
2571+
if (tls->ech.state != PTLS_ECH_STATE_GREASE)
2572+
tls->ech.state = PTLS_ECH_STATE_OFFERED;
2573+
}
2574+
if (tls->ech.state == PTLS_ECH_STATE_GREASE) {
2575+
/* For grease ECH, the server sees the outer CH. Discard the inner state and adopt the outer, then compute the PSK
2576+
* binder over the outer CH using the standard flow. */
2577+
for (size_t i = 0; i < tls->key_schedule->num_hashes; ++i) {
2578+
tls->key_schedule->hashes[i].ctx->final(tls->key_schedule->hashes[i].ctx, NULL, PTLS_HASH_FINAL_MODE_FREE);
2579+
tls->key_schedule->hashes[i].ctx = tls->key_schedule->hashes[i].ctx_outer;
2580+
tls->key_schedule->hashes[i].ctx_outer = NULL;
25482581
}
2582+
ptls_aead_free(tls->ech.aead);
2583+
tls->ech.aead = NULL;
2584+
if ((ret = update_ch_hash_and_binder(tls->key_schedule, emitter->buf->base, mess_start, emitter->buf->off,
2585+
psk.secret.base != NULL, binder_key, 0)) != 0)
2586+
goto Exit;
2587+
} else {
2588+
/* update outer hash */
2589+
ptls__key_schedule_update_hash(tls->key_schedule, emitter->buf->base + mess_start, emitter->buf->off - mess_start, 1);
25492590
}
2550-
/* update hash */
2551-
ptls__key_schedule_update_hash(tls->key_schedule, emitter->buf->base + mess_start, emitter->buf->off - mess_start, 1);
25522591
}
25532592

25542593
/* commit CH to the record layer */
@@ -2691,7 +2730,7 @@ static int decode_server_hello(ptls_t *tls, struct st_ptls_server_hello_t *sh, c
26912730
break;
26922731
case PTLS_EXTENSION_TYPE_ENCRYPTED_CLIENT_HELLO:
26932732
assert(sh->is_retry_request);
2694-
if (!(tls->ech.offered || tls->ech.offered_grease)) {
2733+
if (tls->ech.state == PTLS_ECH_STATE_NONE) {
26952734
ret = PTLS_ALERT_UNSUPPORTED_EXTENSION;
26962735
goto Exit;
26972736
}
@@ -2792,10 +2831,16 @@ static int client_ech_select_hello(ptls_t *tls, ptls_iovec_t message, size_t con
27922831
if ((ret = ech_calc_confirmation(tls->key_schedule, confirm_hash_expected, tls->ech.inner_client_random, label, message)) !=
27932832
0)
27942833
goto Exit;
2795-
tls->ech.accepted = ptls_mem_equal(confirm_hash_delivered, confirm_hash_expected, sizeof(confirm_hash_delivered));
2834+
int accepted = ptls_mem_equal(confirm_hash_delivered, confirm_hash_expected, sizeof(confirm_hash_delivered));
27962835
memcpy(message.base + confirm_hash_off, confirm_hash_delivered, sizeof(confirm_hash_delivered));
2797-
if (tls->ech.accepted)
2836+
if (accepted) {
2837+
tls->ech.state = PTLS_ECH_STATE_ACCEPTED;
2838+
goto Exit;
2839+
} else if (tls->ech.state == PTLS_ECH_STATE_ACCEPTED) {
2840+
/* Per RFC 9849 Section 6.1.5: if HRR confirmed ECH acceptance, ServerHello MUST also confirm it. */
2841+
ret = PTLS_ALERT_ILLEGAL_PARAMETER;
27982842
goto Exit;
2843+
}
27992844
}
28002845

28012846
/* dispose ECH AEAD state to indicate rejection, adopting outer CH for the rest of the handshake */
@@ -2804,8 +2849,8 @@ static int client_ech_select_hello(ptls_t *tls, ptls_iovec_t message, size_t con
28042849
key_schedule_select_outer(tls->key_schedule);
28052850

28062851
Exit:
2807-
PTLS_PROBE(ECH_SELECTION, tls, !!tls->ech.accepted);
2808-
PTLS_LOG_CONN(ech_selection, tls, { PTLS_LOG_ELEMENT_BOOL(is_ech, tls->ech.accepted); });
2852+
PTLS_PROBE(ECH_SELECTION, tls, tls->ech.state == PTLS_ECH_STATE_ACCEPTED);
2853+
PTLS_LOG_CONN(ech_selection, tls, { PTLS_LOG_ELEMENT_BOOL(is_ech, tls->ech.state == PTLS_ECH_STATE_ACCEPTED); });
28092854
ptls_clear_memory(confirm_hash_expected, sizeof(confirm_hash_expected));
28102855
return ret;
28112856
}
@@ -2831,11 +2876,9 @@ static int client_handle_hello(ptls_t *tls, ptls_message_emitter_t *emitter, ptl
28312876
key_schedule_transform_post_ch1hash(tls->key_schedule);
28322877
if (tls->ech.aead != NULL) {
28332878
size_t confirm_hash_off = 0;
2834-
if (tls->ech.offered) {
2879+
if (tls->ech.state != PTLS_ECH_STATE_GREASE) {
28352880
if (sh.retry_request.ech != NULL)
28362881
confirm_hash_off = sh.retry_request.ech - message.base;
2837-
} else {
2838-
assert(tls->ech.offered_grease);
28392882
}
28402883
if ((ret = client_ech_select_hello(tls, message, confirm_hash_off, ECH_CONFIRMATION_HRR)) != 0)
28412884
goto Exit;
@@ -2851,11 +2894,9 @@ static int client_handle_hello(ptls_t *tls, ptls_message_emitter_t *emitter, ptl
28512894
/* check if ECH is accepted */
28522895
if (tls->ech.aead != NULL) {
28532896
size_t confirm_hash_off = 0;
2854-
if (tls->ech.offered) {
2897+
if (tls->ech.state != PTLS_ECH_STATE_GREASE) {
28552898
confirm_hash_off =
28562899
PTLS_HANDSHAKE_HEADER_SIZE + 2 /* legacy_version */ + PTLS_HELLO_RANDOM_SIZE - PTLS_ECH_CONFIRM_LENGTH;
2857-
} else {
2858-
assert(tls->ech.offered_grease);
28592900
}
28602901
if ((ret = client_ech_select_hello(tls, message, confirm_hash_off, ECH_CONFIRMATION_SERVER_HELLO)) != 0)
28612902
goto Exit;
@@ -3003,17 +3044,19 @@ static int client_handle_encrypted_extensions(ptls_t *tls, ptls_iovec_t message,
30033044
src = end;
30043045
break;
30053046
case PTLS_EXTENSION_TYPE_ENCRYPTED_CLIENT_HELLO: {
3006-
/* accept retry_configs only if we offered ECH but rejected */
3007-
if (!((tls->ech.offered || tls->ech.offered_grease) && !ptls_is_ech_handshake(tls, NULL, NULL, NULL))) {
3047+
/* accept retry_configs only if we offered ECH (or grease) but rejected */
3048+
if (!(tls->ech.state == PTLS_ECH_STATE_OFFERED || tls->ech.state == PTLS_ECH_STATE_GREASE)) {
30083049
ret = PTLS_ALERT_UNSUPPORTED_EXTENSION;
30093050
goto Exit;
30103051
}
3011-
/* parse retry_config, and if it is applicable, provide that to the application */
3052+
/* parse retry_config, and if it is applicable, provide that to the application (grease clients just verify syntax) */
30123053
struct st_decoded_ech_config_t decoded;
30133054
if ((ret = client_decode_ech_config_list(tls->ctx, &decoded, ptls_iovec_init(src, end - src))) != 0)
30143055
goto Exit;
3015-
if (decoded.kem != NULL && decoded.cipher != NULL && properties != NULL &&
3016-
properties->client.ech.retry_configs != NULL) {
3056+
if (tls->ech.state == PTLS_ECH_STATE_GREASE) {
3057+
/* GREASE clients ignore retry_configs after verifying the syntax */
3058+
} else if (decoded.kem != NULL && decoded.cipher != NULL && properties != NULL &&
3059+
properties->client.ech.retry_configs != NULL) {
30173060
if ((properties->client.ech.retry_configs->base = malloc(end - src)) == NULL) {
30183061
ret = PTLS_ERROR_NO_MEMORY;
30193062
goto Exit;
@@ -3281,7 +3324,7 @@ static int handle_certificate(ptls_t *tls, const uint8_t *src, const uint8_t *en
32813324
if (tls->ctx->verify_certificate != NULL) {
32823325
const char *server_name = NULL;
32833326
if (!ptls_is_server(tls)) {
3284-
if (tls->ech.offered && !ptls_is_ech_handshake(tls, NULL, NULL, NULL)) {
3327+
if (tls->ech.state == PTLS_ECH_STATE_OFFERED) {
32853328
server_name = tls->ech.client.public_name;
32863329
} else {
32873330
server_name = tls->server_name;
@@ -3452,7 +3495,7 @@ static int server_handle_certificate_verify(ptls_t *tls, ptls_iovec_t message)
34523495
static int client_handle_finished(ptls_t *tls, ptls_message_emitter_t *emitter, ptls_iovec_t message)
34533496
{
34543497
uint8_t send_secret[PTLS_MAX_DIGEST_SIZE];
3455-
int alert_ech_required = tls->ech.offered && !ptls_is_ech_handshake(tls, NULL, NULL, NULL), ret;
3498+
int alert_ech_required = tls->ech.state == PTLS_ECH_STATE_OFFERED, ret;
34563499

34573500
if ((ret = verify_finished(tls, message)) != 0)
34583501
goto Exit;
@@ -4397,7 +4440,7 @@ static int server_handle_hello(ptls_t *tls, ptls_message_emitter_t *emitter, ptl
43974440
goto Exit;
43984441
}
43994442
if (!is_second_flight)
4400-
tls->ech.offered = 1;
4443+
tls->ech.state = PTLS_ECH_STATE_OFFERED;
44014444
/* obtain AEAD context for opening inner CH */
44024445
if (!is_second_flight && ch->ech.payload.base != NULL && tls->ctx->ech.server.create_opener != NULL) {
44034446
if ((tls->ech.aead = tls->ctx->ech.server.create_opener->cb(
@@ -4420,7 +4463,7 @@ static int server_handle_hello(ptls_t *tls, ptls_message_emitter_t *emitter, ptl
44204463
memset(ech.ch_outer_aad + (ch->ech.payload.base - (message.base + PTLS_HANDSHAKE_HEADER_SIZE)), 0, ch->ech.payload.len);
44214464
if (ptls_aead_decrypt(tls->ech.aead, ech.encoded_ch_inner, ch->ech.payload.base, ch->ech.payload.len, is_second_flight,
44224465
ech.ch_outer_aad, message.len - PTLS_HANDSHAKE_HEADER_SIZE) != SIZE_MAX) {
4423-
tls->ech.accepted = 1;
4466+
tls->ech.state = PTLS_ECH_STATE_ACCEPTED;
44244467
/* successfully decrypted EncodedCHInner, build CHInner */
44254468
if ((ret = rebuild_ch_inner(&ech.ch_inner, ech.encoded_ch_inner,
44264469
ech.encoded_ch_inner + ch->ech.payload.len - tls->ech.aead->algo->tag_size, ch,
@@ -4448,7 +4491,7 @@ static int server_handle_hello(ptls_t *tls, ptls_message_emitter_t *emitter, ptl
44484491
tls->ech.aead = NULL;
44494492
}
44504493
}
4451-
} else if (tls->ech.offered) {
4494+
} else if (tls->ech.state != PTLS_ECH_STATE_NONE) {
44524495
assert(is_second_flight);
44534496
ret = PTLS_ALERT_ILLEGAL_PARAMETER;
44544497
goto Exit;
@@ -4822,7 +4865,7 @@ static int server_handle_hello(ptls_t *tls, ptls_message_emitter_t *emitter, ptl
48224865
if (tls->pending_handshake_secret != NULL)
48234866
buffer_push_extension(sendbuf, PTLS_EXTENSION_TYPE_EARLY_DATA, {});
48244867
/* send ECH retry_configs, if ECH was offered by rejected, even though we (the server) could have accepted ECH */
4825-
if (tls->ech.offered && !ptls_is_ech_handshake(tls, NULL, NULL, NULL) && tls->ctx->ech.server.create_opener != NULL &&
4868+
if (tls->ech.state == PTLS_ECH_STATE_OFFERED && tls->ctx->ech.server.create_opener != NULL &&
48264869
tls->ctx->ech.server.retry_configs.len != 0)
48274870
buffer_push_extension(sendbuf, PTLS_EXTENSION_TYPE_ENCRYPTED_CLIENT_HELLO, {
48284871
ptls_buffer_pushv(sendbuf, tls->ctx->ech.server.retry_configs.base, tls->ctx->ech.server.retry_configs.len);
@@ -5588,7 +5631,7 @@ int ptls_is_psk_handshake(ptls_t *tls)
55885631

55895632
int ptls_is_ech_handshake(ptls_t *tls, uint8_t *config_id, ptls_hpke_kem_t **kem, ptls_hpke_cipher_suite_t **cipher)
55905633
{
5591-
if (tls->ech.accepted) {
5634+
if (tls->ech.state == PTLS_ECH_STATE_ACCEPTED) {
55925635
if (config_id != NULL)
55935636
*config_id = tls->ech.config_id;
55945637
if (kem != NULL)

0 commit comments

Comments
 (0)