From 8ccc394472c04770d6b419b298c71feb8a152a24 Mon Sep 17 00:00:00 2001 From: jsing Date: Sat, 4 Sep 2021 16:26:12 +0000 Subject: [PATCH] Factor out the TLSv1.3 code that handles content from TLS records. Currently, the plaintext content from opened TLS records is handled via the rbuf code in the TLSv1.3 record layer. Factor this out and provide a separate struct tls_content, which knows how to track and manipulate the content. This makes the TLSv1.3 code cleaner, however it will also soon also be used to untangle parts of the legacy record layer. ok beck@ tb@ --- lib/libssl/Makefile | 5 +- lib/libssl/tls13_internal.h | 4 +- lib/libssl/tls13_legacy.c | 4 +- lib/libssl/tls13_record_layer.c | 108 ++++++++--------------- lib/libssl/tls_content.c | 149 ++++++++++++++++++++++++++++++++ lib/libssl/tls_content.h | 48 ++++++++++ 6 files changed, 238 insertions(+), 80 deletions(-) create mode 100644 lib/libssl/tls_content.c create mode 100644 lib/libssl/tls_content.h diff --git a/lib/libssl/Makefile b/lib/libssl/Makefile index 61711946297..d468308c7ef 100644 --- a/lib/libssl/Makefile +++ b/lib/libssl/Makefile @@ -1,4 +1,4 @@ -# $OpenBSD: Makefile,v 1.70 2021/05/05 10:05:27 jsing Exp $ +# $OpenBSD: Makefile,v 1.71 2021/09/04 16:26:12 jsing Exp $ .include .ifndef NOMAN @@ -81,7 +81,8 @@ SRCS= \ tls13_lib.c \ tls13_record.c \ tls13_record_layer.c \ - tls13_server.c + tls13_server.c \ + tls_content.c HDRS= dtls1.h srtp.h ssl.h ssl2.h ssl23.h ssl3.h tls1.h diff --git a/lib/libssl/tls13_internal.h b/lib/libssl/tls13_internal.h index 30ef7dd9312..12ed733f2b2 100644 --- a/lib/libssl/tls13_internal.h +++ b/lib/libssl/tls13_internal.h @@ -1,4 +1,4 @@ -/* $OpenBSD: tls13_internal.h,v 1.90 2021/06/28 15:35:14 tb Exp $ */ +/* $OpenBSD: tls13_internal.h,v 1.91 2021/09/04 16:26:12 jsing Exp $ */ /* * Copyright (c) 2018 Bob Beck * Copyright (c) 2018 Theo Buehler @@ -209,7 +209,7 @@ struct tls13_record_layer *tls13_record_layer_new( void tls13_record_layer_free(struct tls13_record_layer *rl); void tls13_record_layer_allow_ccs(struct tls13_record_layer *rl, int allow); void tls13_record_layer_allow_legacy_alerts(struct tls13_record_layer *rl, int allow); -void tls13_record_layer_rbuf(struct tls13_record_layer *rl, CBS *cbs); +void tls13_record_layer_rcontent(struct tls13_record_layer *rl, CBS *cbs); void tls13_record_layer_set_aead(struct tls13_record_layer *rl, const EVP_AEAD *aead); void tls13_record_layer_set_hash(struct tls13_record_layer *rl, diff --git a/lib/libssl/tls13_legacy.c b/lib/libssl/tls13_legacy.c index 477d09d63ed..df4408d9039 100644 --- a/lib/libssl/tls13_legacy.c +++ b/lib/libssl/tls13_legacy.c @@ -1,4 +1,4 @@ -/* $OpenBSD: tls13_legacy.c,v 1.28 2021/09/03 13:16:54 jsing Exp $ */ +/* $OpenBSD: tls13_legacy.c,v 1.29 2021/09/04 16:26:12 jsing Exp $ */ /* * Copyright (c) 2018, 2019 Joel Sing * @@ -304,7 +304,7 @@ tls13_use_legacy_stack(struct tls13_ctx *ctx) goto err; /* Stash any unprocessed data from the last record. */ - tls13_record_layer_rbuf(ctx->rl, &cbs); + tls13_record_layer_rcontent(ctx->rl, &cbs); if (CBS_len(&cbs) > 0) { if (!CBB_init_fixed(&cbb, S3I(s)->rbuf.buf, S3I(s)->rbuf.len)) diff --git a/lib/libssl/tls13_record_layer.c b/lib/libssl/tls13_record_layer.c index 6556547353f..2e32cb8a37c 100644 --- a/lib/libssl/tls13_record_layer.c +++ b/lib/libssl/tls13_record_layer.c @@ -1,4 +1,4 @@ -/* $OpenBSD: tls13_record_layer.c,v 1.62 2021/06/08 18:05:47 tb Exp $ */ +/* $OpenBSD: tls13_record_layer.c,v 1.63 2021/09/04 16:26:12 jsing Exp $ */ /* * Copyright (c) 2018, 2019 Joel Sing * @@ -17,6 +17,7 @@ #include "tls13_internal.h" #include "tls13_record.h" +#include "tls_content.h" static ssize_t tls13_record_layer_write_chunk(struct tls13_record_layer *rl, uint8_t content_type, const uint8_t *buf, size_t n); @@ -99,11 +100,8 @@ struct tls13_record_layer { uint8_t *phh_data; size_t phh_len; - /* Buffer containing plaintext from opened records. */ - uint8_t rbuf_content_type; - uint8_t *rbuf; - size_t rbuf_len; - CBS rbuf_cbs; + /* Content from opened records. */ + struct tls_content *rcontent; /* Record protection. */ const EVP_MD *hash; @@ -116,16 +114,6 @@ struct tls13_record_layer { void *cb_arg; }; -static void -tls13_record_layer_rbuf_free(struct tls13_record_layer *rl) -{ - CBS_init(&rl->rbuf_cbs, NULL, 0); - freezero(rl->rbuf, rl->rbuf_len); - rl->rbuf = NULL; - rl->rbuf_len = 0; - rl->rbuf_content_type = 0; -} - static void tls13_record_layer_rrec_free(struct tls13_record_layer *rl) { @@ -149,6 +137,9 @@ tls13_record_layer_new(const struct tls13_record_layer_callbacks *callbacks, if ((rl = calloc(1, sizeof(struct tls13_record_layer))) == NULL) goto err; + if ((rl->rcontent = tls_content_new()) == NULL) + goto err; + if ((rl->read = tls13_record_protection_new()) == NULL) goto err; if ((rl->write = tls13_record_protection_new()) == NULL) @@ -178,7 +169,7 @@ tls13_record_layer_free(struct tls13_record_layer *rl) freezero(rl->alert_data, rl->alert_len); freezero(rl->phh_data, rl->phh_len); - tls13_record_layer_rbuf_free(rl); + tls_content_free(rl->rcontent); tls13_record_protection_free(rl->read); tls13_record_protection_free(rl->write); @@ -187,9 +178,9 @@ tls13_record_layer_free(struct tls13_record_layer *rl) } void -tls13_record_layer_rbuf(struct tls13_record_layer *rl, CBS *cbs) +tls13_record_layer_rcontent(struct tls13_record_layer *rl, CBS *cbs) { - CBS_dup(&rl->rbuf_cbs, cbs); + CBS_dup(tls_content_cbs(rl->rcontent), cbs); } static const uint8_t tls13_max_seq_num[TLS13_RECORD_SEQ_NUM_LEN] = { @@ -292,22 +283,18 @@ tls13_record_layer_process_alert(struct tls13_record_layer *rl) * will result in one of three things - continuation (user_cancelled), * read channel closure (close_notify) or termination (all others). */ - if (rl->rbuf == NULL) + if (tls_content_type(rl->rcontent) != SSL3_RT_ALERT) return TLS13_IO_FAILURE; - if (rl->rbuf_content_type != SSL3_RT_ALERT) - return TLS13_IO_FAILURE; - - if (!CBS_get_u8(&rl->rbuf_cbs, &alert_level)) + if (!CBS_get_u8(tls_content_cbs(rl->rcontent), &alert_level)) return tls13_send_alert(rl, TLS13_ALERT_DECODE_ERROR); - - if (!CBS_get_u8(&rl->rbuf_cbs, &alert_desc)) + if (!CBS_get_u8(tls_content_cbs(rl->rcontent), &alert_desc)) return tls13_send_alert(rl, TLS13_ALERT_DECODE_ERROR); - if (CBS_len(&rl->rbuf_cbs) != 0) + if (tls_content_remaining(rl->rcontent) != 0) return tls13_send_alert(rl, TLS13_ALERT_DECODE_ERROR); - tls13_record_layer_rbuf_free(rl); + tls_content_clear(rl->rcontent); /* * Alert level is ignored for closure alerts (RFC 8446 section 6.1), @@ -531,15 +518,10 @@ tls13_record_layer_open_record_plaintext(struct tls13_record_layer *rl) return 0; } - tls13_record_layer_rbuf_free(rl); - - if (!CBS_stow(&cbs, &rl->rbuf, &rl->rbuf_len)) + if (!tls_content_dup_data(rl->rcontent, + tls13_record_content_type(rl->rrec), CBS_data(&cbs), CBS_len(&cbs))) return 0; - rl->rbuf_content_type = tls13_record_content_type(rl->rrec); - - CBS_init(&rl->rbuf_cbs, rl->rbuf, rl->rbuf_len); - return 1; } @@ -604,13 +586,7 @@ tls13_record_layer_open_record_protected(struct tls13_record_layer *rl) } content_type = content[inner_len]; - tls13_record_layer_rbuf_free(rl); - - rl->rbuf_content_type = content_type; - rl->rbuf = content; - rl->rbuf_len = inner_len; - - CBS_init(&rl->rbuf_cbs, rl->rbuf, rl->rbuf_len); + tls_content_set_data(rl->rcontent, content_type, content, inner_len); return 1; @@ -877,12 +853,12 @@ tls13_record_layer_read_record(struct tls13_record_layer *rl) * we must terminate the connection with an unexpected_message alert. * See RFC 8446 section 5.4. */ - if (CBS_len(&rl->rbuf_cbs) == 0 && - (rl->rbuf_content_type == SSL3_RT_ALERT || - rl->rbuf_content_type == SSL3_RT_HANDSHAKE)) + if (tls_content_remaining(rl->rcontent) == 0 && + (tls_content_type(rl->rcontent) == SSL3_RT_ALERT || + tls_content_type(rl->rcontent) == SSL3_RT_HANDSHAKE)) return tls13_send_alert(rl, TLS13_ALERT_UNEXPECTED_MESSAGE); - switch (rl->rbuf_content_type) { + switch (tls_content_type(rl->rcontent)) { case SSL3_RT_ALERT: return tls13_record_layer_process_alert(rl); @@ -907,10 +883,10 @@ tls13_record_layer_read_record(struct tls13_record_layer *rl) static ssize_t tls13_record_layer_pending(struct tls13_record_layer *rl, uint8_t content_type) { - if (rl->rbuf_content_type != content_type) + if (tls_content_type(rl->rcontent) != content_type) return 0; - return CBS_len(&rl->rbuf_cbs); + return tls_content_remaining(rl->rcontent); } static ssize_t @@ -929,9 +905,9 @@ tls13_record_layer_recv_phh(struct tls13_record_layer *rl) * TLS13_IO_FAILURE something broke. */ if (rl->cb.phh_recv != NULL) - ret = rl->cb.phh_recv(rl->cb_arg, &rl->rbuf_cbs); + ret = rl->cb.phh_recv(rl->cb_arg, tls_content_cbs(rl->rcontent)); - tls13_record_layer_rbuf_free(rl); + tls_content_clear(rl->rcontent); /* Leave post handshake handshake mode unless we need more data. */ if (ret != TLS13_IO_WANT_POLLIN) @@ -960,7 +936,7 @@ tls13_record_layer_read_internal(struct tls13_record_layer *rl, return TLS13_IO_EOF; /* If necessary, pull up the next record. */ - if (CBS_len(&rl->rbuf_cbs) == 0) { + if (tls_content_remaining(rl->rcontent) == 0) { if ((ret = tls13_record_layer_read_record(rl)) <= 0) return ret; @@ -968,17 +944,15 @@ tls13_record_layer_read_internal(struct tls13_record_layer *rl, * We may have read a valid 0-byte application data record, * in which case we need to read the next record. */ - if (CBS_len(&rl->rbuf_cbs) == 0) { - tls13_record_layer_rbuf_free(rl); + if (tls_content_remaining(rl->rcontent) == 0) return TLS13_IO_WANT_POLLIN; - } } /* * If we are in post handshake handshake mode, we must not see * any record type that isn't a handshake until we are done. */ - if (rl->phh && rl->rbuf_content_type != SSL3_RT_HANDSHAKE) + if (rl->phh && tls_content_type(rl->rcontent) != SSL3_RT_HANDSHAKE) return tls13_send_alert(rl, TLS13_ALERT_UNEXPECTED_MESSAGE); /* @@ -987,32 +961,18 @@ tls13_record_layer_read_internal(struct tls13_record_layer *rl, * be trying to read application data and need to handle a * post-handshake handshake message instead... */ - if (rl->rbuf_content_type != content_type) { - if (rl->rbuf_content_type == SSL3_RT_HANDSHAKE) { + if (tls_content_type(rl->rcontent) != content_type) { + if (tls_content_type(rl->rcontent) == SSL3_RT_HANDSHAKE) { if (rl->handshake_completed) return tls13_record_layer_recv_phh(rl); } return tls13_send_alert(rl, TLS13_ALERT_UNEXPECTED_MESSAGE); } - if (n > CBS_len(&rl->rbuf_cbs)) - n = CBS_len(&rl->rbuf_cbs); + if (peek) + return tls_content_peek(rl->rcontent, buf, n); - /* XXX - CBS_memcpy? CBS_copy_bytes? */ - memcpy(buf, CBS_data(&rl->rbuf_cbs), n); - - if (!peek) { - if (!CBS_skip(&rl->rbuf_cbs, n)) - goto err; - } - - if (CBS_len(&rl->rbuf_cbs) == 0) - tls13_record_layer_rbuf_free(rl); - - return n; - - err: - return TLS13_IO_FAILURE; + return tls_content_read(rl->rcontent, buf, n); } static ssize_t diff --git a/lib/libssl/tls_content.c b/lib/libssl/tls_content.c new file mode 100644 index 00000000000..ede178f84cd --- /dev/null +++ b/lib/libssl/tls_content.c @@ -0,0 +1,149 @@ +/* $OpenBSD: tls_content.c,v 1.1 2021/09/04 16:26:12 jsing Exp $ */ +/* + * Copyright (c) 2020 Joel Sing + * + * Permission to use, copy, modify, and distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */ + +#include +#include + +#include "tls_content.h" + +/* Content from a TLS record. */ +struct tls_content { + uint8_t type; + uint16_t epoch; + + const uint8_t *data; + size_t len; + CBS cbs; +}; + +struct tls_content * +tls_content_new(void) +{ + return calloc(1, sizeof(struct tls_content)); +} + +void +tls_content_clear(struct tls_content *content) +{ + freezero((void *)content->data, content->len); + memset(content, 0, sizeof(*content)); +} + +void +tls_content_free(struct tls_content *content) +{ + if (content == NULL) + return; + + tls_content_clear(content); + + freezero(content, sizeof(struct tls_content)); +} + +CBS * +tls_content_cbs(struct tls_content *content) +{ + return &content->cbs; +} + +int +tls_content_equal(struct tls_content *content, const uint8_t *buf, size_t n) +{ + return CBS_mem_equal(&content->cbs, buf, n); +} + +size_t +tls_content_remaining(struct tls_content *content) +{ + return CBS_len(&content->cbs); +} + +uint8_t +tls_content_type(struct tls_content *content) +{ + return content->type; +} + +int +tls_content_dup_data(struct tls_content *content, uint8_t type, + const uint8_t *data, size_t data_len) +{ + uint8_t *dup; + + if ((dup = calloc(1, data_len)) == NULL) + return 0; + memcpy(dup, data, data_len); + + tls_content_set_data(content, type, dup, data_len); + + return 1; +} + +uint16_t +tls_content_epoch(struct tls_content *content) +{ + return content->epoch; +} + +void +tls_content_set_epoch(struct tls_content *content, uint16_t epoch) +{ + content->epoch = epoch; +} + +void +tls_content_set_data(struct tls_content *content, uint8_t type, + const uint8_t *data, size_t data_len) +{ + tls_content_clear(content); + + content->type = type; + content->data = data; + content->len = data_len; + + CBS_init(&content->cbs, content->data, content->len); +} + +static ssize_t +tls_content_read_internal(struct tls_content *content, uint8_t *buf, size_t n, + int peek) +{ + if (n > CBS_len(&content->cbs)) + n = CBS_len(&content->cbs); + + /* XXX - CBS_memcpy? CBS_copy_bytes? */ + memcpy(buf, CBS_data(&content->cbs), n); + + if (!peek) { + if (!CBS_skip(&content->cbs, n)) + return -1; + } + + return n; +} + +ssize_t +tls_content_peek(struct tls_content *content, uint8_t *buf, size_t n) +{ + return tls_content_read_internal(content, buf, n, 1); +} + +ssize_t +tls_content_read(struct tls_content *content, uint8_t *buf, size_t n) +{ + return tls_content_read_internal(content, buf, n, 0); +} diff --git a/lib/libssl/tls_content.h b/lib/libssl/tls_content.h new file mode 100644 index 00000000000..173af2a740c --- /dev/null +++ b/lib/libssl/tls_content.h @@ -0,0 +1,48 @@ +/* $OpenBSD: tls_content.h,v 1.1 2021/09/04 16:26:12 jsing Exp $ */ +/* + * Copyright (c) 2020 Joel Sing + * + * Permission to use, copy, modify, and distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */ + +#ifndef HEADER_TLS_CONTENT_H +#define HEADER_TLS_CONTENT_H + +#include "bytestring.h" + +__BEGIN_HIDDEN_DECLS + +struct tls_content; + +struct tls_content *tls_content_new(void); +void tls_content_clear(struct tls_content *content); +void tls_content_free(struct tls_content *content); + +CBS *tls_content_cbs(struct tls_content *content); +int tls_content_equal(struct tls_content *content, const uint8_t *buf, size_t n); +size_t tls_content_remaining(struct tls_content *content); +uint8_t tls_content_type(struct tls_content *content); +uint16_t tls_content_epoch(struct tls_content *content); + +int tls_content_dup_data(struct tls_content *content, uint8_t type, + const uint8_t *data, size_t data_len); +void tls_content_set_data(struct tls_content *content, uint8_t type, + const uint8_t *data, size_t data_len); +void tls_content_set_epoch(struct tls_content *content, uint16_t epoch); + +ssize_t tls_content_peek(struct tls_content *content, uint8_t *buf, size_t n); +ssize_t tls_content_read(struct tls_content *content, uint8_t *buf, size_t n); + +__END_HIDDEN_DECLS + +#endif -- 2.20.1