Security/Bug Approval Process - Revision history

Fbraun: the page has moved.

2020-04-30T11:37:41Z

the page has moved.

Tritter: /* Process for Security Bugs (sec-approver Perspective) */

2019-10-25T19:51:51Z

Process for Security Bugs (sec-approver Perspective)

← Older revision		Revision as of 19:51, 25 October 2019
Line 99:		Line 99:
	* Separate out the test and comments in the code into a followup commit we will commit later.		* Separate out the test and comments in the code into a followup commit we will commit later.
	* Remove the commit message and place it in the bug or comments in a followup commit.		* Remove the commit message and place it in the bug or comments in a followup commit.
			* Please land it bundled in with another commit
	* Land today		* Land today
	* Land today, land the tests after <date>		* Land today, land the tests after <date>

Dveditz: adding options for remembering to check in tests later

2019-10-16T17:08:55Z

adding options for remembering to check in tests later

← Older revision		Revision as of 17:08, 16 October 2019
Line 51:		Line 51:
	* Separate out tests into a separate commit. '''Do not commit tests when checking in''' when the security bug fix is initially checked-in. '''Remember we don’t want to 0-day ourselves!'''		* Separate out tests into a separate commit. '''Do not commit tests when checking in''' when the security bug fix is initially checked-in. '''Remember we don’t want to 0-day ourselves!'''
	** 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 beta). 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.		** 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 beta). 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.
			** There are two main techniques for remembering to check in the tests later:
			*** clone the sec bug into a hidden "task" bug "land tests for bug xxxxx" and assign to yourself. It should get a "sec-other" keyword rating.
			*** Or, set the "in-testsuite" flag to "?", and later set it to "+" when the tests get checked in.

Tritter: /* Developing the Patch */

2019-10-10T03:59:12Z

Developing the Patch

← Older revision		Revision as of 03:59, 10 October 2019
Line 31:		Line 31:
	* If you must push to Try servers, with or without tests, try to obfuscate what this patch is for. Try to push it with other, non-security work, in the same area.		* If you must push to Try servers, with or without tests, try to obfuscate what this patch is for. Try to push it with other, non-security work, in the same area.

	Request review of the patch in the same process as normal. After the patch has received an r+ you will request sec-approval.		Request review of the patch in the same process as normal. After the patch has received an r+ you will request sec-approval. See [https://wiki.mozilla.org/Security/Firefox_security_bug_fixing Security/Firefox_security_bug_fixing] for more examples/details of these points.

	=== Requesting sec-approval ===		=== Requesting sec-approval ===

Tritter: /* Process for Security Bugs (sec-approver Perspective) */

2019-10-09T21:11:51Z

Process for Security Bugs (sec-approver Perspective)

@@ Line 87: / Line 87: @@
 # The Security assurance team goes through sec-approval ? bugs daily and approves low risk fixes for central (if early in cycle). Developers can also ping the Security Assurance Team (specifically Tom Ritter & Dan Veditz) in #security on Slack when important.
 # 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.
-(more coming)

Tritter: reorganizing page, plus some minor edits like slack for irc etc

2019-10-09T20:53:44Z

reorganizing page, plus some minor edits like slack for irc etc

← Older revision		Revision as of 20:53, 9 October 2019
Line 15:		Line 15:
	* 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 for Security Bugs==		==Process for Security Bugs (Developer Perspective) ==
	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.

	Core-security bug fixes ~~should just~~ be landed by a developer without any		=== Filing / Managing Bugs ===
	explicit approval if:
			* 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.
			* Avoid linking it to non-security bugs with Blocks, Depends, Regressions, 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. We can always fill in the links later after the fix has shipped.


			=== Developing the Patch ===

			* 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. We can always fill in the links later after the fix has shipped.
			* Do not push to Try servers if possible: this 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 the bug number in the patch'''. Ideally, 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. Try to push it with other, non-security work, in the same area.

			Request review of the patch in the same process as normal. After the patch has received an r+ you will request sec-approval.

			=== Requesting sec-approval ===

			For security bugs with no sec- severity rating assume the worst and follow the rules for sec-critical. During the sec-approval process we will notice it has not been rated and rate it during the process.

			Core-security bug fixes can be landed by a developer without any explicit approval if:

	'''A)''' The bug has a sec-low, sec-moderate, sec-other, or sec-want rating.<br>   '''<u>or</u>'''<br>'''B)''' The bug is a recent regression on mozilla-central. This means		'''A)''' The bug has a sec-low, sec-moderate, sec-other, or sec-want rating.<br>   '''<u>or</u>'''<br>'''B)''' The bug is a recent regression on mozilla-central. This means
Line 28:		Line 46:
	If it meets the above criteria, check that patch in.		If it meets the above criteria, check that patch in.

	Otherwise, if the bug has a patch and is sec-high or sec-critical, the developer should ~~set~~ the sec-approval ~~flag to '?' on the patch when it is ready to be checked into mozilla-central (or elsewhere if it is branch only)~~.		Otherwise, if the bug has a patch and is sec-high or sec-critical, the developer should prepare the patch for sec-approval. This entails:

	~~If you have a patch~~ and the bug is a ~~hidden core-~~security bug ~~with no rating then either:~~		* Commit should occur without specific mention of security, security bugs, or sec-approvers if possible. While comprehensive commit messages are generally encouraged; they should be omitted for security bugs and instead be posted in the bug (which will eventually become public.)
	~~# request sec~~-~~approval (to be safe) and wait for a rating, <br>~~'''OR'''		* Separate out tests into a separate commit. '''Do not commit tests when checking in''' when the security bug fix is initially checked-in. '''Remember we don’t want to 0-day ourselves!'''
	~~# rate it~~ following the ~~[[Security_Severity_Ratings]]~~ and then ~~proceed according~~ to ~~whether~~ the bug is ~~low/moderate or high/critical as above~~.		** 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 beta). 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.

	If developers are unsure about a bug and it has a patch ready, just ~~mark the~~ sec-approval ~~flag to '?'~~ and move on. Don't overthink it!
			Following that, set the sec-approval flag to '?' on the patch when it is ready to be checked into mozilla-central (or elsewhere if it is branch only).

			If developers are unsure about a bug and it has a patch ready, just request sec-approval anyway and move on. Don't overthink it!

	An automatic nomination comment will be added to bugzilla when sec-approval is set to '?'. The questions in this need to be filled out as best as possible when sec-approval is requested for the patch.		An automatic nomination comment will be added to bugzilla when sec-approval is set to '?'. The questions in this need to be filled out as best as possible when sec-approval is requested for the patch.
Line 55:		Line 76:
	This is similar to the ESR approval nomination form and is meant to help us evaluate the risks around approving the patch for checkin.		This is similar to the ESR approval nomination form and is meant to help us evaluate the risks around approving the patch for checkin.

	When the bug is approved for landing, the sec-approval flag will be set to '+' with a comment from the approver to land the patch. At that point, land it.		When the bug is approved for landing, the sec-approval flag will be set to '+' with a comment from the approver to land the patch. At that point, land it according to instructions provided..

	This will allow us to control when we can land security bugs without exposing them too early and to make sure they get landed on the various branches.		This will allow us to control when we can land security bugs without exposing them too early and to make sure they get landed on the various branches.

			If you have any questions or are unsure about anything in this document contact us on Slack in the #security channel or the current sec-approvers Dan Veditz and Tom Ritter.

			==Process for Security Bugs (sec-approver Perspective) ==

	The security assurance team and release management will have their own process for approving bugs:		The security assurance team and release management will have their own process for approving bugs:

	# The Security assurance team goes through sec-approval ? bugs daily and approves low risk fixes for central (if early in cycle). Developers can also ping the Security Assurance Team (specifically ~~Al Billings~~ & Dan Veditz) in #security on ~~IRC~~ when important.		# The Security assurance team goes through sec-approval ? bugs daily and approves low risk fixes for central (if early in cycle). Developers can also ping the Security Assurance Team (specifically Tom Ritter & Dan Veditz) in #security on Slack when important.
	~~# if the requestee or others have identified the regressing bug, add "regression" keyword and put the bug in the "blocks" field~~.
	# 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==~~		(more coming)
	* When sec-approval+ is given, commits should occur without specific mention of security, security bugs, or sec-approvers if possible. As above, if you can check-in with a cover bug in the same area to obfuscate that there is a security fix, that is ideal.
	* 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. We can always fill in the links later after the fix has shipped.
	* '''Do not commit tests when checking in''' to mozilla-central or, later, branches, when the security bug fix is initially checked-in. '''Remember we don’t want to 0-day ourselves!'''
	** 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.

Arroway: Add link to developer guidelines page

2019-06-23T10:44:06Z

Add link to developer guidelines page

← Older revision		Revision as of 10:44, 23 June 2019
Line 1:		Line 1:

			==How to fix a core-security bug in Firefox - developer guidelines==

			Follow these security guidelines if you’re involved in reviewing, testing and landing a security patch: [[Security/Firefox_security_bug_fixing]]

	==Purpose: don't 0-day ourselves==		==Purpose: don't 0-day ourselves==
	People watch our check-ins. They may be able to start exploiting our users before we can get an update out to them if		People watch our check-ins. They may be able to start exploiting our users before we can get an update out to them if

Abillings: /* Patches and Commits */

2017-02-18T00:03:43Z

Patches and Commits

← Older revision		Revision as of 00:03, 18 February 2017
Line 65:		Line 65:
	* 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.		* 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. We can always fill in the links later after the fix has shipped.		* 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. We can always fill in the links later after the fix has shipped.
	* Do not commit tests when checking in to mozilla-central or, later, branches, when the security bug fix is initially checked-in. Remember we don’t want to 0-day ourselves! 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.		* '''Do not commit tests when checking in''' to mozilla-central or, later, branches, when the security bug fix is initially checked-in. '''Remember we don’t want to 0-day ourselves!'''
			** 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.		* 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.

Abillings: /* Patches and Commits */

2017-02-18T00:02:05Z

Patches and Commits

← Older revision		Revision as of 00:02, 18 February 2017
Line 62:		Line 62:

	==Patches and Commits==		==Patches and Commits==
	* When sec-approval+ is given, commits should occur without specific mention of security, security bugs, or sec-approvers if possible.		* When sec-approval+ is given, commits should occur without specific mention of security, security bugs, or sec-approvers if possible. As above, if you can check-in with a cover bug in the same area to obfuscate that there is a security fix, that is ideal.
	* 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.		* 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.		* 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. We can always fill in the links later after the fix has shipped.
	* ~~'''~~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.		* Do not commit tests when checking in to mozilla-central or, later, branches, when the security bug fix is initially checked-in. Remember we don’t want to 0-day ourselves! 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.		* 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.

Abillings: /* Pushing to Try */

2017-02-17T23:58:04Z

Pushing to Try

← Older revision		Revision as of 23:58, 17 February 2017
Line 69:		Line 69:

	==Pushing to Try==		==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.”		* 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 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.		* 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.