Implement structured cloning for push interfaces




DOM: Push Notifications
2 years ago
2 years ago


(Reporter: kitcambridge, Unassigned)



Firefox Tracking Flags

(firefox47 affected)


(Whiteboard: dom-triaged)

MozReview Requests


Submitter Diff Changes Open Issues Last Updated
Error loading review requests:


(2 attachments)

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.
Martin, these would require additions to the spec, correct?
Flags: needinfo?(martin.thomson)
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)
Created attachment 8714127 [details]
MozReview Request: Bug 1244249, Part 1 - Add JS_ReadString to match JS_WriteString.

Review commit:
See other reviews:
Created attachment 8714128 [details]
MozReview Request: Bug 1244249, Part 2 - Implement structured cloning for push subscriptions.

Review commit:
See other reviews:
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 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.
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.
Assignee: nobody → kcambridge
Whiteboard: dom-noted
Whiteboard: dom-noted → dom-triaged
I'll be curious to hear what the others think. Will hold off on requesting review until we have a spec strawman.
Priority: -- → P3
Assignee: kcambridge → nobody
Unassigning me until the spec is updated.
You need to log in before you can comment on or make changes to this bug.