tls13/certificate_request: corrected check of duplicate signature algorithms

Made the check local when parsing a certificate request, as we may
receive multiple requests when post-handshake authentication is
in place. Furthermore check whether this extension has been received
as this is a mandatory one. In addition handle a memory leak when
multiple peer certificates are set.

Signed-off-by: Nikos Mavrogiannopoulos <nmav@redhat.com>

4 files changed, 45 insertions, 22 deletions

diff --git a/lib/gnutls_int.h b/lib/gnutls_int.h
index cae9d7aec7..9c9fb1533a 100644
--- a/lib/gnutls_int.h
+++ b/lib/gnutls_int.h
@@ -1213,7 +1213,6 @@ typedef struct {
 #define HSK_HRR_SENT (1<<3)
 #define HSK_HRR_RECEIVED (1<<4)
 #define HSK_CRT_REQ_SENT (1<<5)
-#define HSK_CRT_REQ_GOT_SIG_ALGO (1<<6)
 #define HSK_KEY_UPDATE_ASKED (1<<7) /* flag is not used during handshake */
 #define HSK_FALSE_START_USED (1<<8) /* TLS1.2 only */
 #define HSK_HAVE_FFDHE (1<<9) /* whether the peer has advertized at least an FFDHE group */
diff --git a/lib/tls13/certificate.c b/lib/tls13/certificate.c
index 1fb8039e8a..072a5d62b4 100644
--- a/lib/tls13/certificate.c
+++ b/lib/tls13/certificate.c
@@ -479,6 +479,15 @@ parse_cert_list(gnutls_session_t session, uint8_t * data, size_t data_size)
 	/* The OCSP entries match the certificate entries, although
 	 * the contents of each OCSP entry may be NULL.
 	 */
+	for(j=0;j<info->ncerts;j++)
+		gnutls_free(info->raw_certificate_list[j].data);
+	gnutls_free(info->raw_certificate_list);
+
+	for(j=0;j<info->nocsp;j++)
+		gnutls_free(info->raw_ocsp_list[j].data);
+	gnutls_free(info->raw_ocsp_list);
+
+
 	info->raw_certificate_list = peer_certs;
 	info->ncerts = npeer_certs;
 
diff --git a/lib/tls13/certificate_request.c b/lib/tls13/certificate_request.c
index 293cc38dcf..09fb56d0bd 100644
--- a/lib/tls13/certificate_request.c
+++ b/lib/tls13/certificate_request.c
@@ -37,6 +37,7 @@
 
 typedef struct crt_req_ctx_st {
 	gnutls_session_t session;
+	unsigned got_sig_algo;
 	gnutls_pk_algorithm_t pk_algos[MAX_ALGOS];
 	unsigned pk_algos_length;
 	const uint8_t *rdn; /* pointer inside the message buffer */
@@ -71,10 +72,10 @@ int parse_cert_extension(void *_ctx, unsigned tls_id, const uint8_t *data, unsig
 		/* signature algorithms; let's use it to decide the certificate to use */
 		unsigned i;
 
-		if (session->internals.hsk_flags & HSK_CRT_REQ_GOT_SIG_ALGO)
+		if (ctx->got_sig_algo)
 			return gnutls_assert_val(GNUTLS_E_RECEIVED_ILLEGAL_EXTENSION);
 
-		session->internals.hsk_flags |= HSK_CRT_REQ_GOT_SIG_ALGO;
+		ctx->got_sig_algo = 1;
 
 		if (data_size < 2)
 			return gnutls_assert_val(GNUTLS_E_TLS_PACKET_DECODING_ERROR);
@@ -167,6 +168,10 @@ int _gnutls13_recv_certificate_request_int(gnutls_session_t session, gnutls_buff
 		goto cleanup;
 	}
 
+	/* The "signature_algorithms" extension MUST be specified */
+	if (!ctx.got_sig_algo)
+		return gnutls_assert_val(GNUTLS_E_RECEIVED_ILLEGAL_EXTENSION);
+
 	session->internals.hsk_flags |= HSK_CRT_ASKED;
 
 	ret = _gnutls_select_client_cert(session, ctx.rdn, ctx.rdn_size,
diff --git a/tests/tls13/post-handshake-with-cert.c b/tests/tls13/post-handshake-with-cert.c
index 22e4376e80..88ed470153 100644
--- a/tests/tls13/post-handshake-with-cert.c
+++ b/tests/tls13/post-handshake-with-cert.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2017 Red Hat, Inc.
+ * Copyright (C) 2017-2018 Red Hat, Inc.
  *
  * Author: Nikos Mavrogiannopoulos
  *
@@ -50,6 +50,8 @@ int main()
 #include "tls13/ext-parse.h"
 #include "utils.h"
 
+#define MAX_AUTHS 4
+
 /* This program tests whether the Post Handshake Auth extension is
  * present in the client hello, and whether it is missing from server
  * hello. In addition it contains basic functionality test for
@@ -74,6 +76,7 @@ static void client(int fd)
 	gnutls_certificate_credentials_t x509_cred;
 	gnutls_session_t session;
 	char buf[64];
+	unsigned i;
 
 	global_init();
 
@@ -116,21 +119,24 @@ static void client(int fd)
 		fail("handshake failed: %s\n", gnutls_strerror(ret));
 	success("client handshake completed\n");
 
-	do {
-		ret = gnutls_record_recv(session, buf, sizeof(buf));
-	} while (ret == GNUTLS_E_AGAIN || ret == GNUTLS_E_INTERRUPTED);
+	for (i=0;i<MAX_AUTHS;i++) {
+		do {
+			ret = gnutls_record_recv(session, buf, sizeof(buf));
+		} while (ret == GNUTLS_E_AGAIN || ret == GNUTLS_E_INTERRUPTED);
 
-	if (ret != GNUTLS_E_REAUTH_REQUEST) {
-		fail("recv: unexpected error: %s\n", gnutls_strerror(ret));
-	}
+		if (ret != GNUTLS_E_REAUTH_REQUEST) {
+			fail("recv: unexpected error: %s\n", gnutls_strerror(ret));
+		}
 
-	success("received reauth request\n");
-	do {
-		ret = gnutls_reauth(session, 0);
-	} while (ret == GNUTLS_E_AGAIN || ret == GNUTLS_E_INTERRUPTED);
+		success("received reauth request\n");
+		do {
+			ret = gnutls_reauth(session, 0);
+		} while (ret == GNUTLS_E_AGAIN || ret == GNUTLS_E_INTERRUPTED);
+
+		if (ret != 0)
+			fail("client: gnutls_reauth did not succeed as expected: %s\n", gnutls_strerror(ret));
+	}
 
-	if (ret != 0)
-		fail("client: gnutls_reauth did not succeed as expected: %s\n", gnutls_strerror(ret));
 
 	close(fd);
 
@@ -182,6 +188,7 @@ static void server(int fd)
 	char buffer[MAX_BUF + 1];
 	gnutls_session_t session;
 	gnutls_certificate_credentials_t x509_cred;
+	unsigned i;
 
 	/* this must be called once in the program
 	 */
@@ -231,13 +238,16 @@ static void server(int fd)
 	success("server handshake completed\n");
 
 	gnutls_certificate_server_set_request(session, GNUTLS_CERT_REQUIRE);
-	/* ask peer for re-authentication */
-	do {
-		ret = gnutls_reauth(session, 0);
-	} while (ret == GNUTLS_E_AGAIN || ret == GNUTLS_E_INTERRUPTED);
 
-	if (ret != 0)
-		fail("server: gnutls_reauth did not succeed as expected: %s\n", gnutls_strerror(ret));
+	for (i=0;i<MAX_AUTHS;i++) {
+		/* ask peer for re-authentication */
+		do {
+			ret = gnutls_reauth(session, 0);
+		} while (ret == GNUTLS_E_AGAIN || ret == GNUTLS_E_INTERRUPTED);
+
+		if (ret != 0)
+			fail("server: gnutls_reauth did not succeed as expected: %s\n", gnutls_strerror(ret));
+	}
 
 	close(fd);
 	gnutls_deinit(session);