[M] Change in openvpn[master]: oob: Add --server-probe-reply to advertise probe priority/weight

Attention is currently required from: plaisthos.

Hello plaisthos, 

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#10).


Change subject: oob: Add --server-probe-reply to advertise probe priority/weight
......................................................................

oob: Add --server-probe-reply to advertise probe priority/weight

Add a server option, --server-probe-reply <priority> <weight>, that sets
the DNS-SRV-style priority and weight the server returns in its OOB
PROBE_REPLY. Both default to 0 (the previous hardcoded behaviour), and are
range-checked to 0..65535.

The values are plumbed through oob_build_probe_reply() into the probe_reply
TLV; the client already reads and ranks remotes by them.

Change-Id: Id74cfae7e9d69029d2ddbf635ee85a2a6cedc3d8
Signed-off-by: Lev Stipakov <[email protected]>
---
M Changes.rst
M doc/man-sections/server-options.rst
M src/openvpn/mudp.c
M src/openvpn/oob.c
M src/openvpn/oob.h
M src/openvpn/options.c
M src/openvpn/options.h
M tests/unit_tests/openvpn/test_oob.c
8 files changed, 91 insertions(+), 15 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/52/1752/10

diff --git a/Changes.rst b/Changes.rst
index 1f992b2..080d928 100644
--- a/Changes.rst
+++ b/Changes.rst
@@ -1,5 +1,13 @@
 Overview of changes in 2.8
 ==========================
+New features
+------------
+Out-of-band server probing and server-controlled selection
+    With ``--server-probe``, a client probes all configured UDP remotes
+    before connecting and reorders them based on the replies: reachable
+    servers are tried first, ordered by server-advertised priority,
+    measured latency and advertised weight with DNS-SRV-like semantics.
+    Servers advertise these values with ``--server-probe-reply``.
 
 
 Overview of changes in 2.7
diff --git a/doc/man-sections/server-options.rst b/doc/man-sections/server-options.rst
index f420588..1277cf7 100644
--- a/doc/man-sections/server-options.rst
+++ b/doc/man-sections/server-options.rst
@@ -662,6 +662,31 @@
   Pushing of the ``--tun-ipv6`` directive is done for older clients which
   require an explicit ``--tun-ipv6`` in their configuration.
 
+--server-probe-reply args
+  Set the values a server advertises in its replies to out-of-band
+  probes from clients using ``--server-probe``.
+
+  Valid syntaxes::
+
+     server-probe-reply max-latency-diff
+     server-probe-reply max-latency-diff weight
+     server-probe-reply max-latency-diff weight priority
+
+  ``max-latency-diff`` is the candidate-band margin in milliseconds: a
+  probing client treats servers of the same priority whose round-trip
+  time is within this margin of the fastest one as equally good.
+  :code:`0` (the default) lets the client use its own margin.
+
+  ``weight`` (default :code:`50`) and ``priority`` (default :code:`100`)
+  have DNS SRV (RFC 2782) semantics: clients try servers with a lower
+  priority value first, and distribute load between equally-good
+  servers of the same priority proportionally to their weights.
+
+  All values are in the range :code:`0` to :code:`65535`. A UDP server
+  answers probes regardless of this option; replies are stateless,
+  replay-protected and rate-limited. The option only controls the
+  advertised values.
+
 --stale-routes-check args
   Remove routes which haven't had activity for ``n`` seconds (i.e. the ageing
   time).  This check is run every ``t`` seconds (i.e. check interval).
diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c
index 2a25053..9e33e3c 100644
--- a/src/openvpn/mudp.c
+++ b/src/openvpn/mudp.c
@@ -239,7 +239,11 @@
          * tls-auth/tls-crypt wrapping). Answer it without creating a session. */
         struct oob_probe_reply reply;
         if (!oob_build_probe_reply(&state->newbuf, (uint64_t)now, (uint64_t)handwindow,
-                                   &state->peer_session_id, &reply))
+                                   &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))
         {
             /* malformed or replayed/stale probe: silently drop */
             return false;
diff --git a/src/openvpn/oob.c b/src/openvpn/oob.c
index 3598d5b..79a2177 100644
--- a/src/openvpn/oob.c
+++ b/src/openvpn/oob.c
@@ -236,7 +236,8 @@
 
 bool
 oob_build_probe_reply(struct buffer *probe_payload, uint64_t now, uint64_t window_secs,
-                      const struct session_id *peer_sid, struct oob_probe_reply *reply)
+                      const struct session_id *peer_sid, uint16_t priority, uint16_t weight,
+                      uint16_t max_latency_diff, struct oob_probe_reply *reply)
 {
     struct oob_probe_parameter param;
     if (!oob_server_probe_read(probe_payload, &param))
@@ -252,7 +253,10 @@
 
     memset(reply, 0, sizeof(*reply));
     reply->peer_session_id = *peer_sid;
-    /* priority/weight/connect_lifetime/flags left at 0 for now */
+    reply->priority = priority;
+    reply->weight = weight;
+    reply->max_latency_diff = max_latency_diff;
+    /* connect_lifetime/flags left at 0 for now */
     return true;
 }
 
diff --git a/src/openvpn/oob.h b/src/openvpn/oob.h
index 2202b60..0f1cd08 100644
--- a/src/openvpn/oob.h
+++ b/src/openvpn/oob.h
@@ -205,8 +205,8 @@
  * 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 remaining fields
- * zeroed, ready to be wrapped and sent.
+ * populated with the peer's session id echoed back and the given priority and
+ * weight (other fields zeroed), ready to be wrapped and sent.
  *
  * This is the transport-agnostic decision step; the caller performs the send.
  *
@@ -214,11 +214,15 @@
  * @param now            current time (UNIX seconds)
  * @param window_secs    acceptable timestamp skew, in either direction
  * @param peer_sid       session id of the requesting peer (echoed in the reply)
+ * @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 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, struct oob_probe_reply *reply);
+                           const struct session_id *peer_sid, uint16_t priority, uint16_t weight,
+                           uint16_t max_latency_diff, struct oob_probe_reply *reply);
 
 /* Candidate-band margin (ms) used when neither the client nor the server
  * specifies one. */
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index dc88a55..6edb78b 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -805,10 +805,14 @@
     o->ce.proto = PROTO_UDP;
     o->ce.af = AF_UNSPEC;
 
-    /* The client latency margin is -1 = "not set": the client's value is
-     * authoritative when given, otherwise each server's advertised margin (or
-     * the built-in default) applies. */
+    /* server-probe defaults. The client latency margin is -1 = "not set": the
+     * client's value is authoritative when given, otherwise each server's
+     * advertised margin (or the built-in default) applies. An (unconfigured)
+     * server advertises weight 50 / priority 100 and no margin (0). */
     o->server_probe_latency_margin = -1;
+    o->server_probe_reply_weight = 50;
+    o->server_probe_reply_priority = 100;
+    o->server_probe_reply_max_latency_diff = 0;
     o->ce.bind_ipv6_only = false;
     o->ce.connect_retry_seconds = 1;
     o->ce.connect_retry_seconds_max = 300;
@@ -6527,6 +6531,26 @@
             options->server_probe_latency_margin = margin;
         }
     }
+    else if (streq(p[0], "server-probe-reply") && !p[4])
+    {
+        VERIFY_PERMISSION(OPT_P_GENERAL);
+        /* --server-probe-reply [max-latency-diff] [weight] [prio]; each optional */
+        int vals[3] = { options->server_probe_reply_max_latency_diff,
+                        options->server_probe_reply_weight,
+                        options->server_probe_reply_priority };
+        for (int i = 0; i < 3 && p[i + 1]; i++)
+        {
+            vals[i] = positive_atoi(p[i + 1], msglevel);
+            if (vals[i] > 0xffff)
+            {
+                msg(msglevel, "--server-probe-reply: values must be 0 to 65535");
+                goto err;
+            }
+        }
+        options->server_probe_reply_max_latency_diff = vals[0];
+        options->server_probe_reply_weight = vals[1];
+        options->server_probe_reply_priority = vals[2];
+    }
     else if (streq(p[0], "nice") && p[1] && !p[2])
     {
         VERIFY_PERMISSION(OPT_P_NICE);
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index 6b1120a..6e4829d 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -339,6 +339,12 @@
     /* client: default candidate-band margin in ms (--server-probe [max-latency-diff]):
      * servers within this RTT of the fastest are treated as equally fast */
     int server_probe_latency_margin;
+    /* server: values advertised in the OOB PROBE_REPLY (--server-probe-reply).
+     * priority/weight follow DNS-SRV semantics; max_latency_diff overrides the
+     * client's margin for this server (0 = defer to the client). */
+    int server_probe_reply_priority;
+    int server_probe_reply_weight;
+    int server_probe_reply_max_latency_diff;
 
     bool mlock;
 
diff --git a/tests/unit_tests/openvpn/test_oob.c b/tests/unit_tests/openvpn/test_oob.c
index 3657291..8861d68 100644
--- a/tests/unit_tests/openvpn/test_oob.c
+++ b/tests/unit_tests/openvpn/test_oob.c
@@ -305,7 +305,7 @@
 }
 
 /* A valid, in-window SERVER_PROBE yields a reply that echoes the peer's
- * session id and zeroes the remaining fields. */
+ * session id and carries the configured priority and weight. */
 static void
 test_build_probe_reply_valid(void **state)
 {
@@ -320,12 +320,13 @@
     memcpy(peer.id, "PEER1234", SID_SIZE);
 
     struct oob_probe_reply reply;
-    assert_true(oob_build_probe_reply(&buf, now, 30, &peer, &reply));
+    assert_true(oob_build_probe_reply(&buf, now, 30, &peer, 5, 50, 25, &reply));
     assert_memory_equal(reply.peer_session_id.id, peer.id, SID_SIZE);
-    assert_int_equal(reply.priority, 0);
-    assert_int_equal(reply.weight, 0);
+    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.max_latency_diff, 25);
 
     gc_free(&gc);
 }
@@ -343,7 +344,7 @@
 
     struct session_id peer = { 0 };
     struct oob_probe_reply reply;
-    assert_false(oob_build_probe_reply(&buf, now, 30, &peer, &reply));
+    assert_false(oob_build_probe_reply(&buf, now, 30, &peer, 0, 0, 0, &reply));
 
     gc_free(&gc);
 }
@@ -361,7 +362,7 @@
 
     struct session_id peer = { 0 };
     struct oob_probe_reply reply;
-    assert_false(oob_build_probe_reply(&buf, 1000000, 30, &peer, &reply));
+    assert_false(oob_build_probe_reply(&buf, 1000000, 30, &peer, 0, 0, 0, &reply));
 
     gc_free(&gc);
 }

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

Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Id74cfae7e9d69029d2ddbf635ee85a2a6cedc3d8
Gerrit-Change-Number: 1752
Gerrit-PatchSet: 10
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