diff --git a/src/meshcore_console/meshcore/channel_db.py b/src/meshcore_console/meshcore/channel_db.py index 29662ff..1f21daf 100644 --- a/src/meshcore_console/meshcore/channel_db.py +++ b/src/meshcore_console/meshcore/channel_db.py @@ -1,5 +1,6 @@ from __future__ import annotations +import hashlib import sqlite3 from dataclasses import dataclass @@ -8,6 +9,29 @@ PUBLIC_CHANNEL_SECRET = "8b3387e9c5cdea6ac9e5edbaa115cd72" +def normalize_channel_name(name: str) -> str: + """Canonical on-the-wire form of a hashtag channel name: no '#', lowercase.""" + return name.strip().lstrip("#").strip().lower() + + +def derive_channel_secret(name: str) -> str: + """Derive the deterministic secret for a hashtag channel. + + MeshCore hashtag channel secrets are the first 32 hex chars of + SHA-256("#" + channel_name), with the name lowercased. E.g. + sha256("#chicago")[:32] == "c1c289b131e5222370cbc2048445844b" (value + verified against a real device in issue #81). + + The lowercasing is essential, not cosmetic: the secret *is* the shared key + for the channel, so two users typing "#Chicago" and "#chicago" must derive + the same key or they silently cannot decrypt each other's messages. + Normalizing here means no caller can get it wrong by passing a display name + that preserved the user's original capitalization. + """ + clean = normalize_channel_name(name) + return hashlib.sha256(f"#{clean}".encode()).hexdigest()[:32] + + @dataclass class ChannelConfig: name: str @@ -31,9 +55,57 @@ def add_channel(self, name: str, secret: str) -> None: ) self._conn.commit() + def ensure_channel_secret(self, name: str) -> str: + """Ensure a secret exists for the named hashtag channel. + + Derives the deterministic secret if no row exists (matched + case-insensitively so an existing 'Public'/'Chicago' entry is never + overwritten). Returns the secret in effect for the channel. + """ + row = self._conn.execute( + "SELECT secret FROM channel_secrets WHERE name = ? COLLATE NOCASE", + (normalize_channel_name(name),), + ).fetchone() + if row is not None: + return row[0] + secret = derive_channel_secret(name) + self.add_channel(normalize_channel_name(name), secret) + return secret + + def resolve_name(self, name: str) -> str | None: + """Return the stored name for a channel, matched case-insensitively. + + pyMC_core matches ``channels_config`` entries by exact name, so senders + must use the name as stored here ("Public"), not whatever casing the UI + happens to display. + """ + row = self._conn.execute( + "SELECT name FROM channel_secrets WHERE name = ? COLLATE NOCASE", + (normalize_channel_name(name),), + ).fetchone() + return row[0] if row is not None else None + def remove_channel(self, name: str) -> None: - self._conn.execute("DELETE FROM channel_secrets WHERE name = ?", (name,)) + self._conn.execute("DELETE FROM channel_secrets WHERE name = ? COLLATE NOCASE", (name,)) + self._conn.commit() + + def remove_derived_secret(self, name: str) -> bool: + """Delete a channel secret only if it is the value derived from its name. + + A hashtag channel's secret can always be re-derived, so dropping it with + the channel is harmless. A secret imported with ``radio_cli + import-channel`` cannot be recovered, so removing the channel from the + UI must not destroy it. Returns True if a row was deleted. + """ + row = self._conn.execute( + "SELECT name, secret FROM channel_secrets WHERE name = ? COLLATE NOCASE", + (normalize_channel_name(name),), + ).fetchone() + if row is None or row[1] != derive_channel_secret(row[0]): + return False + self._conn.execute("DELETE FROM channel_secrets WHERE name = ?", (row[0],)) self._conn.commit() + return True def get_channels(self) -> list[dict[str, str]]: """Return channels in the format expected by pymc_core GroupTextHandler.""" diff --git a/src/meshcore_console/meshcore/client.py b/src/meshcore_console/meshcore/client.py index 88de88d..bf0b567 100644 --- a/src/meshcore_console/meshcore/client.py +++ b/src/meshcore_console/meshcore/client.py @@ -70,6 +70,7 @@ def __init__( self._peer_store = peer_store or PeerStore(self._db) self._channel_store = channel_store or UIChannelStore(self._db) self._repeater_password_store = RepeaterPasswordStore(self._db) + self._channel_secrets = ChannelDatabase(self._db) self._repeater_sessions: dict[str, RepeaterLoginState] = {} # Load persisted state self._messages: list[Message] = self._message_store.get_all() @@ -104,8 +105,7 @@ def __init__( def _sync_channel_secrets_to_ui(self) -> None: """Ensure every channel secret has a corresponding UI channel entry.""" - channel_db = ChannelDatabase(self._db) - for row in channel_db.get_channels(): + for row in self._channel_secrets.get_channels(): original_name = row["name"] # Preserve original case for pyMC_core channel_id = original_name.lower() if channel_id not in self._channels: @@ -252,8 +252,21 @@ def ensure_channel(self, channel_id: str, display_name: str | None = None) -> Ch ) self._channels[normalized_id] = channel self._channel_store.add_or_update(channel) + if is_group: + # Hashtag channels need a row in channel_secrets or pyMC_core cannot + # encrypt for them at send time and cannot match them at receive + # time (issue #81). + self._ensure_channel_secret(channel) return channel + def _ensure_channel_secret(self, channel: Channel) -> str: + """Ensure a group channel has a secret, and return its on-the-wire name.""" + name = channel.display_name.lstrip("#") or channel.channel_id + self._channel_secrets.ensure_channel_secret(name) + # pyMC_core matches channels_config by exact name, so send with the name + # as stored ("Public"), not the UI display name ("#public"). + return self._channel_secrets.resolve_name(name) or name + def list_messages_for_channel(self, channel_id: str, limit: int = 50) -> list[Message]: filtered = [m for m in self._messages if m.channel_id == channel_id] return filtered[-limit:] @@ -262,7 +275,12 @@ def remove_channel(self, channel_id: str) -> bool: """Remove a channel and its messages. Returns False if channel cannot be removed.""" if channel_id == "public": return False - self._channels.pop(channel_id, None) + removed = self._channels.pop(channel_id, None) + if removed is not None and removed.kind == "group": + # Only derived hashtag secrets are dropped; an imported secret is + # not recoverable, so it outlives the channel entry (issue #81). + name = removed.display_name.lstrip("#") or removed.channel_id + self._channel_secrets.remove_derived_secret(name) self._messages = [m for m in self._messages if m.channel_id != channel_id] self._channel_store.remove(channel_id) self._message_store.remove_for_channel(channel_id) @@ -298,10 +316,10 @@ def send_message(self, peer_id: str, body: str) -> Message: self._channel_store.add_or_update(channel) if is_group: - # Resolve the original-case channel name for pyMC_core. - # display_name is "#ChannelName" — strip the "#" prefix. - channel = self._channels[channel_id] - channel_name = channel.display_name.lstrip("#") + # Ensure a secret exists and resolve the name pyMC_core knows the + # channel by. Done on every send, not just for newly created + # channels, so a channel that predates issue #81 is repaired too. + channel_name = self._ensure_channel_secret(self._channels[channel_id]) self._run_async(self._session.send_group_text(channel_name=channel_name, message=body)) else: # Use the original-case peer name from the channel so pyMC_core diff --git a/src/meshcore_console/meshcore/db.py b/src/meshcore_console/meshcore/db.py index 629fe61..d1773b2 100644 --- a/src/meshcore_console/meshcore/db.py +++ b/src/meshcore_console/meshcore/db.py @@ -2,9 +2,11 @@ from __future__ import annotations +import hashlib import logging import sqlite3 +from .channel_db import PUBLIC_CHANNEL_SECRET from .paths import db_path logger = logging.getLogger(__name__) @@ -90,6 +92,11 @@ created_at TEXT NOT NULL )""", ), + # v6 -> v7: backfill channel_secrets for group channels added before the + # fix for issue #81. The secret is sha256("#" + name)[:32], which cannot be + # expressed in SQL, so the work happens in _backfill_channel_secrets(); + # this entry exists only to bump the schema version. + (), ] @@ -102,6 +109,55 @@ def _get_version(conn: sqlite3.Connection) -> int: return 0 +def _backfill_channel_secrets(conn: sqlite3.Connection) -> None: + """Derive missing secrets for group channels (issue #81). + + Hashtag channels added before the fix have a row in `channels` but none in + `channel_secrets`, so sending to them failed with "not in provided + channels_config". Derive the secret for any such channel. + + Public is special: its secret is a fixed constant shared by all MeshCore + devices, *not* the derived sha256("#Public")[:32]. It is normally seeded by + ChannelDatabase.__init__, but that runs after open_db() -> _migrate(), so we + cannot assume it is present yet. Seed it here and exclude it from derivation; + otherwise the backfill would mint a bogus derived secret for #public and + break decryption on the default channel. + + Idempotent: channels that already have a secret (matched case-insensitively) + are left untouched. + """ + # Seed Public up-front so it is never treated as an orphan needing derivation. + conn.execute( + "INSERT OR IGNORE INTO channel_secrets (name, secret) VALUES (?, ?)", + ("Public", PUBLIC_CHANNEL_SECRET), + ) + + # LOWER() on both sides: SQLite binds COLLATE NOCASE to a column, not to an + # expression like LTRIM(...), so relying on it here would silently compare + # case-sensitively. + rows = conn.execute( + """SELECT c.channel_id, c.display_name FROM channels c + WHERE c.kind = 'group' + AND LOWER(c.channel_id) != 'public' + AND NOT EXISTS ( + SELECT 1 FROM channel_secrets s + WHERE LOWER(s.name) = LOWER(c.channel_id) + OR LOWER(s.name) = LOWER(LTRIM(c.display_name, '#')) + )""" + ).fetchall() + for channel_id, display_name in rows: + # Lowercase: the derived secret is keyed on the lowercased name (see + # derive_channel_secret), so the stored name must match, or peers using + # the same channel would end up with a different key. + name = ((display_name or "").lstrip("#") or channel_id).strip().lower() + secret = hashlib.sha256(f"#{name}".encode()).hexdigest()[:32] + conn.execute( + "INSERT OR IGNORE INTO channel_secrets (name, secret) VALUES (?, ?)", + (name, secret), + ) + logger.info("Backfilled channel secret for #%s (issue #81)", name) + + def _migrate(conn: sqlite3.Connection) -> None: """Run any outstanding migrations.""" current = _get_version(conn) @@ -112,6 +168,10 @@ def _migrate(conn: sqlite3.Connection) -> None: for version_index in range(current, target): for stmt in MIGRATIONS[version_index]: conn.execute(stmt) + # Data backfills run after all DDL so the columns they read (e.g. channels.kind, + # added in v5) are guaranteed to exist. + if current < 7: + _backfill_channel_secrets(conn) conn.execute("UPDATE schema_version SET version = ?", (target,)) conn.commit() diff --git a/tests/unit/test_channel_secrets.py b/tests/unit/test_channel_secrets.py new file mode 100644 index 0000000..add05ff --- /dev/null +++ b/tests/unit/test_channel_secrets.py @@ -0,0 +1,288 @@ +"""Regression tests for issue #81: hashtag channels must get derived secrets.""" + +from __future__ import annotations + +import pytest + +from meshcore_console.meshcore.channel_db import ( + PUBLIC_CHANNEL_SECRET, + ChannelDatabase, + derive_channel_secret, +) +from meshcore_console.meshcore.db import open_db + + +@pytest.fixture() +def conn(tmp_path): + c = open_db(str(tmp_path / "test.db")) + yield c + c.close() + + +def test_derive_channel_secret_known_value() -> None: + # Verified against MeshCore: sha256("#chicago")[:32] + assert derive_channel_secret("chicago") == "c1c289b131e5222370cbc2048445844b" + + +def test_derive_channel_secret_strips_hash_prefix() -> None: + assert derive_channel_secret("#chicago") == derive_channel_secret("chicago") + + +def test_derive_channel_secret_is_case_insensitive() -> None: + """The secret IS the shared key: #Chicago and #chicago must match, or two + users who capitalize differently silently cannot decrypt each other.""" + expected = "c1c289b131e5222370cbc2048445844b" # verified on-device, issue #81 + for variant in ("chicago", "Chicago", "#CHICAGO", " #ChIcAgO "): + assert derive_channel_secret(variant) == expected + + +def test_ensure_channel_secret_inserts_derived_secret(conn) -> None: + db = ChannelDatabase(conn) + secret = db.ensure_channel_secret("bot") + assert secret == derive_channel_secret("bot") + row = db.get_channel("bot") + assert row is not None + assert row["secret"] == secret + + +def test_ensure_channel_secret_does_not_overwrite_existing(conn) -> None: + db = ChannelDatabase(conn) + # Public has a hardcoded (non-derived) secret; ensure must not replace it. + assert db.ensure_channel_secret("Public") == PUBLIC_CHANNEL_SECRET + assert db.get_channel("Public")["secret"] == PUBLIC_CHANNEL_SECRET + # Case-insensitive match protects against duplicate rows too. + assert db.ensure_channel_secret("public") == PUBLIC_CHANNEL_SECRET + names = [c["name"] for c in db.get_channels()] + assert names.count("Public") == 1 + assert "public" not in names + + +def test_remove_channel_is_case_insensitive(conn) -> None: + db = ChannelDatabase(conn) + db.add_channel("Bot", derive_channel_secret("Bot")) + db.remove_channel("bot") + assert db.get_channel("Bot") is None + + +def test_client_ensure_channel_stores_secret(tmp_path, monkeypatch) -> None: + """End-to-end: the '+ Add Channel' path must persist a channel secret.""" + from meshcore_console.meshcore import client as client_mod + from meshcore_console.meshcore.client import MeshcoreClient + from meshcore_console.meshcore.config import runtime_config_from_settings + from meshcore_console.meshcore.settings import MeshcoreSettings + from meshcore_console.mock import MockPyMCCoreSession + + db_conn = open_db(str(tmp_path / "client.db")) + monkeypatch.setattr(client_mod, "open_db", lambda *a, **k: db_conn) + + client = MeshcoreClient( + session=MockPyMCCoreSession(runtime_config_from_settings(MeshcoreSettings())), + require_pymc=False, + ) + + # Mirrors the UI '+ Add Channel' handler (messages.py). + client.ensure_channel("bot", "#bot") + + channel_db = ChannelDatabase(db_conn) + row = channel_db.get_channel("bot") + assert row is not None, "ensure_channel must insert into channel_secrets (#81)" + assert row["secret"] == derive_channel_secret("bot") + + # Removing the channel cleans the secret up again. + assert client.remove_channel("bot") is True + assert channel_db.get_channel("bot") is None + + # Public is never removable and keeps its hardcoded secret. + assert client.remove_channel("public") is False + assert channel_db.get_channel("Public")["secret"] == PUBLIC_CHANNEL_SECRET + + +def _client(tmp_path, monkeypatch, name): + from meshcore_console.meshcore import client as client_mod + from meshcore_console.meshcore.client import MeshcoreClient + from meshcore_console.meshcore.config import runtime_config_from_settings + from meshcore_console.meshcore.settings import MeshcoreSettings + from meshcore_console.mock import MockPyMCCoreSession + + db_conn = open_db(str(tmp_path / name)) + monkeypatch.setattr(client_mod, "open_db", lambda *a, **k: db_conn) + client = MeshcoreClient( + session=MockPyMCCoreSession(runtime_config_from_settings(MeshcoreSettings())), + require_pymc=False, + ) + return client, db_conn + + +def test_remove_channel_keeps_imported_secret(tmp_path, monkeypatch) -> None: + """An imported secret cannot be re-derived, so it must survive removal.""" + client, db_conn = _client(tmp_path, monkeypatch, "imported.db") + channel_db = ChannelDatabase(db_conn) + channel_db.add_channel("MyPrivate", "00112233445566778899aabbccddeeff") + client.ensure_channel("MyPrivate", "#MyPrivate") + + assert client.remove_channel("MyPrivate") is True + row = channel_db.get_channel("MyPrivate") + assert row is not None, "removing a channel must not destroy an imported PSK" + assert row["secret"] == "00112233445566778899aabbccddeeff" + + +@pytest.mark.parametrize( + ("channel_id", "display_name"), + [ + ("bot", "#bot"), # added via '+ Add Channel' + ("public", "#Public"), # hardcoded secret, stored capitalised + ("chicago", "#Chicago"), # backfilled lowercase, displayed mixed-case + ], +) +def test_send_uses_the_name_stored_in_channel_secrets( + tmp_path, monkeypatch, channel_id, display_name +) -> None: + """pyMC_core matches channels_config by exact name, so the name passed to + send_group_text must appear verbatim in channel_secrets (issue #81).""" + client, db_conn = _client(tmp_path, monkeypatch, f"send-{channel_id}.db") + client.ensure_channel(channel_id, display_name) + + sent: list[str] = [] + + async def capture(channel_name: str, message: str): + sent.append(channel_name) + return {"ok": True} + + monkeypatch.setattr(client._session, "send_group_text", capture) + client._connected = True # skip radio pre-flight; only the send name matters + client.send_message(peer_id=channel_id, body="hi") + + known = [c["name"] for c in ChannelDatabase(db_conn).get_channels()] + assert sent == [sent[0]] + assert sent[0] in known, f"{sent[0]!r} not in channels_config {known}" + + +# --------------------------------------------------------------------------- +# Migration backfill (issue #81, existing installs) +# --------------------------------------------------------------------------- + + +def _open_v6_db(path): + """Open a db and rewind it to v6 (pre-backfill) state.""" + c = open_db(str(path)) + c.execute("UPDATE schema_version SET version = 6") + c.commit() + return c + + +def test_backfill_repairs_channel_added_before_fix(tmp_path) -> None: + """A user who added #bot before the fix gets a secret on next launch.""" + from meshcore_console.meshcore.db import _backfill_channel_secrets + + conn = _open_v6_db(tmp_path / "old.db") + # Simulate the buggy state: channels row exists, channel_secrets does not. + conn.execute( + "INSERT INTO channels (channel_id, display_name, unread_count, kind) " + "VALUES ('bot', '#bot', 0, 'group')" + ) + conn.execute("DELETE FROM channel_secrets WHERE name = 'bot'") + conn.commit() + assert ChannelDatabase(conn).get_channel("bot") is None + + _backfill_channel_secrets(conn) + conn.commit() + + row = ChannelDatabase(conn).get_channel("bot") + assert row is not None, "backfill must repair pre-existing hashtag channels" + assert row["secret"] == derive_channel_secret("bot") + conn.close() + + +def test_backfill_normalizes_case(tmp_path) -> None: + """A channel stored with display '#Chicago' must still derive the lowercase + key other MeshCore devices use (issue #81's verified value).""" + from meshcore_console.meshcore.db import _backfill_channel_secrets + + conn = _open_v6_db(tmp_path / "case.db") + conn.execute( + "INSERT INTO channels (channel_id, display_name, unread_count, kind) " + "VALUES ('chicago', '#Chicago', 0, 'group')" + ) + conn.commit() + _backfill_channel_secrets(conn) + conn.commit() + + row = ChannelDatabase(conn).get_channel("chicago") + assert row is not None + assert row["name"] == "chicago" + assert row["secret"] == "c1c289b131e5222370cbc2048445844b" + conn.close() + + +def test_backfill_skips_dm_channels(tmp_path) -> None: + from meshcore_console.meshcore.db import _backfill_channel_secrets + + conn = _open_v6_db(tmp_path / "dm.db") + conn.execute( + "INSERT INTO channels (channel_id, display_name, unread_count, kind, peer_name) " + "VALUES ('alice', 'Alice', 0, 'dm', 'Alice')" + ) + conn.commit() + _backfill_channel_secrets(conn) + conn.commit() + + assert ChannelDatabase(conn).get_channel("Alice") is None + conn.close() + + +def test_backfill_does_not_overwrite_public(tmp_path) -> None: + from meshcore_console.meshcore.db import _backfill_channel_secrets + + conn = _open_v6_db(tmp_path / "pub.db") + conn.execute( + "INSERT INTO channels (channel_id, display_name, unread_count, kind) " + "VALUES ('public', '#public', 0, 'group')" + ) + conn.commit() + _backfill_channel_secrets(conn) + conn.commit() + + # Public's hardcoded secret survives; no lowercase duplicate is created. + assert ChannelDatabase(conn).get_channel("Public")["secret"] == PUBLIC_CHANNEL_SECRET + names = [c["name"] for c in ChannelDatabase(conn).get_channels()] + assert names.count("Public") == 1 + assert "public" not in names + conn.close() + + +def test_backfill_is_idempotent(tmp_path) -> None: + from meshcore_console.meshcore.db import _backfill_channel_secrets + + conn = _open_v6_db(tmp_path / "idem.db") + conn.execute( + "INSERT INTO channels (channel_id, display_name, unread_count, kind) " + "VALUES ('bot', '#bot', 0, 'group')" + ) + conn.commit() + for _ in range(3): + _backfill_channel_secrets(conn) + conn.commit() + + names = [c["name"] for c in ChannelDatabase(conn).get_channels()] + assert names.count("bot") == 1 + conn.close() + + +def test_open_db_runs_backfill_on_upgrade(tmp_path) -> None: + """The repair happens automatically when an old db is opened.""" + path = tmp_path / "upgrade.db" + conn = _open_v6_db(path) + conn.execute( + "INSERT INTO channels (channel_id, display_name, unread_count, kind) " + "VALUES ('bot', '#bot', 0, 'group')" + ) + conn.execute("DELETE FROM channel_secrets WHERE name = 'bot'") + conn.commit() + conn.close() + + # Reopening triggers _migrate -> _backfill_channel_secrets. + conn = open_db(str(path)) + row = ChannelDatabase(conn).get_channel("bot") + assert row is not None, "open_db must repair pre-fix databases (#81)" + assert row["secret"] == derive_channel_secret("bot") + conn.close()