[PATCH] Bluetooth: Expose debugfs entry to allow illegal static address

[PATCH] Bluetooth: Expose debugfs entry to allow illegal static address

[mike]

From: Mike Ryan <mike@ice9.us>

The first two bits of valid static addresses must both be 0 because
other values are reserved for resolvable and non-resolvable private
addresses (RPA and NRPA). In order to facilitate impersonation attacks
against devices using RPAs, this debugfs entry allows the user to set
the static address to an illegal value reserved for RPAs.

Signed-off-by: Mike Ryan <mike@ice9.us>
---
 include/net/bluetooth/hci.h |  1 +
 net/bluetooth/hci_debugfs.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 net/bluetooth/mgmt.c        |  4 +++-
 3 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 5bc1e30de..d1e477093 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -280,6 +280,7 @@ enum {
 	HCI_DUT_MODE,
 	HCI_VENDOR_DIAG,
 	HCI_FORCE_BREDR_SMP,
+	HCI_ALLOW_ILLEGAL_STATIC_ADDR,
 	HCI_FORCE_STATIC_ADDR,
 	HCI_LL_RPA_RESOLUTION,
 	HCI_CMD_PENDING,
diff --git a/net/bluetooth/hci_debugfs.c b/net/bluetooth/hci_debugfs.c
index 402e2cc54..0d3006302 100644
--- a/net/bluetooth/hci_debugfs.c
+++ b/net/bluetooth/hci_debugfs.c
@@ -667,6 +667,47 @@ static int static_address_show(struct seq_file *f, void *p)
 
 DEFINE_SHOW_ATTRIBUTE(static_address);
 
+static ssize_t allow_illegal_static_address_read(struct file *file,
+					 char __user *user_buf,
+					 size_t count, loff_t *ppos)
+{
+	struct hci_dev *hdev = file->private_data;
+	char buf[3];
+
+	buf[0] = hci_dev_test_flag(hdev, HCI_ALLOW_ILLEGAL_STATIC_ADDR) ? 'Y': 'N';
+	buf[1] = '\n';
+	buf[2] = '\0';
+	return simple_read_from_buffer(user_buf, count, ppos, buf, 2);
+}
+
+static ssize_t allow_illegal_static_address_write(struct file *file,
+					  const char __user *user_buf,
+					  size_t count, loff_t *ppos)
+{
+	struct hci_dev *hdev = file->private_data;
+	bool enable;
+	int err;
+
+	err = kstrtobool_from_user(user_buf, count, &enable);
+	if (err)
+		return err;
+
+	if (enable == hci_dev_test_flag(hdev, HCI_ALLOW_ILLEGAL_STATIC_ADDR))
+		return -EALREADY;
+
+	hci_dev_change_flag(hdev, HCI_ALLOW_ILLEGAL_STATIC_ADDR);
+
+	return count;
+
+}
+
+static const struct file_operations allow_illegal_static_address_fops = {
+	.open		= simple_open,
+	.read		= allow_illegal_static_address_read,
+	.write		= allow_illegal_static_address_write,
+	.llseek		= default_llseek,
+};
+
 static ssize_t force_static_address_read(struct file *file,
 					 char __user *user_buf,
 					 size_t count, loff_t *ppos)
@@ -1016,6 +1057,8 @@ void hci_debugfs_create_le(struct hci_dev *hdev)
 			    &random_address_fops);
 	debugfs_create_file("static_address", 0444, hdev->debugfs, hdev,
 			    &static_address_fops);
+	debugfs_create_file("allow_illegal_static_address", 0644, hdev->debugfs,
+			    hdev, &allow_illegal_static_address_fops);
 
 	/* For controllers with a public address, provide a debug
 	 * option to force the usage of the configured static
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index acb7c6d56..59945b850 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -4428,10 +4428,12 @@ static int set_static_address(struct sock *sk, struct hci_dev *hdev,
 					       MGMT_STATUS_INVALID_PARAMS);
 
 		/* Two most significant bits shall be set */
-		if ((cp->bdaddr.b[5] & 0xc0) != 0xc0)
+		if ((cp->bdaddr.b[5] & 0xc0) != 0xc0 &&
+		    !hci_dev_test_flag(hdev, HCI_ALLOW_ILLEGAL_STATIC_ADDR)) {
 			return mgmt_cmd_status(sk, hdev->id,
 					       MGMT_OP_SET_STATIC_ADDRESS,
 					       MGMT_STATUS_INVALID_PARAMS);
+		}
 	}
 
 	hci_dev_lock(hdev);
-- 
2.11.0

Re: [PATCH] Bluetooth: Expose debugfs entry to allow illegal static address

[Marcel Holtmann]

Hi Mike,

> The first two bits of valid static addresses must both be 0 because
> other values are reserved for resolvable and non-resolvable private
> addresses (RPA and NRPA). In order to facilitate impersonation attacks
> against devices using RPAs, this debugfs entry allows the user to set
> the static address to an illegal value reserved for RPAs.
> 
> Signed-off-by: Mike Ryan <mike@ice9.us>
> ---
> include/net/bluetooth/hci.h |  1 +
> net/bluetooth/hci_debugfs.c | 43 +++++++++++++++++++++++++++++++++++++++++++
> net/bluetooth/mgmt.c        |  4 +++-
> 3 files changed, 47 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 5bc1e30de..d1e477093 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -280,6 +280,7 @@ enum {
> 	HCI_DUT_MODE,
> 	HCI_VENDOR_DIAG,
> 	HCI_FORCE_BREDR_SMP,
> +	HCI_ALLOW_ILLEGAL_STATIC_ADDR,
> 	HCI_FORCE_STATIC_ADDR,
> 	HCI_LL_RPA_RESOLUTION,
> 	HCI_CMD_PENDING,
> diff --git a/net/bluetooth/hci_debugfs.c b/net/bluetooth/hci_debugfs.c
> index 402e2cc54..0d3006302 100644
> --- a/net/bluetooth/hci_debugfs.c
> +++ b/net/bluetooth/hci_debugfs.c
> @@ -667,6 +667,47 @@ static int static_address_show(struct seq_file *f, void *p)
> 
> DEFINE_SHOW_ATTRIBUTE(static_address);
> 
> +static ssize_t allow_illegal_static_address_read(struct file *file,
> +					 char __user *user_buf,
> +					 size_t count, loff_t *ppos)
> +{
> +	struct hci_dev *hdev = file->private_data;
> +	char buf[3];
> +
> +	buf[0] = hci_dev_test_flag(hdev, HCI_ALLOW_ILLEGAL_STATIC_ADDR) ? 'Y': 'N';
> +	buf[1] = '\n';
> +	buf[2] = '\0';
> +	return simple_read_from_buffer(user_buf, count, ppos, buf, 2);
> +}
> +
> +static ssize_t allow_illegal_static_address_write(struct file *file,
> +					  const char __user *user_buf,
> +					  size_t count, loff_t *ppos)
> +{
> +	struct hci_dev *hdev = file->private_data;
> +	bool enable;
> +	int err;
> +
> +	err = kstrtobool_from_user(user_buf, count, &enable);
> +	if (err)
> +		return err;
> +
> +	if (enable == hci_dev_test_flag(hdev, HCI_ALLOW_ILLEGAL_STATIC_ADDR))
> +		return -EALREADY;
> +
> +	hci_dev_change_flag(hdev, HCI_ALLOW_ILLEGAL_STATIC_ADDR);
> +
> +	return count;
> +
> +}
> +
> +static const struct file_operations allow_illegal_static_address_fops = {
> +	.open		= simple_open,
> +	.read		= allow_illegal_static_address_read,
> +	.write		= allow_illegal_static_address_write,
> +	.llseek		= default_llseek,
> +};
> +
> static ssize_t force_static_address_read(struct file *file,
> 					 char __user *user_buf,
> 					 size_t count, loff_t *ppos)
> @@ -1016,6 +1057,8 @@ void hci_debugfs_create_le(struct hci_dev *hdev)
> 			    &random_address_fops);
> 	debugfs_create_file("static_address", 0444, hdev->debugfs, hdev,
> 			    &static_address_fops);
> +	debugfs_create_file("allow_illegal_static_address", 0644, hdev->debugfs,
> +			    hdev, &allow_illegal_static_address_fops);
> 
> 	/* For controllers with a public address, provide a debug
> 	 * option to force the usage of the configured static
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index acb7c6d56..59945b850 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -4428,10 +4428,12 @@ static int set_static_address(struct sock *sk, struct hci_dev *hdev,
> 					       MGMT_STATUS_INVALID_PARAMS);
> 
> 		/* Two most significant bits shall be set */
> -		if ((cp->bdaddr.b[5] & 0xc0) != 0xc0)
> +		if ((cp->bdaddr.b[5] & 0xc0) != 0xc0 &&
> +		    !hci_dev_test_flag(hdev, HCI_ALLOW_ILLEGAL_STATIC_ADDR)) {
> 			return mgmt_cmd_status(sk, hdev->id,
> 					       MGMT_OP_SET_STATIC_ADDRESS,
> 					       MGMT_STATUS_INVALID_PARAMS);
> +		}
> 	}

this is too much stabbing in the back for my liking. It is really abusing the standard mgmt interface as we defined it. So a patch in this shape and form can not go upstream.

In debugfs we have the two files “static_address” to read out the current address and the “force_static_address” option to use the static address for dual-mode controllers. I would rather add another file that acts the same as “Set Static Address” command from mgmt, but will allow you to provide any random address. Maybe “set_static_address” as a file name, but I am not 100% convinced that this is a good idea.

If the goal is for faking an RPA, then it might be actually better to really require enabling Privacy first and then have a debugfs option to force the usage of a specific RPA. Since I am not sure what might break if we stop enforcing the usage for static address. Since the static address is also behind the identity address. And using a RPA as identity address is not allowed.

Regards

Marcel