From 3def3f0c45fc9e1dfdb164884d5d3285f50fc2b3 Mon Sep 17 00:00:00 2001 From: Damir Dulic Date: Sun, 28 Jun 2026 11:38:46 +0100 Subject: [PATCH 1/2] fix(auth): tolerate device clock skew in login timestamp check The `verify_login_hash` check required the client timestamp to exactly equal the server's stored challenge timestamp. The Supernote device sends its own local epoch time in the login request, so any clock drift (e.g. from disabling NTP and manually setting a timezone) caused a silent auth failure that the device surfaced as "System time is incorrect". Replace the exact equality check with a tolerance window matching the challenge TTL (5 minutes). The random_code nonce + TTL already provides replay protection, so strict equality was over-constraining. Also promote all verify_login_hash failure paths from silent returns to logger.warning with enough context (skew diff in ms, stored vs client timestamps) to diagnose auth failures without needing to reproduce them. Claude-Session: https://claude.ai/code/session_01UgzvCBzPZ9Yvv8nANjtYv3 --- supernote/server/services/user.py | 35 +++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/supernote/server/services/user.py b/supernote/server/services/user.py index 34056af9..849e4035 100644 --- a/supernote/server/services/user.py +++ b/supernote/server/services/user.py @@ -219,20 +219,51 @@ async def verify_login_hash( ) -> bool: user = await self._get_user_do(account) if not user or not user.is_active: + logger.warning("verify_login_hash: user %s not found or inactive", account) return False stored_value = await self._coordination_service.get_value( f"challenge:{account}" ) if not stored_value: + logger.warning( + "verify_login_hash: no challenge found for %s (expired or never issued)", + account, + ) return False random_code, stored_timestamp = stored_value.split("|") - if stored_timestamp != timestamp: + # Accept timestamps within the challenge TTL to tolerate device clock skew. + # Exact equality breaks when the device sends its own local time rather than + # echoing the server's challenge timestamp. The random_code nonce + TTL already + # provides replay protection, so strict equality is not needed here. + try: + ts_diff = abs(int(stored_timestamp) - int(timestamp)) + if ts_diff > int(RANDOM_CODE_TTL.total_seconds() * 1000): + logger.warning( + "verify_login_hash: timestamp skew too large for %s " + "(stored=%s client=%s diff=%dms limit=%dms)", + account, + stored_timestamp, + timestamp, + ts_diff, + int(RANDOM_CODE_TTL.total_seconds() * 1000), + ) + return False + except ValueError: + logger.warning( + "verify_login_hash: non-numeric timestamp for %s (stored=%s client=%s)", + account, + stored_timestamp, + timestamp, + ) return False expected_hash = hash_with_salt(user.password_md5, random_code) - return expected_hash == client_hash + if expected_hash != client_hash: + logger.warning("verify_login_hash: hash mismatch for %s", account) + return False + return True async def login( self, From df1b2ea4e967787c463a1a627265e73fc3d1c914 Mon Sep 17 00:00:00 2001 From: Damir Dulic Date: Sun, 28 Jun 2026 11:43:39 +0100 Subject: [PATCH 2/2] test(auth): add coverage for verify_login_hash skew tolerance Covers all new code paths introduced in the timestamp skew fix: exact timestamp match, small skew within tolerance, skew exceeding the TTL window, non-numeric timestamp, missing challenge, wrong hash, and unknown user. Claude-Session: https://claude.ai/code/session_01UgzvCBzPZ9Yvv8nANjtYv3 --- .../server/services/test_verify_login_hash.py | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 tests/server/services/test_verify_login_hash.py diff --git a/tests/server/services/test_verify_login_hash.py b/tests/server/services/test_verify_login_hash.py new file mode 100644 index 00000000..72a58b90 --- /dev/null +++ b/tests/server/services/test_verify_login_hash.py @@ -0,0 +1,80 @@ +import hashlib + +import pytest + +from supernote.models.user import UserRegisterDTO +from supernote.server.services.user import RANDOM_CODE_TTL, UserService +from supernote.server.utils.hashing import hash_with_salt + + +@pytest.fixture +async def registered_user(user_service: UserService) -> str: + email = "hashtest@example.com" + pw_md5 = hashlib.md5(b"password").hexdigest() + await user_service.register(UserRegisterDTO(email=email, password=pw_md5)) + return email + + +async def test_verify_login_hash_exact_timestamp( + registered_user: str, user_service: UserService +) -> None: + code, ts = await user_service.generate_random_code(registered_user) + pw_md5 = hashlib.md5(b"password").hexdigest() + client_hash = hash_with_salt(pw_md5, code) + assert await user_service.verify_login_hash(registered_user, client_hash, ts) + + +async def test_verify_login_hash_small_clock_skew( + registered_user: str, user_service: UserService +) -> None: + code, ts = await user_service.generate_random_code(registered_user) + pw_md5 = hashlib.md5(b"password").hexdigest() + client_hash = hash_with_salt(pw_md5, code) + # Simulate device clock 30 seconds ahead of server + skewed_ts = str(int(ts) + 30_000) + assert await user_service.verify_login_hash(registered_user, client_hash, skewed_ts) + + +async def test_verify_login_hash_skew_exceeds_ttl( + registered_user: str, user_service: UserService +) -> None: + code, ts = await user_service.generate_random_code(registered_user) + pw_md5 = hashlib.md5(b"password").hexdigest() + client_hash = hash_with_salt(pw_md5, code) + # Simulate device clock 1 hour ahead — exceeds the 5-minute TTL window + skewed_ts = str(int(ts) + int(RANDOM_CODE_TTL.total_seconds() * 1000) + 1) + assert not await user_service.verify_login_hash( + registered_user, client_hash, skewed_ts + ) + + +async def test_verify_login_hash_non_numeric_timestamp( + registered_user: str, user_service: UserService +) -> None: + code, _ts = await user_service.generate_random_code(registered_user) + pw_md5 = hashlib.md5(b"password").hexdigest() + client_hash = hash_with_salt(pw_md5, code) + assert not await user_service.verify_login_hash( + registered_user, client_hash, "not-a-number" + ) + + +async def test_verify_login_hash_no_challenge( + registered_user: str, user_service: UserService +) -> None: + # Never call generate_random_code — no challenge stored + pw_md5 = hashlib.md5(b"password").hexdigest() + assert not await user_service.verify_login_hash(registered_user, pw_md5, "12345678") + + +async def test_verify_login_hash_wrong_hash( + registered_user: str, user_service: UserService +) -> None: + _code, ts = await user_service.generate_random_code(registered_user) + assert not await user_service.verify_login_hash(registered_user, "deadbeef" * 8, ts) + + +async def test_verify_login_hash_unknown_user(user_service: UserService) -> None: + assert not await user_service.verify_login_hash( + "nobody@example.com", "hash", "12345678" + )