From d84da4995df24329e96d57a22136683a9e370f4e Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Sat, 11 Apr 2015 20:13:19 +0200 Subject: ensure lists/ files have correct permissions after apt-cdrom add Its a bit unpredictable which permissons and owners we will encounter on a CD-ROM (or a USB stick, as apt-cdrom is responsible for those too), so we have to ensure in this codepath as well that everything is nicely setup without waiting for a 'apt-get update' to fix up the (potential) mess. --- apt-pkg/acquire-worker.cc | 14 -------------- apt-pkg/cdrom.cc | 13 ++++--------- apt-pkg/contrib/fileutl.cc | 19 +++++++++++++++++++ apt-pkg/contrib/fileutl.h | 13 ++++++++++++- apt-pkg/indexcopy.cc | 5 ++++- test/integration/framework | 1 + test/integration/test-apt-cdrom | 5 +++-- 7 files changed, 43 insertions(+), 27 deletions(-) diff --git a/apt-pkg/acquire-worker.cc b/apt-pkg/acquire-worker.cc index f4d1ad412..bee01e620 100644 --- a/apt-pkg/acquire-worker.cc +++ b/apt-pkg/acquire-worker.cc @@ -43,20 +43,6 @@ using namespace std; -static void ChangeOwnerAndPermissionOfFile(char const * const requester, char const * const file, char const * const user, char const * const group, mode_t const mode) /*{{{*/ -{ - if (getuid() == 0 && strlen(user) != 0 && strlen(group) != 0) // if we aren't root, we can't chown, so don't try it - { - // ensure the file is owned by root and has good permissions - struct passwd const * const pw = getpwnam(user); - struct group const * const gr = getgrnam(group); - if (pw != NULL && gr != NULL && chown(file, pw->pw_uid, gr->gr_gid) != 0) - _error->WarningE(requester, "chown to %s:%s of file %s failed", user, group, file); - } - if (chmod(file, mode) != 0) - _error->WarningE(requester, "chmod 0%o of file %s failed", mode, file); -} - /*}}}*/ // Worker::Worker - Constructor for Queue startup /*{{{*/ // --------------------------------------------------------------------- /* */ diff --git a/apt-pkg/cdrom.cc b/apt-pkg/cdrom.cc index 5eccbe5de..8cec4b78e 100644 --- a/apt-pkg/cdrom.cc +++ b/apt-pkg/cdrom.cc @@ -927,8 +927,7 @@ pkgUdevCdromDevices::pkgUdevCdromDevices() /*{{{*/ } /*}}}*/ -bool -pkgUdevCdromDevices::Dlopen() /*{{{*/ +bool pkgUdevCdromDevices::Dlopen() /*{{{*/ { // alread open if(libudev_handle != NULL) @@ -957,18 +956,14 @@ pkgUdevCdromDevices::Dlopen() /*{{{*/ return true; } /*}}}*/ - /*{{{*/ -// convenience interface, this will just call ScanForRemovable -vector -pkgUdevCdromDevices::Scan() +// convenience interface, this will just call ScanForRemovable /*{{{*/ +vector pkgUdevCdromDevices::Scan() { bool CdromOnly = _config->FindB("APT::cdrom::CdromOnly", true); return ScanForRemovable(CdromOnly); } /*}}}*/ - /*{{{*/ -vector -pkgUdevCdromDevices::ScanForRemovable(bool CdromOnly) +vector pkgUdevCdromDevices::ScanForRemovable(bool CdromOnly)/*{{{*/ { vector cdrom_devices; struct udev_enumerate *enumerate; diff --git a/apt-pkg/contrib/fileutl.cc b/apt-pkg/contrib/fileutl.cc index 47033eadf..afc243b7f 100644 --- a/apt-pkg/contrib/fileutl.cc +++ b/apt-pkg/contrib/fileutl.cc @@ -874,6 +874,25 @@ bool StartsWithGPGClearTextSignature(string const &FileName) return true; } /*}}}*/ +// ChangeOwnerAndPermissionOfFile - set file attributes to requested values /*{{{*/ +bool ChangeOwnerAndPermissionOfFile(char const * const requester, char const * const file, char const * const user, char const * const group, mode_t const mode) +{ + if (strcmp(file, "/dev/null") == 0) + return true; + bool Res = true; + if (getuid() == 0 && strlen(user) != 0 && strlen(group) != 0) // if we aren't root, we can't chown, so don't try it + { + // ensure the file is owned by root and has good permissions + struct passwd const * const pw = getpwnam(user); + struct group const * const gr = getgrnam(group); + if (pw != NULL && gr != NULL && chown(file, pw->pw_uid, gr->gr_gid) != 0) + Res &= _error->WarningE(requester, "chown to %s:%s of file %s failed", user, group, file); + } + if (chmod(file, mode) != 0) + Res &= _error->WarningE(requester, "chmod 0%o of file %s failed", mode, file); + return Res; +} + /*}}}*/ class FileFdPrivate { /*{{{*/ public: diff --git a/apt-pkg/contrib/fileutl.h b/apt-pkg/contrib/fileutl.h index a64d6cb98..97cb05c56 100644 --- a/apt-pkg/contrib/fileutl.h +++ b/apt-pkg/contrib/fileutl.h @@ -195,10 +195,21 @@ pid_t ExecFork(std::set keep_fds); void MergeKeepFdsFromConfiguration(std::set &keep_fds); bool ExecWait(pid_t Pid,const char *Name,bool Reap = false); - // check if the given file starts with a PGP cleartext signature bool StartsWithGPGClearTextSignature(std::string const &FileName); +/** change file attributes to requested known good values + * + * The method skips the user:group setting if not root. + * + * @param requester is printed as functionname in error cases + * @param file is the file to be modified + * @param user is the (new) owner of the file, e.g. _apt + * @param group is the (new) group owning the file, e.g. root + * @param mode is the access mode of the file, e.g. 0644 + */ +bool ChangeOwnerAndPermissionOfFile(char const * const requester, char const * const file, char const * const user, char const * const group, mode_t const mode); + /** * \brief Drop privileges * diff --git a/apt-pkg/indexcopy.cc b/apt-pkg/indexcopy.cc index 5fa57fd8b..144c508be 100644 --- a/apt-pkg/indexcopy.cc +++ b/apt-pkg/indexcopy.cc @@ -216,6 +216,7 @@ bool IndexCopy::CopyPackages(string CDROM,string Name,vector &List, FinalF += URItoFileName(S); if (rename(TargetF.c_str(),FinalF.c_str()) != 0) return _error->Errno("rename","Failed to rename"); + ChangeOwnerAndPermissionOfFile("CopyPackages", FinalF.c_str(), "root", "root", 0644); } /* Mangle the source to be in the proper notation with @@ -546,8 +547,9 @@ bool SigVerify::CopyMetaIndex(string CDROM, string CDName, /*{{{*/ FileFd Rel; Target.Open(TargetF,FileFd::WriteAtomic); Rel.Open(prefix + file,FileFd::ReadOnly); - if (CopyFile(Rel,Target) == false) + if (CopyFile(Rel,Target) == false || Target.Close() == false) return _error->Error("Copying of '%s' for '%s' from '%s' failed", file.c_str(), CDName.c_str(), prefix.c_str()); + ChangeOwnerAndPermissionOfFile("CopyPackages", TargetF.c_str(), "root", "root", 0644); return true; } @@ -760,6 +762,7 @@ bool TranslationsCopy::CopyTranslations(string CDROM,string Name, /*{{{*/ FinalF += URItoFileName(S); if (rename(TargetF.c_str(),FinalF.c_str()) != 0) return _error->Errno("rename","Failed to rename"); + ChangeOwnerAndPermissionOfFile("CopyTranslations", FinalF.c_str(), "root", "root", 0644); } diff --git a/test/integration/framework b/test/integration/framework index 994956b74..642c5f0d0 100644 --- a/test/integration/framework +++ b/test/integration/framework @@ -1525,6 +1525,7 @@ aptautotest_aptget_update() { done } aptautotest_apt_update() { aptautotest_aptget_update "$@"; } +aptautotest_aptcdrom_add() { aptautotest_aptget_update "$@"; } testaptautotestnodpkgwarning() { local TESTCALL="$1" diff --git a/test/integration/test-apt-cdrom b/test/integration/test-apt-cdrom index 9906795ca..34b35f745 100755 --- a/test/integration/test-apt-cdrom +++ b/test/integration/test-apt-cdrom @@ -33,6 +33,7 @@ aptcdromlog() { test ! -e rootdir/media/cdrom || echo "CD-ROM is mounted, but shouldn't be!" test -e rootdir/media/cdrom-unmounted || echo "Unmounted CD-ROM doesn't exist, but it should!" } +aptautotest_aptcdromlog_add() { aptautotest_aptget_update "$@"; } CDROM_PRE="Using CD-ROM mount point $(readlink -f ./rootdir/media)/cdrom/ Unmounting CD-ROM... @@ -133,13 +134,13 @@ aptcache show testing -o Acquire::Languages=en | grep -q '^Description-en: ' && # ensure cdrom method isn't trying to mount the cdrom mv rootdir/media/cdrom-unmounted rootdir/media/cdrom-ejected -# ensure an update doesn't mess with cdrom sources +msgmsg "ensure an update doesn't mess with cdrom sources" testsuccess aptget update testfileequal rootdir/tmp/testsuccess.output 'Reading package lists...' mv rootdir/media/cdrom-ejected rootdir/media/cdrom-unmounted testcdromusage -# and again to check that it withstands the temptation even if it could mount +msgmsg 'and again to check that it withstands the temptation even if it could mount' testsuccess aptget update testfileequal rootdir/tmp/testsuccess.output 'Reading package lists...' testcdromusage -- cgit v1.2.3