Re: pkg/48194: Fixing signed packages in pkg_install and pkgsrc

[Alistair Crooks <agc%pkgsrc.org@localhost>]

On Tue, Sep 10, 2013 at 06:25:52PM +0200, Pierre Pronchery wrote:
>                       Hi there,
> 
> On 09/09/2013 05:45, Alistair Crooks wrote:
> > 
> >  On Sun, Sep 08, 2013 at 11:30:00PM +0000, Pierre Pronchery wrote:
> >  > >Description:
> >  > pkgsrc has been supporting signed packages since 2001, with mechanisms
> >  > based on either GPG keys or X509 certificates. pkg_add(1) may however
> >  > fail at installing such packages in some conditions, due to
> >  > uninitialized variables in the code used to extract the package signed
> >  > from its container.
> >  
> >  These aren't GPG signatures, they're PGP signatures. gnupg is just one
> >  implementation of PGP.
> 
> Is it really so bad to call them GPG signatures and keys? Shouldn't we
> even say "OpenPGP" then instead? In the context of the GPG
> implementation, there are keys and signatures too - hopefully in
> compliance with the standard.

Not sure what you mean about keys and signatures being in compliance
with the standard - if they aren't compliant, they won't work.

As for the names, we don't "gcc" something, we "compile" it, and maybe
not even with gcc.  We don't write a gawk script, we write an awk one.

I'm also (violently) against basing anything on gpg's command line
interface.  Purely because I don't want to lose my lunch any time
soon.
 
> Anyway, I used "GPG" in the patch to be consistent with the existing
> options from pkg_admin(1) and pkg_install.conf(5), which expect an
> implementation of PGP/GPG to be command-line compatible with gnupg.

I think it should be changed from GPG to OpenPGP, as you suggest.
 
> >  > >How-To-Repeat:
> >  > This example uses a GPG key, which has to be generated beforehand.
> >  > 
> >  > Configure pkg_install:
> >  > $ cat /etc/pkg_install.conf
> >  > GPG=/home/khorben/bin/gpg
> >  > GPG_SIGN_AS=root%edgebsd.org@localhost
> >  > VERIFIED_INSTALLATION=always
> >  > 
> >  > Sign a package:
> >  > $ mkdir signed
> >  > $ pkg_admin gpg-sign-package digest-20121220.tgz 
> > signed/digest-20121220.tgz
> >  > 
> >  > Try to install the resulting package:
> >  > $ pkg_add -v signed/digest-20121220.tgz
> >  > gpg: Signature made Sun Sep  8 03:32:11 2013 UTC using RSA key ID 
> > 6F3AF5E2
> >  > gpg: Good signature from "EdgeBSD packages <root%edgebsd.org@localhost>"
> >  > pkg_add: 1 package addition failed
> >  > 
> >  > >Fix:
> >  > 
> >  > X-Git-Url: 
> > http://git.edgebsd.org/gitweb/?p=edgebsd-pkgsrc.git;a=commitdiff_plain;h=1a4a18342a5d49ce9a93ab0689b4aa04dfc40847
> >  > 
> >  > Fixed installation of signed packages (uninitialized variables)
> >  > ---
> >  > 
> >  > diff --git a/pkgtools/pkg_install/files/lib/pkg_signature.c 
> > b/pkgtools/pkg_install/files/lib/pkg_signature.c
> >  > index 089234e..5e837be 100644
> >  > --- a/pkgtools/pkg_install/files/lib/pkg_signature.c
> >  > +++ b/pkgtools/pkg_install/files/lib/pkg_signature.c
> >  > @@ -326,6 +326,9 @@ pkg_verify_signature(const char *archive_name, 
> > struct archive **archive,
> >  >          *pkgname = NULL;
> >  >  
> >  >          state = xmalloc(sizeof(*state));
> >  > +        state->sign_block_len = 0;
> >  > +        state->sign_block_number = 0;
> >  > +        state->sign_cur_block = 0;
> >  >          state->sign_blocks = NULL;
> >  >          state->sign_buf = NULL;
> >  >          state->archive = NULL;
> >  
> >  I'd be mode inclined to initialise with:
> >  
> >     state = xcalloc(1, sizeof(*state));
> >  
> >  and avoid all the explicit initialisations. Scales better.
> 
> Done; the new fix is attached here (for pkg_install in pkgsrc only first).

Yeah, that's great, please commit.

Thanks!

Alistair