Stabilize BLE Security Implementation (#941)

* Refactor BLE broadcast encryption flags

The original spec defines the flags that are used to indicate the keying
material for BLE broadcasts:
...
    kBroadcastDataIsEncrypted   = 1 << 3,
    kBroadcastKeyIsDeviceKey    = 1 << 4,
    kBroadcastKeyIsUserKey      = 1 << 5,
...
Using this implementation only three states can be encoded: no
encryption, encrypted with device key and encrypted with a user key.

New implementation treats this three bytes like enum instead of seperate
bools and introduce a new encryption state (encrypted with null key):

    kNoEncryption = 0,
    kEncryptedWithNullKey = 1,
    kEncryptedWithUserKey = 2,
    kEncryptedWithDeviceKey = 3,

* Consolidate auth characteristics

The BLE authentication flow is completely synchronous so there is no need for
two separate characteristics for the client and the server. This commit unifies
two authentication characteristics into one.

* Modify Security Level 0

For security level 0, the root key is considered to be 32-bytes of zeros and
authentication proceeds exactly the same as level 1 and 2 given that root key.

* Stabilize flags/info ble characteristic

Info characteristic holds 20 bytes
1 byte: version
2 byte: security flags
3-4 bytes: high flags
5-20 bytes: reserved flags

Added RPC IN PROGRESS flag for tracking if the device
handles a RPC and cannot handle another one

* Distinguish password-based user-key and random data

Introduce a 3rd AuthMethod and key: PasswordBasedAuthentication and
PasswordBasedUserKey

The primary benefit is that it distinguishes between when we are looking for
a password that a user could enter from when we are looking for something that
has to be stored in a server or key store somewhere.

* Update comments in test_signed_list_report.py

iotilecore/RELEASE.md

@@ -10,6 +10,7 @@ All major changes in each released version of `iotile-core` are listed here.
 - Add shim to make `iotile-core` compatible with Python 3.8.0 on Windows.  There is a bug in
   that python version that breaks background event loops only on Windows.  It is fixed in
   python 3.8.1.
+- Add new BLE broadcast encryption method: Encrypted with NullKey
 
 ## 5.0.11
 

iotilecore/iotile/core/hw/auth/auth_provider.py

@@ -16,13 +16,17 @@ class AuthProvider:
     ReportKeyMagic = 0x00000002
 
     NoKey = 0
-    UserKey = 1
-    DeviceKey = 2
+    NullKey = 1
+    UserKey = 2
+    DeviceKey = 3
+    PasswordBasedKey = 4
 
     KnownKeyRoots = {
         NoKey: 'no_key',
+        NullKey: 'null_key',
         UserKey: 'user_key',
-        DeviceKey: 'device_key'
+        DeviceKey: 'device_key',
+        PasswordBasedKey: 'pasword_based_userkey'
     }
 
     def __init__(self, args=None):
@@ -48,6 +52,19 @@ def verify_key(self, root_key_type):
         if root_key_type not in self.supported_keys:
             raise NotFoundError("Not supported key type", key=root_key_type)
 
+    @classmethod
+    def DeriveRebootKeyFromPassword(cls, password):
+        """Derive the root key from the user password
+        TODO hashlib.pbkdf2_hmac arguments needs to be revised,
+            current values are not proved to be secure
+
+        Args:
+            password (str): user password
+
+        Returns:
+            bytes: derived key
+        """
+        return hashlib.pbkdf2_hmac('sha256', password.encode('utf-8'), b'salt', 100000)
 
     @classmethod
     def DeriveReportKey(cls, root_key, report_id, sent_timestamp):

iotilecore/iotile/core/hw/auth/cli_auth_provider.py

@@ -6,6 +6,8 @@
 import hashlib
 from iotile.core.exceptions import NotFoundError
 from .rootkey_auth_provider import RootKeyAuthProvider
+from .env_auth_provider import EnvAuthProvider
+from .auth_provider import AuthProvider
 import getpass
 
 
@@ -18,36 +20,29 @@ def __init__(self, args=None):
         if args is None:
             args = {}
 
-        args['supported_keys'] = [self.UserKey]
+        args['supported_keys'] = [self.PasswordBasedKey]
 
         super().__init__(args)
 
-    @classmethod
-    def derive_key(cls, password):
-        """Derive the root key from user password
-        TODO hashlib.pbkdf2_hmac arguments needs to be revised,
-            current values are not proved to be secure
-
-        Args:
-            password (str): user password
-
-        Returns:
-            bytes: derived key
-        """
-        return hashlib.pbkdf2_hmac('sha256', password.encode('utf-8'), b'salt', 100000)
-
     def get_root_key(self, key_type, device_id):
-        """Prompt user for the password and derive root key from it
+        """Prompt a user for the password and derive the root key from it
 
         Args:
             key_type (int): see KnownKeyRoots
-            device_id (int): uuid of the device
+            device_id (int): the uuid of the device
 
         Returns:
             bytes: the root key
         """
         self.verify_key(key_type)
 
-        password = getpass.getpass("Please, input user password for device {} :".format(device_id))
+        password = getpass.getpass("Please input the user password for the device {} :".format(device_id))
+        userkey = AuthProvider.DeriveRebootKeyFromPassword(password)
+
+        if device_id:
+            answer = input("Would you like to save the user-key until the end of the current session? (y/n)")
+            if answer and answer[0].lower() == 'y':
+                variable_name = EnvAuthProvider.construct_var_name(device_id)
+                os.environ[variable_name] = userkey.hex()
 
-        return CliAuthProvider.derive_key(password)
+        return userkey

iotilecore/iotile/core/hw/auth/env_auth_provider.py

@@ -18,6 +18,19 @@ def __init__(self, args=None):
 
         super().__init__(args)
 
+    @classmethod
+    def construct_var_name(cls, device_id):
+        """Build name of environment variable used to store userkey
+
+        Returns:
+            str: variable name
+        """
+        if isinstance(device_id, str):
+            var_name = "USER_KEY_{}".format(device_id)
+        else:
+            var_name = "USER_KEY_{0:08X}".format(device_id)
+        return var_name
+
     def get_root_key(self, key_type, device_id):
         """Attempt to get a user key from an environment variable
 
@@ -30,10 +43,7 @@ def get_root_key(self, key_type, device_id):
         """
         self.verify_key(key_type)
 
-        if isinstance(device_id, str):
-            var_name = "USER_KEY_{}".format(device_id)
-        else:
-            var_name = "USER_KEY_{0:08X}".format(device_id)
+        var_name = EnvAuthProvider.construct_var_name(device_id)
 
         if var_name not in os.environ:
             raise NotFoundError("No key could be found for devices", device_id=device_id,
@@ -43,7 +53,6 @@ def get_root_key(self, key_type, device_id):
         if len(key_var) != 64:
             raise NotFoundError("Key in variable is not the correct length, should be 64 hex characters",
                                 device_id=device_id, key_value=key_var)
-
         try:
             key = binascii.unhexlify(key_var)
         except ValueError as exc:

iotilecore/iotile/core/hw/auth/null_auth_provider.py

@@ -10,7 +10,7 @@ def __init__(self, args=None):
         if args is None:
             args = {}
 
-        args['supported_keys'] = [self.NoKey]
+        args['supported_keys'] = [self.NullKey]
 
         super().__init__(args)
 

iotilecore/test/test_reports/test_signed_list_report.py

@@ -5,9 +5,21 @@
 from iotile.core.hw.reports.signed_list_format import SignedListReport
 from iotile.core.hw.reports.report import IOTileReading
 from iotile.core.hw.auth.env_auth_provider import EnvAuthProvider
+from iotile.core.hw.auth.auth_provider import AuthProvider
 
 
-def make_sequential(iotile_id, stream, num_readings, give_ids=False, root_key=0, signer=None):
+def make_sequential(iotile_id, stream, num_readings, give_ids=False, root_key=AuthProvider.NoKey, signer=None):
+    """Create sequaltial report from reading
+
+    Args:
+        iotile_id (int): The uuid of the device that this report came from
+        stream (int): The stream that these readings are part of
+        num_readings(int): amount of readings
+        give_ids(bool): whether to set sequantial id for every reading
+        root_key(int): type of root key to sign the report
+        signer (AuthProvider): An optional preconfigured AuthProvider that should be used to sign this
+            report.  If no AuthProvider is provided, the default ChainedAuthProvider is used.
+    """
     readings = []
 
     for i in range(0, num_readings):
@@ -22,8 +34,7 @@ def make_sequential(iotile_id, stream, num_readings, give_ids=False, root_key=0,
     return report
 
 def test_basic_parsing():
-    """Make sure we can decode a signed report
-    """
+    """Make sure we can decode a signed report"""
 
     report = make_sequential(1, 0x1000, 10)
     encoded = report.encode()
@@ -41,8 +52,7 @@ def test_basic_parsing():
     assert report.signature_flags == 0
 
 def test_footer_calculation():
-    """
-    """
+    """Test if make_sesuentail set properly ids"""
 
     report1 = make_sequential(1, 0x1000, 10, give_ids=False)
     report2 = make_sequential(1, 0x1000, 10, give_ids=True)
@@ -60,15 +70,15 @@ def test_userkey_signing(monkeypatch):
     signer = EnvAuthProvider()
 
     with pytest.raises(ExternalError):
-        report1 = make_sequential(1, 0x1000, 10, give_ids=True, root_key=1, signer=signer)
+        report1 = make_sequential(1, 0x1000, 10, give_ids=True, root_key=AuthProvider.UserKey, signer=signer)
 
-    report1 = make_sequential(2, 0x1000, 10, give_ids=True, root_key=1, signer=signer)
+    report1 = make_sequential(2, 0x1000, 10, give_ids=True, root_key=AuthProvider.UserKey, signer=signer)
 
     encoded = report1.encode()
     report2 = SignedListReport(encoded)
 
-    assert report1.signature_flags == 1
-    assert report2.signature_flags == 1
+    assert report1.signature_flags == AuthProvider.UserKey
+    assert report2.signature_flags == AuthProvider.UserKey
     assert report1.verified
     assert report1.encrypted
     assert report2.verified

transport_plugins/bled112/RELEASE.md

@@ -2,6 +2,16 @@
 
 All major changes in each released version of the bled112 transport plugin are listed here.
 
+## HEAD
+
+- Refactor BLE broadcast encryption flags: three bits are treated as an enumeration
+- Consilidate two authentication characteristics into one
+- Update info characteristic
+- Remove separate logic for NullKey encryption temp key generation, the temp key is
+  generated the same way it is done for user key
+- Add the password-based authentication method
+- Update the authentication flow
+
 ## 3.0.3
 
 - Fix bled112_auth error handling

transport_plugins/bled112/iotile_transport_bled112/bled112.py

@@ -311,6 +311,10 @@ def send_rpc_async(self, conn_id, address, rpc_id, payload, timeout, callback):
 
         services = self._connections[found_handle]['services']
 
+        if self.check_is_rpc_in_progress(found_handle, services):
+            callback(conn_id, self.id, False, 'RPC still in progress', None, None)
+            return
+
         self._command_task.async_command(['_send_rpc', found_handle, services, address, rpc_id, payload, timeout], self._send_rpc_finished,
                                          {'connection_id': conn_id, 'handle': found_handle,
                                           'callback': callback})
@@ -664,17 +668,13 @@ def _parse_v2_advertisement(self, rssi, sender, data):
         #   bit 0: Has pending data to stream
         #   bit 1: Low voltage indication
         #   bit 2: User connected
-        #   bit 3: Broadcast data is encrypted
-        #   bit 4: Encryption key is device key
-        #   bit 5: Encryption key is user key
+        #   bit 3 - 5: Broadcast encryption key type
         #   bit 6: broadcast data is time synchronized to avoid leaking
         #   information about when it changes
         is_pending_data = bool(flags & (1 << 0))
         is_low_voltage = bool(flags & (1 << 1))
         is_user_connected = bool(flags & (1 << 2))
-        is_encrypted = bool(flags & (1 << 3))
-        is_device_key = bool(flags & (1 << 4))
-        is_user_key = bool(flags & (1 << 5))
+        broadcast_encryption_key_type = (flags >> 3) & 7
 
         self._device_scan_counts.setdefault(device_id, {'v1': 0, 'v2': 0})['v2'] += 1
 
@@ -691,24 +691,17 @@ def _parse_v2_advertisement(self, rssi, sender, data):
                 'battery': battery / 32.0,
                 'advertising_version':2}
 
-        key_type = AuthProvider.NoKey
-        if is_encrypted:
-            if is_device_key:
-                key_type = AuthProvider.DeviceKey
-            elif is_user_key:
-                key_type = AuthProvider.UserKey
-
-        if is_encrypted:
+        if broadcast_encryption_key_type:
             if not _HAS_CRYPTO:
                 return info, timestamp, None, None, None, None, None
 
             try:
-                key = self._key_provider.get_rotated_key(key_type, device_id,
+                key = self._key_provider.get_rotated_key(broadcast_encryption_key_type, device_id,
                     reboot_counter=reboots,
                     rotation_interval_power=EPHEMERAL_KEY_CYCLE_POWER,
                     current_timestamp=timestamp)
             except NotFoundError:
-                self._logger.warning("Key type {} is not found".format(key_type), exc_info=True)
+                self._logger.warning("Key type {} is not found".format(broadcast_encryption_key_type), exc_info=True)
                 return info, timestamp, None, None, None, None, None
 
             nonce = generate_nonce(device_id, timestamp, reboot_low, reboot_high_packed, counter_packed)
@@ -870,6 +863,27 @@ def check_authentication(self, uuid, conn_id, handle, services):
                                           'handle': handle,
                                           'services': services})
 
+    def check_is_rpc_in_progress(self, handle, services):
+        """ Discover if the device handles RPC at the moment
+
+        Another RPC should not be sent to the device if handling of previous
+        is not finished
+
+        Args:
+            handle (int): a handle to the connection on the BLED112 dongle
+            services (dict): A dictionary of GATT services produced by probe_services()
+
+        Returns:
+            bool: True if RPC is being handled at the moment
+        """
+        RPC_IN_PROGRESS_FLAG = 0x0001
+        try:
+            value = self._command_task.sync_command(["get_info_flags", handle, services])
+            version, _, high_flags = struct.unpack("BBH16x", value['data'])
+            return (high_flags & RPC_IN_PROGRESS_FLAG) == 0x01
+        except HardwareError:
+            return False
+
     def initialize_system_sync(self):
         """Remove all active connections and query the maximum number of supported connections
         """
@@ -1125,8 +1139,8 @@ def _on_authentication_check_response(self, result):
         context = result['context']
 
         if result['result']:
-            flags, = struct.unpack("B", result['return_value']['data'])
-            if flags == 0x01:
+            version, security_flags, _ = struct.unpack("BBH16x", result['return_value']['data'])
+            if security_flags == 0x01:
                 self._logger.debug("Authentication is required")
 
                 self.authenticate(context['uuid'], context['connection_id'],