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.

diff --git a/apt-pkg/acquire-worker.cc b/apt-pkg/acquire-worker.cc
index f4d1ad412fdc3c82ad8864fed14d271c49f9bfc1..bee01e6200777c5b2a5a6793b05b0cc63d5c91db 100644 (file)
--- 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 5eccbe5de55ecda2a6e299029a27f113c759ef01..8cec4b78edf0e5dc9bd36c67036e6556a7b3b700 100644 (file)
--- 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<CdromDevice>
-pkgUdevCdromDevices::Scan()
+// convenience interface, this will just call ScanForRemovable         /*{{{*/
+vector<CdromDevice> pkgUdevCdromDevices::Scan()
 {
    bool CdromOnly = _config->FindB("APT::cdrom::CdromOnly", true);
    return ScanForRemovable(CdromOnly);
 }
                                                                        /*}}}*/
-                                                                        /*{{{*/
-vector<CdromDevice>
-pkgUdevCdromDevices::ScanForRemovable(bool CdromOnly)
+vector<CdromDevice> pkgUdevCdromDevices::ScanForRemovable(bool CdromOnly)/*{{{*/
 {
    vector<CdromDevice> cdrom_devices;
    struct udev_enumerate *enumerate;

diff --git a/apt-pkg/contrib/fileutl.cc b/apt-pkg/contrib/fileutl.cc
index 47033eadff01d0f6edac3a912122aa496badb3f1..afc243b7f7dfe8442a8ddd68f0b2165979df3d72 100644 (file)
--- 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 a64d6cb98bc1d76b92cf1748851e3fe903a9f597..97cb05c56ffed3ffc0ddd3b99b059e4d7d2c7490 100644 (file)
--- a/apt-pkg/contrib/fileutl.h
+++ b/apt-pkg/contrib/fileutl.h
@@ -195,10 +195,21 @@ pid_t ExecFork(std::set<int> keep_fds);
 void MergeKeepFdsFromConfiguration(std::set<int> &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 5fa57fd8b0ab2c4eecc478ccec5109c97d9c9059..144c508bef3b7fe8d4bacc255cf417876ccd2d4b 100644 (file)
--- a/apt-pkg/indexcopy.cc
+++ b/apt-pkg/indexcopy.cc
@@ -216,6 +216,7 @@ bool IndexCopy::CopyPackages(string CDROM,string Name,vector<string> &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 994956b74c27de307efff72e63f502b26e834349..642c5f0d0ae77300c771a75e80eb17ac012578eb 100644 (file)
--- 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 9906795ca9115f2c90831d6717b10820babc167e..34b35f745af8ec4ca2f96873ac9dc611cfcde7ca 100755 (executable)
--- 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