Commit graph

20 commits

Author SHA1 Message Date
Timo Tijhof
0980f94e13 resourceloader: Refactor CSP $nonce passing
Follows-up 70941efd35 which broke various public
signatures of the ClientHtml class that I'd prefer to handle
differently.

This commit mainly restores support for all previously public
signatures, and either removes the need for a parameter, or moves
it to the end of the original signature (as optional param).

* ClientHtml::getHeadHtml: Remove the positional/required parameter
  that was added. Restoring the method to being a stateless computer
  that requires no parameters. Pass the option via construct instead.

* ClientHtml::makeLoad:
  - Make $nonce optional.
  - Restore $extraQuery as optional.

* ResourceLoader::makeInlineScript: Document $nonce as optional
  (matching the implementation).

Change-Id: Iaf33f2a060048e6606fba8d875b6d2953b21ef45
2018-05-17 22:28:23 -07:00
Brian Wolff
70941efd35 Initial support for Content Security Policy, disabled by default
The primary goal here is a defense in depth measure to
stop an attacker who found a bug in the parser allowing
them to insert malicious attributes.

This wouldn't stop someone who could insert a full
script tag (since at current it can't distinguish between
malicious and legit user js). It also would not prevent
DOM-based or reflected XSS for anons, as the nonce value
is guessable for anons when receiving a response cached
by varnish. However, the limited protection of just stopping
stored XSS where the attacker only has control of attributes,
is still a big win in my opinion. (But it wouldn't prevent
someone who has that type of xss from abusing things like
data-ooui attribute).

This will likely break many gadgets. Its expected that any
sort of rollout on Wikimedia will be done very slowly, with
lots of testing and the report-only option to begin with.

This is behind feature flags that are off by default, so
merging this patch should not cause any change in default
behaviour.

This may break some extensions (The most obvious one
is charinsert (See fe648d41005), but will probably need
some testing in report-only mode to see if anything else breaks)

This uses the unsafe-eval option of CSP, in order to
support RL's local storage thingy. For better security,
we may want to remove some of the sillier uses of eval
(e.g. jquery.ui.datepicker.js).

For more info, see spec: https://www.w3.org/TR/CSP2/
Additionally see:
https://www.mediawiki.org/wiki/Requests_for_comment/Content-Security-Policy

Bug: T135963
Change-Id: I80f6f469ba4c0b608385483457df96ccb7429ae5
2018-05-13 21:01:11 -07:00
Timo Tijhof
5ab0dd0888 resourceloader: Support loading group=user modules with addModules()
When a module has group=user specified, it means that its module contents
can vary by user. These kinds of requests have two special needs:
1) They need an additional "user" parameter in their load.php request,
   so that the response knows which user-context to use.
2) They need to have their 'version' hash pre-computed based on which assets
   will be loaded for this user. The general 'version' hash associated with
   this module name in the main registry (modules=startup) will be "wrong"
   as that is computed based on logged-out status.

We do this by omitting the module name from the `mw.load.load(Array modules)`
call in the HTML, and instead output a request for the full url.

This currently works fine for most cases, such as the 'user' module loaded
by MediaWiki core. The branch in getData() dealing with legacy 'only=scripts'
behaviour also covers this case.

But the case of an extension registering a group=user module and loading it the
general way (e.g. not with legacy only=scripts behaviour), would currently end
up in the Array-queue and dynamically loaded by the client-side without knowing
the correct version hash. Fortunately, no code exists that I know of that meets
these three critera (extension registered, group=user, non-legacy). However,
for the GlobalCssJs extension to migrate from legacy to non-legacy, they will
need to start doing this. This commit makes sure that that will work.

The makeLoad() method in ClientHtml has code ensuring the full-url form (with
pre-computed 'version' hash) is used for any modules with group=user. Before
this patch, we didn't get to call makeLoad() because getData() was assuming
that we only need makeLoad() when either the module should be embedded (group=private),
or when it is a style/scripts-only module. It didn't consider group=user.

Bug: T188689
Change-Id: Iaab15e5f5c12e7e28b8c81beab90948cd07cd352
2018-04-06 02:30:30 +00:00
Timo Tijhof
cfdc440d40 resourceloader: Replace ClientHtml 'target' param with 'options' array
In preparation for passing down 'safemode' from OutputPage.

Only used in one place in Wikimedia Git: OutputPage::getRlClient().

Bug: T185303
Change-Id: If01eca96986ff8d7dcdaab6910bf183ba7c7311f
2018-03-27 18:17:43 -07:00
Timo Tijhof
4937a7430e resourceloader: Add test for ClientHtml passing down 'target'
This was not previously tested.

Change-Id: I25d9ec28b712b0b699f1369b284faf337b8af5aa
2018-03-27 14:27:05 -07:00
Timo Tijhof
b6dd3b8f61 resourceloader: Add test for non-empty user module in scripts-only queue
The getData() tests did have a test case for a 'user' group module with
isKnownEmpty in the script-only queue, but not yet a non-empty one.

The makeLoad() tests did have both already. Arguably the makeLoad()
tests are higher level and observe its behaviour already (outputting
a script tag means 'loading', outputting nothing means 'ready'),
but adding it to the getData() tests for consistency.

Bug: T188689
Change-Id: I842febf996ba8bb0ea25b5ca3b5ac4503db87376
2018-03-05 23:37:31 +00:00
Umherirrender
63d96c15fd build: Updating mediawiki/mediawiki-codesniffer to 16.0.0
Change-Id: I59b59f79bbf3ce4feff3b3a20c1c31bc16370531
2018-02-17 13:29:13 +01:00
Umherirrender
14d19eb90a resourceloader: Removed tests for deprecated 'position' parameter
This parameter is a no-op for these tests, as such, nothing was being
tested by specifying it in tests. The behaviour 'test' and 'test.top'
was identical.

Bug: T184257
Change-Id: Ia2bb731f00d4b4175f7b75174aeafaca9412329e
2018-02-12 21:39:05 +00:00
Umherirrender
255d76f2a1 build: Updating mediawiki/mediawiki-codesniffer to 15.0.0
Clean up use of @codingStandardsIgnore
- @codingStandardsIgnoreFile -> phpcs:ignoreFile
- @codingStandardsIgnoreLine -> phpcs:ignore
- @codingStandardsIgnoreStart -> phpcs:disable
- @codingStandardsIgnoreEnd -> phpcs:enable

For phpcs:disable always the necessary sniffs are provided.
Some start/end pairs are changed to line ignore

Change-Id: I92ef235849bcc349c69e53504e664a155dd162c8
2018-01-01 14:10:16 +01:00
Kunal Mehta
75160bdd3b Use MediaWikiCoversValidator for tests that don't use MediaWikiTestCase
Change-Id: I8c4de7e9c72c9969088666007b54c6fd23f6cc13
2018-01-01 08:28:02 +00:00
Timo Tijhof
33868f0e46 resourceloader: Add unit test for ClientHtml::makeLoad sync=>true
Used by Special:JavaScriptTest, and we may need to use this
in OutputPage as well (for html5shiv).

Change-Id: If7d7c12056dc3aab78486050f0798d42b158d9a9
2017-10-03 18:22:43 +01:00
Timo Tijhof
0b1a7d4c59 resourceloader: Support isKnownEmpty for general modules
It was already supported for only=scripts and only=styles, but
not yet for general modules loaded as 'combined'.

This is especially useful for general modules that are private/embedded,
as this allows us to skip the loader.implement() boilerplate.

Bug: T176159
Change-Id: I83e9f74dad75867c10ebd0a94b87c31e4adbb57f
2017-09-18 18:37:13 +01:00
Brad Jorsch
f6466732bb resourceloader: Add ResourceLoaderModule::shouldEmbedModule and use it
Rather than only the 'private' group triggering embedding, allow modules
to explicitly specify if they should be embedded.

The default is still to only embed when the group is 'private', and the
'private' group is still special in that ResourceLoader::respond() will
still refuse to serve it from load.php.

Change-Id: Ib9a043c566822e278baecc15e87f9c5cebc2eb98
2017-07-21 17:57:34 +00:00
Timo Tijhof
ed28e106e3 resourceloader: Restrict addModuleStyles() to type=styles modules
If a type=general module is enqueued, don't try to load it as a
stylesheet.

* Per a464d1d41d, state tracking is already disabled for
  these loads (as otherwise we wrongly claim state=ready, when in
  fact only the styles and not the scripts were loaded).

* The warning was added in a464d1d41d.

* Default install (tested in Vagrant), Wikimedia Beta cluster, and
  Wikimedia production have seen zero violations of this warning
  in the past 7 days.

Raise severity to ERROR and add the 'continue' statement so that
these are now not loaded at all.

Bug: T92459
Change-Id: I211d56ac2df479ebf5b98667c613ecf81489539b
2017-06-28 19:18:06 -07:00
Timo Tijhof
c55aae02d0 resourceloader: Improve ResourceLoaderClientHtmlTest tests
The example for a mixed user module is outdated. Current user modules
all separate styles and scripts.

To make the effective change by the next commit easier to review,
update these tests first seperately.

Change-Id: I76a8a96eed8a8c39863c535362b1d0144ef2f866
2017-06-28 19:14:40 -07:00
Gergő Tisza
525bfbc8df Switch to librarized version of TestingAccessWrapper
Replaces \TestingAccessWrapper (defined in core) with
\Wikimedia\TestingAccessWrapper (defined in the composer package
wikimedia/testing-access-wrapper).

See https://gerrit.wikimedia.org/r/#/q/topic:librarize-testing-access-wrapper
for downstream patches.

The core version of the class is kept around for a while to avoid
circular dependency problems.

Bug: T163434
Change-Id: I52cc257e593da3d6c3b01a909e554a950225aec8
2017-04-20 14:15:57 +00:00
Timo Tijhof
44f3f068df resourceloader: Consistent stylesheet order for debug and non-debug
Previously, style modules were only in a predictable order for production mode.
In debug mode, the order was determined by order in which modules were added
to queue at run time. This made it sometimes hard to debug, especially when
dealing with gadgets that apply in a different order among each other.

Change-Id: I4bff0c91d127e4ad8015cd8c1775220fe460cbc3
2017-02-17 15:21:59 -08:00
Timo Tijhof
bc374082fa resourceloader: Remove top/bottom queue distinction
* The styles queue has always been top-only
  (except for a few months in 2015).
* The top queue loads asynchronous since mid-2015. (T107399)
  And LocalStorage eval, previously the last remaining non-async part
  of module loading, is also async as of October 2016. (T142129)

* This change merges the bottom 'mw.loader.load()' queue with the top queue.
  It also moves any other snippets potentially in the bottom queue still:
  - embed: I couldn't find any private modules with position=bottom
     (doesn't make sense due to their blocking nature). If any do exist,
     (third-party extensions?), they'll now be embedded in the <head>.
  - scripts: Any legacy 'only=scripts' requests will now initiate
     from the <head>.

Bug: T109837
Change-Id: I6c21e3e47c23df33a04c42ce94bd4c1964599c7f
2016-11-16 19:29:16 +00:00
Timo Tijhof
45bec76755 resourceloader: Don't cache stale responses in mw.loader.store
Follows-up 6fa1e56. This is already fixed for http caches by
shortening the Cache-Control max-age in case of a version mismatch.

However the client still cached it blindly in mw.loader.store.
Resolve this by communicating to the client what version of the module
was exported. The client can then compare this version to the version
it originally requested and decide not to cache it.

Adopt the module key format (name@version) from mw.loader.store
in mw.loader.implement() as well.

Bug: T117587
Change-Id: I1a7c44d0222893afefac20bef507bdd1a1a87ecd
2016-10-10 19:48:25 +00:00
Timo Tijhof
80e5b160e0 resourceloader: Move queue formatting out of OutputPage
HTML formatting of the queue was distributed over several OutputPage methods.
Each method demanding a snippet of HTML by calling makeResourceLoaderLink()
with a limited amount of information. As such, makeResourceLoaderLink() was
unable to provide the client with the proper state information.

Centralising it also allows it to better reduce duplication in HTML output
and maintain a more accurate state.

Problems fixed by centralising:

1. The 'user' module is special (due to per-user 'version' and 'user' params).
   It is manually requested via script-src. To avoid a separate (and wrong)
   request from something that requires it, we set state=loading directly.
   However, because the module is in the bottom, the old HTML formatter could
   only put state=loading in the bottom also. This sometimes caused a wrong
   request to be fired for modules=user if something in the top queue
   triggered a requirement for it.

2. Since a464d1d4 (T87871) we track states of page-style modules, with purpose
   of allowing dependencies on style modules without risking duplicate loading
   on pages where the styles are loaded already. This didn't work, because the
   state information about page-style modules is output near the stylesheet,
   which is after the script tag with mw.loader.load(). That runs first, and
   mw.loader would still make a duplicate request before it learns the state.

Changes:

* Document reasons for style/script tag order in getHeadHtml (per 09537e83).

* Pass $type from getModuleStyles() to getAllowedModules(). This wasn't needed
  before since a duplicate check in makeResourceLoaderLink() verified the
  origin a second time.

* Declare explicit position 'top' on 'user.options' and 'user.tokens' module.
  Previously, OutputPage hardcoded them in the top. The new formatter doesn't.

* Remove getHeadScripts().
* Remove getInlineHeadScripts().
* Remove getExternalHeadScripts().
* Remove buildCssLinks().
* Remove getScriptsForBottomQueue().

* Change where Skin::setupSkinUserCss() is called. This methods lets the skin
  add modules to the queue. Previously it was called from buildCssLinks(),
  via headElement(), via prepareQuickTemplate(), via OutputPage::output().
  It's now in OutputPage::output() directly (slightly earlier). This is needed
  because prepareQuickTemplate() calls bottomScripts() before headElement().
  And bottomScript() would lazy-initialise the queue and lock it before
  setupSkinUserCss() is called from headElement().
  This makes execution order more predictable instead of being dependent on
  the arbitrary order of data extraction in prepareQuickTemplate (which varies
  from one skin to another).

* Compute isUserModulePreview() and isKnownEmpty() for the 'user' module early
  on so. This avoids wrongful loading and fixes problem 1.

Effective changes in output:

* mw.loader.state() is now before mw.loader.load(). This fixes problem 2.
* mw.loader.state() now sets 'user.options' and 'user.tokens' to "loading".
* mw.loader.state() now sets 'user' (as "loading" or "ready"). Fixes problem 1.

* The <script async src> tag for 'startup' changed position (slightly).
  Previously it was after all inline scripts and stylesheets. It's still after
  all inline scripts and after most stylesheets, but before any user styles.
  Since the queue is now formatted outside OutputPage, it can't inject the
  meta-ResourceLoaderDynamicStyles tag and user-stylesheet hack in the middle
  of existing output. This shouldn't have any noticable impact.

Bug: T87871
Change-Id: I605b8cd1e1fc009b4662a0edbc54d09dd65ee1df
2016-08-08 12:23:09 -07:00