BUG/MINOR: ssl: properly ref-count the tls_keys entries

Commit 200b0fa ("MEDIUM: Add support for updating TLS ticket keys via
socket") introduced support for updating TLS ticket keys from the CLI,
but missed a small corner case : if multiple bind lines reference the
same tls_keys file, the same reference is used (as expected), but during
the clean shutdown, it will lead to a double free when destroying the
bind_conf contexts since none of the lines knows if others still use
it. The impact is very low however, mostly a core and/or a message in
the system's log upon old process termination.

Let's introduce some basic refcounting to prevent this from happening,
so that only the last bind_conf frees it.

Thanks to Janusz Dziemidowicz and Thierry Fournier for both reporting
the same issue with an easy reproducer.

This fix needs to be backported from 1.6 to 1.8.

diff --git a/include/types/ssl_sock.h b/include/types/ssl_sock.h
index 8a5b3eeb898e3365cd59f72e51d07669e814b563..2e02631c5de2690aed87131a00255c8728872a88 100644 (file)
--- a/include/types/ssl_sock.h
+++ b/include/types/ssl_sock.h
@@ -59,6 +59,7 @@ struct tls_keys_ref {
        struct list list; /* Used to chain refs. */
        char *filename;
        int unique_id; /* Each pattern reference have unique id. */
+       int refcount;  /* number of users of this tls_keys_ref. */
        struct tls_sess_key *tlskeys;
        int tls_ticket_enc_index;
        __decl_hathreads(HA_RWLOCK_T lock); /* lock used to protect the ref */

diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index b5547cc9efea0f55a361264222e1aa01d6533b88..3f70b1223c289b7bf3a669cc7f8b7e5d1a87a82f 100644 (file)
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -4823,7 +4823,7 @@ void ssl_sock_destroy_bind_conf(struct bind_conf *bind_conf)
        ssl_sock_free_ssl_conf(&bind_conf->ssl_conf);
        free(bind_conf->ca_sign_file);
        free(bind_conf->ca_sign_pass);
-       if (bind_conf->keys_ref) {
+       if (bind_conf->keys_ref && !--bind_conf->keys_ref->refcount) {
                free(bind_conf->keys_ref->filename);
                free(bind_conf->keys_ref->tlskeys);
                LIST_DEL(&bind_conf->keys_ref->list);
@@ -7672,7 +7672,8 @@ static int bind_parse_tls_ticket_keys(char **args, int cur_arg, struct proxy *px
        }
 
        keys_ref = tlskeys_ref_lookup(args[cur_arg + 1]);
-       if(keys_ref) {
+       if (keys_ref) {
+               keys_ref->refcount++;
                conf->keys_ref = keys_ref;
                return 0;
        }
@@ -7719,6 +7720,7 @@ static int bind_parse_tls_ticket_keys(char **args, int cur_arg, struct proxy *px
        i -= 2;
        keys_ref->tls_ticket_enc_index = i < 0 ? 0 : i % TLS_TICKETS_NO;
        keys_ref->unique_id = -1;
+       keys_ref->refcount = 1;
        HA_RWLOCK_INIT(&keys_ref->lock);
        conf->keys_ref = keys_ref;