diff options
| author | Tim Costen <tim.costen@isode.com> | 2019-08-01 11:05:53 (GMT) | 
|---|---|---|
| committer | Tim Costen <timcosten64@gmail.com> | 2019-09-03 10:14:51 (GMT) | 
| commit | 415870c04a7e6cabf13e6acf3a94bb0f68732907 (patch) | |
| tree | c1d2a509cdfa341efbc9e1d575ad3bb2383100c0 | |
| parent | c62b7b6ce35a77d0a8236ef48155187fe5c30d12 (diff) | |
| download | swift-415870c04a7e6cabf13e6acf3a94bb0f68732907.zip swift-415870c04a7e6cabf13e6acf3a94bb0f68732907.tar.bz2 | |
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
| -rw-r--r-- | Swiften/Network/BOSHConnection.h | 2 | ||||
| -rw-r--r-- | Swiften/TLS/OpenSSL/OpenSSLContext.cpp | 233 | ||||
| -rw-r--r-- | Swiften/TLS/OpenSSL/OpenSSLContext.h | 11 | ||||
| -rw-r--r-- | Swiften/TLS/OpenSSL/OpenSSLContextFactory.cpp | 6 | ||||
| -rw-r--r-- | Swiften/TLS/OpenSSL/OpenSSLContextFactory.h | 2 | ||||
| -rw-r--r-- | Swiften/TLS/TLSContext.h | 2 | ||||
| -rw-r--r-- | Swiften/TLS/TLSOptions.h | 43 | 
7 files changed, 286 insertions, 13 deletions
| 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;      };  } | 
 Swift
 Swift