[L] Change in openvpn[master]: oob: Add control message TLV encoding (P_CONTROL_OOB_V1)
"stipa \(Code Review\) via Openvpn-devel" <[email protected]>
| Newsgroups | gmane.network.openvpn.devel |
|---|---|
| Message-ID | <4940f7ea298fc9b58f532460d7ecf6822385e0cf-EmailReplacePatchSet-HTML@gerrit.openvpn.net> |
Attention is currently required from: flichtenheld, plaisthos.
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 (#12).
Change subject: oob: Add control message TLV encoding (P_CONTROL_OOB_V1)
......................................................................
oob: Add control message TLV encoding (P_CONTROL_OOB_V1)
Out-of-band control messages carry control data but belong to no session, so a
client can ask a server about itself before connecting. Reserve opcode 12 for
them and add the codec for the two messages the probe needs, SERVER_PROBE and
PROBE_REPLY.
The spec uses the same TLV framing for all its control messages, not just the
out-of-band ones, so the message-type header, the TLV header and the TLV scan go
into their 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.
See the OOB control message section of the wire protocol specification
(openvpn-rfc PR #30).
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, 756 insertions(+), 4 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/41/1741/12
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 6eb5954..fbc69f0 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -445,6 +445,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
@@ -523,6 +525,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
@@ -677,6 +681,7 @@
"test_mbuf"
"test_misc"
"test_ncp"
+ "test_oob"
"test_options_parse"
"test_packet_id"
"test_pkt"
@@ -881,6 +886,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 7fd12b4..5e6b4d9 100644
--- a/src/openvpn/Makefile.am
+++ b/src/openvpn/Makefile.am
@@ -53,6 +53,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 \
@@ -107,6 +108,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_show.c options_show.h \
diff --git a/src/openvpn/control_msg.c b/src/openvpn/control_msg.c
new file mode 100644
index 0000000..6d3b3bf
--- /dev/null
+++ b/src/openvpn/control_msg.c
@@ -0,0 +1,102 @@
+/*
+ * 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;
+ bool found = false;
+ 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)
+ {
+ if (!found)
+ {
+ buf_set_read(value, v, hdr.value_len);
+ found = true;
+ }
+ }
+ else if (!hdr.optional)
+ {
+ /* A TLV we do not understand that the sender did not mark optional
+ * carries something it requires us to act on, so the message as a
+ * whole is not ours to interpret. This is why the scan continues
+ * past the wanted type: such a TLV invalidates the message wherever
+ * in the sequence it sits. */
+ return false;
+ }
+ }
+ return found;
+}
diff --git a/src/openvpn/control_msg.h b/src/openvpn/control_msg.h
new file mode 100644
index 0000000..b98dce5
--- /dev/null
+++ b/src/openvpn/control_msg.h
@@ -0,0 +1,106 @@
+/*
+ * 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 type that is marked optional.
+ *
+ * The whole sequence is walked, including the part after the wanted type: a TLV
+ * we do not understand and that is not marked optional invalidates the message
+ * wherever it sits, and rejecting it is the point of the remaining scan.
+ *
+ * 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, a TLV header
+ * or value is malformed or truncated, or a TLV we do not understand is
+ * not marked optional.
+ */
+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 5f5d1f9..c8fc465 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -3676,7 +3676,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 6a6d9f9..39cb648 100644
--- a/src/openvpn/ssl_pkt.c
+++ b/src/openvpn/ssl_pkt.c
@@ -167,7 +167,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 03e8930..d46b8c5 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.
@@ -256,6 +298,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 5954902..9f6b865 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 \
@@ -153,6 +154,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: 12
Gerrit-Owner: stipa <[email protected]>
Gerrit-Reviewer: flichtenheld <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
Gerrit-Attention: flichtenheld <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel