From d14932b0913ef6f21bc09535d4eb1863708fefb6 Mon Sep 17 00:00:00 2001 From: beck <> Date: Sat, 12 Aug 2017 23:39:24 +0000 Subject: Rewrite the TLS status request extension to use the new TLS extension framework. ok jsing@ --- src/lib/libssl/ssl_tlsext.c | 171 +++++++++++++++++++++++++++- src/lib/libssl/ssl_tlsext.h | 9 +- src/lib/libssl/t1_lib.c | 176 +---------------------------- src/regress/lib/libssl/tlsext/tlsexttest.c | 129 ++++++++++++++++++++- 4 files changed, 307 insertions(+), 178 deletions(-) (limited to 'src') diff --git a/src/lib/libssl/ssl_tlsext.c b/src/lib/libssl/ssl_tlsext.c index 9db2d1ab41..646c59e5d6 100644 --- a/src/lib/libssl/ssl_tlsext.c +++ b/src/lib/libssl/ssl_tlsext.c @@ -1,7 +1,8 @@ -/* $OpenBSD: ssl_tlsext.c,v 1.8 2017/08/12 21:47:59 jsing Exp $ */ +/* $OpenBSD: ssl_tlsext.c,v 1.9 2017/08/12 23:38:12 beck Exp $ */ /* * Copyright (c) 2016, 2017 Joel Sing * Copyright (c) 2017 Doug Hogan + * Copyright (c) 2017 Bob Beck * * Permission to use, copy, modify, and distribute this software for any * purpose with or without fee is hereby granted, provided that the above @@ -15,6 +16,7 @@ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ +#include #include "ssl_locl.h" @@ -550,6 +552,160 @@ tlsext_sni_serverhello_parse(SSL *s, CBS *cbs, int *alert) return 1; } +/* + *Certificate Status Request - RFC 6066 section 8. + */ + +int +tlsext_ocsp_clienthello_needs(SSL *s) +{ + return (s->tlsext_status_type == TLSEXT_STATUSTYPE_ocsp && + s->version != DTLS1_VERSION); +} + +int +tlsext_ocsp_clienthello_build(SSL *s, CBB *cbb) +{ + CBB ocsp_respid_list, respid, exts; + unsigned char *ext_data; + size_t ext_len; + int i; + + if (!CBB_add_u8(cbb, TLSEXT_STATUSTYPE_ocsp)) + return 0; + if (!CBB_add_u16_length_prefixed(cbb, &ocsp_respid_list)) + return 0; + if (!CBB_add_u16_length_prefixed(cbb, &exts)) + return 0; + for (i = 0; i < sk_OCSP_RESPID_num(s->internal->tlsext_ocsp_ids); i++) { + unsigned char *respid_data; + OCSP_RESPID *id; + size_t id_len; + + if ((id = sk_OCSP_RESPID_value(s->internal->tlsext_ocsp_ids, + i)) == NULL) + return 0; + if ((id_len = i2d_OCSP_RESPID(id, NULL)) == -1) + return 0; + if (!CBB_add_u16_length_prefixed(&ocsp_respid_list, &respid)) + return 0; + if (!CBB_add_space(&respid, &respid_data, id_len)) + return 0; + if ((i2d_OCSP_RESPID(id, &respid_data)) != id_len) + return 0; + } + if ((ext_len = i2d_X509_EXTENSIONS(s->internal->tlsext_ocsp_exts, + NULL)) == -1) + return 0; + if (!CBB_add_space(&exts, &ext_data, ext_len)) + return 0; + if ((i2d_X509_EXTENSIONS(s->internal->tlsext_ocsp_exts, &ext_data) != + ext_len)) + return 0; + if (!CBB_flush(cbb)) + return 0; + return 1; +} + +int +tlsext_ocsp_clienthello_parse(SSL *s, CBS *cbs, int *alert) +{ + int failure = SSL_AD_DECODE_ERROR; + unsigned char *respid_data = NULL; + unsigned char *ext_data = NULL; + size_t ext_len, respid_len; + uint8_t status_type; + CBS respids, exts; + int ret = 0; + + if (!CBS_get_u8(cbs, &status_type)) + goto err; + if (status_type != TLSEXT_STATUSTYPE_ocsp) { + /* ignore unknown status types */ + s->tlsext_status_type = -1; + return 1; + } + s->tlsext_status_type = status_type; + if (!CBS_get_u16_length_prefixed(cbs, &respids)) + goto err; + + /* XXX */ + sk_OCSP_RESPID_pop_free(s->internal->tlsext_ocsp_ids, OCSP_RESPID_free); + s->internal->tlsext_ocsp_ids = NULL; + if (CBS_len(cbs) > 0) { + s->internal->tlsext_ocsp_ids = sk_OCSP_RESPID_new_null(); + if (s->internal->tlsext_ocsp_ids == NULL) { + failure = SSL_AD_INTERNAL_ERROR; + goto err; + } + } + + while (CBS_len(&respids) > 0) { + OCSP_RESPID *id = NULL; + + if (!CBS_stow(cbs, &respid_data, &respid_len)) + goto err; + if ((id = d2i_OCSP_RESPID(NULL, + (const unsigned char **)&respid_data, respid_len)) == NULL) + goto err; + if (!sk_OCSP_RESPID_push(s->internal->tlsext_ocsp_ids, id)) { + failure = SSL_AD_INTERNAL_ERROR; + OCSP_RESPID_free(id); + goto err; + } + free(respid_data); + respid_data = NULL; + } + + /* Read in request_extensions */ + if (!CBS_get_u16_length_prefixed(cbs, &exts)) + goto err; + if (!CBS_stow(&exts, &ext_data, &ext_len)) + goto err; + if (ext_len > 0) { + sk_X509_EXTENSION_pop_free(s->internal->tlsext_ocsp_exts, + X509_EXTENSION_free); + if ((s->internal->tlsext_ocsp_exts = d2i_X509_EXTENSIONS(NULL, + (const unsigned char **)&ext_data, ext_len)) == NULL) + goto err; + } + /* should be nothing left */ + if (CBS_len(cbs) > 0) + goto err; + + ret = 1; + err: + free(respid_data); + free(ext_data); + if (ret == 0) + *alert = failure; + return ret; +} + +int +tlsext_ocsp_serverhello_needs(SSL *s) +{ + return s->internal->tlsext_status_expected; +} + +int +tlsext_ocsp_serverhello_build(SSL *s, CBB *cbb) +{ + return 1; +} + +int +tlsext_ocsp_serverhello_parse(SSL *s, CBS *cbs, int *alert) +{ + if (s->tlsext_status_type == -1) { + *alert = TLS1_AD_UNSUPPORTED_EXTENSION; + return 0; + } + /* Set flag to expect CertificateStatus message */ + s->internal->tlsext_status_expected = 1; + return 1; +} + /* * SessionTicket extension - RFC 5077 section 3.2 */ @@ -704,6 +860,15 @@ static struct tls_extension tls_extensions[] = { .serverhello_build = tlsext_ri_serverhello_build, .serverhello_parse = tlsext_ri_serverhello_parse, }, + { + .type = TLSEXT_TYPE_status_request, + .clienthello_needs = tlsext_ocsp_clienthello_needs, + .clienthello_build = tlsext_ocsp_clienthello_build, + .clienthello_parse = tlsext_ocsp_clienthello_parse, + .serverhello_needs = tlsext_ocsp_serverhello_needs, + .serverhello_build = tlsext_ocsp_serverhello_build, + .serverhello_parse = tlsext_ocsp_serverhello_parse, + }, { .type = TLSEXT_TYPE_ec_point_formats, .clienthello_needs = tlsext_ecpf_clienthello_needs, @@ -758,7 +923,7 @@ tlsext_clienthello_build(SSL *s, CBB *cbb) if (!tlsext->clienthello_needs(s)) continue; - + if (!CBB_add_u16(cbb, tlsext->type)) return 0; if (!CBB_add_u16_length_prefixed(cbb, &extension_data)) @@ -811,7 +976,7 @@ tlsext_serverhello_build(SSL *s, CBB *cbb) if (!tlsext->serverhello_needs(s)) continue; - + if (!CBB_add_u16(cbb, tlsext->type)) return 0; if (!CBB_add_u16_length_prefixed(cbb, &extension_data)) diff --git a/src/lib/libssl/ssl_tlsext.h b/src/lib/libssl/ssl_tlsext.h index 4f8ae0cf35..bba8bdbea9 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.7 2017/08/12 21:47:59 jsing Exp $ */ +/* $OpenBSD: ssl_tlsext.h,v 1.8 2017/08/12 23:38:12 beck Exp $ */ /* * Copyright (c) 2016, 2017 Joel Sing * Copyright (c) 2017 Doug Hogan @@ -51,6 +51,13 @@ 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_ocsp_clienthello_needs(SSL *s); +int tlsext_ocsp_clienthello_build(SSL *s, CBB *cbb); +int tlsext_ocsp_clienthello_parse(SSL *s, CBS *cbs, int *alert); +int tlsext_ocsp_serverhello_needs(SSL *s); +int tlsext_ocsp_serverhello_build(SSL *s, CBB *cbb); +int tlsext_ocsp_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); diff --git a/src/lib/libssl/t1_lib.c b/src/lib/libssl/t1_lib.c index e27a7d1a59..405f08ed33 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.130 2017/08/12 21:47:59 jsing Exp $ */ +/* $OpenBSD: t1_lib.c,v 1.131 2017/08/12 23:38:12 beck Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -687,51 +687,6 @@ ssl_add_clienthello_tlsext(SSL *s, unsigned char *p, unsigned char *limit) return NULL; ret += len; - if (s->tlsext_status_type == TLSEXT_STATUSTYPE_ocsp && - s->version != DTLS1_VERSION) { - int i; - long extlen, idlen, itmp; - OCSP_RESPID *id; - - idlen = 0; - for (i = 0; i < sk_OCSP_RESPID_num(s->internal->tlsext_ocsp_ids); i++) { - id = sk_OCSP_RESPID_value(s->internal->tlsext_ocsp_ids, i); - itmp = i2d_OCSP_RESPID(id, NULL); - if (itmp <= 0) - return NULL; - idlen += itmp + 2; - } - - if (s->internal->tlsext_ocsp_exts) { - extlen = i2d_X509_EXTENSIONS(s->internal->tlsext_ocsp_exts, NULL); - if (extlen < 0) - return NULL; - } else - extlen = 0; - - if ((size_t)(limit - ret) < 7 + extlen + idlen) - return NULL; - s2n(TLSEXT_TYPE_status_request, ret); - if (extlen + idlen > 0xFFF0) - return NULL; - s2n(extlen + idlen + 5, ret); - *(ret++) = TLSEXT_STATUSTYPE_ocsp; - s2n(idlen, ret); - for (i = 0; i < sk_OCSP_RESPID_num(s->internal->tlsext_ocsp_ids); i++) { - /* save position of id len */ - unsigned char *q = ret; - id = sk_OCSP_RESPID_value(s->internal->tlsext_ocsp_ids, i); - /* skip over id len */ - ret += 2; - itmp = i2d_OCSP_RESPID(id, &ret); - /* write id len */ - s2n(itmp, q); - } - s2n(extlen, ret); - if (extlen > 0) - i2d_X509_EXTENSIONS(s->internal->tlsext_ocsp_exts, &ret); - } - if (s->internal->alpn_client_proto_list != NULL && S3I(s)->tmp.finish_md_len == 0) { if ((size_t)(limit - ret) < @@ -837,14 +792,6 @@ ssl_add_serverhello_tlsext(SSL *s, unsigned char *p, unsigned char *limit) * extension. */ - if (s->internal->tlsext_status_expected) { - if ((size_t)(limit - ret) < 4) - return NULL; - - s2n(TLSEXT_TYPE_status_request, ret); - s2n(0, ret); - } - #ifndef OPENSSL_NO_SRTP if (SSL_IS_DTLS(s) && s->internal->srtp_profile) { int el; @@ -1011,111 +958,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_status_request && - s->version != DTLS1_VERSION) { - - if (size < 5) { - *al = SSL_AD_DECODE_ERROR; - return 0; - } - - s->tlsext_status_type = *data++; - size--; - if (s->tlsext_status_type == TLSEXT_STATUSTYPE_ocsp) { - const unsigned char *sdata; - int dsize; - /* Read in responder_id_list */ - n2s(data, dsize); - size -= 2; - if (dsize > size) { - *al = SSL_AD_DECODE_ERROR; - return 0; - } - - /* - * We remove any OCSP_RESPIDs from a - * previous handshake to prevent - * unbounded memory growth. - */ - sk_OCSP_RESPID_pop_free(s->internal->tlsext_ocsp_ids, - OCSP_RESPID_free); - s->internal->tlsext_ocsp_ids = NULL; - if (dsize > 0) { - s->internal->tlsext_ocsp_ids = - sk_OCSP_RESPID_new_null(); - if (s->internal->tlsext_ocsp_ids == NULL) { - *al = SSL_AD_INTERNAL_ERROR; - return 0; - } - } - - while (dsize > 0) { - OCSP_RESPID *id; - int idsize; - if (dsize < 4) { - *al = SSL_AD_DECODE_ERROR; - return 0; - } - n2s(data, idsize); - dsize -= 2 + idsize; - size -= 2 + idsize; - if (dsize < 0) { - *al = SSL_AD_DECODE_ERROR; - return 0; - } - sdata = data; - data += idsize; - id = d2i_OCSP_RESPID(NULL, - &sdata, idsize); - if (!id) { - *al = SSL_AD_DECODE_ERROR; - return 0; - } - if (data != sdata) { - OCSP_RESPID_free(id); - *al = SSL_AD_DECODE_ERROR; - return 0; - } - if (!sk_OCSP_RESPID_push( - s->internal->tlsext_ocsp_ids, id)) { - OCSP_RESPID_free(id); - *al = SSL_AD_INTERNAL_ERROR; - return 0; - } - } - - /* Read in request_extensions */ - if (size < 2) { - *al = SSL_AD_DECODE_ERROR; - return 0; - } - n2s(data, dsize); - size -= 2; - if (dsize != size) { - *al = SSL_AD_DECODE_ERROR; - return 0; - } - sdata = data; - if (dsize > 0) { - sk_X509_EXTENSION_pop_free(s->internal->tlsext_ocsp_exts, - X509_EXTENSION_free); - - s->internal->tlsext_ocsp_exts = - d2i_X509_EXTENSIONS(NULL, - &sdata, dsize); - if (!s->internal->tlsext_ocsp_exts || - (data + dsize != sdata)) { - *al = SSL_AD_DECODE_ERROR; - return 0; - } - } - } else { - /* We don't know what to do with any other type - * so ignore it. - */ - s->tlsext_status_type = -1; - } - } else if (type == + if (type == TLSEXT_TYPE_application_layer_protocol_negotiation && s->ctx->internal->alpn_select_cb != NULL && S3I(s)->tmp.finish_md_len == 0) { @@ -1123,7 +966,6 @@ ssl_parse_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *d, size, al) != 1) return (0); } - /* session ticket processed earlier */ #ifndef OPENSSL_NO_SRTP else if (SSL_IS_DTLS(s) && type == TLSEXT_TYPE_use_srtp) { @@ -1197,19 +1039,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_status_request && - s->version != DTLS1_VERSION) { - /* MUST be empty and only sent if we've requested - * a status request message. - */ - if ((s->tlsext_status_type == -1) || (size > 0)) { - *al = TLS1_AD_UNSUPPORTED_EXTENSION; - return 0; - } - /* Set flag to expect CertificateStatus message */ - s->internal->tlsext_status_expected = 1; - } else if (type == - TLSEXT_TYPE_application_layer_protocol_negotiation) { + if (type == TLSEXT_TYPE_application_layer_protocol_negotiation) { unsigned int len; /* We must have requested it. */ diff --git a/src/regress/lib/libssl/tlsext/tlsexttest.c b/src/regress/lib/libssl/tlsext/tlsexttest.c index 073ba2f2f5..950588ba47 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.11 2017/08/12 21:49:28 jsing Exp $ */ +/* $OpenBSD: tlsexttest.c,v 1.12 2017/08/12 23:39:24 beck Exp $ */ /* * Copyright (c) 2017 Joel Sing * Copyright (c) 2017 Doug Hogan @@ -1457,6 +1457,130 @@ test_tlsext_sni_serverhello(void) return (failure); } +static unsigned char tls_ocsp_clienthello_default[] = { + 0x01, 0x00, 0x00, 0x00, 0x00 +}; + +static int +test_tlsext_ocsp_clienthello(void) +{ + unsigned char *data = NULL; + SSL_CTX *ssl_ctx = NULL; + SSL *ssl = NULL; + size_t dlen; + int failure; + int alert; + CBB cbb; + CBS cbs; + + failure = 1; + + CBB_init(&cbb, 0); + + 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"); + + if (tlsext_ocsp_clienthello_needs(ssl)) { + FAIL("clienthello should not need ocsp\n"); + goto err; + } + SSL_set_tlsext_status_type(ssl, TLSEXT_STATUSTYPE_ocsp); + + if (!tlsext_ocsp_clienthello_needs(ssl)) { + FAIL("clienthello should need ocsp\n"); + goto err; + } + if (!tlsext_ocsp_clienthello_build(ssl, &cbb)) { + FAIL("clienthello failed to build SNI\n"); + goto err; + } + if (!CBB_finish(&cbb, &data, &dlen)) + errx(1, "failed to finish CBB"); + + if (dlen != sizeof(tls_ocsp_clienthello_default)) { + FAIL("got ocsp clienthello with length %zu, " + "want length %zu\n", dlen, + sizeof(tls_ocsp_clienthello_default)); + goto err; + } + if (memcmp(data, tls_ocsp_clienthello_default, dlen) != 0) { + FAIL("ocsp clienthello differs:\n"); + fprintf(stderr, "received:\n"); + hexdump(data, dlen); + fprintf(stderr, "test data:\n"); + hexdump(tls_ocsp_clienthello_default, + sizeof(tls_ocsp_clienthello_default)); + goto err; + } + CBS_init(&cbs, tls_ocsp_clienthello_default, + sizeof(tls_ocsp_clienthello_default)); + if (!tlsext_ocsp_clienthello_parse(ssl, &cbs, &alert)) { + FAIL("failed to parse ocsp clienthello\n"); + goto err; + } + + failure = 0; + + err: + CBB_cleanup(&cbb); + SSL_CTX_free(ssl_ctx); + SSL_free(ssl); + free(data); + + return (failure); +} + +static int +test_tlsext_ocsp_serverhello(void) +{ + unsigned char *data = NULL; + SSL_CTX *ssl_ctx = NULL; + SSL *ssl = NULL; + size_t dlen; + int failure; + CBB cbb; + + failure = 1; + + CBB_init(&cbb, 0); + + 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"); + + if (tlsext_ocsp_serverhello_needs(ssl)) { + FAIL("serverhello should not need ocsp\n"); + goto err; + } + + ssl->internal->tlsext_status_expected = 1; + + if (!tlsext_ocsp_serverhello_needs(ssl)) { + FAIL("serverhello should need ocsp\n"); + goto err; + } + if (!tlsext_ocsp_serverhello_build(ssl, &cbb)) { + FAIL("serverhello failed to build ocsp\n"); + goto err; + } + + if (!CBB_finish(&cbb, &data, &dlen)) + errx(1, "failed to finish CBB"); + + failure = 0; + + err: + CBB_cleanup(&cbb); + SSL_CTX_free(ssl_ctx); + SSL_free(ssl); + free(data); + + return (failure); +} + /* * Session ticket - RFC 5077 since no known implementations use 4507. * @@ -1777,6 +1901,9 @@ main(int argc, char **argv) failed |= test_tlsext_sni_clienthello(); failed |= test_tlsext_sni_serverhello(); + failed |= test_tlsext_ocsp_clienthello(); + failed |= test_tlsext_ocsp_serverhello(); + failed |= test_tlsext_sessionticket_clienthello(); failed |= test_tlsext_sessionticket_serverhello(); -- cgit v1.2.3-55-g6feb