ext/pre_shared_key: make PSK identity parsing robustertmp-session-ticket-key-name

Previously, to determine whether a PSK identity is a ticket or a PSK
username, it relied on PskIdentity.obfuscated_ticket_age, which
"SHOULD" be 0 if the identity is a PSK username.

This patch instead checks the key name of the ticket first and then
check the constraints of the PSK username.  That way, it can
distinguish tickets and PSK usernames in a more realible manner.

Signed-off-by: Daiki Ueno <dueno@redhat.com>

2 files changed, 23 insertions, 38 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/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);