ensure partial dirs are 0700 and owned by _apt:root

Reworks the API involved in creating and setting up the fetcher to be a
bit more pleasent to look at and work with as e.g. an empty string for
no lock isn't very nice. With the lock we can also stop creating all our
partial directories "just in case". This way we can also be a bit more
aggressive with the partial directory itself as with a lock, we know we
will gone need it.

8 files changed, 81 insertions, 55 deletions

diff --git a/apt-pkg/acquire.cc b/apt-pkg/acquire.cc
index ec565fcfa..9dee1b3cf 100644
--- a/apt-pkg/acquire.cc
+++ b/apt-pkg/acquire.cc
@@ -27,17 +27,20 @@
 #include <vector>
 #include <iostream>
 #include <sstream>
+#include <iomanip>
+
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
-#include <iomanip>
-
+#include <pwd.h>
+#include <grp.h>
 #include <dirent.h>
 #include <sys/time.h>
 #include <sys/select.h>
 #include <errno.h>
 #include <sys/stat.h>
+#include <sys/types.h>
 
 #include <apti18n.h>
 									/*}}}*/
@@ -57,8 +60,8 @@ pkgAcquire::pkgAcquire() : LockFD(-1), Queues(0), Workers(0), Configs(0), Log(NU
    if (strcasecmp(Mode.c_str(),"access") == 0)
       QueueMode = QueueAccess;
 }
-pkgAcquire::pkgAcquire(pkgAcquireStatus *Progress) :  LockFD(-1), Queues(0), Workers(0),
-			   Configs(0), Log(Progress), ToFetch(0),
+pkgAcquire::pkgAcquire(pkgAcquireStatus *Progress) : LockFD(-1), Queues(0), Workers(0),
+			   Configs(0), Log(NULL), ToFetch(0),
 			   Debug(_config->FindB("Debug::pkgAcquire",false)),
 			   Running(false)
 {
@@ -67,40 +70,69 @@ pkgAcquire::pkgAcquire(pkgAcquireStatus *Progress) :  LockFD(-1), Queues(0), Wor
       QueueMode = QueueHost;
    if (strcasecmp(Mode.c_str(),"access") == 0)
       QueueMode = QueueAccess;
-   Setup(Progress, "");
+   SetLog(Progress);
 }
 									/*}}}*/
-// Acquire::Setup - Delayed Constructor					/*{{{*/
-// ---------------------------------------------------------------------
-/* Do everything needed to be a complete Acquire object and report the
-   success (or failure) back so the user knows that something is wrong… */
-bool pkgAcquire::Setup(pkgAcquireStatus *Progress, string const &Lock,
-      bool const createDirectories)
+// Acquire::GetLock - lock directory and prepare for action		/*{{{*/
+static bool SetupAPTPartialDirectory(std::string const &grand, std::string const &parent)
 {
-   Log = Progress;
+   std::string const partial = parent + "partial";
+   if (CreateAPTDirectoryIfNeeded(grand, partial) == false &&
+	 CreateAPTDirectoryIfNeeded(parent, partial) == false)
+      return false;
 
-   // check for existence and possibly create auxiliary directories
-   if (createDirectories == true)
+   if (getuid() == 0) // if we aren't root, we can't chown, so don't try it
+   {
+      struct passwd *pw = getpwnam("_apt");
+      struct group *gr = getgrnam("root");
+      if (pw != NULL && gr != NULL && chown(partial.c_str(), pw->pw_uid, gr->gr_gid) != 0)
+	 _error->WarningE("SetupAPTPartialDirectory", "chown to _apt:root of directory %s failed", partial.c_str());
+   }
+   if (chmod(partial.c_str(), 0700) != 0)
+      _error->WarningE("SetupAPTPartialDirectory", "chmod 0700 of directory %s failed", partial.c_str());
+
+   return true;
+}
+bool pkgAcquire::Setup(pkgAcquireStatus *Progress, string const &Lock)
+{
+   Log = Progress;
+   if (Lock.empty())
    {
       string const listDir = _config->FindDir("Dir::State::lists");
-      string const partialListDir = listDir + "partial/";
+      if (SetupAPTPartialDirectory(_config->FindDir("Dir::State"), listDir) == false)
+	 return _error->Errno("Acquire", _("List directory %spartial is missing."), listDir.c_str());
       string const archivesDir = _config->FindDir("Dir::Cache::Archives");
-      string const partialArchivesDir = archivesDir + "partial/";
+      if (SetupAPTPartialDirectory(_config->FindDir("Dir::Cache"), archivesDir) == false)
+	 return _error->Errno("Acquire", _("Archives directory %spartial is missing."), archivesDir.c_str());
+      return true;
+   }
+   return GetLock(Lock);
+}
+bool pkgAcquire::GetLock(std::string const &Lock)
+{
+   if (Lock.empty() == true)
+      return false;
 
-      if (CreateAPTDirectoryIfNeeded(_config->FindDir("Dir::State"), partialListDir) == false &&
-	    CreateAPTDirectoryIfNeeded(listDir, partialListDir) == false)
-	 return _error->Errno("Acquire", _("List directory %spartial is missing."), listDir.c_str());
+   // check for existence and possibly create auxiliary directories
+   string const listDir = _config->FindDir("Dir::State::lists");
+   string const archivesDir = _config->FindDir("Dir::Cache::Archives");
 
-      if (CreateAPTDirectoryIfNeeded(_config->FindDir("Dir::Cache"), partialArchivesDir) == false &&
-	    CreateAPTDirectoryIfNeeded(archivesDir, partialArchivesDir) == false)
+   if (Lock == listDir)
+   {
+      if (SetupAPTPartialDirectory(_config->FindDir("Dir::State"), listDir) == false)
+	 return _error->Errno("Acquire", _("List directory %spartial is missing."), listDir.c_str());
+   }
+   if (Lock == archivesDir)
+   {
+      if (SetupAPTPartialDirectory(_config->FindDir("Dir::Cache"), archivesDir) == false)
 	 return _error->Errno("Acquire", _("Archives directory %spartial is missing."), archivesDir.c_str());
    }
 
-   if (Lock.empty() == true || _config->FindB("Debug::NoLocking", false) == true)
+   if (_config->FindB("Debug::NoLocking", false) == true)
       return true;
 
    // Lock the directory this acquire object will work in
-   LockFD = GetLock(flCombine(Lock, "lock"));
+   LockFD = ::GetLock(flCombine(Lock, "lock"));
    if (LockFD == -1)
       return _error->Error(_("Unable to lock directory %s"), Lock.c_str());
 
diff --git a/apt-pkg/acquire.h b/apt-pkg/acquire.h
index 7bceb4323..f9eeb1641 100644
--- a/apt-pkg/acquire.h
+++ b/apt-pkg/acquire.h
@@ -351,17 +351,24 @@ class pkgAcquire
     *  long as the pkgAcquire object does.
     *  \param Lock defines a lock file that should be acquired to ensure
     *  only one Acquire class is in action at the time or an empty string
-    *  if no lock file should be used.
-    *  \param createDirectories can be used to disable the creation of directories,
-    *  e.g. if the fetcher is used with different directories later on
+    *  if no lock file should be used. If set also all needed directories
+    *  will be created.
     */
-   bool Setup(pkgAcquireStatus *Progress = NULL, std::string const &Lock = "",
-      bool const createDirectories = true);
+   APT_DEPRECATED bool Setup(pkgAcquireStatus *Progress = NULL, std::string const &Lock = "");
 
    void SetLog(pkgAcquireStatus *Progress) { Log = Progress; }
 
+   /** \brief acquire lock and perform directory setup
+    *
+    *  \param Lock defines a lock file that should be acquired to ensure
+    *  only one Acquire class is in action at the time or an empty string
+    *  if no lock file should be used. If set also all needed directories
+    *  will be created and setup.
+    */
+   bool GetLock(std::string const &Lock);
+
    /** \brief Construct a new pkgAcquire. */
-   pkgAcquire(pkgAcquireStatus *Log) APT_DEPRECATED;
+   pkgAcquire(pkgAcquireStatus *Log);
    pkgAcquire();
 
    /** \brief Destroy this pkgAcquire object.
diff --git a/apt-pkg/update.cc b/apt-pkg/update.cc
index 5d5b19626..2908a4820 100644
--- a/apt-pkg/update.cc
+++ b/apt-pkg/update.cc
@@ -27,8 +27,8 @@ bool ListUpdate(pkgAcquireStatus &Stat,
 		pkgSourceList &List, 
 		int PulseInterval)
 {
-   pkgAcquire Fetcher;
-   if (Fetcher.Setup(&Stat, _config->FindDir("Dir::State::Lists")) == false)
+   pkgAcquire Fetcher(&Stat);
+   if (Fetcher.GetLock(_config->FindDir("Dir::State::Lists")) == false)
       return false;
 
    // Populate it with the source selection
diff --git a/apt-private/private-install.cc b/apt-private/private-install.cc
index 86ba52857..c06caeedd 100644
--- a/apt-private/private-install.cc
+++ b/apt-private/private-install.cc
@@ -119,14 +119,14 @@ bool InstallPackages(CacheFile &Cache,bool ShwKept,bool Ask, bool Safety)
       return false;
 
    // Create the download object
-   pkgAcquire Fetcher;
-   AcqTextStatus Stat(ScreenWidth,_config->FindI("quiet",0));   
+   AcqTextStatus Stat(ScreenWidth,_config->FindI("quiet",0));
+   pkgAcquire Fetcher(&Stat);
    if (_config->FindB("APT::Get::Print-URIs", false) == true)
    {
       // force a hashsum for compatibility reasons
       _config->CndSet("Acquire::ForceHash", "md5sum");
    }
-   else if (Fetcher.Setup(&Stat, _config->FindDir("Dir::Cache::Archives")) == false)
+   else if (Fetcher.GetLock(_config->FindDir("Dir::Cache::Archives")) == false)
       return false;
 
    // Read the source list
diff --git a/apt-private/private-update.cc b/apt-private/private-update.cc
index 1cf3012ed..df77ac33a 100644
--- a/apt-private/private-update.cc
+++ b/apt-private/private-update.cc
@@ -47,9 +47,7 @@ bool DoUpdate(CommandLine &CmdL)
       _config->CndSet("Acquire::ForceHash", "md5sum");
 
       // get a fetcher
-      pkgAcquire Fetcher;
-      if (Fetcher.Setup(&Stat) == false)
-	 return false;
+      pkgAcquire Fetcher(&Stat);
 
       // Populate it with the source selection and get all Indexes 
       // (GetAll=true)
diff --git a/cmdline/apt-get.cc b/cmdline/apt-get.cc
index 0cea05cb3..15696e19f 100644
--- a/cmdline/apt-get.cc
+++ b/cmdline/apt-get.cc
@@ -646,9 +646,7 @@ static bool DoDownload(CommandLine &CmdL)
       return false;
 
    AcqTextStatus Stat(ScreenWidth, _config->FindI("quiet", 0));
-   pkgAcquire Fetcher;
-   if (Fetcher.Setup(&Stat, "", false) == false)
-      return false;
+   pkgAcquire Fetcher(&Stat);
 
    pkgRecords Recs(Cache);
    pkgSourceList *SrcList = Cache.GetSourceList();
@@ -744,9 +742,8 @@ static bool DoSource(CommandLine &CmdL)
       return false;
 
    // Create the download object
-   AcqTextStatus Stat(ScreenWidth,_config->FindI("quiet",0));   
-   pkgAcquire Fetcher;
-   Fetcher.SetLog(&Stat);
+   AcqTextStatus Stat(ScreenWidth,_config->FindI("quiet",0));
+   pkgAcquire Fetcher(&Stat);
 
    SPtrArray<DscFile> Dsc = new DscFile[CmdL.FileSize()];
    
@@ -1048,12 +1045,6 @@ static bool DoBuildDep(CommandLine &CmdL)
    if (_error->PendingError() == true)
       return false;
 
-   // Create the download object
-   AcqTextStatus Stat(ScreenWidth,_config->FindI("quiet",0));   
-   pkgAcquire Fetcher;
-   if (Fetcher.Setup(&Stat) == false)
-      return false;
-
    bool StripMultiArch;
    string hostArch = _config->Find("APT::Get::Host-Architecture");
    if (hostArch.empty() == false)
@@ -1565,8 +1556,7 @@ static bool DoChangelog(CommandLine &CmdL)
    }
 
    AcqTextStatus Stat(ScreenWidth, _config->FindI("quiet",0));
-   if (Fetcher.Setup(&Stat, "",false) == false)
-      return false;
+   Fetcher.SetLog(&Stat);
 
    bool const downOnly = _config->FindB("APT::Get::Download-Only", false);
 
diff --git a/cmdline/apt-helper.cc b/cmdline/apt-helper.cc
index b89df61d6..c240008aa 100644
--- a/cmdline/apt-helper.cc
+++ b/cmdline/apt-helper.cc
@@ -48,11 +48,9 @@ static bool DoDownloadFile(CommandLine &CmdL)
    if (CmdL.FileSize() <= 2)
       return _error->Error(_("Must specify at least one pair url/filename"));
 
-
-   pkgAcquire Fetcher;
    AcqTextStatus Stat(ScreenWidth, _config->FindI("quiet",0));
-   if (Fetcher.Setup(&Stat, "", false) == false)
-      return false;
+   pkgAcquire Fetcher(&Stat);
+
    std::string download_uri = CmdL.FileList[1];
    std::string targetfile = CmdL.FileList[2];
    std::string hash;
diff --git a/test/integration/test-apt-get-clean b/test/integration/test-apt-get-clean
index 646ea31be..98f7c84d0 100755
--- a/test/integration/test-apt-get-clean
+++ b/test/integration/test-apt-get-clean
@@ -18,6 +18,7 @@ testsuccess aptget clean
 
 # generate some dirt and clean it up
 touch rootdir/var/lib/apt/lists/partial/http.debian.net_debian_dists_sid_main_i18n_Translation-en
+mkdir -p rootdir/var/cache/apt/archives
 touch rootdir/var/cache/apt/archives/foo_1_all.deb
 touch rootdir/var/cache/apt/archives/foo_2_all.deb
 touch rootdir/var/cache/apt/archives/foo_3_all.deb