Thread: [Module-build-general] signature verification and some bug fixes
Status: Beta
Brought to you by:
kwilliams
|
From: Dave R. <au...@ur...> - 2003-06-21 04:47:36
|
The patch below my sig does three things: 1. Fix MANIFEST to include INSTALL.txt, not INSTALL 2. Make the distsign action depend on distdir unless $self->dist_dir already exists. Otherwise the sequence of "perl Build.PL", "./Build distsign" fails, which is almost certainly a bug. 3. Adds a verify action which verifies a signature. -dave /*======================= House Absolute Consulting www.houseabsolute.com =======================*/ ? t/Sample/SIGNATURE ? t/Sample/Sample-0.01.tar.gz Index: MANIFEST =================================================================== RCS file: /cvsroot/module-build/Module-Build/MANIFEST,v retrieving revision 1.18 diff -u -r1.18 MANIFEST --- MANIFEST 22 May 2003 17:19:04 -0000 1.18 +++ MANIFEST 21 Jun 2003 04:44:11 -0000 @@ -1,6 +1,6 @@ Build.PL Changes -INSTALL +INSTALL.txt MANIFEST META.yml Makefile.PL Index: lib/Module/Build.pm =================================================================== RCS file: /cvsroot/module-build/Module-Build/lib/Module/Build.pm,v retrieving revision 1.76 diff -u -r1.76 Build.pm --- lib/Module/Build.pm 17 Jun 2003 06:19:13 -0000 1.76 +++ lib/Module/Build.pm 21 Jun 2003 04:44:11 -0000 @@ -141,8 +141,8 @@ distclean skipcheck distdir test distsign testdb - disttest versioninstall - fakeinstall + disttest verify + fakeinstall versioninstall You can run the 'help' action for a complete list of actions. @@ -877,6 +877,10 @@ This is a synonym for the 'test' action with the C<debugger=1> argument. + +=item verify + +Verifies the signatures found in the distribution's SIGNATURE file. =item clean Index: lib/Module/Build/Base.pm =================================================================== RCS file: /cvsroot/module-build/Module-Build/lib/Module/Build/Base.pm,v retrieving revision 1.129 diff -u -r1.129 Base.pm --- lib/Module/Build/Base.pm 17 Jun 2003 19:19:21 -0000 1.129 +++ lib/Module/Build/Base.pm 21 Jun 2003 04:44:16 -0000 @@ -819,6 +819,23 @@ return [sort @tests]; } +sub ACTION_verify { + my ($self) = @_; + + $self->_load_module_signature('verify') or return; + + # We protect the verify with an eval{} to make sure we get back to + # the right directory after a signature failure. + + chdir $self->dist_dir or die "Can't chdir() to " . $self->dist_dir . ": $!"; + my $ok = eval {Module::Signature::verify() == Module::Signature::SIGNATURE_OK()}; + my @err = $@ ? ($@) : (); + chdir $self->base_dir or push @err, "Can't chdir() back to " . $self->base_dir . ": $!"; + die join "\n", @err if @err; + + print "Signature is", ($ok ? "" : " not"), " valid\n"; +} + sub ACTION_testdb { my ($self) = @_; local $self->{properties}{debugger} = 1; @@ -1144,11 +1161,10 @@ sub ACTION_distsign { my ($self) = @_; - - unless (eval { require Module::Signature; 1 }) { - warn "Couldn't load Module::Signature for 'distsign' action:\n $@\n"; - return; - } + + $self->_load_module_signature('distsign') or return; + + $self->depends_on('distdir') unless -d $self->dist_dir; # We protect the signing with an eval{} to make sure we get back to # the right directory after a signature failure. @@ -1160,7 +1176,16 @@ die join "\n", @err if @err; } +sub _load_module_signature { + my ($self, $action) = @_; + unless (eval { require Module::Signature; 1 }) { + warn "Couldn't load Module::Signature for '$action' action:\n $@\n"; + return; + } + + return 1; +} sub ACTION_skipcheck { my ($self) = @_; |
|
From: Ken W. <ke...@ma...> - 2003-06-23 16:22:18
|
I've applied the first one. I'm working kind of slowly at the moment and I'll get to the other two soon. Thanks. -Ken On Friday, June 20, 2003, at 11:46 PM, Dave Rolsky wrote: > The patch below my sig does three things: > > 1. Fix MANIFEST to include INSTALL.txt, not INSTALL > > 2. Make the distsign action depend on distdir unless $self->dist_dir > already exists. Otherwise the sequence of "perl Build.PL", "./Build > distsign" fails, which is almost certainly a bug. > > 3. Adds a verify action which verifies a signature. > > > -dave > > /*======================= > House Absolute Consulting > www.houseabsolute.com > =======================*/ > > ? t/Sample/SIGNATURE > ? t/Sample/Sample-0.01.tar.gz > Index: MANIFEST > =================================================================== > RCS file: /cvsroot/module-build/Module-Build/MANIFEST,v > retrieving revision 1.18 > diff -u -r1.18 MANIFEST > --- MANIFEST 22 May 2003 17:19:04 -0000 1.18 > +++ MANIFEST 21 Jun 2003 04:44:11 -0000 > @@ -1,6 +1,6 @@ > Build.PL > Changes > -INSTALL > +INSTALL.txt > MANIFEST > META.yml > Makefile.PL > Index: lib/Module/Build.pm > =================================================================== > RCS file: /cvsroot/module-build/Module-Build/lib/Module/Build.pm,v > retrieving revision 1.76 > diff -u -r1.76 Build.pm > --- lib/Module/Build.pm 17 Jun 2003 06:19:13 -0000 1.76 > +++ lib/Module/Build.pm 21 Jun 2003 04:44:11 -0000 > @@ -141,8 +141,8 @@ > distclean skipcheck > distdir test > distsign testdb > - disttest versioninstall > - fakeinstall > + disttest verify > + fakeinstall versioninstall > > You can run the 'help' action for a complete list of actions. > > @@ -877,6 +877,10 @@ > > This is a synonym for the 'test' action with the C<debugger=1> > argument. > + > +=item verify > + > +Verifies the signatures found in the distribution's SIGNATURE file. > > =item clean > > Index: lib/Module/Build/Base.pm > =================================================================== > RCS file: /cvsroot/module-build/Module-Build/lib/Module/Build/Base.pm,v > retrieving revision 1.129 > diff -u -r1.129 Base.pm > --- lib/Module/Build/Base.pm 17 Jun 2003 19:19:21 -0000 1.129 > +++ lib/Module/Build/Base.pm 21 Jun 2003 04:44:16 -0000 > @@ -819,6 +819,23 @@ > return [sort @tests]; > } > > +sub ACTION_verify { > + my ($self) = @_; > + > + $self->_load_module_signature('verify') or return; > + > + # We protect the verify with an eval{} to make sure we get back to > + # the right directory after a signature failure. > + > + chdir $self->dist_dir or die "Can't chdir() to " . $self->dist_dir > . ": $!"; > + my $ok = eval {Module::Signature::verify() == > Module::Signature::SIGNATURE_OK()}; > + my @err = $@ ? ($@) : (); > + chdir $self->base_dir or push @err, "Can't chdir() back to " . > $self->base_dir . ": $!"; > + die join "\n", @err if @err; > + > + print "Signature is", ($ok ? "" : " not"), " valid\n"; > +} > + > sub ACTION_testdb { > my ($self) = @_; > local $self->{properties}{debugger} = 1; > @@ -1144,11 +1161,10 @@ > > sub ACTION_distsign { > my ($self) = @_; > - > - unless (eval { require Module::Signature; 1 }) { > - warn "Couldn't load Module::Signature for 'distsign' action:\n > $@\n"; > - return; > - } > + > + $self->_load_module_signature('distsign') or return; > + > + $self->depends_on('distdir') unless -d $self->dist_dir; > > # We protect the signing with an eval{} to make sure we get back to > # the right directory after a signature failure. > @@ -1160,7 +1176,16 @@ > die join "\n", @err if @err; > } > > +sub _load_module_signature { > + my ($self, $action) = @_; > > + unless (eval { require Module::Signature; 1 }) { > + warn "Couldn't load Module::Signature for '$action' action:\n > $@\n"; > + return; > + } > + > + return 1; > +} > > sub ACTION_skipcheck { > my ($self) = @_; |
|
From: Ken W. <ke...@ma...> - 2003-07-03 17:48:37
|
On Friday, June 20, 2003, at 11:46 PM, Dave Rolsky wrote:
>
> 2. Make the distsign action depend on distdir unless $self->dist_dir
> already exists. Otherwise the sequence of "perl Build.PL", "./Build
> distsign" fails, which is almost certainly a bug.
I've applied this fix in a modified form, by increasing the method
granularity. There's now a _sign_dir($dir) method that will do the
signing, and it's invoked by both ACTION_distdir() and
ACTION_distsign(). Still a little clunky, but at least it'll work
correctly in more cases.
>
> 3. Adds a verify action which verifies a signature.
Why does this patch (excerpt below) verify in $self->dist_dir? Seems
like we'd want to verify in the main directory, $self->base_dir.
-Ken
>
> +sub ACTION_verify {
> + my ($self) = @_;
> +
> + $self->_load_module_signature('verify') or return;
> +
> + # We protect the verify with an eval{} to make sure we get back to
> + # the right directory after a signature failure.
> +
> + chdir $self->dist_dir or die "Can't chdir() to " . $self->dist_dir
> . ": $!";
> + my $ok = eval {Module::Signature::verify() ==
> Module::Signature::SIGNATURE_OK()};
> + my @err = $@ ? ($@) : ();
> + chdir $self->base_dir or push @err, "Can't chdir() back to " .
> $self->base_dir . ": $!";
> + die join "\n", @err if @err;
> +
> + print "Signature is", ($ok ? "" : " not"), " valid\n";
> +}
> +
|
|
From: Dave R. <au...@ur...> - 2003-07-03 17:58:58
|
On Thu, 3 Jul 2003, Ken Williams wrote: > > 3. Adds a verify action which verifies a signature. > > Why does this patch (excerpt below) verify in $self->dist_dir? Seems > like we'd want to verify in the main directory, $self->base_dir. Hmm, that's a good question. For the module _author_, verifying should be done in the dist dir, because that's what contains _exactly_ what will be in the final distro. For the installer, verify should be run in the base_dir, I guess. Except, I bet it'll fail because of the presence of the "Build" script. Hmm, I wonder how to handle that. Autrijus? -dave /*======================= House Absolute Consulting www.houseabsolute.com =======================*/ |
|
From: Ken W. <ke...@ma...> - 2003-07-03 19:15:36
|
On Thursday, July 3, 2003, at 12:56 PM, Dave Rolsky wrote: > On Thu, 3 Jul 2003, Ken Williams wrote: > >>> 3. Adds a verify action which verifies a signature. >> >> Why does this patch (excerpt below) verify in $self->dist_dir? Seems >> like we'd want to verify in the main directory, $self->base_dir. > > Hmm, that's a good question. For the module _author_, verifying > should be > done in the dist dir, because that's what contains _exactly_ what will > be > in the final distro. Probably this should just be done automatically after signing, to make sure that the signature just created is actually valid. Or one could just assume it's valid. > For the installer, verify should be run in the > base_dir, I guess. Except, I bet it'll fail because of the presence of > the "Build" script. Extra files will make it fail? -Ken |
|
From: Dave R. <au...@ur...> - 2003-07-03 19:27:37
|
On Thu, 3 Jul 2003, Ken Williams wrote: > > On Thu, 3 Jul 2003, Ken Williams wrote: > > > >>> 3. Adds a verify action which verifies a signature. > >> > >> Why does this patch (excerpt below) verify in $self->dist_dir? Seems > >> like we'd want to verify in the main directory, $self->base_dir. > > > > Hmm, that's a good question. For the module _author_, verifying > > should be > > done in the dist dir, because that's what contains _exactly_ what will > > be > > in the final distro. > > Probably this should just be done automatically after signing, to make > sure that the signature just created is actually valid. Or one could > just assume it's valid. > > > > For the installer, verify should be run in the > > base_dir, I guess. Except, I bet it'll fail because of the presence of > > the "Build" script. > > Extra files will make it fail? Oops, I was wrong. It respects your MANIFEST.SKIP file. But that _would_ need to be included in the released distro, which is probably worth noting somewhere in the docs. -dave /*======================= House Absolute Consulting www.houseabsolute.com =======================*/ |
|
From: Ken W. <ke...@ma...> - 2003-07-03 19:49:16
|
On Thursday, July 3, 2003, at 02:25 PM, Dave Rolsky wrote: > Oops, I was wrong. It respects your MANIFEST.SKIP file. But that > _would_ > need to be included in the released distro, which is probably worth > noting > somewhere in the docs. I think maybe the right thing to do would be to check somewhere in new() for the presence of a SIGNATURE file, and for whether Module::Signature is available, and if so, to attempt to verify the signature. This would (typically) happen before any other files got created, so it should generally work. However, the REAL right time to do it is BEFORE running the Build.PL file, obviously - if you're already running scripts you downloaded off the net, it's a little late to be checking signatures! So I'm thinking we might want to not support this at all, on general principle. -Ken |
|
From: Dave R. <au...@ur...> - 2003-07-04 03:45:04
|
On Thu, 3 Jul 2003, Ken Williams wrote: > > On Thursday, July 3, 2003, at 02:25 PM, Dave Rolsky wrote: > > Oops, I was wrong. It respects your MANIFEST.SKIP file. But that > > _would_ > > need to be included in the released distro, which is probably worth > > noting > > somewhere in the docs. > > I think maybe the right thing to do would be to check somewhere in > new() for the presence of a SIGNATURE file, and for whether > Module::Signature is available, and if so, to attempt to verify the > signature. This would (typically) happen before any other files got > created, so it should generally work. > > However, the REAL right time to do it is BEFORE running the Build.PL > file, obviously - if you're already running scripts you downloaded off > the net, it's a little late to be checking signatures! So I'm thinking > we might want to not support this at all, on general principle. Good point. This should really be integrated in CPAN.pm and CPANPLUS. -dave /*======================= House Absolute Consulting www.houseabsolute.com =======================*/ |