Security/Bug Approval Process: Difference between revisions

Adding security bug best practices
(Updating process to use a bullet list to raise the profile of the important points)
(Adding security bug best practices)
Line 10: Line 10:
* If we don't know the regression range we assume it needs porting to all supported branches
* If we don't know the regression range we assume it needs porting to all supported branches


==Process==
==Process for Security Bugs==
For security bugs with no sec- severity rating assume the worst and follow the rules for sec-critical. If you have experience fixing security bugs you could also take a crack at rating it yourself following the [[Security_Severity_Ratings]]. If you have any questions or are unsure about anything in this document contact us on IRC in the #security channel or ask a senior developer who has worked on a lot of security bugs.
For security bugs with no sec- severity rating assume the worst and follow the rules for sec-critical. If you have experience fixing security bugs you could also take a crack at rating it yourself following the [[Security_Severity_Ratings]]. If you have any questions or are unsure about anything in this document contact us on IRC in the #security channel or ask a senior developer who has worked on a lot of security bugs.


Line 60: Line 60:
# Security team marks tracking flags to ? for all affected versions when approved for central. (This allows release management to decide whether to uplift to branches just like always.)
# Security team marks tracking flags to ? for all affected versions when approved for central. (This allows release management to decide whether to uplift to branches just like always.)
# Weekly security/release management triage meeting goes through sec-approval + and ? bugs where beta and ESR is affected, ? bugs with higher risk (sec-high and sec-critical), or ? bugs near end of cycle.
# Weekly security/release management triage meeting goes through sec-approval + and ? bugs where beta and ESR is affected, ? bugs with higher risk (sec-high and sec-critical), or ? bugs near end of cycle.
==Patches and Commits==
* When sec-approval+ is given, commits should occur without specific mention of security, security bugs, or sec-approvers if possible.
* Comments in the code should not mention a security issue is being fixed. Don’t paint a picture or an arrow pointing to security issues any more than the code changes already do.
* Avoid linking it to non-security bugs with Blocks, Depends, or See Also, especially if those bugs may give a hint to the sort of security issue involved.  Mention the bug in a comment on the security bug instead.
* '''Do not commit tests''' when checking in to mozilla-central or, later, branches, when the security bug fix is initially checked-in. Tests should only be checked in later, after an official Firefox release that contains the fix has gone live and not for at least four weeks following that release. For example, if Firefox 53 contains a fix for a security issue that affects the world and is then fixed in 54, tests for this fix should not be checked in until four weeks after 54 goes live. The exception to this is if there is a security issue that hasn’t shipped in a release build and it is being fixed on multiple development branches (such as mozilla-central and aurora). Since the security problem was never released to the world, once the bug is fixed in all affected places, tests can be checked in to the various branches.
* Try whenever possible to file security bugs marked as such when filing, instead of filing them as open bugs and then closing later.  This is not always possible, but attention to this, especially when filing from crash-stats, is helpful.
==Pushing to Try==
* Do not push to Try servers if possible. “Pushing to Try servers exposes the security issues for these critical and high rated bugs to public viewing. In an ideal case, testing of patches is done locally before final check-in to mozilla-central.”
* If pushing to Try servers is necessary, do not include tests in the push as the tests can illustrate the exact nature of the security problem frequently.
* If you must push to Try servers, with or without tests, try to obfuscate what this patch is for. Either push it with other, non-security work, in the same area or, at the very least, do not mention the hidden security bug anywhere.
canmove, Confirmed users
4,854

edits