Security/SOP/Application Security Reviews

Review Required by: 12th February 2020

Purpose
The purpose of this Standard Operating Procedure (hereinafter referred to as SOP) is to document the requirements for individuals in need of a pre-deployment security code review.

Process
Before a new feature or significant change* to an existing feature or extension, service, or library is deployed or is intended to be deployed to production on SRE-managed hardware, it needs a security review by the Security Team.

Typically, WMF teams or individual MediaWiki developers embarking on a new project should plan to have 2 or 3 check-ins with the Security Team.
 * This SHALL NOT include routine changes which are typically submitted through Gerrit and handled via standard code review
 * This SHALL NOT include security patches or other discreetly-deployed code
 * This SHALL NOT include apps running under Cloud VPS, even high-visibility apps like Quarry, etc.
 * This SHALL NOT include any user-JavaScript or Gadgets which may run on various Wikimedia wikis
 * This SHALL include various Node services and python applications or tooling which will run under SRE-managed hardware (in addition to MW core and extensions)

If there are any questions as to what constitutes the requirement for a security review, please contact the Security Team (security-team@undefinedwikimedia.org).

Concept review (optional)

When considering a new project, if there is any doubt that the feature might not be a good idea for security, or might negatively impact user privacy, you can optionally consult with the Security Team during the Concept phase of your project. Please create a [https://phabricator.wikimedia.org/maniphest/task/edit/form/1/?title=Security%20Concept%20Review%20For%20%7B...%7D&description=%23%23%23Project%20Information%20%0A*%20Name%20of%20project%3A%0A*%20Project%20home%20page%3A%0A*%20Name%20of%20team%20which%20owns%20the%20project%3A%0A*%20Primary%20contact%20for%20the%20project%3A%0A*%20Target%20date%20for%20deployment%3A%0A*%20Link%20to%20code%20repository%3A%0A*%20Is%20this%20a%20brand-new%20project%3A%20%0A*%20Has%20this%20project%20ever%20been%20reviewed%20before%3A%20(Phab%20tasks%2C%20etc.)%0A*%20Has%20any%20risk%20assessment%20(STRIDE%2C%20etc.)%20been%20performed%3A%0A*%20Is%20there%20an%20existing%20RFC%20or%20has%20this%20been%20presented%20to%20the%20community%3A%0A*%20Is%20this%20project%20tied%20to%20a%20team%20quarterly%20goal%3A%0A*%20Does%20this%20project%20require%20its%20own%20privacy%20policy%3A%0A%23%23%23Description%20of%20the%20project%20and%20how%20it%20will%20be%20used%0A%60%2F*%20please%20be%20verbose%20and%20feel%20free%20to%20link%2Fupload%20related%20documents%20*%2F%60%0A%0A%23%23%23Description%20of%20any%20sensitive%20data%20to%20be%20collected%20or%20exposed%0A%60%2F*%20PII%2C%20credit%20cards%2C%20UA%2FIP%2C%20credentials%2C%20etc.%20*%2F%60%0A%0A%23%23%23Technologies%20employed%0A%60%2F*%20please%20list%20all%20relevant%20languages%2C%20platforms%2C%20hardware%2C%20etc.%20*%2F%60%0A%0A%23%23%23Dependencies%20and%20vendor%20code%0A%60%2F*%20please%20list%20all%20known%20internal%20and%20external%20dependencies%2C%20including%20hosting%20providers%20*%2F%60%0A%0A%23%23%23Working%20test%20environment%0A%60%2F*%20this%20is%20NOT%20A%20HARD%20REQUIREMENT%20*%2F%60%0A%60%2F*%20a%20vagrant%20role%2C%20Dockerfile%2C%20install%20instructions%2C%20outside%20proof-of-concept%20or%20ETA%20on%20existence%20*%2F%60%0A%60%2F*%20n.b.%20the%20test%20environment%20will%20determine%20if%20the%20Phabricator%20task%20needs%20to%20be%20security-protected%20*%2F%60&projects=Security-Team-Reviews Security Concept Review request] within Phabricator.

As an example, consider an extension that would allow users to include 's in wiki pages, to embed content from other sites. This would be a concept that would be inappropriate for Wikimedia, as it would allow leaking user IP addresses to third parties, in violation of our Privacy Policy. Having a Concept Review before any work is done on the extension would prevent wasted effort on an idea that is not workable within the context of Wikimedia.

Towards the conclusion of the Concept Review, the Security Team will work to ensure that you will have sufficient controls in place to address specific threats based upon your architecture. The Security Team may also suggest additional ways to reduce the attack surface for your project.

Finally, although the Concept Review is optional, performing one allows issues to be identified early on in a project's lifecycle, which is vastly preferable to discovering serious issues mere days or hours before (or after) a scheduled deployment.

Security review for deployment (mandatory)

Projects MUST have a "Security Readiness Review" task in Phabricator documenting that they have performed a pre-deployment security code review, and addressed any blockers before they can deploy a new extension or major feature.

This review will ensure that the controls identified in the design review have been implemented prior to implementation.

Additionally, the Security Team will attempt to ensure that the project avoids common implementation flaws.

There are three possible results of this review:


 * No issues found, extension can be deployed (provided other requirements at Review queue are met). (Example: T148583)
 * Only minor issues found, or any major issues found are straightforward fixes. Extension can be deployed once the found issues are fixed. (Example: T149808)
 * Major or complex security issues found. Once the identified issues are fixed, the extension will have to be re-reviewed. (Example: T133408)

Expectations

 * All code SHOULD avoid the top 25 CWEs, and comply with the requirements of Security for developers/Architecture.
 * Libraries SHOULD encourage safe practices by the developers who use them, or clearly document when misuse can result in a security flaw.
 * Services SHOULD use a standard framework or service template.
 * External applications (services, or applications such as media conversion utilities) used by the code SHOULD not have known, open security issues. The application SHOULD be supported by a competent security program.
 * For external application or libraries, a WMF team MUST be committed to being alerted about and fixing any security issues that are fixed by the upstream developers.
 * The people developing the code have already reviewed it, and believe it to be secure. See also:
 * What We Are Looking For
 * Third Party Code Review Checklist
 * Security Checklist For Developers
 * Security For Developers Tutorial
 * Security For Developers Architecture

Review process

 * 1) At least 30 days prior to the estimated deployment date, the requester creates a Phabricator Task using the Security Readiness Review request form.  If requesting a Concept/Design Review, please use the [https://phabricator.wikimedia.org/maniphest/task/edit/form/1/?title=Security%20Concept%20Review%20For%20%7B...%7D&description=%23%23%23Project%20Information%20%0A*%20Name%20of%20project%3A%0A*%20Project%20home%20page%3A%0A*%20Name%20of%20team%20which%20owns%20the%20project%3A%0A*%20Primary%20contact%20for%20the%20project%3A%0A*%20Target%20date%20for%20deployment%3A%0A*%20Link%20to%20code%20repository%3A%0A*%20Is%20this%20a%20brand-new%20project%3A%20%0A*%20Has%20this%20project%20ever%20been%20reviewed%20before%3A%20(Phab%20tasks%2C%20etc.)%0A*%20Has%20any%20risk%20assessment%20(STRIDE%2C%20etc.)%20been%20performed%3A%0A*%20Is%20there%20an%20existing%20RFC%20or%20has%20this%20been%20presented%20to%20the%20community%3A%0A*%20Is%20this%20project%20tied%20to%20a%20team%20quarterly%20goal%3A%0A*%20Does%20this%20project%20require%20its%20own%20privacy%20policy%3A%0A%23%23%23Description%20of%20the%20project%20and%20how%20it%20will%20be%20used%0A%60%2F*%20please%20be%20verbose%20and%20feel%20free%20to%20link%2Fupload%20related%20documents%20*%2F%60%0A%0A%23%23%23Description%20of%20any%20sensitive%20data%20to%20be%20collected%20or%20exposed%0A%60%2F*%20PII%2C%20credit%20cards%2C%20UA%2FIP%2C%20credentials%2C%20etc.%20*%2F%60%0A%0A%23%23%23Technologies%20employed%0A%60%2F*%20please%20list%20all%20relevant%20languages%2C%20platforms%2C%20hardware%2C%20etc.%20*%2F%60%0A%0A%23%23%23Dependencies%20and%20vendor%20code%0A%60%2F*%20please%20list%20all%20known%20internal%20and%20external%20dependencies%2C%20including%20hosting%20providers%20*%2F%60%0A%0A%23%23%23Working%20test%20environment%0A%60%2F*%20this%20is%20NOT%20A%20HARD%20REQUIREMENT%20*%2F%60%0A%60%2F*%20a%20vagrant%20role%2C%20Dockerfile%2C%20install%20instructions%2C%20outside%20proof-of-concept%20or%20ETA%20on%20existence%20*%2F%60%0A%60%2F*%20n.b.%20the%20test%20environment%20will%20determine%20if%20the%20Phabricator%20task%20needs%20to%20be%20security-protected%20*%2F%60&projects=Security-Team-Reviews Security Concept Review form].
 * 2) Weekly, the Security Coordinator reviews Requests that come into the queue.
 * 3) The Security Readiness Review request MUST include the following information (as templated within the request form):
 * 4) Name of tool/project
 * 5) Description of the tool/project
 * 6) Description of how the tool will be used at WMF
 * 7) Name of the individual/group requesting review and primary contact
 * 8) Name of the individual/group responsible for tool/project after deployment and primary contact
 * 9) The target date for deployment (or approximate date deployed if already in production or labs)
 * 10) Information from any review of the tool that has already been conducted
 * 11) Working test environment
 * 12) Programming language(s) used
 * 13) Source code repository location
 * 14) Upstream project home page (if applicable)
 * 15) WMF project home page (if applicable)
 * 16) Related Phabricator tickets
 * 17) Related patch set(s)
 * 18) Security Coordinator checks that the Task is at least 30 days prior to deployment date or puts the Task in the queue for prioritization and contacts owner of the Task.
 * 19) Security Coordinator checks that Task has ALL required information; if the Task is missing information, the Security Coordinator sends the Task back to the owner and holds the Task for 3 days awaiting information.
 * 20) If the Task meets the requirements in items (4) and (5), then the Security Coordinator approves the Task, assigns it to a Security Team Engineer and places the Task in the  “In Progress” queue.
 * 21) See the #Security-Team-Reviews workboard for currently planned reviews.
 * 22) The “In Progress” queue reflects all active Security Readiness Reviews. These tasks typically have target completion dates of two to four weeks.
 * 23) A Security Team Engineer will review the Task and if approved, will comment on the Task and update the Task as resolved, if appropriate.
 * 24) If your project is not on the schedule and you believe it should be, or if you have any questions about the Security Teams Readiness Review process, please (contact the Security Team) as soon as possible.
 * 25) If your Task is reviewed by the Security Team Engineer and requires action on your part, the Task will be placed in the Waiting on Response/Mitigation queue. The Task may reside there for no more than 30 days.
 * 26) If the Security Team Engineer has not received a response within 30 days for the above, the Task will be moved to the Frozen column.
 * 27) Tasks that have been on the Frozen column for more than 180 days will be removed from the Security-Team-Reviews project.

Escalations
If there is a need for escalation of a Task, the Director of Security will review.