[PATCH]: restore permissions and mode on HISTFILE when renaming it

[PATCH]: restore permissions and mode on HISTFILE when renaming it

[Arkadiusz Miskiewicz]

[-- Attachment #1: Type: text/plain, Size: 859 bytes --]

Hi,

There is one annoying thing in handling history file in zsh. When it uses 
tmpfile (not append mode) and just does rename(".historyz.new", ".history") 
it looses all permissions and user/owner of the original .historyz file.

Exaple, I'm working as user arekm and .historyz has arekm:users user/group but 
if I do sudo zsh; ls; exit then it's renamed and I end up with .historyz 
being root:root and since I've exited sudo and I'm back as arekm:users my 
history is no longer saved.

Attached patch makes zsh restore original permissions/owner/group of HISTFILE 
after sucessful rename. It's against HEAD (4.3) cvs version.

Any races here (between stat and chown/chmod) shouldn't be unsafe I guess.

Thanks!
-- 
Arkadiusz Miśkiewicz                    PLD/Linux Team
http://www.t17.ds.pwr.wroc.pl/~misiek/  http://ftp.pld-linux.org/

[-- Attachment #2: zsh-restore-histfile.patch --]
[-- Type: text/x-diff, Size: 667 bytes --]

--- zsh.org/Src/hist.c.org	2005-12-16 10:51:24.680963000 +0100
+++ zsh/Src/hist.c	2005-12-16 10:59:14.100963000 +0100
@@ -2127,8 +2127,18 @@
 	}
 	fclose(out);
 	if (tmpfile) {
+	    struct stat sb;
+	    int restore = 0;
+	    if (stat(unmeta(fn), &sb) == 0)
+		    restore = 1;
 	    if (rename(tmpfile, unmeta(fn)) < 0)
 		zerr("can't rename %s.new to $HISTFILE", fn, 0);
+	    else if (restore) {
+		    if (chown(unmeta(fn), sb.st_uid, sb.st_gid) < 0)
+			    zerr("can't restore user/group on $HISTFILE", NULL, 0);
+		    if (chmod(unmeta(fn), sb.st_mode) < 0)
+			    zerr("can't restore permissions on $HISTFILE", NULL, 0);
+	    }
 	    free(tmpfile);
 	}
 

Re: [PATCH]: restore permissions and mode on HISTFILE when renaming it

[Bart Schaefer]

On Dec 16,  9:42am, Arkadiusz Miskiewicz wrote:
}
} Exaple, I'm working as user arekm and .historyz has arekm:users
} user/group but if I do sudo zsh; ls; exit then it's renamed and I end
} up with .historyz being root:root

Ick.  I don't think it's appropriate for root's history to get written
into some other user's SAVEHIST file.  A better solution to this would
probably be to skip writing the history entirely if the file owner is
not the same as the current EUID.

} Attached patch makes zsh restore original permissions/owner/group of
} HISTFILE after sucessful rename. It's against HEAD (4.3) cvs version.

Even if that approach is chosen, the race conditions you mentioned
could be avoided by changing the file attributes *before* renaming it.

Re: [PATCH]: restore permissions and mode on HISTFILE when renaming it

[Wayne Davison]

[-- Attachment #1: Type: text/plain, Size: 795 bytes --]

On Fri, Dec 16, 2005 at 09:42:29AM +0100, Arkadiusz Miskiewicz wrote:
> rename(".historyz.new", ".historyz") [loses] all permissions and
> user/owner of the original .historyz file.

I think that having rsync maintain the current permissions and group is
a desirable feature.  I also like Bart's idea that zsh should refuse to
re-write the history file (with a warning) if it is currently owned by
some other user.  The attached patch does this.  (If folks like the
idea for the upcoming release, I'll go ahead and commit it.)

As for the HISTFILE behavior when using sudo, I have code in my .zshrc
that makes sure that root writes out its history into its own history
file.  You can do this by conditionally redefining HISTFILE based on the
current user (among other possibilities).

..wayne..

[-- Attachment #2: preserve.patch --]
[-- Type: text/plain, Size: 1312 bytes --]

--- configure.ac	9 Dec 2005 19:20:02 -0000	1.46
+++ configure.ac	16 Dec 2005 18:29:59 -0000
@@ -1098,7 +1098,7 @@ dnl AC_FUNC_STRFTIME
 AC_CHECK_FUNCS(strftime difftime gettimeofday \
 	       select poll \
 	       readlink faccessx fchdir ftruncate \
-	       fstat lstat lchown \
+	       fstat lstat lchown fchown fchmod \
 	       fseeko ftello \
 	       mkfifo _mktemp mkstemp \
 	       waitpid wait3 \
--- Src/hist.c	4 Nov 2005 16:20:34 -0000	1.61
+++ Src/hist.c	16 Dec 2005 18:29:59 -0000
@@ -2080,8 +2080,25 @@ savehistfile(char *fn, int err, int writ
 	tmpfile = bicat(unmeta(fn), ".new");
 	if (unlink(tmpfile) < 0 && errno != ENOENT)
 	    out = NULL;
-	else
-	    out = fdopen(open(tmpfile, O_CREAT | O_WRONLY | O_EXCL, 0600), "w");
+	else {
+	    struct stat sb;
+	    int old_exists = stat(unmeta(fn), &sb) == 0;
+
+	    if (old_exists && sb.st_uid != geteuid()) {
+		tmpfile = NULL; /* Avoid an error about HISTFILE.new */
+		out = NULL;
+	    } else
+		out = fdopen(open(tmpfile, O_CREAT | O_WRONLY | O_EXCL, 0600), "w");
+
+#ifdef HAVE_FCHMOD
+	    if (old_exists && out) {
+#ifdef HAVE_FCHOWN
+		fchown(fileno(out), sb.st_uid, sb.st_gid);
+#endif
+		fchmod(fileno(out), sb.st_mode);
+	    }
+#endif
+	}
     }
     if (out) {
 	for (; he && he->histnum <= xcurhist; he = down_histent(he)) {

Re: [PATCH]: restore permissions and mode on HISTFILE when renaming it

[Peter Stephenson]

Wayne Davison <wayned@users.sourceforge.net> wrote:
> I think that having rsync maintain the current permissions and group is
> a desirable feature.  I also like Bart's idea that zsh should refuse to
> re-write the history file (with a warning) if it is currently owned by
> some other user.  The attached patch does this.  (If folks like the
> idea for the upcoming release, I'll go ahead and commit it.)

Seems OK to me.

> As for the HISTFILE behavior when using sudo, I have code in my .zshrc
> that makes sure that root writes out its history into its own history
> file.  You can do this by conditionally redefining HISTFILE based on the
> current user (among other possibilities).

Yes, I think that's the right way to go (we just started using sudo a
couple of weeks ago, coincidentally, and that's what I did).  I admit it's
a bit annoying the first time.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070


This message has been scanned for viruses by BlackSpider MailControl - www.blackspider.com