Security/SOP/Application Security Reviews

Purpose

This document describes the process for requesting an application security review (a.k.a. security code review) and having said request be completed by the Security Team. This is the review described within the current Preparing For Deployment documentation, as opposed to a security preview, due-diligence third-party review, vendor assessment or privacy review.

Note: application security reviews focus very specifically upon application code as opposed to the hardware and infrastructure upon which an application runs. For example: puppet, helm, kubernetes, etc. configurations and associated infrastructure would not implicitly be a part of an application security review of MediaWiki core or a related service. Those would need to be covered within a separate audit focused upon those layers of the Wikimedia production environment.

How do I submit an application security review request?

Please use the current Phabricator form to submit your request. These requests are placed within the Incoming column of the secscrum workboard. Once triaged, they will be placed within the Back Orders column of the same workboard where they will await acceptance and scheduling - see the How are these requests prioritized? section below.

What type of project or code triggers this review process?

In theory, any code related to any Wikimedia project is eligible for an application security review. But given current resources and constraints, the following are some examples of code that the Security Team probably won't review, at least not via the application security review process:

Code not likely to be reviewed:

Routine changes which are submitted through gerrit, github or gitlab and handled via standard code review.
Security patches or other discreetly-deployed code.
Applications running under Cloud VPS or Toolforge, even higher-visibility applications like Quarry.
Any user-JavaScript or Gadgets which may run on various Wikimedia wikis.
Code which has not gone through appropriate processes (Technical decision making process, the creation of a product roadmap or support plan, sponsorship by a Wikimedia Foundation team, etc.) so as to be a strong candidate for deployment upon Wikimedia production infrastructure.

Code that will never be reviewed:

Any variety of stub code - be it a mediawiki extension, service, etc. Boilerplate code is just that and cannot serve as a proxy for reviewing code that may one day exist.
Any Work-In-Progress (WIP) patch sets, regardless of their state of completion. If the code is in a stable, testable state, it should be code-reviewed and merged via gerrit, github or gitlab.
Any relevant patch sets which have not undergone the standard code-review process and are unmerged. Again, if the code is in a stable, testable state, it should be code-reviewed and merged via gerrit, github or gitlab. Exceptions can be made to this policy if a code merge is blocked on another, key review (say from a member of the Performance Team or SRE) but it will be the responsibility of the requester to ask for such an exception from the Security Team and confirm the current state of the code.

How are these requests prioritized?

While the Security Team would love to provide on-demand application security reviews for an unlimited number of projects and codebases each quarter, this is simply not possible given current resources and constraints. Given these realities, the below is a set of steps and guidelines the Security Team uses to prioritize this work:

At the beginning of each quarter, typically within the first few days or the first week, various Security Team engineers and project managers will convene to prioritize any open requests upon the secscrum workboard, where all application security reviews are tracked.
Review requests are then scheduled via the following qualitative list, ordered from highest priority to lowest:
1. Any review request related to an existing or urgent security incident.
2. Any review request related to an imminent production deployment by a Wikimedia Foundation team.
3. Any review request related to an imminent production deployment by volunteer technical contributors.
4. All other review requests.
Number of reviews accepted for a given quarter is determined by our capacity to complete them. Again, while we would love to provide on-demand application security reviews for an unlimited number of projects and codebases each quarter, this is simply not possible given our resources and constraints.
Once a review has been accepted for a given quarter, the task will be placed within the appropriate quarterly review workboard column and a project manager from the Security Team will contact the requester via a comment on the relevant Phabricator review request task. If all valid request information has been provided and all relevant code is in a production-ready state (close to deployment with low volatility), the review request will be scheduled for completion with a specific estimated date for completion. The default estimated completion date for all reviews is the last day of the current quarter for which the review was accepted.
If, after the acceptance of a given review request for a quarter, it is determined by the Security Team that the code does not currently meet the requirements described above for the completion of an application security review, the review request will be placed within the Back Orders column for review within a forthcoming quarter. This process may repeat a number of times until all relevant code meets the Security Team's requirements for review.
While the Security Team will strive to complete an accepted review by a given estimated date of completion, we cannot always guarantee this outcome. In these rare cases, we will work with requesters to adjust expectations as necessary, while doing everything within our power to quickly and efficiently complete the review. In certain cases, reviews may extend into subsequent quarters.

Special circumstances

If certain special circumstances arise concerning the application security review process, the current Director of Security will serve as the initial point of contact to mediate and resolve any issues. Issues may include but are not limited to: overrides to the prioritization of accepted review requests, overrides to the volume of accepted review requests and whether or not a security review request meets all relevant requirements.

References

Secure Coding - Best Practices

All code SHOULD avoid the Top 25 CWEs, the OWASP Top 10 Vulnerabilities and comply with the requirements of the various Known Guides and Documentation below.
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 (e.g. the Service Template for Node).
External applications or dependencies (services, or applications such as media conversion utilities) used by the code SHOULD NOT have known, open security issues. See also the Third Party Code Review Checklist.

Known Guides and Documentation

Notes

Directive terms capitalized requirement level terms are defined within the relevant RFC.
Additional relevant Definitions.