SECURITY UPDATE for CVE-2014-{0488,0487,0489}

incorrect invalidating of unauthenticated data (CVE-2014-0488)
incorect verification of 304 reply (CVE-2014-0487)
incorrect verification of Acquire::Gzip indexes (CVE-2014-0489)

diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc
index 6cb9b01..058b8bf 100644 (file)
--- a/apt-pkg/acquire-item.cc
+++ b/apt-pkg/acquire-item.cc
@@ -1021,6 +1021,31 @@ void pkgAcqIndex::Failed(string Message,pkgAcquire::MethodConfig *Cnf)   /*{{{*/
    Item::Failed(Message,Cnf);
 }
                                                                        /*}}}*/
+// pkgAcqIndex::GetFinalFilename - Return the full final file path      /*{{{*/
+std::string pkgAcqIndex::GetFinalFilename(std::string const &URI,
+                                          std::string const &compExt)
+{
+   std::string FinalFile = _config->FindDir("Dir::State::lists");
+   FinalFile += URItoFileName(URI);
+   if (_config->FindB("Acquire::GzipIndexes",false) && compExt == "gz")
+      FinalFile += ".gz";
+   return FinalFile;
+}
+                                                                       /*}}}*/
+// AcqIndex::ReverifyAfterIMS - Reverify index after an ims-hit                /*{{{*/
+void pkgAcqIndex::ReverifyAfterIMS(std::string const &FileName)
+{
+   std::string const compExt = CompressionExtension.substr(0, CompressionExtension.find(' '));
+   if (_config->FindB("Acquire::GzipIndexes",false) && compExt == "gz")
+      DestFile += ".gz";
+
+   string FinalFile = GetFinalFilename(RealURI, compExt);
+   Rename(FinalFile, FileName);
+   Decompression = true;
+   Desc.URI = "copy:" + FileName;
+   QueueURI(Desc);
+}
+                                                                       /*}}}*/
 // AcqIndex::Done - Finished a fetch                                   /*{{{*/
 // ---------------------------------------------------------------------
 /* This goes through a number of states.. On the initial fetch the
@@ -1032,6 +1057,7 @@ void pkgAcqIndex::Done(string Message,unsigned long long Size,string Hash,
                       pkgAcquire::MethodConfig *Cfg)
 {
    Item::Done(Message,Size,Hash,Cfg);
+   std::string const compExt = CompressionExtension.substr(0, CompressionExtension.find(' '));
 
    if (Decompression == true)
    {
@@ -1043,6 +1069,7 @@ void pkgAcqIndex::Done(string Message,unsigned long long Size,string Hash,
 
       if (!ExpectedHash.empty() && ExpectedHash.toStr() != Hash)
       {
+         Desc.URI = RealURI;
         RenameOnError(HashSumMismatch);
          return;
       }
@@ -1053,9 +1080,9 @@ void pkgAcqIndex::Done(string Message,unsigned long long Size,string Hash,
       /* Always verify the index file for correctness (all indexes must
        * have a Package field) (LP: #346386) (Closes: #627642) 
        */
-      FileFd fd(DestFile, FileFd::ReadOnly);
+      FileFd fd(DestFile, FileFd::ReadOnlyGzip);
       // Only test for correctness if the file is not empty (empty is ok)
-      if (fd.FileSize() > 0)
+      if (fd.Size() > 0)
       {
          pkgTagSection sec;
          pkgTagFile tag(&fd);
@@ -1069,8 +1096,7 @@ void pkgAcqIndex::Done(string Message,unsigned long long Size,string Hash,
       }
        
       // Done, move it into position
-      string FinalFile = _config->FindDir("Dir::State::lists");
-      FinalFile += URItoFileName(RealURI);
+      string FinalFile = GetFinalFilename(RealURI, compExt);
       Rename(DestFile,FinalFile);
       chmod(FinalFile.c_str(),0644);
       
@@ -1078,7 +1104,9 @@ void pkgAcqIndex::Done(string Message,unsigned long long Size,string Hash,
          will work OK */
       DestFile = _config->FindDir("Dir::State::lists") + "partial/";
       DestFile += URItoFileName(RealURI);
-      
+      if (_config->FindB("Acquire::GzipIndexes",false) && compExt == "gz")
+         DestFile += ".gz";
+
       // Remove the compressed version.
       if (Erase == true)
         unlink(DestFile.c_str());
@@ -1094,7 +1122,10 @@ void pkgAcqIndex::Done(string Message,unsigned long long Size,string Hash,
    {
       // The files timestamp matches
       if (StringToBool(LookupTag(Message,"Alt-IMS-Hit"),false) == true)
-        return;
+      {
+         ReverifyAfterIMS(FileName);
+         return;
+      }
       Decompression = true;
       Local = true;
       DestFile += ".decomp";
@@ -1111,15 +1142,12 @@ void pkgAcqIndex::Done(string Message,unsigned long long Size,string Hash,
       ErrorText = "Method gave a blank filename";
    }
 
-   std::string const compExt = CompressionExtension.substr(0, CompressionExtension.find(' '));
-
    // The files timestamp matches
-   if (StringToBool(LookupTag(Message,"IMS-Hit"),false) == true) {
-       if (_config->FindB("Acquire::GzipIndexes",false) && compExt == "gz")
-         // Update DestFile for .gz suffix so that the clean operation keeps it
-         DestFile += ".gz";
+   if (StringToBool(LookupTag(Message,"IMS-Hit"),false) == true)
+   {
+      ReverifyAfterIMS(FileName);
       return;
-    }
+   }
 
    if (FileName == DestFile)
       Erase = true;
@@ -1128,16 +1156,16 @@ void pkgAcqIndex::Done(string Message,unsigned long long Size,string Hash,
    
    string decompProg;
 
-   // If we enable compressed indexes and already have gzip, keep it
-   if (_config->FindB("Acquire::GzipIndexes",false) && compExt == "gz" && !Local) {
-      string FinalFile = _config->FindDir("Dir::State::lists");
-      FinalFile += URItoFileName(RealURI) + ".gz";
-      Rename(DestFile,FinalFile);
-      chmod(FinalFile.c_str(),0644);
-      
-      // Update DestFile for .gz suffix so that the clean operation keeps it
-      DestFile = _config->FindDir("Dir::State::lists") + "partial/";
+   // If we enable compressed indexes, queue for hash verification
+   if (_config->FindB("Acquire::GzipIndexes",false) && compExt == "gz" && !Local) 
+   {
+      DestFile = _config->FindDir("Dir::State::lists");
       DestFile += URItoFileName(RealURI) + ".gz";
+
+      Decompression = true;
+      Desc.URI = "copy:" + FileName;
+      QueueURI(Desc);
+
       return;
     }
 
@@ -1181,6 +1209,9 @@ string pkgAcqIndexTrans::Custom600Headers()
    string Final = _config->FindDir("Dir::State::lists");
    Final += URItoFileName(RealURI);
 
+   if (_config->FindB("Acquire::GzipIndexes",false))
+      Final += ".gz";
+
    struct stat Buf;
    if (stat(Final.c_str(),&Buf) != 0)
       return "\nFail-Ignore: true\nIndex-File: true";
@@ -1510,6 +1541,28 @@ void pkgAcqMetaIndex::AuthDone(string Message)                           /*{{{*/
       std::cerr << "Signature verification succeeded: "
                 << DestFile << std::endl;
 
+   // do not trust any previously unverified content that we may have
+   string LastGoodSigFile = _config->FindDir("Dir::State::lists").append("partial/").append(URItoFileName(RealURI));
+   if (DestFile != SigFile)
+      LastGoodSigFile.append(".gpg");
+   LastGoodSigFile.append(".reverify");
+   if(IMSHit == false && RealFileExists(LastGoodSigFile) == false)
+   {
+      for (vector <struct IndexTarget*>::const_iterator Target = IndexTargets->begin();
+           Target != IndexTargets->end();
+           ++Target)
+      {
+         // remove old indexes
+         std::string index = _config->FindDir("Dir::State::lists") +
+            URItoFileName((*Target)->URI);
+         unlink(index.c_str());
+         // and also old gzipindexes
+         index += ".gz";
+         unlink(index.c_str());
+      }
+   }
+
+
    // Download further indexes with verification
    QueueIndexes(true);
 

diff --git a/apt-pkg/acquire-item.h b/apt-pkg/acquire-item.h
index 06537bf..384c5ee 100644 (file)
--- a/apt-pkg/acquire-item.h
+++ b/apt-pkg/acquire-item.h
@@ -706,6 +706,14 @@ class pkgAcqIndex : public pkgAcquire::Item
     */
    std::string CompressionExtension;
 
+   /** \brief Get the full pathname of the final file for the given URI
+    */
+   std::string GetFinalFilename(std::string const &URI,
+                                std::string const &compExt);
+
+   /** \brief Schedule file for verification after a IMS hit */
+   void ReverifyAfterIMS(std::string const &FileName);
+
    public:
    
    // Specialized action members

diff --git a/apt-pkg/contrib/fileutl.h b/apt-pkg/contrib/fileutl.h
index cc1a98e..6670570 100644 (file)
--- a/apt-pkg/contrib/fileutl.h
+++ b/apt-pkg/contrib/fileutl.h
@@ -85,7 +85,9 @@ class FileFd
    bool Skip(unsigned long long To);
    bool Truncate(unsigned long long To);
    unsigned long long Tell();
+   // the size of the file content (compressed files will be uncompressed first)
    unsigned long long Size();
+   // the size of the file itself
    unsigned long long FileSize();
    time_t ModificationTime();
 

diff --git a/methods/copy.cc b/methods/copy.cc
index d59f032..5570f31 100644 (file)
--- a/methods/copy.cc
+++ b/methods/copy.cc
@@ -16,6 +16,7 @@
 #include <apt-pkg/acquire-method.h>
 #include <apt-pkg/error.h>
 #include <apt-pkg/hashes.h>
+#include <apt-pkg/configuration.h>
 
 #include <string>
 #include <sys/stat.h>
@@ -27,12 +28,28 @@
 class CopyMethod : public pkgAcqMethod
 {
    virtual bool Fetch(FetchItem *Itm);
+   void CalculateHashes(FetchResult &Res);
    
    public:
    
-   CopyMethod() : pkgAcqMethod("1.0",SingleInstance) {};
+   CopyMethod() : pkgAcqMethod("1.0",SingleInstance | SendConfig) {};
 };
 
+void CopyMethod::CalculateHashes(FetchResult &Res)
+{
+   // For gzip indexes we need to look inside the gzip for the hash
+   // We can not use the extension here as its not used in partial 
+   // on a IMS hit
+   FileFd::OpenMode OpenMode = FileFd::ReadOnly;
+   if (_config->FindB("Acquire::GzipIndexes", false) == true)
+      OpenMode = FileFd::ReadOnlyGzip;
+
+   Hashes Hash;
+   FileFd Fd(Res.Filename, OpenMode);
+   Hash.AddFD(Fd);
+   Res.TakeHashes(Hash);
+}
+
 // CopyMethod::Fetch - Fetch a file                                    /*{{{*/
 // ---------------------------------------------------------------------
 /* */
@@ -54,6 +71,14 @@ bool CopyMethod::Fetch(FetchItem *Itm)
    Res.IMSHit = false;      
    URIStart(Res);
    
+   // just calc the hashes if the source and destination are identical
+   if (File == Itm->DestFile)
+   {
+      CalculateHashes(Res);
+      URIDone(Res);
+      return true;
+   }
+
    // See if the file exists
    FileFd From(File,FileFd::ReadOnly);
    FileFd To(Itm->DestFile,FileFd::WriteAtomic);
@@ -82,10 +107,7 @@ bool CopyMethod::Fetch(FetchItem *Itm)
    if (utimes(Res.Filename.c_str(), times) != 0)
       return _error->Errno("utimes",_("Failed to set modification time"));
 
-   Hashes Hash;
-   FileFd Fd(Res.Filename, FileFd::ReadOnly);
-   Hash.AddFD(Fd);
-   Res.TakeHashes(Hash);
+   CalculateHashes(Res);
 
    URIDone(Res);
    return true;

diff --git a/test/integration/test-apt-update-stale b/test/integration/test-apt-update-stale
new file mode 100755 (executable)
index 0000000..780ff79
--- /dev/null
+++ b/test/integration/test-apt-update-stale
@@ -0,0 +1,46 @@
+#!/bin/sh
+#
+# Ensure that a MITM can not stale the Packages/Sources without
+# raising a error message. Note that the Release file is protected
+# via the "Valid-Until" header
+#
+set -e
+
+TESTDIR=$(readlink -f $(dirname $0))
+. $TESTDIR/framework
+
+setupenvironment
+configarchitecture "i386"
+
+insertpackage 'unstable' 'foo' 'all' '1.0'
+
+setupaptarchive
+changetowebserver
+aptget update -qq
+
+# insert new version
+mkdir aptarchive/dists/unstable/main/binary-i386/saved
+cp -p aptarchive/dists/unstable/main/binary-i386/Packages* \
+     aptarchive/dists/unstable/main/binary-i386/saved
+insertpackage 'unstable' 'foo' 'all' '2.0'
+
+# not using compressfile for compat with older apt releases
+gzip -c aptarchive/dists/unstable/main/binary-i386/Packages > \
+  aptarchive/dists/unstable/main/binary-i386/Packages.gz
+generatereleasefiles
+signreleasefiles
+
+# ensure that we do not get a I-M-S hit for the Release file
+touch -d "+1hour" aptarchive/dists/unstable/*Release*
+
+# but now only deliver the previous Packages file instead of the new one
+# (simulating a stale attack)
+cp -p aptarchive/dists/unstable/main/binary-i386/saved/Packages* \
+     aptarchive/dists/unstable/main/binary-i386/
+
+# ensure this raises a error
+testequal "W: Failed to fetch http://localhost:8080/dists/unstable/main/binary-i386/Packages  Hash Sum mismatch
+
+E: Some index files failed to download. They have been ignored, or old ones used instead." aptget update -qq
+
+

diff --git a/test/integration/test-apt-update-unauth b/test/integration/test-apt-update-unauth
new file mode 100755 (executable)
index 0000000..1348760
--- /dev/null
+++ b/test/integration/test-apt-update-unauth
@@ -0,0 +1,48 @@
+#!/bin/sh
+#
+# Ensure that when going from unauthenticated to authenticated all
+# files are checked again
+#
+set -e
+
+TESTDIR=$(readlink -f $(dirname $0))
+. $TESTDIR/framework
+
+setupenvironment
+configarchitecture "i386"
+
+insertpackage 'unstable' 'foo' 'all' '1.0'
+insertsource 'unstable' 'foo' 'all' '1.0'
+
+setupaptarchive
+changetowebserver
+
+runtest() {
+    # start unauthenticated
+    find rootdir/var/lib/apt/lists/ -type f | xargs rm -f
+    rm -f aptarchive/dists/unstable/*Release*
+    aptget update -qq
+
+    # become authenticated
+    generatereleasefiles
+    signreleasefiles
+
+    # and ensure we do download the data again
+    msgtest "Check that the data is check when going to authenticated"
+    if aptget update |grep -q Hit; then
+        msgfail
+    else
+        msgpass
+    fi
+}
+
+for COMPRESSEDINDEXES in 'false' 'true'; do
+       echo "Acquire::GzipIndexes \"$COMPRESSEDINDEXES\";" > rootdir/etc/apt/apt.conf.d/compressindexes
+       if $COMPRESSEDINDEXES; then
+               msgmsg 'Run tests with GzipIndexes enabled'
+       else
+               msgmsg 'Run tests with GzipIndexes disabled'
+       fi
+
+        runtest
+done

diff --git a/test/integration/test-hashsum-verification b/test/integration/test-hashsum-verification
index e77efb4..2a400dc 100755 (executable)
--- a/test/integration/test-hashsum-verification
+++ b/test/integration/test-hashsum-verification
@@ -64,7 +64,7 @@ runtest() {
         msgtest 'No package from the source available'
         [ "$(aptcache show apt 2>&1)" = "E: No packages found" ] && msgpass || msgfail
         msgtest 'No Packages file in /var/lib/apt/lists'
-        [ "$(ls rootdir/var/lib/apt/lists/*Package* 2>/dev/null)" = "" ] && msgpass || msgfail 
+        [ "$(ls rootdir/var/lib/apt/lists/*Package* 2>/dev/null | grep -v FAILED 2>/dev/null)" = "" ] && msgpass || msgfail 
         
         # now with the unsigned Release file
         rm -rf rootdir/var/lib/apt/lists
@@ -75,5 +75,13 @@ runtest() {
 
 }
 
-runtest
-
+for COMPRESSEDINDEXES in 'false' 'true'; do
+       echo "Acquire::GzipIndexes \"$COMPRESSEDINDEXES\";" > rootdir/etc/apt/apt.conf.d/compressindexes
+       if $COMPRESSEDINDEXES; then
+               msgmsg 'Run tests with GzipIndexes enabled'
+       else
+               msgmsg 'Run tests with GzipIndexes disabled'
+       fi
+ 
+        runtest
+done