Merge branch 'tmp-fix-regression-ext-size' into 'master'

When sending no extensions do not include a zero length

See merge request gnutls/gnutls!868

8 files changed, 226 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..76e0040dae
--- /dev/null
+++ b/tests/no-extensions.c
@@ -0,0 +1,207 @@
+/*
+ * Copyright (C) 2019 Red Hat, 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 Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+
+#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 "tls13/ext-parse.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);
+}
+
+static int server_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 int client_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_CLIENT_HELLO);
+
+	switch (htype) {
+	case GNUTLS_HANDSHAKE_CLIENT_HELLO:
+		assert(msg->size >= HANDSHAKE_SESSION_ID_POS);
+		pos = HANDSHAKE_SESSION_ID_POS;
+		SKIP8(pos, msg->size);
+		SKIP16(pos, msg->size);
+		SKIP8(pos, msg->size);
+
+		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 client 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,
+					   server_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);
+
+	gnutls_handshake_set_hook_function(client,
+					   GNUTLS_HANDSHAKE_CLIENT_HELLO,
+					   GNUTLS_HOOK_PRE,
+					   client_handshake_callback);
+
+	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");
+}