From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Google-Smtp-Source: AG47ELsaqvi+gR/rOe3YyvTOPJuUiHlh8PQw792YnAziQ6+5QYQD7bP8D4bHCDVFPQ3vdW190t3Q ARC-Seal: i=1; a=rsa-sha256; t=1519760887; cv=none; d=google.com; s=arc-20160816; b=lZujQtPA2BZ3lRrMoAA1N9Op0R3ePBOPa72caeQYVZv1mcjp3KUeCQkNQdtNqQjwnX B1uThF8hcEYvvYHPc1dc0m3uoF+nVI/pYcNC+I5N1dhjb4Jn5SMFyeKIy65uPIn2sdHB 2BQa6QoQV6QX8iApwHJloD82dOppDNgJ47hZh9rumvz22eK407Zah582PIDBb1FfKM2Y J+dtlABrk4qmPy7quH+BUySCRD2/dLV5/sm/xqfTu5YjGptee01pX5Z7dOzRfn+OnqF2 aXx2G7/2rLclw/qsUhXrz4h6HbbL6CKuDr1Su0bA0gKY0qby9kisezGhAInY42DmeW5f fpFw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=cc:to:subject:message-id:date:from:references:in-reply-to:sender :mime-version:dkim-signature:dkim-signature:delivered-to:list-id :list-subscribe:list-unsubscribe:list-help:list-post:precedence :mailing-list:arc-authentication-results; bh=qOxnbzTVjCyoLUDcWlYxFLCa4lkMbJ5/Yof9NRpM65U=; b=n93Y5BH4adza5+JLFlW2eEOIyGepWGL5asqqcjQorfFyQXxNyBLEU/qCfB9nDffNZ0 B4gP5xi/qsy/cyYao1kRVox45GKIhESeih5flNsFr0ZbpVkpDzblwcxe9sNJoWYHK+wH Mf3TO4Z0Nffdv+boUfplC6LOfi7sRR+HZTQTSRu4gj3eZJaBqjKWLL3+6KLoOl8WBobg +LfbnNT0CrAbExuObvqwiOc0dH0LjhornKEuCnjdsGJCYP1vperPpGrTSCgHPIRuCCje UVejPF3utl1F79y3zlVYWgH1b2G4lMkdGRg67qOVfipGjQdY8CbMIwHmRibkduUljObl qqPA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20161025 header.b=QdWAJzLk; dkim=pass header.i=@chromium.org header.s=google header.b=BBE0g0mD; spf=pass (google.com: domain of kernel-hardening-return-12012-gregkh=linuxfoundation.org@lists.openwall.com designates 195.42.179.200 as permitted sender) smtp.mailfrom=kernel-hardening-return-12012-gregkh=linuxfoundation.org@lists.openwall.com; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20161025 header.b=QdWAJzLk; dkim=pass header.i=@chromium.org header.s=google header.b=BBE0g0mD; spf=pass (google.com: domain of kernel-hardening-return-12012-gregkh=linuxfoundation.org@lists.openwall.com designates 195.42.179.200 as permitted sender) smtp.mailfrom=kernel-hardening-return-12012-gregkh=linuxfoundation.org@lists.openwall.com; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Mailing-List: contact kernel-hardening-help@lists.openwall.com; run by ezmlm List-Post: List-Help: List-Unsubscribe: List-Subscribe: MIME-Version: 1.0 Sender: keescook@google.com In-Reply-To: <1519729200-16056-1-git-send-email-s.mesoraca16@gmail.com> References: <1519729200-16056-1-git-send-email-s.mesoraca16@gmail.com> From: Kees Cook Date: Tue, 27 Feb 2018 11:47:46 -0800 X-Google-Sender-Auth: Awjjgrh_J1Ki5oFIVtJRahWEJFI Message-ID: Subject: Re: [PATCH v4] Protected FIFOs and regular files To: Salvatore Mesoraca Cc: LKML , Kernel Hardening , "linux-fsdevel@vger.kernel.org" , Alan Cox , Alexander Viro , David Laight , Ian Campbell , Jann Horn , Matthew Wilcox , Pavel Vasilyev , Solar Designer , "Eric W. Biederman" , "Tobin C. Harding" Content-Type: text/plain; charset="UTF-8" X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: =?utf-8?q?1593551596949260408?= X-GMAIL-MSGID: =?utf-8?q?1593584792268286313?= X-Mailing-List: linux-kernel@vger.kernel.org List-ID: On Tue, Feb 27, 2018 at 3:00 AM, Salvatore Mesoraca wrote: > Disallows open of FIFOs or regular files not owned by the user in world > writable sticky directories, unless the owner is the same as that of > the directory or the file is opened without the O_CREAT flag. > The purpose is to make data spoofing attacks harder. > This protection can be turned on and off separately for FIFOs and regular > files via sysctl, just like the symlinks/hardlinks protection. > This patch is based on Openwall's "HARDEN_FIFO" feature by Solar > Designer. > > This is a brief list of old vulnerabilities that could have been prevented > by this feature, some of them even allow for privilege escalation: > CVE-2000-1134 > CVE-2007-3852 > CVE-2008-0525 > CVE-2009-0416 > CVE-2011-4834 > CVE-2015-1838 > CVE-2015-7442 > CVE-2016-7489 > > This list is not meant to be complete. It's difficult to track down > all vulnerabilities of this kind because they were often reported > without any mention of this particular attack vector. > In fact, before hardlinks/symlinks restrictions, fifos/regular > files weren't the favorite vehicle to exploit them. > > Suggested-by: Solar Designer > Suggested-by: Kees Cook > Signed-off-by: Salvatore Mesoraca > --- > > Notes: > Changes in v3: > - Fixed format string for uid_t that is unsigned > (suggested by Jann Horn). > Changes in v4: > - Some English fixes (suggested by Tobin C. Harding). > - The original patchset has been split to help this part > land upstream (suggested by Solar Designer). > > Documentation/sysctl/fs.txt | 36 ++++++++++++++++++++++++++ > fs/namei.c | 61 ++++++++++++++++++++++++++++++++++++++++++--- > include/linux/fs.h | 2 ++ > kernel/sysctl.c | 18 +++++++++++++ > 4 files changed, 114 insertions(+), 3 deletions(-) > > diff --git a/Documentation/sysctl/fs.txt b/Documentation/sysctl/fs.txt > index 6c00c1e..819caf8 100644 > --- a/Documentation/sysctl/fs.txt > +++ b/Documentation/sysctl/fs.txt > @@ -34,7 +34,9 @@ Currently, these files are in /proc/sys/fs: > - overflowgid > - pipe-user-pages-hard > - pipe-user-pages-soft > +- protected_fifos > - protected_hardlinks > +- protected_regular > - protected_symlinks > - suid_dumpable > - super-max > @@ -182,6 +184,24 @@ applied. > > ============================================================== > > +protected_fifos: > + > +The intent of this protection is to avoid unintentional writes to > +an attacker-controlled FIFO, where a program expected to create a regular > +file. > + > +When set to "0", writing to FIFOs is unrestricted. > + > +When set to "1" don't allow O_CREAT open on FIFOs that we don't own > +in world writable sticky directories, unless they are owned by the > +owner of the directory. > + > +When set to "2" it also applies to group writable sticky directories. > + > +This protection is based on the restrictions in Openwall. > + > +============================================================== > + > protected_hardlinks: > > A long-standing class of security issues is the hardlink-based > @@ -202,6 +222,22 @@ This protection is based on the restrictions in Openwall and grsecurity. > > ============================================================== > > +protected_regular: > + > +This protection is similar to protected_fifos, but it > +avoids writes to an attacker-controlled regular file, where a program > +expected to create one. > + > +When set to "0", writing to regular files 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. > + > +When set to "2" it also applies to group writable sticky directories. > + > +============================================================== > + > protected_symlinks: > > A long-standing class of security issues is the symlink-based > diff --git a/fs/namei.c b/fs/namei.c > index 921ae32..eaab668 100644 > --- a/fs/namei.c > +++ b/fs/namei.c > @@ -883,6 +883,8 @@ static inline void put_link(struct nameidata *nd) > > int sysctl_protected_symlinks __read_mostly = 0; > int sysctl_protected_hardlinks __read_mostly = 0; > +int sysctl_protected_fifos __read_mostly; > +int sysctl_protected_regular __read_mostly; > > /** > * may_follow_link - Check symlink following for unsafe situations > @@ -996,6 +998,54 @@ static int may_linkat(struct path *link) > return -EPERM; > } > > +/** > + * may_create_in_sticky - Check whether an O_CREAT open in a sticky directory > + * should be allowed, or not, on files that already > + * exist. > + * @dir: the sticky parent directory > + * @name: the file name > + * @inode: the inode of the file to open > + * > + * Block an O_CREAT open of a FIFO (or a regular file) when: > + * - sysctl_protected_fifos (or sysctl_protected_regular) is enabled > + * - the file already exists > + * - we are in a sticky directory > + * - we don't own the file > + * - the owner of the directory doesn't own the file > + * - the directory is world writable > + * If the sysctl_protected_fifos (or sysctl_protected_regular) is set to 2 > + * the directory doesn't have to be world writable: being group writable will > + * be enough. > + * > + * Returns 0 if the open is allowed, -ve on error. > + */ > +static int may_create_in_sticky(struct dentry * const dir, > + const unsigned char * const name, > + struct inode * const inode) > +{ > + if ((!sysctl_protected_fifos && S_ISFIFO(inode->i_mode)) || > + (!sysctl_protected_regular && S_ISREG(inode->i_mode)) || > + likely(!(dir->d_inode->i_mode & S_ISVTX)) || > + uid_eq(inode->i_uid, dir->d_inode->i_uid) || > + uid_eq(current_fsuid(), inode->i_uid)) > + return 0; > + > + if (likely(dir->d_inode->i_mode & 0002) || > + (dir->d_inode->i_mode & 0020 && > + ((sysctl_protected_fifos >= 2 && S_ISFIFO(inode->i_mode)) || > + (sysctl_protected_regular >= 2 && S_ISREG(inode->i_mode))))) { > + pr_notice_ratelimited("denied writing in '%s' of %u.%u in a sticky directory by UID %u, EUID %u, process %s:%d.\n", > + name, > + from_kuid(&init_user_ns, inode->i_uid), > + from_kgid(&init_user_ns, inode->i_gid), > + from_kuid(&init_user_ns, current_uid()), > + from_kuid(&init_user_ns, current_euid()), > + current->comm, current->pid); Instead of this pr_notice_ratelimited(), I think audit_log_link_denied() should be refactored and used instead. Drop this line from this patch, and I think this is great as-is. The logging can be separate (as it may get heavily bike-shed, as I experienced with hard/symlink restrictions). Otherwise, I think this looks great. Acked-by: Kees Cook I'll create a branch for this on git.kernel.org and see if anything surprising pops out. :) -Kees > + return -EACCES; > + } > + return 0; > +} > + > static __always_inline > const char *get_link(struct nameidata *nd) > { > @@ -3355,9 +3405,14 @@ static int do_last(struct nameidata *nd, > if (error) > return error; > audit_inode(nd->name, nd->path.dentry, 0); > - error = -EISDIR; > - if ((open_flag & O_CREAT) && d_is_dir(nd->path.dentry)) > - goto out; > + if (open_flag & O_CREAT) { > + error = -EISDIR; > + if (d_is_dir(nd->path.dentry)) > + goto out; > + error = may_create_in_sticky(dir, nd->last.name, inode); > + if (unlikely(error)) > + goto out; > + } > error = -ENOTDIR; > if ((nd->flags & LOOKUP_DIRECTORY) && !d_can_lookup(nd->path.dentry)) > goto out; > diff --git a/include/linux/fs.h b/include/linux/fs.h > index 2a81556..9bf4e5c 100644 > --- a/include/linux/fs.h > +++ b/include/linux/fs.h > @@ -72,6 +72,8 @@ > extern int leases_enable, lease_break_time; > extern int sysctl_protected_symlinks; > extern int sysctl_protected_hardlinks; > +extern int sysctl_protected_fifos; > +extern int sysctl_protected_regular; > > typedef __kernel_rwf_t rwf_t; > > diff --git a/kernel/sysctl.c b/kernel/sysctl.c > index f98f28c..295f528 100644 > --- a/kernel/sysctl.c > +++ b/kernel/sysctl.c > @@ -1794,6 +1794,24 @@ static int sysrq_sysctl_handler(struct ctl_table *table, int write, > .extra2 = &one, > }, > { > + .procname = "protected_fifos", > + .data = &sysctl_protected_fifos, > + .maxlen = sizeof(int), > + .mode = 0600, > + .proc_handler = proc_dointvec_minmax, > + .extra1 = &zero, > + .extra2 = &two, > + }, > + { > + .procname = "protected_regular", > + .data = &sysctl_protected_regular, > + .maxlen = sizeof(int), > + .mode = 0600, > + .proc_handler = proc_dointvec_minmax, > + .extra1 = &zero, > + .extra2 = &two, > + }, > + { > .procname = "suid_dumpable", > .data = &suid_dumpable, > .maxlen = sizeof(int), > -- > 1.9.1 > -- Kees Cook Pixel Security