BT: Add API for managing pairing; improve security

PUBLISHED_FROM=df72de0a2747f731ba3ced6f8e846a95df90d217

Author: Deomid Ryabkov

README.md

@@ -7,11 +7,26 @@ Currently contains only GATT server implmenetation for ESP32.
 `bt-common` library adds a `bt` configuration section with the following
 settings:
 
-```json
+```
 "bt": {
-  "enable": true,       // Enabled by default. Disabled on first reboot with WiFi on
-  "dev_name": "",       // Device name. If empty, value equals to device.id
-  "adv_enable": true,   // Advertise our Bluetooth services
-  "keep_enabled": true  // Keep enabled after successful boot with WiFi on
+  "enable": true,             // Enabled by default. Disabled on first reboot with WiFi on
+  "dev_name": "",             // Device name. If empty, value equals to device.id
+  "adv_enable": true,         // Advertise our Bluetooth services
+  "keep_enabled": true,       // Keep enabled after successful boot with WiFi on
+  "scan_rsp_data_hex": "",    // Custom scan response data, as hex string (e.g. `48656c6c6f` for `Hello`)
+  "allow_pairing": true,      // Allow pairingbonding with other devices
+  "max_paired_devices": 10,   // Allow pairing with up to this many devices; -1 - no limit
+  "gatts": {
+    "min_sec_level": 0,       // Minimum security level for all attributes of all services.
+                              // 0 - no auth required, 1 - encryption reqd, 2 - encryption + MITM reqd
+    "require_pairing": false  // Require taht device is paired before accessing services
+  }
 }
 ```
+
+## Security
+
+Default settings allow for unrestricted access: anyone can pair with a device and access the services.
+A better idea is to set `bt.gatts.require_pairing` to true, `bt.allow_pairing` to false and only enable it for a limited time via `mgos_bt_gap_set_pairing_enable` when user performs some action, e.g. presses a button.
+Raising `bt.gatts.min_sec_level` to at least 1 is also advisable.
+_Note_: At present, level 2 (MITM protection) is not usable as it requires device to have at least output capability during pairing, and there's no API for displaying the pairing code yet.

include/esp32/esp32_bt.h

@@ -76,7 +76,16 @@ void mgos_bt_ble_set_scan_rsp_data(const struct mg_str scan_rsp_data);
 bool mgos_bt_gap_get_adv_enable(void);
 bool mgos_bt_gap_set_adv_enable(bool adv_enable);
 
+bool mgos_bt_gap_get_pairing_enable(void);
+bool mgos_bt_gap_set_pairing_enable(bool pairing_enable);
+
 int mgos_bt_ble_get_num_paired_devices(void);
+/*
+ * These are actually async. TODO(rojer): Add callbacks to the API.
+ * For now, just allow some time for the calls to complete.
+ */
+void mgos_bt_ble_remove_paired_device(const esp_bd_addr_t addr);
+void mgos_bt_ble_remove_all_paired_devices(void);
 
 #ifdef __cplusplus
 }

include/esp32/esp32_bt_internal.h

@@ -22,6 +22,7 @@ bool esp32_bt_gattc_init(void);
 bool esp32_bt_init(void);
 bool esp32_bt_gap_init(void);
 bool esp32_bt_gatts_init(void);
+void esp32_bt_gatts_auth_cmpl(const esp_bd_addr_t addr);
 
 void esp32_bt_set_is_advertising(bool is_advertising);
 

mos.yml

@@ -17,7 +17,11 @@ config_schema:
   - ["bt.adv_enable", "b", true, {title: "Advertise services"}]
   - ["bt.scan_rsp_data_hex", "s", "", {title: "Scan response data, hex-encoded"}]
   - ["bt.keep_enabled", "b", false, {title: "By default, BT will be disabled once WiFi is configured and connects. Set this to true to keep BT enabled."}]
-  - ["bt.gatts_min_sec_level", "i", 0, {title: "0 - no auth required, 1 - encryption reqd, 2 - encryption + MITM reqd"}]
+  - ["bt.allow_pairing", "b", true, {title: "Allow pairing/bonding with other devices"}]
+  - ["bt.max_paired_devices", "i", -1, {title: "Max number of paired devices; -1 - no limit"}]
+  - ["bt.gatts", "o", {title: "GATTS settings"}]
+  - ["bt.gatts.min_sec_level", "i", 0, {title: "0 - no auth required, 1 - encryption reqd, 2 - encryption + MITM reqd"}]
+  - ["bt.gatts.require_pairing", "b", false, {title: "Require device to be paired before accessing services"}]
 
 tags:
   - bt

src/esp32/esp32_bt.c

@@ -223,7 +223,8 @@ bool mgos_bt_common_init(void) {
     mgos_net_add_event_handler(mgos_bt_net_ev, NULL);
   }
 
-  LOG(LL_INFO, ("Bluetooth init ok, %d paired devices",
+  LOG(LL_INFO, ("Bluetooth init ok, pairing %s, %d paired devices",
+                (mgos_bt_gap_get_pairing_enable() ? "enabled" : "disabled"),
                 mgos_bt_ble_get_num_paired_devices()));
   ret = true;
 

src/esp32/esp32_bt_gap.c

@@ -20,6 +20,7 @@
 #include "mgos_sys_config.h"
 
 #include "esp32_bt_internal.h"
+#include "esp32_bt_gatts.h"
 
 struct scan_cb_info {
   esp_bd_addr_t target_addr;
@@ -64,6 +65,8 @@ static esp_ble_adv_params_t s_adv_params = {
     .adv_filter_policy = ADV_FILTER_ALLOW_SCAN_ANY_CON_ANY,
 };
 
+static bool s_pairing_enable = false;
+
 static bool start_advertising(void) {
   if (s_advertising) return true;
   if (!s_adv_enable || is_scanning()) return false;
@@ -246,6 +249,7 @@ static void esp32_bt_gap_ev(esp_gap_ble_cb_event_t ev,
       LOG(ll, ("AUTH_CMPL peer %s at %d dt %d success %d (fr %d) kp %d kt %d",
                mgos_bt_addr_to_str(p->bd_addr, buf), p->addr_type, p->dev_type,
                p->success, p->fail_reason, p->key_present, p->key_type));
+      if (p->success) esp32_bt_gatts_auth_cmpl(p->bd_addr);
       break;
     }
     case ESP_GAP_BLE_KEY_EVT: {
@@ -492,6 +496,45 @@ void esp32_bt_set_is_advertising(bool is_advertising) {
   s_advertising = is_advertising;
 }
 
+bool mgos_bt_gap_get_pairing_enable(void) {
+  return s_pairing_enable;
+}
+
+bool mgos_bt_gap_set_pairing_enable(bool pairing_enable) {
+  esp_ble_auth_req_t auth_req =
+      (pairing_enable ? ESP_LE_AUTH_BOND : ESP_LE_AUTH_NO_BOND);
+  if (esp_ble_gap_set_security_param(ESP_BLE_SM_AUTHEN_REQ_MODE, &auth_req,
+                                     sizeof(auth_req)) == ESP_OK) {
+    s_pairing_enable = pairing_enable;
+    return true;
+  } else {
+    return false;
+  }
+}
+
+void mgos_bt_ble_remove_paired_device(const esp_bd_addr_t addr) {
+  /* Workaround for https://github.com/espressif/esp-idf/issues/1365 */
+  if (mgos_bt_gatts_get_num_connections() > 0) {
+    esp_ble_gap_disconnect((uint8_t *) addr);
+    /* After disconnecting, some time is required before
+     * esp_ble_remove_bond_device can succeed. */
+    mgos_msleep(200);
+  }
+
+  esp_ble_remove_bond_device((uint8_t *) addr);
+}
+
+void mgos_bt_ble_remove_all_paired_devices(void) {
+  int num = esp_ble_get_bond_device_num();
+  esp_ble_bond_dev_t *list = (esp_ble_bond_dev_t *) calloc(num, sizeof(*list));
+  if (list != NULL && esp_ble_get_bond_device_list(&num, list) == ESP_OK) {
+    for (int i = 0; i < num; i++) {
+      mgos_bt_ble_remove_paired_device(list[i].bd_addr);
+    }
+  }
+  free(list);
+}
+
 bool esp32_bt_gap_init(void) {
   if (esp_ble_gap_register_callback(esp32_bt_gap_ev) != ESP_OK) {
     return false;
@@ -515,9 +558,8 @@ bool esp32_bt_gap_init(void) {
     }
   }
 
-  esp_ble_auth_req_t auth_req = ESP_LE_AUTH_BOND;
-  esp_ble_gap_set_security_param(ESP_BLE_SM_AUTHEN_REQ_MODE, &auth_req,
-                                 sizeof(auth_req));
+  mgos_bt_gap_set_pairing_enable(mgos_sys_config_get_bt_allow_pairing());
+
   esp_ble_io_cap_t io_cap = ESP_IO_CAP_NONE;
   esp_ble_gap_set_security_param(ESP_BLE_SM_IOCAP_MODE, &io_cap,
                                  sizeof(uint8_t));

src/esp32/esp32_bt_gatts.c

@@ -54,6 +54,7 @@ struct esp32_gatts_session_entry {
 
 struct esp32_gatts_connection_entry {
   struct esp32_bt_connection bc;
+  bool need_auth;
   SLIST_HEAD(sessions, esp32_gatts_session_entry) sessions;
   SLIST_ENTRY(esp32_gatts_connection_entry) next;
 };
@@ -267,6 +268,55 @@ static void run_on_mgos_task(esp_gatt_if_t gatts_if,
   mgos_invoke_cb(gatts_ev_mgos, ei, false /* from_isr */);
 }
 
+static bool is_paired(const esp_bd_addr_t addr) {
+  bool result = false;
+  int num = esp_ble_get_bond_device_num();
+  esp_ble_bond_dev_t *list = (esp_ble_bond_dev_t *) calloc(num, sizeof(*list));
+  if (list != NULL && esp_ble_get_bond_device_list(&num, list) == ESP_OK) {
+    for (int i = 0; i < num; i++) {
+      if (mgos_bt_addr_cmp(addr, list[i].bd_addr) == 0) {
+        result = true;
+        break;
+      }
+    }
+  }
+  free(list);
+  return result;
+}
+
+static void create_sessions(struct esp32_gatts_connection_entry *ce) {
+  /* Create a session for each of the currently registered services. */
+  struct esp32_bt_service_entry *se;
+  esp_ble_gatts_cb_param_t ep;
+  ep.connect.conn_id = ce->bc.conn_id;
+  memcpy(ep.connect.remote_bda, ce->bc.peer_addr, ESP_BD_ADDR_LEN);
+  SLIST_FOREACH(se, &s_svcs, next) {
+    struct esp32_gatts_session_entry *sse =
+        (struct esp32_gatts_session_entry *) calloc(1, sizeof(*sse));
+    sse->se = se;
+    sse->bs.bc = &ce->bc;
+    SLIST_INSERT_HEAD(&ce->sessions, sse, next);
+    run_on_mgos_task(ce->bc.gatt_if, sse, sse->se, ESP_GATTS_CONNECT_EVT, &ep);
+  }
+  esp_ble_conn_update_params_t conn_params = {0};
+  memcpy(conn_params.bda, ce->bc.peer_addr, ESP_BD_ADDR_LEN);
+  conn_params.latency = 0;
+  conn_params.max_int = 0x50; /* max_int = 0x50*1.25ms = 100ms */
+  conn_params.min_int = 0x30; /* min_int = 0x30*1.25ms = 60ms */
+  conn_params.timeout = 400;  /* timeout = 400*10ms = 4000ms */
+  esp_ble_gap_update_conn_params(&conn_params);
+}
+
+void esp32_bt_gatts_auth_cmpl(const esp_bd_addr_t addr) {
+  struct esp32_gatts_connection_entry *ce, *ct;
+  SLIST_FOREACH_SAFE(ce, &s_conns, next, ct) {
+    if (mgos_bt_addr_cmp(ce->bc.peer_addr, addr) == 0 && ce->need_auth) {
+      ce->need_auth = false;
+      create_sessions(ce);
+    }
+  }
+}
+
 static void esp32_bt_gatts_ev(esp_gatts_cb_event_t ev, esp_gatt_if_t gatts_if,
                               esp_ble_gatts_cb_param_t *ep) {
   char buf[BT_UUID_STR_LEN];
@@ -436,47 +486,62 @@ static void esp32_bt_gatts_ev(esp_gatts_cb_event_t ev, esp_gatt_if_t gatts_if,
       const struct gatts_connect_evt_param *p = &ep->connect;
       LOG(LL_INFO, ("CONNECT cid %d addr %s", p->conn_id,
                     mgos_bt_addr_to_str(p->remote_bda, buf)));
-      esp_ble_conn_update_params_t conn_params = {0};
-      memcpy(conn_params.bda, p->remote_bda, ESP_BD_ADDR_LEN);
-      conn_params.latency = 0;
-      conn_params.max_int = 0x50; /* max_int = 0x50*1.25ms = 100ms */
-      conn_params.min_int = 0x30; /* min_int = 0x30*1.25ms = 60ms */
-      conn_params.timeout = 400;  /* timeout = 400*10ms = 4000ms */
-      esp_ble_gap_update_conn_params(&conn_params);
       /* Connect disables advertising. Resume, if it's enabled. */
       esp32_bt_set_is_advertising(false);
       mgos_bt_gap_set_adv_enable(mgos_bt_gap_get_adv_enable());
+      bool disconnect = false;
+      esp_ble_sec_act_t sec = 0;
       switch (mgos_sys_config_get_bt_gatts_min_sec_level()) {
         case MGOS_BT_GATT_PERM_LEVEL_NONE:
           break;
         case MGOS_BT_GATT_PERM_LEVEL_ENCR:
-          LOG(LL_DEBUG, ("%s: Requesting encryption",
-                         mgos_bt_addr_to_str(p->remote_bda, buf)));
-          esp_ble_set_encryption((uint8_t *) p->remote_bda,
-                                 ESP_BLE_SEC_ENCRYPT_NO_MITM);
+          sec = ESP_BLE_SEC_ENCRYPT_NO_MITM;
           break;
         case MGOS_BT_GATT_PERM_LEVEL_ENCR_MITM:
-          LOG(LL_DEBUG, ("%s: Requesting encryption + MITM protection",
-                         mgos_bt_addr_to_str(p->remote_bda, buf)));
-          esp_ble_set_encryption((uint8_t *) p->remote_bda,
-                                 ESP_BLE_SEC_ENCRYPT_MITM);
+          sec = ESP_BLE_SEC_ENCRYPT_MITM;
           break;
       }
+      if (mgos_sys_config_get_bt_gatts_require_pairing()) {
+        mgos_bt_addr_to_str(p->remote_bda, buf);
+        int max_devices = mgos_sys_config_get_bt_max_paired_devices();
+        if (is_paired(p->remote_bda)) {
+          LOG(LL_INFO, ("%s: Already paired", buf));
+        } else if (!mgos_bt_gap_get_pairing_enable()) {
+          LOG(LL_ERROR, ("%s: pairing required but is not allowed", buf));
+          disconnect = true;
+        } else if (max_devices >= 0 &&
+                   mgos_bt_ble_get_num_paired_devices() >= max_devices) {
+          LOG(LL_ERROR,
+              ("%s: pairing required but max num devices (%d) reached", buf,
+               max_devices));
+          disconnect = true;
+        } else {
+          LOG(LL_INFO, ("%s: Begin pairing", buf));
+          if (sec == 0) sec = ESP_BLE_SEC_ENCRYPT_NO_MITM;
+        }
+      }
+      if (disconnect) {
+        LOG(LL_ERROR, ("%s: dropping connection",
+                       mgos_bt_addr_to_str(p->remote_bda, buf)));
+        esp_ble_gap_disconnect((uint8_t *) p->remote_bda);
+        break;
+      }
       struct esp32_gatts_connection_entry *ce =
           (struct esp32_gatts_connection_entry *) calloc(1, sizeof(*ce));
       ce->bc.gatt_if = gatts_if;
       ce->bc.conn_id = p->conn_id;
       ce->bc.mtu = ESP_GATT_DEF_BLE_MTU_SIZE;
       memcpy(ce->bc.peer_addr, p->remote_bda, ESP_BD_ADDR_LEN);
-      /* Create a session for each of the currently registered services. */
-      struct esp32_bt_service_entry *se;
-      SLIST_FOREACH(se, &s_svcs, next) {
-        struct esp32_gatts_session_entry *sse =
-            (struct esp32_gatts_session_entry *) calloc(1, sizeof(*sse));
-        sse->se = se;
-        sse->bs.bc = &ce->bc;
-        SLIST_INSERT_HEAD(&ce->sessions, sse, next);
-        run_on_mgos_task(gatts_if, sse, sse->se, ev, ep);
+      if (sec != 0) {
+        LOG(LL_DEBUG,
+            ("%s: Requesting encryption%s",
+             mgos_bt_addr_to_str(p->remote_bda, buf),
+             (sec == ESP_BLE_SEC_ENCRYPT_MITM ? " + MITM protection" : "")));
+        esp_ble_set_encryption((uint8_t *) p->remote_bda, sec);
+        ce->need_auth = true;
+        /* Wait for AUTH_CMPL */
+      } else {
+        create_sessions(ce);
       }
       SLIST_INSERT_HEAD(&s_conns, ce, next);
       break;