From 10ed1b72391ded9853bec417d4d32bd6ec45f916 Mon Sep 17 00:00:00 2001 From: Todd Short Date: Fri, 18 Aug 2017 09:32:29 -0400 Subject: [PATCH] Reorder extensions to put SigAlgs last Force non-empty padding extension. When enabled, force the padding extension to be at least 1 byte long. WebSphere application server cannot handle having an empty extension (e.g. EMS/EtM) as the last extension in a client hello. This moves the SigAlgs extension last for TLSv1.2 to avoid this issue. Reviewed-by: Matt Caswell Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/3921) --- ssl/ssl_locl.h | 2 +- ssl/statem/extensions.c | 17 ++++++++++------- ssl/statem/extensions_clnt.c | 6 ++++-- test/sslapitest.c | 2 +- 4 files changed, 16 insertions(+), 11 deletions(-) diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index 8b8625d7d5..f14148a438 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -678,7 +678,6 @@ typedef enum tlsext_index_en { TLSEXT_IDX_ec_point_formats, TLSEXT_IDX_supported_groups, TLSEXT_IDX_session_ticket, - TLSEXT_IDX_signature_algorithms, TLSEXT_IDX_status_request, TLSEXT_IDX_next_proto_neg, TLSEXT_IDX_application_layer_protocol_negotiation, @@ -686,6 +685,7 @@ typedef enum tlsext_index_en { TLSEXT_IDX_encrypt_then_mac, TLSEXT_IDX_signed_certificate_timestamp, TLSEXT_IDX_extended_master_secret, + TLSEXT_IDX_signature_algorithms, TLSEXT_IDX_supported_versions, TLSEXT_IDX_psk_kex_modes, TLSEXT_IDX_key_share, diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c index a5dda45a96..d569f6c251 100644 --- a/ssl/statem/extensions.c +++ b/ssl/statem/extensions.c @@ -110,6 +110,9 @@ typedef struct extensions_definition_st { * extension is relevant to a particular protocol or protocol version. * * TODO(TLS1.3): Make sure we have a test to check the consistency of these + * + * NOTE: WebSphere Application Server 7+ cannot handle empty extensions at + * the end, keep these extensions before signature_algorithm. */ #define INVALID_EXTENSION { 0x10000, 0, NULL, NULL, NULL, NULL, NULL, NULL } static const EXTENSION_DEFINITION ext_defs[] = { @@ -167,13 +170,6 @@ static const EXTENSION_DEFINITION ext_defs[] = { tls_parse_stoc_session_ticket, tls_construct_stoc_session_ticket, tls_construct_ctos_session_ticket, NULL }, - { - TLSEXT_TYPE_signature_algorithms, - SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS1_3_CERTIFICATE_REQUEST, - init_sig_algs, tls_parse_ctos_sig_algs, - tls_parse_ctos_sig_algs, tls_construct_ctos_sig_algs, - tls_construct_ctos_sig_algs, final_sig_algs - }, #ifndef OPENSSL_NO_OCSP { TLSEXT_TYPE_status_request, @@ -249,6 +245,13 @@ static const EXTENSION_DEFINITION ext_defs[] = { init_ems, tls_parse_ctos_ems, tls_parse_stoc_ems, tls_construct_stoc_ems, tls_construct_ctos_ems, final_ems }, + { + TLSEXT_TYPE_signature_algorithms, + SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS1_3_CERTIFICATE_REQUEST, + init_sig_algs, tls_parse_ctos_sig_algs, + tls_parse_ctos_sig_algs, tls_construct_ctos_sig_algs, + tls_construct_ctos_sig_algs, final_sig_algs + }, { TLSEXT_TYPE_supported_versions, SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS_IMPLEMENTATION_ONLY diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c index 35e7173190..b1c2eb0fff 100644 --- a/ssl/statem/extensions_clnt.c +++ b/ssl/statem/extensions_clnt.c @@ -765,12 +765,14 @@ EXT_RETURN tls_construct_ctos_padding(SSL *s, WPACKET *pkt, /* * Take off the size of extension header itself (2 bytes for type and - * 2 bytes for length bytes) + * 2 bytes for length bytes), but ensure that the extension is at least + * 1 byte long so as not to have an empty extension last (WebSphere 7.x, + * 8.x are intolerant of that condition) */ if (hlen >= 4) hlen -= 4; else - hlen = 0; + hlen = 1; if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_padding) || !WPACKET_sub_allocate_bytes_u16(pkt, hlen, &padbytes)) { diff --git a/test/sslapitest.c b/test/sslapitest.c index dfcbf11c90..571da55294 100644 --- a/test/sslapitest.c +++ b/test/sslapitest.c @@ -417,7 +417,7 @@ static int full_early_callback(SSL *s, int *al, void *arg) #ifndef OPENSSL_NO_EC 11, 10, #endif - 35, 13, 22, 23}; + 35, 22, 23, 13}; size_t len; /* Make sure we can defer processing and get called back. */ -- 2.34.1