From 3cc0bb37e3543052f336abe1d3349ff095f72ae2 Mon Sep 17 00:00:00 2001 From: Garrett D'Amore Date: Mon, 5 Nov 2018 17:21:34 -0800 Subject: fixes #456 TLS configuration object hold fixes #776 Configuration of mbedTLS should warn about license --- docs/man/CMakeLists.txt | 1 + docs/man/nng_http_client_get_tls.3http.adoc | 11 +++++-- docs/man/nng_http_server_get_tls.3http.adoc | 11 +++++-- docs/man/nng_tls.7.adoc | 6 ++++ docs/man/nng_tls_config_alloc.3tls.adoc | 10 +++++- docs/man/nng_tls_config_hold.3tls.adoc | 50 +++++++++++++++++++++++++++++ docs/man/nng_ws.7.adoc | 8 ++++- src/supplemental/http/http_client.c | 1 + src/supplemental/http/http_server.c | 1 + src/supplemental/tls/CMakeLists.txt | 8 ++++- src/supplemental/tls/mbedtls/tls.c | 6 ++++ src/supplemental/tls/tls.h | 5 +++ src/transport/tls/tls.c | 10 ++++-- src/transport/ws/websocket.c | 28 ++++++++++++---- 14 files changed, 139 insertions(+), 17 deletions(-) create mode 100644 docs/man/nng_tls_config_hold.3tls.adoc diff --git a/docs/man/CMakeLists.txt b/docs/man/CMakeLists.txt index 5381f1e5..77efa1c8 100644 --- a/docs/man/CMakeLists.txt +++ b/docs/man/CMakeLists.txt @@ -297,6 +297,7 @@ if (NNG_ENABLE_DOC) nng_tls_config_ca_file nng_tls_config_cert_key_file nng_tls_config_free + nng_tls_config_hold nng_tls_config_own_cert nng_tls_config_server_name ) diff --git a/docs/man/nng_http_client_get_tls.3http.adoc b/docs/man/nng_http_client_get_tls.3http.adoc index 38c59ea5..53e4f426 100644 --- a/docs/man/nng_http_client_get_tls.3http.adoc +++ b/docs/man/nng_http_client_get_tls.3http.adoc @@ -28,8 +28,12 @@ int nng_http_client_get_tls(nng_http_client *client, nng_tls_config **cfgp); The `nng_http_client_get_tls()` obtains the TLS configuration of _client_ and saves a pointer to it in the address referenced by _cfgp_. -The configuration will be `NULL` if the HTTP client instance is not enabled -to use HTTPS. +The object will be returned with an extra hold (see +`<>`) +placed on it on behalf of the caller. +The caller should free this hold by calling +`<>` with it is done +with the TLS configuration. == RETURN VALUES @@ -38,6 +42,7 @@ This function returns 0 on success, and non-zero otherwise. == ERRORS [horizontal] +`NNG_EINVAL`:: TLS not configured on client. `NNG_ENOMEM`:: Insufficient free memory exists. `NNG_ENOTSUP`:: Either HTTP or TLS not supported. @@ -48,5 +53,7 @@ This function returns 0 on success, and non-zero otherwise. <>, <>, <>, +<>, +<>, <>, <> diff --git a/docs/man/nng_http_server_get_tls.3http.adoc b/docs/man/nng_http_server_get_tls.3http.adoc index ec3953e8..e252f46c 100644 --- a/docs/man/nng_http_server_get_tls.3http.adoc +++ b/docs/man/nng_http_server_get_tls.3http.adoc @@ -28,8 +28,12 @@ int nng_http_server_get_tls(nng_http_server *s, nng_tls_config **cfgp); The `nng_http_server_get_tls()` obtains the TLS configuration of server _s_ and saves a pointer to it in the address referenced by _cfgp_. -The configuration will be `NULL` if the HTTP server instance is not enabled -to use HTTPS. +The object will be returned with an extra hold (see +`<>`) +placed on it on behalf of the caller. +The caller should free this hold by calling +`<>` with it is done +with the TLS configuration. == RETURN VALUES @@ -38,6 +42,7 @@ This function returns 0 on success, and non-zero otherwise. == ERRORS [horizontal] +`NNG_EINVAL`:: TLS not configured on server. `NNG_ENOMEM`:: Insufficient free memory exists. `NNG_ENOTSUP`:: Either HTTP or TLS not supported. @@ -48,5 +53,7 @@ This function returns 0 on success, and non-zero otherwise. <>, <>, <>, +<>, +<>, <>, <> diff --git a/docs/man/nng_tls.7.adoc b/docs/man/nng_tls.7.adoc index 43c5a913..59aa77f2 100644 --- a/docs/man/nng_tls.7.adoc +++ b/docs/man/nng_tls.7.adoc @@ -139,6 +139,12 @@ The value is of type `bool` and defaults to `true`. This option is used on an endpoint to access the underlying TLS configuration object. The value is of type `nng_tls_config *`. +If the value is not `NULL`, a hold is placed on the underlying +configuration object before returning it (see +`<>`). +The caller should release the object with +`<>` when it no +longer needs the TLS configuration object. TIP: Use this option when advanced TLS configuration is required. diff --git a/docs/man/nng_tls_config_alloc.3tls.adoc b/docs/man/nng_tls_config_alloc.3tls.adoc index 92ed5d41..edbf1c1e 100644 --- a/docs/man/nng_tls_config_alloc.3tls.adoc +++ b/docs/man/nng_tls_config_alloc.3tls.adoc @@ -47,7 +47,14 @@ or services. The underlying system uses reference counting to ensure that object is not inadvertently freed while in use. -Also note that a TLS configuration object becomes "read-only" after it +A configuration object created with `nng_tls_config_alloc()` starts +with a reference count of one. +The reference count may be incremented using +`<>` and may be +decremented with +`<>`. + +Also note that a TLS configuration object becomes "`read-only`" after it is first used with a service. After this points, attempts to apply further changes to the configuration will result in `NNG_EBUSY`. @@ -71,5 +78,6 @@ This function returns 0 on success, and non-zero otherwise. <>, <>, <>, +<>, <>, <> diff --git a/docs/man/nng_tls_config_hold.3tls.adoc b/docs/man/nng_tls_config_hold.3tls.adoc new file mode 100644 index 00000000..0461a862 --- /dev/null +++ b/docs/man/nng_tls_config_hold.3tls.adoc @@ -0,0 +1,50 @@ += nng_tls_config_hold(3tls) +// +// Copyright 2018 Staysail Systems, Inc. +// +// This document is supplied under the terms of the MIT License, a +// copy of which should be located in the distribution where this +// file was obtained (LICENSE.txt). A copy of the license may also be +// found online at https://opensource.org/licenses/MIT. +// + +== NAME + +nng_tls_config_hold - hold TLS configuration object + +== SYNOPSIS + +[source, c] +---- +#include +#include + +void nng_tls_config_alloc(nng_tls_config *cfg); +---- + +== DESCRIPTION + +The `nng_tls_config_hold()` increments the reference count on +the configuration object named by _cfg_, thereby preventing it +from being freed while in use. + +The hold can be released by calling +`<>`. + +Multiple holds can be placed on a configuration object; the object +will not be freed until the last hold is released. + +== RETURN VALUES + +None. + +== ERRORS + +None. + +== SEE ALSO + +[.text-left] +<>, +<>, +<> diff --git a/docs/man/nng_ws.7.adoc b/docs/man/nng_ws.7.adoc index 1aede210..9d4ef497 100644 --- a/docs/man/nng_ws.7.adoc +++ b/docs/man/nng_ws.7.adoc @@ -129,8 +129,14 @@ This option can be set on listeners, and retrieved from pipes. ((`NNG_OPT_TLS_CONFIG`)):: (opaque) This option is used on an endpoint to access the underlying TLS -configuration object. +configuration object for `wss://` endpoints. The value is of type `nng_tls_config *`. +A hold is placed on the underlying +configuration object before returning it (see +`<>`). +The caller should release the object with +`<>` when it no +longer needs the TLS configuration. TIP: Use this option when advanced TLS configuration is required. diff --git a/src/supplemental/http/http_client.c b/src/supplemental/http/http_client.c index a8260705..c70b7a63 100644 --- a/src/supplemental/http/http_client.c +++ b/src/supplemental/http/http_client.c @@ -223,6 +223,7 @@ nni_http_client_get_tls(nni_http_client *c, struct nng_tls_config **tlsp) nni_mtx_unlock(&c->mtx); return (NNG_EINVAL); } + nni_tls_config_hold(c->tls); *tlsp = c->tls; nni_mtx_unlock(&c->mtx); return (0); diff --git a/src/supplemental/http/http_server.c b/src/supplemental/http/http_server.c index cdbfe3e6..b7ca9f7e 100644 --- a/src/supplemental/http/http_server.c +++ b/src/supplemental/http/http_server.c @@ -1799,6 +1799,7 @@ nni_http_server_get_tls(nni_http_server *s, nng_tls_config **tp) nni_mtx_unlock(&s->mtx); return (NNG_EINVAL); } + nni_tls_config_hold(s->tls); *tp = s->tls; nni_mtx_unlock(&s->mtx); return (0); diff --git a/src/supplemental/tls/CMakeLists.txt b/src/supplemental/tls/CMakeLists.txt index 3d1e6d02..111ff70f 100644 --- a/src/supplemental/tls/CMakeLists.txt +++ b/src/supplemental/tls/CMakeLists.txt @@ -19,7 +19,13 @@ set(_HDRS supplemental/tls/tls.h) # For now we only support the ARM mbedTLS library. if (NNG_SUPP_TLS_MBEDTLS) - Find_Package(mbedTLS REQUIRED) + message(WARNING " + ************************************************************ + Linking against mbedTLS changes license terms (Apache 2.0). + Consult a lawyer and the license files for details. + ************************************************************") + + find_package(mbedTLS REQUIRED) set(_LIBS ${MBEDTLS_LIBRARIES}) set(_INCS ${MBEDTLS_INCLUDE_DIR}) list(APPEND _SRCS supplemental/tls/mbedtls/tls.c) diff --git a/src/supplemental/tls/mbedtls/tls.c b/src/supplemental/tls/mbedtls/tls.c index 8f38df64..f7431ac6 100644 --- a/src/supplemental/tls/mbedtls/tls.c +++ b/src/supplemental/tls/mbedtls/tls.c @@ -1013,3 +1013,9 @@ nng_tls_config_free(nng_tls_config *cfg) { nni_tls_config_fini(cfg); } + +void +nng_tls_config_hold(nng_tls_config *cfg) +{ + nni_tls_config_hold(cfg); +} diff --git a/src/supplemental/tls/tls.h b/src/supplemental/tls/tls.h index 62da9618..5983f3b6 100644 --- a/src/supplemental/tls/tls.h +++ b/src/supplemental/tls/tls.h @@ -41,6 +41,11 @@ typedef enum nng_tls_auth_mode { // with multiple pipes or services/servers. NNG_DECL int nng_tls_config_alloc(nng_tls_config **, nng_tls_mode); +// nng_tls_config_hold increments the reference count on the TLS +// configuration object. The hold can be dropped by calling +// nng_tls_config_free later. +NNG_DECL void nng_tls_config_hold(nng_tls_config *); + // nng_tls_config_free drops the reference count on the TLS // configuration object, and if zero, deallocates it. NNG_DECL void nng_tls_config_free(nng_tls_config *); diff --git a/src/transport/tls/tls.c b/src/transport/tls/tls.c index 8fd59fd7..a8f196f7 100644 --- a/src/transport/tls/tls.c +++ b/src/transport/tls/tls.c @@ -1149,10 +1149,14 @@ tlstran_ep_set_config(void *arg, const void *data, size_t sz, nni_opt_type t) static int tlstran_ep_get_config(void *arg, void *v, size_t *szp, nni_opt_type t) { - tlstran_ep *ep = arg; - int rv; + tlstran_ep * ep = arg; + nng_tls_config *cfg; + int rv; nni_mtx_lock(&ep->mtx); - rv = nni_copyout_ptr(ep->cfg, v, szp, t); + if ((cfg = ep->cfg) != NULL) { + nni_tls_config_hold(cfg); + } + rv = nni_copyout_ptr(cfg, v, szp, t); nni_mtx_unlock(&ep->mtx); return (rv); } diff --git a/src/transport/ws/websocket.c b/src/transport/ws/websocket.c index baa9ea34..73dd8234 100644 --- a/src/transport/ws/websocket.c +++ b/src/transport/ws/websocket.c @@ -1060,7 +1060,9 @@ wss_dialer_set_cert_key_file( ((rv = nni_ws_dialer_get_tls(d->dialer, &tls)) != 0)) { return (rv); } - return (nng_tls_config_cert_key_file(tls, v, NULL)); + rv = nng_tls_config_cert_key_file(tls, v, NULL); + nni_tls_config_fini(tls); + return (rv); } static int @@ -1075,7 +1077,9 @@ wss_listener_set_cert_key_file( ((rv = nni_ws_listener_get_tls(l->listener, &tls)) != 0)) { return (rv); } - return (nng_tls_config_cert_key_file(tls, v, NULL)); + rv = nng_tls_config_cert_key_file(tls, v, NULL); + nni_tls_config_fini(tls); + return (rv); } static int @@ -1089,7 +1093,9 @@ wss_dialer_set_ca_file(void *arg, const void *v, size_t sz, nni_opt_type t) ((rv = nni_ws_dialer_get_tls(d->dialer, &tls)) != 0)) { return (rv); } - return (nng_tls_config_ca_file(tls, v)); + rv = nng_tls_config_ca_file(tls, v); + nni_tls_config_fini(tls); + return (rv); } static int @@ -1103,7 +1109,9 @@ wss_listener_set_ca_file(void *arg, const void *v, size_t sz, nni_opt_type t) ((rv = nni_ws_listener_get_tls(l->listener, &tls)) != 0)) { return (rv); } - return (nng_tls_config_ca_file(tls, v)); + rv = nng_tls_config_ca_file(tls, v); + nni_tls_config_fini(tls); + return (rv); } static int @@ -1128,7 +1136,9 @@ wss_dialer_set_auth_mode(void *arg, const void *v, size_t sz, nni_opt_type t) ((rv = nni_ws_dialer_get_tls(d->dialer, &tls)) != 0)) { return (rv); } - return (nng_tls_config_auth_mode(tls, mode)); + rv = nng_tls_config_auth_mode(tls, mode); + nni_tls_config_fini(tls); + return (rv); } static int @@ -1146,7 +1156,9 @@ wss_listener_set_auth_mode(void *arg, const void *v, size_t sz, nni_opt_type t) ((rv = nni_ws_listener_get_tls(l->listener, &tls)) != 0)) { return (rv); } - return (nng_tls_config_auth_mode(tls, mode)); + rv = nng_tls_config_auth_mode(tls, mode); + nni_tls_config_fini(tls); + return (rv); } static int @@ -1162,7 +1174,9 @@ wss_dialer_set_tls_server_name( return (rv); } - return (nng_tls_config_server_name(tls, v)); + rv = nng_tls_config_server_name(tls, v); + nni_tls_config_fini(tls); + return (rv); } static nni_tran_option wss_dialer_options[] = { -- cgit v1.2.3-70-g09d2