GNU bug report logs -
#27986
26.0.50; `rename-file' can rename files without confirmation
Previous Next
Reported by: Philipp <p.stephani2 <at> gmail.com>
Date: Sun, 6 Aug 2017 15:41:02 UTC
Severity: important
Tags: security
Found in version 26.0.50
Done: Paul Eggert <eggert <at> cs.ucla.edu>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 27986 in the body.
You can then email your comments to 27986 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Sun, 06 Aug 2017 15:41:02 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Philipp <p.stephani2 <at> gmail.com>:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org.
(Sun, 06 Aug 2017 15:41:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
Run the following in *scratch*:
(make-directory "/tmp/emacs")
nil
(write-region "" nil "/tmp/emacs/ß")
nil
(write-region "" nil "/tmp/emacs/ẞ")
nil
(directory-files "/tmp/emacs")
("." ".." "ß" "ẞ")
(rename-file "/tmp/emacs/ẞ" "/tmp/emacs/ß")
nil
Note how `rename-file' has silently overwritten `ß'. This is because on
macOS, `ß' and `ẞ' are different file names, but Emacs treats them as
equal. Probably the test for case-insensitive file names should be
removed altogether (it can't work correctly and introduces a filesystem
race), and `rename-file' should use link(2) + unlink(2) if renameat2
isn't available.
In GNU Emacs 26.0.50 (build 77, x86_64-apple-darwin16.7.0, NS appkit-1504.83 Version 10.12.6 (Build 16G29))
of 2017-08-06 built on p
Repository revision: b1b99edd3ee587a5154106d4d16547eac4916c55
Windowing system distributor 'Apple', version 10.3.1504
Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Configured using:
'configure --with-modules --without-xml2 --without-pop --with-mailutils
--enable-gcc-warnings=yes MAKEINFO=/usr/local/opt/texinfo/bin/makeinfo
'CFLAGS=-O3 -g0' LDFLAGS=-O3'
Configured features:
DBUS NOTIFY ACL GNUTLS ZLIB TOOLKIT_SCROLL_BARS NS MODULES
Important settings:
value of $LANG: de_DE.UTF-8
locale-coding-system: utf-8-unix
Major mode: Lisp Interaction
Minor modes in effect:
tooltip-mode: t
global-eldoc-mode: t
eldoc-mode: t
electric-indent-mode: t
mouse-wheel-mode: t
tool-bar-mode: t
menu-bar-mode: t
file-name-shadow-mode: t
global-font-lock-mode: t
font-lock-mode: t
blink-cursor-mode: t
auto-composition-mode: t
auto-encryption-mode: t
auto-compression-mode: t
line-number-mode: t
transient-mark-mode: t
Load-path shadows:
None found.
Features:
(shadow sort mail-extr emacsbug message subr-x puny seq byte-opt gv
bytecomp byte-compile cconv cl-loaddefs cl-lib dired dired-loaddefs
format-spec rfc822 mml easymenu mml-sec password-cache epa derived epg
epg-config gnus-util rmail rmail-loaddefs mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047
rfc2045 ietf-drums mm-util mail-prsvr mail-utils time-date tooltip eldoc
electric uniquify ediff-hook vc-hooks lisp-float-type mwheel term/ns-win
ns-win ucs-normalize mule-util term/common-win tool-bar dnd fontset
image regexp-opt fringe tabulated-list replace newcomment text-mode
elisp-mode lisp-mode prog-mode register page menu-bar rfn-eshadow
isearch timer select scroll-bar mouse jit-lock font-lock syntax facemenu
font-core term/tty-colors frame cl-generic cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese composite charscript charprop case-table epa-hook
jka-cmpr-hook help simple abbrev obarray minibuffer cl-preloaded nadvice
loaddefs button faces cus-face macroexp files text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote dbusbind kqueue cocoa ns multi-tty
make-network-process emacs)
Memory information:
((conses 16 204143 9065)
(symbols 48 20146 1)
(miscs 40 43 181)
(strings 32 29176 1731)
(string-bytes 1 784929)
(vectors 16 35010)
(vector-slots 8 710995 9656)
(floats 8 48 68)
(intervals 56 205 0)
(buffers 992 11))
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Sun, 06 Aug 2017 17:06:02 GMT)
Full text and
rfc822 format available.
Message #8 received at 27986 <at> debbugs.gnu.org (full text, mbox):
> From: Philipp <p.stephani2 <at> gmail.com>
> Date: Sun, 06 Aug 2017 17:40:18 +0200
>
> (rename-file "/tmp/emacs/ẞ" "/tmp/emacs/ß")
> nil
>
> Note how `rename-file' has silently overwritten `ß'. This is because on
> macOS, `ß' and `ẞ' are different file names, but Emacs treats them as
> equal. Probably the test for case-insensitive file names should be
> removed altogether
Which one? there are two of them.
> (it can't work correctly and introduces a filesystem race)
It cannot work correctly _because_ of a possible race or because of
some other reasons? If the latter, please elaborate. If the former,
then at least on MS-Windows we have a race anyway, because the
underlying system APIs are not atomic. So at least on MS-Windows the
feature should stay, as it introduces no new issues and supports a
valid and quite important use case (the Windows shell supports it as
well, so we really cannot do less).
> and `rename-file' should use link(2) + unlink(2) if renameat2
> isn't available.
'link' and 'unlink' accept strings as arguments, not integer numbers
such as 2.
More to the point, how can this strategy work on a case-insensitive
filesystem? What am I missing?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Fri, 11 Aug 2017 08:16:01 GMT)
Full text and
rfc822 format available.
Message #11 received at 27986 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
>> Probably the test for case-insensitive file names should be
>> removed altogether
>
> Which one? there are two of them.
There should be just one such test, since it's testing the same thing and
there's no point to doing the same test twice. And I notice the code has some
other duplicate system calls. I installed the attached patch, which prunes away
some of these duplicates. With this patch, (rename-file "a" "b/") now takes just
one system call on recent GNU/Linux, whereas it used to take four (and was not
atomic).
However, even with this patch there are races on GNU/Linux which can lead to
potential security problems. Perhaps we can't fix these races on MS-Windows but
we should be able to fix them on a GNUish host. However, we will need to change
the semantics of rename-file etc. slightly, since no single system call supports
the cp-like target rewriting of these functions. I have a fix in mind to do that
in a hopefully compatible-enough way, which I'll try to propose soon. I'll keep
case-insensitive file systems in mind when I do that.
This reminds me of a similar problem in GNU coreutils which I fixed a dozen
years ago by adding the -T option to mv, ln, etc.
[0001-Improve-performance-for-rename-file-etc.patch (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Sun, 13 Aug 2017 22:43:01 GMT)
Full text and
rfc822 format available.
Message #14 received at 27986 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Paul Eggert wrote:
> there are races on GNU/Linux which can lead to potential security problems.
> Perhaps we can't fix these races on MS-Windows but we should be able to fix them
> on a GNUish host. However, we will need to change the semantics of rename-file
> etc. slightly, since no single system call supports the cp-like target rewriting
> of these functions. I have a fix in mind to do that in a hopefully
> compatible-enough way, which I'll try to propose soon. I'll keep
> case-insensitive file systems in mind when I do that.
Attached is a proposed patch to fix this security problem. If I understand
things correctly, the fix should work on MS-Windows and on case-insensitive file
systems. Since this patch entails an incompatible change to the (undocumented)
behavior of (rename-file A B) when B is a directory but is not a directory name,
I'll mention the proposed change on emacs-devel.
[0001-Fix-race-with-rename-file-etc.-with-dir-NEWNAME.patch (text/x-patch, attachment)]
Added tag(s) patch.
Request was from
Paul Eggert <eggert <at> cs.ucla.edu>
to
control <at> debbugs.gnu.org.
(Sun, 13 Aug 2017 22:45:01 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Sun, 13 Aug 2017 23:50:01 GMT)
Full text and
rfc822 format available.
Message #19 received at 27986 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Getting back to Philipp's original bug report, Apple documentation says macOS
has a facility like the Linux renameat2 system call (i.e., it's like 'renameat'
except it can be told to fail if the destination already exists). Attached is a
proposed patch to use this facility, which means that the case-insensitivity
test would no longer need to be done in macOS. If there's some way to implement
renameat_noreplace on MS-Windows we could get rid of the case-insensitivity test
there too.
I don't have easy access to macOS so I have not installed this patch. It'd be
nice, Philipp, if you could try it out.
This patch is independent of the destination-directory patch that I recently
proposed in:
https://bugs.gnu.org/27986#14
[0001-Improve-rename-file-behavior-on-macOS.patch (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Mon, 14 Aug 2017 13:46:02 GMT)
Full text and
rfc822 format available.
Message #22 received at 27986 <at> debbugs.gnu.org (full text, mbox):
On 8/13/2017 7:48 PM, Paul Eggert wrote:
> Getting back to Philipp's original bug report, Apple documentation says
> macOS has a facility like the Linux renameat2 system call (i.e., it's
> like 'renameat' except it can be told to fail if the destination already
> exists). Attached is a proposed patch to use this facility, which means
> that the case-insensitivity test would no longer need to be done in
> macOS. If there's some way to implement renameat_noreplace on MS-Windows
> we could get rid of the case-insensitivity test there too.
I think we still need the case-insensitivity test on Cygwin as well as
on MS-Windows.
Ken
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Mon, 14 Aug 2017 15:22:01 GMT)
Full text and
rfc822 format available.
Message #25 received at 27986 <at> debbugs.gnu.org (full text, mbox):
> From: Ken Brown <kbrown <at> cornell.edu>
> Date: Mon, 14 Aug 2017 09:44:54 -0400
> Cc: 27986 <at> debbugs.gnu.org
>
> I think we still need the case-insensitivity test on Cygwin as well as
> on MS-Windows.
Yes. But I think there are more general issues with this, which I
will describe separately.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Mon, 14 Aug 2017 15:35:02 GMT)
Full text and
rfc822 format available.
Message #28 received at 27986 <at> debbugs.gnu.org (full text, mbox):
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> Cc: Eli Zaretskii <eliz <at> gnu.org>, 27986 <at> debbugs.gnu.org
> Date: Sun, 13 Aug 2017 16:48:59 -0700
>
> Getting back to Philipp's original bug report, Apple documentation says macOS
> has a facility like the Linux renameat2 system call (i.e., it's like 'renameat'
> except it can be told to fail if the destination already exists). Attached is a
> proposed patch to use this facility, which means that the case-insensitivity
> test would no longer need to be done in macOS. If there's some way to implement
> renameat_noreplace on MS-Windows we could get rid of the case-insensitivity test
> there too.
There's nothing easier than implementing renameat_noreplace on
MS-Windows, since the underlying system call does that by default, and
it's emulating the Posix behavior that requires complications. In
fact, we already have this implementation: see sys_rename_replace (in
w32.c) which needs to be called with its last argument FALSE (modulo
the "at" part, which is easily handled).
(Ken, what about Cygwin?)
So I think we may be able to remove the case-sensitivity check right
now. And in any case, that check should be in
barf_or_query_if_file_exists anyway, because renameat_noreplace on
case-insensitive filesystems already supports renaming to a different
letter-case. The reason we had this check before is because we didn't
employ renameat_noreplace, but called 'rename' right away, and that
needed a question before overwriting the target. But now
renameat_noreplace will silently succeed to rename when the user wants
to only change the letter-case, so no such check is needed.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Mon, 14 Aug 2017 15:41:01 GMT)
Full text and
rfc822 format available.
Message #31 received at 27986 <at> debbugs.gnu.org (full text, mbox):
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> Cc: Philipp <p.stephani2 <at> gmail.com>, 27986 <at> debbugs.gnu.org
> Date: Sun, 13 Aug 2017 15:42:05 -0700
>
> Paul Eggert wrote:
> > there are races on GNU/Linux which can lead to potential security problems.
> > Perhaps we can't fix these races on MS-Windows but we should be able to fix them
> > on a GNUish host.
For the record: 'rename' is atomic on Windows when the target doesn't
exist. It's the case when the target exists that cannot be guaranteed
to be handled atomically, AFAIU, because deleting the old target and
renaming are not necessarily an atomic operation on Windows. I don't
know if this is an issue, since the current code in rename-file uses 2
separate system calls in this case anyway, even on Posix platforms.
> Attached is a proposed patch to fix this security problem. If I understand
> things correctly, the fix should work on MS-Windows and on case-insensitive file
> systems. Since this patch entails an incompatible change to the (undocumented)
> behavior of (rename-file A B) when B is a directory but is not a directory name,
> I'll mention the proposed change on emacs-devel.
I'm uneasy, to say the least, to change the semantic of such a veteran
behavior. Could you please take a step back and elaborate on the
races and the security problems related to this, and why the change in
the semantics you propose is the solution? I'd like to understand the
problem better before we decide on the solution.
Thanks.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Mon, 14 Aug 2017 16:35:02 GMT)
Full text and
rfc822 format available.
Message #34 received at 27986 <at> debbugs.gnu.org (full text, mbox):
> Date: Mon, 14 Aug 2017 18:34:30 +0300
> From: Eli Zaretskii <eliz <at> gnu.org>
> Cc: p.stephani2 <at> gmail.com, 27986 <at> debbugs.gnu.org
>
> There's nothing easier than implementing renameat_noreplace on
> MS-Windows, since the underlying system call does that by default, and
> it's emulating the Posix behavior that requires complications. In
> fact, we already have this implementation: see sys_rename_replace (in
> w32.c) which needs to be called with its last argument FALSE (modulo
> the "at" part, which is easily handled).
Which I just did.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Mon, 14 Aug 2017 16:51:02 GMT)
Full text and
rfc822 format available.
Message #37 received at 27986 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Paul Eggert <eggert <at> cs.ucla.edu> schrieb am Mo., 14. Aug. 2017 um 01:49 Uhr:
> Getting back to Philipp's original bug report, Apple documentation says
> macOS
> has a facility like the Linux renameat2 system call (i.e., it's like
> 'renameat'
> except it can be told to fail if the destination already exists). Attached
> is a
> proposed patch to use this facility, which means that the
> case-insensitivity
> test would no longer need to be done in macOS. If there's some way to
> implement
> renameat_noreplace on MS-Windows we could get rid of the
> case-insensitivity test
> there too.
>
> I don't have easy access to macOS so I have not installed this patch. It'd
> be
> nice, Philipp, if you could try it out.
>
>
Thanks, the patch fixes the problem. However, it's still not 100% correct:
now casing changes such as from "A" to "a" where macOS treats the file
names as equivalent trigger the "file already exists" signal as well. I
don't think that can be fixed, though; there's no way to special-case
casing changes while keeping atomicity intact. So I'd rather have Emacs
react conservatively and skip the casing check entirely.
Note that the manpage says:
RENAME_EXCL On file systems that support it (see getattrlist(2)
VOL_CAP_INT_RENAME_EXCL), it will cause EEXIST to be returned if the
destination already exists.
I interpret this such that if the filesystem doesn't support RENAME_EXCL
the rename will succeed even if the destination exists.
Since we probably won't be able to solve all issues across operating
systems and filesystems, probably we should have at least a warning in the
documentation that rename-file attempts to be race-free and atomic, but
only on a best-effort basis.
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Mon, 14 Aug 2017 16:59:02 GMT)
Full text and
rfc822 format available.
Message #40 received at 27986 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> schrieb am Mo., 14. Aug. 2017 um 17:34 Uhr:
> > From: Paul Eggert <eggert <at> cs.ucla.edu>
> > Cc: Eli Zaretskii <eliz <at> gnu.org>, 27986 <at> debbugs.gnu.org
> > Date: Sun, 13 Aug 2017 16:48:59 -0700
> >
> > Getting back to Philipp's original bug report, Apple documentation says
> macOS
> > has a facility like the Linux renameat2 system call (i.e., it's like
> 'renameat'
> > except it can be told to fail if the destination already exists).
> Attached is a
> > proposed patch to use this facility, which means that the
> case-insensitivity
> > test would no longer need to be done in macOS. If there's some way to
> implement
> > renameat_noreplace on MS-Windows we could get rid of the
> case-insensitivity test
> > there too.
>
> There's nothing easier than implementing renameat_noreplace on
> MS-Windows, since the underlying system call does that by default, and
> it's emulating the Posix behavior that requires complications.
>
You might be able to eliminate these complications by calling MoveFileExW
with MOVEFILE_REPLACE_EXISTING.
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Mon, 14 Aug 2017 17:05:02 GMT)
Full text and
rfc822 format available.
Message #43 received at 27986 <at> debbugs.gnu.org (full text, mbox):
> From: Philipp Stephani <p.stephani2 <at> gmail.com>
> Date: Mon, 14 Aug 2017 16:58:04 +0000
> Cc: 27986 <at> debbugs.gnu.org
>
> There's nothing easier than implementing renameat_noreplace on
> MS-Windows, since the underlying system call does that by default, and
> it's emulating the Posix behavior that requires complications.
>
> You might be able to eliminate these complications by calling MoveFileExW with
> MOVEFILE_REPLACE_EXISTING.
Thanks, but I see no need, because using that function doesn't solve
any problems we have (it isn't guaranteed to be atomic if the
destination exists), and adds new complexity (because it won't move
files across volumes, and because it didn't exist before XP). The
"complications" I mentioned above are already implemented and well
tested, so I see no point in eliminating them.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Mon, 14 Aug 2017 17:10:01 GMT)
Full text and
rfc822 format available.
Message #46 received at 27986 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> schrieb am So., 6. Aug. 2017 um 19:05 Uhr:
> > From: Philipp <p.stephani2 <at> gmail.com>
> > Date: Sun, 06 Aug 2017 17:40:18 +0200
> >
> > (rename-file "/tmp/emacs/ẞ" "/tmp/emacs/ß")
> > nil
> >
> > Note how `rename-file' has silently overwritten `ß'. This is because on
> > macOS, `ß' and `ẞ' are different file names, but Emacs treats them as
> > equal. Probably the test for case-insensitive file names should be
> > removed altogether
>
> Which one? there are two of them.
>
I guess all of them where correctness would depend on the outcome.
>
> > (it can't work correctly and introduces a filesystem race)
>
> It cannot work correctly _because_ of a possible race or because of
> some other reasons? If the latter, please elaborate.
As this example shows, there are cases where two case-insensitive filenames
are considered equivalent by Emacs, but different by the actual filesystem.
This is unavoidable, because the definition of "case-insensitive" changes
all the time, both in Emacs and in the filesystems. Generally it's
impossible to detect whether two filenames would refer to the same file
without actually creating the file. And even then the answer depends on how
the file is created, see e.g. FILE_FLAG_POSIX_SEMANTICS. So Emacs can't
compare filenames and make decisions based on the result upon which the
correctness of a critical function depends. Comparing filenames can still
be OK for best-effort attempts at giving the user better error messages or
similar.
> If the former,
> then at least on MS-Windows we have a race anyway, because the
> underlying system APIs are not atomic.
Wouldn't MoveFileExW with MOVE_FILE_REPLACE_EXISTING be atomic?
> and `rename-file' should use link(2) + unlink(2) if renameat2
> > isn't available.
>
> 'link' and 'unlink' accept strings as arguments, not integer numbers
> such as 2.
>
Yes, I mean the functions described in section 2 of the man page. link(2)
is a common markup for this.
>
> More to the point, how can this strategy work on a case-insensitive
> filesystem? What am I missing?
>
IIUC link(2) + unlink(2) would, if successful, guarantee enough atomicity
in the sense that the old file is now guaranteed to be the new file, and
the call is guaranteed to fail if the new file already exists. I don't
think anything can help with the case-changing problem; I think we just
have to live with an occasional false positive signal in this case.
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Mon, 14 Aug 2017 17:23:01 GMT)
Full text and
rfc822 format available.
Message #49 received at 27986 <at> debbugs.gnu.org (full text, mbox):
> From: Philipp Stephani <p.stephani2 <at> gmail.com>
> Date: Mon, 14 Aug 2017 17:09:35 +0000
> Cc: 27986 <at> debbugs.gnu.org
>
> > Note how `rename-file' has silently overwritten `ß'. This is because on
> > macOS, `ß' and `ẞ' are different file names, but Emacs treats them as
> > equal. Probably the test for case-insensitive file names should be
> > removed altogether
>
> Which one? there are two of them.
>
> I guess all of them where correctness would depend on the outcome.
But then we lose a feature which just a few releases ago was deemed
valuable enough to have.
> As this example shows, there are cases where two case-insensitive filenames are considered equivalent by
> Emacs, but different by the actual filesystem. This is unavoidable, because the definition of "case-insensitive"
> changes all the time, both in Emacs and in the filesystems. Generally it's impossible to detect whether two
> filenames would refer to the same file without actually creating the file.
Wouldn't trying to rename it actually tell you that, by failing with EEXIST?
(On Windows, it simply succeeds and changes the letter-case silently,
but I understand that is not what happens on macOS.)
> If the former,
> then at least on MS-Windows we have a race anyway, because the
> underlying system APIs are not atomic.
>
> Wouldn't MoveFileExW with MOVE_FILE_REPLACE_EXISTING be atomic?
No, it isn't guaranteed to be atomic. No documentation says that,
certainly not any official documentation. If I had access to the
sources, I could have looked there, but I don't. Failing that, my
assumption is that it isn't atomic, because meta-data of more than one
file needs to be touched in this case.
> Yes, I mean the functions described in section 2 of the man page. link(2) is a common markup for this.
My point was that GNU coding standards frown on use of such markup.
> More to the point, how can this strategy work on a case-insensitive
> filesystem? What am I missing?
>
> IIUC link(2) + unlink(2) would, if successful, guarantee enough atomicity in the sense that the old file is now
> guaranteed to be the new file, and the call is guaranteed to fail if the new file already exists.
I actually think that if the old and the new name are equal but for
the letter-case, and the filesystem is case-insensitive, doing that
will delete the file, so you are left with nothing.
> I don't think anything can help with the case-changing problem; I
> think we just have to live with an occasional false positive signal
> in this case.
That'd be an unfortunate regression, IMO. It isn't right for us to
back out changes we just introduced, which were considered important
when we did that. We ought to find a solution that doesn't remove
this feature, not even on macOS.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Mon, 14 Aug 2017 23:04:01 GMT)
Full text and
rfc822 format available.
Message #52 received at 27986 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Philipp Stephani wrote:
> there's no way to special-case
> casing changes while keeping atomicity intact. So I'd rather have Emacs
> react conservatively and skip the casing check entirely.
Yes, that makes sense, at least for macOS.
> Note that the manpage says:
>
> RENAME_EXCL On file systems that support it (see getattrlist(2)
> VOL_CAP_INT_RENAME_EXCL), it will cause EEXIST to be returned if the
> destination already exists.
>
> I interpret this such that if the filesystem doesn't support RENAME_EXCL
> the rename will succeed even if the destination exists.
I interpret it to mean that if the filesystem doesn't support RENAME_EXCL, the
rename will fail with errno == EINVAL or ENOSYS. At least, that's how it works
under GNU/Linux. If it behaved the way you suggest, there'd be no good way to
emulate RENAME_EXCL on filesystems lacking it, which would surely not be Apple's
intent.
Please check this, though. I installed the patch on 'master' to help you do that.
Now that renameat_noreplace works on DOS_NT, would it make sense to apply the
attached further patch as well? If we can get renameat_noreplace to work on
Cygwin the we could simplify the fileio.c code even further.
> Since we probably won't be able to solve all issues across operating
> systems and filesystems, probably we should have at least a warning in the
> documentation that rename-file attempts to be race-free and atomic, but
> only on a best-effort basis.
True. I'd like to get the directory issue fixed before worrying about this,
though. (That way I don't have to document the security holes in the current
implementation. :-) I'll follow up separately about that.
[0001-Improve-rename-file-behavior-on-DOS_NT.patch (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Mon, 14 Aug 2017 23:32:02 GMT)
Full text and
rfc822 format available.
Message #55 received at 27986 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii wrote:
> Could you please take a step back and elaborate on the
> races and the security problems related to this, and why the change in
> the semantics you propose is the solution?
Currently (rename-file A B) requires at least two system calls to work: one to
test whether B is a directory, and the other to actually do the rename. This
leads to race conditions if other actors change the file system between the two
calls.
For example, suppose a victim is about to execute (rename-file "/tmp/foo"
"/tmp/bar" t), and suppose an attacker wants to destroy the victim's file
/home/victim/secret/foo. The attacker can do (make-symbolic-link
"/home/victim/secret" "/tmp/bar"), and this will cause the victim to lose all
the data in /home/victim/secret/bar even though the attacker is supposed to lack
access to anything under /home/victim/secret. I doubt whether this is the only
such scenario; it's just the first one that popped into my mind.
As icing on the cake, the current behavior of (rename-file A B) disagrees with
its documentation when B is an existing directory.
There is no good solution to this problem. All solutions are bad, in that either
they are not 100% backward compatible with existing behavior, or they continue
to encourage insecure Elisp code. The proposed patch attempts to choose the
least bad way forward, by making the default behavior more secure, at a
relatively minor cost in compatibility. Most uses of rename-file etc. won't care
about the change, and the ones that do care are likely to have security problems
anyway.
The proposed solution improves security, because a common pattern in Lisp code
when creating a file BAR "atomically" is to create and write a temporary file
FOO and then execute (rename-file FOO BAR). Currently, this approach can be
attacked in the way described when BAR's parent directory is /tmp or any similar
directory. With the proposed patch, this approach cannot be hijacked in this
way, because BAR will be a file name and not a directory name. That is, the call
to rename-file will specify whether the destination-directory semantics are
desired, rather than relying on the state of the filesystem to specify it. This
is more secure because the state of the filesystem is partially under control of
attackers.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Tue, 15 Aug 2017 01:20:02 GMT)
Full text and
rfc822 format available.
Message #58 received at 27986 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Paul Eggert wrote:
> I installed the patch on 'master' to help you do that.
After rereading the Apple manual I installed the attached further patch into
master, so please try updating to the latest master before testing.
[0001-Improve-rename-file-port-to-macOS.patch (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Tue, 15 Aug 2017 02:37:01 GMT)
Full text and
rfc822 format available.
Message #61 received at 27986 <at> debbugs.gnu.org (full text, mbox):
> Cc: Eli Zaretskii <eliz <at> gnu.org>, 27986 <at> debbugs.gnu.org
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> Date: Mon, 14 Aug 2017 16:03:13 -0700
>
> Philipp Stephani wrote:
>
> > there's no way to special-case
> > casing changes while keeping atomicity intact. So I'd rather have Emacs
> > react conservatively and skip the casing check entirely.
>
> Yes, that makes sense, at least for macOS.
As I wrote, we should not lose the feature that renaming a file on
macOS to a different letter-case works without nagging the user with
confirmations. I don't think this runs afoul of security, since the
letter-case check doesn't break atomicity in any way.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Tue, 15 Aug 2017 07:01:01 GMT)
Full text and
rfc822 format available.
Message #64 received at 27986 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii wrote:
> we should not lose the feature that renaming a file on
> macOS to a different letter-case works without nagging the user with
> confirmations.
I was hoping that the current code does that as-is on macOS. Someone needs to
test that, though.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Tue, 15 Aug 2017 12:46:02 GMT)
Full text and
rfc822 format available.
Message #67 received at submit <at> debbugs.gnu.org (full text, mbox):
On Mon 14 Aug 2017, Paul Eggert wrote:
> Paul Eggert wrote:
>> I installed the patch on 'master' to help you do that.
>
> After rereading the Apple manual I installed the attached further patch into
> master, so please try updating to the latest master before testing.
This patch breaks the Windows build for mingw64:
../../src/fileio.c: In function 'Frename_file':
../../src/fileio.c:2300:41: error: duplicate case value
case EEXIST: case EINVAL: case ENOSYS: case ENOTSUP:
^~~~
../../src/fileio.c:2300:28: note: previously used here
case EEXIST: case EINVAL: case ENOSYS: case ENOTSUP:
^~~~
A brief look suggests that the inclusion of ms-w32.h in conf_post.h is
the culprit, as it defines some error numbers before errno.h is
included, and thus overrides the conditional definitions in errno.h.
AndyM
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Tue, 15 Aug 2017 16:05:01 GMT)
Full text and
rfc822 format available.
Message #70 received at 27986 <at> debbugs.gnu.org (full text, mbox):
> Cc: p.stephani2 <at> gmail.com, 27986 <at> debbugs.gnu.org
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> Date: Mon, 14 Aug 2017 16:31:38 -0700
>
> Currently (rename-file A B) requires at least two system calls to work: one to test whether B is a directory, and the other to actually do the rename. This leads to race conditions if other actors change the file system between the two calls.
>
> For example, suppose a victim is about to execute (rename-file "/tmp/foo" "/tmp/bar" t), and suppose an attacker wants to destroy the victim's file /home/victim/secret/foo. The attacker can do (make-symbolic-link "/home/victim/secret" "/tmp/bar")
You mean, the attacker creates this symlink between the time Emacs
tests whether /tmp/bar is a directory and the time Emacs calls
'rename'?
> and this will cause the victim to lose all the data in /home/victim/secret/bar even though the attacker is supposed to lack access to anything under /home/victim/secret.
You mean, "lose data" because files in /tmp/foo will overwrite their
namesakes in /home/victim/secret/bar? IOW, files in
/home/victim/secret/bar which don't have identically-named
counterparts in /tmp/foo will not be lost, right? (I'm not saying
this is not a problem, I'm just trying to understand the meaning of
"lose data" in this case.)
If my understanding of the situation is correct, then such an attack
will still be possible with the proposed change, if /tmp/bar exists,
because Emacs will still issue two separate system calls, and the
symlink can be created in-between, albeit at a price of deleting
/tmp/bar first. Right?
> As icing on the cake, the current behavior of (rename-file A B) disagrees with its documentation when B is an existing directory.
Well, 2/3 of it. The 3rd instance, in the Emacs manual gets it right:
[...] If the argument NEW is just a directory name, the real new name
is in that directory, with the same non-directory component as OLD.
For example, ‘M-x rename-file <RET> ~/foo <RET> /tmp <RET>’ renames
‘~/foo’ to ‘/tmp/foo’.
Note that there's no trailing slash in "/tmp".
> There is no good solution to this problem. All solutions are bad, in that either they are not 100% backward compatible with existing behavior, or they continue to encourage insecure Elisp code. The proposed patch attempts to choose the least bad way forward, by making the default behavior more secure, at a relatively minor cost in compatibility. Most uses of rename-file etc. won't care about the change, and the ones that do care are likely to have security problems anyway.
How about eating the cake and having it, too? We could refrain from
testing whether B is a directory if either (1) B ends in a slash, or
(2) rename_noreplace succeeds. If B doesn't end in a slash and
rename_noreplace fails, and the value of errno from rename_noreplace
doesn't indicate B is a directory, then test if it's a directory and
fall back on the old behavior. We could then change our code that
calls rename-file to make B end in a slash, thus encouraging secure
code and leaving the insecure behavior only as backward-compatibility
feature. This will make the insecure code limited to situation which
are already insecure due to a separate call to rename_noreplace.
> The proposed solution improves security, because a common pattern in Lisp code when creating a file BAR "atomically" is to create and write a temporary file FOO and then execute (rename-file FOO BAR). Currently, this approach can be attacked in the way described when BAR's parent directory is /tmp or any similar directory. With the proposed patch, this approach cannot be hijacked in this way, because BAR will be a file name and not a directory name. That is, the call to rename-file will specify whether the destination-directory semantics are desired, rather than relying on the state of the filesystem to specify it. This is more secure because the state of the filesystem is partially under control of attackers.
What I don't quite understand is what will happen under your proposal
to the calls of the form (rename-file A B) where B names an existing
directory and doesn't end in slash? Will it fail, sometimes or
always? AFAIU, the 'rename' call will fail if B is a non-empty
directory, but what if it's empty, and what does rename_noreplace do
in these situations? Your documentation patches don't cover this use
case; I think we should.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Tue, 15 Aug 2017 16:09:02 GMT)
Full text and
rfc822 format available.
Message #73 received at 27986 <at> debbugs.gnu.org (full text, mbox):
> Cc: p.stephani2 <at> gmail.com, 27986 <at> debbugs.gnu.org
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> Date: Tue, 15 Aug 2017 00:00:49 -0700
>
> Eli Zaretskii wrote:
> > we should not lose the feature that renaming a file on
> > macOS to a different letter-case works without nagging the user with
> > confirmations.
>
> I was hoping that the current code does that as-is on macOS.
In that case, I have no problems with the changes. From what Philipp
wrote I concluded, perhaps wrongly, that this is not happening on
macOS.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Tue, 15 Aug 2017 16:20:01 GMT)
Full text and
rfc822 format available.
Message #76 received at 27986 <at> debbugs.gnu.org (full text, mbox):
> From: Andy Moreton <andrewjmoreton <at> gmail.com>
> Date: Tue, 15 Aug 2017 13:45:02 +0100
>
> ../../src/fileio.c: In function 'Frename_file':
> ../../src/fileio.c:2300:41: error: duplicate case value
> case EEXIST: case EINVAL: case ENOSYS: case ENOTSUP:
> ^~~~
> ../../src/fileio.c:2300:28: note: previously used here
> case EEXIST: case EINVAL: case ENOSYS: case ENOTSUP:
> ^~~~
Should be fixed now.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Tue, 15 Aug 2017 17:25:02 GMT)
Full text and
rfc822 format available.
Message #79 received at 27986 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
>> and this will cause the victim to lose all the data in /home/victim/secret/bar even though the attacker is supposed to lack access to anything under /home/victim/secret.
>
> You mean, "lose data" because files in /tmp/foo will overwrite their
> namesakes in /home/victim/secret/bar?
Yes. It's not just losing data of course; the attacker can add files to the
victim directory.
> If my understanding of the situation is correct, then such an attack
> will still be possible with the proposed change, if /tmp/bar exists,
> because Emacs will still issue two separate system calls, and the
> symlink can be created in-between, albeit at a price of deleting
> /tmp/bar first. Right?
No, such an attack is not possible. Emacs will run rename_noreplace which will
fail because /tmp/bar exists, then the attacker will remove /tmp/bar and replace
it with a symlink, then Emacs will run rename ("/tmp/foo", "/tmp/bar") which
will fail because a directory like /tmp is sticky and the victim cannot remove
the attacker's symlink. The attack would fail in a different way if /tmp were
not sticky: the victim would remove the attacker's symlink.
>> As icing on the cake, the current behavior of (rename-file A B) disagrees with its documentation when B is an existing directory.
>
> Well, 2/3 of it. The 3rd instance, in the Emacs manual gets it right:
Unfortunately the manual does not get it right. The main part of that 3rd
instance agrees with the proposed change, because it says the special case
occurs if NEWNAME "is just a directory name" (on Unix, this means it ends in
"/"). You are correct that the example is missing a "/", so I'll update the
proposed patch to fix that.
That part of the documentation has some other confusion: a phrase "The same rule
applies to all the remaining commands in this section" is clearly an editing
error; it must be a revenant of when the section didn't talk about commands like
insert-file. I just now installed the attached patch to fix that (this patch
doesn't address the directory issue).
The bigger picture here is that this part of Emacs behavior is so poorly
documented that it's unclear from the documentation what the intent was.
> How about eating the cake and having it, too? We could refrain from
> testing whether B is a directory if either (1) B ends in a slash, or
> (2) rename_noreplace succeeds.
That doesn't close the security hole, I'm afraid. For example, the attacker can
create a nonempty directory B, then rename_noreplace will fail, then Emacs will
determine that B is a directory, then the attacker can replace B with a symlink
to the victim directory, and then Emacs will overwrite the victim. I imagine
other attacks are possible, this is just the first one off the top of my head.
> What I don't quite understand is what will happen under your proposal
> to the calls of the form (rename-file A B) where B names an existing
> directory and doesn't end in slash? Will it fail, sometimes or
> always?
On POSIX systems rename-file will fail if B is a nonempty directory, and will
succeed if B names an empty directory (this is all assuming B is not itself a
directory name). Ideally MS-Windows would be compatible; if not, we'd have to
document the incompatibility.
AFAIU, the 'rename' call will fail if B is a non-empty
> directory, but what if it's empty, and what does rename_noreplace do
> in these situations? Your documentation patches don't cover this use
> case; I think we should.
Thanks, good point, I plan to update the proposed patch accordingly and to
follow up soon.
[0001-New-manual-section-Copying-and-Naming.patch (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Tue, 15 Aug 2017 17:44:02 GMT)
Full text and
rfc822 format available.
Message #82 received at 27986 <at> debbugs.gnu.org (full text, mbox):
> Cc: p.stephani2 <at> gmail.com, 27986 <at> debbugs.gnu.org
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> Date: Tue, 15 Aug 2017 10:24:03 -0700
>
> > How about eating the cake and having it, too? We could refrain from
> > testing whether B is a directory if either (1) B ends in a slash, or
> > (2) rename_noreplace succeeds.
>
> That doesn't close the security hole, I'm afraid. For example, the attacker can
> create a nonempty directory B
How would they know to create B before Emacs issues any system call
that uses B?
And how is this case different from the case that Emacs calls
(rename-file A B) thinking B doesn't exist (e.g., because some prior
code tested that)?
Anyway, I firmly believe we should be backward compatible as a
fallback. It is okay for the fallback to be insecure, as the current
code is also insecure. But I don't think we should fail use cases
that previously were legitimate, for many years. If my proposal is
not workable, we should come up with something that is.
> > What I don't quite understand is what will happen under your proposal
> > to the calls of the form (rename-file A B) where B names an existing
> > directory and doesn't end in slash? Will it fail, sometimes or
> > always?
>
> On POSIX systems rename-file will fail if B is a nonempty directory, and will
> succeed if B names an empty directory (this is all assuming B is not itself a
> directory name). Ideally MS-Windows would be compatible; if not, we'd have to
> document the incompatibility.
I see no problems in being compatible in this sense. But wiping out
the empty directory instead of moving the first argument into it is
an incompatible change, and we should avoid that.
> Thanks, good point, I plan to update the proposed patch accordingly and to
> follow up soon.
Please say in the manual explicitly that what you call "directory
name" should end in a slash. Yes, I know this is already described
elsewhere in the manual, but since this is important in this case, it
doesn't hurt repeating it. It's especially important in the doc
string and in lispref manual, since there the slash must be explicit,
whereas in the interactive usage I believe RET might complete the
slash.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Tue, 15 Aug 2017 19:29:02 GMT)
Full text and
rfc822 format available.
Message #85 received at 27986 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii wrote:
> How would they know to create B before Emacs issues any system call
> that uses B?
Because the attackers know how Emacs work and are attempting to exploit its
security hole.
> And how is this case different from the case that Emacs calls
> (rename-file A B) thinking B doesn't exist (e.g., because some prior
> code tested that)?
The case in question trashes a directory that the attacker lacks permission to.
The case you're talking about does not: it merely causes rename-file to fail.
> we should be backward compatible as a fallback.
I don't see how this can work, if the fallback method relies on two system calls
that can fall victim to a race. I tried pretty hard to come up with secure and
backward-compatible approaches before proposing the change. I could not come up
with any, and doubt whether anyone else could either.
Another possibility is to implement new functions (say: file-copy, file-rename,
file-link, file-symlink, and directory-copy) that behave like the existing
functions except without the security hole, modify callers to use these new
functions, and then mark the existing functions as deprecated due to security
concerns. I suspect that this would be more disruptive overall than the proposed
change, though (albeit disruptive in a different way).
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Wed, 16 Aug 2017 02:38:01 GMT)
Full text and
rfc822 format available.
Message #88 received at 27986 <at> debbugs.gnu.org (full text, mbox):
> Cc: p.stephani2 <at> gmail.com, 27986 <at> debbugs.gnu.org
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> Date: Tue, 15 Aug 2017 12:27:52 -0700
>
> Eli Zaretskii wrote:
>
> > How would they know to create B before Emacs issues any system call
> > that uses B?
>
> Because the attackers know how Emacs work and are attempting to exploit its
> security hole.
Knowing how Emacs works is not enough: they need to actually know the
name of the directory to create, and I don't see how they can do that
without seeing some system call which names that directory.
> > And how is this case different from the case that Emacs calls
> > (rename-file A B) thinking B doesn't exist (e.g., because some prior
> > code tested that)?
>
> The case in question trashes a directory that the attacker lacks permission to.
> The case you're talking about does not: it merely causes rename-file to fail.
No, it's the same use case. In both of them the attacker creates a
directory ahead of Emacs using it in some system call.
> Another possibility is to implement new functions (say: file-copy, file-rename,
> file-link, file-symlink, and directory-copy) that behave like the existing
> functions except without the security hole, modify callers to use these new
> functions, and then mark the existing functions as deprecated due to security
> concerns.
If no other solution is possible, maybe this is what we should do. If
we decide to go that way, we should also decide what to do with the
interactive use of those functions: whether to call the old or the new
variant, because we need to keep backward compatibility there as well.
If we decide to use the old variants, deprecation might not be the
right mechanism for promoting the new variants.
> I suspect that this would be more disruptive overall than the proposed
> change, though (albeit disruptive in a different way).
How so?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Wed, 16 Aug 2017 05:07:02 GMT)
Full text and
rfc822 format available.
Message #91 received at 27986 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii wrote:
> Knowing how Emacs works is not enough: they need to actually know the
> name of the directory to create,
By "knowing how Emacs works" I meant all of Emacs, including the Lisp program
that it is running. We cannot rely on security-via-obscurity; we must assume
that the attackers know not only the Emacs C source code, but the Lisp code that
Emacs runs. Such an attacker will know the name of the directory to create.
>>> And how is this case different from the case that Emacs calls
>>> (rename-file A B) thinking B doesn't exist (e.g., because some prior
>>> code tested that)?
>>
>> The case in question trashes a directory that the attacker lacks permission to.
>> The case you're talking about does not: it merely causes rename-file to fail.
>
> No, it's the same use case. In both of them the attacker creates a
> directory ahead of Emacs using it in some system call.
Sure, but in the use case I'm talking about, the attacker can trash the victim's
directory even though it's write-protected. In the case you're talking about,
the attacker can't do anything other than what the attacker could do already.
>> Another possibility is to implement new functions (say: file-copy, file-rename,
>> file-link, file-symlink, and directory-copy) that behave like the existing
>> functions except without the security hole, modify callers to use these new
>> functions, and then mark the existing functions as deprecated due to security
>> concerns.
>
> If no other solution is possible, maybe this is what we should do. If
> we decide to go that way, we should also decide what to do with the
> interactive use of those functions: whether to call the old or the new
> variant, because we need to keep backward compatibility there as well.
I don't see why. If a user calls M-x copy-file interactively they'll get the old
function; if they call M-x file-copy they'll get the new one. Admittedly there
will be confusion (see below).
>> I suspect that this would be more disruptive overall than the proposed
>> change, though (albeit disruptive in a different way).
>
> How so?
It'll be disruption caused by the extra complexity: pairs of functions that do
nearly the same thing, with user confusion over which function to call, and
people calling the wrong one. Tramp will need at least four new methods to
support, probably more. The complexity and confusion will go on and on, and will
cost more than will be saved by the backward compatibility. It would be worth
all this trouble if people needed the old behavior, but they mostly do not.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Wed, 16 Aug 2017 14:22:01 GMT)
Full text and
rfc822 format available.
Message #94 received at 27986 <at> debbugs.gnu.org (full text, mbox):
> Cc: p.stephani2 <at> gmail.com, 27986 <at> debbugs.gnu.org
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> Date: Tue, 15 Aug 2017 22:06:38 -0700
>
> Knowing how Emacs works is not enough: they need to actually know the
> name of the directory to create,
>
> By "knowing how Emacs works" I meant all of Emacs, including the Lisp program that it is running. We cannot rely on security-via-obscurity; we must assume that the attackers know not only the Emacs C source code, but the Lisp code that Emacs runs. Such an attacker will know the name of the directory to create.
It is reasonable to assume that the attacker has access to the Emacs
sources, and so knows what Emacs will do in any specific situation,
given enough information about the files and directories involved in
the particular use case. But it is not necessarily reasonable to
assume the attacker knows exactly what Emacs is doing at this very
moment, and what files/directories it will be accessing in the very
near future, without any evidence from the system calls emitted by
Emacs and/or the changes to the filesystem it made in the recent past.
You are describing a situation where the attacker somehow knows what
file/directory will be accessed _ahead_ of Emacs actually accessing
it. This _might_ be somehow possible when Emacs runs
non-interactively, as part of some deterministic script, but not in
interactive usage, which is how Emacs is normally used. So I don't
see how the attacker could do that in general, except by having full
control of the Emacs process, in the 'ptrace' sense or similar. And
if the attacker has such a control, you cannot defend against it,
because it can simply call any Emacs functions with any arguments.
IOW I don't see any "security by obscurity" here, I see a case where
you assign super-natural abilities to attackers.
I think our security related scenario should start after the initial
call to rename_noreplace, and not before it, because before that call
there's no external evidence that Emacs is going to call rename-file,
and with what arguments.
> If no other solution is possible, maybe this is what we should do. If
> we decide to go that way, we should also decide what to do with the
> interactive use of those functions: whether to call the old or the new
> variant, because we need to keep backward compatibility there as well.
>
> I don't see why. If a user calls M-x copy-file interactively they'll
> get the old function; if they call M-x file-copy they'll get the new
> one.
I thought you were proposing to redirect the interactive commands to
the new functions. If that's not what you meant, then indeed there's
no such issue, but then obsoleting is out of the question, since we
cannot obsolete user commands.
> It'll be disruption caused by the extra complexity: pairs of functions that do nearly the same thing, with user confusion over which function to call, and people calling the wrong one. Tramp will need at least four new methods to support, probably more. The complexity and confusion will go on and on, and will cost more than will be saved by the backward compatibility. It would be worth all this trouble if people needed the old behavior, but they mostly do not.
It's additional complexity, I agree. But if people want secure code,
they _will_ use the more secure variants (which is why I think
rename-file-securely is a better name than file-rename, and similarly
for other functions). It's basically the same situation as with, say,
strtok vs strtok_r, to take just one similar example.
Whether people want "M-x foo bar" produce bar/foo or just bar, is open
to argument. My impression is that they want the former, because it
is what "mv foo bar" does by default when bar is a directory. Note
that mv doesn't require a trailing slash in this case, and it gives
the user an explicit opt-in switch to change the behavior to the one
you proposed. (We could provide such an optional argument as well, as
an alternative to introducing new functions.)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Wed, 16 Aug 2017 15:16:02 GMT)
Full text and
rfc822 format available.
Message #97 received at 27986 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii wrote:
> You are describing a situation where the attacker somehow knows what
> file/directory will be accessed_ahead_ of Emacs actually accessing
> it.
Sure, and this happens all the time. Emacs prepares a copy of a file with the
intent to rename the copy to the original atomically. The attacker will know
that this is what Emacs will do, by looking at the file system or the syscalls
Emacs issues before its code calls rename-file (e.g., Emacs will read the old
file). So I am not supposing any kind of superhuman attack.
I do take your point that interactive use is different. So, here is a proposed
change to the patch: if the ok-is-already-exists flag is an integer (which
suggests interactive use), and if the destination is not a directory name
(trailing "/") but happens to be an existing directory, then Emacs asks the user
if it is OK to rename to a subfile of the destination. This would allay most the
security concerns that I have, and I hope it would address most of the
backward-compatibility concerns that you have.
> I thought you were proposing to redirect the interactive commands to
> the new functions.
I was not proposing to redirect 'M-x rename-file' etc. They would continue to
use the old insecure behavior, for compatibility reasons.
> we cannot obsolete user commands.
Not immediately, no. But we can mark them as obsolescent and warn users about
their use, and remove them eventually.
This issue of obsolescence is moot, though, if you agree with the above
suggestion about ok-if-already-exists.
> if people want secure code,
> they _will_ use the more secure variants
Emacs is a relatively large and complex system, and we cannot expect users to be
familiar with every detail. Emacs should have safe defaults, not unsafe ones.
The situation with "mv" was different, as POSIX and longstanding documentation
required the unsafe behavior and many scripts relied on it. In contrast, the
Emacs documentation is thoroughly muddled and contradictory in this area, and
code using rename-file etc. would more likely benefit from the proposed change
(because of improved security) than be hurt by it (by loss of backward
compatibility with poorly-documented and insecure behavior).
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Wed, 16 Aug 2017 16:07:02 GMT)
Full text and
rfc822 format available.
Message #100 received at 27986 <at> debbugs.gnu.org (full text, mbox):
> Cc: p.stephani2 <at> gmail.com, 27986 <at> debbugs.gnu.org
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> Date: Wed, 16 Aug 2017 08:15:34 -0700
>
> I do take your point that interactive use is different. So, here is a proposed
> change to the patch: if the ok-is-already-exists flag is an integer (which
> suggests interactive use), and if the destination is not a directory name
> (trailing "/") but happens to be an existing directory, then Emacs asks the user
> if it is OK to rename to a subfile of the destination. This would allay most the
> security concerns that I have, and I hope it would address most of the
> backward-compatibility concerns that you have.
I don't know... Did you look at all the users of these functions in
our codebase? E.g., I see at least one use of rename-file in Gnus
that moves a directory, possibly 2 such uses. And I only looked at a
single function. What's more, some of the use cases will not even
signal an error after the change, they will instead silently do
something different from the previous versions, which is really bad.
We could be easily shooting ourselves in the foot with such
incompatible changes. At the very least, all the users in Emacs
should be audited and fixed as needed.
What do others think? Richard, Stefan, John?
> The situation with "mv" was different, as POSIX and longstanding documentation
> required the unsafe behavior and many scripts relied on it. In contrast, the
> Emacs documentation is thoroughly muddled and contradictory in this area, and
> code using rename-file etc. would more likely benefit from the proposed change
> (because of improved security) than be hurt by it (by loss of backward
> compatibility with poorly-documented and insecure behavior).
My problem is not with being able to defend our change in a court of
law, my problem is with people's muscle memory and with existing code
that was working in certain ways since about forever.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Wed, 16 Aug 2017 17:20:01 GMT)
Full text and
rfc822 format available.
Message #103 received at 27986 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii wrote:
> Did you look at all the users of these functions in
> our codebase?
I have not looked at every single one in detail. I've looked at a fair sample.
See below for more discussion.
> E.g., I see at least one use of rename-file in Gnus
> that moves a directory, possibly 2 such uses.
Moving a directory is not a problem. The only problem is when the destination is
a directory but not a directory name and the intent is to change an entry in
that directory rather than to change the original destination.
I agree that some uses in code will need to be adjusted. Most won't, though. For
example, in the first occurrence of the string "rename-file" in Gnus, where
gnus-agent-rename-group calls (gnus-rename-file old-path new-path t), the intent
is to rename OLD-PATH to NEW-PATH, not to rename it to be an subsidiary entry to
NEW-PATH. For this particular example, the proposed change is slightly
beneficial, since it prevents rename-file from doing the wrong thing in the
(admittedly unlikely) event that some other process changes NEW-PATH to a
directory while Gnus is operating.
> What's more, some of the use cases will not even
> signal an error after the change, they will instead silently do
> something different from the previous versions, which is really bad.
This should be quite rare. The only scenario I see matching your concern is if
the source is a directory, the destination is not a directory name but is an
empty directory and is not a symlink, and the destination is not a descendant of
the source. Although not impossible, this will happen so rarely that it doesn't
invalidate the proposed change.
> At the very least, all the users in Emacs
> should be audited and fixed as needed.
Sure, I'll volunteer to do that. There are only 172 lines containing the string
"rename-file" in our Emacs Lisp code base, for example, and it shouldn't be that
much work to check them.
I've looked at this issue fairly carefully, and I'm afraid the solution I've
proposed is the best way forward if we want to close the security hole in Emacs.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Wed, 16 Aug 2017 17:32:01 GMT)
Full text and
rfc822 format available.
Message #106 received at 27986 <at> debbugs.gnu.org (full text, mbox):
> Cc: p.stephani2 <at> gmail.com, 27986 <at> debbugs.gnu.org
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> Date: Wed, 16 Aug 2017 10:19:35 -0700
>
> > What's more, some of the use cases will not even
> > signal an error after the change, they will instead silently do
> > something different from the previous versions, which is really bad.
>
> This should be quite rare. The only scenario I see matching your concern is if
> the source is a directory, the destination is not a directory name but is an
> empty directory and is not a symlink, and the destination is not a descendant of
> the source. Although not impossible, this will happen so rarely that it doesn't
> invalidate the proposed change.
I don't think we know how rare that is. And if it is very rare, I'm
not sure it's better, because it means such problems might go
unnoticed and/or unfixed for years.
> I've looked at this issue fairly carefully, and I'm afraid the solution I've
> proposed is the best way forward if we want to close the security hole in Emacs.
Let's hear more opinions, okay?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Wed, 16 Aug 2017 18:08:01 GMT)
Full text and
rfc822 format available.
Message #109 received at 27986 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii wrote:
> Let's hear more opinions, okay?
FWIW, I think Paul's proposal is eminently sensible.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Wed, 16 Aug 2017 19:34:02 GMT)
Full text and
rfc822 format available.
Message #112 received at 27986 <at> debbugs.gnu.org (full text, mbox):
On 8/14/2017 7:03 PM, Paul Eggert wrote
> Now that renameat_noreplace works on DOS_NT, would it make sense to
> apply the attached further patch as well? If we can get
> renameat_noreplace to work on Cygwin the we could simplify the fileio.c
> code even further.
I'm in the process of writing an implementation of something like
'renameat2', which I'll submit to the Cygwin developers. Even if it's
accepted, however, I think we'll still need to retain the
case-insensitivity test for users of old versions of Cygwin, unless
there's a decision to remove that because of the security concerns
currently being discussed.
Ken
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Wed, 16 Aug 2017 22:32:01 GMT)
Full text and
rfc822 format available.
Message #115 received at 27986 <at> debbugs.gnu.org (full text, mbox):
>> This should be quite rare. The only scenario I see matching your
>> concern is if the source is a directory, the destination is not
>> a directory name but is an empty directory and is not a symlink, and
>> the destination is not a descendant of the source. Although not
>> impossible, this will happen so rarely that it doesn't invalidate
>> the proposed change.
Paul's suggestion makes a lot of sense to me, but I don't quite
understand the above: why does the emptiness of the destination
directory matter?
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Wed, 16 Aug 2017 23:57:01 GMT)
Full text and
rfc822 format available.
Message #118 received at 27986 <at> debbugs.gnu.org (full text, mbox):
On 08/16/2017 03:31 PM, Stefan Monnier wrote:
>>> This should be quite rare. The only scenario I see matching your
>>> concern is if the source is a directory, the destination is not
>>> a directory name but is an empty directory and is not a symlink, and
>>> the destination is not a descendant of the source. Although not
>>> impossible, this will happen so rarely that it doesn't invalidate
>>> the proposed change.
> Paul's suggestion makes a lot of sense to me, but I don't quite
> understand the above: why does the emptiness of the destination
> directory matter?
It's because the system call rename(A,B) always fails when B is a
nonempty directory. Hence the proposed (rename-file A B) will fail if B
is not a directory name but happens to name a nonempty directory, and
therefore rename-file noisily fails instead of silently behaving
differently in this case (which was Eli's concern). Conversely, if B is
an empty directory then rename(A,B) can succeed (and thereby remove the
old B) if all the other conditions are met.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Thu, 17 Aug 2017 00:05:01 GMT)
Full text and
rfc822 format available.
Message #121 received at 27986 <at> debbugs.gnu.org (full text, mbox):
> case (which was Eli's concern). Conversely, if B is an empty directory then
> rename(A,B) can succeed (and thereby remove the old B) if all the other
> conditions are met.
Ah, right, if it's an empty dir then it can be
removed/replaced atomically. Thanks. It does seem like the odd case
should be rare, then, indeed.
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Sat, 19 Aug 2017 06:55:02 GMT)
Full text and
rfc822 format available.
Message #124 received at 27986 <at> debbugs.gnu.org (full text, mbox):
> From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
> Cc: Paul Eggert <eggert <at> cs.ucla.edu>, John Wiegley <johnw <at> gnu.org>,
> rms <at> gnu.org, p.stephani2 <at> gmail.com, 27986 <at> debbugs.gnu.org
> Date: Wed, 16 Aug 2017 18:31:14 -0400
>
> Paul's suggestion makes a lot of sense to me
John, what's your take on this?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Sat, 19 Aug 2017 21:31:02 GMT)
Full text and
rfc822 format available.
Message #127 received at 27986 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On 8/16/2017 3:33 PM, Ken Brown wrote:
> On 8/14/2017 7:03 PM, Paul Eggert wrote
>> Now that renameat_noreplace works on DOS_NT, would it make sense to
>> apply the attached further patch as well? If we can get
>> renameat_noreplace to work on Cygwin the we could simplify the
>> fileio.c code even further.
>
> I'm in the process of writing an implementation of something like
> 'renameat2', which I'll submit to the Cygwin developers.
This is now done. The implementation will appear in the next Cygwin
release. When that release occurs, I'll install something like the
attached patch.
Question: Is the patch OK as is, or should I make the call to renameat2
conditional on CYGWIN? In other words, is it safe to assume that
renameat2 is defined on any platform on which RENAME_NOREPLACE is
defined but not SYS_renameat2?
Ken
[0001-Implement-renameat_noreplace-on-recent-Cygwin.patch (text/plain, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Sat, 19 Aug 2017 21:35:01 GMT)
Full text and
rfc822 format available.
Message #130 received at 27986 <at> debbugs.gnu.org (full text, mbox):
[[[ To any NSA and FBI agents reading my email: please consider ]]]
[[[ whether defending the US Constitution against all enemies, ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]
I haven't followed the discussion, but I'm told there is a proposal to
make this change:
> Btw, in case it isn't clear: the issue at hand is an incompatible
> change to rename-file (and probably also other functions, like
> copy-file). Where previously (rename-file A B) with B a directory
> will move A into B/A, under the proposed change it will only do so if
> B actually ended in a slash; otherwise it will move A to B, deleting B
> if it exists. The incompatibility will manifest itself if some old
> code expects to get B/A, but instead gets either an error (if B is a
> non-empty directory) or B silently removed (if it is empty).
Assuming this applies only when directory B is empty, so that this
won't delete non-empty directories, then I don't have any objection.
I would object to deleting non-empty directories here.
Another option that might be good is to make this operation always
signal an error in the case where B is a directory and does not end
with a slash.
I don't have an opinion about which of those two is better.
--
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Sat, 19 Aug 2017 21:38:01 GMT)
Full text and
rfc822 format available.
Message #133 received at 27986 <at> debbugs.gnu.org (full text, mbox):
Ken Brown wrote:
> This is now done. The implementation will appear in the next Cygwin release.
> When that release occurs, I'll install something like the attached patch.
Thanks.
> Question: Is the patch OK as is, or should I make the call to renameat2
> conditional on CYGWIN? In other words, is it safe to assume that renameat2 is
> defined on any platform on which RENAME_NOREPLACE is defined but not SYS_renameat2?
It should be OK, for the same reason the RENAME_EXCL branch is OK (it assumes
renameatx_np). If we run across some future platform where it doesn't work, we
can port it then. As far as I know, Cygwin will be the first platform with
renameat2 in its C library.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Sat, 19 Aug 2017 22:06:02 GMT)
Full text and
rfc822 format available.
Message #136 received at 27986 <at> debbugs.gnu.org (full text, mbox):
On 8/19/2017 5:37 PM, Paul Eggert wrote:
> Ken Brown wrote:
>> This is now done. The implementation will appear in the next Cygwin
>> release. When that release occurs, I'll install something like the
>> attached patch.
>
> Thanks.
>
>> Question: Is the patch OK as is, or should I make the call to
>> renameat2 conditional on CYGWIN? In other words, is it safe to assume
>> that renameat2 is defined on any platform on which RENAME_NOREPLACE is
>> defined but not SYS_renameat2?
>
> It should be OK, for the same reason the RENAME_EXCL branch is OK (it
> assumes renameatx_np). If we run across some future platform where it
> doesn't work, we can port it then. As far as I know, Cygwin will be the
> first platform with renameat2 in its C library.
Is there any (good) reason that glibc doesn't provide a wrapper for it?
It seems that this would be pretty trivial to do.
Ken
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Sat, 19 Aug 2017 22:39:02 GMT)
Full text and
rfc822 format available.
Message #139 received at 27986 <at> debbugs.gnu.org (full text, mbox):
On 08/19/2017 03:04 PM, Ken Brown wrote:
>
> Is there any (good) reason that glibc doesn't provide a wrapper for
> it? It seems that this would be pretty trivial to do.
On the glibc side there has been a concern that it support a good GNU
libc interface that does not assume a Linux kernel, as opposed to merely
being a thin wrapper around whatever system calls Linux happens to
provide. I don't know what the specific objections to renameat2 are, though.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Sun, 20 Aug 2017 02:38:01 GMT)
Full text and
rfc822 format available.
Message #142 received at 27986 <at> debbugs.gnu.org (full text, mbox):
> From: Richard Stallman <rms <at> gnu.org>
> Date: Sat, 19 Aug 2017 17:33:56 -0400
>
> > Btw, in case it isn't clear: the issue at hand is an incompatible
> > change to rename-file (and probably also other functions, like
> > copy-file). Where previously (rename-file A B) with B a directory
> > will move A into B/A, under the proposed change it will only do so if
> > B actually ended in a slash; otherwise it will move A to B, deleting B
> > if it exists. The incompatibility will manifest itself if some old
> > code expects to get B/A, but instead gets either an error (if B is a
> > non-empty directory) or B silently removed (if it is empty).
>
> Assuming this applies only when directory B is empty, so that this
> won't delete non-empty directories, then I don't have any objection.
> I would object to deleting non-empty directories here.
>
> Another option that might be good is to make this operation always
> signal an error in the case where B is a directory and does not end
> with a slash.
>
> I don't have an opinion about which of those two is better.
Thanks. I do indeed think that making this an error even if B is an
empty directory would be better, as it will reveal the cases that need
to be fixed more prominently.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Fri, 25 Aug 2017 20:44:02 GMT)
Full text and
rfc822 format available.
Message #145 received at 27986 <at> debbugs.gnu.org (full text, mbox):
>>>>> "EZ" == Eli Zaretskii <eliz <at> gnu.org> writes:
EZ> Thanks. I do indeed think that making this an error even if B is an empty
EZ> directory would be better, as it will reveal the cases that need to be
EZ> fixed more prominently.
I prefer that to renaming a directory when the target is a directory. That's
not how /bin/mv behaves, so I'd be surprised if (rename-file) did differently.
Making the Emacs version more restrictive, however, makes sense.
--
John Wiegley GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com 60E1 46C4 BD1A 7AC1 4BA2
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Sat, 26 Aug 2017 07:32:01 GMT)
Full text and
rfc822 format available.
Message #148 received at 27986 <at> debbugs.gnu.org (full text, mbox):
> From: John Wiegley <jwiegley <at> gmail.com>
> Cc: rms <at> gnu.org, 27986 <at> debbugs.gnu.org
> Date: Fri, 25 Aug 2017 13:33:46 -0700
>
> >>>>> "EZ" == Eli Zaretskii <eliz <at> gnu.org> writes:
>
> EZ> Thanks. I do indeed think that making this an error even if B is an empty
> EZ> directory would be better, as it will reveal the cases that need to be
> EZ> fixed more prominently.
>
> I prefer that to renaming a directory when the target is a directory. That's
> not how /bin/mv behaves, so I'd be surprised if (rename-file) did differently.
> Making the Emacs version more restrictive, however, makes sense.
Thanks.
Paul, it sounds like we have a consensus on this, so please go ahead
with the changes. At least Richard, John, and myself prefer to signal
an error even when the target is an empty directory, so I hope the
non-interactive rename-file could do that after your changes.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Sun, 10 Sep 2017 22:50:01 GMT)
Full text and
rfc822 format available.
Message #151 received at 27986 <at> debbugs.gnu.org (full text, mbox):
OK, I installed the patch into the master branch. Also, I reviewed all calls to
the affected functions and have a few followup patches that I plan to install
separately soon, and mention here as I do.
Reply sent
to
Paul Eggert <eggert <at> cs.ucla.edu>:
You have taken responsibility.
(Mon, 11 Sep 2017 06:08:01 GMT)
Full text and
rfc822 format available.
Notification sent
to
Philipp <p.stephani2 <at> gmail.com>:
bug acknowledged by developer.
(Mon, 11 Sep 2017 06:08:02 GMT)
Full text and
rfc822 format available.
Message #156 received at 27986-done <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Paul Eggert wrote:
> I reviewed all calls to the affected functions and have a few followup patches
> that I plan to install separately soon, and mention here as I do.
I installed the patches (attached). Closing the bug report.
I'll CC: this to Michael, as the last patch is to the Tramp tests, and he may
want Tramp to become consistent with the new behavior for rename-file etc.
[0001-Make-copy-directory-act-like-copy-file-etc.patch (text/x-patch, attachment)]
[0002-Make-write-file-act-like-copy-file-etc.patch (text/x-patch, attachment)]
[0003-Make-gnus-copy-file-act-like-copy-file-etc.patch (text/x-patch, attachment)]
[0004-Adjust-ob-tangle-to-new-copy-file-behavior.patch (text/x-patch, attachment)]
[0005-Adjust-thumbs-to-new-rename-file-behavior.patch (text/x-patch, attachment)]
[0006-Port-tramp-tests-to-new-copy-directory-behavior.patch (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Mon, 11 Sep 2017 14:48:02 GMT)
Full text and
rfc822 format available.
Message #159 received at 27986 <at> debbugs.gnu.org (full text, mbox):
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> Cc: johnw <at> gnu.org, rms <at> gnu.org, p.stephani2 <at> gmail.com,
> 27986-done <at> debbugs.gnu.org, Michael Albinus <michael.albinus <at> gmx.de>
> Date: Sun, 10 Sep 2017 23:07:34 -0700
>
> Paul Eggert wrote:
> > I reviewed all calls to the affected functions and have a few followup patches
> > that I plan to install separately soon, and mention here as I do.
>
> I installed the patches (attached). Closing the bug report.
Am I missing something, or do these changes modify interactive
behavior in incompatible ways? I thought we agreed to leave the
interactive behavior intact, and only change the non-interactive uses.
Thanks.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Mon, 11 Sep 2017 16:46:02 GMT)
Full text and
rfc822 format available.
Message #162 received at 27986 <at> debbugs.gnu.org (full text, mbox):
On 09/11/2017 07:47 AM, Eli Zaretskii wrote:
> do these changes modify interactive
> behavior in incompatible ways? I thought we agreed to leave the
> interactive behavior intact, and only change the non-interactive uses.
>
That wasn't my understanding. In our last email exchange about
interactivity I proposed that Emacs prompt the user when the destination
is not a directory name but happens to be a directory (see
Bug#27986#97), but you were dubious about that (Bug#27986#100) so I left
it alone.
I normally use dired to rename files in Emacs, and dired's behavior
hasn't changed as far as I can tell. Where there is a bit of a change is
when using M-x rename-file directly. Here, in the typical case with file
name completion there is still no difference, since names of destination
directories are completed to have trailing /. However, if one uses "M-x
rename-file foo RET and then laboriously types out the name of an
existing directory /tmp/destination-dir without using completion and
without trailing / before hitting RET, one will notice a difference:
rename-file will now say "File /tmp/destination-dir already exists;
rename to it anyway? (yes or no)" and if one types "yes" the rename will
typically fail. So yes, this is a (noisy) incompatibility with previous
usage. If you like I can go back and implement the suggestion in
Bug#27986#97; this would be more-compatible with existing usage. I
suggest leaving it alone, though, as things are simpler and easier to
explain the way they are.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Mon, 11 Sep 2017 17:11:02 GMT)
Full text and
rfc822 format available.
Message #165 received at 27986 <at> debbugs.gnu.org (full text, mbox):
> Cc: monnier <at> IRO.UMontreal.CA, johnw <at> gnu.org, rms <at> gnu.org,
> p.stephani2 <at> gmail.com, 27986 <at> debbugs.gnu.org, michael.albinus <at> gmx.de
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> Date: Mon, 11 Sep 2017 09:45:48 -0700
>
> On 09/11/2017 07:47 AM, Eli Zaretskii wrote:
> > do these changes modify interactive
> > behavior in incompatible ways? I thought we agreed to leave the
> > interactive behavior intact, and only change the non-interactive uses.
> >
> That wasn't my understanding. In our last email exchange about
> interactivity I proposed that Emacs prompt the user when the destination
> is not a directory name but happens to be a directory (see
> Bug#27986#97), but you were dubious about that (Bug#27986#100) so I left
> it alone.
>
> I normally use dired to rename files in Emacs, and dired's behavior
> hasn't changed as far as I can tell. Where there is a bit of a change is
> when using M-x rename-file directly. Here, in the typical case with file
> name completion there is still no difference, since names of destination
> directories are completed to have trailing /. However, if one uses "M-x
> rename-file foo RET and then laboriously types out the name of an
> existing directory /tmp/destination-dir without using completion and
> without trailing / before hitting RET, one will notice a difference:
> rename-file will now say "File /tmp/destination-dir already exists;
> rename to it anyway? (yes or no)" and if one types "yes" the rename will
> typically fail. So yes, this is a (noisy) incompatibility with previous
> usage. If you like I can go back and implement the suggestion in
> Bug#27986#97; this would be more-compatible with existing usage. I
> suggest leaving it alone, though, as things are simpler and easier to
> explain the way they are.
I think there was a confusion here between interactive and
non-interactive uses of rename-file. For interactive use, AFAIR we
agreed that the behavior should stay as before, in particular to be
consistent with e.g. invocation of 'mv' from the shell prompt.
For non-interactive use, we agreed to change the behavior wrt
directories, with a slight preference to signaling an error even if
the target is an empty directory.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Mon, 11 Sep 2017 17:26:01 GMT)
Full text and
rfc822 format available.
Message #168 received at 27986 <at> debbugs.gnu.org (full text, mbox):
On 09/11/2017 10:09 AM, Eli Zaretskii wrote:
> I think there was a confusion here between interactive and
> non-interactive uses of rename-file. For interactive use, AFAIR we
> agreed that the behavior should stay as before, in particular to be
> consistent with e.g. invocation of 'mv' from the shell prompt.
That wasn't my understanding, and the email record is consistent with my
interpretation. Although we discussed mv, the last comment on that topic
was from John, who wrote about mv that "Making the Emacs version more
restrictive, however, makes sense" (Bug#27986#145). This corresponds to
the patch I installed, whose rename-file is more restrictive than mv
because it balks at renaming a regular file F to G when G is not a
directory name but happens to be an existing directory.
As I wrote, I don't think this will matter much in practice because the
point is moot when file name completion is used. If I'm wrong and it is
a significant problem, I can still implement the suggestion in
Bug#27986#97 to ameliorate it. It may be better, though, to try out what
we have now for a while, to see whether that suggestion would actually help.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#27986; Package
emacs.
(Tue, 12 Sep 2017 09:27:02 GMT)
Full text and
rfc822 format available.
Message #171 received at 27986 <at> debbugs.gnu.org (full text, mbox):
Paul Eggert <eggert <at> cs.ucla.edu> writes:
Hi Paul,
> I'll CC: this to Michael, as the last patch is to the Tramp tests, and
> he may want Tramp to become consistent with the new behavior for
> rename-file etc.
I've checked Tramp: it works already proper, thanks to Kai. At least for
the methods handled in tramp-sh.el. Next days, I'll check also
tramp-adb.el, tramp-gvfs.el, and tramp-smb.el.
I've added some additional tests to tramp-tests.el, in order to provoke
an error when a directory as target does not look like a directory name.
Best regards, Michael.
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org.
(Tue, 10 Oct 2017 11:24:05 GMT)
Full text and
rfc822 format available.
This bug report was last modified 7 years and 257 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.