Browse Source

improve CheckDropPrivsMustBeDisabled further

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

Git-Dch: Ignore
tags/debian/1.1.exp12
David Kalnischkies 6 years ago
parent
commit
226c0f64d4
3 changed files with 40 additions and 16 deletions
  1. +22
    -8
      apt-pkg/acquire.cc
  2. +6
    -4
      apt-pkg/contrib/fileutl.cc
  3. +12
    -4
      test/integration/test-http-pipeline-messup

+ 22
- 8
apt-pkg/acquire.cc View File

@@ -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)
{


+ 6
- 4
apt-pkg/contrib/fileutl.cc View File

@@ -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;
}


+ 12
- 4
test/integration/test-http-pipeline-messup View File

@@ -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



Loading…
Cancel
Save