From f18a069fcc5ad5d18bde93d9ee2902354f2add9d Mon Sep 17 00:00:00 2001 From: Lee Clagett Date: Tue, 12 Mar 2019 18:16:47 -0400 Subject: [PATCH] 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;