[L] Change in openvpn[master]: oob: Add control message TLV encoding (P_CONTROL_OOB_V1)
"stipa \(Code Review\) via Openvpn-devel" <[email protected]> Tue, 28 Jul 2026 15:03:23 +0000
| Newsgroups | gmane.network.openvpn.devel |
|---|---|
| Message-ID | <d8237ce85e734f5c60426e411f92b3ceba65ef3b-EmailReplacePatchSet-HTML@gerrit.openvpn.net> |
Attention is currently required from: flichtenheld, stipa.
Hello flichtenheld, plaisthos,
I'd like you to reexamine a change. Please visit
http://gerrit.openvpn.net/c/openvpn/+/1741?usp=email
to look at the new patch set (#9).
The following approvals got outdated and were removed:
Code-Review+2 by flichtenheld
Change subject: oob: Add control message TLV encoding (P_CONTROL_OOB_V1)
......................................................................
oob: Add control message TLV encoding (P_CONTROL_OOB_V1)
Add opcode 12 for out-of-band control messages and the TLV codec for the
SERVER_PROBE and PROBE_REPLY used by server latency checks.
The framing is not OOB-specific, so it goes into its own control_msg.c/h;
oob.c keeps the OOB message and TLV types.
P_LAST_OPCODE becomes 12, and opcode_valid_in_session() replaces the plain
range check because OOB opcodes are answered statelessly and are never legal
on an established session.
Change-Id: I1c8d302ac57c5603d622a7be14be369437388268
Signed-off-by: Lev Stipakov <[email protected]>
---
M CMakeLists.txt
M src/openvpn/Makefile.am
A src/openvpn/control_msg.c
A src/openvpn/control_msg.h
A src/openvpn/oob.c
A src/openvpn/oob.h
M src/openvpn/ssl.c
M src/openvpn/ssl_pkt.c
M src/openvpn/ssl_pkt.h
M tests/unit_tests/openvpn/Makefile.am
A tests/unit_tests/openvpn/test_oob.c
11 files changed, 739 insertions(+), 4 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/41/1741/9
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 7473f15..f4b1b22 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -444,6 +444,8 @@
src/openvpn/console.c
src/openvpn/console_builtin.c
src/openvpn/console.h
+ src/openvpn/control_msg.c
+ src/openvpn/control_msg.h
src/openvpn/crypto.c
src/openvpn/crypto.h
src/openvpn/crypto_backend.h
@@ -520,6 +522,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
@@ -667,6 +671,7 @@
"test_mbuf"
"test_misc"
"test_ncp"
+ "test_oob"
"test_options_parse"
"test_packet_id"
"test_pkt"
@@ -858,6 +863,13 @@
src/openvpn/session_id.c
)
+ target_sources(test_oob PRIVATE
+ tests/unit_tests/openvpn/mock_get_random.c
+ src/openvpn/control_msg.c
+ src/openvpn/oob.c
+ src/openvpn/session_id.c
+ )
+
target_sources(test_pkt PRIVATE
tests/unit_tests/openvpn/mock_win32_execve.c
src/openvpn/argv.c
diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
index ff8cc54..a67d478 100644
--- a/src/openvpn/Makefile.am
+++ b/src/openvpn/Makefile.am
@@ -52,6 +52,7 @@
common.h \
comp.c comp.h compstub.c \
comp-lz4.c comp-lz4.h \
+ control_msg.c control_msg.h \
crypto.c crypto.h crypto_backend.h \
crypto_openssl.c crypto_openssl.h \
crypto_mbedtls_legacy.c crypto_mbedtls_legacy.h \
@@ -106,6 +107,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/control_msg.c b/src/openvpn/control_msg.c
new file mode 100644
index 0000000..95ff210
--- /dev/null
+++ b/src/openvpn/control_msg.c
@@ -0,0 +1,90 @@
+/*
+ * OpenVPN -- An application to securely tunnel IP networks
+ * over a single TCP/UDP port, with support for SSL/TLS-based
+ * session authentication and key exchange,
+ * packet encryption, packet authentication, and
+ * packet compression.
+ *
+ * Copyright (C) 2002-2026 OpenVPN Inc <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <https://www.gnu.org/licenses/>.
+ */
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include "syshead.h"
+
+#include "control_msg.h"
+
+bool
+ctrl_msg_read_header(struct buffer *buf, uint16_t expected_msg_type)
+{
+ int msg_type = buf_read_u16(buf);
+ return msg_type >= 0 && (uint16_t)msg_type == expected_msg_type;
+}
+
+bool
+ctrl_msg_tlv_write_header(struct buffer *buf, uint16_t type, bool optional, uint16_t value_len)
+{
+ uint16_t field = type & CTRL_MSG_TLV_TYPE_MASK;
+ if (optional)
+ {
+ field |= CTRL_MSG_TLV_OPTIONAL_FLAG;
+ }
+ return buf_write_u16(buf, field) && buf_write_u16(buf, value_len);
+}
+
+bool
+ctrl_msg_tlv_read_header(struct buffer *buf, struct ctrl_msg_tlv_header *hdr)
+{
+ int field = buf_read_u16(buf);
+ if (field < 0)
+ {
+ return false;
+ }
+ int len = buf_read_u16(buf);
+ if (len < 0)
+ {
+ return false;
+ }
+ hdr->type = (uint16_t)(field & CTRL_MSG_TLV_TYPE_MASK);
+ hdr->optional = (field & CTRL_MSG_TLV_OPTIONAL_FLAG) != 0;
+ hdr->value_len = (uint16_t)len;
+ return true;
+}
+
+bool
+ctrl_msg_find_tlv(struct buffer *payload, uint16_t wanted_type, struct buffer *value)
+{
+ struct ctrl_msg_tlv_header hdr;
+ while (ctrl_msg_tlv_read_header(payload, &hdr))
+ {
+ /* Take the value out of payload. This validates that the header's
+ * length is really there and advances past it in one step, so the TLV
+ * we are looking for and the ones we skip are bounds-checked alike. */
+ uint8_t *v = buf_read_alloc(payload, hdr.value_len);
+ if (!v)
+ {
+ return false;
+ }
+ if (hdr.type == wanted_type)
+ {
+ buf_set_read(value, v, hdr.value_len);
+ return true;
+ }
+ /* not the TLV we want: keep scanning */
+ }
+ return false;
+}
diff --git a/src/openvpn/control_msg.h b/src/openvpn/control_msg.h
new file mode 100644
index 0000000..e4c8b54
--- /dev/null
+++ b/src/openvpn/control_msg.h
@@ -0,0 +1,101 @@
+/*
+ * OpenVPN -- An application to securely tunnel IP networks
+ * over a single TCP/UDP port, with support for SSL/TLS-based
+ * session authentication and key exchange,
+ * packet encryption, packet authentication, and
+ * packet compression.
+ *
+ * Copyright (C) 2002-2026 OpenVPN Inc <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <https://www.gnu.org/licenses/>.
+ */
+
+/**
+ * @file
+ * Framing shared by the TLV-based control messages of the wire protocol.
+ *
+ * Such a message payload starts with a 16-bit message type, followed by a
+ * sequence of TLV entries. Each TLV starts with a 4-byte header: a 16-bit field
+ * whose most significant bit is the "optional" flag and whose remaining 15 bits
+ * are the type, followed by a 16-bit length giving the size of the value that
+ * follows the header.
+ *
+ * The message-type and TLV-type values themselves are specific to the message
+ * family carried (out-of-band messages define theirs in oob.h), so only the
+ * framing lives here.
+ */
+
+#ifndef CONTROL_MSG_H
+#define CONTROL_MSG_H
+
+#include "buffer.h"
+
+/* TLV header bit layout of the first 16-bit field */
+#define CTRL_MSG_TLV_OPTIONAL_FLAG 0x8000
+#define CTRL_MSG_TLV_TYPE_MASK 0x7fff
+
+/* The header every TLV carries: the 15-bit type and optional flag packed into
+ * the first 16-bit field, then the length of the value that follows. */
+struct ctrl_msg_tlv_header
+{
+ uint16_t type; /**< the 15-bit TLV type */
+ bool optional; /**< value of the optional flag */
+ uint16_t value_len; /**< length of the value following the header */
+};
+
+/**
+ * Read and verify a message-type header from buf, advancing past it.
+ *
+ * @param buf buffer positioned at the message payload
+ * @param expected_msg_type the message type the payload must carry
+ * @return true if a message type was read and equals expected_msg_type,
+ * false on a short buffer or a mismatching type.
+ */
+bool ctrl_msg_read_header(struct buffer *buf, uint16_t expected_msg_type);
+
+/**
+ * Write a TLV header (type + optional flag + value length) to buf.
+ *
+ * @return true on success, false if buf has insufficient space.
+ */
+bool ctrl_msg_tlv_write_header(struct buffer *buf, uint16_t type, bool optional,
+ uint16_t value_len);
+
+/**
+ * Read a TLV header from buf, advancing past it.
+ *
+ * @param buf buffer positioned at the TLV header
+ * @param hdr filled with the type, optional flag and value length on success
+ * @return true on success, false if there are not enough bytes for a header.
+ */
+bool ctrl_msg_tlv_read_header(struct buffer *buf, struct ctrl_msg_tlv_header *hdr);
+
+/**
+ * Scan payload for the first TLV of type wanted_type, skipping any other (e.g.
+ * future) TLV types.
+ *
+ * On success value covers exactly the found TLV's value bytes. The length from
+ * each TLV header is validated against payload as the scan goes, so the whole
+ * value is guaranteed to be present; a header claiming more bytes than payload
+ * holds is rejected rather than reported as found. payload is consumed as it is
+ * read.
+ *
+ * @param payload buffer positioned at a TLV header
+ * @param wanted_type the TLV type to look for
+ * @param value set to a buffer covering the found TLV's value
+ * @return true if the TLV was found, false if it is not present or a TLV
+ * header or value is malformed or truncated.
+ */
+bool ctrl_msg_find_tlv(struct buffer *payload, uint16_t wanted_type, struct buffer *value);
+
+#endif /* ifndef CONTROL_MSG_H */
diff --git a/src/openvpn/oob.c b/src/openvpn/oob.c
new file mode 100644
index 0000000..c807171
--- /dev/null
+++ b/src/openvpn/oob.c
@@ -0,0 +1,83 @@
+/*
+ * OpenVPN -- An application to securely tunnel IP networks
+ * over a single TCP/UDP port, with support for SSL/TLS-based
+ * session authentication and key exchange,
+ * packet encryption, packet authentication, and
+ * packet compression.
+ *
+ * Copyright (C) 2002-2026 OpenVPN Inc <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <https://www.gnu.org/licenses/>.
+ */
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include "syshead.h"
+
+#include "oob.h"
+#include "control_msg.h"
+
+bool
+oob_probe_parameter_write(struct buffer *buf, const struct oob_probe_parameter *p)
+{
+ return ctrl_msg_tlv_write_header(buf, OOB_TLV_PROBE_PARAMETER, false, OOB_PROBE_PARAMETER_LEN)
+ && buf_write_u64(buf, p->timestamp)
+ && buf_write_u32(buf, p->flags);
+}
+
+bool
+oob_probe_parameter_read(struct buffer *buf, struct oob_probe_parameter *p)
+{
+ /* One bounds check covers the whole value: every field read below then fits
+ * by construction (OOB_PROBE_PARAMETER_LEN is the sum of their sizes), so
+ * none of them needs its own error handling. Trailing bytes this version
+ * does not understand are simply left unread. */
+ if (buf_len(buf) < OOB_PROBE_PARAMETER_LEN)
+ {
+ return false;
+ }
+ p->timestamp = buf_read_u64(buf, NULL);
+ p->flags = buf_read_u32(buf, NULL);
+ return true;
+}
+
+bool
+oob_probe_reply_write(struct buffer *buf, const struct oob_probe_reply *r)
+{
+ return ctrl_msg_tlv_write_header(buf, OOB_TLV_PROBE_REPLY, false, OOB_PROBE_REPLY_LEN)
+ && session_id_write(&r->peer_session_id, buf)
+ && buf_write_u16(buf, r->priority)
+ && buf_write_u16(buf, r->weight)
+ && buf_write_u16(buf, r->max_latency_diff)
+ && buf_write_u16(buf, r->connect_lifetime)
+ && buf_write_u32(buf, r->flags);
+}
+
+bool
+oob_probe_reply_read(struct buffer *buf, struct oob_probe_reply *r)
+{
+ /* One bounds check for the whole value, as in oob_probe_parameter_read(). */
+ if (buf_len(buf) < OOB_PROBE_REPLY_LEN)
+ {
+ return false;
+ }
+ session_id_read(&r->peer_session_id, buf);
+ r->priority = (uint16_t)buf_read_u16(buf);
+ r->weight = (uint16_t)buf_read_u16(buf);
+ r->max_latency_diff = (uint16_t)buf_read_u16(buf);
+ r->connect_lifetime = (uint16_t)buf_read_u16(buf);
+ r->flags = buf_read_u32(buf, NULL);
+ return true;
+}
diff --git a/src/openvpn/oob.h b/src/openvpn/oob.h
new file mode 100644
index 0000000..0a9a4bb
--- /dev/null
+++ b/src/openvpn/oob.h
@@ -0,0 +1,110 @@
+/*
+ * OpenVPN -- An application to securely tunnel IP networks
+ * over a single TCP/UDP port, with support for SSL/TLS-based
+ * session authentication and key exchange,
+ * packet encryption, packet authentication, and
+ * packet compression.
+ *
+ * Copyright (C) 2002-2026 OpenVPN Inc <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <https://www.gnu.org/licenses/>.
+ */
+
+/**
+ * @file
+ * Encoding/decoding of out-of-band (P_CONTROL_OOB_V1) control messages.
+ *
+ * An OOB message payload starts with a 16-bit message type (the 0x1xx space:
+ * SERVER_PROBE, PROBE_REPLY, ...) followed by a sequence of TLV entries in the
+ * 0x2xx type space. The framing itself is shared with the other TLV-based
+ * control messages of the wire protocol and lives in control_msg.h; this file
+ * defines the OOB message and TLV types, their values, and the OOB-specific
+ * decisions taken on them.
+ */
+
+#ifndef OOB_H
+#define OOB_H
+
+#include "buffer.h"
+#include "session_id.h"
+
+/* OOB message types: the 16-bit value at the start of an OOB payload, before
+ * its TLV entries. Distinct from the TLV-type space (0x2xx); see the "Messages"
+ * table in the OOB section of the wire protocol spec. */
+#define OOB_MSG_SERVER_PROBE 0x100
+#define OOB_MSG_PROBE_REPLY 0x101
+
+/* TLV types (see the OOB control message section of the wire protocol spec) */
+#define OOB_TLV_PROBE_PARAMETER 0x200
+#define OOB_TLV_PROBE_REPLY 0x201
+
+/* Minimum on-wire value length (excluding the 4-byte TLV header) of each TLV:
+ * the sizes of its fixed fields in wire order (see the structs below). The
+ * value may be longer for forward compatibility; trailing bytes that are not
+ * understood are ignored on read. */
+#define OOB_PROBE_PARAMETER_LEN \
+ ((uint16_t)(sizeof(uint64_t) /* timestamp */ + sizeof(uint32_t) /* flags */))
+#define OOB_PROBE_REPLY_LEN \
+ ((uint16_t)(SID_SIZE /* peer_session_id */ \
+ + 4 * sizeof(uint16_t) /* priority, weight, \
+ * max_latency_diff, \
+ * connect_lifetime */ \
+ + sizeof(uint32_t))) /* flags */
+
+/* probe parameter TLV (sent by the client in a SERVER_PROBE) */
+struct oob_probe_parameter
+{
+ uint64_t timestamp; /**< client clock as a UNIX timestamp */
+ uint32_t flags; /**< client capability flags, currently must be 0 */
+};
+
+/* probe reply TLV (sent by the server in a PROBE_REPLY) */
+struct oob_probe_reply
+{
+ struct session_id peer_session_id; /**< echoes the session id of the request */
+ uint16_t priority; /**< DNS-SRV style priority (lower is preferred) */
+ uint16_t weight; /**< DNS-SRV style weight */
+ uint16_t max_latency_diff; /**< advertised candidate-band margin in ms;
+ * 0 means "defer to the client's setting" */
+ uint16_t connect_lifetime; /**< seconds the reply stays valid as the handshake reset */
+ uint32_t flags; /**< server behaviour flags */
+};
+
+/**
+ * Write a complete probe parameter TLV (header + value) to buf.
+ */
+bool oob_probe_parameter_write(struct buffer *buf, const struct oob_probe_parameter *p);
+
+/**
+ * Read a probe parameter TLV value from buf.
+ *
+ * buf must cover exactly the TLV's value, as returned by ctrl_msg_find_tlv().
+ * Trailing bytes beyond the fields understood here are ignored, so a longer
+ * value from a future version still parses.
+ *
+ * @return true on success, false if buf is shorter than the mandatory fields.
+ */
+bool oob_probe_parameter_read(struct buffer *buf, struct oob_probe_parameter *p);
+
+/**
+ * Write a complete probe reply TLV (header + value) to buf.
+ */
+bool oob_probe_reply_write(struct buffer *buf, const struct oob_probe_reply *r);
+
+/**
+ * Read a probe reply TLV value from buf. See oob_probe_parameter_read() for the
+ * calling convention.
+ */
+bool oob_probe_reply_read(struct buffer *buf, struct oob_probe_reply *r);
+
+#endif /* OOB_H */
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 60df7ce..9e1a6aa 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -3616,7 +3616,7 @@
struct session_id sid; /* remote session ID */
/* verify legal opcode */
- if (op < P_FIRST_OPCODE || op > P_LAST_OPCODE)
+ if (!opcode_valid_in_session(op))
{
if (op == P_CONTROL_HARD_RESET_CLIENT_V1 || op == P_CONTROL_HARD_RESET_SERVER_V1)
{
diff --git a/src/openvpn/ssl_pkt.c b/src/openvpn/ssl_pkt.c
index f8444451..0d882d9 100644
--- a/src/openvpn/ssl_pkt.c
+++ b/src/openvpn/ssl_pkt.c
@@ -166,7 +166,7 @@
bool prepend_ack)
{
ASSERT(ks->key_id >= 0 && ks->key_id <= P_KEY_ID_MASK);
- ASSERT(opcode >= 0 && opcode <= P_LAST_OPCODE);
+ ASSERT(opcode_valid_in_session(opcode));
uint8_t header = (uint8_t)(ks->key_id | (opcode << P_OPCODE_SHIFT));
/* Workaround for Softether servers. Softether has a bug that it only
diff --git a/src/openvpn/ssl_pkt.h b/src/openvpn/ssl_pkt.h
index 82cb5b1..25e50bf 100644
--- a/src/openvpn/ssl_pkt.h
+++ b/src/openvpn/ssl_pkt.h
@@ -58,11 +58,53 @@
* like P_CONTROL_HARD_RESET_CLIENT_V3 */
#define P_CONTROL_WKC_V1 11
-/* define the range of legal opcodes
+/* Out-of-band control message that does not belong to an established
+ * control channel session (e.g. a server probe). Inherently unreliable:
+ * there is no protocol-level retransmission.
+ *
+ * It is not legal on an established session; see opcode_valid_in_session()
+ * below. */
+#define P_CONTROL_OOB_V1 12
+
+/* define the range of defined opcodes, in- and out-of-band. Note this is not
+ * the set of opcodes legal on an established session; see
+ * opcode_valid_in_session().
* Since we do no longer support key-method 1 we consider
* the v1 op codes invalid */
#define P_FIRST_OPCODE 3
-#define P_LAST_OPCODE 11
+#define P_LAST_OPCODE 12
+
+static inline bool
+opcode_is_oob(int op)
+{
+ return op == P_CONTROL_OOB_V1;
+}
+
+/**
+ * Return true if op may occur on an established control-channel session.
+ *
+ * Out-of-band opcodes may not. They are answered statelessly on the
+ * new-connection path, and being rejected here is a permanent property rather
+ * than a handler that is still missing:
+ *
+ * - an OOB message carries its TLV payload directly, with no reliability or
+ * ACK fields (see tls_wrap_oob_standalone()), whereas the established-session
+ * path parses an ACK array and a control packet-id before anything else. The
+ * payload is chosen by the sender and session ids are plaintext on the wire,
+ * so those bytes can be crafted into a valid ACK array, forging ACKs into a
+ * live control channel and stalling a handshake or rekey.
+ * - nothing on that path parses an OOB payload, so there is nothing to gain by
+ * accepting one.
+ *
+ * This cannot be expressed as an opcode range: the inband CONTROL_DATA_V1 of the
+ * wire protocol is legal on an established session, so the OOB opcodes sit
+ * between legal ones.
+ */
+static inline bool
+opcode_valid_in_session(int op)
+{
+ return op >= P_FIRST_OPCODE && op <= P_LAST_OPCODE && !opcode_is_oob(op);
+}
/*
* Define number of buffers for send and receive in the reliability layer.
@@ -264,6 +306,9 @@
case P_CONTROL_WKC_V1:
return "P_CONTROL_WKC_V1";
+ case P_CONTROL_OOB_V1:
+ return "P_CONTROL_OOB_V1";
+
case P_ACK_V1:
return "P_ACK_V1";
diff --git a/tests/unit_tests/openvpn/Makefile.am b/tests/unit_tests/openvpn/Makefile.am
index d861ef9..611097a 100644
--- a/tests/unit_tests/openvpn/Makefile.am
+++ b/tests/unit_tests/openvpn/Makefile.am
@@ -14,6 +14,7 @@
ncp_testdriver \
mbuf_testdriver \
misc_testdriver \
+ oob_testdriver \
options_parse_testdriver \
packet_id_testdriver \
pkt_testdriver \
@@ -150,6 +151,20 @@
$(top_srcdir)/src/openvpn/win32-util.c \
$(top_srcdir)/src/openvpn/session_id.c
+oob_testdriver_CFLAGS = \
+ -I$(top_srcdir)/include -I$(top_srcdir)/src/compat -I$(top_srcdir)/src/openvpn \
+ @TEST_CFLAGS@
+oob_testdriver_LDFLAGS = @TEST_LDFLAGS@
+oob_testdriver_SOURCES = test_oob.c \
+ mock_msg.c mock_msg.h test_common.h \
+ mock_get_random.c \
+ $(top_srcdir)/src/openvpn/buffer.c \
+ $(top_srcdir)/src/openvpn/control_msg.c \
+ $(top_srcdir)/src/openvpn/oob.c \
+ $(top_srcdir)/src/openvpn/platform.c \
+ $(top_srcdir)/src/openvpn/session_id.c \
+ $(top_srcdir)/src/openvpn/win32-util.c
+
pkt_testdriver_CFLAGS = \
-I$(top_srcdir)/include -I$(top_srcdir)/src/compat -I$(top_srcdir)/src/openvpn \
@TEST_CFLAGS@
diff --git a/tests/unit_tests/openvpn/test_oob.c b/tests/unit_tests/openvpn/test_oob.c
new file mode 100644
index 0000000..aa4eb23
--- /dev/null
+++ b/tests/unit_tests/openvpn/test_oob.c
@@ -0,0 +1,277 @@
+/*
+ * OpenVPN -- An application to securely tunnel IP networks
+ * over a single TCP/UDP port, with support for SSL/TLS-based
+ * session authentication and key exchange,
+ * packet encryption, packet authentication, and
+ * packet compression.
+ *
+ * Copyright (C) 2002-2026 OpenVPN Inc <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <https://www.gnu.org/licenses/>.
+ */
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include "syshead.h"
+
+#include <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <cmocka.h>
+
+#include "control_msg.h"
+#include "oob.h"
+#include "test_common.h"
+
+/* Write a probe parameter TLV and read it back; fields must survive the
+ * round trip and the whole buffer must be consumed. */
+static void
+test_probe_parameter_roundtrip(void **state)
+{
+ struct gc_arena gc = gc_new();
+ struct buffer buf = alloc_buf_gc(128, &gc);
+
+ const struct oob_probe_parameter in = {
+ .timestamp = 0x0123456789abcdefULL,
+ .flags = 0,
+ };
+ assert_true(oob_probe_parameter_write(&buf, &in));
+ /* header (4) + value (12) */
+ assert_int_equal(BLEN(&buf), 4 + OOB_PROBE_PARAMETER_LEN);
+
+ /* the header codec, read from a copy so the scan below still sees it */
+ struct buffer peek = buf;
+ struct ctrl_msg_tlv_header hdr;
+ assert_true(ctrl_msg_tlv_read_header(&peek, &hdr));
+ assert_int_equal(hdr.type, OOB_TLV_PROBE_PARAMETER);
+ assert_false(hdr.optional);
+ assert_int_equal(hdr.value_len, OOB_PROBE_PARAMETER_LEN);
+
+ struct buffer value;
+ assert_true(ctrl_msg_find_tlv(&buf, OOB_TLV_PROBE_PARAMETER, &value));
+ assert_int_equal(BLEN(&value), OOB_PROBE_PARAMETER_LEN);
+
+ struct oob_probe_parameter out = { 0 };
+ assert_true(oob_probe_parameter_read(&value, &out));
+ assert_true(in.timestamp == out.timestamp);
+ assert_int_equal(in.flags, out.flags);
+ /* the scan consumed header and value alike */
+ assert_int_equal(BLEN(&buf), 0);
+
+ gc_free(&gc);
+}
+
+/* Write a probe reply TLV and read it back. */
+static void
+test_probe_reply_roundtrip(void **state)
+{
+ struct gc_arena gc = gc_new();
+ struct buffer buf = alloc_buf_gc(128, &gc);
+
+ struct oob_probe_reply in = {
+ .priority = 10,
+ .weight = 100,
+ .connect_lifetime = 30,
+ .flags = 1,
+ .max_latency_diff = 25,
+ };
+ memcpy(in.peer_session_id.id, "ABCDEFGH", SID_SIZE);
+
+ assert_true(oob_probe_reply_write(&buf, &in));
+ assert_int_equal(BLEN(&buf), 4 + OOB_PROBE_REPLY_LEN);
+
+ struct buffer peek = buf;
+ struct ctrl_msg_tlv_header hdr;
+ assert_true(ctrl_msg_tlv_read_header(&peek, &hdr));
+ assert_int_equal(hdr.type, OOB_TLV_PROBE_REPLY);
+ assert_int_equal(hdr.value_len, OOB_PROBE_REPLY_LEN);
+
+ struct buffer value;
+ assert_true(ctrl_msg_find_tlv(&buf, OOB_TLV_PROBE_REPLY, &value));
+
+ struct oob_probe_reply out = { 0 };
+ assert_true(oob_probe_reply_read(&value, &out));
+ assert_memory_equal(in.peer_session_id.id, out.peer_session_id.id, SID_SIZE);
+ assert_int_equal(in.priority, out.priority);
+ assert_int_equal(in.weight, out.weight);
+ assert_int_equal(in.connect_lifetime, out.connect_lifetime);
+ assert_int_equal(in.flags, out.flags);
+ assert_int_equal(in.max_latency_diff, out.max_latency_diff);
+ assert_int_equal(BLEN(&buf), 0);
+
+ gc_free(&gc);
+}
+
+/* The probe reply wire format is locked to the spec's field order
+ * (priority, weight, max_latency_diff, connect_lifetime, flags), big-endian. */
+static void
+test_probe_reply_wire_format(void **state)
+{
+ struct gc_arena gc = gc_new();
+ struct buffer buf = alloc_buf_gc(128, &gc);
+
+ struct oob_probe_reply in = {
+ .priority = 10,
+ .weight = 100,
+ .connect_lifetime = 30,
+ .flags = 1,
+ .max_latency_diff = 25,
+ };
+ memcpy(in.peer_session_id.id, "ABCDEFGH", SID_SIZE);
+
+ assert_true(oob_probe_reply_write(&buf, &in));
+
+ const uint8_t expected[] = {
+ 0x02,
+ 0x01, /* TLV type 0x201 (not optional) */
+ 0x00,
+ 0x14, /* TLV value length = 20 */
+ 'A',
+ 'B',
+ 'C',
+ 'D',
+ 'E',
+ 'F',
+ 'G',
+ 'H', /* peer_session_id */
+ 0x00,
+ 0x0a, /* priority = 10 */
+ 0x00,
+ 0x64, /* weight = 100 */
+ 0x00,
+ 0x19, /* max_latency_diff = 25 */
+ 0x00,
+ 0x1e, /* connect_lifetime = 30 */
+ 0x00,
+ 0x00,
+ 0x00,
+ 0x01, /* flags = 1 */
+ };
+ assert_int_equal(BLEN(&buf), sizeof(expected));
+ assert_memory_equal(BPTR(&buf), expected, sizeof(expected));
+
+ gc_free(&gc);
+}
+
+/* A TLV with a longer-than-known value must still parse: the known fields are
+ * read and the trailing bytes are skipped (forward compatibility). */
+static void
+test_probe_parameter_forward_compat(void **state)
+{
+ struct gc_arena gc = gc_new();
+ struct buffer buf = alloc_buf_gc(128, &gc);
+
+ const uint16_t extended_len = OOB_PROBE_PARAMETER_LEN + 4;
+ assert_true(ctrl_msg_tlv_write_header(&buf, OOB_TLV_PROBE_PARAMETER, false, extended_len));
+ assert_true(buf_write_u32(&buf, 0)); /* timestamp high */
+ assert_true(buf_write_u32(&buf, 0xdeadbeef)); /* timestamp low */
+ assert_true(buf_write_u32(&buf, 0)); /* flags */
+ assert_true(buf_write_u32(&buf, 0x11223344)); /* unknown trailing field */
+
+ struct buffer value;
+ assert_true(ctrl_msg_find_tlv(&buf, OOB_TLV_PROBE_PARAMETER, &value));
+ assert_int_equal(BLEN(&value), extended_len);
+
+ struct oob_probe_parameter out = { 0 };
+ assert_true(oob_probe_parameter_read(&value, &out));
+ assert_true(out.timestamp == 0xdeadbeefULL);
+ assert_int_equal(out.flags, 0);
+ /* the unknown trailing field must have been consumed from the payload */
+ assert_int_equal(BLEN(&buf), 0);
+
+ gc_free(&gc);
+}
+
+/* A value shorter than the mandatory fields must be rejected, even when it
+ * holds enough bytes for some of the individual fields to read successfully. */
+static void
+test_probe_parameter_too_short(void **state)
+{
+ struct gc_arena gc = gc_new();
+ struct buffer buf = alloc_buf_gc(128, &gc);
+
+ /* 4 of the 12 mandatory value bytes: too short for the timestamp, but
+ * enough for a u32 read to succeed on its own */
+ assert_true(buf_write_u32(&buf, 0xdeadbeef));
+
+ struct oob_probe_parameter out = { 0 };
+ assert_false(oob_probe_parameter_read(&buf, &out));
+
+ gc_free(&gc);
+}
+
+/* A TLV header claiming more value bytes than the payload holds must be
+ * rejected by the scan rather than reported as found -- for the TLV being
+ * looked for as much as for one that would merely be skipped. */
+static void
+test_find_tlv_value_truncated(void **state)
+{
+ struct gc_arena gc = gc_new();
+ struct buffer value;
+
+ /* the wanted TLV declares 12 value bytes, only 4 are present */
+ struct buffer buf = alloc_buf_gc(128, &gc);
+ assert_true(ctrl_msg_tlv_write_header(&buf, OOB_TLV_PROBE_PARAMETER, false,
+ OOB_PROBE_PARAMETER_LEN));
+ assert_true(buf_write_u32(&buf, 0xdeadbeef));
+ assert_false(ctrl_msg_find_tlv(&buf, OOB_TLV_PROBE_PARAMETER, &value));
+
+ /* same defect on a TLV that would be skipped: the scan must not walk past
+ * the end of the payload looking for the next header */
+ struct buffer buf2 = alloc_buf_gc(128, &gc);
+ assert_true(ctrl_msg_tlv_write_header(&buf2, 0x7ff, false, 64));
+ assert_true(buf_write_u32(&buf2, 0));
+ assert_false(ctrl_msg_find_tlv(&buf2, OOB_TLV_PROBE_PARAMETER, &value));
+
+ gc_free(&gc);
+}
+
+/* Reading a TLV header must fail when the buffer holds less data than a
+ * complete 4-byte header (empty, or only the type field), rather than read
+ * past the available data. */
+static void
+test_tlv_header_truncated(void **state)
+{
+ struct gc_arena gc = gc_new();
+ struct buffer buf = alloc_buf_gc(128, &gc);
+
+ struct ctrl_msg_tlv_header hdr;
+
+ /* empty buffer */
+ assert_false(ctrl_msg_tlv_read_header(&buf, &hdr));
+
+ /* only the type field present, no length */
+ assert_true(buf_write_u16(&buf, OOB_TLV_PROBE_PARAMETER));
+ assert_false(ctrl_msg_tlv_read_header(&buf, &hdr));
+
+ gc_free(&gc);
+}
+
+int
+main(void)
+{
+ openvpn_unit_test_setup();
+ const struct CMUnitTest tests[] = {
+ cmocka_unit_test(test_probe_parameter_roundtrip),
+ cmocka_unit_test(test_probe_reply_roundtrip),
+ cmocka_unit_test(test_probe_reply_wire_format),
+ cmocka_unit_test(test_probe_parameter_forward_compat),
+ cmocka_unit_test(test_probe_parameter_too_short),
+ cmocka_unit_test(test_find_tlv_value_truncated),
+ cmocka_unit_test(test_tlv_header_truncated),
+ };
+
+ return cmocka_run_group_tests_name("oob tests", tests, NULL, NULL);
+}
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1741?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: I1c8d302ac57c5603d622a7be14be369437388268
Gerrit-Change-Number: 1741
Gerrit-PatchSet: 9
Gerrit-Owner: stipa <[email protected]>
Gerrit-Reviewer: flichtenheld <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: flichtenheld <[email protected]>
Gerrit-Attention: stipa <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel