Le 09/10/2015 10:27, Willy Tarreau a écrit :
Hi Christopher,
I applied the first two ones, but the last one seems to be doing
a lot of stuff at the same time. It's not even clear to me whether
it fixes something or improves something or does both, but the
review is quite hard. Is it possible to cut it into functional
parts ? In practice we always want to do one patch per feature or
per bug fix. If you don't think it can be easily cut by now, we
can still blindly apply it but I still don't feel at ease given
the description which seems to cover multiple aspects :-/
Hi Willy,
Thanks for your work and your feedback. I have split my patch in 3 parts.
--
Christopher Faulet
>From c9ceb5e9f575c012c3c63fc9f18f0416a01d7444 Mon Sep 17 00:00:00 2001
From: Christopher Faulet <cfau...@qualys.com>
Date: Fri, 9 Oct 2015 10:53:31 +0200
Subject: [PATCH 1/3] MINOR: ssl: Read the file used to generate certificates
in any order
the file specified by the SSL option 'ca-sign-file' can now contain the CA
certificate used to dynamically generate certificates and its private key in any
order.
---
src/ssl_sock.c | 24 ++++++++++--------------
1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 0703bc4..54cd6a9 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -2508,43 +2508,39 @@ ssl_sock_load_ca(struct bind_conf *bind_conf, struct proxy *px)
Alert("Proxy '%s': cannot enable certificate generation, "
"no CA certificate File configured at [%s:%d].\n",
px->id, bind_conf->file, bind_conf->line);
- err++;
- }
-
- if (err)
goto load_error;
+ }
/* read in the CA certificate */
if (!(fp = fopen(bind_conf->ca_sign_file, "r"))) {
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);
- err++;
goto load_error;
}
if (!(cacert = PEM_read_X509(fp, NULL, NULL, NULL))) {
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);
- fclose (fp);
- err++;
- goto load_error;
+ goto read_error;
}
+ rewind(fp);
if (!(capkey = PEM_read_PrivateKey(fp, NULL, NULL, bind_conf->ca_sign_pass))) {
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);
- fclose (fp);
- err++;
- goto load_error;
+ goto read_error;
}
- fclose (fp);
+ fclose (fp);
bind_conf->ca_sign_cert = cacert;
bind_conf->ca_sign_pkey = capkey;
return err;
- load_error:
- bind_conf->generate_certs = 0;
+ 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;
}
--
2.4.3
>From 1bcf51472a1944c807c4089e61d4b3ed1fa585a0 Mon Sep 17 00:00:00 2001
From: Christopher Faulet <cfau...@qualys.com>
Date: Fri, 9 Oct 2015 11:15:03 +0200
Subject: [PATCH 2/3] MINOR: ssl: Add support for EC for the CA used to sign
generated certificates
This is done by adding EVP_PKEY_EC type in supported types for the CA private
key when we get the message digest used to sign a generated X509 certificate.
So now, we support DSA, RSA and EC private keys.
And to be sure, when the type of the private key is not directly supported, we
get its default message digest using the function
'EVP_PKEY_get_default_digest_nid'.
We also use the key of the default certificate instead of generated it. So we
are sure to use the same key type instead of always using a RSA key.
---
include/proto/ssl_sock.h | 6 ++---
src/ssl_sock.c | 61 ++++++++++++++++++++++++++++--------------------
2 files changed, 39 insertions(+), 28 deletions(-)
diff --git a/include/proto/ssl_sock.h b/include/proto/ssl_sock.h
index b877580..24b4330 100644
--- a/include/proto/ssl_sock.h
+++ b/include/proto/ssl_sock.h
@@ -71,9 +71,9 @@ void tlskeys_finalize_config(void);
int ssl_sock_load_global_dh_param_from_file(const char *filename);
#endif
-SSL_CTX *ssl_sock_create_cert(const char *servername, unsigned int serial, X509 *cacert, EVP_PKEY *capkey);
-SSL_CTX *ssl_sock_get_generated_cert(unsigned int serial, X509 *cacert);
-int ssl_sock_set_generated_cert(SSL_CTX *ctx, unsigned int serial, X509 *cacert);
+SSL_CTX *ssl_sock_create_cert(struct connection *conn, const char *servername, unsigned int serial);
+SSL_CTX *ssl_sock_get_generated_cert(unsigned int serial, struct bind_conf *bind_conf);
+int ssl_sock_set_generated_cert(SSL_CTX *ctx, unsigned int serial, struct bind_conf *bind_conf);
unsigned int ssl_sock_generated_cert_serial(const void *data, size_t len);
#endif /* _PROTO_SSL_SOCK_H */
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 54cd6a9..03751a3 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -1008,24 +1008,22 @@ static int ssl_sock_advertise_alpn_protos(SSL *s, const unsigned char **out,
#ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
/* Create a X509 certificate with the specified servername and serial. This
* function returns a SSL_CTX object or NULL if an error occurs. */
-SSL_CTX *
-ssl_sock_create_cert(const char *servername, unsigned int serial, X509 *cacert, EVP_PKEY *capkey)
+static SSL_CTX *
+ssl_sock_do_create_cert(const char *servername, unsigned int serial,
+ struct bind_conf *bind_conf, SSL *ssl)
{
+ X509 *cacert = bind_conf->ca_sign_cert;
+ EVP_PKEY *capkey = bind_conf->ca_sign_pkey;
SSL_CTX *ssl_ctx = NULL;
X509 *newcrt = NULL;
EVP_PKEY *pkey = NULL;
- RSA *rsa;
X509_NAME *name;
const EVP_MD *digest;
X509V3_CTX ctx;
unsigned int i;
- /* Generate the public key */
- if (!(rsa = RSA_generate_key(2048, 3, NULL, NULL)))
- goto mkcert_error;
- if (!(pkey = EVP_PKEY_new()))
- goto mkcert_error;
- if (EVP_PKEY_assign_RSA(pkey, rsa) != 1)
+ /* Get the private key of the defautl certificate and use it */
+ if (!(pkey = SSL_get_privatekey(ssl)))
goto mkcert_error;
/* Create the certificate */
@@ -1086,8 +1084,17 @@ ssl_sock_create_cert(const char *servername, unsigned int serial, X509 *cacert,
digest = EVP_dss1();
else if (EVP_PKEY_type (capkey->type) == EVP_PKEY_RSA)
digest = EVP_sha256();
- else
- goto mkcert_error;
+ else if (EVP_PKEY_type (capkey->type) == EVP_PKEY_EC)
+ digest = EVP_sha256();
+ else {
+ int nid;
+
+ if (EVP_PKEY_get_default_digest_nid(capkey, &nid) <= 0)
+ goto mkcert_error;
+ if (!(digest = EVP_get_digestbynid(nid)))
+ goto mkcert_error;
+ }
+
if (!(X509_sign(newcrt, capkey, digest)))
goto mkcert_error;
@@ -1102,25 +1109,31 @@ ssl_sock_create_cert(const char *servername, unsigned int serial, X509 *cacert,
goto mkcert_error;
if (newcrt) X509_free(newcrt);
- if (pkey) EVP_PKEY_free(pkey);
+
return ssl_ctx;
mkcert_error:
if (ssl_ctx) SSL_CTX_free(ssl_ctx);
if (newcrt) X509_free(newcrt);
- if (pkey) EVP_PKEY_free(pkey);
return NULL;
}
+SSL_CTX *
+ssl_sock_create_cert(struct connection *conn, const char *servername, unsigned int serial)
+{
+ struct bind_conf *bind_conf = objt_listener(conn->target)->bind_conf;
+ return ssl_sock_do_create_cert(servername, serial, bind_conf, conn->xprt_ctx);
+}
+
/* Do a lookup for a certificate in the LRU cache used to store generated
* certificates. */
SSL_CTX *
-ssl_sock_get_generated_cert(unsigned int serial, X509 *cacert)
+ssl_sock_get_generated_cert(unsigned int serial, struct bind_conf *bind_conf)
{
struct lru64 *lru = NULL;
if (ssl_ctx_lru_tree) {
- lru = lru64_lookup(serial, ssl_ctx_lru_tree, cacert, 0);
+ lru = lru64_lookup(serial, ssl_ctx_lru_tree, bind_conf->ca_sign_cert, 0);
if (lru && lru->domain)
return (SSL_CTX *)lru->data;
}
@@ -1130,17 +1143,17 @@ ssl_sock_get_generated_cert(unsigned int serial, X509 *cacert)
/* Set a certificate int the LRU cache used to store generated
* certificate. Return 0 on success, otherwise -1 */
int
-ssl_sock_set_generated_cert(SSL_CTX *ssl_ctx, unsigned int serial, X509 *cacert)
+ssl_sock_set_generated_cert(SSL_CTX *ssl_ctx, unsigned int serial, struct bind_conf *bind_conf)
{
struct lru64 *lru = NULL;
if (ssl_ctx_lru_tree) {
- lru = lru64_get(serial, ssl_ctx_lru_tree, cacert, 0);
+ lru = lru64_get(serial, ssl_ctx_lru_tree, bind_conf->ca_sign_cert, 0);
if (!lru)
return -1;
if (lru->domain && lru->data)
lru->free((SSL_CTX *)lru->data);
- lru64_commit(lru, ssl_ctx, cacert, 0, (void (*)(void *))SSL_CTX_free);
+ lru64_commit(lru, ssl_ctx, bind_conf->ca_sign_cert, 0, (void (*)(void *))SSL_CTX_free);
return 0;
}
return -1;
@@ -1154,10 +1167,9 @@ ssl_sock_generated_cert_serial(const void *data, size_t len)
}
static SSL_CTX *
-ssl_sock_generate_certificate(const char *servername, struct bind_conf *bind_conf)
+ssl_sock_generate_certificate(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;
SSL_CTX *ssl_ctx = NULL;
struct lru64 *lru = NULL;
unsigned int serial;
@@ -1168,12 +1180,12 @@ ssl_sock_generate_certificate(const char *servername, struct bind_conf *bind_con
if (lru && lru->domain)
ssl_ctx = (SSL_CTX *)lru->data;
if (!ssl_ctx && lru) {
- ssl_ctx = ssl_sock_create_cert(servername, serial, cacert, capkey);
+ ssl_ctx = ssl_sock_do_create_cert(servername, serial, bind_conf, ssl);
lru64_commit(lru, ssl_ctx, cacert, 0, (void (*)(void *))SSL_CTX_free);
}
}
else
- ssl_ctx = ssl_sock_create_cert(servername, serial, cacert, capkey);
+ ssl_ctx = ssl_sock_do_create_cert(servername, serial, bind_conf, ssl);
return ssl_ctx;
}
@@ -1199,7 +1211,7 @@ static int ssl_sock_switchctx_cbk(SSL *ssl, int *al, struct bind_conf *s)
conn_get_to_addr(conn);
if (conn->flags & CO_FL_ADDR_TO_SET) {
serial = ssl_sock_generated_cert_serial(&conn->addr.to, get_addr_len(&conn->addr.to));
- ctx = ssl_sock_get_generated_cert(serial, s->ca_sign_cert);
+ ctx = ssl_sock_get_generated_cert(serial, s);
if (ctx) {
/* switch ctx */
SSL_set_SSL_CTX(ssl, ctx);
@@ -1238,9 +1250,8 @@ static int ssl_sock_switchctx_cbk(SSL *ssl, int *al, struct bind_conf *s)
}
if (!node || container_of(node, struct sni_ctx, name)->neg) {
SSL_CTX *ctx;
-
if (s->generate_certs &&
- (ctx = ssl_sock_generate_certificate(servername, s))) {
+ (ctx = ssl_sock_generate_certificate(servername, s, ssl))) {
/* switch ctx */
SSL_set_SSL_CTX(ssl, ctx);
return SSL_TLSEXT_ERR_OK;
--
2.4.3
>From 0dad4c850e02dee753b07c1353e02666c3c93f6c Mon Sep 17 00:00:00 2001
From: Christopher Faulet <cfau...@qualys.com>
Date: Fri, 9 Oct 2015 11:46:32 +0200
Subject: [PATCH 3/3] MINOR: ssl: Add callbacks to set DH/ECDH params for
generated certificates
Now, A callback is defined for generated certificates to set DH parameters for
ephemeral key exchange when required.
In same way, when possible, we also defined Elliptic Curve DH (ECDH) parameters.
---
src/ssl_sock.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 03751a3..83214a3 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -1006,6 +1006,8 @@ static int ssl_sock_advertise_alpn_protos(SSL *s, const unsigned char **out,
#endif
#ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
+static DH *ssl_get_tmp_dh(SSL *ssl, int export, int keylen);
+
/* 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 *
@@ -1110,6 +1112,22 @@ ssl_sock_do_create_cert(const char *servername, unsigned int serial,
if (newcrt) X509_free(newcrt);
+ SSL_CTX_set_tmp_dh_callback(ssl_ctx, ssl_get_tmp_dh);
+#if defined(SSL_CTX_set_tmp_ecdh) && !defined(OPENSSL_NO_ECDH)
+ {
+ const char *ecdhe = (bind_conf->ecdhe ? bind_conf->ecdhe : ECDHE_DEFAULT_CURVE);
+ EC_KEY *ecc;
+ int nid;
+
+ if ((nid = OBJ_sn2nid(ecdhe)) == NID_undef)
+ goto end;
+ if (!(ecc = EC_KEY_new_by_curve_name(nid)))
+ goto end;
+ SSL_CTX_set_tmp_ecdh(ssl_ctx, ecc);
+ EC_KEY_free(ecc);
+ }
+#endif
+ end:
return ssl_ctx;
mkcert_error:
--
2.4.3
