status of oopif?

[Nico Weber]

Hi,

is OOPIF currently on or off? I'm adding a bunch of tests to the memory bots in https://chromium-review.googlesource.com/c/chromium/src/+/1110151 and I notice that we have these OOPIF-related test suites:

- not_site_per_process_browser_tests

- site_per_process_components_browsertests

- site_per_process_components_unittests

- site_per_process_content_browsertests

- site_per_process_content_unittests

- site_per_process_extensions_browsertests

- not_site_per_process_interactive_ui_tests

- not_site_per_process_sync_integration_tests

- not_site_per_process_unit_tests

The names confused me a lot. I looked around a bit and found https://chromium.googlesource.com/chromium/src/+/fb1ccf02ee8ca79e1404abfd3a3a7d540b7d2dbd which changed...something...that I don't quite understand, but the CL description says:

"""

Note that features::kSitePerProcess is a //chrome-layer feature.

Therefore, after this CL:

- site-per-process is turned-on by default in //chrome layer and above

  (i.e. in code that uses ChromeContentBrowserClient).

- site-per-process stays off by default in //content layer

Also note that fieldtrial_testing_config.json only affects how default

Chromium tests are run, but doesn't affect the default behavior of

Chrome builds shipped to end-users - these will continue to have

site-per-process off by default (and controlled via field trials).

"""

So it sounds like OOPIF is still not on by default, is that correct? (Then why change the default in tests? So that the memory cq bots test with OOPIF on? But then why chrome layer only? I also read the linked bugs, but neither answered "why?")

The "chrome layer only" explains which of the suites have a not_ prefix and why that's there, something I've been confused about.

If it's true that OOPIF is still off-by-default, do you have a rough estimate for how long we're going to test everything with OOPIF on and off?

Thanks,

Nico

[Charlie Reis]

The short answer is, it's complicated.  From the CL you note, it's enabled by default in the Chrome layer, and we're trying to keep tests covering both the enabled and non-enabled paths.

This is for multiple reasons.  First, we're in the midst of trying to launch Site Isolation by default on Chrome Desktop.  (It is enabled via field trials at a high percentage on M67, and we're still monitoring it and responding to issues.)  However, Site Isolation is not something we want to enable by default in the content module, because it is not currently used on Android or by other content embedders.

As a result, we want both paths to have test coverage for the time being.  We're evaluating our options for how to better test this state-- we hope to get to where certain bots can run in just the default configuration for how Chrome will be deployed on that platform, with something like FYI coverage for non-Chrome content embedders (which might use the non Site Isolation path on desktop).  These are options we plan to decide after the Site Isolation desktop launch sticks, since it's still possible that it might get turned off, and so we really don't want the other path to rot yet.

Charlie

[Łukasz Anforowicz]

Above //content layer:

• site-per-process is the default
• We can start removing non-site-per-process things (e.g. not_site_per_process_browser_tests test step) once site-per-process ships to stable (it is currently turned on at 99% in M67/stable, but it is not yet clear whether it will stick or not).  Hopefully this can happen in Q3 2018.
  

At //content layer and below:

• site-per-process is not the default
• For foreseeable future we will cover *both* site_per_process_content_browsertests and content_browsertests to ensure that both modes work - we need to support //content embedders that need to run without site-per-process (e.g. headless, ChromeCast).
  

Would it make things simpler if we made site-per-process the default at //content layer (and require other //content embedders to opt-out)?  This way we'd only have default and not_site_per_process_... test steps (and avoid having site_per_process_... test steps).  OTOH, AFAIR this is something that creis@ wanted to avoid, but I forgot the arguments that he used.

[Nico Weber]

First: I'm not asking for any changes, I'm just trying to understand. And what I'm writing below is brainstorm quality, so please feel free to tell me it's wrong :-)

You say that once desktop OOPIF is launched, you can remove the not_site_per_process tests since then that's the default at the chrome layer and it's no longer changing. However, don't browser_tests and interactive_ui_tests also test content and below? It sounds like you're saying that you trust the content-level tests to be sufficient for testing with OOPIF off, and that you don't need the coverage of content features through browser_tests, interactive_uit_tests, etc. Correct?

For me, it feels a bit weird to have different defaults at different layers. I would find it least surprising if the defaults are what we shipped, meaning OOPIF on everywhere on desktop (once it has stuck) with headless and cast opting out (I'm not sure if cast currently ships using the linux/cast or the android/cast port -- if Android, cast doesn't need to do anything I suppose), off everywhere on Android -- and then have not_site_per_process binaries for the content layer with a comment saying "headless doesn't use OOPIF, so add test coverage". Come to think of it, I might even use a different bot for tests in headless mode, instead of running the same binary several times in several modes. I'd be curious to hear why creis doesn't like this idea.

[Łukasz]

On Thursday, June 21, 2018 at 11:39:18 AM UTC-7, Nico Weber wrote:

First: I'm not asking for any changes, I'm just trying to understand. And what I'm writing below is brainstorm quality, so please feel free to tell me it's wrong :-)

You say that once desktop OOPIF is launched, you can remove the not_site_per_process tests since then that's the default at the chrome layer and it's no longer changing. However, don't browser_tests and interactive_ui_tests also test content and below?

Hmmm... that is probably true in practice (although ideally the //content features would be fully covered by //content-level tests...).

 

It sounds like you're saying that you trust the content-level tests to be sufficient for testing with OOPIF off, and that you don't need the coverage of content features through browser_tests, interactive_uit_tests, etc. Correct?

For me, it feels a bit weird to have different defaults at different layers. I would find it least surprising if the defaults are what we shipped, meaning OOPIF on everywhere on desktop (once it has stuck) with headless and cast opting out (I'm not sure if cast currently ships using the linux/cast or the android/cast port -- if Android, cast doesn't need to do anything I suppose), off everywhere on Android -- and then have not_site_per_process binaries for the content layer with a comment saying "headless doesn't use OOPIF, so add test coverage". Come to think of it, I might even use a different bot for tests in headless mode, instead of running the same binary several times in several modes. I'd be curious to hear why creis doesn't like this idea.

We've discussed this a bit more and I've opened https://crbug.com/856734 to track making site-per-process the default everywhere on desktop (including at the //content layer). 

[Charlie Reis]

This is mainly a problem for other content embedders, who don't expect Site Isolation to be enabled by default (and have had concerns about this in the past).  We could attempt to do this with enough outreach to the embedders, giving them ample time to opt out before we make it the default.  (I'm already planning to reach out to them to say we've enabled it in Chrome and it's available if they want to use it for Spectre, so we could lay out a timeframe there of making it opt-out rather than opt-in if we want.)

Still, I think we should probably fix the testing problem in the short term by ensuring our own bots are running what we ship, perhaps with some coverage of the non Site Isolation path on desktop on certain bots as well.

(Cross-posted to the bug as well.)

Charlie

[Nico Weber]

Hm, I'm not sure I buy that. We primarily ship chrome-the-browser, and it's the job of content embedders to keep up with upstream, no?

[Avi Drissman]

Charlie asked me this offlist, and this is what I think.

It's OK if we leave it opt-in for a while (as in the next few months) as we figure things out and get more clear on the impact. We really should, though, switch to making this opt-out for embedders. SI is such an important piece of the SSCA mitigation that we have to make the default choice safe for our users, and for the users of our users.

Of course, this should be messaged with care :) to embedder-dev, but I would love to say, "Hey embedders, Chrome has the best security architecture, and the SI team is happy to pass on the benefits of that to you and your users, with no extra work needed on your end." As per concerns, outreach is important, and I'm sure that some will complain, but I think defaulting to it being on is a much better ecosystem choice.

Avi

[Alex Gaynor]

This may be beyond the scope of the site-isolation-dev; but above and beyond wanting to offer SI's current protections to other embedders, I believe the internal architecture that enables fine grained principles to be isolated within the web security model could enable other, similar, isolation primitives. https://github.com/signalapp/Signal-Desktop/issues/2375 is an example use case I layed out.

Alex

--
You received this message because you are subscribed to the Google Groups "Site Isolation Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to site-isolation-...@chromium.org.

--

"I disapprove of what you say, but I will defend to the death your right to say it." -- Evelyn Beatrice Hall (summarizing Voltaire)
"The people's good is the highest law." -- Cicero

GPG Key fingerprint: D1B3 ADC0 E023 8CA6

[Nasko Oskov]

Should we just start a thread on embedder-dev@ now-ish with the goal of getting the conversation going? If we provide easy guidance for embedders to disable Site Isolation in the case they need to, it should not be terribly bad for them. They already have hard time keeping up, so making their life easier will be good.

[Nasko Oskov]

Alex, are you suggesting we isolate on something smaller than origin? In general, I'd suggest starting a new thread to discuss such ideas.

[Alex Gaynor]

As I said, it may be beyond the scope of this list, but I think giving the capability to embedders to craft isolation primitives on things besides traditional web origins (or Site Isolation's eTLD+1), that are specific to their domain (e.g. the process per conversation for a chat app), would be incredibly powerful, and very much in keeping with the pitch Avi's email laid out.

Alex

[David Benjamin]

Do we currently push sandboxed iframes out of process? If not, could we? At least for that particular use case, I suspect it'd work just fine. Things exposed via web platform primitives would probably both be easier for embedders to use and also be generally useful for the web platform.

[Charlie Reis]

On the enable-by-default points:

Thanks!  I agree with the plan to give content-embedders@ a heads up now and move in the direction of making it default with an opt out mechanism.  I'll get a thread started on that point soon.

On the finer-grained isolation points:

Thanks for the suggestion.  David's right that sandboxed iframes are one possibility for exploring this type of idea.  There's interest in that in https://crbug.com/627781, but it hasn't been implemented yet.

Charlie

[Charlie Reis]

To follow up, I emailed embedd...@chromium.org about the idea of making Site Isolation the default in content/ here:

https://groups.google.com/a/chromium.org/d/msg/embedder-dev/O6jGRftgpxY/AcCLJk0KBwAJ

We'll track the work in https://crbug.com/856734, keeping a way to disable it if desired.

Thanks,

Charlie

[Nico Weber]

Thanks, glad to hear it :-)