You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by GitBox <gi...@apache.org> on 2018/05/14 11:46:49 UTC

[GitHub] sjanc closed pull request #73: Refactor RPA handling in controller

sjanc closed pull request #73: Refactor RPA handling in controller
URL: https://github.com/apache/mynewt-nimble/pull/73
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/nimble/controller/include/controller/ble_ll_adv.h b/nimble/controller/include/controller/ble_ll_adv.h
index c2ce24d3..e8fb36c8 100644
--- a/nimble/controller/include/controller/ble_ll_adv.h
+++ b/nimble/controller/include/controller/ble_ll_adv.h
@@ -185,6 +185,9 @@ int ble_ll_adv_ext_set_adv_data(uint8_t *cmdbuf, uint8_t cmdlen);
 int ble_ll_adv_ext_set_scan_rsp(uint8_t *cmdbuf, uint8_t cmdlen);
 int ble_ll_adv_ext_set_enable(uint8_t *cmdbuf, uint8_t len);
 
+/* Called to notify adv code about RPA rotation */
+void ble_ll_adv_rpa_timeout(void);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/nimble/controller/include/controller/ble_ll_resolv.h b/nimble/controller/include/controller/ble_ll_resolv.h
index 6c09bee7..e1dbbf31 100644
--- a/nimble/controller/include/controller/ble_ll_resolv.h
+++ b/nimble/controller/include/controller/ble_ll_resolv.h
@@ -33,13 +33,13 @@ extern "C" {
 struct ble_ll_resolv_entry
 {
     uint8_t rl_addr_type;
-    uint8_t rl_local_rpa_set;
     uint8_t rl_reserved;
     uint8_t rl_priv_mode;
     uint8_t rl_local_irk[16];
     uint8_t rl_peer_irk[16];
     uint8_t rl_identity_addr[BLE_DEV_ADDR_LEN];
     uint8_t rl_local_rpa[BLE_DEV_ADDR_LEN];
+    uint8_t rl_peer_rpa[BLE_DEV_ADDR_LEN];
 };
 
 extern struct ble_ll_resolv_entry g_ble_ll_resolv_list[];
@@ -59,9 +59,10 @@ int ble_ll_resolv_list_rmv(uint8_t *cmdbuf);
 /* Address resolution enable command */
 int ble_ll_resolv_enable_cmd(uint8_t *cmdbuf);
 
-/* XXX: implement */
-int ble_ll_resolv_peer_addr_rd(uint8_t *cmdbuf);
-void ble_ll_resolv_local_addr_rd(uint8_t *cmdbuf);
+int ble_ll_resolv_peer_addr_rd(uint8_t *cmdbuf, uint8_t *rspbuf,
+                               uint8_t *rsplen);
+int ble_ll_resolv_local_addr_rd(uint8_t *cmdbuf, uint8_t *rspbuf,
+                                uint8_t *rsplen);
 
 /* Finds 'addr' in resolving list. Doesnt check if address resolution enabled */
 struct ble_ll_resolv_entry *
@@ -76,7 +77,7 @@ uint8_t ble_ll_resolv_enabled(void);
 /* Reset private address resolution */
 void ble_ll_resolv_list_reset(void);
 
-void ble_ll_resolv_gen_priv_addr(struct ble_ll_resolv_entry *rl, int local,
+void ble_ll_resolv_get_priv_addr(struct ble_ll_resolv_entry *rl, int local,
                                  uint8_t *addr);
 
 /* Generate a resolvable private address. */
diff --git a/nimble/controller/src/ble_ll_adv.c b/nimble/controller/src/ble_ll_adv.c
index 7356bc3c..09ddc63c 100644
--- a/nimble/controller/src/ble_ll_adv.c
+++ b/nimble/controller/src/ble_ll_adv.c
@@ -87,8 +87,8 @@ struct ble_ll_adv_sm
     uint8_t adv_chan;
     uint8_t adv_pdu_len;
     int8_t adv_rpa_index;
-    uint8_t flags;
     int8_t adv_txpwr;
+    uint16_t flags;
     uint16_t props;
     uint16_t adv_itvl_min;
     uint16_t adv_itvl_max;
@@ -126,13 +126,14 @@ struct ble_ll_adv_sm
 #endif
 };
 
-#define BLE_LL_ADV_SM_FLAG_TX_ADD               0x01
-#define BLE_LL_ADV_SM_FLAG_RX_ADD               0x02
-#define BLE_LL_ADV_SM_FLAG_SCAN_REQ_NOTIF       0x04
-#define BLE_LL_ADV_SM_FLAG_CONN_RSP_TXD         0x08
-#define BLE_LL_ADV_SM_FLAG_ACTIVE_CHANSET_MASK  0x30 /* use helpers! */
-#define BLE_LL_ADV_SM_FLAG_ADV_DATA_INCOMPLETE  0x40
-#define BLE_LL_ADV_SM_FLAG_ADV_EXT_HCI          0x80
+#define BLE_LL_ADV_SM_FLAG_TX_ADD               0x0001
+#define BLE_LL_ADV_SM_FLAG_RX_ADD               0x0002
+#define BLE_LL_ADV_SM_FLAG_SCAN_REQ_NOTIF       0x0004
+#define BLE_LL_ADV_SM_FLAG_CONN_RSP_TXD         0x0008
+#define BLE_LL_ADV_SM_FLAG_ACTIVE_CHANSET_MASK  0x0030 /* use helpers! */
+#define BLE_LL_ADV_SM_FLAG_ADV_DATA_INCOMPLETE  0x0040
+#define BLE_LL_ADV_SM_FLAG_ADV_EXT_HCI          0x0080
+#define BLE_LL_ADV_SM_FLAG_ADV_RPA_TMO          0x0100
 
 #define ADV_DATA_LEN(_advsm) \
                 ((_advsm->adv_data) ? OS_MBUF_PKTLEN(advsm->adv_data) : 0)
@@ -239,16 +240,30 @@ ble_ll_adv_rpa_update(struct ble_ll_adv_sm *advsm)
 void
 ble_ll_adv_chk_rpa_timeout(struct ble_ll_adv_sm *advsm)
 {
-    uint32_t now;
-
     if (advsm->own_addr_type < BLE_HCI_ADV_OWN_ADDR_PRIV_PUB) {
         return;
     }
 
-    now = ble_npl_time_get();
-    if ((int32_t)(now - advsm->adv_rpa_timer) >= 0) {
+    if (advsm->flags & BLE_LL_ADV_SM_FLAG_ADV_RPA_TMO) {
         ble_ll_adv_rpa_update(advsm);
-        advsm->adv_rpa_timer = now + ble_ll_resolv_get_rpa_tmo();
+        advsm->flags &= ~BLE_LL_ADV_SM_FLAG_ADV_RPA_TMO;
+    }
+}
+
+void
+ble_ll_adv_rpa_timeout(void)
+{
+    struct ble_ll_adv_sm *advsm;
+    int i;
+
+    for (i = 0; i < BLE_ADV_INSTANCES; i++) {
+        advsm = &g_ble_ll_adv_sm[i];
+
+        if (advsm->adv_enabled &&
+                advsm->own_addr_type > BLE_HCI_ADV_OWN_ADDR_RANDOM) {
+            /* Mark RPA as timed out so we get a new RPA */
+            advsm->flags |= BLE_LL_ADV_SM_FLAG_ADV_RPA_TMO;
+        }
     }
 }
 #endif
@@ -1471,9 +1486,6 @@ ble_ll_adv_set_adv_params(uint8_t *cmd)
     if (own_addr_type > BLE_HCI_ADV_OWN_ADDR_RANDOM) {
         /* Copy peer address */
         memcpy(advsm->peer_addr, cmd + 7, BLE_DEV_ADDR_LEN);
-
-        /* Reset RPA timer so we generate a new RPA */
-        advsm->adv_rpa_timer = ble_npl_time_get();
     }
 #else
     /* If we dont support privacy some address types wont work */
@@ -2214,12 +2226,7 @@ ble_ll_adv_ext_set_param(uint8_t *cmdbuf, uint8_t *rspbuf, uint8_t *rsplen)
         goto done;
     }
 
-#if (MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_PRIVACY) == 1)
-    if (own_addr_type > BLE_HCI_ADV_OWN_ADDR_RANDOM) {
-        /* Reset RPA timer so we generate a new RPA */
-        advsm->adv_rpa_timer = ble_npl_time_get();
-    }
-#else
+#if (MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_PRIVACY) == 0)
     /* If we dont support privacy some address types wont work */
     if (own_addr_type > BLE_HCI_ADV_OWN_ADDR_RANDOM) {
         rc = BLE_ERR_UNSUPPORTED;
diff --git a/nimble/controller/src/ble_ll_conn.c b/nimble/controller/src/ble_ll_conn.c
index d33e9d31..e868d0fa 100644
--- a/nimble/controller/src/ble_ll_conn.c
+++ b/nimble/controller/src/ble_ll_conn.c
@@ -2753,7 +2753,7 @@ ble_ll_conn_req_pdu_update(struct os_mbuf *m, uint8_t *adva, uint8_t addr_type,
          */
         if (rl) {
             hdr |= BLE_ADV_PDU_HDR_TXADD_RAND;
-            ble_ll_resolv_gen_priv_addr(rl, 1, dptr);
+            ble_ll_resolv_get_priv_addr(rl, 1, dptr);
             addr = NULL;
         }
     }
diff --git a/nimble/controller/src/ble_ll_hci.c b/nimble/controller/src/ble_ll_hci.c
index df852d70..91ceb228 100644
--- a/nimble/controller/src/ble_ll_hci.c
+++ b/nimble/controller/src/ble_ll_hci.c
@@ -851,10 +851,10 @@ ble_ll_hci_le_cmd_proc(uint8_t *cmdbuf, uint16_t ocf, uint8_t *rsplen,
         rc = ble_ll_resolv_list_read_size(rspbuf, rsplen);
         break;
     case BLE_HCI_OCF_LE_RD_PEER_RESOLV_ADDR:
-        rc = ble_ll_resolv_peer_addr_rd(cmdbuf);
+        rc = ble_ll_resolv_peer_addr_rd(cmdbuf, rspbuf, rsplen);
         break;
     case BLE_HCI_OCF_LE_RD_LOCAL_RESOLV_ADDR:
-        ble_ll_resolv_local_addr_rd(cmdbuf);
+        rc = ble_ll_resolv_local_addr_rd(cmdbuf, rspbuf, rsplen);
         break;
     case BLE_HCI_OCF_LE_SET_ADDR_RES_EN:
         rc = ble_ll_resolv_enable_cmd(cmdbuf);
diff --git a/nimble/controller/src/ble_ll_resolv.c b/nimble/controller/src/ble_ll_resolv.c
index 52e1e33d..29da6f0d 100644
--- a/nimble/controller/src/ble_ll_resolv.c
+++ b/nimble/controller/src/ble_ll_resolv.c
@@ -72,11 +72,68 @@ ble_ll_resolv_list_chg_allowed(void)
     return rc;
 }
 
+
+/**
+ * Called to generate a resolvable private address in rl structure
+ *
+ * @param rl
+ * @param local
+ */
+static void
+ble_ll_resolv_gen_priv_addr(struct ble_ll_resolv_entry *rl, int local)
+{
+    uint8_t *irk;
+    uint8_t *prand;
+    uint32_t *irk32;
+    uint32_t *key32;
+    uint32_t *pt32;
+    struct ble_encryption_block ecb;
+    uint8_t *addr;
+
+    assert(rl != NULL);
+
+    if (local) {
+        addr = rl->rl_local_rpa;
+        irk = rl->rl_local_irk;
+    } else {
+        addr = rl->rl_peer_rpa;
+        irk = rl->rl_peer_irk;
+    }
+
+    /* Get prand */
+    prand = addr + 3;
+    ble_ll_rand_prand_get(prand);
+
+    /* Calculate hash, hash = ah(local IRK, prand) */
+
+    irk32 = (uint32_t *)irk;
+    key32 = (uint32_t *)&ecb.key[0];
+    key32[0] = irk32[0];
+    key32[1] = irk32[1];
+    key32[2] = irk32[2];
+    key32[3] = irk32[3];
+    pt32 = (uint32_t *)&ecb.plain_text[0];
+    pt32[0] = 0;
+    pt32[1] = 0;
+    pt32[2] = 0;
+    ecb.plain_text[12] = 0;
+    ecb.plain_text[13] = prand[2];
+    ecb.plain_text[14] = prand[1];
+    ecb.plain_text[15] = prand[0];
+
+    /* Calculate hash */
+    ble_hw_encrypt_block(&ecb);
+
+    addr[0] = ecb.cipher_text[15];
+    addr[1] = ecb.cipher_text[14];
+    addr[2] = ecb.cipher_text[13];
+}
+
 /**
  * Called when the Resolvable private address timer expires. This timer
- * is used to regenerate local RPA's in the resolving list.
+ * is used to regenerate local and peers RPA's in the resolving list.
  */
-void
+static void
 ble_ll_resolv_rpa_timer_cb(struct ble_npl_event *ev)
 {
     int i;
@@ -86,14 +143,18 @@ ble_ll_resolv_rpa_timer_cb(struct ble_npl_event *ev)
     rl = &g_ble_ll_resolv_list[0];
     for (i = 0; i < g_ble_ll_resolv_data.rl_cnt; ++i) {
         OS_ENTER_CRITICAL(sr);
-        rl->rl_local_rpa_set = 0;
-        ble_ll_resolv_gen_priv_addr(rl, 1, rl->rl_local_rpa);
-        rl->rl_local_rpa_set = 1;
+        ble_ll_resolv_gen_priv_addr(rl, 1);
+        OS_EXIT_CRITICAL(sr);
+
+        OS_ENTER_CRITICAL(sr);
+        ble_ll_resolv_gen_priv_addr(rl, 0);
         OS_EXIT_CRITICAL(sr);
         ++rl;
     }
     ble_npl_callout_reset(&g_ble_ll_resolv_data.rpa_timer,
                      (int32_t)g_ble_ll_resolv_data.rpa_tmo);
+
+    ble_ll_adv_rpa_timeout();
 }
 
 /**
@@ -237,30 +298,37 @@ ble_ll_resolv_list_add(uint8_t *cmdbuf)
     addr_type = cmdbuf[0];
     ident_addr = cmdbuf + 1;
 
-    rc = BLE_ERR_SUCCESS;
-    if (!ble_ll_is_on_resolv_list(ident_addr, addr_type)) {
-        rl = &g_ble_ll_resolv_list[g_ble_ll_resolv_data.rl_cnt];
-        rl->rl_addr_type = addr_type;
-        rl->rl_local_rpa_set = 0;
-        memcpy(&rl->rl_identity_addr[0], ident_addr, BLE_DEV_ADDR_LEN);
-        swap_buf(rl->rl_peer_irk, cmdbuf + 7, 16);
-        swap_buf(rl->rl_local_irk, cmdbuf + 23, 16);
-
-        /* By default use ptivacy network mode */
-        rl->rl_priv_mode = BLE_HCI_PRIVACY_NETWORK;
-
-        /*
-         * Add peer IRK to HW resolving list. If we can add it, also
-         * generate a local RPA now to save time later.
-         */
-        rc = ble_hw_resolv_list_add(rl->rl_peer_irk);
-        if (!rc) {
-            ble_ll_resolv_gen_priv_addr(rl, 1, rl->rl_local_rpa);
-            rl->rl_local_rpa_set = 1;
-        }
-        ++g_ble_ll_resolv_data.rl_cnt;
+    /* spec is not clear on how to handle this but make sure host is aware
+     * that new keys are not used in that case
+     */
+    if (ble_ll_is_on_resolv_list(ident_addr, addr_type)) {
+        return BLE_ERR_INV_HCI_CMD_PARMS;
     }
 
+    rl = &g_ble_ll_resolv_list[g_ble_ll_resolv_data.rl_cnt];
+    memset (rl, 0, sizeof(*rl));
+
+    rl->rl_addr_type = addr_type;
+    memcpy(&rl->rl_identity_addr[0], ident_addr, BLE_DEV_ADDR_LEN);
+    swap_buf(rl->rl_peer_irk, cmdbuf + 7, 16);
+    swap_buf(rl->rl_local_irk, cmdbuf + 23, 16);
+
+    /* By default use privacy network mode */
+    rl->rl_priv_mode = BLE_HCI_PRIVACY_NETWORK;
+
+    /* Add peer IRK to HW resolving list. Should always succeed since we
+     * already checked if there is room for it.
+     */
+    rc = ble_hw_resolv_list_add(rl->rl_peer_irk);
+    assert (rc == BLE_ERR_SUCCESS);
+
+    /* generate a local and peer RPAs now, those will be updated by timer
+     * when resolution is enabled
+     */
+    ble_ll_resolv_gen_priv_addr(rl, 1);
+    ble_ll_resolv_gen_priv_addr(rl, 0);
+    ++g_ble_ll_resolv_data.rl_cnt;
+
     return rc;
 }
 
@@ -288,7 +356,9 @@ ble_ll_resolv_list_rmv(uint8_t *cmdbuf)
 
     /* Remove from IRK records */
     position = ble_ll_is_on_resolv_list(ident_addr, addr_type);
-    if (position && (position < g_ble_ll_resolv_data.rl_cnt)) {
+    if (position) {
+        assert(position <= g_ble_ll_resolv_data.rl_cnt);
+
         memmove(&g_ble_ll_resolv_list[position - 1],
                 &g_ble_ll_resolv_list[position],
                 g_ble_ll_resolv_data.rl_cnt - position);
@@ -296,9 +366,10 @@ ble_ll_resolv_list_rmv(uint8_t *cmdbuf)
 
         /* Remove from HW list */
         ble_hw_resolv_list_rmv(position - 1);
+        return BLE_ERR_SUCCESS;
     }
 
-    return BLE_ERR_SUCCESS;
+    return BLE_ERR_UNK_CONN_ID;
 }
 
 /**
@@ -340,15 +411,51 @@ ble_ll_resolv_enable_cmd(uint8_t *cmdbuf)
 }
 
 int
-ble_ll_resolv_peer_addr_rd(uint8_t *cmdbuf)
+ble_ll_resolv_peer_addr_rd(uint8_t *cmdbuf, uint8_t *rspbuf, uint8_t *rsplen)
 {
-    /* XXX */
-    return 0;
+    struct ble_ll_resolv_entry *rl;
+    uint8_t addr_type;
+    uint8_t *ident_addr;
+    int rc;
+
+    addr_type = cmdbuf[0];
+    ident_addr = cmdbuf + 1;
+
+    rl = ble_ll_resolv_list_find(ident_addr, addr_type);
+    if (rl) {
+        memcpy(rspbuf, rl->rl_peer_rpa, BLE_DEV_ADDR_LEN);
+        rc = BLE_ERR_SUCCESS;
+    } else {
+        memset(rspbuf, 0, BLE_DEV_ADDR_LEN);
+        rc = BLE_ERR_UNK_CONN_ID;
+    }
+
+    *rsplen = BLE_DEV_ADDR_LEN;
+    return rc;
 }
 
-void
-ble_ll_resolv_local_addr_rd(uint8_t *cmdbuf)
+int
+ble_ll_resolv_local_addr_rd(uint8_t *cmdbuf, uint8_t *rspbuf, uint8_t *rsplen)
 {
+    struct ble_ll_resolv_entry *rl;
+    uint8_t addr_type;
+    uint8_t *ident_addr;
+    int rc;
+
+    addr_type = cmdbuf[0];
+    ident_addr = cmdbuf + 1;
+
+    rl = ble_ll_resolv_list_find(ident_addr, addr_type);
+    if (rl) {
+        memcpy(rspbuf, rl->rl_local_rpa, BLE_DEV_ADDR_LEN);
+        rc = BLE_ERR_SUCCESS;
+    } else {
+        memset(rspbuf, 0, BLE_DEV_ADDR_LEN);
+        rc = BLE_ERR_UNK_CONN_ID;
+    }
+
+    *rsplen = BLE_DEV_ADDR_LEN;
+    return rc;
 }
 
 /**
@@ -421,65 +528,23 @@ ble_ll_resolv_get_rpa_tmo(void)
     return g_ble_ll_resolv_data.rpa_tmo;
 }
 
-/**
- * Called to generate a resolvable private address
- *
- * @param rl
- * @param local
- * @param addr Pointer to resolvable private address
- */
 void
-ble_ll_resolv_gen_priv_addr(struct ble_ll_resolv_entry *rl, int local,
+ble_ll_resolv_get_priv_addr(struct ble_ll_resolv_entry *rl, int local,
                             uint8_t *addr)
 {
-    uint8_t *irk;
-    uint8_t *prand;
-    uint32_t *irk32;
-    uint32_t *key32;
-    uint32_t *pt32;
-    struct ble_encryption_block ecb;
+    os_sr_t sr;
 
     assert(rl != NULL);
     assert(addr != NULL);
 
-    /* If the local rpa has already been generated, just copy it */
-    if (local && rl->rl_local_rpa_set) {
-        memcpy(addr, rl->rl_local_rpa, BLE_DEV_ADDR_LEN);
-        return;
-    }
-
-    /* Get prand */
-    prand = addr + 3;
-    ble_ll_rand_prand_get(prand);
-
-    /* Calculate hash, hash = ah(local IRK, prand) */
+    OS_ENTER_CRITICAL(sr);
     if (local) {
-        irk = rl->rl_local_irk;
+        memcpy(addr, rl->rl_local_rpa, BLE_DEV_ADDR_LEN);
     } else {
-        irk = rl->rl_peer_irk;
+        memcpy(addr, rl->rl_peer_rpa, BLE_DEV_ADDR_LEN);
     }
 
-    irk32 = (uint32_t *)irk;
-    key32 = (uint32_t *)&ecb.key[0];
-    key32[0] = irk32[0];
-    key32[1] = irk32[1];
-    key32[2] = irk32[2];
-    key32[3] = irk32[3];
-    pt32 = (uint32_t *)&ecb.plain_text[0];
-    pt32[0] = 0;
-    pt32[1] = 0;
-    pt32[2] = 0;
-    ecb.plain_text[12] = 0;
-    ecb.plain_text[13] = prand[2];
-    ecb.plain_text[14] = prand[1];
-    ecb.plain_text[15] = prand[0];
-
-    /* Calculate hash */
-    ble_hw_encrypt_block(&ecb);
-
-    addr[0] = ecb.cipher_text[15];
-    addr[1] = ecb.cipher_text[14];
-    addr[2] = ecb.cipher_text[13];
+    OS_EXIT_CRITICAL(sr);
 }
 
 /**
@@ -507,7 +572,7 @@ ble_ll_resolv_gen_rpa(uint8_t *addr, uint8_t addr_type, uint8_t *rpa, int local)
             irk = rl->rl_peer_irk;
         }
         if (ble_ll_resolv_irk_nonzero(irk)) {
-            ble_ll_resolv_gen_priv_addr(rl, local, rpa);
+            ble_ll_resolv_get_priv_addr(rl, local, rpa);
             rc = 1;
         }
     }
diff --git a/nimble/controller/src/ble_ll_scan.c b/nimble/controller/src/ble_ll_scan.c
index 843404d7..6b5ed5d9 100644
--- a/nimble/controller/src/ble_ll_scan.c
+++ b/nimble/controller/src/ble_ll_scan.c
@@ -344,7 +344,7 @@ ble_ll_scan_req_pdu_make(struct ble_ll_scan_sm *scansm, uint8_t *adv_addr,
          * section 6.3).
          */
         if (rl) {
-            ble_ll_resolv_gen_priv_addr(rl, 1, rpa);
+            ble_ll_resolv_get_priv_addr(rl, 1, rpa);
             scana = rpa;
         } else {
             ble_ll_scan_refresh_nrpa(scansm);


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services