summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--apt-pkg/acquire-worker.cc2
-rw-r--r--apt-pkg/acquire.cc113
-rw-r--r--apt-pkg/contrib/fileutl.cc2
-rwxr-xr-xtest/integration/test-apt-get-download10
-rwxr-xr-xtest/integration/test-apt-get-install-deb11
-rwxr-xr-xtest/integration/test-apt-update-file17
-rw-r--r--test/libapt/configuration_test.cc6
7 files changed, 121 insertions, 40 deletions
diff --git a/apt-pkg/acquire-worker.cc b/apt-pkg/acquire-worker.cc
index b5f52a3ca..c2ee8cda3 100644
--- a/apt-pkg/acquire-worker.cc
+++ b/apt-pkg/acquire-worker.cc
@@ -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);
}
diff --git a/apt-pkg/acquire.cc b/apt-pkg/acquire.cc
index 81faee5d8..fb1210750 100644
--- a/apt-pkg/acquire.cc
+++ b/apt-pkg/acquire.cc
@@ -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)
{
diff --git a/apt-pkg/contrib/fileutl.cc b/apt-pkg/contrib/fileutl.cc
index e52c8f219..537b3df49 100644
--- a/apt-pkg/contrib/fileutl.cc
+++ b/apt-pkg/contrib/fileutl.cc
@@ -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
diff --git a/test/integration/test-apt-get-download b/test/integration/test-apt-get-download
index 5c42c7e3c..3ad3b395d 100755
--- a/test/integration/test-apt-get-download
+++ b/test/integration/test-apt-get-download
@@ -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
diff --git a/test/integration/test-apt-get-install-deb b/test/integration/test-apt-get-install-deb
index c41713a92..3f1aee5a0 100755
--- a/test/integration/test-apt-get-install-deb
+++ b/test/integration/test-apt-get-install-deb
@@ -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
diff --git a/test/integration/test-apt-update-file b/test/integration/test-apt-update-file
index 04e26a8f4..78a8ca405 100755
--- a/test/integration/test-apt-update-file
+++ b/test/integration/test-apt-update-file
@@ -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
diff --git a/test/libapt/configuration_test.cc b/test/libapt/configuration_test.cc
index 6300b5256..9fb580a01 100644
--- a/test/libapt/configuration_test.cc
+++ b/test/libapt/configuration_test.cc
@@ -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"));
}
generated by cgit v1.2.3 (git 2.25.1) at 2024-05-26 02:11:52 +0000