[PATCH v3] Bluetooth: Add blacklist support for incoming connections

[PATCH v3] Bluetooth: Add blacklist support for incoming connections

[johan.hedberg]

From: Johan Hedberg <johan.hedberg@nokia.com>

In some circumstances it could be desirable to reject incoming
connections on the baseband level. This patch adds this feature through
two new ioctl's: HCIBLOCKADDR and HCIUNBLOCKADDR. Both take a simple
Bluetooth address as a parameter. BDADDR_ANY can be used with
HCIUNBLOCKADDR to remove all devices from the blacklist.

Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
v3 fixes a memory leak when an HCI device gets unregistered. The
blacklist list is now free'd in hci_dev_do_close along with the other
lists of struct hci_dev.

 fs/compat_ioctl.c                |    2 +
 include/net/bluetooth/hci.h      |    3 +
 include/net/bluetooth/hci_core.h |    9 ++++
 net/bluetooth/hci_core.c         |    3 +
 net/bluetooth/hci_event.c        |    2 +-
 net/bluetooth/hci_sock.c         |   90 ++++++++++++++++++++++++++++++++++++++
 6 files changed, 108 insertions(+), 1 deletions(-)

diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index 641640d..1863896 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -1328,6 +1328,8 @@ COMPATIBLE_IOCTL(HCISETLINKPOL)
 COMPATIBLE_IOCTL(HCISETLINKMODE)
 COMPATIBLE_IOCTL(HCISETACLMTU)
 COMPATIBLE_IOCTL(HCISETSCOMTU)
+COMPATIBLE_IOCTL(HCIBLOCKADDR)
+COMPATIBLE_IOCTL(HCIUNBLOCKADDR)
 COMPATIBLE_IOCTL(HCIINQUIRY)
 COMPATIBLE_IOCTL(HCIUARTSETPROTO)
 COMPATIBLE_IOCTL(HCIUARTGETPROTO)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index fc0c502..ca2518e 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -100,6 +100,9 @@ enum {
 #define HCISETACLMTU	_IOW('H', 227, int)
 #define HCISETSCOMTU	_IOW('H', 228, int)
 
+#define HCIBLOCKADDR	_IOW('H', 230, int)
+#define HCIUNBLOCKADDR	_IOW('H', 231, int)
+
 #define HCIINQUIRY	_IOR('H', 240, int)
 
 /* HCI timeouts */
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index ce3c99e..fba5b74 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -62,6 +62,11 @@ struct hci_conn_hash {
 	unsigned int     sco_num;
 };
 
+struct bdaddr_list {
+	struct list_head list;
+	bdaddr_t bdaddr;
+};
+
 struct hci_dev {
 	struct list_head list;
 	spinlock_t	lock;
@@ -125,6 +130,7 @@ struct hci_dev {
 
 	struct inquiry_cache	inq_cache;
 	struct hci_conn_hash	conn_hash;
+	struct bdaddr_list	blacklist;
 
 	struct hci_dev_stats	stat;
 
@@ -422,6 +428,9 @@ int hci_get_conn_info(struct hci_dev *hdev, void __user *arg);
 int hci_get_auth_info(struct hci_dev *hdev, void __user *arg);
 int hci_inquiry(void __user *arg);
 
+struct bdaddr_list *hci_blacklist_lookup(struct hci_dev *hdev, bdaddr_t *bdaddr);
+int hci_blacklist_clear(struct hci_dev *hdev);
+
 void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb);
 
 int hci_recv_frame(struct sk_buff *skb);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 4ad2319..053a9e9 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -561,6 +561,7 @@ static int hci_dev_do_close(struct hci_dev *hdev)
 	hci_dev_lock_bh(hdev);
 	inquiry_cache_flush(hdev);
 	hci_conn_hash_flush(hdev);
+	hci_blacklist_clear(hdev);
 	hci_dev_unlock_bh(hdev);
 
 	hci_notify(hdev, HCI_DEV_DOWN);
@@ -922,6 +923,8 @@ int hci_register_dev(struct hci_dev *hdev)
 
 	hci_conn_hash_init(hdev);
 
+	INIT_LIST_HEAD(&hdev->blacklist.list);
+
 	memset(&hdev->stat, 0, sizeof(struct hci_dev_stats));
 
 	atomic_set(&hdev->promisc, 0);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 6c57fc7..1efaf50 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -952,7 +952,7 @@ static inline void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *sk
 
 	mask |= hci_proto_connect_ind(hdev, &ev->bdaddr, ev->link_type);
 
-	if (mask & HCI_LM_ACCEPT) {
+	if ((mask & HCI_LM_ACCEPT) && !hci_blacklist_lookup(hdev, &ev->bdaddr)) {
 		/* Connection accepted */
 		struct inquiry_entry *ie;
 		struct hci_conn *conn;
diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
index 38f08f6..4f170a5 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -165,6 +165,86 @@ static int hci_sock_release(struct socket *sock)
 	return 0;
 }
 
+struct bdaddr_list *hci_blacklist_lookup(struct hci_dev *hdev, bdaddr_t *bdaddr)
+{
+	struct list_head *p;
+	struct bdaddr_list *blacklist = &hdev->blacklist;
+
+	list_for_each(p, &blacklist->list) {
+		struct bdaddr_list *b;
+
+		b = list_entry(p, struct bdaddr_list, list);
+
+		if (bacmp(bdaddr, &b->bdaddr) == 0)
+			return b;
+	}
+
+	return NULL;
+}
+
+static int hci_blacklist_add(struct hci_dev *hdev, void __user *arg)
+{
+	bdaddr_t bdaddr;
+	struct bdaddr_list *entry;
+
+	if (copy_from_user(&bdaddr, arg, sizeof(bdaddr)))
+		return -EFAULT;
+
+	if (bacmp(&bdaddr, BDADDR_ANY) == 0)
+		return -EBADF;
+
+	if (hci_blacklist_lookup(hdev, &bdaddr))
+		return -EEXIST;
+
+	entry = kzalloc(sizeof(struct bdaddr_list), GFP_KERNEL);
+	if (!entry)
+		return -ENOMEM;
+
+	bacpy(&entry->bdaddr, &bdaddr);
+
+	list_add(&entry->list, &hdev->blacklist.list);
+
+	return 0;
+}
+
+int hci_blacklist_clear(struct hci_dev *hdev)
+{
+	struct list_head *p, *n;
+	struct bdaddr_list *blacklist = &hdev->blacklist;
+
+	list_for_each_safe(p, n, &blacklist->list) {
+		struct bdaddr_list *b;
+
+		b = list_entry(p, struct bdaddr_list, list);
+
+		list_del(p);
+		kfree(b);
+	}
+
+	return 0;
+}
+
+static int hci_blacklist_del(struct hci_dev *hdev, void __user *arg)
+{
+	bdaddr_t bdaddr;
+	struct bdaddr_list *entry;
+
+	if (copy_from_user(&bdaddr, arg, sizeof(bdaddr)))
+		return -EFAULT;
+
+	if (bacmp(&bdaddr, BDADDR_ANY) == 0)
+		return hci_blacklist_clear(hdev);
+
+	entry = hci_blacklist_lookup(hdev, &bdaddr);
+	if (!entry)
+		return -ENOENT;
+
+	list_del(&entry->list);
+	kfree(entry);
+
+	return 0;
+}
+
 /* Ioctls that require bound socket */
 static inline int hci_sock_bound_ioctl(struct sock *sk, unsigned int cmd, unsigned long arg)
 {
@@ -194,6 +274,16 @@ static inline int hci_sock_bound_ioctl(struct sock *sk, unsigned int cmd, unsign
 	case HCIGETAUTHINFO:
 		return hci_get_auth_info(hdev, (void __user *) arg);
 
+	case HCIBLOCKADDR:
+		if (!capable(CAP_NET_ADMIN))
+			return -EACCES;
+		return hci_blacklist_add(hdev, (void __user *) arg);
+
+	case HCIUNBLOCKADDR:
+		if (!capable(CAP_NET_ADMIN))
+			return -EACCES;
+		return hci_blacklist_del(hdev, (void __user *) arg);
+
 	default:
 		if (hdev->ioctl)
 			return hdev->ioctl(hdev, cmd, arg);
-- 
1.7.0.4

Re: [PATCH v3] Bluetooth: Add blacklist support for incoming connections

[Gustavo F. Padovan]

Hi Johan,

* johan.hedberg@gmail.com <johan.hedberg@gmail.com> [2010-05-18 13:20:32 +0200]:

> From: Johan Hedberg <johan.hedberg@nokia.com>
> 
> In some circumstances it could be desirable to reject incoming
> connections on the baseband level. This patch adds this feature through
> two new ioctl's: HCIBLOCKADDR and HCIUNBLOCKADDR. Both take a simple
> Bluetooth address as a parameter. BDADDR_ANY can be used with
> HCIUNBLOCKADDR to remove all devices from the blacklist.

Which circumstances?



-- 
Gustavo F. Padovan
http://padovan.org

Re: [PATCH v3] Bluetooth: Add blacklist support for incoming connections

[Johan Hedberg]

Hi Gustavo,

On Wed, May 19, 2010, Gustavo F. Padovan wrote:
> > In some circumstances it could be desirable to reject incoming
> > connections on the baseband level. This patch adds this feature through
> > two new ioctl's: HCIBLOCKADDR and HCIUNBLOCKADDR. Both take a simple
> > Bluetooth address as a parameter. BDADDR_ANY can be used with
> > HCIUNBLOCKADDR to remove all devices from the blacklist.
> 
> Which circumstances?

Whenever you have a badly behaving remote device that keeps on trying to
connect to you (e.g. an OPP spambot or someone trying to actively DoS
you). I gave some more details in my reply to Jaikumar a few days ago on
this mailing list.

Johan

Re: [PATCH v3] Bluetooth: Add blacklist support for incoming connections

[Gustavo F. Padovan]

Hi Johan,

* Johan Hedberg <johan.hedberg@nokia.com> [2010-05-19 10:22:05 +0200]:

> Hi Gustavo,
> 
> On Wed, May 19, 2010, Gustavo F. Padovan wrote:
> > > In some circumstances it could be desirable to reject incoming
> > > connections on the baseband level. This patch adds this feature through
> > > two new ioctl's: HCIBLOCKADDR and HCIUNBLOCKADDR. Both take a simple
> > > Bluetooth address as a parameter. BDADDR_ANY can be used with
> > > HCIUNBLOCKADDR to remove all devices from the blacklist.
> > 
> > Which circumstances?
> 
> Whenever you have a badly behaving remote device that keeps on trying to
> connect to you (e.g. an OPP spambot or someone trying to actively DoS
> you). I gave some more details in my reply to Jaikumar a few days ago on
> this mailing list.

Nice. Patch looks ok to me by the way. ;)

-- 
Gustavo F. Padovan
http://padovan.org

Re: [PATCH v3] Bluetooth: Add blacklist support for incoming connections

[Luiz Augusto von Dentz]

Hi Gustavo,

On Wed, May 19, 2010 at 10:48 AM, Gustavo F. Padovan
<gustavo@padovan.org> wrote:
> Hi Johan,
>
> * Johan Hedberg <johan.hedberg@nokia.com> [2010-05-19 10:22:05 +0200]:
>
>> Hi Gustavo,
>>
>> On Wed, May 19, 2010, Gustavo F. Padovan wrote:
>> > > In some circumstances it could be desirable to reject incoming
>> > > connections on the baseband level. This patch adds this feature through
>> > > two new ioctl's: HCIBLOCKADDR and HCIUNBLOCKADDR. Both take a simple
>> > > Bluetooth address as a parameter. BDADDR_ANY can be used with
>> > > HCIUNBLOCKADDR to remove all devices from the blacklist.
>> >
>> > Which circumstances?
>>
>> Whenever you have a badly behaving remote device that keeps on trying to
>> connect to you (e.g. an OPP spambot or someone trying to actively DoS
>> you). I gave some more details in my reply to Jaikumar a few days ago on
>> this mailing list.
>
> Nice. Patch looks ok to me by the way. ;)

Its also very, almost a must, convenient when doing interoperability
testing in some events like upf where we have a lot (annoying) devices
trying to reconnect to us while we are testing with other devices.

@ Johan, Blocked does supersedes Trusted right? Maybe we should rework
those properties (deprecate them?) to something like Authorization
which takes a string where lets say can assume these values: "deny"
("block"), "ask" or "allow".

-- 
Luiz Augusto von Dentz
Computer Engineer

Re: [PATCH v3] Bluetooth: Add blacklist support for incoming connections

[Johan Hedberg]

Hi Luiz,

On Wed, May 19, 2010, Luiz Augusto von Dentz wrote:
> @ Johan, Blocked does supersedes Trusted right?

Right. What also happens when you set Blocked to False is that all the
device drivers are unloaded for that device. When Blocked goes back to
true the drivers get probed again.

> Maybe we should rework those properties (deprecate them?) to something
> like Authorization which takes a string where lets say can assume
> these values: "deny" ("block"), "ask" or "allow".

Sounds like an interesting idea, but I'm not completely convinced about
it since the properties act on different layers. Currently authorization
is on the profile layer and is completely optional for a profile
implementation (i.e. it needs to call btd_request_authorization
(bluetoothd plugin) or RequestAuthorization (D-Bus) whereas there's no
way to skip the blocked check in the kernel. I.e. if we'd have a unified
property, the value "blocked" would be unconditional but "ask" wouldn't
always mean "ask": it would only happen once the connection reaches the
profile level and only for the profiles that actually enforce
authorization.

Johan

Re: [PATCH v3] Bluetooth: Add blacklist support for incoming connections

[Johan Hedberg]

On Wed, May 19, 2010, Johan Hedberg wrote:
> What also happens when you set Blocked to False is that all the device
> drivers are unloaded for that device. When Blocked goes back to true
> the drivers get probed again.

I mixed up True/False there, but you probably realized that :)

Johan