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

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

The following reply was made to PR pkg/48194; it has been noted by GNATS.

From: Alistair Crooks <agc%pkgsrc.org@localhost>
To: Pierre Pronchery <khorben%netbsd.org@localhost>
Cc: gnats-bugs%netbsd.org@localhost, pkg-manager%netbsd.org@localhost, 
pkgsrc-bugs%netbsd.org@localhost
Subject: Re: pkg/48194: Fixing signed packages in pkg_install and pkgsrc
Date: Wed, 11 Sep 2013 05:37:41 +0200

 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