Security issues when running gerrit patches on jenkins

Wed Jul 18 19:10:08 UTC 2012

----- 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"...

> 
> 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
> 
> 
> 