1431771 - consider not requiring object identity for null principal equality

Status: NEW

(Core :: Security: CAPS, enhancement)

Description

[Ben Kelly [:bkelly, not reviewing]]

Currently `nsIPrincipal::Equals()` requires object identity for two null principals to be equal:

https://searchfox.org/mozilla-central/rev/1f9b4f0f0653b129b4db1b9fc5e9068054afaac0/caps/BasePrincipal.h#213

Boris explained this is because we did not have a truly unique UUID implementation when e10s was first implemented.  Therefore we did not feel we could safely rely on the UUID comparison.

Now that we have shipped multi-e10s, however, and are doing more multi-process work this is becoming a problem.  Its much easier to get into situations like this:

  nsIPrincipal* null1 = /* some null principal */;

  PrincipalInfo info;
  PrincipalToPrincipalInfo(null1, info);

  nsIPrincipal* null2 = PrincipalInfoToPrincipal(info);

  null1->Equals(null2);

This also happens a lot more these days because we are doing more off-main-thread and this requires more use of PrincipalInfo.

It would be very nice if we could make BasePrincipal::Equals() consider the null principal UUID when comparing null principals.

Note, this may be impacting our compatibility on tests like this:

https://searchfox.org/mozilla-central/source/testing/web-platform/tests/workers/opaque-origin.html

Comment 1

[Boris Zbarsky [:bzbarsky]]

The cross-thread case could be handled by giving nullprincipals a sequence number.  But that doesn't work for the cross-process case...

Do we actually need to be able to do same-origin compares on nullprincipals in two different processes, or do we just need to be able to round-trip a nullprincipal from process A to process B and then back to process A, where we then compare it to other things in process A?

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

Ideally we should be able to compare in different processes.  Consider the case where:

1. A sandboxed iframe with an opaque origin creates a worker.
2. Both the worker and iframe have the same opaque origin.
3. They try to communicate using a cross-process API like BroadcastChannel.

Its possible BroadcastChannel does optimization to stay single process for opaque origins, but the most direct implementation would be to send it to a service in the parent process and compare principals there.

Of course there are lots of APIs where we want to compare principals in the parent.  I do it in Clients API, but I avoid this problem by working with PrincipalInfo off-the-main-thread right now.

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

We've also talked about moving SharedWorkers (which could inherit a sandbox opaque origin) into a separate process, but that's less likely now with spectre.