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

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 (#13).


Change subject: oob: Add --server-probe-reply to advertise probe reply values
......................................................................

oob: Add --server-probe-reply to advertise probe reply values

Add a server option, --server-probe-reply [max-latency-diff] [weight] [prio],
each argument optional, setting the values the server returns in its OOB
PROBE_REPLY. An unconfigured server advertises weight 50 and priority 100.
max-latency-diff is how much slower than the fastest server a server may be
and still count as equally good; 0 means the client decides. All three 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/options.c
M src/openvpn/options.h
M tests/unit_tests/openvpn/test_oob.c
6 files changed, 81 insertions(+), 9 deletions(-)


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

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 2209407..3e50994 100644
--- a/src/openvpn/mudp.c
+++ b/src/openvpn/mudp.c
@@ -238,7 +238,12 @@
         /* 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. */
-        struct oob_probe_reply reply;
+        /* what we advertise; oob_build_probe_reply() adds the peer's session id */
+        struct oob_probe_reply reply = {
+            .priority = (uint16_t)m->top.options.server_probe_reply_priority,
+            .weight = (uint16_t)m->top.options.server_probe_reply_weight,
+            .max_latency_diff = (uint16_t)m->top.options.server_probe_reply_max_latency_diff,
+        };
         if (!oob_build_probe_reply(&state->newbuf, (uint64_t)now, (uint64_t)handwindow,
                                    &state->peer_session_id, &reply))
         {
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 004ad8f..c8910b8 100644
--- a/tests/unit_tests/openvpn/test_oob.c
+++ b/tests/unit_tests/openvpn/test_oob.c
@@ -372,7 +372,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)
 {
@@ -386,14 +386,18 @@
     struct session_id peer;
     memcpy(peer.id, "PEER1234", SID_SIZE);
 
-    /* left as the caller set them: the function only fills the session id */
-    struct oob_probe_reply reply = { 0 };
+    struct oob_probe_reply reply = {
+        .priority = 5,
+        .weight = 50,
+        .max_latency_diff = 25,
+    };
     assert_true(oob_build_probe_reply(&buf, now, 30, &peer, &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);
 }

-- 
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: 13
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