| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <6000bce133414df79e386e605b8dd1a5@realsil.com.cn>
Date: Mon, 28 Sep 2020 05:47:42 +0000
From: 陆朱伟 <alex_lu@...lsil.com.cn>
To: Marcel Holtmann <marcel@...tmann.org>
CC: Johan Hedberg <johan.hedberg@...il.com>,
linux-bluetooth <linux-bluetooth@...r.kernel.org>,
open list <linux-kernel@...r.kernel.org>,
Max Chou <max.chou@...ltek.com>
Subject: Re [PATCH] Bluetooth: Fix the vulnerable issue on enc key size
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 <alex_lu@...lsil.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;
> }
Yes, you're right. Just to assign a different value is the easiest way.
Previously I struggled to provide complete evidence for qualification test, so I made the change a bit more complicated.
I'm sorry to mess it.
>
> 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.
Thanks for your remainder. It seems that Archie and I have encountered the same problem. I also found this when I did the qualification.
>
> Regards
>
> Marcel
>
>
Powered by blists - more mailing lists