Skip to content

Commit

Permalink
PROTON-2075: [C++] Allow TLS to use system default trusted certificat…
Browse files Browse the repository at this point in the history
…e store

- Make the amqps scheme automatically imply tls with default trusted certs
- Also fix small const issues with ssl option contructors
- Added negative test to json config tests to ensure that
  the connection fails if we're using the system trusted certs
  • Loading branch information
astitcher committed Jul 18, 2019
1 parent 5293be6 commit e152190
Show file tree
Hide file tree
Showing 10 changed files with 290 additions and 194 deletions.
2 changes: 1 addition & 1 deletion cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ set(qpid-proton-cpp-source
src/session_options.cpp
src/source.cpp
src/ssl.cpp
src/ssl_domain.cpp
src/ssl_options.cpp
src/target.cpp
src/terminus.cpp
src/timestamp.cpp
Expand Down
1 change: 0 additions & 1 deletion cpp/examples/service_bus.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,6 @@ int main(int argc, char **argv) {
connection_options()
.user(sb_key_name)
.password(sb_key)
.ssl_client_options(ssl_client_options())
.sasl_allowed_mechs("PLAIN"));
proton::container(seq).run();
return 0;
Expand Down
61 changes: 24 additions & 37 deletions cpp/include/proton/ssl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
#include "./internal/export.hpp"
#include "./internal/config.hpp"


#include <proton/ssl.h>

#include <string>
Expand Down Expand Up @@ -123,74 +122,62 @@ class ssl_certificate {
/// @endcond
};

class ssl_domain_impl;

namespace internal {

// Base class for SSL configuration
class ssl_domain {
public:
PN_CPP_EXTERN ssl_domain(const ssl_domain&);
PN_CPP_EXTERN ssl_domain& operator=(const ssl_domain&);
PN_CPP_EXTERN ~ssl_domain();

protected:
ssl_domain(bool is_server);
pn_ssl_domain_t *pn_domain();

private:
ssl_domain_impl *impl_;
bool server_type_;
};

}

/// **Unsettled API** - SSL configuration for inbound connections.
class ssl_server_options : private internal::ssl_domain {
class ssl_server_options {
public:
/// Server SSL options based on the supplied X.509 certificate
/// specifier.
PN_CPP_EXTERN ssl_server_options(ssl_certificate &cert);
PN_CPP_EXTERN ssl_server_options(const ssl_certificate &cert);

/// Server SSL options requiring connecting clients to provide a
/// client certificate.
PN_CPP_EXTERN ssl_server_options(ssl_certificate &cert, const std::string &trust_db,
PN_CPP_EXTERN ssl_server_options(const ssl_certificate &cert, const std::string &trust_db,
const std::string &advertise_db = std::string(),
enum ssl::verify_mode mode = ssl::VERIFY_PEER);

/// Server SSL options restricted to available anonymous cipher
/// suites on the platform.
PN_CPP_EXTERN ssl_server_options();

PN_CPP_EXTERN ~ssl_server_options();
PN_CPP_EXTERN ssl_server_options(const ssl_server_options&);
PN_CPP_EXTERN ssl_server_options& operator=(const ssl_server_options&);

private:
// Bring pn_domain into scope and allow connection_options to use
// it.
using internal::ssl_domain::pn_domain;
class impl;
impl* impl_;

/// @cond INTERNAL
friend class connection_options;
/// @endcond
};

/// **Unsettled API** - SSL configuration for outbound connections.
class ssl_client_options : private internal::ssl_domain {
class ssl_client_options {
public:
/// Create SSL client options (no client certificate).
/// Create SSL client with defaults (use system certificate trust database and require name verification)
PN_CPP_EXTERN ssl_client_options();

/// Create SSL client with unusual verification policy (but default certificate trust database)
PN_CPP_EXTERN ssl_client_options(enum ssl::verify_mode);

/// Create SSL client specifying the certificate trust database.
PN_CPP_EXTERN ssl_client_options(const std::string &trust_db,
enum ssl::verify_mode = ssl::VERIFY_PEER_NAME);

/// Create SSL client options with a client certificate.
PN_CPP_EXTERN ssl_client_options(ssl_certificate&, const std::string &trust_db,
/// Create SSL client with a client certificate.
PN_CPP_EXTERN ssl_client_options(const ssl_certificate&, const std::string &trust_db,
enum ssl::verify_mode = ssl::VERIFY_PEER_NAME);

/// SSL connections restricted to available anonymous cipher
/// suites on the platform.
PN_CPP_EXTERN ssl_client_options();
PN_CPP_EXTERN ~ssl_client_options();
PN_CPP_EXTERN ssl_client_options(const ssl_client_options&);
PN_CPP_EXTERN ssl_client_options& operator=(const ssl_client_options&);

private:
// Bring pn_domain into scope and allow connection_options to use
// it.
using internal::ssl_domain::pn_domain;
class impl;
impl* impl_;

/// @cond INTERNAL
friend class connection_options;
Expand Down
19 changes: 12 additions & 7 deletions cpp/src/connect_config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -124,20 +124,25 @@ void parse_sasl(Value root, connection_options& opts) {
void parse_tls(const string& scheme, Value root, connection_options& opts) {
Value tls = get(objectValue, root, "tls");
if (scheme == "amqps") { // TLS is enabled
bool verify = get_bool(tls, "verify", true);
ssl::verify_mode mode = verify ? ssl::VERIFY_PEER_NAME : ssl::ANONYMOUS_PEER;
ssl::verify_mode mode = ssl::VERIFY_PEER_NAME;
Value verifyValue = get(booleanValue, tls, "verify");
if (!verifyValue.empty()) {
mode = verifyValue.asBool() ? ssl::VERIFY_PEER_NAME : ssl::ANONYMOUS_PEER;
}
string ca = get_string(tls, "ca", "");
string cert = get_string(tls, "cert", "");
string key = get_string(tls, "key", "");
ssl_client_options ssl_opts;
if (!cert.empty()) {
ssl_certificate sc = key.empty() ? ssl_certificate(cert) : ssl_certificate(cert, key);
ssl_opts = ssl_client_options(sc, ca, mode);
opts.ssl_client_options(ssl_client_options(sc, ca, mode));
} else if (!ca.empty()) {
ssl_opts = ssl_client_options(ca, mode);
opts.ssl_client_options(ssl_client_options(ca, mode));
} else if (!verifyValue.empty()) {
opts.ssl_client_options(ssl_client_options(mode));
} else {
opts.ssl_client_options(ssl_client_options());
}
opts.ssl_client_options(ssl_opts);
} else if (!tls.isNull()) {
} else if (!tls.empty()) {
throw err(msg() << "'tls' object not allowed unless scheme is \"amqps\"");
}
}
Expand Down
40 changes: 39 additions & 1 deletion cpp/src/connect_config_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ class test_handler : public messaging_handler, public listen_handler {
on_listener_start(l.container());
}

connection_options on_accept(listener& ) PN_CPP_OVERRIDE {
connection_options on_accept(listener& l) PN_CPP_OVERRIDE {
return connection_options_;
}

Expand Down Expand Up @@ -191,6 +191,10 @@ class test_handler : public messaging_handler, public listen_handler {
c.connect(configure(opts, config_with_port(bare_config)), opts);
}

void stop_listener() {
listener_.stop();
}

public:
test_handler(const connection_options& listen_opts = connection_options()) :
opened_(false), connection_options_(listen_opts) {}
Expand Down Expand Up @@ -252,6 +256,39 @@ class test_tls : public test_handler {
}
};

class test_tls_default_fail : public test_handler {
static connection_options make_opts() {
ssl_certificate cert("testdata/certs/server-certificate.pem",
"testdata/certs/server-private-key.pem",
"server-password");
connection_options opts;
opts.ssl_server_options(ssl_server_options(cert));
return opts;
}
bool failed_;

public:

test_tls_default_fail() : test_handler(make_opts()), failed_(false) {}

void on_listener_start(proton::container& c) PN_CPP_OVERRIDE {
connect(c, RAW_STRING("scheme":"amqps"));
}

void on_messaging_error(const proton::error_condition& c) PN_CPP_OVERRIDE {
if (failed_) return;

ASSERT_SUBSTRING("verify failed", c.description());
failed_ = true;
stop_listener();
}

void run() {
container(*this).run();
ASSERT(failed_);
}
};

class test_tls_external : public test_handler {

static connection_options make_opts() {
Expand Down Expand Up @@ -332,6 +369,7 @@ int main(int argc, char** argv) {
if (have_ssl) {
pn_ssl_domain_free(have_ssl);
RUN_ARGV_TEST(failed, test_tls().run());
RUN_ARGV_TEST(failed, test_tls_default_fail().run());
RUN_ARGV_TEST(failed, test_tls_external().run());
if (have_sasl) {
RUN_ARGV_TEST(failed, test_tls_plain().run());
Expand Down
12 changes: 8 additions & 4 deletions cpp/src/connection_options.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include "messaging_adapter.hpp"
#include "msg.hpp"
#include "proton_bits.hpp"
#include "ssl_options_impl.hpp"

#include <proton/connection.h>
#include <proton/proactor.h>
Expand Down Expand Up @@ -141,12 +142,15 @@ class connection_options::impl {
// hostname to the connection hostname, which has
// already been adjusted for the virtual_host option.
pn_ssl_t *ssl = pn_ssl(pnt);
if (pn_ssl_init(ssl, ssl_client_options.value.pn_domain(), NULL))
pn_ssl_domain_t* ssl_domain = ssl_client_options.value.impl_ ? ssl_client_options.value.impl_->pn_domain() : NULL;
if (pn_ssl_init(ssl, ssl_domain, NULL)) {
throw error(MSG("client SSL/TLS initialization error"));
}
} else if (!client && ssl_server_options.set) {
pn_ssl_t *ssl = pn_ssl(pnt);
if (pn_ssl_init(ssl, ssl_server_options.value.pn_domain(), NULL))
throw error(MSG("server SSL/TLS initialization error"));
pn_ssl_t *ssl = pn_ssl(pnt);
if (pn_ssl_init(ssl, ssl_server_options.value.impl_->pn_domain(), NULL)) {
throw error(MSG("server SSL/TLS initialization error"));
}
}

}
Expand Down
8 changes: 7 additions & 1 deletion cpp/src/proactor_container_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "proton/listen_handler.hpp"
#include "proton/listener.hpp"
#include "proton/reconnect_options.hpp"
#include "proton/ssl.hpp"
#include "proton/transport.hpp"
#include "proton/url.hpp"

Expand Down Expand Up @@ -181,7 +182,12 @@ pn_connection_t* container::impl::make_connection_lh(
if (stopping_)
throw proton::error("container is stopping");

connection_options opts = client_connection_options_; // Defaults
connection_options opts;
// If scheme is amqps then use default tls settings
if (url.scheme()==url.AMQPS) {
opts.ssl_client_options(ssl_client_options());
}
opts.update(client_connection_options_);
opts.update(user_opts);
messaging_handler* mh = opts.handler();

Expand Down

0 comments on commit e152190

Please sign in to comment.