On Mon, Jul 6, 2020 at 4:37 PM Aleksandar Lazic <al-hapr...@none.at> 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 Илья Шипицин <chipits...@gmail.com
> <mailto:chipits...@gmail.com>> 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 <gers...@gmail.com <mailto:
> gers...@gmail.com>>:
> >
> >         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 <mailto: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
> <mailto: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 <mailto: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 <mailto: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 <mailto:gers...@gmail.com>>:
> >
> >
> >
> >                             On Sun, Jul 5, 2020 at 1:42 PM Илья Шипицин <
> chipits...@gmail.com <mailto: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 <mailto:gers...@gmail.com>>:
> >
> >
> >
> >                                     On Sun, Jul 5, 2020 at 12:28 PM Илья
> Шипицин <chipits...@gmail.com <mailto: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 <mailto:gers...@gmail.com>>:
> >
> >                                             From: Shimi Gersner <
> sgers...@microsoft.com <mailto: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