Optimise three-way handshake condition for S_PRE_START to S_START

We move to the S_START when we have finished the three-way handshake. After
the three way handshake is done, the client will send the TLS Client Hello
packet.

Currently we consider the three way handshake only complete if all
outgoing packets have been acked (which in this case is the one
HARD_RESET_CLIENT or HARD_RESET_SERVER) and also all ACKs for incoming
packets have been sent out.

Waiting for the ack of our own packet is important as it signals that the
other side is really responding. However, the need to also send out all
ACKs for packets we received before moving to the next state breaks
piggybacking the ACKs onto the next control packet.

With this change both server and client will only send a P_CONTROL_V1 with
the TLS Client Hello and the TLS Server Hello with piggybacked ack instead
sending an P_ACK_V1 + P_CONTROL_V1, reducing the number of packets in a
handshake by 2.

This also allows the server to avoid resending P_CONTROL_HARD_RESET_V2
to complete the three-way handshake with HMAC. Only packets with
an ACK contain the remote session id that we need for HMAC session id
verification. The ACK_V1 packet that complets this three-way handshake
can get lost. But the P_CONTROL_V1 with the piggybacked ACK will get
retransmitted. This allows to put the burden of retransmission fully on
the client.

The S_GOT_KEY/S_SENT_KEY -> S_ACTIVE is similar. We do not need to wait
for the ack packet to be sent to move the state forward. This has however
no effect on actual packets since there are normally no outstanding ACKs
here.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20220422142953.3805364-14-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24161.html

Signed-off-by: Gert Doering <gert@greenie.muc.de>

1 files changed, 5 insertions, 10 deletions

diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 2bd8006..a0ba4cf 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -1795,13 +1795,6 @@ flush_payload_buffer(struct key_state *ks)
     }
 }
 
-/* true if no in/out acknowledgements pending */
-static bool
-no_pending_reliable_packets(struct key_state *ks)
-{
-    return (reliable_empty(ks->send_reliable) && reliable_ack_empty(ks->rec_ack));
-}
-
 /*
  * Move the active key to the lame duck key and reinitialize the
  * active key.
@@ -2646,8 +2639,10 @@ tls_process_state(struct tls_multi *multi,
         goto error;
     }
 
-    /* Wait for Initial Handshake ACK */
-    if (ks->state == S_PRE_START && no_pending_reliable_packets(ks))
+    /* Check if the initial three-way Handshake is complete.
+     * We consider the handshake to be complete when our own initial
+     * packet has been successfully ACKed. */
+    if (ks->state == S_PRE_START && reliable_empty(ks->send_reliable))
     {
         ks->state = S_START;
         state_change = true;
@@ -2660,7 +2655,7 @@ tls_process_state(struct tls_multi *multi,
     /* Wait for ACK */
     if (((ks->state == S_GOT_KEY && !session->opt->server)
          || (ks->state == S_SENT_KEY && session->opt->server))
-        && no_pending_reliable_packets(ks))
+        && reliable_empty(ks->send_reliable))
     {
         session_move_active(multi, session, to_link_socket_info, ks);
         state_change = true;