improve CheckDropPrivsMustBeDisabled further

Various smaller improvements so that the check deals better with already
downloaded files, relative paths and other things.

Git-Dch: Ignore

diff --git a/apt-pkg/acquire.cc b/apt-pkg/acquire.cc
index cb32e8f2ba2e6dda4623d1e3c0133517f34db94a..c7bc00e0b50179ac9706147650f8c70182b8e756 100644 (file)
--- a/apt-pkg/acquire.cc
+++ b/apt-pkg/acquire.cc
@@ -460,6 +460,8 @@ static void CheckDropPrivsMustBeDisabled(pkgAcquire const &Fetcher)
    if (pw == NULL)
       return;
 
+   gid_t const old_euid = geteuid();
+   gid_t const old_egid = getegid();
    if (setegid(pw->pw_gid) != 0)
       _error->Errno("setegid", "setegid %u failed", pw->pw_gid);
    if (seteuid(pw->pw_uid) != 0)
@@ -469,31 +471,43 @@ static void CheckDropPrivsMustBeDisabled(pkgAcquire const &Fetcher)
    for (pkgAcquire::ItemCIterator I = Fetcher.ItemsBegin();
        I != Fetcher.ItemsEnd() && dropPrivs == true; ++I)
    {
-      if ((*I)->DestFile.empty())
+      std::string filename = (*I)->DestFile;
+      if (filename.empty())
+        continue;
+
+      // no need to drop privileges for a complete file
+      if ((*I)->Complete == true)
         continue;
 
       // we check directory instead of file as the file might or might not
       // exist already as a link or not which complicates everything…
-      std::string dirname = flNotFile((*I)->DestFile);
+      std::string dirname = flNotFile(filename);
+      if (unlikely(dirname.empty()))
+        continue;
+      // translate relative to absolute for DirectoryExists
+      // FIXME: What about ../ and ./../ ?
+      if (dirname.substr(0,2) == "./")
+        dirname = SafeGetCWD() + dirname.substr(2);
+
       if (DirectoryExists(dirname))
         ;
       else
         continue; // assume it is created correctly by the acquire system
 
-      if (faccessat(AT_FDCWD, dirname.c_str(), R_OK | W_OK | X_OK, AT_EACCESS | AT_SYMLINK_NOFOLLOW) != 0)
+      if (faccessat(-1, dirname.c_str(), R_OK | W_OK | X_OK, AT_EACCESS | AT_SYMLINK_NOFOLLOW) != 0)
       {
         dropPrivs = false;
         _error->WarningE("pkgAcquire::Run", _("Can't drop privileges for downloading as file '%s' couldn't be accessed by user '%s'."),
-              (*I)->DestFile.c_str(), SandboxUser.c_str());
+              filename.c_str(), SandboxUser.c_str());
         _config->Set("APT::Sandbox::User", "");
         break;
       }
    }
 
-   if (seteuid(0) != 0)
-      _error->Errno("seteuid", "seteuid %u failed", 0);
-   if (setegid(0) != 0)
-      _error->Errno("setegid", "setegid %u failed", 0);
+   if (seteuid(old_euid) != 0)
+      _error->Errno("seteuid", "seteuid %u failed", old_euid);
+   if (setegid(old_egid) != 0)
+      _error->Errno("setegid", "setegid %u failed", old_egid);
 }
 pkgAcquire::RunResult pkgAcquire::Run(int PulseIntervall)
 {

diff --git a/apt-pkg/contrib/fileutl.cc b/apt-pkg/contrib/fileutl.cc
index 1d20c9c35148612eacbf87757120c8d5f0469bc7..837edef4b1e015a075549c5cccbc672c7c3149c3 100644 (file)
--- a/apt-pkg/contrib/fileutl.cc
+++ b/apt-pkg/contrib/fileutl.cc
@@ -2141,6 +2141,8 @@ std::string GetTempDir(std::string const &User)
    if (pw == NULL)
       return GetTempDir();
 
+   gid_t const old_euid = geteuid();
+   gid_t const old_egid = getegid();
    if (setegid(pw->pw_gid) != 0)
       _error->Errno("setegid", "setegid %u failed", pw->pw_gid);
    if (seteuid(pw->pw_uid) != 0)
@@ -2148,10 +2150,10 @@ std::string GetTempDir(std::string const &User)
 
    std::string const tmp = GetTempDir();
 
-   if (seteuid(0) != 0)
-      _error->Errno("seteuid", "seteuid %u failed", 0);
-   if (setegid(0) != 0)
-      _error->Errno("setegid", "setegid %u failed", 0);
+   if (seteuid(old_euid) != 0)
+      _error->Errno("seteuid", "seteuid %u failed", old_euid);
+   if (setegid(old_egid) != 0)
+      _error->Errno("setegid", "setegid %u failed", old_egid);
 
    return tmp;
 }

diff --git a/test/integration/test-http-pipeline-messup b/test/integration/test-http-pipeline-messup
index dda8ef7eb1737e03b085ee71e7ed8c40d405395d..a00182150a9cd175f57bfd996030d5c4c2f5ed43 100755 (executable)
--- a/test/integration/test-http-pipeline-messup
+++ b/test/integration/test-http-pipeline-messup
@@ -27,18 +27,26 @@ Debug::pkgAcquire::Worker "true";' > rootdir/etc/apt/apt.conf.d/99debug
 
 testsuccess aptget update
 
-# messup is bigger than pipeline: checks if fixup isn't trying to hard
+cd downloaded
+# messup is bigger than pipeline: checks if fixup isn't trying too hard
 testfailure aptget download pkga pkgb pkgc pkgd -o Acquire::http::Pipeline-Depth=2
-testfailure test -f pkga_1.0_all.deb
+for pkg in 'pkga' 'pkgd'; do
+       testfailure test -f ${pkg}_1.0_all.deb
+done
+for pkg in 'pkgb' 'pkgc'; do
+       testsuccess test -f ${pkg}_1.0_all.deb
+       testsuccess cmp ../incoming/${pkg}_1.0_all.deb ${pkg}_1.0_all.deb
+       rm -f ${pkg}_1.0_all.deb
+done
 
 # ensure that pipeling is enabled for rest of this test
-echo 'Acquire::http::Pipeline-Depth 10;' > rootdir/etc/apt/apt.conf.d/99enable-pipeline
+echo 'Acquire::http::Pipeline-Depth 10;' > ../rootdir/etc/apt/apt.conf.d/99enable-pipeline
 
 # the output is a bit strange: it looks like it has downloaded pkga 4 times
 testwarning aptget download pkga pkgb pkgc pkgd
 for pkg in 'pkga' 'pkgb' 'pkgc' 'pkgd'; do
        testsuccess test -f ${pkg}_1.0_all.deb
-       testsuccess cmp incoming/${pkg}_1.0_all.deb ${pkg}_1.0_all.deb
+       testsuccess cmp ../incoming/${pkg}_1.0_all.deb ${pkg}_1.0_all.deb
        rm -f ${pkg}_1.0_all.deb
 done