]> git.ipfire.org Git - thirdparty/gnutls.git/commitdiff
When sending no extensions do not include a zero length 868/head
authorNikos Mavrogiannopoulos <nmav@redhat.com>
Tue, 8 Jan 2019 11:26:19 +0000 (12:26 +0100)
committerNikos Mavrogiannopoulos <nmav@redhat.com>
Wed, 9 Jan 2019 15:09:58 +0000 (16:09 +0100)
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>
NEWS
lib/extv.h
lib/hello_ext.c
lib/tls13/certificate.c
lib/tls13/certificate_request.c
lib/tls13/session_ticket.c
tests/Makefile.am
tests/no-extensions.c [new file with mode: 0644]

diff --git a/NEWS b/NEWS
index 739ab21651680101ed05c2e7b1b16a2baad79d53..b109e78b6ec298b76adfd7726fdea0eb3bdbbd52 100644 (file)
--- 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.
 
index 9f13f7a2ce65dcff8855935aab6d94648ca2b27c..c791d9bba948bb85d36c1de04c46b3e8cd8e6c87 100644 (file)
@@ -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;
 }
index c4907aaced67f3d57cc4b3c813d49704c683dd4a..5692a14d2d930fc3a69b63bd4a3c5bfbfc541276 100644 (file)
@@ -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);
 
index bf8dbda2f7c5617ee2a7d07375f80156a6692314..2560ca3427fe03b72c8c73575f28fcf3e9e3ab7d 100644 (file)
@@ -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;
index a7ec0e2fd9aace2b1192ef42ba98b267bb93f758..4efeee0377bc4aa77110c9f143b3d420335e34d1 100644 (file)
@@ -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;
index f254a73036f71270b06bcd3fae0363c8ef62f031..c40999575bdcaf2d233dfb59733aafa2e349e22d 100644 (file)
@@ -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;
index ad496b04a736530c9d8ad3ca66075a8a66f9b44b..56149cce5eb3488942509b138a69075f796cf7bd 100644 (file)
@@ -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 (file)
index 0000000..76e0040
--- /dev/null
@@ -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");
+}