This is an automated email from the git hooks/post-receive script. guillem pushed a commit to branch main in repository dpkg.
View the commit online: https://git.dpkg.org/cgit/dpkg/dpkg.git/commit/?id=99793954b46abaeb63bd766a7ed8227800f358e2 commit 99793954b46abaeb63bd766a7ed8227800f358e2 Author: Guillem Jover <[email protected]> AuthorDate: Tue Nov 8 05:12:32 2022 +0100 Dpkg::OpenPGP: Switch functions to return Dpkg::OpenPGP::ErrorCodes Instead of reporting errors from within the OpenPGP functions, switch to return error codes and let the call sites control how these are shown. This will remove much redundancy once we introduce multiple backends, and makes testing the code easier. --- scripts/Dpkg/OpenPGP.pm | 49 +++++++++++++---------------------------- scripts/Dpkg/Source/Package.pm | 23 ++++++++++++++----- scripts/t/Dpkg_OpenPGP.t | 9 +++++--- scripts/t/Dpkg_Source_Package.t | 8 +++---- 4 files changed, 42 insertions(+), 47 deletions(-) diff --git a/scripts/Dpkg/OpenPGP.pm b/scripts/Dpkg/OpenPGP.pm index 9695b2a09..051affd68 100644 --- a/scripts/Dpkg/OpenPGP.pm +++ b/scripts/Dpkg/OpenPGP.pm @@ -27,6 +27,7 @@ use Dpkg::ErrorHandling; use Dpkg::IPC; use Dpkg::File; use Dpkg::Path qw(find_command); +use Dpkg::OpenPGP::ErrorCodes; our $VERSION = '0.01'; @@ -37,7 +38,6 @@ sub new { my $self = { cmd => $opts{cmd} // 'auto', has_cmd => {}, - require_valid_signature => $opts{require_valid_signature} // 1, }; bless $self, $class; @@ -146,18 +146,22 @@ sub armor { my ($self, $type, $bin, $asc) = @_; my $data = file_slurp($bin); - file_dump($asc, _pgp_armor_data($type, $data)); + my $armor = _pgp_armor_data($type, $data); + return OPENPGP_BAD_DATA unless defined $armor; + file_dump($asc, $armor); - return $asc; + return OPENPGP_OK; } sub dearmor { my ($self, $type, $asc, $bin) = @_; my $armor = file_slurp($asc); - file_dump($bin, _pgp_dearmor_data($type, $armor)); + my $data = _pgp_dearmor_data($type, $armor); + return OPENPGP_BAD_DATA unless defined $data; + file_dump($bin, $data); - return $bin; + return OPENPGP_OK; } sub _gpg_exec @@ -187,6 +191,8 @@ sub _gpg_options_weak_digests { sub _gpg_verify { my ($self, $data, $sig, @certs) = @_; + return OPENPGP_MISSING_CMD unless $self->{has_cmd}{gpgv}; + my $gpg_home = File::Temp->newdir('dpkg-gpg-verify.XXXXXXXX', TMPDIR => 1); my @exec = qw(gpgv); @@ -206,45 +212,20 @@ sub _gpg_verify { push @exec, $data; my $status = $self->_gpg_exec(@exec); - if ($status == 1 or ($status && $self->{require_valid_signature})) { - error(g_('cannot verify signature for %s'), $data); - } elsif ($status) { - warning(g_('cannot verify signature for %s'), $data); - } - - return; + return OPENPGP_NO_SIG if $status; + return OPENPGP_OK; } sub inline_verify { my ($self, $data, @certs) = @_; - if ($self->{has_cmd}{gpgv}) { - $self->_gpg_verify($data, undef, @certs); - } elsif ($self->{require_valid_signature}) { - error(g_('cannot verify inline signature on %s since GnuPG is not installed'), - $data); - } else { - warning(g_('cannot verify inline signature on %s since GnuPG is not installed'), - $data); - } - - return; + return $self->_gpg_verify($data, undef, @certs); } sub verify { my ($self, $data, $sig, @certs) = @_; - if ($self->{has_cmd}{gpgv}) { - $self->_gpg_verify($data, $sig, @certs); - } elsif ($self->{require_valid_signature}) { - error(g_('cannot verify signature on %s since GnuPG is not installed'), - $sig); - } else { - warning(g_('cannot verify signature on %s since GnuPG is not installed'), - $sig); - } - - return; + return $self->_gpg_verify($data, $sig, @certs); } 1; diff --git a/scripts/Dpkg/Source/Package.pm b/scripts/Dpkg/Source/Package.pm index d017aded8..be9f720f1 100644 --- a/scripts/Dpkg/Source/Package.pm +++ b/scripts/Dpkg/Source/Package.pm @@ -213,6 +213,7 @@ sub new { format => Dpkg::Source::Format->new(), options => {}, checksums => Dpkg::Checksums->new(), + openpgp => Dpkg::OpenPGP->new(), }; bless $self, $class; if (exists $args{options}) { @@ -227,9 +228,11 @@ sub new { $self->init_options(); } - $self->{openpgp} = Dpkg::OpenPGP->new( - require_valid_signature => $self->{options}{require_valid_signature}, - ); + if ($self->{options}{require_valid_signature}) { + $self->{report_verify} = \&error; + } else { + $self->{report_verify} = \&warning; + } return $self; } @@ -440,7 +443,7 @@ sub armor_original_tarball_signature { return $asc; } - return $self->{openpgp}->armor('SIGNATURE', $bin, $asc); + return $asc if $self->{openpgp}->armor('SIGNATURE', $bin, $asc) == 0; } return; @@ -468,7 +471,11 @@ sub check_original_tarball_signature { my $datafile = $asc =~ s/\.asc$//r; info(g_('verifying %s'), $asc); - $self->{openpgp}->verify($datafile, $asc, $upstream_key); + my $rc = $self->{openpgp}->verify($datafile, $asc, $upstream_key); + if ($rc) { + $self->{report_verify}->(g_('cannot verify upstream tarball signature for %s: %s'), + $datafile, openpgp_errorcode_to_string($rc)); + } } } @@ -508,7 +515,11 @@ sub check_signature { } } - $self->{openpgp}->inline_verify($dsc, @certs); + my $rc = $self->{openpgp}->inline_verify($dsc, @certs); + if ($rc) { + $self->{report_verify}->(g_('cannot verify inline signature for %s: %s'), + $dsc, openpgp_errorcode_to_string($rc)); + } } sub describe_cmdline_options { diff --git a/scripts/t/Dpkg_OpenPGP.t b/scripts/t/Dpkg_OpenPGP.t index 9b3a36656..f21fa3c4f 100644 --- a/scripts/t/Dpkg_OpenPGP.t +++ b/scripts/t/Dpkg_OpenPGP.t @@ -25,9 +25,10 @@ use Dpkg::ErrorHandling; test_needs_command('gpg'); -plan tests => 5; +plan tests => 8; use_ok('Dpkg::OpenPGP'); +use_ok('Dpkg::OpenPGP::ErrorCodes'); report_options(quiet_warnings => 1); @@ -46,14 +47,16 @@ ok($openpgp->is_armored($reffile), 'file ASCII Armored'); $ascfile = "$tmpdir/data-file.asc"; -$openpgp->armor('ARMORED FILE', $binfile, $ascfile); +ok($openpgp->armor('ARMORED FILE', $binfile, $ascfile) == OPENPGP_OK(), + 'armoring succeeded'); ok(compare($ascfile, $reffile) == 0, 'armor binary file into OpenPGP ASCII Armor'); $reffile = "$datadir/data-file"; $ascfile = "$datadir/data-file.asc"; $binfile = "$tmpdir/data-file"; -$openpgp->dearmor('ARMORED FILE', $ascfile, $binfile); +ok($openpgp->dearmor('ARMORED FILE', $ascfile, $binfile) == OPENPGP_OK(), + 'dearmoring succeeded'); ok(compare($binfile, $reffile) == 0, 'dearmor OpenPGP ASCII Armor into binary file'); # TODO: Add actual test cases. diff --git a/scripts/t/Dpkg_Source_Package.t b/scripts/t/Dpkg_Source_Package.t index 641bc0010..5430f2e8e 100644 --- a/scripts/t/Dpkg_Source_Package.t +++ b/scripts/t/Dpkg_Source_Package.t @@ -42,8 +42,8 @@ is($p->armor_original_tarball_signature("$datadir/nonexistent", $ascfile), undef, 'no conversion of inexistent file'); $ascfile = "$tmpdir/package_1.0.orig.tar.sig2asc"; -is($p->armor_original_tarball_signature("$datadir/package_1.0.orig.tar.sig", $ascfile), - $ascfile, 'conversion from binary sig to armored asc'); +ok(defined $p->armor_original_tarball_signature("$datadir/package_1.0.orig.tar.sig", $ascfile), + 'conversion from binary sig to armored asc'); ok(compare($ascfile, "$datadir/package_1.0.orig.tar.asc") == 0, 'binary signature converted to OpenPGP ASCII Armor'); @@ -51,8 +51,8 @@ ok(compare($ascfile, "$datadir/package_1.0.orig.tar.asc") == 0, # Grab the output messages. eval { $ascfile = "$tmpdir/package_1.0.orig.tar.asc2asc"; - is($p->armor_original_tarball_signature("$datadir/package_1.0.orig.tar.asc", $ascfile), - $ascfile, 'copy instead of converting already armored input'); + ok(defined $p->armor_original_tarball_signature("$datadir/package_1.0.orig.tar.asc", $ascfile), + 'copy instead of converting already armored input'); }; ok(compare($ascfile, "$datadir/package_1.0.orig.tar.asc") == 0, -- Dpkg.Org's dpkg
