Re: [Reproducible-builds] Preliminary review of dpkg-genbuildinfo

2015-02-16 Thread Hans-Christoph Steiner
Daniel Kahn Gillmor:
 On Fri 2015-02-13 03:36:20 -0500, Hans-Christoph Steiner wrote:
 I think it would be much simpler to just have the single package signature
 that is embedded in the package file itself, like Android APKs and Java JARs.
  Since the package is built reproducibly, anyone who builds it can just copy
 the canonical signature into their copy of the package they just built, and
 it'll match the sha512sum of the signed apt metadata.
 
 It seems like you're saying everyone will be able to agree on which
 signing authority is canonical for any given package.  I'm not
 convinced that's the case.
 
 The big question there is determining the where the canonical
 signature happens.  It seems like it should be the official Debian
 build process, since it is the only process guaranteed to be the same
 
 Even though i'm personally likely to treat Debian as the canonical
 source i care about, i don't want it to be that way.  I would like
 Debian to be able to be a downstream as well as an upstream (see the
 work feeding back into debian from ubuntu, for example); if a .deb
 package can contain an internal signature, and i'm looking at a given
 .deb in isolation on my debian system, i want to see it signed *by
 debian*, not by whoever happened to produce it first.  Otherwise, it's
 not clear to me that the embedded signature is useful to me as an end
 user at all.
 
 Another question is whether dpkg checks whether the signers match when
 upgrading, like the Android model (a package can only be upgraded by
 another package signed by the same key).  This would be nice, but
 seems optional and hard to do in Debian.
 
 Maybe this is the question we need to answer to move the discussion
 forward to make sure we're taking the desire for embedded signatures
 into account when thinking about reproducible .debs: how exactly do we
 expect an embedded signature to be used/evaluated?  by who, and in what
 context?

 I think the .buildinfo file is useful, but for a separate process.  It should
 be the canonical file for running a reproducible build.
 
 I'm not sure what this means.  I'd be very happy if *all* of my debian
 packages were reproducible builds, and i could have a way of verifying
 it.  I'd consider that more valuable than knowing that all my .debs were
 signed by any individual authority.  So if we're really talking about a
 tradeoff between signed buildinfo files and signed packages, i'd
 certainly prefer signed buildinfo files.
 
 But my proposal was an attempt to let people have both, without forcing
 the entire ecosystem to agree on who is a canonical authority for
 package X, without whom a reproducible package is impossible
 
   --dkg

I think this topic is far too vast with far too many dependencies to really
have a useful discussion on without a full time, dedicated team.  Since that
seems highly unlikely in the near future, we need to break it down into chunks
of work that we can achieve with the time and resources we actually have.

So we need to focus on drilling down to what is the simplest useful form of
package signing that will cause the least amount of problems when we decide to
change how package signing works.  This means we get a prototype out as soon
as possible, and we can learn a lot from that. I think that's pretty easy to
do, something like this:

* make dpkg optionally check package sigs, and refuse to install on bad sig
* use apt signing model: signatures verified from the apt key ring
* signing can start happening in the build tools, by the uploader
* start work towards getting the Debian built/apt infrastructure signing

.hc


-- 
PGP fingerprint: 5E61 C878 0F86 295C E17D  8677 9F0F E587 374B BE81
https://pgp.mit.edu/pks/lookup?op=vindexsearch=0x9F0FE587374BBE81

___
Reproducible-builds mailing list
Reproducible-builds@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/reproducible-builds


Re: [Reproducible-builds] Preliminary review of dpkg-genbuildinfo

2015-02-11 Thread Jérémy Bobbio
Hi!

Now back to your comments on the code:

Guillem Jover:
  diff --git a/debian/usertags b/debian/usertags
  index 0fc26f2..0419488 100644
  --- a/debian/usertags
  +++ b/debian/usertags
  @@ -65,6 +65,7 @@ dpkg-checkbuilddeps   [DPKG-CHECKBUILDDEPS]
   dpkg-deb   [DPKG-DEB]
   dpkg-distaddfile   [DPKG-DISTADDFILE]
   dpkg-divert[DPKG-DIVERT]
  +dpkg-genbuildinfo  [DPKG-GENBUILDINFO]
 
 The pseudo-tag is only needed for old tools for legacy reasons. I
 should just do a round of «bts retitle» at some point.

Removed.

   dpkg-genchanges[DPKG-GENCHANGES]
   dpkg-gencontrol[DPKG-GENCONTROL]
   dpkg-gensymbols[DPKG-GENCSYMBOLS]
  diff --git a/man/dpkg-genbuildinfo.1 b/man/dpkg-genbuildinfo.1
  new file mode 100644
  index 000..626bf66
  --- /dev/null
  +++ b/man/dpkg-genbuildinfo.1
  @@ -0,0 +1,89 @@
  +.\ dpkg manual page - dpkg-genbuildinfo(1)
  +.\
  +.\ Copyright © 2015 Jérémy Bobbio lu...@debian.org
 
 This probably needs to be completed with the © from the sources it's
 borrowing from.

Added.

 The changes to man/po/po4a.conf are missing.

Added.

  diff --git a/scripts/Dpkg/Control/FieldsCore.pm 
  b/scripts/Dpkg/Control/FieldsCore.pm
  index 8a5695b..75de185 100644
  --- a/scripts/Dpkg/Control/FieldsCore.pm
  +++ b/scripts/Dpkg/Control/FieldsCore.pm
  @@ -361,6 +373,11 @@ our %FIELD_ORDER = (
   Vcs-Svn Testsuite), field_list_src_dep(), qw(Package-List),
   @checksum_fields, qw(Files)
   ],
  +CTRL_FILE_BUILDINFO() = [
  +qw(Format Build-Architecture Source Binary Architecture Version
  +Build-Environment Build-Path),
  +@checksum_fields,
  +],
 
 Probably pack all «Build-» fields together at the end after the checksum
 ones, and leave Build-Environment as the last of those, as it's the one
 taking the most vertical space.

Done.

   CTRL_FILE_CHANGES() = [
   qw(Format Date Source Binary Binary-Only Built-For-Profiles 
  Architecture
   Version Distribution Urgency Maintainer Changed-By Description
  diff --git a/scripts/dpkg-buildpackage.pl b/scripts/dpkg-buildpackage.pl
  index a3cca87..018f37d 100755
  --- a/scripts/dpkg-buildpackage.pl
  +++ b/scripts/dpkg-buildpackage.pl
  @@ -158,6 +158,7 @@ my $since;
   my $maint;
   my $changedby;
   my $desc;
  +my @buildinfo_opts;
   my @changes_opts;
   my @hook_names = qw(
   init preclean source build binary changes postclean check sign done
 
 The buildinfo hook name is missing in the @hook_names array.

Added.
 
  @@ -567,6 +570,25 @@ if ($include  BUILD_BINARY) {
   withecho(@debian_rules, $buildtarget);
   run_hook('binary', 1);
   withecho(@rootcommand, @debian_rules, $binarytarget);
  +
  +if (-e ../$pv.dsc) {
 
 Why is the buildinfo file tied to whether there's or not a source
 package? I'd say it might make sense to not generate a buildinfo file
 only if we are not building binary packages. To check for that use the
 BUILD_ constants.

I've added this test after seeing build failures with
cross-binutils which calls `dpkg-buildpackage -B` without a
source previously generated:
https://sources.debian.net/src/cross-binutils/0.22/debian/rules/#L53

In my mind, the buildinfo files are tying a given source to binary
packages. So if they can't contain the hash of a .dsc, then they should
not be generated.

If we say buildinfo are of a more general interest, dpkg-genbuildinfo
could skip including the .dsc if there's none available.

The whole block you quoted is conditional `$include  BUILD_BINARY`. Is
there something more that needs to be added to only generate a buildinfo
file only when building binary packages?

  +run_hook('buildinfo', 1);
 
 The hook should not be conditional to the code being executed. The
 second argument should represent that.

Something like the following, then?

run_hook('buildinfo', $include  BUILD_BINARY);

I'm not sure why this has to be different than the 'binary' hook?

  +push @buildinfo_opts, --admindir=$admindir if $admindir;
  +
  +my $buildinfo_path = ../$pva.buildinfo;
  +my $buildinfo = Dpkg::Control-new(type = CTRL_FILE_BUILDINFO);
  +
  +print { *STDERR }  dpkg-genbuildinfo @buildinfo_opts 
  $buildinfo_path\n;
  +
  +open my $buildinfo_fh, '-|', 'dpkg-genbuildinfo', @buildinfo_opts
  +or subprocerr('dpkg-genbuildinfo');
  +$buildinfo-parse($buildinfo_fh, _g('parse buildinfo file'));
  +$buildinfo-save($buildinfo_path);
  +close $buildinfo_fh or subprocerr(_g('dpkg-genbuildinfo'));
 
 There's no need for all this, just call dpkg-genbuildinfo redirecting
 its output to the destination file. The $changes code where this is
 originating from is accessed later on, so that's why it's being parsed.

Changed.
 
  diff --git a/scripts/dpkg-genbuildinfo.pl b/scripts/dpkg-genbuildinfo.pl
  new file mode 100755
  index 000..d7784ee
  --- /dev/null
  +++