From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Google-Smtp-Source: AG47ELu8sXeE8tTxGfYG9Sz8d6SEvxlxeEwSi01AI9CIA85SR7FinOHvrcvhLyKDJt7xpC30EyXQ ARC-Seal: i=1; a=rsa-sha256; t=1519762960; cv=none; d=google.com; s=arc-20160816; b=Y+/2uc1pXOoVD0sB6iCk5lU/N8WGyUEPHhuIOTcnHP7gazXDCiZdkoh+z+Fiu556hn iUhsqNOhiiLVlRwYHpBXuJPzRYlKycTgd05uT8137Tt1e2BjUtvZPztrga0akyGiZY9e Phw4fakZQ0UazxRy5oKeaYz0QVq0V//0U2lczJQWu1/rx6cwAVr/bwg0CrPbXCLUgHLt vkCieRAwHKVo/wotunhf829G4FjkloG3iu2NNGqcPYybZXfIXMVr2dmhChduI2ppG/Xd pcc7fzK40lfXqdhp6RRH0ifJfY4YzMfYu94mwDuENp2b56fsJp9ctDrgWO7g9LNI1kIA GmTQ== 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=TbgiwYgzNbcvQBOO1LGzjHgj+MbDaH/LBb/xsX8q4wY=; b=QI6uIt57jXQQyt/5mviIIdpK3W4SdH3PV9qKTdpCoaIDELfkgmIdeTiBhiAR07wxon cu/lZMTIs0vDQUMtTrdCfWCsa8/ilhE7hCbwy/QzlYqdb6looGkiPomnAveXF8NYdriF vNxn6Sw2HabWC4hGK6MYr//gYlGb9Msut41bPFxR0TdQv93VvPw5rgEkI5FyPfyobJMN 2g9GWWJCsSVyzTWM33hau3ZDYFbmslxGRElGGkGjkNCDFhRZh3N/nHc/HHSg6qevdmNR 7MriryOq9AffbBCIb+o0MgZuj0BWgBztSCOdgkhPrIE/bJ4qUOcLym/MO0saKOeKgoQ9 XfjA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20161025 header.b=KK+shY3C; dkim=pass header.i=@chromium.org header.s=google header.b=ju46Q0tO; spf=pass (google.com: domain of kernel-hardening-return-12013-gregkh=linuxfoundation.org@lists.openwall.com designates 195.42.179.200 as permitted sender) smtp.mailfrom=kernel-hardening-return-12013-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=KK+shY3C; dkim=pass header.i=@chromium.org header.s=google header.b=ju46Q0tO; spf=pass (google.com: domain of kernel-hardening-return-12013-gregkh=linuxfoundation.org@lists.openwall.com designates 195.42.179.200 as permitted sender) smtp.mailfrom=kernel-hardening-return-12013-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: References: <1519729200-16056-1-git-send-email-s.mesoraca16@gmail.com> From: Kees Cook Date: Tue, 27 Feb 2018 12:22:18 -0800 X-Google-Sender-Auth: LtfsI3fAYX2mgYsxzWgnd_0cp_A 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?1593586965639507114?= X-Mailing-List: linux-kernel@vger.kernel.org List-ID: On Tue, Feb 27, 2018 at 11:47 AM, Kees Cook wrote: > 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've also tested this now; so: Tested-by: Kees Cook # grep . /proc/sys/fs/protected_* /proc/sys/fs/protected_fifos:0 /proc/sys/fs/protected_hardlinks:1 /proc/sys/fs/protected_regular:0 /proc/sys/fs/protected_symlinks:1 # cd /tmp # mkfifo fifo # touch regular # chown nobody fifo regular # chmod a+w fifo regular # chmod a+w regular # cat fifo > output & # su - keescook $ cd /tmp $ python ... >>> import os >>> fd = os.open("fifo", os.O_RDWR | os.O_CREAT) >>> os.write(fd, "OHAI\n") 5 >>> fd = os.open("regular", os.O_RDWR | os.O_CREAT) >>> os.write(fd, "OHAI\n") 5 >>> exit() $ exit # echo 1 > /proc/sys/fs/protected_fifos # echo 1 > /proc/sys/fs/protected_regular # su - keescook $ cd /tmp $ python ... >>> import os >>> fd = os.open("fifo", os.O_RDWR | os.O_CREAT) Traceback (most recent call last): File "", line 1, in OSError: [Errno 13] Permission denied: 'fifo' >>> fd = os.open("regular", os.O_RDWR | os.O_CREAT) Traceback (most recent call last): File "", line 1, in OSError: [Errno 13] Permission denied: 'regular' > I'll create a branch for this on git.kernel.org and see if anything > surprising pops out. :) Here it is with my suggested refactoring of the audit message: https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=kspp/userspace/protected-creat Which produces: [ 146.854080] audit: type=1703 audit(1519762816.978:95): op=fifo ppid=3091 pid=3092 auid=0 uid=1000 gid=1000 euid=1000 suid=1000 fsuid=1000 egid=1000 sgid=1000 fsgid=1000 tty=pts8 ses=1 comm="python" exe="/usr/bin/python2.7" res=0 [ 146.858691] audit: type=1302 audit(1519762816.978:95): item=0 name="/tmp/fifo" inode=531 dev=fd:01 mode=010666 ouid=65534 ogid=0 rdev=00:00 nametype=NORMAL cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0 ... [ 152.993518] audit: type=1703 audit(1519762823.117:96): op=regular ppid=3091 pid=3092 auid=0 uid=1000 gid=1000 euid=1000 suid=1000 fsuid=1000 egid=1000 sgid=1000 fsgid=1000 tty=pts8 ses=1 comm="python" exe="/usr/bin/python2.7" res=0 [ 152.997963] audit: type=1302 audit(1519762823.117:96): item=0 name="/tmp/regular" inode=700 dev=fd:01 mode=0100666 ouid=65534 ogid=0 rdev=00:00 nametype=NORMAL cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0 and other things (uid, etc) -Kees -- Kees Cook Pixel Security