fix(auth): scope embedded key to parent origin + validate MessageChannel sender (ENG-4596) - #131
Conversation
ce2bd7f to
6fcb109
Compare
fainashalts
left a comment
fainashalts
left a comment
There was a problem hiding this comment.
Existing tests pass, but the channel-gate tests only evaluate copied predicates rather than exercising the production listener, so these paths are currently uncovered.
| ":" + | ||
| encodeURIComponent(validatedOrigin); | ||
|
|
||
| embeddedKeyState = { |
There was a problem hiding this comment.
This activates persistent mode before key generation and storage complete. initEphemeralEmbeddedKey() then returns immediately when it sees mode === "persistent", so concurrent bootstrap can observe getEmbeddedKey() === null.
If localStorage.setItem() throws, the previous ephemeral key is also lost and the claimed legacy fallback no longer works.
We should retain the previous state, associate a readiness promise with the new persistent state, make concurrent callers await it, and restore the previous state on rejection, as in #129. Should also add coverage for the initialization race and storage-failure rollback.
| return; | ||
| } | ||
| if (legacyParentOrigin === null) { | ||
| legacyParentOrigin = event.origin; |
There was a problem hiding this comment.
This binds inbound legacy messages to the first origin, but outbound legacy responses are still sent by sendMessageUp() using postMessage(..., "*"). If the parent navigates while an asynchronous request is running, the response can be delivered to the replacement origin.
After the first valid legacy request, we should store this validated origin as the outbound targetOrigin and use it for subsequent responses. Only the initial PUBLIC_KEY_READY, sent before an origin is known, should use "*".
e6631fa to
b8a75d4
Compare
| try { | ||
| // Vuln B fix: Supersede the ephemeral key (if any) with a persistent | ||
| // key scoped to the browser-authenticated parent origin (INT-697). | ||
| await TKHQ.initEmbeddedKey(event.origin); |
There was a problem hiding this comment.
This removes support for event.data.dangerouslyOverrideIframeKeyTtl. Before this PR, a positive numeric override was passed to initEmbeddedKey but now every persistent key is stored with the fixed 48-hour TTL. Callers requesting a shorter lifetime will silently retain the decryption key much longer than configured.
We should preserve the validated override as a second argument, e.g. initEmbeddedKey(event.origin, iframeKeyTtl), and use it in setItemWithExpiry. Let's also add a test confirming the handshake override controls the stored expiry.
Was removing this intentional? If so we should discuss it explicitly as an API/security decision.
43c041d to
cf72314
Compare
…nel sender (ENG-4596)
Fixes two cross-origin vulnerabilities in the auth frame (ENG-4596 / TKA-20260806-016):
## Vuln A — MessageChannel gate accepts any sender
Before this fix the TURNKEY_INIT_MESSAGE_CHANNEL handler gated only on
event.ports?.[0], allowing any window (not just the direct parent) to
establish the privileged MessageChannel.
Fix: mirror the INT-697 export-and-sign pattern. The gate now requires:
- event.source === window.parent
- event.origin is truthy and !== 'null'
- event.ports?.length === 1
## Vuln B — embedded key not scoped to parent origin
Before this fix a single P-256 private key was persisted in localStorage
under the fixed name 'TURNKEY_EMBEDDED_KEY', created before any parent
origin was known. The same key (and PUBLIC_KEY_READY public key) was
served to every embedder, enabling an attacker to replay an auth bundle
from one origin into the same key on another origin.
Fix (mirrors INT-697 export-and-sign approach, applied inline in auth/index.html):
- validateParentOrigin() validates the origin is non-opaque
- initEmbeddedKey(origin) stores the key under a V2 scoped storage key
'TURNKEY_EMBEDDED_KEY_V2:<encodeURIComponent(origin)>'
- initEphemeralEmbeddedKey() creates a memory-only key for legacy clients
- purgeLegacyEmbeddedKey() removes the old fixed key on every init
- embeddedKeyState tracks mode ('persistent' or 'ephemeral') + origin/key
- getBoundOrigin() exposes the current bound origin for re-init
- Standalone mode (window.parent === window): persists key scoped to
window.location.origin
- Embedded mode on DOMContentLoaded: creates ephemeral key for legacy
(@turnkey/iframe-stamper < 2.1.0) clients
- MessageChannel handshake: supersedes ephemeral with persistent key
scoped to event.origin; rolls back channelEstablished on failure
- Legacy embedded path: binds to first sender's origin; rejects others
Linear: ENG-4596 — https://linear.app/turnkey/issue/ENG-4596
cf72314 to
fe9366f
Compare
fainashalts
left a comment
fainashalts
left a comment
There was a problem hiding this comment.
LGTM, would update PR description as we now have 50 tests rather than 34. :)
2 participants
Summary & Motivation (Problem vs. Solution)
Ports the cross-origin hardening to the auth frame
Two fixes:
Any window could open the MessageChannel. The
TURNKEY_INIT_MESSAGE_CHANNELhandler only checked forevent.ports?.[0]. It now also requiresevent.source === window.parent, a real (non-"null") origin, and exactly one port.The embedded key was shared across embedders. One P-256 key lived in localStorage under
TURNKEY_EMBEDDED_KEY, created at DOMContentLoaded before we knew the parent, so every embedder got the samePUBLIC_KEY_READYand an auth bundle could be replayed from any origin. Keys are now per-origin:TURNKEY_EMBEDDED_KEY_V2:<encodeURIComponent(origin)>, bound at the handshake. Legacy iframe-stamper (< 2.1.0) clients get an in-memory-only key until a real origin shows up, and the oldTURNKEY_EMBEDDED_KEYis purged rather than migrated. Binding to a second origin is rejected; a failed handshake rolls back to ephemeral.Tests
auth/index.test.js, 34 passing. Covers per-origin key isolation, second-origin rejection, invalid origins, legacy purge, ephemeral-stays-in-memory, and the five channel-gate rejection cases.Reference to Linear Issue (ENG-XXX)
ENG-4596 / TKA-20260806-016. Siblings: #129 (export-and-sign), #130 (import).