From 87fd344e6e7526f1e9218dfb7fee54217e1afef6 Mon Sep 17 00:00:00 2001 From: James Yonan Date: Wed, 24 Oct 2012 09:32:15 +0000 Subject: [PATCH] Did some refactoring to make it easier for tun and transport objects to communicate specific errors or warnings. Added TUN_IFACE_CREATE event, which indicates an error creating the tun interface. Added REROUTE_GW_NO_DNS error stat, which indicates that redirect-gateway (IPv4) was processed without an accompanying DNS directive. --- openvpn/client/cliconnect.hpp | 10 ++++++- openvpn/client/clievent.hpp | 7 +++++ openvpn/client/cliproto.hpp | 41 ++++++++++++++++---------- openvpn/error/error.hpp | 7 +++++ openvpn/transport/client/httpcli.hpp | 22 +++++++------- openvpn/transport/client/tcpcli.hpp | 8 ++--- openvpn/transport/client/transbase.hpp | 4 +-- openvpn/transport/client/udpcli.hpp | 4 +-- openvpn/tun/builder/client.hpp | 32 +++++++++++++++----- openvpn/tun/client/tunbase.hpp | 2 +- openvpn/tun/linux/client/tuncli.hpp | 3 +- openvpn/tun/mac/client/tuncli.hpp | 3 +- 12 files changed, 96 insertions(+), 47 deletions(-) diff --git a/openvpn/client/cliconnect.hpp b/openvpn/client/cliconnect.hpp index d4f4ecb8..33208481 100644 --- a/openvpn/client/cliconnect.hpp +++ b/openvpn/client/cliconnect.hpp @@ -237,7 +237,7 @@ namespace openvpn { { switch (client->fatal()) { - case Error::SUCCESS: // doesn't necessarily mean success, just that there wasn't a fatal error + case Error::UNDEF: // means that there wasn't a fatal error queue_restart(); break; @@ -268,6 +268,14 @@ namespace openvpn { stop(); } break; + case Error::TUN_IFACE_CREATE: + { + ClientEvent::Base::Ptr ev = new ClientEvent::TunIfaceCreate(client->fatal_reason()); + client_options->events().add_event(ev); + client_options->stats().error(Error::TUN_IFACE_CREATE); + stop(); + } + break; case Error::PROXY_ERROR: { ClientEvent::Base::Ptr ev = new ClientEvent::ProxyError(client->fatal_reason()); diff --git a/openvpn/client/clievent.hpp b/openvpn/client/clievent.hpp index 47982174..da5e69ac 100644 --- a/openvpn/client/clievent.hpp +++ b/openvpn/client/clievent.hpp @@ -40,6 +40,7 @@ namespace openvpn { PROXY_NEED_CREDS, PROXY_ERROR, TUN_SETUP_FAILED, + TUN_IFACE_CREATE, EPKI_ERROR, EPKI_INVALID_ALIAS, @@ -72,6 +73,7 @@ namespace openvpn { "PROXY_NEED_CREDS", "PROXY_ERROR", "TUN_SETUP_FAILED", + "TUN_IFACE_CREATE", "EPKI_ERROR", "EPKI_INVALID_ALIAS", }; @@ -251,6 +253,11 @@ namespace openvpn { TunSetupFailed(const std::string& reason) : ReasonBase(TUN_SETUP_FAILED, reason) {} }; + struct TunIfaceCreate : public ReasonBase + { + TunIfaceCreate(const std::string& reason) : ReasonBase(TUN_IFACE_CREATE, reason) {} + }; + struct EpkiError : public ReasonBase { EpkiError(const std::string& reason) : ReasonBase(EPKI_ERROR, reason) {} diff --git a/openvpn/client/cliproto.hpp b/openvpn/client/cliproto.hpp index dcb8fb22..a69ced37 100644 --- a/openvpn/client/cliproto.hpp +++ b/openvpn/client/cliproto.hpp @@ -102,7 +102,7 @@ namespace openvpn { sent_push_request(false), cli_events(config.cli_events), connected_(false), - fatal_(Error::SUCCESS), + fatal_(Error::UNDEF), max_pushed_options(config.max_pushed_options) { #ifdef OPENVPN_PACKET_LOG @@ -162,7 +162,7 @@ namespace openvpn { bool reached_connected_state() const { return connected_; } // Fatal error means that we shouldn't retry. - // Returns a value != Error::SUCCESS if error + // Returns a value != Error::UNDEF if error Error::Type fatal() const { return fatal_; } const std::string& fatal_reason() const { return fatal_reason_; } @@ -296,28 +296,36 @@ namespace openvpn { } } - virtual void transport_error(const std::string& err) + virtual void transport_error(const Error::Type fatal_err, const std::string& err_text) { + if (fatal_err != Error::UNDEF) + { + fatal_ = fatal_err; + fatal_reason_ = err_text; + } if (notify_callback) { - OPENVPN_LOG("Transport Error: " << err); + OPENVPN_LOG("Transport Error: " << err_text); stop(true); } else - throw transport_exception(err); + throw transport_exception(err_text); } - virtual void proxy_error(const std::string& err, const bool need_creds) + virtual void proxy_error(const Error::Type fatal_err, const std::string& err_text) { - fatal_ = need_creds ? Error::PROXY_NEED_CREDS : Error::PROXY_ERROR; - fatal_reason_ = err; + if (fatal_err != Error::UNDEF) + { + fatal_ = fatal_err; + fatal_reason_ = err_text; + } if (notify_callback) { - OPENVPN_LOG("Proxy Error: " << err); + OPENVPN_LOG("Proxy Error: " << err_text); stop(true); } else - throw proxy_exception(err); + throw proxy_exception(err_text); } void extract_auth_token(const OptionList& opt) @@ -433,17 +441,20 @@ namespace openvpn { notify_callback->client_proto_connected(); } - virtual void tun_error(const std::string& err) + virtual void tun_error(const Error::Type fatal_err, const std::string& err_text) { - fatal_ = Error::TUN_SETUP_FAILED; - fatal_reason_ = err; + if (fatal_err != Error::UNDEF) + { + fatal_ = fatal_err; + fatal_reason_ = err_text; + } if (notify_callback) { - OPENVPN_LOG("TUN Error: " << err); + OPENVPN_LOG("TUN Error: " << err_text); stop(true); } else - throw tun_exception(err); + throw tun_exception(err_text); } // proto base class calls here to get auth credentials diff --git a/openvpn/error/error.hpp b/openvpn/error/error.hpp index 16af4f3d..b75fa8fe 100644 --- a/openvpn/error/error.hpp +++ b/openvpn/error/error.hpp @@ -29,6 +29,8 @@ namespace openvpn { TUN_WRITE_ERROR, // write errors on tun/tap interface TUN_FRAMING_ERROR, // error with tun PF_INET/PF_INET6 prefix TUN_SETUP_FAILED, // error setting up tun/tap interface + TUN_IFACE_CREATE, // error creating tun/tap interface + REROUTE_GW_NO_DNS, // redirect-gateway specified without alt DNS servers TCP_OVERFLOW, // TCP output queue overflow TCP_SIZE_ERROR, // bad embedded uint16_t TCP packet size TCP_CONNECT_ERROR, // client error on TCP connect @@ -49,6 +51,9 @@ namespace openvpn { PROXY_ERROR, // HTTP proxy error PROXY_NEED_CREDS, // HTTP proxy needs credentials N_ERRORS, + + // undefined error + UNDEF=SUCCESS, }; inline const char *name(const size_t type) @@ -71,6 +76,8 @@ namespace openvpn { "TUN_WRITE_ERROR", "TUN_FRAMING_ERROR", "TUN_SETUP_FAILED", + "TUN_IFACE_CREATE", + "REROUTE_GW_NO_DNS", "TCP_OVERFLOW", "TCP_SIZE_ERROR", "TCP_CONNECT_ERROR", diff --git a/openvpn/transport/client/httpcli.hpp b/openvpn/transport/client/httpcli.hpp index 412a03ee..f7ec02d9 100644 --- a/openvpn/transport/client/httpcli.hpp +++ b/openvpn/transport/client/httpcli.hpp @@ -109,7 +109,7 @@ namespace openvpn { { if (!config->http_proxy_options) { - parent.proxy_error("http_proxy_options not defined", false); + parent.proxy_error(Error::PROXY_ERROR, "http_proxy_options not defined"); return; } @@ -201,15 +201,15 @@ namespace openvpn { std::ostringstream os; os << "Transport error on '" << config->server_host << ": " << error << " (HTTP proxy)"; stop(); - parent.transport_error(os.str()); + parent.transport_error(Error::UNDEF, os.str()); } - void proxy_error(const char *what, const bool need_creds) + void proxy_error(const Error::Type fatal_err, const char *what) { std::ostringstream os; os << "on " << config->http_proxy_options->host << ':' << config->http_proxy_options->port << ": " << what; stop(); - parent.proxy_error(os.str(), need_creds); + parent.proxy_error(fatal_err, os.str()); } void tcp_read_handler(BufferAllocated& buf) // called by LinkImpl @@ -223,7 +223,7 @@ namespace openvpn { } catch (const std::exception& e) { - proxy_error(e.what(), false); + proxy_error(Error::PROXY_ERROR, e.what()); } } } @@ -242,7 +242,7 @@ namespace openvpn { } catch (const std::exception& e) { - proxy_error(e.what(), false); + proxy_error(Error::PROXY_ERROR, e.what()); } } } @@ -315,7 +315,7 @@ namespace openvpn { if (config->http_proxy_options->username.empty()) { - proxy_error("HTTP proxy requires credentials", true); + proxy_error(Error::PROXY_NEED_CREDS, "HTTP proxy requires credentials"); return; } @@ -348,7 +348,7 @@ namespace openvpn { } else { - proxy_error("credentials were not accepted", true); + proxy_error(Error::PROXY_NEED_CREDS, "HTTP proxy credentials were not accepted"); return; } } @@ -418,7 +418,7 @@ namespace openvpn { os << "DNS resolve error on '" << config->server_host << "' for TCP (HTTP proxy): " << error; config->stats->error(Error::RESOLVE_ERROR); stop(); - parent.transport_error(os.str()); + parent.transport_error(Error::UNDEF, os.str()); } } } @@ -445,7 +445,7 @@ namespace openvpn { { config->stats->error(Error::SOCKET_PROTECT_ERROR); stop(); - parent.transport_error("socket_protect error (HTTP Proxy)"); + parent.transport_error(Error::UNDEF, "socket_protect error (HTTP Proxy)"); return; } } @@ -484,7 +484,7 @@ namespace openvpn { os << "TCP connect error on '" << config->server_host << "' for TCP-via-HTTP-proxy session: " << error.message(); config->stats->error(Error::TCP_CONNECT_ERROR); stop(); - parent.transport_error(os.str()); + parent.transport_error(Error::UNDEF, os.str()); } } } diff --git a/openvpn/transport/client/tcpcli.hpp b/openvpn/transport/client/tcpcli.hpp index 32971aa1..0e2192b3 100644 --- a/openvpn/transport/client/tcpcli.hpp +++ b/openvpn/transport/client/tcpcli.hpp @@ -160,7 +160,7 @@ namespace openvpn { std::ostringstream os; os << "Transport error on '" << config->server_host << ": " << error; stop(); - parent.transport_error(os.str()); + parent.transport_error(Error::UNDEF, os.str()); } void stop_() @@ -194,7 +194,7 @@ namespace openvpn { os << "DNS resolve error on '" << config->server_host << "' for TCP session: " << error; config->stats->error(Error::RESOLVE_ERROR); stop(); - parent.transport_error(os.str()); + parent.transport_error(Error::UNDEF, os.str()); } } } @@ -210,7 +210,7 @@ namespace openvpn { { config->stats->error(Error::SOCKET_PROTECT_ERROR); stop(); - parent.transport_error("socket_protect error (TCP)"); + parent.transport_error(Error::UNDEF, "socket_protect error (TCP)"); return; } } @@ -243,7 +243,7 @@ namespace openvpn { os << "TCP connect error on '" << config->server_host << "' for TCP session: " << error.message(); config->stats->error(Error::TCP_CONNECT_ERROR); stop(); - parent.transport_error(os.str()); + parent.transport_error(Error::UNDEF, os.str()); } } } diff --git a/openvpn/transport/client/transbase.hpp b/openvpn/transport/client/transbase.hpp index 9a7f49d2..4009ae04 100644 --- a/openvpn/transport/client/transbase.hpp +++ b/openvpn/transport/client/transbase.hpp @@ -34,8 +34,8 @@ namespace openvpn { struct TransportClientParent { virtual void transport_recv(BufferAllocated& buf) = 0; - virtual void transport_error(const std::string& err) = 0; - virtual void proxy_error(const std::string& err, const bool need_creds) = 0; + virtual void transport_error(const Error::Type fatal_err, const std::string& err_text) = 0; + virtual void proxy_error(const Error::Type fatal_err, const std::string& err_text) = 0; // progress notifications virtual void transport_pre_resolve() = 0; diff --git a/openvpn/transport/client/udpcli.hpp b/openvpn/transport/client/udpcli.hpp index d576776e..bbbc1629 100644 --- a/openvpn/transport/client/udpcli.hpp +++ b/openvpn/transport/client/udpcli.hpp @@ -171,7 +171,7 @@ namespace openvpn { os << "DNS resolve error on '" << config->server_host << "' for UDP session: " << error.message(); config->stats->error(Error::RESOLVE_ERROR); stop(); - parent.transport_error(os.str()); + parent.transport_error(Error::UNDEF, os.str()); } } } @@ -188,7 +188,7 @@ namespace openvpn { { config->stats->error(Error::SOCKET_PROTECT_ERROR); stop(); - parent.transport_error("socket_protect error (UDP)"); + parent.transport_error(Error::UNDEF, "socket_protect error (UDP)"); return; } } diff --git a/openvpn/tun/builder/client.hpp b/openvpn/tun/builder/client.hpp index dde883e3..166e668a 100644 --- a/openvpn/tun/builder/client.hpp +++ b/openvpn/tun/builder/client.hpp @@ -163,6 +163,11 @@ namespace openvpn { F_IPv6=(1<<1), }; + // add_dns flags + enum { + F_ADD_DNS=(1<<0), + }; + public: virtual void client_start(const OptionList& opt, TransportClient& transcli) { @@ -184,7 +189,7 @@ namespace openvpn { { copt.reset(new TunBuilderCapture()); try { - configure_builder(copt.get(), NULL, server_addr, *config, opt, true); + configure_builder(copt.get(), NULL, NULL, server_addr, *config, opt, true); } catch (const std::exception& e) { @@ -212,14 +217,17 @@ namespace openvpn { parent.tun_pre_tun_config(); // configure the tun builder - configure_builder(tb, state.get(), server_addr, *config, opt, false); + configure_builder(tb, state.get(), config->stats.get(), server_addr, *config, opt, false); // start tun sd = tb->tun_builder_establish(); } if (sd == -1) - throw tun_builder_error("cannot acquire tun interface socket"); // NOTE: this string is quoted in OpenVPNClientBase.java + { + parent.tun_error(Error::TUN_IFACE_CREATE, "cannot acquire tun interface socket"); + return; + } // persist state if (copt && !use_persisted_tun) @@ -243,11 +251,10 @@ namespace openvpn { } catch (const std::exception& e) { - config->stats->error(Error::TUN_SETUP_FAILED); stop(); if (tun_persist) tun_persist->close(); - parent.tun_error(e.what()); + parent.tun_error(Error::TUN_SETUP_FAILED, e.what()); } } } @@ -327,6 +334,7 @@ namespace openvpn { static void configure_builder(TunBuilderBase* tb, ClientState* state, + SessionStats* stats, const IP::Addr& server_addr, const ClientConfig& config, const OptionList& opt, @@ -344,7 +352,7 @@ namespace openvpn { const bool reroute_dns = should_reroute_dns(opt, reroute_gw_ver_flags, quiet); // add DNS servers and domain prefixes - add_dns(tb, opt, reroute_dns, quiet); + const unsigned int add_dns_flags = add_dns(tb, opt, reroute_dns, quiet); // set remote server address if (!tb->tun_builder_set_remote_address(server_addr.to_string(), @@ -364,6 +372,13 @@ namespace openvpn { if (!tb->tun_builder_set_session_name(config.session_name)) throw tun_builder_error("tun_builder_set_session_name failed"); } + + // warnings + if (stats) + { + if ((reroute_gw_ver_flags & F_IPv4) && !(add_dns_flags & F_ADD_DNS)) + stats->error(Error::REROUTE_GW_NO_DNS); + } } static bool should_reroute_dns(const OptionList& opt, @@ -574,12 +589,13 @@ namespace openvpn { return reroute_gw_ver_flags; } - static void add_dns(TunBuilderBase* tb, const OptionList& opt, const bool reroute_dns, const bool quiet) + static unsigned int add_dns(TunBuilderBase* tb, const OptionList& opt, const bool reroute_dns, const bool quiet) { // Example: // [dhcp-option] [DNS] [172.16.0.23] // [dhcp-option] [DOMAIN] [openvpn.net] // [dhcp-option] [DOMAIN] [example.com] + unsigned int flags = 0; OptionList::IndexMap::const_iterator dopt = opt.map().find("dhcp-option"); // DIRECTIVE if (dopt != opt.map().end()) { @@ -596,6 +612,7 @@ namespace openvpn { ip.version() == IP::Addr::V6, reroute_dns)) throw tun_builder_dhcp_option_error("tun_builder_add_dns_server failed"); + flags |= F_ADD_DNS; } else if (type == "DOMAIN") { @@ -612,6 +629,7 @@ namespace openvpn { } } } + return flags; } boost::asio::io_service& io_service; diff --git a/openvpn/tun/client/tunbase.hpp b/openvpn/tun/client/tunbase.hpp index 67d2d843..0a6f6824 100644 --- a/openvpn/tun/client/tunbase.hpp +++ b/openvpn/tun/client/tunbase.hpp @@ -34,7 +34,7 @@ namespace openvpn { struct TunClientParent { virtual void tun_recv(BufferAllocated& buf) = 0; - virtual void tun_error(const std::string&) = 0; + virtual void tun_error(const Error::Type fatal_err, const std::string& err_text) = 0; // progress notifications virtual void tun_pre_tun_config() = 0; diff --git a/openvpn/tun/linux/client/tuncli.hpp b/openvpn/tun/linux/client/tuncli.hpp index f0ab54a5..b3e0740e 100644 --- a/openvpn/tun/linux/client/tuncli.hpp +++ b/openvpn/tun/linux/client/tuncli.hpp @@ -81,9 +81,8 @@ namespace openvpn { } catch (const std::exception& e) { - config->stats->error(Error::TUN_SETUP_FAILED); stop(); - parent.tun_error(e.what()); + parent.tun_error(Error::TUN_SETUP_FAILED, e.what()); } } } diff --git a/openvpn/tun/mac/client/tuncli.hpp b/openvpn/tun/mac/client/tuncli.hpp index 4387fe43..27afcaa4 100644 --- a/openvpn/tun/mac/client/tuncli.hpp +++ b/openvpn/tun/mac/client/tuncli.hpp @@ -75,9 +75,8 @@ namespace openvpn { } catch (const std::exception& e) { - config->stats->error(Error::TUN_SETUP_FAILED); stop(); - parent.tun_error(e.what()); + parent.tun_error(Error::TUN_SETUP_FAILED, e.what()); } } }