From adacc16cd4536c2f34e10d65435c92ab8a241179 Mon Sep 17 00:00:00 2001 From: Lev Stipakov Date: Tue, 19 Sep 2023 14:14:53 +0200 Subject: [PATCH] push update: base implementation This adds support for parsing PUSH_UPDATE control command, which enables to update options "on the fly", without reconnect. The options presented in the PUSH_UPDATE list overwrite current options with the name. To unset an option, it has to be prefixed with the "-". For example: PUSH_UPDATE,route 10.10.10.0 255.255.255.0,-dns Replaces all existing routes with this new one and removes all "dns" options. If the client doesn't support updating certain option, it reconnects. Except when option is prefixed with "?" - in this case option is considered "optional". For example, this message PUSH_UPDATE,?unsupported_option_a does nothing, but this one: PUSH_UPDATE,dns 0,block-ipv6,unsupported_option_b makes client reconnect, since it contains mandatory unsupported option. OVPN3-1234 Signed-off-by: Lev Stipakov --- openvpn/client/cliproto.hpp | 36 +++++ openvpn/options/continuation.hpp | 212 ++++++++++++++++++++++----- openvpn/tun/client/tunbase.hpp | 2 + test/unittests/test_continuation.cpp | 85 +++++++++++ 4 files changed, 301 insertions(+), 34 deletions(-) diff --git a/openvpn/client/cliproto.hpp b/openvpn/client/cliproto.hpp index d65d4a70..af220c36 100644 --- a/openvpn/client/cliproto.hpp +++ b/openvpn/client/cliproto.hpp @@ -874,6 +874,37 @@ class Session : ProtoContextCallbackInterface, } } + /** + * @brief Handles incoming PUSH_UPDATE message + * + * @param msg Comma-separated list of options prefixed with PUSH_UPDATE tag + */ + void recv_push_update(const std::string &msg) + { + received_options.reset_completion(); + + // parse the received options + auto opt_str = msg.substr(strlen("PUSH_UPDATE,")); + auto opts = OptionList::parse_from_csv_static(opt_str, &pushed_options_limit); + + received_options.add(opts, pushed_options_filter.get(), true); + + if (received_options.complete()) + { + // show options + OPENVPN_LOG("PUSH UPDATE:\n" + << render_options_sanitized(opts, Option::RENDER_PASS_FMT | Option::RENDER_NUMBER | Option::RENDER_BRACKET)); + + // Merge local and pushed options + received_options.finalize(pushed_options_merger); + + if (tun) + { + tun->apply_push_update(received_options, *transport); + } + } + } + // proto base class calls here for app-level control-channel messages received void control_recv(BufferPtr &&app_bp) override { @@ -918,7 +949,12 @@ class Session : ProtoContextCallbackInterface, { recv_custom_control_message(msg); } + else if (string::starts_with(msg, "PUSH_UPDATE,")) + { + recv_push_update(msg); + } } + /** @brief receive, validate, and dispatch ACC messages @param msg the received message diff --git a/openvpn/options/continuation.hpp b/openvpn/options/continuation.hpp index c435d526..d6d71b48 100644 --- a/openvpn/options/continuation.hpp +++ b/openvpn/options/continuation.hpp @@ -26,6 +26,8 @@ #pragma once +#include + #include #include @@ -53,6 +55,7 @@ class OptionListContinuation : public OptionList { public: OPENVPN_SIMPLE_EXCEPTION(olc_complete); // add called when object is already complete + OPENVPN_EXCEPTION(push_update_unsupported_option); OptionListContinuation(const PushOptionsBase::Ptr &push_base_arg) : push_base(push_base_arg) @@ -63,44 +66,61 @@ class OptionListContinuation : public OptionList extend(push_base->multi, nullptr); } - OptionListContinuation() - { - } + OptionListContinuation() = default; - // call with option list fragments - void add(const OptionList &other, OptionList::FilterBase *filt) + /** + * Processes pushed list of options from PUSH_REPLY or PUSH_UPDATE. + * + * For PUSH_REPLY, all incoming options are added subject to filter. + * + * For PUSH_UPDATE, incoming options prefixed with "-" do remove current options. + * If some options cannot be updated, exception is thrown. Incoming options + * prefixed with "?" are considered optional and might be ignored if update is not + * supported. + * + * @param opts Incoming list of options + * @param filt Options filter + * @param push_update true if this is PUSH_UPDATE, false if PUSH_REPLY + */ + void add(const OptionList &other, OptionList::FilterBase *filt, bool push_update = false) { - if (!complete_) + if (complete_) { - partial_ = true; - try - { - // throws if pull-filter rejects - extend(other, filt); - } - catch (const Option::RejectedException &) - { - // remove all server pushed options on reject - clear(); - if (push_base) - extend(push_base->multi, nullptr); - throw; - } - if (!continuation(other)) - { - if (push_base) - { - // Append from base where only a single instance of each option makes sense, - // provided that option wasn't already pushed by server. - update_map(); - extend_nonexistent(push_base->singleton); - } - update_map(); - complete_ = true; - } - } - else throw olc_complete(); + } + + OptionList opts{other}; + if (push_update) + { + update(opts); + } + + partial_ = true; + try + { + // throws if pull-filter rejects + extend(opts, filt); + } + catch (const Option::RejectedException &) + { + // remove all server pushed options on reject + clear(); + if (push_base) + extend(push_base->multi, nullptr); + throw; + } + if (!continuation(opts)) + { + if (push_base) + { + // Append from base where only a single instance of each option makes sense, + // provided that option wasn't already pushed by server. + update_map(); + extend_nonexistent(push_base->singleton); + } + update_map(); + complete_ = true; + } } void finalize(const PushOptionsMerger::Ptr merger) @@ -110,6 +130,8 @@ class OptionListContinuation : public OptionList merger->merge(*this, push_base->merge); update_map(); } + + update_list.clear(); } // returns true if add() was called at least once @@ -124,7 +146,104 @@ class OptionListContinuation : public OptionList return complete_; } + /** + * @brief Resets completion flag. Intended to use by PUSH_UPDATE. + * + */ + void reset_completion() + { + complete_ = false; + } + private: + /** + * Handles PUSH_UPDATE options + * + * This method: + * - throws an exception if an option in the list doesn't support PUSH_UPDATE + * - removes an original option which is prefixed with "-" in incoming options list + * - removes an original option with the same name as incoming options list + * + * Note that options prefixed with "-" are removed from incoming options list + * + * @param opts A list of PUSH_UPDATE options. + */ + void update(OptionList &opts) + { + std::unordered_set opts_to_remove; + std::unordered_set unsupported_mandatory_options; + std::unordered_set unsupported_optional_options; + + for (auto it = opts.begin(); it != opts.end();) + { + std::string &name = it->ref(0); + + // option prefixed with "-" should be removed + bool remove = string::starts_with(name, "-"); + if (remove) + { + name.erase(name.begin()); + } + + // option prefixed with "?" is considered "optional" + bool optional = string::starts_with(name, "?"); + if (optional) + { + name.erase(name.begin()); + } + + if (updatable_options.find(name) == updatable_options.end()) + { + if (optional) + { + unsupported_optional_options.insert(name); + } + else + { + unsupported_mandatory_options.insert(name); + } + } + + if (remove) + { + // remove current option if it is prefixed with "-" in update list + opts_to_remove.insert(name); + it = opts.erase(it); + } + else + { + // if upcoming updated option is not in update list, it should be removed from current options + if (update_list.find(name) == update_list.end()) + { + opts_to_remove.insert(name); + } + + ++it; + } + } + opts.update_map(); + + erase(std::remove_if(begin(), end(), [&opts_to_remove](const Option &o) + { + const std::string &name = o.ref(0); + return opts_to_remove.find(name) != opts_to_remove.end(); }), + end()); + + // we need to remove only original options, not the ones from ongoing PUSH_UPDATE + // make sure that options are considered for removal only once + update_list.insert(opts_to_remove.begin(), opts_to_remove.end()); + + if (!unsupported_mandatory_options.empty()) + { + throw push_update_unsupported_option(string::join(unsupported_mandatory_options, ",")); + } + + if (!unsupported_optional_options.empty()) + { + OPENVPN_LOG("Unsupported optional options: " << string::join(unsupported_optional_options, ",")); + } + } + static bool continuation(const OptionList &opt) { const Option *o = opt.get_ptr("push-continuation"); @@ -135,6 +254,31 @@ class OptionListContinuation : public OptionList bool complete_ = false; PushOptionsBase::Ptr push_base; + + /** + * @brief A list of options to be updated or deleted during the update process. + * Existing options with the same name as in PUSH_UPDATE are replaced, and the ones + * prefixed with "-" in PUSH_UPDATE are deleted. + */ + std::unordered_set update_list; + + inline static std::unordered_set updatable_options = { + "block-ipv4", + "block-ipv6", + "block-outside-dns", + "dhcp-options", + "dns", + "ifconfig", + "ifconfig-ipv6", + "push-continuation", + "redirect-gateway", + "redirect-private", + "route", + "route-gateway", + "route-ipv6", + "route-metric", + "topology", + "tun-mtu"}; }; } // namespace openvpn diff --git a/openvpn/tun/client/tunbase.hpp b/openvpn/tun/client/tunbase.hpp index efb12502..64965d9e 100644 --- a/openvpn/tun/client/tunbase.hpp +++ b/openvpn/tun/client/tunbase.hpp @@ -63,6 +63,8 @@ struct TunClient : public virtual RC virtual int vpn_mtu() const = 0; virtual void adjust_mss(int mss){}; + + virtual void apply_push_update(const OptionList &, TransportClient &){}; }; // Base class for parent of tun interface object, used to diff --git a/test/unittests/test_continuation.cpp b/test/unittests/test_continuation.cpp index 2cd90b7e..dd0b57d5 100644 --- a/test/unittests/test_continuation.cpp +++ b/test/unittests/test_continuation.cpp @@ -223,3 +223,88 @@ TEST(continuation, test2) } #endif } + +TEST(continuation, push_update_add) +{ + OptionListContinuation cc; + + auto orig_opts = OptionList::parse_from_csv_static("a,b,c", nullptr); + cc.add(orig_opts, nullptr); + cc.finalize(nullptr); + + cc.reset_completion(); + + auto update = OptionList::parse_from_csv_static("dns,ifconfig", nullptr); + cc.add(update, nullptr, true); + cc.finalize(nullptr); + + ASSERT_EQ(cc.size(), 5); +} + +TEST(continuation, push_update_add_unsupported) +{ + OptionListContinuation cc; + + auto orig_opts = OptionList::parse_from_csv_static("a,b,c", nullptr); + cc.add(orig_opts, nullptr); + cc.finalize(nullptr); + + cc.reset_completion(); + + auto update = OptionList::parse_from_csv_static("my_unsupported_option,?e", nullptr); + JY_EXPECT_THROW(cc.add(update, nullptr, true), OptionListContinuation::push_update_unsupported_option, "my_unsupported_option"); + cc.finalize(nullptr); + + update = OptionList::parse_from_csv_static("?f,?g", nullptr); + cc.add(update, nullptr, true); + cc.finalize(nullptr); + + ASSERT_EQ(cc.size(), 5); +} + +TEST(continuation, push_update_remove) +{ + OptionListContinuation cc; + + auto update = OptionList::parse_from_csv_static("-my_unsupported_option", nullptr); + JY_EXPECT_THROW(cc.add(update, nullptr, true), OptionListContinuation::push_update_unsupported_option, "my_unsupported_option"); + cc.finalize(nullptr); + cc.reset_completion(); + + update = OptionList::parse_from_csv_static("-?my_unsupported_optional_option", nullptr); + cc.add(update, nullptr, true); + cc.finalize(nullptr); + cc.reset_completion(); +} + +TEST(continuation, push_update_add_multiple) +{ + OptionListContinuation cc; + + // this adds 7 options + auto orig_opts = OptionList::parse_from_csv_static("a,b,c,route 0,ifconfig,f,dns", nullptr); + cc.add(orig_opts, nullptr); + cc.finalize(nullptr); + + cc.reset_completion(); + + // after we should have 9 options + auto update = OptionList::parse_from_csv_static("route 1,route 2,-ifconfig,?bla,push-continuation 2", nullptr); + cc.add(update, nullptr, true); + + // after we should have 10 options (9 + push-continuation) + update = OptionList::parse_from_csv_static("route 3,route 4,-dns", nullptr); + cc.add(update, nullptr, true); + + cc.finalize(nullptr); + + ASSERT_TRUE(cc.exists("f")); + ASSERT_FALSE(cc.exists("dns")); + ASSERT_FALSE(cc.exists("ifconfig")); + ASSERT_TRUE(cc.exists("bla")); + + const auto &idx = cc.get_index_ptr("route"); + ASSERT_EQ(idx->size(), 4); + + ASSERT_EQ(cc.size(), 10); +}