lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Mon,  7 Sep 2020 12:14:55 +0200
From:   Jerome Pouiller <Jerome.Pouiller@...abs.com>
To:     devel@...verdev.osuosl.org, linux-wireless@...r.kernel.org
Cc:     netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Kalle Valo <kvalo@...eaurora.org>,
        "David S . Miller" <davem@...emloft.net>,
        Jérôme Pouiller 
        <jerome.pouiller@...abs.com>
Subject: [PATCH 05/31] staging: wfx: drop 'secure link' feature

From: Jérôme Pouiller <jerome.pouiller@...abs.com>

The Secure Link (slk) feature allows to encrypt (and authenticate) the
traffic between the host and the device. The official implementation of
this feature relies on mbedTLS. For that reason, this implementation is
not included in the current driver. To be included, the implementation
has to rely on kernel crypto API instead of mbedTLS.

So, for now, the driver contained stub functions waiting for the new
implementation based on kernel crypto API.

This new implementation represent a bunch of work and it is unlikely to
be done in near future.  Moreover, we strongly dislike to maintain
useless code. So, drop all the code related to secure link.

Whoever wants to implement secure link should revert this patch before
starting to work on it.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@...abs.com>
---
 drivers/staging/wfx/bh.c              | 48 ++--------------
 drivers/staging/wfx/hif_api_general.h | 80 ---------------------------
 drivers/staging/wfx/hif_rx.c          | 19 -------
 drivers/staging/wfx/hif_tx.c          | 66 ----------------------
 drivers/staging/wfx/hif_tx.h          |  6 --
 drivers/staging/wfx/main.c            |  9 +--
 drivers/staging/wfx/secure_link.h     | 59 --------------------
 drivers/staging/wfx/wfx.h             |  2 -
 8 files changed, 8 insertions(+), 281 deletions(-)
 delete mode 100644 drivers/staging/wfx/secure_link.h

diff --git a/drivers/staging/wfx/bh.c b/drivers/staging/wfx/bh.c
index f07bcee50e3f..72da2f4af49f 100644
--- a/drivers/staging/wfx/bh.c
+++ b/drivers/staging/wfx/bh.c
@@ -12,7 +12,6 @@
 #include "wfx.h"
 #include "hwio.h"
 #include "traces.h"
-#include "secure_link.h"
 #include "hif_rx.h"
 #include "hif_api_cmd.h"
 
@@ -88,20 +87,11 @@ static int rx_helper(struct wfx_dev *wdev, size_t read_len, int *is_cnf)
 	_trace_piggyback(piggyback, false);
 
 	hif = (struct hif_msg *)skb->data;
-	WARN(hif->encrypted & 0x1, "unsupported encryption type");
-	if (hif->encrypted == 0x2) {
-		if (WARN(read_len < sizeof(struct hif_sl_msg), "corrupted read"))
-			goto err;
-		computed_len = le16_to_cpu(((struct hif_sl_msg *)hif)->len);
-		computed_len = round_up(computed_len - sizeof(u16), 16);
-		computed_len += sizeof(struct hif_sl_msg);
-		computed_len += sizeof(struct hif_sl_tag);
-	} else {
-		if (WARN(read_len < sizeof(struct hif_msg), "corrupted read"))
-			goto err;
-		computed_len = le16_to_cpu(hif->len);
-		computed_len = round_up(computed_len, 2);
-	}
+	WARN(hif->encrypted & 0x3, "encryption is unsupported");
+	if (WARN(read_len < sizeof(struct hif_msg), "corrupted read"))
+		goto err;
+	computed_len = le16_to_cpu(hif->len);
+	computed_len = round_up(computed_len, 2);
 	if (computed_len != read_len) {
 		dev_err(wdev->dev, "inconsistent message length: %zu != %zu\n",
 			computed_len, read_len);
@@ -109,16 +99,6 @@ static int rx_helper(struct wfx_dev *wdev, size_t read_len, int *is_cnf)
 			       hif, read_len, true);
 		goto err;
 	}
-	if (hif->encrypted == 0x2) {
-		if (wfx_sl_decode(wdev, (struct hif_sl_msg *)hif)) {
-			dev_kfree_skb(skb);
-			// If frame was a confirmation, expect trouble in next
-			// exchange. However, it is harmless to fail to decode
-			// an indication frame, so try to continue. Anyway,
-			// piggyback is probably correct.
-			return piggyback;
-		}
-	}
 
 	if (!(hif->id & HIF_ID_IS_INDICATION)) {
 		(*is_cnf)++;
@@ -199,23 +179,7 @@ static void tx_helper(struct wfx_dev *wdev, struct hif_msg *hif)
 	hif->seqnum = wdev->hif.tx_seqnum;
 	wdev->hif.tx_seqnum = (wdev->hif.tx_seqnum + 1) % (HIF_COUNTER_MAX + 1);
 
-	if (wfx_is_secure_command(wdev, hif->id)) {
-		len = round_up(len - sizeof(hif->len), 16) + sizeof(hif->len) +
-			sizeof(struct hif_sl_msg_hdr) +
-			sizeof(struct hif_sl_tag);
-		// AES support encryption in-place. However, mac80211 access to
-		// 802.11 header after frame was sent (to get MAC addresses).
-		// So, keep origin buffer clear.
-		data = kmalloc(len, GFP_KERNEL);
-		if (!data)
-			goto end;
-		is_encrypted = true;
-		ret = wfx_sl_encode(wdev, hif, data);
-		if (ret)
-			goto end;
-	} else {
-		data = hif;
-	}
+	data = hif;
 	WARN(len > wdev->hw_caps.size_inp_ch_buf,
 	     "%s: request exceed WFx capability: %zu > %d\n", __func__,
 	     len, wdev->hw_caps.size_inp_ch_buf);
diff --git a/drivers/staging/wfx/hif_api_general.h b/drivers/staging/wfx/hif_api_general.h
index 791d7375bd7f..0dc13176a05e 100644
--- a/drivers/staging/wfx/hif_api_general.h
+++ b/drivers/staging/wfx/hif_api_general.h
@@ -282,84 +282,4 @@ enum hif_secure_link_state {
 	SEC_LINK_ENFORCED    = 0x3
 };
 
-enum hif_sl_encryption_type {
-	NO_ENCRYPTION = 0,
-	TX_ENCRYPTION = 1,
-	RX_ENCRYPTION = 2,
-	HP_ENCRYPTION = 3
-};
-
-struct hif_sl_msg_hdr {
-	u32    seqnum:30;
-	u32    encrypted:2;
-} __packed;
-
-struct hif_sl_msg {
-	struct hif_sl_msg_hdr hdr;
-	__le16 len;
-	u8     payload[];
-} __packed;
-
-#define AES_CCM_TAG_SIZE          16
-
-struct hif_sl_tag {
-	u8     tag[16];
-} __packed;
-
-enum hif_sl_mac_key_dest {
-	SL_MAC_KEY_DEST_OTP = 0x78,
-	SL_MAC_KEY_DEST_RAM = 0x87
-};
-
-#define API_KEY_VALUE_SIZE        32
-
-struct hif_req_set_sl_mac_key {
-	u8     otp_or_ram;
-	u8     key_value[API_KEY_VALUE_SIZE];
-} __packed;
-
-struct hif_cnf_set_sl_mac_key {
-	__le32 status;
-} __packed;
-
-enum hif_sl_session_key_alg {
-	HIF_SL_CURVE25519 = 0x01,
-	HIF_SL_KDF        = 0x02
-};
-
-#define API_HOST_PUB_KEY_SIZE     32
-#define API_HOST_PUB_KEY_MAC_SIZE 64
-
-struct hif_req_sl_exchange_pub_keys {
-	u8     algorithm:2;
-	u8     reserved1:6;
-	u8     reserved2[3];
-	u8     host_pub_key[API_HOST_PUB_KEY_SIZE];
-	u8     host_pub_key_mac[API_HOST_PUB_KEY_MAC_SIZE];
-} __packed;
-
-struct hif_cnf_sl_exchange_pub_keys {
-	__le32 status;
-} __packed;
-
-#define API_NCP_PUB_KEY_SIZE      32
-#define API_NCP_PUB_KEY_MAC_SIZE  64
-
-struct hif_ind_sl_exchange_pub_keys {
-	__le32 status;
-	u8     ncp_pub_key[API_NCP_PUB_KEY_SIZE];
-	u8     ncp_pub_key_mac[API_NCP_PUB_KEY_MAC_SIZE];
-} __packed;
-
-struct hif_req_sl_configure {
-	u8     encr_bmp[32];
-	u8     disable_session_key_protection:1;
-	u8     reserved1:7;
-	u8     reserved2[3];
-} __packed;
-
-struct hif_cnf_sl_configure {
-	__le32 status;
-} __packed;
-
 #endif
diff --git a/drivers/staging/wfx/hif_rx.c b/drivers/staging/wfx/hif_rx.c
index 36b393b92936..cf7a956ef00a 100644
--- a/drivers/staging/wfx/hif_rx.c
+++ b/drivers/staging/wfx/hif_rx.c
@@ -15,7 +15,6 @@
 #include "bh.h"
 #include "sta.h"
 #include "data_rx.h"
-#include "secure_link.h"
 #include "hif_api_cmd.h"
 
 static int hif_generic_confirm(struct wfx_dev *wdev,
@@ -53,8 +52,6 @@ static int hif_generic_confirm(struct wfx_dev *wdev,
 	} else {
 		wdev->hif_cmd.buf_send = NULL;
 		mutex_unlock(&wdev->hif_cmd.lock);
-		if (cmd != HIF_REQ_ID_SL_EXCHANGE_PUB_KEYS)
-			mutex_unlock(&wdev->hif_cmd.key_renew_lock);
 	}
 	return status;
 }
@@ -110,21 +107,6 @@ static int hif_wakeup_indication(struct wfx_dev *wdev,
 	return 0;
 }
 
-static int hif_keys_indication(struct wfx_dev *wdev,
-			       const struct hif_msg *hif, const void *buf)
-{
-	const struct hif_ind_sl_exchange_pub_keys *body = buf;
-	u8 pubkey[API_NCP_PUB_KEY_SIZE];
-
-	// SL_PUB_KEY_EXCHANGE_STATUS_SUCCESS is used by legacy secure link
-	if (body->status && body->status != HIF_STATUS_SLK_NEGO_SUCCESS)
-		dev_warn(wdev->dev, "secure link negotiation error\n");
-	memcpy(pubkey, body->ncp_pub_key, sizeof(pubkey));
-	memreverse(pubkey, sizeof(pubkey));
-	wfx_sl_check_pubkey(wdev, pubkey, body->ncp_pub_key_mac);
-	return 0;
-}
-
 static int hif_receive_indication(struct wfx_dev *wdev,
 				  const struct hif_msg *hif,
 				  const void *buf, struct sk_buff *skb)
@@ -380,7 +362,6 @@ static const struct {
 	{ HIF_IND_ID_SET_PM_MODE_CMPL,     hif_pm_mode_complete_indication },
 	{ HIF_IND_ID_SCAN_CMPL,            hif_scan_complete_indication },
 	{ HIF_IND_ID_SUSPEND_RESUME_TX,    hif_suspend_resume_indication },
-	{ HIF_IND_ID_SL_EXCHANGE_PUB_KEYS, hif_keys_indication },
 	{ HIF_IND_ID_EVENT,                hif_event_indication },
 	{ HIF_IND_ID_GENERIC,              hif_generic_indication },
 	{ HIF_IND_ID_ERROR,                hif_error_indication },
diff --git a/drivers/staging/wfx/hif_tx.c b/drivers/staging/wfx/hif_tx.c
index 6b89e55f03f4..f91b19ddf8e3 100644
--- a/drivers/staging/wfx/hif_tx.c
+++ b/drivers/staging/wfx/hif_tx.c
@@ -20,7 +20,6 @@ void wfx_init_hif_cmd(struct wfx_hif_cmd *hif_cmd)
 	init_completion(&hif_cmd->ready);
 	init_completion(&hif_cmd->done);
 	mutex_init(&hif_cmd->lock);
-	mutex_init(&hif_cmd->key_renew_lock);
 }
 
 static void wfx_fill_header(struct hif_msg *hif, int if_id,
@@ -62,9 +61,6 @@ int wfx_cmd_send(struct wfx_dev *wdev, struct hif_msg *request,
 	if (wdev->chip_frozen)
 		return -ETIMEDOUT;
 
-	if (cmd != HIF_REQ_ID_SL_EXCHANGE_PUB_KEYS)
-		mutex_lock(&wdev->hif_cmd.key_renew_lock);
-
 	mutex_lock(&wdev->hif_cmd.lock);
 	WARN(wdev->hif_cmd.buf_send, "data locking error");
 
@@ -118,8 +114,6 @@ int wfx_cmd_send(struct wfx_dev *wdev, struct hif_msg *request,
 			 "WSM request %s%s%s (%#.2x) on vif %d returned status %d\n",
 			 get_hif_name(cmd), mib_sep, mib_name, cmd, vif, ret);
 
-	if (cmd != HIF_REQ_ID_SL_EXCHANGE_PUB_KEYS)
-		mutex_unlock(&wdev->hif_cmd.key_renew_lock);
 	return ret;
 }
 
@@ -147,7 +141,6 @@ int hif_shutdown(struct wfx_dev *wdev)
 	else
 		control_reg_write(wdev, 0);
 	mutex_unlock(&wdev->hif_cmd.lock);
-	mutex_unlock(&wdev->hif_cmd.key_renew_lock);
 	kfree(hif);
 	return ret;
 }
@@ -535,62 +528,3 @@ int hif_update_ie_beacon(struct wfx_vif *wvif, const u8 *ies, size_t ies_len)
 	kfree(hif);
 	return ret;
 }
-
-int hif_sl_send_pub_keys(struct wfx_dev *wdev,
-			 const u8 *pubkey, const u8 *pubkey_hmac)
-{
-	int ret;
-	struct hif_msg *hif;
-	struct hif_req_sl_exchange_pub_keys *body = wfx_alloc_hif(sizeof(*body),
-								  &hif);
-
-	if (!hif)
-		return -ENOMEM;
-	body->algorithm = HIF_SL_CURVE25519;
-	memcpy(body->host_pub_key, pubkey, sizeof(body->host_pub_key));
-	memcpy(body->host_pub_key_mac, pubkey_hmac,
-	       sizeof(body->host_pub_key_mac));
-	wfx_fill_header(hif, -1, HIF_REQ_ID_SL_EXCHANGE_PUB_KEYS,
-			sizeof(*body));
-	ret = wfx_cmd_send(wdev, hif, NULL, 0, false);
-	kfree(hif);
-	// Compatibility with legacy secure link
-	if (ret == le32_to_cpu(HIF_STATUS_SLK_NEGO_SUCCESS))
-		ret = 0;
-	return ret;
-}
-
-int hif_sl_config(struct wfx_dev *wdev, const unsigned long *bitmap)
-{
-	int ret;
-	struct hif_msg *hif;
-	struct hif_req_sl_configure *body = wfx_alloc_hif(sizeof(*body), &hif);
-
-	if (!hif)
-		return -ENOMEM;
-	memcpy(body->encr_bmp, bitmap, sizeof(body->encr_bmp));
-	wfx_fill_header(hif, -1, HIF_REQ_ID_SL_CONFIGURE, sizeof(*body));
-	ret = wfx_cmd_send(wdev, hif, NULL, 0, false);
-	kfree(hif);
-	return ret;
-}
-
-int hif_sl_set_mac_key(struct wfx_dev *wdev, const u8 *slk_key, int destination)
-{
-	int ret;
-	struct hif_msg *hif;
-	struct hif_req_set_sl_mac_key *body = wfx_alloc_hif(sizeof(*body),
-							    &hif);
-
-	if (!hif)
-		return -ENOMEM;
-	memcpy(body->key_value, slk_key, sizeof(body->key_value));
-	body->otp_or_ram = destination;
-	wfx_fill_header(hif, -1, HIF_REQ_ID_SET_SL_MAC_KEY, sizeof(*body));
-	ret = wfx_cmd_send(wdev, hif, NULL, 0, false);
-	kfree(hif);
-	// Compatibility with legacy secure link
-	if (ret == le32_to_cpu(HIF_STATUS_SLK_SET_KEY_SUCCESS))
-		ret = 0;
-	return ret;
-}
diff --git a/drivers/staging/wfx/hif_tx.h b/drivers/staging/wfx/hif_tx.h
index b371b3777a31..e8aca39e7497 100644
--- a/drivers/staging/wfx/hif_tx.h
+++ b/drivers/staging/wfx/hif_tx.h
@@ -20,7 +20,6 @@ struct wfx_vif;
 
 struct wfx_hif_cmd {
 	struct mutex      lock;
-	struct mutex      key_renew_lock;
 	struct completion ready;
 	struct completion done;
 	bool              async;
@@ -58,10 +57,5 @@ int hif_beacon_transmit(struct wfx_vif *wvif, bool enable);
 int hif_map_link(struct wfx_vif *wvif,
 		 bool unmap, u8 *mac_addr, int sta_id, bool mfp);
 int hif_update_ie_beacon(struct wfx_vif *wvif, const u8 *ies, size_t ies_len);
-int hif_sl_set_mac_key(struct wfx_dev *wdev,
-		       const u8 *slk_key, int destination);
-int hif_sl_config(struct wfx_dev *wdev, const unsigned long *bitmap);
-int hif_sl_send_pub_keys(struct wfx_dev *wdev,
-			 const u8 *pubkey, const u8 *pubkey_hmac);
 
 #endif
diff --git a/drivers/staging/wfx/main.c b/drivers/staging/wfx/main.c
index 5e2b82499004..2af4914e905c 100644
--- a/drivers/staging/wfx/main.c
+++ b/drivers/staging/wfx/main.c
@@ -30,7 +30,6 @@
 #include "scan.h"
 #include "debug.h"
 #include "data_tx.h"
-#include "secure_link.h"
 #include "hif_tx_mib.h"
 #include "hif_api_cmd.h"
 
@@ -271,8 +270,7 @@ struct wfx_dev *wfx_init_common(struct device *dev,
 	hw->queues = 4;
 	hw->max_rates = 8;
 	hw->max_rate_tries = 8;
-	hw->extra_tx_headroom = sizeof(struct hif_sl_msg_hdr) +
-				sizeof(struct hif_msg)
+	hw->extra_tx_headroom = sizeof(struct hif_msg)
 				+ sizeof(struct hif_req_tx)
 				+ 4 /* alignment */ + 8 /* TKIP IV */;
 	hw->wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION) |
@@ -309,7 +307,6 @@ struct wfx_dev *wfx_init_common(struct device *dev,
 		return ERR_CAST(wdev->pdata.gpio_wakeup);
 	if (wdev->pdata.gpio_wakeup)
 		gpiod_set_consumer_name(wdev->pdata.gpio_wakeup, "wfx wakeup");
-	wfx_sl_fill_pdata(dev, &wdev->pdata);
 
 	mutex_init(&wdev->conf_mutex);
 	mutex_init(&wdev->rx_stats_lock);
@@ -381,8 +378,7 @@ int wfx_probe(struct wfx_dev *wdev)
 		goto err0;
 	}
 
-	err = wfx_sl_init(wdev);
-	if (err && wdev->hw_caps.capabilities.link_mode == SEC_LINK_ENFORCED) {
+	if (wdev->hw_caps.capabilities.link_mode == SEC_LINK_ENFORCED) {
 		dev_err(wdev->dev,
 			"chip require secure_link, but can't negotiate it\n");
 		goto err0;
@@ -466,7 +462,6 @@ void wfx_release(struct wfx_dev *wdev)
 	hif_shutdown(wdev);
 	wdev->hwbus_ops->irq_unsubscribe(wdev->hwbus_priv);
 	wfx_bh_unregister(wdev);
-	wfx_sl_deinit(wdev);
 }
 
 static int __init wfx_core_init(void)
diff --git a/drivers/staging/wfx/secure_link.h b/drivers/staging/wfx/secure_link.h
deleted file mode 100644
index c3d055b2f8b1..000000000000
--- a/drivers/staging/wfx/secure_link.h
+++ /dev/null
@@ -1,59 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-/*
- * Copyright (c) 2019, Silicon Laboratories, Inc.
- */
-#ifndef WFX_SECURE_LINK_H
-#define WFX_SECURE_LINK_H
-
-#include <linux/of.h>
-
-#include "hif_api_general.h"
-
-struct wfx_dev;
-
-
-struct sl_context {
-};
-
-static inline bool wfx_is_secure_command(struct wfx_dev *wdev, int cmd_id)
-{
-	return false;
-}
-
-static inline int wfx_sl_decode(struct wfx_dev *wdev, struct hif_sl_msg *m)
-{
-	return -EIO;
-}
-
-static inline int wfx_sl_encode(struct wfx_dev *wdev,
-				const struct hif_msg *input,
-				struct hif_sl_msg *output)
-{
-	return -EIO;
-}
-
-static inline int wfx_sl_check_pubkey(struct wfx_dev *wdev,
-				      const u8 *ncp_pubkey,
-				      const u8 *ncp_pubmac)
-{
-	return -EIO;
-}
-
-static inline void wfx_sl_fill_pdata(struct device *dev,
-				     struct wfx_platform_data *pdata)
-{
-	if (of_find_property(dev->of_node, "slk_key", NULL))
-		dev_err(dev, "secure link is not supported by this driver, ignoring provided key\n");
-}
-
-static inline int wfx_sl_init(struct wfx_dev *wdev)
-{
-	return -EIO;
-}
-
-static inline void wfx_sl_deinit(struct wfx_dev *wdev)
-{
-}
-
-
-#endif
diff --git a/drivers/staging/wfx/wfx.h b/drivers/staging/wfx/wfx.h
index 38e24d7f72f2..241eaaf71f5e 100644
--- a/drivers/staging/wfx/wfx.h
+++ b/drivers/staging/wfx/wfx.h
@@ -20,7 +20,6 @@
 #include "data_tx.h"
 #include "main.h"
 #include "queue.h"
-#include "secure_link.h"
 #include "hif_tx.h"
 
 #define USEC_PER_TXOP 32 // see struct ieee80211_tx_queue_params
@@ -41,7 +40,6 @@ struct wfx_dev {
 	struct completion	firmware_ready;
 	struct hif_ind_startup	hw_caps;
 	struct wfx_hif		hif;
-	struct sl_context	sl;
 	struct delayed_work	cooling_timeout_work;
 	bool			poll_irq;
 	bool			chip_frozen;
-- 
2.28.0

Powered by blists - more mailing lists