Browse Source

Fail on non-signature lines in Release.gpg

The exploit for CVE-2019-3462 uses the fact that a Release.gpg file can
contain additional content beside the expected detached signature(s).
We were passing the file unchecked to gpgv which ignores these extras
without complains, so we reuse the same line-reading implementation we
use for InRelease splitting to detect if a Release.gpg file contains
unexpected data and fail in this case given that we in the previous
commit we established that we fail in the similar InRelease case now.
tags/debian/1.8.0_rc1
David Kalnischkies 2 years ago
parent
commit
e2965b0b6b
3 changed files with 139 additions and 36 deletions
  1. +64
    -20
      apt-pkg/contrib/gpgv.cc
  2. +43
    -0
      test/integration/test-cve-2019-3462-Release.gpg-payload
  3. +32
    -16
      test/integration/test-method-gpgv

+ 64
- 20
apt-pkg/contrib/gpgv.cc View File

@@ -27,6 +27,28 @@

#include <apti18n.h>
/*}}}*/

static bool GetLineErrno(std::unique_ptr<char, decltype(&free)> &buffer, size_t *n, FILE *stream, std::string const &InFile, bool acceptEoF = false)/*{{{*/
{
errno = 0;
auto lineptr = buffer.release();
auto const result = getline(&lineptr, n, stream);
buffer.reset(lineptr);
if (errno != 0)
return _error->Errno("getline", "Could not read from %s", InFile.c_str());
if (result == -1)
{
if (acceptEoF)
return false;
return _error->Error("Splitting of clearsigned file %s failed as it doesn't contain all expected parts", InFile.c_str());
}
// We remove all whitespaces including newline here as
// a) gpgv ignores them for signature
// b) we can write out a \n in code later instead of dealing with \r\n or not
_strrstrip(buffer.get());
return true;
}
/*}}}*/
static char * GenerateTemporaryFileTemplate(const char *basename) /*{{{*/
{
std::string out;
@@ -176,6 +198,48 @@ void ExecGPGV(std::string const &File, std::string const &FileGPG,

if (releaseSignature == DETACHED)
{
std::unique_ptr<FILE, decltype(&fclose)> detached{fopen(FileGPG.c_str(), "r"), &fclose};
if (detached.get() == nullptr)
{
apt_error(std::cerr, statusfd, fd, "Detached signature file '%s' could not be opened", FileGPG.c_str());
local_exit(EINTERNAL);
}
std::unique_ptr<char, decltype(&free)> buf{nullptr, &free};
size_t buf_size = 0;
bool open_signature = false;
bool found_badcontent = false;
size_t found_signatures = 0;
while (GetLineErrno(buf, &buf_size, detached.get(), FileGPG, true))
{
if (open_signature && strcmp(buf.get(), "-----END PGP SIGNATURE-----") == 0)
open_signature = false;
else if (open_signature == false && strcmp(buf.get(), "-----BEGIN PGP SIGNATURE-----") == 0)
{
open_signature = true;
++found_signatures;
}
else if (open_signature == false)
found_badcontent = true;
}
if (found_signatures == 0 && statusfd != -1)
{
// This is not an attack attempt but a file even gpgv would complain about
// likely the result of a paywall which is covered by the gpgv method
auto const errtag = "[GNUPG:] NODATA\n";
FileFd::Write(fd[1], errtag, strlen(errtag));
local_exit(113);
}
else if (found_badcontent)
{
apt_error(std::cerr, statusfd, fd, "Detached signature file '%s' contains lines not belonging to a signature", FileGPG.c_str());
local_exit(112);
}
if (open_signature == true)
{
apt_error(std::cerr, statusfd, fd, "Detached signature file '%s' contains unclosed signatures", FileGPG.c_str());
local_exit(112);
}

Args.push_back(FileGPG.c_str());
Args.push_back(File.c_str());
}
@@ -290,26 +354,6 @@ void ExecGPGV(std::string const &File, std::string const &FileGPG,
}
/*}}}*/
// SplitClearSignedFile - split message into data/signature /*{{{*/
static bool GetLineErrno(std::unique_ptr<char, decltype(&free)> &buffer, size_t *n, FILE *stream, std::string const &InFile, bool acceptEoF = false)
{
errno = 0;
auto lineptr = buffer.release();
auto const result = getline(&lineptr, n, stream);
buffer.reset(lineptr);
if (errno != 0)
return _error->Errno("getline", "Could not read from %s", InFile.c_str());
if (result == -1)
{
if (acceptEoF)
return false;
return _error->Error("Splitting of clearsigned file %s failed as it doesn't contain all expected parts", InFile.c_str());
}
// We remove all whitespaces including newline here as
// a) gpgv ignores them for signature
// b) we can write out a \n in code later instead of dealing with \r\n or not
_strrstrip(buffer.get());
return true;
}
bool SplitClearSignedFile(std::string const &InFile, FileFd * const ContentFile,
std::vector<std::string> * const ContentHeader, FileFd * const SignatureFile)
{


+ 43
- 0
test/integration/test-cve-2019-3462-Release.gpg-payload View File

@@ -0,0 +1,43 @@
#!/bin/sh
set -e

# This is not covered by the CVE and harmless by itself, but used in
# the exploit and while harmless it is also pointless to allow it

TESTDIR="$(readlink -f "$(dirname "$0")")"
. "$TESTDIR/framework"

setupenvironment
configarchitecture 'amd64'

export APT_DONT_SIGN='InRelease'

insertpackage 'unstable' 'foo' 'all' '1'
setupaptarchive
rm -rf rootdir/var/lib/apt/lists

verify() {
testfailure apt update
testsuccess grep '^ Detached signature file' rootdir/tmp/testfailure.output
testfailure apt show foo
}

msgmsg 'Payload after detached signature'
find aptarchive -name 'Release.gpg' | while read FILE; do
cp -a "$FILE" "${FILE}.bak"
echo "evil payload" >> "$FILE"
done
verify

msgmsg 'Payload in-between detached signatures'
find aptarchive -name 'Release.gpg' | while read FILE; do
cat "${FILE}.bak" >> "$FILE"
done
verify

msgmsg 'Payload before detached signature'
find aptarchive -name 'Release.gpg' | while read FILE; do
echo "evil payload" > "$FILE"
cat "${FILE}.bak" >> "$FILE"
done
verify

+ 32
- 16
test/integration/test-method-gpgv View File

@@ -71,44 +71,60 @@ testrun() {
[GNUPG:] VALIDSIG 891CC50E605796A0C6E733F74BC0A39C27CE74F9 2016-09-01 1472742629 0 4 0 1 11 00 891CC50E605796A0C6E733F74BC0A39C27CE74F9'
}

echo 'Test' > message.data
cat >message.sig <<EOF
-----BEGIN PGP SIGNATURE-----

iQFEBAEBCgAuFiEENKjp0Y2zIPNn6OqgWpDRQdusja4FAlhT7+kQHGpvZUBleGFt
cGxlLm9yZwAKCRBakNFB26yNrjvEB/9/e3jA1l0fvPafx9LEXcH8CLpUFQK7ra9l
3M4YAH4JKQlTG1be7ixruBRlCTh3YiSs66fKMeJeUYoxA2HPhvbGFEjQFAxunEYg
X/LBKv1mQWa+Q34P5GBjK8kQdLCN+yJAiUErmWNQG3GPninrxsC9tY5jcWvHeP1k
V7N3MLnNqzXaCJM24mnKidC5IDadUdQ8qC8c3rjUexQ8vBz0eucH56jbqV5oOcvx
pjlW965dCPIf3OI8q6J7bIOjyY+u/PTcVlqPq3TUz/ti6RkVbKpLH0D4ll3lUTns
JQt/+gJCPxHUJphy8sccBKhW29CLELJIIafvU30E1nWn9szh2Xjq
=TB1F
-----END PGP SIGNATURE-----
EOF


gpgvmethod() {
echo '601 Configuration
echo "601 Configuration
Config-Item: Debug::Acquire::gpgv=1
Config-Item: Dir::Bin::apt-key=./faked-apt-key
Config-Item: APT::Hashes::SHA1::Weak=true

600 URI Acquire
URI: file:///dev/null
Filename: /dev/zero
' | runapt "${METHODSDIR}/gpgv"
URI: file://${TMPWORKINGDIRECTORY}/message.sig
Filename: ${TMPWORKINGDIRECTORY}/message.data
" | runapt "${METHODSDIR}/gpgv"
}
testrun

gpgvmethod() {
echo '601 Configuration
echo "601 Configuration
Config-Item: Debug::Acquire::gpgv=1
Config-Item: Dir::Bin::apt-key=./faked-apt-key
Config-Item: APT::Hashes::SHA1::Weak=true

600 URI Acquire
URI: file:///dev/null
Filename: /dev/zero
URI: file://${TMPWORKINGDIRECTORY}/message.sig
Filename: ${TMPWORKINGDIRECTORY}/message.data
Signed-By: /dev/null,34A8E9D18DB320F367E8EAA05A90D141DBAC8DAE
' | runapt "${METHODSDIR}/gpgv"
" | runapt "${METHODSDIR}/gpgv"
}
testrun

gpgvmethod() {
echo '601 Configuration
echo "601 Configuration
Config-Item: Debug::Acquire::gpgv=1
Config-Item: Dir::Bin::apt-key=./faked-apt-key
Config-Item: APT::Hashes::SHA1::Weak=true

600 URI Acquire
URI: file:///dev/null
Filename: /dev/zero
URI: file://${TMPWORKINGDIRECTORY}/message.sig
Filename: ${TMPWORKINGDIRECTORY}/message.data
Signed-By: 34A8E9D18DB320F367E8EAA05A90D141DBAC8DAE,/dev/null
' | runapt "${METHODSDIR}/gpgv"
" | runapt "${METHODSDIR}/gpgv"
}
testrun

@@ -122,16 +138,16 @@ testsuccess grep '^\s\+Good:\s\+$' method.output
testsuccess grep 'verified because the public key is not available: GOODSIG' method.output

gpgvmethod() {
echo '601 Configuration
echo "601 Configuration
Config-Item: Debug::Acquire::gpgv=1
Config-Item: Dir::Bin::apt-key=./faked-apt-key
Config-Item: APT::Hashes::SHA1::Weak=true

600 URI Acquire
URI: file:///dev/null
Filename: /dev/zero
URI: file://${TMPWORKINGDIRECTORY}/message.sig
Filename: ${TMPWORKINGDIRECTORY}/message.data
Signed-By: 34A8E9D18DB320F367E8EAA05A90D141DBAC8DAE!
' | runapt "${METHODSDIR}/gpgv"
" | runapt "${METHODSDIR}/gpgv"
}
testgpgv 'Exact matched subkey signed with long keyid' 'Good: GOODSIG 5A90D141DBAC8DAE' '34A8E9D18DB320F367E8EAA05A90D141DBAC8DAE!' '[GNUPG:] GOODSIG 5A90D141DBAC8DAE Sebastian Subkey <subkey@example.org>
[GNUPG:] VALIDSIG 34A8E9D18DB320F367E8EAA05A90D141DBAC8DAE 2018-08-16 1534459673 0 4 0 1 11 00 4281DEDBD466EAE8C1F4157E5B6896415D44C43E'


Loading…
Cancel
Save