PR#63: Add package signature verification - pungi

pungi

#63 Add package signature verification

Closed 6 years ago by lsedlar. Opened 8 years ago by marmarek.

marmarek/pungi gpgkey into master

Verify downloaded packages

Marek Marczykowski-Górecki • 8 years ago

b5e9499

Set repository gpgkey option

Marek Marczykowski-Górecki • 8 years ago

06a6a83

pungi/gather.py

file modified

+24 -3

		`@@ -281,7 +281,7 @@`

		`def _add_yum_repo(self, name, url, mirrorlist=False, groups=True,`
		`cost=1000, includepkgs=None, excludepkgs=None,`
		`- proxy=None):`
		`+ proxy=None, gpgkey=None):`
		`"""This function adds a repo to the yum object.`
		`name: Name of the repo`
		`url: Full url to the repo`
		`@@ -318,6 +318,10 @@`
		`thisrepo.exclude = excludepkgs`
		`thisrepo.includepkgs = includepkgs`
		`thisrepo.cost = cost`
		`+ if gpgkey:`
		`+ thisrepo.gpgcheck = True`
		`+ thisrepo.gpgkey = yum.parser.varReplace(gpgkey,`
		`+ self.ayum.conf.yumvar)`
ausil commented 8 years ago There is no verification that the key is accesable or readable.
marmarek commented 8 years ago I think yum/dnf will handle that - this would be exactly the same situation as specifying invalid path in `/etc/yum.repos.d/xxx.repo`.
		`# Yum doesn't like proxy being None`
		`if proxy:`
		`thisrepo.proxy = proxy`
		`@@ -349,6 +353,7 @@`
		`yumconf.installroot = os.path.join(self.workdir, 'yumroot')`
		`yumconf.uid = os.geteuid()`
		`yumconf.cache = 0`
		`+ yumconf.assumeyes = True`
		`yumconf.failovermethod = 'priority'`
		`yumconf.deltarpm = 0`
		`yumvars = yum.config._getEnvVar()`
		`@@ -379,7 +384,8 @@`
		`cost=repo.cost,`
		`includepkgs=repo.includepkgs,`
		`excludepkgs=repo.excludepkgs,`
		`- proxy=repo.proxy)`
		`+ proxy=repo.proxy,`
		`+ gpgkey=repo.gpgkey)`
		`else:`
		`self._add_yum_repo(repo.name, repo.baseurl,`
		`mirrorlist=False,`
		`@@ -387,7 +393,8 @@`
		`cost=repo.cost,`
		`includepkgs=repo.includepkgs,`
		`excludepkgs=repo.excludepkgs,`
		`- proxy=repo.proxy)`
		`+ proxy=repo.proxy,`
		`+ gpgkey=repo.gpgkey)`

		`self.logger.info('Getting sacks for arches %s' % self.valid_arches)`
		`self.ayum._getSacks(archlist=self.valid_arches)`
		`@@ -1096,6 +1103,20 @@`
		`sys.exit(1)`

		`for po in polist:`
		`+ # before doing anything with the package, verify its signature`
		`+ result, errmsg = self.ayum.sigCheckPkg(po)`
		`+ if result == 0:`
		`+ # Verified ok, or verify not req'd`
		`+ pass`
		`+ elif result == 1:`
		`+ # keys are provided through kickstart, so treat this as consent`
		`+ # for importing them`
		`+ self.ayum.getKeyForPackage(po, lambda x, y, z: True)`
ausil commented 8 years ago import the key where? pungi makes installation media. It's output does not have a rpmdb that persists
marmarek commented 8 years ago To rpmdb used during downloading packages for the installation media. Yes, that rpmdb itself will not be included in the installation media. But on the other hand, when no key will be imported there, none will be used for package verification... So yes, this means it need to import it during each pungi run.
		`+ else:`
		`+ # Fatal error`
		`+ self.logger.error(errmsg)`
		`+ sys.exit(1)`
		`+`
		`basename = os.path.basename(po.relativepath)`

		`local = po.localPkg()`

no initial comment

marmarek commented 8 years ago

This depends on pykickstart support for "repo --gpgkey" option:
https://github.com/rhinstaller/pykickstart/pull/32

Without this patch, pungi happily use whatever it downloads from the network, even over plain HTTP.

ausil commented on line 61 of pungi/gather.py 8 years ago

import the key where? pungi makes installation media. It's output does not have a rpmdb that persists

ausil commented on line 17 of pungi/gather.py 8 years ago

There is no verification that the key is accesable or readable.

ausil commented 8 years ago

pungi does rely on you feeding it known good data. pungi has nothing to import the gpg key to. I am really not sure what you are wanting here. there is no error checking. importing the same gpg thousands of times seems suboptimal. I think you should perhaps start a mailing list thread to outline what your end goal is so that we can help you.

marmarek commented 8 years ago

This all is about downloading rpm packages from the network. The result will will be used in two places:

Repository stored on installation media (Packages directory)
Lorax - for installer system itself

Without this patch, both places happily will use unverified packages downloaded from the network (for example over plain HTTP).

So are you saying that the only way to build known good image is to create local repository mirror, manually verify each rpm package signature and point pungi to that repo?

marmarek commented on line 61 of pungi/gather.py 8 years ago

To rpmdb used during downloading packages for the installation media. Yes, that rpmdb itself will not be included in the installation media.
But on the other hand, when no key will be imported there, none will be used for package verification... So yes, this means it need to import it during each pungi run.

marmarek commented on line 17 of pungi/gather.py 8 years ago

I think yum/dnf will handle that - this would be exactly the same situation as specifying invalid path in /etc/yum.repos.d/xxx.repo.

ausil commented 8 years ago

This all is about downloading rpm packages from the network. The result will will be used in two places:

Repository stored on installation media (Packages directory)
Lorax - for installer system itself

Without this patch, both places happily will use unverified packages downloaded from the network (for example over plain HTTP).
So are you saying that the only way to build known good image is to create local repository mirror, manually verify each rpm package signature and point pungi to that repo?

I am saying pungi was designed with the Fedora use cases in mind. where we pull the rpms from koji and make a repo and feed that in. it is designed as a distribution compose tool. It is expected that the distribution's using it will have strong controls on the contents taht feed into the compose. We can certainly change but I think you are talking about use cases we have not covered. So we need to figure out the best way forward. you can feed lorax a repo and make a boot.iso without pungi, so lorax will need patching also.

I do not see any reason to import the gpg key at all. and I would like to see verification that the gpg key is accessible.

additionally we will need documentation on how to use this feature.

marmarek commented 8 years ago

I am saying pungi was designed with the Fedora use cases in mind. where we pull the rpms from koji and make a repo and feed that in. it is designed as a distribution compose tool. It is expected that the distribution's using it will have strong controls on the contents taht feed into the compose. We can certainly change but I think you are talking about use cases we have not covered. So we need to figure out the best way forward.

Ok, so anyone who want to build Fedora-based image is out of luck...

you can feed lorax a repo and make a boot.iso without pungi, so lorax will need patching also.

How that lorax retrieve repository configuration in that case?

I do not see any reason to import the gpg key at all.

How to check package signature then? I don't think rpm support that.

and I would like to see verification that the gpg key is accessible.

Sure, can add this if you feel it is necessary here.

additionally we will need documentation on how to use this feature.

What exactly do you have in mind? Pungi uses kickstart as its input, including repository configuration, and that kickstart pull request already contains documentation on the new repo --gpgkey option:

--gpgkey

    Specify GPG key used to verify packages downloaded from this repo. Setting
    a key here enables package verification, for this repository.

This is all somehow unfortunate that repository object is constructed by each kickstart user (not by the pykickstart module itself) - so if any new option is added there, all the users needs to be updated... But I guess there is some reason for that (or a reason for not changing it).

ausil commented 8 years ago

I am saying pungi was designed with the Fedora use cases in mind. where we pull the rpms from koji and make a repo and feed that in. it is designed as a distribution compose tool. It is expected that the distribution's using it will have strong controls on the contents taht feed into the compose. We can certainly change but I think you are talking about use cases we have not covered. So we need to figure out the best way forward.

Ok, so anyone who want to build Fedora-based image is out of luck...

not at all. We just need to know what the use cases are and figure out how to best support them.

you can feed lorax a repo and make a boot.iso without pungi, so lorax will need patching also.

How that lorax retrieve repository configuration in that case?

lorax --help will tell you, you can point it at mirrorlists and urls

I do not see any reason to import the gpg key at all.

How to check package signature then? I don't think rpm support that.

I think it can be supported from in memory, if not we should only import it once, not on every single rpm 100,000+ imports would not be good.

and I would like to see verification that the gpg key is accessible.

Sure, can add this if you feel it is necessary here.

additionally we will need documentation on how to use this feature.

What exactly do you have in mind? Pungi uses kickstart as its input, including repository configuration, and that kickstart pull request already contains documentation on the new repo --gpgkey option:
--gpgkey

pungi does not use just kickstarts as its input, there has been a lot of change in pungi recently and there is two modes of operation. one requires a config and a xml file.

https://ausil.us/pungi/ is a copy of the current documentation that we have. the source for it is in the doc dir

Specify GPG key used to verify packages downloaded from this repo. Setting
a key here enables package verification, for this repository.

This is all somehow unfortunate that repository object is constructed by each kickstart user (not by the pykickstart module itself) - so if any new option is added there, all the users needs to be updated... But I guess there is some reason for that (or a reason for not changing it).

you would need to ask the pykickstart guys why this is. Note at some point very soon we will be working to support dnf as well as yum in pungi

marmarek commented 8 years ago

Ok, so anyone who want to build Fedora-based image is out of luck...

not at all. We just need to know what the use cases are and figure out how to best support them.

Ok, so few examples:

Someone want to build own Fedora installation image for whatever reason (some preferred packages included? some kernel patch to enable installation at all on given hardware?). The easiest way would be to get pungi configuration used in official images, make some adjustments and build it. This includes downloading all the packages from the Fedora network repositories (not a local koji), so without signature verification, it may result in getting subverted packages (should I explain terms like MITM, or mirror compromise?).
Take a look here: https://git.fedorahosted.org/cgit/spin-kickstarts.git/tree/
Random file: fedora-install-workstation.ks:

repo --name=rawhide --mirrorlist=http://...
The above actually applies not only to Fedora images itself, but also some distributions based on Fedora/RH - for example Qubes OS.

you can feed lorax a repo and make a boot.iso without pungi, so lorax will need patching also.
How that lorax retrieve repository configuration in that case?

lorax --help will tell you, you can point it at mirrorlists and urls

Ok, so this also needs to be taken care about. But this is totally independent of pungi.

I do not see any reason to import the gpg key at all.
How to check package signature then? I don't think rpm support that.

I think it can be supported from in memory, if not we should only import it once, not on every single rpm 100,000+ imports would not be good.

Ah, there is misunderstanding here. The key is imported only once. This works exactly like yum/dnf (actually this is exactly the same function called here). It tries to verify package signature first, and when it fails because of missing public key, it looks at repository definition, ask the user whether to import the key (this is the callback provided there), imports the key and retry. So for the for the first package the key will be indeed imported, but all the rest of them will have already present in rpm keyring.

additionally we will need documentation on how to use this feature.
What exactly do you have in mind? Pungi uses kickstart as its input, including repository configuration, and that kickstart pull request already contains documentation on the new repo --gpgkey option:
--gpgkey

pungi does not use just kickstarts as its input, there has been a lot of change in pungi recently and there is two modes of operation. one requires a config and a xml file.
https://ausil.us/pungi/ is a copy of the current documentation that we have. the source for it is in the doc dir

Ok, so the package verification (and the key path) needs also some new setting in that config. But I'd treat that as a separate feature.

This is all somehow unfortunate that repository object is constructed by each kickstart user (not by the pykickstart module itself) - so if any new option is added there, all the users needs to be updated... But I guess there is some reason for that (or a reason for not changing it).

you would need to ask the pykickstart guys why this is. Note at some point very soon we will be working to support dnf as well as yum in pungi

dnf implementation in regard of package verification is pretty the same. I've already implemented the same repo --gpgkey for anaconda (with dnf).

Pull-Request has been updated

8 years ago

lsedlar commented 6 years ago

The code path that is updated here will be going away when we move away from Yum to DNF (which should be in relatively close future). If you still want to have this feature, it would have to be ported to the new codebase.