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

@@ -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			/*{{{*/
 // ---------------------------------------------------------------------
 /* */

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;

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:

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
  *

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

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"

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