Bug#696234: apt: Signed Debian control block parsing can be fooled

[Guillem Jover <guillem@debian.org>]

Hi!

On Thu, 2013-03-14 at 18:17:52 +0100, David Kalnischkies wrote:
> On Tue, Dec 18, 2012 at 2:33 PM, Guillem Jover <guillem@debian.org> wrote:
> > The SigVerify::RunGPGV() function is too strict and will error out on
> > correct Armor Header Lines (as per RFC4880), those with trailing
> > whitespace. The function SourcesWriter::DoPackage() will not correctly
> > strip the PGP signature from the dsc if the Armor Header Line contains
> > trailing whitespace, it does not correctly handle OpenPGP blank lines
> > (those with only whitespaces), or surrounding non-signed "garbage".
> 
> Could you point me to the section allowing this?
> 
> Paragraph 7. sounds like the header should be only the quoted text and
> in general reduces the featureset given in 6.2 while the last sentence of
> 7.1. suggests that trailing whitespaces are forbidden on any line in the
> clear-signed message as they are removed from the unsigned message
> at signature generation time.

I think you might have been confused by the nomenclature, and my lack
of more explicit detail, perhaps? Or I missunderstood your comment.
When I was talking about "Armor Header Line" I referred to lines such
as:

   "-----BEGIN PGP SIGNATURE-----\n"

The "Armor Headers" are things like "Hash: SHA1" just immediately after
the "Armor Header Line".

As you say, 6.2 defines these, I'll copy paste some excerpts:

,---
An Armor Header Line consists of the appropriate header line text
surrounded by five (5) dashes ('-', 0x2D) on either side of the
header line text. [...]
`---

,---
Note that all these Armor Header Lines are to consist of a complete
line.  That is to say, there is always a line ending preceding the
starting five dashes, and following the ending five dashes.  The
header lines, therefore, MUST start at the beginning of a line, and
MUST NOT have text other than whitespace following them on the same
line.  These line endings are considered a part of the Armor Header
Line for the purposes of determining the content they delimit.  This
is particularly important when computing a cleartext signature (see
below).
`---

So (from what I wrote on the initial bug report) SigVerify::RunGPGV()
would not be able to parse something like:

    "-----BEGIN PGP SIGNATURE-----   \t \n"

And in additiona SourcesWriter::DoPackage() should not be able to
handle an OpenPGP message starting with stuff like:

    "-----BEGIN PGP MESSAGE-----   \t \n"
    "Hash: SHA1\n"
    "       \n"
    "<SIGNED MESSAGE>\n"

> But yeah, gpg(v) seems to indeed allow all this stuff … *shrugs*

The problem is that someone might doctor a message so that apt can
process a part that's not signed, which still verifies fine in gpg(v).

> (I wonder now how someone is supposed to sign a message
>  containing whitespace sourcecode …)

I don't think trailing whitespace is in this context usually (ever?)
significant anyway?

> APT code also doesn't support dash-escaped text so far.

Neither does dpkg, but that should be fine because they only accept
deb822 (?) which only allows message lines starting with fields names
or spaces, and field names cannot start with a dash.

Thanks,
Guillem