#944008 apt: SplitClearSignedFile mishandles lines with trailing whitespace

Package:
apt
Source:
apt
Description:
commandline package manager
Submitter:
John Bazik
Date:
2019-11-02 18:03:04 UTC
Severity:
important
#944008#5
Date:
2019-11-01 21:58:06 UTC
From:
To:
Dear Maintainer,

When debmirror splits InRelease files using split_clearsigned_file, it can produce text and signature files that gpgv reports as having a "BAD signature."  Yet gpgv reports "Good signature" for the original InRelease file, by itself.  What I found is that most files work but some do not.  Attached is a standalone split command, using the code from debmirror.  This is what I see when I test the debian-archive wheezy-backports InRelease file:

# md5sum wheezy-inrelease
a3f7caeef19f3e3797ec08748409d413  wheezy-inrelease
# head -n 20 wheezy-inrelease
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Origin: Debian Backports
Label: Debian Backports
Suite: wheezy-backports
Version:
Codename: wheezy-backports
Date: Wed, 24 Jan 2018 08:51:34 UTC
NotAutomatic: yes
ButAutomaticUpgrades: yes
Architectures: amd64 armel armhf i386 ia64 kfreebsd-amd64 kfreebsd-i386 mips mipsel powerpc s390 s390x sparc
Components: main contrib non-free
Description: Backports for the Wheezy Distribution
MD5Sum:
 21206181d8c101b785f51c82820acef7   118763 contrib/Contents-amd64
 85c8255dffc0437f45d71e2e0d27401b     2704 contrib/Contents-amd64.diff/Index
 01c60695e6465dc1a3f2035d7060de57    10211 contrib/Contents-amd64.gz
 01d265b9bcabbad6969c560a69550890    72100 contrib/Contents-armel
 e03cee735398401fedf5b505fdc0cdbc     1720 contrib/Contents-armel.diff/Index
# gpgv --keyring /usr/share/keyrings/debian-archive-keyring.gpg --keyring /usr/share/keyrings/debian-archive-removed-keys.gpg -v wheezy-inrelease
gpgv: armor header: Hash: SHA256
gpgv: original file name=''
gpgv: Signature made Wed 24 Jan 2018 03:51:53 AM EST
gpgv:                using RSA key A1BD8E9D78F7FE5C3E65D8AF8B48AD6246925553
gpgv: Good signature from "Debian Archive Automatic Signing Key (7.0/wheezy) <ftpmaster@debian.org>"
gpgv: textmode signature, digest algorithm SHA256, key algorithm rsa4096
gpgv: Signature made Wed 24 Jan 2018 03:51:53 AM EST
gpgv:                using RSA key 126C0D24BD8A2942CC7DF8AC7638D0442B90D010
gpgv: Good signature from "Debian Archive Automatic Signing Key (8/jessie) <ftpmaster@debian.org>"
gpgv: textmode signature, digest algorithm SHA256, key algorithm rsa4096
# ./split_clearsigned_file wheezy-inrelease
# gpgv --keyring /usr/share/keyrings/debian-archive-keyring.gpg --keyring /usr/share/keyrings/debian-archive-removed-keys.gpg -v wheezy-inrelease-sig wheezy-inrelease-txt
gpgv: Signature made Wed 24 Jan 2018 03:51:53 AM EST
gpgv:                using RSA key A1BD8E9D78F7FE5C3E65D8AF8B48AD6246925553
gpgv: BAD signature from "Debian Archive Automatic Signing Key (7.0/wheezy) <ftpmaster@debian.org>"
gpgv: textmode signature, digest algorithm SHA256, key algorithm rsa4096

It does not always fail in this way.  The jessie-backports InRelease file works fine.

Here's the source I used for split_clearsigned_file:

#!/usr/bin/perl -w
# isolate split_clearsigned_file from debmirror

my $infile = $ARGV[0];
open my $sfd, '>', "$infile-sig" or die "$infile-sig\n";
open my $tfd, '>', "$infile-txt" or die "$infile-txt\n";

split_clearsigned_file($infile, $tfd, $sfd) or die "split failed\n";

# Split a clearsigned message into data and signature.
# Based on the similar SplitClearSignedFile in APT.
sub split_clearsigned_file {
  my ($filename, $content_fh, $signature_fh) = @_;
  my $found_message_start = '';
  my $found_message_end = '';
  my $skip_until_empty_line = '';
  my $found_signature = '';
  my $first_line = 1;
  my $signed_message_not_on_first_line = '';
  my $found_garbage = '';
  open my $handle, "<", $filename or die "can't open $filename: $1";
  while (my $line = <$handle>) {
    $line =~ s/[\n\r]+$//;
    if (not $found_message_start) {
      if ($line eq '-----BEGIN PGP SIGNED MESSAGE-----') {
        $found_message_start = 1;
        $skip_until_empty_line = 1;
      } else {
        $signed_message_not_on_first_line = 1;
        $found_garbage = 1;
      }
    } elsif ($skip_until_empty_line) {
      if ($line eq '') {
        $skip_until_empty_line = '';
      }
    } elsif (not $found_signature) {
      if ($line eq '-----BEGIN PGP SIGNATURE-----') {
        $found_signature = 1;
        $found_message_end = 1;
        print $signature_fh "$line\n";
      } elsif (not $found_message_end) {  # we are in the message block
        # We don't have any fields that need to be dash-escaped, but
        # implementations are free to encode all lines.
        $line =~ s/^- //;
        if ($first_line) {  # first line does not need a newline
          $first_line = '';
        } else {
          print $content_fh "\n";
        }
        print $content_fh $line;
      } else {
        $found_garbage = 1;
      }
    } else {
      print $signature_fh "$line\n";
      if ($line eq '-----END PGP SIGNATURE-----') {
        $found_signature = '';
      }
    }
  }

  $content_fh->flush;
  $signature_fh->flush;

  if ($found_message_start) {
    if ($signed_message_not_on_first_line) {
      die "Clearsigned file '$filename' does not start with a signed message block.\n";
    } elsif ($found_garbage) {
      die "Clearsigned file '$filename' contains unsigned lines or multiple signed message blocks.\n";
    }
  }

  if ($found_signature) {
    die "Signature in file $filename wasn't closed.\n";
  }

  if ($first_line and not $found_message_start and not $found_message_end) {
    # This is an unsigned file, so don't generate an error, but splitting
    # was unsuccessful nonetheless.
    return 0;
  } elsif ($first_line or not $found_message_start or not $found_message_end) {
    # Syntax error.
    die "Splitting of $filename failed as it doesn't contain all expected signature parts.";
  }
  return 1;
}

The system information below is not from the system running debmirror, but it is running buster.

#944008#10
Date:
2019-11-02 18:01:23 UTC
From:
To:
Control: clone -1 -2
Control: reassign -2 apt
Control: retitle -2 apt: SplitClearSignedFile mishandles lines with trailing whitespace
space, in the InRelease file for wheezy-backports.

RFC 4880 section 7.1 says:

   Also, any trailing whitespace -- spaces (0x20) and tabs (0x09) -- at
   the end of any line is removed when the cleartext signature is
   generated.

Remarkable; but we have to cope with it.  Apparently the clearsigning
process is not intended to be reversible.

As the comment notes, I translated the split_clearsigned_file function
from similar code in APT, and as far as I can see by code inspection it
has the same bug.  APT maintainers: I think you need to remove any
trailing space or tab characters from buf before writing it to
ContentFile.  There should be a message posted to #943970 shortly with a
link to my fix in debmirror.

Thanks,