Page MenuHomePhabricator

Proposal for partial opt-out method for Content security policy
Open, Needs TriagePublic

Description

Originally I had planned to write an additional public RFC on the user opt-out method of Content Security Policy on Wikimedia (See T135963 for original RFC). Since the original RFC, a number of things have changed:

  • A re-focus to first focus on forbidding external scripts, rather than concentrating on the anti-XSS provisions of CSP
  • A recent string of account compromises using external scripts has caused security team to expedite deploying CSP in report only mode in order to gain logging insight into external scripts that are being loaded.

With that in mind, the new threat model we are trying to address in the near-term with CSP is:

  • Privacy protection: Prevent both malicious and accidental privacy leaks by preventing loading third-party resources (e.g. Prevent the helpful interface-admin who doesn't realize the implications of putting google analytics into the site JS)
  • Force people who attack our sites with malicious javascript (e.g. by taking over an interface-admin account and editing MediaWiki:Common.js) to place all malicious JS on-site so that we can determine exactly what the attacker was doing after the fact.

We believe concentrating solely on preventing third party resource usage will be a much easier transition for the community and also provide significant immediate benefits in terms of security and privacy. We are still very much interested in the anti-XSS measures of CSP (e.g. banning 'unsafe-inline'), but intend to leave that to the distant future for now.

There's also some on-wiki notes about this at https://www.mediawiki.org/wiki/User:BWolff_(WMF)/CSP_plan

On to the meat of this specific proposal:

One use case that is common on wiki is to load data from external APIs. Often this is Cloud VPS/Toolforge (See counter vandalism network. There are several others). While we intend to fully ban loading external scripts, I personally think that allowing external API access on an opt-in basis is a totally reasonable thing for users to want.

To that end, I propose adding a special page which allows users to add a list of domains to include in the default-src directive of their CSP rule. The reason not to use a user preference is that I think changing this value should require re-authentication, as adjusting it would be useful to a malicious person.

This would only modify default-src and not script-src. So people still would only be able to directly load wikimedia sources via importScript() and friends. However as we are not prepared at this time to disable eval() and variants, people could still get around this if they wanted to. I hope banning external JS in script-src will at least discourage this practise, and perhaps in some glorious future we will be able to get rid of unsafe-eval and unsafe-inline. That is undoubtedly a long way off.

This will be backed by a custom DB table that lives in the centralauth DB (Thus it will be a global option).

The schema for this table will be something like:

CREATE TABLE /*_*/csp_sources (
	csp_user unsigned int NOT NULL,
	csp_domain VARBINARY(255) NOT NULL,
	csp_timestamp BINARY(14) NOT NULL
);

There's some question as to what the allowed values should be. Originally I envisioned this as a simple checkbox that would add * to the default-src value, allowing access to the entire internet. Upon further reflection, I think we should discourage users from doing that. Forcing users to input specific domains reduces the potential risk to the specific domains they actually use. It also means it is less likely for a wide number of users to all use the same domain, meaning an attacker won't be able to just compromise a single domain to target all the opted-out users. Thus I would like to require users to input specific domains, and furthermore in the case of Cloud VPS they wouldn't be allowed to specify *.wmflabs.org but instead must specify someproject.wmflabs.org. In the case of toolforge, they would have to specify a path, so tools.wmflabs.org/bawolff/ and not simply tools.wmflabs.org.

Thanks, I appreciate feedback on this proposal.

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 29 2018, 5:57 AM
TheDJ added a comment.EditedOct 29 2018, 12:29 PM

I like the idea and think it wouldn't be unreasonable.

@Bawolff I was also thinking a little bit about CSP improvements. We should make CSP enforcing for higher level accounts (stewards and checkuser) right now. This would likely be hard to detect for the attacker in question, as we already deployed CSP in report only. And if we are careful about deploying, I'm suspecting that most of those users themselves would never even notice this is active for them. If those users themselves do find out, their reports of it might drown out in the discussions about the console error the report-only mode is already throwing for other users.

With this in place, it might take the attacker another attack cycle to figure out that he can no longer use external scripts to compromise those types of accounts. I think this should be possible with the caching rules as they are right now, and it will protect our most sensitive accounts. We could even implement the functionality in the security patches branches, so that he doesn't figure out we serve different CSPs for different usergroups now.

Anomie added a subscriber: Anomie.EditedOct 29 2018, 2:10 PM

Some thoughts:

  • I wonder whether it would be workable to have a whitelist of domain-regexes that users could enable, rather than the blacklist for Toolforge and Cloud VPS you're talking about.
  • What's csp_timestamp for?
  • Implementation-wise, since CSP support is in core I suppose there will be configuration settings that work like $wgBotPasswordsCluster and $wgBotPasswordsDatabase, and csp_user will be the ID returned by CentralIdLookup.
  • I also suppose we'll intentionally not make an Action API module for this feature since it's very unlikely there'll be any need for API clients to manage the domains.
  • Since this is only for logged-in users and affects the page at the OutputPage or Skin level rather than the ParserOutput level, and as far as I can tell doesn't require any changes in responses from the load.php endpoint (which don't vary on the session), I think our existing practice of varying the varnish caches on the session (e.g. the session cookie) should suffice for both WMF and third parties.

Prevent the helpful interface-admin who doesn't realize the implications of putting google analytics into the site JS

Part of the point of creating interface-admins was that people who get the right should already understand that sort of thing. It's unfortunate that some of our wiki communities don't seem to have agreed when setting their local policies for granting it.

It also means it is less likely for a wide number of users to all use the same domain

I don't know about that. The domains of popular tools will likely get relatively wide use (although likely still small as a fraction of all users, or even all "power" users).

I like the idea and think it wouldn't be unreasonable.
@Bawolff I was also thinking a little bit about CSP improvements. We should make CSP enforcing for higher level accounts (stewards and checkuser) right now. This would likely be hard to detect for the attacker in question, as we already deployed CSP in report only. And if we are careful about deploying, I'm suspecting that most of those users themselves would never even notice this is active for them. If those users themselves do find out, their reports of it might drown out in the discussions about the console error the report-only mode is already throwing for other users.
With this in place, it might take the attacker another attack cycle to figure out that he can no longer use external scripts to compromise those types of accounts. I think this should be possible with the caching rules as they are right now, and it will protect our most sensitive accounts. We could even implement the functionality in the security patches branches, so that he doesn't figure out we serve different CSPs for different usergroups now.

Hmm. Interesting idea. Although maybe people could get around it with cross wiki trickery (assuming full lookup of user groups on all wikis too expensive on every request) but it would certainly make things more difficult.

Lots of privd people are using pathoschild scripts, thats most likely breakage.

Some thoughts:

  • I wonder whether it would be workable to have a whitelist of domain-regexes that users could enable, rather than the blacklist for Toolforge and Cloud VPS you're talking about.

I guess maybe, but who makes the list? I imagine community would want to vary it over time.

Another suggestion that ive heard is have some sort of friendlier interstitial similar to oauth that people can agree to. Or have it part of the gadget config and people opt in when they enable a gadget that needs it

  • What's csp_timestamp for?

When we were investigating recent breach we had to correlate times from logs with bot password inserts which was a pain. Having an insert timestamp makes investigation/cleanup much easier

  • Implementation-wise, since CSP support is in core I suppose there will be configuration settings that work like $wgBotPasswordsCluster and $wgBotPasswordsDatabase, and csp_user will be the ID returned by CentralIdLookup.

Yes. I am modelling this on bot passwords

  • I also suppose we'll intentionally not make an Action API module for this feature since it's very unlikely there'll be any need for API clients to manage the domains.

Yes. And i dont want non-humans to be messing with it

  • Since this is only for logged-in users and affects the page at the OutputPage or Skin level rather than the ParserOutput level, and as far as I can tell doesn't require any changes in responses from the load.php endpoint (which don't vary on the session), I think our existing practice of varying the varnish caches on the session (e.g. the session cookie) should suffice for both WMF and third parties.

Well long term we might set a restrictive csp on load.php and api.php for paranoia, but yes, we only have to change the headers on responses that would already be varried by session

Prevent the helpful interface-admin who doesn't realize the implications of putting google analytics into the site JS

Part of the point of creating interface-admins was that people who get the right should already understand that sort of thing. It's unfortunate that some of our wiki communities don't seem to have agreed when setting their local policies for granting it.

It also means it is less likely for a wide number of users to all use the same domain

I don't know about that. The domains of popular tools will likely get relatively wide use (although likely still small as a fraction of all users, or even all "power" users).

Indeed. More like best effort. At the very least i want to avoid whitelisting tools.wmflabs.org wholesale where an attacker can just create their own toolforge account to be on the whitelist

  • I wonder whether it would be workable to have a whitelist of domain-regexes that users could enable, rather than the blacklist for Toolforge and Cloud VPS you're talking about.

I guess maybe, but who makes the list? I imagine community would want to vary it over time.

Make it a config setting and it can be a config change Phab task.

It looks like you already have a start on a list at https://www.mediawiki.org/wiki/User:BWolff_(WMF)/CSP_plan.

Another suggestion that ive heard is have some sort of friendlier interstitial similar to oauth that people can agree to.

How do you do an interstitial on an importScript call?

Or have it part of the gadget config and people opt in when they enable a gadget that needs it

Do you trust wiki users to not put "*" or the like in the gadget config, though? This could potentially work in conjunction with the whitelist though, if exceptions specified by Gadgets still need to pass the whitelist before taking effect.

Joe added a subscriber: daniel.Oct 31 2018, 8:43 PM
daniel edited projects, added TechCom; removed TechCom-RFC.Oct 31 2018, 8:45 PM

Removing RFC tag, since a non-public RFC kind of defeats the point. TechCom will try to review this in private soon.

Bawolff updated the task description. (Show Details)Nov 1 2018, 11:45 PM

Removing RFC tag, since a non-public RFC kind of defeats the point. TechCom will try to review this in private soon.

I think this is the sort of thing, where we have to talk about it in public, as its going to be user effecting. With that in mind, I am going to make this bug be public again.

  • I wonder whether it would be workable to have a whitelist of domain-regexes that users could enable, rather than the blacklist for Toolforge and Cloud VPS you're talking about.

I guess maybe, but who makes the list? I imagine community would want to vary it over time.

Make it a config setting and it can be a config change Phab task.
It looks like you already have a start on a list at https://www.mediawiki.org/wiki/User:BWolff_(WMF)/CSP_plan.

I've been thinking about this, and I'm not a fan of having bundles of common external sources as:

  • Users should consent to each source. Adding any source affects the user's privacy. Having a bundle reduces the ability for users to understand what they are agreeing to in my opinion.
  • Most users only want one of the common sources. I haven't extensively looked at this, but from what I've seen so far, there is not a whole lot of users wanting a significant portion of the common exceptions.

Another suggestion that ive heard is have some sort of friendlier interstitial similar to oauth that people can agree to.

How do you do an interstitial on an importScript call?

The idea would be, that during the install process, users could be directed to some interstitial in the install instructions. They would not be given an interstitial on first call.

Or have it part of the gadget config and people opt in when they enable a gadget that needs it

Do you trust wiki users to not put "*" or the like in the gadget config, though? This could potentially work in conjunction with the whitelist though, if exceptions specified by Gadgets still need to pass the whitelist before taking effect.

I'm not sure I would describe this as a whitelisting approach, there is no pre-approved allowed list of sources. But yes, I would imagine if the gadget registration metadata approach was taken, it would similarly restrict the allowed sources list to not be overly broad, so that at the very least a domain would have to be specified.

Bawolff changed the visibility from "Custom Policy" to "Public (No Login Required)".Nov 1 2018, 11:53 PM
Anomie added a comment.Nov 2 2018, 3:20 PM

I've been thinking about this, and I'm not a fan of having bundles of common external sources as:

Who said anything about having bundles of common external sources?

Your original suggestion was vaguely along the lines of a blacklist: prevent people from specifying "*", or "*.wmflabs.org", or "tools.wmflabs.org" without a first path component. I guess something like

$blacklist = [
    '^\*$',
    '\*\.wmflabs\.org',
    'tools\.wmflabs\.org(?!/[^/]+/)',
];

That would prevent "*", "*.wmflabs.org", and "tools.wmflabs.org"-without-a-path. But if an attacker creates an external evil site, it wouldn't stop them from adding that evil site to any compromised account, it would just add one more step. A bit of social engineering might get people to authorize the evil site based on just an injected script somehow activating the interstitial.

My suggestion was to consider turning that around: you'd have a whitelist that only allows people to specify sources matching certain patterns. If the whitelist looked like

$whitelist = [
    '^https://(?!tools|\*)[^./]+\.wmflabs\.org(?:$|/)',
    '^https://tools\.wmflabs\.org/[^/]+/',
];

That should allow them to specify any domain at wmflabs.org except tools.wmflabs.org or *.wmflabs.org, or tools.wmflabs.org along with a first-level path. But it still would require that each user specifically enter each source they want to allow. And it would be a bit harder for the attacker to create an evil site and have compromised accounts allow it, since they'd have to make it match an existing whitelist entry.

The idea would be, that during the install process, users could be directed to some interstitial in the install instructions. They would not be given an interstitial on first call.

I note the current install process for most scripts is "edit Special:MyPage/common.js and paste in this code". There's not much opportunity for an interstitial there. Instead the install instructions would have to include another step telling the user to authorize the endpoint (however that ends up working).

For a few, the process is "go to Special:Preferences#mw-prefsection-gadgets, check a checkbox, and click Save". It's possible an interstitial could somehow be added to the Gadget extension for that case.

Perhaps the script should have some way to check whether an endpoint is allowed by the user before trying to use it, so the script itself can display the interstitial when needed.

I've been thinking about this, and I'm not a fan of having bundles of common external sources as:

Who said anything about having bundles of common external sources?
Your original suggestion was vaguely along the lines of a blacklist: prevent people from specifying "*", or "*.wmflabs.org", or "tools.wmflabs.org" without a first path component. I guess something like

$blacklist = [
    '^\*$',
    '\*\.wmflabs\.org',
    'tools\.wmflabs\.org(?!/[^/]+/)',
];

That would prevent "*", "*.wmflabs.org", and "tools.wmflabs.org"-without-a-path. But if an attacker creates an external evil site, it wouldn't stop them from adding that evil site to any compromised account, it would just add one more step. A bit of social engineering might get people to authorize the evil site based on just an injected script somehow activating the interstitial.
My suggestion was to consider turning that around: you'd have a whitelist that only allows people to specify sources matching certain patterns. If the whitelist looked like

$whitelist = [
    '^https://(?!tools|\*)[^./]+\.wmflabs\.org(?:$|/)',
    '^https://tools\.wmflabs\.org/[^/]+/',
];

That should allow them to specify any domain at wmflabs.org except tools.wmflabs.org or *.wmflabs.org, or tools.wmflabs.org along with a first-level path. But it still would require that each user specifically enter each source they want to allow. And it would be a bit harder for the attacker to create an evil site and have compromised accounts allow it, since they'd have to make it match an existing whitelist entry.

The idea would be, that during the install process, users could be directed to some interstitial in the install instructions. They would not be given an interstitial on first call.

I note the current install process for most scripts is "edit Special:MyPage/common.js and paste in this code". There's not much opportunity for an interstitial there. Instead the install instructions would have to include another step telling the user to authorize the endpoint (however that ends up working).
For a few, the process is "go to Special:Preferences#mw-prefsection-gadgets, check a checkbox, and click Save". It's possible an interstitial could somehow be added to the Gadget extension for that case.
Perhaps the script should have some way to check whether an endpoint is allowed by the user before trying to use it, so the script itself can display the interstitial when needed.

My apologies, I think I totally misunderstood you before.

In terms of having a whitelist, I guess that is a security vs convenience trade-off, and depends on how much we care/how big the long tail is. Already there are people asking about gadgets that talk to services I've never heard of (T208329), so the long tail of requests might be long, which either means lots of things added to whitelist, or we don't let non-popular things on the whitelist.

Anomie added a comment.Nov 5 2018, 7:46 PM

Could we configure CSP to not restrict CORS requests while still restricting <script> and the like? Or would there be no point then since an attacker could just load evil.js via CORS and eval it?

Could we configure CSP to not restrict CORS requests while still restricting <script> and the like? Or would there be no point then since an attacker could just load evil.js via CORS and eval it?

Basically yes we can, but also yes there isnt much point.

CSP defines multiple src types. script-src is for script tags, and default-src is anything not otherwise specified (including CORS loads. It doesnt really work on a cors vs non-cors basis, it will restrict all loads regardless of whether the same-origin-policy allows reading the result)

As you say, restricting just scripts isnt effective as the attacker could just load via CORS and eval(). The two possible solutions are to either restrict all eval() and inline scripts or restrict other type of loads too. Since for privacy reasons we want to restrict other loads anyways, I prefer that solution, particularly in the short term. With an opt-out for advanced users who want to use user-js to make mash-ups.

chasemp added a subscriber: chasemp.Nov 6 2018, 3:55 PM
brion added a subscriber: brion.Nov 8 2018, 6:44 AM

regarding:

Another suggestion that ive heard is have some sort of friendlier interstitial similar to oauth that people can agree to. Or have it part of the gadget config and people opt in when they enable a gadget that needs it

&

I note the current install process for most scripts is "edit Special:MyPage/common.js and paste in this code". There's not much opportunity for an interstitial there. Instead the install instructions would have to include another step telling the user to authorize the endpoint (however that ends up working).
For a few, the process is "go to Special:Preferences#mw-prefsection-gadgets, check a checkbox, and click Save". It's possible an interstitial could somehow be added to the Gadget extension for that case.
Perhaps the script should have some way to check whether an endpoint is allowed by the user before trying to use it, so the script itself can display the interstitial when needed.

I tend to agree that making this a first-class citizen in the process of setting up a custom script is really, really important to usability. Adding someone's script to your trusted set of code & data sources needs to have a sensible user experience, so the source-enabling needs to be either part of the setup UX, or needs to be promptable from the code.

The problem with prompting it from the code is that attack-driven code might be able to trigger the add-source UX at an unexpected time... but that's a general problem with on-demand permissions.

brion added a comment.Nov 8 2018, 6:52 AM

(my concern being the possibility of getting people to agree to click-through things they shouldn't; attacker script wouldn't be able to auth/confirm the form, in theory)

brion added a comment.Nov 8 2018, 7:35 AM

Storage note: may need/want to include a reverse-domain copy of the domain for indexing purposes for bulk lookups.

daniel moved this task from Inbox to Watching on the TechCom board.EditedNov 8 2018, 7:45 AM
daniel added a project: TechCom-RFC.

Accepted as an RFC per today's TechCom meeting. This is considered as "under discussion" for now. When the discussion has settled down, move this to the "request IRC meeting" on the TechCom-RFC board (or to thhe inbox column for review).

daniel moved this task from Inbox to Request IRC meeting on the TechCom-RFC board.Nov 8 2018, 7:46 AM
daniel added a comment.Nov 8 2018, 7:49 AM

Storage note: may need/want to include a reverse-domain copy of the domain for indexing purposes for bulk lookups.

The idea is to provide an easy way to see which domains are authorized, using the same mechanism used by externallinks.el_index: in addition to recording https://foo.bar.example.com/some/path, also record //com.example.bar.foo/some/path, for prefix lookups.

A few bits from the TechCom conversation today:

For gadgets we think the simplest and most secure approach would be to declare the whitelisted host names in the gadget definition. On the preferences page, enabling the checkbox for a gadget would be delayed by a modal asking the user whether they agree to trust the given domains to be connected to while they navigate and interact with the wiki.

For user scripts, it's a bit more complicated as there isn't a straight-forward place for authors to declare this information, and there also isn't a straight-forward way for users to be asked for permission when they are "installing" a user script because it is essentially just copying text into the editor and saving their common.js page.

I see at least two ways we could accomodate this:

  • We could not support CSP exemptions from user scripts. This would mean that initially, only gadgets can obtain this exemption and that a user can't on their own write a script that sends to or obtains data from elsewhere and have it be used by users. Instead, communities may establish some kind of convention whereby they'd have a no-op gadget maintained by trusted administrators for specific domains used by user scripts on that wiki. A user script author would need to request from this gadget maintainer for any new domains to be added before their script can work and be shared, and they'd need to instruct users to ensure they have said gadget enabled before they can use the user script. I'm expecting this won't be "acceptable" by product and by communities but I'll mention it as an option due to the potentially high cost and complexity of the alternative.
  • We could invent some kind of syntax (probably in the form of a JavaScript comment) that users would need to paste into their common.js page alongside any importScript() call that declares trusted third-party domains to allow connections to. For example: /* @trust https://translate-api.bing.com */ importScript('User:1/translate.js');. These comment would not by themselves be an exemption. Rather, they'd be a declaration for needing an exemption. Upon saving edits of this form, the comments would need to be extracted and interpreted, and if there are any new ones not previously trusted by this means, the save would be aborted (conceptually similar to captchas and abuse-filter warnings, but UX-wise similar to OAuth permissions) and the user would need to allow/reject these new domains first.
Bawolff added a comment.EditedNov 8 2018, 8:19 AM

Just FYI, the list of current report-only violations is at https://logstash.wikimedia.org/goto/17095b2a3c54b2165336719890abafb7

For just the one's to wmflabs: https://logstash.wikimedia.org/goto/51408797e6620727335244cb6312d381

For just script loads from wmflabs (aka, people I should probably reach out to): https://logstash.wikimedia.org/goto/2cfa52d54a6a46003443d0ef49ccc907

daniel added a subscriber: Bmueller.Nov 8 2018, 2:47 PM
Anomie added a comment.Nov 8 2018, 3:42 PM

I see at least two ways we could accomodate this:

  • We could not support CSP exemptions from user scripts. This would mean that initially, only gadgets can obtain this exemption and that a user can't on their own write a script that sends to or obtains data from elsewhere and have it be used by users. Instead, communities may establish some kind of convention whereby they'd have a no-op gadget maintained by trusted administrators for specific domains used by user scripts on that wiki. A user script author would need to request from this gadget maintainer for any new domains to be added before their script can work and be shared, and they'd need to instruct users to ensure they have said gadget enabled before they can use the user script. I'm expecting this won't be "acceptable" by product and by communities but I'll mention it as an option due to the potentially high cost and complexity of the alternative.

I think you're right about it not being acceptable.

  • We could invent some kind of syntax (probably in the form of a JavaScript comment) that users would need to paste into their common.js page alongside any importScript() call that declares trusted third-party domains to allow connections to. For example: /* @trust https://translate-api.bing.com */ importScript('User:1/translate.js');. These comment would not by themselves be an exemption. Rather, they'd be a declaration for needing an exemption. Upon saving edits of this form, the comments would need to be extracted and interpreted, and if there are any new ones not previously trusted by this means, the save would be aborted (conceptually similar to captchas and abuse-filter warnings, but UX-wise similar to OAuth permissions) and the user would need to allow/reject these new domains first.

That seems rather complex and fragile.

Other options:

  • If we don't mind somewhat poor UX, just let the user script's instructions direct the user to go to Special:MyCSPExceptions to add the exception.
  • Provide a module in resource/src/ that provides some sort of "mw.csp.hasException()" and "mw.csp.requestException()" methods for the user script code to call. The latter would open a new window/tab with something like an OAuth request.
RP88 added a subscriber: RP88.Nov 9 2018, 6:54 AM
Tgr added a subscriber: Tgr.Nov 9 2018, 9:13 AM

Looks like I put this under "request IRC meeting" by mistake last week. @Bawolff, do you think this would benefit from a public IRC meeting soon?

Bawolff moved this task from Backlog to In Progress on the Security-Team board.Nov 14 2018, 11:55 PM

Looks like I put this under "request IRC meeting" by mistake last week. @Bawolff, do you think this would benefit from a public IRC meeting soon?

Yes, i intend to reply to a couple comments on this ticket, but i think this would be ready for a meeting at the next available time.

Quiddity updated the task description. (Show Details)Nov 16 2018, 1:10 AM
Quiddity added a subscriber: Quiddity.
Izno added a subscriber: Izno.Nov 17 2018, 5:04 PM
JJMC89 added a subscriber: JJMC89.Nov 17 2018, 11:11 PM

TechCom is hosting an IRC meeting on this: Wednesday December 5th 11pm PST(December 6th 07:00 UTC, 08:00 CET) in #wikimedia-office

Cyberpower678 added a subscriber: Cyberpower678.EditedDec 2 2018, 3:43 PM

I would very much love to see a whitelist implemented. I for one would like to xtools.wmflabs.org and archive.org whitelisted as it's blocking an existing, and trusted, gadget from working, and blocking a new one that I'm trying to port for Internet Archive.

And having just finished reading this ticket, it seems we are heading towards user authorized exemptions. It brings up the question what about scripts going into the site's common.js that may make queries to external APIs. That would even the hit the readers or break the JS for non-users.

Anomie added a comment.Dec 3 2018, 4:12 PM

In general, there shouldn't be scripts in the site's common.js that are hitting external APIs. If such scripts are needed for some reason, the necessary domains could be added to $wgCSPHeader after being evaluated by the Security team.

In general, there shouldn't be scripts in the site's common.js that are hitting external APIs. If such scripts are needed for some reason, the necessary domains could be added to $wgCSPHeader after being evaluated by the Security team.

With that in mind, any chance of, if only temporary, adding the domains xtools.wmflabs.org and archive.org to that variable? A widely used gadget is being blocked from working correctly because of this.

Anomie added a comment.Dec 3 2018, 9:02 PM

I don't think we're enforcing CSP anywhere yet, so it shouldn't be blocking anything from working.

It is being enforced. I have the console errors to go with it.

[Error] [Report Only] Refused to connect to https://archive.org/services/context/books?url=https://en.wikipedia.org/wiki/Easter_Island because it appears in neither the connect-src directive nor the default-src directive of the Content Security Policy.
[Error] Origin https://en.wikipedia.org is not allowed by Access-Control-Allow-Origin.
[Error] XMLHttpRequest cannot load https://archive.org/services/context/books?url=https://en.wikipedia.org/wiki/Easter_Island due to access control checks.
[Log] The requested service https://archive.org/services/context/books?url=https://en.wikipedia.org/wiki/Easter_Island failed: 0, error (index.php, line 160)
[Error] Failed to load resource: Origin https://en.wikipedia.org is not allowed by Access-Control-Allow-Origin. (books, line 0)

Tgr added a comment.Dec 3 2018, 9:38 PM

As suggested by [Report Only], that is only a report, not a real error. The real errors have to do with CORS, not CSP (and the restriction is on the archive.org side).

Anomie added a comment.EditedDec 3 2018, 9:39 PM

[Error] [Report Only] Refused to connect to https://archive.org/services/context/books?url=https://en.wikipedia.org/wiki/Easter_Island because it appears in neither the connect-src directive nor the default-src directive of the Content Security Policy.

This is from CSP. See how it says "Report Only", that means it didn't block, just reported.

[Error] Origin https://en.wikipedia.org is not allowed by Access-Control-Allow-Origin.

This says that the response from the remote server did not allow CORS.

When I try making a CORS request to your URL, it seems to have ignored the Origin header and issued a non-CORS response.

[Error] XMLHttpRequest cannot load https://archive.org/services/context/books?url=https://en.wikipedia.org/wiki/Easter_Island due to access control checks.
[Log] The requested service https://archive.org/services/context/books?url=https://en.wikipedia.org/wiki/Easter_Island failed: 0, error (index.php, line 160)
[Error] Failed to load resource: Origin https://en.wikipedia.org is not allowed by Access-Control-Allow-Origin. (books, line 0)

All that is because of the CORS failure, not CSP.

In general, there shouldn't be scripts in the site's common.js that are hitting external APIs. If such scripts are needed for some reason, the necessary domains could be added to $wgCSPHeader after being evaluated by the Security team.

With that in mind, any chance of, if only temporary, adding the domains xtools.wmflabs.org and archive.org to that variable? A widely used gadget is being blocked from working correctly because of this.

Anomie added a comment.Dec 5 2018, 6:27 PM

Looks like you somehow reposted T208188#4795556? It was already answered above.

Looks like you somehow reposted T208188#4795556? It was already answered above.

Oops I did. It is indeed a CORS issue, but it still would be fantastic to have those domains Whitelisted before CSP gets enforced. :-)

One of the things being discussed in here is a way for gadgets to easily prompt users to add the necessary CSP opt-outs.

One of the things being discussed in here is a way for gadgets to easily prompt users to add the necessary CSP opt-outs.

We should make sure this behaves correctly for gadgets that are enabled per default (including for anon users). "Correctly" here means "no opt outs for default gadgets".

daniel added a comment.Dec 6 2018, 6:21 AM

It seems to me that we will need a global blacklist, so ops can effectively override user opt-in on this, e.g. when a given source is compromised.

daniel added a comment.Dec 6 2018, 1:20 PM

I just realized this this is misleading:

csp_domain VARBINARY(255) NOT NULL

We want URL prefixes (or full URLs), not domains. So this should be:

csp_url VARBINARY(255) NOT NULL
daniel added a subscriber: hoo.EditedDec 6 2018, 1:22 PM

I just had a brief chat with @hoo, and he noted that we'll have to protect the special page against gadgets that try to exempt themselves via that special page. And of course against external attackers that try to force exemptions via XRF. The very minimum is POST-only and a token, but we may have to force re-authentication. Also, we may want to suppress the loading of gadgets and user scripts on that page, like we do on login and preferences pages.

Another option is to not do this work. I don't see a good justification for letting any users bypass CSP. The ones who will complain the most about this are also the ones who shouldn't be allowed (sysops, interface admins, stewards, …). Yes, toolforge has a number of neat tools, but this is proposing a lot of fiddly work which I don't think adds value.

If the demand is really high, a much simpler alternative would be to have a "blessed scripts" repo that lets proxy scripts for toolforge tools be made available after manual security review on each patch, and which is exposed via docroot from mediawiki.org/magic or whatever.

Tgr added a comment.Dec 6 2018, 10:42 PM

If the demand is really high, a much simpler alternative would be to have a "blessed scripts" repo that lets proxy scripts for toolforge tools be made available after manual security review on each patch, and which is exposed via docroot from mediawiki.org/magic or whatever.

Let's not do that, we are already nowhere near able to deal with code review requests, even without having to deal with users who don't speak English and scripts which only make sense in the context of some workflow that only exists on the Zulu Wikisource. Also reviewers tend to force fairly arbitrary and diverse code style requirements on people (which is reasonable when done for a repo with shared maintainership, not so reasonable when the reviewer has no stake in the code ever running, and I'm not sure we could avoid still doing it in that situation).

Nirmos added a subscriber: Nirmos.Dec 7 2018, 8:47 AM
daniel added a comment.Dec 7 2018, 6:22 PM

Another option is to not do this work. I don't see a good justification for letting any users bypass CSP.

Making it effectively impossible to call to ToolForge from gadgets seriously diminishes the usefulness of ToolForge.

Another option is to not do this work. I don't see a good justification for letting any users bypass CSP.

Making it effectively impossible to call to ToolForge from gadgets seriously diminishes the usefulness of ToolForge.

Nothing stops a ToolForge-based tool wrapping a Wikimedia wiki page or set of API call; it's just mildly less "cool" for the users to use the tool from tools.wmflabs.org/foo rather than de.wiktionary.org.

Tgr added a comment.Dec 8 2018, 7:02 AM

One thing I would consider is adding some kind of usage tracking (I think we can do that by having a stricter report-only and a looser full one and including the user id in the report uri) so people can see which URLs they are actually using and which can be removed. Otherwise they will come back after a year and have no clue which domain is for what (assuming we don't limit them to toolforge, those are least reasonably easy to interpret) and so they'll keep collecting dust forever even when the tool is defunct (users rarely remove gadgets that stop working) and will become ripe targets for attack.

However as we are not prepared at this time to disable eval() and variants, people could still get around this if they wanted to.

Or even if they did not want to, if they use something like $.getScript without realizing that that circumvents the CSP policy.

assuming full lookup of user groups on all wikis too expensive on every request

I don't see why it would be, it's easily cacheable.

Another suggestion that ive heard is have some sort of friendlier interstitial similar to oauth that people can agree to.

I was just about to suggest that, too. For gadgets it could be part of the enabling process (we might want to move away from the current system of gadget management happening on a slightly customized preferences tabs, although it's not strictly necessary). For user scripts (and as Anomie said above, it's not really realistic to do away with user scripts, short of something like T36958: User-level gadget repositories) we could use SecurityPolicyViolationEvent to trigger the interstitial (sorry Internet Explorer users) or provide an mw.csp.authorize().

(from the IRC meeting)

<duesen> I have a question about transitioning: if I'm usinx gadget X, which did not used to require access to an external resource, but is changed to now require it, what happens? does it break? is it disabled, and I have to re-enable it? how does this happen? how do I notice?

Whenever the CSP whitelist is changed, the gadget gets temporarily disabled for everyone, and they get an echo notification about it (which doesn't go away until you reenable/fully disable)? Would require a third state for gadgets besides enabled / not used, which is probably not hard to do as an ugly hack on top of the existing ugly hack of enabling gadgets via user options. We'd probably want some warnings in the gadget preferences UI, but it is well past time to turn that UI into something more BetaFeatures-like, anyway.

<_joe_> so you can authorize github.com/<someone> if you trust them

(and then several other people commenting about github user paths)

This part of the IRC discussion left me a bit confused. There is nothing interesting a script can do with github.com/<someone>. Either you want to run remote scripts and then it's raw.githubusercontent.com/<someone>, but I thought the plan is to ban all external script loading period (I certainly don't see any use case for allowing it). Or you want the github api, which is api.github.com, paths are generally not prefixed with the username, and anyway what would be the point of limiting GitHub API access to certain repos?

Krinkle added a comment.EditedDec 20 2018, 9:32 AM

From last week's TechCom meeting:

The current proposal of extending the CSP policy with a dynamically determined list of exemptions from a new database table, managed by the user via a special page seems solid and we found no concerns with this approach.

However, we've been hesitant to put this on Last Call because we want to first make sure that this would suffice from a product perspective. That isn't a decision we can make, and if this has been approved already then a brief note indicating that would suffice. If not, then I think a bit more research and iteration might be required before we can proceed. In particular, because that research could lead you to prefer a different approach.

Two use cases in particular that I (personally) think may need to be considered are:

  • Gadgets - This one is probably fairly straight-forward. The gadgets definition format is easy to extend, and the preferences page provides an existing secure venue for an interstitial to ask the user's consent.
  • User scripts - This one is tricky. Depending on how we tackle this, it's quite likely to involve significant changes in many areas of MediaWiki that I believe should be discussed in scope of this RFC.

@Bawolff For user scripts, see T208188#4730946 and T208188#4732305 about how exemptions might be added/requested. There's also the story of how/if they would be removed after scripts change internally or are uninstalled.

eranroz added a subscriber: eranroz.EditedFeb 9 2019, 1:11 PM

@Bawolff I see no one have commented here in the last weeks. CSP is enabled for ~3 months and there are important privacy concerns with current situation (T207900#4846582).
Would you mind closing the RFC and breaking this to tasks based on what was agreed here?

This is summary of the above discussion as far as I understand

Core requirements

There is more or less agreement about the requirement from core in sense of DB/UI:

  • Define exemptions table
CREATE TABLE /*_*/csp_sources (
	csp_user unsigned int NOT NULL,
	csp_url VARBINARY(255) NOT NULL, /* not csp_domain ; per Daniel's comment */
	csp_timestamp BINARY(14) NOT NULL
);
  • Define user interface to enable it

Suggested methods to populate it

Regarding population of this table the following methods were suggested:

Methodproscons
SecurityPolicyViolationEventBrowsers builtin feature [covers gadgets/userscripts etc]Experimental, not supported by IE, non intuitive mapping between feature (gadgetX) and resource (http://...)
gadget-definitionEasy to define and trackonly gadgets [not user scripts]
mw.csp.authorize / mw.csp.requestExceptionWould work for gadgets/user scriptsImperative approach (non declarative) requires change in all user scripts
magic comments (/* @trust https://... )can work for user scriptscomplex and fragile

I think we should go for degrade gracefully approach mixing the above solutions:

MW Core:

  1. Core defines CSP table and simple UI though special page (solution for any MW instance, whether or not gadgets extension is installed; works in IE)
  2. SecurityPolicyViolationEvent to popup oauth like dialog for ease of use (no need to access to special page). E.g: document.addEventListener("securitypolicyviolation", (e) => { mw.csp.requestException(featureName=NULL, resources=[e.blockedURI]) }); // "Some unknown script have tried to load external code X. This may pose a security/privacy risk. [Block] [Approve]")

Aside from core solution we can OPTIONALLY (recommended but not required) expose it in a user friendly whenever we want to modify it. lazy gadget/userjs developers who don't define trusted urls gives the core to handle it in less user friendly solution (no feature name).

Extension Gadget:

  • Add to gadget-defintions trusted URLs
  • Expose the core capabilities: in Gadget section whenever enabling gadget:

onGadgetInstall(gadget) => { mw.csp.requestException(featureName=mw.msg(gadget.name), resources=gadget.trustedUrls) } // "Gadget-X would like to allow to load external resource X. This may pose a security/privacy risk. [Block] [Approve]") }

User scripts:

  • Optionally implement

mw.csp.requestException(featureName='MY USER SCRIPT', resources=myTrustedUrls) } } // "MY USER SCRIPT would like to allow to load external resource X. This may pose a security/privacy risk. [Block] [Approve]")