Oh, wasn't aware of that. Is there some automation to test this or should I manually verify this?
On Sun, Jul 5, 2020 at 2:13 PM Илья Шипицин <chipits...@gmail.com> wrote: > I recall some issues with LibreSSL and chaining trust. Like it was > declared but never worked. > we'll see that in runtime if there are such issues > > вс, 5 июл. 2020 г. в 16:06, Илья Шипицин <chipits...@gmail.com>: > >> nice, all ssl variants build well >> https://travis-ci.com/github/chipitsine/haproxy/builds/174323866 >> >> вс, 5 июл. 2020 г. в 15:48, Gersner <gers...@gmail.com>: >> >>> >>> >>> On Sun, Jul 5, 2020 at 1:42 PM Илья Шипицин <chipits...@gmail.com> >>> wrote: >>> >>>> do you have your patches on github fork ? >>>> (I could not find your fork) >>>> >>> Yes. See branch >>> https://github.com/Azure/haproxy/tree/wip/sgersner/ca-sign-extra >>> >>>> >>>> вс, 5 июл. 2020 г. в 15:13, Gersner <gers...@gmail.com>: >>>> >>>>> >>>>> >>>>> On Sun, Jul 5, 2020 at 12:28 PM Илья Шипицин <chipits...@gmail.com> >>>>> wrote: >>>>> >>>>>> does it clearly applies to current master ? either gmail scrambled >>>>>> patch or it is not. >>>>>> can you try please ? >>>>>> >>>>> Exporting the eml and running 'git am' it works cleanly. >>>>> >>>>> I've reproduced the exact same output when copy-pasting from gmail. It >>>>> seems gmail converts the tabs to spaces and this fails the patch (Not sure >>>>> why). >>>>> Running patch with '-l' will resolve this, but it's probably safer to >>>>> run git am on the email. >>>>> >>>>> >>>>>> >>>>>> $ patch -p1 < 1.patch >>>>>> patching file doc/configuration.txt >>>>>> patching file include/haproxy/listener-t.h >>>>>> Hunk #1 FAILED at 163. >>>>>> 1 out of 1 hunk FAILED -- saving rejects to file >>>>>> include/haproxy/listener-t.h.rej >>>>>> patching file src/cfgparse-ssl.c >>>>>> Hunk #1 succeeded at 538 with fuzz 1. >>>>>> Hunk #2 FAILED at 1720. >>>>>> 1 out of 2 hunks FAILED -- saving rejects to file >>>>>> src/cfgparse-ssl.c.rej >>>>>> patching file src/ssl_sock.c >>>>>> Hunk #1 FAILED at 1750. >>>>>> Hunk #2 FAILED at 1864. >>>>>> Hunk #3 FAILED at 1912. >>>>>> Hunk #4 FAILED at 1943. >>>>>> Hunk #5 FAILED at 1970. >>>>>> Hunk #6 FAILED at 4823. >>>>>> Hunk #7 FAILED at 4843. >>>>>> 7 out of 7 hunks FAILED -- saving rejects to file src/ssl_sock.c.rej >>>>>> >>>>>> вс, 5 июл. 2020 г. в 11:46, <gers...@gmail.com>: >>>>>> >>>>>>> From: Shimi Gersner <sgers...@microsoft.com> >>>>>>> >>>>>>> haproxy supports generating SSL certificates based on SNI using a >>>>>>> provided >>>>>>> CA signing certificate. Because CA certificates may be signed by >>>>>>> multiple >>>>>>> CAs, in some scenarios, it is neccesary for the server to attach the >>>>>>> trust chain >>>>>>> in addition to the generated certificate. >>>>>>> >>>>>>> The following patch adds the ability to optionally serve all public >>>>>>> certificates provided in the `ca-sign-file` PEM file. >>>>>>> Certificate loading was ported to use `ca_sign_use_chain` structure, >>>>>>> instead of directly reading public/private keys. >>>>>>> --- >>>>>>> doc/configuration.txt | 8 +++ >>>>>>> include/haproxy/listener-t.h | 4 +- >>>>>>> src/cfgparse-ssl.c | 13 +++++ >>>>>>> src/ssl_sock.c | 98 >>>>>>> ++++++++++++++++++++---------------- >>>>>>> 4 files changed, 78 insertions(+), 45 deletions(-) >>>>>>> >>>>>>> diff --git a/doc/configuration.txt b/doc/configuration.txt >>>>>>> index 6d472134e..1d3878bc1 100644 >>>>>>> --- a/doc/configuration.txt >>>>>>> +++ b/doc/configuration.txt >>>>>>> @@ -12158,6 +12158,14 @@ ca-sign-pass <passphrase> >>>>>>> the dynamic generation of certificates is enabled. See >>>>>>> 'generate-certificates' for details. >>>>>>> >>>>>>> +ca-sign-use-chain >>>>>>> + This setting is only available when support for OpenSSL was built >>>>>>> in. It is >>>>>>> + the CA private key passphrase. This setting is optional and used >>>>>>> only when >>>>>>> + the dynamic generation of certificates is enabled. See >>>>>>> + 'generate-certificates' for details. >>>>>>> + Enabling this flag will attach all public certificates encoded in >>>>>>> `ca-sign-file` >>>>>>> + to the served certificate to the client, enabling trust. >>>>>>> + >>>>>>> ca-verify-file <cafile> >>>>>>> This setting designates a PEM file from which to load CA >>>>>>> certificates used to >>>>>>> verify client's certificate. It designates CA certificates which >>>>>>> must not be >>>>>>> diff --git a/include/haproxy/listener-t.h >>>>>>> b/include/haproxy/listener-t.h >>>>>>> index 224e32513..38ca2839f 100644 >>>>>>> --- a/include/haproxy/listener-t.h >>>>>>> +++ b/include/haproxy/listener-t.h >>>>>>> @@ -163,8 +163,8 @@ struct bind_conf { >>>>>>> char *ca_sign_file; /* CAFile used to generate and >>>>>>> sign server certificates */ >>>>>>> char *ca_sign_pass; /* CAKey passphrase */ >>>>>>> >>>>>>> - X509 *ca_sign_cert; /* CA certificate referenced by >>>>>>> ca_file */ >>>>>>> - EVP_PKEY *ca_sign_pkey; /* CA private key referenced by >>>>>>> ca_key */ >>>>>>> + int ca_sign_use_chain; /* Optionally attached the >>>>>>> certificate chain to the served certificate */ >>>>>>> + struct cert_key_and_chain * ca_sign_ckch; /* CA and >>>>>>> possible certificate chain for ca generation */ >>>>>>> #endif >>>>>>> struct proxy *frontend; /* the frontend all these >>>>>>> listeners belong to, or NULL */ >>>>>>> const struct mux_proto_list *mux_proto; /* the mux to use >>>>>>> for all incoming connections (specified by the "proto" keyword) */ >>>>>>> diff --git a/src/cfgparse-ssl.c b/src/cfgparse-ssl.c >>>>>>> index 144cef882..270c857f9 100644 >>>>>>> --- a/src/cfgparse-ssl.c >>>>>>> +++ b/src/cfgparse-ssl.c >>>>>>> @@ -538,6 +538,18 @@ static int bind_parse_ca_sign_file(char **args, >>>>>>> int cur_arg, struct proxy *px, s >>>>>>> return 0; >>>>>>> } >>>>>>> >>>>>>> +/* parse the "ca-sign-use-chain" bind keyword */ >>>>>>> +static int bind_parse_ca_sign_use_chain(char **args, int cur_arg, >>>>>>> struct proxy *px, struct bind_conf *conf, char **err) >>>>>>> +{ >>>>>>> +#if (defined SSL_CTRL_SET_TLSEXT_HOSTNAME && !defined >>>>>>> SSL_NO_GENERATE_CERTIFICATES && defined SSL_CTX_set1_chain) >>>>>>> + conf->ca_sign_use_chain = 1; >>>>>>> +#else >>>>>>> + memprintf(err, "%sthis version of openssl cannot attach >>>>>>> certificate chain for SSL certificate generation.\n", >>>>>>> + err && *err ? *err : ""); >>>>>>> +#endif >>>>>>> + return 0; >>>>>>> +} >>>>>>> + >>>>>>> /* parse the "ca-sign-pass" bind keyword */ >>>>>>> static int bind_parse_ca_sign_pass(char **args, int cur_arg, struct >>>>>>> proxy *px, struct bind_conf *conf, char **err) >>>>>>> { >>>>>>> @@ -1708,6 +1720,7 @@ static struct bind_kw_list bind_kws = { "SSL", >>>>>>> { }, { >>>>>>> { "ca-ignore-err", bind_parse_ignore_err, 1 >>>>>>> }, /* set error IDs to ignore on verify depth > 0 */ >>>>>>> { "ca-sign-file", bind_parse_ca_sign_file, 1 >>>>>>> }, /* set CAFile used to generate and sign server certs */ >>>>>>> { "ca-sign-pass", bind_parse_ca_sign_pass, 1 >>>>>>> }, /* set CAKey passphrase */ >>>>>>> + { "ca-sign-use-chain", bind_parse_ca_sign_use_chain, 1 >>>>>>> }, /* enable attaching ca chain to generated certificate */ >>>>>>> { "ciphers", bind_parse_ciphers, 1 >>>>>>> }, /* set SSL cipher suite */ >>>>>>> #if (HA_OPENSSL_VERSION_NUMBER >= 0x10101000L) >>>>>>> { "ciphersuites", bind_parse_ciphersuites, 1 >>>>>>> }, /* set TLS 1.3 cipher suite */ >>>>>>> diff --git a/src/ssl_sock.c b/src/ssl_sock.c >>>>>>> index a32db1a28..54829eb98 100644 >>>>>>> --- a/src/ssl_sock.c >>>>>>> +++ b/src/ssl_sock.c >>>>>>> @@ -1750,8 +1750,8 @@ static int ssl_sock_advertise_alpn_protos(SSL >>>>>>> *s, const unsigned char **out, >>>>>>> static SSL_CTX * >>>>>>> ssl_sock_do_create_cert(const char *servername, struct bind_conf >>>>>>> *bind_conf, SSL *ssl) >>>>>>> { >>>>>>> - X509 *cacert = bind_conf->ca_sign_cert; >>>>>>> - EVP_PKEY *capkey = bind_conf->ca_sign_pkey; >>>>>>> + X509 *cacert = bind_conf->ca_sign_ckch->cert; >>>>>>> + EVP_PKEY *capkey = bind_conf->ca_sign_ckch->key; >>>>>>> SSL_CTX *ssl_ctx = NULL; >>>>>>> X509 *newcrt = NULL; >>>>>>> EVP_PKEY *pkey = NULL; >>>>>>> @@ -1864,6 +1864,16 @@ ssl_sock_do_create_cert(const char >>>>>>> *servername, struct bind_conf *bind_conf, SSL >>>>>>> if (!SSL_CTX_check_private_key(ssl_ctx)) >>>>>>> goto mkcert_error; >>>>>>> >>>>>>> + /* Assign chain if any */ >>>>>>> + if (bind_conf->ca_sign_use_chain && >>>>>>> bind_conf->ca_sign_ckch->chain) { >>>>>>> + if (!SSL_CTX_set1_chain(ssl_ctx, >>>>>>> bind_conf->ca_sign_ckch->chain)) { >>>>>>> + goto mkcert_error; >>>>>>> + } >>>>>>> + if (!SSL_CTX_add1_chain_cert(ssl_ctx, >>>>>>> bind_conf->ca_sign_ckch->cert)) { >>>>>>> + goto mkcert_error; >>>>>>> + } >>>>>>> + } >>>>>>> + >>>>>>> if (newcrt) X509_free(newcrt); >>>>>>> >>>>>>> #ifndef OPENSSL_NO_DH >>>>>>> @@ -1912,7 +1922,7 @@ ssl_sock_assign_generated_cert(unsigned int >>>>>>> key, struct bind_conf *bind_conf, SS >>>>>>> >>>>>>> if (ssl_ctx_lru_tree) { >>>>>>> HA_RWLOCK_WRLOCK(SSL_GEN_CERTS_LOCK, >>>>>>> &ssl_ctx_lru_rwlock); >>>>>>> - lru = lru64_lookup(key, ssl_ctx_lru_tree, >>>>>>> bind_conf->ca_sign_cert, 0); >>>>>>> + lru = lru64_lookup(key, ssl_ctx_lru_tree, >>>>>>> bind_conf->ca_sign_ckch->cert, 0); >>>>>>> if (lru && lru->domain) { >>>>>>> if (ssl) >>>>>>> SSL_set_SSL_CTX(ssl, (SSL_CTX >>>>>>> *)lru->data); >>>>>>> @@ -1943,14 +1953,14 @@ ssl_sock_set_generated_cert(SSL_CTX >>>>>>> *ssl_ctx, unsigned int key, struct bind_conf >>>>>>> >>>>>>> if (ssl_ctx_lru_tree) { >>>>>>> HA_RWLOCK_WRLOCK(SSL_GEN_CERTS_LOCK, >>>>>>> &ssl_ctx_lru_rwlock); >>>>>>> - lru = lru64_get(key, ssl_ctx_lru_tree, >>>>>>> bind_conf->ca_sign_cert, 0); >>>>>>> + lru = lru64_get(key, ssl_ctx_lru_tree, >>>>>>> bind_conf->ca_sign_ckch->cert, 0); >>>>>>> if (!lru) { >>>>>>> HA_RWLOCK_WRUNLOCK(SSL_GEN_CERTS_LOCK, >>>>>>> &ssl_ctx_lru_rwlock); >>>>>>> return -1; >>>>>>> } >>>>>>> if (lru->domain && lru->data) >>>>>>> lru->free((SSL_CTX *)lru->data); >>>>>>> - lru64_commit(lru, ssl_ctx, bind_conf->ca_sign_cert, >>>>>>> 0, (void (*)(void *))SSL_CTX_free); >>>>>>> + lru64_commit(lru, ssl_ctx, >>>>>>> bind_conf->ca_sign_ckch->cert, 0, (void (*)(void *))SSL_CTX_free); >>>>>>> HA_RWLOCK_WRUNLOCK(SSL_GEN_CERTS_LOCK, >>>>>>> &ssl_ctx_lru_rwlock); >>>>>>> return 0; >>>>>>> } >>>>>>> @@ -1970,7 +1980,7 @@ ssl_sock_generated_cert_key(const void *data, >>>>>>> size_t len) >>>>>>> static int >>>>>>> ssl_sock_generate_certificate(const char *servername, struct >>>>>>> bind_conf *bind_conf, SSL *ssl) >>>>>>> { >>>>>>> - X509 *cacert = bind_conf->ca_sign_cert; >>>>>>> + X509 *cacert = bind_conf->ca_sign_ckch->cert; >>>>>>> SSL_CTX *ssl_ctx = NULL; >>>>>>> struct lru64 *lru = NULL; >>>>>>> unsigned int key; >>>>>>> @@ -4823,13 +4833,12 @@ int >>>>>>> ssl_sock_load_ca(struct bind_conf *bind_conf) >>>>>>> { >>>>>>> struct proxy *px = bind_conf->frontend; >>>>>>> - FILE *fp; >>>>>>> - X509 *cacert = NULL; >>>>>>> - EVP_PKEY *capkey = NULL; >>>>>>> - int err = 0; >>>>>>> + struct cert_key_and_chain *ckch = NULL; >>>>>>> + int ret = 0; >>>>>>> + char *err = NULL; >>>>>>> >>>>>>> if (!bind_conf->generate_certs) >>>>>>> - return err; >>>>>>> + return ret; >>>>>>> >>>>>>> #if (defined SSL_CTRL_SET_TLSEXT_HOSTNAME && !defined >>>>>>> SSL_NO_GENERATE_CERTIFICATES) >>>>>>> if (global_ssl.ctx_cache) { >>>>>>> @@ -4843,52 +4852,55 @@ ssl_sock_load_ca(struct bind_conf *bind_conf) >>>>>>> ha_alert("Proxy '%s': cannot enable certificate >>>>>>> generation, " >>>>>>> "no CA certificate File configured at >>>>>>> [%s:%d].\n", >>>>>>> px->id, bind_conf->file, bind_conf->line); >>>>>>> - goto load_error; >>>>>>> + goto failed; >>>>>>> } >>>>>>> >>>>>>> - /* read in the CA certificate */ >>>>>>> - if (!(fp = fopen(bind_conf->ca_sign_file, "r"))) { >>>>>>> - ha_alert("Proxy '%s': Failed to read CA certificate >>>>>>> file '%s' at [%s:%d].\n", >>>>>>> - px->id, bind_conf->ca_sign_file, >>>>>>> bind_conf->file, bind_conf->line); >>>>>>> - goto load_error; >>>>>>> + /* Allocate cert structure */ >>>>>>> + ckch = calloc(1, sizeof(struct cert_key_and_chain)); >>>>>>> + if (!ckch) { >>>>>>> + ha_alert("Proxy '%s': Failed to read CA certificate >>>>>>> file '%s' at [%s:%d]. Chain allocation failure\n", >>>>>>> + px->id, bind_conf->ca_sign_file, >>>>>>> bind_conf->file, bind_conf->line); >>>>>>> } >>>>>>> - if (!(cacert = PEM_read_X509(fp, NULL, NULL, NULL))) { >>>>>>> - ha_alert("Proxy '%s': Failed to read CA certificate >>>>>>> file '%s' at [%s:%d].\n", >>>>>>> - px->id, bind_conf->ca_sign_file, >>>>>>> bind_conf->file, bind_conf->line); >>>>>>> - goto read_error; >>>>>>> + >>>>>>> + /* Try to parse file */ >>>>>>> + if (ssl_sock_load_files_into_ckch(bind_conf->ca_sign_file, >>>>>>> ckch, &err)) { >>>>>>> + ha_alert("Proxy '%s': Failed to read CA certificate >>>>>>> file '%s' at [%s:%d]. Chain loading failed: %s\n", >>>>>>> + px->id, bind_conf->ca_sign_file, >>>>>>> bind_conf->file, bind_conf->line, err); >>>>>>> + if (err) free(err); >>>>>>> + goto failed; >>>>>>> } >>>>>>> - rewind(fp); >>>>>>> - if (!(capkey = PEM_read_PrivateKey(fp, NULL, NULL, >>>>>>> bind_conf->ca_sign_pass))) { >>>>>>> - ha_alert("Proxy '%s': Failed to read CA private key >>>>>>> file '%s' at [%s:%d].\n", >>>>>>> - px->id, bind_conf->ca_sign_file, >>>>>>> bind_conf->file, bind_conf->line); >>>>>>> - goto read_error; >>>>>>> + >>>>>>> + /* Fail if missing cert or pkey */ >>>>>>> + if ((!ckch->cert) || (!ckch->key)) { >>>>>>> + ha_alert("Proxy '%s': Failed to read CA certificate >>>>>>> file '%s' at [%s:%d]. Chain missing certificate or private key\n", >>>>>>> + px->id, bind_conf->ca_sign_file, >>>>>>> bind_conf->file, bind_conf->line); >>>>>>> + goto failed; >>>>>>> } >>>>>>> >>>>>>> - fclose (fp); >>>>>>> - bind_conf->ca_sign_cert = cacert; >>>>>>> - bind_conf->ca_sign_pkey = capkey; >>>>>>> - return err; >>>>>>> + /* Final assignment to bind */ >>>>>>> + bind_conf->ca_sign_ckch = ckch; >>>>>>> + return ret; >>>>>>> + >>>>>>> + failed: >>>>>>> + if (ckch) { >>>>>>> + ssl_sock_free_cert_key_and_chain_contents(ckch); >>>>>>> + free(ckch); >>>>>>> + } >>>>>>> >>>>>>> - read_error: >>>>>>> - fclose (fp); >>>>>>> - if (capkey) EVP_PKEY_free(capkey); >>>>>>> - if (cacert) X509_free(cacert); >>>>>>> - load_error: >>>>>>> bind_conf->generate_certs = 0; >>>>>>> - err++; >>>>>>> - return err; >>>>>>> + ret++; >>>>>>> + return ret; >>>>>>> } >>>>>>> >>>>>>> /* Release CA cert and private key used to generate certificated */ >>>>>>> void >>>>>>> ssl_sock_free_ca(struct bind_conf *bind_conf) >>>>>>> { >>>>>>> - if (bind_conf->ca_sign_pkey) >>>>>>> - EVP_PKEY_free(bind_conf->ca_sign_pkey); >>>>>>> - if (bind_conf->ca_sign_cert) >>>>>>> - X509_free(bind_conf->ca_sign_cert); >>>>>>> - bind_conf->ca_sign_pkey = NULL; >>>>>>> - bind_conf->ca_sign_cert = NULL; >>>>>>> + if (bind_conf->ca_sign_ckch) { >>>>>>> + >>>>>>> ssl_sock_free_cert_key_and_chain_contents(bind_conf->ca_sign_ckch); >>>>>>> + free(bind_conf->ca_sign_ckch); >>>>>>> + bind_conf->ca_sign_ckch = NULL; >>>>>>> + } >>>>>>> } >>>>>>> >>>>>>> /* >>>>>>> -- >>>>>>> 2.27.0 >>>>>>> >>>>>>> >>>>>>>
