ext/pre_shared_key: make PSK identity parsing robuster

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>

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;
 		}
 	}
 

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);