Attention is currently required from: flichtenheld, plaisthos, ralf_lici, stipa.
Hello flichtenheld, plaisthos, ralf_lici,
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 (#19).
The following approvals got outdated and were removed:
Code-Review-1 by plaisthos
Change subject: oob: Add P_CONTROL_OOB_V1 and the probe request and reply TLVs
......................................................................
oob: Add P_CONTROL_OOB_V1 and the probe request and reply TLVs
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 TLV codecs the probe needs: the
probe request a client probes a server with, and the probe reply the
server answers with.
A message is just a sequence of TLVs, and the TLVs it carries say what
it is. The probe request carries a request_id chosen by the client,
which the probe reply echoes.
The TLVs use the shared TLV codec, control_msg.c/h, and their two types
join the others in control_msg.h. The codec gains ctrl_msg_find_tlv(),
which scans a payload for the one mandatory TLV of a message. The scan
is strict: a truncated TLV header or value, or an unknown TLV not marked
optional, rejects the whole message.
P_LAST_OPCODE becomes 12. tls_pre_decrypt() rejects the OOB opcodes
explicitly: they are not part of the reliable control channel (no
control message id, no ACK array); the server answers them
statelessly in a follow-up.
Change-Id: I1c8d302ac57c5603d622a7be14be369437388268
Signed-off-by: Lev Stipakov <[email protected]>
---
M CMakeLists.txt
M src/openvpn/Makefile.am
M src/openvpn/control_msg.c
M 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
M tests/unit_tests/openvpn/test_pkt.c
12 files changed, 632 insertions(+), 6 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/41/1741/19
diff --git a/CMakeLists.txt b/CMakeLists.txt
index e1a6079f..8a062b8 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -525,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
@@ -886,7 +888,9 @@
target_sources(test_pkt PRIVATE
tests/unit_tests/openvpn/mock_win32_execve.c
tests/unit_tests/openvpn/test_control_msg.c
+ tests/unit_tests/openvpn/test_oob.c
src/openvpn/control_msg.c
+ src/openvpn/oob.c
src/openvpn/argv.c
src/openvpn/base64.c
src/openvpn/crypto_epoch.c
diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
index a17ae8e..5e6b4d9 100644
--- a/src/openvpn/Makefile.am
+++ b/src/openvpn/Makefile.am
@@ -108,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
index ca3aab9..76f0cfa 100644
--- a/src/openvpn/control_msg.c
+++ b/src/openvpn/control_msg.c
@@ -81,3 +81,31 @@
buf_set_read(value, v, hdr->value_len);
return true;
}
+
+bool
+ctrl_msg_find_tlv(struct buffer *payload, uint16_t wanted_type, struct buffer
*value)
+{
+ bool found = false;
+ while (BLEN(payload) > 0)
+ {
+ struct ctrl_msg_tlv_header hdr;
+ struct buffer v;
+ if (!ctrl_msg_tlv_next(payload, &hdr, &v))
+ {
+ return false;
+ }
+ if (hdr.type == wanted_type)
+ {
+ if (!found)
+ {
+ *value = v;
+ found = true;
+ }
+ }
+ else if (!hdr.optional)
+ {
+ return false; /* a mandatory TLV we do not understand, wherever it
sits */
+ }
+ }
+ return found;
+}
diff --git a/src/openvpn/control_msg.h b/src/openvpn/control_msg.h
index 73fd0a1..e731590 100644
--- a/src/openvpn/control_msg.h
+++ b/src/openvpn/control_msg.h
@@ -41,6 +41,8 @@
/* TLV types */
#define TLV_TYPE_EARLY_NEG_FLAGS 0x0001 /* early negotiation, in the reset
packets */
+#define TLV_TYPE_PROBE_REQUEST 0x0002 /* out-of-band, sent by a client
probing a server */
+#define TLV_TYPE_PROBE_REPLY 0x0003 /* out-of-band, a server's answer to a
probe request */
/* TLV header bit layout of the first 16-bit field */
#define CTRL_MSG_TLV_OPTIONAL_FLAG 0x8000
@@ -96,4 +98,28 @@
*/
bool ctrl_msg_tlv_next(struct buffer *buf, struct ctrl_msg_tlv_header *hdr,
struct buffer *value);
+/**
+ * Scan payload for the TLV of type wanted_type, skipping any other (e.g.
+ * future) TLV type that is marked optional.
+ *
+ * This is for messages that carry exactly one mandatory TLV, which matches the
+ * currently supported OOB messages: any other TLV that is not marked optional
+ * invalidates the message wherever it is present, so the whole sequence is
+ * walked. A message with several mandatory TLVs walks them with
+ * ctrl_msg_tlv_next() instead.
+ *
+ * On success value covers exactly the found TLV's value bytes; as with
+ * ctrl_msg_tlv_next(), 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; it
points
+ * into payload and owns no storage
+ * @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..5f624be
--- /dev/null
+++ b/src/openvpn/oob.c
@@ -0,0 +1,85 @@
+/*
+ * 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_request_write(struct buffer *buf, const struct oob_probe_request *r)
+{
+ return ctrl_msg_tlv_write_header(buf, TLV_TYPE_PROBE_REQUEST, false,
OOB_PROBE_REQUEST_LEN)
+ && buf_write_u32(buf, r->request_id)
+ && buf_write_u64(buf, r->timestamp)
+ && buf_write_u32(buf, r->flags);
+}
+
+bool
+oob_probe_request_read(struct buffer *buf, struct oob_probe_request *r)
+{
+ /* One bounds check covers the whole value: every field read below then
fits
+ * by construction (OOB_PROBE_REQUEST_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_REQUEST_LEN)
+ {
+ return false;
+ }
+ r->request_id = buf_read_u32(buf, NULL);
+ r->timestamp = buf_read_u64(buf, NULL);
+ r->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, TLV_TYPE_PROBE_REPLY, false,
OOB_PROBE_REPLY_LEN)
+ && buf_write_u32(buf, r->request_id)
+ && 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_u16(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_request_read(). */
+ if (buf_len(buf) < OOB_PROBE_REPLY_LEN)
+ {
+ return false;
+ }
+ r->request_id = buf_read_u32(buf, NULL);
+ 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 = (uint16_t)buf_read_u16(buf);
+ return true;
+}
diff --git a/src/openvpn/oob.h b/src/openvpn/oob.h
new file mode 100644
index 0000000..e3e4144
--- /dev/null
+++ b/src/openvpn/oob.h
@@ -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/>.
+ */
+
+/**
+ * @file
+ * Encoding/decoding of out-of-band (P_CONTROL_OOB_V1) control messages.
+ *
+ * An OOB message payload is a sequence of TLV entries; the TLVs it carries say
+ * what the message is: a client probes a server with a probe request TLV, and
+ * the server answers with a probe reply TLV. The TLV framing and the TLV types
+ * are shared with the other TLV-based control messages and live in
+ * control_msg.h; this file defines the values of the OOB TLVs and the
+ * OOB-specific decisions taken on them.
+ */
+
+#ifndef OOB_H
+#define OOB_H
+
+#include "buffer.h"
+#include "control_msg.h"
+#include "session_id.h"
+
+/* 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.
+ *
+ * probe request: request_id (u32), timestamp (u64), flags (u32)
+ * probe reply: request_id (u32), then priority, weight, max_latency_diff,
+ * connect_lifetime and flags (u16 each) */
+#define OOB_PROBE_REQUEST_LEN ((uint16_t)(sizeof(uint32_t) + sizeof(uint64_t)
+ sizeof(uint32_t)))
+#define OOB_PROBE_REPLY_LEN ((uint16_t)(sizeof(uint32_t) + 5 *
sizeof(uint16_t)))
+
+/* probe request TLV (sent by the client to probe a server) */
+struct oob_probe_request
+{
+ uint32_t request_id; /**< chosen by the client, echoed in the reply */
+ 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 to answer a probe request) */
+struct oob_probe_reply
+{
+ uint32_t request_id; /**< echoes the request_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;
+ * used unless the client configured its own
*/
+ uint16_t connect_lifetime; /**< seconds the reply stays valid as the
handshake reset */
+ uint16_t flags; /**< server behaviour flags */
+};
+
+/**
+ * Write a complete probe request TLV (header + value) to buf. It is the whole
+ * payload of the OOB message probing a server.
+ */
+bool oob_probe_request_write(struct buffer *buf, const struct
oob_probe_request *r);
+
+/**
+ * Read a probe request 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_request_read(struct buffer *buf, struct oob_probe_request *r);
+
+/**
+ * Write a complete probe reply TLV (header + value) to buf. It is the whole
+ * payload of the OOB message answering a probe request.
+ */
+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_request_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 92f1991..8e2a459 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -3701,8 +3701,9 @@
bool new_link = false;
struct session_id sid; /* remote session ID */
- /* verify legal opcode */
- if (op < P_FIRST_OPCODE || op > P_LAST_OPCODE)
+ /* verify legal opcode. An out-of-band packet has no control message id and
+ * no ACK array, which is what we parse next. */
+ if (op < P_FIRST_OPCODE || op > P_LAST_OPCODE || opcode_is_oob(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 405e5b4..38fb8ed 100644
--- a/src/openvpn/ssl_pkt.c
+++ b/src/openvpn/ssl_pkt.c
@@ -169,6 +169,8 @@
{
ASSERT(ks->key_id >= 0 && ks->key_id <= P_KEY_ID_MASK);
ASSERT(opcode >= 0 && opcode <= P_LAST_OPCODE);
+ /* OOB packets carry no message id or ACK array */
+ ASSERT(!opcode_is_oob(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 5a1d194..d477d5a 100644
--- a/src/openvpn/ssl_pkt.h
+++ b/src/openvpn/ssl_pkt.h
@@ -58,11 +58,23 @@
* like P_CONTROL_HARD_RESET_CLIENT_V3 */
#define P_CONTROL_WKC_V1 11
-/* define the range of legal opcodes
+/* Out-of-band control message, e.g. a server probe. Not part of the reliable
+ * control channel: no control message id and no ACK array, just TLVs
+ * after the session id; never handled by tls_pre_decrypt(). */
+#define P_CONTROL_OOB_V1 12
+
+/* define the range of defined opcodes, in- and out-of-band
* 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
+
+/* Is op one of the out-of-band opcodes? */
+static inline bool
+opcode_is_oob(int op)
+{
+ return op == P_CONTROL_OOB_V1;
+}
/*
* Define number of buffers for send and receive in the reliability layer.
@@ -256,6 +268,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 d9caa03..cb2ffd8 100644
--- a/tests/unit_tests/openvpn/Makefile.am
+++ b/tests/unit_tests/openvpn/Makefile.am
@@ -157,12 +157,13 @@
-I$(top_srcdir)/include -I$(top_srcdir)/src/compat
-I$(top_srcdir)/src/openvpn \
@TEST_CFLAGS@
pkt_testdriver_LDFLAGS = @TEST_LDFLAGS@
-pkt_testdriver_SOURCES = test_pkt.c test_control_msg.c mock_msg.c mock_msg.h
mock_win32_execve.c \
- test_common.h \
+pkt_testdriver_SOURCES = test_pkt.c test_control_msg.c test_oob.c mock_msg.c
mock_msg.h \
+ mock_win32_execve.c test_common.h \
$(top_srcdir)/src/openvpn/argv.c \
$(top_srcdir)/src/openvpn/base64.c \
$(top_srcdir)/src/openvpn/buffer.c \
$(top_srcdir)/src/openvpn/control_msg.c \
+ $(top_srcdir)/src/openvpn/oob.c \
$(top_srcdir)/src/openvpn/crypto.c \
$(top_srcdir)/src/openvpn/crypto_epoch.c \
$(top_srcdir)/src/openvpn/crypto_mbedtls.c \
diff --git a/tests/unit_tests/openvpn/test_oob.c
b/tests/unit_tests/openvpn/test_oob.c
new file mode 100644
index 0000000..4e06cfd
--- /dev/null
+++ b/tests/unit_tests/openvpn/test_oob.c
@@ -0,0 +1,359 @@
+/*
+ * 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 request TLV and read it back; fields must survive the round
+ * trip and the whole buffer must be consumed. */
+static void
+test_probe_request_roundtrip(void **state)
+{
+ struct gc_arena gc = gc_new();
+ struct buffer buf = alloc_buf_gc(128, &gc);
+
+ const struct oob_probe_request in = {
+ .request_id = 0xdeadbeef,
+ .timestamp = 0x0123456789abcdefULL,
+ .flags = 0,
+ };
+ assert_true(oob_probe_request_write(&buf, &in));
+ /* header (4) + value (16) */
+ assert_int_equal(BLEN(&buf), 4 + OOB_PROBE_REQUEST_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, TLV_TYPE_PROBE_REQUEST);
+ assert_false(hdr.optional);
+ assert_int_equal(hdr.value_len, OOB_PROBE_REQUEST_LEN);
+
+ struct buffer value;
+ assert_true(ctrl_msg_find_tlv(&buf, TLV_TYPE_PROBE_REQUEST, &value));
+ assert_int_equal(BLEN(&value), OOB_PROBE_REQUEST_LEN);
+
+ struct oob_probe_request out = { 0 };
+ assert_true(oob_probe_request_read(&value, &out));
+ assert_int_equal(in.request_id, out.request_id);
+ 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);
+}
+
+/* The probe request wire format is locked to the spec's field order
+ * (request_id, timestamp, flags), big-endian. */
+static void
+test_probe_request_wire_format(void **state)
+{
+ struct gc_arena gc = gc_new();
+ struct buffer buf = alloc_buf_gc(128, &gc);
+
+ const struct oob_probe_request in = {
+ .request_id = 0x11223344,
+ .timestamp = 0x0102030405060708ULL,
+ .flags = 0,
+ };
+ assert_true(oob_probe_request_write(&buf, &in));
+
+ const uint8_t expected[] = {
+ 0x00,
+ 0x02, /* TLV type 2 (not optional) */
+ 0x00,
+ 0x10, /* TLV value length = 16 */
+ 0x11,
+ 0x22,
+ 0x33,
+ 0x44, /* request_id */
+ 0x01,
+ 0x02,
+ 0x03,
+ 0x04,
+ 0x05,
+ 0x06,
+ 0x07,
+ 0x08, /* timestamp */
+ 0x00,
+ 0x00,
+ 0x00,
+ 0x00, /* flags */
+ };
+ assert_int_equal(BLEN(&buf), sizeof(expected));
+ assert_memory_equal(BPTR(&buf), expected, sizeof(expected));
+
+ 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 = {
+ .request_id = 0x11223344,
+ .priority = 10,
+ .weight = 100,
+ .connect_lifetime = 30,
+ .flags = 1,
+ .max_latency_diff = 25,
+ };
+
+ 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, TLV_TYPE_PROBE_REPLY);
+ assert_int_equal(hdr.value_len, OOB_PROBE_REPLY_LEN);
+
+ struct buffer value;
+ assert_true(ctrl_msg_find_tlv(&buf, TLV_TYPE_PROBE_REPLY, &value));
+
+ struct oob_probe_reply out = { 0 };
+ assert_true(oob_probe_reply_read(&value, &out));
+ assert_int_equal(in.request_id, out.request_id);
+ 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 (request_id,
+ * 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 = {
+ .request_id = 0x11223344,
+ .priority = 10,
+ .weight = 100,
+ .connect_lifetime = 30,
+ .flags = 1,
+ .max_latency_diff = 25,
+ };
+
+ assert_true(oob_probe_reply_write(&buf, &in));
+
+ const uint8_t expected[] = {
+ 0x00,
+ 0x03, /* TLV type 3 (not optional) */
+ 0x00,
+ 0x0e, /* TLV value length = 14 */
+ 0x11,
+ 0x22,
+ 0x33,
+ 0x44, /* request_id */
+ 0x00,
+ 0x0a, /* priority = 10 */
+ 0x00,
+ 0x64, /* weight = 100 */
+ 0x00,
+ 0x19, /* max_latency_diff = 25 */
+ 0x00,
+ 0x1e, /* connect_lifetime = 30 */
+ 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_request_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_REQUEST_LEN + 4;
+ assert_true(ctrl_msg_tlv_write_header(&buf, TLV_TYPE_PROBE_REQUEST, false,
extended_len));
+ assert_true(buf_write_u32(&buf, 7)); /* request_id */
+ 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, TLV_TYPE_PROBE_REQUEST, &value));
+ assert_int_equal(BLEN(&value), extended_len);
+
+ struct oob_probe_request out = { 0 };
+ assert_true(oob_probe_request_read(&value, &out));
+ assert_int_equal(out.request_id, 7);
+ 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_request_too_short(void **state)
+{
+ struct gc_arena gc = gc_new();
+ struct buffer buf = alloc_buf_gc(128, &gc);
+
+ /* 4 of the 16 mandatory value bytes: enough for the request_id alone */
+ assert_true(buf_write_u32(&buf, 0xdeadbeef));
+
+ struct oob_probe_request out = { 0 };
+ assert_false(oob_probe_request_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, TLV_TYPE_PROBE_REQUEST, false,
+ OOB_PROBE_REQUEST_LEN));
+ assert_true(buf_write_u32(&buf, 0xdeadbeef));
+ assert_false(ctrl_msg_find_tlv(&buf, TLV_TYPE_PROBE_REQUEST, &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, TLV_TYPE_PROBE_REQUEST, &value));
+
+ gc_free(&gc);
+}
+
+/* Bytes left over after the last TLV that are too few for a header make the
+ * message malformed, even when the wanted TLV was already found. */
+static void
+test_find_tlv_trailing_bytes(void **state)
+{
+ struct gc_arena gc = gc_new();
+ struct buffer value;
+
+ /* 1 to 3 bytes: less than a 4-byte TLV header */
+ for (int trailing = 1; trailing <= 3; trailing++)
+ {
+ struct buffer buf = alloc_buf_gc(128, &gc);
+ const struct oob_probe_request in = { .timestamp = 1, .flags = 0 };
+ assert_true(oob_probe_request_write(&buf, &in));
+ for (int i = 0; i < trailing; i++)
+ {
+ assert_true(buf_write_u8(&buf, 0));
+ }
+ assert_false(ctrl_msg_find_tlv(&buf, TLV_TYPE_PROBE_REQUEST, &value));
+ }
+
+ gc_free(&gc);
+}
+
+/* An unknown TLV marked optional after the wanted one is skipped, and the
+ * wanted value is still the one returned. */
+static void
+test_find_tlv_skips_optional_after_wanted(void **state)
+{
+ struct gc_arena gc = gc_new();
+ struct buffer buf = alloc_buf_gc(128, &gc);
+ struct buffer value;
+
+ const struct oob_probe_request in = { .timestamp = 7, .flags = 0 };
+ assert_true(oob_probe_request_write(&buf, &in));
+ assert_true(ctrl_msg_tlv_write_header(&buf, 0x7ff, true, 4));
+ assert_true(buf_write_u32(&buf, 0));
+
+ assert_true(ctrl_msg_find_tlv(&buf, TLV_TYPE_PROBE_REQUEST, &value));
+ assert_int_equal(BLEN(&value), OOB_PROBE_REQUEST_LEN);
+ struct oob_probe_request out = { 0 };
+ assert_true(oob_probe_request_read(&value, &out));
+ assert_true(out.timestamp == 7);
+
+ gc_free(&gc);
+}
+
+/* An empty payload holds no TLV. */
+static void
+test_find_tlv_empty_payload(void **state)
+{
+ struct gc_arena gc = gc_new();
+ struct buffer buf = alloc_buf_gc(16, &gc);
+ struct buffer value;
+
+ assert_false(ctrl_msg_find_tlv(&buf, TLV_TYPE_PROBE_REQUEST, &value));
+
+ gc_free(&gc);
+}
+
+/* The OOB tests run as a second group of pkt_testdriver; see test_pkt.c. */
+int
+run_oob_tests(void)
+{
+ const struct CMUnitTest tests[] = {
+ cmocka_unit_test(test_probe_request_roundtrip),
+ cmocka_unit_test(test_probe_request_wire_format),
+ cmocka_unit_test(test_probe_reply_roundtrip),
+ cmocka_unit_test(test_probe_reply_wire_format),
+ cmocka_unit_test(test_probe_request_forward_compat),
+ cmocka_unit_test(test_probe_request_too_short),
+ cmocka_unit_test(test_find_tlv_value_truncated),
+ cmocka_unit_test(test_find_tlv_trailing_bytes),
+ cmocka_unit_test(test_find_tlv_skips_optional_after_wanted),
+ cmocka_unit_test(test_find_tlv_empty_payload),
+ };
+
+ return cmocka_run_group_tests_name("oob tests", tests, NULL, NULL);
+}
diff --git a/tests/unit_tests/openvpn/test_pkt.c
b/tests/unit_tests/openvpn/test_pkt.c
index 9ce9c50..0398a1f 100644
--- a/tests/unit_tests/openvpn/test_pkt.c
+++ b/tests/unit_tests/openvpn/test_pkt.c
@@ -46,6 +46,7 @@
#include "siphash.h"
int run_control_msg_tests(void); /* test_control_msg.c */
+int run_oob_tests(void); /* test_oob.c */
int
parse_line(const char *line, char **p, const int n, const char *file, const
int line_num,
@@ -794,5 +795,6 @@
int failed = cmocka_run_group_tests_name("pkt tests", tests, NULL, NULL);
failed += run_control_msg_tests();
+ failed += run_oob_tests();
return failed ? 1 : 0;
}
--
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: 19
Gerrit-Owner: stipa <[email protected]>
Gerrit-Reviewer: flichtenheld <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-Reviewer: ralf_lici <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: ralf_lici <[email protected]>
Gerrit-Attention: stipa <[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
