From 0416764caec5b2d504c820e71d74a4934c3e2c3e Mon Sep 17 00:00:00 2001
From: Lee Clagett <code@leeclagett.com>
Date: Sun, 17 Mar 2019 22:06:36 -0400
Subject: [PATCH] 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<boost::asio::ip::tcp::socket> &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<boost::asio::ip::tcp::socket> &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<boost::asio::ip::tcp::soc
     {
       // preverified means it passed system or user CA check. System CA is never loaded
       // when fingerprints are whitelisted.
-      if (!preverified && verification == ssl_verification_t::user_certificates && !has_fingerprint(ctx)) {
-        MERROR("Certificate is not in the allowed list, connection droppped");
-        return false;
+      if (!preverified && !has_fingerprint(ctx))
+      {
+        // autodetect will reconnect without SSL - warn and keep connection encrypted
+        if (support != ssl_support_t::e_ssl_support_autodetect)
+        {
+          MERROR("SSL certificate is not in the allowed list, connection droppped");
+          return false;
+        }
+        MWARNING("SSL peer has not been verified");
       }
-      verified = true;
       return true;
     });
   }
@@ -348,12 +352,7 @@ bool ssl_options_t::handshake(boost::asio::ssl::stream<boost::asio::ip::tcp::soc
   socket.handshake(type, ec);
   if (ec)
   {
-    MERROR("handshake failed, connection dropped: " << ec.message());
-    return false;
-  }
-  if (verification == ssl_verification_t::none && !verified)
-  {
-    MERROR("Peer did not provide a certificate in the allowed list, connection dropped");
+    MERROR("SSL handshake failed, connection dropped: " << ec.message());
     return false;
   }
   MDEBUG("SSL handshake success");