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.

7 files changed, 65 insertions, 76 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<std::string, t_connection_type> 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<std::string, std::string> &private_key_and_certificate_path = std::make_pair(std::string(), std::string()), const std::list<std::string> &allowed_certificates = {}, const std::vector<std::vector<uint8_t>> &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<std::string, std::string> &private_key_and_certificate_path = std::make_pair(std::string(), std::string()), const std::list<std::string> &allowed_certificates = {}, const std::vector<std::vector<uint8_t>> &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<std::string, std::string> &private_key_and_certificate_path = std::make_pair(std::string(), std::string()), const std::string &ca_file = {}, const std::vector<std::vector<uint8_t>> &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<std::string, std::string> &private_key_and_certificate_path = std::make_pair(std::string(), std::string()), const std::string &ca_file = {}, const std::vector<std::vector<uint8_t>> &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<connection_ptr> connections_;
 
     ssl_context_t m_ssl_context;
-    std::list<std::string> 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<class t_protocol_handler>
-  bool boosted_tcp_server<t_protocol_handler>::init_server(uint32_t port, const std::string address, epee::net_utils::ssl_support_t ssl_support, const std::pair<std::string, std::string> &private_key_and_certificate_path, const std::list<std::string> &allowed_certificates, const std::vector<std::vector<uint8_t>> &allowed_fingerprints, bool allow_any_cert)
+  bool boosted_tcp_server<t_protocol_handler>::init_server(uint32_t port, const std::string address, epee::net_utils::ssl_support_t ssl_support, const std::pair<std::string, std::string> &private_key_and_certificate_path, const std::string &ca_file, const std::vector<std::vector<uint8_t>> &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<std::string>(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<class t_protocol_handler>
-  bool boosted_tcp_server<t_protocol_handler>::init_server(const std::string port, const std::string& address, epee::net_utils::ssl_support_t ssl_support, const std::pair<std::string, std::string> &private_key_and_certificate_path, const std::list<std::string> &allowed_certificates, const std::vector<std::vector<uint8_t>> &allowed_fingerprints, bool allow_any_cert)
+  bool boosted_tcp_server<t_protocol_handler>::init_server(const std::string port, const std::string& address, epee::net_utils::ssl_support_t ssl_support, const std::pair<std::string, std::string> &private_key_and_certificate_path, const std::string &ca_file, const std::vector<std::vector<uint8_t>> &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<std::string, std::string> m_ssl_private_key_and_certificate_path;
-			std::list<std::string> m_ssl_allowed_certificates;
+			std::string m_ssl_ca_file;
 			std::vector<std::vector<uint8_t>> 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<login> user, epee::net_utils::ssl_support_t ssl_support = epee::net_utils::ssl_support_t::e_ssl_support_autodetect, const std::pair<std::string, std::string> &private_key_and_certificate_path = {}, const std::list<std::string> &allowed_ssl_certificates = {}, const std::vector<std::vector<uint8_t>> &allowed_ssl_fingerprints = {}, bool allow_any_cert = false)
+			bool set_server(const std::string& address, boost::optional<login> user, epee::net_utils::ssl_support_t ssl_support = epee::net_utils::ssl_support_t::e_ssl_support_autodetect, const std::pair<std::string, std::string> &private_key_and_certificate_path = {}, std::string ca_file = {}, const std::vector<std::vector<uint8_t>> &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<login> user, epee::net_utils::ssl_support_t ssl_support = epee::net_utils::ssl_support_t::e_ssl_support_autodetect, const std::pair<std::string, std::string> &private_key_and_certificate_path = {}, const std::list<std::string> &allowed_ssl_certificates = {}, const std::vector<std::vector<uint8_t>> &allowed_ssl_fingerprints = {}, bool allow_any_cert = false)
+			void set_server(std::string host, std::string port, boost::optional<login> user, epee::net_utils::ssl_support_t ssl_support = epee::net_utils::ssl_support_t::e_ssl_support_autodetect, const std::pair<std::string, std::string> &private_key_and_certificate_path = {}, std::string ca_file = {}, const std::vector<std::vector<uint8_t>> &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<typename F>
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<net_utils::http::login> user = boost::none,
       epee::net_utils::ssl_support_t ssl_support = epee::net_utils::ssl_support_t::e_ssl_support_autodetect,
       const std::pair<std::string, std::string> &private_key_and_certificate_path = {},
-      std::list<std::string> allowed_certificates = {},
+      const std::string &ca_path = {},
       std::vector<std::vector<uint8_t>> 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<std::string, std::string> &private_key_and_certificate_path = {}, std::list<std::string> allowed_certificates = {}, std::vector<std::vector<uint8_t>> 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<std::string, std::string> &private_key_and_certificate_path = {}, const std::string &ca_path = {}, std::vector<std::vector<uint8_t>> 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<boost::asio::ip::tcp::socket>(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 <stdint.h>
 #include <string>
-#include <list>
+#include <vector>
 #include <boost/utility/string_ref.hpp>
 #include <boost/asio/ip/tcp.hpp>
 #include <boost/asio/ssl.hpp>
+#include <boost/system/error_code.hpp>
 
 namespace epee
 {
@@ -49,7 +50,7 @@ namespace net_utils
 	struct ssl_context_t
 	{
 		boost::asio::ssl::context context;
-		std::list<std::string> allowed_certificates;
+		std::string ca_path;
 		std::vector<std::vector<uint8_t>> 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<std::string, std::string> &private_key_and_certificate_path, std::list<std::string> allowed_certificates, std::vector<std::vector<uint8_t>> allowed_fingerprints, bool allow_any_cert);
+	ssl_context_t create_ssl_context(const std::pair<std::string, std::string> &private_key_and_certificate_path, const std::string &ca_path, std::vector<std::vector<uint8_t>> allowed_fingerprints, bool allow_any_cert);
 	void use_ssl_certificate(ssl_context_t &ssl_context, const std::pair<std::string, std::string> &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<boost::asio::ip::tcp::socket> &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<BIGNUM, openssl_bignum_free>;
+
+  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<std::string, std::string> &private_key_and_certificate_path, std::list<std::string> allowed_certificates, std::vector<std::vector<uint8_t>> allowed_fingerprints, bool allow_any_cert)
+ssl_context_t create_ssl_context(const std::pair<std::string, std::string> &private_key_and_certificate_path, const std::string &ca_path, std::vector<std::vector<uint8_t>> 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<std::string, std::string> &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<uint8_t> 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<boost::asio::ip::tcp::socket> &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;
     }