[M] Change in openvpn[master]: oob: Unwrap tls-crypt-v2 server probes (P_CONTROL_OOB_WKC_V1)

"stipa \(Code Review\) via Openvpn-devel" <[email protected]>
Newsgroups gmane.network.openvpn.devel
Message-ID <352887ec624bc7699d88f2ec43dd0959e2317de4-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/+/1758?usp=email

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


Change subject: oob: Unwrap tls-crypt-v2 server probes (P_CONTROL_OOB_WKC_V1)
......................................................................

oob: Unwrap tls-crypt-v2 server probes (P_CONTROL_OOB_WKC_V1)

A tls-crypt-v2 server learns the per-client key only from the wrapped
client key (WKc). An out-of-band probe belongs to no session, so the
client must carry the WKc with the probe for the server to unwrap it.

Add the receive/protocol side of this:

  - new opcode P_CONTROL_OOB_WKC_V1 (13): like P_CONTROL_OOB_V1 but with
    the WKc appended, mirroring CONTROL_HARD_RESET_CLIENT_V3/CONTROL_WKC_V1
  - tls_wrap_control() appends the WKc for opcode 13; read_control_auth()
    runs tls_crypt_v2_extract_client_key() for it, loading the per-client
    key before unwrapping the message
  - tls_pre_decrypt_lite() accepts opcode 13 and returns VERDICT_VALID_OOB_V1
  - tls_wrap_oob_standalone() takes the opcode; the server reply stays a
    plain P_CONTROL_OOB_V1 (the server already holds the key after unwrap)
  - test_pkt: round-trip test of the wrap/extract/unwrap path

The client still sends plain probes; sending P_CONTROL_OOB_WKC_V1 is added
in a following commit.

Change-Id: Idb8c0c660a98de8ec549b69f6692a7b41b092eaf
---
M src/openvpn/mudp.c
M src/openvpn/oob_client.c
M src/openvpn/ssl_pkt.c
M src/openvpn/ssl_pkt.h
M tests/unit_tests/openvpn/test_pkt.c
5 files changed, 157 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/58/1758/7

diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c
index 9e33e3c..77718e7 100644
--- a/src/openvpn/mudp.c
+++ b/src/openvpn/mudp.c
@@ -109,7 +109,12 @@
     reset_packet_id_send(&state->tls_wrap_tmp.opt.packet_id.send);
     state->tls_wrap_tmp.opt.packet_id.rec.initialized = true;
 
-    struct buffer buf = tls_wrap_oob_standalone(&state->tls_wrap_tmp, tas, own_sid, &payload);
+    /* The reply carries no WKc: with tls-crypt-v2 the server has already
+     * recovered the per-client key from the request's WKc (it is loaded into
+     * state->tls_wrap_tmp), so the reply is wrapped with that key as a plain
+     * P_CONTROL_OOB_V1 message. */
+    struct buffer buf =
+        tls_wrap_oob_standalone(&state->tls_wrap_tmp, tas, own_sid, &payload, P_CONTROL_OOB_V1);
     send_standalone_reply(m, &buf, "Server Probe", "Server probe from client, sending probe reply",
                           sock);
 
diff --git a/src/openvpn/oob_client.c b/src/openvpn/oob_client.c
index 7705258..6397568 100644
--- a/src/openvpn/oob_client.c
+++ b/src/openvpn/oob_client.c
@@ -493,7 +493,8 @@
         return;
     }
 
-    struct buffer probe = tls_wrap_oob_standalone(&tas->tls_wrap, tas, &client_sid, &payload);
+    struct buffer probe =
+        tls_wrap_oob_standalone(&tas->tls_wrap, tas, &client_sid, &payload, P_CONTROL_OOB_V1);
     if (!BLEN(&probe))
     {
         msg(D_LOW, "server-probe: could not wrap probe packet; using configured order");
diff --git a/src/openvpn/ssl_pkt.c b/src/openvpn/ssl_pkt.c
index 121535c..ba3dce5 100644
--- a/src/openvpn/ssl_pkt.c
+++ b/src/openvpn/ssl_pkt.c
@@ -144,7 +144,8 @@
         }
 
         if ((header >> P_OPCODE_SHIFT) == P_CONTROL_HARD_RESET_CLIENT_V3
-            || (header >> P_OPCODE_SHIFT) == P_CONTROL_WKC_V1)
+            || (header >> P_OPCODE_SHIFT) == P_CONTROL_WKC_V1
+            || (header >> P_OPCODE_SHIFT) == P_CONTROL_OOB_WKC_V1)
         {
             if (!buf_copy(&ctx->work, ctx->tls_crypt_v2_wkc))
             {
@@ -198,7 +199,8 @@
     bool ret = false;
 
     const uint8_t opcode = *(BPTR(buf)) >> P_OPCODE_SHIFT;
-    if ((opcode == P_CONTROL_HARD_RESET_CLIENT_V3 || opcode == P_CONTROL_WKC_V1)
+    if ((opcode == P_CONTROL_HARD_RESET_CLIENT_V3 || opcode == P_CONTROL_WKC_V1
+         || opcode == P_CONTROL_OOB_WKC_V1)
         && !tls_crypt_v2_extract_client_key(buf, ctx, opt))
     {
         msg(D_TLS_ERRORS, "TLS Error: can not extract tls-crypt-v2 client key from %s",
@@ -314,7 +316,7 @@
     /* Allow only the reset packet or the first packet of the actual handshake. */
     if (op != P_CONTROL_HARD_RESET_CLIENT_V2 && op != P_CONTROL_HARD_RESET_CLIENT_V3
         && op != P_CONTROL_V1 && op != P_CONTROL_WKC_V1 && op != P_ACK_V1
-        && op != P_CONTROL_OOB_V1)
+        && op != P_CONTROL_OOB_V1 && op != P_CONTROL_OOB_WKC_V1)
     {
         /*
          * This can occur due to bogus data or DoS packets.
@@ -389,8 +391,12 @@
     {
         return VERDICT_VALID_WKC_V1;
     }
-    else if (op == P_CONTROL_OOB_V1)
+    else if (op == P_CONTROL_OOB_V1 || op == P_CONTROL_OOB_WKC_V1)
     {
+        /* A tls-crypt-v2 probe (P_CONTROL_OOB_WKC_V1) has, at this point,
+         * already had its WKc unwrapped by read_control_auth() above, so the
+         * per-client key is loaded into state->tls_wrap_tmp and the rest of the
+         * handling is identical to a plain OOB probe. */
         return VERDICT_VALID_OOB_V1;
     }
     else
@@ -449,8 +455,10 @@
 
 struct buffer
 tls_wrap_oob_standalone(struct tls_wrap_ctx *ctx, struct tls_auth_standalone *tas,
-                        struct session_id *own_sid, const struct buffer *payload)
+                        struct session_id *own_sid, const struct buffer *payload, int opcode)
 {
+    ASSERT(opcode == P_CONTROL_OOB_V1 || opcode == P_CONTROL_OOB_WKC_V1);
+
     /* Copy buffer here to point at the same data but allow tls_wrap_control
      * to potentially change buf to point to another buffer without
      * modifying the buffer in tas */
@@ -461,10 +469,11 @@
      * or ACK fields. */
     ASSERT(buf_copy(&buf, payload));
 
-    uint8_t header = (uint8_t)(P_CONTROL_OOB_V1 << P_OPCODE_SHIFT);
+    uint8_t header = (uint8_t)(opcode << P_OPCODE_SHIFT);
 
     /* Add tls-auth/tls-crypt wrapping, this might replace buf with
-     * ctx->work */
+     * ctx->work. For P_CONTROL_OOB_WKC_V1 the wrapped client key is appended
+     * here too (tls-crypt-v2). */
     tls_wrap_control(ctx, header, &buf, own_sid);
 
     return buf;
diff --git a/src/openvpn/ssl_pkt.h b/src/openvpn/ssl_pkt.h
index 7e9a801..e9eec9a 100644
--- a/src/openvpn/ssl_pkt.h
+++ b/src/openvpn/ssl_pkt.h
@@ -71,6 +71,14 @@
  * as a control packet. */
 #define P_CONTROL_OOB_V1 12
 
+/* Variant of P_CONTROL_OOB_V1 with an appended wrapped client key (WKc), like
+ * P_CONTROL_HARD_RESET_CLIENT_V3 / P_CONTROL_WKC_V1. Used when tls-crypt-v2 is
+ * configured: an out-of-band message belongs to no established session, so the
+ * client must carry the WKc for the server to recover the per-client key and
+ * unwrap the message. See doc/tls-crypt-v2.txt and the openvpn-rfc wire
+ * protocol (CONTROL_OOB_WKC_V1). */
+#define P_CONTROL_OOB_WKC_V1 13
+
 /* define the range of legal opcodes
  * Since we do no longer support key-method 1 we consider
  * the v1 op codes invalid */
@@ -248,10 +256,14 @@
  * @param tas       standalone auth context providing the work buffer
  * @param own_sid   session id to use as our session id in the header
  * @param payload   the OOB message payload (TLV stream) to wrap
+ * @param opcode    the OOB opcode to use: P_CONTROL_OOB_V1, or
+ *                  P_CONTROL_OOB_WKC_V1 to append the tls-crypt-v2 wrapped
+ *                  client key (the context must then carry it).
  * @return          the wrapped packet buffer, ready to send
  */
 struct buffer tls_wrap_oob_standalone(struct tls_wrap_ctx *ctx, struct tls_auth_standalone *tas,
-                                      struct session_id *own_sid, const struct buffer *payload);
+                                      struct session_id *own_sid, const struct buffer *payload,
+                                      int opcode);
 
 
 /**
diff --git a/tests/unit_tests/openvpn/test_pkt.c b/tests/unit_tests/openvpn/test_pkt.c
index cad2ce0..e64a698 100644
--- a/tests/unit_tests/openvpn/test_pkt.c
+++ b/tests/unit_tests/openvpn/test_pkt.c
@@ -712,6 +712,125 @@
     free_tas(&tas_server);
 }
 
+/* A matching tls-crypt-v2 server/client key pair (the client key's WKc was
+ * wrapped with this server key), reused from the tls-crypt-v2 unit tests. */
+static const char *oob_v2_server_key =
+    "-----BEGIN OpenVPN tls-crypt-v2 server key-----\n"
+    "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8gISIjJCUmJygpKissLS4v\n"
+    "MDEyMzQ1Njc4OTo7PD0+P0BBQkNERUZHSElKS0xNTk9QUVJTVFVWV1hZWltcXV5f\n"
+    "YGFiY2RlZmdoaWprbG1ub3BxcnN0dXZ3eHl6e3x9fn8=\n"
+    "-----END OpenVPN tls-crypt-v2 server key-----\n";
+
+static const char *oob_v2_client_key =
+    "-----BEGIN OpenVPN tls-crypt-v2 client key-----\n"
+    "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8gISIjJCUmJygpKissLS4v\n"
+    "MDEyMzQ1Njc4OTo7PD0+P0BBQkNERUZHSElKS0xNTk9QUVJTVFVWV1hZWltcXV5f\n"
+    "YGFiY2RlZmdoaWprbG1ub3BxcnN0dXZ3eHl6e3x9fn+AgYKDhIWGh4iJiouMjY6P\n"
+    "kJGSk5SVlpeYmZqbnJ2en6ChoqOkpaanqKmqq6ytrq+wsbKztLW2t7i5uru8vb6/\n"
+    "wMHCw8TFxsfIycrLzM3Oz9DR0tPU1dbX2Nna29zd3t/g4eLj5OXm5+jp6uvs7e7v\n"
+    "8PHy8/T19vf4+fr7/P3+/xd9pcB0qUYZsWvkrLcfGmzPJPM8a7r0mEWdXwbDadSV\n"
+    "LHg5bv2TwlmPR3HgaMr8o9LTh9hxUTkrH3S0PfKRNwcso86ua/dBFTyXsM9tg4aw\n"
+    "3dS6ogH9AkaT+kRRDgNcKWkQCbwmJK2JlfkXHBwbAtmn78AkNuho6QCFqCdqGab3\n"
+    "zh2vheFqGMPdGpukbFrT3rcO3VLxUeG+RdzXiMTCpJSovFBP1lDkYwYJPnz6daEh\n"
+    "j0TzJ3BVru9W3CpotdNt7u09knxAfpCxjtrP3semsDew/gTBtcfQ/OoTFyFHnN5k\n"
+    "RZ+q17SC4nba3Pp8/Fs0+hSbv2tJozoD8SElFq7SIWJsciTYh8q8f5yQxjdt4Wxu\n"
+    "/Z5wtPCAZ0tOzj4ItTI77fBOYRTfEayzHgEr\n"
+    "-----END OpenVPN tls-crypt-v2 client key-----\n";
+
+/* End-to-end check of the tls-crypt-v2 out-of-band probe: the client wraps an
+ * OOB payload as a P_CONTROL_OOB_WKC_V1 message (the payload encrypted with the
+ * per-client key Kc, with the wrapped client key WKc appended), and the server's
+ * stateless first-packet path (tls_pre_decrypt_lite) recovers Kc from the WKc,
+ * decrypts the payload, and returns VERDICT_VALID_OOB_V1. */
+static void
+test_oob_wkc_v1_roundtrip(void **ut_state)
+{
+    now = 0x5f000000;
+
+    struct frame frame = { .buf = { .headroom = 200, .payload_size = 1400 }, 0 };
+
+    /* ---- Client: wrap an OOB probe as P_CONTROL_OOB_WKC_V1 (WKc appended) ---- */
+    struct tls_auth_standalone tas_client = { 0 };
+    tas_client.frame = frame;
+    tas_client.tls_wrap.mode = TLS_WRAP_CRYPT;
+    tas_client.tls_wrap.opt.flags |= (CO_IGNORE_PACKET_ID | CO_PACKET_ID_LONG_FORM);
+    packet_id_init(&tas_client.tls_wrap.opt.packet_id, 0, 0, "UT-CLIENT", 0);
+    tas_client.workbuf = alloc_buf(1600);
+    tas_client.tls_wrap.work = alloc_buf(1600);
+
+    /* Load Kc and obtain the WKc, exactly as a tls-crypt-v2 client does. */
+    struct buffer wkc = { 0 };
+    tls_crypt_v2_init_client_key(&tas_client.tls_wrap.opt.key_ctx_bi,
+                                 &tas_client.tls_wrap.original_wrap_keydata, &wkc, oob_v2_client_key,
+                                 true);
+    tas_client.tls_wrap.tls_crypt_v2_wkc = &wkc;
+
+    const uint8_t payload_bytes[] = { 0x01, 0x00, 0x02, 0x00, 0x0c, 0xde,
+                                      0xad, 0xbe, 0xef, 0x00, 0x11, 0x22 };
+    struct buffer payload = alloc_buf(64);
+    buf_write(&payload, payload_bytes, sizeof(payload_bytes));
+
+    struct session_id client_sid = { { 1, 2, 3, 4, 5, 6, 7, 8 } };
+    reset_packet_id_send(&tas_client.tls_wrap.opt.packet_id.send);
+
+    struct buffer probe = tls_wrap_oob_standalone(&tas_client.tls_wrap, &tas_client, &client_sid,
+                                                  &payload, P_CONTROL_OOB_WKC_V1);
+    assert_true(BLEN(&probe) > 0);
+    assert_int_equal((BPTR(&probe))[0] >> P_OPCODE_SHIFT, P_CONTROL_OOB_WKC_V1);
+
+    /* ---- Server: stateless path recovers Kc from the WKc and decrypts ---- */
+    struct tls_auth_standalone tas_server = { 0 };
+    tas_server.frame = frame;
+    /* No static wrapping, only a tls-crypt-v2 server key (as a v2 server runs). */
+    tas_server.tls_wrap.mode = TLS_WRAP_NONE;
+    tas_server.tls_wrap.opt.flags |= (CO_IGNORE_PACKET_ID | CO_PACKET_ID_LONG_FORM);
+    packet_id_init(&tas_server.tls_wrap.opt.packet_id, 0, 0, "UT-SERVER", 0);
+    tls_crypt_v2_init_server_key(&tas_server.tls_wrap.tls_crypt_v2_server_key, true, oob_v2_server_key,
+                                 true);
+    tas_server.workbuf = alloc_buf(1600);
+
+    struct link_socket_actual from = { 0 };
+    struct buffer srv = alloc_buf(1600);
+    buf_write(&srv, BPTR(&probe), BLEN(&probe));
+
+    struct tls_pre_decrypt_state state = { 0 };
+    enum first_packet_verdict verdict = tls_pre_decrypt_lite(&tas_server, &state, &from, &srv);
+    assert_int_equal(verdict, VERDICT_VALID_OOB_V1);
+
+    /* The decrypted payload must equal what the client sent, and the client's
+     * session id must be recovered. */
+    assert_int_equal(BLEN(&state.newbuf), (int)sizeof(payload_bytes));
+    assert_memory_equal(BPTR(&state.newbuf), payload_bytes, sizeof(payload_bytes));
+    assert_memory_equal(state.peer_session_id.id, client_sid.id, SID_SIZE);
+    free_tls_pre_decrypt_state(&state);
+
+    /* Flipping any single byte (header, session id, ciphertext or WKc) must make
+     * the probe fail to validate. */
+    for (size_t i = 0; i < (size_t)BLEN(&probe); i++)
+    {
+        struct tls_pre_decrypt_state tstate = { 0 };
+        buf_reset_len(&srv);
+        buf_write(&srv, BPTR(&probe), BLEN(&probe));
+        (BPTR(&srv))[i] ^= 0xff;
+        enum first_packet_verdict v = tls_pre_decrypt_lite(&tas_server, &tstate, &from, &srv);
+        assert_int_equal(v, VERDICT_INVALID);
+        free_tls_pre_decrypt_state(&tstate);
+    }
+
+    free_key_ctx_bi(&tas_client.tls_wrap.opt.key_ctx_bi);
+    packet_id_free(&tas_client.tls_wrap.opt.packet_id);
+    free_buf(&wkc);
+    free_buf(&payload);
+    free_buf(&tas_client.workbuf);
+    free_buf(&tas_client.tls_wrap.work);
+
+    free_key_ctx(&tas_server.tls_wrap.tls_crypt_v2_server_key);
+    packet_id_free(&tas_server.tls_wrap.opt.packet_id);
+    free_buf(&tas_server.workbuf);
+
+    free_buf(&srv);
+}
+
 static void
 test_extract_control_message(void **ut_state)
 {
@@ -762,6 +881,7 @@
         cmocka_unit_test(test_verify_hmac_none_out_of_range_ack),
         cmocka_unit_test(test_generate_reset_packet_plain),
         cmocka_unit_test(test_generate_reset_packet_tls_auth),
+        cmocka_unit_test(test_oob_wkc_v1_roundtrip),
         cmocka_unit_test(test_extract_control_message)
     };
 

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