Restore historical behavior for absent ServerHello extensions

In OpenSSL 1.1.0, when there were no extensions added to the ServerHello,
we did not write the extension data length bytes to the end of the
ServerHello; this is needed for compatibility with old client implementations
that do not support TLS extensions (such as the default configuration of
OpenSSL 0.9.8).  When ServerHello extension construction was converted
to the new extensions framework in commit
7da160b0f46d832dbf285cb0b48ae56d4a8b884d, this behavior was inadvertently
limited to cases when SSLv3 was negotiated (and similarly for ClientHellos),
presumably since extensions are not defined at all for SSLv3.  However,
extensions for TLS prior to TLS 1.3 have been defined in separate
RFCs (6066, 4366, and 3546) from the TLS protocol specifications, and as such
should be considered an optional protocol feature in those cases.

Accordingly, be conservative in what we send, and skip the extensions block
when there are no extensions to be sent, regardless of the TLS/SSL version.
(TLS 1.3 requires extensions and can safely be treated differently.)

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/4296)

diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index b8ab5c880e32dfb030f70255abd2d707dff32ca2..61203ed6a31dc144356ff18ed1e25b30f0df2dc6 100644 (file)
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -703,11 +703,11 @@ int tls_construct_extensions(SSL *s, WPACKET *pkt, unsigned int context,
     if (!WPACKET_start_sub_packet_u16(pkt)
                /*
                 * If extensions are of zero length then we don't even add the
     if (!WPACKET_start_sub_packet_u16(pkt)
                /*
                 * If extensions are of zero length then we don't even add the

-                * extensions length bytes to a ClientHello/ServerHello in SSLv3
+                * extensions length bytes to a ClientHello/ServerHello
+                * (for non-TLSv1.3).

                 */
             || ((context &
                  (SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS1_2_SERVER_HELLO)) != 0
                 */
             || ((context &
                  (SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS1_2_SERVER_HELLO)) != 0

-                && s->version == SSL3_VERSION

                 && !WPACKET_set_flags(pkt,
                                      WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH))) {
         SSLerr(SSL_F_TLS_CONSTRUCT_EXTENSIONS, ERR_R_INTERNAL_ERROR);
                 && !WPACKET_set_flags(pkt,
                                      WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH))) {
         SSLerr(SSL_F_TLS_CONSTRUCT_EXTENSIONS, ERR_R_INTERNAL_ERROR);