Security/Firefox security bug fixing: Difference between revisions

Line 47: Line 47:
If you need to discuss about a security bug, use a private channel (protected with a password or with proper right access management)
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==
== During Development ==
===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===
===Testing sec-high and sec-critical bugs===
Line 73: Line 63:
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.)
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===
===Requesting sec-approval===


When a patch is ready for check-in, you’ll need to '''request for security approval before landing it'''.
See [https://wiki.mozilla.org/Security/Bug_Approval_Process] for more details
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===
===Landing tests===
Line 100: Line 76:


This is the responsibility of the security management team.
This is the responsibility of the security management team.


==Other bug cases==
==Other bug cases==
124

edits