From f23a86e4a4c3f7498be0eb298f740a6390d1b9a5 Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Fri, 15 May 2020 14:29:16 +0900 Subject: [PATCH 1/4] add failing test --- include/picotls.h | 4 ++++ t/picotls.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/include/picotls.h b/include/picotls.h index 135f44ddc..e4c4499d7 100644 --- a/include/picotls.h +++ b/include/picotls.h @@ -512,6 +512,10 @@ typedef struct st_ptls_on_client_hello_parameters_t { * if ESNI was used */ unsigned esni : 1; + /** + * set to 1 if ClientHello is too old (or too new) to be handled by picotls + */ + unsigned incompatible_version : 1; } ptls_on_client_hello_parameters_t; /** diff --git a/t/picotls.c b/t/picotls.c index eece9656b..b85109c03 100644 --- a/t/picotls.c +++ b/t/picotls.c @@ -1544,6 +1544,50 @@ static void test_quic(void) subtest("block", test_quicblock); } +static const uint8_t tls12_client_hello[] = { + 0x16, 0x03, 0x01, 0x00, 0xd2, 0x01, 0x00, 0x00, 0xce, 0x03, 0x03, 0xd1, 0x01, 0x0e, 0x39, 0xea, 0x22, 0x28, 0x89, 0x99, + 0x42, 0xec, 0x70, 0xfa, 0xb3, 0x47, 0x01, 0xce, 0x61, 0x8d, 0xee, 0x0e, 0x3e, 0xf7, 0xe9, 0x4f, 0x0a, 0x8e, 0x94, 0x28, + 0xe5, 0xe3, 0xd3, 0x00, 0x00, 0x5c, 0xc0, 0x30, 0xc0, 0x2c, 0xc0, 0x28, 0xc0, 0x24, 0xc0, 0x14, 0xc0, 0x0a, 0x00, 0x9f, + 0x00, 0x6b, 0x00, 0x39, 0xcc, 0xa9, 0xcc, 0xa8, 0xcc, 0xaa, 0xff, 0x85, 0x00, 0xc4, 0x00, 0x88, 0x00, 0x81, 0x00, 0x9d, + 0x00, 0x3d, 0x00, 0x35, 0x00, 0xc0, 0x00, 0x84, 0xc0, 0x2f, 0xc0, 0x2b, 0xc0, 0x27, 0xc0, 0x23, 0xc0, 0x13, 0xc0, 0x09, + 0x00, 0x9e, 0x00, 0x67, 0x00, 0x33, 0x00, 0xbe, 0x00, 0x45, 0x00, 0x9c, 0x00, 0x3c, 0x00, 0x2f, 0x00, 0xba, 0x00, 0x41, + 0xc0, 0x11, 0xc0, 0x07, 0x00, 0x05, 0x00, 0x04, 0xc0, 0x12, 0xc0, 0x08, 0x00, 0x16, 0x00, 0x0a, 0x00, 0xff, 0x01, 0x00, + 0x00, 0x49, 0x00, 0x00, 0x00, 0x0f, 0x00, 0x0d, 0x00, 0x00, 0x0a, 0x69, 0x5f, 0x6e, 0x65, 0x65, 0x64, 0x5f, 0x73, 0x6e, + 0x69, 0x00, 0x0b, 0x00, 0x02, 0x01, 0x00, 0x00, 0x0a, 0x00, 0x08, 0x00, 0x06, 0x00, 0x1d, 0x00, 0x17, 0x00, 0x18, 0x00, + 0x23, 0x00, 0x00, 0x00, 0x0d, 0x00, 0x1c, 0x00, 0x1a, 0x06, 0x01, 0x06, 0x03, 0xef, 0xef, 0x05, 0x01, 0x05, 0x03, 0x04, + 0x01, 0x04, 0x03, 0xee, 0xee, 0xed, 0xed, 0x03, 0x01, 0x03, 0x03, 0x02, 0x01, 0x02, 0x03}; +static int test_tls12_on_client_hello_called = 0; + +static int test_tls12_on_client_hello(ptls_on_client_hello_t *self, ptls_t *tls, ptls_on_client_hello_parameters_t *params) +{ + test_tls12_on_client_hello_called = 1; + ok(params->incompatible_version); + ok(sizeof(tls12_client_hello) - 5 == params->raw_message.len); + ok(memcmp(tls12_client_hello + 5, params->raw_message.base, params->raw_message.len) == 0); + ok(params->server_name.len == sizeof("i-need_sni") - 1); + ok(memcmp(params->server_name.base, "i_need_sni", sizeof("i-need_sni") - 1) == 0); + return 0; +} + +static void test_tls12_hello(void) +{ + ptls_on_client_hello_t on_client_hello = {test_tls12_on_client_hello}, *orig = ctx->on_client_hello; + ctx->on_client_hello = &on_client_hello; + + test_tls12_on_client_hello_called = 0; + + ptls_t *tls = ptls_new(ctx, 1); + ptls_buffer_t sendbuf; + ptls_buffer_init(&sendbuf, "", 0); + size_t len = sizeof(tls12_client_hello); + int ret = ptls_handshake(tls, &sendbuf, tls12_client_hello, &len, NULL); + ok(ret == PTLS_ALERT_PROTOCOL_VERSION); + + ok(test_tls12_on_client_hello_called); + + ctx->on_client_hello = orig; +} + void test_picotls(void) { subtest("is_ipaddr", test_is_ipaddr); @@ -1563,6 +1607,7 @@ void test_picotls(void) subtest("fragmented-message", test_fragmented_message); subtest("handshake", test_all_handshakes); subtest("quic", test_quic); + subtest("tls12-hello", test_tls12_hello); } void test_picotls_esni(ptls_key_exchange_context_t **keys) From 6f7c2cb6582721390c144d0536ec8bcfe8be0c37 Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Fri, 15 May 2020 14:59:02 +0900 Subject: [PATCH 2/4] provided parameters found in legacy CH --- lib/picotls.c | 94 +++++++++++++++++++++++++++++---------------------- 1 file changed, 53 insertions(+), 41 deletions(-) diff --git a/lib/picotls.c b/lib/picotls.c index 78fa0f15a..1355ed753 100644 --- a/lib/picotls.c +++ b/lib/picotls.c @@ -286,6 +286,7 @@ struct st_ptls_client_hello_psk_t { #define MAX_CLIENT_CIPHERS 32 struct st_ptls_client_hello_t { + uint16_t legacy_version; const uint8_t *random_bytes; ptls_iovec_t legacy_session_id; struct { @@ -331,7 +332,8 @@ struct st_ptls_client_hello_t { size_t count; } identities; unsigned ke_modes; - int early_data_indication; + unsigned early_data_indication : 1; + unsigned is_last_extension : 1; } psk; ptls_raw_extension_t unknown_extensions[MAX_UNKNOWN_EXTENSIONS + 1]; unsigned status_request : 1; @@ -3144,14 +3146,12 @@ static int decode_client_hello(ptls_t *tls, struct st_ptls_client_hello_t *ch, c uint16_t exttype = 0; int ret; - { /* check protocol version */ - uint16_t protver; - if ((ret = ptls_decode16(&protver, &src, end)) != 0) - goto Exit; - if (protver != 0x0303) { - ret = PTLS_ALERT_HANDSHAKE_FAILURE; - goto Exit; - } + /* decode protocol version (do not bare to decode something older than TLS 1.0) */ + if ((ret = ptls_decode16(&ch->legacy_version, &src, end)) != 0) + goto Exit; + if (ch->legacy_version < 0x0301) { + ret = PTLS_ALERT_PROTOCOL_VERSION; + goto Exit; } /* skip random */ @@ -3201,6 +3201,7 @@ static int decode_client_hello(ptls_t *tls, struct st_ptls_client_hello_t *ch, c /* decode extensions */ decode_extensions(src, end, PTLS_HANDSHAKE_TYPE_CLIENT_HELLO, &exttype, { + ch->psk.is_last_extension = 0; if (tls->ctx->on_extension != NULL && (ret = tls->ctx->on_extension->cb(tls->ctx->on_extension, tls, PTLS_HANDSHAKE_TYPE_CLIENT_HELLO, exttype, ptls_iovec_init(src, end - src)) != 0)) @@ -3379,6 +3380,7 @@ static int decode_client_hello(ptls_t *tls, struct st_ptls_client_hello_t *ch, c goto Exit; } }); + ch->psk.is_last_extension = 1; } break; case PTLS_EXTENSION_TYPE_PSK_KEY_EXCHANGE_MODES: ptls_decode_block(src, end, 1, { @@ -3405,37 +3407,6 @@ static int decode_client_hello(ptls_t *tls, struct st_ptls_client_hello_t *ch, c src = end; }); - /* check if client hello make sense */ - if (is_supported_version(ch->selected_version)) { - if (!(ch->compression_methods.count == 1 && ch->compression_methods.ids[0] == 0)) { - ret = PTLS_ALERT_ILLEGAL_PARAMETER; - goto Exit; - } - /* esni */ - if (ch->esni.cipher != NULL) { - if (ch->key_shares.base == NULL) { - ret = PTLS_ALERT_ILLEGAL_PARAMETER; - goto Exit; - } - } - /* pre-shared key */ - if (ch->psk.hash_end != NULL) { - /* PSK must be the last extension */ - if (exttype != PTLS_EXTENSION_TYPE_PRE_SHARED_KEY) { - ret = PTLS_ALERT_ILLEGAL_PARAMETER; - goto Exit; - } - } else { - if (ch->psk.early_data_indication) { - ret = PTLS_ALERT_ILLEGAL_PARAMETER; - goto Exit; - } - } - } else { - ret = PTLS_ALERT_PROTOCOL_VERSION; - goto Exit; - } - ret = 0; Exit: return ret; @@ -3633,7 +3604,7 @@ static int server_handle_hello(ptls_t *tls, ptls_message_emitter_t *emitter, ptl additional_extensions \ } while (0); \ }) - struct st_ptls_client_hello_t ch = {NULL, {NULL}, {NULL}, 0, {NULL}, {NULL}, {NULL}, {{0}}, + struct st_ptls_client_hello_t ch = {0, NULL, {NULL}, {NULL}, 0, {NULL}, {NULL}, {NULL}, {{0}}, {NULL}, {NULL}, {{{NULL}}}, {{0}}, {{0}}, {{NULL}}, {NULL}, {{UINT16_MAX}}}; struct { ptls_key_exchange_algorithm_t *algorithm; @@ -3648,6 +3619,47 @@ static int server_handle_hello(ptls_t *tls, ptls_message_emitter_t *emitter, ptl if ((ret = decode_client_hello(tls, &ch, message.base + PTLS_HANDSHAKE_HEADER_SIZE, message.base + message.len, properties)) != 0) goto Exit; + + /* check if ClientHello makes sense */ + if (!(ch.legacy_version == 0x0303 && is_supported_version(ch.selected_version))) { + if (!is_second_flight && tls->ctx->on_client_hello != NULL) { + ptls_on_client_hello_parameters_t params = { + .server_name = ch.server_name, + .raw_message = message, + .negotiated_protocols = {ch.alpn.list, ch.alpn.count}, + .incompatible_version = 1, + }; + if ((ret = tls->ctx->on_client_hello->cb(tls->ctx->on_client_hello, tls, ¶ms)) != 0) + goto Exit; + } + ret = PTLS_ALERT_PROTOCOL_VERSION; + goto Exit; + } + if (!(ch.compression_methods.count == 1 && ch.compression_methods.ids[0] == 0)) { + ret = PTLS_ALERT_ILLEGAL_PARAMETER; + goto Exit; + } + /* esni */ + if (ch.esni.cipher != NULL) { + if (ch.key_shares.base == NULL) { + ret = PTLS_ALERT_ILLEGAL_PARAMETER; + goto Exit; + } + } + /* pre-shared key */ + if (ch.psk.hash_end != NULL) { + /* PSK must be the last extension */ + if (!ch.psk.is_last_extension) { + ret = PTLS_ALERT_ILLEGAL_PARAMETER; + goto Exit; + } + } else { + if (ch.psk.early_data_indication) { + ret = PTLS_ALERT_ILLEGAL_PARAMETER; + goto Exit; + } + } + if (tls->ctx->require_dhe_on_psk) ch.psk.ke_modes &= ~(1u << PTLS_PSK_KE_MODE_PSK); From f79ad7362d10933f7dcb4a31bad0699a57cf3bd8 Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Fri, 15 May 2020 15:07:04 +0900 Subject: [PATCH 3/4] add TLS 1.1 test vector (without SNI) --- t/picotls.c | 56 +++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 42 insertions(+), 14 deletions(-) diff --git a/t/picotls.c b/t/picotls.c index b85109c03..ca07a90ca 100644 --- a/t/picotls.c +++ b/t/picotls.c @@ -1544,7 +1544,9 @@ static void test_quic(void) subtest("block", test_quicblock); } -static const uint8_t tls12_client_hello[] = { +static int test_legacy_ch_callback_called = 0; + +static const uint8_t legacy_ch_tls12[] = { 0x16, 0x03, 0x01, 0x00, 0xd2, 0x01, 0x00, 0x00, 0xce, 0x03, 0x03, 0xd1, 0x01, 0x0e, 0x39, 0xea, 0x22, 0x28, 0x89, 0x99, 0x42, 0xec, 0x70, 0xfa, 0xb3, 0x47, 0x01, 0xce, 0x61, 0x8d, 0xee, 0x0e, 0x3e, 0xf7, 0xe9, 0x4f, 0x0a, 0x8e, 0x94, 0x28, 0xe5, 0xe3, 0xd3, 0x00, 0x00, 0x5c, 0xc0, 0x30, 0xc0, 0x2c, 0xc0, 0x28, 0xc0, 0x24, 0xc0, 0x14, 0xc0, 0x0a, 0x00, 0x9f, @@ -1556,34 +1558,60 @@ static const uint8_t tls12_client_hello[] = { 0x69, 0x00, 0x0b, 0x00, 0x02, 0x01, 0x00, 0x00, 0x0a, 0x00, 0x08, 0x00, 0x06, 0x00, 0x1d, 0x00, 0x17, 0x00, 0x18, 0x00, 0x23, 0x00, 0x00, 0x00, 0x0d, 0x00, 0x1c, 0x00, 0x1a, 0x06, 0x01, 0x06, 0x03, 0xef, 0xef, 0x05, 0x01, 0x05, 0x03, 0x04, 0x01, 0x04, 0x03, 0xee, 0xee, 0xed, 0xed, 0x03, 0x01, 0x03, 0x03, 0x02, 0x01, 0x02, 0x03}; -static int test_tls12_on_client_hello_called = 0; -static int test_tls12_on_client_hello(ptls_on_client_hello_t *self, ptls_t *tls, ptls_on_client_hello_parameters_t *params) +static int test_legacy_ch_tls12_callback(ptls_on_client_hello_t *self, ptls_t *tls, ptls_on_client_hello_parameters_t *params) { - test_tls12_on_client_hello_called = 1; + test_legacy_ch_callback_called = 1; ok(params->incompatible_version); - ok(sizeof(tls12_client_hello) - 5 == params->raw_message.len); - ok(memcmp(tls12_client_hello + 5, params->raw_message.base, params->raw_message.len) == 0); + ok(sizeof(legacy_ch_tls12) - 5 == params->raw_message.len); + ok(memcmp(legacy_ch_tls12 + 5, params->raw_message.base, params->raw_message.len) == 0); ok(params->server_name.len == sizeof("i-need_sni") - 1); ok(memcmp(params->server_name.base, "i_need_sni", sizeof("i-need_sni") - 1) == 0); return 0; } +static const uint8_t legacy_ch_tls11[] = { + 0x16, 0x03, 0x01, 0x00, 0x71, 0x01, 0x00, 0x00, 0x6d, 0x03, 0x02, 0xa5, 0xac, 0xfc, 0xef, 0x36, 0xa0, 0x4e, 0x1b, 0xa1, + 0x9d, 0x01, 0x98, 0x3e, 0xae, 0x07, 0x2e, 0x23, 0xdc, 0xce, 0x62, 0xc8, 0xb6, 0x7e, 0xd0, 0x5c, 0x2e, 0xeb, 0x63, 0x26, + 0x74, 0xe7, 0x61, 0x00, 0x00, 0x2e, 0xc0, 0x14, 0xc0, 0x0a, 0x00, 0x39, 0xff, 0x85, 0x00, 0x88, 0x00, 0x81, 0x00, 0x35, + 0x00, 0x84, 0xc0, 0x13, 0xc0, 0x09, 0x00, 0x33, 0x00, 0x45, 0x00, 0x2f, 0x00, 0x41, 0xc0, 0x11, 0xc0, 0x07, 0x00, 0x05, + 0x00, 0x04, 0xc0, 0x12, 0xc0, 0x08, 0x00, 0x16, 0x00, 0x0a, 0x00, 0xff, 0x01, 0x00, 0x00, 0x16, 0x00, 0x0b, 0x00, 0x02, + 0x01, 0x00, 0x00, 0x0a, 0x00, 0x08, 0x00, 0x06, 0x00, 0x1d, 0x00, 0x17, 0x00, 0x18, 0x00, 0x23, 0x00, 0x00}; + +static int test_legacy_ch_tls11_callback(ptls_on_client_hello_t *self, ptls_t *tls, ptls_on_client_hello_parameters_t *params) +{ + test_legacy_ch_callback_called = 1; + ok(params->incompatible_version); + ok(sizeof(legacy_ch_tls11) - 5 == params->raw_message.len); + ok(memcmp(legacy_ch_tls11 + 5, params->raw_message.base, params->raw_message.len) == 0); + ok(params->server_name.len == 0); + ok(params->server_name.base == NULL); + return 0; +} + static void test_tls12_hello(void) { - ptls_on_client_hello_t on_client_hello = {test_tls12_on_client_hello}, *orig = ctx->on_client_hello; + ptls_on_client_hello_t on_client_hello = {test_legacy_ch_tls12_callback}, *orig = ctx->on_client_hello; ctx->on_client_hello = &on_client_hello; - test_tls12_on_client_hello_called = 0; - - ptls_t *tls = ptls_new(ctx, 1); ptls_buffer_t sendbuf; ptls_buffer_init(&sendbuf, "", 0); - size_t len = sizeof(tls12_client_hello); - int ret = ptls_handshake(tls, &sendbuf, tls12_client_hello, &len, NULL); - ok(ret == PTLS_ALERT_PROTOCOL_VERSION); - ok(test_tls12_on_client_hello_called); + test_legacy_ch_callback_called = 0; + ptls_t *tls = ptls_new(ctx, 1); + size_t len = sizeof(legacy_ch_tls12); + int ret = ptls_handshake(tls, &sendbuf, legacy_ch_tls12, &len, NULL); + ptls_free(tls); + ok(ret == PTLS_ALERT_PROTOCOL_VERSION); + ok(test_legacy_ch_callback_called); + + on_client_hello.cb = test_legacy_ch_tls11_callback; + test_legacy_ch_callback_called = 0; + tls = ptls_new(ctx, 1); + len = sizeof(legacy_ch_tls11); + ret = ptls_handshake(tls, &sendbuf, legacy_ch_tls11, &len, NULL); + ptls_free(tls); + ok(ret == PTLS_ALERT_PROTOCOL_VERSION); ctx->on_client_hello = orig; } From 7ef490fc27d2ba2b84797d7f0c23929798a48a40 Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Sat, 16 May 2020 04:43:22 +0900 Subject: [PATCH 4/4] picotls, as a TLS 1.3-only stack, determines the Hello version by `supported_versions`. `legacy_version` is checked after determining the TLS version --- lib/picotls.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/picotls.c b/lib/picotls.c index 1355ed753..519ef7734 100644 --- a/lib/picotls.c +++ b/lib/picotls.c @@ -3620,8 +3620,8 @@ static int server_handle_hello(ptls_t *tls, ptls_message_emitter_t *emitter, ptl 0) goto Exit; - /* check if ClientHello makes sense */ - if (!(ch.legacy_version == 0x0303 && is_supported_version(ch.selected_version))) { + /* bail out if CH cannot be handled as TLS 1.3, providing the application the raw CH and SNI, to help them fallback */ + if (!is_supported_version(ch.selected_version)) { if (!is_second_flight && tls->ctx->on_client_hello != NULL) { ptls_on_client_hello_parameters_t params = { .server_name = ch.server_name, @@ -3635,6 +3635,15 @@ static int server_handle_hello(ptls_t *tls, ptls_message_emitter_t *emitter, ptl ret = PTLS_ALERT_PROTOCOL_VERSION; goto Exit; } + + /* Check TLS 1.3-specific constraints. Hereafter, we might exit without calling on_client_hello. That's fine because this CH is + * ought to be rejected. */ + if (ch.legacy_version <= 0x0300) { + /* RFC 8446 Appendix D.5: any endpoint receiving a Hello message with legacy_version set to 0x0300 MUST abort the handshake + * with a "protocol_version" alert. */ + ret = PTLS_ALERT_PROTOCOL_VERSION; + goto Exit; + } if (!(ch.compression_methods.count == 1 && ch.compression_methods.ids[0] == 0)) { ret = PTLS_ALERT_ILLEGAL_PARAMETER; goto Exit;