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.
This commit is contained in:
Willy Tarreau 2018-07-17 10:05:32 +02:00
parent faf4aac742
commit 17b4aa1adc
2 changed files with 5 additions and 2 deletions

View File

@ -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 */

View File

@ -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;