fix: normalize peer address before duplicate-identity comparison

src/ble_reticulum/BLEInterface.py

@@ -1047,7 +1047,15 @@ def _check_duplicate_identity(self, address: str, peer_identity: bytes) -> bool:
         identity_hash = self._compute_identity_hash(peer_identity)
         existing_address = self.identity_to_address.get(identity_hash)
 
-        if existing_address and existing_address != address:
+        # Compare normalized addresses so the same physical peer is recognized
+        # regardless of whether the stored/incoming form carries the BlueZ
+        # "dev:" prefix or differs in case. Without this, a fixed-MAC peer that
+        # is connected via one mode (e.g. peripheral, "dev:AA:BB") and
+        # reconnects via the other (central, "AA:BB") is wrongly treated as a
+        # different MAC (false Android MAC rotation) and rejected.
+        normalized_incoming = self._normalize_address(address)
+
+        if existing_address and self._normalize_address(existing_address) != normalized_incoming:
             # Same identity, different MAC - check if old connection is still alive
 
             # Check 1: Is there a pending detach for this identity?
@@ -1690,7 +1698,13 @@ def _select_peers_to_connect(self):
                 if identity_hash in self.spawned_interfaces:
                     # Check if existing interface is still connected
                     existing_address = self.identity_to_address.get(identity_hash)
-                    if existing_address and existing_address != address:
+                    # Normalize so a stored "dev:AA:BB" (peripheral form) and an
+                    # incoming "AA:BB" (central form) for the same fixed-MAC
+                    # peer compare equal instead of being treated as a MAC
+                    # rotation. See _normalize_address.
+                    norm_existing = self._normalize_address(existing_address)
+                    norm_address = self._normalize_address(address)
+                    if existing_address and norm_existing != norm_address:
                         # Same identity at different MAC = MAC rotation
                         # Check if old connection is still alive
                         if existing_address in self.peers:
@@ -1709,8 +1723,17 @@ def _select_peers_to_connect(self):
                             score = self._score_peer(peer)
                             scored_peers.append((score, peer))
                             continue  # Skip remaining checks, peer already added
-                    elif existing_address == address:
-                        # Same address, interface exists - skip
+                    elif existing_address and norm_existing == norm_address:
+                        # Same physical peer (MAC matches once normalized, e.g.
+                        # a stored "dev:AA:BB" peripheral form vs a scanned
+                        # "AA:BB" central form for the same fixed-MAC peer).
+                        # An interface already exists for this identity, so skip
+                        # it here - the same-MAC reconnect path is the standard
+                        # top-level gate (the "address in self.peers" skip), not
+                        # this branch. This decision is pinned by
+                        # tests/test_ble_dup_identity_mac_normalize.py
+                        # (TestScanLoopSameAddressRegression) so a future change
+                        # to it cannot silently alter reconnect behavior.
                         RNS.log(f"{self} [v2.2] skipping {peer.name} - interface exists for identity {identity_hash[:8]}",
                                 RNS.LOG_DEBUG)
                         continue
@@ -1889,6 +1912,37 @@ def _compute_identity_hash(self, peer_identity):
         # Just convert to hex, don't re-hash (that would corrupt the identity!)
         return peer_identity.hex()[:16]
 
+    def _normalize_address(self, address):
+        """
+        Normalize a peer address for identity-mapping comparisons.
+
+        The same physical peer can be represented by more than one address
+        string depending on which code path saw it:
+
+          * peripheral (GATT) callbacks carry the BlueZ D-Bus device-path form
+            with a "dev:" prefix  -> "dev:B8:27:EB:43:04:BC"
+          * central (scan/connect) paths carry the bare MAC      -> "B8:27:EB:43:04:BC"
+
+        identity_to_address is written by whichever path stores it first, so a
+        later comparison against the other path's form would see "dev:AA:BB"
+        != "AA:BB" and wrongly conclude a different MAC (a false Android MAC
+        rotation) for a fixed-MAC peer. Stripping the "dev:" prefix and
+        upper-casing makes the same physical MAC compare equal regardless of
+        which form it arrived in, while genuinely different MACs still differ.
+
+        Args:
+            address: peer address string (may carry a "dev:" prefix)
+
+        Returns:
+            str: normalized address, or "" if address is empty
+        """
+        if not address:
+            return ""
+        addr = address.strip()
+        if addr.lower().startswith("dev:"):
+            addr = addr[4:]
+        return addr.upper()
+
     def _spawn_peer_interface(self, address, name, peer_identity, client=None, mtu=None, connection_type="central"):
         """
         Create a peer interface for a BLE connection.

tests/test_ble_dup_identity_mac_normalize.py

@@ -0,0 +1,258 @@
+"""
+TDD test for the false-positive "Android MAC rotation" duplicate-identity rejection.
+
+BUG DESCRIPTION
+---------------
+On two peers with FIXED MACs that both run central + peripheral mode, the same
+physical peer is represented by two different address strings:
+
+  * peripheral (GATT) path  -> "dev:B8:27:EB:43:04:BC"   (BlueZ D-Bus 'dev:' prefix)
+  * central (scan) path     -> "B8:27:EB:43:04:BC"       (bare MAC)
+
+BLEInterface.identity_to_address stores the peripheral (dev:-prefixed) form,
+while the incoming central-mode handshake arrives with the bare form. The
+_check_duplicate_identity() comparison:
+
+    if existing_address and existing_address != address:   # "different MAC"?
+
+therefore sees "dev:AA:BB" != "AA:BB" and concludes a false Android MAC rotation.
+Because the existing (peripheral) connection is still alive, it REJECTS the
+incoming connection and the peer interface is detached after the grace period.
+The data path is then dropped: discovery announces are routed to 0 peers and the
+peer never appears in the destination table.
+
+OBSERVED (from logs, two touching Pi Zero 2 W, fixed MACs):
+    GATTServer: Central connected: dev:B8:27:EB:43:04:BC (MTU: 517)
+    duplicate identity detected: 211155f3 already connected via dev:B8:27:EB:43:04:BC,
+        rejecting connection from B8:27:EB:43:04:BC (Android MAC rotation)
+    scheduled detach ... / detached interface ... (grace period)
+    TX: 183 bytes to 0 peer(s)
+
+EXPECTED BEHAVIOR
+-----------------
+_check_duplicate_identity must treat "dev:AA:BB:.." and "AA:BB:.." as the SAME
+address (same physical peer), so a fixed-MAC peer reconnecting via a different
+mode is NOT rejected as a duplicate. True Android MAC rotation (genuinely
+different MACs) must still be handled.
+
+This test calls the REAL _check_duplicate_identity (not a replicated copy) and
+should FAIL before the fix and PASS after.
+"""
+
+import pytest
+import sys
+import os
+
+# Add src to path
+sys.path.insert(0, os.path.join(os.path.dirname(__file__), '..'))
+sys.path.insert(0, os.path.join(os.path.dirname(__file__), '../src'))
+
+from unittest.mock import MagicMock
+
+import RNS
+if not hasattr(RNS, 'LOG_INFO'):
+    RNS.LOG_CRITICAL = 0
+    RNS.LOG_ERROR = 1
+    RNS.LOG_WARNING = 2
+    RNS.LOG_NOTICE = 3
+    RNS.LOG_INFO = 4
+    RNS.LOG_VERBOSE = 5
+    RNS.LOG_DEBUG = 6
+    RNS.LOG_EXTREME = 7
+    RNS.log = lambda msg, level=4: None
+    RNS.prettyhexrep = lambda data: data.hex() if isinstance(data, bytes) else str(data)
+    RNS.hexrep = lambda data, delimit=True: data.hex() if isinstance(data, bytes) else str(data)
+
+if not hasattr(RNS, 'Transport'):
+    RNS.Transport = MagicMock()
+    RNS.Transport.interfaces = []
+
+if not hasattr(RNS, 'Identity'):
+    RNS.Identity = MagicMock()
+    RNS.Identity.full_hash = lambda x: (x * 2)[:16]
+
+import sys as _sys
+if 'ble_reticulum.Interface' not in _sys.modules:
+    class MockInterface:
+        MODE_FULL = 1
+        def __init__(self):
+            self.IN = True
+            self.OUT = True
+            self.online = False
+        @staticmethod
+        def get_config_obj(configuration):
+            class ConfigObj:
+                def __init__(self, config):
+                    self._config = config if config else {}
+                def __getitem__(self, key):
+                    return self._config.get(key)
+                def get(self, key, default=None):
+                    return self._config.get(key, default)
+                def as_string(self, key, default=None):
+                    val = self._config.get(key)
+                    return str(val) if val is not None else default
+                def as_int(self, key, default=None):
+                    val = self._config.get(key)
+                    return int(val) if val is not None else default
+                def as_bool(self, key, default=False):
+                    val = self._config.get(key)
+                    if isinstance(val, bool):
+                        return val
+                    if isinstance(val, str):
+                        return val.lower() in ('true', 'yes', '1', 'on')
+                    return bool(val) if val is not None else default
+            return ConfigObj(configuration)
+
+    interface_module = MagicMock()
+    interface_module.Interface = MockInterface
+    _sys.modules['ble_reticulum.Interface'] = interface_module
+
+from tests.mock_ble_driver import MockBLEDriver
+from ble_reticulum.BLEInterface import BLEInterface
+
+def _make_interface(local_mac):
+    driver = MockBLEDriver(local_address=local_mac)
+    owner = MagicMock()
+    owner.inbound = MagicMock()
+    config = {"name": "Test", "enable_central": True, "enable_peripheral": True}
+    interface = BLEInterface(owner, config)
+    interface.driver = driver
+    interface.local_address = driver.local_address
+    return interface
+
+
+class TestDuplicateIdentityMacNormalization:
+    """Fixed-MAC peers must not be rejected as a false Android MAC rotation."""
+
+    def test_dev_prefix_same_mac_is_not_duplicate(self):
+        """
+        The exact observed case: identity_to_address holds the dev:-prefixed
+        peripheral address and is still connected; the incoming central handshake
+        carries the bare form of the SAME MAC. This is the same physical peer,
+        NOT a MAC rotation, so it must NOT be rejected.
+        """
+        interface = _make_interface("11:22:33:44:55:66")
+        peer_identity = b"\x01" * 16
+        h = interface._compute_identity_hash(peer_identity)
+
+        # Peripheral (GATT) connection is alive and stored with the dev: prefix.
+        dev_addr = "dev:B8:27:EB:43:04:BC"
+        bare_addr = "B8:27:EB:43:04:BC"
+        interface.identity_to_address[h] = dev_addr
+        interface.peers[dev_addr] = (MagicMock(), 0, 517)
+        interface.driver.connect(dev_addr)  # keep it in driver.connected_peers
+
+        # Central-mode handshake arrives with the bare MAC (same physical peer).
+        rejected = interface._check_duplicate_identity(bare_addr, peer_identity)
+
+        # Same MAC (differing only by the dev: prefix) is NOT a duplicate.
+        assert rejected is False, (
+            "A fixed-MAC peer reconnecting via a different mode was falsely "
+            "rejected as an Android MAC rotation (dev: prefix not normalized)."
+        )
+
+    def test_case_only_difference_is_not_duplicate(self):
+        """
+        Uppercase vs lowercase of the same MAC must also compare equal.
+        """
+        interface = _make_interface("11:22:33:44:55:66")
+        peer_identity = b"\x02" * 16
+        h = interface._compute_identity_hash(peer_identity)
+
+        stored = "B8:27:EB:43:04:BC"
+        incoming = "b8:27:eb:43:04:bc"
+        interface.identity_to_address[h] = stored
+        interface.peers[stored] = (MagicMock(), 0, 517)
+        interface.driver.connect(stored)
+
+        rejected = interface._check_duplicate_identity(incoming, peer_identity)
+        assert rejected is False, (
+            "Case-only difference of the same MAC was falsely rejected as a "
+            "duplicate identity."
+        )
+
+    def test_genuine_different_mac_still_evaluated(self):
+        """
+        A genuinely different MAC for the same identity (true Android MAC
+        rotation) must still be rejected while the old connection is alive,
+        so the fix does not disable MAC-rotation protection.
+        """
+        interface = _make_interface("11:22:33:44:55:66")
+        peer_identity = b"\x03" * 16
+        h = interface._compute_identity_hash(peer_identity)
+
+        old_mac = "B8:27:EB:43:04:BC"
+        new_mac = "CA:FE:DE:AD:00:01"
+        interface.identity_to_address[h] = old_mac
+        interface.peers[old_mac] = (MagicMock(), 0, 517)
+        interface.driver.connect(old_mac)  # old connection alive
+
+        rejected = interface._check_duplicate_identity(new_mac, peer_identity)
+        assert rejected is True, (
+            "A genuinely different MAC for the same identity (true MAC rotation) "
+            "with the old connection still alive must still be rejected."
+        )
+
+
+class TestBareMacMobileSafety:
+    """
+    Cross-platform blast-radius pin for the shared BLEInterface.
+
+    Android (AndroidBLEInterface) and iOS both SUBCLASS the shared
+    BLEInterface and wire on_duplicate_identity_detected -> _check_duplicate_identity
+    with no override. Their drivers (Kotlin CoreBluetooth/Bluetooth LE) produce
+    BARE, consistently-cased MACs and never the BlueZ "dev:" prefix that is the
+    whole point of the headline Linux fix. So on mobile the normalizer is a
+    no-op for the common case (same-case identical MAC) and the change must be
+    strictly safer / behavior-identical there.
+
+    These tests pin that contract on the REAL method so a future edit cannot
+    silently change mobile behavior:
+      * bare MAC, identical case, connection alive -> NOT a duplicate (unchanged)
+      * bare MAC, different MAC, connection alive  -> still rejected (guard holds)
+    """
+
+    def test_bare_mac_identical_same_case_is_not_duplicate(self):
+        """
+        The common mobile case: stored and incoming are the same bare MAC in the
+        same case, connection alive. Must NOT be treated as a duplicate - i.e.
+        the normalizer changes nothing here (no-op), exactly as before the fix.
+        """
+        interface = _make_interface("11:22:33:44:55:66")
+        peer_identity = b"\x0a" * 16
+        h = interface._compute_identity_hash(peer_identity)
+
+        bare = "B8:27:EB:43:04:BC"
+        interface.identity_to_address[h] = bare
+        interface.peers[bare] = (MagicMock(), 0, 517)
+        interface.driver.connect(bare)  # alive
+
+        rejected = interface._check_duplicate_identity(bare, peer_identity)
+        assert rejected is False, (
+            "Mobile no-op regression: a bare same-case MAC for an already-"
+            "connected peer must NOT be rejected (behavior unchanged by the "
+            "normalizer)."
+        )
+
+    def test_bare_mac_genuine_rotation_still_rejected(self):
+        """
+        Mobile guard: a genuinely different BARE MAC for the same identity with
+        the old connection alive must STILL be rejected - normalization preserves
+        the hex difference, so true MAC rotation is not disabled on mobile.
+        """
+        interface = _make_interface("11:22:33:44:55:66")
+        peer_identity = b"\x0b" * 16
+        h = interface._compute_identity_hash(peer_identity)
+
+        old_mac = "B8:27:EB:43:04:BC"
+        new_mac = "CA:FE:DE:AD:00:02"  # genuinely different bare MAC
+        interface.identity_to_address[h] = old_mac
+        interface.peers[old_mac] = (MagicMock(), 0, 517)
+        interface.driver.connect(old_mac)  # old connection alive
+
+        rejected = interface._check_duplicate_identity(new_mac, peer_identity)
+        assert rejected is True, (
+            "Mobile guard regression: a genuinely different bare MAC for the "
+            "same identity (true rotation) with the old connection alive must "
+            "still be rejected - the fix must not disable rotation protection."
+        )

tests/test_mac_rotation_blacklist_bug.py

@@ -71,6 +71,7 @@ def mock_ble_interface(self):
         interface._record_connection_failure = lambda addr: RealInterface._record_connection_failure(interface, addr)
         interface._is_blacklisted = lambda addr: RealInterface._is_blacklisted(interface, addr)
         interface._compute_identity_hash = lambda identity: RealInterface._compute_identity_hash(interface, identity)
+        interface._normalize_address = lambda addr: RealInterface._normalize_address(interface, addr)
 
         return interface
 
@@ -402,6 +403,7 @@ def mock_ble_interface(self):
 
         interface._check_duplicate_identity = lambda addr, identity: RealInterface._check_duplicate_identity(interface, addr, identity)
         interface._compute_identity_hash = lambda identity: RealInterface._compute_identity_hash(interface, identity)
+        interface._normalize_address = lambda addr: RealInterface._normalize_address(interface, addr)
         interface._handle_identity_handshake = lambda addr, data: RealInterface._handle_identity_handshake(interface, addr, data)
 
         return interface