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.

diff --git a/daemon/tls.c b/daemon/tls.c
index 8e6c52f1348f81c5333e40f1de43bd55dd01c322..d65bafd1283d7432850467ba74c4fe6d2bb8bf73 100644 (file)
--- 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 19a4848a94100cf91dce7ecf7ad67a637ce7994c..be2b8a03564a01babc82596a5bed27e227eaeea8 100644 (file)
--- 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 cdd2ae24bdd59e2fa07950f0e359a9e26d10a5a1..5ab936d5683f5099478902a53ce13b3757f3aa50 100644 (file)
--- 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;
                        }