Security/SOP/Security Readiness Reviews
This document describes the process for requesting a security readiness 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.
How do I submit a security readiness 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 a security readiness 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 security readiness 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.
- Code which has not gone through appropriate processes (TechCom RFC, the creation of a 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 security readiness 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 security readiness reviews are tracked.
- Review requests are then scheduled via the following qualitative list, ordered from highest priority to lowest:
- Any review request related to an existing or urgent security incident.
- Any review request related to an imminent production deployment by a Wikimedia Foundation team.
- Any review request related to an imminent production deployment by volunteer technical contributors.
- All other review requests.
- A maximum of 6 reviews requests will be accepted to be completed for a given quarter. For some quarters, fewer than 6 reviews may be accepted, given the volume of outstanding review requests and other commitments of the Security Team.
- 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.
- 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 a security readiness review, the review request will be placed within the Back Orders column for review next 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.
- If certain special circumstances arise concerning the security readiness 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.
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
- What We Are Looking For (somewhat dated)
- Security Training Materials (somewhat dated)
- Third Party Code Review Checklist
- Security Checklist For Developers
- Security For Developers Tutorial
- Security For Developers Architecture
- Directive terms capitalized requirement level terms are defined within the relevant RFC.
- Additional relevant Definitions.