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/+/1758?usp=email
to review the following change.
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/1
diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c
index 0e9414d..7ce7ecc 100644
--- a/src/openvpn/mudp.c
+++ b/src/openvpn/mudp.c
@@ -106,7 +106,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 7391d1e..2b1b732 100644
--- a/src/openvpn/oob_client.c
+++ b/src/openvpn/oob_client.c
@@ -467,7 +467,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 86e42c1..b68c67a 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))
{
@@ -199,7 +200,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, initial_packet))
{
msg(D_TLS_ERRORS, "TLS Error: can not extract tls-crypt-v2 client key
from %s",
@@ -316,7 +318,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.
@@ -391,8 +393,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
@@ -451,8 +457,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 */
@@ -463,10 +471,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 6f05a19..4efebac 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 */
@@ -250,10 +258,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: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Idb8c0c660a98de8ec549b69f6692a7b41b092eaf
Gerrit-Change-Number: 1758
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
