lkml.org 
[lkml]   [2020]   [Sep]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH] Bluetooth: Fix the vulnerable issue on enc key size
From
Date
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@realsil.com.cn>
>>>>> ---
>>>>> 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

the easiest way to fake this is just to assign a different value than the one returned by Read Encryption Key Size on the acceptor side. No need to mess with LMP details.

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 1df95145f574..741b7ad31ff8 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3034,7 +3034,7 @@ static void read_enc_key_size_complete(struct hci_dev *hdev, u8 status,
handle);
conn->enc_key_size = 0;
} else {
- conn->enc_key_size = rp->key_size;
+ conn->enc_key_size = 1;
}

If you add this change to both sides, what are the steps to reproduce this and what does btmon show? You might have to also enable dynamic_debug for l2cap.ko so that we see the function call trace.

I am a bit pedantic with this one, since it is critical to understand where the current changes to handle anything KNOB related have their shortcomings. I spent so much time testing every single corner case. Certainly I could have missed something, but if I really did, this time around I want to either simplify the code or properly comment it at least.
Please see the other email thread / patch from Archie trying to also add another encryption key size check.

Regards

Marcel

\
 
 \ /
  Last update: 2020-09-27 14:05    [W:0.040 / U:7.276 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site