From a3b0284837c9ea10865e6ddeb7f1244d621ae5c0 Mon Sep 17 00:00:00 2001 From: Lee Clagett Date: Mon, 11 Mar 2019 22:01:03 -0400 Subject: [PATCH 01/11] Change SSL certificate file list to OpenSSL builtin load_verify_location Specifying SSL certificates for peer verification does an exact match, making it a not-so-obvious alias for the fingerprints option. This changes the checks to OpenSSL which loads concatenated certificate(s) from a single file and does a certificate-authority (chain of trust) check instead. There is no drop in security - a compromised exact match fingerprint has the same worse case failure. There is increased security in allowing separate long-term CA key and short-term SSL server keys. This also removes loading of the system-default CA files if a custom CA file or certificate fingerprint is specified. --- .../epee/include/net/abstract_tcp_server2.h | 6 +- .../epee/include/net/abstract_tcp_server2.inl | 8 +- contrib/epee/include/net/http_client.h | 12 +-- .../epee/include/net/http_server_impl_base.h | 4 +- contrib/epee/include/net/net_helper.h | 6 +- contrib/epee/include/net/net_ssl.h | 7 +- contrib/epee/src/net_ssl.cpp | 98 +++++++++---------- src/rpc/core_rpc_server.cpp | 24 ++--- src/rpc/core_rpc_server.h | 2 +- src/wallet/wallet2.cpp | 31 ++---- src/wallet/wallet2.h | 4 +- src/wallet/wallet_rpc_server.cpp | 20 +--- src/wallet/wallet_rpc_server_commands_defs.h | 4 +- 13 files changed, 92 insertions(+), 134 deletions(-) diff --git a/contrib/epee/include/net/abstract_tcp_server2.h b/contrib/epee/include/net/abstract_tcp_server2.h index ec08c0f4b..f28b1fb7d 100644 --- a/contrib/epee/include/net/abstract_tcp_server2.h +++ b/contrib/epee/include/net/abstract_tcp_server2.h @@ -228,8 +228,8 @@ namespace net_utils std::map server_type_map; void create_server_type_map(); - bool init_server(uint32_t port, const std::string address = "0.0.0.0", epee::net_utils::ssl_support_t ssl_support = epee::net_utils::ssl_support_t::e_ssl_support_autodetect, const std::pair &private_key_and_certificate_path = std::make_pair(std::string(), std::string()), const std::list &allowed_certificates = {}, const std::vector> &allowed_fingerprints = {}, bool allow_any_cert = false); - bool init_server(const std::string port, const std::string& address = "0.0.0.0", epee::net_utils::ssl_support_t ssl_support = epee::net_utils::ssl_support_t::e_ssl_support_autodetect, const std::pair &private_key_and_certificate_path = std::make_pair(std::string(), std::string()), const std::list &allowed_certificates = {}, const std::vector> &allowed_fingerprints = {}, bool allow_any_cert = false); + bool init_server(uint32_t port, const std::string address = "0.0.0.0", epee::net_utils::ssl_support_t ssl_support = epee::net_utils::ssl_support_t::e_ssl_support_autodetect, const std::pair &private_key_and_certificate_path = std::make_pair(std::string(), std::string()), const std::string &ca_file = {}, const std::vector> &allowed_fingerprints = {}, bool allow_any_cert = false); + bool init_server(const std::string port, const std::string& address = "0.0.0.0", epee::net_utils::ssl_support_t ssl_support = epee::net_utils::ssl_support_t::e_ssl_support_autodetect, const std::pair &private_key_and_certificate_path = std::make_pair(std::string(), std::string()), const std::string &ca_file = {}, const std::vector> &allowed_fingerprints = {}, bool allow_any_cert = false); /// Run the server's io_service loop. bool run_server(size_t threads_count, bool wait = true, const boost::thread::attributes& attrs = boost::thread::attributes()); @@ -382,8 +382,6 @@ namespace net_utils std::set connections_; ssl_context_t m_ssl_context; - std::list m_allowed_certificates; - }; // class <>boosted_tcp_server diff --git a/contrib/epee/include/net/abstract_tcp_server2.inl b/contrib/epee/include/net/abstract_tcp_server2.inl index 67c63cca5..e8b5b6c1e 100644 --- a/contrib/epee/include/net/abstract_tcp_server2.inl +++ b/contrib/epee/include/net/abstract_tcp_server2.inl @@ -941,14 +941,14 @@ PRAGMA_WARNING_DISABLE_VS(4355) } //--------------------------------------------------------------------------------- template - bool boosted_tcp_server::init_server(uint32_t port, const std::string address, epee::net_utils::ssl_support_t ssl_support, const std::pair &private_key_and_certificate_path, const std::list &allowed_certificates, const std::vector> &allowed_fingerprints, bool allow_any_cert) + bool boosted_tcp_server::init_server(uint32_t port, const std::string address, epee::net_utils::ssl_support_t ssl_support, const std::pair &private_key_and_certificate_path, const std::string &ca_file, const std::vector> &allowed_fingerprints, bool allow_any_cert) { TRY_ENTRY(); m_stop_signal_sent = false; m_port = port; m_address = address; if (ssl_support != epee::net_utils::ssl_support_t::e_ssl_support_disabled) - m_ssl_context = create_ssl_context(private_key_and_certificate_path, allowed_certificates, allowed_fingerprints, allow_any_cert); + m_ssl_context = create_ssl_context(private_key_and_certificate_path, ca_file, allowed_fingerprints, allow_any_cert); // Open the acceptor with the option to reuse the address (i.e. SO_REUSEADDR). boost::asio::ip::tcp::resolver resolver(io_service_); boost::asio::ip::tcp::resolver::query query(address, boost::lexical_cast(port), boost::asio::ip::tcp::resolver::query::canonical_name); @@ -982,7 +982,7 @@ PRAGMA_WARNING_DISABLE_VS(4355) PUSH_WARNINGS DISABLE_GCC_WARNING(maybe-uninitialized) template - bool boosted_tcp_server::init_server(const std::string port, const std::string& address, epee::net_utils::ssl_support_t ssl_support, const std::pair &private_key_and_certificate_path, const std::list &allowed_certificates, const std::vector> &allowed_fingerprints, bool allow_any_cert) + bool boosted_tcp_server::init_server(const std::string port, const std::string& address, epee::net_utils::ssl_support_t ssl_support, const std::pair &private_key_and_certificate_path, const std::string &ca_file, const std::vector> &allowed_fingerprints, bool allow_any_cert) { uint32_t p = 0; @@ -990,7 +990,7 @@ DISABLE_GCC_WARNING(maybe-uninitialized) MERROR("Failed to convert port no = " << port); return false; } - return this->init_server(p, address, ssl_support, private_key_and_certificate_path, allowed_certificates, allowed_fingerprints, allow_any_cert); + return this->init_server(p, address, ssl_support, private_key_and_certificate_path, ca_file, allowed_fingerprints, allow_any_cert); } POP_WARNINGS //--------------------------------------------------------------------------------- diff --git a/contrib/epee/include/net/http_client.h b/contrib/epee/include/net/http_client.h index 1864c77ad..36158b99f 100644 --- a/contrib/epee/include/net/http_client.h +++ b/contrib/epee/include/net/http_client.h @@ -277,7 +277,7 @@ namespace net_utils critical_section m_lock; epee::net_utils::ssl_support_t m_ssl_support; std::pair m_ssl_private_key_and_certificate_path; - std::list m_ssl_allowed_certificates; + std::string m_ssl_ca_file; std::vector> m_ssl_allowed_fingerprints; bool m_ssl_allow_any_cert; @@ -303,16 +303,16 @@ namespace net_utils const std::string &get_host() const { return m_host_buff; }; const std::string &get_port() const { return m_port; }; - bool set_server(const std::string& address, boost::optional user, epee::net_utils::ssl_support_t ssl_support = epee::net_utils::ssl_support_t::e_ssl_support_autodetect, const std::pair &private_key_and_certificate_path = {}, const std::list &allowed_ssl_certificates = {}, const std::vector> &allowed_ssl_fingerprints = {}, bool allow_any_cert = false) + bool set_server(const std::string& address, boost::optional user, epee::net_utils::ssl_support_t ssl_support = epee::net_utils::ssl_support_t::e_ssl_support_autodetect, const std::pair &private_key_and_certificate_path = {}, std::string ca_file = {}, const std::vector> &allowed_ssl_fingerprints = {}, bool allow_any_cert = false) { http::url_content parsed{}; const bool r = parse_url(address, parsed); CHECK_AND_ASSERT_MES(r, false, "failed to parse url: " << address); - set_server(std::move(parsed.host), std::to_string(parsed.port), std::move(user), ssl_support, private_key_and_certificate_path, allowed_ssl_certificates, allowed_ssl_fingerprints, allow_any_cert); + set_server(std::move(parsed.host), std::to_string(parsed.port), std::move(user), ssl_support, private_key_and_certificate_path, std::move(ca_file), allowed_ssl_fingerprints, allow_any_cert); return true; } - void set_server(std::string host, std::string port, boost::optional user, epee::net_utils::ssl_support_t ssl_support = epee::net_utils::ssl_support_t::e_ssl_support_autodetect, const std::pair &private_key_and_certificate_path = {}, const std::list &allowed_ssl_certificates = {}, const std::vector> &allowed_ssl_fingerprints = {}, bool allow_any_cert = false) + void set_server(std::string host, std::string port, boost::optional user, epee::net_utils::ssl_support_t ssl_support = epee::net_utils::ssl_support_t::e_ssl_support_autodetect, const std::pair &private_key_and_certificate_path = {}, std::string ca_file = {}, const std::vector> &allowed_ssl_fingerprints = {}, bool allow_any_cert = false) { CRITICAL_REGION_LOCAL(m_lock); disconnect(); @@ -321,10 +321,10 @@ namespace net_utils m_auth = user ? http_client_auth{std::move(*user)} : http_client_auth{}; m_ssl_support = ssl_support; m_ssl_private_key_and_certificate_path = private_key_and_certificate_path; - m_ssl_allowed_certificates = allowed_ssl_certificates; + m_ssl_ca_file = std::move(ca_file); m_ssl_allowed_fingerprints = allowed_ssl_fingerprints; m_ssl_allow_any_cert = allow_any_cert; - m_net_client.set_ssl(m_ssl_support, m_ssl_private_key_and_certificate_path, m_ssl_allowed_certificates, m_ssl_allowed_fingerprints, m_ssl_allow_any_cert); + m_net_client.set_ssl(m_ssl_support, m_ssl_private_key_and_certificate_path, m_ssl_ca_file, m_ssl_allowed_fingerprints, m_ssl_allow_any_cert); } template diff --git a/contrib/epee/include/net/http_server_impl_base.h b/contrib/epee/include/net/http_server_impl_base.h index 0922f21f2..8a6b32943 100644 --- a/contrib/epee/include/net/http_server_impl_base.h +++ b/contrib/epee/include/net/http_server_impl_base.h @@ -61,7 +61,7 @@ namespace epee boost::optional user = boost::none, epee::net_utils::ssl_support_t ssl_support = epee::net_utils::ssl_support_t::e_ssl_support_autodetect, const std::pair &private_key_and_certificate_path = {}, - std::list allowed_certificates = {}, + const std::string &ca_path = {}, std::vector> allowed_fingerprints = {}, bool allow_any_cert = false) { @@ -80,7 +80,7 @@ namespace epee m_net_server.get_config_object().m_user = std::move(user); MGINFO("Binding on " << bind_ip << ":" << bind_port); - bool res = m_net_server.init_server(bind_port, bind_ip, ssl_support, private_key_and_certificate_path, std::move(allowed_certificates), std::move(allowed_fingerprints), allow_any_cert); + bool res = m_net_server.init_server(bind_port, bind_ip, ssl_support, private_key_and_certificate_path, ca_path, std::move(allowed_fingerprints), allow_any_cert); if(!res) { LOG_ERROR("Failed to bind server"); diff --git a/contrib/epee/include/net/net_helper.h b/contrib/epee/include/net/net_helper.h index aa3df7160..2b220bb0f 100644 --- a/contrib/epee/include/net/net_helper.h +++ b/contrib/epee/include/net/net_helper.h @@ -136,12 +136,12 @@ namespace net_utils catch(...) { /* ignore */ } } - inline void set_ssl(epee::net_utils::ssl_support_t ssl_support = epee::net_utils::ssl_support_t::e_ssl_support_autodetect, const std::pair &private_key_and_certificate_path = {}, std::list allowed_certificates = {}, std::vector> allowed_fingerprints = {}, bool allow_any_cert = false) + inline void set_ssl(epee::net_utils::ssl_support_t ssl_support = epee::net_utils::ssl_support_t::e_ssl_support_autodetect, const std::pair &private_key_and_certificate_path = {}, const std::string &ca_path = {}, std::vector> allowed_fingerprints = {}, bool allow_any_cert = false) { if (ssl_support == epee::net_utils::ssl_support_t::e_ssl_support_disabled) m_ctx = {boost::asio::ssl::context(boost::asio::ssl::context::tlsv12), {}, {}}; else - m_ctx = create_ssl_context(private_key_and_certificate_path, std::move(allowed_certificates), std::move(allowed_fingerprints), allow_any_cert); + m_ctx = create_ssl_context(private_key_and_certificate_path, ca_path, std::move(allowed_fingerprints), allow_any_cert); m_ssl_support = ssl_support; } @@ -212,8 +212,6 @@ namespace net_utils // Set SSL options // disable sslv2 - m_ctx.context.set_options(boost::asio::ssl::context::default_workarounds | boost::asio::ssl::context::no_sslv2); - m_ctx.context.set_default_verify_paths(); m_ssl_socket.reset(new boost::asio::ssl::stream(m_io_service, m_ctx.context)); // Get a list of endpoints corresponding to the server name. diff --git a/contrib/epee/include/net/net_ssl.h b/contrib/epee/include/net/net_ssl.h index f7b102164..1b90a948d 100644 --- a/contrib/epee/include/net/net_ssl.h +++ b/contrib/epee/include/net/net_ssl.h @@ -31,10 +31,11 @@ #include #include -#include +#include #include #include #include +#include namespace epee { @@ -49,7 +50,7 @@ namespace net_utils struct ssl_context_t { boost::asio::ssl::context context; - std::list allowed_certificates; + std::string ca_path; std::vector> allowed_fingerprints; bool allow_any_cert; }; @@ -57,7 +58,7 @@ namespace net_utils // https://security.stackexchange.com/questions/34780/checking-client-hello-for-https-classification constexpr size_t get_ssl_magic_size() { return 9; } bool is_ssl(const unsigned char *data, size_t len); - ssl_context_t create_ssl_context(const std::pair &private_key_and_certificate_path, std::list allowed_certificates, std::vector> allowed_fingerprints, bool allow_any_cert); + ssl_context_t create_ssl_context(const std::pair &private_key_and_certificate_path, const std::string &ca_path, std::vector> allowed_fingerprints, bool allow_any_cert); void use_ssl_certificate(ssl_context_t &ssl_context, const std::pair &private_key_and_certificate_path); bool is_certificate_allowed(boost::asio::ssl::verify_context &ctx, const ssl_context_t &ssl_context); bool ssl_handshake(boost::asio::ssl::stream &socket, boost::asio::ssl::stream_base::handshake_type type, const epee::net_utils::ssl_context_t &ssl_context); diff --git a/contrib/epee/src/net_ssl.cpp b/contrib/epee/src/net_ssl.cpp index cb65121bd..1caed53ea 100644 --- a/contrib/epee/src/net_ssl.cpp +++ b/contrib/epee/src/net_ssl.cpp @@ -77,6 +77,21 @@ namespace } }; using openssl_bignum = std::unique_ptr; + + boost::system::error_code load_ca_file(boost::asio::ssl::context& ctx, const std::string& path) + { + SSL_CTX* const ssl_ctx = ctx.native_handle(); // could be moved from context + if (ssl_ctx == nullptr) + return {boost::asio::error::invalid_argument}; + + if (!SSL_CTX_load_verify_locations(ssl_ctx, path.c_str(), nullptr)) + { + return boost::system::error_code{ + int(::ERR_get_error()), boost::asio::error::get_ssl_category() + }; + } + return boost::system::error_code{}; + } } namespace epee @@ -84,6 +99,7 @@ namespace epee namespace net_utils { + // https://stackoverflow.com/questions/256405/programmatically-create-x509-certificate-using-openssl bool create_ssl_certificate(EVP_PKEY *&pkey, X509 *&cert) { @@ -155,9 +171,9 @@ bool create_ssl_certificate(EVP_PKEY *&pkey, X509 *&cert) return true; } -ssl_context_t create_ssl_context(const std::pair &private_key_and_certificate_path, std::list allowed_certificates, std::vector> allowed_fingerprints, bool allow_any_cert) +ssl_context_t create_ssl_context(const std::pair &private_key_and_certificate_path, const std::string &ca_path, std::vector> allowed_fingerprints, bool allow_any_cert) { - ssl_context_t ssl_context{boost::asio::ssl::context(boost::asio::ssl::context::tlsv12), std::move(allowed_certificates), std::move(allowed_fingerprints)}; + ssl_context_t ssl_context{boost::asio::ssl::context(boost::asio::ssl::context::tlsv12), std::move(ca_path), std::move(allowed_fingerprints)}; // only allow tls v1.2 and up ssl_context.context.set_options(boost::asio::ssl::context::default_workarounds); @@ -186,7 +202,15 @@ ssl_context_t create_ssl_context(const std::pair &priv #ifdef SSL_OP_NO_COMPRESSION SSL_CTX_set_options(ctx, SSL_OP_NO_COMPRESSION); #endif - ssl_context.context.set_default_verify_paths(); + + if (!ssl_context.ca_path.empty()) + { + const boost::system::error_code err = load_ca_file(ssl_context.context, ssl_context.ca_path); + if (err) + throw boost::system::system_error{err, "Failed to load user CA file at " + ssl_context.ca_path}; + } + else if (allowed_fingerprints.empty()) + ssl_context.context.set_default_verify_paths(); // only use system-CAs if no user-supplied cert info CHECK_AND_ASSERT_THROW_MES(private_key_and_certificate_path.first.empty() == private_key_and_certificate_path.second.empty(), "private key and certificate must be either both given or both empty"); if (private_key_and_certificate_path.second.empty()) @@ -237,21 +261,21 @@ bool is_ssl(const unsigned char *data, size_t len) bool is_certificate_allowed(boost::asio::ssl::verify_context &ctx, const ssl_context_t &ssl_context) { - X509_STORE_CTX *sctx = ctx.native_handle(); - if (!sctx) - { - MERROR("Error getting verify_context handle"); - return false; - } - X509 *cert =X509_STORE_CTX_get_current_cert(sctx); - if (!cert) - { - MERROR("No certificate found in verify_context"); - return false; - } - // can we check the certificate against a list of fingerprints? if (!ssl_context.allowed_fingerprints.empty()) { + X509_STORE_CTX *sctx = ctx.native_handle(); + if (!sctx) + { + MERROR("Error getting verify_context handle"); + return false; + } + X509 *cert =X509_STORE_CTX_get_current_cert(sctx); + if (!cert) + { + MERROR("No certificate found in verify_context"); + return false; + } + // buffer for the certificate digest and the size of the result std::vector digest(EVP_MAX_MD_SIZE); unsigned int size{ 0 }; @@ -270,52 +294,20 @@ bool is_certificate_allowed(boost::asio::ssl::verify_context &ctx, const ssl_con return true; } - if (!ssl_context.allowed_certificates.empty()) { - BIO *bio_cert = BIO_new(BIO_s_mem()); - bool success = PEM_write_bio_X509(bio_cert, cert); - if (!success) - { - BIO_free(bio_cert); - MERROR("Failed to print certificate"); - return false; - } - BUF_MEM *buf = NULL; - BIO_get_mem_ptr(bio_cert, &buf); - if (!buf || !buf->data || !buf->length) - { - BIO_free(bio_cert); - MERROR("Failed to write certificate: " << ERR_get_error()); - return false; - } - std::string certificate(std::string(buf->data, buf->length)); - BIO_free(bio_cert); - if (std::find(ssl_context.allowed_certificates.begin(), ssl_context.allowed_certificates.end(), certificate) != ssl_context.allowed_certificates.end()) - return true; - } - - // if either checklist is non-empty we must have failed it - return ssl_context.allowed_fingerprints.empty() && ssl_context.allowed_certificates.empty(); + return ssl_context.allowed_fingerprints.empty() && ssl_context.ca_path.empty(); } bool ssl_handshake(boost::asio::ssl::stream &socket, boost::asio::ssl::stream_base::handshake_type type, const epee::net_utils::ssl_context_t &ssl_context) { bool verified = false; socket.next_layer().set_option(boost::asio::ip::tcp::no_delay(true)); + socket.set_verify_mode(boost::asio::ssl::verify_peer); socket.set_verify_callback([&](bool preverified, boost::asio::ssl::verify_context &ctx) { - if (!preverified) - { - const int err = X509_STORE_CTX_get_error(ctx.native_handle()); - const int depth = X509_STORE_CTX_get_error_depth(ctx.native_handle()); - if (err != X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT || depth != 0) - { - MERROR("Invalid SSL certificate, error " << err << " at depth " << depth << ", connection dropped"); - return false; - } - } - if (!ssl_context.allow_any_cert && !is_certificate_allowed(ctx, ssl_context)) - { + // preverified means it passed system or user CA check. System CA is never loaded + // when fingerprints are whitelisted. + if (!preverified && !ssl_context.allow_any_cert && !is_certificate_allowed(ctx, ssl_context)) { MERROR("Certificate is not in the allowed list, connection droppped"); return false; } diff --git a/src/rpc/core_rpc_server.cpp b/src/rpc/core_rpc_server.cpp index 3e580a0fb..6273feaf4 100644 --- a/src/rpc/core_rpc_server.cpp +++ b/src/rpc/core_rpc_server.cpp @@ -91,7 +91,7 @@ namespace cryptonote command_line::add_arg(desc, arg_rpc_ssl); command_line::add_arg(desc, arg_rpc_ssl_private_key); command_line::add_arg(desc, arg_rpc_ssl_certificate); - command_line::add_arg(desc, arg_rpc_ssl_allowed_certificates); + command_line::add_arg(desc, arg_rpc_ssl_ca_certificates); command_line::add_arg(desc, arg_rpc_ssl_allowed_fingerprints); command_line::add_arg(desc, arg_rpc_ssl_allow_any_cert); command_line::add_arg(desc, arg_bootstrap_daemon_address); @@ -158,17 +158,7 @@ namespace cryptonote } const std::string ssl_private_key = command_line::get_arg(vm, arg_rpc_ssl_private_key); const std::string ssl_certificate = command_line::get_arg(vm, arg_rpc_ssl_certificate); - const std::vector ssl_allowed_certificate_paths = command_line::get_arg(vm, arg_rpc_ssl_allowed_certificates); - std::list ssl_allowed_certificates; - for (const std::string &path: ssl_allowed_certificate_paths) - { - ssl_allowed_certificates.push_back({}); - if (!epee::file_io_utils::load_file_to_string(path, ssl_allowed_certificates.back())) - { - MERROR("Failed to load certificate: " << path); - ssl_allowed_certificates.back() = std::string(); - } - } + std::string ssl_ca_path = command_line::get_arg(vm, arg_rpc_ssl_ca_certificates); const std::vector ssl_allowed_fingerprint_strings = command_line::get_arg(vm, arg_rpc_ssl_allowed_fingerprints); std::vector> ssl_allowed_fingerprints{ ssl_allowed_fingerprint_strings.size() }; @@ -178,7 +168,7 @@ namespace cryptonote auto rng = [](size_t len, uint8_t *ptr){ return crypto::rand(len, ptr); }; return epee::http_server_impl_base::init( rng, std::move(port), std::move(rpc_config->bind_ip), std::move(rpc_config->access_control_origins), std::move(http_login), - ssl_support, std::make_pair(ssl_private_key, ssl_certificate), std::move(ssl_allowed_certificates), std::move(ssl_allowed_fingerprints), ssl_allow_any_cert + ssl_support, std::make_pair(ssl_private_key, ssl_certificate), std::move(ssl_ca_path), std::move(ssl_allowed_fingerprints), ssl_allow_any_cert ); } //------------------------------------------------------------------------------------------------------------------------------ @@ -2408,9 +2398,9 @@ namespace cryptonote , "" }; - const command_line::arg_descriptor> core_rpc_server::arg_rpc_ssl_allowed_certificates = { - "rpc-ssl-allowed-certificates" - , "List of paths to PEM format certificates of allowed peers (all allowed if empty)" + const command_line::arg_descriptor core_rpc_server::arg_rpc_ssl_ca_certificates = { + "rpc-ssl-ca-certificates" + , "Path to file containing concatenated PEM format certificate(s) to replace system CA(s)." }; const command_line::arg_descriptor> core_rpc_server::arg_rpc_ssl_allowed_fingerprints = { @@ -2420,7 +2410,7 @@ namespace cryptonote const command_line::arg_descriptor core_rpc_server::arg_rpc_ssl_allow_any_cert = { "rpc-ssl-allow-any-cert" - , "Allow any peer certificate, rather than just those on the allowed list" + , "Allow any peer certificate" , false }; diff --git a/src/rpc/core_rpc_server.h b/src/rpc/core_rpc_server.h index 8f5d83f1b..a42ca2494 100644 --- a/src/rpc/core_rpc_server.h +++ b/src/rpc/core_rpc_server.h @@ -60,7 +60,7 @@ namespace cryptonote static const command_line::arg_descriptor arg_rpc_ssl; static const command_line::arg_descriptor arg_rpc_ssl_private_key; static const command_line::arg_descriptor arg_rpc_ssl_certificate; - static const command_line::arg_descriptor> arg_rpc_ssl_allowed_certificates; + static const command_line::arg_descriptor arg_rpc_ssl_ca_certificates; static const command_line::arg_descriptor> arg_rpc_ssl_allowed_fingerprints; static const command_line::arg_descriptor arg_rpc_ssl_allow_any_cert; static const command_line::arg_descriptor arg_bootstrap_daemon_address; diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index 54fbd9ca8..c4f466699 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -243,7 +243,7 @@ struct options { const command_line::arg_descriptor daemon_ssl = {"daemon-ssl", tools::wallet2::tr("Enable SSL on daemon RPC connections: enabled|disabled|autodetect"), "autodetect"}; const command_line::arg_descriptor daemon_ssl_private_key = {"daemon-ssl-private-key", tools::wallet2::tr("Path to a PEM format private key"), ""}; const command_line::arg_descriptor daemon_ssl_certificate = {"daemon-ssl-certificate", tools::wallet2::tr("Path to a PEM format certificate"), ""}; - const command_line::arg_descriptor> daemon_ssl_allowed_certificates = {"daemon-ssl-allowed-certificates", tools::wallet2::tr("List of paths to PEM format certificates of allowed RPC servers")}; + const command_line::arg_descriptor daemon_ssl_ca_certificates = {"daemon-ssl-ca-certificates", tools::wallet2::tr("Path to file containing concatenated PEM format certificate(s) to replace system CA(s).")}; const command_line::arg_descriptor> daemon_ssl_allowed_fingerprints = {"daemon-ssl-allowed-fingerprints", tools::wallet2::tr("List of valid fingerprints of allowed RPC servers")}; const command_line::arg_descriptor daemon_ssl_allow_any_cert = {"daemon-ssl-allow-any-cert", tools::wallet2::tr("Allow any SSL certificate from the daemon"), false}; const command_line::arg_descriptor testnet = {"testnet", tools::wallet2::tr("For testnet. Daemon must also be launched with --testnet flag"), false}; @@ -321,7 +321,7 @@ std::unique_ptr make_basic(const boost::program_options::variabl auto device_derivation_path = command_line::get_arg(vm, opts.hw_device_derivation_path); auto daemon_ssl_private_key = command_line::get_arg(vm, opts.daemon_ssl_private_key); auto daemon_ssl_certificate = command_line::get_arg(vm, opts.daemon_ssl_certificate); - auto daemon_ssl_allowed_certificates = command_line::get_arg(vm, opts.daemon_ssl_allowed_certificates); + auto daemon_ssl_ca_file = command_line::get_arg(vm, opts.daemon_ssl_ca_certificates); auto daemon_ssl_allowed_fingerprints = command_line::get_arg(vm, opts.daemon_ssl_allowed_fingerprints); auto daemon_ssl_allow_any_cert = command_line::get_arg(vm, opts.daemon_ssl_allow_any_cert); auto daemon_ssl = command_line::get_arg(vm, opts.daemon_ssl); @@ -367,11 +367,11 @@ std::unique_ptr make_basic(const boost::program_options::variabl // which both authenticates and encrypts const bool unencrypted_proxy = !real_daemon.ends_with(".onion") && !real_daemon.ends_with(".i2p") && - daemon_ssl_allowed_certificates.empty() && daemon_ssl_allowed_fingerprints.empty(); + daemon_ssl_ca_file.empty() && daemon_ssl_allowed_fingerprints.empty(); THROW_WALLET_EXCEPTION_IF( unencrypted_proxy, tools::error::wallet_internal_error, - std::string{"Use of --"} + opts.proxy.name + " requires --" + opts.daemon_ssl_allowed_certificates.name + " or --" + opts.daemon_ssl_allowed_fingerprints.name + " or use of a .onion/.i2p domain" + std::string{"Use of --"} + opts.proxy.name + " requires --" + opts.daemon_ssl_ca_certificates.name + " or --" + opts.daemon_ssl_allowed_fingerprints.name + " or use of a .onion/.i2p domain" ); const auto proxy_address = command_line::get_arg(vm, opts.proxy); @@ -416,22 +416,11 @@ std::unique_ptr make_basic(const boost::program_options::variabl catch (const std::exception &e) { } } - std::list ssl_allowed_certificates; - for (const std::string &path: daemon_ssl_allowed_certificates) - { - ssl_allowed_certificates.push_back({}); - if (!epee::file_io_utils::load_file_to_string(path, ssl_allowed_certificates.back())) - { - MERROR("Failed to load certificate: " << path); - ssl_allowed_certificates.back() = std::string(); - } - } - std::vector> ssl_allowed_fingerprints{ daemon_ssl_allowed_fingerprints.size() }; std::transform(daemon_ssl_allowed_fingerprints.begin(), daemon_ssl_allowed_fingerprints.end(), ssl_allowed_fingerprints.begin(), epee::from_hex::vector); std::unique_ptr wallet(new tools::wallet2(nettype, kdf_rounds, unattended)); - wallet->init(std::move(daemon_address), std::move(login), std::move(proxy), 0, *trusted_daemon, ssl_support, std::make_pair(daemon_ssl_private_key, daemon_ssl_certificate), ssl_allowed_certificates, ssl_allowed_fingerprints, daemon_ssl_allow_any_cert); + wallet->init(std::move(daemon_address), std::move(login), std::move(proxy), 0, *trusted_daemon, ssl_support, std::make_pair(daemon_ssl_private_key, daemon_ssl_certificate), std::move(daemon_ssl_ca_file), ssl_allowed_fingerprints, daemon_ssl_allow_any_cert); boost::filesystem::path ringdb_path = command_line::get_arg(vm, opts.shared_ringdb_dir); wallet->set_ring_database(ringdb_path.string()); wallet->get_message_store().set_options(vm); @@ -1100,7 +1089,7 @@ void wallet2::init_options(boost::program_options::options_description& desc_par command_line::add_arg(desc_params, opts.daemon_ssl); command_line::add_arg(desc_params, opts.daemon_ssl_private_key); command_line::add_arg(desc_params, opts.daemon_ssl_certificate); - command_line::add_arg(desc_params, opts.daemon_ssl_allowed_certificates); + command_line::add_arg(desc_params, opts.daemon_ssl_ca_certificates); command_line::add_arg(desc_params, opts.daemon_ssl_allowed_fingerprints); command_line::add_arg(desc_params, opts.daemon_ssl_allow_any_cert); command_line::add_arg(desc_params, opts.testnet); @@ -1156,7 +1145,7 @@ std::unique_ptr wallet2::make_dummy(const boost::program_options::varia //---------------------------------------------------------------------------------------------------- bool wallet2::set_daemon(std::string daemon_address, boost::optional daemon_login, bool trusted_daemon, epee::net_utils::ssl_support_t ssl_support, const std::pair &private_key_and_certificate_path, - const std::list &allowed_certificates, const std::vector> &allowed_fingerprints, + std::string ca_file, const std::vector> &allowed_fingerprints, bool allow_any_cert) { if(m_http_client.is_connected()) @@ -1166,17 +1155,17 @@ bool wallet2::set_daemon(std::string daemon_address, boost::optional daemon_login, boost::asio::ip::tcp::endpoint proxy, uint64_t upper_transaction_weight_limit, bool trusted_daemon, epee::net_utils::ssl_support_t ssl_support, const std::pair &private_key_and_certificate_path, const std::list &allowed_certificates, const std::vector> &allowed_fingerprints, bool allow_any_cert) +bool wallet2::init(std::string daemon_address, boost::optional daemon_login, boost::asio::ip::tcp::endpoint proxy, uint64_t upper_transaction_weight_limit, bool trusted_daemon, epee::net_utils::ssl_support_t ssl_support, const std::pair &private_key_and_certificate_path, std::string ca_file, const std::vector> &allowed_fingerprints, bool allow_any_cert) { m_checkpoints.init_default_checkpoints(m_nettype); m_is_initialized = true; m_upper_transaction_weight_limit = upper_transaction_weight_limit; if (proxy != boost::asio::ip::tcp::endpoint{}) m_http_client.set_connector(net::socks::connector{std::move(proxy)}); - return set_daemon(daemon_address, daemon_login, trusted_daemon, ssl_support, private_key_and_certificate_path, allowed_certificates, allowed_fingerprints, allow_any_cert); + return set_daemon(daemon_address, daemon_login, trusted_daemon, ssl_support, private_key_and_certificate_path, std::move(ca_file), allowed_fingerprints, allow_any_cert); } //---------------------------------------------------------------------------------------------------- bool wallet2::is_deterministic() const diff --git a/src/wallet/wallet2.h b/src/wallet/wallet2.h index a24127800..58dc5f082 100644 --- a/src/wallet/wallet2.h +++ b/src/wallet/wallet2.h @@ -691,13 +691,13 @@ namespace tools bool trusted_daemon = true, epee::net_utils::ssl_support_t ssl_support = epee::net_utils::ssl_support_t::e_ssl_support_autodetect, const std::pair &private_key_and_certificate_path = {}, - const std::list &allowed_certificates = {}, const std::vector> &allowed_fingerprints = {}, + std::string ca_file = {}, const std::vector> &allowed_fingerprints = {}, bool allow_any_cert = false); bool set_daemon(std::string daemon_address = "http://localhost:8080", boost::optional daemon_login = boost::none, bool trusted_daemon = true, epee::net_utils::ssl_support_t ssl_support = epee::net_utils::ssl_support_t::e_ssl_support_autodetect, const std::pair &private_key_and_certificate_path = {}, - const std::list &allowed_certificates = {}, const std::vector> &allowed_fingerprints = {}, + std::string ca_file = {}, const std::vector> &allowed_fingerprints = {}, bool allow_any_cert = false); void stop() { m_run.store(false, std::memory_order_relaxed); m_message_store.stop(); } diff --git a/src/wallet/wallet_rpc_server.cpp b/src/wallet/wallet_rpc_server.cpp index 92265d954..7824ed62e 100644 --- a/src/wallet/wallet_rpc_server.cpp +++ b/src/wallet/wallet_rpc_server.cpp @@ -68,7 +68,7 @@ namespace const command_line::arg_descriptor arg_rpc_ssl = {"rpc-ssl", tools::wallet2::tr("Enable SSL on wallet RPC connections: enabled|disabled|autodetect"), "autodetect"}; const command_line::arg_descriptor arg_rpc_ssl_private_key = {"rpc-ssl-private-key", tools::wallet2::tr("Path to a PEM format private key"), ""}; const command_line::arg_descriptor arg_rpc_ssl_certificate = {"rpc-ssl-certificate", tools::wallet2::tr("Path to a PEM format certificate"), ""}; - const command_line::arg_descriptor> arg_rpc_ssl_allowed_certificates = {"rpc-ssl-allowed-certificates", tools::wallet2::tr("List of paths to PEM format certificates of allowed RPC servers (all allowed if empty)")}; + const command_line::arg_descriptor arg_rpc_ssl_ca_certificates = {"rpc-ssl-ca-certificates", tools::wallet2::tr("Path to file containing concatenated PEM format certificate(s) to replace system CA(s).")}; const command_line::arg_descriptor> arg_rpc_ssl_allowed_fingerprints = {"rpc-ssl-allowed-fingerprints", tools::wallet2::tr("List of certificate fingerprints to allow")}; constexpr const char default_rpc_username[] = "monero"; @@ -247,7 +247,7 @@ namespace tools auto rpc_ssl_private_key = command_line::get_arg(vm, arg_rpc_ssl_private_key); auto rpc_ssl_certificate = command_line::get_arg(vm, arg_rpc_ssl_certificate); - auto rpc_ssl_allowed_certificates = command_line::get_arg(vm, arg_rpc_ssl_allowed_certificates); + auto rpc_ssl_ca_file = command_line::get_arg(vm, arg_rpc_ssl_ca_certificates); auto rpc_ssl_allowed_fingerprints = command_line::get_arg(vm, arg_rpc_ssl_allowed_fingerprints); auto rpc_ssl = command_line::get_arg(vm, arg_rpc_ssl); epee::net_utils::ssl_support_t rpc_ssl_support; @@ -256,16 +256,6 @@ namespace tools MERROR("Invalid argument for " << std::string(arg_rpc_ssl.name)); return false; } - std::list allowed_certificates; - for (const std::string &path: rpc_ssl_allowed_certificates) - { - allowed_certificates.push_back({}); - if (!epee::file_io_utils::load_file_to_string(path, allowed_certificates.back())) - { - MERROR("Failed to load certificate: " << path); - allowed_certificates.back() = std::string(); - } - } std::vector> allowed_fingerprints{ rpc_ssl_allowed_fingerprints.size() }; std::transform(rpc_ssl_allowed_fingerprints.begin(), rpc_ssl_allowed_fingerprints.end(), allowed_fingerprints.begin(), epee::from_hex::vector); @@ -277,7 +267,7 @@ namespace tools auto rng = [](size_t len, uint8_t *ptr) { return crypto::rand(len, ptr); }; return epee::http_server_impl_base::init( rng, std::move(bind_port), std::move(rpc_config->bind_ip), std::move(rpc_config->access_control_origins), std::move(http_login), - rpc_ssl_support, std::make_pair(rpc_ssl_private_key, rpc_ssl_certificate), std::move(allowed_certificates), std::move(allowed_fingerprints) + rpc_ssl_support, std::make_pair(rpc_ssl_private_key, rpc_ssl_certificate), std::move(rpc_ssl_ca_file), std::move(allowed_fingerprints) ); } //------------------------------------------------------------------------------------------------------------------------------ @@ -4070,7 +4060,7 @@ namespace tools for (auto c: fp) v.push_back(c); } - if (!m_wallet->set_daemon(req.address, boost::none, req.trusted, ssl_support, std::make_pair(req.ssl_private_key_path, req.ssl_certificate_path), req.ssl_allowed_certificates, ssl_allowed_fingerprints, req.ssl_allow_any_cert)) + if (!m_wallet->set_daemon(req.address, boost::none, req.trusted, ssl_support, std::make_pair(req.ssl_private_key_path, req.ssl_certificate_path), std::move(req.ssl_ca_file), ssl_allowed_fingerprints, req.ssl_allow_any_cert)) { er.code = WALLET_RPC_ERROR_CODE_NO_DAEMON_CONNECTION; er.message = std::string("Unable to set daemon"); @@ -4278,7 +4268,7 @@ int main(int argc, char** argv) { command_line::add_arg(desc_params, arg_rpc_ssl); command_line::add_arg(desc_params, arg_rpc_ssl_private_key); command_line::add_arg(desc_params, arg_rpc_ssl_certificate); - command_line::add_arg(desc_params, arg_rpc_ssl_allowed_certificates); + command_line::add_arg(desc_params, arg_rpc_ssl_ca_certificates); command_line::add_arg(desc_params, arg_rpc_ssl_allowed_fingerprints); daemonizer::init_options(hidden_options, desc_params); diff --git a/src/wallet/wallet_rpc_server_commands_defs.h b/src/wallet/wallet_rpc_server_commands_defs.h index 7984f6584..4c945ab41 100644 --- a/src/wallet/wallet_rpc_server_commands_defs.h +++ b/src/wallet/wallet_rpc_server_commands_defs.h @@ -2448,7 +2448,7 @@ namespace wallet_rpc std::string ssl_support; // disabled, enabled, autodetect std::string ssl_private_key_path; std::string ssl_certificate_path; - std::list ssl_allowed_certificates; + std::string ssl_ca_file; std::vector ssl_allowed_fingerprints; bool ssl_allow_any_cert; @@ -2458,7 +2458,7 @@ namespace wallet_rpc KV_SERIALIZE_OPT(ssl_support, (std::string)"autodetect") KV_SERIALIZE(ssl_private_key_path) KV_SERIALIZE(ssl_certificate_path) - KV_SERIALIZE(ssl_allowed_certificates) + KV_SERIALIZE(ssl_ca_file) KV_SERIALIZE(ssl_allowed_fingerprints) KV_SERIALIZE_OPT(ssl_allow_any_cert, false) END_KV_SERIALIZE_MAP() From f18a069fcc5ad5d18bde93d9ee2902354f2add9d Mon Sep 17 00:00:00 2001 From: Lee Clagett Date: Tue, 12 Mar 2019 18:16:47 -0400 Subject: [PATCH 02/11] Do not require client certificate unless server has some whitelisted. Currently a client must provide a certificate, even if the server is configured to allow all certificates. This drops that requirement from the client - unless the server is configured to use a CA file or fingerprint(s) for verification - which is the standard behavior for SSL servers. The "system-wide" CA is not being used as a "fallback" to verify clients before or after this patch. --- contrib/epee/src/net_ssl.cpp | 39 +++++++++++++++++++++++++----------- 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/contrib/epee/src/net_ssl.cpp b/contrib/epee/src/net_ssl.cpp index 1caed53ea..6b156f238 100644 --- a/contrib/epee/src/net_ssl.cpp +++ b/contrib/epee/src/net_ssl.cpp @@ -302,18 +302,33 @@ bool ssl_handshake(boost::asio::ssl::stream &socke bool verified = false; socket.next_layer().set_option(boost::asio::ip::tcp::no_delay(true)); - socket.set_verify_mode(boost::asio::ssl::verify_peer); - socket.set_verify_callback([&](bool preverified, boost::asio::ssl::verify_context &ctx) + /* Using system-wide CA store for client verification is funky - there is + no expected hostname for server to verify against. If server doesn't have + specific whitelisted certificates for client, don't require client to + send certificate at all. */ + const bool no_verification = ssl_context.allow_any_cert || + (type == boost::asio::ssl::stream_base::server && ssl_context.allowed_fingerprints.empty() && ssl_context.ca_path.empty()); + + /* According to OpenSSL documentation (and SSL specifications), server must + always send certificate unless "anonymous" cipher mode is used which are + disabled by default. Either way, the certificate is never inspected. */ + if (no_verification) + socket.set_verify_mode(boost::asio::ssl::verify_none); + else { - // preverified means it passed system or user CA check. System CA is never loaded - // when fingerprints are whitelisted. - if (!preverified && !ssl_context.allow_any_cert && !is_certificate_allowed(ctx, ssl_context)) { - MERROR("Certificate is not in the allowed list, connection droppped"); - return false; - } - verified = true; - return true; - }); + socket.set_verify_mode(boost::asio::ssl::verify_peer); + socket.set_verify_callback([&](bool preverified, boost::asio::ssl::verify_context &ctx) + { + // preverified means it passed system or user CA check. System CA is never loaded + // when fingerprints are whitelisted. + if (!preverified && !is_certificate_allowed(ctx, ssl_context)) { + MERROR("Certificate is not in the allowed list, connection droppped"); + return false; + } + verified = true; + return true; + }); + } boost::system::error_code ec; socket.handshake(type, ec); @@ -322,7 +337,7 @@ bool ssl_handshake(boost::asio::ssl::stream &socke MERROR("handshake failed, connection dropped: " << ec.message()); return false; } - if (!ssl_context.allow_any_cert && !verified) + if (!no_verification && !verified) { MERROR("Peer did not provide a certificate in the allowed list, connection dropped"); return false; From 1f5ed328aa3b0501bd85774dc960c17a73d79db3 Mon Sep 17 00:00:00 2001 From: Lee Clagett Date: Wed, 13 Mar 2019 20:01:14 -0400 Subject: [PATCH 03/11] Change default SSL to "enabled" if user specifies fingerprint/certificate Currently if a user specifies a ca file or fingerprint to verify peer, the default behavior is SSL autodetect which allows for mitm downgrade attacks. It should be investigated whether a manual override should be allowed - the configuration is likely always invalid. --- src/rpc/core_rpc_server.cpp | 19 ++++++++++++------- src/wallet/wallet2.cpp | 11 ++++++++--- src/wallet/wallet_rpc_server.cpp | 13 +++++++++---- 3 files changed, 29 insertions(+), 14 deletions(-) diff --git a/src/rpc/core_rpc_server.cpp b/src/rpc/core_rpc_server.cpp index 6273feaf4..0a45fca27 100644 --- a/src/rpc/core_rpc_server.cpp +++ b/src/rpc/core_rpc_server.cpp @@ -149,13 +149,6 @@ namespace cryptonote if (rpc_config->login) http_login.emplace(std::move(rpc_config->login->username), std::move(rpc_config->login->password).password()); - epee::net_utils::ssl_support_t ssl_support; - const std::string ssl = command_line::get_arg(vm, arg_rpc_ssl); - if (!epee::net_utils::ssl_support_from_string(ssl_support, ssl)) - { - MFATAL("Invalid RPC SSL support: " << ssl); - return false; - } const std::string ssl_private_key = command_line::get_arg(vm, arg_rpc_ssl_private_key); const std::string ssl_certificate = command_line::get_arg(vm, arg_rpc_ssl_certificate); std::string ssl_ca_path = command_line::get_arg(vm, arg_rpc_ssl_ca_certificates); @@ -165,6 +158,18 @@ namespace cryptonote std::transform(ssl_allowed_fingerprint_strings.begin(), ssl_allowed_fingerprint_strings.end(), ssl_allowed_fingerprints.begin(), epee::from_hex::vector); const bool ssl_allow_any_cert = command_line::get_arg(vm, arg_rpc_ssl_allow_any_cert); + // user specified CA file or fingeprints implies enabled SSL by default + epee::net_utils::ssl_support_t ssl_support = epee::net_utils::ssl_support_t::e_ssl_support_enabled; + if ((ssl_allowed_fingerprints.empty() && ssl_ca_path.empty()) || !command_line::is_arg_defaulted(vm, arg_rpc_ssl)) + { + const std::string ssl = command_line::get_arg(vm, arg_rpc_ssl); + if (!epee::net_utils::ssl_support_from_string(ssl_support, ssl)) + { + MFATAL("Invalid RPC SSL support: " << ssl); + return false; + } + } + auto rng = [](size_t len, uint8_t *ptr){ return crypto::rand(len, ptr); }; return epee::http_server_impl_base::init( rng, std::move(port), std::move(rpc_config->bind_ip), std::move(rpc_config->access_control_origins), std::move(http_login), diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index c4f466699..9e2a826d9 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -325,9 +325,14 @@ std::unique_ptr make_basic(const boost::program_options::variabl auto daemon_ssl_allowed_fingerprints = command_line::get_arg(vm, opts.daemon_ssl_allowed_fingerprints); auto daemon_ssl_allow_any_cert = command_line::get_arg(vm, opts.daemon_ssl_allow_any_cert); auto daemon_ssl = command_line::get_arg(vm, opts.daemon_ssl); - epee::net_utils::ssl_support_t ssl_support; - THROW_WALLET_EXCEPTION_IF(!epee::net_utils::ssl_support_from_string(ssl_support, daemon_ssl), tools::error::wallet_internal_error, - tools::wallet2::tr("Invalid argument for ") + std::string(opts.daemon_ssl.name)); + + // user specified CA file or fingeprints implies enabled SSL by default + epee::net_utils::ssl_support_t ssl_support = epee::net_utils::ssl_support_t::e_ssl_support_enabled; + if ((daemon_ssl_ca_file.empty() && daemon_ssl_allowed_fingerprints.empty()) || !command_line::is_arg_defaulted(vm, opts.daemon_ssl)) + { + THROW_WALLET_EXCEPTION_IF(!epee::net_utils::ssl_support_from_string(ssl_support, daemon_ssl), tools::error::wallet_internal_error, + tools::wallet2::tr("Invalid argument for ") + std::string(opts.daemon_ssl.name)); + } THROW_WALLET_EXCEPTION_IF(!daemon_address.empty() && !daemon_host.empty() && 0 != daemon_port, tools::error::wallet_internal_error, tools::wallet2::tr("can't specify daemon host or port more than once")); diff --git a/src/wallet/wallet_rpc_server.cpp b/src/wallet/wallet_rpc_server.cpp index 7824ed62e..a07e8e767 100644 --- a/src/wallet/wallet_rpc_server.cpp +++ b/src/wallet/wallet_rpc_server.cpp @@ -250,11 +250,16 @@ namespace tools auto rpc_ssl_ca_file = command_line::get_arg(vm, arg_rpc_ssl_ca_certificates); auto rpc_ssl_allowed_fingerprints = command_line::get_arg(vm, arg_rpc_ssl_allowed_fingerprints); auto rpc_ssl = command_line::get_arg(vm, arg_rpc_ssl); - epee::net_utils::ssl_support_t rpc_ssl_support; - if (!epee::net_utils::ssl_support_from_string(rpc_ssl_support, rpc_ssl)) + epee::net_utils::ssl_support_t rpc_ssl_support = epee::net_utils::ssl_support_t::e_ssl_support_enabled; + + // user specified CA file or fingeprints implies enabled SSL by default + if ((rpc_ssl_ca_file.empty() && rpc_ssl_allowed_fingerprints.empty()) || !command_line::is_arg_defaulted(vm, arg_rpc_ssl)) { - MERROR("Invalid argument for " << std::string(arg_rpc_ssl.name)); - return false; + if (!epee::net_utils::ssl_support_from_string(rpc_ssl_support, rpc_ssl)) + { + MERROR("Invalid argument for " << std::string(arg_rpc_ssl.name)); + return false; + } } std::vector> allowed_fingerprints{ rpc_ssl_allowed_fingerprints.size() }; From 21eb1b0725717ad013d3e2b00fbfc3b84ad04699 Mon Sep 17 00:00:00 2001 From: Lee Clagett Date: Fri, 15 Mar 2019 00:03:32 -0400 Subject: [PATCH 04/11] Pass SSL arguments via one class and use shared_ptr instead of reference --- .../epee/include/net/abstract_tcp_server2.h | 16 ++-- .../epee/include/net/abstract_tcp_server2.inl | 44 +++++----- contrib/epee/include/net/connection_basic.hpp | 52 +++++++---- contrib/epee/include/net/http_client.h | 19 +--- .../epee/include/net/http_server_impl_base.h | 8 +- contrib/epee/include/net/net_helper.h | 48 +++++------ contrib/epee/include/net/net_ssl.h | 74 +++++++++++++--- contrib/epee/src/connection_basic.cpp | 47 +++++----- contrib/epee/src/net_ssl.cpp | 86 +++++++++++-------- src/rpc/core_rpc_server.cpp | 31 ++++--- src/wallet/wallet2.cpp | 38 +++++--- src/wallet/wallet2.h | 10 +-- src/wallet/wallet_rpc_server.cpp | 38 ++++++-- 13 files changed, 302 insertions(+), 209 deletions(-) diff --git a/contrib/epee/include/net/abstract_tcp_server2.h b/contrib/epee/include/net/abstract_tcp_server2.h index f28b1fb7d..d0eabbba5 100644 --- a/contrib/epee/include/net/abstract_tcp_server2.h +++ b/contrib/epee/include/net/abstract_tcp_server2.h @@ -90,10 +90,10 @@ namespace net_utils public: typedef typename t_protocol_handler::connection_context t_connection_context; - struct shared_state : socket_stats + struct shared_state : connection_basic_shared_state { shared_state() - : socket_stats(), pfilter(nullptr), config() + : connection_basic_shared_state(), pfilter(nullptr), config() {} i_connection_filter* pfilter; @@ -104,14 +104,12 @@ namespace net_utils explicit connection( boost::asio::io_service& io_service, boost::shared_ptr state, t_connection_type connection_type, - epee::net_utils::ssl_support_t ssl_support, - ssl_context_t &ssl_context); + epee::net_utils::ssl_support_t ssl_support); explicit connection( boost::asio::ip::tcp::socket&& sock, boost::shared_ptr state, t_connection_type connection_type, - epee::net_utils::ssl_support_t ssl_support, - ssl_context_t &ssl_context); + epee::net_utils::ssl_support_t ssl_support); @@ -228,8 +226,8 @@ namespace net_utils std::map server_type_map; void create_server_type_map(); - bool init_server(uint32_t port, const std::string address = "0.0.0.0", epee::net_utils::ssl_support_t ssl_support = epee::net_utils::ssl_support_t::e_ssl_support_autodetect, const std::pair &private_key_and_certificate_path = std::make_pair(std::string(), std::string()), const std::string &ca_file = {}, const std::vector> &allowed_fingerprints = {}, bool allow_any_cert = false); - bool init_server(const std::string port, const std::string& address = "0.0.0.0", epee::net_utils::ssl_support_t ssl_support = epee::net_utils::ssl_support_t::e_ssl_support_autodetect, const std::pair &private_key_and_certificate_path = std::make_pair(std::string(), std::string()), const std::string &ca_file = {}, const std::vector> &allowed_fingerprints = {}, bool allow_any_cert = false); + bool init_server(uint32_t port, const std::string address = "0.0.0.0", ssl_options_t ssl_options = ssl_support_t::e_ssl_support_autodetect); + bool init_server(const std::string port, const std::string& address = "0.0.0.0", ssl_options_t ssl_options = ssl_support_t::e_ssl_support_autodetect); /// Run the server's io_service loop. bool run_server(size_t threads_count, bool wait = true, const boost::thread::attributes& attrs = boost::thread::attributes()); @@ -380,8 +378,6 @@ namespace net_utils boost::mutex connections_mutex; std::set connections_; - - ssl_context_t m_ssl_context; }; // class <>boosted_tcp_server diff --git a/contrib/epee/include/net/abstract_tcp_server2.inl b/contrib/epee/include/net/abstract_tcp_server2.inl index e8b5b6c1e..58f899a73 100644 --- a/contrib/epee/include/net/abstract_tcp_server2.inl +++ b/contrib/epee/include/net/abstract_tcp_server2.inl @@ -80,10 +80,9 @@ PRAGMA_WARNING_DISABLE_VS(4355) connection::connection( boost::asio::io_service& io_service, boost::shared_ptr state, t_connection_type connection_type, - epee::net_utils::ssl_support_t ssl_support, - ssl_context_t &ssl_context + ssl_support_t ssl_support ) - : connection(boost::asio::ip::tcp::socket{io_service}, std::move(state), connection_type, ssl_support, ssl_context) + : connection(boost::asio::ip::tcp::socket{io_service}, std::move(state), connection_type, ssl_support) { } @@ -91,11 +90,10 @@ PRAGMA_WARNING_DISABLE_VS(4355) connection::connection( boost::asio::ip::tcp::socket&& sock, boost::shared_ptr state, t_connection_type connection_type, - epee::net_utils::ssl_support_t ssl_support, - ssl_context_t &ssl_context + ssl_support_t ssl_support ) : - connection_basic(std::move(sock), state, ssl_support, ssl_context), + connection_basic(std::move(sock), state, ssl_support), m_protocol_handler(this, check_and_get(state).config, context), m_connection_type( connection_type ), m_throttle_speed_in("speed_in", "throttle_speed_in"), @@ -176,9 +174,9 @@ PRAGMA_WARNING_DISABLE_VS(4355) _dbg3("[sock " << socket_.native_handle() << "] new connection from " << print_connection_context_short(context) << " to " << local_ep.address().to_string() << ':' << local_ep.port() << - ", total sockets objects " << get_stats().sock_count); + ", total sockets objects " << get_state().sock_count); - if(static_cast(get_stats()).pfilter && !static_cast(get_stats()).pfilter->is_remote_host_allowed(context.m_remote_address)) + if(static_cast(get_state()).pfilter && !static_cast(get_state()).pfilter->is_remote_host_allowed(context.m_remote_address)) { _dbg2("[sock " << socket().native_handle() << "] host denied " << context.m_remote_address.host_str() << ", shutdowning connection"); close(); @@ -901,8 +899,7 @@ PRAGMA_WARNING_DISABLE_VS(4355) m_threads_count(0), m_thread_index(0), m_connection_type( connection_type ), - new_connection_(), - m_ssl_context({boost::asio::ssl::context(boost::asio::ssl::context::tlsv12), {}}) + new_connection_() { create_server_type_map(); m_thread_name_prefix = "NET"; @@ -918,8 +915,7 @@ PRAGMA_WARNING_DISABLE_VS(4355) m_threads_count(0), m_thread_index(0), m_connection_type(connection_type), - new_connection_(), - m_ssl_context({boost::asio::ssl::context(boost::asio::ssl::context::sslv23), {}}) + new_connection_() { create_server_type_map(); m_thread_name_prefix = "NET"; @@ -941,14 +937,14 @@ PRAGMA_WARNING_DISABLE_VS(4355) } //--------------------------------------------------------------------------------- template - bool boosted_tcp_server::init_server(uint32_t port, const std::string address, epee::net_utils::ssl_support_t ssl_support, const std::pair &private_key_and_certificate_path, const std::string &ca_file, const std::vector> &allowed_fingerprints, bool allow_any_cert) + bool boosted_tcp_server::init_server(uint32_t port, const std::string address, ssl_options_t ssl_options) { TRY_ENTRY(); m_stop_signal_sent = false; m_port = port; m_address = address; - if (ssl_support != epee::net_utils::ssl_support_t::e_ssl_support_disabled) - m_ssl_context = create_ssl_context(private_key_and_certificate_path, ca_file, allowed_fingerprints, allow_any_cert); + if (ssl_options) + m_state->configure_ssl(std::move(ssl_options)); // Open the acceptor with the option to reuse the address (i.e. SO_REUSEADDR). boost::asio::ip::tcp::resolver resolver(io_service_); boost::asio::ip::tcp::resolver::query query(address, boost::lexical_cast(port), boost::asio::ip::tcp::resolver::query::canonical_name); @@ -960,7 +956,7 @@ PRAGMA_WARNING_DISABLE_VS(4355) boost::asio::ip::tcp::endpoint binded_endpoint = acceptor_.local_endpoint(); m_port = binded_endpoint.port(); MDEBUG("start accept"); - new_connection_.reset(new connection(io_service_, m_state, m_connection_type, ssl_support, m_ssl_context)); + new_connection_.reset(new connection(io_service_, m_state, m_connection_type, m_state->ssl_options().support)); acceptor_.async_accept(new_connection_->socket(), boost::bind(&boosted_tcp_server::handle_accept, this, boost::asio::placeholders::error)); @@ -982,7 +978,7 @@ PRAGMA_WARNING_DISABLE_VS(4355) PUSH_WARNINGS DISABLE_GCC_WARNING(maybe-uninitialized) template - bool boosted_tcp_server::init_server(const std::string port, const std::string& address, epee::net_utils::ssl_support_t ssl_support, const std::pair &private_key_and_certificate_path, const std::string &ca_file, const std::vector> &allowed_fingerprints, bool allow_any_cert) + bool boosted_tcp_server::init_server(const std::string port, const std::string& address, ssl_options_t ssl_options) { uint32_t p = 0; @@ -990,7 +986,7 @@ DISABLE_GCC_WARNING(maybe-uninitialized) MERROR("Failed to convert port no = " << port); return false; } - return this->init_server(p, address, ssl_support, private_key_and_certificate_path, ca_file, allowed_fingerprints, allow_any_cert); + return this->init_server(p, address, std::move(ssl_options)); } POP_WARNINGS //--------------------------------------------------------------------------------- @@ -1165,7 +1161,7 @@ POP_WARNINGS new_connection_->setRpcStation(); // hopefully this is not needed actually } connection_ptr conn(std::move(new_connection_)); - new_connection_.reset(new connection(io_service_, m_state, m_connection_type, conn->get_ssl_support(), m_ssl_context)); + new_connection_.reset(new connection(io_service_, m_state, m_connection_type, conn->get_ssl_support())); acceptor_.async_accept(new_connection_->socket(), boost::bind(&boosted_tcp_server::handle_accept, this, boost::asio::placeholders::error)); @@ -1200,7 +1196,7 @@ POP_WARNINGS assert(m_state != nullptr); // always set in constructor _erro("Some problems at accept: " << e.message() << ", connections_count = " << m_state->sock_count); misc_utils::sleep_no_w(100); - new_connection_.reset(new connection(io_service_, m_state, m_connection_type, new_connection_->get_ssl_support(), m_ssl_context)); + new_connection_.reset(new connection(io_service_, m_state, m_connection_type, new_connection_->get_ssl_support())); acceptor_.async_accept(new_connection_->socket(), boost::bind(&boosted_tcp_server::handle_accept, this, boost::asio::placeholders::error)); @@ -1211,7 +1207,7 @@ POP_WARNINGS { if(std::addressof(get_io_service()) == std::addressof(GET_IO_SERVICE(sock))) { - connection_ptr conn(new connection(std::move(sock), m_state, m_connection_type, ssl_support, m_ssl_context)); + connection_ptr conn(new connection(std::move(sock), m_state, m_connection_type, ssl_support)); if(conn->start(false, 1 < m_threads_count, std::move(real_remote))) { conn->get_context(out); @@ -1298,7 +1294,7 @@ POP_WARNINGS _dbg3("Connected success to " << adr << ':' << port); - const epee::net_utils::ssl_support_t ssl_support = new_connection_l->get_ssl_support(); + const ssl_support_t ssl_support = new_connection_l->get_ssl_support(); if (ssl_support == epee::net_utils::ssl_support_t::e_ssl_support_enabled || ssl_support == epee::net_utils::ssl_support_t::e_ssl_support_autodetect) { // Handshake @@ -1329,7 +1325,7 @@ POP_WARNINGS { TRY_ENTRY(); - connection_ptr new_connection_l(new connection(io_service_, m_state, m_connection_type, ssl_support, m_ssl_context) ); + connection_ptr new_connection_l(new connection(io_service_, m_state, m_connection_type, ssl_support) ); connections_mutex.lock(); connections_.insert(new_connection_l); MDEBUG("connections_ size now " << connections_.size()); @@ -1393,7 +1389,7 @@ POP_WARNINGS bool boosted_tcp_server::connect_async(const std::string& adr, const std::string& port, uint32_t conn_timeout, const t_callback &cb, const std::string& bind_ip, epee::net_utils::ssl_support_t ssl_support) { TRY_ENTRY(); - connection_ptr new_connection_l(new connection(io_service_, m_state, m_connection_type, ssl_support, m_ssl_context) ); + connection_ptr new_connection_l(new connection(io_service_, m_state, m_connection_type, ssl_support) ); connections_mutex.lock(); connections_.insert(new_connection_l); MDEBUG("connections_ size now " << connections_.size()); diff --git a/contrib/epee/include/net/connection_basic.hpp b/contrib/epee/include/net/connection_basic.hpp index feedc6895..d3f5f4f24 100644 --- a/contrib/epee/include/net/connection_basic.hpp +++ b/contrib/epee/include/net/connection_basic.hpp @@ -57,15 +57,30 @@ namespace epee { namespace net_utils { - struct socket_stats - { - socket_stats() - : sock_count(0), sock_number(0) - {} - std::atomic sock_count; - std::atomic sock_number; - }; + class connection_basic_shared_state + { + ssl_options_t ssl_options_; + public: + boost::asio::ssl::context ssl_context; + std::atomic sock_count; + std::atomic sock_number; + + connection_basic_shared_state() + : ssl_options_(ssl_support_t::e_ssl_support_disabled), + ssl_context(boost::asio::ssl::context::tlsv12), + sock_count(0), + sock_number(0) + {} + + void configure_ssl(ssl_options_t src) + { + ssl_options_ = std::move(src); + ssl_context = ssl_options_.create_context(); + } + + const ssl_options_t& ssl_options() const noexcept { return ssl_options_; } + }; /************************************************************************/ /* */ @@ -83,9 +98,10 @@ class connection_basic_pimpl; // PIMPL for this class std::string to_string(t_connection_type type); class connection_basic { // not-templated base class for rapid developmet of some code parts - // beware of removing const, net_utils::connection is sketchily doing a cast to prevent storing ptr twice - const boost::shared_ptr m_stats; + // beware of removing const, net_utils::connection is sketchily doing a cast to prevent storing ptr twice + const boost::shared_ptr m_state; public: + std::unique_ptr< connection_basic_pimpl > mI; // my Implementation // moved here from orginal connecton<> - common member variables that do not depend on template in connection<> @@ -97,20 +113,19 @@ class connection_basic { // not-templated base class for rapid developmet of som /// Strand to ensure the connection's handlers are not called concurrently. boost::asio::io_service::strand strand_; /// Socket for the connection. - ssl_context_t &m_ssl_context; - ssl_support_t m_ssl_support; boost::asio::ssl::stream socket_; + ssl_support_t m_ssl_support; public: // first counter is the ++/-- count of current sockets, the other socket_number is only-increasing ++ number generator - connection_basic(boost::asio::ip::tcp::socket&& socket, boost::shared_ptr stats, ssl_support_t ssl_support, ssl_context_t &ssl_context); - connection_basic(boost::asio::io_service &io_service, boost::shared_ptr stats, ssl_support_t ssl_support, ssl_context_t &ssl_context); + connection_basic(boost::asio::ip::tcp::socket&& socket, boost::shared_ptr state, ssl_support_t ssl_support); + connection_basic(boost::asio::io_service &io_service, boost::shared_ptr state, ssl_support_t ssl_support); virtual ~connection_basic() noexcept(false); - //! \return `socket_stats` object passed in construction (ptr never changes). - socket_stats& get_stats() noexcept { return *m_stats; /* verified in constructor */ } - connection_basic(boost::asio::io_service& io_service, std::atomic &ref_sock_count, std::atomic &sock_number, ssl_support_t ssl, ssl_context_t &ssl_context); + //! \return `shared_state` object passed in construction (ptr never changes). + connection_basic_shared_state& get_state() noexcept { return *m_state; /* verified in constructor */ } + connection_basic(boost::asio::io_service& io_service, std::atomic &ref_sock_count, std::atomic &sock_number, ssl_support_t ssl); boost::asio::ip::tcp::socket& socket() { return socket_.next_layer(); } ssl_support_t get_ssl_support() const { return m_ssl_support; } @@ -118,7 +133,8 @@ class connection_basic { // not-templated base class for rapid developmet of som bool handshake(boost::asio::ssl::stream_base::handshake_type type) { - return ssl_handshake(socket_, type, m_ssl_context); + //m_state != nullptr verified in constructor + return m_state->ssl_options().handshake(socket_, type); } template diff --git a/contrib/epee/include/net/http_client.h b/contrib/epee/include/net/http_client.h index 36158b99f..a18a1d30a 100644 --- a/contrib/epee/include/net/http_client.h +++ b/contrib/epee/include/net/http_client.h @@ -275,11 +275,6 @@ namespace net_utils chunked_state m_chunked_state; std::string m_chunked_cache; critical_section m_lock; - epee::net_utils::ssl_support_t m_ssl_support; - std::pair m_ssl_private_key_and_certificate_path; - std::string m_ssl_ca_file; - std::vector> m_ssl_allowed_fingerprints; - bool m_ssl_allow_any_cert; public: explicit http_simple_client_template() @@ -297,34 +292,28 @@ namespace net_utils , m_chunked_state() , m_chunked_cache() , m_lock() - , m_ssl_support(epee::net_utils::ssl_support_t::e_ssl_support_autodetect) {} const std::string &get_host() const { return m_host_buff; }; const std::string &get_port() const { return m_port; }; - bool set_server(const std::string& address, boost::optional user, epee::net_utils::ssl_support_t ssl_support = epee::net_utils::ssl_support_t::e_ssl_support_autodetect, const std::pair &private_key_and_certificate_path = {}, std::string ca_file = {}, const std::vector> &allowed_ssl_fingerprints = {}, bool allow_any_cert = false) + bool set_server(const std::string& address, boost::optional user, ssl_options_t ssl_options = ssl_support_t::e_ssl_support_autodetect) { http::url_content parsed{}; const bool r = parse_url(address, parsed); CHECK_AND_ASSERT_MES(r, false, "failed to parse url: " << address); - set_server(std::move(parsed.host), std::to_string(parsed.port), std::move(user), ssl_support, private_key_and_certificate_path, std::move(ca_file), allowed_ssl_fingerprints, allow_any_cert); + set_server(std::move(parsed.host), std::to_string(parsed.port), std::move(user), std::move(ssl_options)); return true; } - void set_server(std::string host, std::string port, boost::optional user, epee::net_utils::ssl_support_t ssl_support = epee::net_utils::ssl_support_t::e_ssl_support_autodetect, const std::pair &private_key_and_certificate_path = {}, std::string ca_file = {}, const std::vector> &allowed_ssl_fingerprints = {}, bool allow_any_cert = false) + void set_server(std::string host, std::string port, boost::optional user, ssl_options_t ssl_options = ssl_support_t::e_ssl_support_autodetect) { CRITICAL_REGION_LOCAL(m_lock); disconnect(); m_host_buff = std::move(host); m_port = std::move(port); m_auth = user ? http_client_auth{std::move(*user)} : http_client_auth{}; - m_ssl_support = ssl_support; - m_ssl_private_key_and_certificate_path = private_key_and_certificate_path; - m_ssl_ca_file = std::move(ca_file); - m_ssl_allowed_fingerprints = allowed_ssl_fingerprints; - m_ssl_allow_any_cert = allow_any_cert; - m_net_client.set_ssl(m_ssl_support, m_ssl_private_key_and_certificate_path, m_ssl_ca_file, m_ssl_allowed_fingerprints, m_ssl_allow_any_cert); + m_net_client.set_ssl(std::move(ssl_options)); } template diff --git a/contrib/epee/include/net/http_server_impl_base.h b/contrib/epee/include/net/http_server_impl_base.h index 8a6b32943..fc2dcbf67 100644 --- a/contrib/epee/include/net/http_server_impl_base.h +++ b/contrib/epee/include/net/http_server_impl_base.h @@ -59,11 +59,7 @@ namespace epee bool init(std::function rng, const std::string& bind_port = "0", const std::string& bind_ip = "0.0.0.0", std::vector access_control_origins = std::vector(), boost::optional user = boost::none, - epee::net_utils::ssl_support_t ssl_support = epee::net_utils::ssl_support_t::e_ssl_support_autodetect, - const std::pair &private_key_and_certificate_path = {}, - const std::string &ca_path = {}, - std::vector> allowed_fingerprints = {}, - bool allow_any_cert = false) + net_utils::ssl_options_t ssl_options = net_utils::ssl_support_t::e_ssl_support_autodetect) { //set self as callback handler @@ -80,7 +76,7 @@ namespace epee m_net_server.get_config_object().m_user = std::move(user); MGINFO("Binding on " << bind_ip << ":" << bind_port); - bool res = m_net_server.init_server(bind_port, bind_ip, ssl_support, private_key_and_certificate_path, ca_path, std::move(allowed_fingerprints), allow_any_cert); + bool res = m_net_server.init_server(bind_port, bind_ip, std::move(ssl_options)); if(!res) { LOG_ERROR("Failed to bind server"); diff --git a/contrib/epee/include/net/net_helper.h b/contrib/epee/include/net/net_helper.h index 2b220bb0f..a9bfd6baa 100644 --- a/contrib/epee/include/net/net_helper.h +++ b/contrib/epee/include/net/net_helper.h @@ -101,10 +101,10 @@ namespace net_utils inline blocked_mode_client() : m_io_service(), - m_ctx({boost::asio::ssl::context(boost::asio::ssl::context::tlsv12), {}}), + m_ctx(boost::asio::ssl::context::tlsv12), m_connector(direct_connect{}), - m_ssl_socket(new boost::asio::ssl::stream(m_io_service, m_ctx.context)), - m_ssl_support(epee::net_utils::ssl_support_t::e_ssl_support_autodetect), + m_ssl_socket(new boost::asio::ssl::stream(m_io_service, m_ctx)), + m_ssl_options(epee::net_utils::ssl_support_t::e_ssl_support_autodetect), m_initialized(true), m_connected(false), m_deadline(m_io_service), @@ -136,13 +136,13 @@ namespace net_utils catch(...) { /* ignore */ } } - inline void set_ssl(epee::net_utils::ssl_support_t ssl_support = epee::net_utils::ssl_support_t::e_ssl_support_autodetect, const std::pair &private_key_and_certificate_path = {}, const std::string &ca_path = {}, std::vector> allowed_fingerprints = {}, bool allow_any_cert = false) + inline void set_ssl(ssl_options_t ssl_options) { - if (ssl_support == epee::net_utils::ssl_support_t::e_ssl_support_disabled) - m_ctx = {boost::asio::ssl::context(boost::asio::ssl::context::tlsv12), {}, {}}; + if (ssl_options) + m_ctx = ssl_options.create_context(); else - m_ctx = create_ssl_context(private_key_and_certificate_path, ca_path, std::move(allowed_fingerprints), allow_any_cert); - m_ssl_support = ssl_support; + m_ctx = boost::asio::ssl::context(boost::asio::ssl::context::tlsv12); + m_ssl_options = std::move(ssl_options); } inline @@ -174,7 +174,7 @@ namespace net_utils // SSL Options if (ssl_support == epee::net_utils::ssl_support_t::e_ssl_support_enabled || ssl_support == epee::net_utils::ssl_support_t::e_ssl_support_autodetect) { - if (!ssl_handshake(*m_ssl_socket, boost::asio::ssl::stream_base::client, m_ctx)) + if (!m_ssl_options.handshake(*m_ssl_socket, boost::asio::ssl::stream_base::client)) { if (ssl_support == epee::net_utils::ssl_support_t::e_ssl_support_autodetect) { @@ -191,7 +191,7 @@ namespace net_utils return CONNECT_FAILURE; } } - m_ssl_support = epee::net_utils::ssl_support_t::e_ssl_support_enabled; + m_ssl_options.support = ssl_support_t::e_ssl_support_enabled; } return CONNECT_SUCCESS; }else @@ -212,21 +212,21 @@ namespace net_utils // Set SSL options // disable sslv2 - m_ssl_socket.reset(new boost::asio::ssl::stream(m_io_service, m_ctx.context)); + m_ssl_socket.reset(new boost::asio::ssl::stream(m_io_service, m_ctx)); // Get a list of endpoints corresponding to the server name. - try_connect_result_t try_connect_result = try_connect(addr, port, timeout, m_ssl_support); + try_connect_result_t try_connect_result = try_connect(addr, port, timeout, m_ssl_options.support); if (try_connect_result == CONNECT_FAILURE) return false; - if (m_ssl_support == epee::net_utils::ssl_support_t::e_ssl_support_autodetect) + if (m_ssl_options.support == epee::net_utils::ssl_support_t::e_ssl_support_autodetect) { - m_ssl_support = epee::net_utils::ssl_support_t::e_ssl_support_enabled; + m_ssl_options.support = epee::net_utils::ssl_support_t::e_ssl_support_enabled; if (try_connect_result == CONNECT_NO_SSL) { MERROR("SSL handshake failed on an autodetect connection, reconnecting without SSL"); - m_ssl_support = epee::net_utils::ssl_support_t::e_ssl_support_disabled; - if (try_connect(addr, port, timeout, m_ssl_support) != CONNECT_SUCCESS) + m_ssl_options.support = epee::net_utils::ssl_support_t::e_ssl_support_disabled; + if (try_connect(addr, port, timeout, m_ssl_options.support) != CONNECT_SUCCESS) return false; } } @@ -258,7 +258,7 @@ namespace net_utils if(m_connected) { m_connected = false; - if(m_ssl_support != epee::net_utils::ssl_support_t::e_ssl_support_disabled) + if(m_ssl_options) shutdown_ssl(); m_ssl_socket->next_layer().shutdown(boost::asio::ip::tcp::socket::shutdown_both); } @@ -392,7 +392,7 @@ namespace net_utils if (!m_connected || !m_ssl_socket->next_layer().is_open()) return false; if (ssl) - *ssl = m_ssl_support == epee::net_utils::ssl_support_t::e_ssl_support_enabled; + *ssl = m_ssl_options.support == ssl_support_t::e_ssl_support_enabled; return true; } @@ -556,7 +556,7 @@ namespace net_utils { m_deadline.cancel(); boost::system::error_code ec; - if(m_ssl_support != epee::net_utils::ssl_support_t::e_ssl_support_disabled) + if(m_ssl_options.support != ssl_support_t::e_ssl_support_disabled) shutdown_ssl(); m_ssl_socket->next_layer().cancel(ec); if(ec) @@ -633,7 +633,7 @@ namespace net_utils bool write(const void* data, size_t sz, boost::system::error_code& ec) { bool success; - if(m_ssl_support == epee::net_utils::ssl_support_t::e_ssl_support_enabled) + if(m_ssl_options.support == ssl_support_t::e_ssl_support_enabled) success = boost::asio::write(*m_ssl_socket, boost::asio::buffer(data, sz), ec); else success = boost::asio::write(m_ssl_socket->next_layer(), boost::asio::buffer(data, sz), ec); @@ -642,7 +642,7 @@ namespace net_utils void async_write(const void* data, size_t sz, boost::system::error_code& ec) { - if(m_ssl_support == epee::net_utils::ssl_support_t::e_ssl_support_enabled) + if(m_ssl_options.support == ssl_support_t::e_ssl_support_enabled) boost::asio::async_write(*m_ssl_socket, boost::asio::buffer(data, sz), boost::lambda::var(ec) = boost::lambda::_1); else boost::asio::async_write(m_ssl_socket->next_layer(), boost::asio::buffer(data, sz), boost::lambda::var(ec) = boost::lambda::_1); @@ -650,7 +650,7 @@ namespace net_utils void async_read(char* buff, size_t sz, boost::asio::detail::transfer_at_least_t transfer_at_least, handler_obj& hndlr) { - if(m_ssl_support != epee::net_utils::ssl_support_t::e_ssl_support_enabled) + if(m_ssl_options.support != ssl_support_t::e_ssl_support_enabled) boost::asio::async_read(m_ssl_socket->next_layer(), boost::asio::buffer(buff, sz), transfer_at_least, hndlr); else boost::asio::async_read(*m_ssl_socket, boost::asio::buffer(buff, sz), transfer_at_least, hndlr); @@ -659,10 +659,10 @@ namespace net_utils protected: boost::asio::io_service m_io_service; - epee::net_utils::ssl_context_t m_ctx; + boost::asio::ssl::context m_ctx; std::shared_ptr> m_ssl_socket; std::function m_connector; - epee::net_utils::ssl_support_t m_ssl_support; + ssl_options_t m_ssl_options; bool m_initialized; bool m_connected; boost::asio::steady_timer m_deadline; diff --git a/contrib/epee/include/net/net_ssl.h b/contrib/epee/include/net/net_ssl.h index 1b90a948d..5107f4db6 100644 --- a/contrib/epee/include/net/net_ssl.h +++ b/contrib/epee/include/net/net_ssl.h @@ -46,23 +46,71 @@ namespace net_utils e_ssl_support_enabled, e_ssl_support_autodetect, }; - - struct ssl_context_t - { - boost::asio::ssl::context context; - std::string ca_path; - std::vector> allowed_fingerprints; - bool allow_any_cert; - }; + + enum class ssl_verification_t : uint8_t + { + none = 0, //!< Do not verify peer. + system_ca, //!< Verify peer via system ca only (do not inspect user certificates) + user_certificates //!< Verify peer via user certificate(s) only. + }; + + struct ssl_authentication_t + { + std::string private_key_path; //!< Private key used for authentication + std::string certificate_path; //!< Certificate used for authentication to peer. + + //! Load `private_key_path` and `certificate_path` into `ssl_context`. + void use_ssl_certificate(boost::asio::ssl::context &ssl_context) const; + }; + + /*! + \note `verification != disabled && support == disabled` is currently + "allowed" via public interface but obviously invalid configuation. + */ + class ssl_options_t + { + // force sorted behavior in private + std::vector> fingerprints_; + + public: + std::string ca_path; + ssl_authentication_t auth; + ssl_support_t support; + ssl_verification_t verification; + + //! Verification is set to system ca unless SSL is disabled. + ssl_options_t(ssl_support_t support) + : fingerprints_(), + ca_path(), + auth(), + support(support), + verification(support == ssl_support_t::e_ssl_support_disabled ? ssl_verification_t::none : ssl_verification_t::system_ca) + {} + + //! Provide user fingerprints and/or ca path. Enables SSL and user_certificate verification + ssl_options_t(std::vector> fingerprints, std::string ca_path); + + ssl_options_t(const ssl_options_t&) = default; + ssl_options_t(ssl_options_t&&) = default; + + ssl_options_t& operator=(const ssl_options_t&) = default; + ssl_options_t& operator=(ssl_options_t&&) = default; + + //! \return False iff ssl is disabled, otherwise true. + explicit operator bool() const noexcept { return support != ssl_support_t::e_ssl_support_disabled; } + + //! Search against internal fingerprints. Always false if `behavior() != user_certificate_check`. + bool has_fingerprint(boost::asio::ssl::verify_context &ctx) const; + + boost::asio::ssl::context create_context() const; + + bool handshake(boost::asio::ssl::stream &socket, boost::asio::ssl::stream_base::handshake_type type) const; + }; // https://security.stackexchange.com/questions/34780/checking-client-hello-for-https-classification constexpr size_t get_ssl_magic_size() { return 9; } bool is_ssl(const unsigned char *data, size_t len); - ssl_context_t create_ssl_context(const std::pair &private_key_and_certificate_path, const std::string &ca_path, std::vector> allowed_fingerprints, bool allow_any_cert); - void use_ssl_certificate(ssl_context_t &ssl_context, const std::pair &private_key_and_certificate_path); - bool is_certificate_allowed(boost::asio::ssl::verify_context &ctx, const ssl_context_t &ssl_context); - bool ssl_handshake(boost::asio::ssl::stream &socket, boost::asio::ssl::stream_base::handshake_type type, const epee::net_utils::ssl_context_t &ssl_context); - bool ssl_support_from_string(ssl_support_t &ssl, boost::string_ref s); + bool ssl_support_from_string(ssl_support_t &ssl, boost::string_ref s); } } diff --git a/contrib/epee/src/connection_basic.cpp b/contrib/epee/src/connection_basic.cpp index cafc08ead..19f2c7b02 100644 --- a/contrib/epee/src/connection_basic.cpp +++ b/contrib/epee/src/connection_basic.cpp @@ -65,6 +65,15 @@ namespace epee namespace net_utils { +namespace +{ + boost::asio::ssl::context& get_context(connection_basic_shared_state* state) + { + CHECK_AND_ASSERT_THROW_MES(state != nullptr, "state shared_ptr cannot be null"); + return state->ssl_context; + } +} + std::string to_string(t_connection_type type) { if (type == e_connection_type_NET) @@ -119,56 +128,54 @@ connection_basic_pimpl::connection_basic_pimpl(const std::string &name) : m_thro int connection_basic_pimpl::m_default_tos; // methods: -connection_basic::connection_basic(boost::asio::ip::tcp::socket&& sock, boost::shared_ptr stats, ssl_support_t ssl_support, ssl_context_t &ssl_context) +connection_basic::connection_basic(boost::asio::ip::tcp::socket&& sock, boost::shared_ptr state, ssl_support_t ssl_support) : - m_stats(std::move(stats)), + m_state(std::move(state)), mI( new connection_basic_pimpl("peer") ), strand_(GET_IO_SERVICE(sock)), - socket_(GET_IO_SERVICE(sock), ssl_context.context), + socket_(GET_IO_SERVICE(sock), get_context(m_state.get())), m_want_close_connection(false), m_was_shutdown(false), - m_ssl_support(ssl_support), - m_ssl_context(ssl_context) + m_ssl_support(ssl_support) { // add nullptr checks if removed - CHECK_AND_ASSERT_THROW_MES(bool(m_stats), "stats shared_ptr cannot be null"); + assert(m_state != nullptr); // release runtime check in get_context socket_.next_layer() = std::move(sock); - ++(m_stats->sock_count); // increase the global counter - mI->m_peer_number = m_stats->sock_number.fetch_add(1); // use, and increase the generated number + ++(m_state->sock_count); // increase the global counter + mI->m_peer_number = m_state->sock_number.fetch_add(1); // use, and increase the generated number std::string remote_addr_str = "?"; try { boost::system::error_code e; remote_addr_str = socket().remote_endpoint(e).address().to_string(); } catch(...){} ; - _note("Spawned connection #"<m_peer_number<<" to " << remote_addr_str << " currently we have sockets count:" << m_stats->sock_count); + _note("Spawned connection #"<m_peer_number<<" to " << remote_addr_str << " currently we have sockets count:" << m_state->sock_count); } -connection_basic::connection_basic(boost::asio::io_service &io_service, boost::shared_ptr stats, ssl_support_t ssl_support, ssl_context_t &ssl_context) +connection_basic::connection_basic(boost::asio::io_service &io_service, boost::shared_ptr state, ssl_support_t ssl_support) : - m_stats(std::move(stats)), + m_state(std::move(state)), mI( new connection_basic_pimpl("peer") ), strand_(io_service), - socket_(io_service, ssl_context.context), - m_want_close_connection(false), + socket_(io_service, get_context(m_state.get())), + m_want_close_connection(false), m_was_shutdown(false), - m_ssl_support(ssl_support), - m_ssl_context(ssl_context) + m_ssl_support(ssl_support) { // add nullptr checks if removed - CHECK_AND_ASSERT_THROW_MES(bool(m_stats), "stats shared_ptr cannot be null"); + assert(m_state != nullptr); // release runtime check in get_context - ++(m_stats->sock_count); // increase the global counter - mI->m_peer_number = m_stats->sock_number.fetch_add(1); // use, and increase the generated number + ++(m_state->sock_count); // increase the global counter + mI->m_peer_number = m_state->sock_number.fetch_add(1); // use, and increase the generated number std::string remote_addr_str = "?"; try { boost::system::error_code e; remote_addr_str = socket().remote_endpoint(e).address().to_string(); } catch(...){} ; - _note("Spawned connection #"<m_peer_number<<" to " << remote_addr_str << " currently we have sockets count:" << m_stats->sock_count); + _note("Spawned connection #"<m_peer_number<<" to " << remote_addr_str << " currently we have sockets count:" << m_state->sock_count); } connection_basic::~connection_basic() noexcept(false) { - --(m_stats->sock_count); + --(m_state->sock_count); std::string remote_addr_str = "?"; try { boost::system::error_code e; remote_addr_str = socket().remote_endpoint(e).address().to_string(); } catch(...){} ; diff --git a/contrib/epee/src/net_ssl.cpp b/contrib/epee/src/net_ssl.cpp index 6b156f238..1d137219a 100644 --- a/contrib/epee/src/net_ssl.cpp +++ b/contrib/epee/src/net_ssl.cpp @@ -171,22 +171,34 @@ bool create_ssl_certificate(EVP_PKEY *&pkey, X509 *&cert) return true; } -ssl_context_t create_ssl_context(const std::pair &private_key_and_certificate_path, const std::string &ca_path, std::vector> allowed_fingerprints, bool allow_any_cert) +ssl_options_t::ssl_options_t(std::vector> fingerprints, std::string ca_path) + : fingerprints_(std::move(fingerprints)), + ca_path(std::move(ca_path)), + auth(), + support(ssl_support_t::e_ssl_support_enabled), + verification(ssl_verification_t::user_certificates) { - ssl_context_t ssl_context{boost::asio::ssl::context(boost::asio::ssl::context::tlsv12), std::move(ca_path), std::move(allowed_fingerprints)}; + std::sort(fingerprints_.begin(), fingerprints_.end()); +} + +boost::asio::ssl::context ssl_options_t::create_context() const +{ + boost::asio::ssl::context ssl_context{boost::asio::ssl::context::tlsv12}; + if (!bool(*this)) + return ssl_context; // only allow tls v1.2 and up - ssl_context.context.set_options(boost::asio::ssl::context::default_workarounds); - ssl_context.context.set_options(boost::asio::ssl::context::no_sslv2); - ssl_context.context.set_options(boost::asio::ssl::context::no_sslv3); - ssl_context.context.set_options(boost::asio::ssl::context::no_tlsv1); - ssl_context.context.set_options(boost::asio::ssl::context::no_tlsv1_1); + ssl_context.set_options(boost::asio::ssl::context::default_workarounds); + ssl_context.set_options(boost::asio::ssl::context::no_sslv2); + ssl_context.set_options(boost::asio::ssl::context::no_sslv3); + ssl_context.set_options(boost::asio::ssl::context::no_tlsv1); + ssl_context.set_options(boost::asio::ssl::context::no_tlsv1_1); // only allow a select handful of tls v1.3 and v1.2 ciphers to be used - SSL_CTX_set_cipher_list(ssl_context.context.native_handle(), "ECDHE-ECDSA-CHACHA20-POLY1305-SHA256:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES256-SHA384:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-CHACHA20-POLY1305"); + SSL_CTX_set_cipher_list(ssl_context.native_handle(), "ECDHE-ECDSA-CHACHA20-POLY1305-SHA256:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES256-SHA384:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-CHACHA20-POLY1305"); // set options on the SSL context for added security - SSL_CTX *ctx = ssl_context.context.native_handle(); + SSL_CTX *ctx = ssl_context.native_handle(); CHECK_AND_ASSERT_THROW_MES(ctx, "Failed to get SSL context"); SSL_CTX_clear_options(ctx, SSL_OP_LEGACY_SERVER_CONNECT); // SSL_CTX_SET_OPTIONS(3) SSL_CTX_set_session_cache_mode(ctx, SSL_SESS_CACHE_OFF); // https://stackoverflow.com/questions/22378442 @@ -203,17 +215,25 @@ ssl_context_t create_ssl_context(const std::pair &priv SSL_CTX_set_options(ctx, SSL_OP_NO_COMPRESSION); #endif - if (!ssl_context.ca_path.empty()) + switch (verification) { - const boost::system::error_code err = load_ca_file(ssl_context.context, ssl_context.ca_path); - if (err) - throw boost::system::system_error{err, "Failed to load user CA file at " + ssl_context.ca_path}; + case ssl_verification_t::system_ca: + ssl_context.set_default_verify_paths(); + break; + case ssl_verification_t::user_certificates: + if (!ca_path.empty()) + { + const boost::system::error_code err = load_ca_file(ssl_context, ca_path); + if (err) + throw boost::system::system_error{err, "Failed to load user CA file at " + ca_path}; + } + break; + default: + break; } - else if (allowed_fingerprints.empty()) - ssl_context.context.set_default_verify_paths(); // only use system-CAs if no user-supplied cert info - CHECK_AND_ASSERT_THROW_MES(private_key_and_certificate_path.first.empty() == private_key_and_certificate_path.second.empty(), "private key and certificate must be either both given or both empty"); - if (private_key_and_certificate_path.second.empty()) + CHECK_AND_ASSERT_THROW_MES(auth.private_key_path.empty() == auth.certificate_path.empty(), "private key and certificate must be either both given or both empty"); + if (auth.private_key_path.empty()) { EVP_PKEY *pkey; X509 *cert; @@ -224,19 +244,15 @@ ssl_context_t create_ssl_context(const std::pair &priv EVP_PKEY_free(pkey); } else - { - ssl_context.context.use_private_key_file(private_key_and_certificate_path.first, boost::asio::ssl::context::pem); - ssl_context.context.use_certificate_file(private_key_and_certificate_path.second, boost::asio::ssl::context::pem); - } - ssl_context.allow_any_cert = allow_any_cert; + auth.use_ssl_certificate(ssl_context); return ssl_context; } -void use_ssl_certificate(ssl_context_t &ssl_context, const std::pair &private_key_and_certificate_path) +void ssl_authentication_t::use_ssl_certificate(boost::asio::ssl::context &ssl_context) const { - ssl_context.context.use_private_key_file(private_key_and_certificate_path.first, boost::asio::ssl::context::pem); - ssl_context.context.use_certificate_file(private_key_and_certificate_path.second, boost::asio::ssl::context::pem); + ssl_context.use_private_key_file(private_key_path, boost::asio::ssl::context::pem); + ssl_context.use_certificate_file(certificate_path, boost::asio::ssl::context::pem); } bool is_ssl(const unsigned char *data, size_t len) @@ -259,10 +275,10 @@ bool is_ssl(const unsigned char *data, size_t len) return false; } -bool is_certificate_allowed(boost::asio::ssl::verify_context &ctx, const ssl_context_t &ssl_context) +bool ssl_options_t::has_fingerprint(boost::asio::ssl::verify_context &ctx) const { // can we check the certificate against a list of fingerprints? - if (!ssl_context.allowed_fingerprints.empty()) { + if (!fingerprints_.empty()) { X509_STORE_CTX *sctx = ctx.native_handle(); if (!sctx) { @@ -289,15 +305,13 @@ bool is_certificate_allowed(boost::asio::ssl::verify_context &ctx, const ssl_con // strip unnecessary bytes from the digest digest.resize(size); - // is the certificate fingerprint inside the list of allowed fingerprints? - if (std::find(ssl_context.allowed_fingerprints.begin(), ssl_context.allowed_fingerprints.end(), digest) != ssl_context.allowed_fingerprints.end()) - return true; + return std::binary_search(fingerprints_.begin(), fingerprints_.end(), digest); } - return ssl_context.allowed_fingerprints.empty() && ssl_context.ca_path.empty(); + return false; } -bool ssl_handshake(boost::asio::ssl::stream &socket, boost::asio::ssl::stream_base::handshake_type type, const epee::net_utils::ssl_context_t &ssl_context) +bool ssl_options_t::handshake(boost::asio::ssl::stream &socket, boost::asio::ssl::stream_base::handshake_type type) const { bool verified = false; socket.next_layer().set_option(boost::asio::ip::tcp::no_delay(true)); @@ -306,8 +320,8 @@ bool ssl_handshake(boost::asio::ssl::stream &socke no expected hostname for server to verify against. If server doesn't have specific whitelisted certificates for client, don't require client to send certificate at all. */ - const bool no_verification = ssl_context.allow_any_cert || - (type == boost::asio::ssl::stream_base::server && ssl_context.allowed_fingerprints.empty() && ssl_context.ca_path.empty()); + const bool no_verification = verification == ssl_verification_t::none || + (type == boost::asio::ssl::stream_base::server && fingerprints_.empty() && ca_path.empty()); /* According to OpenSSL documentation (and SSL specifications), server must always send certificate unless "anonymous" cipher mode is used which are @@ -321,7 +335,7 @@ bool ssl_handshake(boost::asio::ssl::stream &socke { // preverified means it passed system or user CA check. System CA is never loaded // when fingerprints are whitelisted. - if (!preverified && !is_certificate_allowed(ctx, ssl_context)) { + if (!preverified && verification == ssl_verification_t::user_certificates && !has_fingerprint(ctx)) { MERROR("Certificate is not in the allowed list, connection droppped"); return false; } @@ -337,7 +351,7 @@ bool ssl_handshake(boost::asio::ssl::stream &socke MERROR("handshake failed, connection dropped: " << ec.message()); return false; } - if (!no_verification && !verified) + if (verification == ssl_verification_t::none && !verified) { MERROR("Peer did not provide a certificate in the allowed list, connection dropped"); return false; diff --git a/src/rpc/core_rpc_server.cpp b/src/rpc/core_rpc_server.cpp index 0a45fca27..161ad2951 100644 --- a/src/rpc/core_rpc_server.cpp +++ b/src/rpc/core_rpc_server.cpp @@ -149,21 +149,29 @@ namespace cryptonote if (rpc_config->login) http_login.emplace(std::move(rpc_config->login->username), std::move(rpc_config->login->password).password()); - const std::string ssl_private_key = command_line::get_arg(vm, arg_rpc_ssl_private_key); - const std::string ssl_certificate = command_line::get_arg(vm, arg_rpc_ssl_certificate); - std::string ssl_ca_path = command_line::get_arg(vm, arg_rpc_ssl_ca_certificates); + epee::net_utils::ssl_options_t ssl_options = epee::net_utils::ssl_support_t::e_ssl_support_autodetect; + if (command_line::get_arg(vm, arg_rpc_ssl_allow_any_cert)) + ssl_options.verification = epee::net_utils::ssl_verification_t::none; + else + { + std::string ssl_ca_path = command_line::get_arg(vm, arg_rpc_ssl_ca_certificates); + const std::vector ssl_allowed_fingerprint_strings = command_line::get_arg(vm, arg_rpc_ssl_allowed_fingerprints); + std::vector> ssl_allowed_fingerprints{ ssl_allowed_fingerprint_strings.size() }; + std::transform(ssl_allowed_fingerprint_strings.begin(), ssl_allowed_fingerprint_strings.end(), ssl_allowed_fingerprints.begin(), epee::from_hex::vector); - const std::vector ssl_allowed_fingerprint_strings = command_line::get_arg(vm, arg_rpc_ssl_allowed_fingerprints); - std::vector> ssl_allowed_fingerprints{ ssl_allowed_fingerprint_strings.size() }; - std::transform(ssl_allowed_fingerprint_strings.begin(), ssl_allowed_fingerprint_strings.end(), ssl_allowed_fingerprints.begin(), epee::from_hex::vector); - const bool ssl_allow_any_cert = command_line::get_arg(vm, arg_rpc_ssl_allow_any_cert); + if (!ssl_ca_path.empty() || !ssl_allowed_fingerprints.empty()) + ssl_options = epee::net_utils::ssl_options_t{std::move(ssl_allowed_fingerprints), std::move(ssl_ca_path)}; + } + + ssl_options.auth = epee::net_utils::ssl_authentication_t{ + command_line::get_arg(vm, arg_rpc_ssl_private_key), command_line::get_arg(vm, arg_rpc_ssl_certificate) + }; // user specified CA file or fingeprints implies enabled SSL by default - epee::net_utils::ssl_support_t ssl_support = epee::net_utils::ssl_support_t::e_ssl_support_enabled; - if ((ssl_allowed_fingerprints.empty() && ssl_ca_path.empty()) || !command_line::is_arg_defaulted(vm, arg_rpc_ssl)) + if (ssl_options.verification != epee::net_utils::ssl_verification_t::user_certificates || !command_line::is_arg_defaulted(vm, arg_rpc_ssl)) { const std::string ssl = command_line::get_arg(vm, arg_rpc_ssl); - if (!epee::net_utils::ssl_support_from_string(ssl_support, ssl)) + if (!epee::net_utils::ssl_support_from_string(ssl_options.support, ssl)) { MFATAL("Invalid RPC SSL support: " << ssl); return false; @@ -172,8 +180,7 @@ namespace cryptonote auto rng = [](size_t len, uint8_t *ptr){ return crypto::rand(len, ptr); }; return epee::http_server_impl_base::init( - rng, std::move(port), std::move(rpc_config->bind_ip), std::move(rpc_config->access_control_origins), std::move(http_login), - ssl_support, std::make_pair(ssl_private_key, ssl_certificate), std::move(ssl_ca_path), std::move(ssl_allowed_fingerprints), ssl_allow_any_cert + rng, std::move(port), std::move(rpc_config->bind_ip), std::move(rpc_config->access_control_origins), std::move(http_login), std::move(ssl_options) ); } //------------------------------------------------------------------------------------------------------------------------------ diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index 9e2a826d9..9f2cd2a41 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -327,13 +327,29 @@ std::unique_ptr make_basic(const boost::program_options::variabl auto daemon_ssl = command_line::get_arg(vm, opts.daemon_ssl); // user specified CA file or fingeprints implies enabled SSL by default - epee::net_utils::ssl_support_t ssl_support = epee::net_utils::ssl_support_t::e_ssl_support_enabled; - if ((daemon_ssl_ca_file.empty() && daemon_ssl_allowed_fingerprints.empty()) || !command_line::is_arg_defaulted(vm, opts.daemon_ssl)) + epee::net_utils::ssl_options_t ssl_options = epee::net_utils::ssl_support_t::e_ssl_support_enabled; + if (command_line::get_arg(vm, opts.daemon_ssl_allow_any_cert)) + ssl_options.verification = epee::net_utils::ssl_verification_t::none; + else if (!daemon_ssl_ca_file.empty() || !daemon_ssl_allowed_fingerprints.empty()) { - THROW_WALLET_EXCEPTION_IF(!epee::net_utils::ssl_support_from_string(ssl_support, daemon_ssl), tools::error::wallet_internal_error, + std::vector> ssl_allowed_fingerprints{ daemon_ssl_allowed_fingerprints.size() }; + std::transform(daemon_ssl_allowed_fingerprints.begin(), daemon_ssl_allowed_fingerprints.end(), ssl_allowed_fingerprints.begin(), epee::from_hex::vector); + + ssl_options = epee::net_utils::ssl_options_t{ + std::move(ssl_allowed_fingerprints), std::move(daemon_ssl_ca_file) + }; + } + + if (ssl_options.verification != epee::net_utils::ssl_verification_t::user_certificates || !command_line::is_arg_defaulted(vm, opts.daemon_ssl)) + { + THROW_WALLET_EXCEPTION_IF(!epee::net_utils::ssl_support_from_string(ssl_options.support, daemon_ssl), tools::error::wallet_internal_error, tools::wallet2::tr("Invalid argument for ") + std::string(opts.daemon_ssl.name)); } + ssl_options.auth = epee::net_utils::ssl_authentication_t{ + std::move(daemon_ssl_private_key), std::move(daemon_ssl_certificate) + }; + THROW_WALLET_EXCEPTION_IF(!daemon_address.empty() && !daemon_host.empty() && 0 != daemon_port, tools::error::wallet_internal_error, tools::wallet2::tr("can't specify daemon host or port more than once")); @@ -421,11 +437,8 @@ std::unique_ptr make_basic(const boost::program_options::variabl catch (const std::exception &e) { } } - std::vector> ssl_allowed_fingerprints{ daemon_ssl_allowed_fingerprints.size() }; - std::transform(daemon_ssl_allowed_fingerprints.begin(), daemon_ssl_allowed_fingerprints.end(), ssl_allowed_fingerprints.begin(), epee::from_hex::vector); - std::unique_ptr wallet(new tools::wallet2(nettype, kdf_rounds, unattended)); - wallet->init(std::move(daemon_address), std::move(login), std::move(proxy), 0, *trusted_daemon, ssl_support, std::make_pair(daemon_ssl_private_key, daemon_ssl_certificate), std::move(daemon_ssl_ca_file), ssl_allowed_fingerprints, daemon_ssl_allow_any_cert); + wallet->init(std::move(daemon_address), std::move(login), std::move(proxy), 0, *trusted_daemon, std::move(ssl_options)); boost::filesystem::path ringdb_path = command_line::get_arg(vm, opts.shared_ringdb_dir); wallet->set_ring_database(ringdb_path.string()); wallet->get_message_store().set_options(vm); @@ -1148,10 +1161,7 @@ std::unique_ptr wallet2::make_dummy(const boost::program_options::varia } //---------------------------------------------------------------------------------------------------- -bool wallet2::set_daemon(std::string daemon_address, boost::optional daemon_login, bool trusted_daemon, - epee::net_utils::ssl_support_t ssl_support, const std::pair &private_key_and_certificate_path, - std::string ca_file, const std::vector> &allowed_fingerprints, - bool allow_any_cert) +bool wallet2::set_daemon(std::string daemon_address, boost::optional daemon_login, bool trusted_daemon, epee::net_utils::ssl_options_t ssl_options) { if(m_http_client.is_connected()) m_http_client.disconnect(); @@ -1160,17 +1170,17 @@ bool wallet2::set_daemon(std::string daemon_address, boost::optional daemon_login, boost::asio::ip::tcp::endpoint proxy, uint64_t upper_transaction_weight_limit, bool trusted_daemon, epee::net_utils::ssl_support_t ssl_support, const std::pair &private_key_and_certificate_path, std::string ca_file, const std::vector> &allowed_fingerprints, bool allow_any_cert) +bool wallet2::init(std::string daemon_address, boost::optional daemon_login, boost::asio::ip::tcp::endpoint proxy, uint64_t upper_transaction_weight_limit, bool trusted_daemon, epee::net_utils::ssl_options_t ssl_options) { m_checkpoints.init_default_checkpoints(m_nettype); m_is_initialized = true; m_upper_transaction_weight_limit = upper_transaction_weight_limit; if (proxy != boost::asio::ip::tcp::endpoint{}) m_http_client.set_connector(net::socks::connector{std::move(proxy)}); - return set_daemon(daemon_address, daemon_login, trusted_daemon, ssl_support, private_key_and_certificate_path, std::move(ca_file), allowed_fingerprints, allow_any_cert); + return set_daemon(daemon_address, daemon_login, trusted_daemon, std::move(ssl_options)); } //---------------------------------------------------------------------------------------------------- bool wallet2::is_deterministic() const diff --git a/src/wallet/wallet2.h b/src/wallet/wallet2.h index 58dc5f082..38cd68784 100644 --- a/src/wallet/wallet2.h +++ b/src/wallet/wallet2.h @@ -689,16 +689,10 @@ namespace tools boost::asio::ip::tcp::endpoint proxy = {}, uint64_t upper_transaction_weight_limit = 0, bool trusted_daemon = true, - epee::net_utils::ssl_support_t ssl_support = epee::net_utils::ssl_support_t::e_ssl_support_autodetect, - const std::pair &private_key_and_certificate_path = {}, - std::string ca_file = {}, const std::vector> &allowed_fingerprints = {}, - bool allow_any_cert = false); + epee::net_utils::ssl_options_t ssl_options = epee::net_utils::ssl_support_t::e_ssl_support_autodetect); bool set_daemon(std::string daemon_address = "http://localhost:8080", boost::optional daemon_login = boost::none, bool trusted_daemon = true, - epee::net_utils::ssl_support_t ssl_support = epee::net_utils::ssl_support_t::e_ssl_support_autodetect, - const std::pair &private_key_and_certificate_path = {}, - std::string ca_file = {}, const std::vector> &allowed_fingerprints = {}, - bool allow_any_cert = false); + epee::net_utils::ssl_options_t ssl_options = epee::net_utils::ssl_support_t::e_ssl_support_autodetect); void stop() { m_run.store(false, std::memory_order_relaxed); m_message_store.stop(); } diff --git a/src/wallet/wallet_rpc_server.cpp b/src/wallet/wallet_rpc_server.cpp index a07e8e767..474e0d101 100644 --- a/src/wallet/wallet_rpc_server.cpp +++ b/src/wallet/wallet_rpc_server.cpp @@ -250,20 +250,31 @@ namespace tools auto rpc_ssl_ca_file = command_line::get_arg(vm, arg_rpc_ssl_ca_certificates); auto rpc_ssl_allowed_fingerprints = command_line::get_arg(vm, arg_rpc_ssl_allowed_fingerprints); auto rpc_ssl = command_line::get_arg(vm, arg_rpc_ssl); - epee::net_utils::ssl_support_t rpc_ssl_support = epee::net_utils::ssl_support_t::e_ssl_support_enabled; + epee::net_utils::ssl_options_t rpc_ssl_options = epee::net_utils::ssl_support_t::e_ssl_support_enabled; + + if (!rpc_ssl_ca_file.empty() || !rpc_ssl_allowed_fingerprints.empty()) + { + std::vector> allowed_fingerprints{ rpc_ssl_allowed_fingerprints.size() }; + std::transform(rpc_ssl_allowed_fingerprints.begin(), rpc_ssl_allowed_fingerprints.end(), allowed_fingerprints.begin(), epee::from_hex::vector); + + rpc_ssl_options = epee::net_utils::ssl_options_t{ + std::move(allowed_fingerprints), std::move(rpc_ssl_ca_file) + }; + } // user specified CA file or fingeprints implies enabled SSL by default - if ((rpc_ssl_ca_file.empty() && rpc_ssl_allowed_fingerprints.empty()) || !command_line::is_arg_defaulted(vm, arg_rpc_ssl)) + if (rpc_ssl_options.verification != epee::net_utils::ssl_verification_t::user_certificates || !command_line::is_arg_defaulted(vm, arg_rpc_ssl)) { - if (!epee::net_utils::ssl_support_from_string(rpc_ssl_support, rpc_ssl)) + if (!epee::net_utils::ssl_support_from_string(rpc_ssl_options.support, rpc_ssl)) { MERROR("Invalid argument for " << std::string(arg_rpc_ssl.name)); return false; } } - std::vector> allowed_fingerprints{ rpc_ssl_allowed_fingerprints.size() }; - std::transform(rpc_ssl_allowed_fingerprints.begin(), rpc_ssl_allowed_fingerprints.end(), allowed_fingerprints.begin(), epee::from_hex::vector); + rpc_ssl_options.auth = epee::net_utils::ssl_authentication_t{ + std::move(rpc_ssl_private_key), std::move(rpc_ssl_certificate) + }; m_auto_refresh_period = DEFAULT_AUTO_REFRESH_PERIOD; m_last_auto_refresh_time = boost::posix_time::min_date_time; @@ -272,7 +283,7 @@ namespace tools auto rng = [](size_t len, uint8_t *ptr) { return crypto::rand(len, ptr); }; return epee::http_server_impl_base::init( rng, std::move(bind_port), std::move(rpc_config->bind_ip), std::move(rpc_config->access_control_origins), std::move(http_login), - rpc_ssl_support, std::make_pair(rpc_ssl_private_key, rpc_ssl_certificate), std::move(rpc_ssl_ca_file), std::move(allowed_fingerprints) + std::move(rpc_ssl_options) ); } //------------------------------------------------------------------------------------------------------------------------------ @@ -4049,8 +4060,8 @@ namespace tools er.message = "Command unavailable in restricted mode."; return false; } - epee::net_utils::ssl_support_t ssl_support; - if (!epee::net_utils::ssl_support_from_string(ssl_support, req.ssl_support)) + epee::net_utils::ssl_options_t ssl_options = epee::net_utils::ssl_support_t::e_ssl_support_enabled; + if (!epee::net_utils::ssl_support_from_string(ssl_options.support, req.ssl_support)) { er.code = WALLET_RPC_ERROR_CODE_NO_DAEMON_CONNECTION; er.message = std::string("Invalid ssl support mode"); @@ -4065,7 +4076,16 @@ namespace tools for (auto c: fp) v.push_back(c); } - if (!m_wallet->set_daemon(req.address, boost::none, req.trusted, ssl_support, std::make_pair(req.ssl_private_key_path, req.ssl_certificate_path), std::move(req.ssl_ca_file), ssl_allowed_fingerprints, req.ssl_allow_any_cert)) + if (req.ssl_allow_any_cert) + ssl_options.verification = epee::net_utils::ssl_verification_t::none; + else if (!ssl_allowed_fingerprints.empty() || !req.ssl_ca_file.empty()) + ssl_options = epee::net_utils::ssl_options_t{std::move(ssl_allowed_fingerprints), std::move(req.ssl_ca_file)}; + + ssl_options.auth = epee::net_utils::ssl_authentication_t{ + std::move(req.ssl_private_key_path), std::move(req.ssl_certificate_path) + }; + + if (!m_wallet->set_daemon(req.address, boost::none, req.trusted, std::move(ssl_options))) { er.code = WALLET_RPC_ERROR_CODE_NO_DAEMON_CONNECTION; er.message = std::string("Unable to set daemon"); From 96d602ac84d856c26a9065bfccbe2b98237db271 Mon Sep 17 00:00:00 2001 From: Lee Clagett Date: Sun, 17 Mar 2019 16:11:42 -0400 Subject: [PATCH 05/11] Add `verify_fail_if_no_cert` option for proper client authentication Using `verify_peer` on server side requests a certificate from the client. If no certificate is provided, the server silently accepts the connection and rejects if the client sends an unexpected certificate. Adding `verify_fail_if_no_cert` has no affect on client and for server requires that the peer sends a certificate or fails the handshake. This is the desired behavior when the user specifies a fingerprint or CA file. --- contrib/epee/src/net_ssl.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/epee/src/net_ssl.cpp b/contrib/epee/src/net_ssl.cpp index 1d137219a..a87792fb8 100644 --- a/contrib/epee/src/net_ssl.cpp +++ b/contrib/epee/src/net_ssl.cpp @@ -330,7 +330,7 @@ bool ssl_options_t::handshake(boost::asio::ssl::stream Date: Sun, 17 Mar 2019 22:06:36 -0400 Subject: [PATCH 06/11] Require server verification when SSL is enabled. If SSL is "enabled" via command line without specifying a fingerprint or certificate, the system CA list is checked for server verification and _now_ fails the handshake if that check fails. This change was made to remain consistent with standard SSL/TLS client behavior. This can still be overridden by using the allow any certificate flag. If the SSL behavior is autodetect, the system CA list is still checked but a warning is logged if this fails. The stream is not rejected because a re-connect will be attempted - its better to have an unverified encrypted stream than an unverified + unencrypted stream. --- contrib/epee/include/net/net_ssl.h | 6 ++++++ contrib/epee/src/net_ssl.cpp | 21 ++++++++++----------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/contrib/epee/include/net/net_ssl.h b/contrib/epee/include/net/net_ssl.h index 5107f4db6..f36755013 100644 --- a/contrib/epee/include/net/net_ssl.h +++ b/contrib/epee/include/net/net_ssl.h @@ -104,6 +104,12 @@ namespace net_utils boost::asio::ssl::context create_context() const; + /*! \note If `this->support == autodetect && this->verification != none`, + then the handshake will not fail when peer verification fails. The + assumption is that a re-connect will be attempted, so a warning is + logged instead of failure. + \return True if the SSL handshake completes with peer verification + settings. */ bool handshake(boost::asio::ssl::stream &socket, boost::asio::ssl::stream_base::handshake_type type) const; }; diff --git a/contrib/epee/src/net_ssl.cpp b/contrib/epee/src/net_ssl.cpp index a87792fb8..cf8fa68ee 100644 --- a/contrib/epee/src/net_ssl.cpp +++ b/contrib/epee/src/net_ssl.cpp @@ -313,7 +313,6 @@ bool ssl_options_t::has_fingerprint(boost::asio::ssl::verify_context &ctx) const bool ssl_options_t::handshake(boost::asio::ssl::stream &socket, boost::asio::ssl::stream_base::handshake_type type) const { - bool verified = false; socket.next_layer().set_option(boost::asio::ip::tcp::no_delay(true)); /* Using system-wide CA store for client verification is funky - there is @@ -335,11 +334,16 @@ bool ssl_options_t::handshake(boost::asio::ssl::stream Date: Tue, 19 Mar 2019 16:04:32 -0400 Subject: [PATCH 07/11] Perform RFC 2818 hostname verification in client SSL handshakes If the verification mode is `system_ca`, clients will now do hostname verification. Thus, only certificates from expected hostnames are allowed when SSL is enabled. This can be overridden by forcible setting the SSL mode to autodetect. Clients will also send the hostname even when `system_ca` is not being performed. This leaks possible metadata, but allows servers providing multiple hostnames to respond with the correct certificate. One example is cloudflare, which getmonero.org is currently using. --- contrib/epee/include/net/net_helper.h | 2 +- contrib/epee/include/net/net_ssl.h | 14 +++++++++++++- contrib/epee/src/net_ssl.cpp | 15 ++++++++++++--- 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/contrib/epee/include/net/net_helper.h b/contrib/epee/include/net/net_helper.h index a9bfd6baa..66a307c97 100644 --- a/contrib/epee/include/net/net_helper.h +++ b/contrib/epee/include/net/net_helper.h @@ -174,7 +174,7 @@ namespace net_utils // SSL Options if (ssl_support == epee::net_utils::ssl_support_t::e_ssl_support_enabled || ssl_support == epee::net_utils::ssl_support_t::e_ssl_support_autodetect) { - if (!m_ssl_options.handshake(*m_ssl_socket, boost::asio::ssl::stream_base::client)) + if (!m_ssl_options.handshake(*m_ssl_socket, boost::asio::ssl::stream_base::client, addr)) { if (ssl_support == epee::net_utils::ssl_support_t::e_ssl_support_autodetect) { diff --git a/contrib/epee/include/net/net_ssl.h b/contrib/epee/include/net/net_ssl.h index f36755013..ba6e2ee6d 100644 --- a/contrib/epee/include/net/net_ssl.h +++ b/contrib/epee/include/net/net_ssl.h @@ -108,9 +108,21 @@ namespace net_utils then the handshake will not fail when peer verification fails. The assumption is that a re-connect will be attempted, so a warning is logged instead of failure. + + \note It is strongly encouraged that clients using `system_ca` + verification provide a non-empty `host` for rfc2818 verification. + + \param socket Used in SSL handshake and verification + \param type Client or server + \param host This parameter is only used when + `type == client && !host.empty()`. The value is sent to the server for + situations where multiple hostnames are being handled by a server. If + `verification == system_ca` the client also does a rfc2818 check to + ensure that the server certificate is to the provided hostname. + \return True if the SSL handshake completes with peer verification settings. */ - bool handshake(boost::asio::ssl::stream &socket, boost::asio::ssl::stream_base::handshake_type type) const; + bool handshake(boost::asio::ssl::stream &socket, boost::asio::ssl::stream_base::handshake_type type, const std::string& host = {}) const; }; // https://security.stackexchange.com/questions/34780/checking-client-hello-for-https-classification diff --git a/contrib/epee/src/net_ssl.cpp b/contrib/epee/src/net_ssl.cpp index cf8fa68ee..fbc809043 100644 --- a/contrib/epee/src/net_ssl.cpp +++ b/contrib/epee/src/net_ssl.cpp @@ -311,7 +311,7 @@ bool ssl_options_t::has_fingerprint(boost::asio::ssl::verify_context &ctx) const return false; } -bool ssl_options_t::handshake(boost::asio::ssl::stream &socket, boost::asio::ssl::stream_base::handshake_type type) const +bool ssl_options_t::handshake(boost::asio::ssl::stream &socket, boost::asio::ssl::stream_base::handshake_type type, const std::string& host) const { socket.next_layer().set_option(boost::asio::ip::tcp::no_delay(true)); @@ -330,11 +330,20 @@ bool ssl_options_t::handshake(boost::asio::ssl::stream Date: Wed, 20 Mar 2019 01:26:36 -0400 Subject: [PATCH 08/11] Call `use_certificate_chain_file` instead of `use_certificate_file` The former has the same behavior with single self signed certificates while allowing the server to have separate short-term authentication keys with long-term authorization keys. --- contrib/epee/src/net_ssl.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/epee/src/net_ssl.cpp b/contrib/epee/src/net_ssl.cpp index fbc809043..0ac452343 100644 --- a/contrib/epee/src/net_ssl.cpp +++ b/contrib/epee/src/net_ssl.cpp @@ -252,7 +252,7 @@ boost::asio::ssl::context ssl_options_t::create_context() const void ssl_authentication_t::use_ssl_certificate(boost::asio::ssl::context &ssl_context) const { ssl_context.use_private_key_file(private_key_path, boost::asio::ssl::context::pem); - ssl_context.use_certificate_file(certificate_path, boost::asio::ssl::context::pem); + ssl_context.use_certificate_chain_file(certificate_path); } bool is_ssl(const unsigned char *data, size_t len) From 97cd1fa98d58fa354ebbade47e894f169ee0c1e2 Mon Sep 17 00:00:00 2001 From: Lee Clagett Date: Thu, 4 Apr 2019 01:48:55 -0400 Subject: [PATCH 09/11] Only check top-level certificate against fingerprint list. This allows "chain" certificates to be used with the fingerprint whitelist option. A user can get a system-ca signature as backup while clients explicitly whitelist the server certificate. The user specified CA can also be combined with fingerprint whitelisting. --- contrib/epee/src/net_ssl.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/contrib/epee/src/net_ssl.cpp b/contrib/epee/src/net_ssl.cpp index 0ac452343..77eaa43e2 100644 --- a/contrib/epee/src/net_ssl.cpp +++ b/contrib/epee/src/net_ssl.cpp @@ -285,8 +285,10 @@ bool ssl_options_t::has_fingerprint(boost::asio::ssl::verify_context &ctx) const MERROR("Error getting verify_context handle"); return false; } - X509 *cert =X509_STORE_CTX_get_current_cert(sctx); - if (!cert) + + X509* cert = nullptr; + const STACK_OF(X509)* chain = X509_STORE_CTX_get_chain(sctx); + if (!chain || sk_X509_num(chain) < 1 || !(cert = sk_X509_value(chain, 0))) { MERROR("No certificate found in verify_context"); return false; From d58f368289709e0869c9b7927778339670cb85a7 Mon Sep 17 00:00:00 2001 From: Lee Clagett Date: Thu, 4 Apr 2019 13:35:33 -0400 Subject: [PATCH 10/11] Require manual override for user chain certificates. An override for the wallet to daemon connection is provided, but not for other SSL contexts. The intent is to prevent users from supplying a system CA as the "user" whitelisted certificate, which is less secure since the key is controlled by a third party. --- contrib/epee/include/net/net_ssl.h | 3 ++- contrib/epee/src/net_ssl.cpp | 3 +++ src/wallet/wallet2.cpp | 5 +++++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/contrib/epee/include/net/net_ssl.h b/contrib/epee/include/net/net_ssl.h index ba6e2ee6d..726dcb61a 100644 --- a/contrib/epee/include/net/net_ssl.h +++ b/contrib/epee/include/net/net_ssl.h @@ -51,7 +51,8 @@ namespace net_utils { none = 0, //!< Do not verify peer. system_ca, //!< Verify peer via system ca only (do not inspect user certificates) - user_certificates //!< Verify peer via user certificate(s) only. + user_certificates,//!< Verify peer via specific (non-chain) certificate(s) only. + user_ca //!< Verify peer via specific (possibly chain) certificate(s) only. }; struct ssl_authentication_t diff --git a/contrib/epee/src/net_ssl.cpp b/contrib/epee/src/net_ssl.cpp index 77eaa43e2..1bc6f91b8 100644 --- a/contrib/epee/src/net_ssl.cpp +++ b/contrib/epee/src/net_ssl.cpp @@ -221,6 +221,9 @@ boost::asio::ssl::context ssl_options_t::create_context() const ssl_context.set_default_verify_paths(); break; case ssl_verification_t::user_certificates: + ssl_context.set_verify_depth(0); + /* fallthrough */ + case ssl_verification_t::user_ca: if (!ca_path.empty()) { const boost::system::error_code err = load_ca_file(ssl_context, ca_path); diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index 9f2cd2a41..2939ed8a4 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -246,6 +246,7 @@ struct options { const command_line::arg_descriptor daemon_ssl_ca_certificates = {"daemon-ssl-ca-certificates", tools::wallet2::tr("Path to file containing concatenated PEM format certificate(s) to replace system CA(s).")}; const command_line::arg_descriptor> daemon_ssl_allowed_fingerprints = {"daemon-ssl-allowed-fingerprints", tools::wallet2::tr("List of valid fingerprints of allowed RPC servers")}; const command_line::arg_descriptor daemon_ssl_allow_any_cert = {"daemon-ssl-allow-any-cert", tools::wallet2::tr("Allow any SSL certificate from the daemon"), false}; + const command_line::arg_descriptor daemon_ssl_allow_chained = {"daemon-ssl-allow-chained", tools::wallet2::tr("Allow user (via --daemon-ssl-ca-certificates) chain certificates"), false}; const command_line::arg_descriptor testnet = {"testnet", tools::wallet2::tr("For testnet. Daemon must also be launched with --testnet flag"), false}; const command_line::arg_descriptor stagenet = {"stagenet", tools::wallet2::tr("For stagenet. Daemon must also be launched with --stagenet flag"), false}; const command_line::arg_descriptor shared_ringdb_dir = { @@ -338,6 +339,9 @@ std::unique_ptr make_basic(const boost::program_options::variabl ssl_options = epee::net_utils::ssl_options_t{ std::move(ssl_allowed_fingerprints), std::move(daemon_ssl_ca_file) }; + + if (command_line::get_arg(vm, opts.daemon_ssl_allow_chained)) + ssl_options.verification = epee::net_utils::ssl_verification_t::user_ca; } if (ssl_options.verification != epee::net_utils::ssl_verification_t::user_certificates || !command_line::is_arg_defaulted(vm, opts.daemon_ssl)) @@ -1110,6 +1114,7 @@ void wallet2::init_options(boost::program_options::options_description& desc_par command_line::add_arg(desc_params, opts.daemon_ssl_ca_certificates); command_line::add_arg(desc_params, opts.daemon_ssl_allowed_fingerprints); command_line::add_arg(desc_params, opts.daemon_ssl_allow_any_cert); + command_line::add_arg(desc_params, opts.daemon_ssl_allow_chained); command_line::add_arg(desc_params, opts.testnet); command_line::add_arg(desc_params, opts.stagenet); command_line::add_arg(desc_params, opts.shared_ringdb_dir); From 2e578b8214b8b47d7ddefceb1cbf2d8129e85a5a Mon Sep 17 00:00:00 2001 From: Lee Clagett Date: Sat, 6 Apr 2019 21:28:37 -0400 Subject: [PATCH 11/11] Enabling daemon-rpc SSL now requires non-system CA verification If `--daemon-ssl enabled` is set in the wallet, then a user certificate, fingerprint, or onion/i2p address must be provided. --- contrib/epee/include/net/net_ssl.h | 3 +++ contrib/epee/src/net_ssl.cpp | 19 +++++++++++++++++++ src/wallet/wallet2.cpp | 23 +++++++++++++---------- src/wallet/wallet_rpc_server.cpp | 23 ++++++++++++++++------- 4 files changed, 51 insertions(+), 17 deletions(-) diff --git a/contrib/epee/include/net/net_ssl.h b/contrib/epee/include/net/net_ssl.h index 726dcb61a..957903ff8 100644 --- a/contrib/epee/include/net/net_ssl.h +++ b/contrib/epee/include/net/net_ssl.h @@ -100,6 +100,9 @@ namespace net_utils //! \return False iff ssl is disabled, otherwise true. explicit operator bool() const noexcept { return support != ssl_support_t::e_ssl_support_disabled; } + //! \retrurn True if `host` can be verified using `this` configuration WITHOUT system "root" CAs. + bool has_strong_verification(boost::string_ref host) const noexcept; + //! Search against internal fingerprints. Always false if `behavior() != user_certificate_check`. bool has_fingerprint(boost::asio::ssl::verify_context &ctx) const; diff --git a/contrib/epee/src/net_ssl.cpp b/contrib/epee/src/net_ssl.cpp index 1bc6f91b8..7bedb18ac 100644 --- a/contrib/epee/src/net_ssl.cpp +++ b/contrib/epee/src/net_ssl.cpp @@ -278,6 +278,25 @@ bool is_ssl(const unsigned char *data, size_t len) return false; } +bool ssl_options_t::has_strong_verification(boost::string_ref host) const noexcept +{ + // onion and i2p addresses contain information about the server cert + // which both authenticates and encrypts + if (host.ends_with(".onion") || host.ends_with(".i2p")) + return true; + switch (verification) + { + default: + case ssl_verification_t::none: + case ssl_verification_t::system_ca: + return false; + case ssl_verification_t::user_certificates: + case ssl_verification_t::user_ca: + break; + } + return true; +} + bool ssl_options_t::has_fingerprint(boost::asio::ssl::verify_context &ctx) const { // can we check the certificate against a list of fingerprints? diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index 2939ed8a4..1d3ba900d 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -315,6 +315,7 @@ std::unique_ptr make_basic(const boost::program_options::variabl const uint64_t kdf_rounds = command_line::get_arg(vm, opts.kdf_rounds); THROW_WALLET_EXCEPTION_IF(kdf_rounds == 0, tools::error::wallet_internal_error, "KDF rounds must not be 0"); + const bool use_proxy = command_line::has_arg(vm, opts.proxy); auto daemon_address = command_line::get_arg(vm, opts.daemon_address); auto daemon_host = command_line::get_arg(vm, opts.daemon_host); auto daemon_port = command_line::get_arg(vm, opts.daemon_port); @@ -382,22 +383,24 @@ std::unique_ptr make_basic(const boost::program_options::variabl if (daemon_address.empty()) daemon_address = std::string("http://") + daemon_host + ":" + std::to_string(daemon_port); - boost::asio::ip::tcp::endpoint proxy{}; - if (command_line::has_arg(vm, opts.proxy)) { - namespace ip = boost::asio::ip; const boost::string_ref real_daemon = boost::string_ref{daemon_address}.substr(0, daemon_address.rfind(':')); - // onion and i2p addresses contain information about the server cert - // which both authenticates and encrypts - const bool unencrypted_proxy = - !real_daemon.ends_with(".onion") && !real_daemon.ends_with(".i2p") && - daemon_ssl_ca_file.empty() && daemon_ssl_allowed_fingerprints.empty(); + const bool verification_required = + ssl_options.support == epee::net_utils::ssl_support_t::e_ssl_support_enabled || use_proxy; + THROW_WALLET_EXCEPTION_IF( - unencrypted_proxy, + verification_required && !ssl_options.has_strong_verification(real_daemon), tools::error::wallet_internal_error, - std::string{"Use of --"} + opts.proxy.name + " requires --" + opts.daemon_ssl_ca_certificates.name + " or --" + opts.daemon_ssl_allowed_fingerprints.name + " or use of a .onion/.i2p domain" + tools::wallet2::tr("Enabling --") + std::string{use_proxy ? opts.proxy.name : opts.daemon_ssl.name} + tools::wallet2::tr(" requires --") + + opts.daemon_ssl_ca_certificates.name + tools::wallet2::tr(" or --") + opts.daemon_ssl_allowed_fingerprints.name + tools::wallet2::tr(" or use of a .onion/.i2p domain") ); + } + + boost::asio::ip::tcp::endpoint proxy{}; + if (use_proxy) + { + namespace ip = boost::asio::ip; const auto proxy_address = command_line::get_arg(vm, opts.proxy); diff --git a/src/wallet/wallet_rpc_server.cpp b/src/wallet/wallet_rpc_server.cpp index 474e0d101..d456797a9 100644 --- a/src/wallet/wallet_rpc_server.cpp +++ b/src/wallet/wallet_rpc_server.cpp @@ -4060,13 +4060,7 @@ namespace tools er.message = "Command unavailable in restricted mode."; return false; } - epee::net_utils::ssl_options_t ssl_options = epee::net_utils::ssl_support_t::e_ssl_support_enabled; - if (!epee::net_utils::ssl_support_from_string(ssl_options.support, req.ssl_support)) - { - er.code = WALLET_RPC_ERROR_CODE_NO_DAEMON_CONNECTION; - er.message = std::string("Invalid ssl support mode"); - return false; - } + std::vector> ssl_allowed_fingerprints; ssl_allowed_fingerprints.reserve(req.ssl_allowed_fingerprints.size()); for (const std::string &fp: req.ssl_allowed_fingerprints) @@ -4076,15 +4070,30 @@ namespace tools for (auto c: fp) v.push_back(c); } + + epee::net_utils::ssl_options_t ssl_options = epee::net_utils::ssl_support_t::e_ssl_support_enabled; if (req.ssl_allow_any_cert) ssl_options.verification = epee::net_utils::ssl_verification_t::none; else if (!ssl_allowed_fingerprints.empty() || !req.ssl_ca_file.empty()) ssl_options = epee::net_utils::ssl_options_t{std::move(ssl_allowed_fingerprints), std::move(req.ssl_ca_file)}; + if (!epee::net_utils::ssl_support_from_string(ssl_options.support, req.ssl_support)) + { + er.code = WALLET_RPC_ERROR_CODE_NO_DAEMON_CONNECTION; + er.message = std::string("Invalid ssl support mode"); + return false; + } + ssl_options.auth = epee::net_utils::ssl_authentication_t{ std::move(req.ssl_private_key_path), std::move(req.ssl_certificate_path) }; + if (ssl_options.support == epee::net_utils::ssl_support_t::e_ssl_support_enabled && !ssl_options.has_strong_verification(boost::string_ref{})) + { + er.code = WALLET_RPC_ERROR_CODE_NO_DAEMON_CONNECTION; + er.message = "SSL is enabled but no user certificate or fingerprints were provided"; + } + if (!m_wallet->set_daemon(req.address, boost::none, req.trusted, std::move(ssl_options))) { er.code = WALLET_RPC_ERROR_CODE_NO_DAEMON_CONNECTION;