Repository navigation
feat: add deviceBoundSessions fuse for Device Bound Session Credentials - #54074
MarshallOfSound wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
API LGTM
I think this is fine as a first step, but there are several things that probably should be addressed in follow ups:
-
There's no way to ask whether it worked. The fuse can be on and DBSC still be entirely inert. Maybe add something like
session.isDeviceBoundSessionsAvailable(). -
No per-origin logout.
cookies.remove()doesn't end a session; the only way out is clearing all cookies for the session. Chromium's DBSC service can enumerate and terminate individual session — none of that is surfaced. -
session.clearData'scookiestype now means also clearing device bound sessions. It makes sense given how they are tied together, but maybe we should adddeviceBoundSessions. -
No per-session granularity. Maybe we could add something like
session.fromPartition(name, { deviceBoundSessions: true }). -
No runtime observability. No events for register, refresh, or terminate.
|
Agree on the potential follow ups. Specifically didn't want to expose new JS APIs here as they aren't required for actually surfacing the raw DBSC capability. Will look into JS APIs as a follow up |
3cdddfe to
6605f38
Compare
Adds an opt-in fuse that lets websites bind sessions to a hardware-backed key (TPM on Windows, Secure Enclave on macOS) so a stolen cookie cannot be replayed from another device. Keys are managed by a key service in the browser process. On macOS it uses a keychain access group derived from the app's code signature. With the fuse on, software keys can never be enabled by a command line flag; with it off, that flag is the way to try DBSC in development. Clearing cookies also ends the sessions bound to them.
6605f38 to
d2f8aad
Compare
ckerr
left a comment
There was a problem hiding this comment.
Reviewer 1: Opus 5.5 xhigh
Reviewer 2: me
Three issues look serious to me, blocking on the first one. If this behavior's intentional, we should document it.
- With the fuse off, the software-keys flag turns DBSC on in any app.
- Origin-scoped cookie clears don't line up with the DBSC sessions they should end.
clearDatamisses sessions on subdomains, andclearStorageDataends all of them. Splitting DBSC back out ofcookieslater would be a breaking change, so I'd like the scoped paths right before we commit to that mapping. - The three "clearing data" specs fail on CI, so (2) is currently untested.
Details inline.
| return true; | ||
| } | ||
|
|
||
| return software_keys; |
There was a problem hiding this comment.
With the fuse off, this makes the software-keys flag a runtime switch that turns DBSC on.
Before this PR, Electron never set device_bound_sessions_enabled, so DBSC was always off (the mojom default is false). After it, any app that never opted in gets DBSC when launched with --enable-features=EnableBoundSessionCredentialsSoftwareKeysForManualTesting. Since the fuse defaults to off, that's every app. A site that registers a session will treat it as device bound while the private key sits in software in the session database.
This contradicts fuses.md ("When it is disabled, DBSC is off and websites cannot register sessions"). With the target/44-x-y and target/45-x-y labels, it would also add this switch to existing apps on stable lines.
Could we honor the flag only when the app isn't packaged (the same notion as app.isPackaged), or behind something the app opts into? If the intent is that the flag works in any app with the fuse off, the docs need to say that explicitly.
| BrowsingDataRemover::DATA_TYPE_CACHE_STORAGE}, | ||
| {"cookies", BrowsingDataRemover::DATA_TYPE_COOKIES}, | ||
| {"cookies", BrowsingDataRemover::DATA_TYPE_COOKIES | | ||
| BrowsingDataRemover::DATA_TYPE_DEVICE_BOUND_SESSIONS}, |
There was a problem hiding this comment.
When origins or excludeOrigins is set, ClearDataTask::Run moves only DATA_TYPE_COOKIES into the registrable-domain operation. This new DATA_TYPE_DEVICE_BOUND_SESSIONS bit stays in the origin-scoped operation, and DeviceBoundSessionManager::DeleteAllSessions matches that filter against session.origin() exactly.
So clearData({ dataTypes: ['cookies'], origins: ['https://example.com'] }) deletes every example.com cookie but keeps a session registered from https://login.example.com. That session then re-issues its cookie, which is the logout case fuses.md says this change handles. excludeOrigins goes the other way: subdomain cookies are kept while their sessions are deleted.
Suggest clearing both bits from data_type_mask and starting the registrable-domain operation with DATA_TYPE_COOKIES | DATA_TYPE_DEVICE_BOUND_SESSIONS. BuildNetworkServiceFilter() carries the domains, and the session matcher compares them against the session site's eTLD+1.
| // they have to go whenever those cookies are cleared. | ||
| {"cookies", | ||
| StoragePartition::REMOVE_DATA_MASK_COOKIES | | ||
| StoragePartition::REMOVE_DATA_MASK_DEVICE_BOUND_SESSIONS}, |
There was a problem hiding this comment.
clearStorageData calls StoragePartition::ClearData(mask, storage_key, ...), which passes no BrowsingDataFilterBuilder. The DBSC removal therefore becomes DeleteAllSessions(..., /*filter=*/nullptr).
With this mapping, ses.clearStorageData({ origin: 'https://a.example', storages: ['cookies'] }) deletes cookies for a.example only but ends every device bound session in the session. Once the other sites' short-lived cookies expire, the user is logged out of all of them.
When origin is set, could the DBSC bit go through the ClearData overload that takes a filter builder, scoped to that origin? If not, an origin-scoped clear that ends all sessions should at least be documented.
| const listsBefore = g.dbscSessionLists.length; | ||
| await cdp.sendCommand('Network.enableDeviceBoundSessions', { enable: false }); | ||
| await cdp.sendCommand('Network.enableDeviceBoundSessions', { enable: true }); | ||
| while (g.dbscSessionLists.length === listsBefore) await setTimeout(10); |
There was a problem hiding this comment.
This runs inside rc.remotely, so setTimeout here is the remote app's global callback-style timer, not the node:timers/promises import. Network.enableDeviceBoundSessions responds synchronously and deviceBoundSessionsAdded arrives later over mojo, so the loop always runs and throws. All three "clearing data" specs fail this way on CI (e.g. linux-x64 shard 2, macos-arm64 shard 2):
remote error: TypeError [ERR_INVALID_ARG_TYPE]: The "callback" argument must be of type function. Received type number (10)
await new Promise((resolve) => setTimeout(resolve, 10)) works remotely. Once these run, it would be worth adding origin-scoped cases for the two session comments above.
Description of Change
Adds an opt-in
deviceBoundSessionsfuse that lets websites bind sessions to a hardware-backed key (TPM on Windows, Secure Enclave on macOS), so a stolen cookie can't be replayed from another device.FuseV1Options.EnableDeviceBoundSessions, off by default. On, it enables DBSC for every session and keeps sessions on disk for persistent partitions.<TeamID>.<BundleID>.unexportable-keysis derived from the app's code signature. If the app is unsigned or lacks the entitlement, DBSC stays off and the reason is logged. In-memory partitions aren't supported.EnableBoundSessionCredentialsSoftwareKeysForManualTestingis ignored. With it off, that flag enables DBSC with mock keys for development, including Linux.clearStorageData/clearDatawithcookiesnow also end DBSC sessions, otherwise a session would re-issue the cleared cookie.fuses-spec.tsagainst a local HTTPS DBSC server, including rebinding after a network service restart.Draft notes:
FuseV1Options.EnableDeviceBoundSessions(index 9) is added to@electron/fusesin feat: add FuseV1Options.EnableDeviceBoundSessions fuses#138, so the documented name resolves once that ships.Checklist
npm testpassesRelease Notes
Notes: Added the
deviceBoundSessionsfuse for Device Bound Session Credentials.