summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNikos Mavrogiannopoulos <nmav@redhat.com>2016-04-29 10:23:45 +0200
committerNikos Mavrogiannopoulos <nmav@redhat.com>2016-04-29 10:30:03 +0200
commit9466b800f03a91ef1538dc6f562d58b4607b88e6 (patch)
tree88385ec566a43015cd9598feb7b2d4ad3022e815
parent96cca97371237e31e6c98d705cd31f6b3b268d25 (diff)
downloadgnutls-9466b800f03a91ef1538dc6f562d58b4607b88e6.tar.gz
handshake: enhance same certificate checks to apply to PSK/SRP username
That is, unless GNUTLS_ALLOW_ID_CHANGE is specified, during a rehandshake clients will not be allowed to present another certificate than the original, or change their username for PSK or SRP ciphersuites.
-rw-r--r--NEWS2
-rw-r--r--lib/Makefile.am2
-rw-r--r--lib/alert.c1
-rw-r--r--lib/errors.c4
-rw-r--r--lib/gnutls_int.h4
-rw-r--r--lib/handshake-checks.c120
-rw-r--r--lib/handshake.c38
-rw-r--r--lib/handshake.h3
-rw-r--r--lib/includes/gnutls/gnutls.h.in6
-rw-r--r--tests/rehandshake-switch-cert-allow.c2
-rw-r--r--tests/rehandshake-switch-cert-client-allow.c2
-rw-r--r--tests/rehandshake-switch-cert-client.c2
-rw-r--r--tests/rehandshake-switch-cert.c2
13 files changed, 141 insertions, 47 deletions
diff --git a/NEWS b/NEWS
index 08e183f866..fe9e94c7b9 100644
--- a/NEWS
+++ b/NEWS
@@ -22,7 +22,7 @@ See the end for copying conditions.
available) is the same as in previous handshakes. That is to protect
applications which do not check user credentials on rehandshakes from
attacks related to unsafe renegotiation. This can be overriden using
- the %GNUTLS_ALLOW_CERT_CHANGE flag in gnutls_init().
+ the %GNUTLS_ALLOW_ID_CHANGE flag in gnutls_init().
** libgnutls: Be strict in TLS extension decoding. That is, do not tolerate
parsing errors in the extensions field and treat it as a typical Hello
diff --git a/lib/Makefile.am b/lib/Makefile.am
index 0d434a7206..39646770ef 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -77,7 +77,7 @@ COBJECTS = range.c record.c compress.c debug.c cipher.c \
system_override.c crypto-backend.c verify-tofu.c pin.c tpm.c fips.c \
safe-memfuncs.c inet_pton.c atfork.c atfork.h randomart.c \
system-keys.h urls.c urls.h prf.c auto-verify.c dh-session.c \
- cert-session.c
+ cert-session.c handshake-checks.c
if WINDOWS
COBJECTS += system-keys-win.c
diff --git a/lib/alert.c b/lib/alert.c
index b8a4389e55..786f7bc6bb 100644
--- a/lib/alert.c
+++ b/lib/alert.c
@@ -292,6 +292,7 @@ int gnutls_error_to_alert(int err, int *level)
break;
case GNUTLS_E_DH_PRIME_UNACCEPTABLE:
case GNUTLS_E_NO_CERTIFICATE_FOUND:
+ case GNUTLS_E_SESSION_USER_ID_CHANGED:
ret = GNUTLS_A_INSUFFICIENT_SECURITY;
_level = GNUTLS_AL_FATAL;
break;
diff --git a/lib/errors.c b/lib/errors.c
index 34fcf21153..42c5010b21 100644
--- a/lib/errors.c
+++ b/lib/errors.c
@@ -376,8 +376,8 @@ static const gnutls_error_entry error_entries[] = {
GNUTLS_E_PK_GENERATION_ERROR),
ERROR_ENTRY(N_("Invalid TLS extensions length field."),
GNUTLS_E_UNEXPECTED_EXTENSIONS_LENGTH),
- ERROR_ENTRY(N_("Peer's certificate has changed during a rehandshake."),
- GNUTLS_E_SESSION_CERTIFICATE_CHANGED),
+ ERROR_ENTRY(N_("Peer's certificate or username has changed during a rehandshake."),
+ GNUTLS_E_SESSION_USER_ID_CHANGED),
ERROR_ENTRY(N_("The provided string has an embedded null."),
GNUTLS_E_ASN1_EMBEDDED_NULL_IN_STRING),
ERROR_ENTRY(N_("Attempted handshake during false start."),
diff --git a/lib/gnutls_int.h b/lib/gnutls_int.h
index e968745416..fd899461cd 100644
--- a/lib/gnutls_int.h
+++ b/lib/gnutls_int.h
@@ -1014,6 +1014,10 @@ typedef struct {
uint8_t cert_hash[32];
bool cert_hash_set;
+ /* The saved username from PSK or SRP auth */
+ char saved_username[MAX_USERNAME_SIZE+1];
+ bool saved_username_set;
+
bool false_start_used; /* non-zero if false start was used for appdata */
/* If you add anything here, check _gnutls_handshake_internal_state_clear().
diff --git a/lib/handshake-checks.c b/lib/handshake-checks.c
new file mode 100644
index 0000000000..6f13776ec8
--- /dev/null
+++ b/lib/handshake-checks.c
@@ -0,0 +1,120 @@
+/*
+ * Copyright (C) 2015-2016 Red Hat, Inc.
+ *
+ * Author: Nikos Mavrogiannopoulos
+ *
+ * This file is part of GnuTLS.
+ *
+ * The GnuTLS is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public License
+ * as published by the Free Software Foundation; either version 2.1 of
+ * the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser 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/>
+ *
+ */
+
+/* Functions that relate to the TLS handshake procedure.
+ */
+
+#include "gnutls_int.h"
+#include "errors.h"
+#include "debug.h"
+#include "handshake.h"
+#include <auth/cert.h>
+#include "constate.h"
+#include <record.h>
+#include <state.h>
+#include <ext/safe_renegotiation.h>
+#include <auth/anon.h> /* for gnutls_anon_server_credentials_t */
+#include <auth/psk.h> /* for gnutls_psk_server_credentials_t */
+#ifdef ENABLE_SRP
+# include <auth/srp_kx.h>
+#endif
+
+int _gnutls_check_id_for_change(gnutls_session_t session)
+{
+ int cred_type;
+
+ /* This checks in PSK and SRP ciphersuites that the username remained the
+ * same on a rehandshake. */
+ if (session->internals.flags & GNUTLS_ALLOW_ID_CHANGE)
+ return 0;
+
+ cred_type = gnutls_auth_get_type(session);
+ if (cred_type == GNUTLS_CRD_PSK || cred_type == GNUTLS_CRD_SRP) {
+ const char *username;
+
+ if (cred_type == GNUTLS_CRD_PSK) {
+ psk_auth_info_t ai;
+
+ ai = _gnutls_get_auth_info(session, GNUTLS_CRD_PSK);
+ if (ai == NULL)
+ return gnutls_assert_val(GNUTLS_E_INTERNAL_ERROR);
+
+ username = ai->username;
+#ifdef ENABLE_SRP
+ } else {
+ srp_server_auth_info_t ai = _gnutls_get_auth_info(session, GNUTLS_CRD_SRP);
+ if (ai == NULL)
+ return gnutls_assert_val(GNUTLS_E_INTERNAL_ERROR);
+
+ username = ai->username;
+#endif
+ }
+
+ if (session->internals.saved_username_set) {
+ if (strcmp(session->internals.saved_username, username) != 0) {
+ _gnutls_debug_log("Session's PSK username changed during rehandshake; aborting!\n");
+ return gnutls_assert_val(GNUTLS_E_SESSION_USER_ID_CHANGED);
+ }
+ } else {
+ size_t len = strlen(username);
+
+ memcpy(session->internals.saved_username, username, len);
+ session->internals.saved_username[len] = 0;
+ session->internals.saved_username_set = 1;
+ }
+ }
+
+ return 0;
+}
+
+int _gnutls_check_if_cert_hash_is_same(gnutls_session_t session, gnutls_certificate_credentials_t cred)
+{
+ cert_auth_info_t ai;
+ char tmp[32];
+ int ret;
+
+ if (session->internals.flags & GNUTLS_ALLOW_ID_CHANGE)
+ return 0;
+
+ ai = _gnutls_get_auth_info(session, GNUTLS_CRD_CERTIFICATE);
+ if (ai == NULL || ai->ncerts == 0)
+ return 0;
+
+ ret = gnutls_hash_fast(GNUTLS_DIG_SHA256,
+ ai->raw_certificate_list[0].data,
+ ai->raw_certificate_list[0].size,
+ tmp);
+ if (ret < 0)
+ return gnutls_assert_val(ret);
+
+ if (session->internals.cert_hash_set) {
+ if (memcmp(tmp, session->internals.cert_hash, 32) != 0) {
+ _gnutls_debug_log("Session certificate changed during rehandshake; aborting!\n");
+ return gnutls_assert_val(GNUTLS_E_SESSION_USER_ID_CHANGED);
+ }
+ } else {
+ memcpy(session->internals.cert_hash, tmp, 32);
+ session->internals.cert_hash_set = 1;
+ }
+
+ return 0;
+}
diff --git a/lib/handshake.c b/lib/handshake.c
index 0fdd1ae98d..5e04f5a3bf 100644
--- a/lib/handshake.c
+++ b/lib/handshake.c
@@ -2671,39 +2671,6 @@ gnutls_handshake_set_timeout(gnutls_session_t session, unsigned int ms)
} } while (0)
-static int check_if_cert_hash_is_same(gnutls_session_t session, gnutls_certificate_credentials_t cred)
-{
- cert_auth_info_t ai;
- char tmp[32];
- int ret;
-
- if (session->internals.flags & GNUTLS_ALLOW_CERT_CHANGE)
- return 0;
-
- ai = _gnutls_get_auth_info(session, GNUTLS_CRD_CERTIFICATE);
- if (ai == NULL || ai->ncerts == 0)
- return 0;
-
- ret = gnutls_hash_fast(GNUTLS_DIG_SHA256,
- ai->raw_certificate_list[0].data,
- ai->raw_certificate_list[0].size,
- tmp);
- if (ret < 0)
- return gnutls_assert_val(ret);
-
- if (session->internals.cert_hash_set) {
- if (memcmp(tmp, session->internals.cert_hash, 32) != 0) {
- _gnutls_debug_log("Session certificate changed during rehandshake; aborting!\n");
- return gnutls_assert_val(GNUTLS_E_SESSION_CERTIFICATE_CHANGED);
- }
- } else {
- memcpy(session->internals.cert_hash, tmp, 32);
- session->internals.cert_hash_set = 1;
- }
-
- return 0;
-}
-
/* Runs the certificate verification callback.
* side is either GNUTLS_CLIENT or GNUTLS_SERVER.
*/
@@ -2727,7 +2694,7 @@ static int run_verify_callback(gnutls_session_t session, unsigned int side)
/* verify whether the certificate of the peer remained the same
* as with any previous handshakes */
if (cred != NULL) {
- ret = check_if_cert_hash_is_same(session, cred);
+ ret = _gnutls_check_if_cert_hash_is_same(session, cred);
if (ret < 0) {
return gnutls_assert_val(ret);
}
@@ -3354,8 +3321,7 @@ static int handshake_server(gnutls_session_t session)
break;
}
-
- return 0;
+ return _gnutls_check_id_for_change(session);
}
int _gnutls_generate_session_id(uint8_t * session_id, uint8_t * len)
diff --git a/lib/handshake.h b/lib/handshake.h
index ba491232f5..3e24c2aeb2 100644
--- a/lib/handshake.h
+++ b/lib/handshake.h
@@ -73,4 +73,7 @@ inline static int handshake_remaining_time(gnutls_session_t session)
int _gnutls_handshake_get_session_hash(gnutls_session_t session, gnutls_datum_t *shash);
+int _gnutls_check_id_for_change(gnutls_session_t session);
+int _gnutls_check_if_cert_hash_is_same(gnutls_session_t session, gnutls_certificate_credentials_t cred);
+
#endif
diff --git a/lib/includes/gnutls/gnutls.h.in b/lib/includes/gnutls/gnutls.h.in
index f6afddfc45..432c26911a 100644
--- a/lib/includes/gnutls/gnutls.h.in
+++ b/lib/includes/gnutls/gnutls.h.in
@@ -353,7 +353,7 @@ typedef enum {
* @GNUTLS_NO_SIGNAL: In systems where SIGPIPE is delivered on send, it will be disabled. That flag has effect in systems which support the MSG_NOSIGNAL sockets flag (since 3.4.2).
* @GNUTLS_NO_EXTENSIONS: Do not enable any TLS extensions by default (since 3.1.2).
* @GNUTLS_NO_REPLAY_PROTECTION: Disable any replay protection in DTLS. This must only be used if replay protection is achieved using other means.
- * @GNUTLS_ALLOW_CERT_CHANGE: Allow the peer to replace its certificate during a rehandshake. This change is often used in attacks and thus prohibited by default. Since 3.5.0.
+ * @GNUTLS_ALLOW_ID_CHANGE: Allow the peer to replace its certificate, or change its ID during a rehandshake. This change is often used in attacks and thus prohibited by default. Since 3.5.0.
* @GNUTLS_ENABLE_FALSE_START: Enable the TLS false start on client side if the negotiated ciphersuites allow it. This will enable sending data prior to the handshake being complete, and may introduce a risk of crypto failure when combined with certain key exchanged; for that GnuTLS may not enable that option in ciphersuites that are known to be not safe for false start. Since 3.5.0.
* @GNUTLS_FORCE_CLIENT_CERT: When in client side and only a single cert is specified, send that certificate irrespective of the issuers expectated by the server. Since 3.5.0.
*
@@ -369,7 +369,7 @@ typedef enum {
GNUTLS_NO_EXTENSIONS = (1<<4),
GNUTLS_NO_REPLAY_PROTECTION = (1<<5),
GNUTLS_NO_SIGNAL = (1<<6),
- GNUTLS_ALLOW_CERT_CHANGE = (1<<7),
+ GNUTLS_ALLOW_ID_CHANGE = (1<<7),
GNUTLS_ENABLE_FALSE_START = (1<<8),
GNUTLS_FORCE_CLIENT_CERT = (1<<9)
} gnutls_init_flags_t;
@@ -2741,7 +2741,7 @@ int gnutls_fips140_mode_enabled(void);
#define GNUTLS_E_IDNA_ERROR -404
#define GNUTLS_E_NEED_FALLBACK -405
-#define GNUTLS_E_SESSION_CERTIFICATE_CHANGED -406
+#define GNUTLS_E_SESSION_USER_ID_CHANGED -406
#define GNUTLS_E_HANDSHAKE_DURING_FALSE_START -407
#define GNUTLS_E_UNIMPLEMENTED_FEATURE -1250
diff --git a/tests/rehandshake-switch-cert-allow.c b/tests/rehandshake-switch-cert-allow.c
index 0671206eee..fc365d149d 100644
--- a/tests/rehandshake-switch-cert-allow.c
+++ b/tests/rehandshake-switch-cert-allow.c
@@ -100,7 +100,7 @@ static void try(void)
if (ret < 0)
exit(1);
- ret = gnutls_init(&client, GNUTLS_CLIENT|GNUTLS_ALLOW_CERT_CHANGE);
+ ret = gnutls_init(&client, GNUTLS_CLIENT|GNUTLS_ALLOW_ID_CHANGE);
if (ret < 0)
exit(1);
diff --git a/tests/rehandshake-switch-cert-client-allow.c b/tests/rehandshake-switch-cert-client-allow.c
index 62193e3479..c4b0bf38b8 100644
--- a/tests/rehandshake-switch-cert-client-allow.c
+++ b/tests/rehandshake-switch-cert-client-allow.c
@@ -75,7 +75,7 @@ static void try(void)
gnutls_dh_params_import_pkcs3(dh_params, &p3, GNUTLS_X509_FMT_PEM);
gnutls_certificate_set_dh_params(serverx509cred, dh_params);
- gnutls_init(&server, GNUTLS_SERVER|GNUTLS_ALLOW_CERT_CHANGE);
+ gnutls_init(&server, GNUTLS_SERVER|GNUTLS_ALLOW_ID_CHANGE);
gnutls_certificate_server_set_request(server, GNUTLS_CERT_REQUEST);
gnutls_credentials_set(server, GNUTLS_CRD_CERTIFICATE,
serverx509cred);
diff --git a/tests/rehandshake-switch-cert-client.c b/tests/rehandshake-switch-cert-client.c
index f340beda07..d79db49ef4 100644
--- a/tests/rehandshake-switch-cert-client.c
+++ b/tests/rehandshake-switch-cert-client.c
@@ -141,7 +141,7 @@ static void try(void)
gnutls_credentials_set(client, GNUTLS_CRD_CERTIFICATE,
clientx509cred2);
- HANDSHAKE_EXPECT(client, server, GNUTLS_E_AGAIN, GNUTLS_E_SESSION_CERTIFICATE_CHANGED);
+ HANDSHAKE_EXPECT(client, server, GNUTLS_E_AGAIN, GNUTLS_E_SESSION_USER_ID_CHANGED);
gnutls_deinit(client);
gnutls_deinit(server);
diff --git a/tests/rehandshake-switch-cert.c b/tests/rehandshake-switch-cert.c
index f5e929ae5c..45f4666b76 100644
--- a/tests/rehandshake-switch-cert.c
+++ b/tests/rehandshake-switch-cert.c
@@ -128,7 +128,7 @@ static void try(void)
gnutls_credentials_set(server, GNUTLS_CRD_CERTIFICATE,
serverx509cred2);
- HANDSHAKE_EXPECT(client, server, GNUTLS_E_SESSION_CERTIFICATE_CHANGED, GNUTLS_E_AGAIN);
+ HANDSHAKE_EXPECT(client, server, GNUTLS_E_SESSION_USER_ID_CHANGED, GNUTLS_E_AGAIN);
gnutls_deinit(client);
gnutls_deinit(server);