Commit graph

17 commits

Author SHA1 Message Date
Timo Tijhof
6b3b05558f API: Fix 'user_id' field of ApiCSPReport
Was accidentally turned into an always-true boolean in 5f343617,
due to confusion with JavaScript's default '||' operator.

Change-Id: I24071e22f8bb7a296ae138303f63acdb8ea4950a
2019-07-19 22:58:39 +01:00
jenkins-bot
f79c9e6ca3 Merge "ApiCSPReport: Support origin/path matching for false positives" 2019-06-18 18:32:40 +00:00
Timo Tijhof
5f34361759 ApiCSPReport: Log user ID instead of name, and limit urls to origin
These reports often contain false-positives from gadgets and
browser extensions that use a cross-domain requests for retreiving
information from a web API. (E.g. not for fetching executable JS
code, or for sending data elsewhere.)

Those API requests aren't static like "/foo.js?v2" but rather
dynamic, like /query/input+from+user, containing information about
what the user was reading, who or what they interacted with on
the wiki and/or text they entered or selected specifically.
(e.g. investigating user behaviour, counter-vandalism,
Google Translate tools, WHOIS gadgets, etc.)

Details of such action don't need to be recorded, and shown on
Logstash dashboards by default in the 'message' field. In fact,
I don't think it is needed for anything by default. If there's a
security problem, I imagine the origin suffices for a CSP block
and/or to start investigating.

Same for the user name. I don't want to see "[enwiki] John, referer
/wiki/Topic_read, chrome-extension/xyz, vandal-query.org/George".

These now log: "[enwiki] user_id 123, referer /wiki/Topic_read,
chrome-extension/xyz, vandal-query.org"

The user name still available when purposely investigating (via
public tools) by resolving the user ID.

Bug: T207900
Change-Id: Ic9855400c8cfedfa92b6659a4ad29c4dc28fb256
2019-04-17 00:01:22 +00:00
Timo Tijhof
0ca1b8a0e6 ApiCSPReport: Support origin/path matching for false positives
According to https://www.tollmanz.com/content-security-policy-report-samples/,
browsers are meant to normalise blocked-url to just the origin,
similar to referer.

However, not all browsers do this in practice, and even in Chrome
it only applies if CORS is not authorising the origin to see the
full url, which means it is usually still the full url for things
like CORS API requests to things under wmflabs.org.

The purpose of this change is to allow a wmflabs.org subdomain
and certain subdirectories to be set as false positive and have
it not log to Logstash in wmf-production.

Bug: T207900
Change-Id: I21f93223e0e3a6ca2dbbb95163a02cd88e4dfc8f
2019-04-16 23:51:56 +00:00
Bartosz Dziewoński
485f66f174 Use PHP 7 '??' operator instead of '?:' with 'isset()' where convenient
Find: /isset\(\s*([^()]+?)\s*\)\s*\?\s*\1\s*:\s*/
Replace with: '\1 ?? '

(Everywhere except includes/PHPVersionCheck.php)
(Then, manually fix some line length and indentation issues)

Then manually reviewed the replacements for cases where confusing
operator precedence would result in incorrect results
(fixing those in I478db046a1cc162c6767003ce45c9b56270f3372).

Change-Id: I33b421c8cb11cdd4ce896488c9ff5313f03a38cf
2018-05-30 18:06:13 -07:00
Kunal Mehta
bfb5cd8bb3 ApiCSPReport: Fix undefined $userAgent variable
Bug: T194899
Change-Id: Ia83f961da1db2d1245859ae584db883b7a11081c
2018-05-17 22:18:20 -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
Fomafix
d59af4c341 Use PHP's implode() with the suggested order of arguments
https://secure.php.net/manual/en/function.implode.php defines the order
of arguments as

 string implode ( string $glue , array $pieces )
 string implode ( array $pieces )

Note:
  implode() can, for historical reasons, accept its parameters in
  either order. For consistency with explode(), however, it may be less
  confusing to use the documented order of arguments.

Change-Id: I03bf5712204e283f52d3ede54af9b9ec117d4280
2018-02-22 20:24:00 +01:00
Umherirrender
f739a8f368 Improve some parameter docs
Add missing @return and @param to function docs and fixed some @param

Change-Id: I810727961057cfdcc274428b239af5975c57468d
2017-09-10 20:32:31 +02:00
jenkins-bot
84b6d5c2e5 Merge "Add missing type to @param documentation" 2017-08-11 21:31:51 +00:00
WMDE-Fisch
6df9ed1ad6 update mediawiki-codesniffer to 0.11.0 and fix issues
- mostly auto fixes
- some too long lines fixed
- ignore amp space in one case  passing by reference

Change-Id: I6472f83bc3cbf4bd629d83050cc3319b19ec465c
2017-08-11 22:27:51 +02:00
Umherirrender
5544cef16b Add missing type to @param documentation
Change-Id: I6b2c9c7af9a281fe457099cc3a336a60a25e74aa
2017-08-11 20:37:35 +02:00
Filippo Giunchedi
ba614300bc Return 400 on invalid CSP reports
Not really a server error since there's nothing we can do about invalid
user-provided data.

Bug: T166229
Change-Id: I87a7be32ae7e80c112be556bc13db19f11e614ca
2017-05-26 14:59:45 +02:00
Brad Jorsch
4e6810e4a2 API: i18n for warnings and errors
API warnings and error messages are currently hard-coded English
strings. This patch changes that.

With a few exceptions, this patch should be compatible with non-updated
extensions:
* The change to ApiBase::$messageMap will blow up anything trying to
  mess with it.
* The changes to the 'ApiCheckCanExecute' hook will cause a wrong
  (probably unparsed) error message to be emitted for extensions not
  already using an ApiMessage. Unless they're currently broken like
  Wikibase.

Bug: T37074
Bug: T47843
Depends-On: Ia2b66b57cd4eaddc30b3ffdd7b97d6ca3e02d898
Depends-On: I2e1bb975bb0045476c03ebe6cdec00259bae22ec
Depends-On: I53987bf87c48f6c00deec17a8e957d24fcc3eaa6
Depends-On: Ibf93a459eb62d30f7c70d20e91ec9faeb80d10ed
Depends-On: I3cf889811f44a15935e454dd42f081164d4a098c
Depends-On: Ieae527de86735ddcba34724730e8730fb277b99b
Depends-On: I535344c29d51521147c2a26c341dae38cec3e931
Change-Id: Iae0e2ce3bd42dd4776a9779664086119ac188412
2016-12-06 10:20:48 -05:00
Max Semenik
f0c8cbc6d9 Remove a few unused variables
Change-Id: Ibfc4c6cbbc08b5917f1a84d86d2d4a0855e371a1
2016-09-26 17:03:32 -07:00
Brian Wolff
d84479c4cd Add urls from various adware to the CSP false positive list
URLs are based on spam that comes into the Wikimedia log files,
based on testing on elwiki.

Change-Id: Iee86633abaae86c55764365042681bda1f2304be
2016-08-25 22:52:28 +00:00
Brian Wolff
ae0bae92af Add API module to receive CSP reports.
There are two expected usecases for this:
* The proposed builtin CSP support at I80f6f4
* Setting CSP headers on media served from upload.wikimedia.org

This was split from I80f6f46

For details on CSP, see http://www.w3.org/TR/CSP2/
See also https://www.mediawiki.org/wiki/Requests_for_comment/Content-Security-Policy

Related to (but not directly a fix for) T117618

Bug: T135963
Change-Id: Id92126ca7707186757e77fe50cd336ff1acb8b3f
2016-06-28 15:37:27 -04:00