Hi everyone, >> Be that as it may, BoringSSL changed the internal structure because >> of a specific feature, so whether we use the API or access directly >> is irrelevant because the structure is different: >> >> https://boringssl.googlesource.com/boringssl/+/858a88daf27975f67d9f63e18f95645be2886bfb%5E!/ >> >> >> Perhaps we should #ifdef the particular code in haproxy out when >> boringssl is used (and be silent or simply assume that DHE is used). > > I don't have a working BoringSSL build right now, but it seems that > SSL_get_ciphers() still returns a valid STACK_OF(SSL_CIPHER) *, so I > think the previous fix would still work. The new internal structure > seems to be still using the same STACK_OF(SSL_CIPHER) *, with an > additional field to handle ciphers groups.
I finally found the time to test with a proper boringssl build. This minor patch cleans the way haproxy checks for enabled DHE ciphers at configuration time, replacing a direct access to the cipher_list member by a call to SSL_get_ciphers(), as suggested by Lukas Tribus. It enables the check for enabled DHE ciphers when compiled against boringssl, and may prevent issues later should the SSL_CTX struct be modified in OpenSSL. -- Rémi
From 22d7f5782a18555f8a19510bd776d47c4521f917 Mon Sep 17 00:00:00 2001
From: Remi Gacogne <rgacogne[at]aquaray[dot]fr>
Date: Fri, 10 Oct 2014 17:04:26 +0200
Subject: [PATCH] MINOR: ssl: use SSL_get_ciphers() instead of directly
accessing the cipher list.
---
src/ssl_sock.c | 36 ++++++++++++++++++------------------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index c35d7ff..4e39c8b 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -1478,9 +1478,8 @@ int ssl_sock_prepare_ctx(struct bind_conf *bind_conf, SSL_CTX *ctx, struct proxy
SSL_MODE_ENABLE_PARTIAL_WRITE |
SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER |
SSL_MODE_RELEASE_BUFFERS;
-#ifndef OPENSSL_IS_BORINGSSL
STACK_OF(SSL_CIPHER) * ciphers = NULL;
- SSL_CIPHER * cipher = NULL;
+ SSL_CIPHER const * cipher = NULL;
char cipher_description[128];
/* The description of ciphers using an Ephemeral Diffie Hellman key exchange
contains " Kx=DH " or " Kx=DH(". Beware of " Kx=DH/",
@@ -1489,10 +1488,7 @@ int ssl_sock_prepare_ctx(struct bind_conf *bind_conf, SSL_CTX *ctx, struct proxy
const char dhe_export_description[] = " Kx=DH(";
int idx = 0;
int dhe_found = 0;
-#else /* OPENSSL_IS_BORINGSSL */
- /* assume dhe_found if boringssl is detected */
- int dhe_found = 1;
-#endif
+ SSL *ssl = NULL;
/* Make sure openssl opens /dev/urandom before the chroot */
if (!ssl_initialize_random()) {
@@ -1585,22 +1581,26 @@ int ssl_sock_prepare_ctx(struct bind_conf *bind_conf, SSL_CTX *ctx, struct proxy
no static DH params were in the certificate file. */
if (global.tune.ssl_default_dh_param == 0) {
-#ifndef OPENSSL_IS_BORINGSSL
- ciphers = ctx->cipher_list;
-
- if (ciphers) {
- for (idx = 0; idx < sk_SSL_CIPHER_num(ciphers); idx++) {
- cipher = sk_SSL_CIPHER_value(ciphers, idx);
- if (SSL_CIPHER_description(cipher, cipher_description, sizeof (cipher_description)) == cipher_description) {
- if (strstr(cipher_description, dhe_description) != NULL ||
- strstr(cipher_description, dhe_export_description) != NULL) {
- dhe_found = 1;
- break;
+ ssl = SSL_new(ctx);
+
+ if (ssl) {
+ ciphers = SSL_get_ciphers(ssl);
+
+ if (ciphers) {
+ for (idx = 0; idx < sk_SSL_CIPHER_num(ciphers); idx++) {
+ cipher = sk_SSL_CIPHER_value(ciphers, idx);
+ if (SSL_CIPHER_description(cipher, cipher_description, sizeof (cipher_description)) == cipher_description) {
+ if (strstr(cipher_description, dhe_description) != NULL ||
+ strstr(cipher_description, dhe_export_description) != NULL) {
+ dhe_found = 1;
+ break;
+ }
}
}
}
+ SSL_free(ssl);
+ ssl = NULL;
}
-#endif /* OPENSSL_IS_BORINGSSL */
if (dhe_found) {
Warning("Setting tune.ssl.default-dh-param to 1024 by default, if your workload permits it you should set it to at least 2048. Please set a value >= 1024 to make this warning disappear.\n");
--
2.1.2
signature.asc
Description: OpenPGP digital signature
