From 426504b5b39fc9fd52d50ca20c564fe4b2b96f73 Mon Sep 17 00:00:00 2001 From: Christopher Faulet Date: Thu, 12 Nov 2015 11:35:51 +0100 Subject: BUG/MINOR: ssl: Be sure to use unique serial for regenerated certificates The serial number for a generated certificate was computed using the requested servername, without any variable/random part. It is not a problem from the moment it is not regenerated. But if the cache is disabled or when the certificate is evicted from the cache, we may need to regenerate it. It is important to not reuse the same serial number for the new certificate. Else clients (especially browsers) trigger a warning because 2 certificates issued by the same CA have the same serial number. So now, the serial is a static variable initialized with now_ms (internal date in milliseconds) and incremented at each new certificate generation. (Ref MPS-2031) (cherry picked from commit 635c0adec2a5e45ef6abbd0a60a7e32d8f187ab2) --- include/proto/ssl_sock.h | 8 ++++---- src/ssl_sock.c | 42 +++++++++++++++++++++++------------------- 2 files changed, 27 insertions(+), 23 deletions(-) diff --git a/include/proto/ssl_sock.h b/include/proto/ssl_sock.h index 24b4330..cb9a1e9 100644 --- a/include/proto/ssl_sock.h +++ b/include/proto/ssl_sock.h @@ -71,10 +71,10 @@ void tlskeys_finalize_config(void); int ssl_sock_load_global_dh_param_from_file(const char *filename); #endif -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); +SSL_CTX *ssl_sock_create_cert(struct connection *conn, const char *servername, unsigned int key); +SSL_CTX *ssl_sock_get_generated_cert(unsigned int key, struct bind_conf *bind_conf); +int ssl_sock_set_generated_cert(SSL_CTX *ctx, unsigned int key, struct bind_conf *bind_conf); +unsigned int ssl_sock_generated_cert_key(const void *data, size_t len); #endif /* _PROTO_SSL_SOCK_H */ diff --git a/src/ssl_sock.c b/src/ssl_sock.c index c67cd56..a6c71dc 100644 --- a/src/ssl_sock.c +++ b/src/ssl_sock.c @@ -1011,9 +1011,10 @@ 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 * -ssl_sock_do_create_cert(const char *servername, unsigned int serial, - struct bind_conf *bind_conf, SSL *ssl) +ssl_sock_do_create_cert(const char *servername, struct bind_conf *bind_conf, SSL *ssl) { + static unsigned int serial = 0; + X509 *cacert = bind_conf->ca_sign_cert; EVP_PKEY *capkey = bind_conf->ca_sign_pkey; SSL_CTX *ssl_ctx = NULL; @@ -1036,7 +1037,9 @@ ssl_sock_do_create_cert(const char *servername, unsigned int serial, * number */ if (X509_set_version(newcrt, 2L) != 1) goto mkcert_error; - ASN1_INTEGER_set(X509_get_serialNumber(newcrt), serial); + if (!serial) + serial = now_ms; + ASN1_INTEGER_set(X509_get_serialNumber(newcrt), serial++); /* Set duration for the certificate */ if (!X509_gmtime_adj(X509_get_notBefore(newcrt), (long)-60*60*24) || @@ -1141,21 +1144,22 @@ ssl_sock_do_create_cert(const char *servername, unsigned int serial, } SSL_CTX * -ssl_sock_create_cert(struct connection *conn, const char *servername, unsigned int serial) +ssl_sock_create_cert(struct connection *conn, const char *servername, unsigned int key) { struct bind_conf *bind_conf = objt_listener(conn->target)->bind_conf; - return ssl_sock_do_create_cert(servername, serial, bind_conf, conn->xprt_ctx); + + return ssl_sock_do_create_cert(servername, 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, struct bind_conf *bind_conf) +ssl_sock_get_generated_cert(unsigned int key, struct bind_conf *bind_conf) { struct lru64 *lru = NULL; if (ssl_ctx_lru_tree) { - lru = lru64_lookup(serial, ssl_ctx_lru_tree, bind_conf->ca_sign_cert, 0); + lru = lru64_lookup(key, ssl_ctx_lru_tree, bind_conf->ca_sign_cert, 0); if (lru && lru->domain) return (SSL_CTX *)lru->data; } @@ -1165,12 +1169,12 @@ ssl_sock_get_generated_cert(unsigned int serial, struct bind_conf *bind_conf) /* 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, struct bind_conf *bind_conf) +ssl_sock_set_generated_cert(SSL_CTX *ssl_ctx, unsigned int key, struct bind_conf *bind_conf) { struct lru64 *lru = NULL; if (ssl_ctx_lru_tree) { - lru = lru64_get(serial, ssl_ctx_lru_tree, bind_conf->ca_sign_cert, 0); + lru = lru64_get(key, ssl_ctx_lru_tree, bind_conf->ca_sign_cert, 0); if (!lru) return -1; if (lru->domain && lru->data) @@ -1181,9 +1185,9 @@ ssl_sock_set_generated_cert(SSL_CTX *ssl_ctx, unsigned int serial, struct bind_c return -1; } -/* Compute the serial that will be used to create/set/get a certificate. */ +/* Compute the key of the certificate. */ unsigned int -ssl_sock_generated_cert_serial(const void *data, size_t len) +ssl_sock_generated_cert_key(const void *data, size_t len) { return XXH32(data, len, ssl_ctx_lru_seed); } @@ -1197,21 +1201,21 @@ ssl_sock_generate_certificate(const char *servername, struct bind_conf *bind_con X509 *cacert = bind_conf->ca_sign_cert; SSL_CTX *ssl_ctx = NULL; struct lru64 *lru = NULL; - unsigned int serial; + unsigned int key; - serial = ssl_sock_generated_cert_serial(servername, strlen(servername)); + key = ssl_sock_generated_cert_key(servername, strlen(servername)); if (ssl_ctx_lru_tree) { - lru = lru64_get(serial, ssl_ctx_lru_tree, cacert, 0); + lru = lru64_get(key, ssl_ctx_lru_tree, cacert, 0); if (lru && lru->domain) ssl_ctx = (SSL_CTX *)lru->data; if (!ssl_ctx && lru) { - ssl_ctx = ssl_sock_do_create_cert(servername, serial, bind_conf, ssl); + ssl_ctx = ssl_sock_do_create_cert(servername, bind_conf, ssl); lru64_commit(lru, ssl_ctx, cacert, 0, (void (*)(void *))SSL_CTX_free); } SSL_set_SSL_CTX(ssl, ssl_ctx); } else { - ssl_ctx = ssl_sock_do_create_cert(servername, serial, bind_conf, ssl); + ssl_ctx = ssl_sock_do_create_cert(servername, bind_conf, ssl); SSL_set_SSL_CTX(ssl, ssl_ctx); /* No LRU cache, this CTX will be released as soon as the session dies */ SSL_CTX_free(ssl_ctx); @@ -1235,13 +1239,13 @@ static int ssl_sock_switchctx_cbk(SSL *ssl, int *al, struct bind_conf *s) if (!servername) { if (s->generate_certs) { struct connection *conn = (struct connection *)SSL_get_app_data(ssl); - unsigned int serial; + unsigned int key; SSL_CTX *ctx; 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); + key = ssl_sock_generated_cert_key(&conn->addr.to, get_addr_len(&conn->addr.to)); + ctx = ssl_sock_get_generated_cert(key, s); if (ctx) { /* switch ctx */ SSL_set_SSL_CTX(ssl, ctx); -- 1.7.12.1