From 5a78c7fa7ad71951da4d5bc0fe920a215aef7b4e Mon Sep 17 00:00:00 2001 From: =?utf8?q?Marek=20Vavru=C5=A1a?= Date: Thu, 9 Aug 2018 11:54:45 -0700 Subject: [PATCH] daemon/tls: TLS parameters are refcounted by client sessions The TLS parameters are shared between client sessions, but they can be removed from the server during runtime, so a care must be taken so that the parameters are not freed while sessions use them. This commit adds reference counting to TLS parameters, so that they remain valid until the last session using them is closed. --- daemon/tls.c | 50 ++++++++++++++++++++++++++++++++++++++++--------- daemon/tls.h | 7 +++---- daemon/worker.c | 2 +- 3 files changed, 45 insertions(+), 14 deletions(-) diff --git a/daemon/tls.c b/daemon/tls.c index 8e6c52f13..d65bafd12 100644 --- a/daemon/tls.c +++ b/daemon/tls.c @@ -590,9 +590,9 @@ void tls_credentials_free(struct tls_credentials *tls_credentials) { free(tls_credentials); } -static int client_paramlist_entry_clear(const char *k, void *v, void *baton) +static int client_paramlist_entry_free(struct tls_client_paramlist_entry *entry) { - struct tls_client_paramlist_entry *entry = (struct tls_client_paramlist_entry *)v; + DEBUG_MSG("freeing TLS parameters %p\n", entry); while (entry->ca_files.len > 0) { if (entry->ca_files.at[0] != NULL) { @@ -632,6 +632,32 @@ static int client_paramlist_entry_clear(const char *k, void *v, void *baton) return 0; } +static void client_paramlist_entry_ref(struct tls_client_paramlist_entry *entry) +{ + if (entry != NULL) { + entry->refs += 1; + } +} + +static void client_paramlist_entry_unref(struct tls_client_paramlist_entry *entry) +{ + if (entry != NULL) { + assert(entry->refs > 0); + entry->refs -= 1; + + /* Last reference frees the object */ + if (entry->refs == 0) { + client_paramlist_entry_free(entry); + } + } +} + +static int client_paramlist_entry_clear(const char *k, void *v, void *baton) +{ + struct tls_client_paramlist_entry *entry = (struct tls_client_paramlist_entry *)v; + return client_paramlist_entry_free(entry); +} + struct tls_client_paramlist_entry *tls_client_try_upgrade(map_t *tls_client_paramlist, const struct sockaddr *addr) { @@ -668,7 +694,7 @@ int tls_client_params_clear(map_t *tls_client_paramlist, const char *addr, uint1 struct tls_client_paramlist_entry *entry = map_get(tls_client_paramlist, key); if (entry != NULL) { - client_paramlist_entry_clear(NULL, (void *)entry, NULL); + client_paramlist_entry_unref(entry); map_del(tls_client_paramlist, key); } @@ -716,6 +742,7 @@ int tls_client_params_set(map_t *tls_client_paramlist, return kr_error(ENOMEM); } gnutls_certificate_set_verify_function(entry->credentials, client_verify_certificate); + client_paramlist_entry_ref(entry); } int ret = kr_ok(); @@ -815,7 +842,7 @@ int tls_client_params_set(map_t *tls_client_paramlist, } if ((ret != kr_ok()) && is_first_entry) { - client_paramlist_entry_clear(NULL, (void *)entry, NULL); + client_paramlist_entry_unref(entry); } return ret; @@ -950,7 +977,7 @@ skip_pins: return GNUTLS_E_CERTIFICATE_ERROR; } -struct tls_client_ctx_t *tls_client_ctx_new(const struct tls_client_paramlist_entry *entry, +struct tls_client_ctx_t *tls_client_ctx_new(struct tls_client_paramlist_entry *entry, struct worker_ctx *worker) { struct tls_client_ctx_t *ctx = calloc(1, sizeof (struct tls_client_ctx_t)); @@ -970,6 +997,11 @@ struct tls_client_ctx_t *tls_client_ctx_new(const struct tls_client_paramlist_en return NULL; } + /* Must take a reference on parameters as the credentials are owned by it + * and must not be freed while the session is active. */ + client_paramlist_entry_ref(entry); + ctx->params = entry; + ret = gnutls_credentials_set(ctx->c.tls_session, GNUTLS_CRD_CERTIFICATE, entry->credentials); if (ret != GNUTLS_E_SUCCESS) { @@ -997,6 +1029,9 @@ void tls_client_ctx_free(struct tls_client_ctx_t *ctx) ctx->c.tls_session = NULL; } + /* Must decrease the refcount for referenced parameters */ + client_paramlist_entry_unref(ctx->params); + free (ctx); } @@ -1050,14 +1085,11 @@ int tls_set_hs_state(struct tls_common_ctx *ctx, tls_hs_state_t state) return kr_ok(); } -int tls_client_ctx_set_params(struct tls_client_ctx_t *ctx, - struct tls_client_paramlist_entry *entry, - struct session *session) +int tls_client_ctx_set_session(struct tls_client_ctx_t *ctx, struct session *session) { if (!ctx) { return kr_error(EINVAL); } - ctx->params = entry; ctx->c.session = session; return kr_ok(); } diff --git a/daemon/tls.h b/daemon/tls.h index 19a4848a9..be2b8a035 100644 --- a/daemon/tls.h +++ b/daemon/tls.h @@ -59,6 +59,7 @@ struct tls_client_paramlist_entry { array_t(const char *) pins; gnutls_certificate_credentials_t credentials; gnutls_datum_t session_data; + uint32_t refs; }; struct worker_ctx; @@ -179,7 +180,7 @@ int tls_client_params_set(map_t *tls_client_paramlist, int tls_client_params_free(map_t *tls_client_paramlist); /*! Allocate new client TLS context */ -struct tls_client_ctx_t *tls_client_ctx_new(const struct tls_client_paramlist_entry *entry, +struct tls_client_ctx_t *tls_client_ctx_new(struct tls_client_paramlist_entry *entry, struct worker_ctx *worker); /*! Free client TLS context */ @@ -189,9 +190,7 @@ int tls_client_connect_start(struct tls_client_ctx_t *client_ctx, struct session *session, tls_handshake_cb handshake_cb); -int tls_client_ctx_set_params(struct tls_client_ctx_t *ctx, - struct tls_client_paramlist_entry *entry, - struct session *session); +int tls_client_ctx_set_session(struct tls_client_ctx_t *ctx, struct session *session); /* Session tickets, server side. Implementation in ./tls_session_ticket-srv.c */ diff --git a/daemon/worker.c b/daemon/worker.c index cdd2ae24b..5ab936d56 100644 --- a/daemon/worker.c +++ b/daemon/worker.c @@ -1848,7 +1848,7 @@ static int qr_task_step(struct qr_task *task, subreq_finalize(task, packet_source, packet); return qr_task_step(task, NULL, NULL); } - tls_client_ctx_set_params(tls_ctx, tls_entry, session); + tls_client_ctx_set_session(tls_ctx, session); session->tls_client_ctx = tls_ctx; session->has_tls = true; } -- 2.47.2