diff options
| -rw-r--r-- | apt-pkg/contrib/gpgv.cc | 84 | ||||
| -rwxr-xr-x | test/integration/test-cve-2019-3462-Release.gpg-payload | 43 | ||||
| -rwxr-xr-x | test/integration/test-method-gpgv | 48 |
3 files changed, 139 insertions, 36 deletions
diff --git a/apt-pkg/contrib/gpgv.cc b/apt-pkg/contrib/gpgv.cc index f6594e62e..be71b1eed 100644 --- a/apt-pkg/contrib/gpgv.cc +++ b/apt-pkg/contrib/gpgv.cc @@ -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) { diff --git a/test/integration/test-cve-2019-3462-Release.gpg-payload b/test/integration/test-cve-2019-3462-Release.gpg-payload new file mode 100755 index 000000000..fd0f96713 --- /dev/null +++ b/test/integration/test-cve-2019-3462-Release.gpg-payload @@ -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 diff --git a/test/integration/test-method-gpgv b/test/integration/test-method-gpgv index 70521881d..bfa5af4c2 100755 --- a/test/integration/test-method-gpgv +++ b/test/integration/test-method-gpgv @@ -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' |