l2cap: allow opening authenticated channel after unauthenticated.

bt-embedded/acl.c

@@ -273,6 +273,9 @@ static void auth_requested_cb(
     BteHci *hci, const BteHciAuthRequestedReply *reply, void *userdata)
 {
     BteAcl *acl = userdata;
+    if (reply->status == 0) {
+        acl->authentication_completed = true;
+    }
     acl->connected_cb(acl, reply->status);
 }
 
@@ -283,9 +286,8 @@ static void connect_cb(BteHci *hci, const BteHciCreateConnectionReply *reply,
     if (reply->status == 0) {
         acl->conn_handle = reply->conn_handle;
         acl->encryption_mode = reply->encryption_mode;
-        if (acl->authentication_requested) {
-            bte_hci_auth_requested(acl->hci, acl->conn_handle, NULL,
-                                   auth_requested_cb, acl);
+        if (acl->authentication_required && !acl->authentication_completed) {
+            bte_acl_request_auth(acl);
             return;
         }
     }
@@ -296,7 +298,7 @@ void bte_acl_connect(BteAcl *acl, const BteHciConnectParams *params,
                      BteAclConnectFlags flags)
 {
     if (flags & BTE_ACL_CONNECT_FLAG_AUTH) {
-        acl->authentication_requested = true;
+        acl->authentication_required = true;
     }
     bte_hci_create_connection(acl->hci, &acl->address, params,
                               connect_status_cb, connect_cb, acl);
@@ -315,6 +317,15 @@ void bte_acl_disconnect(BteAcl *acl)
                        NULL, NULL);
 }
 
+void bte_acl_request_auth(BteAcl *acl)
+{
+    if (!acl->authentication_requested) {
+        acl->authentication_requested = true;
+        bte_hci_auth_requested(acl->hci, acl->conn_handle, NULL,
+                               auth_requested_cb, acl);
+    }
+}
+
 bool bte_acl_create_message(BteAcl *acl, BteBufferWriter *writer, uint16_t size,
                             uint8_t broadcast)
 {

bt-embedded/acl.h

@@ -25,7 +25,9 @@ struct bte_acl_t {
     BteConnHandle conn_handle;
     uint8_t encryption_mode;
     bool is_incoming : 1;
+    bool authentication_required : 1;
     bool authentication_requested : 1;
+    bool authentication_completed : 1;
 
     BteBuffer *fragmented_message;
     uint16_t fragmented_message_size;
@@ -59,6 +61,8 @@ void bte_acl_connect(BteAcl *acl, const BteHciConnectParams *params,
                      BteAclConnectFlags flags);
 void bte_acl_disconnect(BteAcl *acl);
 
+void bte_acl_request_auth(BteAcl *acl);
+
 #define BTE_ACL_BROADCAST_PTP    (uint8_t)0
 #define BTE_ACL_BROADCAST_ACTIVE (uint8_t)1
 #define BTE_ACL_BROADCAST_PARKED (uint8_t)2

bt-embedded/l2cap.c

@@ -192,6 +192,8 @@ static void l2cap_handle_error(BteL2cap *l2cap)
 
 static bool l2cap_connection_request(BteL2cap *l2cap)
 {
+    if (l2cap->state > BTE_L2CAP_WAIT_CONNECT) return true;
+
     uint16_t data[2];
     data[0] = htole16(l2cap->psm);
     l2cap->local_channel_id = next_local_channel_id();
@@ -1449,6 +1451,10 @@ void bte_l2cap_new_outgoing(BteClient *client, const BteBdAddr *address,
     BteAcl *acl = bte_acl_get_for_address(hci, address);
     if (acl) {
         l2cap->acl = bte_acl_ref(acl);
+        if (flags & BTE_L2CAP_CONNECT_FLAG_AUTH) {
+            acl->authentication_required = true;
+            bte_acl_request_auth(l2cap->acl);
+        }
     } else {
         acl = bte_acl_new(hci, address, sizeof(BteAclL2cap));
         l2cap_setup_acl(acl);
@@ -1470,7 +1476,8 @@ void bte_l2cap_new_outgoing(BteClient *client, const BteBdAddr *address,
     l2cap->userdata = userdata;
     l2cap->psm = psm;
     l2cap->cmd_data.connect.client_cb = callback;
-    if (acl->conn_handle != BTE_CONN_HANDLE_INVALID) {
+    if (acl->conn_handle != BTE_CONN_HANDLE_INVALID &&
+        acl->authentication_completed == acl->authentication_required) {
         bool ok = l2cap_connection_request(l2cap);
         if (UNLIKELY(!ok)) {
             bte_l2cap_unref(l2cap);

tests/bte_l2cap_cpp.h

@@ -99,7 +99,7 @@ class L2cap {
 
     L2cap &operator=(const L2cap &other) {
         if (m_l2cap) bte_l2cap_unref(m_l2cap);
-        m_l2cap = bte_l2cap_ref(other.m_l2cap);
+        m_l2cap = other.m_l2cap ? bte_l2cap_ref(other.m_l2cap) : nullptr;
         return *this;
     }
 

tests/test_l2cap_connect.cpp

@@ -169,6 +169,126 @@ TEST_F(TestL2capConnect, testOutgoingWithAuthentication) {
     ASSERT_EQ(replies, expectedReplies);
 }
 
+TEST_F(TestL2capConnect, testOutgoingWithAddedAuthentication) {
+    /* We want to test that if a L2CAP channel is created with the
+     * BTE_L2CAP_CONNECT_FLAG_AUTH flag after another L2CAP channel has been
+     * created on the same ACL without authentication, the authentication is
+     * performed.
+     * This can be the case with HID devices, where the SDP channel might be
+     * opened on an unauthenticated ACL, followed by the authenticated HID
+     * channels. */
+    using L = Bte::L2cap;
+    BteBdAddr address = {1, 2, 3, 4, 5, 6};
+    std::vector<BteL2capConnectionResponse> replies;
+    Bte::L2cap createdChannel;
+    auto onConnected = [&](std::optional<Bte::L2cap> l2cap,
+                           const BteL2capConnectionResponse &reply) {
+        ASSERT_TRUE(l2cap.has_value());
+        createdChannel = l2cap.value();
+        replies.push_back(reply);
+    };
+    BteL2capPsm psm = BTE_L2CAP_PSM_SDP;
+    BteL2CapConnectFlags flags = BTE_L2CAP_CONNECT_FLAG_NONE;
+    L::newOutgoing(m_client, address, psm, {}, flags, onConnected);
+
+    /* Default values */
+    BtePacketType packetType = BTE_PACKET_TYPE_DM1 | BTE_PACKET_TYPE_DH1;
+    uint16_t clockOffset = 0;
+    uint8_t pageScanRepMode = 1;
+    bool roleSwitch = true;
+    std::vector<Buffer> expectedCommands {
+        makeHciCreateConnection(address, packetType, pageScanRepMode,
+                                clockOffset, roleSwitch),
+    };
+    ASSERT_EQ(m_backend.sentCommands(), expectedCommands);
+
+    /* Send the status reply for HCI create connection */
+    uint8_t status = 0;
+    m_backend.sendEvent({HCI_COMMAND_STATUS, 4, status, 1, 0x5, 0x4});
+    bte_handle_events();
+    /* Send the actual reply */
+    sendHciConnectionComplete(address);
+    bte_handle_events();
+
+    /* Read the L2CAP connection request */
+    uint8_t reqId = m_cmdId++;
+    Buffer expectedData = makeConnectRequest(reqId, m_connHandle, psm);
+    ASSERT_EQ(m_backend.lastData(), expectedData);
+
+    /* Send the L2cap connect response */
+    sendConnectResponse(reqId);
+    bte_handle_events();
+    Bte::L2cap sdpChannel(createdChannel);
+
+    std::vector<BteL2capConnectionResponse> expectedReplies = {
+        {0x40, 0x40, 0, 0},
+    };
+    ASSERT_EQ(replies, expectedReplies);
+    m_backend.clear();
+    replies.clear();
+
+    /* Now create a new L2CAP channel, requiring authentication */
+    m_remoteCid++;
+    m_localCid++;
+    psm = BTE_L2CAP_PSM_HID_CTRL;
+    flags = BTE_L2CAP_CONNECT_FLAG_AUTH;
+    L::newOutgoing(m_client, address, psm, {}, flags, onConnected);
+
+    /* We should have requested the authentication */
+    expectedCommands = { makeHciAuthRequested(m_connHandle) };
+    ASSERT_EQ(m_backend.sentCommands(), expectedCommands);
+
+    /* Send the command status... */
+    m_backend.sendEvent({HCI_COMMAND_STATUS, 4, status, 1, 0x11, 0x4});
+    bte_handle_events();
+    /* ..And the reply */
+    sendHciAuthComplete(m_connHandle);
+    bte_handle_events();
+
+    /* Read the L2CAP connection request */
+    reqId = m_cmdId++;
+    expectedData = makeConnectRequest(reqId, m_connHandle, psm);
+    ASSERT_EQ(m_backend.lastData(), expectedData);
+
+    /* Send the L2cap connect response */
+    sendConnectResponse(reqId);
+    bte_handle_events();
+    Bte::L2cap hidCtrlChannel = createdChannel;
+
+    expectedReplies = {
+        {0x41, 0x41, 0, 0},
+    };
+    ASSERT_EQ(replies, expectedReplies);
+    m_backend.clear();
+    replies.clear();
+
+    /* Open yet another channel, it should just work no matter whether the
+     * authentication flag is set. */
+    m_remoteCid++;
+    m_localCid++;
+    psm = BTE_L2CAP_PSM_HID_INTR;
+    flags = BTE_L2CAP_CONNECT_FLAG_NONE;
+    L::newOutgoing(m_client, address, psm, {}, flags, onConnected);
+
+    /* We should have requested the authentication */
+    ASSERT_TRUE(m_backend.sentCommands().empty());
+
+    /* Read the L2CAP connection request */
+    reqId = m_cmdId++;
+    expectedData = makeConnectRequest(reqId, m_connHandle, psm);
+    ASSERT_EQ(m_backend.lastData(), expectedData);
+
+    /* Send the L2cap connect response */
+    sendConnectResponse(reqId);
+    bte_handle_events();
+    Bte::L2cap hidIntrChannel = createdChannel;
+
+    expectedReplies = {
+        {0x42, 0x42, 0, 0},
+    };
+    ASSERT_EQ(replies, expectedReplies);
+}
+
 TEST_F(TestL2capConnect, testOutgoingPending) {
     using L = Bte::L2cap;
     BteBdAddr address = {1, 2, 3, 4, 5, 6};