1758389 - Create a fine-grained RFP check that uses Principal and OriginAttributes

Status: RESOLVED FIXED

(Core :: DOM: Workers, enhancement)

Description

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

While working on Bug 1758125 I was finding myself in places where I had a principal, and OriginAttributes - but no window, no document, no channel and I'm pretty sure there is no way to get any of those for e.g. a ServiceWorker or something similar.

In Bug 1635603 we created a ShouldRFP function took in an nsIChannel. Our code for the channel is, roughly:

bool PBMCheck = ...;
bool extensionCheck = ...;
bool uriCheck = ...;

bool ShouldRFP(nsIChannel* channel) {
	auto loadInfo = channel->LoadInfo();

	if(PBMCheck && loadinfo->OriginAttributes->mPrivateBrowsingId == 0) {
	  return false;
	}

	if (loadInfo->GetExternalContentPolicyType() == ExtContentPolicy::TYPE_DOCUMENT) {
	  auto channelURI = NS_GetFinalChannelURI(aChannel);

	  if(channelURI.scheme in ["about", "chrome", "resource", "view-source"]) {
	    return false;
  	  }

	  if (extensionCheck && channelURI.scheme == "web-extension") {
	    return false;
	  }

	  if (uriCheck && IsExempt(channelURI)) {
	    return false;
	  }
	} else { // Not a top-level load
	  auto loadingPrincipal = loadInfo->loadingPrincipal;

	  if(loadingPrincipal->IsSystemPrincipal()) {
	    return false;
	  }

	  if(extensionCheck && BasePrincipal::Cast(loadingPrincipal)->AddonPolicy()) {
	    return false;
	  }

	  auto principalURI = loadingPrincipal->GetURI();
	  if (uriCheck && IsExempt(principalURI)) {
	    return false;
	  }
	}
}

My proposal would be

1. to replace the IsSystemPrincipal() check in the lower half with an identical scheme check as the upper half. This should preserve the correct behavior for e.g. a javascript file loaded on a null-privileged about page. (I haven't verified this is broken though, but it seems like it would be...)

2. create a function like this which should behave the same as above. (I might even be able to delete the second half of the above function and replace it with a call to this one...)

bool PBMCheck = ...;
bool extensionCheck = ...;
bool uriCheck = ...;

bool ShouldRFP(nsIPrincpal* principal, OriginAttributes originAttributes) {
	auto loadInfo = channel->LoadInfo();

	if(PBMCheck && originAttributes->mPrivateBrowsingId == 0) {
	  return false;
	}

	auto principalURI = principal->GetURI();

	if(principalURI.scheme in ["about", "chrome", "resource", "view-source"]) {
	  return false;
  	}

	if(extensionCheck && BasePrincipal::Cast(principal)->AddonPolicy()) {
	  return false;
	}

	if (uriCheck && IsExempt(principalURI)) {
	  return false;
	}
}

Comment 1

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Tom Ritter [:tjr] (ni? for response to CVE/sec-approval/advisories/etc) from comment #0)

While working on Bug 1758125 I was finding myself in places where I had a principal, and OriginAttributes - but no window, no document, no channel and I'm pretty sure there is no way to get any of those for e.g. a ServiceWorker or something similar.

Yes, there are certainly cases where we do not have a channel available. Your proposal sounds solid to me (and hopefully there are not too many corner cases that need carve-outs).

Comment 2

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Attachment: Bug 1758389: Create a nuanced RFP check taking a principal and OA r?ckerschb

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5f45447629d4
Create a nuanced RFP check taking a principal and OA r=ckerschb

Comment 4

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/5f45447629d4