Security/Firefox security bug fixing: Difference between revisions
(short explanation why obfuscation isn't always a toxic anti-pattern) |
(sec-approval+ may not be for immediate landing) |
||
Line 89: | Line 89: | ||
* How likely is this patch to cause regressions; how much testing does it need? | * How likely is this patch to cause regressions; how much testing does it need? | ||
3. | 3. When you get '''‘sec-approval+''' read the associated comment before landing. The patch approval may be conditional on waiting until a later date before landing depending on the severity of the bug and the complexity of the patch. For a severe bug with a more obvious and safe patch we want the narrowest possible window of time between when the patch is public and when the fix is shipped. For other patches we may want to wait until the next cycle if we're too close to the release date to take the regression risk. Come talk to us if the suggested date causes conflicts and we'll see if we can adjust the date or find someone to land the patches for you. | ||
===Landing tests=== | ===Landing tests=== |
Revision as of 19:39, 10 September 2019
How to patch a core-security bug in Firefox - developer guidelines
A bug has been reported as security-sensitive in Bugzilla and received a security rating.
If this bug is private - which is most likely for a reported security bug - the process for patching is slightly different than the usual process for fixing a bug.
Here are security guidelines to follow if you’re involved in reviewing, testing and landing a security patch.
Keeping private information private
A security-sensitive bug in Bugzilla means that all information about the bug except its ID number are hidden. This includes the title, comments, reporter, assignee and CC’d people.
A security-sensitive bug usually remains private until a fix is shipped in a new release, and after a certain amount of time to ensure that a maximum number of users updated their version of Firefox. Bugs are usually made public after 6 months and a couple of releases.
From the moment a security bug has been privately reported to the moment a fix is shipped and the bug is set public, all information about that bug need to be handled carefully in order to avoid an unmitigated vulnerability to be known and exploited out there before we release a fix (0-day).
During a normal process, information about the nature of bug can be accessed through:
- Bug comments (Bugzilla, GitHub issue)
- Commit message (visible on Bugzilla, tree check-ins and test servers)
- Code comments
- Test cases
- Bug content can potentially be discussed on public IRC/Slack channels and mailing list emails.
When patching for a security bug, you’ll need to be mindful about what type of information you share and where.
In commit messages
People are watching code check-ins, so we want to avoid sharing information which would disclose or help finding a vulnerability too easily before we shipped the fix to our users. This includes:
- The nature of the vulnerability (overflow, use-after-free, XSS, CSP bypass...)
- Ways to trigger and exploit that vulnerability
- In commit messages, code comments and test cases.
- The fact that a bug / commit is security-related :
- Trigger words in the commit message or code comments such as “security”, “exploitable” or the nature of a security vulnerability (overflow, use-after-free…)
- Security approver’s name in a commit message.
- The Firefox versions and components affected by the vulnerability.
- Patches with an obvious fix.
In Bugzilla and other public channels
In addition to commits, you’ll need to be mindful of not disclosing sensitive information about the bug in public places, such as Bugzilla:
- Do not add public bugs in the “duplicate”, “depends on”, “blocks” or “see also” section if these bugs could give hints about the nature of the security issue.
- Mention the bugs in comment of the private bug instead.
- Do not comment sensitive information in public related bugs.
- Also be careful about who you give bug access to: double check before CC’ing the wrong person or alias.
On IRC, Slack channels, GitHub issues, mailing lists: If you need to discuss about a security bug, use a private channel (protected with a password or with proper right access management)
Process for sec-high and sec-critical bugs
Reviewing sec-high and sec-critical core-security bugs
So the bug you’re working on has received a security rating ranging from sec-high to sec-critical.
When your patch is ready for review:
- The commit message and code comments should not disclose any sensitive information (remember, people are watching our code check-ins). Do not mention that it is a security fix, its nature or security approvers when describing what the patch is doing.
- While the Developer guide specifies that “The checkin comment for the change you push should include the bug number, the names of the reviewers, and a clear explanation of the fix. Please say what changes are made, not what problem was fixed,” in the case of a security bug you’ll have to be careful about not being too “obvious” in your explanation.
- The patch which is going to land should not include any tests.
The next step is to ask for security approval for checkin.
Testing sec-high and sec-critical bugs
Pushing to Try servers requires Level 1 Commit access but content viewing is publicly accessible.
As much as possible, do not push to Try servers. Testing should be done locally before checkin in order to prevent public disclosing of the bug.
If you need to push to Try servers, make sure your tests don’t disclose what the vulnerability is about or how to trigger it. Do not mention anywhere it is security related.
Obfuscating a security patch
If your security patch looks obvious because of the code it contains (e.g. a one-line fix), or if you really need to push to Try servers, consider integrating your security-related patch to non-security work in the same area. And/or pretend it is related to something else, like some performance improvement or a correctness fix. This will help making the security issue less easily identifiable. (The absolute ban against "Security through Obscurity" is in relation to cryptographic systems. In other situations you still can't rely on obscurity but it can sometimes buy you a little time. In this context we need to get the fixes into the hands of our users faster than attackers can weaponize and deploy attacks and a little extra time can help.)
Landing sec-high and sec-critical bugs
When a patch is ready for check-in, you’ll need to request for security approval before landing it. Security approval by the security management team is necessary to evaluate the risks around a patch and the right time to land it on branches.
1. Set the sec-approval flag to ‘?’ for the patch ( in attachment section)
2. Provide information about your patch in a security approval request comment:
[Security approval request comment] * How easily can the security issue be deduced from the patch? * Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? * Which older supported branches are affected by this flaw? * If not all supported branches, which bug introduced the flaw? * Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? * How likely is this patch to cause regressions; how much testing does it need?
3. When you get ‘sec-approval+ read the associated comment before landing. The patch approval may be conditional on waiting until a later date before landing depending on the severity of the bug and the complexity of the patch. For a severe bug with a more obvious and safe patch we want the narrowest possible window of time between when the patch is public and when the fix is shipped. For other patches we may want to wait until the next cycle if we're too close to the release date to take the regression risk. Come talk to us if the suggested date causes conflicts and we'll see if we can adjust the date or find someone to land the patches for you.
Landing tests
Tests can be landed after the release has gone live and not before at least 4 weeks following that release.
The exception is if a security issue has never been shipped in a release build and has been fixed in all development branches.
Making a security bug public
This is the responsibility of the security management team.
Other bug cases
Sec-low, sec-moderate, sec-other, sec-want bugs
The developer can land the patch without any explicit approval (sec-approval).
All other guidelines regarding keeping bug information private apply.
Recent regressions (development branches only)
If a specific regression check-in
- has been identified
and
- has never been shipped in anything other than a nightly build
and
- does not affect ESR and Beta
then the developer can land the patch without any explicit approval (sec-approval). Tests can be landed after the issue has been fixed in all affected branches.
All other guidelines regarding keeping bug information private apply.
Security-core bugs with no ratings
If a security bug hasn’t received any rating, you should either rate it following the Security_Severity_Ratings before proceeding or request help from a more experienced Mozilla developer. If no one on your team can help mail security@mozill.org or ask in the #security channel. NOTE: #security is not private. Do NOT describe or explain the bug. Simply ask "Can someone give a security rating to bug XXXXX?" If the auto-linking bot isn't around then an actual bug link is appreciated.
Essentials
- Do not disclose any information about the vulnerability before a release with a fix has gone live for enough time for users to update their software.
- This includes code comments, commit messages, tests, public communication channels.
- If any doubt: request sec-approval?
- If any doubt: needinfo security folks.
- If there’s no rating, assume the worst and treat the bug as sec-critical.
Documentation & Contacts
Normal process for submitting a patch: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch
How to file a security bug: Security/Fileabug
Handling Mozilla security bugs (policy): https://www.mozilla.org/en-US/about/governance/policies/security-group/bugs/
Security Bug Approval Process: Security/Bug_Approval_Process
Contacting the Security team(s) at Mozilla: Security