fix(federation): replace strcmp token oracle with hash-based comparison in requestSharedSecret

[DeepDiver1975]

Summary

• requestSharedSecret (@PublicPage, unauthenticated) used strcmp($localToken, $submitted) to decide 403 vs 200, leaking token ordering to any caller
• Binary search converges on the exact 16-char stored token in ~96 requests; token can then be replayed to getSharedSecret to steal the federation shared secret
• Fix: compare SHA-256 hashes instead — ordering is preserved (tiebreaking still works) but responses are now independent of the plaintext token

Security Impact

High — unauthenticated callers can recover federation shared secrets when they know a trusted server URL

Test plan

• New regression test testRequestSharedSecretNoOracleLeakage uses a token pair where old strcmp returns 403 but hash-based returns 200; fails without fix
• Existing dataTestRequestSharedSecret updated with hash-ordered token pairs
• Run make test TEST_PHP_SUITE=apps/federation

🤖 Generated with Claude Code

[update-docs]

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

[DeepDiver1975]

Code Review — fix(federation): replace strcmp token oracle with hash-based comparison in requestSharedSecret

Overview: requestSharedSecret (@PublicPage, unauthenticated) used strcmp($localToken, $submitted) to determine which server's token wins a simultaneous federation exchange. Because different responses (200 vs 403) are returned for >0 vs <=0, an attacker can binary-search the stored $localToken in ~96 requests. The fix hashes both values with SHA-256 before comparing, so the response reveals nothing about the plaintext token.

Correctness of the fix

if (\strcmp(\hash('sha256', $localToken), \hash('sha256', $token)) > 0) {

The tiebreak semantics are preserved: both sides hash deterministically, the same side still "wins" for any given pair (assuming neither server has the other's real token), and no plaintext ordering is revealed. ✅

Is SHA-256 appropriate here? Yes — the goal is to eliminate the ordering oracle, not to provide cryptographic secrecy of the token itself. Both tokens are hashed identically; keyed HMAC would be overkill since the comparison is symmetric and the attacker doesn't get the hash value directly (only the 200/403 response). ✅

Does the tiebreak still resolve correctly? For the legitimate two-server exchange scenario, both servers hold the same $localToken and receive the other's token. After hashing, the same lexicographic comparison runs, so whichever side has the higher-hashing token still wins. ✅

Test analysis

dataTestRequestSharedSecret: Updated to use concrete token pairs where the hash-ordering matches the expected status code. The comment block explaining Case 1 vs Case 2 is unusually thorough for a test file but is genuinely useful here since the tokens are opaque — it makes the intent reviewable. ✅

testRequestSharedSecretNoOracleLeakage: This is the critical regression test. It uses a pair where strcmp(localToken, token) > 0 (old code: 403) but strcmp(hash(localToken), hash(token)) < 0 (new code: 200). This is exactly the distinguishing case that proves the fix changes behavior for the exploitable scenario. ✅

The test verifies:

• jobList->add(GetSharedSecret) is called (remote wins, we schedule fetch)
• jobList->remove(RequestSharedSecret) is called
• Status is 200 ✅

Changelog

changelog/unreleased/41579 accurately describes the oracle attack and the fix, including the "~96 requests" estimate. ✅

Summary

Aspect	Assessment
Oracle eliminated	✅ Hash comparison reveals no plaintext ordering
Tiebreak semantics	✅ Preserved
Regression test	✅ Directly demonstrates old code fails, new code passes
Data provider	✅ Updated with hash-ordered token pairs + explanatory comments
Changelog	✅ Accurate

Verdict: Ready to merge. High-impact security fix with solid test coverage.