audreyt / module-signature

Module signature file manipulation
http://github.com/audreyt/module-signature
16 stars 28 forks source link

Fails own signature test in 0.75 #7

Closed pghmcfc closed 9 years ago

pghmcfc commented 9 years ago
$ make test TEST_SIGNATURE=1
PERL_DL_NONLAZY=1 "/usr/bin/perl" "-MExtUtils::Command::MM" "-MTest::Harness" "-e" "undef *Test::Harness::Switches; test_harness(0, 'inc', 'blib/lib', 'blib/arch')" t/*.t
WARNING: This key is not certified with a trusted signature!
Primary key fingerprint: 66B2 B78E D1B7 7641 4861  D592 B4B3 DD37 3C35 01A0
Not in MANIFEST: blib/arch/.exists
Not in MANIFEST: blib/arch/auto/Module/Signature/.exists
Not in MANIFEST: blib/bin/.exists
Not in MANIFEST: blib/lib/auto/Module/Signature/.exists
Not in MANIFEST: blib/lib/Module/.exists
Not in MANIFEST: blib/lib/Module/Signature.pm
Not in MANIFEST: blib/man1/.exists
Not in MANIFEST: blib/man1/cpansign.1
Not in MANIFEST: blib/man3/.exists
Not in MANIFEST: blib/man3/Module::Signature.3pm
Not in MANIFEST: blib/script/.exists
Not in MANIFEST: blib/script/cpansign
Not in MANIFEST: Makefile
Not in MANIFEST: pm_to_blib
==> MISMATCHED content between MANIFEST and distribution files! <==

#   Failed test 'Valid signature'
#   at t/0-signature.t line 34.
#          got: -5
#     expected: 0
# Looks like you failed 1 test of 1.
t/0-signature.t .. 
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/1 subtests 
t/1-basic.t ...... ok
t/2-cygwin.t ..... skipped: Cygwin only tests
t/3-verify.t ..... ok

Test Summary Report
-------------------
t/0-signature.t (Wstat: 256 Tests: 1 Failed: 1)
  Failed test:  1
  Non-zero exit status: 1
Files=4, Tests=7,  1 wallclock secs ( 0.03 usr  0.00 sys +  0.62 cusr  0.10 csys =  0.75 CPU)
Result: FAIL
Failed 1/4 test programs. 1/7 subtests failed.
Makefile:796: recipe for target 'test_dynamic' failed
make: *** [test_dynamic] Error 255

Presumably this is due to ignoring MANIFEST.SKIP - what was the reason for making that change?

This is also affecting other dists that use Test-Signature (Net-SSH-Perl for instance).

audreyt commented 9 years ago

Thanks for the report and sorry for the breakage. It's now fixed in 0.76, freshly uploaded to CPAN. :rainbow:

audreyt commented 9 years ago

The reason for ignoring MANIFEST.SKIP (which 0.76 relaxed when TEST_SIGNATURE is set to true), is that an attacker could inject t/CVS.t to a regular CPAN distribution and have it run during make test, bypassing the signature check, because \bCVS\b was part of default skip list.

(John Lightsey)++ for reporting this vulnerability.

pghmcfc commented 9 years ago

Thanks for the rapid response!

Perhaps Test-Signature should also set skip => 1 to avoid the same issue there when TEST_SIGNATURE might not be set?

Whilst I have your attention, perhaps you could consider a couple of other nits I've noticed with this dist:

  1. I previously raised https://rt.cpan.org/Public/Bug/Display.html?id=85466 regarding the licensing of the cpansign script, which appears to contradict what's in the README and Changes files.
  2. t/3-verify.t test 3 requires Andreas J. Koenig's public key (A317C15D), which has to be pulled from a keyserver during the test as it isn't shipped in the distribution; please consider including it as you do with your own and the PAUSE keys.
audreyt commented 9 years ago

Done, done and done! :rainbow:

pghmcfc commented 9 years ago

Thanks again!

Regarding the included public keys, Test-Signature could do with having your key (3C3501A0) shipped for the same reasons.