selinux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Paul Moore <paul@paul-moore.com>
To: Demi Marie Obenour <demiobenour@gmail.com>
Cc: Stephen Smalley <stephen.smalley.work@gmail.com>,
	Eric Paris <eparis@parisplace.org>,
	selinux@vger.kernel.org, linux-kernel@vger.kernel.org,
	selinux-refpolicy@vger.kernel.org
Subject: Re: [PATCH] SELinux: Always allow FIOCLEX and FIONCLEX
Date: Wed, 26 Jan 2022 17:41:44 -0500	[thread overview]
Message-ID: <CAHC9VhQTZdeNOx3AXdoc9LXUzDk5n7wyGBX-tV-ZaovhPAdWwQ@mail.gmail.com> (raw)
In-Reply-To: <c8a616e4-26a6-af51-212c-31dca0e265cd@gmail.com>

On Tue, Jan 25, 2022 at 5:50 PM Demi Marie Obenour
<demiobenour@gmail.com> wrote:
> On 1/25/22 17:27, Paul Moore wrote:
> > On Tue, Jan 25, 2022 at 4:34 PM Demi Marie Obenour
> > <demiobenour@gmail.com> wrote:
> >>
> >> These ioctls are equivalent to fcntl(fd, F_SETFD, flags), which SELinux
> >> always allows too.  Furthermore, a failed FIOCLEX could result in a file
> >> descriptor being leaked to a process that should not have access to it.
> >>
> >> Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
> >> ---
> >>  security/selinux/hooks.c | 5 +++++
> >>  1 file changed, 5 insertions(+)
> >
> > I'm not convinced that these two ioctls should be exempt from SELinux
> > policy control, can you explain why allowing these ioctls with the
> > file:ioctl permission is not sufficient for your use case?  Is it a
> > matter of granularity?
>
> FIOCLEX and FIONCLEX are applicable to *all* file descriptors, not just
> files.  If I want to allow them with SELinux policy, I have to grant
> *:ioctl to all processes and use xperm rules to determine what ioctls
> are actually allowed.  That is incompatible with existing policies and
> needs frequent maintenance when new ioctls are added.
>
> Furthermore, these ioctls do not allow one to do anything that cannot
> already be done by fcntl(F_SETFD), and (unless I have missed something)
> SELinux unconditionally allows that.  Therefore, blocking these ioctls
> does not improve security, but does risk breaking userspace programs.
> The risk is especially great because in the absence of SELinux, I
> believe FIOCLEX and FIONCLEX *will* always succeed, and userspace
> programs may rely on this.  Worse, if a failure of FIOCLEX is ignored,
> a file descriptor can be leaked to a child process that should not have
> access to it, but which SELinux allows access to.  Userspace
> SELinux-naive sandboxes are one way this could happen.  Therefore,
> blocking FIOCLEX may *create* a security issue, and it cannot solve one.

I can see you are frustrated with my initial take on this, but please
understand that excluding an operation from the security policy is not
something to take lightly and needs discussion.  I've added the
SELinux refpolicy list to this thread as I believe their input would
be helpful here.

--
paul-moore.com

  reply	other threads:[~2022-01-26 22:42 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-25 21:34 [PATCH] SELinux: Always allow FIOCLEX and FIONCLEX Demi Marie Obenour
2022-01-25 22:27 ` Paul Moore
2022-01-25 22:50   ` Demi Marie Obenour
2022-01-26 22:41     ` Paul Moore [this message]
2022-01-30  3:40       ` Demi Marie Obenour
2022-02-01 17:26         ` Paul Moore
2022-02-02 10:13           ` Demi Marie Obenour
2022-02-03 23:44             ` Paul Moore
2022-02-04 13:48               ` Chris PeBenito
2022-02-05 11:19                 ` Dominick Grift
2022-02-05 13:13                   ` Demi Marie Obenour
2022-02-08 14:17                   ` William Roberts
2022-02-08 15:47                     ` Chris PeBenito
2022-02-08 16:47                       ` Dominick Grift
2022-02-08 23:44                         ` David Laight
2022-02-14  7:11                     ` Jeffrey Vander Stoep
2022-02-15 20:34                       ` Paul Moore
2022-02-17 15:04                         ` Christian Göttsche
2022-02-17 22:25                           ` Paul Moore
2022-02-17 23:55                         ` Demi Marie Obenour
2022-02-18 15:06                           ` Richard Haines
2022-02-18 15:39                           ` Richard Haines
2022-02-20  1:15                             ` Demi Marie Obenour
2022-02-07 17:00               ` William Roberts
2022-02-07 17:08                 ` Demi Marie Obenour
2022-02-07 18:35                   ` William Roberts
2022-02-07 21:12                     ` Demi Marie Obenour
2022-02-07 21:42                       ` William Roberts
2022-02-07 21:50                         ` William Roberts
2022-02-08  0:01                           ` Paul Moore
2022-02-08 14:05                             ` William Roberts
2022-02-08 16:26                               ` Paul Moore

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAHC9VhQTZdeNOx3AXdoc9LXUzDk5n7wyGBX-tV-ZaovhPAdWwQ@mail.gmail.com \
    --to=paul@paul-moore.com \
    --cc=demiobenour@gmail.com \
    --cc=eparis@parisplace.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=selinux-refpolicy@vger.kernel.org \
    --cc=selinux@vger.kernel.org \
    --cc=stephen.smalley.work@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).