Re [PATCH] Bluetooth: Fix the vulnerable issue on enc key size

2020-09-27 Thread 陆朱伟
Hi Marcel,

> On September 27, 2020 20:05, Marcel Holtmann wrote:
> 
> Hi Alex,
> 
> > When someone attacks the service provider, it creates connection,
> > authenticates. Then it requests key size of one byte and it identifies
> > the key with brute force methods.
> >
> > After l2cap info req/resp exchange is complete. the attacker sends
> l2cap
> > connect with specific PSM.
> >
> > In above procedure, there is no chance for the service provider to
> check
> > the encryption key size before l2cap_connect(). Because the state of
> > l2cap chan in conn->chan_l is BT_LISTEN, there is no l2cap chan with
> the
> > state of BT_CONNECT or BT_CONNECT2.
> >
> > So service provider should check the encryption key size in
> > l2cap_connect()
> >
> > Signed-off-by: Alex Lu 
> > ---
> > net/bluetooth/l2cap_core.c | 7 +++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> > index ade83e224567..63df961d402d 100644
> > --- a/net/bluetooth/l2cap_core.c
> > +++ b/net/bluetooth/l2cap_core.c
> > @@ -4150,6 +4150,13 @@ static struct l2cap_chan
> *l2cap_connect(struct
>  l2cap_conn *conn,
> >
> > if (conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_DONE) {
> > if (l2cap_chan_check_security(chan, false)) {
> > +   if (!l2cap_check_enc_key_size(conn->hcon))
> {
> > +   l2cap_state_change(chan,
> BT_DISCONN);
> > +   __set_chan_timer(chan,
>  L2CAP_DISC_TIMEOUT);
> > +   result = L2CAP_CR_SEC_BLOCK;
> > +   status = L2CAP_CS_NO_INFO;
> > +   goto response;
> > +   }
> > if (test_bit(FLAG_DEFER_SETUP, &chan->flags)) {
> > l2cap_state_change(chan, BT_CONNECT2);
> > result = L2CAP_CR_PEND;
> 
>  I am not following what you are trying to fix here. Can you show this
> with
> >> a
>  btmon trace from an attacking device?
> 
>  Regards
> 
>  Marcel
> 
> 
> >>>
> >>> I'm sorry, I didn't have btmon trace from an attacking device.
> >>> I didn't have the real attacking device. I just simulate the attacking.
> >>> I have a device that can create one byte size encryption key.
> >>> It uses the link key that was produced by pairing with the service
> provider.
> >> Actually the KNOB (Key Negotiation of Bluetooth Attack) says, the link key
> is
> >> unnecessary for the reconnection.
> >>> I use this device to reconnect to service provider, and then initiate the
> Key
> >> Negotiation for one byte size encryption key. Actually the attacker
> identified
> >> the encryption key with some brute force methods.
> >>>
> >>> I want to provide the trace on service provider side.
> >>
> >> what kernel version are you running? I wonder if we should always return
> >> L2CAP_CR_PEND here. Do you have a reproducer code?
> >
> > I'm running kernel 5.8.0-rc6 on acceptor and kernel 5.8.5 on the initiator
> which acts as an attacker.
> > For the attack simulation, some code needs to be changed on each size.
> > On the acceptor, the master parameter for bt_io_listen() in bluetoothd
> should be changed to FALSE in profiles/audio/a2dp.c a2dp_server_listen()
> and profiles/audio/avctp.c avctp_server_socket().
> > The change makes the kernel not to change the role to master when it
> receives hci conn req event.
> > I did the change in order to make the controller to send
> LMP_ENCRYPTION_KEY_SIZE_REQ PDU for one byte key size.
> >
> > On the initiator, the below encryption key size check should be removed.
> > @@ -1622,10 +1624,13 @@ static void l2cap_conn_start(struct l2cap_conn
> *conn)
> >continue;
> >}
> >
> > -   if (l2cap_check_enc_key_size(conn->hcon))
> > -   l2cap_start_connection(chan);
> > -   else
> > -   l2cap_chan_close(chan, ECONNREFUSED);
> > +   /* Just simulate KNOB */
> > +   l2cap_start_connection(chan);
> > +   /* if (l2cap_check_enc_key_size(conn->hcon))
> > +*  l2cap_start_connection(chan);
> > +* else
> > +*  l2cap_chan_close(chan, ECONNREFUSED);
> > +*/
> >
> > At last, I did the test as below:
> > 1. On the initiator, pair acceptor
> > 2. Run l2test -r -P 3 on the acceptor
> > 3. Run l2test -n -P 3  on the initiator
> >
> >>
> >> The problem really is that the MASK_REQ_DONE indication is not enough
> to
> >> make a decision for the key size. We have to ensure that also the key size
> is
>

Re: [PATCH] Bluetooth: Fix the vulnerable issue on enc key size

2020-09-27 Thread Marcel Holtmann
Hi Alex,

> When someone attacks the service provider, it creates connection,
> authenticates. Then it requests key size of one byte and it identifies
> the key with brute force methods.
> 
> After l2cap info req/resp exchange is complete. the attacker sends l2cap
> connect with specific PSM.
> 
> In above procedure, there is no chance for the service provider to check
> the encryption key size before l2cap_connect(). Because the state of
> l2cap chan in conn->chan_l is BT_LISTEN, there is no l2cap chan with the
> state of BT_CONNECT or BT_CONNECT2.
> 
> So service provider should check the encryption key size in
> l2cap_connect()
> 
> Signed-off-by: Alex Lu 
> ---
> net/bluetooth/l2cap_core.c | 7 +++
> 1 file changed, 7 insertions(+)
> 
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index ade83e224567..63df961d402d 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -4150,6 +4150,13 @@ static struct l2cap_chan *l2cap_connect(struct
 l2cap_conn *conn,
> 
>   if (conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_DONE) {
>   if (l2cap_chan_check_security(chan, false)) {
> + if (!l2cap_check_enc_key_size(conn->hcon)) {
> + l2cap_state_change(chan, BT_DISCONN);
> + __set_chan_timer(chan,
 L2CAP_DISC_TIMEOUT);
> + result = L2CAP_CR_SEC_BLOCK;
> + status = L2CAP_CS_NO_INFO;
> + goto response;
> + }
>   if (test_bit(FLAG_DEFER_SETUP, &chan->flags)) {
>   l2cap_state_change(chan, BT_CONNECT2);
>   result = L2CAP_CR_PEND;
 
 I am not following what you are trying to fix here. Can you show this with
>> a
 btmon trace from an attacking device?
 
 Regards
 
 Marcel
 
 
>>> 
>>> I'm sorry, I didn't have btmon trace from an attacking device.
>>> I didn't have the real attacking device. I just simulate the attacking.
>>> I have a device that can create one byte size encryption key.
>>> It uses the link key that was produced by pairing with the service provider.
>> Actually the KNOB (Key Negotiation of Bluetooth Attack) says, the link key is
>> unnecessary for the reconnection.
>>> I use this device to reconnect to service provider, and then initiate the 
>>> Key
>> Negotiation for one byte size encryption key. Actually the attacker 
>> identified
>> the encryption key with some brute force methods.
>>> 
>>> I want to provide the trace on service provider side.
>> 
>> what kernel version are you running? I wonder if we should always return
>> L2CAP_CR_PEND here. Do you have a reproducer code?
> 
> I'm running kernel 5.8.0-rc6 on acceptor and kernel 5.8.5 on the initiator 
> which acts as an attacker.
> For the attack simulation, some code needs to be changed on each size.
> On the acceptor, the master parameter for bt_io_listen() in bluetoothd should 
> be changed to FALSE in profiles/audio/a2dp.c a2dp_server_listen() and 
> profiles/audio/avctp.c avctp_server_socket().
> The change makes the kernel not to change the role to master when it receives 
> hci conn req event.
> I did the change in order to make the controller to send 
> LMP_ENCRYPTION_KEY_SIZE_REQ PDU for one byte key size.
> 
> On the initiator, the below encryption key size check should be removed.
> @@ -1622,10 +1624,13 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
>continue;
>}
> 
> -   if (l2cap_check_enc_key_size(conn->hcon))
> -   l2cap_start_connection(chan);
> -   else
> -   l2cap_chan_close(chan, ECONNREFUSED);
> +   /* Just simulate KNOB */
> +   l2cap_start_connection(chan);
> +   /* if (l2cap_check_enc_key_size(conn->hcon))
> +*  l2cap_start_connection(chan);
> +* else
> +*  l2cap_chan_close(chan, ECONNREFUSED);
> +*/
> 
> At last, I did the test as below:
> 1. On the initiator, pair acceptor
> 2. Run l2test -r -P 3 on the acceptor
> 3. Run l2test -n -P 3  on the initiator
> 
>> 
>> The problem really is that the MASK_REQ_DONE indication is not enough to
>> make a decision for the key size. We have to ensure that also the key size is
>> actually available. If that is not yet done, then we should not check it. 
>> This
>> means that any response to L2CAP_Connect_Request PDU needs to be
>> delayed until the key size has been read.
> 
> In my test case, the key size has been read from controller before the l2cap 
> conn request PDU is received.
> 
> < HCI Command: 

Re: [PATCH] Bluetooth: Fix the vulnerable issue on enc key size

2020-09-26 Thread 陆朱伟
Hi Marcel,

> On 26 September 2020 at 1:34, Marcel Holtmann wrote:
> 
> Hi Alex,
> 
> >>> When someone attacks the service provider, it creates connection,
> >>> authenticates. Then it requests key size of one byte and it identifies
> >>> the key with brute force methods.
> >>>
> >>> After l2cap info req/resp exchange is complete. the attacker sends l2cap
> >>> connect with specific PSM.
> >>>
> >>> In above procedure, there is no chance for the service provider to check
> >>> the encryption key size before l2cap_connect(). Because the state of
> >>> l2cap chan in conn->chan_l is BT_LISTEN, there is no l2cap chan with the
> >>> state of BT_CONNECT or BT_CONNECT2.
> >>>
> >>> So service provider should check the encryption key size in
> >>> l2cap_connect()
> >>>
> >>> Signed-off-by: Alex Lu 
> >>> ---
> >>> net/bluetooth/l2cap_core.c | 7 +++
> >>> 1 file changed, 7 insertions(+)
> >>>
> >>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> >>> index ade83e224567..63df961d402d 100644
> >>> --- a/net/bluetooth/l2cap_core.c
> >>> +++ b/net/bluetooth/l2cap_core.c
> >>> @@ -4150,6 +4150,13 @@ static struct l2cap_chan *l2cap_connect(struct
> >> l2cap_conn *conn,
> >>>
> >>>   if (conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_DONE) {
> >>>   if (l2cap_chan_check_security(chan, false)) {
> >>> + if (!l2cap_check_enc_key_size(conn->hcon)) {
> >>> + l2cap_state_change(chan, BT_DISCONN);
> >>> + __set_chan_timer(chan,
> >> L2CAP_DISC_TIMEOUT);
> >>> + result = L2CAP_CR_SEC_BLOCK;
> >>> + status = L2CAP_CS_NO_INFO;
> >>> + goto response;
> >>> + }
> >>>   if (test_bit(FLAG_DEFER_SETUP, &chan->flags)) {
> >>>   l2cap_state_change(chan, BT_CONNECT2);
> >>>   result = L2CAP_CR_PEND;
> >>
> >> I am not following what you are trying to fix here. Can you show this with
> a
> >> btmon trace from an attacking device?
> >>
> >> Regards
> >>
> >> Marcel
> >>
> >>
> >
> > I'm sorry, I didn't have btmon trace from an attacking device.
> > I didn't have the real attacking device. I just simulate the attacking.
> > I have a device that can create one byte size encryption key.
> > It uses the link key that was produced by pairing with the service provider.
> Actually the KNOB (Key Negotiation of Bluetooth Attack) says, the link key is
> unnecessary for the reconnection.
> > I use this device to reconnect to service provider, and then initiate the 
> > Key
> Negotiation for one byte size encryption key. Actually the attacker identified
> the encryption key with some brute force methods.
> >
> > I want to provide the trace on service provider side.
> 
> what kernel version are you running? I wonder if we should always return
> L2CAP_CR_PEND here. Do you have a reproducer code?

I'm running kernel 5.8.0-rc6 on acceptor and kernel 5.8.5 on the initiator 
which acts as an attacker.
For the attack simulation, some code needs to be changed on each size.
On the acceptor, the master parameter for bt_io_listen() in bluetoothd should 
be changed to FALSE in profiles/audio/a2dp.c a2dp_server_listen() and 
profiles/audio/avctp.c avctp_server_socket().
The change makes the kernel not to change the role to master when it receives 
hci conn req event.
I did the change in order to make the controller to send 
LMP_ENCRYPTION_KEY_SIZE_REQ PDU for one byte key size.

On the initiator, the below encryption key size check should be removed.
@@ -1622,10 +1624,13 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
continue;
}
 
-   if (l2cap_check_enc_key_size(conn->hcon))
-   l2cap_start_connection(chan);
-   else
-   l2cap_chan_close(chan, ECONNREFUSED);
+   /* Just simulate KNOB */
+   l2cap_start_connection(chan);
+   /* if (l2cap_check_enc_key_size(conn->hcon))
+*  l2cap_start_connection(chan);
+* else
+*  l2cap_chan_close(chan, ECONNREFUSED);
+*/

At last, I did the test as below:
1. On the initiator, pair acceptor
2. Run l2test -r -P 3 on the acceptor
3. Run l2test -n -P 3  on the initiator

> 
> The problem really is that the MASK_REQ_DONE indication is not enough to
> make a decision for the key size. We have to ensure that also the key size is
> actually available. If that is not yet done, then we should not check it. This
> means that any response to L2CAP_Connect_Request PDU needs to be
> delayed until the key size has been read.

In my test case, the key size has been read from controller before the l2cap 
conn request PDU is received.

< HCI Command: Read Encryption Key Size (0x05|0x000

Re: [PATCH] Bluetooth: Fix the vulnerable issue on enc key size

2020-09-25 Thread Marcel Holtmann
Hi Alex,

>>> When someone attacks the service provider, it creates connection,
>>> authenticates. Then it requests key size of one byte and it identifies
>>> the key with brute force methods.
>>> 
>>> After l2cap info req/resp exchange is complete. the attacker sends l2cap
>>> connect with specific PSM.
>>> 
>>> In above procedure, there is no chance for the service provider to check
>>> the encryption key size before l2cap_connect(). Because the state of
>>> l2cap chan in conn->chan_l is BT_LISTEN, there is no l2cap chan with the
>>> state of BT_CONNECT or BT_CONNECT2.
>>> 
>>> So service provider should check the encryption key size in
>>> l2cap_connect()
>>> 
>>> Signed-off-by: Alex Lu 
>>> ---
>>> net/bluetooth/l2cap_core.c | 7 +++
>>> 1 file changed, 7 insertions(+)
>>> 
>>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
>>> index ade83e224567..63df961d402d 100644
>>> --- a/net/bluetooth/l2cap_core.c
>>> +++ b/net/bluetooth/l2cap_core.c
>>> @@ -4150,6 +4150,13 @@ static struct l2cap_chan *l2cap_connect(struct
>> l2cap_conn *conn,
>>> 
>>> if (conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_DONE) {
>>> if (l2cap_chan_check_security(chan, false)) {
>>> +   if (!l2cap_check_enc_key_size(conn->hcon)) {
>>> +   l2cap_state_change(chan, BT_DISCONN);
>>> +   __set_chan_timer(chan,
>> L2CAP_DISC_TIMEOUT);
>>> +   result = L2CAP_CR_SEC_BLOCK;
>>> +   status = L2CAP_CS_NO_INFO;
>>> +   goto response;
>>> +   }
>>> if (test_bit(FLAG_DEFER_SETUP, &chan->flags)) {
>>> l2cap_state_change(chan, BT_CONNECT2);
>>> result = L2CAP_CR_PEND;
>> 
>> I am not following what you are trying to fix here. Can you show this with a
>> btmon trace from an attacking device?
>> 
>> Regards
>> 
>> Marcel
>> 
>> 
> 
> I'm sorry, I didn't have btmon trace from an attacking device.
> I didn't have the real attacking device. I just simulate the attacking.
> I have a device that can create one byte size encryption key.
> It uses the link key that was produced by pairing with the service provider. 
> Actually the KNOB (Key Negotiation of Bluetooth Attack) says, the link key is 
> unnecessary for the reconnection.
> I use this device to reconnect to service provider, and then initiate the Key 
> Negotiation for one byte size encryption key. Actually the attacker 
> identified the encryption key with some brute force methods.
> 
> I want to provide the trace on service provider side.

what kernel version are you running? I wonder if we should always return 
L2CAP_CR_PEND here. Do you have a reproducer code?

The problem really is that the MASK_REQ_DONE indication is not enough to make a 
decision for the key size. We have to ensure that also the key size is actually 
available. If that is not yet done, then we should not check it. This means 
that any response to L2CAP_Connect_Request PDU needs to be delayed until the 
key size has been read.

Regards

Marcel



Re: [PATCH] Bluetooth: Fix the vulnerable issue on enc key size

2020-09-20 Thread 陆朱伟
Hi Marcel,

> On September 20, 2020 14:10, Marcel Holtmann wrote:
> 
> Hi Alex,
> 
> > When someone attacks the service provider, it creates connection,
> > authenticates. Then it requests key size of one byte and it identifies
> > the key with brute force methods.
> >
> > After l2cap info req/resp exchange is complete. the attacker sends l2cap
> > connect with specific PSM.
> >
> > In above procedure, there is no chance for the service provider to check
> > the encryption key size before l2cap_connect(). Because the state of
> > l2cap chan in conn->chan_l is BT_LISTEN, there is no l2cap chan with the
> > state of BT_CONNECT or BT_CONNECT2.
> >
> > So service provider should check the encryption key size in
> > l2cap_connect()
> >
> > Signed-off-by: Alex Lu 
> > ---
> > net/bluetooth/l2cap_core.c | 7 +++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> > index ade83e224567..63df961d402d 100644
> > --- a/net/bluetooth/l2cap_core.c
> > +++ b/net/bluetooth/l2cap_core.c
> > @@ -4150,6 +4150,13 @@ static struct l2cap_chan *l2cap_connect(struct
> l2cap_conn *conn,
> >
> > if (conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_DONE) {
> > if (l2cap_chan_check_security(chan, false)) {
> > +   if (!l2cap_check_enc_key_size(conn->hcon)) {
> > +   l2cap_state_change(chan, BT_DISCONN);
> > +   __set_chan_timer(chan,
> L2CAP_DISC_TIMEOUT);
> > +   result = L2CAP_CR_SEC_BLOCK;
> > +   status = L2CAP_CS_NO_INFO;
> > +   goto response;
> > +   }
> > if (test_bit(FLAG_DEFER_SETUP, &chan->flags)) {
> > l2cap_state_change(chan, BT_CONNECT2);
> > result = L2CAP_CR_PEND;
> 
> I am not following what you are trying to fix here. Can you show this with a
> btmon trace from an attacking device?
> 
> Regards
> 
> Marcel
> 
> 

I'm sorry, I didn't have btmon trace from an attacking device.
I didn't have the real attacking device. I just simulate the attacking.
I have a device that can create one byte size encryption key.
It uses the link key that was produced by pairing with the service provider. 
Actually the KNOB (Key Negotiation of Bluetooth Attack) says, the link key is 
unnecessary for the reconnection.
I use this device to reconnect to service provider, and then initiate the Key 
Negotiation for one byte size encryption key. Actually the attacker identified 
the encryption key with some brute force methods.

I want to provide the trace on service provider side.

> HCI Event: Connect Request (0x04) plen 10 
>  #1 
> [hci0] 42.932585
Address: 00:3F:22:EE:11:33 (OUI 00-3F-22)
Class: 0x00010c
  Major class: Computer (desktop, notebook, PDA, organizers)
  Minor class: Laptop
Link type: ACL (0x01)
< HCI Command: Accept Connection Request (0x01|0x0009) plen 7   
 #2 [hci0] 
42.932795
Address: 00:3F:22:EE:11:33 (OUI 00-3F-22)
Role: Slave (0x01)
> HCI Event: Command Status (0x0f) plen 4   
>  #3 
> [hci0] 42.934509
  Accept Connection Request (0x01|0x0009) ncmd 2
Status: Success (0x00)
> HCI Event: Connect Complete (0x03) plen 11
>  #4 
> [hci0] 42.964568
Status: Success (0x00)
Handle: 1
Address: 00:3F:22:EE:11:33 (OUI 00-3F-22)
Link type: ACL (0x01)
Encryption: Disabled (0x00)
< HCI Command: Read Remote Supported Features (0x01|0x001b) plen 2  
 #5 [hci0] 
42.964815
Handle: 1
> HCI Event: Max Slots Change (0x1b) plen 3 
>  #6 
> [hci0] 42.970516
Handle: 1
Max slots: 5
> HCI Event: Command Status (0x0f) plen 4   
>  #7 
> [hci0] 42.971592
  Read Remote Supported Features (0x01|0x001b) ncmd 2
Status: Success (0x00)
> HCI Event: Max Slots Change (0x1b) plen 3 
>  #8 
> [hci0] 42.976516
Handle: 1
Max slots: 5
> HCI Event: Read Remote Supported Features (0x0b) plen 11  
>  #9 
> [hci0] 42.980521
Status: Success (0x

Re: [PATCH] Bluetooth: Fix the vulnerable issue on enc key size

2020-09-19 Thread Marcel Holtmann
Hi Alex,

> When someone attacks the service provider, it creates connection,
> authenticates. Then it requests key size of one byte and it identifies
> the key with brute force methods.
> 
> After l2cap info req/resp exchange is complete. the attacker sends l2cap
> connect with specific PSM.
> 
> In above procedure, there is no chance for the service provider to check
> the encryption key size before l2cap_connect(). Because the state of
> l2cap chan in conn->chan_l is BT_LISTEN, there is no l2cap chan with the
> state of BT_CONNECT or BT_CONNECT2.
> 
> So service provider should check the encryption key size in
> l2cap_connect()
> 
> Signed-off-by: Alex Lu 
> ---
> net/bluetooth/l2cap_core.c | 7 +++
> 1 file changed, 7 insertions(+)
> 
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index ade83e224567..63df961d402d 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -4150,6 +4150,13 @@ static struct l2cap_chan *l2cap_connect(struct 
> l2cap_conn *conn,
> 
>   if (conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_DONE) {
>   if (l2cap_chan_check_security(chan, false)) {
> + if (!l2cap_check_enc_key_size(conn->hcon)) {
> + l2cap_state_change(chan, BT_DISCONN);
> + __set_chan_timer(chan, L2CAP_DISC_TIMEOUT);
> + result = L2CAP_CR_SEC_BLOCK;
> + status = L2CAP_CS_NO_INFO;
> + goto response;
> + }
>   if (test_bit(FLAG_DEFER_SETUP, &chan->flags)) {
>   l2cap_state_change(chan, BT_CONNECT2);
>   result = L2CAP_CR_PEND;

I am not following what you are trying to fix here. Can you show this with a 
btmon trace from an attacking device?

Regards

Marcel