Re: Security issues when running gerrit patches on jenkins

----- Original Message ----- > From: "Mike Burns" <mburns(a)redhat.com&gt; > To: "Eyal Edri" <eedri(a)redhat.com&gt; > Cc: "Robert Middleswarth" <robert(a)middleswarth.net&gt;, infra(a)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(a)middleswarth.net&gt; >>> To: infra(a)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"...

> 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 >>>> >>>> >>>> _______________________________________________ >>>> Infra mailing list >>>> Infra(a)ovirt.org >>>> http://lists.ovirt.org/mailman/listinfo/infra >>> _______________________________________________ >>> Infra mailing list >>> Infra(a)ovirt.org >>> http://lists.ovirt.org/mailman/listinfo/infra >>> >> _______________________________________________ >> Infra mailing list >> Infra(a)ovirt.org >> http://lists.ovirt.org/mailman/listinfo/infra > >