diff --git a/README.md b/README.md index a9abf0f..2ec7458 100644 --- a/README.md +++ b/README.md @@ -58,9 +58,11 @@ Authenticating to a password-protected applet uses the standard YKOATH scheme challenge-response). `--set-password` only sets a password on an applet that has none; it does not change or remove an existing one. `--reset` clears everything. -The input is validated before any reader is touched. Duplicate credential names, -invalid Base32, over-long names/keys, and unsupported algorithms halt the run -with a report so they can be fixed in the source data. +The input is validated before any reader is touched. The run halts with a +per-row report if it finds: duplicate credential names; invalid Base32; +over-long names or keys; an unsupported algorithm; control characters or a +`:`/`/` inside an issuer or account (they break the OATH name convention); or a +digit count outside 6-8 / a non-positive period. Fix the source data and re-run. ## Tests diff --git a/apex_otp_import.py b/apex_otp_import.py index 63b10e5..e733bef 100644 --- a/apex_otp_import.py +++ b/apex_otp_import.py @@ -180,9 +180,38 @@ def validate_input(creds): else: seen[cid] = c.source - # name length - if len(cid.encode("utf-8")) > MAX_NAME: - problems.append(f"Name too long (>{MAX_NAME} bytes): '{cid}' ({c.source}).") + # invalid characters in issuer/account. The applet stores raw bytes, but + # control characters and the OATH name separators (':' / 'period/') + # corrupt display and round-trip in authenticator apps. + for fname, fval in (("issuer", c.issuer), ("account", c.account)): + ctrl = next((ch for ch in fval if ord(ch) < 0x20 or ord(ch) == 0x7F), None) + if ctrl is not None: + problems.append( + f"Control character (0x{ord(ctrl):02X}) in {fname} for " + f"'{cid}' ({c.source}).") + if ":" in fval: + problems.append( + f"Character ':' in {fname} for '{cid}' ({c.source}); it " + f"collides with the issuer:account separator.") + if "/" in fval: + problems.append( + f"Character '/' in {fname} for '{cid}' ({c.source}); it " + f"collides with the 'period/' name prefix.") + + # composed name length (the applet's hard 64-byte limit) + name_bytes = len(cid.encode("utf-8")) + if name_bytes > MAX_NAME: + problems.append( + f"Composed name too long ({name_bytes}>{MAX_NAME} bytes): " + f"'{cid}' ({c.source}); shorten the issuer or account.") + + # digits / period sanity + if c.digits not in (6, 7, 8): + problems.append( + f"Unusual digit count {c.digits} for '{cid}' ({c.source}); " + f"OATH codes are 6-8 digits.") + if not isinstance(c.period, int) or c.period <= 0: + problems.append(f"Invalid period {c.period!r} for '{cid}' ({c.source}).") # algorithm supported by the applet if c.algorithm not in (HMAC_SHA1, HMAC_SHA256): diff --git a/docs/design.md b/docs/design.md index 5d94e16..0c685ea 100644 --- a/docs/design.md +++ b/docs/design.md @@ -144,12 +144,21 @@ Companion `test_oath.py` — hardware-free unit tests. ## Pre-flight validation rules (halt on any) -- Duplicate `cred_id` within the input (e.g. the two - `SAW:michelle@dangerousthings.com` entries with different secrets). +- Duplicate `cred_id` within the input (e.g. two entries that map to the same + `issuer:account`). - Secret missing or not valid Base32 after uppercasing and removing spaces. - Decoded key length > 64 bytes. -- `cred_id` (UTF-8) length > 64 bytes. +- Composed name (`issuer:account`, UTF-8) length > 64 bytes. - Algorithm SHA512 (unsupported by the applet), or a non-TOTP/HOTP type. +- Control characters (byte < 0x20 or 0x7F) anywhere in the issuer or account. +- A literal `:` or `/` inside the issuer or account (they collide with the OATH + `issuer:account` separator and the `period/` name prefix, so they break + round-trip/display in authenticator apps, even though the applet stores them). +- Digit count outside 6-8, or a non-positive period. + +The applet itself imposes only the length limits; the character and +digit/period rules are host-side guards for OATH-convention and app +compatibility. Each problem is reported with the offending issuer/account and source row. diff --git a/test_oath.py b/test_oath.py index 8a0e9bd..b08e16a 100644 --- a/test_oath.py +++ b/test_oath.py @@ -173,6 +173,30 @@ class TestValidateInput(unittest.TestCase): self.assertTrue(any("sha512" in p.lower() or "algorithm" in p.lower() for p in problems)) + def test_control_char_in_account_reported(self): + problems = validate_input([cred(account="a\x01b")]) + self.assertTrue(any("control" in p.lower() for p in problems)) + + def test_colon_in_issuer_reported(self): + problems = validate_input([cred(issuer="Goo:gle")]) + self.assertTrue(any(":" in p and "issuer" in p.lower() for p in problems)) + + def test_slash_in_account_reported(self): + problems = validate_input([cred(account="a/b")]) + self.assertTrue(any("/" in p and "account" in p.lower() for p in problems)) + + def test_digits_out_of_range_reported(self): + problems = validate_input([cred(digits=9)]) + self.assertTrue(any("digit" in p.lower() for p in problems)) + + def test_bad_period_reported(self): + problems = validate_input([cred(period=0)]) + self.assertTrue(any("period" in p.lower() for p in problems)) + + def test_tab_is_control_char(self): + problems = validate_input([cred(issuer="Goo\tgle")]) + self.assertTrue(any("control" in p.lower() for p in problems)) + class TestLoadCredentials(unittest.TestCase): def _make_xlsx(self, rows):