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/+/1745?usp=email
to look at the new patch set (#20).
Change subject: oob: Add client probe reply parser
......................................................................
oob: Add client probe reply parser
Add oob_probe_reply_find(), the client-side counterpart of
oob_probe_request_find(): it scans a received OOB message for the probe
reply TLV, skipping other and future TLV types that are marked
optional. The reply's request_id names the probe it answers, with
which the client matches the reply to its probe.
Exercised by unit tests; the client probe path calls it in a follow-up.
Change-Id: If04ce09d4c353f5384c0c48f48f63e05869a373f
Signed-off-by: Lev Stipakov <[email protected]>
---
M src/openvpn/oob.c
M src/openvpn/oob.h
M tests/unit_tests/openvpn/test_oob.c
3 files changed, 123 insertions(+), 0 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/45/1745/20
diff --git a/src/openvpn/oob.c b/src/openvpn/oob.c
index 954694a..5bd3ea6 100644
--- a/src/openvpn/oob.c
+++ b/src/openvpn/oob.c
@@ -93,6 +93,14 @@
}
bool
+oob_probe_reply_find(struct buffer *payload, struct oob_probe_reply *reply)
+{
+ struct buffer value;
+ return ctrl_msg_find_tlv(payload, TLV_TYPE_PROBE_REPLY, &value)
+ && oob_probe_reply_read(&value, reply);
+}
+
+bool
oob_timestamp_in_window(uint64_t probe_ts, uint64_t now, uint64_t window_secs)
{
uint64_t diff = (now > probe_ts) ? (now - probe_ts) : (probe_ts - now);
diff --git a/src/openvpn/oob.h b/src/openvpn/oob.h
index 95b3afc..a388e61 100644
--- a/src/openvpn/oob.h
+++ b/src/openvpn/oob.h
@@ -111,6 +111,16 @@
bool oob_probe_request_find(struct buffer *payload, struct oob_probe_request
*req);
/**
+ * Scan the payload of a received OOB message for the probe reply TLV; the
+ * client-side counterpart of oob_probe_request_find().
+ *
+ * @param payload buffer positioned at the start of the OOB message payload
+ * @param reply filled with the parsed probe reply on success
+ * @return true if a well-formed probe reply was found, false otherwise
+ */
+bool oob_probe_reply_find(struct buffer *payload, struct oob_probe_reply
*reply);
+
+/**
* Check whether a probe timestamp is within an acceptable window around the
* current time. Used to cheaply drop replayed or implausibly-timed probes
* before doing any further work (see the probe request timestamp rationale
diff --git a/tests/unit_tests/openvpn/test_oob.c
b/tests/unit_tests/openvpn/test_oob.c
index 8af595d..f1e64b7 100644
--- a/tests/unit_tests/openvpn/test_oob.c
+++ b/tests/unit_tests/openvpn/test_oob.c
@@ -527,6 +527,106 @@
gc_free(&gc);
}
+/* A payload of just a probe reply is found by the client scan, with all
+ * fields surviving. */
+static void
+test_probe_reply_find(void **state)
+{
+ struct gc_arena gc = gc_new();
+ struct buffer buf = alloc_buf_gc(128, &gc);
+
+ struct oob_probe_reply in = {
+ .request_id = 7,
+ .priority = 5,
+ .weight = 50,
+ .max_latency_diff = 25,
+ .connect_lifetime = 120,
+ .flags = 1,
+ };
+ assert_true(oob_probe_reply_write(&buf, &in));
+
+ struct oob_probe_reply out = { 0 };
+ assert_true(oob_probe_reply_find(&buf, &out));
+ assert_int_equal(out.request_id, in.request_id);
+ assert_int_equal(out.priority, in.priority);
+ assert_int_equal(out.weight, in.weight);
+ assert_int_equal(out.max_latency_diff, in.max_latency_diff);
+ assert_int_equal(out.connect_lifetime, in.connect_lifetime);
+ assert_int_equal(out.flags, in.flags);
+
+ gc_free(&gc);
+}
+
+/* TLVs other than the probe reply are skipped. */
+static void
+test_probe_reply_find_skips_unknown(void **state)
+{
+ struct gc_arena gc = gc_new();
+ struct buffer buf = alloc_buf_gc(128, &gc);
+
+ assert_true(ctrl_msg_tlv_write_header(&buf, 0x7ff, true, 4));
+ assert_true(buf_write_u32(&buf, 0xabad1dea));
+ struct oob_probe_reply in = { .priority = 7 };
+ assert_true(oob_probe_reply_write(&buf, &in));
+
+ struct oob_probe_reply out = { 0 };
+ assert_true(oob_probe_reply_find(&buf, &out));
+ assert_int_equal(out.priority, 7);
+
+ gc_free(&gc);
+}
+
+/* As on the probe side, a mandatory TLV we do not understand -- here trailing
+ * the probe reply -- invalidates the reply. */
+static void
+test_probe_reply_find_rejects_unknown_mandatory(void **state)
+{
+ struct gc_arena gc = gc_new();
+ struct buffer buf = alloc_buf_gc(128, &gc);
+
+ struct oob_probe_reply in = { .priority = 7 };
+ assert_true(oob_probe_reply_write(&buf, &in));
+ assert_true(ctrl_msg_tlv_write_header(&buf, 0x7ff, false, 4));
+ assert_true(buf_write_u32(&buf, 0xabad1dea));
+
+ struct oob_probe_reply out = { 0 };
+ assert_false(oob_probe_reply_find(&buf, &out));
+
+ gc_free(&gc);
+}
+
+/* A payload with no probe reply is rejected. */
+static void
+test_probe_reply_find_missing(void **state)
+{
+ struct gc_arena gc = gc_new();
+ struct buffer buf = alloc_buf_gc(128, &gc);
+
+ assert_true(ctrl_msg_tlv_write_header(&buf, 0x7ff, true, 4));
+ assert_true(buf_write_u32(&buf, 0));
+
+ struct oob_probe_reply out = { 0 };
+ assert_false(oob_probe_reply_find(&buf, &out));
+
+ gc_free(&gc);
+}
+
+/* Likewise, a payload carrying a probe request instead holds no probe reply.
*/
+static void
+test_probe_reply_find_request_tlv(void **state)
+{
+ struct gc_arena gc = gc_new();
+ struct buffer buf = alloc_buf_gc(128, &gc);
+
+ const struct oob_probe_request in = { .request_id = 7 };
+ assert_true(oob_probe_request_write(&buf, &in));
+
+ struct oob_probe_reply out = { 0 };
+ assert_false(oob_probe_reply_find(&buf, &out));
+
+ gc_free(&gc);
+}
+
/* The OOB tests run as a second group of pkt_testdriver; see test_pkt.c. */
int
run_oob_tests(void)
@@ -552,6 +652,11 @@
cmocka_unit_test(test_probe_request_check_valid),
cmocka_unit_test(test_probe_request_check_stale),
cmocka_unit_test(test_probe_request_check_no_request),
+ cmocka_unit_test(test_probe_reply_find),
+ cmocka_unit_test(test_probe_reply_find_skips_unknown),
+ cmocka_unit_test(test_probe_reply_find_rejects_unknown_mandatory),
+ cmocka_unit_test(test_probe_reply_find_missing),
+ cmocka_unit_test(test_probe_reply_find_request_tlv),
};
return cmocka_run_group_tests_name("oob tests", tests, NULL, NULL);
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1745?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: If04ce09d4c353f5384c0c48f48f63e05869a373f
Gerrit-Change-Number: 1745
Gerrit-PatchSet: 20
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
