[M] Change in openvpn[master]: oob: Answer SERVER_PROBE on the server (P_CONTROL_OOB_V1)
"stipa \(Code Review\) via Openvpn-devel" <[email protected]>
| Newsgroups | gmane.network.openvpn.devel |
|---|---|
| Message-ID | <[email protected]> |
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/+/1744?usp=email
to review the following change.
Change subject: oob: Answer SERVER_PROBE on the server (P_CONTROL_OOB_V1)
......................................................................
oob: Answer SERVER_PROBE on the server (P_CONTROL_OOB_V1)
Make a --mode server UDP listener answer an out-of-band SERVER_PROBE without
creating a session:
- tls_pre_decrypt_lite() accepts P_CONTROL_OOB_V1 and returns the new
VERDICT_VALID_OOB_V1 verdict.
- tls_wrap_oob_standalone() builds a session-less P_CONTROL_OOB_V1 packet:
opcode + session id + the usual tls-auth/tls-crypt wrapping around a bare
TLV payload (no reliability/ACK fields), mirroring tls_reset_standalone().
- do_pre_decrypt_check() handles the new verdict: it asks
oob_build_probe_reply() whether to answer, and if so sends a PROBE_REPLY
via send_probe_reply() (synchronous, stateless, like the HMAC reset path)
and returns false so no session is created.
The reply's own session id is the stateless SYN-cookie used by the three-way
handshake, so the responder keeps no state and the reply can later serve as
the handshake reset (connect_lifetime shortcut).
P_LAST_OPCODE is intentionally not widened: the server path uses the
tls_pre_decrypt_lite() allowlist and tls_wrap_control() directly, so opcode 12
need not pass the established-session opcode gate yet. That gate (and the
matching tls_pre_decrypt() handler) is widened together with the client
receive path.
oob.c is now wired into the openvpn build as it has a real caller.
Change-Id: I930d3789e0313aa0c3bc51ee5fd1d108343d59f0
Signed-off-by: Lev Stipakov <[email protected]>
---
M CMakeLists.txt
M src/openvpn/Makefile.am
M src/openvpn/mudp.c
M src/openvpn/oob.c
M src/openvpn/oob.h
M src/openvpn/ssl_pkt.c
M src/openvpn/ssl_pkt.h
7 files changed, 131 insertions(+), 6 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/44/1744/1
diff --git a/CMakeLists.txt b/CMakeLists.txt
index d2b88c0..608e39e4 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -518,6 +518,8 @@
src/openvpn/multi_io.c
src/openvpn/occ.c
src/openvpn/occ.h
+ src/openvpn/oob.c
+ src/openvpn/oob.h
src/openvpn/openvpn.c
src/openvpn/openvpn.h
src/openvpn/openvpn_win32_resources.rc
diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
index ff8cc54..b4b1b92 100644
--- a/src/openvpn/Makefile.am
+++ b/src/openvpn/Makefile.am
@@ -106,6 +106,7 @@
pkcs11.c pkcs11.h pkcs11_backend.h \
pkcs11_openssl.c \
pkcs11_mbedtls.c \
+ oob.c oob.h \
openvpn.c openvpn.h \
options.c options.h \
options_util.c options_util.h \
diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c
index d3b529f..9bc2d9c 100644
--- a/src/openvpn/mudp.c
+++ b/src/openvpn/mudp.c
@@ -32,6 +32,7 @@
#include "memdbg.h"
#include "ssl_pkt.h"
+#include "oob.h"
#ifdef HAVE_SYS_INOTIFY_H
#include <sys/inotify.h>
@@ -84,6 +85,34 @@
"Reset packet from client, sending HMAC based reset challenge", sock);
}
+/* Send an out-of-band PROBE_REPLY back to the source of a SERVER_PROBE,
+ * synchronously and without keeping any state, mirroring the reset path. */
+static void
+send_probe_reply(struct multi_context *m, struct tls_pre_decrypt_state *state,
+ struct tls_auth_standalone *tas, const struct oob_probe_reply *reply,
+ struct session_id *own_sid, struct link_socket *sock)
+{
+ struct gc_arena gc = gc_new();
+
+ /* Build the payload of the reply (message-type header + probe_reply TLV) */
+ struct buffer payload = alloc_buf_gc(128, &gc);
+ if (!oob_client_reply_write(&payload, reply))
+ {
+ gc_free(&gc);
+ return;
+ }
+
+ /* OOB replies use the same control-channel wrapping as the request */
+ 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);
+ send_standalone_reply(m, &buf, "Server Probe", "Server probe from client, sending probe reply",
+ sock);
+
+ gc_free(&gc);
+}
+
/* Returns true if this packet should create a new session */
static bool
@@ -200,6 +229,39 @@
return ret;
}
+ else if (verdict == VERDICT_VALID_OOB_V1)
+ {
+ /* 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;
+ if (!oob_build_probe_reply(&state->newbuf, (uint64_t)now, (uint64_t)handwindow,
+ &state->peer_session_id, &reply))
+ {
+ /* malformed or replayed/stale probe: silently drop */
+ return false;
+ }
+
+ /* Rate-limit replies, as the reset path does, so an unauthenticated
+ * probe flood (no tls-auth/tls-crypt) cannot use us as a reflector. */
+ if (!reflect_filter_rate_limit_check(m->initial_rate_limiter))
+ {
+ return false;
+ }
+
+ /* Our session id is a stateless SYN cookie (the same HMAC the three-way
+ * handshake uses): we keep no per-probe state, and the reply can later
+ * also serve as the server's CONTROL_HARD_RESET_SERVER_V2, letting a
+ * client skip the first two handshake messages (the connect_lifetime
+ * shortcut). */
+ struct session_id sid =
+ calculate_session_id_hmac(state->peer_session_id, from, hmac, handwindow, 0);
+
+ send_probe_reply(m, state, tas, &reply, &sid, sock);
+
+ /* An OOB probe never creates a session */
+ return false;
+ }
/* VERDICT_INVALID */
return false;
diff --git a/src/openvpn/oob.c b/src/openvpn/oob.c
index 58d47c0..f7af0ef 100644
--- a/src/openvpn/oob.c
+++ b/src/openvpn/oob.c
@@ -193,6 +193,12 @@
}
bool
+oob_client_reply_write(struct buffer *buf, const struct oob_probe_reply *reply)
+{
+ return oob_msg_write_header(buf, OOB_MSG_PROBE_REPLY) && oob_probe_reply_write(buf, reply);
+}
+
+bool
oob_timestamp_in_window(uint64_t probe_ts, uint64_t now, uint64_t window_secs)
{
uint64_t diff = (now > probe_ts) ? (now - probe_ts) : (probe_ts - now);
diff --git a/src/openvpn/oob.h b/src/openvpn/oob.h
index 48fcfa1..93c44b0 100644
--- a/src/openvpn/oob.h
+++ b/src/openvpn/oob.h
@@ -164,6 +164,12 @@
bool oob_server_probe_read(struct buffer *payload, struct oob_probe_parameter *param);
/**
+ * Write a complete PROBE_REPLY message (message-type header + probe_reply TLV)
+ * to @p buf. Sent by the server.
+ */
+bool oob_client_reply_write(struct buffer *buf, const struct oob_probe_reply *reply);
+
+/**
* Check whether a probe timestamp is within an acceptable window around the
* current time. Used to cheaply drop replayed or implausibly-timed probes
* before doing any further work (see the probe_parameter timestamp rationale
diff --git a/src/openvpn/ssl_pkt.c b/src/openvpn/ssl_pkt.c
index 1805995..86e42c1 100644
--- a/src/openvpn/ssl_pkt.c
+++ b/src/openvpn/ssl_pkt.c
@@ -315,7 +315,8 @@
/* 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_V1 && op != P_CONTROL_WKC_V1 && op != P_ACK_V1
+ && op != P_CONTROL_OOB_V1)
{
/*
* This can occur due to bogus data or DoS packets.
@@ -390,6 +391,10 @@
{
return VERDICT_VALID_WKC_V1;
}
+ else if (op == P_CONTROL_OOB_V1)
+ {
+ return VERDICT_VALID_OOB_V1;
+ }
else
{
return VERDICT_VALID_RESET_V2;
@@ -444,6 +449,29 @@
return buf;
}
+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)
+{
+ /* 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 */
+ struct buffer buf = tas->workbuf;
+ ASSERT(buf_init(&buf, tas->frame.buf.headroom));
+
+ /* Out-of-band messages carry the payload directly, with no reliability
+ * or ACK fields. */
+ ASSERT(buf_copy(&buf, payload));
+
+ uint8_t header = (uint8_t)(P_CONTROL_OOB_V1 << P_OPCODE_SHIFT);
+
+ /* Add tls-auth/tls-crypt wrapping, this might replace buf with
+ * ctx->work */
+ tls_wrap_control(ctx, header, &buf, own_sid);
+
+ return buf;
+}
+
hmac_ctx_t *
session_id_hmac_init(void)
{
diff --git a/src/openvpn/ssl_pkt.h b/src/openvpn/ssl_pkt.h
index 0dab524..6f05a19 100644
--- a/src/openvpn/ssl_pkt.h
+++ b/src/openvpn/ssl_pkt.h
@@ -62,11 +62,13 @@
* control channel session (e.g. a server probe). Inherently unreliable:
* there is no protocol-level retransmission.
*
- * NOTE: intentionally not yet included in the P_FIRST_OPCODE..P_LAST_OPCODE
- * legal range. The range acts as an acceptance gate in tls_pre_decrypt();
- * widening it before an OOB receive handler exists would let OOB packets
- * be misparsed as established-session control packets. Bump P_LAST_OPCODE
- * to 12 in the same change that adds the handler. */
+ * On the server this is accepted via the tls_pre_decrypt_lite() allowlist and
+ * handled in the new-connection path (it never creates a session). It is
+ * deliberately still excluded from the P_FIRST_OPCODE..P_LAST_OPCODE range
+ * that gates the established-session tls_pre_decrypt(): that range is widened
+ * to 12 only together with the established-session/client OOB receive handler,
+ * otherwise an OOB packet arriving on an existing session would be misparsed
+ * as a control packet. */
#define P_CONTROL_OOB_V1 12
/* define the range of legal opcodes
@@ -105,6 +107,9 @@
VERDICT_VALID_ACK_V1,
/** The packet is a valid control packet with appended wrapped client key */
VERDICT_VALID_WKC_V1,
+ /** This packet is a valid out-of-band control message (e.g. a server
+ * probe). It does not belong to a session and must not create one. */
+ VERDICT_VALID_OOB_V1,
/** the packet failed on of the various checks */
VERDICT_INVALID
};
@@ -235,6 +240,21 @@
struct session_id *own_sid, struct session_id *remote_sid,
uint8_t header, bool request_resend_wkc);
+/**
+ * Wrap an already-built out-of-band payload (e.g. probe-reply TLVs) into a
+ * standalone, session-less P_CONTROL_OOB_V1 packet: it prepends the opcode and
+ * @p own_sid and applies the same tls-auth/tls-crypt wrapping as a regular
+ * control packet, but carries no reliability/ACK fields.
+ *
+ * @param ctx tls wrapping context (from the pre-decrypt state)
+ * @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
+ * @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);
+
/**
* Extracts a control channel message from buf and adjusts the size of
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1744?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: I930d3789e0313aa0c3bc51ee5fd1d108343d59f0
Gerrit-Change-Number: 1744
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