Control: clone -1 -2
Control: reassign -2 dgit
Control: retitle -2 dgit: Incorrect use of Dpkg::Source::Package argument

On Fri, 2020-07-03 at 11:09:40 +0100, Ian Jackson wrote:
> Guillem Jover writes ("Re: Bug#964017: grep-excuses"):
> > On Tue, 2020-06-30 at 14:15:13 +0100, Ian Jackson wrote:
> > > The string "failed to verify signature" is not generated by code in
> > > dgit.  Looking at the code in dgit, I think the error happens here:
> > > 
> > >     my $dp = new Dpkg::Source::Package filename => $dscfn,
> > >         require_valid_signature => $needsig;
> > >     {
> > >         local $SIG{__WARN__} = sub {
> > >             print STDERR $_[0];
> > >             return unless $needsig;
> > >             fail __ "import-dsc signature check failed";
> > >         };
> > >         if (!$dp->is_signed()) {
> > >             warn f_ "%s: warning: importing unsigned .dsc\n", $us;
> > >         } else {
> > >             my $r = $dp->check_signature();
> > >             confess "->check_signature => $r" if $needsig && $r;
> > >         }
> > >     }
> > > 
> > > I think this rather complex code is trying to deal with API
> > > compatibility issues surrounding require_valid_signature etc.  Anyway,
> > > I think the message is generated by the call to
> > > Dpkg::Source::Package::new.  I think that function inserted $0 into
> > > the error message.
> > > 
> > > I don't know why it is verifying the signature.  I think in this
> > > particular test $needsig is 0.  I searched the code for the variable
> > > and the only place dgit sets it trueish is if dgit import-dsc is
> > > told --require-valid-signature.
> > 
> > This error message comes from Dpkg::OpenPGP::verify_signature() called
> > by Dpkg::Source::Package->check_signature(), so if you do not want to
> > verify the signature I guess you'd need to conditionalize that call
> > also with $needsig.
> 
> Sorry, I was confused before.  Yes, I see that it does verify the
> signature even if $needsig is 0.  That's desirable because I still
> want to print a warning if the signature doesn't verify.
> 
> Previously this worked, I think.
> 
> The problem is that, now, check_signature exits the process when the
> signature check fails, rather than returning a falseish value.
> I'm pretty sure this must be a change in src:dpkg, since ci.d.n does
> rerun the dgit test suite for gpg[v] migrations.

Ok, so I've tracked this down, and there is a behavior regression with
the Dpkg::Source::Package module (but not dpkg-source itself), which
due to some refactoring now defaults to require_valid_signature being
«1», I'm fixing this and setting all other supported options to their
implicit old default value.

The other part is that the dgit code, and its test suite seems to have
never passed the correct option to that module, and the new regression
just surfaced that problem now.

The require_valid_signatures needs to be passed within the «options»
hashref, and not as part of the main args. Something like:

   my $dp = Dpkg::Source::Package->new(
       filename => $dscfn,
       options => {
           require_valid_signature => $needsig,
       },
   );

Trying to extract one of the failing source packages (f.ex.
«tests/tmp/import-dsc/mirror/pool/main/example_1.2.dsc») with the buster
version of dpkg-source using --require-valid-signature, shows that
would fail there too.

> > > So I don't know what a "trustedkeys.kbx" file is or why I need one
> > > now.  (dgit's test suite naturally has a set of test keys, so it has
> > > its own idea of the public keys to use for signature verifications.
> > > But this test case should not involve any of that.)
> > 
> > Hmm, I guess I should be passing --homedir to gpg also within the
> > verify_signature(), like I did for the import_key() call. But I'm
> > assuming you are setting GNUPGHOME in the test suite as well, which
> > is what would make gpg look for the trustedkeys db in there.
> 
> Yes, I do pass GNUPGHOME.  But it's quite possible that this test,
> which is not supposed to have a valid signature, has a .dsc with a
> signature whose public key is not available.

So while I'm still doing this out of robustness, I don't think this is
what is causing the problem at hand.

Thanks,
Guillem

Reply via email to