> Le 28 juil. 2017 à 12:41, Emmanuel Hocdet <[email protected]> a écrit :
>
>
> Hi Christopher
>
>> Le 28 juil. 2017 à 11:08, Christopher Faulet <[email protected]
>> <mailto:[email protected]>> a écrit :
>>
>> Le 27/07/2017 à 18:16, Emmanuel Hocdet a écrit :
>>> Hi Willy, Emeric
>>> Can you consider this patch? I think it’s safe and it not depend on any
>>> openssl version.
>>> (It’s possible since patch f6b37c67)
>>
>> Hi Manu,
>>
>> Since this commit, the certificates generation doesn't work anymore. I'm
>> working on a patch. I'll send it today I guess.
>>
>> To work, the certificates generation uses the default certificate, to get
>> its private key. It uses it for the generated certificates. Generate keys is
>> pretty expensive and the one from the default certificate is not worse than
>> another.
>>
>> Since commit f6b37c67, when certificates generation is performed, instead of
>> the default certificate (default_ctx), the SSL connection is attached to the
>> initial one (initial_ctx). The last one does not have private key, so the
>> generation always fails.
>>
>
> Oh i remember now. I have discarded the split between initial_ctx and
> default_ctx to not touch any dependancy with generate certificat.
> And introduce it finally in f6b37c67 to fix a bug.
>
> The only dependancy to default_ctx is the private key?
>
>> My first idea to fix the patch was to remove the initial_ctx. Because by
>> checking all recent changes, it seems useless. Its initialization is done in
>> the same time than default_ctx (that was not the case when initial_ctx was
>> introduced in commit f6b37c67).
>>
>> But it is definitely in conflict with you current patch. Because without
>> initial_ctx, we need to have a default_ctx. So I can probably work around
>> this problem. But before doing it, I prefer to know if your patch will be
>> accepted or not :)
>
> Private key is the only missing thing for generate certificat?
>
> such patch can fix the issue?
> diff --git a/src/ssl_sock.c b/src/ssl_sock.c
> index c71c2e3..00ebb80 100644
> --- a/src/ssl_sock.c
> +++ b/src/ssl_sock.c
> @@ -4328,6 +4328,12 @@ int ssl_sock_prepare_all_ctx(struct bind_conf
> *bind_conf)
> err += ssl_sock_prepare_ctx(bind_conf, sni->conf,
> sni->ctx);
> node = ebmb_next(node);
> }
> +
> +#ifndef SSL_NO_GENERATE_CERTIFICATES
> + if (bind_conf->default_ctx) {
> + SSL_CTX_use_PrivateKey(bind_conf->initial_ctx,
> SSL_CTX_get0_privatekey(bind_conf->default_ctx));
> + } /* else generate error */
> +#endif
> return err;
> }
>
>
. fix generate_certificates issue
perhaps it’s more simple to do:
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index c71c2e3..311d465 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -1587,7 +1587,7 @@ ssl_sock_do_create_cert(const char *servername, struct
bind_conf *bind_conf, SSL
int key_type;
/* Get the private key of the defautl certificate and use it */
- if (!(pkey = SSL_get_privatekey(ssl)))
+ if (!pkey = SSL_CTX_get0_privatekey(bind_conf->default_ctx))
goto mkcert_error;
/* Create the certificate */
. for the patch "allow haproxy to start without default certificate"
default_ctx could be required when bind_conf.generate_certs is set.
>
>> From my point of view, I can't see why anyone would want to start a SSL
>> frontend without any certificate. Because it will reject all connections.
>> Simos explained his use-case with Letsencrypt. But it is only useful for the
>> first generation of the first certificate. It is easy to comment the SSL
>> frontend at this step. Get a certificate from Letsencrypt or generate a
>> default one by hand is quick and trivial. To automate this part, you can
>> have a default certificate, probably self-signed, and the strict-sni
>> parameter on the bind line (You already proposed this solution). For me,
>> this is not a workaround, this is the way to do it. But that's just my
>> opinion, I can understand the need :) So I'll let Willy and/or Emeric make
>> the final decision.
>
> A useless certificat should be provide with haproxy configuration?, it’s
> definitely a workaround. It’s legacy from pre SNI.
>
> ++
> Manu
