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>

diff --git a/lib/ext/pre_shared_key.c b/lib/ext/pre_shared_key.c
index 5c8a80c4a28c6c4da6b2635e68ef399200547746..dce24d80a13d28e5e9eeb472424d5dc6e99af1f2 100644 (file)
--- 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 25e067fc00868381597c206313ff35f1d3b53f68..d98475094ab4c05750e3cde55d6c377b3bd4b93e 100644 (file)
--- 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);