ext/pre_shared_key: fix binder calculation when HRR is sent

In that case, ClientHello1 and HelloRetryRequest are included in the
PSK binder computation, not only the truncated ClientHello2.

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 0d3468c40d564c4d96f9239b44d27661e6048c31..0fa7df2d271a5c1d9966483d5b359f2192ef2a95 100644 (file)
--- a/lib/ext/pre_shared_key.c
+++ b/lib/ext/pre_shared_key.c
@@ -58,20 +58,31 @@ compute_binder_key(const mac_entry_st *prf,
 }
 
 static int
-compute_psk_binder(unsigned entity,
+compute_psk_binder(gnutls_session_t session,
                const mac_entry_st *prf, unsigned binders_length, unsigned hash_size,
                int exts_length, int ext_offset,
                const gnutls_datum_t *psk, const gnutls_datum_t *client_hello,
                void *out)
 {
        int ret;
-       unsigned extensions_len_pos;
+       unsigned client_hello_pos, extensions_len_pos;
        gnutls_buffer_st handshake_buf;
        uint8_t binder_key[MAX_HASH_SIZE];
 
        _gnutls_buffer_init(&handshake_buf);
 
-       if (entity == GNUTLS_CLIENT) {
+       if (session->security_parameters.entity == GNUTLS_CLIENT) {
+               if (session->internals.hsk_flags & HSK_HRR_RECEIVED) {
+                       ret = gnutls_buffer_append_data(&handshake_buf,
+                                                       (const void *) session->internals.handshake_hash_buffer.data,
+                                                       session->internals.handshake_hash_buffer.length);
+                       if (ret < 0) {
+                               gnutls_assert();
+                               goto error;
+                       }
+               }
+
+               client_hello_pos = handshake_buf.length;
                ret = gnutls_buffer_append_data(&handshake_buf,
                                (const void *)  client_hello->data,
                                client_hello->size);
@@ -81,23 +92,40 @@ compute_psk_binder(unsigned entity,
                }
 
                /* This is a ClientHello message */
-               handshake_buf.data[0] = GNUTLS_HANDSHAKE_CLIENT_HELLO;
+               handshake_buf.data[client_hello_pos] = GNUTLS_HANDSHAKE_CLIENT_HELLO;
 
                /*
                 * At this point we have not yet added the binders to the ClientHello,
                 * but we have to overwrite the size field, pretending as if binders
                 * of the correct length were present.
                 */
-               _gnutls_write_uint24(handshake_buf.length + binders_length - 2, &handshake_buf.data[1]);
-               _gnutls_write_uint16(handshake_buf.length + binders_length - ext_offset,
-                               &handshake_buf.data[ext_offset]);
+               _gnutls_write_uint24(handshake_buf.length - client_hello_pos + binders_length - 2, &handshake_buf.data[client_hello_pos + 1]);
+               _gnutls_write_uint16(handshake_buf.length - client_hello_pos + binders_length - ext_offset,
+                               &handshake_buf.data[client_hello_pos + ext_offset]);
 
-               extensions_len_pos = handshake_buf.length - exts_length - 2;
+               extensions_len_pos = handshake_buf.length - client_hello_pos - exts_length - 2;
                _gnutls_write_uint16(exts_length + binders_length + 2,
-                               &handshake_buf.data[extensions_len_pos]);
+                               &handshake_buf.data[client_hello_pos + extensions_len_pos]);
        } else {
-               if (unlikely(client_hello->size <= binders_length))
-                       return gnutls_assert_val(GNUTLS_E_RECEIVED_ILLEGAL_PARAMETER);
+               if (session->internals.hsk_flags & HSK_HRR_SENT) {
+                       if (unlikely(session->internals.handshake_hash_buffer.length <= client_hello->size)) {
+                               ret = gnutls_assert_val(GNUTLS_E_RECEIVED_ILLEGAL_PARAMETER);
+                               goto error;
+                       }
+
+                       ret = gnutls_buffer_append_data(&handshake_buf,
+                                                       (const void *) session->internals.handshake_hash_buffer.data,
+                                                       session->internals.handshake_hash_buffer.length - client_hello->size);
+                       if (ret < 0) {
+                               gnutls_assert();
+                               goto error;
+                       }
+               }
+
+               if (unlikely(client_hello->size <= binders_length)) {
+                       ret = gnutls_assert_val(GNUTLS_E_RECEIVED_ILLEGAL_PARAMETER);
+                       goto error;
+               }
 
                ret = gnutls_buffer_append_data(&handshake_buf,
                                                (const void *) client_hello->data,
@@ -194,7 +222,7 @@ client_send_params(gnutls_session_t session,
        client_hello.data = extdata->data+sizeof(mbuffer_st);
        client_hello.size = extdata->length-sizeof(mbuffer_st);
 
-       ret = compute_psk_binder(GNUTLS_CLIENT, prf,
+       ret = compute_psk_binder(session, prf,
                                 hash_size+1, hash_size, extdata->length-pos,
                                 ext_offset, &key, &client_hello,
                                 binder_value);
@@ -314,7 +342,7 @@ static int server_recv_params(gnutls_session_t session,
        /* Compute the binder value for this PSK */
        prf = pskcred->binder_algo;
        hash_size = prf->output_size;
-       ret = compute_psk_binder(GNUTLS_SERVER, prf, psk_parser.binder_len+2, hash_size, 0, 0,
+       ret = compute_psk_binder(session, prf, psk_parser.binder_len+2, hash_size, 0, 0,
                                 &key, &full_client_hello,
                                 binder_value);
        if (ret < 0) {

diff --git a/lib/tls13/hello_retry.c b/lib/tls13/hello_retry.c
index 51f545ec00606638edfb0274062f2301db1bf4ac..5676c52780e24b8a6fc3ad96567d1567d1465239 100644 (file)
--- a/lib/tls13/hello_retry.c
+++ b/lib/tls13/hello_retry.c
@@ -89,6 +89,9 @@ int _gnutls13_send_hello_retry_request(gnutls_session_t session, unsigned again)
                /* reset extensions sent by this session to allow re-sending them */
                session->internals.used_exts = 0;
 
+               if (session->key.psk_needs_free)
+                       _gnutls_free_temp_key_datum(&session->key.psk);
+
                bufel = _gnutls_buffer_to_mbuffer(&buf);
        }
 

diff --git a/tests/psk-file.c b/tests/psk-file.c
index a73031193f35379936bf96fafb312c9be6e1b5d9..e1e058ffe95b4d9e38ad696c921d5b42f0d5ee9b 100644 (file)
--- a/tests/psk-file.c
+++ b/tests/psk-file.c
@@ -87,7 +87,7 @@ static void client(int sd, const char *prio, const char *user, const gnutls_datu
 
        /* Initialize TLS session
         */
-       gnutls_init(&session, GNUTLS_CLIENT);
+       gnutls_init(&session, GNUTLS_CLIENT|GNUTLS_KEY_SHARE_TOP);
 
        /* Use default priorities */
        assert(gnutls_priority_set_direct(session, prio, NULL)>=0);
@@ -392,6 +392,9 @@ void doit(void)
        run_test2("NORMAL:-VERS-ALL:+VERS-TLS1.3:+PSK:+DHE-PSK", NULL, "non-hex", &key, 0, 0, GNUTLS_E_FATAL_ALERT_RECEIVED, GNUTLS_E_KEYFILE_ERROR);
        run_test2("NORMAL:-VERS-ALL:+VERS-TLS1.3:+PSK:+DHE-PSK", NULL, "unknown", &key, 0, 0, GNUTLS_E_FATAL_ALERT_RECEIVED, GNUTLS_E_RECEIVED_ILLEGAL_PARAMETER);
        run_test2("NORMAL:-VERS-ALL:+VERS-TLS1.3:+PSK:+DHE-PSK", NULL, "jas", &wrong_key, 0, 0, GNUTLS_E_FATAL_ALERT_RECEIVED, GNUTLS_E_RECEIVED_ILLEGAL_PARAMETER);
+
+       /* try with HelloRetryRequest and PSK */
+       run_test2("NORMAL:-VERS-ALL:+VERS-TLS1.3:+DHE-PSK:-GROUP-ALL:+GROUP-FFDHE2048:+GROUP-FFDHE4096", "NORMAL:-VERS-ALL:+VERS-TLS1.3:+DHE-PSK:-GROUP-ALL:+GROUP-FFDHE4096", "jas", &key, 0, GNUTLS_KX_DHE_PSK, 0, 0);
 }
 
 #endif                         /* _WIN32 */