Issue #5092 has been updated by Daniel Pittman.
The first part of my proposed fix for this issue is now available as a branch in github: <http://github.com/daniel-pittman/puppet/commits/feature/5092/> It improves two key areas: It replaces the hand-written (and insecure) implementation of Tempfile with Tempfile, ensuring among other things that puppet is no longer vulnerable to a TOCTOU write-through-symlink attack if it manages files in any user controlled directory. (Tempfile uses O_CREAT|O_EXCL which causes that particular attack to fail, where the old code would happily write through a symlink created after it checked that it didn't exist.) It also handles file writes nicely in that, ensuring that the data is pushed to disk as safely as possible even if the system uncleanly restarts during a puppet run. Finally, it removes the window where permissions on the file could be wrong; previously puppet would rename the file into place, then push through the permissions. Now it sets those correctly before the rename operation, but after the content is written, and ensures that change is persistent. This ensures there is no longer a window where users of that file will see incorrect permissions. This is not a security vulnerability, per se, but rather a usability issue: the permissions would almost always have been *more* restrictive rather than less restrictive than they should have been. The failure mode here would be that an application would be unable to read the configuration file during the race window between property_fix applying the setting and the client application trying to acess the file. Further work required: * the property_fix solution is kind of ugly, and could probably do with more cleaning up than it currently has. * this only fixed the file type/provider, other places in puppet still get this wrong, potentially in very globally risky (and perhaps security critical) ways. ---------------------------------------- Feature #5092: ensure that files written to disk persist over machine crashes https://projects.puppetlabs.com/issues/5092 Author: Daniel Pittman Status: Unreviewed Priority: Normal Assignee: Category: file Target version: Affected Puppet version: development Keywords: Branch: At the moment puppet writes files in a wide range of ways, including some that are unsafe over a machine crash because they assume that 'rename' is atomic *and* durable, when it is only the former on many POSIX platforms. To achieve that puppet needs to ensure that the content of the replacement file is safely on disk before issuing the rename; any other mode of operation will result in a window (potentially minutes long) where some platforms will have neither the old or new versions of the file in place after an unclean restart of the filesystem. The puppet file type/provider is probably the most important target for fixing this, but it would be ideal to provide standard support for the 'atomic replace' operation across puppet - so that external users can also get this relatively tricky operation right. -- You have received this notification because you have either subscribed to it, or are involved in it. To change your notification preferences, please click here: http://projects.puppetlabs.com/my/account -- You received this message because you are subscribed to the Google Groups "Puppet Bugs" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/puppet-bugs?hl=en.
