From 622f173fe044608da8e31c2322e322ee7e98b0b7 Mon Sep 17 00:00:00 2001 From: jsing Date: Wed, 20 Jul 2022 06:20:44 +0000 Subject: [PATCH] Correct server-side handling of TLSv1.3 key updates. The existing code updates the correct secret, however then sets it for the wrong direction. Fix this, while untangling the code and consistenly using 'read' and 'write' rather than 'local' and 'peer'. ok beck@ tb@ --- lib/libssl/tls13_lib.c | 50 +++++++++++++++++++++++++----------------- 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/lib/libssl/tls13_lib.c b/lib/libssl/tls13_lib.c index 6522c104d63..8b28bf55a45 100644 --- a/lib/libssl/tls13_lib.c +++ b/lib/libssl/tls13_lib.c @@ -1,4 +1,4 @@ -/* $OpenBSD: tls13_lib.c,v 1.65 2022/07/17 15:51:06 jsing Exp $ */ +/* $OpenBSD: tls13_lib.c,v 1.66 2022/07/20 06:20:44 jsing Exp $ */ /* * Copyright (c) 2018, 2019 Joel Sing * Copyright (c) 2019 Bob Beck @@ -215,31 +215,41 @@ tls13_legacy_ocsp_status_recv_cb(void *arg) } static int -tls13_phh_update_local_traffic_secret(struct tls13_ctx *ctx) +tls13_phh_update_read_traffic_secret(struct tls13_ctx *ctx) { struct tls13_secrets *secrets = ctx->hs->tls13.secrets; + struct tls13_secret *secret; - if (ctx->mode == TLS13_HS_CLIENT) - return (tls13_update_client_traffic_secret(secrets) && - tls13_record_layer_set_write_traffic_key(ctx->rl, - &secrets->client_application_traffic)); - return (tls13_update_server_traffic_secret(secrets) && - tls13_record_layer_set_read_traffic_key(ctx->rl, - &secrets->server_application_traffic)); + if (ctx->mode == TLS13_HS_CLIENT) { + secret = &secrets->server_application_traffic; + if (!tls13_update_server_traffic_secret(secrets)) + return 0; + } else { + secret = &secrets->client_application_traffic; + if (!tls13_update_client_traffic_secret(secrets)) + return 0; + } + + return tls13_record_layer_set_read_traffic_key(ctx->rl, secret); } static int -tls13_phh_update_peer_traffic_secret(struct tls13_ctx *ctx) +tls13_phh_update_write_traffic_secret(struct tls13_ctx *ctx) { struct tls13_secrets *secrets = ctx->hs->tls13.secrets; + struct tls13_secret *secret; + + if (ctx->mode == TLS13_HS_CLIENT) { + secret = &secrets->client_application_traffic; + if (!tls13_update_client_traffic_secret(secrets)) + return 0; + } else { + secret = &secrets->server_application_traffic; + if (!tls13_update_server_traffic_secret(secrets)) + return 0; + } - if (ctx->mode == TLS13_HS_CLIENT) - return (tls13_update_server_traffic_secret(secrets) && - tls13_record_layer_set_read_traffic_key(ctx->rl, - &secrets->server_application_traffic)); - return (tls13_update_client_traffic_secret(secrets) && - tls13_record_layer_set_write_traffic_key(ctx->rl, - &secrets->client_application_traffic)); + return tls13_record_layer_set_write_traffic_key(ctx->rl, secret); } /* @@ -285,13 +295,13 @@ tls13_key_update_recv(struct tls13_ctx *ctx, CBS *cbs) goto err; } - if (!tls13_phh_update_peer_traffic_secret(ctx)) + if (!tls13_phh_update_read_traffic_secret(ctx)) goto err; if (key_update_request == 0) return TLS13_IO_SUCCESS; - /* key_update_request == 1 */ + /* Our peer requested that we update our write traffic keys. */ if ((hs_msg = tls13_handshake_msg_new()) == NULL) goto err; if (!tls13_handshake_msg_start(hs_msg, &cbb_hs, TLS13_MT_KEY_UPDATE)) @@ -322,7 +332,7 @@ tls13_phh_done_cb(void *cb_arg) struct tls13_ctx *ctx = cb_arg; if (ctx->key_update_request) { - tls13_phh_update_local_traffic_secret(ctx); + tls13_phh_update_write_traffic_secret(ctx); ctx->key_update_request = 0; } } -- 2.20.1