On Mon, Jul 6, 2020 at 4:37 PM Aleksandar Lazic <[email protected]> wrote:
> Should a blank be after '%s'? > > + memprintf(err, "%sthis version of openssl cannot attach > certificate chain for SSL certificate generation.\n", > + err && *err ? *err : ""); > > Looked around in the file and that seemed like the current practice. It assumes that nested err messages always end with a newline, which makes sense. > On 05.07.20 14:09, Gersner wrote: > > That's my fault. I was aware of the versioning but forgot to wrap in > ifdef there. > > Configuration prevents from setting those settings on unsupported > versions. > > > > > > On Sun, Jul 5, 2020 at 2:57 PM Илья Шипицин <[email protected] > <mailto:[email protected]>> wrote: > > > > https://cirrus-ci.com/task/6191727960653824 > > > > seems, openssl-1.0.0 (used in CentOS6/RHEL6) does not support those > methods. > > > > haproxy claims to support openssl starting 0.9.8, I guess > openssl-0.9.8 is rarely tested > > > > вс, 5 июл. 2020 г. в 16:48, Gersner <[email protected] <mailto: > [email protected]>>: > > > > Awesome. I will run the manual tests on the variants later today. > > Thanks. > > > > On Sun, Jul 5, 2020 at 2:45 PM Илья Шипицин < > [email protected] <mailto:[email protected]>> 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 <[email protected] > <mailto:[email protected]>>: > > > > 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 Илья Шипицин < > [email protected] <mailto:[email protected]>> 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, Илья Шипицин < > [email protected] <mailto:[email protected]>>: > > > > nice, all ssl variants build well > > > https://travis-ci.com/github/chipitsine/haproxy/builds/174323866 > > > > вс, 5 июл. 2020 г. в 15:48, Gersner < > [email protected] <mailto:[email protected]>>: > > > > > > > > On Sun, Jul 5, 2020 at 1:42 PM Илья Шипицин < > [email protected] <mailto:[email protected]>> 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 < > [email protected] <mailto:[email protected]>>: > > > > > > > > On Sun, Jul 5, 2020 at 12:28 PM Илья > Шипицин <[email protected] <mailto:[email protected]>> 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, < > [email protected] <mailto:[email protected]>>: > > > > From: Shimi Gersner < > [email protected] <mailto:[email protected]>> > > > > 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 > > > > > >
