Re: [PATCH 3/3] oe.gpg_sign: support obs-signd

Markus Lehtonen
2016-01-12T16:25:51Z

Hi Mark,

Thank you for your review! Comments below.

On Mon, 2016-01-11 at 10:33 -0600, Mark Hatle wrote:
> On 1/11/16 10:13 AM, Markus Lehtonen wrote:
> > Implement support for remote signing using obs-signd. It is now possible
> > to sign both RPM packages and package feeds with this method. The user
> > just needs to set RPM_GPG_BACKEND and/or PACKAGE_FEED_GPG_BACKEND
> > variables to 'obssign' in the bitbake config. Of course, in addition,
> > one needs to setup the signing server and the configure the 'sign'
> > client command on the build host. The *_PASSPHRASE_FILE settings are not
> > used when the obssign backend is enabled.
> >
> > [YOCTO #8755]
> >
> > Signed-off-by: Markus Lehtonen <markus.lehtonen@linux.intel.com>
> > ---
> > meta/classes/sign_package_feed.bbclass | 5 +++-
> > meta/classes/sign_rpm.bbclass | 5 +++-
> > meta/lib/oe/gpg_sign.py | 48 ++++++++++++++++++++++++++++++++++
> > 3 files changed, 56 insertions(+), 2 deletions(-)
> >
> > diff --git a/meta/classes/sign_package_feed.bbclass b/meta/classes/sign_package_feed.bbclass
> > index d5df8af..953fa85 100644
> > --- a/meta/classes/sign_package_feed.bbclass
> > +++ b/meta/classes/sign_package_feed.bbclass
> > @@ -24,7 +24,10 @@ PACKAGE_FEED_GPG_BACKEND ?= 'local'
> >
> > python () {
> > # Check sanity of configuration
> > - for var in ('PACKAGE_FEED_GPG_NAME', 'PACKAGE_FEED_GPG_PASSPHRASE_FILE'):
> > + required = ['PACKAGE_FEED_GPG_NAME']
> > + if d.getVar('PACKAGE_FEED_GPG_BACKEND', True) != 'obssign':
> > + required.append('PACKAGE_FEED_GPG_PASSPHRASE_FILE')
> > + for var in required:
> > if not d.getVar(var, True):
> > raise_sanity_error("You need to define %s in the config" % var, d)
> >
> > diff --git a/meta/classes/sign_rpm.bbclass b/meta/classes/sign_rpm.bbclass
> > index 8bcabee..8be1c35 100644
> > --- a/meta/classes/sign_rpm.bbclass
> > +++ b/meta/classes/sign_rpm.bbclass
> > @@ -23,7 +23,10 @@ RPM_GPG_BACKEND ?= 'local'
> >
> > python () {
> > # Check configuration
> > - for var in ('RPM_GPG_NAME', 'RPM_GPG_PASSPHRASE_FILE'):
> > + required = ['RPM_GPG_NAME']
> > + if d.getVar('RPM_GPG_BACKEND', True) != 'obssign':
> > + required.append('RPM_GPG_PASSPHRASE_FILE')
> > + for var in required:
> > if not d.getVar(var, True):
> > raise_sanity_error("You need to define %s in the config" % var, d)
> >
> > diff --git a/meta/lib/oe/gpg_sign.py b/meta/lib/oe/gpg_sign.py
> > index 55abad8..d8ab816 100644
> > --- a/meta/lib/oe/gpg_sign.py
> > +++ b/meta/lib/oe/gpg_sign.py
> > @@ -66,11 +66,59 @@ class LocalSigner(object):
> > (input_file, output))
> >
> >
> > +class ObsSigner(object):
> > + """Class for handling signing with obs-signd"""
> > + def __init__(self, keyid):
> > + self.keyid = keyid
> > + self.rpm_bin = bb.utils.which(os.getenv('PATH'), "rpm")
> > +
> > + def export_pubkey(self, output_file):
> > + """Export GPG public key to a file"""
> > + cmd = "sign -u '%s' -p" % self.keyid
> > + status, output = oe.utils.getstatusoutput(cmd)
> > + if status:
> > + raise bb.build.FuncFailed('Failed to export gpg public key (%s): %s' %
> > + (self.keyid, output))
> > + with open(output_file, 'w') as fobj:
> > + fobj.write(output)
> > + fobj.write('\n')
> > +
> > + def sign_rpms(self, files):
> > + """Sign RPM files"""
> > + import pexpect
> > +
> > + # Remove existing signatures
> > + cmd = "%s --delsign %s" % (self.rpm_bin, ' '.join(files))
>
> Why are you removing existing signatures? I believe for many cases this is
> actually incorrect.
>
> RPM (5) has the ability to have an endless number of signatures within a given
> package. The package SHOULD included the internal non-repudiable signature...
>
> (to refresh memory) all RPM 5 packages include an internal non-repudiable
> signature. Think of this as an extended md5sum, sha256sum, etc. It doesn't
> change that a package is 'authentic' in any way (often the purpose of signatures
> like what this code is doing), but instead keeps a high reliability way to sign
> and verify the package is signed properly.
>
> This is used for validation if the system doing the install does not have the
> public key that the package was signed with.
>
> ... as well as one or more repudiable signatures that can be used to verify that
> it's "authentic" in some way. A system could very easily have OSV, OEM, and ISV
> keys install on them. You can program RPM in such a way that it will refused to
> install packages with unknown authentication keys or the non-repudiable key as well.
>
> So, I believe running delsign is wrong. If the obs-signd can't handle ADDING
> signatures to packages, then I'd say it is broken and should be fixed in some
> way -- or at least the signature deletion code should be optional.

Yes, unfortunately this is currently the limitation of obs-signd. It
refuses to sign if there are signatures present in the rpm package.
Using --delsign is "unfortunate" consequence of this and that should've
probably been described in a comment. Making signature deletion a
configurable setting is hopefully a decent resolution for now. I will
send a new version of the patchset later.

Thanks,
Markus

> > + status, output = oe.utils.getstatusoutput(cmd)
> > + if status:
> > + raise bb.build.FuncFailed("Failed to remove RPM signatures: %s" %
> > + output)
> > + # Sign packages
> > + cmd = "sign -u '%s' -r %s" % (self.keyid, ' '.join(files))
> > + status, output = oe.utils.getstatusoutput(cmd)
> > + if status:
> > + raise bb.build.FuncFailed("Failed to sign RPM packages: %s" %
> > + output)
> > +
> > + def detach_sign(self, input_file):
> > + """Create a detached signature of a file"""
> > + cmd = "sign -u '%s' -d %s" % (self.keyid, input_file)
> > + status, output = oe.utils.getstatusoutput(cmd)
> > + if status:
> > + raise bb.build.FuncFailed("Failed to create signature for '%s': %s" %
> > + (input_file, output))
> > +
> > +
> > def get_signer(d, backend, keyid, passphrase_file):
> > """Get signer object for the specified backend"""
> > # Use local signing by default
> > if backend == 'local':
> > return LocalSigner(d, keyid, passphrase_file)
> > + elif backend == 'obssign':
> > + if passphrase_file:
> > + bb.note("GPG passphrase file setting not used when 'obssign' "
> > + "backend is used.")
> > + return ObsSigner(keyid)
> > else:
> > bb.fatal("Unsupported signing backend '%s'" % backend)
> >
> >
>