fix(security): use constant-time comparison for TLS fingerprint verification by haoyu-haoyu · Pull Request #49083 · openclaw/openclaw

haoyu-haoyu · 2026-03-17T14:57:08Z

Summary

TLS certificate fingerprint verification in src/gateway/client.ts uses !== (standard JavaScript string equality), which is vulnerable to timing side-channel attacks (CWE-208).

An attacker on the network path can measure response time differences to iteratively guess the correct fingerprint byte-by-byte, potentially bypassing TLS pinning for MITM attacks on gateway connections.

Root cause

Two locations perform fingerprint comparison with !==:

Location	Context
Line 205	WebSocket `checkServerIdentity` callback
Line 681	HTTP `validateTlsFingerprint` method

Fix

Replace both with safeEqualSecret() from src/security/secret-equal.ts, which uses SHA-256 hashing + crypto.timingSafeEqual for constant-time comparison.

This is the same function already used for pairing token verification elsewhere in the codebase — just missed for TLS fingerprints.

- if (fingerprint !== expected) {
+ if (!safeEqualSecret(fingerprint, expected)) {

Why safeEqualSecret and not raw timingSafeEqual

timingSafeEqual requires both buffers to have equal length, which would leak the length of the expected fingerprint. safeEqualSecret avoids this by hashing both values with SHA-256 first (producing fixed 32-byte digests), then comparing the digests in constant time.

Test plan

pnpm tsgo — compiles cleanly
pnpm check — lint/format passes
Existing safeEqualSecret tests in src/security/audit-extra.sync.test.ts cover equal, unequal, different-length, and undefined inputs
No remaining fingerprint !== expected patterns in codebase

…ication The TLS certificate fingerprint checks in gateway/client.ts used !== (JavaScript string equality) which is vulnerable to timing attacks. An attacker can measure response time differences to iteratively guess the correct fingerprint byte-by-byte, potentially bypassing TLS pinning for MITM attacks on gateway connections. Replace both fingerprint comparisons (WebSocket and HTTP) with safeEqualSecret() from security/secret-equal.ts, which uses SHA-256 hashing + crypto.timingSafeEqual for constant-time comparison — the same pattern already used for pairing token verification elsewhere in the codebase. Co-Authored-By: Claude Opus 4.6 <[email protected]>

greptile-apps · 2026-03-17T14:58:38Z

Greptile Summary

This PR replaces two instances of standard !== string equality for TLS certificate fingerprint comparison in src/gateway/client.ts with safeEqualSecret(), a constant-time comparison function that hashes both values with SHA-256 before calling crypto.timingSafeEqual. The change is minimal, targeted, and consistent with how the same safeEqualSecret utility is already used elsewhere in the codebase for pairing token verification.

Both changed call sites (checkServerIdentity callback at line 205 and validateTlsFingerprint at line 681) already guard against null/undefined/empty values before reaching the comparison, so the type-widened signature of safeEqualSecret introduces no regression.
The import path ../security/secret-equal.js is correct relative to src/gateway/client.ts.
safeEqualSecret is independently unit-tested (equal, unequal, different-length, and undefined/null inputs) in src/security/audit-extra.sync.test.ts.
No new issues were found; the change is clean and complete.

Confidence Score: 5/5

This PR is safe to merge — it is a minimal, correct security hardening change with no functional regressions.
The diff is two one-line substitutions in a single file. The replacement function (safeEqualSecret) is already battle-tested in the codebase, its type signature safely handles all edge cases, and both call sites already validate their inputs before reaching the comparison. No logic paths are altered.
No files require special attention.

_{Last reviewed commit: 290dcd0}

clawsweeper · 2026-04-28T11:58:49Z

Codex review: needs changes before merge.

Summary
The PR imports safeEqualSecret into src/gateway/client.ts and uses it for both Gateway TLS fingerprint mismatch checks.

Reproducibility: yes. Source inspection on current main reproduces the remaining issue: both Gateway TLS fingerprint checks still use fingerprint !== expected, and v2026.5.3 contains the same comparisons.

Next step before merge
A repair worker can handle the narrow landing cleanup by adding the required changelog entry and, if the external branch cannot be updated safely, preparing a replacement PR; maintainers still decide final security review and merge.

Security
Cleared: The diff only swaps two local string comparisons to an existing local timing-safe helper and adds no dependency, workflow, permission, secret, or artifact-download surface.

Review findings

[P3] Add the required changelog entry — src/gateway/client.ts:317

Review details

Best possible solution:

Land or maintainer-apply the narrow Gateway hardening change using the existing timing-safe helper at both normalized fingerprint comparison sites, with a changelog entry and no broader TLS behavior changes.

Do we have a high-confidence way to reproduce the issue?

Yes. Source inspection on current main reproduces the remaining issue: both Gateway TLS fingerprint checks still use fingerprint !== expected, and v2026.5.3 contains the same comparisons.

Is this the best way to solve the issue?

Yes, with landing cleanup. Reusing the existing safeEqualSecret helper at the two normalized comparison sites is the narrowest maintainable fix; the branch still needs changelog cleanup and final maintainer review.

Full review comments:

[P3] Add the required changelog entry — src/gateway/client.ts:317
This is a user-facing Gateway security fix, but the branch does not add a CHANGELOG.md entry under the active Unreleased fixes. Repo policy requires one before merge; keep it to a single line and credit the human contributor, not the generated co-author.
Confidence: 0.88

Overall correctness: patch is correct
Overall confidence: 0.9

Acceptance criteria:

pnpm test src/gateway/client.watchdog.test.ts src/security/audit-extra.sync.test.ts
pnpm check:changed in Testbox before handoff

What I checked:

Current WebSocket TLS pinning still uses strict equality: GatewayClient.start() normalizes the peer and expected TLS fingerprints, then rejects mismatches with fingerprint !== expected. (src/gateway/client.ts:317, 89db1e5440f5)
Current post-open TLS validation still uses strict equality: validateTlsFingerprint() normalizes the peer and expected TLS fingerprints, then rejects mismatches with fingerprint !== expected. (src/gateway/client.ts:1004, 89db1e5440f5)
Latest release is not fixed: v2026.5.3 still contains both fingerprint !== expected Gateway TLS fingerprint comparisons, so the PR is not already shipped. (src/gateway/client.ts:317, 06d46f7cf638)
Existing timing-safe helper is available: safeEqualSecret accepts nullable strings, pads both buffers to a common length, uses timingSafeEqual, and preserves equal-length semantics. (src/security/secret-equal.ts:12, 89db1e5440f5)
Helper behavior has focused tests: Existing tests cover equal, unequal, different-length, empty, undefined, and null inputs for safeEqualSecret. (src/security/audit-extra.sync.test.ts:50, 89db1e5440f5)
TLS pinning is documented behavior: Gateway protocol docs state that clients may pin the gateway certificate fingerprint through Gateway TLS config, gateway.remote.tlsFingerprint, or CLI --tls-fingerprint. Public docs: docs/gateway/protocol.md. (docs/gateway/protocol.md:701, 89db1e5440f5)

Likely related people:

steipete: Commit metadata shows Peter Steinberger wired and tightened Gateway TLS fingerprint handling in the Gateway client path this PR changes; current blame also points the relevant lines to a recent Gateway refresh commit. (role: introduced Gateway TLS fingerprint behavior and recent Gateway maintainer; confidence: high; commits: 66193dab9263, dcb8d16591b5, 759068304ee0; files: src/gateway/client.ts, src/gateway/call.ts, src/infra/tls/fingerprint.ts)
vincentkoc: Commit metadata and changelog history show recent security remediation work around safeEqualSecret and its padded timing-safe comparison contract. (role: recent security helper maintainer; confidence: medium; commits: 7c5bf1c67574, d4268b1b2b74; files: src/security/secret-equal.ts, src/security/audit-extra.sync.test.ts, CHANGELOG.md)
David Rudduck: Commit metadata shows prior hardening work on safeEqualSecret for timing-safe comparison and length-leak concerns, directly related to the helper contract used here. (role: prior constant-time helper hardening contributor; confidence: medium; commits: f1e1ad73ade4; files: src/security/secret-equal.ts, src/security/audit-extra.sync.test.ts)

Remaining risk / open question:

The change touches Gateway TLS pinning security behavior from an external contributor, so final maintainer security/ownership review is still appropriate before merge.
The PR currently lacks the required single-line CHANGELOG.md entry for a user-facing security fix.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 89db1e5440f5.

…audit Gap 1 (Critical CWE-208): Replace !== with safeEqualSecret() for TLS fingerprint comparison in client.ts, eliminating timing side-channel attacks on WebSocket and HTTP TLS pinning. Refs: openclaw/openclaw#49083 Gap 2 (High revocation race): Add sync invalidated flag on GatewayClient, set before respond() in device.token.rotate/revoke and device.pair.remove handlers, with per-RPC dispatch guard in message-handler to reject pipelined RPCs from revoked credentials. Refs: openclaw/openclaw#70707 Gap 4 (High unbounded sessions): Add authenticated-connection-budget.ts per-device connection cap (default 8) with env override. Refs: openclaw/openclaw#59842 Gap 6 (High HTTP flood): Add request-rate-limit.ts sliding-window per-IP HTTP request rate limiter (default 120/min) for REST endpoints. Refs: openclaw/openclaw#44884 Gap 7 (High TLS enforcement): Add startup-security-checks.ts for network-exposure safety checks at startup. Refs: openclaw/openclaw#44884 Gap 8 (Medium auto HSTS): Auto-inject HSTS when TLS is active. Refs: openclaw/openclaw#44884 Gap 9 (Medium bind safety): Bind-all-interfaces detection via startup checks. Refs: openclaw/openclaw#44884 Gaps 3, 5, 10 already fixed in fork — no changes needed.

openclaw-barnacle Bot added gateway Gateway runtime size: XS labels Mar 17, 2026

BingqingLyu mentioned this pull request Apr 27, 2026

fix(security): use constant-time comparison for TLS fingerprint verification BingqingLyu/openclaw#942

Open

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(security): use constant-time comparison for TLS fingerprint verification#49083

fix(security): use constant-time comparison for TLS fingerprint verification#49083
haoyu-haoyu wants to merge 1 commit intoopenclaw:mainfrom
haoyu-haoyu:fix/tls-fingerprint-timing-safe-comparison

haoyu-haoyu commented Mar 17, 2026

Uh oh!

greptile-apps Bot commented Mar 17, 2026

Uh oh!

clawsweeper Bot commented Apr 28, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

haoyu-haoyu commented Mar 17, 2026

Summary

Root cause

Fix

Why safeEqualSecret and not raw timingSafeEqual

Test plan

Uh oh!

greptile-apps Bot commented Mar 17, 2026

Greptile Summary

Confidence Score: 5/5

Uh oh!

clawsweeper Bot commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

clawsweeper Bot commented Apr 28, 2026 •

edited

Loading