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 >>>> >>>> >>>>
