1207721 - change the insider group from core-security to core-security-release

Status: RESOLVED FIXED

(bugzilla.mozilla.org :: Administration, task)

Description

[(no longer active)]

This makes it a pain to say something security sensitive on a bug that we don't want to close down...  I resorted to emailing people manually today.

Comment 1

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I see a "private" checkbox above and to the right of the comment text box. Unless you're talking about retroactively setting a comment as private?

Comment 2

[David Lawrence [:dkl]]

Are you using the new bug form? If so, you need to click 'Edit' first at the top to see the private checkboxes.

Comment 3

[(no longer active)]

No, I am talking about show_bug.cgi.  I cannot find a private checkbox here...

Comment 4

[David Lawrence [:dkl]]

(In reply to Ehsan Akhgari (don't ask for review please) from comment #3)
> No, I am talking about show_bug.cgi.  I cannot find a private checkbox
> here...

Ah sorry for not thinking of this before, but you are no longer in the insider group (core-security) which would explain why you do not see the checkbox any more. Please see dveditz if this is in error as there was some cleanup work that happened not too long ago.

Comment 5

[(no longer active)]

(In reply to David Lawrence [:dkl] from comment #4)
> (In reply to Ehsan Akhgari (don't ask for review please) from comment #3)
> > No, I am talking about show_bug.cgi.  I cannot find a private checkbox
> > here...
> 
> Ah sorry for not thinking of this before, but you are no longer in the
> insider group (core-security) which would explain why you do not see the
> checkbox any more. Please see dveditz if this is in error as there was some
> cleanup work that happened not too long ago.

Well, but I still need to be able to make private comments, even if I'm not part of core-security (which I shouldn't be, I have talked to dveditz about this before), and should see the private comments on the bugs in the components for which I am in the right security group.

Lacking this gets in the way of me getting work done when I am dealing with a security bug (which can be time sensitive.)  Note that this need doesn't arise very often for people, but at the times where it's needed, it can be critical.  One example is a private comment on a bug saying "Stop talking about this in a public bug, comment in bug whatever instead.".  :-)

Comment 6

[:glob ✱]

> [i] should see the private comments on the bugs in the components for which I am in the right security group

bugzilla only supports a single group for private comments and attachments, and right now that group is core-security.  if you aren't a member of this 'insider group', then that feature is not available to you.

an option here is to create a new group named "can-use-private" or similar, place all of the new security groups into it, then change to using that as the insider group.

it's dveditz's call if this should change.  if we want to take that path we'll need to audit the code to ensure core-security isn't hardcoded in places where it should be using the insidergroup parameter.

Comment 7

[Daniel Veditz [:dveditz]]

It's unfortunate that the same feature controls both private comments and private attachments, or that there isn't another setting for "super-sekret" attachments. One of the things we use this for is for hiding exploit attachments (not mere testcases, but working exploits) when security bugs are fixed and unhidden, because these exploits can be used against people who have not yet upgraded. We know one of the things the recent unauthorized access led to was an unknown person looking at some of these old exploits. Since these aren't the useful testcases for the bugs they weren't necessary for QA or anyone once the bug has been fixed, so access is pure risk. To that extent having the insidergroup stick with the smaller uber-access group (client-security-triage-team) was a benefit.

Everyone else losing the ability to mark comments as private has hurt, and I've received feedback along those lines from several people. (I also think it has been over-used, but definitely used to good purpose in many cases.)

Creating a can-use-private group would certainly be the most flexible. For instance, admins might also want to be able to use it, and people who handle web site bugs. Another choice might be to move the hardcoded linkage of insidergroup from core-security to core-security-release. This would restore the previous state of all the people with any level of client security bug access being able to do this.

The last option (a terrible one with lots of wasted extra work) would be to reverse the inheritance direction again so everyone inherits core-security instead of core-security-release, then move all the FIXED bugs from core-security-release back to core-security. Then change the default new security group from core-security to core-security-triage (a new group, or rename the now-useless core-security-release). That seems like a crazy amount of work compared to the other options.

I think the simplest option to restore the status quo would be to change the insidergroup to 'core-security-release' here:
https://github.com/mozilla/webtools-bmo-bugzilla/blob/bb5c5a5e62d2fd96f9b4a7339cbb0cbb33209745/docker/generate_bmo_data.pl#L416

I didn't find anywhere in the codebase that hardcoded 'core-security' for that functionality.

Comment 8

[Gervase Markham [:gerv]]

(In reply to Daniel Veditz [:dveditz] from comment #7)
> I think the simplest option to restore the status quo would be to change the
> insidergroup to 'core-security-release'

That seems to me like a good start.

Gerv

Comment 9

[(no longer active)]

(In reply to Gervase Markham [:gerv] from comment #8)
> (In reply to Daniel Veditz [:dveditz] from comment #7)
> > I think the simplest option to restore the status quo would be to change the
> > insidergroup to 'core-security-release'
> 
> That seems to me like a good start.

Sounds good to me as well!

Comment 10

[:glob ✱]

ok - there's actually quite a few places that need to be updated, with the actual change being an admin setting.

Comment 11

[:glob ✱]

Attachment: 1207721_1.patch

actually fewer changes than i initially thought :)

Comment 12

[David Lawrence [:dkl]]

Comment on attachment 8667308 [details] [diff] [review]
1207721_1.patch

Review of attachment 8667308 [details] [diff] [review]:
-----------------------------------------------------------------

r=dkl

Comment 13

[:glob ✱]

To ssh://[e-mail]/webtools/bmo/bugzilla.git
   d9d7e95..6050008  master -> master

Comment 14

[:glob ✱]

backed this change out for causing test failures

To ssh://[e-mail]/webtools/bmo/bugzilla.git
   6050008..a7a445a  master -> master

Comment 15

[Dylan Hardison [:dylan] (he/him)]

One of the places this is failing is the test_strict_isolation.t test:

my $master_gid = $sel->get_attribute('//input[@type="checkbox" and @name="groups" and @value="Master"]@id');
$sel->check_ok($master_gid);
$master_gid =~ s/group_//;

$master_gid should end up being a number, like 21, but is instead the string Master.
What's not clear is how this change triggers this.

I also note this patch didn't change every occurrence of core-security in the qa tests -- only some of them. That might be what is tripping up this test.

I'll work on isolating the cause of the other failure. 

Question for glob: 
Do we need to test strict isolation in BMO?

Comment 16

[:glob ✱]

(In reply to Dylan William Hardison [:dylan] from comment #15)
> Do we need to test strict isolation in BMO?

no.

Comment 17

[Dylan Hardison [:dylan] (he/him)]

i've isolated the breakage to the generate_bmo_data.pl script.

Comment 18

[Dylan Hardison [:dylan] (he/him)]

Attachment: 1207721_2.patch

Removing test isolation seems to make this not fail. Does this sound sane?

Comment 19

[David Lawrence [:dkl]]

Comment on attachment 8679108 [details] [diff] [review]
1207721_2.patch

Review of attachment 8679108 [details] [diff] [review]:
-----------------------------------------------------------------

r=dkl

note to self: this will require a rebuild of the docker image at https://hub.docker.com/r/dklawren/webtools-bmo-bugzilla

Comment 20

[Dylan Hardison [:dylan] (he/him)]

Should I wait to commit this until the docker image is updated?

Comment 21

[David Lawrence [:dkl]]

(In reply to Dylan William Hardison [:dylan] from comment #20)
> Should I wait to commit this until the docker image is updated?

No it needs to be committed first and then I will rebuild the docker image once github syncs up. 

Dkl

Comment 22

[Dylan Hardison [:dylan] (he/him)]

To ssh://[e-mail]/webtools/bmo/bugzilla.git
   5852295..2c08375  master -> master

Comment 23

[:glob ✱]

reopening: the parameter still needs to be changed once this passes tests.

Comment 24

[David Lawrence [:dkl]]

(In reply to Byron Jones ‹:glob› from comment #23)
> reopening: the parameter still needs to be changed once this passes tests.
 
Tests passed. Param changed.

dkl