diff options
-rw-r--r-- | .travis.yml | 2 | ||||
-rw-r--r-- | Makefile.am | 2 | ||||
-rw-r--r-- | NEWS | 5 | ||||
-rw-r--r-- | lib/alert.c | 2 | ||||
-rw-r--r-- | lib/extv.h | 11 | ||||
-rw-r--r-- | lib/hello_ext.c | 2 | ||||
-rw-r--r-- | lib/tls-sig.c | 19 | ||||
-rw-r--r-- | lib/tls13/certificate.c | 2 | ||||
-rw-r--r-- | lib/tls13/certificate_request.c | 2 | ||||
-rw-r--r-- | lib/tls13/session_ticket.c | 2 | ||||
-rw-r--r-- | lib/x509/x509_ext.c | 2 | ||||
-rw-r--r-- | tests/Makefile.am | 2 | ||||
-rw-r--r-- | tests/no-extensions.c | 207 | ||||
-rw-r--r-- | tests/suite/tls-fuzzer/gnutls-cert.json | 6 | ||||
m--------- | tests/suite/tls-fuzzer/tlsfuzzer | 0 | ||||
m--------- | tests/suite/tls-fuzzer/tlslite-ng | 0 |
16 files changed, 250 insertions, 16 deletions
diff --git a/.travis.yml b/.travis.yml index d060703983..017a3788fe 100644 --- a/.travis.yml +++ b/.travis.yml @@ -27,7 +27,7 @@ before_install: fi script: - - PATH=/usr/local/opt/gettext/bin:$PATH make autoreconf + - PATH=/usr/local/opt/gettext/bin:$PATH ./bootstrap - PATH=/usr/local/opt/gettext/bin:$PATH ./configure --disable-full-test-suite --disable-valgrind-tests --disable-doc --disable-guile --disable-dependency-tracking - make -j$(sysctl -n hw.ncpu) - make -j$(sysctl -n hw.ncpu) check gl_public_submodule_commit= diff --git a/Makefile.am b/Makefile.am index 06427f3395..57e42359bf 100644 --- a/Makefile.am +++ b/Makefile.am @@ -50,7 +50,7 @@ if ENABLE_DOC SUBDIRS += doc endif -ACLOCAL_AMFLAGS = -I m4 -I src/libopts/m4 -I src/gl/m4 -I lib/unistring/m4 +ACLOCAL_AMFLAGS = -I m4 -I src/libopts/m4 -I src/gl/m4 -I lib/unistring/m4 --install EXTRA_DIST = cfg.mk maint.mk CONTRIBUTING.md README.md LICENSE AUTHORS NEWS \ ChangeLog THANKS INSTALL.md symbols.last @@ -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/alert.c b/lib/alert.c index b9aa7bd9ba..a7770da676 100644 --- a/lib/alert.c +++ b/lib/alert.c @@ -223,6 +223,7 @@ int gnutls_error_to_alert(int err, int *level) case GNUTLS_E_RECEIVED_ILLEGAL_PARAMETER: case GNUTLS_E_ILLEGAL_SRP_USERNAME: case GNUTLS_E_PK_INVALID_PUBKEY: + case GNUTLS_E_UNKNOWN_COMPRESSION_ALGORITHM: ret = GNUTLS_A_ILLEGAL_PARAMETER; _level = GNUTLS_AL_FATAL; break; @@ -247,7 +248,6 @@ int gnutls_error_to_alert(int err, int *level) _level = GNUTLS_AL_FATAL; break; case GNUTLS_E_UNKNOWN_CIPHER_SUITE: - case GNUTLS_E_UNKNOWN_COMPRESSION_ALGORITHM: case GNUTLS_E_INSUFFICIENT_CREDENTIALS: case GNUTLS_E_NO_CIPHER_SUITES: case GNUTLS_E_NO_COMPRESSION_ALGORITHMS: 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/tls-sig.c b/lib/tls-sig.c index 75f88e5fbd..19357c06a1 100644 --- a/lib/tls-sig.c +++ b/lib/tls-sig.c @@ -271,10 +271,11 @@ _gnutls_handshake_verify_data12(gnutls_session_t session, gnutls_datum_t dconcat; int ret; const version_entry_st *ver = get_version(session); + const gnutls_sign_entry_st *se = _gnutls_sign_to_entry(sign_algo); _gnutls_handshake_log ("HSK[%p]: verify TLS 1.2 handshake data: using %s\n", session, - gnutls_sign_algorithm_get_name(sign_algo)); + se->name); ret = _gnutls_pubkey_compatible_with_sig(session, @@ -283,6 +284,12 @@ _gnutls_handshake_verify_data12(gnutls_session_t session, if (ret < 0) return gnutls_assert_val(ret); + if (unlikely(sign_supports_cert_pk_algorithm(se, cert->pubkey->params.algo) == 0)) { + _gnutls_handshake_log("HSK[%p]: certificate of %s cannot be combined with %s sig\n", + session, gnutls_pk_get_name(cert->pubkey->params.algo), se->name); + return gnutls_assert_val(GNUTLS_E_RECEIVED_ILLEGAL_PARAMETER); + } + ret = _gnutls_session_sign_algo_enabled(session, sign_algo); if (ret < 0) @@ -356,11 +363,18 @@ _gnutls_handshake_verify_crt_vrfy12(gnutls_session_t session, { int ret; gnutls_datum_t dconcat; + const gnutls_sign_entry_st *se = _gnutls_sign_to_entry(sign_algo); ret = _gnutls_session_sign_algo_enabled(session, sign_algo); if (ret < 0) return gnutls_assert_val(ret); + if (unlikely(sign_supports_cert_pk_algorithm(se, cert->pubkey->params.algo) == 0)) { + _gnutls_handshake_log("HSK[%p]: certificate of %s cannot be combined with %s sig\n", + session, gnutls_pk_get_name(cert->pubkey->params.algo), se->name); + return gnutls_assert_val(GNUTLS_E_RECEIVED_ILLEGAL_PARAMETER); + } + dconcat.data = session->internals.handshake_hash_buffer.data; dconcat.size = session->internals.handshake_hash_buffer_prev_len; @@ -567,6 +581,9 @@ _gnutls_handshake_sign_crt_vrfy12(gnutls_session_t session, gnutls_sign_algorithm_set_client(session, sign_algo); + if (unlikely(gnutls_sign_supports_pk_algorithm(sign_algo, pkey->pk_algorithm) == 0)) + return gnutls_assert_val(GNUTLS_E_RECEIVED_ILLEGAL_PARAMETER); + _gnutls_debug_log("sign handshake cert vrfy: picked %s\n", gnutls_sign_algorithm_get_name(sign_algo)); 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/lib/x509/x509_ext.c b/lib/x509/x509_ext.c index 8213c6e427..ffc05bc0a3 100644 --- a/lib/x509/x509_ext.c +++ b/lib/x509/x509_ext.c @@ -1052,7 +1052,7 @@ int gnutls_x509_ext_export_authority_key_id(gnutls_x509_aki_t aki, san.data, aki->cert_issuer. names[i].san.size); - if (result < 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"); +} diff --git a/tests/suite/tls-fuzzer/gnutls-cert.json b/tests/suite/tls-fuzzer/gnutls-cert.json index f9de174699..c2b28c5569 100644 --- a/tests/suite/tls-fuzzer/gnutls-cert.json +++ b/tests/suite/tls-fuzzer/gnutls-cert.json @@ -37,13 +37,11 @@ "-p", "@PORT@"] }, {"name" : "test-rsa-pss-sigs-on-certificate-verify.py", - "comment" : "FIXME: We shouldn't allow rsa_pss_pss* schemes as there is only RSA key #645", + "comment": "tlsfuzzer doesn't know ed25519 scheme which we advertise", "arguments" : ["-k", "tests/clientX509Key.pem", "-c", "tests/clientX509Cert.pem", "-e", "check CertificateRequest sigalgs", - "-e", "rsa_pss_pss_sha256 in CertificateVerify with rsa key", - "-e", "rsa_pss_pss_sha384 in CertificateVerify with rsa key", - "-e", "rsa_pss_pss_sha512 in CertificateVerify with rsa key", + "--illegpar", "-n", "100", "-p", "@PORT@"] }, diff --git a/tests/suite/tls-fuzzer/tlsfuzzer b/tests/suite/tls-fuzzer/tlsfuzzer -Subproject cd624f68c671f339b3a1e0ef90db984760bcfea +Subproject b9dec4fde7bedfac90850b86c2c3f644349f6c3 diff --git a/tests/suite/tls-fuzzer/tlslite-ng b/tests/suite/tls-fuzzer/tlslite-ng -Subproject d00ad94272be90172ecc5c422c923d679c23416 +Subproject 3696909715ba73ee807d3959a26d36b56f718ba |