[libvirt] [PATCH 2/4] Introduce virFileRewrite for safe file rewrite

[Eric Blake]

On 10/26/2011 03:16 PM, Jiri Denemark wrote:
> On Mon, Oct 24, 2011 at 16:48:58 -0600, Eric Blake wrote:
>> On 10/19/2011 11:26 AM, Jiri Denemark wrote:
>>> When saving config files we just overwrite old content of the file. In
>>> case something fails during that process (e.g. disk gets full) we lose
>>> both old and new content. This patch makes the process more robust by
>>> writing the new content into a separate file and only if that succeeds
>>> the original file is atomically replaced with the new one.
>>> ---

>>> +    if ((fd = open(newfile, O_WRONLY | O_CREAT | O_TRUNC, mode))<   0) {
>>
>> Should we be using O_EXCL instead of O_TRUNC, and insisting that no .new
>> file already exists, so as to protect ourselves from symlink attacks
>> where someone installs a symlink and tricks us into truncating a random
>> file?
>
> I think this would need to be configurable and can be safely deferred to the
> future when there is a need to use this API for other purposes than writing
> libvirt's XML files. In this case we don't care about existing .new files and
> it's much easier for the user running libvirtd to replace a random file than
> trick libvirtd to do it.

On the other hand, while the lack of O_EXCL isn't as robust as it could 
be, if someone already has enough write permissions to inject a symlink 
attack into our libvirt-internal directory with .new files, they already 
have enough permissions to do a lot of other damage, too.  I'm okay with 
pushing this as-is for now, and worrying about further improvements with 
O_EXCL at a later date, if we ever use this function in more than just 
libvirt-internal directories.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org