[S] Change in openvpn[master]: oob: fall back quickly when a handshake shortcut is ignored

Attention is currently required from: plaisthos.

Hello plaisthos,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1771?usp=email

to review the following change.


Change subject: oob: fall back quickly when a handshake shortcut is ignored
......................................................................

oob: fall back quickly when a handshake shortcut is ignored

If the server does not accept the shortcut third packet (e.g. a middlebox
drops the larger P_CONTROL_WKC_V1, or a version/bug mismatch), the client
previously stalled for the full handshake_window (~60s) before recovering.

Give the shortcut session a short first-response deadline
(min(handshake_window, 5s)); once the server answers, tls_pre_decrypt()
restores the full window so a slow-but-working handshake is not cut off. If no
response arrives, the session times out quickly and the normal handshake
recovery kicks in, so a rejected shortcut costs a few seconds instead of ~60.

Change-Id: Icec4696ff263ab39ebab06e8d478395a5ddfaab9
Signed-off-by: Lev Stipakov <[email protected]>
---
M src/openvpn/ssl.c
M src/openvpn/ssl_common.h
2 files changed, 31 insertions(+), 1 deletion(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/71/1771/1

diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 4503a05..66f7af4 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -2516,6 +2516,11 @@
     return session_move_pre_start(session, ks, true);
 }
 
+/* Seconds to wait for the server's first response to an OOB handshake shortcut
+ * before giving up and falling back to a normal handshake. A couple of
+ * control-channel retransmits; capped at handshake_window by the caller. */
+#define OOB_SHORTCUT_FALLBACK_SECS 5
+
 bool
 session_skip_to_pre_start_client(struct tls_session *session, const struct session_id *client_sid,
                                  const struct session_id *server_sid,
@@ -2566,6 +2571,14 @@
         return false;
     }
     ks->state = S_PRE_START;
+
+    /* Fail fast if the server ignores the shortcut: shorten the negotiation
+     * window to a few seconds (a couple of control-channel retransmits). If the
+     * server answers, tls_pre_decrypt() restores the full window; if not, the
+     * session times out quickly and reset_session() retries with a normal
+     * handshake, rather than stalling for the whole handshake_window. */
+    ks->oob_shortcut = true;
+    ks->must_negotiate = now + min_int(session->opt->handshake_window, OOB_SHORTCUT_FALLBACK_SECS);
     return true;
 }
 
@@ -2807,9 +2820,14 @@
     /* Are we timed out on receive? */
     if (now >= ks->must_negotiate && ks->state >= S_UNDEF && ks->state < S_ACTIVE)
     {
+        /* Report the window that actually applied: an unanswered OOB handshake
+         * shortcut times out on the short fallback deadline, not handshake_window. */
+        int window = ks->oob_shortcut
+                         ? min_int(session->opt->handshake_window, OOB_SHORTCUT_FALLBACK_SECS)
+                         : session->opt->handshake_window;
         msg(D_TLS_ERRORS,
             "TLS Error: TLS key negotiation failed to occur within %d seconds (check your network connectivity)",
-            session->opt->handshake_window);
+            window);
         goto error;
     }
 
@@ -3879,6 +3897,15 @@
     /* Let our caller know we processed a control channel packet */
     ret = true;
 
+    /* First valid response to an OOB handshake shortcut: the server accepted it,
+     * so restore the normal negotiation window (it was shortened to fail fast if
+     * the shortcut had been ignored). */
+    if (ks->oob_shortcut)
+    {
+        ks->oob_shortcut = false;
+        ks->must_negotiate = now + session->opt->handshake_window;
+    }
+
     /*
      * Set our remote address and remote session_id
      */
diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
index 6f310a5..44f9dc0 100644
--- a/src/openvpn/ssl_common.h
+++ b/src/openvpn/ssl_common.h
@@ -227,6 +227,9 @@
     time_t initial;                        /* when we created this session */
     time_t established;                    /* when our state went S_ACTIVE */
     time_t must_negotiate;                 /* key negotiation times out if not finished before this time */
+    bool oob_shortcut;                     /* OOB handshake shortcut in progress: must_negotiate is a
+                                            * short first-response deadline, restored to the full window
+                                            * once the server answers (see session_skip_to_pre_start_client) */
     time_t must_die;                       /* this object is destroyed at this time */
     time_t peer_last_packet;               /* Last time we received a packet in this control session */
 

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1771?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Icec4696ff263ab39ebab06e8d478395a5ddfaab9
Gerrit-Change-Number: 1771
Gerrit-PatchSet: 1
Gerrit-Owner: stipa <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>

_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel