Hey guys,

Sorry for the radio silence, other projects picked up and so I was unable
to put any time into this until now.

I¹ve gotten the feature to work against OpenSSL 1.0.2. I am able to do it
without peeking into any internal OpenSSL #defines and without using any
additional callbacks.

However, I did have to restructure the SNI and CTX processing logic.

Here¹s a TL;DR:

Previously - SSL Context was created and populated with cert/pkey/chain,
then it is inserted into the SNI trees based on the CN/SANs.

New - HAProxy will iterate through the CN/SAN¹s and determine whether or
not a new CTX is needed based on existing entries in the tree. If an
existing entry is found and does not contain the key type, HAProxy will
add the new cert/pkey into the existing context. If no existing context is
found a new context is created and populated. Only a single new context
will be created for each file, regardless of how many CN¹s and SAN¹s.

Let me know if there¹s a better way for you guys to code review it.

Here is the diff:

diff --git a/include/types/ssl_sock.h b/include/types/ssl_sock.h
index e71ba79..dcf00fc 100644
--- a/include/types/ssl_sock.h
+++ b/include/types/ssl_sock.h
@@ -29,6 +29,9 @@ struct sni_ctx {
        SSL_CTX *ctx;             /* context associated to the certificate */
        int order;                /* load order for the certificate */
        int neg;                  /* reject if match */
+       short has_rsa_cert;       /* 1 if CTX contains an RSA certificate */
+       short has_dsa_cert;       /* 1 if CTX contains an DSA certificate */
+       short has_ecc_cert;       /* 1 if CTX contains an ECC certificate */
        struct ebmb_node name;    /* node holding the servername value */
 };
 
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 3bd6fa2..aca0b03 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -1285,7 +1285,23 @@ end:
 }
 #endif
 
-static int ssl_sock_add_cert_sni(SSL_CTX *ctx, struct bind_conf *s, char
*name, int order)
+static void ssl_sock_update_sni_ctx_keytype(struct sni_ctx *sc, int
cert_keytype)
+{
+    switch(cert_keytype){
+    case EVP_PKEY_RSA:
+        sc->has_rsa_cert = 1;
+        break;
+    case EVP_PKEY_DSA:
+        sc->has_dsa_cert = 1;
+        break;
+    case EVP_PKEY_EC:
+        sc->has_ecc_cert = 1;
+        break;
+    }
+
+}
+
+static int ssl_sock_add_cert_sni(SSL_CTX *ctx, struct bind_conf *s, char
*name, int order, int ctx_cert_keytype)
 {
        struct sni_ctx *sc;
        int wild = 0, neg = 0;
@@ -1311,6 +1327,12 @@ static int ssl_sock_add_cert_sni(SSL_CTX *ctx,
struct bind_conf *s, char *name,
                sc->ctx = ctx;
                sc->order = order++;
                sc->neg = neg;
+        sc->has_rsa_cert = 0;
+        sc->has_dsa_cert = 0;
+        sc->has_ecc_cert = 0;
+
+        ssl_sock_update_sni_ctx_keytype(sc,ctx_cert_keytype);
+
                if (wild)
                        ebst_insert(&s->sni_w_ctx, &sc->name);
                else
@@ -1319,21 +1341,17 @@ static int ssl_sock_add_cert_sni(SSL_CTX *ctx,
struct bind_conf *s, char *name,
        return order;
 }
 
+#if OPENSSL_VERSION_NUMBER < 0x1000200fL
+
 /* Loads a certificate key and CA chain from a file. Returns 0 on error,
-1 if
  * an early error happens and the caller must call SSL_CTX_free() by
itelf.
  */
-static int ssl_sock_load_cert_chain_file(SSL_CTX *ctx, const char *file,
struct bind_conf *s, char **sni_filter, int fcount)
+static int ssl_sock_load_cert_chain_file(SSL_CTX *ctx, const char *file,
struct bind_conf *s)
 {
        BIO *in;
        X509 *x = NULL, *ca;
-       int i, err;
+       int err;
        int ret = -1;
-       int order = 0;
-       X509_NAME *xname;
-       char *str;
-#ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
-       STACK_OF(GENERAL_NAME) *names;
-#endif
 
        in = BIO_new(BIO_s_file());
        if (in == NULL)
@@ -1346,37 +1364,6 @@ static int ssl_sock_load_cert_chain_file(SSL_CTX
*ctx, const char *file, struct
        if (x == NULL)
                goto end;
 
-       if (fcount) {
-               while (fcount--)
-                       order = ssl_sock_add_cert_sni(ctx, s, 
sni_filter[fcount], order);
-       }
-       else {
-#ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
-               names = X509_get_ext_d2i(x, NID_subject_alt_name, NULL, NULL);
-               if (names) {
-                       for (i = 0; i < sk_GENERAL_NAME_num(names); i++) {
-                               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, s, str, order);
-                                               OPENSSL_free(str);
-                                       }
-                               }
-                       }
-                       sk_GENERAL_NAME_pop_free(names, GENERAL_NAME_free);
-               }
-#endif /* SSL_CTRL_SET_TLSEXT_HOSTNAME */
-               xname = X509_get_subject_name(x);
-               i = -1;
-               while ((i = X509_NAME_get_index_by_NID(xname, NID_commonName, 
i)) !=
-1) {
-                       X509_NAME_ENTRY *entry = X509_NAME_get_entry(xname, i);
-                       if (ASN1_STRING_to_UTF8((unsigned char **)&str, 
entry->value) >= 0) {
-                               order = ssl_sock_add_cert_sni(ctx, s, str, 
order);
-                               OPENSSL_free(str);
-                       }
-               }
-       }
-
        ret = 0; /* the caller must not free the SSL_CTX argument anymore */
        if (!SSL_CTX_use_certificate(ctx, x))
                goto end;
@@ -1410,92 +1397,313 @@ end:
        return ret;
 }
 
-static int ssl_sock_load_cert_file(const char *path, struct bind_conf
*bind_conf, struct proxy *curproxy, char **sni_filter, int fcount, char
**err)
+#endif /* OPENSSL_VERSION_NUMBER < 0x1000200fL */
+
+#if OPENSSL_VERSION_NUMBER >= 0x1000200fL
+static struct ebmb_node* ssl_sock_get_ctx_from_eb_tree(struct bind_conf*
bind_conf, const char* name){
+
+    struct ebmb_node *node = NULL;
+    int i;
+
+    for (i = 0; i < trash.size; i++) {
+        if (!name[i])
+            break;
+        trash.str[i] = tolower(name[i]);
+    }
+    trash.str[i] = 0;
+
+    node = ebst_lookup(&bind_conf->sni_ctx, trash.str);
+    if(node)
+        return node;
+
+    node = ebst_lookup(&bind_conf->sni_w_ctx, trash.str);
+
+    return node;
+}
+#endif /* OPENSSL_VERSION_NUMBER >= 0x1000200fL */
+
+static int ssl_sock_node_contains_keytype(struct ebmb_node *node, int
keytype){
+    switch(keytype){
+    case EVP_PKEY_RSA:
+        return container_of(node, struct sni_ctx, name)->has_rsa_cert;
+    case EVP_PKEY_DSA:
+        return container_of(node, struct sni_ctx, name)->has_dsa_cert;
+    case EVP_PKEY_EC:
+        return container_of(node, struct sni_ctx, name)->has_ecc_cert;
+    }
+
+    return 0;
+
+}
+
+static int ssl_sock_cert_file_into_ctx(const char *path, struct bind_conf
*bind_conf, SSL_CTX* ctx, char **err)
 {
-       int ret;
-       SSL_CTX *ctx;
+    int ret = 0;
 
-       ctx = SSL_CTX_new(SSLv23_server_method());
-       if (!ctx) {
-               memprintf(err, "%sunable to allocate SSL context for cert 
'%s'.\n",
-                         err && *err ? *err : "", path);
-               return 1;
-       }
+    if (SSL_CTX_use_PrivateKey_file(ctx, path, SSL_FILETYPE_PEM) <= 0) {
+        memprintf(err, "%sunable to load SSL private key from PEM file
'%s'.\n",
+                  err && *err ? *err : "", path);
+        return 1;
+    }
 
-       if (SSL_CTX_use_PrivateKey_file(ctx, path, SSL_FILETYPE_PEM) <= 0) {
-               memprintf(err, "%sunable to load SSL private key from PEM file 
'%s'.\n",
-                         err && *err ? *err : "", path);
-               SSL_CTX_free(ctx);
-               return 1;
-       }
+#if OPENSSL_VERSION_NUMBER >= 0x1000200fL
+    ret = SSL_CTX_use_certificate_chain_file(ctx,path);
+#else
+    ret = ssl_sock_load_cert_chain_file(ctx, path, bind_conf);
+#endif
 
-       ret = ssl_sock_load_cert_chain_file(ctx, path, bind_conf, sni_filter,
fcount);
-       if (ret <= 0) {
-               memprintf(err, "%sunable to load SSL certificate from PEM file 
'%s'.\n",
-                         err && *err ? *err : "", path);
-               if (ret < 0) /* serious error, must do that ourselves */
-                       SSL_CTX_free(ctx);
-               return 1;
-       }
+    if (ret <= 0) {
+        memprintf(err, "%sunable to load SSL certificate from PEM file
'%s'.\n",
+                  err && *err ? *err : "", path);
+        return 1;
+    }
 
-       if (SSL_CTX_check_private_key(ctx) <= 0) {
-               memprintf(err, "%sinconsistencies between private key and 
certificate
loaded from PEM file '%s'.\n",
-                         err && *err ? *err : "", path);
-               return 1;
-       }
+    if (SSL_CTX_check_private_key(ctx) <= 0) {
+        memprintf(err, "%sinconsistencies between private key and
certificate loaded from PEM file '%s'.\n",
+                  err && *err ? *err : "", path);
+        return 1;
+    }
 
-       /* we must not free the SSL_CTX anymore below, since it's already in
-        * the tree, so it will be discovered and cleaned in time.
-        */
 #ifndef OPENSSL_NO_DH
-       /* store a NULL pointer to indicate we have not yet loaded
-          a custom DH param file */
-       if (ssl_dh_ptr_index >= 0) {
-               SSL_CTX_set_ex_data(ctx, ssl_dh_ptr_index, NULL);
-       }
-
-       ret = ssl_sock_load_dh_params(ctx, path);
-       if (ret < 0) {
-               if (err)
-                       memprintf(err, "%sunable to load DH parameters from 
file '%s'.\n",
-                                 *err ? *err : "", path);
-               return 1;
-       }
+    /* store a NULL pointer to indicate we have not yet loaded
+       a custom DH param file */
+    if (ssl_dh_ptr_index >= 0) {
+        SSL_CTX_set_ex_data(ctx, ssl_dh_ptr_index, NULL);
+    }
+
+    ret = ssl_sock_load_dh_params(ctx, path);
+    if (ret < 0) {
+        if (err)
+            memprintf(err, "%sunable to load DH parameters from file
'%s'.\n",
+                    *err ? *err : "", path);
+        return 1;
+    }
 #endif
 
 #if (defined SSL_CTRL_SET_TLSEXT_STATUS_REQ_CB && !defined
OPENSSL_NO_OCSP)
-       ret = ssl_sock_load_ocsp(ctx, path);
-       if (ret < 0) {
-               if (err)
-                       memprintf(err, "%s '%s.ocsp' is present and activates 
OCSP but it is
impossible to compute the OCSP certificate ID (maybe the issuer could not
be found)'.\n",
-                                 *err ? *err : "", path);
-               return 1;
-       }
+    ret = ssl_sock_load_ocsp(ctx, path);
+    if (ret < 0) {
+        if (err)
+            memprintf(err, "%s '%s.ocsp' is present and activates OCSP
but it is impossible to compute the OCSP certificate ID (maybe the issuer
could not be found)'.\n",
+                    *err ? *err : "", path);
+        return 1;
+    }
 #endif
 
 #if (OPENSSL_VERSION_NUMBER >= 0x1000200fL && !defined OPENSSL_NO_TLSEXT
&& !defined OPENSSL_IS_BORINGSSL)
-       if (sctl_ex_index >= 0) {
-               ret = ssl_sock_load_sctl(ctx, path);
-               if (ret < 0) {
-                       if (err)
-                               memprintf(err, "%s '%s.sctl' is present but 
cannot be read or
parsed'.\n",
-                                         *err ? *err : "", path);
-                       return 1;
-               }
-       }
+    if (sctl_ex_index >= 0) {
+        ret = ssl_sock_load_sctl(ctx, path);
+        if (ret < 0) {
+            if (err)
+                memprintf(err, "%s '%s.sctl' is present but cannot be
read or parsed'.\n",
+                        *err ? *err : "", path);
+            return 1;
+        }
+    }
 #endif
 
+    return 0;
+}
+
+/* Just wraps up the error line*/
+static SSL_CTX* ssl_sock_new_ssl_ctx(char** err){
+    SSL_CTX* ctx = NULL;
+    ctx = SSL_CTX_new(SSLv23_server_method());
+    if (!ctx) {
+        memprintf(err, "%sunable to allocate SSL context\n",
+                  err && *err ? *err : "");
+        return NULL;
+    }
+
+    return ctx;
+}
+
+/* Wraps the processing of an SSL CTX given a SNI entry
+ * Return Values:
+ * 0 = No Error
+ * 1 = Error in OpenSSL Calls
+ *
+ * Note that the default_ctx is passed in as a **,
+ * this function will allocate a new SSL_CTX if the
+ * SNI entry is not found in the tree and the *default_ctx is NULL.
+ * It will then update *default_ctx
+ *
+ *
+ * */
+static int ssl_sock_process_ctx_and_sni(const char *path, struct
bind_conf *bind_conf, X509* path_cert, SSL_CTX** default_ctx, char
*sni_name, char **err)
+{
+    struct ebmb_node *node = NULL;
+    int key_type = 0;
+    SSL_CTX* ctx = NULL;
+    int order = 0;
+
+    /* Without 1.0.2, node will always be NULL, which is fine */
+#if (OPENSSL_VERSION_NUMBER >= 0x1000200fL && !defined OPENSSL_NO_TLSEXT
&& !defined OPENSSL_IS_BORINGSSL)
+    node  = ssl_sock_get_ctx_from_eb_tree(bind_conf, sni_name);
+# endif
+
+    if(node == NULL && *default_ctx == NULL)
+    {
+        /* No previous SNI in tree and no new ctx for this file yet */
+        ctx = ssl_sock_new_ssl_ctx(err);
+        if (ctx == NULL)
+            return 1;
+
+        if (ssl_sock_cert_file_into_ctx(path,bind_conf,ctx,err) != 0) {
+            memprintf(err, "%sUnable to load cert file into SSL Context
'%s'.\n",
+                      err && *err ? *err : "", path);
+            SSL_CTX_free(ctx);
+            return 1;
+        }
+
+        /* Ctx is now valid */
+        *default_ctx = ctx;
+
+    }
+    else if(node == NULL)
+        ctx = *default_ctx;
+    else
+        ctx = container_of(node, struct sni_ctx, name)->ctx;
+
+    key_type = EVP_PKEY_id(X509_get_pubkey(path_cert));
+    /* Check node to see if node has a cert of this keytype already */
+    if(node && ssl_sock_node_contains_keytype(node,key_type))
+        return 0;
+
+    /* we pulled a ctx from the SNI tree, load cert file into it */
+    if(ctx != *default_ctx){
+        if (ssl_sock_cert_file_into_ctx(path,bind_conf,ctx,err) != 0) {
+            memprintf(err, "%sUnable to load cert file into SSL Context
'%s'.\n",
+                      err && *err ? *err : "", path);
+            return 1;
+        }
+
+        /* Update node's keytypes */
+        ssl_sock_update_sni_ctx_keytype(container_of(node, struct
sni_ctx, name), key_type);
+
+    }
+    else /* This is a new SNI, add it to the tree */
+    {
+        /* Add to SNI */
+        order = 0;
+        order = ssl_sock_add_cert_sni(ctx, bind_conf, sni_name, order,
key_type);
+    }
+
+    return 0;
+}
+
+static int ssl_sock_load_cert_file(const char *path, struct bind_conf
*bind_conf, struct proxy *curproxy, char **sni_filter, int fcount, char
**err)
+{
+       int ret = 1;
+       SSL_CTX *new_ctx = NULL;
+    FILE* file = NULL;
+    X509* cert = NULL;
+    X509_NAME *xname = NULL;
+    char *str=NULL;
+    int i;
+    int order = 0;
+    int key_type;
+#ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
+    STACK_OF(GENERAL_NAME) *names = NULL;
+#endif
+    struct ebmb_node *n = NULL;
+
+    file = fopen(path,"r");
+    PEM_read_X509(file,&cert,NULL,NULL);
+    fclose(file);
+
+    if(!cert){
+        memprintf(err, "%sunable to read first certificate in '%s'.\n",
+                  err && *err ? *err : "", path);
+        goto end;
+    }
+
+    key_type = EVP_PKEY_id(X509_get_pubkey(cert));
+
+    /* fcount indicates that the user provided an SNI for the list of
certs, just use that one instead of CN/SAN*/
+    if (fcount) {
+        new_ctx = ssl_sock_new_ssl_ctx(err);
+        if(!new_ctx)
+            goto end;
+
+        ret = ssl_sock_cert_file_into_ctx(path,bind_conf,new_ctx,err);
+
+        if (ret != 0) {
+            memprintf(err, "%sUnable to load cert file into SSL Context
'%s'.\n",
+                      err && *err ? *err : "", path);
+            SSL_CTX_free(new_ctx);
+            goto end;
+        }
+
+        /* Keytype does not matter here */
+        while (fcount--)
+            order = ssl_sock_add_cert_sni(new_ctx, bind_conf,
sni_filter[fcount], order, key_type);
+
+        ret = 0;
+        goto end;
+    }
+
+    /* If using OpenSSL 1.0.2+, find context to load cert from SNI EB
tree for each CN/SAN, creating a ctx if needed
+     * Otherwise, create a new ctx and insert it into the EB tree
+     */
+       /* Check if CN already exists in SNI Tree */
+    xname = X509_get_subject_name(cert);
+    i = -1;
+    while ((i = X509_NAME_get_index_by_NID(xname, NID_commonName, i)) !=
-1) {
+        X509_NAME_ENTRY *entry = X509_NAME_get_entry(xname, i);
+        if (ASN1_STRING_to_UTF8((unsigned char **)&str, entry->value) >=
0) {
+
+            
if(ssl_sock_process_ctx_and_sni(path,bind_conf,cert,&new_ctx,str,err) != 0)
+                goto end;
+            OPENSSL_free(str);
+            str = NULL;
+
 #ifndef SSL_CTRL_SET_TLSEXT_HOSTNAME
-       if (bind_conf->default_ctx) {
-               memprintf(err, "%sthis version of openssl cannot load multiple 
SSL
certificates.\n",
-                         err && *err ? *err : "");
-               return 1;
-       }
+            if (bind_conf->default_ctx) {
+                memprintf(err, "%sthis version of openssl cannot load
multiple SSL certificates.\n",
+                        err && *err ? *err : "");
+                goto end;
+            }
 #endif
-       if (!bind_conf->default_ctx)
-               bind_conf->default_ctx = ctx;
 
-       return 0;
+            if (!bind_conf->default_ctx)
+                bind_conf->default_ctx = new_ctx;
+        }
+    }
+
+    /* Do the above logic for each SAN */
+#ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
+    names = X509_get_ext_d2i(cert, NID_subject_alt_name, NULL, NULL);
+    if (names) {
+        for (i = 0; i < sk_GENERAL_NAME_num(names); i++) {
+            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) {
+                  
if(ssl_sock_process_ctx_and_sni(path,bind_conf,cert,&new_ctx,str,err) != 0)
+                        goto end;
+
+                    OPENSSL_free(str);
+                    str = NULL;
+                }
+            }
+        }
+    }
+#endif /* SSL_CTRL_SET_TLSEXT_HOSTNAME */
+
+    ret = 0;
+
+end:
+
+    if(names)
+        sk_GENERAL_NAME_pop_free(names, GENERAL_NAME_free);
+
+    if(cert)
+        X509_free(cert);
+
+    if(str)
+        OPENSSL_free(str);
+
+       return ret;
 }
 
 int ssl_sock_load_cert(char *path, struct bind_conf *bind_conf, struct
proxy *curproxy, char **err)


Reply via email to