Re: [PATCH v3 0/2] Certificate Generation Enhancements

2020-08-28 Thread Gersner
On Tue, Aug 25, 2020 at 5:42 PM William Lallemand 
wrote:

> On Sun, Aug 23, 2020 at 01:58:11PM +0300, gers...@gmail.com wrote:
> > From: Shimi Gersner 
> >
> > Hi Team, William,
> >
> > Took me some time to get back to this. This version resolves all
> > comments from previous patch.
> > As suggested, this is now the default behaviour.
> >
> > PR Reference
> https://github.com/Azure/haproxy/tree/wip/sgersner/ca-features
> >
> > Thanks,
> > Shimi.
> >
> > Shimi Gersner (2):
> >   MEDIUM: ssl: Support certificate chaining for certificate generation
> >   MINOR: ssl: Support SAN extension for certificate generation
> >
> >  include/haproxy/listener-t.h |   3 +-
> >  src/ssl_sock.c   | 147 +--
> >  2 files changed, 105 insertions(+), 45 deletions(-)
> >
> > --
> > 2.27.0
>
>
> I just pushed the two patches in the master repository.
>
> It made me realize that we don't have any reg-tests for this feature in
> HAProxy. Contributions are also welcomed regarding the reg-tests :-)
>
> Thanks!
>
> --
> William Lallemand
>

Thanks William!

I will commit some time to add those reg-tests.

Shimi.


[PATCH v3 0/2] Certificate Generation Enhancements

2020-08-23 Thread gersner
From: Shimi Gersner 

Hi Team, William,

Took me some time to get back to this. This version resolves all
comments from previous patch.
As suggested, this is now the default behaviour.

PR Reference https://github.com/Azure/haproxy/tree/wip/sgersner/ca-features

Thanks,
Shimi.

Shimi Gersner (2):
  MEDIUM: ssl: Support certificate chaining for certificate generation
  MINOR: ssl: Support SAN extension for certificate generation

 include/haproxy/listener-t.h |   3 +-
 src/ssl_sock.c   | 147 +--
 2 files changed, 105 insertions(+), 45 deletions(-)

-- 
2.27.0




[PATCH v3 1/2] MEDIUM: ssl: Support certificate chaining for certificate generation

2020-08-23 Thread gersner
From: Shimi Gersner 

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 serve the entire trust chain with
the generated certificate. The chain is loaded from the provided
`ca-sign-file` PEM file.
---
 include/haproxy/listener-t.h |   3 +-
 src/ssl_sock.c   | 105 +--
 2 files changed, 63 insertions(+), 45 deletions(-)

diff --git a/include/haproxy/listener-t.h b/include/haproxy/listener-t.h
index 224e32513..7198669b2 100644
--- a/include/haproxy/listener-t.h
+++ b/include/haproxy/listener-t.h
@@ -163,8 +163,7 @@ 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 */
+   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/ssl_sock.c b/src/ssl_sock.c
index ac6537f38..72d2a0ca9 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -1829,8 +1829,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;
@@ -1943,6 +1943,22 @@ 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;
 
+   /* Build chaining the CA cert and the rest of the chain, keep these 
order */
+#if defined(SSL_CTX_add1_chain_cert)
+   if (!SSL_CTX_add1_chain_cert(ssl_ctx, bind_conf->ca_sign_ckch->cert)) {
+   goto mkcert_error;
+   }
+
+   if (bind_conf->ca_sign_ckch->chain) {
+   for (i = 0; i < sk_X509_num(bind_conf->ca_sign_ckch->chain); 
i++) {
+   X509 *chain_cert = 
sk_X509_value(bind_conf->ca_sign_ckch->chain, i);
+   if (!SSL_CTX_add1_chain_cert(ssl_ctx, chain_cert)) {
+   goto mkcert_error;
+   }
+   }
+   }
+#endif
+
if (newcrt) X509_free(newcrt);
 
 #ifndef OPENSSL_NO_DH
@@ -1991,7 +2007,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, _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);
@@ -2022,14 +2038,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, _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, 
_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, _ctx_lru_rwlock);
return 0;
}
@@ -2049,7 +2065,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_ce

[PATCH v3 2/2] MINOR: ssl: Support SAN extension for certificate generation

2020-08-23 Thread gersner
From: Shimi Gersner 

The use of Common Name is fading out in favor of the RFC recommended
way of using SAN extensions. For example, Chrome from version 58
will only match server name against SAN.

The following patch adds SAN extension by default to all generated certificates.
The SAN extension will be of type DNS and based on the server name.
---
 src/ssl_sock.c | 42 ++
 1 file changed, 42 insertions(+)

diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 72d2a0ca9..6e6f337ff 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -1824,6 +1824,43 @@ static int ssl_sock_advertise_alpn_protos(SSL *s, const 
unsigned char **out,
 #ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
 #ifndef SSL_NO_GENERATE_CERTIFICATES
 
+/* Configure a DNS SAN extenion on a certificate. */
+int ssl_sock_add_san_ext(X509V3_CTX* ctx, X509* cert, const char *servername) {
+   int failure = 0;
+   X509_EXTENSION *san_ext = NULL;
+   CONF *conf = NULL;
+   struct buffer *san_name = get_trash_chunk();
+
+   conf = NCONF_new(NULL);
+   if (!conf) {
+   failure = 1;
+   goto cleanup;
+   }
+
+   /* Build an extension based on the DNS entry above */
+   chunk_appendf(san_name, "DNS:%s", servername);
+   san_ext = X509V3_EXT_nconf_nid(conf, ctx, NID_subject_alt_name, 
san_name->area);
+   if (!san_ext) {
+   failure = 1;
+   goto cleanup;
+   }
+
+   /* Add the extension */
+   if (!X509_add_ext(cert, san_ext, -1 /* Add to end */)) {
+   failure = 1;
+   goto cleanup;
+   }
+
+   /* Success */
+   failure = 0;
+
+cleanup:
+   if (NULL != san_ext) X509_EXTENSION_free(san_ext);
+   if (NULL != conf) NCONF_free(conf);
+
+   return failure;
+}
+
 /* Create a X509 certificate with the specified servername and serial. This
  * function returns a SSL_CTX object or NULL if an error occurs. */
 static SSL_CTX *
@@ -1907,6 +1944,11 @@ ssl_sock_do_create_cert(const char *servername, struct 
bind_conf *bind_conf, SSL
X509_EXTENSION_free(ext);
}
 
+   /* Add SAN extension */
+   if (ssl_sock_add_san_ext(, newcrt, servername)) {
+   goto mkcert_error;
+   }
+
/* Sign the certificate with the CA private key */
 
key_type = EVP_PKEY_base_id(capkey);
-- 
2.27.0




Re: [PATCH 2/2] SMALL: ssl: Support SAN extension for certificate generation

2020-07-11 Thread Gersner
On Fri, Jul 10, 2020 at 4:15 PM William Lallemand 
wrote:

> Hello,
>
> On Sun, Jul 05, 2020 at 09:43:23AM +0300, gers...@gmail.com wrote:
> >
> > Subject: Re: [PATCH 2/2] SMALL: ssl: Support SAN extension for
> certificate generation
>
> We commonly use the 'MINOR' tag instead of 'SMALL' here.
> > The use of Common Name is fading out in favor of the RFC recommended
> > way of using SAN extensions. For example, Chrome from version 58
> > will only match server name against SAN.
> >
> > The following patch adds an optional flag to attach SAN extension
> > of type DNS to the generated certificate based on the server name.
> > ---
> >  doc/configuration.txt|  8 ++
> >  include/haproxy/listener-t.h |  1 +
> >  src/cfgparse-ssl.c   | 13 +
> >  src/ssl_sock.c   | 53 
> >  4 files changed, 75 insertions(+)
> >
> > diff --git a/doc/configuration.txt b/doc/configuration.txt
> > index 1d3878bc1..9a7ae43f0 100644
> > --- a/doc/configuration.txt
> > +++ b/doc/configuration.txt
> > @@ -12166,6 +12166,14 @@ ca-sign-use-chain
> >Enabling this flag will attach all public certificates encoded in
> `ca-sign-file`
> >to the served certificate to the client, enabling trust.
> >
> > +ca-sign-use-san
> > +  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 add SAN extenstion of type DNS with the
> requested server name
> > +  inside the generated certificate.
> > +
>
> As your other patch I think that should be the default here, I don't
> think we need an option for this, I'm even suprised it wasn't working
> this way in the first place.
>
Got it.


>
> >  ca-verify-file 
> >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 38ca2839f..47524ffd9 100644
> > --- a/include/haproxy/listener-t.h
> > +++ b/include/haproxy/listener-t.h
> > @@ -164,6 +164,7 @@ struct bind_conf {
> >   char *ca_sign_pass;/* CAKey passphrase */
> >
> >   int ca_sign_use_chain; /* Optionally attached the certificate
> chain to the served certificate */
> > + int ca_sign_use_san;   /* Optionally add SAN entry to the
> generated 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 */
> > diff --git a/src/cfgparse-ssl.c b/src/cfgparse-ssl.c
> > index 270c857f9..62f754522 100644
> > --- a/src/cfgparse-ssl.c
> > +++ b/src/cfgparse-ssl.c
> > @@ -550,6 +550,18 @@ static int bind_parse_ca_sign_use_chain(char
> **args, int cur_arg, struct proxy *
> >   return 0;
> >  }
> >
> > +/* parse the "ca-sign-use-san" bind keyword */
> > +static int bind_parse_ca_sign_use_san(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)
> > + conf->ca_sign_use_san = 1;
> > +#else
> > + memprintf(err, "%sthis version of openssl cannot generate SSL
> certificates.\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)
> >  {
> > @@ -1721,6 +1733,7 @@ static struct bind_kw_list bind_kws = { "SSL", {
> }, {
> >   { "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 */
> > + { "ca-sign-use-san",   bind_parse_ca_sign_use_san,1 }, /*
> enable adding SAN extension 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 54829eb98..a16635341 100644
> > --- a/src/ssl_sock.c
> > +++ b/src/ssl_sock.c
> > @@ -1745,6 +1745,54 @@ static int ssl_sock_advertise_alpn_protos(SSL *s,
> const unsigned char **out,
> >  #ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
> >  #ifndef SSL_NO_GENERATE_CERTIFICATES
> >
>
> > +int 

Re: [PATCH 1/2] MEDIUM: ssl: Support certificate chaining for certificate generation

2020-07-11 Thread Gersner
On Fri, Jul 10, 2020 at 3:51 PM William Lallemand 
wrote:

> Hello,
>
>
> On Sun, Jul 05, 2020 at 09:43:22AM +0300, gers...@gmail.com wrote:
> > From: Shimi Gersner 
> >
> > 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.
>
>
> Totally make sense in my opinion. But if I understand correctly you only
> need the certificate to be signed by the leaf CA in the chain and
> provides all the chain to the client. We probably don't need a new
> option for this.
>
> So what I suggest is to put the chain in the "ca-sign-file" so it will
> works the same way as the "crt" keyword.
>
> Yes, that is how it is currently implemented, chain is read from
"ca-sign-file".
I wasn't worried too much, but felt it's better not to introduce a
potential change
in behaviour and disabled this by default.

I'm going forward with enabling this by default and removing the flag - Is
that correct?


>
> > ---
> >  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 
> >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
>
> Copy-paste error there :-)
>
Ack!


>
> > +  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.
> > +
>
> --
> William Lallemand
>


Re: [PATCH v2 0/2] Certificate Generation Enhancements

2020-07-11 Thread Gersner
Oh, yes, missed the mail from William.

Will go over the comments shortly. Thanks

On Sat, Jul 11, 2020 at 1:54 PM Tim Düsterhus  wrote:

> Shimi,
>
> Am 11.07.20 um 09:28 schrieb Gersner:
> > Gentle ping on this. Can I assist with providing more information?
>
> William responded on the v1 of your patch. I assume he didn't see that
> there was a v2, because it's a separate email thread. I put him in Cc.
>
> https://www.mail-archive.com/haproxy@formilux.org/msg37884.html
> https://www.mail-archive.com/haproxy@formilux.org/msg37885.html
>
> Best regards
> Tim Düsterhus
>


Re: [PATCH v2 0/2] Certificate Generation Enhancements

2020-07-11 Thread Gersner
Hi Iliya, Team,

Gentle ping on this. Can I assist with providing more information?

Shimi.

On Mon, Jul 6, 2020 at 4:29 PM Gersner  wrote:

> The current implementation fallbacks to the default context certificate if
> I recall correctly. No certificate will be generated in that case.
>
> On Mon, Jul 6, 2020 at 3:01 PM Илья Шипицин  wrote:
>
>> Hello, Gersner.
>>
>> smal question. what will happen if client does not provide SNI (and we
>> are supposed to create certificate)?
>>
>> пн, 6 июл. 2020 г. в 05:12, :
>>
>>> From: Shimi Gersner 
>>>
>>> Hi Team, Ilya,
>>>
>>> Following the conversation yesterday I have added a fix and manually
>>> tested the following openssl variants
>>>   - openssl-{1.0.1e,1.0.2u,1.1.1g}
>>>   - libressl-{2.9.2,3.1.1}
>>>
>>> Additionally I have re-ran travis/cirrus
>>>   - https://travis-ci.com/github/gersner/haproxy/builds/174353855
>>>   - https://cirrus-ci.com/build/5482853758664704
>>>
>>>
>>> PR Reference
>>> https://github.com/Azure/haproxy/tree/wip/sgersner/ca-sign-extra
>>>
>>> Thanks,
>>> Shimi.
>>>
>>>
>>> Shimi Gersner (2):
>>>   MEDIUM: ssl: Support certificate chaining for certificate generation
>>>   SMALL: ssl: Support SAN extension for certificate generation
>>>
>>>  doc/configuration.txt|  16 
>>>  include/haproxy/listener-t.h |   5 +-
>>>  src/cfgparse-ssl.c   |  29 +++
>>>  src/ssl_sock.c   | 153 +--
>>>  4 files changed, 158 insertions(+), 45 deletions(-)
>>>
>>> --
>>> 2.27.0
>>>
>>>


Re: [PATCH 1/2] MEDIUM: ssl: Support certificate chaining for certificate generation

2020-07-06 Thread Gersner
On Mon, Jul 6, 2020 at 4:37 PM Aleksandar Lazic  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 Илья Шипицин  <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>>:
> >
> > 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  <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 Илья
> Шипицин 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
> re

Re: [PATCH v2 0/2] Certificate Generation Enhancements

2020-07-06 Thread Gersner
The current implementation fallbacks to the default context certificate if
I recall correctly. No certificate will be generated in that case.

On Mon, Jul 6, 2020 at 3:01 PM Илья Шипицин  wrote:

> Hello, Gersner.
>
> smal question. what will happen if client does not provide SNI (and we are
> supposed to create certificate)?
>
> пн, 6 июл. 2020 г. в 05:12, :
>
>> From: Shimi Gersner 
>>
>> Hi Team, Ilya,
>>
>> Following the conversation yesterday I have added a fix and manually
>> tested the following openssl variants
>>   - openssl-{1.0.1e,1.0.2u,1.1.1g}
>>   - libressl-{2.9.2,3.1.1}
>>
>> Additionally I have re-ran travis/cirrus
>>   - https://travis-ci.com/github/gersner/haproxy/builds/174353855
>>   - https://cirrus-ci.com/build/5482853758664704
>>
>>
>> PR Reference
>> https://github.com/Azure/haproxy/tree/wip/sgersner/ca-sign-extra
>>
>> Thanks,
>> Shimi.
>>
>>
>> Shimi Gersner (2):
>>   MEDIUM: ssl: Support certificate chaining for certificate generation
>>   SMALL: ssl: Support SAN extension for certificate generation
>>
>>  doc/configuration.txt|  16 
>>  include/haproxy/listener-t.h |   5 +-
>>  src/cfgparse-ssl.c   |  29 +++
>>  src/ssl_sock.c   | 153 +--
>>  4 files changed, 158 insertions(+), 45 deletions(-)
>>
>> --
>> 2.27.0
>>
>>


[PATCH v2 0/2] Certificate Generation Enhancements

2020-07-05 Thread gersner
From: Shimi Gersner 

Hi Team, Ilya,

Following the conversation yesterday I have added a fix and manually
tested the following openssl variants
  - openssl-{1.0.1e,1.0.2u,1.1.1g}
  - libressl-{2.9.2,3.1.1}

Additionally I have re-ran travis/cirrus
  - https://travis-ci.com/github/gersner/haproxy/builds/174353855
  - https://cirrus-ci.com/build/5482853758664704


PR Reference https://github.com/Azure/haproxy/tree/wip/sgersner/ca-sign-extra

Thanks,
Shimi.


Shimi Gersner (2):
  MEDIUM: ssl: Support certificate chaining for certificate generation
  SMALL: ssl: Support SAN extension for certificate generation

 doc/configuration.txt|  16 
 include/haproxy/listener-t.h |   5 +-
 src/cfgparse-ssl.c   |  29 +++
 src/ssl_sock.c   | 153 +--
 4 files changed, 158 insertions(+), 45 deletions(-)

-- 
2.27.0




[PATCH v2 1/2] MEDIUM: ssl: Support certificate chaining for certificate generation

2020-07-05 Thread gersner
From: Shimi Gersner 

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   |  15 ++
 src/ssl_sock.c   | 100 ---
 4 files changed, 82 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 
   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 
   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..bf8d95a61 100644
--- a/src/cfgparse-ssl.c
+++ b/src/cfgparse-ssl.c
@@ -538,6 +538,20 @@ 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_NO_GENERATE_CERTIFICATES) && 
defined(SSL_CTRL_SET_TLSEXT_HOSTNAME) && \
+   defined(SSL_CTX_set1_chain) && defined(SSL_CTX_add1_chain_cert))
+   conf->ca_sign_use_chain = 1;
+   return 0;
+#else
+   memprintf(err, "%sthis version of openssl cannot attach certificate 
chain for SSL certificate generation.\n",
+ err && *err ? *err : "");
+   return ERR_ALERT | ERR_FATAL;
+#endif
+}
+
 /* 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 +1722,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,  0 }, /* 
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..7b2c6caef 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 *ss

[PATCH v2 2/2] SMALL: ssl: Support SAN extension for certificate generation

2020-07-05 Thread gersner
From: Shimi Gersner 

The use of Common Name is fading out in favor of the RFC recommended
way of using SAN extensions. For example, Chrome from version 58
will only match server name against SAN.

The following patch adds an optional flag to attach SAN extension
of type DNS to the generated certificate based on the server name.
---
 doc/configuration.txt|  8 ++
 include/haproxy/listener-t.h |  1 +
 src/cfgparse-ssl.c   | 14 ++
 src/ssl_sock.c   | 53 
 4 files changed, 76 insertions(+)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 1d3878bc1..9a7ae43f0 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -12166,6 +12166,14 @@ ca-sign-use-chain
   Enabling this flag will attach all public certificates encoded in 
`ca-sign-file`
   to the served certificate to the client, enabling trust.
 
+ca-sign-use-san
+  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 add SAN extenstion of type DNS with the requested 
server name
+  inside the generated certificate.
+
 ca-verify-file 
   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 38ca2839f..47524ffd9 100644
--- a/include/haproxy/listener-t.h
+++ b/include/haproxy/listener-t.h
@@ -164,6 +164,7 @@ struct bind_conf {
char *ca_sign_pass;/* CAKey passphrase */
 
int ca_sign_use_chain; /* Optionally attached the certificate chain 
to the served certificate */
+   int ca_sign_use_san;   /* Optionally add SAN entry to the generated 
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 */
diff --git a/src/cfgparse-ssl.c b/src/cfgparse-ssl.c
index bf8d95a61..9d8d16dae 100644
--- a/src/cfgparse-ssl.c
+++ b/src/cfgparse-ssl.c
@@ -552,6 +552,19 @@ static int bind_parse_ca_sign_use_chain(char **args, int 
cur_arg, struct proxy *
 #endif
 }
 
+/* parse the "ca-sign-use-san" bind keyword */
+static int bind_parse_ca_sign_use_san(char **args, int cur_arg, struct proxy 
*px, struct bind_conf *conf, char **err)
+{
+#if (!defined(SSL_NO_GENERATE_CERTIFICATES) && 
defined(SSL_CTRL_SET_TLSEXT_HOSTNAME))
+   conf->ca_sign_use_san = 1;
+   return 0;
+#else
+   memprintf(err, "%sthis version of openssl cannot generate SSL 
certificates.\n",
+ err && *err ? *err : "");
+   return ERR_ALERT | ERR_FATAL;
+#endif
+}
+
 /* 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)
 {
@@ -1723,6 +1736,7 @@ static struct bind_kw_list bind_kws = { "SSL", { }, {
{ "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,  0 }, /* 
enable attaching ca chain to generated certificate */
+   { "ca-sign-use-san",   bind_parse_ca_sign_use_san,0 }, /* 
enable adding SAN extension 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 7b2c6caef..adb27cc4a 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -1745,6 +1745,54 @@ static int ssl_sock_advertise_alpn_protos(SSL *s, const 
unsigned char **out,
 #ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
 #ifndef SSL_NO_GENERATE_CERTIFICATES
 
+int ssl_sock_add_san_ext(X509V3_CTX* ctx, X509* cert, const char *servername) {
+   int failure = 0;
+   X509_EXTENSION *san_ext = NULL;
+   char *san_name = NULL;
+   CONF *conf = NULL;
+   size_t buffer_size = strlen(servername)+sizeof("DNS:"); // Includes 
terminator
+
+   conf = NCONF_new(NULL);
+   if (!conf) {
+   failure = 1;
+   goto cleanup;
+   }
+
+   san_name = calloc(1, buffer_size);
+   if (!san_name) {
+   failure = 1;
+   goto cleanup;
+   }
+
+   if ((buffer_size-1) != snprintf(san_name, buffer_size, "DNS:%s&

Re: [PATCH 1/2] MEDIUM: ssl: Support certificate chaining for certificate generation

2020-07-05 Thread Gersner
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 Илья Шипицин  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 :
>
>> Awesome. I will run the manual tests on the variants later today.
>> Thanks.
>>
>> On Sun, Jul 5, 2020 at 2:45 PM Илья Шипицин  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 :
>>>
>>>> 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 Илья Шипицин 
>>>> 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, Илья Шипицин :
>>>>>
>>>>>> nice, all ssl variants build well
>>>>>> https://travis-ci.com/github/chipitsine/haproxy/builds/174323866
>>>>>>
>>>>>> вс, 5 июл. 2020 г. в 15:48, Gersner :
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Sun, Jul 5, 2020 at 1:42 PM Илья Шипицин 
>>>>>>> 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 :
>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Sun, Jul 5, 2020 at 12:28 PM Илья Шипицин 
>>>>>>>>> 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 1

Re: [PATCH 1/2] MEDIUM: ssl: Support certificate chaining for certificate generation

2020-07-05 Thread Gersner
Awesome. I will run the manual tests on the variants later today.
Thanks.

On Sun, Jul 5, 2020 at 2:45 PM Илья Шипицин  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 :
>
>> 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 Илья Шипицин  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, Илья Шипицин :
>>>
>>>> nice, all ssl variants build well
>>>> https://travis-ci.com/github/chipitsine/haproxy/builds/174323866
>>>>
>>>> вс, 5 июл. 2020 г. в 15:48, Gersner :
>>>>
>>>>>
>>>>>
>>>>> On Sun, Jul 5, 2020 at 1:42 PM Илья Шипицин 
>>>>> 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 :
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Sun, Jul 5, 2020 at 12:28 PM Илья Шипицин 
>>>>>>> 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, :
>>>>>>>>
>>>>>>>>> From: Shimi Gersner 
>>>>>>>>>
>>>>>>>>> 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 

Re: [PATCH 1/2] MEDIUM: ssl: Support certificate chaining for certificate generation

2020-07-05 Thread Gersner
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 Илья Шипицин  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, Илья Шипицин :
>
>> nice, all ssl variants build well
>> https://travis-ci.com/github/chipitsine/haproxy/builds/174323866
>>
>> вс, 5 июл. 2020 г. в 15:48, Gersner :
>>
>>>
>>>
>>> On Sun, Jul 5, 2020 at 1:42 PM Илья Шипицин 
>>> 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 :
>>>>
>>>>>
>>>>>
>>>>> On Sun, Jul 5, 2020 at 12:28 PM Илья Шипицин 
>>>>> 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, :
>>>>>>
>>>>>>> From: Shimi Gersner 
>>>>>>>
>>>>>>> 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 
>>>>>>>the dynamic generation of certificates is enabled. See
>>>>

Re: [PATCH 1/2] MEDIUM: ssl: Support certificate chaining for certificate generation

2020-07-05 Thread Gersner
On Sun, Jul 5, 2020 at 1:42 PM Илья Шипицин  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 :
>
>>
>>
>> On Sun, Jul 5, 2020 at 12:28 PM Илья Шипицин 
>> 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, :
>>>
>>>> From: Shimi Gersner 
>>>>
>>>> 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 
>>>>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 
>>>>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_pke

Re: [PATCH 1/2] MEDIUM: ssl: Support certificate chaining for certificate generation

2020-07-05 Thread Gersner
On Sun, Jul 5, 2020 at 12:28 PM Илья Шипицин  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, :
>
>> From: Shimi Gersner 
>>
>> 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 
>>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 
>>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 

[PATCH 1/2] MEDIUM: ssl: Support certificate chaining for certificate generation

2020-07-05 Thread gersner
From: Shimi Gersner 

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

[PATCH 0/2] Certificate Generation Enhancements

2020-07-05 Thread gersner
From: Shimi Gersner 

Hi folks,

The following patches add two enhancements to the certificate
generation feature.
  - SAN extension on the generated certificate
  - Chaining the full trust of the original CA certificate

While evaluating HAP for a new product we realized that these
two features are needed for real world proxy scenarios.

Thank you,
Shimi.

Shimi Gersner (2):
  MEDIUM: ssl: Support certificate chaining for certificate generation
  SMALL: ssl: Support SAN extension for certificate generation

 doc/configuration.txt|  16 
 include/haproxy/listener-t.h |   5 +-
 src/cfgparse-ssl.c   |  26 ++
 src/ssl_sock.c   | 151 +--
 4 files changed, 153 insertions(+), 45 deletions(-)

-- 
2.27.0




[PATCH 2/2] SMALL: ssl: Support SAN extension for certificate generation

2020-07-05 Thread gersner
From: Shimi Gersner 

The use of Common Name is fading out in favor of the RFC recommended
way of using SAN extensions. For example, Chrome from version 58
will only match server name against SAN.

The following patch adds an optional flag to attach SAN extension
of type DNS to the generated certificate based on the server name.
---
 doc/configuration.txt|  8 ++
 include/haproxy/listener-t.h |  1 +
 src/cfgparse-ssl.c   | 13 +
 src/ssl_sock.c   | 53 
 4 files changed, 75 insertions(+)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 1d3878bc1..9a7ae43f0 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -12166,6 +12166,14 @@ ca-sign-use-chain
   Enabling this flag will attach all public certificates encoded in 
`ca-sign-file`
   to the served certificate to the client, enabling trust.
 
+ca-sign-use-san
+  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 add SAN extenstion of type DNS with the requested 
server name
+  inside the generated certificate.
+
 ca-verify-file 
   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 38ca2839f..47524ffd9 100644
--- a/include/haproxy/listener-t.h
+++ b/include/haproxy/listener-t.h
@@ -164,6 +164,7 @@ struct bind_conf {
char *ca_sign_pass;/* CAKey passphrase */
 
int ca_sign_use_chain; /* Optionally attached the certificate chain 
to the served certificate */
+   int ca_sign_use_san;   /* Optionally add SAN entry to the generated 
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 */
diff --git a/src/cfgparse-ssl.c b/src/cfgparse-ssl.c
index 270c857f9..62f754522 100644
--- a/src/cfgparse-ssl.c
+++ b/src/cfgparse-ssl.c
@@ -550,6 +550,18 @@ static int bind_parse_ca_sign_use_chain(char **args, int 
cur_arg, struct proxy *
return 0;
 }
 
+/* parse the "ca-sign-use-san" bind keyword */
+static int bind_parse_ca_sign_use_san(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)
+   conf->ca_sign_use_san = 1;
+#else
+   memprintf(err, "%sthis version of openssl cannot generate SSL 
certificates.\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)
 {
@@ -1721,6 +1733,7 @@ static struct bind_kw_list bind_kws = { "SSL", { }, {
{ "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 */
+   { "ca-sign-use-san",   bind_parse_ca_sign_use_san,1 }, /* 
enable adding SAN extension 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 54829eb98..a16635341 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -1745,6 +1745,54 @@ static int ssl_sock_advertise_alpn_protos(SSL *s, const 
unsigned char **out,
 #ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
 #ifndef SSL_NO_GENERATE_CERTIFICATES
 
+int ssl_sock_add_san_ext(X509V3_CTX* ctx, X509* cert, const char *servername) {
+   int failure = 0;
+   X509_EXTENSION *san_ext = NULL;
+   char *san_name = NULL;
+   CONF *conf = NULL;
+   size_t buffer_size = strlen(servername)+sizeof("DNS:"); // Includes 
terminator
+
+   conf = NCONF_new(NULL);
+   if (!conf) {
+   failure = 1;
+   goto cleanup;
+   }
+
+   san_name = calloc(1, buffer_size);
+   if (!san_name) {
+   failure = 1;
+   goto cleanup;
+   }
+
+   if ((buffer_size-1) != snprintf(san_name, buffer_size, "DNS:%s", 
serverna