2009-01-10 12:33:47

by Ville Tervo

[permalink] [raw]
Subject: [RFC] Some kernel changes

Hi,


Here is set of unclean patches for discussion. These are still untested
and contain issues, but I would like to know if we should continue with
these or change something radically. Also commit messages are somewhat
misleading in some places.

Basically idea is to implement connection rejection support for l2cap
and rfcomm sockets. IOW possibility to check initiator bd-address and
ask for authorization before accepting connection.

Other goal is to fix encryption and authentication levels to fulfill 2.1
requirements.

--
Ville


Attachments:
0001-Support-for-deferring-rfcomm-connection.patch (10.95 kB)
0002-bluetooth-Introduce-security-levels.patch (7.57 kB)
0004-bluetooth-l2cap-seclevel.patch (5.69 kB)
0005-Compilation-fix.patch (1.05 kB)
0006--BLUETOOTH-Pause-rfcomm-TX-if-encryption-is-dropped.patch (2.16 kB)
0007--BLUETOOTH-Preliminary-l2cap-connection-rejection-s.patch (7.64 kB)
Download all attachments

2009-01-28 10:35:50

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [RFC] Some kernel changes

Hi Jaikumar,

> >> >> >>>>> why do you wanna set AUTH_PENDING again. That is not needed we only
> >> >> >>>>> wanna know once encryption comes back on. If no in time, then we just
> >> >> >>>>> disconnect the link. That simple.
> >> >> >>>>>
> >> >> >>>>>> b) I also see that we are not clearing the timer and hence after
> >> >> >>>>>> RFCOMM_AUTH_TIMEOUT period expires we bring down the RFCOMM connection
> >> >> >>>>>> even though the encryption has been established.
> >> >> >>>>>
> >> >> >>>>> Good catch. Fixed it.
> >> >> >>>>
> >> >> >>>> Cool. Can you upload it to bluetooth-testing git ? I saw a related
> >> >> >>>> problem while looking at the timer issue and wanted to see if the fix
> >> >> >>>> takes care of it.
> >> >> >>>
> >> >> >>> I am in the process in doing so, but there are other important things
> >> >> >>> that need to be done first.
> >> >> >>
> >> >> >> Sorry to push you on this, is this patch been uploaded ?
> >> >> >
> >> >> > since a few days now. Check the bluetooth-testing.git and it is even rebased
> >> >> > against 2.6.29-rc2.
> >> >> >
> >> >> I applied the patches and I see that we actually don't pause the
> >> >> RFCOMM tx traffic because we don't check on
> >> >> the RFCOMM_SEC_PENDING flag when sending the tx packets in rfcomm_process_dlcs.
> >> >> I verified this using the "attest" command - we keep sending RFCOMM
> >> >> traffic even though encryption is off.
> >> >>
> >> >> Please look at the patch attached.
> >> >>
> >> >> Also should we drop the "rx" packets too when the encryption is dropped ?
> >> >
> >> > can you get me a version with a proper commit message. Just the subject
> >> > is not gonna be good enough.
> >> >
> >>
> >> Sorry, its attached. Also like mentioned above the patch checks only
> >> while sending tx packets ?
> >> Should we drop "rx" packets also ?
> >
> > patch looks fine to me, but you need to fix/revert the comment change ;)
> >
> > diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
> > index ae2ecaa..73ffc0c 100644
> > --- a/net/bluetooth/rfcomm/core.c
> > +++ b/net/bluetooth/rfcomm/core.c
> > @@ -1,4 +1,4 @@
> > -/*
> > +f*
> > RFCOMM implementation for Linux Bluetooth stack (BlueZ).
> > Copyright (C) 2002 Maxim Krasnyansky <[email protected]>
> > Copyright (C) 2002 Marcel Holtmann <[email protected]>
> >
> > And I like to have a little bit more detailed commit message please.
> >
> Sure I will resubmit
> Also, should we drop the "rx" packets ?

no, we can't do that since RFCOMM support to be a reliable stream.

Regards

Marcel



2009-01-28 10:18:38

by Jaikumar Ganesh

[permalink] [raw]
Subject: Re: [RFC] Some kernel changes

Hi Marcel,

On Wed, Jan 28, 2009 at 1:59 AM, Marcel Holtmann <[email protected]> wrote:
> Hi Jaikumar,
>
>> >> >>>>> why do you wanna set AUTH_PENDING again. That is not needed we only
>> >> >>>>> wanna know once encryption comes back on. If no in time, then we just
>> >> >>>>> disconnect the link. That simple.
>> >> >>>>>
>> >> >>>>>> b) I also see that we are not clearing the timer and hence after
>> >> >>>>>> RFCOMM_AUTH_TIMEOUT period expires we bring down the RFCOMM connection
>> >> >>>>>> even though the encryption has been established.
>> >> >>>>>
>> >> >>>>> Good catch. Fixed it.
>> >> >>>>
>> >> >>>> Cool. Can you upload it to bluetooth-testing git ? I saw a related
>> >> >>>> problem while looking at the timer issue and wanted to see if the fix
>> >> >>>> takes care of it.
>> >> >>>
>> >> >>> I am in the process in doing so, but there are other important things
>> >> >>> that need to be done first.
>> >> >>
>> >> >> Sorry to push you on this, is this patch been uploaded ?
>> >> >
>> >> > since a few days now. Check the bluetooth-testing.git and it is even rebased
>> >> > against 2.6.29-rc2.
>> >> >
>> >> I applied the patches and I see that we actually don't pause the
>> >> RFCOMM tx traffic because we don't check on
>> >> the RFCOMM_SEC_PENDING flag when sending the tx packets in rfcomm_process_dlcs.
>> >> I verified this using the "attest" command - we keep sending RFCOMM
>> >> traffic even though encryption is off.
>> >>
>> >> Please look at the patch attached.
>> >>
>> >> Also should we drop the "rx" packets too when the encryption is dropped ?
>> >
>> > can you get me a version with a proper commit message. Just the subject
>> > is not gonna be good enough.
>> >
>>
>> Sorry, its attached. Also like mentioned above the patch checks only
>> while sending tx packets ?
>> Should we drop "rx" packets also ?
>
> patch looks fine to me, but you need to fix/revert the comment change ;)
>
> diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
> index ae2ecaa..73ffc0c 100644
> --- a/net/bluetooth/rfcomm/core.c
> +++ b/net/bluetooth/rfcomm/core.c
> @@ -1,4 +1,4 @@
> -/*
> +f*
> RFCOMM implementation for Linux Bluetooth stack (BlueZ).
> Copyright (C) 2002 Maxim Krasnyansky <[email protected]>
> Copyright (C) 2002 Marcel Holtmann <[email protected]>
>
> And I like to have a little bit more detailed commit message please.
>
Sure I will resubmit
Also, should we drop the "rx" packets ?

Thanks
Jaikumar
> Regards
>
> Marcel
>

>
>

2009-01-28 09:59:35

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [RFC] Some kernel changes

Hi Jaikumar,

> >> >>>>> why do you wanna set AUTH_PENDING again. That is not needed we only
> >> >>>>> wanna know once encryption comes back on. If no in time, then we just
> >> >>>>> disconnect the link. That simple.
> >> >>>>>
> >> >>>>>> b) I also see that we are not clearing the timer and hence after
> >> >>>>>> RFCOMM_AUTH_TIMEOUT period expires we bring down the RFCOMM connection
> >> >>>>>> even though the encryption has been established.
> >> >>>>>
> >> >>>>> Good catch. Fixed it.
> >> >>>>
> >> >>>> Cool. Can you upload it to bluetooth-testing git ? I saw a related
> >> >>>> problem while looking at the timer issue and wanted to see if the fix
> >> >>>> takes care of it.
> >> >>>
> >> >>> I am in the process in doing so, but there are other important things
> >> >>> that need to be done first.
> >> >>
> >> >> Sorry to push you on this, is this patch been uploaded ?
> >> >
> >> > since a few days now. Check the bluetooth-testing.git and it is even rebased
> >> > against 2.6.29-rc2.
> >> >
> >> I applied the patches and I see that we actually don't pause the
> >> RFCOMM tx traffic because we don't check on
> >> the RFCOMM_SEC_PENDING flag when sending the tx packets in rfcomm_process_dlcs.
> >> I verified this using the "attest" command - we keep sending RFCOMM
> >> traffic even though encryption is off.
> >>
> >> Please look at the patch attached.
> >>
> >> Also should we drop the "rx" packets too when the encryption is dropped ?
> >
> > can you get me a version with a proper commit message. Just the subject
> > is not gonna be good enough.
> >
>
> Sorry, its attached. Also like mentioned above the patch checks only
> while sending tx packets ?
> Should we drop "rx" packets also ?

patch looks fine to me, but you need to fix/revert the comment change ;)

diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index ae2ecaa..73ffc0c 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -1,4 +1,4 @@
-/*
+f*
RFCOMM implementation for Linux Bluetooth stack (BlueZ).
Copyright (C) 2002 Maxim Krasnyansky <[email protected]>
Copyright (C) 2002 Marcel Holtmann <[email protected]>

And I like to have a little bit more detailed commit message please.

Regards

Marcel



2009-01-28 09:27:37

by Jaikumar Ganesh

[permalink] [raw]
Subject: Re: [RFC] Some kernel changes

Hi Marcel,

On Wed, Jan 21, 2009 at 9:46 AM, jaikumar Ganesh <[email protected]> wrote:
> Hi Marcel,
>
> On Wed, Jan 21, 2009 at 3:19 AM, Marcel Holtmann <[email protected]> wrote:
>> Hi Jaikumar,
>>
>>> >>>>> why do you wanna set AUTH_PENDING again. That is not needed we only
>>> >>>>> wanna know once encryption comes back on. If no in time, then we just
>>> >>>>> disconnect the link. That simple.
>>> >>>>>
>>> >>>>>> b) I also see that we are not clearing the timer and hence after
>>> >>>>>> RFCOMM_AUTH_TIMEOUT period expires we bring down the RFCOMM connection
>>> >>>>>> even though the encryption has been established.
>>> >>>>>
>>> >>>>> Good catch. Fixed it.
>>> >>>>
>>> >>>> Cool. Can you upload it to bluetooth-testing git ? I saw a related
>>> >>>> problem while looking at the timer issue and wanted to see if the fix
>>> >>>> takes care of it.
>>> >>>
>>> >>> I am in the process in doing so, but there are other important things
>>> >>> that need to be done first.
>>> >>
>>> >> Sorry to push you on this, is this patch been uploaded ?
>>> >
>>> > since a few days now. Check the bluetooth-testing.git and it is even rebased
>>> > against 2.6.29-rc2.
>>> >
>>> I applied the patches and I see that we actually don't pause the
>>> RFCOMM tx traffic because we don't check on
>>> the RFCOMM_SEC_PENDING flag when sending the tx packets in rfcomm_process_dlcs.
>>> I verified this using the "attest" command - we keep sending RFCOMM
>>> traffic even though encryption is off.
>>>
>>> Please look at the patch attached.
>>>
>>> Also should we drop the "rx" packets too when the encryption is dropped ?
>>
>> can you get me a version with a proper commit message. Just the subject
>> is not gonna be good enough.
>>
>
> Sorry, its attached. Also like mentioned above the patch checks only
> while sending tx packets ?
> Should we drop "rx" packets also ?
>
>> Regards
>>
>> Marcel
>>
>>
>>
>
Hope you got a chance to look at the above patch and comment
regarding rx packets.

-Thanks
Jaikumar

2009-01-21 17:46:09

by Jaikumar Ganesh

[permalink] [raw]
Subject: Re: [RFC] Some kernel changes

Hi Marcel,

On Wed, Jan 21, 2009 at 3:19 AM, Marcel Holtmann <[email protected]> wrote:
> Hi Jaikumar,
>
>> >>>>> why do you wanna set AUTH_PENDING again. That is not needed we only
>> >>>>> wanna know once encryption comes back on. If no in time, then we just
>> >>>>> disconnect the link. That simple.
>> >>>>>
>> >>>>>> b) I also see that we are not clearing the timer and hence after
>> >>>>>> RFCOMM_AUTH_TIMEOUT period expires we bring down the RFCOMM connection
>> >>>>>> even though the encryption has been established.
>> >>>>>
>> >>>>> Good catch. Fixed it.
>> >>>>
>> >>>> Cool. Can you upload it to bluetooth-testing git ? I saw a related
>> >>>> problem while looking at the timer issue and wanted to see if the fix
>> >>>> takes care of it.
>> >>>
>> >>> I am in the process in doing so, but there are other important things
>> >>> that need to be done first.
>> >>
>> >> Sorry to push you on this, is this patch been uploaded ?
>> >
>> > since a few days now. Check the bluetooth-testing.git and it is even rebased
>> > against 2.6.29-rc2.
>> >
>> I applied the patches and I see that we actually don't pause the
>> RFCOMM tx traffic because we don't check on
>> the RFCOMM_SEC_PENDING flag when sending the tx packets in rfcomm_process_dlcs.
>> I verified this using the "attest" command - we keep sending RFCOMM
>> traffic even though encryption is off.
>>
>> Please look at the patch attached.
>>
>> Also should we drop the "rx" packets too when the encryption is dropped ?
>
> can you get me a version with a proper commit message. Just the subject
> is not gonna be good enough.
>

Sorry, its attached. Also like mentioned above the patch checks only
while sending tx packets ?
Should we drop "rx" packets also ?

> Regards
>
> Marcel
>
>
>


Attachments:
0001-Bluetooth-When-encyption-is-dropped-do-not-send-RF.patch (1.10 kB)

2009-01-21 11:19:12

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [RFC] Some kernel changes

Hi Jaikumar,

> >>>>> why do you wanna set AUTH_PENDING again. That is not needed we only
> >>>>> wanna know once encryption comes back on. If no in time, then we just
> >>>>> disconnect the link. That simple.
> >>>>>
> >>>>>> b) I also see that we are not clearing the timer and hence after
> >>>>>> RFCOMM_AUTH_TIMEOUT period expires we bring down the RFCOMM connection
> >>>>>> even though the encryption has been established.
> >>>>>
> >>>>> Good catch. Fixed it.
> >>>>
> >>>> Cool. Can you upload it to bluetooth-testing git ? I saw a related
> >>>> problem while looking at the timer issue and wanted to see if the fix
> >>>> takes care of it.
> >>>
> >>> I am in the process in doing so, but there are other important things
> >>> that need to be done first.
> >>
> >> Sorry to push you on this, is this patch been uploaded ?
> >
> > since a few days now. Check the bluetooth-testing.git and it is even rebased
> > against 2.6.29-rc2.
> >
> I applied the patches and I see that we actually don't pause the
> RFCOMM tx traffic because we don't check on
> the RFCOMM_SEC_PENDING flag when sending the tx packets in rfcomm_process_dlcs.
> I verified this using the "attest" command - we keep sending RFCOMM
> traffic even though encryption is off.
>
> Please look at the patch attached.
>
> Also should we drop the "rx" packets too when the encryption is dropped ?

can you get me a version with a proper commit message. Just the subject
is not gonna be good enough.

Regards

Marcel



2009-01-21 00:28:46

by Jaikumar Ganesh

[permalink] [raw]
Subject: Re: [RFC] Some kernel changes

Hi Marcel

On Tue, Jan 20, 2009 at 12:02 PM, Marcel Holtmann <[email protected]> wrote:
> Hi,
>
>>>>> why do you wanna set AUTH_PENDING again. That is not needed we only
>>>>> wanna know once encryption comes back on. If no in time, then we just
>>>>> disconnect the link. That simple.
>>>>>
>>>>>> b) I also see that we are not clearing the timer and hence after
>>>>>> RFCOMM_AUTH_TIMEOUT period expires we bring down the RFCOMM connection
>>>>>> even though the encryption has been established.
>>>>>
>>>>> Good catch. Fixed it.
>>>>
>>>> Cool. Can you upload it to bluetooth-testing git ? I saw a related
>>>> problem while looking at the timer issue and wanted to see if the fix
>>>> takes care of it.
>>>
>>> I am in the process in doing so, but there are other important things
>>> that need to be done first.
>>
>> Sorry to push you on this, is this patch been uploaded ?
>
> since a few days now. Check the bluetooth-testing.git and it is even rebased
> against 2.6.29-rc2.
>
> Regards
>
> Marcel
>
>
I applied the patches and I see that we actually don't pause the
RFCOMM tx traffic because we don't check on
the RFCOMM_SEC_PENDING flag when sending the tx packets in rfcomm_process_dlcs.
I verified this using the "attest" command - we keep sending RFCOMM
traffic even though encryption is off.

Please look at the patch attached.

Also should we drop the "rx" packets too when the encryption is dropped ?

Thanks
Jaikumar


Attachments:
0001-Bluetooth-When-encryption-is-off-do-not-send-RFCOM.patch (804.00 B)

2009-01-20 20:02:01

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [RFC] Some kernel changes

Hi,

>>>> why do you wanna set AUTH_PENDING again. That is not needed we only
>>>> wanna know once encryption comes back on. If no in time, then we
>>>> just
>>>> disconnect the link. That simple.
>>>>
>>>>> b) I also see that we are not clearing the timer and hence after
>>>>> RFCOMM_AUTH_TIMEOUT period expires we bring down the RFCOMM
>>>>> connection
>>>>> even though the encryption has been established.
>>>>
>>>> Good catch. Fixed it.
>>>
>>> Cool. Can you upload it to bluetooth-testing git ? I saw a related
>>> problem while looking at the timer issue and wanted to see if the
>>> fix
>>> takes care of it.
>>
>> I am in the process in doing so, but there are other important things
>> that need to be done first.
>
> Sorry to push you on this, is this patch been uploaded ?

since a few days now. Check the bluetooth-testing.git and it is even
rebased against 2.6.29-rc2.

Regards

Marcel


2009-01-20 18:54:24

by Jaikumar Ganesh

[permalink] [raw]
Subject: Re: [RFC] Some kernel changes

Hi Marcel,

On Fri, Jan 16, 2009 at 1:47 AM, Marcel Holtmann <[email protected]> wrote:
> Hi,
>
>> > why do you wanna set AUTH_PENDING again. That is not needed we only
>> > wanna know once encryption comes back on. If no in time, then we just
>> > disconnect the link. That simple.
>> >
>> >> b) I also see that we are not clearing the timer and hence after
>> >> RFCOMM_AUTH_TIMEOUT period expires we bring down the RFCOMM connection
>> >> even though the encryption has been established.
>> >
>> > Good catch. Fixed it.
>>
>> Cool. Can you upload it to bluetooth-testing git ? I saw a related
>> problem while looking at the timer issue and wanted to see if the fix
>> takes care of it.
>
> I am in the process in doing so, but there are other important things
> that need to be done first.

Sorry to push you on this, is this patch been uploaded ?


>> The timer thats started in rfcomm_security_cfm (for the drop in
>> encryption) does get cleared in rfcomm_process_dlcs
>> but then we have some data to send. So we send the RFCOMM PN packet
>> just after encryption is re-established.
>> This results in a new timer being started in rfcomm_process_dlcs and
>> we don't get the UA packet to clear this timer and hence when
>> the timer expires, the connection is dropped.
>
> I think you are confusing the work-flow here. The initial stage here is
> that we get security_cfm when establishing the connection. Then we will
> either reject or accept the link depending if encryption is enabled or
> not. In case of dropping encryption during an existing connection we
> don't need to send PN again. We do have to be in BT_CONNECTED state to
> have the encryption droping check to kick in. The SEC_PENDING only
> ensures that we stay encrypted. It has nothing to with the auth step.
> Also we combine auth+encrypt since one without the other makes no sense
> at all. So the initial call of security_cfm will always include auth and
> encrypt enabled.
>
> Regards
>
> Marcel
>


Regards
Jaikumar

2009-01-16 09:47:24

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [RFC] Some kernel changes

Hi,

> > why do you wanna set AUTH_PENDING again. That is not needed we only
> > wanna know once encryption comes back on. If no in time, then we just
> > disconnect the link. That simple.
> >
> >> b) I also see that we are not clearing the timer and hence after
> >> RFCOMM_AUTH_TIMEOUT period expires we bring down the RFCOMM connection
> >> even though the encryption has been established.
> >
> > Good catch. Fixed it.
>
> Cool. Can you upload it to bluetooth-testing git ? I saw a related
> problem while looking at the timer issue and wanted to see if the fix
> takes care of it.

I am in the process in doing so, but there are other important things
that need to be done first.

> The timer thats started in rfcomm_security_cfm (for the drop in
> encryption) does get cleared in rfcomm_process_dlcs
> but then we have some data to send. So we send the RFCOMM PN packet
> just after encryption is re-established.
> This results in a new timer being started in rfcomm_process_dlcs and
> we don't get the UA packet to clear this timer and hence when
> the timer expires, the connection is dropped.

I think you are confusing the work-flow here. The initial stage here is
that we get security_cfm when establishing the connection. Then we will
either reject or accept the link depending if encryption is enabled or
not. In case of dropping encryption during an existing connection we
don't need to send PN again. We do have to be in BT_CONNECTED state to
have the encryption droping check to kick in. The SEC_PENDING only
ensures that we stay encrypted. It has nothing to with the auth step.
Also we combine auth+encrypt since one without the other makes no sense
at all. So the initial call of security_cfm will always include auth and
encrypt enabled.

Regards

Marcel



2009-01-15 23:52:28

by Jaikumar Ganesh

[permalink] [raw]
Subject: Re: [RFC] Some kernel changes

Hi Marcel,

On Thu, Jan 15, 2009 at 3:25 PM, Marcel Holtmann <[email protected]> wrote:
> Hi,
>
>> so I pushed another set of patches to the
>> bluetooth-testing.git tree and
>> it should include the full BT_SECURITY implemention,
>> BT_DEFER_SETUP for
>> RFCOMM and SCO rejection if no listen socket is present.
>>
>> It currently only drops the connection is encryption is
>> disabled when
>> using BT_SECURITY_HIGH. That is only used for SAP anyway. For
>> all other
>> profiles we use BT_SECURITY_MEDIUM.
>>
>> I updated our build with the patches and after picking up 6e26576c
>> (Pause RFCOMM TX when encryption drops) and fixes upto f32ef1836
>> (Enforce authentication before encryption) I see a couple of
>> problems::
>>
>> a) I see that in rfcomm/core.c in function rfcomm_security_cfm:
>> when the remote side has dropped the encryption for the role change,
>> we set the RFCOMM_SEC_PENDING bit but we don't set RFCOMM_AUTH_PENDING
>> as suggested in Ville's original patch and so when encryption is
>> re-established we dont get past the - test_and_clear_bit
>> (RFCOMM_AUTH_PENDING) check in the function.
>
> why do you wanna set AUTH_PENDING again. That is not needed we only
> wanna know once encryption comes back on. If no in time, then we just
> disconnect the link. That simple.
>
>> b) I also see that we are not clearing the timer and hence after
>> RFCOMM_AUTH_TIMEOUT period expires we bring down the RFCOMM connection
>> even though the encryption has been established.
>
> Good catch. Fixed it.

Cool. Can you upload it to bluetooth-testing git ? I saw a related
problem while looking at the timer issue and wanted to see if the fix
takes care of it.

The timer thats started in rfcomm_security_cfm (for the drop in
encryption) does get cleared in rfcomm_process_dlcs
but then we have some data to send. So we send the RFCOMM PN packet
just after encryption is re-established.
This results in a new timer being started in rfcomm_process_dlcs and
we don't get the UA packet to clear this timer and hence when
the timer expires, the connection is dropped.

Thanks
Jaikumar

> Regards
>
> Marcel
>
>
>

2009-01-15 23:25:46

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [RFC] Some kernel changes

Hi,

> so I pushed another set of patches to the
> bluetooth-testing.git tree and
> it should include the full BT_SECURITY implemention,
> BT_DEFER_SETUP for
> RFCOMM and SCO rejection if no listen socket is present.
>
> It currently only drops the connection is encryption is
> disabled when
> using BT_SECURITY_HIGH. That is only used for SAP anyway. For
> all other
> profiles we use BT_SECURITY_MEDIUM.
>
> I updated our build with the patches and after picking up 6e26576c
> (Pause RFCOMM TX when encryption drops) and fixes upto f32ef1836
> (Enforce authentication before encryption) I see a couple of
> problems::
>
> a) I see that in rfcomm/core.c in function rfcomm_security_cfm:
> when the remote side has dropped the encryption for the role change,
> we set the RFCOMM_SEC_PENDING bit but we don't set RFCOMM_AUTH_PENDING
> as suggested in Ville's original patch and so when encryption is
> re-established we dont get past the - test_and_clear_bit
> (RFCOMM_AUTH_PENDING) check in the function.

why do you wanna set AUTH_PENDING again. That is not needed we only
wanna know once encryption comes back on. If no in time, then we just
disconnect the link. That simple.

> b) I also see that we are not clearing the timer and hence after
> RFCOMM_AUTH_TIMEOUT period expires we bring down the RFCOMM connection
> even though the encryption has been established.

Good catch. Fixed it.

Regards

Marcel



2009-01-15 19:14:59

by Jaikumar Ganesh

[permalink] [raw]
Subject: Re: [RFC] Some kernel changes

Hi Marcel
[Sorry sending again as the mail bounced]

On Tue, Jan 13, 2009 at 11:12 AM, Marcel Holtmann <[email protected]> wrote:
> Hi Nick,
>
>> >> Here is set of unclean patches for discussion. These are still untested
>> >> and contain issues, but I would like to know if we should continue with
>> >> these or change something radically. Also commit messages are somewhat
>> >> misleading in some places.
>> >>
>> >> Basically idea is to implement connection rejection support for l2cap
>> >> and rfcomm sockets. IOW possibility to check initiator bd-address and
>> >> ask for authorization before accepting connection.
>> >>
>> >> Other goal is to fix encryption and authentication levels to fulfill 2.1
>> >> requirements.
>> >
>> > so I pushed my re-worked and pending patches to bluetooth-testing.git
>> > now. This adds support for BT_DEFER_SETUP and BT_SECURITY (only the
>> > logic and not the actual socket option).
>>
>> Ville: thanks for the patches, pausing RFCOMM while encryption is
>> disabled is a nice fix.
>>
>> I imagine that if encryption is not quickly re-established we need to
>> drop the RFCOMM link rather than leaving it paused though.
>>
>> We will do some testing of the current rfcomm-pause patches.
>
> so I pushed another set of patches to the bluetooth-testing.git tree and
> it should include the full BT_SECURITY implemention, BT_DEFER_SETUP for
> RFCOMM and SCO rejection if no listen socket is present.
>
> It currently only drops the connection is encryption is disabled when
> using BT_SECURITY_HIGH. That is only used for SAP anyway. For all other
> profiles we use BT_SECURITY_MEDIUM.

I updated our build with the patches and after picking up 6e26576c
(Pause RFCOMM TX when encryption drops) and fixes upto f32ef1836
(Enforce authentication before encryption) I see a couple of
problems::

a) I see that in rfcomm/core.c in function rfcomm_security_cfm:
when the remote side has dropped the encryption for the role change,
we set the RFCOMM_SEC_PENDING bit but we don't set RFCOMM_AUTH_PENDING
as suggested in Ville's original patch and so when encryption is
re-established we dont get past the - test_and_clear_bit
(RFCOMM_AUTH_PENDING) check in the function.

b) I also see that we are not clearing the timer and hence after
RFCOMM_AUTH_TIMEOUT period expires we bring down the RFCOMM connection
even though the encryption has been established.

Sorry, I don't have a patch ready as yet and hence the description
above to check if you have encountered the same.

Thanks
Jaikumar

> Regards
>
> Marcel
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2009-01-15 19:13:15

by Jaikumar Ganesh

[permalink] [raw]
Subject: Re: [RFC] Some kernel changes

Hi Marcel,

On Tue, Jan 13, 2009 at 11:12 AM, Marcel Holtmann <[email protected]>wrote:

> Hi Nick,
>
> > >> Here is set of unclean patches for discussion. These are still
> untested
> > >> and contain issues, but I would like to know if we should continue
> with
> > >> these or change something radically. Also commit messages are somewhat
> > >> misleading in some places.
> > >>
> > >> Basically idea is to implement connection rejection support for l2cap
> > >> and rfcomm sockets. IOW possibility to check initiator bd-address and
> > >> ask for authorization before accepting connection.
> > >>
> > >> Other goal is to fix encryption and authentication levels to fulfill
> 2.1
> > >> requirements.
> > >
> > > so I pushed my re-worked and pending patches to bluetooth-testing.git
> > > now. This adds support for BT_DEFER_SETUP and BT_SECURITY (only the
> > > logic and not the actual socket option).
> >
> > Ville: thanks for the patches, pausing RFCOMM while encryption is
> > disabled is a nice fix.
> >
> > I imagine that if encryption is not quickly re-established we need to
> > drop the RFCOMM link rather than leaving it paused though.
> >
> > We will do some testing of the current rfcomm-pause patches.
>
> so I pushed another set of patches to the bluetooth-testing.git tree and
> it should include the full BT_SECURITY implemention, BT_DEFER_SETUP for
> RFCOMM and SCO rejection if no listen socket is present.
>
> It currently only drops the connection is encryption is disabled when
> using BT_SECURITY_HIGH. That is only used for SAP anyway. For all other
> profiles we use BT_SECURITY_MEDIUM.


I updated our build with the patches and after picking up 6e26576c
(Pause RFCOMM TX when encryption drops) and fixes upto f32ef1836 (Enforce
authentication before encryption) I see a couple of problems::

a) I see that in rfcomm/core.c in function rfcomm_security_cfm: when
the remote side has dropped the encryption for the role change, we set the
RFCOMM_SEC_PENDING bit but we don't set RFCOMM_AUTH_PENDING as suggested in
Ville's original patch and so when encryption is re-established we dont get
past the - test_and_clear_bit (RFCOMM_AUTH_PENDING) check in the function.

b) I also see that we are not clearing the timer and hence after
RFCOMM_AUTH_TIMEOUT period expires we bring down the RFCOMM connection even
though the encryption has been established.

Sorry, I don't have a patch ready as yet and hence the description above
to check if you have encountered the same.

Thanks
Jaikumar

>
>
> Regards
>
> Marcel
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth"
> in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>


Attachments:
(No filename) (2.71 kB)
(No filename) (3.67 kB)
Download all attachments

2009-01-13 19:12:46

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [RFC] Some kernel changes

Hi Nick,

> >> Here is set of unclean patches for discussion. These are still untested
> >> and contain issues, but I would like to know if we should continue with
> >> these or change something radically. Also commit messages are somewhat
> >> misleading in some places.
> >>
> >> Basically idea is to implement connection rejection support for l2cap
> >> and rfcomm sockets. IOW possibility to check initiator bd-address and
> >> ask for authorization before accepting connection.
> >>
> >> Other goal is to fix encryption and authentication levels to fulfill 2.1
> >> requirements.
> >
> > so I pushed my re-worked and pending patches to bluetooth-testing.git
> > now. This adds support for BT_DEFER_SETUP and BT_SECURITY (only the
> > logic and not the actual socket option).
>
> Ville: thanks for the patches, pausing RFCOMM while encryption is
> disabled is a nice fix.
>
> I imagine that if encryption is not quickly re-established we need to
> drop the RFCOMM link rather than leaving it paused though.
>
> We will do some testing of the current rfcomm-pause patches.

so I pushed another set of patches to the bluetooth-testing.git tree and
it should include the full BT_SECURITY implemention, BT_DEFER_SETUP for
RFCOMM and SCO rejection if no listen socket is present.

It currently only drops the connection is encryption is disabled when
using BT_SECURITY_HIGH. That is only used for SAP anyway. For all other
profiles we use BT_SECURITY_MEDIUM.

Regards

Marcel



2009-01-12 17:15:10

by Nick Pelly

[permalink] [raw]
Subject: Re: [RFC] Some kernel changes

On Mon, Jan 12, 2009 at 5:53 AM, Marcel Holtmann <[email protected]> wrote:
> Hi Ville,
>
>> Here is set of unclean patches for discussion. These are still untested
>> and contain issues, but I would like to know if we should continue with
>> these or change something radically. Also commit messages are somewhat
>> misleading in some places.
>>
>> Basically idea is to implement connection rejection support for l2cap
>> and rfcomm sockets. IOW possibility to check initiator bd-address and
>> ask for authorization before accepting connection.
>>
>> Other goal is to fix encryption and authentication levels to fulfill 2.1
>> requirements.
>
> so I pushed my re-worked and pending patches to bluetooth-testing.git
> now. This adds support for BT_DEFER_SETUP and BT_SECURITY (only the
> logic and not the actual socket option).

Ville: thanks for the patches, pausing RFCOMM while encryption is
disabled is a nice fix.

I imagine that if encryption is not quickly re-established we need to
drop the RFCOMM link rather than leaving it paused though.

We will do some testing of the current rfcomm-pause patches.

Nick

2009-01-12 13:53:33

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [RFC] Some kernel changes

Hi Ville,

> Here is set of unclean patches for discussion. These are still untested
> and contain issues, but I would like to know if we should continue with
> these or change something radically. Also commit messages are somewhat
> misleading in some places.
>
> Basically idea is to implement connection rejection support for l2cap
> and rfcomm sockets. IOW possibility to check initiator bd-address and
> ask for authorization before accepting connection.
>
> Other goal is to fix encryption and authentication levels to fulfill 2.1
> requirements.

so I pushed my re-worked and pending patches to bluetooth-testing.git
now. This adds support for BT_DEFER_SETUP and BT_SECURITY (only the
logic and not the actual socket option).

Regards

Marcel



2009-01-10 14:26:33

by Johan Hedberg

[permalink] [raw]
Subject: Re: [RFC] Some kernel changes

Hi,

On Jan 10, 2009, at 16:18, ext Marcel Holtmann wrote:
>> Here is set of unclean patches for discussion. These are still
>> untested
>> and contain issues, but I would like to know if we should continue
>> with
>> these or change something radically. Also commit messages are
>> somewhat
>> misleading in some places.
>
> any reason why patch 0003 is missing?

Just checked with Ville and it was apparently some non-bluetooth
related OMAP patch, i.e. it has been left out on purpose.

Johan

2009-01-10 14:18:18

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [RFC] Some kernel changes

Hi Ville,

> Here is set of unclean patches for discussion. These are still untested
> and contain issues, but I would like to know if we should continue with
> these or change something radically. Also commit messages are somewhat
> misleading in some places.

any reason why patch 0003 is missing?

Regards

Marcel