The Puppet Labs Issue Tracker has Moved: https://tickets.puppetlabs.com
introduce better, and more secure, file handling abstractions, then use them in our code
|Assignee:||Dominic Maraglia||% Done:|
|Affected Puppet version:||Branch:|
Ticket tracking is now hosted in JIRA: https://tickets.puppetlabs.com
This issue is currently not available for export. If you are experiencing the issue described below, please file a new ticket in JIRA. Once a new ticket has been created, please add a link to it that points back to this Redmine ticket.
We have a bunch of places that implement “secure” file handling, and a bunch of old ways to do that safely. For example,
secure_open is rarely used and has some … very strange behaviours, but implements one pattern for securely writing to a file.
Other places, such as the
file type, the
ssh_authorized_keys type, and the
k5login type (and their providers), have other patterns for securely handling this:
* change to the target user before operating, potentially writing through a symlink anyway.
* write your own “secure temporary file” operation with a predictable filename and bad symlink behaviour
We also have some methods over in the settings, and other parts of the code, that work to write files out safely.
We should unify these into a single, sane file handling system. This should be useful in all the places we write files, and replace them.
My work on
replace_file is a start to this, but we should talk about the details.
Jeff references this:
Basically what’s described in the safeopen paper  which mainly revolves around validating the path is only modifiable by trusted ID’s (EUID and root to start).  http://research.cs.wisc.edu/mist/safefile/safeopen_ares2008.pdf
#2 Updated by Deepak Giridharagopal about 2 years ago
- Assignee changed from Deepak Giridharagopal to Dominic Maraglia
#3 Updated by Daniel Pittman about 2 years ago
I helpfully reviewed both branches, and left a bunch of comments. Pretty much everything on the 2.6.x branch also applies to the 2.7.x branch; the comments there are deliberately just the extra stuff that, eg, Windows added.
Apparently I had more things on my mental todo list for this code than was obvious to me when I wrote the tickets. Sorry that wasn’t super well communicated.