Add gerrit.wikimedia.org to the Phabricator CSP
Closed, DeclinedPublic
Actions

Assigned To

None

Authored By

	Jdlrobson
	Mar 14 2019, 3:15 PM

Description

As of Chrome 73, the Pherrit Chrome extension is broken 😭

In Phabricator, open up the developer console and run

fetch("https://gerrit.wikimedia.org/r/changes/?pp=0&o=TRACKING_IDS&o=DETAILED_LABELS&q=bug:T210653+OR+bug:T195482+OR+bug:T212465+OR+bug:T216495+OR+bug:T210658+OR+bug:T195478+OR+bug:T217102+OR+bug:T186737+OR+bug:T213847+OR+bug:T150377+OR+bug:T217298+OR+bug:T215477+OR+bug:T201339+OR+bug:T218173+OR+bug:T217296+OR+bug:T216198+OR+bug:T204627+OR+bug:T213352+OR+bug:T217820+OR+bug:T168861+OR+bug:T216264+OR+bug:T139317+OR+bug:T212961+OR+bug:T208605", {
			"headers":{"accept":"application/json",
				'Content-Type': 'application/json',
				'Access-Control-Allow-Origin': '*'},"method":"GET"}).then((e)=>e.text()).then((e)=>console.log(e));

Expected: Request goes through
Actual: VM581:1 Refused to connect to 'https://gerrit.wikimedia.org/r/changes/?pp=0&o=TRACKING_IDS&o=DETAILED_LABELS&q=bug:T210653+OR+bug:T195482+OR+bug:T212465+OR+bug:T216495+OR+bug:T210658+OR+bug:T195478+OR+bug:T217102+OR+bug:T186737+OR+bug:T213847+OR+bug:T150377+OR+bug:T217298+OR+bug:T215477+OR+bug:T201339+OR+bug:T218173+OR+bug:T217296+OR+bug:T216198+OR+bug:T204627+OR+bug:T213352+OR+bug:T217820+OR+bug:T168861+OR+bug:T216264+OR+bug:T139317+OR+bug:T212961+OR+bug:T208605' because it violates the document's Content Security Policy.

This used to work in Chrome 72, it no longer does.

Event Timeline

Jdlrobson created this task.Mar 14 2019, 3:15 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 14 2019, 3:15 PM

Paladox added a project: Traffic.Mar 14 2019, 3:26 PM

Paladox subscribed.

• chasemp added a project: Security-Team.Mar 14 2019, 3:35 PM

• chasemp added a subscriber: Bawolff.

• Niedzielski awarded a token.Mar 14 2019, 4:03 PM

Did something change regarding https://gerrit-review.googlesource.com/Documentation/config-gerrit.html#site.allowOriginRegex recently?

(and to be clear I'm only interested in read only requests here)

I don't see allowOriginRegex in our Gerrit config at all. That should mean "By default, unset, denying all cross-origin requests." applies. I could only see that if the default changed in the latest Gerrit upgrade.

It should be the CSP on the Phabricator side.

Jdrewniak renamed this task from No longer possible to make CORS requests from Phabricator to Gerrit to Add gerrit.wikimedia.org to the Phabricator CSP.Mar 15 2019, 7:51 AM

Jdrewniak updated the task description. (Show Details)

Jdrewniak added subscribers: • mmodell, Jdrewniak.

Vgutierrez removed a project: Traffic.Mar 15 2019, 10:21 AM

jijiki triaged this task as Medium priority.Mar 19 2019, 6:10 PM

Paladox moved this task from Bugs & stuff to WMF customizations on the Gerrit board.Mar 22 2019, 11:19 PM

Nikerabbit subscribed.Sep 10 2019, 2:21 PM

• chasemp moved this task from Incoming to Back Orders on the Security-Team board.Dec 2 2019, 8:52 PM

Dzahn added a project: Traffic.Dec 6 2019, 8:41 PM

Bawolff added a project: ContentSecurityPolicy.Dec 6 2019, 11:49 PM

• ema moved this task from Backlog to Radar/Not for service by Traffic on the Traffic board.Dec 20 2019, 12:35 PM

The CSP header is being added here https://github.com/phacility/phabricator/blob/241f06c9ffc63909ec04e29f67f53cb310fb1953/src/aphront/response/AphrontResponse.php#L102 (https://github.com/phacility/phabricator/blob/241f06c9ffc63909ec04e29f67f53cb310fb1953/src/aphront/response/AphrontResponse.php#L111)

So we can hack that file for our install to whitelist gerrit.wikimedia.org.

Looks like the headers are coming from "Aphront".

https://github.com/phacility/phabricator/blob/241f06c9ffc63909ec04e29f67f53cb310fb1953/src/aphront/response/AphrontResponse.php#L102

So.. the value for $csp is set to the $altdom / $alt_host setting which is in Hiera as hiera('phabricator_altdomain', 'phab.wmfusercontent.org'),. We can't simply set multiple altdoms. So this would indeed need a hack in AphrontResponse.php afaict.

• holger.knust subscribed.Jan 31 2020, 1:13 AM

• chasemp edited projects, added Release-Engineering-Team; removed Security-Team.Feb 20 2020, 9:10 PM

Bawolff moved this task from Backlog to misc services on the ContentSecurityPolicy board.Feb 24 2020, 11:00 AM

MusikAnimal subscribed.Mar 2 2020, 4:46 PM

MusikAnimal awarded a token.Mar 2 2020, 4:48 PM

Isn't this a problem with the extension? Content scripts are subject to the document's CSP but the main extension code isn't, so the fetch should be done by the extension and then postMessage'd to the content script.

Closing this since it is an old task. Either Pherrit got fixed or nobody uses it at all. If there is a need for a specific CSP please mention it and it should be trivial to add. Gerrit and Phabricator each use Apache as a frontend so it is all about adding an apache conf (Header always set Content-Security-Policy "xxxx").

FWIW I stopped using Pherrit because I couldn't get it to work without this fix. I don't think that's a great reason to decline this task. There will be future attempts at browser extensions which hit exactly the same problem.

I don't know enough about the problem described in T218308#5758494 to know how to suggest fixing that.

Yeah, I would also use Pherrit if not for this issue. Nevertheless, I don't think a CSP exemption is the right way to fix it.

Add gerrit.wikimedia.org to the Phabricator CSPClosed, DeclinedPublicActions

Description

Event Timeline

Add gerrit.wikimedia.org to the Phabricator CSP
Closed, DeclinedPublic
Actions