chore(core): slip21 keychain and path improvements
What changed, and why it matters
This commit is a routine internal cleanup in the Trezor firmware. It moves the SLIP21 path validation into its own helper method, adds a new function to convert SLIP21 paths to human-readable strings, and updates tests. There is no indication it fixes a security vulnerability or changes user-visible behavior in a risky way.
No security action required; treat as normal code hygiene. Reviewers may optionally verify that `verify_slip21_path()` is called in all code paths that previously performed the inline check.
Security signals we found
Refactoring only: existing strict path check is preserved, not weakened
New helper is purely presentational (path-to-string conversion)
Test expectations updated to match exception class hierarchy
No changelog entry suggests non-user-visible maintenance
Evidence from the diff
The patch refactors Keychain.derive_slip21() to call a new verify_slip21_path() method, which performs the same strict safety-check logic as before. It also introduces address_n_slip21_to_str() in paths.py for pretty-printing SLIP21 label paths with escaping for backslashes, forward slashes, and non-printable bytes. Tests are updated to expect ForbiddenKeyPath (a subclass of DataError) and to exercise the new string formatter. No access-control rules are relaxed or bypassed.
Changed components
core/src/apps/common/keychain.pycore/src/apps/common/paths.pycore/tests/test_apps.common.seed.pyInspect captured patch +73 / −8
diff --git a/core/src/apps/common/keychain.py b/core/src/apps/common/keychain.py
index d4992999..9af6ead6 100644
--- a/core/src/apps/common/keychain.py
+++ b/core/src/apps/common/keychain.py
@@ -107,6 +107,15 @@ class Keychain:
raise ForbiddenKeyPath()
+ def verify_slip21_path(self, path: paths.Slip21Path) -> None:
+ if not safety_checks.is_strict():
+ return
+
+ if any(ns == path[: len(ns)] for ns in self.slip21_namespaces):
+ return
+
+ raise ForbiddenKeyPath()
+
def is_in_keychain(self, path: paths.Bip32Path) -> bool:
return any(schema.match(path) for schema in self.schemas)
@@ -149,10 +158,7 @@ class Keychain:
def derive_slip21(self, path: paths.Slip21Path) -> Slip21Node:
from .seed import Slip21Node
- if safety_checks.is_strict() and not any(
- ns == path[: len(ns)] for ns in self.slip21_namespaces
- ):
- raise ForbiddenKeyPath()
+ self.verify_slip21_path(path)
return self._derive_with_cache(
1,
diff --git a/core/src/apps/common/paths.py b/core/src/apps/common/paths.py
index 9e5cde2f..93affdeb 100644
--- a/core/src/apps/common/paths.py
+++ b/core/src/apps/common/paths.py
@@ -367,6 +367,29 @@ def address_n_to_str(address_n: Iterable[int]) -> str:
return "m/" + "/".join(_path_item(i) for i in address_n)
+def address_n_slip21_to_str(address_n: Slip21Path) -> str:
+ if not address_n:
+ return "m"
+
+ def label_to_str(label: bytes) -> str:
+ out = []
+ for b in label:
+ if b == ord("\\"):
+ # Escape \ as \\
+ out.append("\\\\")
+ elif b == ord("/"):
+ # Escape / as \/
+ out.append("\\/")
+ elif 32 <= b <= 126:
+ out.append(chr(b))
+ else:
+ # Display non-ASCII-printable bytes as \xNN
+ out.append(f"\\x{b:02x}")
+ return "".join(out)
+
+ return "m/" + "/".join(label_to_str(label) for label in address_n)
+
+
def unharden(item: int) -> int:
return item ^ (item & HARDENED)
diff --git a/core/tests/test_apps.common.seed.py b/core/tests/test_apps.common.seed.py
index 1757c12d..37e82d21 100644
--- a/core/tests/test_apps.common.seed.py
+++ b/core/tests/test_apps.common.seed.py
@@ -1,10 +1,11 @@
# flake8: noqa: F403,F405
from common import * # isort:skip
-from trezor import wire
from trezor.crypto import bip39
+from trezor.wire import DataError
-from apps.common.keychain import Keychain
+from apps.common.keychain import ForbiddenKeyPath, Keychain
+from apps.common.paths import address_n_slip21_to_str
from apps.common.seed import Slip21Node
@@ -52,11 +53,46 @@ class TestSeed(unittest.TestCase):
)
# Forbidden paths.
- with self.assertRaises(wire.DataError):
+ with self.assertRaises(ForbiddenKeyPath):
keychain.derive_slip21([])
- with self.assertRaises(wire.DataError):
+ with self.assertRaises(ForbiddenKeyPath):
keychain.derive_slip21([b"SLIP-9999", b"Authentication key"])
+ # Verify that ForbiddenKeyPath is a subclass of DataError
+ self.assertTrue(issubclass(ForbiddenKeyPath, DataError))
+
+ def test_slip21_to_str(self):
+ # Empty path is "m"
+ self.assertEqual(address_n_slip21_to_str([]), "m")
+
+ # Normal-use path
+ self.assertEqual(
+ address_n_slip21_to_str([b"SLIP-0021", b"Authentication key"]),
+ "m/SLIP-0021/Authentication key",
+ )
+
+ # Path with "/" and "\" - escaped as "\/" and "\\" respectively
+ label = b" \\ peace / among / worlds \\"
+ self.assertEqual(
+ address_n_slip21_to_str([label, label, label]),
+ "m/ \\\\ peace \\/ among \\/ worlds \\\\/ \\\\ peace \\/ among \\/ worlds \\\\/ \\\\ peace \\/ among \\/ worlds \\\\",
+ )
+
+ # Paths with non-ASCII-printable bytes - escaped as \xNN
+ self.assertEqual(
+ address_n_slip21_to_str([b"\x00"]),
+ "m/\\x00",
+ )
+ self.assertEqual(
+ address_n_slip21_to_str([b"\x00\x01\x80\xff pass"]),
+ "m/\\x00\\x01\\x80\\xff pass",
+ )
+ label = bytes("řeřicha", "utf-8")
+ self.assertEqual(
+ address_n_slip21_to_str([label, label, label]),
+ "m/\\xc5\\x99e\\xc5\\x99icha/\\xc5\\x99e\\xc5\\x99icha/\\xc5\\x99e\\xc5\\x99icha",
+ )
+
if __name__ == "__main__":
unittest.main()
Why this scored 12/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.