Validate cell characters and digit/period sanity
Halt import on control characters or a ':'/'/' inside an issuer or account (they collide with the OATH issuer:account separator and period/ prefix), on a composed name over 64 bytes with a clearer per-field message, and on a digit count outside 6-8 or a non-positive period.
This commit is contained in:
@@ -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
|
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.
|
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,
|
The input is validated before any reader is touched. The run halts with a
|
||||||
invalid Base32, over-long names/keys, and unsupported algorithms halt the run
|
per-row report if it finds: duplicate credential names; invalid Base32;
|
||||||
with a report so they can be fixed in the source data.
|
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
|
## Tests
|
||||||
|
|
||||||
|
|||||||
@@ -180,9 +180,38 @@ def validate_input(creds):
|
|||||||
else:
|
else:
|
||||||
seen[cid] = c.source
|
seen[cid] = c.source
|
||||||
|
|
||||||
# name length
|
# invalid characters in issuer/account. The applet stores raw bytes, but
|
||||||
if len(cid.encode("utf-8")) > MAX_NAME:
|
# control characters and the OATH name separators (':' / 'period/')
|
||||||
problems.append(f"Name too long (>{MAX_NAME} bytes): '{cid}' ({c.source}).")
|
# 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
|
# algorithm supported by the applet
|
||||||
if c.algorithm not in (HMAC_SHA1, HMAC_SHA256):
|
if c.algorithm not in (HMAC_SHA1, HMAC_SHA256):
|
||||||
|
|||||||
@@ -144,12 +144,21 @@ Companion `test_oath.py` — hardware-free unit tests.
|
|||||||
|
|
||||||
## Pre-flight validation rules (halt on any)
|
## Pre-flight validation rules (halt on any)
|
||||||
|
|
||||||
- Duplicate `cred_id` within the input (e.g. the two
|
- Duplicate `cred_id` within the input (e.g. two entries that map to the same
|
||||||
`SAW:michelle@dangerousthings.com` entries with different secrets).
|
`issuer:account`).
|
||||||
- Secret missing or not valid Base32 after uppercasing and removing spaces.
|
- Secret missing or not valid Base32 after uppercasing and removing spaces.
|
||||||
- Decoded key length > 64 bytes.
|
- 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.
|
- 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.
|
Each problem is reported with the offending issuer/account and source row.
|
||||||
|
|
||||||
|
|||||||
24
test_oath.py
24
test_oath.py
@@ -173,6 +173,30 @@ class TestValidateInput(unittest.TestCase):
|
|||||||
self.assertTrue(any("sha512" in p.lower() or "algorithm" in p.lower()
|
self.assertTrue(any("sha512" in p.lower() or "algorithm" in p.lower()
|
||||||
for p in problems))
|
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):
|
class TestLoadCredentials(unittest.TestCase):
|
||||||
def _make_xlsx(self, rows):
|
def _make_xlsx(self, rows):
|
||||||
|
|||||||
Reference in New Issue
Block a user