Awesome. I will run the manual tests on the variants later today. Thanks. On Sun, Jul 5, 2020 at 2:45 PM Илья Шипицин <chipits...@gmail.com> wrote:
> if you have tested your code (I'm sure you did), maybe manual testing will > be simple enough > you just need to rebuild haproxy against LibreSSL, BoringSSL, older openssl > > examples how to build ssl lib and build haproxy against it might be taken > from .travis.yml (I was about to write an article, but I'm lazy) > > вс, 5 июл. 2020 г. в 16:16, Gersner <gers...@gmail.com>: > >> 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 >>>>>>>>> >>>>>>>>> >>>>>>>>>
