MEDIUM: ssl: split ssl_sock_add_cert_sni()
In order to allow the creation of sni_ctx in runtime, we need to split
the function to allow rollback.
We need to be able to allocate all sni_ctxs required before inserting
them in case we need to rollback if we didn't succeed the allocation.
The function was splitted in 2 parts.
The first one ckch_inst_add_cert_sni() allocates a struct sni_ctx, fill
it with the right data and insert it in the ckch_inst's list of sni_ctx.
The second will take every sni_ctx in the ckch_inst and insert them in
the bind_conf's sni tree.
diff --git a/include/types/ssl_sock.h b/include/types/ssl_sock.h
index 0d6ed63..f7960c2 100644
--- a/include/types/ssl_sock.h
+++ b/include/types/ssl_sock.h
@@ -36,7 +36,8 @@
struct sni_ctx {
SSL_CTX *ctx; /* context associated to the certificate */
int order; /* load order for the certificate */
- uint8_t neg; /* reject if match */
+ uint8_t neg:1; /* reject if match */
+ uint8_t wild:1; /* wildcard sni */
struct pkey_info kinfo; /* pkey info */
struct ssl_bind_conf *conf; /* ssl "bind" conf for the certificate */
struct list by_ckch_inst; /* chained in ckch_inst's list of sni_ctx */
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 0ef56e0..cbbf43a 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -2754,13 +2754,12 @@
/* This function allocates a sni_ctx and adds it to the ckch_inst */
-static int ssl_sock_add_cert_sni(SSL_CTX *ctx, struct ckch_inst *ckch_inst,
+static int ckch_inst_add_cert_sni(SSL_CTX *ctx, struct ckch_inst *ckch_inst,
struct bind_conf *s, struct ssl_bind_conf *conf,
struct pkey_info kinfo, char *name, int order)
{
struct sni_ctx *sc;
int wild = 0, neg = 0;
- struct ebmb_node *node;
if (*name == '!') {
neg = 1;
@@ -2782,17 +2781,6 @@
return -1;
trash.area[j] = 0;
- /* Check for duplicates. */
- if (wild)
- node = ebst_lookup(&s->sni_w_ctx, trash.area);
- else
- node = ebst_lookup(&s->sni_ctx, trash.area);
- for (; node; node = ebmb_next_dup(node)) {
- sc = ebmb_entry(node, struct sni_ctx, name);
- if (sc->ctx == ctx && sc->conf == conf && sc->neg == neg)
- return order;
- }
-
sc = malloc(sizeof(struct sni_ctx) + len + 1);
if (!sc)
return -1;
@@ -2802,20 +2790,63 @@
sc->kinfo = kinfo;
sc->order = order++;
sc->neg = neg;
+ sc->wild = wild;
+ sc->name.node.leaf_p = NULL;
if (kinfo.sig != TLSEXT_signature_anonymous)
SSL_CTX_set_ex_data(ctx, ssl_pkey_info_index, &sc->kinfo);
- if (wild)
- ebst_insert(&s->sni_w_ctx, &sc->name);
- else
- ebst_insert(&s->sni_ctx, &sc->name);
- if (ckch_inst != NULL) /* TODO: remove this test later once the code is converted */
- LIST_ADDQ(&ckch_inst->sni_ctx, &sc->by_ckch_inst);
+ LIST_ADDQ(&ckch_inst->sni_ctx, &sc->by_ckch_inst);
}
return order;
}
/*
+ * Insert the sni_ctxs that are listed in the ckch_inst, in the bind_conf's sni_ctx tree
+ * This function can't return an error.
+ *
+ * *CAUTION*: The caller must lock the sni tree if called in multithreading mode
+ */
+static void ssl_sock_load_cert_sni(struct ckch_inst *ckch_inst, struct bind_conf *bind_conf)
+{
+
+ struct sni_ctx *sc0, *sc0b, *sc1;
+ struct ebmb_node *node;
+
+ list_for_each_entry_safe(sc0, sc0b, &ckch_inst->sni_ctx, by_ckch_inst) {
+
+ /* ignore if sc0 was already inserted in a tree */
+ if (sc0->name.node.leaf_p)
+ continue;
+
+ /* Check for duplicates. */
+ if (sc0->wild)
+ node = ebst_lookup(&bind_conf->sni_w_ctx, (char *)sc0->name.key);
+ else
+ node = ebst_lookup(&bind_conf->sni_ctx, (char *)sc0->name.key);
+
+ for (; node; node = ebmb_next_dup(node)) {
+ sc1 = ebmb_entry(node, struct sni_ctx, name);
+ if (sc1->ctx == sc0->ctx && sc1->conf == sc0->conf
+ && sc1->neg == sc0->neg && sc1->wild == sc0->wild) {
+ /* it's a duplicate, we should remove and free it */
+ LIST_DEL(&sc0->by_ckch_inst);
+ free(sc0);
+ sc0 = NULL;
+ }
+ }
+
+ /* if duplicate, ignore the insertion */
+ if (!sc0)
+ continue;
+
+ if (sc0->wild)
+ ebst_insert(&bind_conf->sni_w_ctx, &sc0->name);
+ else
+ ebst_insert(&bind_conf->sni_ctx, &sc0->name);
+ }
+}
+
+/*
* tree used to store the ckchs ordered by filename/bundle name
*/
struct eb_root ckchs_tree = EB_ROOT_UNIQUE;
@@ -3376,7 +3407,7 @@
/* Update SNI Tree */
- key_combos[i-1].order = ssl_sock_add_cert_sni(cur_ctx, ckch_inst, bind_conf, ssl_conf,
+ key_combos[i-1].order = ckch_inst_add_cert_sni(cur_ctx, ckch_inst, bind_conf, ssl_conf,
kinfo, str, key_combos[i-1].order);
if (key_combos[i-1].order < 0) {
memprintf(err, "%sunable to create a sni context.\n", err && *err ? *err : "");
@@ -3398,6 +3429,7 @@
}
}
+ ssl_sock_load_cert_sni(ckch_inst, bind_conf);
end:
if (names)
@@ -3455,6 +3487,8 @@
return 1;
}
+ /* TODO: replace every 'return 1' by an error fallback which free everything */
+
if (ssl_sock_put_ckch_into_ctx(path, ckch, ctx, err) != 0) {
SSL_CTX_free(ctx);
return 1;
@@ -3479,7 +3513,7 @@
if (fcount) {
while (fcount--) {
- order = ssl_sock_add_cert_sni(ctx, ckch_inst, bind_conf, ssl_conf, kinfo, sni_filter[fcount], order);
+ order = ckch_inst_add_cert_sni(ctx, ckch_inst, bind_conf, ssl_conf, kinfo, sni_filter[fcount], order);
if (order < 0) {
memprintf(err, "%sunable to create a sni context.\n", err && *err ? *err : "");
return 1;
@@ -3494,7 +3528,7 @@
GENERAL_NAME *name = sk_GENERAL_NAME_value(names, i);
if (name->type == GEN_DNS) {
if (ASN1_STRING_to_UTF8((unsigned char **)&str, name->d.dNSName) >= 0) {
- order = ssl_sock_add_cert_sni(ctx, ckch_inst, bind_conf, ssl_conf, kinfo, str, order);
+ order = ckch_inst_add_cert_sni(ctx, ckch_inst, bind_conf, ssl_conf, kinfo, str, order);
OPENSSL_free(str);
if (order < 0) {
memprintf(err, "%sunable to create a sni context.\n", err && *err ? *err : "");
@@ -3514,7 +3548,7 @@
value = X509_NAME_ENTRY_get_data(entry);
if (ASN1_STRING_to_UTF8((unsigned char **)&str, value) >= 0) {
- order = ssl_sock_add_cert_sni(ctx, ckch_inst, bind_conf, ssl_conf, kinfo, str, order);
+ order = ckch_inst_add_cert_sni(ctx, ckch_inst, bind_conf, ssl_conf, kinfo, str, order);
OPENSSL_free(str);
if (order < 0) {
memprintf(err, "%sunable to create a sni context.\n", err && *err ? *err : "");
@@ -3561,6 +3595,8 @@
bind_conf->default_ssl_conf = ssl_conf;
}
+ ssl_sock_load_cert_sni(ckch_inst, bind_conf);
+
/* everything succeed, the ckch instance can be used */
ckch_inst->bind_conf = bind_conf;
LIST_ADDQ(&ckchs->ckch_inst, &ckch_inst->by_ckchs);