From f31fba876697df9f4535117f479bf94781412abb Mon Sep 17 00:00:00 2001 From: Erik Boasson Date: Wed, 30 Jan 2019 14:39:34 +0100 Subject: [PATCH 1/7] fix and enable SSL support when OpenSSL is available Signed-off-by: Erik Boasson --- src/core/CMakeLists.txt | 9 +- src/core/ddsi/include/ddsi/ddsi_ssl.h | 7 +- src/core/ddsi/include/ddsi/ddsi_tcp.h | 15 +- src/core/ddsi/src/ddsi_ssl.c | 301 ++++++++++---------------- src/core/ddsi/src/ddsi_tcp.c | 48 ++-- src/core/ddsi/src/ddsi_tran.c | 15 +- src/core/ddsi/src/q_config.c | 2 +- 7 files changed, 158 insertions(+), 239 deletions(-) diff --git a/src/core/CMakeLists.txt b/src/core/CMakeLists.txt index fd87477..a1af207 100644 --- a/src/core/CMakeLists.txt +++ b/src/core/CMakeLists.txt @@ -19,7 +19,6 @@ FUNCTION(PREPEND var prefix) SET(${var} "${listVar}" PARENT_SCOPE) ENDFUNCTION(PREPEND) - option(DDSC_SHARED "Build DDSC as a shared library" ON) if(DDSC_SHARED AND ((NOT DEFINED BUILD_SHARED_LIBS) OR BUILD_SHARED_LIBS)) @@ -34,6 +33,14 @@ endif() add_definitions(-DDDSI_INCLUDE_NETWORK_PARTITIONS -DDDSI_INCLUDE_SSM) +find_package(OpenSSL) +if(OPENSSL_FOUND) + add_definitions(-DDDSI_INCLUDE_SSL) + include_directories(${OPENSSL_INCLUDE_DIR}) + target_link_libraries(ddsc PRIVATE ${OPENSSL_LIBRARIES}) + message(STATUS "Using OpenSSL ${OPENSSL_VERSION} at ${OPENSSL_INCLUDE_DIR}") +endif() + include(ddsi/CMakeLists.txt) include(ddsc/CMakeLists.txt) diff --git a/src/core/ddsi/include/ddsi/ddsi_ssl.h b/src/core/ddsi/include/ddsi/ddsi_ssl.h index c6c102a..493289e 100644 --- a/src/core/ddsi/include/ddsi/ddsi_ssl.h +++ b/src/core/ddsi/include/ddsi/ddsi_ssl.h @@ -15,14 +15,13 @@ #ifdef DDSI_INCLUDE_SSL #ifdef _WIN32 -/* WinSock2 must be included before openssl headers - otherwise winsock will be used */ +/* supposedly WinSock2 must be included before openssl headers otherwise winsock will be used */ #include #endif - #include -void ddsi_ssl_plugin (void); +struct ddsi_ssl_plugins; +void ddsi_ssl_config_plugin (struct ddsi_ssl_plugins *plugin); #endif #endif diff --git a/src/core/ddsi/include/ddsi/ddsi_tcp.h b/src/core/ddsi/include/ddsi/ddsi_tcp.h index feda2aa..b7f6fc0 100644 --- a/src/core/ddsi/include/ddsi/ddsi_tcp.h +++ b/src/core/ddsi/include/ddsi/ddsi_tcp.h @@ -20,20 +20,17 @@ struct ddsi_ssl_plugins { - void (*config) (void); - c_bool (*init) (void); + bool (*init) (void); void (*fini) (void); - void (*ssl_free) (SSL * ssl); - void (*bio_vfree) (BIO * bio); - os_ssize_t (*read) (SSL * ssl, void * buf, os_size_t len, int * err); - os_ssize_t (*write) (SSL * ssl, const void * msg, os_size_t len, int * err); + void (*ssl_free) (SSL *ssl); + void (*bio_vfree) (BIO *bio); + ssize_t (*read) (SSL *ssl, void *buf, size_t len, int *err); + ssize_t (*write) (SSL *ssl, const void *msg, size_t len, int *err); SSL * (*connect) (os_socket sock); BIO * (*listen) (os_socket sock); - SSL * (*accept) (BIO * bio, os_socket * sock); + SSL * (*accept) (BIO *bio, os_socket *sock); }; -struct ddsi_ssl_plugins ddsi_tcp_ssl_plugin; - #endif int ddsi_tcp_init (void); diff --git a/src/core/ddsi/src/ddsi_ssl.c b/src/core/ddsi/src/ddsi_ssl.c index a3bd0c0..5ae6a19 100644 --- a/src/core/ddsi/src/ddsi_ssl.c +++ b/src/core/ddsi/src/ddsi_ssl.c @@ -9,323 +9,242 @@ * * SPDX-License-Identifier: EPL-2.0 OR BSD-3-Clause */ +#include "os/os.h" +#include "ddsi/ddsi_tcp.h" #include "ddsi/ddsi_ssl.h" #include "ddsi/q_config.h" #include "ddsi/q_log.h" -#include "os/os_heap.h" -#include "ddsi/ddsi_tcp.h" #ifdef DDSI_INCLUDE_SSL +#include +#include +#include #include #include +#include -static SSL_CTX * ddsi_ssl_ctx = NULL; +static SSL_CTX *ddsi_ssl_ctx = NULL; -static SSL * ddsi_ssl_new (void) +static SSL *ddsi_ssl_new (void) { return SSL_new (ddsi_ssl_ctx); } -static void ddsi_ssl_error (SSL * ssl, const char * str, int err) +static void ddsi_ssl_error (SSL *ssl, const char *str, int err) { - char buff [128]; + char buff[128]; ERR_error_string ((unsigned) SSL_get_error (ssl, err), buff); - DDS_ERROR("tcp/ssl %s %s %d\n", str, buff, err); + DDS_ERROR ("tcp/ssl %s %s %d\n", str, buff, err); } -static int ddsi_ssl_verify (int ok, X509_STORE_CTX * store) +static int ddsi_ssl_verify (int ok, X509_STORE_CTX *store) { if (!ok) { char issuer[256]; - X509 * cert = X509_STORE_CTX_get_current_cert (store); + X509 *cert = X509_STORE_CTX_get_current_cert (store); int err = X509_STORE_CTX_get_error (store); /* Check if allowing self-signed certificates */ - - if - ( - config.ssl_self_signed && - ((err == X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT) || - (err == X509_V_ERR_SELF_SIGNED_CERT_IN_CHAIN)) - ) - { + if (config.ssl_self_signed && ((err == X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT) || (err == X509_V_ERR_SELF_SIGNED_CERT_IN_CHAIN))) ok = 1; - } else { X509_NAME_oneline (X509_get_issuer_name (cert), issuer, sizeof (issuer)); - DDS_ERROR - ( - "tcp/ssl failed to verify certificate from %s : %s\n", - issuer, - X509_verify_cert_error_string (err) - ); + DDS_ERROR ("tcp/ssl failed to verify certificate from %s: %s\n", issuer, X509_verify_cert_error_string (err)); } } return ok; } -static os_ssize_t ddsi_ssl_read (SSL * ssl, void * buf, os_size_t len, int * err) +static ssize_t ddsi_ssl_read (SSL *ssl, void *buf, size_t len, int *err) { - int ret; - assert (len <= INT32_MAX); - if (SSL_get_shutdown (ssl) != 0) - { return -1; - } /* Returns -1 on error or 0 on shutdown */ - - ret = SSL_read (ssl, buf, (int) len); + const int ret = SSL_read (ssl, buf, (int) len); switch (SSL_get_error (ssl, ret)) { case SSL_ERROR_NONE: - { - /* Success */ - - break; - } + return ret; case SSL_ERROR_WANT_READ: case SSL_ERROR_WANT_WRITE: - { *err = os_sockEAGAIN; - ret = -1; - break; - } + return -1; case SSL_ERROR_ZERO_RETURN: default: - { /* Connection closed or error */ - *err = os_getErrno (); - ret = -1; - break; - } + return -1; } - - return ret; } -static os_ssize_t ddsi_ssl_write (SSL * ssl, const void * buf, os_size_t len, int * err) +static ssize_t ddsi_ssl_write (SSL *ssl, const void *buf, size_t len, int *err) { - int ret; - assert(len <= INT32_MAX); if (SSL_get_shutdown (ssl) != 0) - { return -1; - } /* Returns -1 on error or 0 on shutdown */ - - ret = SSL_write (ssl, buf, (int) len); + const int ret = SSL_write (ssl, buf, (int) len); switch (SSL_get_error (ssl, ret)) { case SSL_ERROR_NONE: - { - /* Success */ - - break; - } + return ret; case SSL_ERROR_WANT_READ: case SSL_ERROR_WANT_WRITE: - { *err = os_sockEAGAIN; - ret = -1; - break; - } + return -1; case SSL_ERROR_ZERO_RETURN: default: - { /* Connection closed or error */ - *err = os_getErrno (); - ret = -1; - break; - } + return -1; } - - return ret; } /* Standard OpenSSL init and thread support routines. See O'Reilly. */ - +#if OPENSSL_VERSION_NUMBER < 0x10100000L static unsigned long ddsi_ssl_id (void) { - return os_threadIdToInteger (os_threadIdSelf ()); + return (unsigned long) os_threadIdToInteger (os_threadIdSelf ()); } -typedef struct CRYPTO_dynlock_value -{ +typedef struct CRYPTO_dynlock_value { os_mutex m_mutex; -} -CRYPTO_dynlock_value; +} CRYPTO_dynlock_value; -static CRYPTO_dynlock_value * ddsi_ssl_locks = NULL; +static CRYPTO_dynlock_value *ddsi_ssl_locks = NULL; -static void ddsi_ssl_dynlock_lock (int mode, CRYPTO_dynlock_value * lock, const char * file, int line) +static void ddsi_ssl_dynlock_lock (int mode, CRYPTO_dynlock_value *lock, const char *file, int line) { (void) file; (void) line; if (mode & CRYPTO_LOCK) - { os_mutexLock (&lock->m_mutex); - } else - { os_mutexUnlock (&lock->m_mutex); - } } -static void ddsi_ssl_lock (int mode, int n, const char * file, int line) +static void ddsi_ssl_lock (int mode, int n, const char *file, int line) { ddsi_ssl_dynlock_lock (mode, &ddsi_ssl_locks[n], file, line); } -static CRYPTO_dynlock_value * ddsi_ssl_dynlock_create (const char * file, int line) +static CRYPTO_dynlock_value *ddsi_ssl_dynlock_create (const char *file, int line) { - CRYPTO_dynlock_value * val = os_malloc (sizeof (*val)); - (void) file; (void) line; + CRYPTO_dynlock_value *val = os_malloc (sizeof (*val)); os_mutexInit (&val->m_mutex); return val; } -static void ddsi_ssl_dynlock_destroy (CRYPTO_dynlock_value * lock, const char * file, int line) +static void ddsi_ssl_dynlock_destroy (CRYPTO_dynlock_value *lock, const char *file, int line) { (void) file; (void) line; os_mutexDestroy (&lock->m_mutex); os_free (lock); } +#endif -static int ddsi_ssl_password (char * buf, int num, int rwflag, void * udata) +static int ddsi_ssl_password (char *buf, int num, int rwflag, void *udata) { (void) rwflag; (void) udata; - if ((unsigned int) num < strlen (config.ssl_key_pass) + 1) - { - return (0); - } + if (num < 0 || (size_t) num < strlen (config.ssl_key_pass) + 1) + return 0; + OS_WARNING_MSVC_OFF(4996); strcpy (buf, config.ssl_key_pass); + OS_WARNING_MSVC_ON(4996); return (int) strlen (config.ssl_key_pass); } -static SSL_CTX * ddsi_ssl_ctx_init (void) +static SSL_CTX *ddsi_ssl_ctx_init (void) { - int i; - SSL_CTX * ctx = SSL_CTX_new (TLSv1_method ()); + SSL_CTX *ctx = SSL_CTX_new (SSLv23_method ()); /* Load certificates */ - if (! SSL_CTX_use_certificate_file (ctx, config.ssl_keystore, SSL_FILETYPE_PEM)) { - DDS_LOG - ( - DDS_LC_ERROR | DDS_LC_CONFIG, - "tcp/ssl failed to load certificate from file: %s\n", - config.ssl_keystore - ); + DDS_LOG (DDS_LC_ERROR | DDS_LC_CONFIG, "tcp/ssl failed to load certificate from file: %s\n", config.ssl_keystore); goto fail; } /* Set password and callback */ - SSL_CTX_set_default_passwd_cb (ctx, ddsi_ssl_password); /* Get private key */ - if (! SSL_CTX_use_PrivateKey_file (ctx, config.ssl_keystore, SSL_FILETYPE_PEM)) { - DDS_LOG - ( - DDS_LC_ERROR | DDS_LC_CONFIG, - "tcp/ssl failed to load private key from file: %s\n", - config.ssl_keystore - ); + DDS_LOG (DDS_LC_ERROR | DDS_LC_CONFIG, "tcp/ssl failed to load private key from file: %s\n", config.ssl_keystore); goto fail; } /* Load CAs */ - if (! SSL_CTX_load_verify_locations (ctx, config.ssl_keystore, 0)) { - DDS_LOG - ( - DDS_LC_ERROR | DDS_LC_CONFIG, - "tcp/ssl failed to load CA from file: %s\n", - config.ssl_keystore - ); + DDS_LOG (DDS_LC_ERROR | DDS_LC_CONFIG, "tcp/ssl failed to load CA from file: %s\n", config.ssl_keystore); goto fail; } /* Set ciphers */ - if (! SSL_CTX_set_cipher_list (ctx, config.ssl_ciphers)) { - DDS_LOG - ( - DDS_LC_ERROR | DDS_LC_CONFIG, - "tcp/ssl failed to set ciphers: %s\n", - config.ssl_ciphers - ); + DDS_LOG (DDS_LC_ERROR | DDS_LC_CONFIG, "tcp/ssl failed to set ciphers: %s\n", config.ssl_ciphers); goto fail; } /* Load randomness from file (optional) */ - if (config.ssl_rand_file[0] != '\0') { if (! RAND_load_file (config.ssl_rand_file, 4096)) { - DDS_LOG - ( - DDS_LC_ERROR | DDS_LC_CONFIG, - "tcp/ssl failed to load random seed from file: %s\n", - config.ssl_rand_file - ); + DDS_LOG (DDS_LC_ERROR | DDS_LC_CONFIG, "tcp/ssl failed to load random seed from file: %s\n", config.ssl_rand_file); goto fail; } } /* Set certificate verification policy from configuration */ - - if (config.ssl_verify) - { - i = SSL_VERIFY_PEER; - if (config.ssl_verify_client) - { - i |= SSL_VERIFY_FAIL_IF_NO_PEER_CERT; - } - SSL_CTX_set_verify (ctx, i, ddsi_ssl_verify); - } + if (!config.ssl_verify) + SSL_CTX_set_verify (ctx, SSL_VERIFY_NONE, NULL); else { - SSL_CTX_set_verify (ctx, SSL_VERIFY_NONE, NULL); + int i = SSL_VERIFY_PEER; + if (config.ssl_verify_client) + i |= SSL_VERIFY_FAIL_IF_NO_PEER_CERT; + SSL_CTX_set_verify (ctx, i, ddsi_ssl_verify); } - SSL_CTX_set_options (ctx, SSL_OP_ALL | SSL_OP_NO_SSLv2); - + SSL_CTX_set_options (ctx, SSL_OP_ALL | SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3 | SSL_OP_NO_TLSv1 | SSL_OP_NO_TLSv1_1); return ctx; fail: - SSL_CTX_free (ctx); return NULL; } -static SSL * ddsi_ssl_connect (os_socket sock) +static void dds_report_tls_version (const SSL *ssl, const char *oper) { - SSL * ssl; + if (ssl) + { + char issuer[256], subject[256]; + X509_NAME_oneline (X509_get_issuer_name (SSL_get_peer_certificate (ssl)), issuer, sizeof (issuer)); + X509_NAME_oneline (X509_get_subject_name (SSL_get_peer_certificate (ssl)), subject, sizeof (subject)); + DDS_TRACE("tcp/ssl %s %s issued by %s [%s]\n", oper, subject, issuer, SSL_get_version (ssl)); + } +} + +static SSL *ddsi_ssl_connect (os_socket sock) +{ + SSL *ssl; int err; /* Connect SSL over connected socket */ - ssl = ddsi_ssl_new (); SSL_set_fd (ssl, sock); err = SSL_connect (ssl); @@ -335,20 +254,21 @@ static SSL * ddsi_ssl_connect (os_socket sock) SSL_free (ssl); ssl = NULL; } + dds_report_tls_version (ssl, "connected to"); return ssl; } -static BIO * ddsi_ssl_listen (os_socket sock) +static BIO *ddsi_ssl_listen (os_socket sock) { BIO * bio = BIO_new (BIO_s_accept ()); BIO_set_fd (bio, sock, BIO_NOCLOSE); return bio; } -static SSL * ddsi_ssl_accept (BIO * bio, os_socket * sock) +static SSL *ddsi_ssl_accept (BIO *bio, os_socket *sock) { - SSL * ssl = NULL; - BIO * nbio; + SSL *ssl = NULL; + BIO *nbio; int err; if (BIO_do_accept (bio) > 0) @@ -365,23 +285,29 @@ static SSL * ddsi_ssl_accept (BIO * bio, os_socket * sock) ssl = NULL; } } + dds_report_tls_version (ssl, "accepted from"); return ssl; } -static c_bool ddsi_ssl_init (void) +static bool ddsi_ssl_init (void) { - unsigned locks = (unsigned) CRYPTO_num_locks (); - unsigned i; - - ddsi_ssl_locks = os_malloc (sizeof (CRYPTO_dynlock_value) * locks); - for (i = 0; i < locks; i++) - { - os_mutexInit (&ddsi_ssl_locks[i].m_mutex); - } ERR_load_BIO_strings (); SSL_load_error_strings (); SSL_library_init (); OpenSSL_add_all_algorithms (); + +#if OPENSSL_VERSION_NUMBER < 0x10100000L + { + const int locks = CRYPTO_num_locks (); + assert (locks >= 0); + ddsi_ssl_locks = os_malloc (sizeof (CRYPTO_dynlock_value) * (size_t) locks); + for (int i = 0; i < locks; i++) + os_mutexInit (&ddsi_ssl_locks[i].m_mutex); + } +#endif + /* Leave these in place: OpenSSL 1.1 defines them as no-op macros that not even reference the symbol, + therefore leaving them in means we get compile time errors if we the library expects the callbacks + to be defined and we somehow failed to detect that previously */ CRYPTO_set_id_callback (ddsi_ssl_id); CRYPTO_set_locking_callback (ddsi_ssl_lock); CRYPTO_set_dynlock_create_callback (ddsi_ssl_dynlock_create); @@ -394,43 +320,36 @@ static c_bool ddsi_ssl_init (void) static void ddsi_ssl_fini (void) { - unsigned locks = (unsigned) CRYPTO_num_locks (); - unsigned i; - SSL_CTX_free (ddsi_ssl_ctx); - CRYPTO_set_id_callback (NULL); - CRYPTO_set_locking_callback (NULL); - CRYPTO_set_dynlock_create_callback (NULL); - CRYPTO_set_dynlock_lock_callback (NULL); - CRYPTO_set_dynlock_destroy_callback (NULL); + CRYPTO_set_id_callback (0); + CRYPTO_set_locking_callback (0); + CRYPTO_set_dynlock_create_callback (0); + CRYPTO_set_dynlock_lock_callback (0); + CRYPTO_set_dynlock_destroy_callback (0); ERR_free_strings (); EVP_cleanup (); - for (i = 0; i < locks; i++) + +#if OPENSSL_VERSION_NUMBER < 0x10100000L { - os_mutexDestroy (&ddsi_ssl_locks[i].m_mutex); + const int locks = CRYPTO_num_locks (); + for (int i = 0; i < locks; i++) + os_mutexDestroy (&ddsi_ssl_locks[i].m_mutex); + os_free (ddsi_ssl_locks); } - os_free (ddsi_ssl_locks); +#endif } -static void ddsi_ssl_config (void) +void ddsi_ssl_config_plugin (struct ddsi_ssl_plugins *plugin) { - if (config.ssl_enable) - { - ddsi_tcp_ssl_plugin.init = ddsi_ssl_init; - ddsi_tcp_ssl_plugin.fini = ddsi_ssl_fini; - ddsi_tcp_ssl_plugin.ssl_free = SSL_free; - ddsi_tcp_ssl_plugin.bio_vfree = BIO_vfree; - ddsi_tcp_ssl_plugin.read = ddsi_ssl_read; - ddsi_tcp_ssl_plugin.write = ddsi_ssl_write; - ddsi_tcp_ssl_plugin.connect = ddsi_ssl_connect; - ddsi_tcp_ssl_plugin.listen = ddsi_ssl_listen; - ddsi_tcp_ssl_plugin.accept = ddsi_ssl_accept; - } -} - -void ddsi_ssl_plugin (void) -{ - ddsi_tcp_ssl_plugin.config = ddsi_ssl_config; + plugin->init = ddsi_ssl_init; + plugin->fini = ddsi_ssl_fini; + plugin->ssl_free = SSL_free; + plugin->bio_vfree = BIO_vfree; + plugin->read = ddsi_ssl_read; + plugin->write = ddsi_ssl_write; + plugin->connect = ddsi_ssl_connect; + plugin->listen = ddsi_ssl_listen; + plugin->accept = ddsi_ssl_accept; } #endif /* DDSI_INCLUDE_SSL */ diff --git a/src/core/ddsi/src/ddsi_tcp.c b/src/core/ddsi/src/ddsi_tcp.c index 328a566..58b2ed4 100644 --- a/src/core/ddsi/src/ddsi_tcp.c +++ b/src/core/ddsi/src/ddsi_tcp.c @@ -30,16 +30,11 @@ typedef struct ddsi_tran_factory * ddsi_tcp_factory_g_t; static os_atomic_uint32_t ddsi_tcp_init_g = OS_ATOMIC_UINT32_INIT(0); #ifdef DDSI_INCLUDE_SSL -struct ddsi_ssl_plugins ddsi_tcp_ssl_plugin = - { NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL }; +static struct ddsi_ssl_plugins ddsi_tcp_ssl_plugin; #endif static const char * ddsi_name = "tcp"; -/* Stateless singleton instance handed out as client connection */ - -static struct ddsi_tran_conn ddsi_tcp_conn_client; - /* ddsi_tcp_conn: TCP connection for reading and writing. Mutex prevents concurrent writes to socket. Is reference counted. Peer port is actually contained in peer @@ -74,6 +69,10 @@ typedef struct ddsi_tcp_listener } * ddsi_tcp_listener_t; +/* Stateless singleton instance handed out as client connection */ + +static struct ddsi_tcp_conn ddsi_tcp_conn_client; + static int ddsi_tcp_cmp_conn (const struct ddsi_tcp_conn *c1, const struct ddsi_tcp_conn *c2) { const os_sockaddr *a1s = (os_sockaddr *)&c1->m_peer_addr; @@ -348,7 +347,7 @@ OS_WARNING_MSVC_ON(4267); } #ifdef DDSI_INCLUDE_SSL -static os_ssize_t ddsi_tcp_conn_read_ssl (ddsi_tcp_conn_t tcp, void * buf, os_size_t len, int * err) +static ssize_t ddsi_tcp_conn_read_ssl (ddsi_tcp_conn_t tcp, void * buf, size_t len, int * err) { return (ddsi_tcp_ssl_plugin.read) (tcp->m_ssl, buf, len, err); } @@ -471,7 +470,7 @@ OS_WARNING_MSVC_OFF(4267); } #ifdef DDSI_INCLUDE_SSL -static os_ssize_t ddsi_tcp_conn_write_ssl (ddsi_tcp_conn_t conn, const void * buf, os_size_t len, int * err) +static ssize_t ddsi_tcp_conn_write_ssl (ddsi_tcp_conn_t conn, const void * buf, size_t len, int * err) { return (ddsi_tcp_ssl_plugin.write) (conn->m_ssl, buf, len, err); } @@ -541,7 +540,7 @@ static ssize_t ddsi_tcp_conn_write (ddsi_tran_conn_t base, const nn_locator_t *d { #ifdef DDSI_INCLUDE_SSL char msgbuf[4096]; /* stack buffer for merging smallish writes without requiring allocations */ - struct iovec iovec; /* iovec used for msgbuf */ + os_iovec_t iovec; /* iovec used for msgbuf */ #endif ssize_t ret; size_t len; @@ -732,8 +731,7 @@ static ddsi_tran_conn_t ddsi_tcp_create_conn (uint32_t port, ddsi_tran_qos_t qos { (void) qos; (void) port; - - return (ddsi_tran_conn_t) &ddsi_tcp_conn_client; + return &ddsi_tcp_conn_client.m_base; } static int ddsi_tcp_listen (ddsi_tran_listener_t listener) @@ -936,7 +934,7 @@ static void ddsi_tcp_conn_delete (ddsi_tcp_conn_t conn) static void ddsi_tcp_close_conn (ddsi_tran_conn_t tc) { - if (tc != (ddsi_tran_conn_t) &ddsi_tcp_conn_client) + if (tc != &ddsi_tcp_conn_client.m_base) { char buff[DDSI_LOCSTRLEN]; nn_locator_t loc; @@ -952,7 +950,7 @@ static void ddsi_tcp_close_conn (ddsi_tran_conn_t tc) static void ddsi_tcp_release_conn (ddsi_tran_conn_t conn) { - if (conn != (ddsi_tran_conn_t) &ddsi_tcp_conn_client) + if (conn != &ddsi_tcp_conn_client.m_base) { ddsi_tcp_conn_delete ((ddsi_tcp_conn_t) conn); } @@ -964,13 +962,6 @@ static void ddsi_tcp_unblock_listener (ddsi_tran_listener_t listener) os_socket sock; int ret; -#ifdef DDSI_INCLUDE_SSL - if (ddsi_tcp_ssl_plugin.bio_vfree) - { - (ddsi_tcp_ssl_plugin.bio_vfree) (tl->m_bio); - } -#endif - /* Connect to own listener socket to wake listener from blocking 'accept()' */ ddsi_tcp_sock_new (&sock, 0); if (sock != OS_INVALID_SOCKET) @@ -1020,6 +1011,12 @@ static void ddsi_tcp_unblock_listener (ddsi_tran_listener_t listener) static void ddsi_tcp_release_listener (ddsi_tran_listener_t listener) { ddsi_tcp_listener_t tl = (ddsi_tcp_listener_t) listener; +#ifdef DDSI_INCLUDE_SSL + if (ddsi_tcp_ssl_plugin.bio_vfree) + { + (ddsi_tcp_ssl_plugin.bio_vfree) (tl->m_bio); + } +#endif ddsi_tcp_sock_free (tl->m_sock, "listener"); os_free (tl); } @@ -1097,17 +1094,14 @@ int ddsi_tcp_init (void) #endif memset (&ddsi_tcp_conn_client, 0, sizeof (ddsi_tcp_conn_client)); - ddsi_tcp_base_init (&ddsi_tcp_conn_client); + ddsi_tcp_base_init (&ddsi_tcp_conn_client.m_base); #ifdef DDSI_INCLUDE_SSL - if (ddsi_tcp_ssl_plugin.config) - { - (ddsi_tcp_ssl_plugin.config) (); - } - if (ddsi_tcp_ssl_plugin.init) + if (config.ssl_enable) { ddsi_name = "tcp/ssl"; - if (! (ddsi_tcp_ssl_plugin.init) ()) + ddsi_ssl_config_plugin (&ddsi_tcp_ssl_plugin); + if (! ddsi_tcp_ssl_plugin.init ()) { DDS_ERROR("Failed to initialize OpenSSL\n"); return -1; diff --git a/src/core/ddsi/src/ddsi_tran.c b/src/core/ddsi/src/ddsi_tran.c index eef0616..2ecfdce 100644 --- a/src/core/ddsi/src/ddsi_tran.c +++ b/src/core/ddsi/src/ddsi_tran.c @@ -59,12 +59,15 @@ ddsi_tran_factory_t ddsi_factory_find (const char * type) void ddsi_tran_factories_fini (void) { - ddsi_tran_factory_t factory; - - while ((factory = ddsi_tran_factories) != NULL) { - ddsi_tran_factories = factory->m_factory; - ddsi_factory_free(factory); - } + ddsi_tran_factory_t factory; + while ((factory = ddsi_tran_factories) != NULL) + { + /* Keep the factory in the list for the duration of "factory_free" so that + conversion of locator kind to factory remains possible. */ + ddsi_tran_factory_t next = factory->m_factory; + ddsi_factory_free (factory); + ddsi_tran_factories = next; + } } static ddsi_tran_factory_t ddsi_factory_find_with_len (const char * type, size_t len) diff --git a/src/core/ddsi/src/q_config.c b/src/core/ddsi/src/q_config.c index df153d9..6cea54a 100644 --- a/src/core/ddsi/src/q_config.c +++ b/src/core/ddsi/src/q_config.c @@ -679,7 +679,7 @@ static const struct cfgelem ssl_cfgelems[] = { "

This enables SSL/TLS for TCP.

" }, { LEAF("CertificateVerification"), 1, "true", ABSOFF(ssl_verify), 0, uf_boolean, 0, pf_boolean, "

If disabled this allows SSL connections to occur even if an X509 certificate fails verification.

" }, - { LEAF("VerifyClient"), 1, "false", ABSOFF(ssl_verify_client), 0, uf_boolean, 0, pf_boolean, + { LEAF("VerifyClient"), 1, "true", ABSOFF(ssl_verify_client), 0, uf_boolean, 0, pf_boolean, "

This enables an SSL server checking the X509 certificate of a connecting client.

" }, { LEAF("SelfSignedCertificates"), 1, "false", ABSOFF(ssl_self_signed), 0, uf_boolean, 0, pf_boolean, "

This enables the use of self signed X509 certificates.

" }, From 334a85e0f17c33ceef8b5f8e203e0182b118d21c Mon Sep 17 00:00:00 2001 From: Jeroen Koekkoek Date: Thu, 31 Jan 2019 17:09:05 +0100 Subject: [PATCH 2/7] Make usage of Conan provided OpenSSL transparent Signed-off-by: Jeroen Koekkoek --- src/CMakeLists.txt | 1 + src/cmake/modules/FindOpenSSL.cmake | 34 +++++++++++++++++++++++++++++ src/core/CMakeLists.txt | 4 +--- 3 files changed, 36 insertions(+), 3 deletions(-) create mode 100644 src/cmake/modules/FindOpenSSL.cmake diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 9de5427..08a48ad 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -80,6 +80,7 @@ if(EXISTS "${CMAKE_BINARY_DIR}/conanbuildinfo.cmake") else() conan_basic_setup() endif() + conan_define_targets() endif() # Set reasonably strict warning options for clang, gcc, msvc diff --git a/src/cmake/modules/FindOpenSSL.cmake b/src/cmake/modules/FindOpenSSL.cmake new file mode 100644 index 0000000..402bd72 --- /dev/null +++ b/src/cmake/modules/FindOpenSSL.cmake @@ -0,0 +1,34 @@ +# +# Copyright(c) 2006 to 2018 ADLINK Technology Limited and others +# +# This program and the accompanying materials are made available under the +# terms of the Eclipse Public License v. 2.0 which is available at +# http://www.eclipse.org/legal/epl-2.0, or the Eclipse Distribution License +# v. 1.0 which is available at +# http://www.eclipse.org/org/documents/edl-v10.php. +# +# SPDX-License-Identifier: EPL-2.0 OR BSD-3-Clause +# +if(TARGET CONAN_PKG::OpenSSL) + add_library(OpenSSL INTERFACE IMPORTED) #ALIAS CONAN_PKG::OpenSSL) + target_link_libraries(OpenSSL INTERFACE CONAN_PKG::OpenSSL) + set(OPENSSL_FOUND TRUE) +else() + # Loop over a list of possible module paths (without the current directory). + get_filename_component(DIR "${CMAKE_CURRENT_LIST_DIR}" ABSOLUTE) + + foreach(MODULE_DIR ${CMAKE_MODULE_PATH} ${CMAKE_ROOT}/Modules) + get_filename_component(MODULE_DIR "${MODULE_DIR}" ABSOLUTE) + if(NOT MODULE_DIR STREQUAL DIR) + if(EXISTS "${MODULE_DIR}/FindOpenSSL.cmake") + set(FIND_PACKAGE_FILE "${MODULE_DIR}/FindOpenSSL.cmake") + break() + endif() + endif() + endforeach() + + if(FIND_PACKAGE_FILE) + include("${FIND_PACKAGE_FILE}") + endif() +endif() + diff --git a/src/core/CMakeLists.txt b/src/core/CMakeLists.txt index a1af207..88993b5 100644 --- a/src/core/CMakeLists.txt +++ b/src/core/CMakeLists.txt @@ -36,9 +36,7 @@ add_definitions(-DDDSI_INCLUDE_NETWORK_PARTITIONS -DDDSI_INCLUDE_SSM) find_package(OpenSSL) if(OPENSSL_FOUND) add_definitions(-DDDSI_INCLUDE_SSL) - include_directories(${OPENSSL_INCLUDE_DIR}) - target_link_libraries(ddsc PRIVATE ${OPENSSL_LIBRARIES}) - message(STATUS "Using OpenSSL ${OPENSSL_VERSION} at ${OPENSSL_INCLUDE_DIR}") + target_link_libraries(ddsc PRIVATE OpenSSL) endif() include(ddsi/CMakeLists.txt) From b8329ce20638234c789f298dcf8186dbfda8f226 Mon Sep 17 00:00:00 2001 From: Erik Boasson Date: Thu, 31 Jan 2019 17:19:18 +0100 Subject: [PATCH 3/7] Require OpenSSL by default and add list it as a dependency for Conan OpenSSL support remains optional, but instead of including it or not solely depending on whether cmake manages to find it, there is now a DDSC_ENABLE_OPENSSL option that defaults to "ON". Setting to OFF removes the dependency. The Conan configuration has been updated to automatically provide openssl 1.1.1a. Signed-off-by: Erik Boasson --- conanfile.txt | 1 + src/cmake/modules/FindOpenSSL.cmake | 4 ++-- src/core/CMakeLists.txt | 16 ++++++++++++---- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/conanfile.txt b/conanfile.txt index 6d20935..7a4c6fe 100644 --- a/conanfile.txt +++ b/conanfile.txt @@ -1,5 +1,6 @@ [requires] cunit/2.1-3@bincrafters/stable +OpenSSL/1.1.1a@conan/stable [generators] cmake diff --git a/src/cmake/modules/FindOpenSSL.cmake b/src/cmake/modules/FindOpenSSL.cmake index 402bd72..92c39bc 100644 --- a/src/cmake/modules/FindOpenSSL.cmake +++ b/src/cmake/modules/FindOpenSSL.cmake @@ -10,8 +10,8 @@ # SPDX-License-Identifier: EPL-2.0 OR BSD-3-Clause # if(TARGET CONAN_PKG::OpenSSL) - add_library(OpenSSL INTERFACE IMPORTED) #ALIAS CONAN_PKG::OpenSSL) - target_link_libraries(OpenSSL INTERFACE CONAN_PKG::OpenSSL) + add_library(OpenSSL::SSL INTERFACE IMPORTED) + target_link_libraries(OpenSSL::SSL INTERFACE CONAN_PKG::OpenSSL) set(OPENSSL_FOUND TRUE) else() # Loop over a list of possible module paths (without the current directory). diff --git a/src/core/CMakeLists.txt b/src/core/CMakeLists.txt index 88993b5..a307c08 100644 --- a/src/core/CMakeLists.txt +++ b/src/core/CMakeLists.txt @@ -33,10 +33,18 @@ endif() add_definitions(-DDDSI_INCLUDE_NETWORK_PARTITIONS -DDDSI_INCLUDE_SSM) -find_package(OpenSSL) -if(OPENSSL_FOUND) - add_definitions(-DDDSI_INCLUDE_SSL) - target_link_libraries(ddsc PRIVATE OpenSSL) +option(DDSC_ENABLE_OPENSSL "Enable openssl support" ON) +if(DDSC_ENABLE_OPENSSL) + find_package(OpenSSL) + if(OPENSSL_FOUND) + add_definitions(-DDDSI_INCLUDE_SSL) + target_link_libraries(ddsc PRIVATE OpenSSL::SSL) + if(CMAKE_GENERATOR MATCHES "Visual Studio") + set_target_properties(ddsc PROPERTIES LINK_FLAGS "/ignore:4099") + endif() + else() + message(FATAL_ERROR "To build without openssl support, set DDSC_ENABLE_OPENSSL to OFF") + endif() endif() include(ddsi/CMakeLists.txt) From 228aa71967b15785d48cb18a82cd46ab981b646e Mon Sep 17 00:00:00 2001 From: Erik Boasson Date: Thu, 31 Jan 2019 17:25:16 +0100 Subject: [PATCH 4/7] TLS1.3 requires handling "spurious reads" With TLS1.3 the socket can indicate data is available even when there is no application data. This commit ignores a 0-byte read when no data is required. Long-term, handling short reads/writes in TCP mode need to be handled completely differently, but that is a story for another day. Signed-off-by: Erik Boasson --- src/core/ddsi/include/ddsi/ddsi_tran.h | 6 +++--- src/core/ddsi/src/ddsi_raweth.c | 3 ++- src/core/ddsi/src/ddsi_tcp.c | 8 ++++---- src/core/ddsi/src/ddsi_tran.c | 2 +- src/core/ddsi/src/ddsi_udp.c | 3 ++- src/core/ddsi/src/q_receive.c | 12 +++++++++--- 6 files changed, 21 insertions(+), 13 deletions(-) diff --git a/src/core/ddsi/include/ddsi/ddsi_tran.h b/src/core/ddsi/include/ddsi/ddsi_tran.h index e573591..5ae1687 100644 --- a/src/core/ddsi/include/ddsi/ddsi_tran.h +++ b/src/core/ddsi/include/ddsi/ddsi_tran.h @@ -36,7 +36,7 @@ typedef struct ddsi_tran_qos * ddsi_tran_qos_t; /* Function pointer types */ -typedef ssize_t (*ddsi_tran_read_fn_t) (ddsi_tran_conn_t, unsigned char *, size_t, nn_locator_t *); +typedef ssize_t (*ddsi_tran_read_fn_t) (ddsi_tran_conn_t, unsigned char *, size_t, bool, nn_locator_t *); typedef ssize_t (*ddsi_tran_write_fn_t) (ddsi_tran_conn_t, const nn_locator_t *, size_t, const os_iovec_t *, uint32_t); typedef int (*ddsi_tran_locator_fn_t) (ddsi_tran_base_t, nn_locator_t *); typedef bool (*ddsi_tran_supports_fn_t) (int32_t); @@ -220,8 +220,8 @@ inline int ddsi_conn_locator (ddsi_tran_conn_t conn, nn_locator_t * loc) { inline ssize_t ddsi_conn_write (ddsi_tran_conn_t conn, const nn_locator_t *dst, size_t niov, const os_iovec_t *iov, uint32_t flags) { return conn->m_closed ? -1 : (conn->m_write_fn) (conn, dst, niov, iov, flags); } -inline ssize_t ddsi_conn_read (ddsi_tran_conn_t conn, unsigned char * buf, size_t len, nn_locator_t *srcloc) { - return conn->m_closed ? -1 : conn->m_read_fn (conn, buf, len, srcloc); +inline ssize_t ddsi_conn_read (ddsi_tran_conn_t conn, unsigned char * buf, size_t len, bool allow_spurious, nn_locator_t *srcloc) { + return conn->m_closed ? -1 : conn->m_read_fn (conn, buf, len, allow_spurious, srcloc); } bool ddsi_conn_peer_locator (ddsi_tran_conn_t conn, nn_locator_t * loc); void ddsi_conn_disable_multiplexing (ddsi_tran_conn_t conn); diff --git a/src/core/ddsi/src/ddsi_raweth.c b/src/core/ddsi/src/ddsi_raweth.c index 4c8cb2d..46c01f9 100644 --- a/src/core/ddsi/src/ddsi_raweth.c +++ b/src/core/ddsi/src/ddsi_raweth.c @@ -63,7 +63,7 @@ static char *ddsi_raweth_to_string (ddsi_tran_factory_t tran, char *dst, size_t return dst; } -static ssize_t ddsi_raweth_conn_read (ddsi_tran_conn_t conn, unsigned char * buf, size_t len, nn_locator_t *srcloc) +static ssize_t ddsi_raweth_conn_read (ddsi_tran_conn_t conn, unsigned char * buf, size_t len, bool allow_spurious, nn_locator_t *srcloc) { int err; ssize_t ret; @@ -71,6 +71,7 @@ static ssize_t ddsi_raweth_conn_read (ddsi_tran_conn_t conn, unsigned char * buf struct sockaddr_ll src; struct iovec msg_iov; socklen_t srclen = (socklen_t) sizeof (src); + (void) allow_spurious; msg_iov.iov_base = (void*) buf; msg_iov.iov_len = len; diff --git a/src/core/ddsi/src/ddsi_tcp.c b/src/core/ddsi/src/ddsi_tcp.c index 58b2ed4..3ec01d9 100644 --- a/src/core/ddsi/src/ddsi_tcp.c +++ b/src/core/ddsi/src/ddsi_tcp.c @@ -395,7 +395,7 @@ static int err_is_AGAIN_or_WOULDBLOCK (int err) return 0; } -static ssize_t ddsi_tcp_conn_read (ddsi_tran_conn_t conn, unsigned char * buf, size_t len, nn_locator_t *srcloc) +static ssize_t ddsi_tcp_conn_read (ddsi_tran_conn_t conn, unsigned char * buf, size_t len, bool allow_spurious, nn_locator_t *srcloc) { ddsi_tcp_conn_t tcp = (ddsi_tcp_conn_t) conn; ssize_t (*rd) (ddsi_tcp_conn_t, void *, size_t, int * err) = ddsi_tcp_conn_read_plain; @@ -436,10 +436,10 @@ static ssize_t ddsi_tcp_conn_read (ddsi_tran_conn_t conn, unsigned char * buf, s { if (err_is_AGAIN_or_WOULDBLOCK (err)) { - if (ddsi_tcp_select (tcp->m_sock, true, pos) == false) - { + if (allow_spurious && pos == 0) + return 0; + else if (ddsi_tcp_select (tcp->m_sock, true, pos) == false) break; - } } else { diff --git a/src/core/ddsi/src/ddsi_tran.c b/src/core/ddsi/src/ddsi_tran.c index 2ecfdce..31c5276 100644 --- a/src/core/ddsi/src/ddsi_tran.c +++ b/src/core/ddsi/src/ddsi_tran.c @@ -31,7 +31,7 @@ extern inline int ddsi_tran_locator (ddsi_tran_base_t base, nn_locator_t * loc); extern inline int ddsi_listener_locator (ddsi_tran_listener_t listener, nn_locator_t * loc); extern inline int ddsi_listener_listen (ddsi_tran_listener_t listener); extern inline ddsi_tran_conn_t ddsi_listener_accept (ddsi_tran_listener_t listener); -extern inline ssize_t ddsi_conn_read (ddsi_tran_conn_t conn, unsigned char * buf, size_t len, nn_locator_t *srcloc); +extern inline ssize_t ddsi_conn_read (ddsi_tran_conn_t conn, unsigned char * buf, size_t len, bool allow_spurious, nn_locator_t *srcloc); extern inline ssize_t ddsi_conn_write (ddsi_tran_conn_t conn, const nn_locator_t *dst, size_t niov, const os_iovec_t *iov, uint32_t flags); void ddsi_factory_add (ddsi_tran_factory_t factory) diff --git a/src/core/ddsi/src/ddsi_udp.c b/src/core/ddsi/src/ddsi_udp.c index aaa12db..6d5afd6 100644 --- a/src/core/ddsi/src/ddsi_udp.c +++ b/src/core/ddsi/src/ddsi_udp.c @@ -48,7 +48,7 @@ static struct ddsi_udp_config ddsi_udp_config_g; static struct ddsi_tran_factory ddsi_udp_factory_g; static os_atomic_uint32_t ddsi_udp_init_g = OS_ATOMIC_UINT32_INIT(0); -static ssize_t ddsi_udp_conn_read (ddsi_tran_conn_t conn, unsigned char * buf, size_t len, nn_locator_t *srcloc) +static ssize_t ddsi_udp_conn_read (ddsi_tran_conn_t conn, unsigned char * buf, size_t len, bool allow_spurious, nn_locator_t *srcloc) { int err; ssize_t ret; @@ -56,6 +56,7 @@ static ssize_t ddsi_udp_conn_read (ddsi_tran_conn_t conn, unsigned char * buf, s os_sockaddr_storage src; os_iovec_t msg_iov; socklen_t srclen = (socklen_t) sizeof (src); + (void) allow_spurious; msg_iov.iov_base = (void*) buf; msg_iov.iov_len = (os_iov_len_t)len; /* Windows uses unsigned, POSIX (except Linux) int */ diff --git a/src/core/ddsi/src/q_receive.c b/src/core/ddsi/src/q_receive.c index 16d193f..7a2cd8f 100644 --- a/src/core/ddsi/src/q_receive.c +++ b/src/core/ddsi/src/q_receive.c @@ -2970,7 +2970,13 @@ static bool do_packet /* Read in DDSI header plus MSG_LEN sub message that follows it */ - sz = ddsi_conn_read (conn, buff, stream_hdr_size, &srcloc); + sz = ddsi_conn_read (conn, buff, stream_hdr_size, true, &srcloc); + if (sz == 0) + { + /* Spurious read -- which at this point is still ok */ + nn_rmsg_commit (rmsg); + return true; + } /* Read in remainder of packet */ @@ -2998,7 +3004,7 @@ static bool do_packet } else { - sz = ddsi_conn_read (conn, buff + stream_hdr_size, ml->length - stream_hdr_size, NULL); + sz = ddsi_conn_read (conn, buff + stream_hdr_size, ml->length - stream_hdr_size, false, NULL); if (sz > 0) { sz = (ssize_t) ml->length; @@ -3010,7 +3016,7 @@ static bool do_packet { /* Get next packet */ - sz = ddsi_conn_read (conn, buff, buff_len, &srcloc); + sz = ddsi_conn_read (conn, buff, buff_len, true, &srcloc); } if (sz > 0 && !gv.deaf) From c5e1c5b2f1595f5110c9a2daf0e25dac7ba67efa Mon Sep 17 00:00:00 2001 From: Erik Boasson Date: Thu, 31 Jan 2019 17:37:27 +0100 Subject: [PATCH 5/7] By default require TLS1.3 A configuration setting to allow negotiating TLS1.2 is included. If I got things done correctly, OpenSSL pre-1.1 can be used but requires explicitly lowering the minimum allowed TLS version in the configuration file. Signed-off-by: Erik Boasson --- src/core/ddsi/include/ddsi/q_config.h | 8 ++++++++ src/core/ddsi/src/ddsi_ssl.c | 28 +++++++++++++++++++++++++- src/core/ddsi/src/q_config.c | 29 +++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 1 deletion(-) diff --git a/src/core/ddsi/include/ddsi/q_config.h b/src/core/ddsi/include/ddsi/q_config.h index 7810ea4..6f842b2 100644 --- a/src/core/ddsi/include/ddsi/q_config.h +++ b/src/core/ddsi/include/ddsi/q_config.h @@ -213,6 +213,13 @@ enum many_sockets_mode { MSM_MANY_UNICAST }; +#ifdef DDSI_INCLUDE_SSL +struct ssl_min_version { + int major; + int minor; +}; +#endif + struct config { int valid; @@ -301,6 +308,7 @@ struct config char * ssl_rand_file; char * ssl_key_pass; char * ssl_ciphers; + struct ssl_min_version ssl_min_version; #endif diff --git a/src/core/ddsi/src/ddsi_ssl.c b/src/core/ddsi/src/ddsi_ssl.c index 5ae6a19..ae99a2b 100644 --- a/src/core/ddsi/src/ddsi_ssl.c +++ b/src/core/ddsi/src/ddsi_ssl.c @@ -168,6 +168,7 @@ static int ddsi_ssl_password (char *buf, int num, int rwflag, void *udata) static SSL_CTX *ddsi_ssl_ctx_init (void) { SSL_CTX *ctx = SSL_CTX_new (SSLv23_method ()); + unsigned disallow_TLSv1_2; /* Load certificates */ if (! SSL_CTX_use_certificate_file (ctx, config.ssl_keystore, SSL_FILETYPE_PEM)) @@ -220,7 +221,32 @@ static SSL_CTX *ddsi_ssl_ctx_init (void) i |= SSL_VERIFY_FAIL_IF_NO_PEER_CERT; SSL_CTX_set_verify (ctx, i, ddsi_ssl_verify); } - SSL_CTX_set_options (ctx, SSL_OP_ALL | SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3 | SSL_OP_NO_TLSv1 | SSL_OP_NO_TLSv1_1); + switch (config.ssl_min_version.major) + { + case 1: + switch (config.ssl_min_version.minor) + { + case 2: + disallow_TLSv1_2 = 0; + break; + case 3: +#ifdef SSL_OP_NO_TLSv1_2 + disallow_TLSv1_2 = SSL_OP_NO_TLSv1_2; +#else + DDS_LOG (DDS_LC_ERROR | DDS_LC_CONFIG, "tcp/ssl: openssl version does not support disabling TLSv1.2 as required by config\n"); + goto fail; +#endif + break; + default: + DDS_LOG (DDS_LC_ERROR | DDS_LC_CONFIG, "tcp/ssl: can't set minimum requested TLS version to %d.%d\n", config.ssl_min_version.major, config.ssl_min_version.minor); + goto fail; + } + break; + default: + DDS_LOG (DDS_LC_ERROR | DDS_LC_CONFIG, "tcp/ssl: can't set minimum requested TLS version to %d.%d\n", config.ssl_min_version.major, config.ssl_min_version.minor); + goto fail; + } + SSL_CTX_set_options (ctx, SSL_OP_ALL | SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3 | SSL_OP_NO_TLSv1 | SSL_OP_NO_TLSv1_1 | disallow_TLSv1_2); return ctx; fail: diff --git a/src/core/ddsi/src/q_config.c b/src/core/ddsi/src/q_config.c index 6cea54a..ecfbd53 100644 --- a/src/core/ddsi/src/q_config.c +++ b/src/core/ddsi/src/q_config.c @@ -165,6 +165,9 @@ DUPF(durability_cdr); DUPF(transport_selector); DUPF(many_sockets_mode); DU(deaf_mute); +#ifdef DDSI_INCLUDE_SSL +DUPF(min_tls_version); +#endif #undef DUPF #undef DU #undef PF @@ -691,6 +694,8 @@ static const struct cfgelem ssl_cfgelems[] = { "

The set of ciphers used by SSL/TLS

" }, { LEAF("EntropyFile"), 1, "", ABSOFF(ssl_rand_file), 0, uf_string, ff_free, pf_string, "

The SSL/TLS random entropy file name.

" }, + { LEAF("MinimumTLSVersion"), 1, "1.3", ABSOFF(ssl_min_version), 0, uf_min_tls_version, 0, pf_min_tls_version, + "

The minimum TLS version that may be negotiated, valid values are 1.2 and 1.3.

" }, END_MARKER }; #endif @@ -1408,6 +1413,30 @@ static void pf_besmode(struct cfgst *cfgst, void *parent, struct cfgelem const * cfg_log(cfgst, "%s%s", str, is_default ? " [def]" : ""); } +#ifdef DDSI_INCLUDE_SSL +static int uf_min_tls_version(struct cfgst *cfgst, UNUSED_ARG(void *parent), UNUSED_ARG(struct cfgelem const * const cfgelem), UNUSED_ARG(int first), const char *value) +{ + static const char *vs[] = { + "1.2", "1.3", NULL + }; + static const struct ssl_min_version ms[] = { + {1,2}, {1,3}, {0,0} + }; + int idx = list_index(vs, value); + struct ssl_min_version *elem = cfg_address(cfgst, parent, cfgelem); + assert(sizeof(vs) / sizeof(*vs) == sizeof(ms) / sizeof(*ms)); + if ( idx < 0 ) + return cfg_error(cfgst, "'%s': undefined value", value); + *elem = ms[idx]; + return 1; +} + +static void pf_min_tls_version(struct cfgst *cfgst, void *parent, struct cfgelem const * const cfgelem, int is_default) +{ + struct ssl_min_version *p = cfg_address(cfgst, parent, cfgelem); + cfg_log(cfgst, "%d.%d%s", p->major, p->minor, is_default ? " [def]" : ""); +} +#endif static int uf_durability_cdr(struct cfgst *cfgst, UNUSED_ARG(void *parent), UNUSED_ARG(struct cfgelem const * const cfgelem), UNUSED_ARG(int first), const char *value) { From 69e55b04e3b97bf425e03b4579d1b51f80f20fb3 Mon Sep 17 00:00:00 2001 From: Erik Boasson Date: Thu, 31 Jan 2019 19:42:14 +0100 Subject: [PATCH 6/7] suppress openssl-induced SOCKET-to-int warnings on Win64 One is reasonable (a difference between size_t and the type used for a blob in an iovec), the others are SOCKET-to-int conversions that are caused by the openssl API. Since I'm not going to fix openssl and every indication is that the conversion is safe in practice, silencing the compiler is a sensible thing to do. Signed-off-by: Erik Boasson --- src/core/ddsi/src/ddsi_ssl.c | 10 +++++++++- src/core/ddsi/src/ddsi_tcp.c | 2 +- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/core/ddsi/src/ddsi_ssl.c b/src/core/ddsi/src/ddsi_ssl.c index ae99a2b..a6b5fdf 100644 --- a/src/core/ddsi/src/ddsi_ssl.c +++ b/src/core/ddsi/src/ddsi_ssl.c @@ -270,9 +270,14 @@ static SSL *ddsi_ssl_connect (os_socket sock) SSL *ssl; int err; - /* Connect SSL over connected socket */ + /* Connect SSL over connected socket; on Win64 a SOCKET is 64-bit type is forced into an int by + the OpenSSL API. Lots of software does use openssl on Win64, so it appears that it is + safe to do so, and moreover, that it will remain safe to do so, given Microsoft's track + record of maintaining backwards compatibility. The SSL API is in the wrong of course ... */ ssl = ddsi_ssl_new (); + OS_WARNING_MSVC_OFF(4244); SSL_set_fd (ssl, sock); + OS_WARNING_MSVC_ON(4244); err = SSL_connect (ssl); if (err != 1) { @@ -286,8 +291,11 @@ static SSL *ddsi_ssl_connect (os_socket sock) static BIO *ddsi_ssl_listen (os_socket sock) { + /* See comment in ddsi_ssl_connect concerning casting the socket to an int */ BIO * bio = BIO_new (BIO_s_accept ()); + OS_WARNING_MSVC_OFF(4244); BIO_set_fd (bio, sock, BIO_NOCLOSE); + OS_WARNING_MSVC_ON(4244); return bio; } diff --git a/src/core/ddsi/src/ddsi_tcp.c b/src/core/ddsi/src/ddsi_tcp.c index 3ec01d9..6ec865c 100644 --- a/src/core/ddsi/src/ddsi_tcp.c +++ b/src/core/ddsi/src/ddsi_tcp.c @@ -602,7 +602,7 @@ static ssize_t ddsi_tcp_conn_write (ddsi_tran_conn_t base, const nn_locator_t *d { int i; char * ptr; - iovec.iov_len = len; + iovec.iov_len = (os_iov_len_t) len; iovec.iov_base = (len <= sizeof (msgbuf)) ? msgbuf : os_malloc (len); ptr = iovec.iov_base; for (i = 0; i < (int) msg.msg_iovlen; i++) From 9e63fe404fca80965908c33f4d9c0ed9d494ff01 Mon Sep 17 00:00:00 2001 From: Erik Boasson Date: Thu, 31 Jan 2019 18:09:49 +0100 Subject: [PATCH 7/7] update Travis-CI build configuration Getting cmake to work with/without conan on macOS, Linux and Windows seems to be trickier than it should be when dealing with older cmake versions. Switching to an Ubuntu Xenial image on Travis CI at least makes it build again. The update then also eliminates the need to update cmake, clang and maven, saving quite a bit of build time. A few small tweaks and an update to the macOS image version reduces some 5 minutes from the macOS build time. The minimum required version for cmake needs to be updated, too, but really only when openssl support is included. So instead of raising the required version in the CMakeFile I am in favour of simply hoping for the best. Signed-off-by: Erik Boasson --- .travis.yml | 35 ++++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/.travis.yml b/.travis.yml index 312d0dd..f757bfc 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,27 +5,33 @@ language: c # anchor/alias YAML features. linux_gcc8: &linux_gcc8 os: linux + dist: xenial compiler: gcc addons: apt: update: true sources: [ ubuntu-toolchain-r-test ] - packages: [ gcc-8 g++-8 oracle-java8-set-default maven ] + #packages: [ gcc-8 g++-8 maven cmake ] + packages: [ gcc-8 g++-8 ] -linux_clang60: &linux_clang60 +linux_clang: &linux_clang os: linux + dist: xenial compiler: clang addons: apt: update: true - sources: [ llvm-toolchain-trusty-6.0, ubuntu-toolchain-r-test ] - packages: [ clang-6.0 oracle-java8-set-default maven ] + #sources: [ ubuntu-toolchain-r-test ] + #packages: [ maven clang ] -osx_xcode94: &osx_xcode94 +osx_xcode10_1: &osx_xcode10_1 os: osx - osx_image: xcode94 + osx_image: xcode10.1 compiler: clang - + addons: + homebrew: + packages: + - pyenv-virtualenv matrix: include: @@ -33,13 +39,13 @@ matrix: env: [ BUILD_TYPE=Debug, C_COMPILER=gcc-8, CXX_COMPILER=g++-8, USE_SANITIZER=none ] - <<: *linux_gcc8 env: [ BUILD_TYPE=Release, C_COMPILER=gcc-8, CXX_COMPILER=g++-8, USE_SANITIZER=none ] - - <<: *linux_clang60 - env: [ BUILD_TYPE=Debug, C_COMPILER=clang-6.0, CXX_COMPILER=clang++-6.0, USE_SANITIZER=address ] - - <<: *linux_clang60 - env: [ BUILT_TYPE=Release, C_COMPILER=clang-6.0, CXX_COMPILER=clang++-6.0, USE_SANITIZER=none ] - - <<: *osx_xcode94 + - <<: *linux_clang env: [ BUILD_TYPE=Debug, C_COMPILER=clang, CXX_COMPILER=clang++, USE_SANITIZER=address ] - - <<: *osx_xcode94 + - <<: *linux_clang + env: [ BUILT_TYPE=Release, C_COMPILER=clang, CXX_COMPILER=clang++, USE_SANITIZER=none ] + - <<: *osx_xcode10_1 + env: [ BUILD_TYPE=Debug, C_COMPILER=clang, CXX_COMPILER=clang++, USE_SANITIZER=address ] + - <<: *osx_xcode10_1 env: [ BUILD_TYPE=Release, C_COMPILER=clang, CXX_COMPILER=clang++, USE_SANITIZER=none ] @@ -49,7 +55,6 @@ before_install: install: - if [ "${TRAVIS_OS_NAME}" = "osx" ]; then - brew install pyenv-virtualenv; eval "$(pyenv init -)"; pyenv virtualenv conan; pyenv rehash; @@ -66,7 +71,7 @@ before_script: script: - mkdir build - cd build - - conan install .. + - conan install .. --build missing - cmake -DBUILD_TESTING=on -DCMAKE_BUILD_TYPE=${BUILD_TYPE} -DUSE_SANITIZER=${USE_SANITIZER} -DCMAKE_INSTALL_PREFIX=${PWD}/install ../src - cmake --build . --target install - mkdir install/share/CycloneDDS/examples/helloworld/build