Allow to open/close a BLE pairing&bonding window

[igiona]

It is a common use case to have a gesture that enables a "pairing window" with the device.
The goal is to control who can have remote access to the device, by forcing some kind of physical access.
This is especially important in devices with limited UI capabilities that need to use the rather poor justworks security.

It seems that peripheral::advertise_pairable vs peripheral::advertise_connectable suggest this feature, but it either doesn't work properly or I misinterpret it.

In both cases, pairing (key exchange) still happens successfully, only bonding is not performed when peripheral::advertise_connectable is used.
This means that, at any time, a central device can access the encrypted GATT services and characteristics without ever having to access the device physically.

As far I could test, the current implementation does not allow to enable pairing only in a certain window.
After digging into the code, I think that maybe it comes down to simply setting the security params as follow when the BLE_GAP_EVTS_BLE_GAP_EVT_SEC_PARAMS_REQUEST is received and no security handler is provided:

      let mut sec_params: raw::ble_gap_sec_params_t = core::mem::zeroed();
      sec_params.min_key_size = 7;
      sec_params.max_key_size = 16;
      sec_params.set_io_caps(raw::BLE_GAP_IO_CAPS_NONE as u8);

Any thoughts on this? Am I missing something?

[alexmoon]

#320 adds a method to SecurityHandler that allows you to fully customize the ble_gap_sec_params_t that should be used. That should cover your use case. It's a bit odd to me though that you would accept connections from an unknown central when you're not willing to pair with them.

[igiona]

#320

I guess 320 is a typo? => #230

I want to accept connections and pairing for already known devices, but I want only to allow connections & bonding requests when explicitly requested.
My understanding is that in order to allow to connect with an already bonded peer I require peripheral::advertise_pairable , but this allows every peer to bond.
Is there a way to prevent this?
Maybe I simply have to change the directed/undirected field in the adv packet? But then I can accept only a connection from the last known peer the device was connected to.

Probably the best would be to check upon connection if the cebtral is already bonded, and disconnected if it is not?

[alexmoon]

I guess 320 is a typo? => #230

Yes, oops.

Probably the best would be to check upon connection if the cebtral is already bonded, and disconnected if it is not?

The best way to do this is with a whitelist. Use ble::set_whitelist with the list of your peers then set ble::peripheral::Config.filter_policy to define which requests you want to filter. If any of your peers use private resolvable addresses, you'll need to call ble::set_device_identities_list before calling set_whitelist to give the Softdevice the identity resolution keys.

[igiona]

I guess 320 is a typo? => #230

Yes, oops.

Probably the best would be to check upon connection if the cebtral is already bonded, and disconnected if it is not?

The best way to do this is with a whitelist. Use ble::set_whitelist with the list of your peers then set ble::peripheral::Config.filter_policy to define which requests you want to filter. If any of your peers use private resolvable addresses, you'll need to call ble::set_device_identities_list before calling set_whitelist to give the Softdevice the identity resolution keys.

Many thanks for the advice! It works like a charm!
This is what I had to do:

    pub(crate) fn whitelist_known_peers(&self, sd: &Softdevice) -> Result<(), RawError> {
        const MAX_LEN: usize = nrf_softdevice::raw::BLE_GAP_WHITELIST_ADDR_MAX_COUNT as usize;
        assert!(self.bond_info_map.borrow().len() <= MAX_LEN);

        let mut addresses : [Address; MAX_LEN] = unsafe { core::mem::zeroed() };
        let mut id_keys : [IdentityKey; MAX_LEN] = unsafe { core::mem::zeroed() };
    
        let mut valid_id_keys : usize = 0;
        for (i, bond) in self.bond_info_map.borrow().iter() {
            addresses[*i as usize] = bond.peer.peer_id.addr;

            warn!("Whitelisting {}", bond.peer.peer_id.addr);
            let already_exist = id_keys.iter().find(|idk| idk.is_match(bond.peer.peer_id.addr)).is_some();
            if !already_exist {
                id_keys[*i as usize] = bond.peer.peer_id;
                valid_id_keys += 1;
            }
        }

        ble::set_device_identities_list(sd, &id_keys[..valid_id_keys], None)?;
        ble::set_whitelist(sd, &addresses[..self.bond_info_map.borrow().len()])?;
        Ok(())
    }

I added this function the Bonder struct that was discussed here #237 (comment)

Then to allow pairing, in my BLE task I do something like

let mut config = peripheral::Config::default();

match bonder.whitelist_known_peers(sd) {
    Ok(_) => info!("Whitelisting stored successfully"),
    Err(e) => error!("Unable to configure whitelisting {}", e),
}

if pairing_allowed {
  let adv_data: LegacyAdvertisementPayload = LegacyAdvertisementBuilder::new()
                          .flags(&[Flag::GeneralDiscovery, Flag::LE_Only])
                          .full_name(name)
                          //add more settings if needed
                          .build();
  let adv = peripheral::ConnectableAdvertisement::ScannableUndirected {
      adv_data: &adv_data,
      scan_data: &SCAN_DATA,
  };
  config.filter_policy = FilterPolicy::Any;
  unwrap!(peripheral::advertise_pairable(sd, adv, &config, bonder).await)
}
else {
  let adv_data: LegacyAdvertisementPayload = LegacyAdvertisementBuilder::new()
                          .flags(&[Flag::LE_Only])
                          .build();
  let adv = peripheral::ConnectableAdvertisement::ScannableUndirected {
      adv_data: &adv_data,
      scan_data: &SCAN_DATA,
  };
  config.filter_policy = FilterPolicy::Both;
  unwrap!(peripheral::advertise_pairable(sd, adv, &config, bonder).await)
}

Every suggestion for improvement in performance, style or anything is very welcome :)
I'm not really happy about having to create a copy of rather "big objects" like IdentityKey and Address... but rust has a quite steep learning curve so I'll live with it for now 😄