From d702ad121c18b43f61832318a9e61b8d42aaa06c Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Thu, 16 Mar 2017 10:18:39 +0000 Subject: [PATCH] Fix the Padding extension In OpenSSL 1.1.0 the padding extension MUST be last because it calculates the length of everything that has been written into the ClientHello to determine whether it needs to be padded or not. With TLSv1.3 that isn't possible because the specification requires that the PSK extension is last. Therefore we need to fix the padding extension to take account of any PSK extension that will be later added. Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/2968) --- ssl/statem/extensions.c | 1 - ssl/statem/extensions_clnt.c | 47 +++++++++++++++++++++++++++++++++--- 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c index f11f5e03b9..d62c5af3b6 100644 --- a/ssl/statem/extensions.c +++ b/ssl/statem/extensions.c @@ -289,7 +289,6 @@ static const EXTENSION_DEFINITION ext_defs[] = { }, { /* Must be immediately before pre_shared_key */ - /* TODO(TLS1.3): Fix me */ TLSEXT_TYPE_padding, EXT_CLIENT_HELLO, NULL, diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c index 84bfb3c265..400de3fb30 100644 --- a/ssl/statem/extensions_clnt.c +++ b/ssl/statem/extensions_clnt.c @@ -691,6 +691,20 @@ int tls_construct_ctos_early_data(SSL *s, WPACKET *pkt, unsigned int context, #define F5_WORKAROUND_MIN_MSG_LEN 0xff #define F5_WORKAROUND_MAX_MSG_LEN 0x200 +/* + * PSK pre binder overhead = + * 2 bytes for TLSEXT_TYPE_psk + * 2 bytes for extension length + * 2 bytes for identities list length + * 2 bytes for identity length + * 4 bytes for obfuscated_ticket_age + * 2 bytes for binder list length + * 1 byte for binder length + * The above excludes the number of bytes for the identity itself and the + * subsequent binder bytes + */ +#define PSK_PRE_BINDER_OVERHEAD (2 + 2 + 2 + 2 + 4 + 2 + 1) + int tls_construct_ctos_padding(SSL *s, WPACKET *pkt, unsigned int context, X509 *x, size_t chainidx, int *al) { @@ -701,16 +715,35 @@ int tls_construct_ctos_padding(SSL *s, WPACKET *pkt, unsigned int context, return 1; /* - * Add padding to workaround bugs in F5 terminators. See - * https://tools.ietf.org/html/draft-agl-tls-padding-03 NB: because this - * code calculates the length of all existing extensions it MUST always - * appear last. + * Add padding to workaround bugs in F5 terminators. See RFC7685. + * This code calculates the length of all extensions added so far but + * excludes the PSK extension (because that MUST be written last). Therefore + * this extension MUST always appear second to last. */ if (!WPACKET_get_total_written(pkt, &hlen)) { SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_PADDING, ERR_R_INTERNAL_ERROR); return 0; } + /* + * If we're going to send a PSK then that will be written out after this + * extension, so we need to calculate how long it is going to be. + */ + if (s->session->ssl_version == TLS1_3_VERSION + && s->session->ext.ticklen != 0 + && s->session->cipher != NULL) { + const EVP_MD *md = ssl_md(s->session->cipher->algorithm2); + + if (md != NULL) { + /* + * Add the fixed PSK overhead, the identity length and the binder + * length. + */ + hlen += PSK_PRE_BINDER_OVERHEAD + s->session->ext.ticklen + + EVP_MD_size(md); + } + } + if (hlen > F5_WORKAROUND_MIN_MSG_LEN && hlen < F5_WORKAROUND_MAX_MSG_LEN) { /* Calculate the amond of padding we need to add */ hlen = F5_WORKAROUND_MAX_MSG_LEN - hlen; @@ -750,6 +783,12 @@ int tls_construct_ctos_psk(SSL *s, WPACKET *pkt, unsigned int context, X509 *x, s->session->ext.tick_identity = TLSEXT_PSK_BAD_IDENTITY; + /* + * Note: At this stage of the code we only support adding a single + * resumption PSK. If we add support for multiple PSKs then the length + * calculations in the padding extension will need to be adjusted. + */ + /* * If this is an incompatible or new session then we have nothing to resume * so don't add this extension. -- 2.34.1