On Thu, Jun 27, 2013 at 02:43:12PM -0300, Lucas Meneghel Rodrigues wrote: > On 06/27/2013 04:41 AM, Michael S. Tsirkin wrote: > >On Wed, Jun 26, 2013 at 09:56:11PM -0300, Lucas Meneghel Rodrigues wrote: > >>Sometimes we want to check qemu from a git tag, that allows > >>us to verify whether the code we are downloading actually > >>comes from the repository owner. > >> > >>Introduce this support to our git helper. If for some reason > >>this verification fails, be merciless and fail the entire > >>build test. > >> > >>CC: Michael S. Tirskin <[email protected]> > >>Signed-off-by: Lucas Meneghel Rodrigues <[email protected]> > >>--- > >> qemu/cfg/build.cfg | 9 +++++++++ > >> virttest/build_helper.py | 39 ++++++++++++++++++++++++++++++++++++++- > >> 2 files changed, 47 insertions(+), 1 deletion(-) > >> > >>diff --git a/qemu/cfg/build.cfg b/qemu/cfg/build.cfg > >>index a5e5333..5fbd1af 100644 > >>--- a/qemu/cfg/build.cfg > >>+++ b/qemu/cfg/build.cfg > >>@@ -42,6 +42,15 @@ variants: > >> # use it to fetch object first from it, and then later from > >> "upstream" > >> # git_repo_qemu_base_uri = /home/user/code/qemu > >> > >>+ > >>+ # QEMU installation from a GIT repo (signed tags) > >>+ #git_repo_qemu_uri = > >>git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git > >>+ #git_repo_qemu_configure_options = --target-list=x86_64-softmmu > >>+ #git_repo_qemu_tag = for_anthony > >>+ # If tag_signed is provided, then we will be strict and verify the > >>+ # tag. If the tag verification fails, the entire build test will > >>fail. > >>+ #git_repo_qemu_tag_signed = pgp.mit.edu:AFBE8E67 > >>+ > >> # SPICE installation from a GIT repo > >> git_repo_spice_uri = git://anongit.freedesktop.org/spice/spice > >> git_repo_spice_common_uri = > >> git://anongit.freedesktop.org/spice/spice-common > >>diff --git a/virttest/build_helper.py b/virttest/build_helper.py > >>index 9e31779..2879908 100644 > >>--- a/virttest/build_helper.py > >>+++ b/virttest/build_helper.py > >>@@ -69,6 +69,26 @@ class GitRepoParamHelper(git.GitRepoHelper): > >> else: > >> logging.debug('Git repo %s commit: %s' % (self.name, > >> self.commit)) > >> > >>+ self.tag = self.params.get('%s_tag' % config_prefix) > >>+ if self.tag is None: > >>+ logging.debug('Git repo %s tag is not set' % self.name) > >>+ else: > >>+ logging.debug('Git repo %s tag: %s' % (self.name, self.tag)) > >>+ > >>+ self.key_id = None > >>+ self.key_server = None > >>+ tag_signed = self.params.get('%s_tag_signed' % config_prefix) > >>+ if tag_signed is None: > >>+ logging.warning('Git repo %s tag is not signed' % self.name) > >>+ logging.warning('This means we will not verify if the key was ' > >>+ 'made by whomever claims to have made it ' > >>+ '(dangerous)') > >>+ else: > >>+ self.key_server, self.key_id = tag_signed.split(":") > >>+ logging.debug('Git repo %s tag %s was signed with GPG key ID > >>%s ' > >>+ 'present on key server %s', self.name, self.tag, > >>+ self.key_id, self.key_server) > >>+ > >> self.cmd = os_dep.command('git') > >> > >> self.recursive = self.params.get('%s_recursive', 'yes') > >>@@ -76,12 +96,29 @@ class GitRepoParamHelper(git.GitRepoHelper): > >> > >> def execute(self): > >> super(GitRepoParamHelper, self).execute() > >>+ > >> cwd = os.path.curdir > >> os.chdir(self.destination_dir) > >>- utils.system('git remote add origin %s' % self.uri) > >>+ utils.system('git remote add origin %s' % self.uri, > >>ignore_status=True) > >> if self.recursive == 'yes': > >> utils.system('git submodule init') > >> utils.system('git submodule update') > >>+ > >>+ if self.tag: > >>+ utils.system('git checkout %s' % self.tag) > >>+ if self.key_server is not None and self.key_id is not None: > >>+ try: > >>+ logging.debug('Downloading GPG key ID %s from key > >>server ' > >>+ '%s', self.key_id, self.key_server) > >>+ utils.system('gpg --batch --keyserver %s --recv-keys > >>%s' % > >>+ (self.key_server, self.key_id)) > >>+ logging.debug('Verifying if tag is actually signed > >>with ' > >>+ 'GPG key ID %s' % self.key_id) > >>+ utils.system('git tag -v %s' % self.tag) > >>+ except error.CmdError: > >>+ raise error.TestError("GPG signature check for git > >>repo " > >>+ "%s failed" % self.name) > >>+ > >> os.chdir(cwd) > >> > > > >There are two big problems with this approach, I think they must be > >fixed before it's workable. > > > >- if the tag is signed by Malicious.Hacker@somewhere > >and you trick gpg into downloading this key, it will still > >happily accept the tag as long as it's in the server. > > Well, correct me if I'm wrong, but isn't the idea here that you > trust key ID XXX that developer YYY asked us to trust? The problem > you mentioned is more of an issue if we have a public testing > service, which we don't yet...
Yes but git tag -v merely checks that the key is in the db. For that to be useful, you need a chain of trust and that's a complex issue. Much easier to just have user specify the trusted keys explicitly. > >- if I run this from my account, it will add all kind of random > >stuff to my .gnupg. > > Ok, that's fair. > > > > >I think what we really want is specifying a list of legal public keys in > >the config file. > >That's really easy and bypasses the whole issue of servers > >an dchain of trust completely: you trust your config. > > > >Here's how I would do it: user does > > > >gpg --export -a "Michael S. Tsirkin <[email protected]>" >> mst.keys > > > >and put the file mst.keys with the config file > > > >Script should set GNUPGHOME to a temporary directory, > >and do > >gpg --import *.keys > > > >Now copy public.key to where your config is. > > > >You should also check commits this way e.g. by > >git show --show-signature. > > OK, thanks for the feedback, I'll work on the ideas. Thanks a lot. _______________________________________________ Virt-test-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/virt-test-devel
