You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by ac...@apache.org on 2017/10/06 20:11:44 UTC
qpid-proton git commit: PROTON-1571: c++: ssl example is leaky
Repository: qpid-proton
Updated Branches:
refs/heads/master ace1970ef -> 3e4b963a6
PROTON-1571: c++: ssl example is leaky
Nothing to do with SSL, the problem was the example code throwing from an event
handling function. We should not leak even in this case, but I will raise a
separate issue.
Made the following positive changes while investigating:
- Added trival copy/assign/dtor to proton::listener as future-proofing.
- Added C TLS tests to proactor.c
Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/3e4b963a
Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/3e4b963a
Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/3e4b963a
Branch: refs/heads/master
Commit: 3e4b963a6c77723dd1796fe64af970d87b239d36
Parents: ace1970
Author: Alan Conway <ac...@redhat.com>
Authored: Fri Oct 6 10:25:18 2017 -0400
Committer: Alan Conway <ac...@redhat.com>
Committed: Fri Oct 6 15:55:53 2017 -0400
----------------------------------------------------------------------
examples/cpp/ssl.cpp | 24 +---
.../bindings/cpp/include/proton/listener.hpp | 3 +
proton-c/bindings/cpp/src/listener.cpp | 8 +-
proton-c/src/tests/proactor.c | 119 ++++++++++++++-----
proton-c/src/tests/test_handler.h | 2 +
5 files changed, 103 insertions(+), 53 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/3e4b963a/examples/cpp/ssl.cpp
----------------------------------------------------------------------
diff --git a/examples/cpp/ssl.cpp b/examples/cpp/ssl.cpp
index 388f05b..d93454f 100644
--- a/examples/cpp/ssl.cpp
+++ b/examples/cpp/ssl.cpp
@@ -52,15 +52,7 @@ namespace {
const std::string verify_noname("noname"); // Skip matching host name against the certificate
const std::string verify_fail("fail"); // Force name mismatch failure
std::string verify(verify_full); // Default for example
- bool verify_failed(false);
std::string cert_directory;
-
- class example_cert_error : public std::runtime_error
- {
- public:
- explicit example_cert_error(const std::string& s) : std::runtime_error(s) {}
- };
-
}
@@ -136,9 +128,11 @@ class hello_world_direct : public proton::messaging_handler {
void on_transport_error(proton::transport &t) OVERRIDE {
std::string err = t.error().what();
- if (err.find("certificate") != std::string::npos) {
- verify_failed = true;
- throw example_cert_error(err);
+ if (verify == verify_fail && err.find("certificate") != std::string::npos) {
+ std::cout << "Expected failure of connection with wrong peer name: " << err
+ << std::endl;
+ } else {
+ std::cout << "Unexpected transport error: " << err << std::endl;
}
}
@@ -181,14 +175,6 @@ int main(int argc, char **argv) {
proton::container(hwd).run();
return 0;
} catch (const std::exception& e) {
- if (verify_failed) {
- if (verify == verify_fail) {
- std::cout << "Expected failure of connection with wrong peer name: " << e.what() << std::endl;
- return 0;
- } else {
- std::cerr << "unexpected internal certificate failure: ";
- }
- }
std::cerr << e.what() << std::endl;
}
return 1;
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/3e4b963a/proton-c/bindings/cpp/include/proton/listener.hpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/include/proton/listener.hpp b/proton-c/bindings/cpp/include/proton/listener.hpp
index 5f6ec6c..d8421be 100644
--- a/proton-c/bindings/cpp/include/proton/listener.hpp
+++ b/proton-c/bindings/cpp/include/proton/listener.hpp
@@ -38,6 +38,9 @@ class PN_CPP_CLASS_EXTERN listener {
public:
/// Create an empty listener.
PN_CPP_EXTERN listener();
+ PN_CPP_EXTERN listener(const listener&);
+ PN_CPP_EXTERN ~listener();
+ PN_CPP_EXTERN listener& operator=(const listener&);
/// Stop listening on the address provided to the call to
/// container::listen that returned this listener.
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/3e4b963a/proton-c/bindings/cpp/src/listener.cpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/src/listener.cpp b/proton-c/bindings/cpp/src/listener.cpp
index a9ca53d..2e38076 100644
--- a/proton-c/bindings/cpp/src/listener.cpp
+++ b/proton-c/bindings/cpp/src/listener.cpp
@@ -26,7 +26,13 @@
namespace proton {
listener::listener(): listener_(0) {}
-listener::listener(pn_listener_t* l) : listener_(l) {}
+listener::listener(pn_listener_t* l): listener_(l) {}
+// Out-of-line big-3 with trivial implementations, in case we need them in future.
+listener::listener(const listener& l) : listener_(l.listener_) {}
+listener::~listener() {}
+listener& listener::operator=(const listener& l) { listener_ = l.listener_; return *this; }
+
+// FIXME aconway 2017-10-06: should be a no-op if already closed
void listener::stop() { if (listener_) pn_listener_close(listener_); }
}
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/3e4b963a/proton-c/src/tests/proactor.c
----------------------------------------------------------------------
diff --git a/proton-c/src/tests/proactor.c b/proton-c/src/tests/proactor.c
index c34e0ed..38ee46a 100644
--- a/proton-c/src/tests/proactor.c
+++ b/proton-c/src/tests/proactor.c
@@ -185,7 +185,7 @@ static pn_event_type_t test_proactors_run(test_proactor_t *tps, size_t n) {
return e;
}
-/* Run an array of proactors till a handler returns an event. */
+/* Run an array of proactors till a handler returns the desired event. */
void test_proactors_run_until(test_proactor_t *tps, size_t n, pn_event_type_t want) {
while (test_proactors_get(tps, n) != want)
;
@@ -324,17 +324,19 @@ static pn_event_type_t common_handler(test_handler_t *th, pn_event_t *e) {
}
}
-/* Like common_handler but does not auto-close the listener after one accept */
+/* Like common_handler but does not auto-close the listener after one accept,
+ and returns on LISTENER_CLOSE
+*/
static pn_event_type_t listen_handler(test_handler_t *th, pn_event_t *e) {
switch (pn_event_type(e)) {
case PN_LISTENER_ACCEPT:
/* No automatic listener close/free for tests that accept multiple connections */
last_accepted = pn_connection();
pn_listener_accept(pn_event_listener(e), last_accepted);
+ /* No automatic close */
return PN_EVENT_NONE;
case PN_LISTENER_CLOSE:
- /* No automatic free */
return PN_LISTENER_CLOSE;
default:
@@ -747,60 +749,111 @@ static void test_release_free(test_t *t) {
pn_connection_free(pn_connection());
}
-/* TODO aconway 2017-03-27: need windows version with .p12 certs */
-#define CERTFILE(NAME) CMAKE_CURRENT_SOURCE_DIR "/ssl_certs/" NAME ".pem"
+#define SSL_FILE(NAME) CMAKE_CURRENT_SOURCE_DIR "/ssl_certs/" NAME
+#define SSL_PW "tserverpw"
+/* Windows vs. OpenSSL certificates */
+#if defined(_WIN32)
+# define CERTIFICATE(NAME) SSL_FILE(NAME "-certificate.p12")
+# define SET_CREDENTIALS(DOMAIN, NAME) \
+ pn_ssl_domain_set_credentials(DOMAIN, SSL_FILE(NAME "-full.p12"), "", SSL_PW)
+
+#else
+# define CERTIFICATE(NAME) SSL_FILE(NAME "-certificate.pem")
+# define SET_CREDENTIALS(DOMAIN, NAME) \
+ pn_ssl_domain_set_credentials(DOMAIN, CERTIFICATE(NAME), SSL_FILE(NAME "-private-key.pem"), SSL_PW)
+#endif
-static pn_event_type_t ssl_handler(test_handler_t *th, pn_event_t *e) {
- pn_connection_t *c = pn_event_connection(e);
+static pn_event_type_t ssl_handler(test_handler_t *h, pn_event_t *e) {
switch (pn_event_type(e)) {
- case PN_CONNECTION_BOUND: {
- bool incoming = (pn_connection_state(c) & PN_LOCAL_UNINIT);
- pn_ssl_domain_t *ssld = pn_ssl_domain(incoming ? PN_SSL_MODE_SERVER : PN_SSL_MODE_CLIENT);
- TEST_CHECK(th->t, 0 == pn_ssl_domain_set_credentials(
- ssld, CERTFILE("tserver-certificate"), CERTFILE("tserver-private-key"), "tserverpw"));
- TEST_CHECK(th->t, 0 == pn_ssl_init(pn_ssl(pn_event_transport(e)), ssld, NULL));
- pn_ssl_domain_free(ssld);
- return PN_EVENT_NONE;
+ case PN_CONNECTION_BOUND:
+ TEST_CHECK(h->t, 0 == pn_ssl_init(pn_ssl(pn_event_transport(e)), h->ssl_domain, NULL));
+ return PN_EVENT_NONE;
+
+ case PN_CONNECTION_REMOTE_OPEN: {
+ pn_ssl_t *ssl = pn_ssl(pn_event_transport(e));
+ TEST_CHECK(h->t, ssl);
+ TEST_CHECK(h->t, pn_ssl_get_protocol_name(ssl, NULL, 0));
+ return PN_CONNECTION_REMOTE_OPEN;
}
+ default:
+ return PN_EVENT_NONE;
+ }
+}
+static pn_event_type_t ssl_server_handler(test_handler_t *h, pn_event_t *e) {
+ switch (pn_event_type(e)) {
+ case PN_CONNECTION_BOUND:
case PN_CONNECTION_REMOTE_OPEN: {
- if (pn_connection_state(c) & PN_LOCAL_ACTIVE) {
- /* Outgoing connection is complete, close it */
- pn_connection_close(c);
- } else {
- /* Incoming connection, check for SSL */
- pn_ssl_t *ssl = pn_ssl(pn_event_transport(e));
- TEST_CHECK(th->t, ssl);
- TEST_CHECK(th->t, pn_ssl_get_protocol_name(ssl, NULL, 0));
- pn_connection_open(c); /* Return the open (no-op if already open) */
- }
- return PN_CONNECTION_REMOTE_OPEN;
+ pn_event_type_t et = ssl_handler(h, e);
+ pn_connection_open(pn_event_connection(e));
+ return et;
}
+ default:
+ return listen_handler(h, e);
+ }
+}
+static pn_event_type_t ssl_client_handler(test_handler_t *h, pn_event_t *e) {
+ switch (pn_event_type(e)) {
+ case PN_CONNECTION_BOUND:
+ case PN_CONNECTION_REMOTE_OPEN: {
+ pn_event_type_t et = ssl_handler(h, e);
+ pn_connection_close(pn_event_connection(e));
+ return et;
+ }
+ break;
default:
- return common_handler(th, e);
+ return common_handler(h, e);
}
}
-/* Establish an SSL connection between proactors*/
+/* Test various SSL connections between proactors*/
static void test_ssl(test_t *t) {
if (!pn_ssl_present()) {
TEST_LOGF(t, "Skip SSL test, no support");
return;
}
- test_proactor_t tps[] ={ test_proactor(t, ssl_handler), test_proactor(t, ssl_handler) };
- pn_proactor_t *client = tps[0].proactor;
- test_listener_t l = test_listen(&tps[1], localhost);
- pn_connection_t *c = pn_connection();
- pn_proactor_connect(client, c, l.port.host_port);
+ test_proactor_t tps[] ={ test_proactor(t, ssl_client_handler), test_proactor(t, ssl_server_handler) };
+ test_proactor_t *client = &tps[0], *server = &tps[1];
+ pn_ssl_domain_t *cd = client->handler.ssl_domain = pn_ssl_domain(PN_SSL_MODE_CLIENT);
+ pn_ssl_domain_t *sd = server->handler.ssl_domain = pn_ssl_domain(PN_SSL_MODE_SERVER);
+ TEST_CHECK(t, 0 == SET_CREDENTIALS(sd, "tserver"));
+ test_listener_t l = test_listen(server, localhost);
+
+ /* Basic SSL connection */
+ pn_proactor_connect(client->proactor, pn_connection(), l.port.host_port);
/* Open ok at both ends */
TEST_ETYPE_EQUAL(t, PN_CONNECTION_REMOTE_OPEN, TEST_PROACTORS_RUN(tps));
TEST_COND_EMPTY(t, last_condition);
TEST_ETYPE_EQUAL(t, PN_CONNECTION_REMOTE_OPEN, TEST_PROACTORS_RUN(tps));
TEST_COND_EMPTY(t, last_condition);
+ TEST_PROACTORS_DRAIN(tps);
+
+ /* Verify peer with good hostname */
+ TEST_INT_EQUAL(t, 0, pn_ssl_domain_set_trusted_ca_db(cd, CERTIFICATE("tserver")));
+ TEST_INT_EQUAL(t, 0, pn_ssl_domain_set_peer_authentication(cd, PN_SSL_VERIFY_PEER_NAME, NULL));
+ pn_connection_t *c = pn_connection();
+ pn_connection_set_hostname(c, "test_server");
+ pn_proactor_connect(client->proactor, c, l.port.host_port);
+ TEST_ETYPE_EQUAL(t, PN_CONNECTION_REMOTE_OPEN, TEST_PROACTORS_RUN(tps));
+ TEST_COND_EMPTY(t, last_condition);
+ TEST_ETYPE_EQUAL(t, PN_CONNECTION_REMOTE_OPEN, TEST_PROACTORS_RUN(tps));
+ TEST_COND_EMPTY(t, last_condition);
+ TEST_PROACTORS_DRAIN(tps);
+
+ /* Verify peer with bad hostname */
+ c = pn_connection();
+ pn_connection_set_hostname(c, "wrongname");
+ pn_proactor_connect(client->proactor, c, l.port.host_port);
+ TEST_ETYPE_EQUAL(t, PN_TRANSPORT_CLOSED, TEST_PROACTORS_RUN(tps));
+ TEST_COND_NAME(t, "amqp:connection:framing-error", last_condition);
+ TEST_COND_DESC(t, "SSL", last_condition);
+ TEST_PROACTORS_DRAIN(tps);
+ pn_ssl_domain_free(cd);
+ pn_ssl_domain_free(sd);
TEST_PROACTORS_DESTROY(tps);
}
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/3e4b963a/proton-c/src/tests/test_handler.h
----------------------------------------------------------------------
diff --git a/proton-c/src/tests/test_handler.h b/proton-c/src/tests/test_handler.h
index 6eb9079..108d0d9 100644
--- a/proton-c/src/tests/test_handler.h
+++ b/proton-c/src/tests/test_handler.h
@@ -21,6 +21,7 @@
*/
#include "test_tools.h"
+#include <proton/ssl.h>
/* C event handlers for tests */
@@ -45,6 +46,7 @@ typedef struct test_handler_t {
pn_link_t *sender;
pn_link_t *receiver;
pn_delivery_t *delivery;
+ pn_ssl_domain_t *ssl_domain;
} test_handler_t;
void test_handler_init(test_handler_t *th, test_t *t, test_handler_fn f) {
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org