| Message ID | 20200519202519.219335-1-luiz.dentz@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Marcel Holtmann |
| Headers | show |
| Series | [1/4] Bluetooth: Disconnect if E0 is used for Level 4 | expand |
Hi Luiz, > E0 is not allowed with Level 4: > > BLUETOOTH CORE SPECIFICATION Version 5.2 | Vol 3, Part C page 1319: > > '128-bit equivalent strength for link and encryption keys > required using FIPS approved algorithms (E0 not allowed, > SAFER+ not allowed, and P-192 not allowed; encryption key > not shortened' > > SC enabled: > >> HCI Event: Read Remote Extended Features (0x23) plen 13 > Status: Success (0x00) > Handle: 256 > Page: 1/2 > Features: 0x0b 0x00 0x00 0x00 0x00 0x00 0x00 0x00 > Secure Simple Pairing (Host Support) > LE Supported (Host) > Secure Connections (Host Support) >> HCI Event: Encryption Change (0x08) plen 4 > Status: Success (0x00) > Handle: 256 > Encryption: Enabled with AES-CCM (0x02) > > SC disabled: > >> HCI Event: Read Remote Extended Features (0x23) plen 13 > Status: Success (0x00) > Handle: 256 > Page: 1/2 > Features: 0x03 0x00 0x00 0x00 0x00 0x00 0x00 0x00 > Secure Simple Pairing (Host Support) > LE Supported (Host) >> HCI Event: Encryption Change (0x08) plen 4 > Status: Success (0x00) > Handle: 256 > Encryption: Enabled with E0 (0x01) > [May 8 20:23] Bluetooth: hci0: Invalid security: expect AES but E0 was used > < HCI Command: Disconnect (0x01|0x0006) plen 3 > Handle: 256 > Reason: Authentication Failure (0x05) > > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > --- > net/bluetooth/hci_conn.c | 17 +++++++++++++++++ > net/bluetooth/hci_event.c | 6 ++---- > 2 files changed, 19 insertions(+), 4 deletions(-) > > diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c > index 07c34c55fc50..0c1cae83c8dc 100644 > --- a/net/bluetooth/hci_conn.c > +++ b/net/bluetooth/hci_conn.c > @@ -1325,6 +1325,23 @@ int hci_conn_check_link_mode(struct hci_conn *conn) > return 0; > } > > + /* AES encryption is required for Level 4: > + * > + * BLUETOOTH CORE SPECIFICATION Version 5.2 | Vol 3, Part C > + * page 1319: > + * > + * 128-bit equivalent strength for link and encryption keys > + * required using FIPS approved algorithms (E0 not allowed, > + * SAFER+ not allowed, and P-192 not allowed; encryption key > + * not shortened) > + */ > + if (conn->sec_level == BT_SECURITY_FIPS && > + !test_bit(HCI_CONN_AES_CCM, &conn->flags)) { > + bt_dev_err(conn->hdev, "Invalid security: expect AES but E0 " > + "was used"); make this bt_dev_warn since it is something that can happen. We fail appropriately now. Also try to fit the error in a single line. “Invalid security: Missing AES-CCM usage”. > + return 0; > + } > + > if (hci_conn_ssp_enabled(conn) && > !test_bit(HCI_CONN_ENCRYPT, &conn->flags)) > return 0; > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c > index 006c24e04b44..dc1cc3c4348c 100644 > --- a/net/bluetooth/hci_event.c > +++ b/net/bluetooth/hci_event.c > @@ -3078,10 +3078,8 @@ static void hci_encrypt_change_evt(struct hci_dev *hdev, struct sk_buff *skb) > * that are not encrypted with AES-CCM using a P-256 authenticated > * combination key. > */ > - if (hci_dev_test_flag(hdev, HCI_SC_ONLY) && > - (!test_bit(HCI_CONN_AES_CCM, &conn->flags) || > - conn->key_type != HCI_LK_AUTH_COMBINATION_P256)) { > - hci_connect_cfm(conn, HCI_ERROR_AUTH_FAILURE); > + if (!hci_conn_check_link_mode(conn)) { > + hci_disconnect(conn, HCI_ERROR_AUTH_FAILURE); Why is this a disconnect now? If this is better, then this needs to be explained above, but I have the feeling we really want to respond to the connect request in a clean fashion. > hci_conn_drop(conn); > goto unlock; Regards Marcel
Hi Marcel, On Wed, May 20, 2020 at 7:23 AM Marcel Holtmann <marcel@holtmann.org> wrote: > > Hi Luiz, > > > E0 is not allowed with Level 4: > > > > BLUETOOTH CORE SPECIFICATION Version 5.2 | Vol 3, Part C page 1319: > > > > '128-bit equivalent strength for link and encryption keys > > required using FIPS approved algorithms (E0 not allowed, > > SAFER+ not allowed, and P-192 not allowed; encryption key > > not shortened' > > > > SC enabled: > > > >> HCI Event: Read Remote Extended Features (0x23) plen 13 > > Status: Success (0x00) > > Handle: 256 > > Page: 1/2 > > Features: 0x0b 0x00 0x00 0x00 0x00 0x00 0x00 0x00 > > Secure Simple Pairing (Host Support) > > LE Supported (Host) > > Secure Connections (Host Support) > >> HCI Event: Encryption Change (0x08) plen 4 > > Status: Success (0x00) > > Handle: 256 > > Encryption: Enabled with AES-CCM (0x02) > > > > SC disabled: > > > >> HCI Event: Read Remote Extended Features (0x23) plen 13 > > Status: Success (0x00) > > Handle: 256 > > Page: 1/2 > > Features: 0x03 0x00 0x00 0x00 0x00 0x00 0x00 0x00 > > Secure Simple Pairing (Host Support) > > LE Supported (Host) > >> HCI Event: Encryption Change (0x08) plen 4 > > Status: Success (0x00) > > Handle: 256 > > Encryption: Enabled with E0 (0x01) > > [May 8 20:23] Bluetooth: hci0: Invalid security: expect AES but E0 was used > > < HCI Command: Disconnect (0x01|0x0006) plen 3 > > Handle: 256 > > Reason: Authentication Failure (0x05) > > > > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > --- > > net/bluetooth/hci_conn.c | 17 +++++++++++++++++ > > net/bluetooth/hci_event.c | 6 ++---- > > 2 files changed, 19 insertions(+), 4 deletions(-) > > > > diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c > > index 07c34c55fc50..0c1cae83c8dc 100644 > > --- a/net/bluetooth/hci_conn.c > > +++ b/net/bluetooth/hci_conn.c > > @@ -1325,6 +1325,23 @@ int hci_conn_check_link_mode(struct hci_conn *conn) > > return 0; > > } > > > > + /* AES encryption is required for Level 4: > > + * > > + * BLUETOOTH CORE SPECIFICATION Version 5.2 | Vol 3, Part C > > + * page 1319: > > + * > > + * 128-bit equivalent strength for link and encryption keys > > + * required using FIPS approved algorithms (E0 not allowed, > > + * SAFER+ not allowed, and P-192 not allowed; encryption key > > + * not shortened) > > + */ > > + if (conn->sec_level == BT_SECURITY_FIPS && > > + !test_bit(HCI_CONN_AES_CCM, &conn->flags)) { > > + bt_dev_err(conn->hdev, "Invalid security: expect AES but E0 " > > + "was used"); > > make this bt_dev_warn since it is something that can happen. We fail appropriately now. Also try to fit the error in a single line. “Invalid security: Missing AES-CCM usage”. Sure will update this. > > + return 0; > > + } > > + > > if (hci_conn_ssp_enabled(conn) && > > !test_bit(HCI_CONN_ENCRYPT, &conn->flags)) > > return 0; > > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c > > index 006c24e04b44..dc1cc3c4348c 100644 > > --- a/net/bluetooth/hci_event.c > > +++ b/net/bluetooth/hci_event.c > > @@ -3078,10 +3078,8 @@ static void hci_encrypt_change_evt(struct hci_dev *hdev, struct sk_buff *skb) > > * that are not encrypted with AES-CCM using a P-256 authenticated > > * combination key. > > */ > > - if (hci_dev_test_flag(hdev, HCI_SC_ONLY) && > > - (!test_bit(HCI_CONN_AES_CCM, &conn->flags) || > > - conn->key_type != HCI_LK_AUTH_COMBINATION_P256)) { > > - hci_connect_cfm(conn, HCI_ERROR_AUTH_FAILURE); > > + if (!hci_conn_check_link_mode(conn)) { > > + hci_disconnect(conn, HCI_ERROR_AUTH_FAILURE); > > Why is this a disconnect now? If this is better, then this needs to be explained above, but I have the feeling we really want to respond to the connect request in a clean fashion. While testing this I discover that calling connect_cfm doesn't actually cause a disconnect, at least not immediately, and by disconnecting it actually provides the right error to the remote. Note that there is a similar usage in the if statement right above this code, but I believe that even in case we want to cleanup the earlier what we should be calling is hci_encrypt_cfm. > > hci_conn_drop(conn); > > goto unlock; > > Regards > > Marcel >
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index 07c34c55fc50..0c1cae83c8dc 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -1325,6 +1325,23 @@ int hci_conn_check_link_mode(struct hci_conn *conn) return 0; } + /* AES encryption is required for Level 4: + * + * BLUETOOTH CORE SPECIFICATION Version 5.2 | Vol 3, Part C + * page 1319: + * + * 128-bit equivalent strength for link and encryption keys + * required using FIPS approved algorithms (E0 not allowed, + * SAFER+ not allowed, and P-192 not allowed; encryption key + * not shortened) + */ + if (conn->sec_level == BT_SECURITY_FIPS && + !test_bit(HCI_CONN_AES_CCM, &conn->flags)) { + bt_dev_err(conn->hdev, "Invalid security: expect AES but E0 " + "was used"); + return 0; + } + if (hci_conn_ssp_enabled(conn) && !test_bit(HCI_CONN_ENCRYPT, &conn->flags)) return 0; diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 006c24e04b44..dc1cc3c4348c 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -3078,10 +3078,8 @@ static void hci_encrypt_change_evt(struct hci_dev *hdev, struct sk_buff *skb) * that are not encrypted with AES-CCM using a P-256 authenticated * combination key. */ - if (hci_dev_test_flag(hdev, HCI_SC_ONLY) && - (!test_bit(HCI_CONN_AES_CCM, &conn->flags) || - conn->key_type != HCI_LK_AUTH_COMBINATION_P256)) { - hci_connect_cfm(conn, HCI_ERROR_AUTH_FAILURE); + if (!hci_conn_check_link_mode(conn)) { + hci_disconnect(conn, HCI_ERROR_AUTH_FAILURE); hci_conn_drop(conn); goto unlock; }