Closed
Bug 1244249
Opened 9 years ago
Closed 1 year ago
Implement structured cloning for push interfaces
Categories
(Core :: DOM: Notifications, enhancement, P3)
Core
DOM: Notifications
Tracking
()
RESOLVED
WONTFIX
Tracking | Status | |
---|---|---|
firefox47 | --- | affected |
People
(Reporter: lina, Unassigned)
References
(Blocks 1 open bug)
Details
(Whiteboard: dom-triaged)
Attachments
(2 files)
Martin pointed out in bug 1191931 that we don't currently support this. I could see a case for sending subscriptions across frames via `postMessage`, if you've registered a push-only worker in an iframe and want to manage them from the parent page. You could also persist them...though `serviceWorkerRegistration.pushManager.getSubscription()` will always return the subscription, so there's no need to do that yourself.
Adding structured clone support for `PushMessageData` would be nice. That way, you could persist payloads directly to IndexedDB.
Reporter | ||
Comment 1•9 years ago
|
||
Martin, these would require additions to the spec, correct?
Flags: needinfo?(martin.thomson)
Comment 2•9 years ago
|
||
Yes, we need to explicitly define how to clone something for it to be usable with structured cloning. I think that we should be careful to preserve the origin (i.e., principal) when we transfer the object so that calls to serialize and getKey() fail on other origins. At least that way we stop little accidental leaks of keying material.
I think that we can save a principal property on the subscription and do caller.principal.subsumes(subscription.principal) when calling into getKey() and when serializing. Both need to throw a SecurityError at that point.
So yes, this isn't that easy to get right. But it will be useful to be able to (for example) store a subscription in indexedDB.
Flags: needinfo?(martin.thomson)
Reporter | ||
Comment 3•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/32949/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/32949/
Reporter | ||
Comment 4•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/32951/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/32951/
Reporter | ||
Comment 5•9 years ago
|
||
Here's some progress I made on the train, although it doesn't handle the principals correctly.
This looks tricky to implement, too, because nsIPrincipal is main-thread only. Going from main thread to worker, I think we'd need to perform the security check in a WorkerMainThreadRunnable when we create the WorkerPushSubscription. We can get the principal from the WorkerPrivate and check if it subsumes the PushSubscription's principal.
Going the other way, I guess we'd also use a WorkerMainThreadRunnable to serialize the principal, then perform the check when we create the PushSubscription. I'm assuming it's not safe to use the worker thread's JSStructuredCloneWriter on the main thread...so we may need to call PrincipalToPrincipalInfo in the runnable, and pass the PrincipalInfo struct back to the worker thread. In that case, we should extract the serialization logic from nsJSPrincipals::write (and ReadKnownPrincipalType).
It seems like there should be an easier way to do this. AFAICT, though, the existing interfaces that support structured cloning don't do these security checks.
Comment 6•9 years ago
|
||
Hmm, thinking about this some more, I think that the security checks aren't going to do any good anyway. It's not like we can stop someone from extracting keys and passing the raw values. And I don't see any way we might *use* a subscription in another origin in a way that would let them *use* the public-private key pair. I did really want to avoid accidental leaking of the authentication key, but I guess if it's hard and pointless we can just see if a simple clone is acceptable.
Updated•9 years ago
|
Assignee: nobody → kcambridge
Whiteboard: dom-noted
Updated•9 years ago
|
Whiteboard: dom-noted → dom-triaged
Reporter | ||
Comment 7•9 years ago
|
||
I'll be curious to hear what the others think. Will hold off on requesting review until we have a spec strawman.
Updated•9 years ago
|
Priority: -- → P3
Updated•9 years ago
|
Assignee: kcambridge → nobody
Reporter | ||
Comment 8•9 years ago
|
||
Unassigning me until the spec is updated.
Updated•3 years ago
|
Type: defect → enhancement
Updated•2 years ago
|
Severity: normal → S3
The spec doesn't have this, btw: https://w3c.github.io/push-api/#dom-pushsubscription (No [Transferable]
)
I don't think we should do this. Maybe the spec changed since the initial report?
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•