]> git.saurik.com Git - apt.git/commitdiff
promote filesize to a hashstring
authorDavid Kalnischkies <david@kalnischkies.de>
Thu, 23 Oct 2014 14:54:00 +0000 (16:54 +0200)
committerDavid Kalnischkies <david@kalnischkies.de>
Fri, 24 Oct 2014 21:54:59 +0000 (23:54 +0200)
It is a very simple hashstring, which is why it isn't contributing to
the usability of a list of them, but it is also trivial to check and
calculate, so it doesn't hurt checking it either as it can combined even
with the simplest other hashes greatly complicate attacks on them as you
suddenly need a same-size hash collision, which is usually a lot harder
to achieve.

apt-pkg/contrib/hashes.cc
apt-pkg/contrib/hashes.h
apt-pkg/indexrecords.cc
ftparchive/cachedb.cc
ftparchive/writer.cc
test/integration/test-apt-update-filesize-mismatch [new file with mode: 0755]
test/integration/test-apt-update-hashsum-mismatch [new file with mode: 0755]
test/integration/test-apt-update-ims
test/interactive-helper/aptwebserver.cc
test/libapt/hashsums_test.cc

index 417982343c68b9945436e78c575a67c3db957696..55180c64232d8704176ddc183218bb1175748c9e 100644 (file)
@@ -29,7 +29,7 @@
 
 const char * HashString::_SupportedHashes[] =
 {
-   "SHA512", "SHA256", "SHA1", "MD5Sum", NULL
+   "SHA512", "SHA256", "SHA1", "MD5Sum", "Checksum-FileSize", NULL
 };
 
 HashString::HashString()
@@ -111,6 +111,8 @@ std::string HashString::GetHashForFile(std::string filename) const      /*{{{*/
       SHA512.AddFD(Fd);
       fileHash = (std::string)SHA512.Result();
    }
+   else if (strcasecmp(Type.c_str(), "Checksum-FileSize") == 0)
+      strprintf(fileHash, "%llu", Fd.FileSize());
    Fd.Close();
 
    return fileHash;
@@ -147,7 +149,13 @@ bool HashStringList::usable() const                                        /*{{{*/
       return false;
    std::string const forcedType = _config->Find("Acquire::ForceHash", "");
    if (forcedType.empty() == true)
-      return true;
+   {
+      // FileSize alone isn't usable
+      for (std::vector<HashString>::const_iterator hs = list.begin(); hs != list.end(); ++hs)
+        if (hs->HashType() != "Checksum-FileSize")
+           return true;
+      return false;
+   }
    return find(forcedType) != NULL;
 }
                                                                        /*}}}*/
@@ -201,6 +209,9 @@ bool HashStringList::VerifyFile(std::string filename) const         /*{{{*/
    HashString const * const hs = find(NULL);
    if (hs == NULL || hs->VerifyFile(filename) == false)
       return false;
+   HashString const * const hsf = find("Checksum-FileSize");
+   if (hsf != NULL && hsf->VerifyFile(filename) == false)
+      return false;
    return true;
 }
                                                                        /*}}}*/
@@ -235,6 +246,14 @@ bool HashStringList::operator!=(HashStringList const &other) const
 }
                                                                        /*}}}*/
 
+// PrivateHashes                                                       /*{{{*/
+class PrivateHashes {
+public:
+   unsigned long long FileSize;
+
+   PrivateHashes() : FileSize(0) {}
+};
+                                                                       /*}}}*/
 // Hashes::Add* - Add the contents of data or FD                       /*{{{*/
 bool Hashes::Add(const unsigned char * const Data,unsigned long long const Size, unsigned int const Hashes)
 {
@@ -254,6 +273,7 @@ bool Hashes::Add(const unsigned char * const Data,unsigned long long const Size,
 #if __GNUC__ >= 4
        #pragma GCC diagnostic pop
 #endif
+   d->FileSize += Size;
    return Res;
 }
 bool Hashes::AddFD(int const Fd,unsigned long long Size, unsigned int const Hashes)
@@ -314,15 +334,17 @@ HashStringList Hashes::GetHashStringList()
 #if __GNUC__ >= 4
        #pragma GCC diagnostic pop
 #endif
+   std::string SizeStr;
+   strprintf(SizeStr, "%llu", d->FileSize);
+   hashes.push_back(HashString("Checksum-FileSize", SizeStr));
    return hashes;
 }
 #if __GNUC__ >= 4
        #pragma GCC diagnostic push
        #pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-       #pragma GCC diagnostic ignored "-Wsuggest-attribute=const"
 #endif
-Hashes::Hashes() {}
-Hashes::~Hashes() {}
+Hashes::Hashes() { d = new PrivateHashes(); }
+Hashes::~Hashes() { delete d; }
 #if __GNUC__ >= 4
        #pragma GCC diagnostic pop
 #endif
index caeba006d04d6a8b465ae613233ee0a607995e54..e2e2138551d62ba6a7cbc387ed9ffd5f9df31562 100644 (file)
@@ -161,10 +161,10 @@ class HashStringList
    std::vector<HashString> list;
 };
 
+class PrivateHashes;
 class Hashes
 {
-   /** \brief dpointer placeholder */
-   void *d;
+   PrivateHashes *d;
 
    public:
    /* those will disappear in the future as it is hard to add new ones this way.
index bf1901e11c4ae947c8e633c831296f465feb3ed0..e1e9ba657f3881a2236e41f24cd36e24e87e8ea3 100644 (file)
@@ -116,6 +116,9 @@ bool indexRecords::Load(const string Filename)                              /*{{{*/
             indexRecords::checkSum *Sum = new indexRecords::checkSum;
             Sum->MetaKeyFilename = Name;
             Sum->Size = Size;
+           std::string SizeStr;
+           strprintf(SizeStr, "%llu", Size);
+           Sum->Hashes.push_back(HashString("Checksum-FileSize", SizeStr));
 #if __GNUC__ >= 4
        #pragma GCC diagnostic push
        #pragma GCC diagnostic ignored "-Wdeprecated-declarations"
index da45eb8d25229b5364fe5be7fee9bedc498b37ba..df5eb1451b1ede4d46821f23ac06858fa68669b8 100644 (file)
@@ -473,6 +473,10 @@ bool CacheDB::GetHashes(bool const GenOnly, unsigned int const DoHashes)
            hex2bytes(CurStat.MD5, hs->HashValue().data(), sizeof(CurStat.MD5));
            CurStat.Flags |= FlMD5;
         }
+        else if (strcasecmp(hs->HashType().c_str(), "Checksum-FileSize") == 0)
+        {
+           // we store it in a different field already
+        }
         else
            return _error->Error("Got unknown unrequested hashtype %s", hs->HashType().c_str());
       }
index db617e92ac9bfd43bd0f9aa3cf98a85023f34acf..ded8715f8b134fb72f3eb9d467a20f62cc67ecda 100644 (file)
@@ -471,6 +471,8 @@ bool PackagesWriter::DoPackage(string FileName)
    {
       if (hs->HashType() == "MD5Sum")
         Changes.push_back(SetTFRewriteData("MD5sum", hs->HashValue().c_str()));
+      else if (hs->HashType() == "Checksum-FileSize")
+        continue;
       else
         Changes.push_back(SetTFRewriteData(hs->HashType().c_str(), hs->HashValue().c_str()));
    }
@@ -785,7 +787,7 @@ bool SourcesWriter::DoPackage(string FileName)
 
          for (HashStringList::const_iterator hs = Db.HashesList.begin(); hs != Db.HashesList.end(); ++hs)
         {
-           if (hs->HashType() == "MD5Sum")
+           if (hs->HashType() == "MD5Sum" || hs->HashType() == "Checksum-FileSize")
               continue;
            char const * fieldname;
            std::ostream * out;
diff --git a/test/integration/test-apt-update-filesize-mismatch b/test/integration/test-apt-update-filesize-mismatch
new file mode 100755 (executable)
index 0000000..8c73c05
--- /dev/null
@@ -0,0 +1,55 @@
+#!/bin/sh
+set -e
+
+TESTDIR=$(readlink -f $(dirname $0))
+. $TESTDIR/framework
+setupenvironment
+configarchitecture 'i386'
+configcompression 'gz'
+
+insertpackage 'testing' 'foo' 'all' '1'
+insertpackage 'testing' 'foo2' 'all' '1'
+insertsource 'testing' 'foo' 'all' '1'
+insertsource 'testing' 'foo2' 'all' '1'
+
+setupaptarchive --no-update
+changetowebserver
+
+find aptarchive \( -name 'Packages' -o -name 'Sources' -o -name 'Translation-en' \) -delete
+for release in $(find aptarchive -name 'Release'); do
+       cp "$release" "${release}.backup"
+done
+
+testsuccess aptget update
+testsuccess aptcache show foo
+testsuccess aptget install foo -s
+
+for get in $(sed -n 's#^GET /\([^ ]\+\.gz\) HTTP.\+$#\1#p' aptarchive/webserver.log); do
+       for ext in '' '.gz'; do
+               COMPRESSFILE="$get"
+               get="${get}${ext}"
+               FILE="$(basename -s '.gz' "$get")"
+               msgmsg 'Test filesize mismatch with file' "$FILE"
+               rm -rf rootdir/var/lib/apt/lists
+
+               for release in $(find aptarchive -name 'Release'); do
+                       SIZE="$(awk "/$FILE\$/ { print \$2; exit }" "${release}.backup")"
+                       sed "s# $SIZE # $(($SIZE + 111)) #" "${release}.backup" > "$release"
+               done
+               signreleasefiles
+
+               TEST='testfailure'
+               if expr match "$COMPRESSFILE" '^.*Translation-.*$' >/dev/null; then
+                       TEST='testsuccess'
+                       unset COMPRESSFILE
+               fi
+               $TEST aptget update -o Debug::pkgAcquire::Worker=1
+               cp rootdir/tmp/${TEST}.output rootdir/tmp/update.output
+               testsuccess grep -E "$(basename -s '.gz' "$COMPRESSFILE").*Hash Sum mismatch" rootdir/tmp/update.output
+               $TEST aptcache show foo
+               $TEST aptget install foo -s
+
+               testfailure aptcache show bar
+               testfailure aptget install bar -s
+       done
+done
diff --git a/test/integration/test-apt-update-hashsum-mismatch b/test/integration/test-apt-update-hashsum-mismatch
new file mode 100755 (executable)
index 0000000..747418c
--- /dev/null
@@ -0,0 +1,49 @@
+#!/bin/sh
+set -e
+
+TESTDIR=$(readlink -f $(dirname $0))
+. $TESTDIR/framework
+setupenvironment
+configarchitecture 'i386'
+configcompression 'gz'
+
+insertpackage 'testing' 'foo' 'all' '1'
+insertpackage 'testing' 'foo2' 'all' '1'
+insertsource 'testing' 'foo' 'all' '1'
+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
+testsuccess aptcache show foo
+testsuccess aptget install foo -s
+
+for get in $(sed -n 's#^GET /\([^ ]\+\.gz\) HTTP.\+$#\1#p' aptarchive/webserver.log); do
+       msgmsg 'Test hashsum mismatch with file' "$get"
+       rm -rf rootdir/var/lib/apt/lists
+       webserverconfig 'aptwebserver::overwrite' ''
+       webserverconfig "aptwebserver::overwrite::$(printf '%s' "${get}" | sed 's#/#%2F#g' )::filename" '%2FDoctorEvil.gz'
+
+       TEST='testfailure'
+       if expr match "$get" '^.*Translation-.*$' >/dev/null; then
+               TEST='testsuccess'
+               unset get
+       fi
+       $TEST aptget update
+       cp rootdir/tmp/${TEST}.output rootdir/tmp/update.output
+       testsuccess grep -E "$(basename -s '.gz' "$get").*Hash Sum mismatch" rootdir/tmp/update.output
+       $TEST aptcache show foo
+       $TEST aptget install foo -s
+
+       testfailure aptcache show bar
+       testfailure aptget install bar -s
+done
index afae9956385bd909ae689168b126bc65c6ef06ad..5394a9f3051f36f3562611d52d0fcce52465380f 100755 (executable)
@@ -30,7 +30,7 @@ runtest() {
     
     # ensure that we still do a hash check on ims hit
     msgtest 'Test I-M-S' 'reverify'
-    aptget update -o Debug::pkgAcquire::Auth=1 2>&1 | grep -A1 'RecivedHash:' | grep -q -- '- SHA' && msgpass || msgfail
+    aptget update -o Debug::pkgAcquire::Auth=1 2>&1 | grep -A2 'RecivedHash:' | grep -q -- '- SHA' && msgpass || msgfail
 
     # ensure no leftovers in partial
     testfailure ls "rootdir/var/lib/apt/lists/partial/*"
index 7474cf1486e8ee4b50a031a4adf75d694e145a6b..00004a524ffbe2b2ec4a9a6e1ef2ba1b81828ff5 100644 (file)
@@ -499,9 +499,11 @@ static bool parseFirstLine(int const client, std::string const &request,/*{{{*/
    return true;
 }
                                                                        /*}}}*/
-static bool handleOnTheFlyReconfiguration(int const client, std::string const &request, std::vector<std::string> const &parts)/*{{{*/
+static bool handleOnTheFlyReconfiguration(int const client, std::string const &request, std::vector<std::string> parts)/*{{{*/
 {
    size_t const pcount = parts.size();
+   for (size_t i = 0; i < pcount; ++i)
+      parts[i] = DeQuoteString(parts[i]);
    if (pcount == 4 && parts[1] == "set")
    {
       _config->Set(parts[2], parts[3]);
index 2159996ffcca15c9ca153fca7b089e175fa70dab..a19a0befd36c2e3e3c4ad009389389fe6b473982 100644 (file)
@@ -163,30 +163,34 @@ TEST(HashSumsTest, FileBased)
 
    FileFd fd(__FILE__, FileFd::ReadOnly);
    EXPECT_TRUE(fd.IsOpen());
+   std::string FileSize;
+   strprintf(FileSize, "%llu", fd.FileSize());
 
    {
       Hashes hashes;
       hashes.AddFD(fd.Fd());
       HashStringList list = hashes.GetHashStringList();
       EXPECT_FALSE(list.empty());
-      EXPECT_EQ(4, list.size());
+      EXPECT_EQ(5, list.size());
       EXPECT_EQ(md5.Value(), list.find("MD5Sum")->HashValue());
       EXPECT_EQ(sha1.Value(), list.find("SHA1")->HashValue());
       EXPECT_EQ(sha256.Value(), list.find("SHA256")->HashValue());
       EXPECT_EQ(sha512.Value(), list.find("SHA512")->HashValue());
+      EXPECT_EQ(FileSize, list.find("Checksum-FileSize")->HashValue());
    }
-   unsigned long sz = fd.FileSize();
+   unsigned long long sz = fd.FileSize();
    fd.Seek(0);
    {
       Hashes hashes;
       hashes.AddFD(fd.Fd(), sz);
       HashStringList list = hashes.GetHashStringList();
       EXPECT_FALSE(list.empty());
-      EXPECT_EQ(4, list.size());
+      EXPECT_EQ(5, list.size());
       EXPECT_EQ(md5.Value(), list.find("MD5Sum")->HashValue());
       EXPECT_EQ(sha1.Value(), list.find("SHA1")->HashValue());
       EXPECT_EQ(sha256.Value(), list.find("SHA256")->HashValue());
       EXPECT_EQ(sha512.Value(), list.find("SHA512")->HashValue());
+      EXPECT_EQ(FileSize, list.find("Checksum-FileSize")->HashValue());
    }
    fd.Seek(0);
    {
@@ -279,37 +283,46 @@ TEST(HashSumsTest, HashStringList)
    EXPECT_EQ(NULL, list.find(""));
    EXPECT_EQ(NULL, list.find("MD5Sum"));
 
+   // empty lists aren't equal
    HashStringList list2;
    EXPECT_FALSE(list == list2);
    EXPECT_TRUE(list != list2);
 
+   // some hashes don't really contribute to usability
+   list.push_back(HashString("Checksum-FileSize", "29"));
+   EXPECT_FALSE(list.empty());
+   EXPECT_FALSE(list.usable());
+
    Hashes hashes;
    hashes.Add("The quick brown fox jumps over the lazy dog");
    list = hashes.GetHashStringList();
    EXPECT_FALSE(list.empty());
    EXPECT_TRUE(list.usable());
-   EXPECT_EQ(4, list.size());
+   EXPECT_EQ(5, list.size());
    EXPECT_TRUE(NULL != list.find(NULL));
    EXPECT_TRUE(NULL != list.find(""));
    EXPECT_TRUE(NULL != list.find("MD5Sum"));
+   EXPECT_TRUE(NULL != list.find("Checksum-FileSize"));
    EXPECT_TRUE(NULL == list.find("ROT26"));
 
    _config->Set("Acquire::ForceHash", "MD5Sum");
    EXPECT_FALSE(list.empty());
    EXPECT_TRUE(list.usable());
-   EXPECT_EQ(4, list.size());
+   EXPECT_EQ(5, list.size());
    EXPECT_TRUE(NULL != list.find(NULL));
    EXPECT_TRUE(NULL != list.find(""));
    EXPECT_TRUE(NULL != list.find("MD5Sum"));
+   EXPECT_TRUE(NULL != list.find("Checksum-FileSize"));
    EXPECT_TRUE(NULL == list.find("ROT26"));
 
    _config->Set("Acquire::ForceHash", "ROT26");
    EXPECT_FALSE(list.empty());
    EXPECT_FALSE(list.usable());
-   EXPECT_EQ(4, list.size());
+   EXPECT_EQ(5, list.size());
    EXPECT_TRUE(NULL == list.find(NULL));
    EXPECT_TRUE(NULL == list.find(""));
    EXPECT_TRUE(NULL != list.find("MD5Sum"));
+   EXPECT_TRUE(NULL != list.find("Checksum-FileSize"));
    EXPECT_TRUE(NULL == list.find("ROT26"));
 
    _config->Clear("Acquire::ForceHash");