From 1f5ed328aa3b0501bd85774dc960c17a73d79db3 Mon Sep 17 00:00:00 2001 From: Lee Clagett Date: Wed, 13 Mar 2019 20:01:14 -0400 Subject: [PATCH] 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() };