]> git.saurik.com Git - apt.git/commitdiff
do not use _apt for file/copy sources if it isn't world-accessible
authorDavid Kalnischkies <david@kalnischkies.de>
Wed, 18 Nov 2015 18:31:40 +0000 (19:31 +0100)
committerDavid Kalnischkies <david@kalnischkies.de>
Thu, 19 Nov 2015 15:46:29 +0000 (16:46 +0100)
In 0940230d we started dropping privileges for file (and a bit later for
copy, too) with the intend of uniforming this for all methods. The
commit message says that the source will likely fail based on the
compressors already – and there isn't much secret in the repository
content. After all, after apt has run the update everyone can access the
content via apt anyway…

There are sources through which worked before which are mostly
single-deb (and those with the uncompressed files available).
The first one being especially surprising for users maybe, so instead of
failing, we make it so that apt detects that it can't access a source as
_apt and if so doesn't drop (for all sources!) privileges – but we limit
this to file/copy, so the uncompress which might be needed will still
fail – but that failed before this regression.

We display a notice about this, mostly so that if it still fails (e.g.
compressed) the user has some idea what is wrong.

Closes: 805069
apt-pkg/acquire-worker.cc
apt-pkg/acquire.cc
apt-pkg/contrib/fileutl.cc
test/integration/test-apt-get-download
test/integration/test-apt-get-install-deb
test/integration/test-apt-update-file
test/libapt/configuration_test.cc

index b5f52a3caf6781aaa62f751a57f82de25b962dbd..c2ee8cda39fdba8962fae476ad602cdc84e14464 100644 (file)
@@ -639,7 +639,7 @@ bool pkgAcquire::Worker::QueueItem(pkgAcquire::Queue::QItem *Item)
 
    if (RealFileExists(Item->Owner->DestFile))
    {
-      std::string SandboxUser = _config->Find("APT::Sandbox::User");
+      std::string const SandboxUser = _config->Find("APT::Sandbox::User");
       ChangeOwnerAndPermissionOfFile("Item::QueueURI", Item->Owner->DestFile.c_str(),
                                      SandboxUser.c_str(), "root", 0600);
    }
index 81faee5d8d07086dc2ba762fb689f56067354c20..fb1210750f41b3b2699be4e9c9e5d50ccc64662e 100644 (file)
@@ -30,6 +30,7 @@
 #include <iostream>
 #include <sstream>
 #include <iomanip>
+#include <memory>
 
 #include <stdio.h>
 #include <stdlib.h>
@@ -76,7 +77,7 @@ void pkgAcquire::Initialize()
 
    // chown the auth.conf file as it will be accessed by our methods
    std::string const SandboxUser = _config->Find("APT::Sandbox::User");
-   if (getuid() == 0 && SandboxUser.empty() == false) // if we aren't root, we can't chown, so don't try it
+   if (getuid() == 0 && SandboxUser.empty() == false && SandboxUser != "root") // if we aren't root, we can't chown, so don't try it
    {
       struct passwd const * const pw = getpwnam(SandboxUser.c_str());
       struct group const * const gr = getgrnam("root");
@@ -102,7 +103,7 @@ static bool SetupAPTPartialDirectory(std::string const &grand, std::string const
       return false;
 
    std::string const SandboxUser = _config->Find("APT::Sandbox::User");
-   if (getuid() == 0 && SandboxUser.empty() == false) // if we aren't root, we can't chown, so don't try it
+   if (getuid() == 0 && SandboxUser.empty() == false && SandboxUser != "root") // if we aren't root, we can't chown, so don't try it
    {
       struct passwd const * const pw = getpwnam(SandboxUser.c_str());
       struct group const * const gr = getgrnam("root");
@@ -448,13 +449,50 @@ void pkgAcquire::RunFds(fd_set *RSet,fd_set *WSet)
 /* This runs the queues. It manages a select loop for all of the
    Worker tasks. The workers interact with the queues and items to
    manage the actual fetch. */
+static bool IsAccessibleBySandboxUser(std::string const &filename, bool const ReadWrite)
+{
+   // you would think this is easily to answer with faccessat, right? Wrong!
+   // It e.g. gets groups wrong, so the only thing which works reliable is trying
+   // to open the file we want to open later on…
+   if (unlikely(filename.empty()))
+      return true;
+
+   if (ReadWrite == false)
+   {
+      errno = 0;
+      // can we read a file? Note that non-existing files are "fine"
+      int const fd = open(filename.c_str(), O_RDONLY | O_CLOEXEC);
+      if (fd == -1 && errno == EACCES)
+        return false;
+      close(fd);
+      return true;
+   }
+   else
+   {
+      // the file might not exist yet and even if it does we will fix permissions,
+      // so important is here just that the directory it is in allows that
+      std::string const dirname = flNotFile(filename);
+      if (unlikely(dirname.empty()))
+        return true;
+
+      char const * const filetag = ".apt-acquire-privs-test.XXXXXX";
+      std::string const tmpfile_tpl = flCombine(dirname, filetag);
+      std::unique_ptr<char, decltype(std::free) *> tmpfile { strdup(tmpfile_tpl.c_str()), std::free };
+      int const fd = mkstemp(tmpfile.get());
+      if (fd == -1 && errno == EACCES)
+        return false;
+      RemoveFile("IsAccessibleBySandboxUser", tmpfile.get());
+      close(fd);
+      return true;
+   }
+}
 static void CheckDropPrivsMustBeDisabled(pkgAcquire const &Fetcher)
 {
    if(getuid() != 0)
       return;
 
-   std::string SandboxUser = _config->Find("APT::Sandbox::User");
-   if (SandboxUser.empty())
+   std::string const SandboxUser = _config->Find("APT::Sandbox::User");
+   if (SandboxUser.empty() || SandboxUser == "root")
       return;
 
    struct passwd const * const pw = getpwnam(SandboxUser.c_str());
@@ -463,50 +501,67 @@ static void CheckDropPrivsMustBeDisabled(pkgAcquire const &Fetcher)
 
    gid_t const old_euid = geteuid();
    gid_t const old_egid = getegid();
+
+   long const ngroups_max = sysconf(_SC_NGROUPS_MAX);
+   std::unique_ptr<gid_t[]> old_gidlist(new gid_t[ngroups_max]);
+   if (unlikely(old_gidlist == NULL))
+      return;
+   ssize_t old_gidlist_nr;
+   if ((old_gidlist_nr = getgroups(ngroups_max, old_gidlist.get())) < 0)
+   {
+      _error->FatalE("getgroups", "getgroups %lu failed", ngroups_max);
+      old_gidlist[0] = 0;
+      old_gidlist_nr = 1;
+   }
+   if (setgroups(1, &pw->pw_gid))
+      _error->FatalE("setgroups", "setgroups %u failed", pw->pw_gid);
+
    if (setegid(pw->pw_gid) != 0)
-      _error->Errno("setegid", "setegid %u failed", pw->pw_gid);
+      _error->FatalE("setegid", "setegid %u failed", pw->pw_gid);
    if (seteuid(pw->pw_uid) != 0)
-      _error->Errno("seteuid", "seteuid %u failed", pw->pw_uid);
+      _error->FatalE("seteuid", "seteuid %u failed", pw->pw_uid);
 
    for (pkgAcquire::ItemCIterator I = Fetcher.ItemsBegin();
        I != Fetcher.ItemsEnd(); ++I)
    {
-      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(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(-1, dirname.c_str(), R_OK | W_OK | X_OK, AT_EACCESS | AT_SYMLINK_NOFOLLOW) != 0)
+      // if destination file is inaccessible all hope is lost for privilege dropping
+      if (IsAccessibleBySandboxUser((*I)->DestFile, true) == false)
       {
         _error->WarningE("pkgAcquire::Run", _("Can't drop privileges for downloading as file '%s' couldn't be accessed by user '%s'."),
-              filename.c_str(), SandboxUser.c_str());
+              (*I)->DestFile.c_str(), SandboxUser.c_str());
         _config->Set("APT::Sandbox::User", "");
         break;
       }
+
+      // if its the source file (e.g. local sources) we might be lucky
+      // by dropping the dropping only for some methods.
+      URI const source = (*I)->DescURI();
+      if (source.Access == "file" || source.Access == "copy")
+      {
+        std::string const conf = "Binary::" + source.Access + "::APT::Sandbox::User";
+        if (_config->Exists(conf) == true)
+           continue;
+
+        if (IsAccessibleBySandboxUser(source.Path, false) == false)
+        {
+           _error->NoticeE("pkgAcquire::Run", _("Can't drop privileges for downloading as file '%s' couldn't be accessed by user '%s'."),
+                 source.Path.c_str(), SandboxUser.c_str());
+           _config->CndSet("Binary::file::APT::Sandbox::User", "root");
+           _config->CndSet("Binary::copy::APT::Sandbox::User", "root");
+        }
+      }
    }
 
    if (seteuid(old_euid) != 0)
-      _error->Errno("seteuid", "seteuid %u failed", old_euid);
+      _error->FatalE("seteuid", "seteuid %u failed", old_euid);
    if (setegid(old_egid) != 0)
-      _error->Errno("setegid", "setegid %u failed", old_egid);
+      _error->FatalE("setegid", "setegid %u failed", old_egid);
+   if (setgroups(old_gidlist_nr, old_gidlist.get()))
+      _error->FatalE("setgroups", "setgroups %u failed", 0);
 }
 pkgAcquire::RunResult pkgAcquire::Run(int PulseIntervall)
 {
index e52c8f219c5baa9442e7630ceab172b335895966..537b3df496c8e08702850ac215861e6d88cf760a 100644 (file)
@@ -2280,7 +2280,7 @@ bool DropPrivileges()                                                     /*{{{*/
    // empty setting disables privilege dropping - this also ensures
    // backward compatibility, see bug #764506
    const std::string toUser = _config->Find("APT::Sandbox::User");
-   if (toUser.empty())
+   if (toUser.empty() || toUser == "root")
       return true;
 
    // uid will be 0 in the end, but gid might be different anyway
index 5c42c7e3cd2cc2efba9b0b531e9593e4838cf9b1..3ad3b395de8709a22cd00366cde21656a6472237 100755 (executable)
@@ -30,14 +30,8 @@ find aptarchive/dists -name '*Release*' -type f | while read file; do
        testaccessrights "$file" '640'
 done
 if [ "$(id -u)" = '0' ]; then
-       # permission errors an everything
-       testfailure aptget update
-
-       find aptarchive/dists -name '*Packages*' -type f | while read file; do
-               chmod 777 "$file"
-       done
-       # permission errors on Release
-       testwarning aptget update
+       # Release file can't be accessed by _apt
+       testsuccesswithnotice aptget update -q=0
 fi
 
 #everything (too) permissive
index c41713a923a0a26103ed13bb2b056c7602ea4dc1..3f1aee5a0cf5dc0f330d42adc856d2878016575e 100755 (executable)
@@ -103,3 +103,14 @@ createpkg 'trailing-newline' '' '
 testsuccess aptget install ./incoming/pkg-as-it-should-be_0_all.deb
 testsuccess aptget install ./incoming/pkg-leading-newline_0_all.deb
 testsuccess aptget install ./incoming/pkg-trailing-newline_0_all.deb
+
+# see if permission dropping is checked before usage
+if [ "$(id -u)" = '0' ]; then
+       apt clean
+       chmod 711 ./incoming
+       testsuccess aptget install -y --allow-downgrades ./incoming/pkg-as-it-should-be_0_all.deb -q=0
+       chmod 710 ./incoming
+       testsuccesswithnotice aptget install -y --allow-downgrades ./incoming/pkg-as-it-should-be_0_all.deb -q=0
+       chmod 700 ./incoming
+       testsuccesswithnotice aptget install -y --allow-downgrades ./incoming/pkg-as-it-should-be_0_all.deb -q=0
+fi
index 04e26a8f4c1d3e0e557653489e8b856b5535a8a9..78a8ca4054fe78dacd7b5fd1aa22b6656e332e64 100755 (executable)
@@ -14,6 +14,7 @@ configcompression 'bz2' 'gz'
 confighashes 'SHA512'
 
 insertpackage 'unstable' 'foo' 'all' '1'
+insertpackage 'unstable' 'bar' 'amd64' '1'
 insertsource 'unstable' 'foo' 'all' '1'
 
 setupaptarchive --no-update
@@ -21,8 +22,22 @@ setupaptarchive --no-update
 # ensure the archive is not writable
 addtrap 'prefix' 'chmod 755 aptarchive/dists/unstable/main/binary-all;'
 if [ "$(id -u)" = '0' ]; then
-       chmod 550 aptarchive/dists/unstable/main/binary-all
+       # too deep to notice it, but it also unlikely that files in the same repo have different permissions
+       chmod 500 aptarchive/dists/unstable/main/binary-all
        testfailure aptget update
+       rm -rf rootdir/var/lib/apt/lists
+       chmod 755 aptarchive/dists/unstable/main/binary-all
+       testsuccess aptget update
+       rm -rf rootdir/var/lib/apt/lists
+       chmod 511 aptarchive/dists/
+       testsuccess aptget update
+       rm -rf rootdir/var/lib/apt/lists
+       chmod 510 aptarchive/dists/
+       testsuccesswithnotice aptget update -q=0
+       rm -rf rootdir/var/lib/apt/lists
+       chmod 500 aptarchive/dists/
+       testsuccesswithnotice aptget update -q=0
+       exit
 fi
 chmod 555 aptarchive/dists/unstable/main/binary-all
 testsuccess aptget update
index 6300b5256378d56846478173af67995f602c260c..9fb580a011f7c79f68e7a316e3a876458816dbe3 100644 (file)
@@ -148,6 +148,7 @@ TEST(ConfigurationTest,Merge)
 {
        Configuration Cnf;
        Cnf.Set("Binary::apt::option::foo", "bar");
+       Cnf.Set("Binary::apt::option::empty", "");
        Cnf.Set("option::foo", "foo");
 
        Cnf.MoveSubTree("Binary::apt", "Binary::apt2");
@@ -156,8 +157,13 @@ TEST(ConfigurationTest,Merge)
        EXPECT_EQ("foo", Cnf.Find("option::foo"));
        EXPECT_EQ("bar", Cnf.Find("Binary::apt2::option::foo"));
 
+       EXPECT_FALSE(Cnf.Exists("option::empty"));
+       EXPECT_TRUE(Cnf.Exists("Binary::apt2::option::empty"));
+       Cnf.Set("option::empty", "not");
+
        Cnf.MoveSubTree("Binary::apt2", NULL);
        EXPECT_FALSE(Cnf.Exists("Binary::apt2::option"));
        EXPECT_TRUE(Cnf.Exists("option"));
        EXPECT_EQ("bar", Cnf.Find("option::foo"));
+       EXPECT_EQ("", Cnf.Find("option::empty"));
 }