From 3734cceb44b02ca4d5ee3c6f5cbfe1e12f17cffb Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Wed, 23 Jan 2019 17:47:49 +0100 Subject: Fail instead of warn for unsigned lines in InRelease The warnings were introduced 2 years ago without any reports from the wild about them actually appearing for anyone, so now seems to be an as good time as any to switch them to errors. This allows rewritting the code by failing earlier instead of trying to keep going which makes the diff a bit hard to follow but should help simplifying reasoning about it. References: 6376dfb8dfb99b9d182c2fb13aa34b2ac89805e3 --- test/integration/test-cve-2013-1051-InRelease-parsing | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'test/integration') diff --git a/test/integration/test-cve-2013-1051-InRelease-parsing b/test/integration/test-cve-2013-1051-InRelease-parsing index 6238057c3..1f0cbda04 100755 --- a/test/integration/test-cve-2013-1051-InRelease-parsing +++ b/test/integration/test-cve-2013-1051-InRelease-parsing @@ -46,9 +46,12 @@ touch -d '+1hour' aptarchive/dists/stable/InRelease listcurrentlistsdirectory | sed '/_InRelease/ d' > listsdir.lst msgtest 'apt-get update should ignore unsigned data in the' 'InRelease' testwarningequal "Get:1 http://localhost:${APTHTTPPORT} stable InRelease [$(stat -c%s aptarchive/dists/stable/InRelease) B] +Err:1 http://localhost:${APTHTTPPORT} stable InRelease + Splitting up ${TMPWORKINGDIRECTORY}/rootdir/var/lib/apt/lists/partial/localhost:${APTHTTPPORT}_dists_stable_InRelease into data and signature failed Reading package lists... -W: Clearsigned file '${TMPWORKINGDIRECTORY}/rootdir/var/lib/apt/lists/partial/localhost:${APTHTTPPORT}_dists_stable_InRelease' contains unsigned lines. -W: Clearsigned file '${TMPWORKINGDIRECTORY}/rootdir/var/lib/apt/lists/localhost:${APTHTTPPORT}_dists_stable_InRelease' contains unsigned lines." --nomsg aptget update +W: An error occurred during the signature verification. The repository is not updated and the previous index files will be used. GPG error: http://localhost:${APTHTTPPORT} stable InRelease: Splitting up ${TMPWORKINGDIRECTORY}/rootdir/var/lib/apt/lists/partial/localhost:${APTHTTPPORT}_dists_stable_InRelease into data and signature failed +W: Failed to fetch http://localhost:${APTHTTPPORT}/dists/stable/InRelease Splitting up ${TMPWORKINGDIRECTORY}/rootdir/var/lib/apt/lists/partial/localhost:${APTHTTPPORT}_dists_stable_InRelease into data and signature failed +W: Some index files failed to download. They have been ignored, or old ones used instead." --nomsg aptget update testfileequal './listsdir.lst' "$(listcurrentlistsdirectory | sed '/_InRelease/ d')" # ensure there is no package -- cgit v1.2.3 From e2965b0b6bdd68ffcad0e06d11755412a7e16e50 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Wed, 23 Jan 2019 20:50:29 +0100 Subject: 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. --- .../test-cve-2019-3462-Release.gpg-payload | 43 +++++++++++++++++++ test/integration/test-method-gpgv | 48 ++++++++++++++-------- 2 files changed, 75 insertions(+), 16 deletions(-) create mode 100755 test/integration/test-cve-2019-3462-Release.gpg-payload (limited to 'test/integration') 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 < [GNUPG:] VALIDSIG 34A8E9D18DB320F367E8EAA05A90D141DBAC8DAE 2018-08-16 1534459673 0 4 0 1 11 00 4281DEDBD466EAE8C1F4157E5B6896415D44C43E' -- cgit v1.2.3 From 73e3459689c05cd62f15c29d2faddb0fc215ef5e Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Wed, 23 Jan 2019 22:50:45 +0100 Subject: Merge and reuse tmp file handling across the board Having many rather similar implementations especially if one is exported while others aren't (and the rest of it not factored out at all) seems suboptimal. --- test/integration/test-apt-extracttemplates | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'test/integration') diff --git a/test/integration/test-apt-extracttemplates b/test/integration/test-apt-extracttemplates index 9b07ef79f..a47257cfd 100755 --- a/test/integration/test-apt-extracttemplates +++ b/test/integration/test-apt-extracttemplates @@ -44,6 +44,13 @@ Description: Some bar var testfileequal "$TEMPLATE" "$TEMPLATE_STR" CONFIG=$(cut -f4 -d' ' $OUT) testfileequal "$CONFIG" "$CONFIG_STR" + msgtest 'No extra files or directories in extraction directory' + if [ "$(find ./extracttemplates-out | wc -l)" = '3' ]; then + msgpass + else + msgfail + ls -l ./extracttemplates-out + fi # ensure that the format of the output string has the right number of dots for s in "$CONFIG" "$TEMPLATE"; do -- cgit v1.2.3