Open Stack

On Tue, Oct 8, 2013 at 5:03 AM, Thierry Carrez <thierry at openstack.org> wrote:
> Zaro wrote:
>> We are getting close to standing up a private gerrit instance for
>> security reviews (https://bugs.launchpad.net/openstack-ci/+bug/1083101).
>>  As mentioned in the bug our plan is to run a second gerrit to
>> facilitate code review for embargoed patches. But we will not run an
>> entire second shadow environment (too much effort for ~50 patches a year).
>
> That's awesome ! Thanks for working on it.
>
>> A few of the implementation details were unclear so I would like to make
>> a proposal and get feedback before continuing to work on the bug.
>>
>> A few members of infra team had a discussion on the workflow and fungi
>> proposed the following..
>> 1. git clone from security gerrit  (review-security.o.o)
>> 2. git review patch to security gerrit
>> 3. The patch is review-able by  vmt member, change owner and any
>> manually added reviewer.
>> 4. patch is reviewed and approved on review-security.o.o
>> 5. patch is copied from security gerrit to public gerrit..
>>      a. git review -d from review-security.o.o
>>      b*. git review to review.o.o
>>
>> *Note - security review information (votes/notes/etc..) will not get
>> copied to review.o.o
>>
>> Does this seem like the right workflow for approving and integrating
>> security patches?
>
> That sounds perfectly acceptable. A few questions:
>
> - would it be possible to reuse a classic repo clone (from git.o.o) and
> add an option to git review to submit to the security gerrit ? Otherwise
> we have to teach security patch developers to use specific clones from
> security gerrit before they work on security patches ?
>
It would be possible to teach git review to push to a second remote
with an option. Personally I almost prefer having two clones so that
you don't have to remember to use a special option (just thinking
about preventing accidental pushes to the public gerrit). Curious to
hear what Fungi thinks about an option like this for git review.
> - do we have the ability to run check tests (no gate tests) ?
>
At the Portland summit I think we decided that adding shadow test
infra would be too much work, so no check tests.
> - could you briefly explain how you addressed the potential leaks
> identified in gerrit (ability to download arbitrary changes from a
> guessed review number, etc) ?
>
The reason we have potential leaks here is we allow Anonymous users
read access to our repositories. On the secure Gerrit we would give
the VMT group read access by default then for each project expand that
to a project specific group. This doesn't completely eliminate the
potential for leaks but limits them to people we in theory already
trust for a given project.
>> We are also proposing that, instead of syncing accounts from public
>> gerrit to security gerrit, we shoud keep the accounts independent for
>> the following reasons:
>> 1. It would make it a little harder to unintentionally push to the wrong
>> gerrit.
>> 2. Some people might want to configure their account profiles
>> differently on each gerrit (something like project watches).
>> 3. It will minimize the potential for leakage.
>> 4. Syncing accounts requires more management overhead.
>>
>> With the above proposals we would replicate the gerrit git repository
>> from review.o.o to review-security.o.o but NOT the gerrit database.
>
> Would that mean that everyone has to specifically create an account in
> the security Gerrit (i.e. log in there once) before they can be
> subscribed to an arbitrary review ? Or just that the accounts are kept
> separate ? It might be inconvenient to have to ask everyone to join
> first, in order to get all the right people available for review help
> later...
>
Correct you would need to log in at least once. I liked this option
because it allows users to setup distinct ssh keys if they wish to
again help prevent accidentally pushing to the wrong gerrit.

Note zaro is out for the next few days so I am doing my best to answer.

Clark