[M] Change in openvpn[master]: oob: advertise a connect_lifetime handshake shortcut

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/+/1768?usp=email

to review the following change.


Change subject: oob: advertise a connect_lifetime handshake shortcut
......................................................................

oob: advertise a connect_lifetime handshake shortcut

A server answering an out-of-band SERVER_PROBE now also advertises a
connect_lifetime in the PROBE_REPLY: the seconds the reply doubles as a
handshake shortcut, letting a probing client reuse it as the server's
HARD_RESET and start the three-way handshake at the third packet.

The value is inferred, not configurable: the reply is only valid as a shortcut
while its stateless SYN-cookie is, i.e. the guaranteed cookie window of
~handshake_window (2 quantised buckets; see check_session_hmac_and_pkt_id), so
the server advertises exactly that. A tls-crypt-v2 probe (unwrapped via its
WKc) also sets OOB_PROBE_REPLY_FLAG_RESEND_WKC, telling the client to resend
the WKc when completing the handshake, since the server keeps no state.

Only the wire advertisement and its plumbing through oob_build_probe_reply()
are added here; the client side that acts on it follows.

Change-Id: Ib2b6c2246f9d9c0a505292ee8d879f714901ffae
Signed-off-by: Lev Stipakov <[email protected]>
---
M src/openvpn/mudp.c
M src/openvpn/oob.c
M src/openvpn/oob.h
M tests/unit_tests/openvpn/test_oob.c
4 files changed, 39 insertions(+), 11 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/68/1768/1

diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c
index 7ce7ecc..4566fd7 100644
--- a/src/openvpn/mudp.c
+++ b/src/openvpn/mudp.c
@@ -239,13 +239,29 @@
         /* Out-of-band server probe. state->newbuf points at the TLV payload
          * (read_control_auth has stripped the opcode, session id and any
          * tls-auth/tls-crypt wrapping). Answer it without creating a session. */
+
+        /* A tls-crypt-v2 probe was unwrapped via its WKc (cleanup_key_ctx is set
+         * by tls_crypt_v2_extract_client_key); such a client must resend the WKc
+         * if it later uses this reply as a handshake shortcut, since we keep no
+         * state. */
+        uint32_t reply_flags =
+            state->tls_wrap_tmp.cleanup_key_ctx ? OOB_PROBE_REPLY_FLAG_RESEND_WKC : 0;
+
+        /* The shortcut third packet validates only while its SYN-cookie does, so
+         * the advertised connect_lifetime is inferred (not configurable): the
+         * guaranteed cookie window of ~handshake_window (2 quantised buckets; see
+         * check_session_hmac_and_pkt_id). Advertising more would make the client
+         * trust an already-expired cookie. (RFC: connect_lifetime is how long the
+         * server considers the reply valid.) */
+        int connect_lifetime = 2 * ((handwindow + 1) / 2);
+
         struct oob_probe_reply reply;
         if (!oob_build_probe_reply(&state->newbuf, (uint64_t)now, (uint64_t)handwindow,
                                    &state->peer_session_id,
                                    (uint16_t)m->top.options.server_probe_reply_priority,
                                    (uint16_t)m->top.options.server_probe_reply_weight,
                                    (uint16_t)m->top.options.server_probe_reply_max_latency_diff,
-                                   &reply))
+                                   (uint16_t)connect_lifetime, reply_flags, &reply))
         {
             /* malformed or replayed/stale probe: silently drop */
             return false;
diff --git a/src/openvpn/oob.c b/src/openvpn/oob.c
index 2f51ad5..00dd9a4 100644
--- a/src/openvpn/oob.c
+++ b/src/openvpn/oob.c
@@ -223,7 +223,8 @@
 bool
 oob_build_probe_reply(struct buffer *probe_payload, uint64_t now, uint64_t window_secs,
                       const struct session_id *peer_sid, uint16_t priority, uint16_t weight,
-                      uint16_t max_latency_diff, struct oob_probe_reply *reply)
+                      uint16_t max_latency_diff, uint16_t connect_lifetime, uint32_t flags,
+                      struct oob_probe_reply *reply)
 {
     struct oob_probe_parameter param;
     if (!oob_server_probe_read(probe_payload, &param))
@@ -242,7 +243,8 @@
     reply->priority = priority;
     reply->weight = weight;
     reply->max_latency_diff = max_latency_diff;
-    /* connect_lifetime/flags left at 0 for now */
+    reply->connect_lifetime = connect_lifetime;
+    reply->flags = flags;
     return true;
 }
 
diff --git a/src/openvpn/oob.h b/src/openvpn/oob.h
index 0acad80..44a51ec 100644
--- a/src/openvpn/oob.h
+++ b/src/openvpn/oob.h
@@ -197,13 +197,19 @@
  */
 bool oob_timestamp_in_window(uint64_t probe_ts, uint64_t now, uint64_t window_secs);
 
+/* probe_reply flags (the reply TLV's 32-bit flags field) */
+/* bit 0: the client must resend the wrapped client key (via P_CONTROL_WKC_V1)
+ * when it completes the three-way handshake using this reply as a shortcut. Set
+ * by a tls-crypt-v2 server, which is stateless and discarded the WKc. */
+#define OOB_PROBE_REPLY_FLAG_RESEND_WKC 0x1
+
 /**
  * Process the TLV payload of a received SERVER_PROBE and decide whether to
  * answer it. Combines oob_server_probe_read() and oob_timestamp_in_window():
  * the probe is dropped (false returned) if it has no valid probe_parameter or
  * its timestamp is outside the acceptable window. On success @p reply is
- * populated with the peer's session id echoed back and the given priority and
- * weight (other fields zeroed), ready to be wrapped and sent.
+ * populated with the peer's session id echoed back and the advertised values,
+ * ready to be wrapped and sent.
  *
  * This is the transport-agnostic decision step; the caller performs the send.
  *
@@ -214,12 +220,15 @@
  * @param priority       priority to advertise (DNS-SRV semantics; lower preferred)
  * @param weight         weight to advertise (DNS-SRV semantics; higher preferred)
  * @param max_latency_diff  candidate-band margin (ms) to advertise; 0 = defer to client
+ * @param connect_lifetime  seconds the reply is valid as a handshake shortcut; 0 = none
+ * @param flags          reply flags to advertise (OOB_PROBE_REPLY_FLAG_*)
  * @param reply          filled with the reply to send on success
  * @return true if a reply should be sent, false to silently drop the probe
  */
 bool oob_build_probe_reply(struct buffer *probe_payload, uint64_t now, uint64_t window_secs,
                            const struct session_id *peer_sid, uint16_t priority, uint16_t weight,
-                           uint16_t max_latency_diff, struct oob_probe_reply *reply);
+                           uint16_t max_latency_diff, uint16_t connect_lifetime, uint32_t flags,
+                           struct oob_probe_reply *reply);
 
 /* Candidate-band margin (ms) used when neither the client nor the server
  * specifies one. */
diff --git a/tests/unit_tests/openvpn/test_oob.c b/tests/unit_tests/openvpn/test_oob.c
index e703db5..8ffbdc7 100644
--- a/tests/unit_tests/openvpn/test_oob.c
+++ b/tests/unit_tests/openvpn/test_oob.c
@@ -300,12 +300,13 @@
     memcpy(peer.id, "PEER1234", SID_SIZE);
 
     struct oob_probe_reply reply;
-    assert_true(oob_build_probe_reply(&buf, now, 30, &peer, 5, 50, 25, &reply));
+    assert_true(oob_build_probe_reply(&buf, now, 30, &peer, 5, 50, 25, 120,
+                                      OOB_PROBE_REPLY_FLAG_RESEND_WKC, &reply));
     assert_memory_equal(reply.peer_session_id.id, peer.id, SID_SIZE);
     assert_int_equal(reply.priority, 5);
     assert_int_equal(reply.weight, 50);
-    assert_int_equal(reply.connect_lifetime, 0);
-    assert_int_equal(reply.flags, 0);
+    assert_int_equal(reply.connect_lifetime, 120);
+    assert_int_equal(reply.flags, OOB_PROBE_REPLY_FLAG_RESEND_WKC);
     assert_int_equal(reply.max_latency_diff, 25);
 
     gc_free(&gc);
@@ -324,7 +325,7 @@
 
     struct session_id peer = { 0 };
     struct oob_probe_reply reply;
-    assert_false(oob_build_probe_reply(&buf, now, 30, &peer, 0, 0, 0, &reply));
+    assert_false(oob_build_probe_reply(&buf, now, 30, &peer, 0, 0, 0, 0, 0, &reply));
 
     gc_free(&gc);
 }
@@ -342,7 +343,7 @@
 
     struct session_id peer = { 0 };
     struct oob_probe_reply reply;
-    assert_false(oob_build_probe_reply(&buf, 1000000, 30, &peer, 0, 0, 0, &reply));
+    assert_false(oob_build_probe_reply(&buf, 1000000, 30, &peer, 0, 0, 0, 0, 0, &reply));
 
     gc_free(&gc);
 }

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1768?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: Ib2b6c2246f9d9c0a505292ee8d879f714901ffae
Gerrit-Change-Number: 1768
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