Netdev Archive on lore.kernel.org help / color / mirror / Atom feed
* [PATCH v2] Bluetooth: Check for encryption key size on connect @ 2020-09-21 7:55 Archie Pusaka 2020-09-21 17:15 ` Luiz Augusto von Dentz 0 siblings, 1 reply; 5+ messages in thread From: Archie Pusaka @ 2020-09-21 7:55 UTC (permalink / raw) To: linux-bluetooth, Marcel Holtmann Cc: CrosBT Upstreaming, Archie Pusaka, Alain Michaud, David S. Miller, Jakub Kicinski, Johan Hedberg, linux-kernel, netdev From: Archie Pusaka <apusaka@chromium.org> When receiving connection, we only check whether the link has been encrypted, but not the encryption key size of the link. This patch adds check for encryption key size, and reject L2CAP connection which size is below the specified threshold (default 7) with security block. Here is some btmon trace. @ MGMT Event: New Link Key (0x0009) plen 26 {0x0001} [hci0] 5.847722 Store hint: No (0x00) BR/EDR Address: 38:00:25:F7:F1:B0 (OUI 38-00-25) Key type: Unauthenticated Combination key from P-192 (0x04) Link key: 7bf2f68c81305d63a6b0ee2c5a7a34bc PIN length: 0 > HCI Event: Encryption Change (0x08) plen 4 #29 [hci0] 5.871537 Status: Success (0x00) Handle: 256 Encryption: Enabled with E0 (0x01) < HCI Command: Read Encryp... (0x05|0x0008) plen 2 #30 [hci0] 5.871609 Handle: 256 > HCI Event: Command Complete (0x0e) plen 7 #31 [hci0] 5.872524 Read Encryption Key Size (0x05|0x0008) ncmd 1 Status: Success (0x00) Handle: 256 Key size: 3 ////// WITHOUT PATCH ////// > ACL Data RX: Handle 256 flags 0x02 dlen 12 #42 [hci0] 5.895023 L2CAP: Connection Request (0x02) ident 3 len 4 PSM: 4097 (0x1001) Source CID: 64 < ACL Data TX: Handle 256 flags 0x00 dlen 16 #43 [hci0] 5.895213 L2CAP: Connection Response (0x03) ident 3 len 8 Destination CID: 64 Source CID: 64 Result: Connection successful (0x0000) Status: No further information available (0x0000) ////// WITH PATCH ////// > ACL Data RX: Handle 256 flags 0x02 dlen 12 #42 [hci0] 4.887024 L2CAP: Connection Request (0x02) ident 3 len 4 PSM: 4097 (0x1001) Source CID: 64 < ACL Data TX: Handle 256 flags 0x00 dlen 16 #43 [hci0] 4.887127 L2CAP: Connection Response (0x03) ident 3 len 8 Destination CID: 0 Source CID: 64 Result: Connection refused - security block (0x0003) Status: No further information available (0x0000) Signed-off-by: Archie Pusaka <apusaka@chromium.org> Reviewed-by: Alain Michaud <alainm@chromium.org> --- Btw, it looks like the patch sent by Alex Lu with the title [PATCH] Bluetooth: Fix the vulnerable issue on enc key size also solves the exact same issue. Changes in v2: * Add btmon trace to the commit message net/bluetooth/l2cap_core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index ade83e224567..b4fc0ad38aaa 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -4101,7 +4101,8 @@ static struct l2cap_chan *l2cap_connect(struct l2cap_conn *conn, /* Check if the ACL is secure enough (if not SDP) */ if (psm != cpu_to_le16(L2CAP_PSM_SDP) && - !hci_conn_check_link_mode(conn->hcon)) { + (!hci_conn_check_link_mode(conn->hcon) || + !l2cap_check_enc_key_size(conn->hcon))) { conn->disc_reason = HCI_ERROR_AUTH_FAILURE; result = L2CAP_CR_SEC_BLOCK; goto response; -- 2.28.0.681.g6f77f65b4e-goog ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] Bluetooth: Check for encryption key size on connect 2020-09-21 7:55 [PATCH v2] Bluetooth: Check for encryption key size on connect Archie Pusaka @ 2020-09-21 17:15 ` Luiz Augusto von Dentz 2020-09-22 7:47 ` Archie Pusaka 0 siblings, 1 reply; 5+ messages in thread From: Luiz Augusto von Dentz @ 2020-09-21 17:15 UTC (permalink / raw) To: Archie Pusaka Cc: linux-bluetooth, Marcel Holtmann, CrosBT Upstreaming, Archie Pusaka, Alain Michaud, David S. Miller, Jakub Kicinski, Johan Hedberg, Linux Kernel Mailing List, open list:NETWORKING [GENERAL] Hi Archie, On Mon, Sep 21, 2020 at 12:56 AM Archie Pusaka <apusaka@google.com> wrote: > > From: Archie Pusaka <apusaka@chromium.org> > > When receiving connection, we only check whether the link has been > encrypted, but not the encryption key size of the link. > > This patch adds check for encryption key size, and reject L2CAP > connection which size is below the specified threshold (default 7) > with security block. > > Here is some btmon trace. > @ MGMT Event: New Link Key (0x0009) plen 26 {0x0001} [hci0] 5.847722 > Store hint: No (0x00) > BR/EDR Address: 38:00:25:F7:F1:B0 (OUI 38-00-25) > Key type: Unauthenticated Combination key from P-192 (0x04) > Link key: 7bf2f68c81305d63a6b0ee2c5a7a34bc > PIN length: 0 > > HCI Event: Encryption Change (0x08) plen 4 #29 [hci0] 5.871537 > Status: Success (0x00) > Handle: 256 > Encryption: Enabled with E0 (0x01) > < HCI Command: Read Encryp... (0x05|0x0008) plen 2 #30 [hci0] 5.871609 > Handle: 256 > > HCI Event: Command Complete (0x0e) plen 7 #31 [hci0] 5.872524 > Read Encryption Key Size (0x05|0x0008) ncmd 1 > Status: Success (0x00) > Handle: 256 > Key size: 3 > > ////// WITHOUT PATCH ////// > > ACL Data RX: Handle 256 flags 0x02 dlen 12 #42 [hci0] 5.895023 > L2CAP: Connection Request (0x02) ident 3 len 4 > PSM: 4097 (0x1001) > Source CID: 64 > < ACL Data TX: Handle 256 flags 0x00 dlen 16 #43 [hci0] 5.895213 > L2CAP: Connection Response (0x03) ident 3 len 8 > Destination CID: 64 > Source CID: 64 > Result: Connection successful (0x0000) > Status: No further information available (0x0000) > > ////// WITH PATCH ////// > > ACL Data RX: Handle 256 flags 0x02 dlen 12 #42 [hci0] 4.887024 > L2CAP: Connection Request (0x02) ident 3 len 4 > PSM: 4097 (0x1001) > Source CID: 64 > < ACL Data TX: Handle 256 flags 0x00 dlen 16 #43 [hci0] 4.887127 > L2CAP: Connection Response (0x03) ident 3 len 8 > Destination CID: 0 > Source CID: 64 > Result: Connection refused - security block (0x0003) > Status: No further information available (0x0000) > > Signed-off-by: Archie Pusaka <apusaka@chromium.org> > Reviewed-by: Alain Michaud <alainm@chromium.org> > > --- > Btw, it looks like the patch sent by Alex Lu with the title > [PATCH] Bluetooth: Fix the vulnerable issue on enc key size > also solves the exact same issue. > > Changes in v2: > * Add btmon trace to the commit message > > net/bluetooth/l2cap_core.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > index ade83e224567..b4fc0ad38aaa 100644 > --- a/net/bluetooth/l2cap_core.c > +++ b/net/bluetooth/l2cap_core.c > @@ -4101,7 +4101,8 @@ static struct l2cap_chan *l2cap_connect(struct l2cap_conn *conn, > > /* Check if the ACL is secure enough (if not SDP) */ > if (psm != cpu_to_le16(L2CAP_PSM_SDP) && > - !hci_conn_check_link_mode(conn->hcon)) { > + (!hci_conn_check_link_mode(conn->hcon) || > + !l2cap_check_enc_key_size(conn->hcon))) { I wonder if we couldn't incorporate the check of key size into hci_conn_check_link_mode, like I said in the first patch checking the enc key size should not be specific to L2CAP. > conn->disc_reason = HCI_ERROR_AUTH_FAILURE; > result = L2CAP_CR_SEC_BLOCK; > goto response; > -- > 2.28.0.681.g6f77f65b4e-goog > -- Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] Bluetooth: Check for encryption key size on connect 2020-09-21 17:15 ` Luiz Augusto von Dentz @ 2020-09-22 7:47 ` Archie Pusaka 2020-09-22 17:03 ` Luiz Augusto von Dentz 0 siblings, 1 reply; 5+ messages in thread From: Archie Pusaka @ 2020-09-22 7:47 UTC (permalink / raw) To: Luiz Augusto von Dentz Cc: linux-bluetooth, Marcel Holtmann, CrosBT Upstreaming, Archie Pusaka, Alain Michaud, David S. Miller, Jakub Kicinski, Johan Hedberg, Linux Kernel Mailing List, open list:NETWORKING [GENERAL] Hi Luiz, On Tue, 22 Sep 2020 at 01:15, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > Hi Archie, > > > On Mon, Sep 21, 2020 at 12:56 AM Archie Pusaka <apusaka@google.com> wrote: > > > > From: Archie Pusaka <apusaka@chromium.org> > > > > When receiving connection, we only check whether the link has been > > encrypted, but not the encryption key size of the link. > > > > This patch adds check for encryption key size, and reject L2CAP > > connection which size is below the specified threshold (default 7) > > with security block. > > > > Here is some btmon trace. > > @ MGMT Event: New Link Key (0x0009) plen 26 {0x0001} [hci0] 5.847722 > > Store hint: No (0x00) > > BR/EDR Address: 38:00:25:F7:F1:B0 (OUI 38-00-25) > > Key type: Unauthenticated Combination key from P-192 (0x04) > > Link key: 7bf2f68c81305d63a6b0ee2c5a7a34bc > > PIN length: 0 > > > HCI Event: Encryption Change (0x08) plen 4 #29 [hci0] 5.871537 > > Status: Success (0x00) > > Handle: 256 > > Encryption: Enabled with E0 (0x01) > > < HCI Command: Read Encryp... (0x05|0x0008) plen 2 #30 [hci0] 5.871609 > > Handle: 256 > > > HCI Event: Command Complete (0x0e) plen 7 #31 [hci0] 5.872524 > > Read Encryption Key Size (0x05|0x0008) ncmd 1 > > Status: Success (0x00) > > Handle: 256 > > Key size: 3 > > > > ////// WITHOUT PATCH ////// > > > ACL Data RX: Handle 256 flags 0x02 dlen 12 #42 [hci0] 5.895023 > > L2CAP: Connection Request (0x02) ident 3 len 4 > > PSM: 4097 (0x1001) > > Source CID: 64 > > < ACL Data TX: Handle 256 flags 0x00 dlen 16 #43 [hci0] 5.895213 > > L2CAP: Connection Response (0x03) ident 3 len 8 > > Destination CID: 64 > > Source CID: 64 > > Result: Connection successful (0x0000) > > Status: No further information available (0x0000) > > > > ////// WITH PATCH ////// > > > ACL Data RX: Handle 256 flags 0x02 dlen 12 #42 [hci0] 4.887024 > > L2CAP: Connection Request (0x02) ident 3 len 4 > > PSM: 4097 (0x1001) > > Source CID: 64 > > < ACL Data TX: Handle 256 flags 0x00 dlen 16 #43 [hci0] 4.887127 > > L2CAP: Connection Response (0x03) ident 3 len 8 > > Destination CID: 0 > > Source CID: 64 > > Result: Connection refused - security block (0x0003) > > Status: No further information available (0x0000) > > > > Signed-off-by: Archie Pusaka <apusaka@chromium.org> > > Reviewed-by: Alain Michaud <alainm@chromium.org> > > > > --- > > Btw, it looks like the patch sent by Alex Lu with the title > > [PATCH] Bluetooth: Fix the vulnerable issue on enc key size > > also solves the exact same issue. > > > > Changes in v2: > > * Add btmon trace to the commit message > > > > net/bluetooth/l2cap_core.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > > index ade83e224567..b4fc0ad38aaa 100644 > > --- a/net/bluetooth/l2cap_core.c > > +++ b/net/bluetooth/l2cap_core.c > > @@ -4101,7 +4101,8 @@ static struct l2cap_chan *l2cap_connect(struct l2cap_conn *conn, > > > > /* Check if the ACL is secure enough (if not SDP) */ > > if (psm != cpu_to_le16(L2CAP_PSM_SDP) && > > - !hci_conn_check_link_mode(conn->hcon)) { > > + (!hci_conn_check_link_mode(conn->hcon) || > > + !l2cap_check_enc_key_size(conn->hcon))) { > > I wonder if we couldn't incorporate the check of key size into > hci_conn_check_link_mode, like I said in the first patch checking the > enc key size should not be specific to L2CAP. Yes, I could move the check into hci_conn_check_link_mode. At first look, this function is also called by AMP which I am not familiar with. In addition, I found this patch which moves this check outside hci_conn, so I have my doubts there. https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/commit/?id=693cd8ce3f882524a5d06f7800dd8492411877b3 > > > conn->disc_reason = HCI_ERROR_AUTH_FAILURE; > > result = L2CAP_CR_SEC_BLOCK; > > goto response; > > -- > > 2.28.0.681.g6f77f65b4e-goog > > > > > -- > Luiz Augusto von Dentz Thanks, Archie ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] Bluetooth: Check for encryption key size on connect 2020-09-22 7:47 ` Archie Pusaka @ 2020-09-22 17:03 ` Luiz Augusto von Dentz 2020-09-25 12:03 ` Archie Pusaka 0 siblings, 1 reply; 5+ messages in thread From: Luiz Augusto von Dentz @ 2020-09-22 17:03 UTC (permalink / raw) To: Archie Pusaka Cc: linux-bluetooth, Marcel Holtmann, CrosBT Upstreaming, Archie Pusaka, Alain Michaud, David S. Miller, Jakub Kicinski, Johan Hedberg, Linux Kernel Mailing List, open list:NETWORKING [GENERAL] Hi Archie, On Tue, Sep 22, 2020 at 12:48 AM Archie Pusaka <apusaka@google.com> wrote: > > Hi Luiz, > > On Tue, 22 Sep 2020 at 01:15, Luiz Augusto von Dentz > <luiz.dentz@gmail.com> wrote: > > > > Hi Archie, > > > > > > On Mon, Sep 21, 2020 at 12:56 AM Archie Pusaka <apusaka@google.com> wrote: > > > > > > From: Archie Pusaka <apusaka@chromium.org> > > > > > > When receiving connection, we only check whether the link has been > > > encrypted, but not the encryption key size of the link. > > > > > > This patch adds check for encryption key size, and reject L2CAP > > > connection which size is below the specified threshold (default 7) > > > with security block. > > > > > > Here is some btmon trace. > > > @ MGMT Event: New Link Key (0x0009) plen 26 {0x0001} [hci0] 5.847722 > > > Store hint: No (0x00) > > > BR/EDR Address: 38:00:25:F7:F1:B0 (OUI 38-00-25) > > > Key type: Unauthenticated Combination key from P-192 (0x04) > > > Link key: 7bf2f68c81305d63a6b0ee2c5a7a34bc > > > PIN length: 0 > > > > HCI Event: Encryption Change (0x08) plen 4 #29 [hci0] 5.871537 > > > Status: Success (0x00) > > > Handle: 256 > > > Encryption: Enabled with E0 (0x01) > > > < HCI Command: Read Encryp... (0x05|0x0008) plen 2 #30 [hci0] 5.871609 > > > Handle: 256 > > > > HCI Event: Command Complete (0x0e) plen 7 #31 [hci0] 5.872524 > > > Read Encryption Key Size (0x05|0x0008) ncmd 1 > > > Status: Success (0x00) > > > Handle: 256 > > > Key size: 3 > > > > > > ////// WITHOUT PATCH ////// > > > > ACL Data RX: Handle 256 flags 0x02 dlen 12 #42 [hci0] 5.895023 > > > L2CAP: Connection Request (0x02) ident 3 len 4 > > > PSM: 4097 (0x1001) > > > Source CID: 64 > > > < ACL Data TX: Handle 256 flags 0x00 dlen 16 #43 [hci0] 5.895213 > > > L2CAP: Connection Response (0x03) ident 3 len 8 > > > Destination CID: 64 > > > Source CID: 64 > > > Result: Connection successful (0x0000) > > > Status: No further information available (0x0000) > > > > > > ////// WITH PATCH ////// > > > > ACL Data RX: Handle 256 flags 0x02 dlen 12 #42 [hci0] 4.887024 > > > L2CAP: Connection Request (0x02) ident 3 len 4 > > > PSM: 4097 (0x1001) > > > Source CID: 64 > > > < ACL Data TX: Handle 256 flags 0x00 dlen 16 #43 [hci0] 4.887127 > > > L2CAP: Connection Response (0x03) ident 3 len 8 > > > Destination CID: 0 > > > Source CID: 64 > > > Result: Connection refused - security block (0x0003) > > > Status: No further information available (0x0000) > > > > > > Signed-off-by: Archie Pusaka <apusaka@chromium.org> > > > Reviewed-by: Alain Michaud <alainm@chromium.org> > > > > > > --- > > > Btw, it looks like the patch sent by Alex Lu with the title > > > [PATCH] Bluetooth: Fix the vulnerable issue on enc key size > > > also solves the exact same issue. > > > > > > Changes in v2: > > > * Add btmon trace to the commit message > > > > > > net/bluetooth/l2cap_core.c | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > > > index ade83e224567..b4fc0ad38aaa 100644 > > > --- a/net/bluetooth/l2cap_core.c > > > +++ b/net/bluetooth/l2cap_core.c > > > @@ -4101,7 +4101,8 @@ static struct l2cap_chan *l2cap_connect(struct l2cap_conn *conn, > > > > > > /* Check if the ACL is secure enough (if not SDP) */ > > > if (psm != cpu_to_le16(L2CAP_PSM_SDP) && > > > - !hci_conn_check_link_mode(conn->hcon)) { > > > + (!hci_conn_check_link_mode(conn->hcon) || > > > + !l2cap_check_enc_key_size(conn->hcon))) { > > > > I wonder if we couldn't incorporate the check of key size into > > hci_conn_check_link_mode, like I said in the first patch checking the > > enc key size should not be specific to L2CAP. > > Yes, I could move the check into hci_conn_check_link_mode. > At first look, this function is also called by AMP which I am not > familiar with. In addition, I found this patch which moves this check > outside hci_conn, so I have my doubts there. > https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/commit/?id=693cd8ce3f882524a5d06f7800dd8492411877b3 Right, I think we can have it as part of the hci_conn_check_link_mode, that said it is perhaps better to have it as hci_conn_check_enc_key_size instead as it is not L2CAP expecific. Other than that it looks good to me. > > > > > conn->disc_reason = HCI_ERROR_AUTH_FAILURE; > > > result = L2CAP_CR_SEC_BLOCK; > > > goto response; > > > -- > > > 2.28.0.681.g6f77f65b4e-goog > > > > > > > > > -- > > Luiz Augusto von Dentz > > Thanks, > Archie -- Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] Bluetooth: Check for encryption key size on connect 2020-09-22 17:03 ` Luiz Augusto von Dentz @ 2020-09-25 12:03 ` Archie Pusaka 0 siblings, 0 replies; 5+ messages in thread From: Archie Pusaka @ 2020-09-25 12:03 UTC (permalink / raw) To: Luiz Augusto von Dentz Cc: linux-bluetooth, Marcel Holtmann, CrosBT Upstreaming, Archie Pusaka, Alain Michaud, David S. Miller, Jakub Kicinski, Johan Hedberg, Linux Kernel Mailing List, open list:NETWORKING [GENERAL] Hi Luiz, On Wed, 23 Sep 2020 at 01:03, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > Hi Archie, > > On Tue, Sep 22, 2020 at 12:48 AM Archie Pusaka <apusaka@google.com> wrote: > > > > Hi Luiz, > > > > On Tue, 22 Sep 2020 at 01:15, Luiz Augusto von Dentz > > <luiz.dentz@gmail.com> wrote: > > > > > > Hi Archie, > > > > > > > > > On Mon, Sep 21, 2020 at 12:56 AM Archie Pusaka <apusaka@google.com> wrote: > > > > > > > > From: Archie Pusaka <apusaka@chromium.org> > > > > > > > > When receiving connection, we only check whether the link has been > > > > encrypted, but not the encryption key size of the link. > > > > > > > > This patch adds check for encryption key size, and reject L2CAP > > > > connection which size is below the specified threshold (default 7) > > > > with security block. > > > > > > > > Here is some btmon trace. > > > > @ MGMT Event: New Link Key (0x0009) plen 26 {0x0001} [hci0] 5.847722 > > > > Store hint: No (0x00) > > > > BR/EDR Address: 38:00:25:F7:F1:B0 (OUI 38-00-25) > > > > Key type: Unauthenticated Combination key from P-192 (0x04) > > > > Link key: 7bf2f68c81305d63a6b0ee2c5a7a34bc > > > > PIN length: 0 > > > > > HCI Event: Encryption Change (0x08) plen 4 #29 [hci0] 5.871537 > > > > Status: Success (0x00) > > > > Handle: 256 > > > > Encryption: Enabled with E0 (0x01) > > > > < HCI Command: Read Encryp... (0x05|0x0008) plen 2 #30 [hci0] 5.871609 > > > > Handle: 256 > > > > > HCI Event: Command Complete (0x0e) plen 7 #31 [hci0] 5.872524 > > > > Read Encryption Key Size (0x05|0x0008) ncmd 1 > > > > Status: Success (0x00) > > > > Handle: 256 > > > > Key size: 3 > > > > > > > > ////// WITHOUT PATCH ////// > > > > > ACL Data RX: Handle 256 flags 0x02 dlen 12 #42 [hci0] 5.895023 > > > > L2CAP: Connection Request (0x02) ident 3 len 4 > > > > PSM: 4097 (0x1001) > > > > Source CID: 64 > > > > < ACL Data TX: Handle 256 flags 0x00 dlen 16 #43 [hci0] 5.895213 > > > > L2CAP: Connection Response (0x03) ident 3 len 8 > > > > Destination CID: 64 > > > > Source CID: 64 > > > > Result: Connection successful (0x0000) > > > > Status: No further information available (0x0000) > > > > > > > > ////// WITH PATCH ////// > > > > > ACL Data RX: Handle 256 flags 0x02 dlen 12 #42 [hci0] 4.887024 > > > > L2CAP: Connection Request (0x02) ident 3 len 4 > > > > PSM: 4097 (0x1001) > > > > Source CID: 64 > > > > < ACL Data TX: Handle 256 flags 0x00 dlen 16 #43 [hci0] 4.887127 > > > > L2CAP: Connection Response (0x03) ident 3 len 8 > > > > Destination CID: 0 > > > > Source CID: 64 > > > > Result: Connection refused - security block (0x0003) > > > > Status: No further information available (0x0000) > > > > > > > > Signed-off-by: Archie Pusaka <apusaka@chromium.org> > > > > Reviewed-by: Alain Michaud <alainm@chromium.org> > > > > > > > > --- > > > > Btw, it looks like the patch sent by Alex Lu with the title > > > > [PATCH] Bluetooth: Fix the vulnerable issue on enc key size > > > > also solves the exact same issue. > > > > > > > > Changes in v2: > > > > * Add btmon trace to the commit message > > > > > > > > net/bluetooth/l2cap_core.c | 3 ++- > > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > > > > index ade83e224567..b4fc0ad38aaa 100644 > > > > --- a/net/bluetooth/l2cap_core.c > > > > +++ b/net/bluetooth/l2cap_core.c > > > > @@ -4101,7 +4101,8 @@ static struct l2cap_chan *l2cap_connect(struct l2cap_conn *conn, > > > > > > > > /* Check if the ACL is secure enough (if not SDP) */ > > > > if (psm != cpu_to_le16(L2CAP_PSM_SDP) && > > > > - !hci_conn_check_link_mode(conn->hcon)) { > > > > + (!hci_conn_check_link_mode(conn->hcon) || > > > > + !l2cap_check_enc_key_size(conn->hcon))) { > > > > > > I wonder if we couldn't incorporate the check of key size into > > > hci_conn_check_link_mode, like I said in the first patch checking the > > > enc key size should not be specific to L2CAP. > > > > Yes, I could move the check into hci_conn_check_link_mode. > > At first look, this function is also called by AMP which I am not > > familiar with. In addition, I found this patch which moves this check > > outside hci_conn, so I have my doubts there. > > https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/commit/?id=693cd8ce3f882524a5d06f7800dd8492411877b3 > > Right, I think we can have it as part of the hci_conn_check_link_mode, > that said it is perhaps better to have it as > hci_conn_check_enc_key_size instead as it is not L2CAP expecific. > Other than that it looks good to me. Do you mean we should move l2cap_conn_check_enc_key_size to hci_conn_check_enc_key_size? I think that is a good idea. We also have hci_conn_check_secure which I am unsure what the purpose is. I'll try to merge them together. > > > > > > > > conn->disc_reason = HCI_ERROR_AUTH_FAILURE; > > > > result = L2CAP_CR_SEC_BLOCK; > > > > goto response; > > > > -- > > > > 2.28.0.681.g6f77f65b4e-goog > > > > > > > > > > > > > -- > > > Luiz Augusto von Dentz > > > > Thanks, > > Archie > > > > -- > Luiz Augusto von Dentz Thanks, Archie ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-09-25 12:03 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-09-21 7:55 [PATCH v2] Bluetooth: Check for encryption key size on connect Archie Pusaka 2020-09-21 17:15 ` Luiz Augusto von Dentz 2020-09-22 7:47 ` Archie Pusaka 2020-09-22 17:03 ` Luiz Augusto von Dentz 2020-09-25 12:03 ` Archie Pusaka
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).