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

Reply via email to