From 415870c04a7e6cabf13e6acf3a94bb0f68732907 Mon Sep 17 00:00:00 2001
From: Tim Costen <tim.costen@isode.com>
Date: Thu, 1 Aug 2019 12:05:53 +0100
Subject: Add enhanced OpenSSL configuration
Adds TLSOptions to the OpenSSLContext, which invokes a new private
'configure' method which allows various OpenSSL options to be set.
Also add standard verification callbacks and external (via a
std::function field in TLSOptions) to allow the user
to specify their own method which will perform client certificate
checking when a new TLS connection is accepted. Only set up
the internal verifyCertCallback if the user-supplied hook is set.
All callback hooks are set up in the 'configure' method, and only
then if TLSOptions.verifyMode is present (i.e. not defaulted
to boost::none), to preserve compatibility for users of
this class (e.g. Swift) which want to use OpenSSL's own
internal validation functions rather than setting the
callbacks.
Test-information:
Used new code under development in M-Link when setting up a TLSContext,
setting verify-mode=require, and set up verifyCertCallback with a local
method. Making a client TLS connection which includes a client
certificate results in the local verify callback being invoked.
Change-Id: Idbb7279e1711fca8123f430bfca0dcfb65bc8da6
diff --git a/Swiften/Network/BOSHConnection.h b/Swiften/Network/BOSHConnection.h
index c492ac4..f0a946a 100644
--- a/Swiften/Network/BOSHConnection.h
+++ b/Swiften/Network/BOSHConnection.h
@@ -31,7 +31,7 @@ namespace Swift {
class XMLParserFactory;
class TLSContextFactory;
class TLSLayer;
- struct TLSOptions;
+ class TLSOptions;
class HighLayer;
class SWIFTEN_API BOSHError : public SessionStream::SessionStreamError {
diff --git a/Swiften/TLS/OpenSSL/OpenSSLContext.cpp b/Swiften/TLS/OpenSSL/OpenSSLContext.cpp
index 5692e74..e585766 100644
--- a/Swiften/TLS/OpenSSL/OpenSSLContext.cpp
+++ b/Swiften/TLS/OpenSSL/OpenSSLContext.cpp
@@ -42,6 +42,14 @@ namespace Swift {
static const int MAX_FINISHED_SIZE = 4096;
static const int SSL_READ_BUFFERSIZE = 8192;
+#define SSL_DEFAULT_VERIFY_DEPTH 5
+
+// Callback function declarations for certificate verification
+extern "C" {
+ static int certVerifyCallback(X509_STORE_CTX *store_ctx, void*);
+ static int verifyCallback(int preverify_ok, X509_STORE_CTX *ctx);
+}
+
static void freeX509Stack(STACK_OF(X509)* stack) {
sk_X509_free(stack);
}
@@ -90,7 +98,7 @@ namespace {
}
}
-OpenSSLContext::OpenSSLContext(Mode mode) : mode_(mode), state_(State::Start) {
+OpenSSLContext::OpenSSLContext(const TLSOptions& options, Mode mode) : mode_(mode), state_(State::Start) {
ensureLibraryInitialized();
context_ = createSSL_CTX(mode_);
SSL_CTX_set_options(context_.get(), SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3);
@@ -118,9 +126,9 @@ OpenSSLContext::OpenSSLContext(Mode mode) : mode_(mode), state_(State::Start) {
X509_STORE* store = SSL_CTX_get_cert_store(context_.get());
HCERTSTORE systemStore = CertOpenSystemStore(0, "ROOT");
if (systemStore) {
- PCCERT_CONTEXT certContext = NULL;
+ PCCERT_CONTEXT certContext = nullptr;
while (true) {
- certContext = CertFindCertificateInStore(systemStore, X509_ASN_ENCODING | PKCS_7_ASN_ENCODING, 0, CERT_FIND_ANY, NULL, certContext);
+ certContext = CertFindCertificateInStore(systemStore, X509_ASN_ENCODING | PKCS_7_ASN_ENCODING, 0, CERT_FIND_ANY, nullptr, certContext);
if (!certContext) {
break;
}
@@ -159,6 +167,7 @@ OpenSSLContext::OpenSSLContext(Mode mode) : mode_(mode), state_(State::Start) {
CFRelease(anchorCertificates);
}
#endif
+ configure(options);
}
OpenSSLContext::~OpenSSLContext() {
@@ -175,6 +184,222 @@ void OpenSSLContext::initAndSetBIOs() {
SSL_set_bio(handle_.get(), readBIO_, writeBIO_);
}
+// This callback is called by OpenSSL when a client certificate needs to be verified.
+// In turn, this calls the verification callback which the user
+// of this OpenSSLContext has configured (if any).
+static int certVerifyCallback(X509_STORE_CTX* store_ctx, void* arg)
+{
+ OpenSSLContext* context = static_cast<OpenSSLContext *>(arg);
+
+ // Need to stash store_ctx pointer for use within verification
+ context->setX509StoreContext(store_ctx);
+
+ int ret;
+
+ // This callback shouldn't have been set up if the context doesn't
+ // have a verifyCertCallback set, but it doesn't hurt to double check
+ std::function<int (const TLSContext *)> cb = context->getVerifyCertCallback();
+ if (cb != nullptr) {
+ ret = cb(static_cast<const OpenSSLContext*>(context));
+ } else {
+ SWIFT_LOG(warning) << "certVerifyCallback called but context.verifyCertCallback is unset" << std::endl;
+ ret = 0;
+ }
+
+ context->setX509StoreContext(nullptr);
+ return ret;
+}
+
+// Convenience function to generate a text representation
+// of an X509 Name. This information is only used for logging.
+static std::string X509_NAME_to_text(X509_NAME* name)
+{
+ std::string nameString;
+
+ if (!name) {
+ return nameString;
+ }
+
+ std::unique_ptr<BIO, decltype(&BIO_free)> io(BIO_new(BIO_s_mem()), &BIO_free);
+ int r = X509_NAME_print_ex(io.get(), name, 0, XN_FLAG_RFC2253);
+ BIO_write(io.get(), "\0", 1);
+
+ if (r > 0) {
+ BUF_MEM* ptr = nullptr;
+ BIO_get_mem_ptr(io.get(), &ptr);
+ nameString = ptr->data;
+ }
+
+ return nameString;
+}
+
+// Check depth of certificate chain
+static int verifyCallback(int preverifyOk, X509_STORE_CTX* ctx)
+{
+ // Retrieve the pointer to the SSL of the connection currently treated
+ // and the application specific data stored into the SSL object.
+
+ int err = X509_STORE_CTX_get_error(ctx);
+ int depth = X509_STORE_CTX_get_error_depth(ctx);
+
+ SSL* ssl = static_cast<SSL*>(X509_STORE_CTX_get_ex_data(ctx, SSL_get_ex_data_X509_STORE_CTX_idx()));
+ SSL_CTX* sslctx = ssl ? SSL_get_SSL_CTX(ssl) : nullptr;
+ if (!sslctx) {
+ SWIFT_LOG(error) << "verifyCallback: internal error" << std::endl;
+ return preverifyOk;
+ }
+
+ if (SSL_CTX_get_verify_mode(sslctx) == SSL_VERIFY_NONE) {
+ SWIFT_LOG(info) << "verifyCallback: no verification required" << std::endl;
+ // No verification requested
+ return 1;
+ }
+
+ X509* errCert = X509_STORE_CTX_get_current_cert(ctx);
+ std::string subjectString;
+ if (errCert) {
+ X509_NAME* subjectName = X509_get_subject_name(errCert);
+ subjectString = X509_NAME_to_text(subjectName);
+ }
+
+ // Catch a too long certificate chain. The depth limit set using
+ // SSL_CTX_set_verify_depth() is by purpose set to "limit+1" so
+ // that whenever the "depth>verify_depth" condition is met, we
+ // have violated the limit and want to log this error condition.
+ // We must do it here, because the CHAIN_TOO_LONG error would not
+ // be found explicitly; only errors introduced by cutting off the
+ // additional certificates would be logged.
+ if (depth >= SSL_CTX_get_verify_depth(sslctx)) {
+ preverifyOk = 0;
+ err = X509_V_ERR_CERT_CHAIN_TOO_LONG;
+ X509_STORE_CTX_set_error(ctx, err);
+ }
+
+ if (!preverifyOk) {
+ std::string issuerString;
+ if (errCert) {
+ X509_NAME* issuerName = X509_get_issuer_name(errCert);
+ issuerString = X509_NAME_to_text(issuerName);
+ }
+ SWIFT_LOG(error) << "verifyCallback: verification error" <<
+ X509_verify_cert_error_string(err) << " depth: " <<
+ depth << " issuer: " << ((issuerString.length() > 0) ? issuerString : "<unknown>") << std::endl;
+ } else {
+ SWIFT_LOG(info) << "verifyCallback: SSL depth: " << depth << " Subject: " <<
+ ((subjectString.length() > 0) ? subjectString : "<>") << std::endl;
+ }
+ return preverifyOk;
+}
+
+bool OpenSSLContext::configure(const TLSOptions &options)
+{
+ if (options.cipherSuites) {
+ std::string cipherSuites = *(options.cipherSuites);
+ if (SSL_CTX_set_cipher_list(context_.get(), cipherSuites.c_str()) != 1 ) {
+ SWIFT_LOG(error) << "Failed to set cipher-suites" << std::endl;
+ return false;
+ }
+ }
+
+ if (options.context) {
+ const auto& contextId = *options.context;
+
+ if (SSL_CTX_set_session_id_context(context_.get(),
+ reinterpret_cast<const unsigned char *>(contextId.c_str()),
+ contextId.length()) != 1) {
+ SWIFT_LOG(error) << "Failed to set context-id" << std::endl;
+ return false;
+ }
+ }
+
+ if (options.sessionCacheTimeout) {
+ int scto = *options.sessionCacheTimeout;
+ if (scto <= 0) {
+ SWIFT_LOG(error) << "Invalid value for session-cache-timeout" << std::endl;
+ return false;
+ }
+ (void)SSL_CTX_set_timeout(context_.get(), scto);
+ if (SSL_CTX_get_timeout(context_.get()) != scto) {
+ SWIFT_LOG(error) << "Failed to set session-cache-timeout" << std::endl;
+ return false;
+ }
+ }
+
+ if (options.verifyCertificateCallback) {
+ verifyCertCallback = *options.verifyCertificateCallback;
+ } else {
+ verifyCertCallback = nullptr;
+ }
+
+ if (options.verifyMode) {
+ TLSOptions::VerifyMode verify_mode = *options.verifyMode;
+ int mode;
+ switch (verify_mode) {
+ case TLSOptions::VerifyMode::NONE:
+ mode = SSL_VERIFY_NONE;
+ break;
+ case TLSOptions::VerifyMode::REQUIRED:
+ mode = SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT | SSL_VERIFY_CLIENT_ONCE;
+ break;
+ case TLSOptions::VerifyMode::OPTIONAL:
+ mode = SSL_VERIFY_PEER | SSL_VERIFY_CLIENT_ONCE;
+ break;
+ }
+
+ // Set up default certificate chain verification depth - may be overridden below
+ SSL_CTX_set_verify_depth(context_.get(), SSL_DEFAULT_VERIFY_DEPTH + 1);
+
+ // Set callbacks up
+ SSL_CTX_set_verify(context_.get(), mode, verifyCallback);
+
+ // Only set up certificate verification callback if a user callback has
+ // been configured via the TLSOptions
+ if (verifyCertCallback != nullptr) {
+ SSL_CTX_set_cert_verify_callback(context_.get(), certVerifyCallback, this);
+ }
+ }
+
+ if (options.verifyDepth) {
+ int depth = *options.verifyDepth;
+ if (depth <= 0) {
+ SWIFT_LOG(error) << "Invalid value for verify-depth" << std::endl;
+ return false;
+ }
+
+ // Increase depth limit by one, so that verifyCallback() will log it
+ SSL_CTX_set_verify_depth(context_.get(), depth + 1);
+ }
+
+ auto updateOptionIfPresent = [this](boost::optional<bool> option, int flag) {
+ if (option) {
+ if (*option) {
+ SSL_CTX_set_options(context_.get(), flag);
+ }
+ else {
+ SSL_CTX_clear_options(context_.get(), flag);
+ }
+ }
+ };
+ updateOptionIfPresent(options.workaroundMicrosoftSessID, SSL_OP_MICROSOFT_SESS_ID_BUG);
+ updateOptionIfPresent(options.workaroundNetscapeChallenge, SSL_OP_NETSCAPE_CHALLENGE_BUG);
+ updateOptionIfPresent(options.workaroundNetscapeReuseCipherChange, SSL_OP_NETSCAPE_REUSE_CIPHER_CHANGE_BUG);
+ updateOptionIfPresent(options.workaroundSSLRef2ReuseCertType, SSL_OP_SSLREF2_REUSE_CERT_TYPE_BUG);
+ updateOptionIfPresent(options.workaroundMicrosoftBigSSLv3Buffer, SSL_OP_MICROSOFT_BIG_SSLV3_BUFFER);
+ updateOptionIfPresent(options.workaroundSSLeay080ClientDH, SSL_OP_SSLEAY_080_CLIENT_DH_BUG);
+ updateOptionIfPresent(options.workaroundTLSD5, SSL_OP_TLS_D5_BUG);
+ updateOptionIfPresent(options.workaroundTLSBlockPadding, SSL_OP_TLS_BLOCK_PADDING_BUG);
+ updateOptionIfPresent(options.workaroundDontInsertEmptyFragments, SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS);
+ updateOptionIfPresent(options.workaroundAll, SSL_OP_ALL);
+ updateOptionIfPresent(options.suppressSSLv2, SSL_OP_NO_SSLv2);
+ updateOptionIfPresent(options.suppressSSLv3, SSL_OP_NO_SSLv3);
+ updateOptionIfPresent(options.suppressTLSv1, SSL_OP_NO_TLSv1);
+ updateOptionIfPresent(options.disableTLSRollBackBug, SSL_OP_TLS_ROLLBACK_BUG);
+ updateOptionIfPresent(options.singleDHUse, SSL_OP_SINGLE_DH_USE);
+
+ return true;
+}
+
+
void OpenSSLContext::accept() {
assert(mode_ == Mode::Server);
handle_ = std::unique_ptr<SSL>(SSL_new(context_.get()));
@@ -486,7 +711,7 @@ bool OpenSSLContext::setDiffieHellmanParameters(const ByteArray& parametersInOpe
if (bio) {
BIO_write(bio.get(), vecptr(parametersInOpenSslDer), parametersInOpenSslDer.size());
auto result = 0L;
- if (auto dhparams = d2i_DHparams_bio(bio.get(), NULL)) {
+ if (auto dhparams = d2i_DHparams_bio(bio.get(), nullptr)) {
if (handle_) {
result = SSL_set_tmp_dh(handle_.get(), dhparams);
}
diff --git a/Swiften/TLS/OpenSSL/OpenSSLContext.h b/Swiften/TLS/OpenSSL/OpenSSLContext.h
index c18a6f4..885b1fe 100644
--- a/Swiften/TLS/OpenSSL/OpenSSLContext.h
+++ b/Swiften/TLS/OpenSSL/OpenSSLContext.h
@@ -16,6 +16,7 @@
#include <Swiften/Base/ByteArray.h>
#include <Swiften/TLS/CertificateWithKey.h>
#include <Swiften/TLS/TLSContext.h>
+#include <Swiften/TLS/TLSOptions.h>
namespace std {
template<>
@@ -38,7 +39,7 @@ namespace std {
namespace Swift {
class OpenSSLContext : public TLSContext, boost::noncopyable {
public:
- OpenSSLContext(Mode mode);
+ OpenSSLContext(const TLSOptions& options, Mode mode);
virtual ~OpenSSLContext() override final;
void accept() override final;
@@ -60,7 +61,11 @@ namespace Swift {
virtual ByteArray getFinishMessage() const override final;
virtual ByteArray getPeerFinishMessage() const override final;
+ void setX509StoreContext(X509_STORE_CTX *ptr) { x509_store_ctx = ptr; }
+ std::function<int (const TLSContext *)> getVerifyCertCallback() { return verifyCertCallback; }
+
private:
+ bool configure(const TLSOptions& options);
static void ensureLibraryInitialized();
static int handleServerNameCallback(SSL *ssl, int *ad, void *arg);
static CertificateVerificationError::Type getVerificationErrorTypeForResult(int);
@@ -81,5 +86,7 @@ namespace Swift {
BIO* readBIO_ = nullptr;
BIO* writeBIO_ = nullptr;
bool abortTLSHandshake_ = false;
- };
+ X509_STORE_CTX *x509_store_ctx = nullptr;
+ std::function<int (const TLSContext *)> verifyCertCallback = nullptr;
+ };
}
diff --git a/Swiften/TLS/OpenSSL/OpenSSLContextFactory.cpp b/Swiften/TLS/OpenSSL/OpenSSLContextFactory.cpp
index a9ba5ab..12445fd 100644
--- a/Swiften/TLS/OpenSSL/OpenSSLContextFactory.cpp
+++ b/Swiften/TLS/OpenSSL/OpenSSLContextFactory.cpp
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2010-2018 Isode Limited.
+ * Copyright (c) 2010-2019 Isode Limited.
* All rights reserved.
* See the COPYING file for more information.
*/
@@ -21,8 +21,8 @@ bool OpenSSLContextFactory::canCreate() const {
return true;
}
-std::unique_ptr<TLSContext> OpenSSLContextFactory::createTLSContext(const TLSOptions&, TLSContext::Mode mode) {
- return std::unique_ptr<TLSContext>(new OpenSSLContext(mode));
+std::unique_ptr<TLSContext> OpenSSLContextFactory::createTLSContext(const TLSOptions& options, TLSContext::Mode mode) {
+ return std::make_unique<OpenSSLContext>(options, mode);
}
ByteArray OpenSSLContextFactory::convertDHParametersFromPEMToDER(const std::string& dhParametersInPEM) {
diff --git a/Swiften/TLS/OpenSSL/OpenSSLContextFactory.h b/Swiften/TLS/OpenSSL/OpenSSLContextFactory.h
index 95a2b0c..834e479 100644
--- a/Swiften/TLS/OpenSSL/OpenSSLContextFactory.h
+++ b/Swiften/TLS/OpenSSL/OpenSSLContextFactory.h
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2010-2018 Isode Limited.
+ * Copyright (c) 2010-2019 Isode Limited.
* All rights reserved.
* See the COPYING file for more information.
*/
diff --git a/Swiften/TLS/TLSContext.h b/Swiften/TLS/TLSContext.h
index 003069f..85776d8 100644
--- a/Swiften/TLS/TLSContext.h
+++ b/Swiften/TLS/TLSContext.h
@@ -50,7 +50,7 @@ namespace Swift {
virtual ByteArray getFinishMessage() const = 0;
virtual ByteArray getPeerFinishMessage() const;
- public:
+ public:
enum class Mode {
Client,
Server
diff --git a/Swiften/TLS/TLSOptions.h b/Swiften/TLS/TLSOptions.h
index dd7e920..7a38aa2 100644
--- a/Swiften/TLS/TLSOptions.h
+++ b/Swiften/TLS/TLSOptions.h
@@ -7,8 +7,10 @@
#pragma once
namespace Swift {
+ class TLSContext;
- struct TLSOptions {
+ class TLSOptions {
+ public:
TLSOptions() : schannelTLS1_0Workaround(false) {
}
@@ -21,5 +23,44 @@ namespace Swift {
*/
bool schannelTLS1_0Workaround;
+ /**
+ * OpenSSL configuration flags
+ */
+ boost::optional<bool> workaroundMicrosoftSessID;
+ boost::optional<bool> workaroundNetscapeChallenge;
+ boost::optional<bool> workaroundNetscapeReuseCipherChange;
+ boost::optional<bool> workaroundSSLRef2ReuseCertType;
+ boost::optional<bool> workaroundMicrosoftBigSSLv3Buffer;
+ boost::optional<bool> workaroundSSLeay080ClientDH;
+ boost::optional<bool> workaroundTLSD5;
+ boost::optional<bool> workaroundTLSBlockPadding;
+ boost::optional<bool> workaroundDontInsertEmptyFragments;
+ boost::optional<bool> workaroundAll;
+ boost::optional<bool> suppressSSLv2;
+ boost::optional<bool> suppressSSLv3;
+ boost::optional<bool> suppressTLSv1;
+ boost::optional<bool> disableTLSRollBackBug;
+ boost::optional<bool> singleDHUse;
+
+ /**
+ * Other OpenSSL configuration items
+ */
+ boost::optional<std::string> cipherSuites;
+ boost::optional<std::string> context;
+ boost::optional<int> sessionCacheTimeout;
+ boost::optional<int> verifyDepth;
+
+ enum class VerifyMode {
+ NONE,
+ REQUIRED,
+ OPTIONAL
+ } ;
+ boost::optional<VerifyMode> verifyMode;
+
+ /**
+ * Callback for certificate verification
+ */
+
+ boost::optional<std::function<int(const TLSContext *)>> verifyCertificateCallback;
};
}
--
cgit v0.10.2-6-g49f6