Browse Source

fail early in http if server answer is too small as well

Failing on too much data is good, but we can do better by checking for
exact filesizes as we know with hashsums how large a file should be, so
if we get a file which has a size we do not expect we can drop it
directly, regardless of if the file is larger or smaller than what we
expect which should catch most cases which would end up as hashsum
errors later now a lot sooner.
tags/debian/1.5_beta2
David Kalnischkies 3 years ago
parent
commit
f2f8e89f08
13 changed files with 98 additions and 64 deletions
  1. +33
    -2
      methods/basehttp.cc
  2. +1
    -1
      methods/curl.cc
  3. +1
    -1
      methods/ftp.cc
  4. +2
    -2
      methods/http.cc
  5. +14
    -0
      test/integration/framework
  6. +20
    -2
      test/integration/test-apt-update-expected-size
  7. +5
    -1
      test/integration/test-apt-update-filesize-mismatch
  8. +3
    -8
      test/integration/test-apt-update-hashsum-mismatch
  9. +10
    -22
      test/integration/test-apt-update-not-modified
  10. +2
    -2
      test/integration/test-apt-update-stale
  11. +2
    -8
      test/integration/test-apt-update-transactions
  12. +1
    -3
      test/integration/test-pdiff-usage
  13. +4
    -12
      test/integration/test-ubuntu-bug-1098738-apt-get-source-md5sum

+ 33
- 2
methods/basehttp.cc View File

@@ -660,8 +660,39 @@ int BaseHttpMethod::Loop()
// so instead we use the size of the biggest item in the queue
Req.MaximumSize = FindMaximumObjectSizeInQueue();

if (Req.HaveContent)
Result = Server->RunData(Req);
if (Req.HaveContent)
{
/* If the server provides Content-Length we can figure out with it if
this satisfies any request we have made so far (in the pipeline).
If not we can kill the connection as whatever file the server is trying
to send to us would be rejected with a hashsum mismatch later or triggers
a maximum size error. We don't run the data to /dev/null as this can be MBs
of junk data we would waste bandwidth on and instead just close the connection
to reopen a fresh one which should be more cost/time efficient */
if (Req.DownloadSize > 0)
{
decltype(Queue->ExpectedHashes.FileSize()) const filesize = Req.StartPos + Req.DownloadSize;
bool found = false;
for (FetchItem const *I = Queue; I != 0 && I != QueueBack; I = I->Next)
{
auto const fs = I->ExpectedHashes.FileSize();
if (fs == 0 || fs == filesize)
{
found = true;
break;
}
}
if (found == false)
{
SetFailReason("MaximumSizeExceeded");
_error->Error(_("File has unexpected size (%llu != %llu). Mirror sync in progress?"),
filesize, Queue->ExpectedHashes.FileSize());
Result = false;
}
}
if (Result)
Result = Server->RunData(Req);
}

/* If the server is sending back sizeless responses then fill in
the size now */


+ 1
- 1
methods/curl.cc View File

@@ -139,7 +139,7 @@ HttpsMethod::write_data(void *buffer, size_t size, size_t nmemb, void *userp)
if (TotalWritten > me->https->Queue->MaximumSize)
{
me->https->SetFailReason("MaximumSizeExceeded");
_error->Error(_("File is larger than expected (%llu > %llu). Mirror sync in progress?"),
_error->Error(_("File has unexpected size (%llu != %llu). Mirror sync in progress?"),
TotalWritten, me->https->Queue->MaximumSize);
return 0;
}


+ 1
- 1
methods/ftp.cc View File

@@ -940,7 +940,7 @@ bool FTPConn::Get(const char *Path,FileFd &To,unsigned long long Resume,
if (MaximumSize > 0 && To.Tell() > MaximumSize)
{
Owner->SetFailReason("MaximumSizeExceeded");
return _error->Error(_("File is larger than expected (%llu > %llu). Mirror sync in progress?"),
return _error->Error(_("File has unexpected size (%llu != %llu). Mirror sync in progress?"),
To.Tell(), MaximumSize);
}
}


+ 2
- 2
methods/http.cc View File

@@ -596,7 +596,7 @@ bool HttpServerState::RunData(RequestState &Req)
if (Req.MaximumSize != 0 && Req.DownloadSize > Req.MaximumSize)
{
Owner->SetFailReason("MaximumSizeExceeded");
return _error->Error(_("File is larger than expected (%llu > %llu). Mirror sync in progress?"),
return _error->Error(_("File has unexpected size (%llu != %llu). Mirror sync in progress?"),
Req.DownloadSize, Req.MaximumSize);
}
In.Limit(Req.DownloadSize);
@@ -837,7 +837,7 @@ bool HttpServerState::Go(bool ToFile, RequestState &Req)
if (Req.MaximumSize > 0 && Req.File.IsOpen() && Req.File.Failed() == false && Req.File.Tell() > Req.MaximumSize)
{
Owner->SetFailReason("MaximumSizeExceeded");
return _error->Error(_("File is larger than expected (%llu > %llu). Mirror sync in progress?"),
return _error->Error(_("File has unexpected size (%llu != %llu). Mirror sync in progress?"),
Req.File.Tell(), Req.MaximumSize);
}



+ 14
- 0
test/integration/framework View File

@@ -2002,6 +2002,20 @@ forallsupportedcompressors() {
done
}

breakfiles() {
while [ -n "$1" ]; do
mv -f "${1}" "${1}.bak"
testsuccess dd if=/dev/zero of="${1}" bs="$(stat -c %s "${1}.bak")" count=1
shift
done
}
unbreakfiles() {
while [ -n "$1" ]; do
mv -f "${1}.bak" "${1}"
shift
done
}

### convenience hacks ###
mkdir() {
# creating some directories by hand is a tedious task, so make it look simple


+ 20
- 2
test/integration/test-apt-update-expected-size View File

@@ -21,7 +21,7 @@ test_inreleasetoobig() {
testsuccess aptget update -o Apt::Get::List-Cleanup=0 -o acquire::MaxReleaseFileSize=$((1*1000*1000)) -o Debug::pkgAcquire::worker=0
msgtest 'Check that the max write warning is triggered'
cp rootdir/tmp/testsuccess.output update.output
testsuccess --nomsg grep -q 'File is larger than expected' update.output
testsuccess --nomsg grep -q 'File has unexpected size' update.output
rm -f update.output
# ensure the failed InRelease file got renamed
testsuccess ls rootdir/var/lib/apt/lists/partial/*InRelease.FAILED
@@ -39,12 +39,30 @@ test_packagestoobig() {
touch -d '+1hour' "$pkg"
done
NEW_SIZE="$(stat --printf=%s aptarchive/dists/unstable/main/binary-i386/Packages.gz)"
testfailuremsg "E: Failed to fetch ${1}/dists/unstable/main/binary-i386/Packages.gz File is larger than expected ($NEW_SIZE > $SIZE). Mirror sync in progress?
testfailuremsg "E: Failed to fetch ${1}/dists/unstable/main/binary-i386/Packages.gz File has unexpected size ($NEW_SIZE != $SIZE). Mirror sync in progress?
E: Some index files failed to download. They have been ignored, or old ones used instead." aptget update -o Debug::pkgAcquire::Worker=1 -o Debug::Acquire::Transaction=0
testsuccess ls rootdir/var/lib/apt/lists/partial/*Packages*.FAILED
testfailure test -e rootdir/var/lib/apt/lists/partial/Old.FAILED
}

test_packagestoosmall() {
insertpackage 'unstable' 'foo' 'i386' '1.0'
buildaptarchivefromfiles '+1 hour'
signreleasefiles
# replace Packages.gz/Packages with short junk
SIZE="$(stat --printf=%s aptarchive/dists/unstable/main/binary-i386/Packages.gz)"
find aptarchive/dists -name 'Packages*' | while read pkg; do
echo "1234567890" > "$pkg"
touch -d '+1hour' "$pkg"
done
NEW_SIZE="$(stat --printf=%s aptarchive/dists/unstable/main/binary-i386/Packages.gz)"
testfailuremsg "E: Failed to fetch ${1}/dists/unstable/main/binary-i386/Packages.gz File is smaller than expected ($NEW_SIZE < $SIZE). Mirror sync in progress?
E: Some index files failed to download. They have been ignored, or old ones used instead." aptget update -o Debug::pkgAcquire::Worker=1 -o Debug::Acquire::Transaction=0
testsuccess ls rootdir/var/lib/apt/lists/partial/*Packages*.FAILED
testfailure test -e rootdir/var/lib/apt/lists/partial/Old.FAILED
}


methodtest() {
# less complicated test setup this way
webserverconfig 'aptwebserver::support::modified-since' 'false' "$1"


+ 5
- 1
test/integration/test-apt-update-filesize-mismatch View File

@@ -40,7 +40,11 @@ for get in $(sed -n 's#^GET /\([^ ]\+\.gz\) HTTP.\+$#\1#p' aptarchive/webserver.

testfailure aptget update -o Debug::pkgAcquire::Worker=1
cp rootdir/tmp/testfailure.output rootdir/tmp/update.output
testsuccess grep -E "$(basename "$COMPRESSFILE" '.gz').*Hash Sum mismatch" rootdir/tmp/update.output
if [ -z "$ext" ]; then
testsuccess grep -E "$(basename "$COMPRESSFILE" '.gz').*Hash Sum mismatch" rootdir/tmp/update.output
else
testsuccess grep -E "$(basename "$COMPRESSFILE" '.gz').*File has unexpected size" rootdir/tmp/update.output
fi
testfailure aptcache show foo
testfailure aptget install foo -s



+ 3
- 8
test/integration/test-apt-update-hashsum-mismatch View File

@@ -15,12 +15,6 @@ insertsource 'testing' 'foo2' 'all' '1'
setupaptarchive --no-update
changetowebserver

echo 'Package: bar
Maintainer: Doctor Evil <evil@example.com>
Description: come to the dark side
' > aptarchive/DoctorEvil
compressfile aptarchive/DoctorEvil

find aptarchive \( -name 'Packages' -o -name 'Sources' -o -name 'Translation-en' \) -delete

testsuccess aptget update
@@ -29,9 +23,8 @@ testsuccess aptget install foo -s

for get in $(sed -n 's#^GET /\([^ ]\+\.gz\) HTTP.\+$#\1#p' aptarchive/webserver.log.client*.log); do
msgmsg 'Test hashsum mismatch with file' "$get"
breakfiles "aptarchive/${get}"
rm -rf rootdir/var/lib/apt/lists
webserverconfig 'aptwebserver::overwrite' ''
webserverconfig "aptwebserver::overwrite::$(printf '%s' "${get}" | sed 's#/#%2F#g' )::filename" '%2FDoctorEvil.gz'

testfailure aptget update
cp rootdir/tmp/testfailure.output rootdir/tmp/update.output
@@ -41,4 +34,6 @@ for get in $(sed -n 's#^GET /\([^ ]\+\.gz\) HTTP.\+$#\1#p' aptarchive/webserver.

testfailure aptcache show bar
testfailure aptget install bar -s

unbreakfiles "aptarchive/${get}"
done

+ 10
- 22
test/integration/test-apt-update-not-modified View File

@@ -37,20 +37,14 @@ Reading package lists..." aptget update
configarchitecture 'amd64' 'i386'
# … but oh noes, hashsum mismatch!
SIZE=$(stat -c '%s' 'aptarchive/dists/unstable/main/binary-amd64/Packages.gz')
mv aptarchive/dists/unstable/main/binary-amd64/Packages.gz aptarchive/dists/unstable/main/binary-amd64/Packages.gz.orig
cat > aptarchive/dists/unstable/main/binary-amd64/Packages <<EOF
Package: thisisbad
Architecture: amd64
Version: 1
EOF
compressfile aptarchive/dists/unstable/main/binary-amd64/Packages
breakfiles aptarchive/dists/unstable/main/binary-amd64/Packages.gz
testfailureequal "Hit:1 $1 unstable InRelease
Get:2 $1 unstable/main amd64 Packages [$SIZE B]
Err:2 $1 unstable/main amd64 Packages
Hash Sum mismatch
Hashes of expected file:
- Filesize:$(stat -c '%s' 'aptarchive/dists/unstable/main/binary-amd64/Packages.gz.orig') [weak]
- SHA256:$(sha256sum 'aptarchive/dists/unstable/main/binary-amd64/Packages.gz.orig' | cut -d' ' -f 1)
- Filesize:$(stat -c '%s' 'aptarchive/dists/unstable/main/binary-amd64/Packages.gz.bak') [weak]
- SHA256:$(sha256sum 'aptarchive/dists/unstable/main/binary-amd64/Packages.gz.bak' | cut -d' ' -f 1)
Hashes of received file:
- SHA256:$(sha256sum 'aptarchive/dists/unstable/main/binary-amd64/Packages.gz' | cut -d' ' -f 1)
- Filesize:$(stat -c '%s' 'aptarchive/dists/unstable/main/binary-amd64/Packages.gz') [weak]
@@ -59,8 +53,8 @@ Err:2 $1 unstable/main amd64 Packages
Reading package lists...
E: Failed to fetch $1/dists/unstable/main/binary-amd64/Packages.gz Hash Sum mismatch
Hashes of expected file:
- Filesize:$(stat -c '%s' 'aptarchive/dists/unstable/main/binary-amd64/Packages.gz.orig') [weak]
- SHA256:$(sha256sum 'aptarchive/dists/unstable/main/binary-amd64/Packages.gz.orig' | cut -d' ' -f 1)
- Filesize:$(stat -c '%s' 'aptarchive/dists/unstable/main/binary-amd64/Packages.gz.bak') [weak]
- SHA256:$(sha256sum 'aptarchive/dists/unstable/main/binary-amd64/Packages.gz.bak' | cut -d' ' -f 1)
Hashes of received file:
- SHA256:$(sha256sum 'aptarchive/dists/unstable/main/binary-amd64/Packages.gz' | cut -d' ' -f 1)
- Filesize:$(stat -c '%s' 'aptarchive/dists/unstable/main/binary-amd64/Packages.gz') [weak]
@@ -111,13 +105,7 @@ Reading package lists..." aptget update
configarchitecture 'amd64' 'i386'
# … but oh noes, hashsum mismatch!
SIZE=$(stat -c '%s' 'aptarchive/dists/unstable/main/binary-amd64/Packages.gz')
mv aptarchive/dists/unstable/main/binary-amd64/Packages.gz aptarchive/dists/unstable/main/binary-amd64/Packages.gz.orig
cat > aptarchive/dists/unstable/main/binary-amd64/Packages <<EOF
Package: thisisbad
Architecture: amd64
Version: 1
EOF
compressfile aptarchive/dists/unstable/main/binary-amd64/Packages
breakfiles 'aptarchive/dists/unstable/main/binary-amd64/Packages.gz'
testfailureequal "Ign:1 $1 unstable InRelease
404 Not Found
Hit:2 $1 unstable Release
@@ -125,8 +113,8 @@ Get:4 $1 unstable/main amd64 Packages [$SIZE B]
Err:4 $1 unstable/main amd64 Packages
Hash Sum mismatch
Hashes of expected file:
- Filesize:$(stat -c '%s' 'aptarchive/dists/unstable/main/binary-amd64/Packages.gz.orig') [weak]
- SHA256:$(sha256sum 'aptarchive/dists/unstable/main/binary-amd64/Packages.gz.orig' | cut -d' ' -f 1)
- Filesize:$(stat -c '%s' 'aptarchive/dists/unstable/main/binary-amd64/Packages.gz.bak') [weak]
- SHA256:$(sha256sum 'aptarchive/dists/unstable/main/binary-amd64/Packages.gz.bak' | cut -d' ' -f 1)
Hashes of received file:
- SHA256:$(sha256sum 'aptarchive/dists/unstable/main/binary-amd64/Packages.gz' | cut -d' ' -f 1)
- Filesize:$(stat -c '%s' 'aptarchive/dists/unstable/main/binary-amd64/Packages.gz') [weak]
@@ -135,8 +123,8 @@ Err:4 $1 unstable/main amd64 Packages
Reading package lists...
E: Failed to fetch $1/dists/unstable/main/binary-amd64/Packages.gz Hash Sum mismatch
Hashes of expected file:
- Filesize:$(stat -c '%s' 'aptarchive/dists/unstable/main/binary-amd64/Packages.gz.orig') [weak]
- SHA256:$(sha256sum 'aptarchive/dists/unstable/main/binary-amd64/Packages.gz.orig' | cut -d' ' -f 1)
- Filesize:$(stat -c '%s' 'aptarchive/dists/unstable/main/binary-amd64/Packages.gz.bak') [weak]
- SHA256:$(sha256sum 'aptarchive/dists/unstable/main/binary-amd64/Packages.gz.bak' | cut -d' ' -f 1)
Hashes of received file:
- SHA256:$(sha256sum 'aptarchive/dists/unstable/main/binary-amd64/Packages.gz' | cut -d' ' -f 1)
- Filesize:$(stat -c '%s' 'aptarchive/dists/unstable/main/binary-amd64/Packages.gz') [weak]


+ 2
- 2
test/integration/test-apt-update-stale View File

@@ -39,6 +39,6 @@ cp -p aptarchive/dists/unstable/main/binary-i386/saved/Packages* \
aptarchive/dists/unstable/main/binary-i386/

# ensure this raises an error
testfailuremsg "E: Failed to fetch http://localhost:${APTHTTPPORT}/dists/unstable/main/binary-i386/Packages.gz Hash Sum mismatch
E: Some index files failed to download. They have been ignored, or old ones used instead." aptget update -o Debug::pkgAcquire::Worker=1 -o Debug::Acquire::http=1
testfailure aptget update -o Debug::pkgAcquire::Worker=1 -o Debug::Acquire::http=1
testsuccess grep 'File has unexpected size' rootdir/tmp/testfailure.output
testfileequal lists.before "$(listcurrentlistsdirectory)"

+ 2
- 8
test/integration/test-apt-update-transactions View File

@@ -16,16 +16,10 @@ insertsource 'unstable' 'foo' 'i386' '1.0'
setupaptarchive --no-update

breakfile() {
mv "${1}" "${1}.bak"
mv "${1}.gz" "${1}.gz.bak"
cat > "$1" <<EOF
Package: bar
EOF
compressfile "$1"
breakfiles "$1" "${1}.gz"
}
restorefile() {
mv "${1}.bak" "$1"
mv "${1}.gz.bak" "${1}.gz"
unbreakfiles "$1" "${1}.gz"
}

testrun() {


+ 1
- 3
test/integration/test-pdiff-usage View File

@@ -298,9 +298,7 @@ SHA256-Patches:
SHA256-Download:
d2a1b33187ed2d248eeae3b1223ea71791ea35f2138a713ed371332a6421f467 197 2010-08-18-2013.28.gz
$(sha256sum "${PATCHFILE}.gz" | cut -d' ' -f 1) $(stat -c%s "${PATCHFILE}.gz") $(basename "${PATCHFILE}.gz")" > "$PATCHINDEX"
# needs to look like a valid command, otherwise the parser will fail before hashes are checked
echo '1d' > "$PATCHFILE"
cat "$PATCHFILE" | gzip > "${PATCHFILE}.gz"
breakfiles "$PATCHFILE" "${PATCHFILE}.gz"
generatereleasefiles '+1hour'
signreleasefiles
testsuccess apt update "$@"


+ 4
- 12
test/integration/test-ubuntu-bug-1098738-apt-get-source-md5sum View File

@@ -216,32 +216,24 @@ testmismatch() {
Need to get 6 B of source archives.
Get:1 http://localhost:${APTHTTPPORT} $1 1.0 (dsc) [2 B]
Err:1 http://localhost:${APTHTTPPORT} $1 1.0 (dsc)
File is larger than expected (3 > 2). Mirror sync in progress?
File has unexpected size (3 != 2). Mirror sync in progress?
Hashes of expected file:
- SHA256:943d3bf22ac661fb0f59bc4ff68cc12b04ff17a838dfcc2537008eb9c7f3770a
- Filesize:2 [weak]
Get:2 http://localhost:${APTHTTPPORT} $1 1.0 (tar) [4 B]
Err:2 http://localhost:${APTHTTPPORT} $1 1.0 (tar)
Hash Sum mismatch
File has unexpected size (3 != 4). Mirror sync in progress?
Hashes of expected file:
- SHA256:90aebae315675cbf04612de4f7d5874850f48e0b8dd82becbeaa47ca93f5ebfb
- Filesize:4 [weak]
Hashes of received file:
- SHA256:90aebae315675cbf04612de4f7d5874850f48e0b8dd82becbeaa47ca93f5ebfb
- Filesize:3 [weak]
Last modification reported: $(lastmodification "aptarchive/${1}_1.0.dsc")
E: Failed to fetch http://localhost:${APTHTTPPORT}/${1}_1.0.dsc File is larger than expected (3 > 2). Mirror sync in progress?
E: Failed to fetch http://localhost:${APTHTTPPORT}/${1}_1.0.dsc File has unexpected size (3 != 2). Mirror sync in progress?
Hashes of expected file:
- SHA256:943d3bf22ac661fb0f59bc4ff68cc12b04ff17a838dfcc2537008eb9c7f3770a
- Filesize:2 [weak]
E: Failed to fetch http://localhost:${APTHTTPPORT}/${1}_1.0.tar.gz Hash Sum mismatch
E: Failed to fetch http://localhost:${APTHTTPPORT}/${1}_1.0.tar.gz File has unexpected size (3 != 4). Mirror sync in progress?
Hashes of expected file:
- SHA256:90aebae315675cbf04612de4f7d5874850f48e0b8dd82becbeaa47ca93f5ebfb
- Filesize:4 [weak]
Hashes of received file:
- SHA256:90aebae315675cbf04612de4f7d5874850f48e0b8dd82becbeaa47ca93f5ebfb
- Filesize:3 [weak]
Last modification reported: $(lastmodification "aptarchive/${1}_1.0.dsc")
E: Failed to fetch some archives."
elif [ "$1" = 'pkg-md5-bad' ]; then
FAILURE="Reading package lists...


Loading…
Cancel
Save