Merge branch 'tmp-session-ticket-key-name' into 'master'

psk: add deterministic detection of session tickets

Closes #450

See merge request gnutls/gnutls!651

3 files changed, 30 insertions, 47 deletions

diff --git a/lib/ext/pre_shared_key.c b/lib/ext/pre_shared_key.c
index 5c8a80c4a2..dce24d80a1 100644
--- a/lib/ext/pre_shared_key.c
+++ b/lib/ext/pre_shared_key.c
@@ -466,7 +466,6 @@ static int server_recv_params(gnutls_session_t session,
 	int psk_index;
 	gnutls_datum_t binder_recvd = { NULL, 0 };
 	gnutls_datum_t key = {NULL, 0};
-	unsigned cand_index;
 	psk_ext_parser_st psk_parser;
 	struct psk_st psk;
 	psk_auth_info_t info;
@@ -481,44 +480,13 @@ static int server_recv_params(gnutls_session_t session,
 		return gnutls_assert_val(ret);
 	}
 
-	psk_index = -1;
-
-	while ((ret = _gnutls13_psk_ext_parser_next_psk(&psk_parser, &psk)) >= 0) {
-		cand_index = ret;
-
-		/* Is this a PSK? */
-		if (psk.ob_ticket_age == 0) {
-			/* _gnutls_psk_pwd_find_entry() expects 0-terminated identities */
-			if (psk.identity.size > 0 && psk.identity.size <= MAX_USERNAME_SIZE) {
-				char identity_str[psk.identity.size + 1];
-
-				prf = pskcred->binder_algo;
-
-				memcpy(identity_str, psk.identity.data, psk.identity.size);
-				identity_str[psk.identity.size] = 0;
-
-				/* this fails only on configuration errors; as such we always
-				 * return its error code in that case */
-				ret = _gnutls_psk_pwd_find_entry(session, identity_str, &key);
-				if (ret < 0)
-					return gnutls_assert_val(ret);
-
-				psk_index = cand_index;
-				resuming = 0;
-				break;
-			}
-		}
-
-		/* Is this a session ticket? */
+	while ((psk_index = _gnutls13_psk_ext_parser_next_psk(&psk_parser, &psk)) >= 0) {
+		/* This will unpack the session ticket if it is well
+		 * formed and has the expected name */
 		if (!(session->internals.flags & GNUTLS_NO_TICKETS) &&
 		    (ret = _gnutls13_unpack_session_ticket(session, &psk.identity, &ticket_data)) == 0) {
 			prf = ticket_data.prf;
 
-			if (!prf) {
-				tls13_ticket_deinit(&ticket_data);
-				continue;
-			}
-
 			/* Check whether ticket is stale or not */
 			ticket_age = psk.ob_ticket_age - ticket_data.age_add;
 			if (ticket_age < 0) {
@@ -539,9 +507,26 @@ static int server_recv_params(gnutls_session_t session,
 
 			tls13_ticket_deinit(&ticket_data);
 
-			psk_index = cand_index;
 			resuming = 1;
 			break;
+		} else if (psk.ob_ticket_age == 0 &&
+			   psk.identity.size > 0 && psk.identity.size <= MAX_USERNAME_SIZE) {
+			/* _gnutls_psk_pwd_find_entry() expects 0-terminated identities */
+			char identity_str[psk.identity.size + 1];
+
+			prf = pskcred->binder_algo;
+
+			memcpy(identity_str, psk.identity.data, psk.identity.size);
+			identity_str[psk.identity.size] = 0;
+
+			/* this fails only on configuration errors; as such we always
+			 * return its error code in that case */
+			ret = _gnutls_psk_pwd_find_entry(session, identity_str, &key);
+			if (ret < 0)
+				return gnutls_assert_val(ret);
+
+			resuming = 0;
+			break;
 		}
 	}
 
diff --git a/lib/ext/session_ticket.c b/lib/ext/session_ticket.c
index 40bbe5b112..11d5db75c4 100644
--- a/lib/ext/session_ticket.c
+++ b/lib/ext/session_ticket.c
@@ -198,19 +198,17 @@ _gnutls_decrypt_session_ticket(gnutls_session_t session,
 	struct ticket_st ticket;
 	int ret;
 
+	/* If the key name of the ticket does not match the one that we
+	   hold, issue a new ticket. */
+	if (ticket_data->size < TICKET_KEY_NAME_SIZE ||
+	    memcmp(ticket_data->data, &session->key.session_ticket_key[NAME_POS],
+		   TICKET_KEY_NAME_SIZE))
+		return gnutls_assert_val(GNUTLS_E_DECRYPTION_FAILED);
+
 	ret = unpack_ticket(ticket_data, &ticket);
 	if (ret < 0)
 		return ret;
 
-	/* If the key name of the ticket does not match the one that we
-	   hold, issue a new ticket. */
-	if (memcmp
-	    (ticket.key_name, &session->key.session_ticket_key[NAME_POS],
-	     TICKET_KEY_NAME_SIZE)) {
-		ret = GNUTLS_E_DECRYPTION_FAILED;
-		goto cleanup;
-	}
-
 	/* Check the integrity of ticket */
 	mac_secret.data = (void *) &session->key.session_ticket_key[MAC_SECRET_POS];
 	mac_secret.size = TICKET_MAC_SECRET_SIZE;
diff --git a/lib/tls13/session_ticket.c b/lib/tls13/session_ticket.c
index 25e067fc00..d98475094a 100644
--- a/lib/tls13/session_ticket.c
+++ b/lib/tls13/session_ticket.c
@@ -112,7 +112,7 @@ unpack_ticket(gnutls_session_t session, gnutls_datum_t *packed, tls13_ticket_t *
 	/* Check if the MAC ID we got is valid */
 	prf = _gnutls_mac_to_entry(kdf);
 	if (prf == NULL)
-		return gnutls_assert_val(GNUTLS_E_INTERNAL_ERROR);
+		return gnutls_assert_val(GNUTLS_E_ILLEGAL_PARAMETER);
 
 	/* Read the ticket age add and the ticket lifetime */
 	DECR_LEN(len, 4);
@@ -133,7 +133,7 @@ unpack_ticket(gnutls_session_t session, gnutls_datum_t *packed, tls13_ticket_t *
 
 	/* Check if the size of resumption_master_secret matches the PRF */
 	if (resumption_master_secret_size != prf->output_size)
-		return gnutls_assert_val(GNUTLS_E_INTERNAL_ERROR);
+		return gnutls_assert_val(GNUTLS_E_ILLEGAL_PARAMETER);
 
 	DECR_LEN(len, resumption_master_secret_size);
 	memcpy(resumption_master_secret, p, resumption_master_secret_size);