don't change owner/perms/times through file:// symlinks

If we have files in partial/ from a previous invocation or similar such
those could be symlinks created by file:// sources. The code is
expecting only real files through and happily changes owner,
modification times and permission on the file the symlink points to
which tend to be files we have no business in touching in this way.
Permissions of symlinks shouldn't be changed, changing owner is usually
pointless to, but just to be sure we pick the easy way out and use
lchown, check for symlinks before chmod/utimes.

Reported-By: Mattia Rizzolo on IRC

diff --git a/apt-pkg/contrib/fileutl.cc b/apt-pkg/contrib/fileutl.cc
index c623f1a2a5bbac58d2b6c264d04018f76db7b5e4..e410f52d6f892e3648e1655cb2b81d9c1ec3c12b 100644 (file)
--- a/apt-pkg/contrib/fileutl.cc
+++ b/apt-pkg/contrib/fileutl.cc
@@ -916,9 +916,12 @@ bool ChangeOwnerAndPermissionOfFile(char const * const requester, char const * c
       // 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)
+      if (pw != NULL && gr != NULL && lchown(file, pw->pw_uid, gr->gr_gid) != 0)
         Res &= _error->WarningE(requester, "chown to %s:%s of file %s failed", user, group, file);
    }
+   struct stat Buf;
+   if (lstat(file, &Buf) != 0 || S_ISLNK(Buf.st_mode))
+      return Res;
    if (chmod(file, mode) != 0)
       Res &= _error->WarningE(requester, "chmod 0%o of file %s failed", mode, file);
    return Res;

diff --git a/methods/aptmethod.h b/methods/aptmethod.h
index bef61a8bc6c8a32a60952cf3c5af9233832d5087..d3c9486365f22be07115332c6842c591dbfcbc85 100644 (file)
--- a/methods/aptmethod.h
+++ b/methods/aptmethod.h
@@ -3,10 +3,18 @@
 
 #include <apt-pkg/acquire-method.h>
 #include <apt-pkg/configuration.h>
+#include <apt-pkg/error.h>
 
 #include <locale>
 #include <string>
 
+#include <sys/time.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
+
+#include <apti18n.h>
+
 class aptMethod : public pkgAcqMethod
 {
    char const * const Binary;
@@ -43,6 +51,29 @@ public:
       va_end(args);
    }
 
+   bool TransferModificationTimes(char const * const From, char const * const To, time_t &LastModified)
+   {
+      if (strcmp(To, "/dev/null") == 0)
+        return true;
+
+      struct stat Buf2;
+      if (lstat(To, &Buf2) != 0 || S_ISLNK(Buf2.st_mode))
+        return true;
+
+      struct stat Buf;
+      if (stat(From, &Buf) != 0)
+        return _error->Errno("stat",_("Failed to stat"));
+
+      // we don't use utimensat here for compatibility reasons: #738567
+      struct timeval times[2];
+      times[0].tv_sec = Buf.st_atime;
+      LastModified = times[1].tv_sec = Buf.st_mtime;
+      times[0].tv_usec = times[1].tv_usec = 0;
+      if (utimes(To, times) != 0)
+        return _error->Errno("utimes",_("Failed to set modification time"));
+      return true;
+   }
+
    aptMethod(char const * const Binary, char const * const Ver, unsigned long const Flags) :
       pkgAcqMethod(Ver, Flags), Binary(Binary)
    {

diff --git a/methods/copy.cc b/methods/copy.cc
index 5e36543897eb244637ad86c2d114d1902db5894d..810fc2f38356612649fd8d58e14b275639773262 100644 (file)
--- a/methods/copy.cc
+++ b/methods/copy.cc
@@ -78,13 +78,8 @@ bool CopyMethod::Fetch(FetchItem *Itm)
    From.Close();
    To.Close();
 
-   // Transfer the modification times
-   struct timeval times[2];
-   times[0].tv_sec = Buf.st_atime;
-   times[1].tv_sec = Buf.st_mtime;
-   times[0].tv_usec = times[1].tv_usec = 0;
-   if (utimes(Res.Filename.c_str(), times) != 0)
-      return _error->Errno("utimes",_("Failed to set modification time"));
+   if (TransferModificationTimes(File.c_str(), Res.Filename.c_str(), Res.LastModified) == false)
+      return false;
 
    CalculateHashes(Itm, Res);
    URIDone(Res);

diff --git a/methods/store.cc b/methods/store.cc
index 934e1a188bcb751c47a702687d0b5be118f0eca3..fa02d4597cadd22ca00bb0d1611b3111a7b09f6d 100644 (file)
--- a/methods/store.cc
+++ b/methods/store.cc
@@ -126,20 +126,8 @@ bool StoreMethod::Fetch(FetchItem *Itm)                                    /*{{{*/
    if (Failed == true)
       return false;
 
-   // Transfer the modification times
-   if (Itm->DestFile != "/dev/null")
-   {
-      struct stat Buf;
-      if (stat(Path.c_str(),&Buf) != 0)
-        return _error->Errno("stat",_("Failed to stat"));
-
-      struct timeval times[2];
-      times[0].tv_sec = Buf.st_atime;
-      Res.LastModified = times[1].tv_sec = Buf.st_mtime;
-      times[0].tv_usec = times[1].tv_usec = 0;
-      if (utimes(Itm->DestFile.c_str(), times) != 0)
-        return _error->Errno("utimes",_("Failed to set modification time"));
-   }
+   if (TransferModificationTimes(Path.c_str(), Itm->DestFile.c_str(), Res.LastModified) == false)
+      return false;
 
    // Return a Done response
    Res.TakeHashes(Hash);

diff --git a/test/integration/framework b/test/integration/framework
index fe6f82ac9779f5effeb27e0ed739c435a1ed7eaa..ea9402d61a8a03bc7541d2acf72f5ecff5ec6d88 100644 (file)
--- a/test/integration/framework
+++ b/test/integration/framework
@@ -1886,6 +1886,11 @@ pause() {
        read IGNORE
 }
 
+logcurrentarchivedirectory() {
+       find "${TMPWORKINGDIRECTORY}/aptarchive/dists" -type f | while read line; do
+               stat --format '%U:%G:%a:%n' "$line"
+       done | sort > "${TMPWORKINGDIRECTORY}/rootdir/var/log/aptgetupdate.before.lst"
+}
 listcurrentlistsdirectory() {
        {
                find rootdir/var/lib/apt/lists -maxdepth 1 -type d | while read line; do
@@ -1964,6 +1969,10 @@ aptautotest_aptget_update() {
                # failure cases can retain partial files and such
                testempty find "${TMPWORKINGDIRECTORY}/rootdir/var/lib/apt/lists/partial" -mindepth 1 ! \( -name 'lock' -o -name '*.FAILED' \)
        fi
+       if [ -s "${TMPWORKINGDIRECTORY}/rootdir/var/log/aptgetupdate.before.lst" ]; then
+               testfileequal "${TMPWORKINGDIRECTORY}/rootdir/var/log/aptgetupdate.before.lst" \
+                       "$(find "${TMPWORKINGDIRECTORY}/aptarchive/dists" -type f | while read line; do stat --format '%U:%G:%a:%n' "$line"; done | sort)"
+       fi
 }
 aptautotest_apt_update() { aptautotest_aptget_update "$@"; }
 aptautotest_aptcdrom_add() { aptautotest_aptget_update "$@"; }

diff --git a/test/integration/test-apt-update-file b/test/integration/test-apt-update-file
index 20f6046957fd95b36c2da7c6349b673c52ba1964..8da4ec35b352f3c883c38c92b75bf8a17db0f864 100755 (executable)
--- a/test/integration/test-apt-update-file
+++ b/test/integration/test-apt-update-file
@@ -18,6 +18,7 @@ insertpackage 'unstable' 'bar' 'amd64' '1'
 insertsource 'unstable' 'foo' 'all' '1'
 
 setupaptarchive --no-update
+logcurrentarchivedirectory
 
 # ensure the archive is not writable
 addtrap 'prefix' 'chmod 755 aptarchive/dists/unstable/main/binary-all;'
@@ -37,8 +38,11 @@ if [ "$(id -u)" = '0' ]; then
        rm -rf rootdir/var/lib/apt/lists
        chmod 500 aptarchive/dists/
        testsuccesswithnotice aptget update
-       exit
+       chmod 755 aptarchive/dists/
+else
+       testsuccess aptget update
 fi
+mv rootdir/var/lib/apt/lists/_* rootdir/var/lib/apt/lists/partial
 chmod 555 aptarchive/dists/unstable/main/binary-all
 testsuccess aptget update -o Debug::pkgAcquire::Worker=1
 cp -a rootdir/tmp/testsuccess.output rootdir/tmp/update.output

diff --git a/test/integration/test-apt-update-ims b/test/integration/test-apt-update-ims
index f118f7c11b269b38cad662220ced7ca2c891c21a..1894c3adfeafb8c15c0e27c1a859f658943de115 100755 (executable)
--- a/test/integration/test-apt-update-ims
+++ b/test/integration/test-apt-update-ims
@@ -12,6 +12,7 @@ insertsource 'unstable' 'unrelated' 'all' '0.5~squeeze1'
 
 export APT_DONT_SIGN=""
 setupaptarchive --no-update
+logcurrentarchivedirectory
 changetowebserver
 
 runtest() {
@@ -71,6 +72,7 @@ EXPECT="Ign:1 http://localhost:${APTHTTPPORT} unstable InRelease
 Hit:2 http://localhost:${APTHTTPPORT} unstable Release
 Reading package lists..."
 find aptarchive -name 'InRelease' -delete
+logcurrentarchivedirectory
 echo 'Acquire::GzipIndexes "0";' > rootdir/etc/apt/apt.conf.d/02compressindex
 runtest
 echo 'Acquire::GzipIndexes "1";' > rootdir/etc/apt/apt.conf.d/02compressindex
@@ -87,6 +89,7 @@ W: The repository 'http://localhost:${APTHTTPPORT} unstable Release' is not sign
 N: Data from such a repository can't be authenticated and is therefore potentially dangerous to use.
 N: See apt-secure(8) manpage for repository creation and user configuration details."
 find aptarchive -name 'Release.gpg' -delete
+logcurrentarchivedirectory
 echo 'Acquire::GzipIndexes "0";' > rootdir/etc/apt/apt.conf.d/02compressindex
 runtest 'warning'
 echo 'Acquire::GzipIndexes "1";' > rootdir/etc/apt/apt.conf.d/02compressindex
@@ -99,6 +102,7 @@ find aptarchive -name '*Release' -exec sed -i \
        -e '/^Valid-Until: / d' -e "/^Date: / a\
 Valid-Until: $(date -ud '-1 weeks' '+%a, %d %b %Y %H:%M:%S %Z')" '{}' \;
 signreleasefiles
+logcurrentarchivedirectory
 
 msgmsg 'expired InRelease'
 EXPECT="Hit:1 http://localhost:${APTHTTPPORT} unstable InRelease
@@ -116,6 +120,7 @@ Hit:2 http://localhost:${APTHTTPPORT} unstable Release
 Reading package lists...
 E: Release file for http://localhost:${APTHTTPPORT}/dists/unstable/Release is expired (invalid since). Updates for this repository will not be applied."
 find aptarchive -name 'InRelease' -delete
+logcurrentarchivedirectory
 echo 'Acquire::GzipIndexes "0";' > rootdir/etc/apt/apt.conf.d/02compressindex
 runtest 'failure'
 echo 'Acquire::GzipIndexes "1";' > rootdir/etc/apt/apt.conf.d/02compressindex
@@ -133,6 +138,7 @@ N: Data from such a repository can't be authenticated and is therefore potential
 N: See apt-secure(8) manpage for repository creation and user configuration details.
 E: Release file for http://localhost:${APTHTTPPORT}/dists/unstable/Release is expired (invalid since). Updates for this repository will not be applied."
 find aptarchive -name 'Release.gpg' -delete
+logcurrentarchivedirectory
 echo 'Acquire::GzipIndexes "0";' > rootdir/etc/apt/apt.conf.d/02compressindex
 runtest 'failure' 'warning'
 echo 'Acquire::GzipIndexes "1";' > rootdir/etc/apt/apt.conf.d/02compressindex
@@ -177,6 +183,7 @@ W: The repository 'http://localhost:${APTHTTPPORT} unstable Release' does not ha
 N: Data from such a repository can't be authenticated and is therefore potentially dangerous to use.
 N: See apt-secure(8) manpage for repository creation and user configuration details."
 find aptarchive -name '*Release*' -delete
+logcurrentarchivedirectory
 echo 'Acquire::GzipIndexes "0";
 Acquire::PDiffs "0";' > rootdir/etc/apt/apt.conf.d/02compressindex
 runtest 'warning'