[M] Change in openvpn[master]: oob: Wrap the client SERVER_PROBE with tls-auth/tls-crypt

"stipa \(Code Review\) via Openvpn-devel" <[email protected]>
Newsgroups gmane.network.openvpn.devel
Message-ID <d462cffbbd7e62b898ffae1d943529253c7b888c-EmailReplacePatchSet-HTML@gerrit.openvpn.net>
Attention is currently required from: plaisthos.

Hello plaisthos, 

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

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

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


Change subject: oob: Wrap the client SERVER_PROBE with tls-auth/tls-crypt
......................................................................

oob: Wrap the client SERVER_PROBE with tls-auth/tls-crypt

The client --server-probe previously sent a plaintext SERVER_PROBE and
parsed replies by hand, so it only worked against a server with no
control-channel wrapping. Build a standalone wrapping context for the
probe (mirroring the server's tls_auth_standalone) and route both the
outgoing probe and the incoming replies through the same control-channel
path the rest of the code uses:

  - tls_wrap_oob_standalone() wraps the probe payload, applying the
    tls-auth HMAC or tls-crypt encryption (or nothing, when neither is
    configured).
  - read_control_auth() unwraps each reply, verifying the HMAC /
    decrypting and stripping the opcode + session id, on a per-packet
    copy of the wrapping context (as tls_pre_decrypt_lite() does).

With neither tls-auth nor tls-crypt configured the context stays in
TLS_WRAP_NONE and the on-wire probe is byte-for-byte identical to before,
so the plaintext case is unchanged.

tls-crypt-v2 is not supported yet: the server only learns the client key
from the WKc carried in the TLS handshake, which an out-of-band probe
cannot provide. Such configurations skip probing and keep the configured
remote order.

Change-Id: I1f9d7b5a5ec19bf77c0c212795f583c5ba4c03ac
Signed-off-by: Lev Stipakov <[email protected]>
---
M src/openvpn/oob_client.c
1 file changed, 112 insertions(+), 30 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/50/1750/9

diff --git a/src/openvpn/oob_client.c b/src/openvpn/oob_client.c
index ca0b051..b7e9a5b 100644
--- a/src/openvpn/oob_client.c
+++ b/src/openvpn/oob_client.c
@@ -29,6 +29,8 @@
 #include "oob_client.h"
 #include "openvpn.h"
 #include "oob.h"
+#include "init.h"
+#include "ssl.h"
 #include "ssl_pkt.h"
 #include "session_id.h"
 #include "socket.h"
@@ -80,21 +82,54 @@
     return (af == AF_INET6) ? PROBE_AF_V6 : PROBE_AF_V4;
 }
 
-/* Build a plaintext SERVER_PROBE packet:
- *   [opcode | key_id=0] [client session id] [SERVER_PROBE message]
- * This is the unauthenticated OOB wire format; adding tls-auth/tls-crypt
- * wrapping for the probe is a follow-up (it only works against a server with
- * no control-channel wrapping for now). */
-static bool
-oob_probe_build_packet(struct buffer *buf, const struct session_id *client_sid)
+/* Build a standalone control-channel wrapping context for the probe, mirroring
+ * the tls_auth_standalone the server uses to answer it. With neither tls-auth
+ * nor tls-crypt configured the context stays in TLS_WRAP_NONE and the probe is
+ * sent in plaintext, exactly as before; with either configured the probe is
+ * authenticated/encrypted like any other control packet. Returns NULL (and
+ * logs) for configurations the probe cannot wrap yet, in which case the caller
+ * skips probing and keeps the configured remote order. */
+static struct tls_auth_standalone *
+oob_probe_init_tls_auth_standalone(struct context *c, struct gc_arena *gc)
 {
-    const struct oob_probe_parameter param = {
-        .timestamp = (uint64_t)now,
-        .flags = 0,
-    };
-    uint8_t header = (uint8_t)(P_CONTROL_OOB_V1 << P_OPCODE_SHIFT);
-    return buf_write_u8(buf, header) && session_id_write(client_sid, buf)
-           && oob_server_probe_write(buf, &param);
+    /* The probe runs before next_connection_entry() maps a connection entry
+     * into options.ce, so take the wrapping config from the first connection
+     * entry directly. A global --tls-auth/--tls-crypt is copied into every
+     * connection-list entry by options_postprocess_mutate_ce(), so the first
+     * entry carries it even though options.ce does not yet. (Probing wraps a
+     * single packet for all remotes, so per-connection-block keys are not
+     * supported; the first entry's wrapping is used for all.) */
+    const struct connection_entry *ce = c->options.connection_list->array[0];
+
+    /* tls-crypt-v2 wraps with a per-client key the server only learns from the
+     * wrapped client key (WKc) carried in the TLS handshake. An out-of-band
+     * probe carries no WKc, so the server cannot unwrap it; skip probing rather
+     * than send something unverifiable. */
+    if (ce->tls_crypt_v2_file)
+    {
+        msg(D_LOW, "server-probe: not supported with tls-crypt-v2; using configured order");
+        return NULL;
+    }
+
+    /* Load the tls-auth/tls-crypt key material into c->c1.ks (a no-op if neither
+     * is configured). This is run again per-connection later; calling it early
+     * here is harmless. */
+    do_init_tls_wrap_key(c, ce);
+
+    struct tls_options to;
+    CLEAR(to);
+    init_tls_wrap_ctx(&to.tls_wrap, ce, c->options.tls_client, &c->c1.ks, &c->c1.pid_persist);
+    to.replay_window = c->options.replay_window;
+    to.replay_time = c->options.replay_time;
+
+    struct tls_auth_standalone *tas = tls_auth_standalone_init(&to, gc);
+
+    /* Control-channel frame and work buffers, mirroring do_init_frame_tls(). */
+    tls_init_control_channel_frame_parameters(&tas->frame, ce->tls_mtu);
+    tas->tls_wrap.work = alloc_buf_gc(BUF_SIZE(&tas->frame), gc);
+    tas->workbuf = alloc_buf_gc(BUF_SIZE(&tas->frame), gc);
+
+    return tas;
 }
 
 /* Open one native UDP probe socket per address family (v4 and v6). Each is
@@ -142,12 +177,13 @@
     }
 }
 
+
 /* Parse one received datagram as a PROBE_REPLY and, if valid and matching one
  * of the probes we sent, record the reply in @p results. */
 static void
-oob_probe_handle_reply(const uint8_t *data, int len, const struct session_id *client_sid,
-                       const struct sockaddr_storage *from, const struct probe_target *targets,
-                       struct oob_probe_result *results, int n)
+oob_probe_handle_reply(uint8_t *data, int len, const struct session_id *client_sid,
+                       const struct tls_wrap_ctx *base_wrap, const struct sockaddr_storage *from,
+                       const struct probe_target *targets, struct oob_probe_result *results, int n)
 {
     /* Need at least the opcode byte and the session id. */
     if (len < 1 + (int)SID_SIZE || (data[0] >> P_OPCODE_SHIFT) != P_CONTROL_OOB_V1)
@@ -157,7 +193,18 @@
 
     struct buffer buf;
     buf_set_read(&buf, data, (size_t)len);
-    buf_advance(&buf, 1 + SID_SIZE); /* skip opcode + server session id */
+
+    /* Unwrap the reply with the same control-channel path the server used to
+     * wrap it: this verifies the tls-auth HMAC / decrypts tls-crypt, and (in all
+     * modes) strips the opcode + server session id, leaving buf at the TLV
+     * payload. With no tls-auth/tls-crypt it just strips those header bytes, as
+     * before. read_control_auth() mutates the wrapping context, so we work on a
+     * per-packet copy (as tls_pre_decrypt_lite() does on the server). */
+    struct tls_wrap_ctx wrap = *base_wrap;
+    if (!read_control_auth(&buf, &wrap, NULL, NULL))
+    {
+        return; /* not for us, or failed authentication */
+    }
 
     struct oob_probe_reply reply;
     if (!oob_client_reply_read(&buf, &reply))
@@ -210,8 +257,9 @@
  * matches a probe we sent. Returns true if every sent probe has been answered. */
 static bool
 oob_probe_receive_slice(const struct probe_ctx *pc, const struct timeval *deadline,
-                        const struct session_id *client_sid, const struct probe_target *targets,
-                        struct oob_probe_result *results, int n, int outstanding)
+                        const struct session_id *client_sid, const struct tls_wrap_ctx *wrap,
+                        const struct probe_target *targets, struct oob_probe_result *results, int n,
+                        int outstanding)
 {
     while (true)
     {
@@ -261,7 +309,7 @@
                                     (struct sockaddr *)&from, &fromlen);
             if (len > 0)
             {
-                oob_probe_handle_reply(data, len, client_sid, &from, targets, results, n);
+                oob_probe_handle_reply(data, len, client_sid, wrap, &from, targets, results, n);
             }
         }
         if (oob_count_answered(targets, results, n) >= outstanding)
@@ -298,8 +346,8 @@
  * once the window elapses or every sent probe has been answered. */
 static void
 oob_probe_collect(const struct probe_ctx *pc, const struct buffer *probe,
-                  const struct session_id *client_sid, const struct probe_target *targets,
-                  struct oob_probe_result *results, int n)
+                  const struct session_id *client_sid, const struct tls_wrap_ctx *wrap,
+                  const struct probe_target *targets, struct oob_probe_result *results, int n)
 {
     /* number of probes we actually sent: stop early once they all answer */
     int want = 0;
@@ -323,7 +371,7 @@
             deadline.tv_usec -= 1000000;
         }
 
-        if (oob_probe_receive_slice(pc, &deadline, client_sid, targets, results, n, want))
+        if (oob_probe_receive_slice(pc, &deadline, client_sid, wrap, targets, results, n, want))
         {
             return; /* all answered */
         }
@@ -365,6 +413,16 @@
 
     struct gc_arena gc = gc_new();
 
+    /* Wrapping context for the probe (tls-auth/tls-crypt, or plaintext if
+     * neither). NULL means this configuration cannot be probed; keep the
+     * configured order. */
+    struct tls_auth_standalone *tas = oob_probe_init_tls_auth_standalone(c, &gc);
+    if (!tas)
+    {
+        gc_free(&gc);
+        return;
+    }
+
     /* A single random session id identifies all of our probes; servers echo it
      * back in the reply's peer_session_id, letting us reject spoofed replies. */
     struct session_id client_sid;
@@ -374,6 +432,7 @@
     if (oob_probe_sockets_open(&pc) == 0)
     {
         msg(D_LOW, "server-probe: could not open probe socket; using configured order");
+        tls_auth_standalone_free(tas);
         gc_free(&gc);
         return;
     }
@@ -381,17 +440,39 @@
     struct probe_target *targets = gc_malloc(sizeof(*targets) * l->len, true, &gc);
     struct oob_probe_result *results = gc_malloc(sizeof(*results) * l->len, true, &gc);
 
-    struct buffer probe = alloc_buf_gc(256, &gc);
-    if (!oob_probe_build_packet(&probe, &client_sid))
+    /* Build the probe once and reuse the same bytes for every remote and for
+     * resends: the SERVER_PROBE payload is a single probe_parameter TLV, wrapped
+     * (or sent in plaintext) like any other control packet. The client session
+     * id is prepended as the sender session id. */
+    struct buffer payload = alloc_buf_gc(64, &gc);
+    const struct oob_probe_parameter param = {
+        .timestamp = (uint64_t)now,
+        .flags = 0,
+    };
+    if (!oob_server_probe_write(&payload, &param))
     {
-        msg(D_LOW, "server-probe: could not build probe packet; using configured order");
+        msg(D_LOW, "server-probe: could not build probe payload; using configured order");
         oob_probe_sockets_close(&pc);
+        tls_auth_standalone_free(tas);
         gc_free(&gc);
         return;
     }
 
-    msg(D_LOW, "server-probe: probing %d remote(s) with a %d ms window", l->len,
-        OOB_PROBE_WINDOW_MS);
+    struct buffer probe = tls_wrap_oob_standalone(&tas->tls_wrap, tas, &client_sid, &payload);
+    if (!BLEN(&probe))
+    {
+        msg(D_LOW, "server-probe: could not wrap probe packet; using configured order");
+        oob_probe_sockets_close(&pc);
+        tls_auth_standalone_free(tas);
+        gc_free(&gc);
+        return;
+    }
+
+    const char *wrap_name = (tas->tls_wrap.mode == TLS_WRAP_CRYPT)  ? "tls-crypt"
+                            : (tas->tls_wrap.mode == TLS_WRAP_AUTH) ? "tls-auth"
+                                                                    : "none (plaintext)";
+    msg(D_LOW, "server-probe: probing %d remote(s) with a %d ms window, control-channel wrapping: %s",
+        l->len, OOB_PROBE_WINDOW_MS, wrap_name);
 
     /* Send a probe to each configured remote. */
     int sent_count = 0;
@@ -452,7 +533,7 @@
 
     if (sent_count > 0)
     {
-        oob_probe_collect(&pc, &probe, &client_sid, targets, results, l->len);
+        oob_probe_collect(&pc, &probe, &client_sid, &tas->tls_wrap, targets, results, l->len);
     }
     oob_probe_sockets_close(&pc);
 
@@ -498,5 +579,6 @@
     msg(M_INFO, "server-probe: %d of %d remote(s) answered; connecting best-first", responded,
         l->len);
 
+    tls_auth_standalone_free(tas);
     gc_free(&gc);
 }

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1750?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: I1f9d7b5a5ec19bf77c0c212795f583c5ba4c03ac
Gerrit-Change-Number: 1750
Gerrit-PatchSet: 9
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.