vipw does not work with some editors

vipw does not work with some editors

[Mantas M.]

Some editors, such as Vim with 'writebackup' mode enabled, use "atomic
save" in which the old file is deleted and a new one with the same name
created in its place. When `vipw` is used with such an editor, it never
detects changes, because it compares fstat() on a file descriptor
pointing to the old file.

Can this be fixed in `vipw`, or should I just start configuring Vim to
stop making backups for vipw's temporary files?

-- 
Mantas Mikulėnas <grawity@gmail.com>

Re: vipw does not work with some editors

[Karel Zak]

On Tue, Jul 31, 2012 at 12:20:10AM +0300, Mantas M. wrote:
> Some editors, such as Vim with 'writebackup' mode enabled, use "atomic
> save" in which the old file is deleted and a new one with the same name
> created in its place. When `vipw` is used with such an editor, it never
> detects changes, because it compares fstat() on a file descriptor
> pointing to the old file.
> 
> Can this be fixed in `vipw`, or should I just start configuring Vim to
> stop making backups for vipw's temporary files?

hmm.. vipw from shadow-utils uses stat() rather than fstat(), so it
depends on filename only. Maybe we can use the same solution.

    Karel


-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: vipw does not work with some editors

[Bernhard Voelker]

On 07/30/2012 11:20 PM, Mantas M. wrote:
> Some editors, such as Vim with 'writebackup' mode enabled, use "atomic
> save" in which the old file is deleted and a new one with the same name
> created in its place.

Why's that "atomic"?
I see a race in between the unlink and the creat call.
I'd tend to consider such editor mode "unlucky" for use with vipw.

Have a nice day,
Berny

Re: vipw does not work with some editors

[Mantas Mikulėnas]

On Tue, Jul 31, 2012 at 12:33 PM, Bernhard Voelker
<mail@bernhard-voelker.de> wrote:
>
>
> On 07/30/2012 11:20 PM, Mantas M. wrote:
>> Some editors, such as Vim with 'writebackup' mode enabled, use "atomic
>> save" in which the old file is deleted and a new one with the same name
>> created in its place.
>
> Why's that "atomic"?
> I see a race in between the unlink and the creat call.
> I'd tend to consider such editor mode "unlucky" for use with vipw.

Perhaps I picked the wrong name for it. Some other editors actually implement it
atomically using rename(). Meanwhile, Vim has three settings with at least 36
different combinations, some of which copy and overwrite, others
rename and recreate, etc.

I did find this paragraph in `:help crontab`, however:

						*'backupcopy'* *'bkc'*
'backupcopy' 'bkc'	string	(Vi default for Unix: "yes", otherwise: "auto")
	[snip]
							*crontab*
	One situation where "no" and "auto" will cause problems: A program
	that opens a file, invokes Vim to edit that file, and then tests if
	the open file was changed (through the file descriptor) will check the
	backup file instead of the newly created file.  "crontab -e" is an
	example.

Seems like I could :set bkc=yes and avoid this problem with `vipw`.

I don't know anymore if this is better fixed in vim or vipw...

-- 
Mantas Mikulėnas

Re: vipw does not work with some editors

[Sami Kerola]

On Tue, Jul 31, 2012 at 12:46 PM, Mantas Mikulėnas <grawity@gmail.com> wrote:
> On Tue, Jul 31, 2012 at 12:33 PM, Bernhard Voelker
> <mail@bernhard-voelker.de> wrote:
>>
>>
>> On 07/30/2012 11:20 PM, Mantas M. wrote:
>>> Some editors, such as Vim with 'writebackup' mode enabled, use "atomic
>>> save" in which the old file is deleted and a new one with the same name
>>> created in its place.
>>
>> Why's that "atomic"?
>> I see a race in between the unlink and the creat call.
>> I'd tend to consider such editor mode "unlucky" for use with vipw.
>
> Perhaps I picked the wrong name for it. Some other editors actually implement it
> atomically using rename(). Meanwhile, Vim has three settings with at least 36
> different combinations, some of which copy and overwrite, others
> rename and recreate, etc.
>
> I did find this paragraph in `:help crontab`, however:
>
>                                                 *'backupcopy'* *'bkc'*
> 'backupcopy' 'bkc'      string  (Vi default for Unix: "yes", otherwise: "auto")
>         [snip]
>                                                         *crontab*
>         One situation where "no" and "auto" will cause problems: A program
>         that opens a file, invokes Vim to edit that file, and then tests if
>         the open file was changed (through the file descriptor) will check the
>         backup file instead of the newly created file.  "crontab -e" is an
>         example.
>
> Seems like I could :set bkc=yes and avoid this problem with `vipw`.
>
> I don't know anymore if this is better fixed in vim or vipw...

Perhaps the vipw could check hard link count to open file handle. If
hard link count is zero vipw code should stat(), and inform user a
file with similar path, was put in place of passwd. That might make
vim backupcopy to work, but might also cause some sort of security
issue. At least in theory one could replace temporary file with
malicious copy while root is editing (although that requires root
permissions to begin with, which makes this exploit a bit dumb).

BTW I have some time management challenges at the moment, so I leave
implementing that for later (or someone quicker than me).

-- 
   Sami Kerola
   http://www.iki.fi/kerolasa/