On Mon, Sep 18, 2017 at 02:00:50PM -0700, Kees Cook wrote: > On Fri, Sep 15, 2017 at 1:43 AM, Salvatore Mesoraca <s.mesoraca16@xxxxxxxxx> wrote: > > The purpose is to make data spoofing attacks harder. > > Do you have any examples of attacks (CVEs, blog posts, etc) that you > could link to in this commit? I doubt there are (m)any examples of attacks and blog posts on this, because most systems didn't have similar (sym)link restrictions until recently and those attacks are simpler. As to CVEs, I expect that a large subset of CVEs assigned to "improper usage of temporary files" or "symlink attacks" or such are about vulnerabilities that are also exploitable into data spoofing, even if perhaps with different (and often lesser) impact than they are when exploited via (sym)links. IIRC, the attacks were first pointed out on LKML circa 1997 as an argument against (sym)link restrictions - that they're not enough on their own against some attacks anyway, because FIFOs and even regular files could also be used for those. My response was to include FIFO restrictions (on top of (sym)link restrictions) into -ow patches, and now the same became relevant for mainline (as it has the optional (sym)link restrictions now). One thing that has changed since is the addition of inotify, which probably made use of regular files for similar attacks much more practical. This is one reason why we might also protect against maybe-maliciously pre-created regular files now. Another reason is that we, or at least some users/sysadmins, may want to discourage misuse of /tmp, /dev/shm, and similar directories in general. Those directories are supposed to only be accessed through appropriate library interfaces, or using specific known-safe procedures. Trying to O_CREAT a file in there without O_EXCL is almost never right. It makes sense to provide an easy way to disallow that as policy enforcement. > bike shed: I think "_files" is redundant, since we're already in proc/sys/fs/ +1 > > +protected_regular_files: > > + > > +This protection is similar to protected_fifos, but it > > +avoids writes to an attacker-controlled regular file, where program > > +expected to create one. > > + > > +When set to "0", regular files writing is unrestricted. > > + > > +When set to "1" don't allow O_CREAT open on regular files that we > > +don't own in world writable sticky directories, unless they are > > +owned by the owner of the directory. > > + > > +This protection is based on the restrictions in Openwall. While symlink/hardlink/FIFO restrictions were in -ow patches, the regular file restriction was not - we're just inventing it now, even if upon my suggestion. Although this is sufficient against attacks (if the kernel's check for these properties is not racy; I don't know if it is), for the policy enforcement use case and reason we might want to support a simpler mode where O_CREAT without O_EXCL would be disallowed in sticky directories (only world writable? or also writable by anyone other than us? - e.g., it'd catch some unsafe uses of mail spools) regardless of whether a file of that name already exists or not. Maybe extra settings: When set to "2" also don't allow O_CREAT open without O_EXCL in world-writable sticky directories (even if the files don't already exist, for consistent policy enforcement) When set to "3" also don't allow O_CREAT open on regular files that we don't own in sticky directories writable by anyone else, unless the files are owned by the owner of the directory. When set to "4" also don't allow O_CREAT open without O_EXCL in sticky directories writable by anyone else (even if the files don't already exist, for consistent policy enforcement) Or maybe "2" and "4" should be a separate knob, so that "3" could be used without the policy enforcement aspect of "2", although enabling this separate knob at the highest level would make protected_regular redundant. I could envision further levels for non-sticky world-writable and writable-by-others directories, and even for unsafe writes without O_CREAT and unsafe reads, but then the protected_regular name would become even more misleading as without O_CREAT the program could actually intend to work with a non-regular file. Let's avoid further scope creep for now, but have this in mind. As I had mentioned in another thread on kernel-hardening, policy enforcement like this implemented in a kernel module helped me find weaknesses in old Postfix' privsep implementation, which were promptly patched (that was many years ago). Having this generally available and easy to enable could result in more findings like this by more people. A setting similar to "3" above should probably also exist for protected_fifos (would be "2" there). > Otherwise, yeah, looks good! :) +1 Alexander
