When sending no extensions do not include a zero lengthtmp-fix-regression-ext-size

According to RFC5246:
   The presence of extensions can be detected by determining whether
   there are bytes following the compression_method field at the end of
   the ServerHello.

and as such we correct our behavior to not send the zero length bytes.
This was our behavior in 3.5.x and 3.3.x branch, and thus this corrects
a regression of gnutls with these branches.

Signed-off-by: Nikos Mavrogiannopoulos <nmav@redhat.com>
Signed-off-by: David Woodhouse <dwmw2@infradead.org>

8 files changed, 198 insertions, 7 deletions

diff --git a/NEWS b/NEWS
index 739ab21651..b109e78b6e 100644
--- a/NEWS
+++ b/NEWS
@@ -15,6 +15,11 @@ See the end for copying conditions.
    types via the priority strings. The raw public-key mechanism must be explicitly
    enabled via the GNUTLS_ENABLE_RAWPK init flag.
 
+** libgnutls: When on server or client side we are sending no extensions we do
+   not set an empty extensions field but we rather remove that field competely.
+   This solves a regression since 3.5.x and improves compatibility of the server
+   side with certain clients.
+
 ** GNUTLS_X509_NO_WELL_DEFINED_EXPIRATION was marked as deprecated. The previous
    definition was buggy and non-functional.
 
diff --git a/lib/extv.h b/lib/extv.h
index 9f13f7a2ce..c791d9bba9 100644
--- a/lib/extv.h
+++ b/lib/extv.h
@@ -48,9 +48,11 @@ int _gnutls_extv_append_init(gnutls_buffer_st *buf)
 	return pos;
 }
 
-/* its input is the buffer and the return value of _gnutls_extv_append_init() */
+/* its input is the buffer and the return value of _gnutls_extv_append_init()
+ * @is_hello: should be true for client and server hello messages.
+ */
 inline static
-int _gnutls_extv_append_final(gnutls_buffer_st *buf, unsigned init)
+int _gnutls_extv_append_final(gnutls_buffer_st *buf, unsigned init, unsigned is_hello)
 {
 	unsigned size = buf->length - init - 2;
 
@@ -59,6 +61,11 @@ int _gnutls_extv_append_final(gnutls_buffer_st *buf, unsigned init)
 
 	if (size > 0)
 		_gnutls_write_uint16(size, &buf->data[init]);
+	else if (is_hello && size == 0) {
+		/* there is no point to send empty extension bytes, and
+		 * they are known to break certain clients */
+		buf->length -= 2;
+	}
 
 	return 0;
 }
diff --git a/lib/hello_ext.c b/lib/hello_ext.c
index c4907aaced..5692a14d2d 100644
--- a/lib/hello_ext.c
+++ b/lib/hello_ext.c
@@ -442,7 +442,7 @@ _gnutls_gen_hello_extensions(gnutls_session_t session,
 				     session, ctx.ext->name, (int)ctx.ext->tls_id, ret-4);
 	}
 
-	ret = _gnutls_extv_append_final(buf, pos);
+	ret = _gnutls_extv_append_final(buf, pos, !(msg & GNUTLS_EXT_FLAG_EE));
 	if (ret < 0)
 		return gnutls_assert_val(ret);
 
diff --git a/lib/tls13/certificate.c b/lib/tls13/certificate.c
index bf8dbda2f7..2560ca3427 100644
--- a/lib/tls13/certificate.c
+++ b/lib/tls13/certificate.c
@@ -288,7 +288,7 @@ int _gnutls13_send_certificate(gnutls_session_t session, unsigned again)
 					goto cleanup;
 				}
 
-				ret = _gnutls_extv_append_final(&buf, ext_pos_mark);
+				ret = _gnutls_extv_append_final(&buf, ext_pos_mark, 0);
 				if (ret < 0) {
 					gnutls_assert();
 					goto cleanup;
diff --git a/lib/tls13/certificate_request.c b/lib/tls13/certificate_request.c
index a7ec0e2fd9..4efeee0377 100644
--- a/lib/tls13/certificate_request.c
+++ b/lib/tls13/certificate_request.c
@@ -320,7 +320,7 @@ int _gnutls13_send_certificate_request(gnutls_session_t session, unsigned again)
 			goto cleanup;
 		}
 
-		ret = _gnutls_extv_append_final(&buf, init_pos);
+		ret = _gnutls_extv_append_final(&buf, init_pos, 0);
 		if (ret < 0) {
 			gnutls_assert();
 			goto cleanup;
diff --git a/lib/tls13/session_ticket.c b/lib/tls13/session_ticket.c
index f254a73036..c40999575b 100644
--- a/lib/tls13/session_ticket.c
+++ b/lib/tls13/session_ticket.c
@@ -353,7 +353,7 @@ int _gnutls13_send_session_ticket(gnutls_session_t session, unsigned nr, unsigne
 				goto cleanup;
 			}
 
-			ret = _gnutls_extv_append_final(&buf, init_pos);
+			ret = _gnutls_extv_append_final(&buf, init_pos, 0);
 			if (ret < 0) {
 				gnutls_assert();
 				goto cleanup;
diff --git a/tests/Makefile.am b/tests/Makefile.am
index ad496b04a7..56149cce5e 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -200,7 +200,7 @@ ctests += mini-record-2 simple gnutls_hmac_fast set_pkcs12_cred cert certuniquei
 	 set_known_dh_params_anon set_known_dh_params_psk session-tickets-ok \
 	 session-tickets-missing set_x509_key_file_legacy status-request-ext \
 	 gnutls_x509_crt_sign gnutls_x509_crq_sign dtls-repro-20170915 \
-	 rng-no-onload dtls1-2-mtu-check crl_apis cert_verify_inv_utf8 \
+	 rng-no-onload dtls1-2-mtu-check crl_apis cert_verify_inv_utf8 no-extensions \
 	 hostname-check-utf8 pkcs8-key-decode-encrypted priority-mix pkcs7 \
 	 send-data-before-handshake recv-data-before-handshake crt_inv_write \
 	 x509sign-verify-error rng-op-nonce rng-op-random rng-op-key x509-dn-decode-compat \
diff --git a/tests/no-extensions.c b/tests/no-extensions.c
new file mode 100644
index 0000000000..e4ba1e9254
--- /dev/null
+++ b/tests/no-extensions.c
@@ -0,0 +1,179 @@
+/*
+ * Copyright (C) 2008-2012 Free Software Foundation, Inc.
+ *
+ * Author: Nikos Mavrogiannopoulos
+ *
+ * This file is part of GnuTLS.
+ *
+ * GnuTLS is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * GnuTLS is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with GnuTLS; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <string.h>
+#include <errno.h>
+#include <gnutls/gnutls.h>
+#include "utils.h"
+#include "eagain-common.h"
+#include "cert-common.h"
+#include <assert.h>
+
+const char *side;
+
+static void tls_log_func(int level, const char *str)
+{
+	fprintf(stderr, "%s|<%d>| %s", side, level, str);
+}
+
+#define SKIP8(pos, total) { \
+	uint8_t _s; \
+	if (pos+1 > total) fail("error\n"); \
+	_s = msg->data[pos]; \
+	if ((size_t)(pos+1+_s) > total) fail("error\n"); \
+	pos += 1+_s; \
+	}
+
+#define HANDSHAKE_SESSION_ID_POS 34
+
+static int handshake_callback(gnutls_session_t session, unsigned int htype,
+	unsigned post, unsigned int incoming, const gnutls_datum_t *msg)
+{
+	unsigned pos;
+	gnutls_datum_t mmsg;
+
+	assert(post && htype == GNUTLS_HANDSHAKE_SERVER_HELLO);
+
+	switch (htype) {
+	case GNUTLS_HANDSHAKE_SERVER_HELLO:
+		assert(msg->size >= HANDSHAKE_SESSION_ID_POS);
+		pos = HANDSHAKE_SESSION_ID_POS;
+		SKIP8(pos, msg->size);
+		pos += 3; /* ciphersuite + compression */
+
+		mmsg.data = &msg->data[pos];
+		mmsg.size = msg->size - pos;
+		if (pos != msg->size) {
+			if (pos < msg->size-1) {
+				fprintf(stderr, "additional bytes: %.2x%.2x\n", mmsg.data[0], mmsg.data[1]);
+			}
+			fail("the server hello contains additional bytes\n");
+		}
+		break;
+	}
+	return 0;
+}
+
+static
+void start(const char *prio)
+{
+	int ret;
+	/* Server stuff. */
+	gnutls_certificate_credentials_t serverx509cred;
+	gnutls_session_t server;
+	int sret = GNUTLS_E_AGAIN;
+	/* Client stuff. */
+	gnutls_certificate_credentials_t clientx509cred;
+	gnutls_session_t client;
+	int cret = GNUTLS_E_AGAIN;
+
+	success("trying %s\n", prio);
+
+	/* General init. */
+	global_init();
+	gnutls_global_set_log_function(tls_log_func);
+	if (debug)
+		gnutls_global_set_log_level(6);
+
+	/* Init server */
+	gnutls_certificate_allocate_credentials(&serverx509cred);
+	gnutls_certificate_set_x509_key_mem(serverx509cred,
+					    &server_cert, &server_key,
+					    GNUTLS_X509_FMT_PEM);
+
+	gnutls_init(&server, GNUTLS_SERVER|GNUTLS_NO_EXTENSIONS);
+	gnutls_credentials_set(server, GNUTLS_CRD_CERTIFICATE,
+				serverx509cred);
+	assert(gnutls_priority_set_direct(server, prio, NULL)>=0);
+	gnutls_transport_set_push_function(server, server_push);
+	gnutls_transport_set_pull_function(server, server_pull);
+	gnutls_transport_set_ptr(server, server);
+
+	gnutls_handshake_set_hook_function(server,
+					   GNUTLS_HANDSHAKE_SERVER_HELLO,
+					   GNUTLS_HOOK_POST,
+					   handshake_callback);
+
+	/* Init client */
+	ret = gnutls_certificate_allocate_credentials(&clientx509cred);
+	if (ret < 0)
+		exit(1);
+
+	ret = gnutls_certificate_set_x509_trust_mem(clientx509cred, &ca_cert, GNUTLS_X509_FMT_PEM);
+	if (ret < 0)
+		exit(1);
+
+	ret = gnutls_init(&client, GNUTLS_CLIENT|GNUTLS_NO_EXTENSIONS);
+	if (ret < 0)
+		exit(1);
+
+	ret = gnutls_credentials_set(client, GNUTLS_CRD_CERTIFICATE,
+				clientx509cred);
+	if (ret < 0)
+		exit(1);
+
+	gnutls_priority_set_direct(client, prio, NULL);
+	gnutls_transport_set_push_function(client, client_push);
+	gnutls_transport_set_pull_function(client, client_pull);
+	gnutls_transport_set_ptr(client, client);
+
+	HANDSHAKE(client, server);
+
+	/* check gnutls_certificate_get_ours() - client side */
+	{
+		const gnutls_datum_t *mcert;
+
+		mcert = gnutls_certificate_get_ours(client);
+		if (mcert != NULL) {
+			fail("gnutls_certificate_get_ours(): failed\n");
+			exit(1);
+		}
+	}
+
+	assert(gnutls_certificate_type_get(server)==GNUTLS_CRT_X509);
+	assert(gnutls_certificate_type_get(client)==GNUTLS_CRT_X509);
+
+	gnutls_bye(client, GNUTLS_SHUT_RDWR);
+	gnutls_bye(server, GNUTLS_SHUT_RDWR);
+
+	gnutls_deinit(client);
+	gnutls_deinit(server);
+
+	gnutls_certificate_free_credentials(serverx509cred);
+	gnutls_certificate_free_credentials(clientx509cred);
+
+	gnutls_global_deinit();
+
+	reset_buffers();
+}
+
+void doit(void)
+{
+	start("NORMAL:-VERS-ALL:+VERS-TLS1.0:%NO_EXTENSIONS:%DISABLE_SAFE_RENEGOTIATION");
+}