From 7b1e2bed428777bb9ee601d6775aa91efdc80340 Mon Sep 17 00:00:00 2001 From: doug <> Date: Sat, 12 Aug 2017 21:17:03 +0000 Subject: Rewrite session ticket TLS extension handling using CBB/CBS and the new extension framework. ok jsing@ beck@ --- src/lib/libssl/ssl_tlsext.c | 136 ++++++++++++- src/lib/libssl/ssl_tlsext.h | 8 +- src/lib/libssl/t1_lib.c | 66 +------ src/regress/lib/libssl/tlsext/tlsexttest.c | 303 ++++++++++++++++++++++++++++- 4 files changed, 447 insertions(+), 66 deletions(-) (limited to 'src') diff --git a/src/lib/libssl/ssl_tlsext.c b/src/lib/libssl/ssl_tlsext.c index c050224c70..1813d46f41 100644 --- a/src/lib/libssl/ssl_tlsext.c +++ b/src/lib/libssl/ssl_tlsext.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_tlsext.c,v 1.6 2017/08/11 20:14:13 doug Exp $ */ +/* $OpenBSD: ssl_tlsext.c,v 1.7 2017/08/12 21:17:03 doug Exp $ */ /* * Copyright (c) 2016, 2017 Joel Sing * Copyright (c) 2017 Doug Hogan @@ -492,6 +492,131 @@ tlsext_sni_serverhello_parse(SSL *s, CBS *cbs, int *alert) return 1; } +/* + * SessionTicket extension - RFC 5077 section 3.2 + */ +int +tlsext_sessionticket_clienthello_needs(SSL *s) +{ + /* + * Send session ticket extension when enabled and not overridden. + * + * When renegotiating, send an empty session ticket to indicate support. + */ + if ((SSL_get_options(s) & SSL_OP_NO_TICKET) != 0) + return 0; + + if (s->internal->new_session) + return 1; + + if (s->internal->tlsext_session_ticket != NULL && + s->internal->tlsext_session_ticket->data == NULL) + return 0; + + return 1; +} + +int +tlsext_sessionticket_clienthello_build(SSL *s, CBB *cbb) +{ + /* + * Signal that we support session tickets by sending an empty + * extension when renegotiating or no session found. + */ + if (s->internal->new_session || s->session == NULL) + return 1; + + if (s->session->tlsext_tick != NULL) { + /* Attempt to resume with an existing session ticket */ + if (!CBB_add_bytes(cbb, s->session->tlsext_tick, + s->session->tlsext_ticklen)) + return 0; + + } else if (s->internal->tlsext_session_ticket != NULL) { + /* + * Attempt to resume with a custom provided session ticket set + * by SSL_set_session_ticket_ext(). + */ + if (s->internal->tlsext_session_ticket->length > 0) { + size_t ticklen = s->internal->tlsext_session_ticket->length; + + if ((s->session->tlsext_tick = malloc(ticklen)) == NULL) + return 0; + memcpy(s->session->tlsext_tick, + s->internal->tlsext_session_ticket->data, + ticklen); + s->session->tlsext_ticklen = ticklen; + + if (!CBB_add_bytes(cbb, s->session->tlsext_tick, + s->session->tlsext_ticklen)) + return 0; + } + } + + if (!CBB_flush(cbb)) + return 0; + + return 1; +} + +int +tlsext_sessionticket_clienthello_parse(SSL *s, CBS *cbs, int *alert) +{ + if (s->internal->tls_session_ticket_ext_cb) { + if (!s->internal->tls_session_ticket_ext_cb(s, CBS_data(cbs), + (int)CBS_len(cbs), + s->internal->tls_session_ticket_ext_cb_arg)) { + *alert = TLS1_AD_INTERNAL_ERROR; + return 0; + } + } + + /* We need to signal that this was processed fully */ + if (!CBS_skip(cbs, CBS_len(cbs))) { + *alert = TLS1_AD_INTERNAL_ERROR; + return 0; + } + + return 1; +} + +int +tlsext_sessionticket_serverhello_needs(SSL *s) +{ + return (s->internal->tlsext_ticket_expected && + !(SSL_get_options(s) & SSL_OP_NO_TICKET)); +} + +int +tlsext_sessionticket_serverhello_build(SSL *s, CBB *cbb) +{ + /* Empty ticket */ + + return 1; +} + +int +tlsext_sessionticket_serverhello_parse(SSL *s, CBS *cbs, int *alert) +{ + if (s->internal->tls_session_ticket_ext_cb) { + if (!s->internal->tls_session_ticket_ext_cb(s, CBS_data(cbs), + (int)CBS_len(cbs), + s->internal->tls_session_ticket_ext_cb_arg)) { + *alert = TLS1_AD_INTERNAL_ERROR; + return 0; + } + } + + if ((SSL_get_options(s) & SSL_OP_NO_TICKET) != 0 || CBS_len(cbs) > 0) { + *alert = TLS1_AD_UNSUPPORTED_EXTENSION; + return 0; + } + + s->internal->tlsext_ticket_expected = 1; + + return 1; +} + struct tls_extension { uint16_t type; int (*clienthello_needs)(SSL *s); @@ -539,6 +664,15 @@ static struct tls_extension tls_extensions[] = { .serverhello_build = tlsext_ec_serverhello_build, .serverhello_parse = tlsext_ec_serverhello_parse, }, + { + .type = TLSEXT_TYPE_session_ticket, + .clienthello_needs = tlsext_sessionticket_clienthello_needs, + .clienthello_build = tlsext_sessionticket_clienthello_build, + .clienthello_parse = tlsext_sessionticket_clienthello_parse, + .serverhello_needs = tlsext_sessionticket_serverhello_needs, + .serverhello_build = tlsext_sessionticket_serverhello_build, + .serverhello_parse = tlsext_sessionticket_serverhello_parse, + }, }; #define N_TLS_EXTENSIONS (sizeof(tls_extensions) / sizeof(*tls_extensions)) diff --git a/src/lib/libssl/ssl_tlsext.h b/src/lib/libssl/ssl_tlsext.h index 38f8ffaa65..1e701e941a 100644 --- a/src/lib/libssl/ssl_tlsext.h +++ b/src/lib/libssl/ssl_tlsext.h @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_tlsext.h,v 1.5 2017/08/11 20:14:13 doug Exp $ */ +/* $OpenBSD: ssl_tlsext.h,v 1.6 2017/08/12 21:17:03 doug Exp $ */ /* * Copyright (c) 2016, 2017 Joel Sing * Copyright (c) 2017 Doug Hogan @@ -44,6 +44,12 @@ int tlsext_ecpf_serverhello_needs(SSL *s); int tlsext_ecpf_serverhello_build(SSL *s, CBB *cbb); int tlsext_ecpf_serverhello_parse(SSL *s, CBS *cbs, int *alert); +int tlsext_sessionticket_clienthello_needs(SSL *s); +int tlsext_sessionticket_clienthello_build(SSL *s, CBB *cbb); +int tlsext_sessionticket_clienthello_parse(SSL *s, CBS *cbs, int *alert); +int tlsext_sessionticket_serverhello_needs(SSL *s); +int tlsext_sessionticket_serverhello_build(SSL *s, CBB *cbb); +int tlsext_sessionticket_serverhello_parse(SSL *s, CBS *cbs, int *alert); int tlsext_clienthello_build(SSL *s, CBB *cbb); int tlsext_clienthello_parse_one(SSL *s, CBS *cbs, uint16_t tlsext_type, diff --git a/src/lib/libssl/t1_lib.c b/src/lib/libssl/t1_lib.c index 911e8d3f4e..63d401c337 100644 --- a/src/lib/libssl/t1_lib.c +++ b/src/lib/libssl/t1_lib.c @@ -1,4 +1,4 @@ -/* $OpenBSD: t1_lib.c,v 1.128 2017/08/12 21:03:08 jsing Exp $ */ +/* $OpenBSD: t1_lib.c,v 1.129 2017/08/12 21:17:03 doug Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -690,39 +690,6 @@ ssl_add_clienthello_tlsext(SSL *s, unsigned char *p, unsigned char *limit) return NULL; ret += len; - if (!(SSL_get_options(s) & SSL_OP_NO_TICKET)) { - int ticklen; - if (!s->internal->new_session && s->session && s->session->tlsext_tick) - ticklen = s->session->tlsext_ticklen; - else if (s->session && s->internal->tlsext_session_ticket && - s->internal->tlsext_session_ticket->data) { - ticklen = s->internal->tlsext_session_ticket->length; - s->session->tlsext_tick = malloc(ticklen); - if (!s->session->tlsext_tick) - return NULL; - memcpy(s->session->tlsext_tick, - s->internal->tlsext_session_ticket->data, ticklen); - s->session->tlsext_ticklen = ticklen; - } else - ticklen = 0; - if (ticklen == 0 && s->internal->tlsext_session_ticket && - s->internal->tlsext_session_ticket->data == NULL) - goto skip_ext; - /* Check for enough room 2 for extension type, 2 for len - * rest for ticket - */ - if ((size_t)(limit - ret) < 4 + ticklen) - return NULL; - s2n(TLSEXT_TYPE_session_ticket, ret); - - s2n(ticklen, ret); - if (ticklen) { - memcpy(ret, s->session->tlsext_tick, ticklen); - ret += ticklen; - } - } -skip_ext: - if (TLS1_get_client_version(s) >= TLS1_2_VERSION) { if ((size_t)(limit - ret) < sizeof(tls12_sigalgs) + 6) return NULL; @@ -884,15 +851,6 @@ ssl_add_serverhello_tlsext(SSL *s, unsigned char *p, unsigned char *limit) * extension. */ - if (s->internal->tlsext_ticket_expected && - !(SSL_get_options(s) & SSL_OP_NO_TICKET)) { - if ((size_t)(limit - ret) < 4) - return NULL; - - s2n(TLSEXT_TYPE_session_ticket, ret); - s2n(0, ret); - } - if (s->internal->tlsext_status_expected) { if ((size_t)(limit - ret) < 4) return NULL; @@ -1068,13 +1026,7 @@ ssl_parse_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *d, if (!tlsext_clienthello_parse_one(s, &cbs, type, al)) return 0; - if (type == TLSEXT_TYPE_session_ticket) { - if (s->internal->tls_session_ticket_ext_cb && - !s->internal->tls_session_ticket_ext_cb(s, data, size, s->internal->tls_session_ticket_ext_cb_arg)) { - *al = TLS1_AD_INTERNAL_ERROR; - return 0; - } - } else if (type == TLSEXT_TYPE_signature_algorithms) { + if (type == TLSEXT_TYPE_signature_algorithms) { int dsize; if (sigalg_seen || size < 2) { *al = SSL_AD_DECODE_ERROR; @@ -1277,19 +1229,7 @@ ssl_parse_serverhello_tlsext(SSL *s, unsigned char **p, size_t n, int *al) if (!tlsext_serverhello_parse_one(s, &cbs, type, al)) return 0; - if (type == TLSEXT_TYPE_session_ticket) { - if (s->internal->tls_session_ticket_ext_cb && - !s->internal->tls_session_ticket_ext_cb(s, data, size, s->internal->tls_session_ticket_ext_cb_arg)) { - *al = TLS1_AD_INTERNAL_ERROR; - return 0; - } - if ((SSL_get_options(s) & SSL_OP_NO_TICKET) || (size > 0)) { - *al = TLS1_AD_UNSUPPORTED_EXTENSION; - return 0; - } - s->internal->tlsext_ticket_expected = 1; - } - else if (type == TLSEXT_TYPE_status_request && + if (type == TLSEXT_TYPE_status_request && s->version != DTLS1_VERSION) { /* MUST be empty and only sent if we've requested * a status request message. diff --git a/src/regress/lib/libssl/tlsext/tlsexttest.c b/src/regress/lib/libssl/tlsext/tlsexttest.c index 880142202e..1267f50a49 100644 --- a/src/regress/lib/libssl/tlsext/tlsexttest.c +++ b/src/regress/lib/libssl/tlsext/tlsexttest.c @@ -1,4 +1,4 @@ -/* $OpenBSD: tlsexttest.c,v 1.9 2017/08/12 19:09:37 beck Exp $ */ +/* $OpenBSD: tlsexttest.c,v 1.10 2017/08/12 21:17:03 doug Exp $ */ /* * Copyright (c) 2017 Joel Sing * Copyright (c) 2017 Doug Hogan @@ -1298,6 +1298,304 @@ test_tlsext_sni_serverhello(void) return (failure); } +/* + * Session ticket - RFC 5077 since no known implementations use 4507. + * + * Session tickets can be length 0 (special case) to 2^16-1. + * + * The state is encrypted by the server so it is opaque to the client. + */ +static uint8_t tlsext_sessionticket_hello_min[1]; +static uint8_t tlsext_sessionticket_hello_max[65535]; + +static int +test_tlsext_sessionticket_clienthello(void) +{ + unsigned char *data = NULL; + SSL_CTX *ssl_ctx = NULL; + SSL *ssl = NULL; + int failure; + CBB cbb; + size_t dlen; + uint8_t dummy[1234]; + + failure = 1; + + CBB_init(&cbb, 0); + + /* Create fake session tickets with random data. */ + arc4random_buf(tlsext_sessionticket_hello_min, + sizeof(tlsext_sessionticket_hello_min)); + arc4random_buf(tlsext_sessionticket_hello_max, + sizeof(tlsext_sessionticket_hello_max)); + + if ((ssl_ctx = SSL_CTX_new(TLS_client_method())) == NULL) + errx(1, "failed to create SSL_CTX"); + if ((ssl = SSL_new(ssl_ctx)) == NULL) + errx(1, "failed to create SSL"); + + /* Should need a ticket by default. */ + if (!tlsext_sessionticket_clienthello_needs(ssl)) { + FAIL("clienthello should need Sessionticket for default " + "ciphers\n"); + goto err; + } + + /* Test disabling tickets. */ + if ((SSL_set_options(ssl, SSL_OP_NO_TICKET) & SSL_OP_NO_TICKET) == 0) { + FAIL("Cannot disable tickets in the TLS connection"); + return 0; + } + if (tlsext_sessionticket_clienthello_needs(ssl)) { + FAIL("clienthello should not need SessionTicket if it was disabled"); + goto err; + } + + /* Test re-enabling tickets. */ + if ((SSL_clear_options(ssl, SSL_OP_NO_TICKET) & SSL_OP_NO_TICKET) != 0) { + FAIL("Cannot re-enable tickets in the TLS connection"); + return 0; + } + if (!tlsext_sessionticket_clienthello_needs(ssl)) { + FAIL("clienthello should need SessionTicket if it was disabled"); + goto err; + } + + /* Since we don't have a session, we should build an empty ticket. */ + if (!tlsext_sessionticket_clienthello_build(ssl, &cbb)) { + FAIL("Cannot build a ticket"); + goto err; + } + if (!CBB_finish(&cbb, &data, &dlen)) { + FAIL("Cannot finish CBB"); + goto err; + } + if (dlen != 0) { + FAIL("Expected 0 length but found %zu\n", dlen); + goto err; + } + + CBB_cleanup(&cbb); + CBB_init(&cbb, 0); + free(data); + data = NULL; + + /* With a new session (but no ticket), we should still have 0 length */ + if ((ssl->session = SSL_SESSION_new()) == NULL) + errx(1, "failed to create session"); + if (!tlsext_sessionticket_clienthello_needs(ssl)) { + FAIL("Should still want a session ticket with a new session"); + goto err; + } + if (!tlsext_sessionticket_clienthello_build(ssl, &cbb)) { + FAIL("Cannot build a ticket"); + goto err; + } + if (!CBB_finish(&cbb, &data, &dlen)) { + FAIL("Cannot finish CBB"); + goto err; + } + if (dlen != 0) { + FAIL("Expected 0 length but found %zu\n", dlen); + goto err; + } + + CBB_cleanup(&cbb); + CBB_init(&cbb, 0); + free(data); + data = NULL; + + /* With a new session (and ticket), we should use that ticket */ + SSL_SESSION_free(ssl->session); + if ((ssl->session = SSL_SESSION_new()) == NULL) + errx(1, "failed to create session"); + + arc4random_buf(&dummy, sizeof(dummy)); + if ((ssl->session->tlsext_tick = malloc(sizeof(dummy))) == NULL) { + errx(1, "failed to malloc"); + } + memcpy(ssl->session->tlsext_tick, dummy, sizeof(dummy)); + ssl->session->tlsext_ticklen = sizeof(dummy); + + if (!tlsext_sessionticket_clienthello_needs(ssl)) { + FAIL("Should still want a session ticket with a new session"); + goto err; + } + if (!tlsext_sessionticket_clienthello_build(ssl, &cbb)) { + FAIL("Cannot build a ticket"); + goto err; + } + if (!CBB_finish(&cbb, &data, &dlen)) { + FAIL("Cannot finish CBB"); + goto err; + } + if (dlen != sizeof(dummy)) { + FAIL("Expected %zu length but found %zu\n", sizeof(dummy), dlen); + goto err; + } + if (memcmp(data, dummy, dlen) != 0) { + FAIL("serverhello SNI differs:\n"); + compare_data(data, dlen, + dummy, sizeof(dummy)); + goto err; + } + + CBB_cleanup(&cbb); + CBB_init(&cbb, 0); + free(data); + data = NULL; + free(ssl->session->tlsext_tick); + ssl->session->tlsext_tick = NULL; + ssl->session->tlsext_ticklen = 0; + + /* + * Send in NULL to disable session tickets at runtime without going + * through SSL_set_options(). + */ + if (!SSL_set_session_ticket_ext(ssl, NULL, 0)) { + FAIL("Could not set a NULL custom ticket"); + goto err; + } + /* Should not need a ticket in this case */ + if (tlsext_sessionticket_clienthello_needs(ssl)) { + FAIL("Should not want to use session tickets with a NULL custom"); + goto err; + } + + /* + * If you want to remove the tlsext_session_ticket behavior, you have + * to do it manually. + */ + free(ssl->internal->tlsext_session_ticket); + ssl->internal->tlsext_session_ticket = NULL; + + if (!tlsext_sessionticket_clienthello_needs(ssl)) { + FAIL("Should need a session ticket again when the custom one is removed"); + goto err; + } + + /* Test a custom session ticket (not recommended in practice) */ + if (!SSL_set_session_ticket_ext(ssl, tlsext_sessionticket_hello_max, + sizeof(tlsext_sessionticket_hello_max))) { + FAIL("Should be able to set a custom ticket"); + goto err; + } + if (!tlsext_sessionticket_clienthello_needs(ssl)) { + FAIL("Should need a session ticket again when the custom one is not empty"); + goto err; + } + if (!tlsext_sessionticket_clienthello_build(ssl, &cbb)) { + FAIL("Cannot build a ticket with a max length random payload"); + goto err; + } + if (!CBB_finish(&cbb, &data, &dlen)) { + FAIL("Cannot finish CBB"); + goto err; + } + if (dlen != sizeof(tlsext_sessionticket_hello_max)) { + FAIL("Expected %zu length but found %zu\n", + sizeof(tlsext_sessionticket_hello_max), dlen); + goto err; + } + if (memcmp(data, tlsext_sessionticket_hello_max, + sizeof(tlsext_sessionticket_hello_max)) != 0) { + FAIL("Expected to get what we passed in"); + compare_data(data, dlen, + tlsext_sessionticket_hello_max, + sizeof(tlsext_sessionticket_hello_max)); + goto err; + } + + failure = 0; + + err: + CBB_cleanup(&cbb); + SSL_CTX_free(ssl_ctx); + SSL_free(ssl); + free(data); + + return (failure); +} + + +static int +test_tlsext_sessionticket_serverhello(void) +{ + SSL_CTX *ssl_ctx = NULL; + SSL *ssl = NULL; + int failure; + uint8_t *data; + size_t dlen; + CBB cbb; + + CBB_init(&cbb, 0); + + failure = 1; + + if ((ssl_ctx = SSL_CTX_new(TLS_server_method())) == NULL) + errx(1, "failed to create SSL_CTX"); + if ((ssl = SSL_new(ssl_ctx)) == NULL) + errx(1, "failed to create SSL"); + + /* + * By default, should not need a session ticket since the ticket + * is not yet expected. + */ + if (tlsext_sessionticket_serverhello_needs(ssl)) { + FAIL("serverhello should not need SessionTicket by default\n"); + goto err; + } + + /* Test disabling tickets. */ + if ((SSL_set_options(ssl, SSL_OP_NO_TICKET) & SSL_OP_NO_TICKET) == 0) { + FAIL("Cannot disable tickets in the TLS connection"); + return 0; + } + if (tlsext_sessionticket_serverhello_needs(ssl)) { + FAIL("serverhello should not need SessionTicket if it was disabled"); + goto err; + } + + /* Test re-enabling tickets. */ + if ((SSL_clear_options(ssl, SSL_OP_NO_TICKET) & SSL_OP_NO_TICKET) != 0) { + FAIL("Cannot re-enable tickets in the TLS connection"); + return 0; + } + if (tlsext_sessionticket_serverhello_needs(ssl)) { + FAIL("serverhello should not need SessionTicket yet"); + goto err; + } + + /* Set expected to require it. */ + ssl->internal->tlsext_ticket_expected = 1; + if (!tlsext_sessionticket_serverhello_needs(ssl)) { + FAIL("serverhello should now be required for SessionTicket"); + goto err; + } + + /* server hello's session ticket should always be 0 length payload. */ + if (!tlsext_sessionticket_serverhello_build(ssl, &cbb)) { + FAIL("Cannot build a ticket with a max length random payload"); + goto err; + } + if (!CBB_finish(&cbb, &data, &dlen)) { + FAIL("Cannot finish CBB"); + goto err; + } + if (dlen != 0) { + FAIL("Expected 0 length but found %zu\n", dlen); + goto err; + } + + failure = 0; + + err: + SSL_CTX_free(ssl_ctx); + SSL_free(ssl); + + return (failure); +} + int main(int argc, char **argv) { @@ -1317,5 +1615,8 @@ main(int argc, char **argv) failed |= test_tlsext_sni_clienthello(); failed |= test_tlsext_sni_serverhello(); + failed |= test_tlsext_sessionticket_clienthello(); + failed |= test_tlsext_sessionticket_serverhello(); + return (failed); } -- cgit v1.2.3-55-g6feb