linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] bluetooth: fix not setting security level when creating a rfcomm session
@ 2010-08-03  7:26 Luiz Augusto von Dentz
  2010-08-03  7:59 ` Marcel Holtmann
  0 siblings, 1 reply; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2010-08-03  7:26 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>

This cause 'No Bonding' to be used if userspace has not yet been paired
with remote device since the l2cap socket used to create the rfcomm
session does not have any security level set.

Signed-off-by: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
---
 net/bluetooth/rfcomm/core.c |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index 7dca91b..08f5757 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -79,7 +79,10 @@ static void rfcomm_make_uih(struct sk_buff *skb, u8 addr);
 
 static void rfcomm_process_connect(struct rfcomm_session *s);
 
-static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src, bdaddr_t *dst, int *err);
+static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src,
+							bdaddr_t *dst,
+							u8 sec_level,
+							int *err);
 static struct rfcomm_session *rfcomm_session_get(bdaddr_t *src, bdaddr_t *dst);
 static void rfcomm_session_del(struct rfcomm_session *s);
 
@@ -402,7 +405,7 @@ static int __rfcomm_dlc_open(struct rfcomm_dlc *d, bdaddr_t *src, bdaddr_t *dst,
 
 	s = rfcomm_session_get(src, dst);
 	if (!s) {
-		s = rfcomm_session_create(src, dst, &err);
+		s = rfcomm_session_create(src, dst, d->sec_level, &err);
 		if (!s)
 			return err;
 	}
@@ -680,7 +683,10 @@ static void rfcomm_session_close(struct rfcomm_session *s, int err)
 	rfcomm_session_put(s);
 }
 
-static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src, bdaddr_t *dst, int *err)
+static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src,
+							bdaddr_t *dst,
+							u8 sec_level,
+							int *err)
 {
 	struct rfcomm_session *s = NULL;
 	struct sockaddr_l2 addr;
@@ -705,6 +711,7 @@ static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src, bdaddr_t *dst
 	sk = sock->sk;
 	lock_sock(sk);
 	l2cap_pi(sk)->imtu = l2cap_mtu;
+	l2cap_pi(sk)->sec_level = sec_level;
 	if (l2cap_ertm)
 		l2cap_pi(sk)->mode = L2CAP_MODE_ERTM;
 	release_sock(sk);
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] bluetooth: fix not setting security level when creating a rfcomm session
  2010-08-03  7:26 Luiz Augusto von Dentz
@ 2010-08-03  7:59 ` Marcel Holtmann
  2010-08-03  8:27   ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 6+ messages in thread
From: Marcel Holtmann @ 2010-08-03  7:59 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

Hi Luiz,

> This cause 'No Bonding' to be used if userspace has not yet been paired
> with remote device since the l2cap socket used to create the rfcomm
> session does not have any security level set.
> 
> Signed-off-by: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
> ---
>  net/bluetooth/rfcomm/core.c |   13 ++++++++++---
>  1 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
> index 7dca91b..08f5757 100644
> --- a/net/bluetooth/rfcomm/core.c
> +++ b/net/bluetooth/rfcomm/core.c
> @@ -79,7 +79,10 @@ static void rfcomm_make_uih(struct sk_buff *skb, u8 addr);
>  
>  static void rfcomm_process_connect(struct rfcomm_session *s);
>  
> -static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src, bdaddr_t *dst, int *err);
> +static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src,
> +							bdaddr_t *dst,
> +							u8 sec_level,
> +							int *err);
>  static struct rfcomm_session *rfcomm_session_get(bdaddr_t *src, bdaddr_t *dst);
>  static void rfcomm_session_del(struct rfcomm_session *s);
>  
> @@ -402,7 +405,7 @@ static int __rfcomm_dlc_open(struct rfcomm_dlc *d, bdaddr_t *src, bdaddr_t *dst,
>  
>  	s = rfcomm_session_get(src, dst);
>  	if (!s) {
> -		s = rfcomm_session_create(src, dst, &err);
> +		s = rfcomm_session_create(src, dst, d->sec_level, &err);
>  		if (!s)
>  			return err;
>  	}
> @@ -680,7 +683,10 @@ static void rfcomm_session_close(struct rfcomm_session *s, int err)
>  	rfcomm_session_put(s);
>  }
>  
> -static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src, bdaddr_t *dst, int *err)
> +static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src,
> +							bdaddr_t *dst,
> +							u8 sec_level,
> +							int *err)
>  {
>  	struct rfcomm_session *s = NULL;
>  	struct sockaddr_l2 addr;
> @@ -705,6 +711,7 @@ static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src, bdaddr_t *dst
>  	sk = sock->sk;
>  	lock_sock(sk);
>  	l2cap_pi(sk)->imtu = l2cap_mtu;
> +	l2cap_pi(sk)->sec_level = sec_level;
>  	if (l2cap_ertm)
>  		l2cap_pi(sk)->mode = L2CAP_MODE_ERTM;
>  	release_sock(sk);

I see what you are trying to do here, but why is this needed? Shouldn't
it be already correctly set for PSM 3?

Regards

Marcel



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] bluetooth: fix not setting security level when creating a rfcomm session
  2010-08-03  7:59 ` Marcel Holtmann
@ 2010-08-03  8:27   ` Luiz Augusto von Dentz
  0 siblings, 0 replies; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2010-08-03  8:27 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth

Hi Marcel,

On Tue, Aug 3, 2010 at 10:59 AM, Marcel Holtmann <marcel@holtmann.org> wrot=
e:
> Hi Luiz,
>
>> This cause 'No Bonding' to be used if userspace has not yet been paired
>> with remote device since the l2cap socket used to create the rfcomm
>> session does not have any security level set.
>>
>> Signed-off-by: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
>> ---
>> =A0net/bluetooth/rfcomm/core.c | =A0 13 ++++++++++---
>> =A01 files changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
>> index 7dca91b..08f5757 100644
>> --- a/net/bluetooth/rfcomm/core.c
>> +++ b/net/bluetooth/rfcomm/core.c
>> @@ -79,7 +79,10 @@ static void rfcomm_make_uih(struct sk_buff *skb, u8 a=
ddr);
>>
>> =A0static void rfcomm_process_connect(struct rfcomm_session *s);
>>
>> -static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src, bdad=
dr_t *dst, int *err);
>> +static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 bdaddr_t *dst,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 u8 sec_level,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 int *err);
>> =A0static struct rfcomm_session *rfcomm_session_get(bdaddr_t *src, bdadd=
r_t *dst);
>> =A0static void rfcomm_session_del(struct rfcomm_session *s);
>>
>> @@ -402,7 +405,7 @@ static int __rfcomm_dlc_open(struct rfcomm_dlc *d, b=
daddr_t *src, bdaddr_t *dst,
>>
>> =A0 =A0 =A0 s =3D rfcomm_session_get(src, dst);
>> =A0 =A0 =A0 if (!s) {
>> - =A0 =A0 =A0 =A0 =A0 =A0 s =3D rfcomm_session_create(src, dst, &err);
>> + =A0 =A0 =A0 =A0 =A0 =A0 s =3D rfcomm_session_create(src, dst, d->sec_l=
evel, &err);
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!s)
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return err;
>> =A0 =A0 =A0 }
>> @@ -680,7 +683,10 @@ static void rfcomm_session_close(struct rfcomm_sess=
ion *s, int err)
>> =A0 =A0 =A0 rfcomm_session_put(s);
>> =A0}
>>
>> -static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src, bdad=
dr_t *dst, int *err)
>> +static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 bdaddr_t *dst,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 u8 sec_level,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 int *err)
>> =A0{
>> =A0 =A0 =A0 struct rfcomm_session *s =3D NULL;
>> =A0 =A0 =A0 struct sockaddr_l2 addr;
>> @@ -705,6 +711,7 @@ static struct rfcomm_session *rfcomm_session_create(=
bdaddr_t *src, bdaddr_t *dst
>> =A0 =A0 =A0 sk =3D sock->sk;
>> =A0 =A0 =A0 lock_sock(sk);
>> =A0 =A0 =A0 l2cap_pi(sk)->imtu =3D l2cap_mtu;
>> + =A0 =A0 l2cap_pi(sk)->sec_level =3D sec_level;
>> =A0 =A0 =A0 if (l2cap_ertm)
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 l2cap_pi(sk)->mode =3D L2CAP_MODE_ERTM;
>> =A0 =A0 =A0 release_sock(sk);
>
> I see what you are trying to do here, but why is this needed? Shouldn't
> it be already correctly set for PSM 3?

I couldn't find any special check for PSM 3, but I guess it would make
sense to make sure it always set the security level from dlc which
triggered the session, doesn't it? As it is now it first triggers No
Bonding and then when we actually connect the dlc it might have a
different sec_level which might triggers another round of io
capability negotiation, not only this doesn't work always, because
there is a bug in bluetoothd that won't invalidate/remove stored link
keys once we got a new one that is not supposed to be stored (No
Bonding), but IMO much simpler too. Maybe we can do as SDP which seems
to do the check in l2cap.c:l2cap_do_connect, but for rfcomm we
actually have to look in the dlcs to check witch security level should
be use.

--=20
Luiz Augusto von Dentz
Computer Engineer

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] bluetooth: fix not setting security level when creating a rfcomm session
@ 2010-08-19 11:06 Luiz Augusto von Dentz
  2010-08-20 10:55 ` Ville Tervo
  2010-10-28 13:07 ` Gustavo F. Padovan
  0 siblings, 2 replies; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2010-08-19 11:06 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: ville.tervo

From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>

This cause 'No Bonding' to be used if userspace has not yet been paired
with remote device since the l2cap socket used to create the rfcomm
session does not have any security level set.

Signed-off-by: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
---
 net/bluetooth/rfcomm/core.c |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index 7dca91b..08f5757 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -79,7 +79,10 @@ static void rfcomm_make_uih(struct sk_buff *skb, u8 addr);
 
 static void rfcomm_process_connect(struct rfcomm_session *s);
 
-static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src, bdaddr_t *dst, int *err);
+static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src,
+							bdaddr_t *dst,
+							u8 sec_level,
+							int *err);
 static struct rfcomm_session *rfcomm_session_get(bdaddr_t *src, bdaddr_t *dst);
 static void rfcomm_session_del(struct rfcomm_session *s);
 
@@ -402,7 +405,7 @@ static int __rfcomm_dlc_open(struct rfcomm_dlc *d, bdaddr_t *src, bdaddr_t *dst,
 
 	s = rfcomm_session_get(src, dst);
 	if (!s) {
-		s = rfcomm_session_create(src, dst, &err);
+		s = rfcomm_session_create(src, dst, d->sec_level, &err);
 		if (!s)
 			return err;
 	}
@@ -680,7 +683,10 @@ static void rfcomm_session_close(struct rfcomm_session *s, int err)
 	rfcomm_session_put(s);
 }
 
-static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src, bdaddr_t *dst, int *err)
+static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src,
+							bdaddr_t *dst,
+							u8 sec_level,
+							int *err)
 {
 	struct rfcomm_session *s = NULL;
 	struct sockaddr_l2 addr;
@@ -705,6 +711,7 @@ static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src, bdaddr_t *dst
 	sk = sock->sk;
 	lock_sock(sk);
 	l2cap_pi(sk)->imtu = l2cap_mtu;
+	l2cap_pi(sk)->sec_level = sec_level;
 	if (l2cap_ertm)
 		l2cap_pi(sk)->mode = L2CAP_MODE_ERTM;
 	release_sock(sk);
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] bluetooth: fix not setting security level when creating a rfcomm session
  2010-08-19 11:06 [PATCH] bluetooth: fix not setting security level when creating a rfcomm session Luiz Augusto von Dentz
@ 2010-08-20 10:55 ` Ville Tervo
  2010-10-28 13:07 ` Gustavo F. Padovan
  1 sibling, 0 replies; 6+ messages in thread
From: Ville Tervo @ 2010-08-20 10:55 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org

On Thu, Aug 19, 2010 at 01:06:10PM +0200, Luiz Augusto von Dentz wrote:
> From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
> 
> This cause 'No Bonding' to be used if userspace has not yet been paired
> with remote device since the l2cap socket used to create the rfcomm
> session does not have any security level set.

Looks good.

> Signed-off-by: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>

Acked-by: Ville Tervo <ville.tervo@nokia.com>

-- 
Ville

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] bluetooth: fix not setting security level when creating a rfcomm session
  2010-08-19 11:06 [PATCH] bluetooth: fix not setting security level when creating a rfcomm session Luiz Augusto von Dentz
  2010-08-20 10:55 ` Ville Tervo
@ 2010-10-28 13:07 ` Gustavo F. Padovan
  1 sibling, 0 replies; 6+ messages in thread
From: Gustavo F. Padovan @ 2010-10-28 13:07 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth, ville.tervo

Hi Luiz,

* Luiz Augusto von Dentz <luiz.dentz@gmail.com> [2010-08-19 14:06:10 +0300]:

> From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
> 
> This cause 'No Bonding' to be used if userspace has not yet been paired
> with remote device since the l2cap socket used to create the rfcomm
> session does not have any security level set.
> 
> Signed-off-by: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
> ---
>  net/bluetooth/rfcomm/core.c |   13 ++++++++++---
>  1 files changed, 10 insertions(+), 3 deletions(-)

Patch has been applied to the bluetooth-2.6 tree. Thanks.

-- 
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2010-10-28 13:07 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-08-19 11:06 [PATCH] bluetooth: fix not setting security level when creating a rfcomm session Luiz Augusto von Dentz
2010-08-20 10:55 ` Ville Tervo
2010-10-28 13:07 ` Gustavo F. Padovan
  -- strict thread matches above, loose matches on Subject: below --
2010-08-03  7:26 Luiz Augusto von Dentz
2010-08-03  7:59 ` Marcel Holtmann
2010-08-03  8:27   ` Luiz Augusto von Dentz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).