Security issues when running gerrit patches on jenkins

Wed Jul 18 19:17:36 UTC 2012

On 07/18/2012 03:10 PM, Eyal Edri wrote:
>
> ----- Original Message -----
>> From: "Mike Burns" <mburns at redhat.com>
>> To: "Eyal Edri" <eedri at redhat.com>
>> Cc: "Robert Middleswarth" <robert at middleswarth.net>, infra at ovirt.org
>> Sent: Wednesday, July 18, 2012 8:45:05 PM
>> Subject: Re: Security issues when running gerrit patches on jenkins
>>
>> On Wed, 2012-07-18 at 13:03 -0400, Eyal Edri wrote:
>>> ----- Original Message -----
>>>> From: "Robert Middleswarth" <robert at middleswarth.net>
>>>> To: infra at ovirt.org
>>>> Sent: Wednesday, July 18, 2012 8:00:44 PM
>>>> Subject: Re: Security issues when running gerrit patches on
>>>> jenkins
>>>>
>>>> On 07/18/2012 10:40 AM, Karsten 'quaid' Wade wrote:
>>>>> -----BEGIN PGP SIGNED MESSAGE-----
>>>>> Hash: SHA1
>>>>>
>>>>> On 07/18/2012 06:20 AM, Dan Kenigsberg wrote:
>>>>>> On Wed, Jul 18, 2012 at 07:05:16AM -0400, Eyal Edri wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> Following last infra meeting, i want to open for discussion
>>>>>>> the
>>>>>>> security issues that may arise if we allow Jenkins to run
>>>>>>> jobs
>>>>>>> (i.e any code) with every gerrit patch.
>>>>>>>
>>>>>>> The problem:
>>>>>>>
>>>>>>> In theory, any user that is registered to gerrit might send a
>>>>>>> patch to any ovirt project. That code might contain malicious
>>>>>>> code, malware, harmfull or just not-related ovirt code that
>>>>>>> he
>>>>>>> wants to use our resources for it. Even though we use limited
>>>>>>> sudo on hosts, we can't be sure an exploit will be used
>>>>>>> against
>>>>>>> one of the jenkins slaves.
>>>>>>>
>>>>>>>
>>>>>>> The proposed solutions:
>>>>>>>
>>>>>>> - black-listing authors (published on ovirt.org?) -
>>>>>>> white-listing
>>>>>>> authors (published on ovirt.org?) - auto approve patch via
>>>>>>> comparing to lastest commits - check if author recent patches
>>>>>>> were approved in the past?
>>>>>>>
>>>>>>> adding dan since he raised this issue when we wanted to add
>>>>>>> vdsm
>>>>>>> gerrit tests.
>>>>>> In my opinion, we can trust anyone who has already contributed
>>>>>> code
>>>>>> to the relevant project. We can even say: someone who
>>>>>> contributed
>>>>>> more than 3 commits over a month ago.
>>>>> This seems like a reasonable approach. Trust people first, and
>>>>> it's
>>>>> fine to have a method to untrust people if the need arises.
>>>>> That
>>>>> shouldn't surprise or disappoint anyone - it's just simple
>>>>> sanity.
>>>>>
>>>>> The alternatives are to build untrust in to the process from
>>>>> the
>>>>> start, which becomes a barrier to getting things done, and
>>>>> perpetuates
>>>>> a culture of untrust.
>>>>>
>>>>> I just remind myself, if someone is going to worm their way in
>>>>> to
>>>>> our
>>>>> trust to run malicious code on our Jenkins instances, there is
>>>>> so
>>>>> much
>>>>> more damage they can do with that trust.
>>>>>
>>>>> Trust is like fertilizer, water, and sunshine in the garden -
>>>>> it
>>>>> makes
>>>>> amazing things grow. :)
>>>> I am on the opposite side of this issue.  Maybe I have been
>>>> attacked
>>>> by
>>>> 1 to many bot's or been a manager when someone I know and trusted
>>>> stole
>>>> from the company.  I need trust to be earned so I +1 on
>>>> whitelist.
>>>>   With
>>>> that said I think getting on the whitelist should be pretty easy.
>>>>   We
>>>> are not talking about blocking there commit's we are talking
>>>> about
>>>> should the automated system run test/code against there patch.  I
>>>> am
>>>> still learning Jekins when using a whitelist is there a way to
>>>> flag
>>>> commits for users not in the list?  I wonder if there is some way
>>>> to
>>>> create a list that someone can go though and whitelist the user
>>>> or
>>>> reject the user for commits not in the whitelist?
>>>>
>>> i've never done this in the past.
>>> i assume we'll need to read the author email/name from the gerrit
>>> patch (before running the code)
>>> wget the whitelist page from ovirt.org and match it..
>>>
>>> or alternatively run git log and search it there...
>>>
>>> if there isn't a match, fail the job before running any code.
>> No, I don't think we want to have failures in the main build job.
>>   What
>> about a separate job that verifies the patch submitter, then triggers
>> the build job.  Would probably need to pass the GERRIT_REFSPEC
>> variable
>> between the 2 builds somehow.
> of course, the main job won't be touched.
> i was talking about a new job that will only be used with gerrit patches.
> and it can fail and give '-1' if the submitter is not authorized.
> i need to check if we can add an 'error msg' to the user with something like
> "Sorry, but you're not authorized to send patch to xxxx project, please send a request to infra/devel list"...
I think the idea was to not run until someone can take a look at the 
patch.  Not reject the patch.

Thanks
Robert

>
>
>> Mike
>>
>>>> Thanks
>>>> Robert
>>>>> - - Karsten
>>>>> - --
>>>>> Karsten 'quaid' Wade, Sr. Analyst - Community Growth
>>>>> http://TheOpenSourceWay.org  .^\  http://community.redhat.com
>>>>> @quaid (identi.ca/twitter/IRC)  \v'  gpg: AD0E0C41
>>>>>
>>>>>
>>>>> -----BEGIN PGP SIGNATURE-----
>>>>> Version: GnuPG v1.4.12 (GNU/Linux)
>>>>> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
>>>>>
>>>>> iD8DBQFQBsrp2ZIOBq0ODEERAlfyAKDiJCl6RLXVQluAw9hsX9Uc4ftzMgCgjH6G
>>>>> 0Ejk6rXviSMbc+oiKVTjMUs=
>>>>> =3Hf2
>>>>> -----END PGP SIGNATURE-----
>>>>> _______________________________________________
>>>>> Infra mailing list
>>>>> Infra at ovirt.org
>>>>> http://lists.ovirt.org/mailman/listinfo/infra
>>>> _______________________________________________
>>>> Infra mailing list
>>>> Infra at ovirt.org
>>>> http://lists.ovirt.org/mailman/listinfo/infra
>>>>
>>> _______________________________________________
>>> Infra mailing list
>>> Infra at ovirt.org
>>> http://lists.ovirt.org/mailman/listinfo/infra
>>
>>