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

[陆朱伟]

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 <alex_lu@xxxxxxxxxxxxxx>
> >>> ---
> >>> 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 <bdaddr> 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|0x0008) plen 2                                                                                        #22 [hci0] 43.089859
        Handle: 1
> HCI Event: Command Complete (0x0e) plen 7                                                                                                         #23 [hci0] 43.091528
      Read Encryption Key Size (0x05|0x0008) ncmd 2
        Status: Success (0x00)
        Handle: 1
        Key size: 1
> ACL Data RX: Handle 1 flags 0x02 dlen 10                                                                                                          #24 [hci0] 43.140888
      L2CAP: Information Request (0x0a) ident 1 len 2
        Type: Extended features supported (0x0002)
......
> ACL Data RX: Handle 1 flags 0x02 dlen 12                                                                                                          #34 [hci0] 43.148405
      L2CAP: Connection Request (0x02) ident 3 len 4
        PSM: 3 (0x0003)
        Source CID: 64

> 
> Regards
> 
> Marcel
> 
>