Code review comment for ~mirespace/ubuntu/+source/libmail-dkim-perl:reverting-upstream-debian-ed25519-noble-proposed

TL;DR: Checking that amavisd-new or sympa still work ok when dropping this change: OK

All tested against the package in the ppa.

A. Code Insights: No one of the packages calls directly or indirectly the added (and removed here) code.

In both cases, an 'ack ed25519' search doesn't return anything.

Files modified by these MP are

lib/Mail/DKIM/Signature.pm
lib/Mail/DKIM/Verifier.pm
 -- > Modified functions
       _check_and_verify_signature → private, called by finish_body
lib/Mail/DKIM/PublicKey.pm
lib/Mail/DKIM/Algorithm/ed25519_sha256.pm

A.1. Sympa:

sympa
-------------
❯ ack Mail::DKIM::Signature
❯ ack Mail::DKIM::PublicKey
❯ ack Mail::DKIM::Verifier
cpanfile
182:recommends 'Mail::DKIM::Verifier', '>= 0.37';
272:feature 'Mail::DKIM::Verifier', 'Required in order to use DKIM features (both for signature verification and signature insertion).' => sub {
273: requires 'Mail::DKIM::Verifier', '>= 0.37';

src/lib/Sympa/Message.pm
648: eval 'use Mail::DKIM::Verifier';
655: return unless $Mail::DKIM::Verifier::VERSION;
668: unless ($dkim = Mail::DKIM::Verifier->new()) {
669: $log->syslog('err', 'Could not create Mail::DKIM::Verifier');

Checking Algorithm used is rsa:

❯ ack -C2 Algorithm
src/lib/Sympa/Message.pm
502- # create a signer object
503- my $dkim = Mail::DKIM::Signer->new(
504: Algorithm => "rsa-sha256",
505- Method => "relaxed",
506- Domain => $dkim_d,
--
600- # create a signer object
601- my $arc = Mail::DKIM::ARC::Signer->new(
602: Algorithm => "rsa-sha256",
603- Chain => $arc_cv,
604- SrvId => $arc_srvid,

A.2. amavisd-new:

amavisd-new
-------------------

❯ ack Mail::DKIM::Signature
lib/Amavis/Tools.pm
116: $dkim->add_signature( Mail::DKIM::Signature->new(

lib/Amavis/DKIM.pm
27:use Mail::DKIM::Signature;
336:# returning them as a list of Mail::DKIM::Signature objects
694: $dkim->add_signature( Mail::DKIM::Signature->new(
838: # map a Mail::DKIM::Signature result into an RFC 7601 result value
❯ ack Mail::DKIM::PublicKey
❯ ack Mail::DKIM::Verifier
lib/Amavis.pm
900: Net::Patricia Net::LDAP Mail::SpamAssassin Mail::DKIM::Verifier
6221: if (!defined $dns_resolver && Mail::DKIM::Verifier->VERSION >= 0.40) {
6223: # of Mail::DKIM::Verifier; this avoids repeating initializations
6254: $dkim_verifier = Mail::DKIM::Verifier->new;

lib/Amavis/SpamControl/SpamAssassin.pm
123: push(@modules, qw(Mail::DKIM Mail::DKIM::Verifier Net::DNS::Resolver))

lib/Amavis/Tools.pm
139: my $dkim_verifier = Mail::DKIM::Verifier->new;
140: $dkim_verifier or die "Could not create a Mail::DKIM::Verifier object";

lib/Amavis/DKIM.pm
24:use Mail::DKIM::Verifier 0.31;

Checking Algorithm used is rsa:

❯ ack -C2 Algorithm
lib/Amavis/Tools.pm
116- $dkim->add_signature( Mail::DKIM::Signature->new(
117- Selector => $selector_ace, Domain => $domain_ace,
118: Method => 'simple/simple', Algorithm => 'rsa-sha256',
119- Timestamp => int($now), Expiration => int($now)+24*3600, Key => $key,
120- )); under;

lib/Amavis/DKIM.pm
299-
300-# a CustomSigner callback routine passed to Mail::DKIM in place of a key;
301:# the routine will be called by Mail::DKIM::Algorithm::*rsa_sha* routines
302-# instead of calling their own Mail::DKIM::PrivateKey::sign_digest()

B. Tests : We don't have a baseline for testing sympa. For amavisd-new it's Ok (DEP-8 and building tests).

B.1. Sympa

Sympa hasn't got an autopackage DEP-8 tests suite.

It has the build tests marked as 'nocheck', so overrided. I removed this on d/rules to run it and I have to add more dependencies to make it works, because now is:

Test Summary Report
-------------------
t/DataSource_LDAP2.t (Wstat: 512 (exited 2) Tests: 0 Failed: 0)
  Non-zero exit status: 2
  Parse errors: No plan found in TAP output
t/DatabaseManager.t (Wstat: 512 (exited 2) Tests: 1 Failed: 0)
  Non-zero exit status: 2
  Parse errors: No plan found in TAP output
t/LockedFile.t (Wstat: 512 (exited 2) Tests: 0 Failed: 0)
  Non-zero exit status: 2
  Parse errors: No plan found in TAP output
t/Message_smime.t (Wstat: 512 (exited 2) Tests: 0 Failed: 0)
  Non-zero exit status: 2
  Parse errors: No plan found in TAP output
t/Message_urlize.t (Wstat: 512 (exited 2) Tests: 1 Failed: 1)
  Failed test: 1
  Non-zero exit status: 2
  Parse errors: No plan found in TAP output
t/Request_Handler_add+del.t (Wstat: 512 (exited 2) Tests: 0 Failed: 0)
  Non-zero exit status: 2
  Parse errors: No plan found in TAP output
t/Scenario.t (Wstat: 65280 (exited 255) Tests: 1 Failed: 1)
  Failed test: 1
  Non-zero exit status: 255
  Parse errors: No plan found in TAP output
t/WWW_Tools.t (Wstat: 65280 (exited 255) Tests: 1 Failed: 1)
  Failed test: 1
  Non-zero exit status: 255
  Parse errors: No plan found in TAP output
t/compile_scenarios.t (Wstat: 512 (exited 2) Tests: 0 Failed: 0)
  Non-zero exit status: 2
  Parse errors: No plan found in TAP output
Files=24, Tests=546, 45 wallclock secs ( 0.13 usr 0.06 sys + 6.55 cusr 1.29 csys = 8.03 CPU)
Result: FAIL
Failed 9/24 test programs. 3/546 subtests failed.

I found mention of dkim only I'm a regex on Scenario.t and it seems that it's not used in the tests conf per:

dkim_feature off

in t/data/sympa.conf

B.2. Amavisd-new

DEP8 tests:

autopkgtest [14:14:59]: @@@@@@@@@@@@@@@@@@@@ summary
test-config PASS
qemu-system-x86_64: terminating on signal 15 from pid 144670 (/usr/bin/python3)

Build tests:

All tests successful.
Files=67, Tests=216, 8 wallclock secs ( 0.18 usr 0.08 sys + 6.44 cusr 1.81 csys = 8.51 CPU)
Result: PASS

And related to DKIM:

# Amavis::DKIM::CustomSignerTest->constructor
t/Amavis/DKIM/CustomSignerTest.t .........
1..4
ok 1 - use Amavis::DKIM::CustomSigner;
ok 2 - Amavis::DKIM::CustomSigner->can('new')
ok 3 - ... and the constructor should succeed
ok 4 - '... and the object it returns' isa 'Amavis::DKIM::CustomSigner'
ok
#
# Amavis::DKIMTest->empty
t/Amavis/DKIMTest.t ......................
ok 1 - use Amavis::DKIM;
1..1
ok