mirror/openvpn3
0
0
Fork 0
mirror of https://github.com/OpenVPN/openvpn3.git synced 2024-09-20 04:02:15 +02:00
Code

Improve dealing with unknown options

Browse Source 
This commit add several improvements to dealing with unknown options
in client configuration files:
 - implement ignore-unknown-option
 - categorise the OpenVPN2 options in multiple categories and
   warn/error out depending on the category
 - error out when unsupported/unknown options are found. This avoids
   problems like with --tls-crypt/--tls-crypt-v2 before where client
   would ignore these options and not connect at all

Signed-off-by: Arne Schwabe <arne@openvpn.net>
This commit is contained in:
Arne Schwabe 2022-08-30 18:03:52 +02:00 committed by David Sommerseth
parent ef6dbd250d
commit 5b5d80fad3
No known key found for this signature in database
GPG Key ID: 86CF944C9671FDF2
5 changed files with 565 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

357
openvpn/client/cliopt.hpp
View File

@ -29,6 +29,7 @@
#define OPENVPN_CLIENT_CLIOPT_H
#include <string>
#include <unordered_set>
#include <openvpn/error/excode.hpp>
@ -105,7 +106,7 @@
#endif
#ifndef OPENVPN_UNUSED_OPTIONS
#define OPENVPN_UNUSED_OPTIONS "UNUSED OPTIONS"
#define OPENVPN_UNUSED_OPTIONS "UNKNOWN/UNSUPPORTED OPTIONS"
#endif
namespace openvpn {
@ -316,13 +317,7 @@ namespace openvpn {
http_proxy_options->proxy_server_set_enable_cache(config.tun_persist);
}
// secret option not supported
if (opt.exists("secret"))
throw option_error("sorry, static key encryption mode (non-SSL/TLS) is not supported");
// fragment option not supported
if (opt.exists("fragment"))
throw option_error("sorry, 'fragment' directive is not supported, nor is connecting to a server that uses 'fragment' directive");
check_for_incompatible_options(opt);
#ifdef OPENVPN_PLATFORM_UWP
// workaround for OVPN3-62 Busy loop in win_event.hpp
@ -571,8 +566,350 @@ namespace openvpn {
}
}
// show unused options
opt.show_unused_options(OPENVPN_UNUSED_OPTIONS);
handle_unused_options(opt);
}
void check_for_incompatible_options(const OptionList &opt) {
// secret option not supported
if (opt.exists("secret"))
throw option_error("sorry, static key encryption mode (non-SSL/TLS) is not supported");
// fragment option not supported
if (opt.exists("fragment"))
throw option_error("sorry, 'fragment' directive is not supported, nor is connecting to a server that uses 'fragment' directive");
// Only p2p mode accept
if (opt.exists("mode"))
{
auto mode = opt.get("mode");
if(mode.size() != 1 || mode.get(1, 128) != "p2p")
{
throw option_error("Only 'mode p2p' supported");
}
}
}
std::unordered_set<std::string> settings_ignoreWithWarning = {
"allow-compression", /* TODO: maybe check against our client option compression setting? */
"allow-recursive-routing",
"auth-nocache",
"auth-retry",
"compat-mode",
"connect-retry",
"connect-retry-max",
"connect-timeout", /* TODO: this should be really implemented */
"data-ciphers", /* TODO: maybe add more special warning that checks it against our supported ciphers */
"data-ciphers-fallback",
"disable-dco", /* TODO: maybe throw an error if DCO is active? */
"disable-occ",
"engine",
"explicit-exit-notify", /* ignoring it in config does not break connection or functionality */
"group",
"ifconfig-nowarn", /* v3 does not do OCC checks */
"ip-win32",
"keepalive", /* A push only feature (ping/ping-restart) in v3. Ignore with warning since often present in configs too */
"link-mtu",
"machine-readable-output", /* would be set by a CliOptions */
"mark", /* enables SO_MARK */
"mute",
"ncp-ciphers",
"nice",
"opt-verify",
"passtos",
"persist-key",
"persist-tun",
"preresolve",
"providers", /* Done via client options */
"remap-usr1",
"reneg-bytes",
"reneg-pkts",
"replay-window",
"resolv-retry",
"route-method", /* Windows specific fine tuning option */
"show-net-up",
"socket-flags",
"suppress-timestamps", /* harmless to ignore */
"tcp-nodelay",
"tls-version-max", /* We don't allow restricting max version */
"udp-mtu", /* Alias for link-mtu */
"user",
};
std::unordered_set<std::string> settings_serverOnlyOptions = {
"auth-gen-token",
"auth-gen-token-secret",
"auth-user-pass-optional",
"auth-user-pass-verify",
"bcast-buffers",
"ccd-exclusive",
"client-config-dir",
"client-connect",
"client-disconnect",
"client-to-client",
"connect-freq",
"dh",
"disable",
"duplicate-cn",
"hash-size",
"ifconfig-ipv6-pool",
"ifconfig-pool",
"ifconfig-pool-persist",
"ifconfig-push",
"ifconfig-push-constraint",
"iroute",
"iroute-ipv6",
"max-clients",
"max-routes-per-client",
"push",
"push-remove",
"push-reset",
"server",
"server-bridge",
"server-ipv6",
"stale-routes-check",
"tls-crypt-v2-verify",
"username-as-common-name",
"verify-client-cert",
"vlan-accept",
"vlan-pvid",
"vlan-tagging",
};
/* Features not implemented and not safe to ignore */
std::unordered_set<std::string> settings_feature_not_implemented_fatal = {
"askpass",
"capath",
"cd",
"chroot",
"client-nat",
"cryptoapicert",
"daemon",
"daemon",
"errors-to-stderr",
"gremlin",
"lladdr",
"log",
"log",
"log-append",
"management",
"memstats",
"msg-channel", /* (Windows service in v2) */
"ping-timer-rem",
"single-session", /* This option is quite obscure but changes behaviour enough to not ignore it */
"socks-proxy",
"status",
"status-version",
"syslog",
"tls-server", /* No p2p mode in v3 */
"tun-mtu-extra", /*(only really used in tap in OpenVPN 2.x)*/
"verify-hash",
"win-sys",
"writepid",
"x509-username-field",
};
/* Features not implemented but safe enough to ignore */
std::unordered_set<std::string> settings_feature_not_implemented_warn = {
"allow-pull-fqdn",
"bind",
"local",
"lport",
"mlock",
"mtu-disc",
"mtu-test",
"persist-local-ip",
"persist-remote-ip",
"shaper",
"tls-exit",
};
/* Push only options (some are allowed in the config in OpenVPN 2
* but really push only options) */
std::unordered_set<std::string> settings_pushonlyoptions = {
"auth-token",
"auth-token-user",
"echo",
"parameter",
"ping",
"ping-exit",
"ping-restart", /* ping related options are pull only in v3, v2 needs them in the config for pure p2p */
"key-derivation",
"peer-id",
"protocol-flags",
};
/* Features related to scripts/plugins */
std::unordered_set<std::string> settings_script_plugin_feature = {
"down",
"down-pre",
"ifconfig-noexec",
"ipchange",
"learn-address",
"plugin",
"route-noexec",
"route-pre-down",
"route-up",
"setenv-safe",
"tls-export-cert",
"tls-verify",
"up",
"up-delay",
"x509-track"};
/* Standalone OpenVPN v2 modes */
std::unordered_set<std::string> settings_standalone_options = {
"genkey",
"mktun",
"rmtun",
"show-ciphers",
"show-curves",
"show-digests",
"show-engines",
"show-groups",
"show-tls",
"test-crypto"};
/* Deprecated/throwing error in OpenVPN 2.x already: */
std::unordered_set<std::string> settings_removedOptions = {
"mtu-dynamic", "no-replay", "no-name-remapping", "compat-names", "ncp-disable"};
std::unordered_set<std::string> settings_ignoreSilently = {
"ecdh-curve", /* Deprecated in v2, not needed with modern OpenSSL */
"fast-io",
"max-routes",
"mute-replay-warnings",
"nobind", /* only behaviour in v3 client anyway */
"prng",
"rcvbuf", /* present in many configs */
"replay-persist", /* Makes little sense in TLS mode */
"script-security",
"sndbuf",
"tls-client", /* Always enabled */
"tmp-dir",
"tun-ipv6", /* ignored in v2 as well */
"txqueuelen", /* so platforms evaluate that in tun, some do not, do not warn about that */
"verb"};
/**
* This groups all the options that OpenVPN 2.x supports that the
* OpenVPN v3 client does not support into a number of different groups
* and warns or errors out if with a specific message for that group.
*
* If any option that is not touched() after going through all groups
* the function will print them as unknown unsupported option(s) and
* error out
*/
void handle_unused_options(const OptionList &opt)
{
/* Meta options that AS profiles often have that we do not parse and
* can ignore without warning */
std::unordered_set<std::string> ignoreMetaOptions = {
"CLI_PREF_ALLOW_WEB_IMPORT",
"CLI_PREF_BASIC_CLIENT",
"CLI_PREF_ENABLE_CONNECT",
"CLI_PREF_ENABLE_XD_PROXY",
"WSHOST",
"WEB_CA_BUNDLE",
"IS_OPENVPN_WEB_CA",
"OVPN_ACCESS_SERVER_NO_WEB",
};
std::unordered_set<std::string> ignore_unknown_option_list;
if (opt.exists("ignore-unknown-option"))
{
auto igOptlist = opt.get_index("ignore-unknown-option");
for (auto igUnOptIdx : igOptlist)
{
const Option &o = opt[igUnOptIdx];
for (size_t i = 1; i < o.size(); i++)
{
auto optionToIgnore = o.get(i, 0);
ignore_unknown_option_list.insert(optionToIgnore);
}
o.touch();
}
}
for (const auto &o : opt)
{
if (!o.meta() && settings_ignoreSilently.find(o.get(0, 0)) != settings_ignoreSilently.end())
{
o.touch();
}
if (o.meta() && ignoreMetaOptions.find(o.get(0, 0)) != ignoreMetaOptions.end())
{
o.touch();
}
}
/* Mark all options that will not trigger any kind of message
* as touched to avoid an empty message with unused options */
if (opt.n_unused() == 0)
return;
OPENVPN_LOG_NTNL("NOTE: This configuration contains options that were not used:" << std::endl);
/* Go through all options and check all options that have not been
* touched (parsed) yet */
showUnusedOptionsByList(opt, settings_removedOptions, "Removed deprecated option", true);
showUnusedOptionsByList(opt, settings_serverOnlyOptions, "Server only option", true);
showUnusedOptionsByList(opt, settings_standalone_options, "OpenVPN 2.x command line operation", true);
showUnusedOptionsByList(opt, settings_feature_not_implemented_warn, "Feature not implemented (option ignored)", false);
showUnusedOptionsByList(opt, settings_pushonlyoptions, "Option allowed only to be pushed by the server", true);
showUnusedOptionsByList(opt, settings_feature_not_implemented_warn, "feature not implemented/available", false);
showUnusedOptionsByList(opt, settings_script_plugin_feature, "Ignored (no script/plugin support)", false);
showUnusedOptionsByList(opt, ignore_unknown_option_list, "Ignored by option 'ignore-unknown-option'", false);
showUnusedOptionsByList(opt, settings_ignoreWithWarning, "Unsupported option (ignored)", false);
auto ignoredBySetenvOpt = [](const Option &option) { return !option.touched() && option.warnonlyunknown(); };
showOptionsByFunction(opt, ignoredBySetenvOpt, "Ignored options prefixed with 'setenv opt'", false);
auto unusedMetaOpt = [](const Option &option) { return !option.touched() && option.meta(); };
showOptionsByFunction(opt, unusedMetaOpt, "Unused ignored meta options", false);
auto managmentOpt = [](const Option &option) { return !option.touched() && option.get(0, 0).rfind("management", 0) == 0; };
showOptionsByFunction(opt, managmentOpt, "OpenVPN management interface is not supported by this client", true);
// If we still have options that are unaccounted for, we print them and throw an error
auto nonTouchedOptions = [](const Option &option) { return !option.touched(); };
showOptionsByFunction(opt, nonTouchedOptions, OPENVPN_UNUSED_OPTIONS, true);
}
void showUnusedOptionsByList(const OptionList &optlist, std::unordered_set<std::string> option_set, const std::string &message, bool fatal)
{
auto func = [&option_set](const Option &opt) { return !opt.touched() && option_set.find(opt.get(0, 0)) != option_set.end(); };
showOptionsByFunction(optlist, func, message, fatal);
}
/* lambda expression that capture variables have complex signatures, avoid these by letting the compiler
* itself figure it out with a template */
template <typename T>
void showOptionsByFunction(const OptionList &opt, T func, const std::string &message, bool fatal)
{
bool messageShown = false;
for (size_t i = 0; i < opt.size(); ++i)
{
auto &o = opt[i];
if (func(o))
{
if (!messageShown)
{
OPENVPN_LOG(message);
messageShown = true;
}
o.touch();
OPENVPN_LOG_NTNL(std::to_string(i) << ' ' << o.render(Option::RENDER_BRACKET | Option::RENDER_TRUNC_64) << std::endl);
}
}
if (fatal && messageShown)
{
throw option_error("sorry, unsupported options present in configuration: " + message);
}
}
static PeerInfo::Set::Ptr build_peer_info(const Config& config, const ParseClientConfig& pcc, const bool autologin_sessions)

3
openvpn/client/cliopthelper.hpp
View File

@ -638,7 +638,10 @@ namespace openvpn {
{
Option& o = *i;
if (o.size() >= 3 && o.ref(0) == "setenv" && o.ref(1) == "opt")
{
o.remove_first(2);
o.enableWarnOnly();
}
}
}

61
openvpn/common/options.hpp
View File

@ -321,12 +321,18 @@ namespace openvpn {
return out.str();
}
void clear() {
data.clear();
touched_ = false;
warn_only_if_unknown_ = false;
meta_ = false;
}
// delegate to data
size_t size() const { return data.size(); }
bool empty() const { return data.empty(); }
void push_back(const std::string& item) { data.push_back(item); }
void push_back(std::string&& item) { data.push_back(std::move(item)); }
void clear() { data.clear(); }
void reserve(const size_t n) { data.reserve(n); }
void resize(const size_t n) { data.resize(n); }
@ -354,6 +360,16 @@ namespace openvpn {
touched_ = true;
}
void enableWarnOnly()
{
warn_only_if_unknown_ = true;
}
bool warnonlyunknown() const
{
return warn_only_if_unknown_;
}
// was this option processed?
bool touched() const { return touched_; }
@ -370,7 +386,25 @@ namespace openvpn {
return ret;
}
private:
/**
* Marks this option as parsed from a meta options like
* # OVPN_ACCESS_SERVER_USERNAME=username
*/
void set_meta(bool value = true) {
meta_ = value;
}
/**
* This option has been parsed from a meta options like
* # OVPN_ACCESS_SERVER_USERNAME=username
* @return
*/
bool meta() const
{
return meta_;
}
private:
void from_list(std::string arg)
{
push_back(std::move(arg));
@ -412,6 +446,8 @@ namespace openvpn {
}
volatile mutable bool touched_ = false;
bool warn_only_if_unknown_ = false;
bool meta_ = false;
std::vector<std::string> data;
};
@ -882,6 +918,7 @@ namespace openvpn {
lim->add_opt();
lim->validate_directive(multiline);
}
multiline.set_meta(true);
push_back(std::move(multiline));
multiline.clear();
in_multiline = false;
@ -949,6 +986,7 @@ namespace openvpn {
lim->add_opt();
lim->validate_directive(multiline);
}
multiline.set_meta(true);
push_back(std::move(multiline));
multiline.clear();
in_multiline = false;
@ -981,6 +1019,7 @@ namespace openvpn {
lim->add_opt();
lim->validate_directive(opt);
}
opt.set_meta(true);
push_back(std::move(opt));
}
}
@ -1374,18 +1413,32 @@ namespace openvpn {
// Return number of unused options based on the notion that
// all used options have been touched.
size_t n_unused() const
size_t n_unused(bool ignore_meta=false) const
{
size_t n = 0;
for (std::vector<Option>::const_iterator i = begin(); i != end(); ++i)
{
const Option& opt = *i;
if (!opt.touched())
if (!opt.touched() && !(opt.meta() && ignore_meta))
++n;
}
return n;
}
// Return number of unused meta options based on the notion that
// all used options have been touched.
size_t meta_unused() const
{
size_t n = 0;
for (std::vector<Option>::const_iterator i = begin(); i != end(); ++i)
{
const Option& opt = *i;
if (opt.meta() && !opt.touched())
++n;
}
return n;
}
void show_unused_options(const char *title=nullptr) const
{
// show unused options

4
test/unittests/CMakeLists.txt
View File

@ -61,8 +61,7 @@ add_executable(coreUnitTests
test_peer_fingerprint.cpp
test_safestr.cpp
test_dns.cpp
test_header_deps.cpp
test_header_deps.cpp
test_capture.cpp
test_cleanup.cpp
test_crypto_hashstr.cpp
@ -83,6 +82,7 @@ add_executable(coreUnitTests
test_typeindex.cpp
test_validatecreds.cpp
test_weak.cpp
test_cliopt.cpp
)
if (${USE_MBEDTLS})

156
test/unittests/test_cliopt.cpp Normal file
View File

@ -0,0 +1,156 @@
#include <iostream>
#include "test_common.h"
/* if initproces.hpp is not included, mingw/windows compilation fails with
* weird stack struct creation related errors in OpenSSL */
#include <openvpn/init/initprocess.hpp>
#include <openvpn/time/asiotimer.hpp>
#include <client/ovpncli.hpp>
#include <openvpn/client/cliopt.hpp>
using namespace openvpn;
/* The config parser checks for valid certificates, provide valid ones */
std::string dummysecp256cert = "-----BEGIN CERTIFICATE-----\n"
"MIIBETCBuAIJAImY2B4ODlQuMAoGCCqGSM49BAMCMBExDzANBgNVBAMMBnNlcnZl\n"
"cjAeFw0yMjA4MzAxNTA3NDJaFw0zMjA4MjcxNTA3NDJaMBExDzANBgNVBAMMBnNl\n"
"cnZlcjBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABDwU0GWKxTxYXP/L448OlaQr\n"
"fhF2p83eg/55LJB7Aiq7xckQImGa3w2heo01hFQXQ/4mK3wsLZr7ZZl7IDC4hhMw\n"
"CgYIKoZIzj0EAwIDSAAwRQIhAKDmwivsD4qjRtbaXmUNc3src6oFOCus32ZRZw0p\n"
"Oz9zAiBZ47YdsJ985ID5COg1+nCKk+0d7jWjICbPcODHyzH4fg==\n"
"-----END CERTIFICATE-----\n";
std::string dummysecp256key = "-----BEGIN PRIVATE KEY-----\n"
"MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgbzZUYL0jZM05vB2O\n"
"kIKcA1OxSKw9ZVQ8UnlUCf6l/8ChRANCAAQ8FNBlisU8WFz/y+OPDpWkK34RdqfN\n"
"3oP+eSyQewIqu8XJECJhmt8NoXqNNYRUF0P+Jit8LC2a+2WZeyAwuIYT\n"
"-----END PRIVATE KEY-----\n";
std::string minimalConfig = "remote wooden.box\n"
"<ca>\n"
+ dummysecp256cert + "</ca>\n"
+ "<cert>\n"
+ dummysecp256cert + "</cert>\n"
+ "<key>\n" + dummysecp256key
+ "</key>\n";
void load_client_config(const std::string &config_content)
{
OptionList options;
ClientOptions::Config config;
ClientAPI::OpenVPNClientHelper client_helper;
ParseClientConfig conf = ParseClientConfig::parse(config_content);
auto parsed_config = ParseClientConfig::parse(config_content, nullptr, options);
ClientOptions cliopt(options, config);
}
TEST(config, missingRequiredOption)
{
ParseClientConfig conf = ParseClientConfig::parse("mode server");
EXPECT_EQ(conf.error(), true);
EXPECT_EQ(conf.message(), "ERR_PROFILE_OPTION: option_error: remote option not specified");
}
TEST(config, parse_missing_option)
{
OVPN_EXPECT_THROW(
load_client_config(std::string("remote wooden.box\n") + "mode server"
+ "\n<ca>\n" + dummysecp256cert + "</ca>\n"),
option_error,
"option 'cert' not found");
}
TEST(config, parse_forbidden_option)
{
OVPN_EXPECT_THROW(
load_client_config(minimalConfig + "mode server"),
option_error,
"Only 'mode p2p' supported");
OVPN_EXPECT_THROW(
load_client_config(minimalConfig + "fragment"),
option_error,
"sorry, 'fragment' directive is not supported");
}
TEST(config, parse_unknown_option)
{
OVPN_EXPECT_THROW(
load_client_config(minimalConfig + "bikeshed-color green"),
option_error,
"sorry, unsupported options present in configuration: UNKNOWN/UNSUPPORTED OPTIONS");
}
TEST(config, parse_ignore_unkown)
{
/* Bikeshed colour is ignored should throw no error */
load_client_config(minimalConfig + "ignore-unknown-option bikeshed-colour bikeshed-color\n"
"ignore-unknown-option danish axe phk\n"
"bikeshed-colour green");
load_client_config(minimalConfig + "setenv opt bikeshed-paint silver with sparkling");
}
TEST(config, ignore_warning_option)
{
load_client_config(minimalConfig + "tun-ipv6\n");
load_client_config(minimalConfig + "opt-verify\n");
}
TEST(config, parse_management)
{
OVPN_EXPECT_THROW(
load_client_config(minimalConfig + "management-is-blue"),
option_error,
"OpenVPN management interface is not supported by this client");
OVPN_EXPECT_THROW(
load_client_config(minimalConfig + "management"),
option_error,
"OpenVPN management interface is not supported by this client");
}
TEST(config, duplicate_options_sets)
{
/* Do the whole dance to get a ClientOption object to access the list */
OptionList options;
ClientOptions::Config config;
ClientAPI::OpenVPNClientHelper client_helper;
ParseClientConfig conf = ParseClientConfig::parse(minimalConfig);
auto parsed_config = ParseClientConfig::parse(minimalConfig, nullptr, options);
ClientOptions cliopt(options, config);
std::vector<std::unordered_set<std::string>> allsets = {
cliopt.settings_feature_not_implemented_fatal,
cliopt.settings_feature_not_implemented_warn,
cliopt.settings_ignoreSilently,
cliopt.settings_ignoreWithWarning,
cliopt.settings_pushonlyoptions,
cliopt.settings_removedOptions,
cliopt.settings_serverOnlyOptions,
cliopt.settings_standalone_options};
std::unordered_set<std::string> allOptions;
for (auto set : allsets)
{
for (auto optname : set)
{
/* Use an expection instead of an assert to get the name of the option
* that is a duplicate */
if (!(allOptions.find(optname) == allOptions.end()))
throw std::runtime_error("duplicate element: " + optname);
allOptions.insert(optname);
}
}
}