LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Stephen Smalley <sds@tycho.nsa.gov>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: James Morris <jmorris@namei.org>, Andrew Morton <akpm@osdl.org>,
	Ingo Molnar <mingo@elte.hu>,
	tglx@linutronix.de, linux-kernel@vger.kernel.org,
	selinux@tycho.nsa.gov
Subject: Re: [PATCH] sysctl selinux: Don't look at table->de
Date: Mon, 29 Jan 2007 14:26:43 -0500	[thread overview]
Message-ID: <1170098803.8720.143.camel@moss-spartans.epoch.ncsc.mil> (raw)
In-Reply-To: <m14pq9lntk.fsf@ebiederm.dsl.xmission.com>

On Mon, 2007-01-29 at 10:55 -0700, Eric W. Biederman wrote:
> James Morris <jmorris@namei.org> writes:
> 
> > On Mon, 29 Jan 2007, Stephen Smalley wrote:
> >
> >> NAK.  Mapping all sysctls to a single security label prevents any kind
> >> of fine-grained security on sysctls, and current policies already make
> >> use of the current distinctions to limit access to particular sets of
> >> sysctls to particular processes.  As is, I'd expect breakage of current
> >> systems running SELinux from this patch, because (confined) processes
> >> that formerly only required access to specific sysctl labels will
> >> suddenly run into denials on the generic fallback label.
> >
> > Agreed, 100% NACK.
> >
> > Please don't just simply remove long-researched & analyzed MAC security 
> > which has been in the kernel for years, which is being used in the field 
> > for high assurance systems, because you neglected to consider it during a 
> > code cleanup.
> 
> Please don't shoot the messenger when a weakness is found in your code.

I'm not sure how breaking our code with your set of patches qualifies as
finding a weakness.  I will agree that the current handling of sysctl in
selinux is fragile and can be improved, but it did work (prior to your
patches).

> Systems that increase security without worry that their implementation
> is correct do not impress me, but I do understand that security has
> little to do with correctness and everything to do with making it
> _expensive_ for the other guy to do what he isn't supposed to.

I think you misunderstand; we are concerned about the correctness of our
implementation.  I think that possibly you are misunderstanding one of
the SELinux FAQ answers outside of its historical context (the initial
release of a proof-of-concept implementation of flexible MAC in Dec
2000).

> This code path was always in the selinux code for when /proc was
> compiled out.  I could see no way to preserve it so I removed
> it.
> 
> Not knowing if it was a problem, or if we needed to do something more
> I copied the people who did, at the first available opportunity.
> Before this code makes it's way into peoples production systems.

Which we appreciate, although it would be nice if you tried building
with selinux (and ideally testing with it) in the first place.

> Of course after all of the rants against path based security I was
> amazed to find a code path that was using exactly that in selinux.

To clarify, in this case, the pathname (relative to the root of proc) is
derived from the proc_dir_entry hierarchy and is thus not ambiguous or
mutable by userspace, unlike the pathname-based approaches that we have
criticized.  There is a difference.  But we are open to improving the
approach via explicit marking of the ctl_table entries with sufficient
information.

> I'm trying to make things correct, and simple and will be happy to
> work with you in a way to achieve what you need in a way that does
> not conflict with the rest of the kernel.

Good.

-- 
Stephen Smalley
National Security Agency


  reply	other threads:[~2007-01-29 19:30 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <200701280106.l0S16CG3019873@shell0.pdx.osdl.net>
     [not found] ` <20070127172410.2b041952.akpm@osdl.org>
     [not found]   ` <1169972718.17469.164.camel@localhost.localdomain>
     [not found]     ` <20070128003549.2ca38dc8.akpm@osdl.org>
     [not found]       ` <20070128093358.GA2071@elte.hu>
     [not found]         ` <20070128095712.GA6485@elte.hu>
     [not found]           ` <20070128100627.GA8416@elte.hu>
     [not found]             ` <20070128104548.a835d859.akpm@osdl.org>
2007-01-28 19:21               ` Eric W. Biederman
2007-01-29 13:04                 ` Stephen Smalley
2007-01-29 15:23                   ` James Morris
2007-01-29 17:55                     ` Eric W. Biederman
2007-01-29 19:26                       ` Stephen Smalley [this message]
2007-01-29 17:43                   ` Eric W. Biederman
2007-01-29 18:43                     ` Stephen Smalley
2007-01-29 19:08                       ` Casey Schaufler
2007-01-29 20:07                         ` Stephen Smalley
2007-01-30 10:25                         ` Christoph Hellwig
2007-01-30 17:19                           ` Casey Schaufler
2007-01-29 19:16                       ` Eric W. Biederman
2007-01-29 23:28                       ` Russell Coker
2007-02-06 21:16                   ` [PATCH 1/2] sysctl: Add a parent entry to ctl_table and set the parent entry Eric W. Biederman
2007-02-06 21:21                     ` [PATCH 2/2] sysctl: Restore the selinux path based label lookup for sysctls Eric W. Biederman
2007-02-07 18:24                       ` Stephen Smalley
2007-02-07 21:12                         ` Stephen Smalley
2007-02-07 21:54                           ` Stephen Smalley
2007-02-07 22:21                             ` Eric W. Biederman
2007-02-08 15:07                               ` Stephen Smalley
2007-02-08  1:57                           ` Eric W. Biederman
2007-02-08 15:01                             ` Stephen Smalley
2007-02-08 17:53                               ` Eric W. Biederman
2007-02-08 18:13                                 ` Stephen Smalley
2007-02-08 22:17                                   ` Eric W. Biederman
2007-02-08 22:51                                     ` [PATCH 0/5] sysctl cleanup selinux fixes Eric W. Biederman
2007-02-08 22:53                                       ` [PATCH 1/5] sysctl: Remove declaration of nonexistent sysctl_init() Eric W. Biederman
2007-02-08 22:54                                         ` [PATCH 2/5] sysctl: Set the parent field in the root sysctl table Eric W. Biederman
2007-02-08 22:55                                           ` [PATCH 3/5] sysctl: Fix the selinux_sysctl_get_sid Eric W. Biederman
2007-02-08 23:02                                             ` [PATCH 4/5] selinux: Enhance selinux to always ignore private inodes Eric W. Biederman
2007-02-08 23:04                                               ` [PATCH 5/5] sysctl: Hide the sysctl proc inodes from selinux Eric W. Biederman
2007-02-09 12:26                                               ` [PATCH 4/5] selinux: Enhance selinux to always ignore private inodes Stephen Smalley
2007-02-09 12:24                                             ` [PATCH 3/5] sysctl: Fix the selinux_sysctl_get_sid Stephen Smalley
2007-02-09 11:05                                       ` [PATCH 0/5] sysctl cleanup selinux fixes Andrew Morton
2007-02-09 18:09                                         ` Eric W. Biederman

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=1170098803.8720.143.camel@moss-spartans.epoch.ncsc.mil \
    --to=sds@tycho.nsa.gov \
    --cc=akpm@osdl.org \
    --cc=ebiederm@xmission.com \
    --cc=jmorris@namei.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=selinux@tycho.nsa.gov \
    --cc=tglx@linutronix.de \
    --subject='Re: [PATCH] sysctl selinux: Don'\''t look at table->de' \
    /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

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).