[1/3] TLS: remove tls_config.openssl_ciphers

Submitted by Hristo Venev on April 20, 2017, 1:32 p.m.

Details

Message ID 1492695162.2148.6.camel@venev.name
State Changes Requested
Headers show

Commit Message

Hristo Venev April 20, 2017, 1:32 p.m.
It is already present in tls_connection_params.

Signed-off-by: Hristo Venev <hristo@venev.name>
---
 src/crypto/tls.h               |  1 -
 src/crypto/tls_openssl.c       | 13 -------------
 src/eap_peer/eap.c             |  1 -
 src/eap_peer/eap.h             |  8 --------
 src/eapol_supp/eapol_supp_sm.c |  1 -
 src/eapol_supp/eapol_supp_sm.h |  9 ---------
 wpa_supplicant/config.c        |  8 ++++++++
 wpa_supplicant/config_file.c   | 29 ++++-------------------------
 wpa_supplicant/config_winreg.c | 23 +++++------------------
 wpa_supplicant/wpas_glue.c     |  1 -
 10 files changed, 17 insertions(+), 77 deletions(-)

Comments

Hristo Venev April 20, 2017, 1:34 p.m.

Jouni Malinen May 8, 2017, 3:46 p.m.
On Thu, Apr 20, 2017 at 02:32:42PM +0100, Hristo Venev wrote:
> It is already present in tls_connection_params.

Could you please clarify how this would maintain the current behavior as
far as setting the default value to "DEFAULT:!EXP:!LOW" is concerned?
The only reference to that string disappears with this patch..

>  wpa_supplicant/config_file.c   | 29 ++++-------------------------
>  wpa_supplicant/config_winreg.c | 23 +++++------------------

And those changes in config_*.c do not seem to have anything to do with
openssl_ciphers. Why are they included in this patch?

> diff --git a/src/crypto/tls_openssl.c b/src/crypto/tls_openssl.c
> @@ -1022,18 +1021,6 @@ void * tls_init(const struct tls_config *conf)
>  	}
>  #endif /* OPENSSL_NO_ENGINE */
>  
> -	if (conf && conf->openssl_ciphers)
> -		ciphers = conf->openssl_ciphers;
> -	else
> -		ciphers = "DEFAULT:!EXP:!LOW";
> -	if (SSL_CTX_set_cipher_list(ssl, ciphers) != 1) {

this is the only place where "DEFAULT:!EXP:!LOW" was used.. After this
patch, it looks like there would be no SSL_CTX_set_cipher_list() unless
the configuration has a specific parameter.

> diff --git a/wpa_supplicant/config.c b/wpa_supplicant/config.c
> @@ -2573,6 +2573,14 @@ struct wpa_ssid * wpa_config_add_network(struct wpa_config *config)
>  		return NULL;
>  	ssid->id = id;
>  	dl_list_init(&ssid->psk_list);
> +
> +	if (config->openssl_ciphers != NULL) {
> +		ssid->eap.openssl_ciphers = os_strdup(config->openssl_ciphers);
> +		if (ssid->eap.openssl_ciphers == NULL)
> +			os_free(ssid);
> +			return NULL;
> +	}

This seems to assume that config->openssl_ciphers is set before adding a
new network block. That does not really necessarily be the case and
global openssl_ciphers changes should update behavior for existing
network blocks.

Or is this setting of the default here the reason for config_*.c changes
since they would now use wpa_config_add_network()? If so, please provide
more justification for doing these changes. The changes for reading the
network blocks and adding the networks one by one would be less
efficient due to having to assign the ssid->id and prio ordering
separately for each network block.

Patch hide | download patch | download mbox

diff --git a/src/crypto/tls.h b/src/crypto/tls.h
index 11d504a97..5859a6287 100644
--- a/src/crypto/tls.h
+++ b/src/crypto/tls.h
@@ -78,7 +78,6 @@  struct tls_config {
 	const char *pkcs11_module_path;
 	int fips_mode;
 	int cert_in_cb;
-	const char *openssl_ciphers;
 	unsigned int tls_session_lifetime;
 
 	void (*event_cb)(void *ctx, enum tls_event ev,
diff --git a/src/crypto/tls_openssl.c b/src/crypto/tls_openssl.c
index fc169e71e..eddca859b 100644
--- a/src/crypto/tls_openssl.c
+++ b/src/crypto/tls_openssl.c
@@ -885,7 +885,6 @@  void * tls_init(const struct tls_config *conf)
 	struct tls_data *data;
 	SSL_CTX *ssl;
 	struct tls_context *context;
-	const char *ciphers;
 
 	if (tls_openssl_ref_count == 0) {
 		tls_global = context = tls_context_new(conf);
@@ -1022,18 +1021,6 @@  void * tls_init(const struct tls_config *conf)
 	}
 #endif /* OPENSSL_NO_ENGINE */
 
-	if (conf && conf->openssl_ciphers)
-		ciphers = conf->openssl_ciphers;
-	else
-		ciphers = "DEFAULT:!EXP:!LOW";
-	if (SSL_CTX_set_cipher_list(ssl, ciphers) != 1) {
-		wpa_printf(MSG_ERROR,
-			   "OpenSSL: Failed to set cipher string '%s'",
-			   ciphers);
-		tls_deinit(data);
-		return NULL;
-	}
-
 	return data;
 }
 
diff --git a/src/eap_peer/eap.c b/src/eap_peer/eap.c
index d0f305f1a..fb1b3f0f2 100644
--- a/src/eap_peer/eap.c
+++ b/src/eap_peer/eap.c
@@ -2038,7 +2038,6 @@  struct eap_sm * eap_peer_sm_init(void *eapol_ctx,
 	tlsconf.opensc_engine_path = conf->opensc_engine_path;
 	tlsconf.pkcs11_engine_path = conf->pkcs11_engine_path;
 	tlsconf.pkcs11_module_path = conf->pkcs11_module_path;
-	tlsconf.openssl_ciphers = conf->openssl_ciphers;
 #ifdef CONFIG_FIPS
 	tlsconf.fips_mode = 1;
 #endif /* CONFIG_FIPS */
diff --git a/src/eap_peer/eap.h b/src/eap_peer/eap.h
index 883ba2423..fb080b048 100644
--- a/src/eap_peer/eap.h
+++ b/src/eap_peer/eap.h
@@ -294,14 +294,6 @@  struct eap_config {
 	 */
 	const char *pkcs11_module_path;
 	/**
-	 * openssl_ciphers - OpenSSL cipher string
-	 *
-	 * This is an OpenSSL specific configuration option for configuring the
-	 * default ciphers. If not set, "DEFAULT:!EXP:!LOW" is used as the
-	 * default.
-	 */
-	const char *openssl_ciphers;
-	/**
 	 * wps - WPS context data
 	 *
 	 * This is only used by EAP-WSC and can be left %NULL if not available.
diff --git a/src/eapol_supp/eapol_supp_sm.c b/src/eapol_supp/eapol_supp_sm.c
index 81761b189..8ae6d04b3 100644
--- a/src/eapol_supp/eapol_supp_sm.c
+++ b/src/eapol_supp/eapol_supp_sm.c
@@ -2086,7 +2086,6 @@  struct eapol_sm *eapol_sm_init(struct eapol_ctx *ctx)
 	conf.opensc_engine_path = ctx->opensc_engine_path;
 	conf.pkcs11_engine_path = ctx->pkcs11_engine_path;
 	conf.pkcs11_module_path = ctx->pkcs11_module_path;
-	conf.openssl_ciphers = ctx->openssl_ciphers;
 	conf.wps = ctx->wps;
 	conf.cert_in_cb = ctx->cert_in_cb;
 
diff --git a/src/eapol_supp/eapol_supp_sm.h b/src/eapol_supp/eapol_supp_sm.h
index aa91b8cd5..871c63e13 100644
--- a/src/eapol_supp/eapol_supp_sm.h
+++ b/src/eapol_supp/eapol_supp_sm.h
@@ -212,15 +212,6 @@  struct eapol_ctx {
 	const char *pkcs11_module_path;
 
 	/**
-	 * openssl_ciphers - OpenSSL cipher string
-	 *
-	 * This is an OpenSSL specific configuration option for configuring the
-	 * default ciphers. If not set, "DEFAULT:!EXP:!LOW" is used as the
-	 * default.
-	 */
-	const char *openssl_ciphers;
-
-	/**
 	 * wps - WPS context data
 	 *
 	 * This is only used by EAP-WSC and can be left %NULL if not available.
diff --git a/wpa_supplicant/config.c b/wpa_supplicant/config.c
index 9e54f6cad..f3e5cdde1 100644
--- a/wpa_supplicant/config.c
+++ b/wpa_supplicant/config.c
@@ -2573,6 +2573,14 @@  struct wpa_ssid * wpa_config_add_network(struct wpa_config *config)
 		return NULL;
 	ssid->id = id;
 	dl_list_init(&ssid->psk_list);
+
+	if (config->openssl_ciphers != NULL) {
+		ssid->eap.openssl_ciphers = os_strdup(config->openssl_ciphers);
+		if (ssid->eap.openssl_ciphers == NULL)
+			os_free(ssid);
+			return NULL;
+	}
+
 	if (last)
 		last->next = ssid;
 	else
diff --git a/wpa_supplicant/config_file.c b/wpa_supplicant/config_file.c
index e8f11493e..1ee5e92e1 100644
--- a/wpa_supplicant/config_file.c
+++ b/wpa_supplicant/config_file.c
@@ -163,7 +163,7 @@  static int wpa_config_validate_network(struct wpa_ssid *ssid, int line)
 }
 
 
-static struct wpa_ssid * wpa_config_read_network(FILE *f, int *line, int id)
+static struct wpa_ssid * wpa_config_read_network(struct wpa_config *config, FILE *f, int *line)
 {
 	struct wpa_ssid *ssid;
 	int errors = 0, end = 0;
@@ -171,11 +171,9 @@  static struct wpa_ssid * wpa_config_read_network(FILE *f, int *line, int id)
 
 	wpa_printf(MSG_MSGDUMP, "Line: %d - start of a new network block",
 		   *line);
-	ssid = os_zalloc(sizeof(*ssid));
+	ssid = wpa_config_add_network(config);
 	if (ssid == NULL)
 		return NULL;
-	dl_list_init(&ssid->psk_list);
-	ssid->id = id;
 
 	wpa_config_set_network_defaults(ssid);
 
@@ -368,10 +366,9 @@  struct wpa_config * wpa_config_read(const char *name, struct wpa_config *cfgp)
 	FILE *f;
 	char buf[512], *pos;
 	int errors = 0, line = 0;
-	struct wpa_ssid *ssid, *tail, *head;
+	struct wpa_ssid *ssid;
 	struct wpa_cred *cred, *cred_tail, *cred_head;
 	struct wpa_config *config;
-	int id = 0;
 	int cred_id = 0;
 
 	if (name == NULL)
@@ -385,9 +382,6 @@  struct wpa_config * wpa_config_read(const char *name, struct wpa_config *cfgp)
 			   "structure");
 		return NULL;
 	}
-	tail = head = config->ssid;
-	while (tail && tail->next)
-		tail = tail->next;
 	cred_tail = cred_head = config->cred;
 	while (cred_tail && cred_tail->next)
 		cred_tail = cred_tail->next;
@@ -403,26 +397,13 @@  struct wpa_config * wpa_config_read(const char *name, struct wpa_config *cfgp)
 
 	while (wpa_config_get_line(buf, sizeof(buf), f, &line, &pos)) {
 		if (os_strcmp(pos, "network={") == 0) {
-			ssid = wpa_config_read_network(f, &line, id++);
+			ssid = wpa_config_read_network(config, f, &line);
 			if (ssid == NULL) {
 				wpa_printf(MSG_ERROR, "Line %d: failed to "
 					   "parse network block.", line);
 				errors++;
 				continue;
 			}
-			if (head == NULL) {
-				head = tail = ssid;
-			} else {
-				tail->next = ssid;
-				tail = ssid;
-			}
-			if (wpa_config_add_prio_network(config, ssid)) {
-				wpa_printf(MSG_ERROR, "Line %d: failed to add "
-					   "network block to priority list.",
-					   line);
-				errors++;
-				continue;
-			}
 		} else if (os_strcmp(pos, "cred={") == 0) {
 			cred = wpa_config_read_cred(f, &line, cred_id++);
 			if (cred == NULL) {
@@ -457,7 +438,6 @@  struct wpa_config * wpa_config_read(const char *name, struct wpa_config *cfgp)
 
 	fclose(f);
 
-	config->ssid = head;
 	wpa_config_debug_dump_networks(config);
 	config->cred = cred_head;
 
@@ -465,7 +445,6 @@  struct wpa_config * wpa_config_read(const char *name, struct wpa_config *cfgp)
 	if (errors) {
 		wpa_config_free(config);
 		config = NULL;
-		head = NULL;
 	}
 #endif /* WPA_IGNORE_CONFIG_ERRORS */
 
diff --git a/wpa_supplicant/config_winreg.c b/wpa_supplicant/config_winreg.c
index 6bea59760..9092c8ae9 100644
--- a/wpa_supplicant/config_winreg.c
+++ b/wpa_supplicant/config_winreg.c
@@ -279,8 +279,7 @@  static int wpa_config_read_global(struct wpa_config *config, HKEY hk)
 }
 
 
-static struct wpa_ssid * wpa_config_read_network(HKEY hk, const TCHAR *netw,
-						 int id)
+static struct wpa_ssid * wpa_config_read_network(struct wpa_config *config, HKEY hk, const TCHAR *netw)
 {
 	HKEY nhk;
 	LONG ret;
@@ -296,13 +295,9 @@  static struct wpa_ssid * wpa_config_read_network(HKEY hk, const TCHAR *netw,
 	}
 
 	wpa_printf(MSG_MSGDUMP, "Start of a new network '" TSTR "'", netw);
-	ssid = os_zalloc(sizeof(*ssid));
-	if (ssid == NULL) {
-		RegCloseKey(nhk);
+	ssid = wpa_config_add_network(config);
+	if (ssid == NULL)
 		return NULL;
-	}
-	dl_list_init(&ssid->psk_list);
-	ssid->id = id;
 
 	wpa_config_set_network_defaults(ssid);
 
@@ -371,7 +366,7 @@  static struct wpa_ssid * wpa_config_read_network(HKEY hk, const TCHAR *netw,
 static int wpa_config_read_networks(struct wpa_config *config, HKEY hk)
 {
 	HKEY nhk;
-	struct wpa_ssid *ssid, *tail = NULL, *head = NULL;
+	struct wpa_ssid *ssid;
 	int errors = 0;
 	LONG ret;
 	DWORD i;
@@ -405,19 +400,13 @@  static int wpa_config_read_networks(struct wpa_config *config, HKEY hk)
 			namelen = 255 - 1;
 		name[namelen] = '\0';
 
-		ssid = wpa_config_read_network(nhk, name, i);
+		ssid = wpa_config_read_network(config, nhk, name);
 		if (ssid == NULL) {
 			wpa_printf(MSG_ERROR, "Failed to parse network "
 				   "profile '%s'.", name);
 			errors++;
 			continue;
 		}
-		if (head == NULL) {
-			head = tail = ssid;
-		} else {
-			tail->next = ssid;
-			tail = ssid;
-		}
 		if (wpa_config_add_prio_network(config, ssid)) {
 			wpa_printf(MSG_ERROR, "Failed to add network profile "
 				   "'%s' to priority list.", name);
@@ -428,8 +417,6 @@  static int wpa_config_read_networks(struct wpa_config *config, HKEY hk)
 
 	RegCloseKey(nhk);
 
-	config->ssid = head;
-
 	return errors ? -1 : 0;
 }
 
diff --git a/wpa_supplicant/wpas_glue.c b/wpa_supplicant/wpas_glue.c
index fb383962a..c7c033137 100644
--- a/wpa_supplicant/wpas_glue.c
+++ b/wpa_supplicant/wpas_glue.c
@@ -1091,7 +1091,6 @@  int wpa_supplicant_init_eapol(struct wpa_supplicant *wpa_s)
 	ctx->opensc_engine_path = wpa_s->conf->opensc_engine_path;
 	ctx->pkcs11_engine_path = wpa_s->conf->pkcs11_engine_path;
 	ctx->pkcs11_module_path = wpa_s->conf->pkcs11_module_path;
-	ctx->openssl_ciphers = wpa_s->conf->openssl_ciphers;
 	ctx->wps = wpa_s->wps;
 	ctx->eap_param_needed = wpa_supplicant_eap_param_needed;
 #ifdef CONFIG_EAP_PROXY