283 lines
12 KiB
Diff
283 lines
12 KiB
Diff
From d3015bfd65348db629dab51e20a9d4e2f3b23493 Mon Sep 17 00:00:00 2001
|
|
From: Arne Schwabe <arne@rfc2549.org>
|
|
Date: Tue, 1 Apr 2025 19:30:37 +0200
|
|
Subject: [PATCH] Allow tls-crypt-v2 to be setup only on initial packet of a
|
|
session
|
|
|
|
This fixes an internal server error condition that can be triggered by a
|
|
malicous authenticated client, a very unlucky corruption of packets in
|
|
transit or by an attacker that is able to inject a specially created
|
|
packet at the right time and is able to observe the traffic to construct
|
|
the packet.
|
|
|
|
The error condition results in an ASSERT statement being triggered,
|
|
|
|
NOTE: due to the security sensitive nature, this patch was prepared
|
|
under embargo on the security@openvpn.net mailing list, and thus has
|
|
no publically available "mailing list discussion before merge" URL.
|
|
|
|
CVE: 2025-2704
|
|
Change-Id: I07c1352204d308e5bde5f0b85e561a5dd0bc63c8
|
|
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
|
|
Acked-by: Gert Doering <gert@greenie.muc.de>
|
|
Message-Id: <385d88f0-d7c9-4330-82ff-9f5931183afd@rfc2549.org>
|
|
Signed-off-by: Gert Doering <gert@greenie.muc.de>
|
|
(cherry picked from commit 82ee2fe4b42d9988c59ae3f83bd56a54d54e8c76)
|
|
---
|
|
src/openvpn/ssl.c | 26 +++++++++++++++++++----
|
|
src/openvpn/ssl_common.h | 15 +++++++------
|
|
src/openvpn/ssl_pkt.c | 7 +++---
|
|
src/openvpn/ssl_pkt.h | 12 +++++++++--
|
|
src/openvpn/tls_crypt.c | 24 ++++++++++++++++++++-
|
|
src/openvpn/tls_crypt.h | 7 +++++-
|
|
tests/unit_tests/openvpn/test_tls_crypt.c | 2 +-
|
|
7 files changed, 75 insertions(+), 18 deletions(-)
|
|
|
|
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
|
|
index 4fa7ea6fc45..5a0bf95aace 100644
|
|
--- a/src/openvpn/ssl.c
|
|
+++ b/src/openvpn/ssl.c
|
|
@@ -848,6 +848,9 @@ state_name(int state)
|
|
case S_INITIAL:
|
|
return "S_INITIAL";
|
|
|
|
+ case S_PRE_START_SKIP:
|
|
+ return "S_PRE_START_SKIP";
|
|
+
|
|
case S_PRE_START:
|
|
return "S_PRE_START";
|
|
|
|
@@ -2598,7 +2601,7 @@ session_move_pre_start(const struct tls_session *session,
|
|
}
|
|
INCR_GENERATED;
|
|
|
|
- ks->state = S_PRE_START;
|
|
+ ks->state = skip_initial_send ? S_PRE_START_SKIP : S_PRE_START;
|
|
|
|
struct gc_arena gc = gc_new();
|
|
dmsg(D_TLS_DEBUG, "TLS: Initial Handshake, sid=%s",
|
|
@@ -3801,7 +3804,7 @@ tls_pre_decrypt(struct tls_multi *multi,
|
|
}
|
|
|
|
if (!read_control_auth(buf, tls_session_get_tls_wrap(session, key_id), from,
|
|
- session->opt))
|
|
+ session->opt, true))
|
|
{
|
|
goto error;
|
|
}
|
|
@@ -3871,7 +3874,7 @@ tls_pre_decrypt(struct tls_multi *multi,
|
|
if (op == P_CONTROL_SOFT_RESET_V1 && ks->state >= S_GENERATED_KEYS)
|
|
{
|
|
if (!read_control_auth(buf, tls_session_get_tls_wrap(session, key_id),
|
|
- from, session->opt))
|
|
+ from, session->opt, false))
|
|
{
|
|
goto error;
|
|
}
|
|
@@ -3884,6 +3887,15 @@ tls_pre_decrypt(struct tls_multi *multi,
|
|
}
|
|
else
|
|
{
|
|
+ bool initial_packet = false;
|
|
+ if (ks->state == S_PRE_START_SKIP)
|
|
+ {
|
|
+ /* When we are coming from the session_skip_to_pre_start
|
|
+ * method, we allow this initial packet to setup the
|
|
+ * tls-crypt-v2 peer specific key */
|
|
+ initial_packet = true;
|
|
+ ks->state = S_PRE_START;
|
|
+ }
|
|
/*
|
|
* Remote responding to our key renegotiation request?
|
|
*/
|
|
@@ -3893,8 +3905,14 @@ tls_pre_decrypt(struct tls_multi *multi,
|
|
}
|
|
|
|
if (!read_control_auth(buf, tls_session_get_tls_wrap(session, key_id),
|
|
- from, session->opt))
|
|
+ from, session->opt, initial_packet))
|
|
{
|
|
+ /* if an initial packet in read_control_auth, we rather
|
|
+ * error out than anything else */
|
|
+ if (initial_packet)
|
|
+ {
|
|
+ multi->n_hard_errors++;
|
|
+ }
|
|
goto error;
|
|
}
|
|
|
|
diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
|
|
index 085256347ec..8b6ae3f2430 100644
|
|
--- a/src/openvpn/ssl_common.h
|
|
+++ b/src/openvpn/ssl_common.h
|
|
@@ -80,22 +80,25 @@
|
|
#define S_INITIAL 1 /**< Initial \c key_state state after
|
|
* initialization by \c key_state_init()
|
|
* before start of three-way handshake. */
|
|
-#define S_PRE_START 2 /**< Waiting for the remote OpenVPN peer
|
|
+#define S_PRE_START_SKIP 2 /**< Waiting for the remote OpenVPN peer
|
|
* to acknowledge during the initial
|
|
* three-way handshake. */
|
|
-#define S_START 3 /**< Three-way handshake is complete,
|
|
+#define S_PRE_START 3 /**< Waiting for the remote OpenVPN peer
|
|
+ * to acknowledge during the initial
|
|
+ * three-way handshake. */
|
|
+#define S_START 4 /**< Three-way handshake is complete,
|
|
* start of key exchange. */
|
|
-#define S_SENT_KEY 4 /**< Local OpenVPN process has sent its
|
|
+#define S_SENT_KEY 5 /**< Local OpenVPN process has sent its
|
|
* part of the key material. */
|
|
-#define S_GOT_KEY 5 /**< Local OpenVPN process has received
|
|
+#define S_GOT_KEY 6 /**< Local OpenVPN process has received
|
|
* the remote's part of the key
|
|
* material. */
|
|
-#define S_ACTIVE 6 /**< Operational \c key_state state
|
|
+#define S_ACTIVE 7 /**< Operational \c key_state state
|
|
* immediately after negotiation has
|
|
* completed while still within the
|
|
* handshake window. Deferred auth and
|
|
* client connect can still be pending. */
|
|
-#define S_GENERATED_KEYS 7 /**< The data channel keys have been generated
|
|
+#define S_GENERATED_KEYS 8 /**< The data channel keys have been generated
|
|
* The TLS session is fully authenticated
|
|
* when reaching this state. */
|
|
|
|
diff --git a/src/openvpn/ssl_pkt.c b/src/openvpn/ssl_pkt.c
|
|
index 689cd7f99f9..41299f462db 100644
|
|
--- a/src/openvpn/ssl_pkt.c
|
|
+++ b/src/openvpn/ssl_pkt.c
|
|
@@ -200,7 +200,8 @@ bool
|
|
read_control_auth(struct buffer *buf,
|
|
struct tls_wrap_ctx *ctx,
|
|
const struct link_socket_actual *from,
|
|
- const struct tls_options *opt)
|
|
+ const struct tls_options *opt,
|
|
+ bool initial_packet)
|
|
{
|
|
struct gc_arena gc = gc_new();
|
|
bool ret = false;
|
|
@@ -208,7 +209,7 @@ read_control_auth(struct buffer *buf,
|
|
const uint8_t opcode = *(BPTR(buf)) >> P_OPCODE_SHIFT;
|
|
if ((opcode == P_CONTROL_HARD_RESET_CLIENT_V3
|
|
|| opcode == P_CONTROL_WKC_V1)
|
|
- && !tls_crypt_v2_extract_client_key(buf, ctx, opt))
|
|
+ && !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",
|
|
@@ -373,7 +374,7 @@ tls_pre_decrypt_lite(const struct tls_auth_standalone *tas,
|
|
* into newbuf or just setting newbuf to point to the start of control
|
|
* message */
|
|
bool status = read_control_auth(&state->newbuf, &state->tls_wrap_tmp,
|
|
- from, NULL);
|
|
+ from, NULL, true);
|
|
|
|
if (!status)
|
|
{
|
|
diff --git a/src/openvpn/ssl_pkt.h b/src/openvpn/ssl_pkt.h
|
|
index c8a27fba9d7..2033da61ff7 100644
|
|
--- a/src/openvpn/ssl_pkt.h
|
|
+++ b/src/openvpn/ssl_pkt.h
|
|
@@ -207,14 +207,22 @@ write_control_auth(struct tls_session *session,
|
|
bool prepend_ack);
|
|
|
|
|
|
-/*
|
|
+
|
|
+/**
|
|
* Read a control channel authentication record.
|
|
+ * @param buf buffer that holds the incoming packet
|
|
+ * @param ctx control channel security context
|
|
+ * @param from incoming link socket address
|
|
+ * @param opt tls options struct for the session
|
|
+ * @param initial_packet whether this is the initial packet for the connection
|
|
+ * @return if the packet was successfully processed
|
|
*/
|
|
bool
|
|
read_control_auth(struct buffer *buf,
|
|
struct tls_wrap_ctx *ctx,
|
|
const struct link_socket_actual *from,
|
|
- const struct tls_options *opt);
|
|
+ const struct tls_options *opt,
|
|
+ bool initial_packet);
|
|
|
|
|
|
/**
|
|
diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c
|
|
index 975d31fafb5..50228e786e0 100644
|
|
--- a/src/openvpn/tls_crypt.c
|
|
+++ b/src/openvpn/tls_crypt.c
|
|
@@ -612,7 +612,8 @@ tls_crypt_v2_verify_metadata(const struct tls_wrap_ctx *ctx,
|
|
bool
|
|
tls_crypt_v2_extract_client_key(struct buffer *buf,
|
|
struct tls_wrap_ctx *ctx,
|
|
- const struct tls_options *opt)
|
|
+ const struct tls_options *opt,
|
|
+ bool initial_packet)
|
|
{
|
|
if (!ctx->tls_crypt_v2_server_key.cipher)
|
|
{
|
|
@@ -641,6 +642,27 @@ tls_crypt_v2_extract_client_key(struct buffer *buf,
|
|
return false;
|
|
}
|
|
|
|
+ if (!initial_packet)
|
|
+ {
|
|
+ /* This might be a harmless resend of the packet but it is better to
|
|
+ * just ignore the WKC part than trying to setup tls-crypt keys again.
|
|
+ *
|
|
+ * A CONTROL_WKC_V1 packets has a normal packet part and an appended
|
|
+ * wrapped control key. These are authenticated individually. We already
|
|
+ * set up tls-crypt with the wrapped key, so we are ignoring this part
|
|
+ * of the message but we return the normal packet part as the normal
|
|
+ * part of the message might have been corrupted earlier and discarded
|
|
+ * and this is resend. So return the normal part of the packet,
|
|
+ * basically transforming the CONTROL_WKC_V1 into a normal CONTROL_V1
|
|
+ * packet*/
|
|
+ msg(D_TLS_ERRORS, "control channel security already setup ignoring "
|
|
+ "wrapped key part of packet.");
|
|
+
|
|
+ /* Remove client key from buffer so tls-crypt code can unwrap message */
|
|
+ ASSERT(buf_inc_len(buf, -(BLEN(&wrapped_client_key))));
|
|
+ return true;
|
|
+ }
|
|
+
|
|
ctx->tls_crypt_v2_metadata = alloc_buf(TLS_CRYPT_V2_MAX_METADATA_LEN);
|
|
if (!tls_crypt_v2_unwrap_client_key(&ctx->original_wrap_keydata,
|
|
&ctx->tls_crypt_v2_metadata,
|
|
diff --git a/src/openvpn/tls_crypt.h b/src/openvpn/tls_crypt.h
|
|
index 8c87e2080a1..331c0c060a7 100644
|
|
--- a/src/openvpn/tls_crypt.h
|
|
+++ b/src/openvpn/tls_crypt.h
|
|
@@ -207,11 +207,16 @@ void tls_crypt_v2_init_client_key(struct key_ctx_bi *key,
|
|
* message.
|
|
* @param ctx tls-wrap context to be initialized with the client key.
|
|
*
|
|
+ * @param initial_packet whether this is the initial packet of the
|
|
+ * connection. Only in these scenarios unwrapping
|
|
+ * of a tls-crypt-v2 key is allowed
|
|
+ *
|
|
* @returns true if a key was successfully extracted.
|
|
*/
|
|
bool tls_crypt_v2_extract_client_key(struct buffer *buf,
|
|
struct tls_wrap_ctx *ctx,
|
|
- const struct tls_options *opt);
|
|
+ const struct tls_options *opt,
|
|
+ bool initial_packet);
|
|
|
|
/**
|
|
* Generate a tls-crypt-v2 server key, and write to file.
|
|
diff --git a/tests/unit_tests/openvpn/test_tls_crypt.c b/tests/unit_tests/openvpn/test_tls_crypt.c
|
|
index 465543a740e..3eac04cf9ce 100644
|
|
--- a/tests/unit_tests/openvpn/test_tls_crypt.c
|
|
+++ b/tests/unit_tests/openvpn/test_tls_crypt.c
|
|
@@ -533,7 +533,7 @@ tls_crypt_v2_wrap_unwrap_max_metadata(void **state)
|
|
.mode = TLS_WRAP_CRYPT,
|
|
.tls_crypt_v2_server_key = ctx->server_keys.encrypt,
|
|
};
|
|
- assert_true(tls_crypt_v2_extract_client_key(&ctx->wkc, &wrap_ctx, NULL));
|
|
+ assert_true(tls_crypt_v2_extract_client_key(&ctx->wkc, &wrap_ctx, NULL, true));
|
|
tls_wrap_free(&wrap_ctx);
|
|
}
|
|
|