diff options
author | David Kalnischkies <david@kalnischkies.de> | 2018-02-02 19:14:09 +0100 |
---|---|---|
committer | David Kalnischkies <david@kalnischkies.de> | 2018-02-19 15:56:09 +0100 |
commit | b3e7a16265e7c6c3b6892b9ec8a787d692ced6e6 (patch) | |
tree | 631acc6779df934c8cac78f4ffa5776ab7d75d2a | |
parent | 7aaf9b2c63aa8bdd87de4c19dcf1742c686a1cc2 (diff) |
ensure correct file permissions for auxfiles
The interesting takeaway here is perhaps that 'chmod +w' is effected by
the umask – obvious in hindsight of course. The usual setup helps with
hiding that applying that recursively on all directories (and files)
isn't correct. Ensuring files will not be stored with the wrong
permissions even if in strange umask contexts is trivial in comparison.
Fixing the test also highlighted that it wasn't bulletproof as apt will
automatically fix the permissions of the directories it works with, so
for this test we actually need to introduce a shortcut in the code.
Reported-By: Ubuntu autopkgtest CI
-rw-r--r-- | apt-pkg/acquire-item.cc | 4 | ||||
-rw-r--r-- | apt-pkg/acquire.cc | 2 | ||||
-rw-r--r-- | doc/examples/configure-index | 1 | ||||
-rwxr-xr-x | test/integration/test-method-mirror | 31 |
4 files changed, 31 insertions, 7 deletions
diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc index 792465b90..86ffe9e49 100644 --- a/apt-pkg/acquire-item.cc +++ b/apt-pkg/acquire-item.cc @@ -3622,7 +3622,7 @@ void pkgAcqChangelog::Init(std::string const &DestDir, std::string const &DestFi } TemporaryDirectory = tmpname; - ChangeOwnerAndPermissionOfFile("Item::QueueURI", TemporaryDirectory.c_str(), + ChangeOwnerAndPermissionOfFile("pkgAcqChangelog::Init", TemporaryDirectory.c_str(), SandboxUser.c_str(), ROOT_GROUP, 0700); DestFile = flCombine(TemporaryDirectory, DestFileName); @@ -3635,6 +3635,7 @@ void pkgAcqChangelog::Init(std::string const &DestDir, std::string const &DestFi if (file1.Open(DestFile, FileFd::WriteOnly | FileFd::Create | FileFd::Exclusive) && file2.Open(d->FinalFile, FileFd::ReadOnly) && CopyFile(file2, file1)) { + ChangeOwnerAndPermissionOfFile("pkgAcqChangelog::Init", DestFile.c_str(), "root", ROOT_GROUP, 0644); struct timeval times[2]; times[0].tv_sec = times[1].tv_sec = file2.ModificationTime(); times[0].tv_usec = times[1].tv_usec = 0; @@ -4007,6 +4008,7 @@ static std::string GetAuxFileNameFromURI(std::string const &uri) { FileFd out(filename, FileFd::WriteOnly | FileFd::Create | FileFd::Exclusive); CopyFile(in, out); + ChangeOwnerAndPermissionOfFile("GetAuxFileNameFromURI", filename.c_str(), "root", ROOT_GROUP, 0644); } _error->RevertToStack(); return filename; diff --git a/apt-pkg/acquire.cc b/apt-pkg/acquire.cc index ba895ed44..983a0fb41 100644 --- a/apt-pkg/acquire.cc +++ b/apt-pkg/acquire.cc @@ -80,6 +80,8 @@ void pkgAcquire::Initialize() // Acquire::GetLock - lock directory and prepare for action /*{{{*/ static bool SetupAPTPartialDirectory(std::string const &grand, std::string const &parent, std::string const &postfix, mode_t const mode) { + if (_config->FindB("Debug::SetupAPTPartialDirectory::AssumeGood", false)) + return true; std::string const partial = parent + postfix; bool const partialExists = DirectoryExists(partial); if (partialExists == false) diff --git a/doc/examples/configure-index b/doc/examples/configure-index index b5a0b5657..00c2ee183 100644 --- a/doc/examples/configure-index +++ b/doc/examples/configure-index @@ -536,6 +536,7 @@ Debug EDSP::WriteSolution "<BOOL>"; InstallProgress::Fancy "<BOOL>"; APT::Progress::PackageManagerFd "<BOOL>"; + SetupAPTPartialDirectory::AssumeGood "<BOOL>"; }; pkgCacheGen diff --git a/test/integration/test-method-mirror b/test/integration/test-method-mirror index a770f52ac..38d6be9a9 100755 --- a/test/integration/test-method-mirror +++ b/test/integration/test-method-mirror @@ -1,5 +1,6 @@ #!/bin/sh set -e +umask 0 TESTDIR="$(readlink -f "$(dirname "$0")")" . "$TESTDIR/framework" @@ -15,6 +16,8 @@ setupaptarchive --no-update changetowebserver webserverconfig 'aptwebserver::redirect::replace::/redirectme/' "http://localhost:${APTHTTPPORT}/" addtrap 'prefix' "chmod -f -R +w $PWD/rootdir/var/lib/apt/lists || true;" +mkdir -m 755 -p rootdir/var/lib/apt +touch rootdir/var/lib/apt/extended_states testrundownload_internal_debs() { local WORKDIR="$1" @@ -54,17 +57,27 @@ testrundownload() { if [ "$(id -u)" = '0' ]; then testrundownload_internal "$@" else - chmod -f -R -w rootdir/var/lib/apt/lists + mkdir -p rootdir/var/cache/apt/archives/partial + echo 'Debug::SetupAPTPartialDirectory::AssumeGood "true";' > rootdir/etc/apt/apt.conf.d/assumegooddirs.conf + chmod -f 555 rootdir/var/lib/apt/lists rootdir/var/lib/apt/lists/auxfiles rootdir/var/lib/apt/lists/partial testrundownload_internal "$@" - chmod -f -R +w rootdir/var/lib/apt/lists/auxfiles + testnotempty find rootdir/var/lib/apt/lists/auxfiles + testfilestats "${TMPWORKINGDIRECTORY}/rootdir/var/lib/apt/lists/auxfiles" '%U:%G:%a' '=' "${TEST_DEFAULT_USER}:${TEST_DEFAULT_GROUP}:555" + + chmod -f 755 rootdir/var/lib/apt/lists/auxfiles rm -f rootdir/var/lib/apt/lists/auxfiles/* - chmod -f -R -w rootdir/var/lib/apt/lists/auxfiles + chmod -f 555 rootdir/var/lib/apt/lists/auxfiles testrundownload_internal "$@" - chmod -f -R +w rootdir/var/lib/apt/lists + testfilestats "${TMPWORKINGDIRECTORY}/rootdir/var/lib/apt/lists/auxfiles" '%U:%G:%a' '=' "${TEST_DEFAULT_USER}:${TEST_DEFAULT_GROUP}:555" + testempty find rootdir/var/lib/apt/lists/auxfiles ! -type d + + chmod -f 755 rootdir/var/lib/apt/lists rm -rf rootdir/var/lib/apt/lists/auxfiles - chmod -f -R -w rootdir/var/lib/apt/lists + chmod -f 555 rootdir/var/lib/apt/lists testrundownload_internal "$@" - chmod -f -R +w rootdir/var/lib/apt/lists + chmod -f 755 rootdir/var/lib/apt/lists + testsuccess test ! -d rootdir/var/lib/apt/lists/auxfiles + rm -f rootdir/etc/apt/apt.conf.d/assumegooddirs.conf fi } testrun() { @@ -159,6 +172,7 @@ Ign:2 https://localhost:${APTHTTPSPORT}/failure unstable InRelease Hit:2 http://localhost:${APTHTTPPORT}/redirectme unstable InRelease Reading package lists... Building dependency tree... +Reading state information... All packages are up to date." "Get:1 file:${APTARCHIVE}/mirror.txt Mirrorlist [$(stat -c%s 'aptarchive/mirror.txt') B] Ign:2 https://localhost:${APTHTTPSPORT}/failure unstable InRelease 404 Not Found @@ -167,6 +181,7 @@ Ign:2 http://localhost:${APTHTTPPORT}/failure2 unstable InRelease Hit:2 http://localhost:${APTHTTPPORT}/failure2 unstable InRelease Reading package lists... Building dependency tree... +Reading state information... All packages are up to date." apt update testfailure grep '/unused-failure' aptarchive/webserver.log @@ -188,6 +203,7 @@ testsuccessequal "Get:1 file:${APTARCHIVE}/mirror.txt Mirrorlist [$(stat -c%s 'a Hit:2 http://localhost:${APTHTTPPORT}/redirectme unstable InRelease Reading package lists... Building dependency tree... +Reading state information... All packages are up to date." apt update testrundownload 'foo=2' sed -i -e 's# mirror+# foo+mirror+#' rootdir/etc/apt/sources.list.d/* @@ -198,6 +214,7 @@ testsuccessequal "Get:1 foo+file:${APTARCHIVE}/mirror.txt Mirrorlist [$(stat -c% Hit:2 foo+http://localhost:${APTHTTPPORT}/redirectme unstable InRelease Reading package lists... Building dependency tree... +Reading state information... All packages are up to date." apt update testrundownload 'foo=2' @@ -211,6 +228,7 @@ Ign:2 foo+file:/nonexistent/apt/archive unstable InRelease Hit:2 foo+http://localhost:${APTHTTPPORT}/redirectme unstable InRelease Reading package lists... Building dependency tree... +Reading state information... All packages are up to date." apt update sed -i -e "s#+file:${APTARCHIVE}#+http://localhost:${APTHTTPPORT}#" rootdir/etc/apt/sources.list.d/* testsuccess apt update @@ -220,5 +238,6 @@ testsuccessequal "Get:1 foo+http://localhost:${APTHTTPPORT}/mirror.txt Mirrorlis Hit:2 foo+http://localhost:${APTHTTPPORT}/redirectme unstable InRelease Reading package lists... Building dependency tree... +Reading state information... All packages are up to date." apt update testrundownload 'foo=2' |