Re: The NSA's Security-Enhanced Linux (fwd)

2000-12-27 Thread Stephen Smalley


On Sat, 23 Dec 2000, Kurt Garloff wrote:

 I wonder how their approach compares to the RSBAC stuff, though.
 The RSBAC (by Amon Ott) has all the infrastructure available to have
 policy based access control; whenever an access decision has to be
 taken, a call via some interface is made to a module, which then
 takes the decision ... Just like PAM in userspace.
 http://www.rsbac.org/

The Security-Enhanced Linux has a well-defined architecture (named Flask)
for flexible mandatory access controls that has been experimentally
validated through several prototype systems (DTMach, DTOS, and Flask).
The architecture provides clean separation of policy from enforcement,
well-defined policy decision interfaces, flexibility in labeling
and access decisions, support for policy changes, and fine-grained
controls over the kernel abstractions.  Detailed studies have been
performed of the ability of the architecture to support a wide variety of
security policies and are available on the DTOS and Flask web pages
accessible via the Background page
(http://www.nsa.gov/selinux/background.html).  A published paper about
the Flask architecture is also available on the Background page.  The
architecture and its implementation in Linux are described in detail in
the documentation (http://www.nsa.gov/selinux/docs.html).  

RSBAC appears to have similar goals to the Security-Enhanced Linux.
Like the Security-Enhanced Linux, it separates policy from enforcement
and supports a variety of security policies.  RSBAC uses a different
architecture (the Generalized Framework for Access Control or GFAC) than
the Security-Enhanced Linux, although the Flask paper notes that at the
highest level of abstraction, the the Flask architecture is consistent
with the GFAC.  However, the GFAC does not seem to fully address the issue
of policy changes and revocation, as discussed in the Flask paper.  RSBAC
also differs in the specifics of its policy interfaces and its controls,
but a careful evaluation of the significance of these differences has
not been performed.

--
Stephen D. Smalley, NAI Labs
[EMAIL PROTECTED]



-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: AppArmor FAQ

2007-04-19 Thread Stephen Smalley
On Wed, 2007-04-18 at 12:41 -0700, Crispin Cowan wrote:
 James Morris wrote:
  On Tue, 17 Apr 2007, Alan Cox wrote:

  I'm not sure if AppArmor can be made good security for the general case,
  but it is a model that works in the limited http environment
  (eg .htaccess) and is something people can play with and hack on and may
  be possible to configure to be very secure.
  
  Perhaps -- until your httpd is compromised via a buffer overflow or 
  simply misbehaves due to a software or configuration flaw, then the 
  assumptions being made about its use of pathnames and their security 
  properties are out the window.

 How is it that you think a buffer overflow in httpd could allow an
 attacker to break out of an AppArmor profile? This is exactly what
 AppArmor was designed to do, and without specifics, this is just FUD.
 
  Without security labeling of the objects being accessed, you can't protect 
  against software flaws, which has been a pretty fundamental and widely 
  understood requirement in general computing for at least a decade.

 Please explain why labels are necessary for effective confinement. Many
 systems besides AppArmor have used non-label schemes for effective
 confinement: TRON, Janus, LIDS, Systrace, BSD Jail, EROS, PSOS, KeyOS,
 AS400, to name just a few. This claim seems bogus. Labels may be your
 method of choice for confinement, but they are far from the only way.

Confinement in its traditional sense (e.g. the 1973 Lampson paper, ACM
Vol 16 No 10) means information flow control, which you have agreed
AppArmor does not and cannot provide.  Yes?  As to the (genuine)
capability-based systems, have a look at the DTOS General System
Security and Assurability Assessment Report for why we have concerns
with their approach.  But that has nothing to do with AppArmor.

Look, if you would just refrain from making misleading statements about
SELinux (not only in this FAQ but throughout your documents and talks),
especially when we've previously refuted those same statements (as in
the first AppArmor submission and its discussion), and honestly
acknowledged the limitations of your approach without trying to spin
them as strengths, I think that there would be nothing to discuss here.
We could just agree to disagree, and you could just focus on addressing
the issues raised by the vfs folks about how to get your changes into an
acceptable form.

-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: AppArmor FAQ

2007-04-19 Thread Stephen Smalley
On Wed, 2007-04-18 at 13:15 -0700, David Lang wrote:
 On Wed, 18 Apr 2007, James Morris wrote:
 
  On Tue, 17 Apr 2007, Alan Cox wrote:
 
  I'm not sure if AppArmor can be made good security for the general case,
  but it is a model that works in the limited http environment
  (eg .htaccess) and is something people can play with and hack on and may
  be possible to configure to be very secure.
 
  Perhaps -- until your httpd is compromised via a buffer overflow or
  simply misbehaves due to a software or configuration flaw, then the
  assumptions being made about its use of pathnames and their security
  properties are out the window.
 
 since AA defines a whitelist of files that httpd is allowed to access, a 
 comprimised one may be able to mess up it's files, but it's still not going 
 to 
 be able to touch other files on the system.
 
  Without security labeling of the objects being accessed, you can't protect
  against software flaws, which has been a pretty fundamental and widely
  understood requirement in general computing for at least a decade.
 
 this is not true. you don't need to label all object and chunks of memory, 
 you 
 just need to have a way to list (and enforce) the objects and memory that the 
 program is allowed to use. labeling them is one way of doing this, but not 
 the 
 only way.

You need a way of providing global and persistent security guarantees
for the data, and per-program profiles based on pathname don't get you
there.  There is no system view in AA, just a bunch of disconnected
profiles.

-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: AppArmor FAQ

2007-04-19 Thread Stephen Smalley
On Thu, 2007-04-19 at 16:35 +, David Wagner wrote:
 James Morris  wrote:
 On Wed, 18 Apr 2007, Crispin Cowan wrote:
  How is it that you think a buffer overflow in httpd could allow an
  attacker to break out of an AppArmor profile?
 
 Because you can change the behavior of the application and then bypass 
 policy entirely by utilizing any mechanism other than direct filesystem 
 access: IPC, shared memory, Unix domain sockets, local IP networking, 
 remote networking etc.
 
 Any halfway decent jail will let you control access to all of those
 things, thereby preventing an 0wned httpd from breaking out of the jail.
 (For instance, Janus did.  So does Systrace.)
 
 Are you saying AppArmor does not allow that kind of control?  Specifics
 would be useful.

Just look at their code and their own description of AppArmor.  It does
not provide that level of control, and by your own metric, it is thus
not a halfway decent jail.  Which begs the question - if that is the
kind of approach you want, why not use a real jail/containers mechanism
for it?

 Also worth noting here is that you have to consider any limited 
 environment as enforcing security policy, and thus its configuration 
 becomes an additional component of security policy.
 
 I don't understand what you are saying.  Yes, the AppArmor policy
 file is part of policy.  Is that what you mean?

I think he means the dependencies on which AppArmor relies, not just its
policy, e.g. since it is name-based, it presumes the filesystem
namespace has been set up by a trusted agent and is correct. 

-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: AppArmor FAQ

2007-04-19 Thread Stephen Smalley
On Tue, 2007-04-17 at 20:05 +0200, Andi Kleen wrote:
 Karl MacMillan [EMAIL PROTECTED] writes:
  
  No - the real fix is to change the applications or to run under a policy
  that confines all applications. Most of the problems with resolv.conf,
  mtab, etc. stem from admin processes (e.g., editors or shell scripts)
  all running under the same unconfined domain.
  
  In some cases applications need modification as only the application has
  enough information to determine the correct label. Usually this means
  preserving labels from input files or separating the output into
  distinct directories so type transitions or label inheritance will work.
  
  restorecond is just a hack not a requirement or a sign that something is
  wrong with the model. That is why it is a userspace application and not
  integrated into the kernel mechanism.
 
 You nicely show one of the major disadvantages of the label model vs the path 
 model here: it requires modification of a lot of applications. 

It is true that many applications that already deal with mode bits need
to become aware of labels, just as with ACLs, and that this makes it a
bit harder and slower to roll out something that is label-based.  But
the right solution is rarely quick and easy, and a lot of work has
already happened to integrate such support into userland.

To look at it in a slightly different way, the AA emphasis on not
modifying applications could be viewed as a limitation.  Ultimately,
users have security goals that go beyond just what the OS can directly
enforce and at least some applications (notably things like X, D-BUS,
PostgreSQL, etc) need to likewise support strong domain separation and
controlled information flow through their own internal objects and
operations.  SELinux provides APIs and infrastructure for such
applications, and has already done quite a bit of work in that space
(D-BUS support, XACE/XSELinux, SE-PostgreSQL), whereas AA seems to have
no interest in going there (and would have to recant its emphasis on no
application mods to do so).  If you actually want to truly confine a
desktop application, you can't limit yourself to the kernel.  And the
label model provides a unifying abstraction for dealing with all of
these various objects, whereas the path/natural abstraction model has
no unifying abstraction at all.

-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: AppArmor FAQ

2007-04-19 Thread Stephen Smalley
On Tue, 2007-04-17 at 16:09 -0700, Crispin Cowan wrote:
 David Safford wrote:
  On Mon, 2007-04-16 at 20:20 -0400, James Morris wrote:

  On Mon, 16 Apr 2007, John Johansen wrote:
  
  Label-based security (exemplified by SELinux, and its predecessors in
  MLS systems) attaches security policy to the data. As the data flows
  through the system, the label sticks to the data, and so security
  policy with respect to this data stays intact. This is a good approach
  for ensuring secrecy, the kind of problem that intelligence agencies have.

  Labels are also a good approach for ensuring integrity, which is one of 
  the most fundamental aspects of the security model implemented by SELinux. 
   
 
  Some may infer otherwise from your document.
  
  In fact, I am not sure how you can provide integrity support without
  labels. AppArmor confines a process, but does not effectively confine 
  its output files, precisely because the output files are not labeled. 
  Other processes are free to access the unlabeled, potentially malicious 
  output files without restriction. 

 That depends on what you mean by integrity. It is true that AppArmor
 does not directly manage information flow. AppArmor assumes that if you
 granted write permission to /etc/resolv.conf, that you meant to do that.
 Whether any other process is permitted to access /etc/resolv.conf is
 determined by those other process's respective profiles.

Integrity protection requires information flow control; you can't
protect a high integrity process from being corrupted by a low integrity
process if you don't control the flow of information.  Plenty of attacks
take the form of a untrusted process injecting data that will ultimately
be used by a more trusted process with a surprising side effect.

And you can't do information flow control if you can't provide global
and persistent protection of the data, which requires labeling it and
preserving that label for its lifetime.

 What AppArmor provides is a way for administrators to confine software
 that they have to run but do not trust. The use of pathnames means that
 the administrator can understand the exact meaning of the security
 policy, without having to do a complete labeling of the file system. The
 flip side of not managing information flow is that each profile is
 independent of every other profile.

They aren't truly independent; the composition may lead to surprising
results where each individual program is confined exactly as you
specified, but in combination, one is able to corrupt the higher
integrity subject by actions taken by the lower integrity subject.
Particularly in the fun area of publically writable directories, where
pathnames are largely useless as an indicator.

-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: AppArmor FAQ

2007-04-19 Thread Stephen Smalley
On Thu, 2007-04-19 at 20:08 +, David Wagner wrote:
 Stephen Smalley  wrote:
 Confinement in its traditional sense (e.g. the 1973 Lampson paper, ACM
 Vol 16 No 10) means information flow control, which you have agreed
 AppArmor does not and cannot provide.
 
 Right, that's how I understand it, too.
 
 However, I think some more caveats are in order.  In all honesty,
 I don't think SELinux solves Lampson's problem, either.
 
 It is useful to distinguish between bit-confinement (confining the
 flow of information, a la Lampson) vs authority-confinement (confining
 the flow of privileges and the ability of the untrusted app to cause
 side effects on the rest of the system).
 
 No Linux system provides bit-confinement, if the confined app is
 malicious.  AppArmor does not provide bit-confinement.  Neither does
 SELinux.  SELinux can stop some kinds of accidental leakage of secrets,
 but it cannot prevent deliberate attempts to leak the secrets that are
 known to malicious apps.  The reason is that, in every system under
 consideration, it is easy for a malicious app to leak any secrets it might
 have to the outside world by using covert channels (e.g., wall-banging).
 In practical terms, Lampson's bit-confinement problem is just not
 solvable.  Oh well, so it goes.
 
 A good jail needs to provide authority-confinement, but thankfully,
 it doesn't need to provide bit-confinement.  I don't know enough about
 AppArmor to know whether it is able to do a good job of providing
 authority-confinement.  If it cannot, then it deserves criticism on
 those grounds.
 
 Often the pragmatic solution to the covert channel problem is to ensure
 that untrusted apps are never given access to critical secrets in the
 first place.  They can't leak something they don't know.  This solves the
 confidentiality problem by avoiding any attempt to tackle the unsolvable
 bit-confinement problem.
 
 Note that the problem of building a good jail is a little different from
 the information flow control problem.

First, I think there is practical value in providing confidentiality
control on the overt channels, even if covert channels remain.  We have
to start somewhere.

Second, information flow is not just a confidentiality issue - see my
other email.  It is quite important as well for integrity, and integrity
corruption in order to assume control over a privileged subject or trick
it into abusing its power can be done solely via a data channel - it
doesn't require explicit flow of authority.

Lastly, if you want to judge AA as a jail mechanism, I think you'll find
it fails there too.  So where does that leave it?  An easy-to-use yet
inadequate solution for MAC or jail.

-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: AppArmor FAQ

2007-04-19 Thread Stephen Smalley
On Thu, 2007-04-19 at 20:54 +, David Wagner wrote:
 Stephen Smalley  wrote:
 Integrity protection requires information flow control; you can't
 protect a high integrity process from being corrupted by a low integrity
 process if you don't control the flow of information.  Plenty of attacks
 take the form of a untrusted process injecting data that will ultimately
 be used by a more trusted process with a surprising side effect.
 
 I don't agree with this blanket statement.  In a number of cases
 of practical interest, useful integrity protection can be achieved
 without full information flow control.  Suppose you have a malicious
 (low integrity) process A, and a target (high integrity) process B.
 We want to prevent A from attacking B.  One way to do that is to ensure
 that A has no overt channel it can use to attack process B, by severely
 restricting A's ability to cause side effects on the rest of the world.
 This is often sufficient to contain the damage that A can do.

If you could do that, I'd call that information flow control - I wasn't
saying you had to eliminate covert channels.  As you said, we don't deal
with those even in SELinux.  The point is that AA can't even do that,
not only because it has incomplete controls but because it bases its
decisions on unreliable identifiers (paths) that doesn't let it provide
global and persistent protection of the data.

 Of course, if the intended functionality of the system requires A to
 communicate data to B, and if you don't trust B's ability to handle
 that data carefully enough, and if A is malicious, then you've got a
 serious problem.
 
 But in a number of cases (enough cases to be useful), you can provide
 a useful level of security without needing information flow control and
 without needing global, persistent labels.

Without a reliable way of identifying the data in a system view, you
can't say anything at all about the data flows.  The labels provide you
with a way of doing that.  The paths are ambiguous, highly mutable, and
often meaningless (particularly for runtime files, temporary files, etc)
from a security pov.

Simple example:  malicious symlink attacks.

-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC][Patch 2/6] integrity: fs hook placement

2007-03-08 Thread Stephen Smalley
On Thu, 2007-03-08 at 12:01 -0600, Serge E. Hallyn wrote:
 Quoting Chris Wright ([EMAIL PROTECTED]):
  * Serge E. Hallyn ([EMAIL PROTECTED]) wrote:
   Are you objecting only to the duplication at the callsites, so that an
   fsnotify-type of consolidation of security and integrity hooks would be
   ok?  Or are you complaining that the security_inode_setxattr and
   integrity_inode_setxattr hooks are too similar anyway, and integrity
   modules should just use some lsm hooks for anything which will be
   authoritative?
  
  It's duplication of callsites with many identical implementations
  that's the problem.
 
 Yes it's ugly...
 
 But I guess it gets a point across :)
 
   (I could see an argument that integirty subsystem should be purely for
   measuring and hence its hooks should never return a value.  Only hitch
   there is that if integrity subsystem hits ENOMEM it should be able to
   refuse the action...)
  
  Right, that's what I was expecting to see, just the measurement
  infrastructure.
 
 So what you are saying is EVM would stay an LSM, with a cooperating
 integrity subsystem *just* doing measurements?
 
 That's kind of what i was expecting too, however that doesn't fit as
 well with the idea that an integrity subsystem prevents the need for lsm
 stacking.  I think the idea was that evm would still be able to enforce
 integrity of selinux xattrs without it stack with selinux.  So I can see
 where this approach came from.

The enforcement mechanism should be directly integrated into SELinux,
not stacked as a separate module.

-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: oprofile / selinux / security_port_sid

2007-03-27 Thread Stephen Smalley
On Tue, 2007-03-27 at 13:06 +0300, Sami Farin wrote:
 is there room for improvement in security_port_sid() ?

Yes, lots of room.  Also, it won't get called per-packet if you enable
secmark (echo 0  /selinux/compat_net or boot with selinux_compat_net=0
or build with SECURITY_SELINUX_ENABLE_SECMARK_DEFAULT), although that
may require updating your policy.

 little test with dns queries (dnsfilter (the client) on local host
 using poll() and dnscache (the server) using epoll (at max 4000 concurrent
 queries):
 (stats for only vmlinux)
 
 CPU: P4 / Xeon, speed 2797.32 MHz (estimated)
 Counted GLOBAL_POWER_EVENTS events (time during which processor is not 
 stopped) with a unit mask of 0x01 (mandatory) count 45000
 Counted FSB_DATA_ACTIVITY events (DRDY or DBSY events on the front side bus) 
 with a unit mask of 0x03 (multiple flags) count 45000
 Counted BRANCH_RETIRED events (retired branches) with a unit mask of 0x05 
 (multiple flags) count 45000
 Counted BRANCH_RETIRED events (retired branches) with a unit mask of 0x0a 
 (multiple flags) count 45000
 samples  %samples  %samples  %samples  %
 symbol name
 220663   10.2181  6704 17.9737  5735  7.5171  271.1989  
 datagram_poll
 1400866.4869  3239  8.6839  3786  4.9624  241.0657  
 sock_poll
 1196365.5399  2172  5.8232  7168  9.3954  241.0657  
 do_poll
 1015124.7006  3987 10.6893  812   1.0643  140.6217  
 udp_get_port
 71008 3.2881  1017  2.7266  2694  3.5311  397  17.6288  
 security_port_sid
 64350 2.9798  144   0.3861  1912  2.5061  6 0.2664  
 add_wait_queue
 60815 2.8161  187   0.5014  3246  4.2546  2 0.0888  
 remove_wait_queue
 47456 2.1975  1823  4.8875  476   0.6239  311.3766  
 udp_v4_lookup_longway
 
 if dnsfilter had used epoll, security_port_sid would
 probably (?) be number one (or two or three) CPU user in kernel.
 
 also note that 17.6% of mispredicted branches occurr in security_port_sid.
 
-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC] [PATCH -mm] file caps: make on-disk capabilities future-proof

2007-02-20 Thread Stephen Smalley
On Mon, 2007-02-19 at 11:01 -0600, Serge E. Hallyn wrote:
 From: Serge E. Hallyn [EMAIL PROTECTED]
 Subject: [PATCH -mm] file caps: make on-disk capabilities future-proof
 
 Stephen Smalley has pointed out that the current file capabilities
 will eventually pose a problem.
 
 As the capability set changes and distributions start tagging
 binaries with capabilities, we would like for running an older
 kernel to not necessarily make those binaries unusable.  To
 that end,
 
   1. If capabilities are specified which we don't know
  about, just ignore them, do not return -EPERM as we
  were doing before.

I didn't advocate that change - it is a separate issue from allowing the
capability bitmaps to grow in size in a backward compatible manner.  In
the one case, you have a binary that needs a capability that is unknown
to the kernel, so running it could lead to unexpected failure.  In the
other case, you simply have a binary labeled by newer userspace with a
newer on-disk representation that supports larger bitmaps, but the
binary might only have capabilities set that are known to the kernel.

   2. Specify a size with the on-disk capability implementation.
  In this implementation the size is the number of __u32's
  used for each of (eff,perm,inh).  For now, sz is 1.
  When we move to 64-bit capabilities, it becomes 2.

You could alternatively split them into separate xattrs, e.g.
security.cap.eff, security.cap.perm, security.cap.inh, and determine the
bitmap size from the xattr length rather than a separate field.

 diff --git a/security/commoncap.c b/security/commoncap.c
 index be86acb..dc8bf4f 100644
 --- a/security/commoncap.c
 +++ b/security/commoncap.c

 @@ -148,50 +145,65 @@ static int set_file_caps(struct linux_bi
  {
   struct dentry *dentry;
   ssize_t rc;
 - struct vfs_cap_data_disk dcaps;
 + struct vfs_cap_data_disk *dcaps;
   struct vfs_cap_data caps;
   struct inode *inode;
 - int err;
  
   if (bprm-file-f_vfsmnt-mnt_flags  MNT_NOSUID)
   return 0;
  
   dentry = dget(bprm-file-f_dentry);
   inode = dentry-d_inode;
 - if (!inode-i_op || !inode-i_op-getxattr) {
 - dput(dentry);
 - return 0;
 + rc = 0;
 + if (!inode-i_op || !inode-i_op-getxattr)
 + goto out;
 +
 + rc = inode-i_op-getxattr(dentry, XATTR_NAME_CAPS, NULL, 0);
 + if (rc == -ENODATA) {
 + rc = 0;
 + goto out;
 + }

I'd allocate an initial buffer with an expected size and try first to
avoid always having to make the two -getxattr calls in the common case.

 + if (rc  0)
 + goto out;
 + if (rc  sizeof(struct vfs_cap_data_disk)) {

You could make this a bit stricter, as you know that it will have at
least three additional u32 values beyond the header.

 + rc = -EINVAL;
 + goto out;
 + }
 +
 + dcaps = kmalloc(rc, GFP_KERNEL);
 + if (!dcaps) {
 + rc = -ENOMEM;
 + goto out;
 + }
 + rc = inode-i_op-getxattr(dentry, XATTR_NAME_CAPS, dcaps,
 + XATTR_CAPS_SZ);

I'm confused - you just asked for the actual length of the xattr and
allocated a buffer for it, and then don't use the length in this second
call to -getxattr.  And since you said you were organizing it as
eff[0..sz-1],perm[0..sz-1],inh[0..sz-1], you do need to read the entire
thing to get all three values even if you only use the lower 32 bits of
each.  Or if you change the organization to avoid the need to read the
entire thing, you don't need the first getxattr call at all, and you
need to change how cap_from_disk extracts the values.

-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [GIT PULL] audit patches

2007-02-22 Thread Stephen Smalley
On Wed, 2007-02-21 at 16:03 -0800, Andrew Morton wrote:
 On Sun, 18 Feb 2007 04:01:27 + Al Viro [EMAIL PROTECTED] wrote:
 
  Misc audit patches (resend again...); the most intrusive one is 
  AUDIT_FD_PAIR,
  allowing to log descriptor numbers from syscalls that do not return them in
  usual way (i.e. pipe() and socketpair()).  It took some massage of
  the failure exits in sys_socketpair(); the rest is absolutely trivial.
  Please, pull from
  git.kernel.org/pub/scm/linux/kernel/git/viro/audit-current.git/ audit.b37
 
 Please send patches to the list for review if practical?  In this case it
 was.  I trust davem has had a look at the non-trivial changes to
 sys_socketpair().
 
 
 
 Looking at the changes to audit_receive_msg():
 
 
   if (sid) {
   if (selinux_sid_to_string(
   sid, ctx, len)) {
   audit_log_format(ab,
ssid=%u, sid);
   /* Maybe call audit_panic? */
   } else
   audit_log_format(ab,
subj=%s, ctx);
   kfree(ctx);
   }
 
 This is assuming that selinux_sid_to_string() always initialises `ctx'.
 
 But AFAICT there are two error paths in security_sid_to_context() which
 forget to do that, so we end up doing kfree(uninitialised-local).
 
 I'd consider that a shortcoming in security_sid_to_context(), so not a
 problem in this patch, as long as people agree with my blaming above.

I wouldn't assume that the function initializes an argument if it
returns an error, and at least some of the callers (in auditsc.c) appear
to correctly initialize ctx to NULL themselves before calling
selinux_sid_to_string().  But if you'd prefer the function to always
handle it, we can do that.

 
 The coding style in there is a bit odd-looking.
 
 The new __audit_fd_pair() has unneeded braces in it.
-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[patch 1/1] selinux: always initialize arguments to security_sid_to_context (Was: Re: [GIT PULL] audit patches)

2007-02-23 Thread Stephen Smalley
On Thu, 2007-02-22 at 13:19 -0800, Andrew Morton wrote:
  On Thu, 22 Feb 2007 08:22:47 -0500 Stephen Smalley [EMAIL PROTECTED] 
  wrote:
  On Wed, 2007-02-21 at 16:03 -0800, Andrew Morton wrote:
   
   Looking at the changes to audit_receive_msg():
   
   
 if (sid) {
 if (selinux_sid_to_string(
 sid, ctx, len)) {
 audit_log_format(ab,
  ssid=%u, sid);
 /* Maybe call audit_panic? */
 } else
 audit_log_format(ab,
  subj=%s, ctx);
 kfree(ctx);
 }
   
   This is assuming that selinux_sid_to_string() always initialises `ctx'.
   
   But AFAICT there are two error paths in security_sid_to_context() which
   forget to do that, so we end up doing kfree(uninitialised-local).
   
   I'd consider that a shortcoming in security_sid_to_context(), so not a
   problem in this patch, as long as people agree with my blaming above.
  
  I wouldn't assume that the function initializes an argument if it
  returns an error, and at least some of the callers (in auditsc.c) appear
  to correctly initialize ctx to NULL themselves before calling
  selinux_sid_to_string().  But if you'd prefer the function to always
  handle it, we can do that.
  
 
 Well we now have (at least) one caller which assumes that *ctx is
 initialied in error cases.
 
 And I think it's sane to make it do that: safer, and will simplify coding
 in the callers.

Ok, patch below.

Always initialize *scontext and *scontext_len in security_sid_to_context.

Signed-off-by:  Stephen Smalley [EMAIL PROTECTED]

---

 security/selinux/ss/services.c |3 +++
 1 file changed, 3 insertions(+)

diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index ca9154d..1e52356 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -609,6 +609,9 @@ int security_sid_to_context(u32 sid, char **scontext, u32 
*scontext_len)
struct context *context;
int rc = 0;
 
+   *scontext = NULL;
+   *scontext_len  = 0;
+
if (!ss_initialized) {
if (sid = SECINITSID_NUM) {
char *scontextp;

-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 08/28] SECURITY: Allow kernel services to override LSM settings for task actions [try #2]

2008-01-09 Thread Stephen Smalley

On Wed, 2008-01-09 at 16:51 +, David Howells wrote:
 Okay.  I can:
 
  (1) Have cachefilesd (the daemon) pass a security context string to the
  cachefiles kernel module, which can then convert it to a secID.  It'll
  require a security_secctx_to_secid() function, but I'm fairly certain I
  have a patch to add such kicking around somewhere.

Already planned for 2.6.25, see:
http://marc.info/?l=selinuxm=119973017423487w=2

That is in the labeled networking tree.

  (2) Make security_task_kernel_act_as() take a task_security struct and a
  secID and just assign the latter to the former.  I'm not sure it makes
  sense to do any checks here, other than checking that under SELinux the
  secID is of SECCLASS_PROCESS class.
 
 However, I need to write a check that the cachefilesd daemon is permitted to
 nominate the secID it did.  Can someone tell me how to do this?  The obvious
 way to do this is to add another PROCESS__xxx security permit specifically for
 cachefiles, but that seems like a waste of a bit when there are only two spare
 bits.
 
   avc_has_perm(daemon_tsec-sid, nominated_sid,
SECCLASS_PROCESS, PROCESS__CACHEFILES_USE, NULL);
 
 Now, I recall the addition of another security class being mentioned, which
 presumably would give something like:
 
   avc_has_perm(daemon_tsec-sid, nominated_sid,
SECCLASS_CACHE, CACHE__USE_AS_OVERRIDE, NULL);
 
 And I assume this doesn't care if one, the other or both of the two SIDs
 mentioned are of SECCLASS_PROCESS rather than of SECCLASS_CACHE.

Right, the latter is reasonable.
Requires adding the class and permission definition to
policy/flask/security_classes and policy/flask/access_vectors and then
regenerating the kernel headers from those files, ala:
  svn co http://oss.tresys.com/repos/refpolicy/trunk refpolicy
  cd refpolicy/policy/flask
  vi security_classes access_vectors
  add new class to end
  make
  make LINUX_D=/path/to/linux-2.6 tokern
 
Dan knows how to do that.

-- 
Stephen Smalley
National Security Agency

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 08/28] SECURITY: Allow kernel services to override LSM settings for task actions [try #2]

2008-01-09 Thread Stephen Smalley

On Wed, 2008-01-09 at 18:56 +, David Howells wrote:
 Stephen Smalley [EMAIL PROTECTED] wrote:
 
  Right, the latter is reasonable.
  Requires adding the class and permission definition to
  policy/flask/security_classes and policy/flask/access_vectors and then
  regenerating the kernel headers from those files, ala:
svn co http://oss.tresys.com/repos/refpolicy/trunk refpolicy
cd refpolicy/policy/flask
vi security_classes access_vectors
add new class to end
make
make LINUX_D=/path/to/linux-2.6 tokern
 
 Does this require rebuilding and updating all the SELinux rpms to know about
 the new class?

Policy ultimately has to be updated in order to start writing allow
rules based on the new class/perm.  libselinux et al doesn't have to
change.

If you have a SELinux:  policy loaded with handle_unknown=allow
message in your /var/log/messages, then new classes/perms that are not
yet known to the policy will be allowed by default, so the operation
will be permitted by the kernel.

-- 
Stephen Smalley
National Security Agency

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC]selinux: Improving SELinux read/write performance

2007-09-13 Thread Stephen Smalley
On Wed, 2007-09-12 at 17:51 +0900, Yuichi Nakamura wrote:
 Hi. 
 
 Stephen Smalley pointed out possibility of race condition
 in off-list discussion.
 Stephen Smalley said:
  One other observation about the patch:  it presently leaves open a
  (small) race window in which the file could get relabeled or policy gets
  reloaded between the time of the normal permission check (from
  selinux_inode_permission) and the time you copy the inode SID and policy
  seqno to the file security struct.  In which case you might end up never
  revalidating access upon read/write even though conditions changed since
  the open-time permission check.  Not sure how to cleanly fix in a
  lock-free manner, and adding locks here will only make matters worse.
 
 To fix that, permission has to be checked in selinux_dentry_open.
 Therefore, in open, number of permission checks increased.
 As shown in benchmark result below, it does not affect open/close 
 performance so much.
 
 Following is benchmark result.
 * Benchmark
 lmbench simple read,write,open/close.
 
 * Before tuning
 1) Result for x86(Pentium 4 2.6Ghz), kernel 2.6.22
 Base  SELinux  Overhead(%)
 Simple read 1.10  1.24 12.3
 Simple write1.02  1.14 14.0
 open/close  5.97  7.45 24.9
 * Base: kernel compiled without SELinux support
 
 2) Result for SH(SH4, SH7751R), kernel 2.6.22
 BaseSELinux   Overhead(%)
 Simple read 2.395.49  130.5
 Simple write2.075.10  146.6
 open/close  32.662.8  93.0
 
 * After tuning
 1) Result for x86(Pentium 4 2.6Ghz), kernel 2.6.22
 Base  SELinux  Overhead(%)
 Simple read 1.10  1.13 2.3(Before 12.3)
 Simple write1.02  1.0240.6(Before 14.0)
 open/close  5.97  7.48 25.3(Before 24.9)
 * Base: kernel compiled without SELinux support
 
 2) Result for SH(SH4, SH7751R), kernel 2.6.22
 BaseSELinux   Overhead(%)
 Simple read 2.392.63  10.4(Before 130.5)
 Simple write2.072.34  13.1(Before 146.6)
 open/close  32.658.7  80.2(before 93.0)
 
 Next is a patch.

Thanks, a few comments below.

 
 * Description of patch
 This patch improves performance of read/write in SELinux.
 It improves performance by skipping permission check in 
 selinux_file_permission. Permission is only checked when 
 sid change or policy load is detected after file open.
 To detect sid change, new LSM hook securiy_dentry_open is added.

I think I'd reword this a little, e.g.

It reduces the selinux overhead on read/write by only revalidating
permissions in selinux_file_permission if the task or inode labels have
changed or the policy has changed since the open-time check.  A new LSM
hook, security_dentry_open, is added to capture the necessary state at
open time to allow this optimization.

 
 Signed-off-by: Yuichi Nakamura[EMAIL PROTECTED]
 ---
  fs/open.c |5 
  include/linux/security.h  |   16 ++
  security/dummy.c  |6 +
  security/selinux/avc.c|5 
  security/selinux/hooks.c  |   43 
 +-
  security/selinux/include/avc.h|2 +
  security/selinux/include/objsec.h |2 +
  7 files changed, 78 insertions(+), 1 deletion(-)
snip
 diff -purN -X linux-2.6.22/Documentation/dontdiff 
 linux-2.6.22.orig/security/selinux/hooks.c 
 linux-2.6.22/security/selinux/hooks.c
 --- linux-2.6.22.orig/security/selinux/hooks.c2007-07-09 
 08:32:17.0 +0900
 +++ linux-2.6.22/security/selinux/hooks.c 2007-09-12 08:42:49.0 
 +0900
 @@ -80,6 +82,7 @@
  
  #define XATTR_SELINUX_SUFFIX selinux
  #define XATTR_NAME_SELINUX XATTR_SECURITY_PREFIX XATTR_SELINUX_SUFFIX
 +#define ACC_MODE(x) (\000\004\002\006[(x)O_ACCMODE])

Leftover from prior version of the patch, no longer needed.

snip
 @@ -2715,6 +2737,23 @@ static int selinux_file_receive(struct f
   return file_has_perm(current, file, file_to_av(file));
  }
  
 +static int selinux_dentry_open(struct file *file)
 +{
 + struct file_security_struct *fsec;
 + struct inode *inode;
 + struct inode_security_struct *isec;
 + inode = file-f_path.dentry-d_inode;
 + fsec = file-f_security;
 + isec = inode-i_security;

I'd add a comment here, e.g.
  /* 
   * Save inode label and policy sequence number
   * at open-time so that selinux_file_permission
   * can determine whether revalidation is necessary.
   * Task label is already saved in the file security
   * struct as its SID.
   */

 + fsec-isid = isec-sid;
 + fsec-pseqno = avc_policy_seqno();
 +
 + /*Permission has to be rechecked here.
 +   Policy load of inode sid can happen between
 +   may_open and selinux_dentry_open.*/

Typo in the comment (s/of/or/), coding style isn't right for a
multi-line comment, and likely needs clarification, e.g

Re: [PATCH] selinux: Improving SELinux read/write performance

2007-09-17 Thread Stephen Smalley
On Fri, 2007-09-14 at 09:27 +0900, Yuichi Nakamura wrote:
 Hello.
 
 I would like to propose patch that reduces overhead in read/write by SELinux.
 I sent RFC in previous thread.
 http://lkml.org/lkml/2007/9/6/14
 As a result of discussion in previous thread, 
 quality of code has improved, so I would like to submit patch here.
 
 1. Background
 Look at benchmark result below.
 lmbench simple read/write(average of 5 run).
 Big overhead exists especially on SH(SuperH) arch.
 
 1) Result for x86(Pentium 4 2.6Ghz), kernel 2.6.22
 Base  SELinux  Overhead(%)
 Simple read 1.10  1.24 12.3
 Simple write1.02  1.14 14.0
 * Base: kernel compiled without SELinux support
 
 2) Result for SH(SH4, SH7751R), kernel 2.6.22
 BaseSELinux   Overhead(%)
 Simple read 2.395.49  130.5
 Simple write2.075.10  146.6
 
 2. About patch
 It reduces the selinux overhead on read/write by only revalidating
 permissions in selinux_file_permission if the task or inode labels have
 changed or the policy has changed since the open-time check.  
 A new LSM hook, security_dentry_open,  is added to capture 
 the necessary state at open time to allow this optimization.
 
 3. Result of benchmark after applying patch
 1) Result for x86(Pentium 4 2.6Ghz), kernel 2.6.22
 Base  SELinux  Overhead(%)
 Simple read 1.10  1.13 2.3(Before 12.3)
 Simple write1.02  1.0240.6(Before 14.0)
 * Base: kernel compiled without SELinux support
 
 2) Result for SH(SH4, SH7751R), kernel 2.6.22
 BaseSELinux   Overhead(%)
 Simple read 2.392.63  10.4(Before 130.5)
 Simple write2.072.34  13.1(Before 146.6)
 
 Overhead in read/write is reduced a lot.
 This patch adds permission check at open time(in __dentry_open), 
 but open/close performance does not get worse as shown below.
 
 * Lmbench simple open/close
 Pentium 4(before patch):
 Base  SELinux  Overhead(%)
 open/close  5.97  7.45 24.9
 after patch:
 open/close  5.97  7.48 25.3
 
 SH(before patch):
 Base  SELinux  Overhead(%)
 open/close  32.6  62.8   93.0
 after patch:
 open/close  32.658.780.2
 
 Next is a patch for 2.6.22.
 
 It reduces the selinux overhead on read/write by only revalidating
 permissions in selinux_file_permission if the task or inode labels have
 changed or the policy has changed since the open-time check.  A new LSM
 hook, security_dentry_open, is added to capture the necessary state at
 open time to allow this optimization.
 
 Signed-off-by: Yuichi Nakamura[EMAIL PROTECTED]

Thanks, looks good.

Acked-by:  Stephen Smalley [EMAIL PROTECTED]

 ---
  fs/open.c |4 ++
  include/linux/security.h  |   18 
  security/dummy.c  |6 
  security/selinux/avc.c|5 +++
  security/selinux/hooks.c  |   53 
 +-
  security/selinux/include/avc.h|2 +
  security/selinux/include/objsec.h |2 +
  7 files changed, 89 insertions(+), 1 deletion(-)
 diff -purN -X linux-2.6.22/Documentation/dontdiff 
 linux-2.6.22.orig/security/selinux/avc.c linux-2.6.22/security/selinux/avc.c
 --- linux-2.6.22.orig/security/selinux/avc.c  2007-07-09 08:32:17.0 
 +0900
 +++ linux-2.6.22/security/selinux/avc.c   2007-09-14 08:35:04.0 
 +0900
 @@ -913,3 +913,8 @@ int avc_has_perm(u32 ssid, u32 tsid, u16
   avc_audit(ssid, tsid, tclass, requested, avd, rc, auditdata);
   return rc;
  }
 +
 +u32 avc_policy_seqno(void)
 +{
 + return avc_cache.latest_notif;
 +}
 diff -purN -X linux-2.6.22/Documentation/dontdiff 
 linux-2.6.22.orig/security/selinux/hooks.c 
 linux-2.6.22/security/selinux/hooks.c
 --- linux-2.6.22.orig/security/selinux/hooks.c2007-07-09 
 08:32:17.0 +0900
 +++ linux-2.6.22/security/selinux/hooks.c 2007-09-14 08:43:51.0 
 +0900
 @@ -14,6 +14,8 @@
   *  [EMAIL PROTECTED]
   *  Copyright (C) 2006 Hewlett-Packard Development Company, L.P.
   * Paul Moore, [EMAIL PROTECTED]
 + *  Copyright (C) 2007 Hitachi Software Engineering Co., Ltd.
 + * Yuichi Nakamura [EMAIL PROTECTED]
   *
   *   This program is free software; you can redistribute it and/or modify
   *   it under the terms of the GNU General Public License version 2,
 @@ -2458,7 +2460,7 @@ static int selinux_inode_listsecurity(st
  
  /* file security operations */
  
 -static int selinux_file_permission(struct file *file, int mask)
 +static int selinux_revalidate_file_permission(struct file *file, int mask)
  {
   int rc;
   struct inode *inode = file-f_path.dentry-d_inode;
 @@ -2480,6 +2482,25 @@ static int selinux_file_permission(struc
   return selinux_netlbl_inode_permission(inode, mask);
  }
  
 +static int selinux_file_permission(struct file *file, int mask)
 +{
 + struct inode *inode

Re: [PATCH 08/28] SECURITY: Allow kernel services to override LSM settings for task actions [try #2]

2007-12-19 Thread Stephen Smalley
On Tue, 2007-12-18 at 19:28 -0800, Crispin Cowan wrote:
 Stephen Smalley wrote:
  It is if I have to maintain a special pieces of code for each possible LSM.
  One piece for SELinux, one piece for AppArmour, one piece for Smack, one 
  piece
  for Casey's security system.  That sounds like a pain.
  
  All your code has to do is invoke a function provided by libselinux.  If
  at some later time a liblsm is introduced that provides a common
  front-end to a libselinux, libsmack, ..., then you can use that.  But it
  doesn't exist today.   But it all just becomes a simple function call
  regardless.

 libapparmor exists. It only had one API, and now it has 2, but just 2
 versions on the same concept (change_hat and change_profile).
 
 This is the API for change_hat http://man-wiki.net/index.php/2:change_hat
 
 What does the corresponding API in SELinux look like?

The SELinux API for changing context is described in:
http://linux.die.net/man/3/setcon

However, setting the current context (SELinux) or profile (AppArmor) for
a userspace task doesn't really provide the functionality required here
for cachefiles, nor does it solve the (different, yet related) nfsd
problem.

cachefiles is a kernel module that needs to assume a different set of
credentials for its internal accesses to the cache files it manages when
a userspace process tries to access a file that has been cached, as the
userspace process in whose context it is operating may (and should) lack
direct permission to those cache files.  The userspace API being talked
about is simply how one configures the credentials to be used by
cachefiles kernel module for its internal accesses, and is done up front
when cachefiles is configured to create a cache.  The internal switching
of the active set of credentials is done via a kernel-internal API (or
just by switching the pointer to the credential structure previously set
up) when the cachefiles kernel module wants to access a cache file.
Further, when this internal switching occurs, we have to be careful that
there are no user-visible side effects on the current task - no change
in how others may operate on that task e.g. signal permission checks or
on how the task appears to others e.g. via /proc.

Neither change_hat nor setcon helps with that problem.

For AppArmor, I suspect that you just want the cachefiles kernel module
to act as unconfined for its internal accesses, nothing more.

-- 
Stephen Smalley
National Security Agency

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 4/7] KEYS: Add keyctl function to get a security label

2007-12-05 Thread Stephen Smalley
)
  +   ret = -EFAULT;
  +   }
  +
  +   kfree(context);
  +   }
  +
  +   key_ref_put(key_ref);
  +   return ret;
  +}
  +
  
 
 /*/
   /*
* the key control system call
  @@ -1160,6 +1221,11 @@ asmlinkage long sys_keyctl(int option, unsigned long
  arg2, unsigned long arg3,
  case KEYCTL_ASSUME_AUTHORITY:
  return keyctl_assume_authority((key_serial_t) arg2);
   
  +   case KEYCTL_GET_SECURITY:
  +   return keyctl_get_security((key_serial_t) arg2,
  +  (char *) arg3,
  +  (size_t) arg4);
  +
  default:
  return -EOPNOTSUPP;
  }
  diff --git a/security/security.c b/security/security.c
  index 0e1f1f1..16213e3 100644
  --- a/security/security.c
  +++ b/security/security.c
  @@ -1079,4 +1079,9 @@ int security_key_permission(key_ref_t key_ref,
  return security_ops-key_permission(key_ref, context, perm);
   }
   
  +int security_key_getsecurity(struct key *key, char **_buffer)
  +{
  +   return security_ops-key_getsecurity(key, _buffer);
  +}
  +
   #endif /* CONFIG_KEYS */
  diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
  index 9f3124b..bd4cfab 100644
  --- a/security/selinux/hooks.c
  +++ b/security/selinux/hooks.c
  @@ -4768,6 +4768,20 @@ static int selinux_key_permission(key_ref_t key_ref,
  SECCLASS_KEY, perm, NULL);
   }
   
  +static int selinux_key_getsecurity(struct key *key, char **_buffer)
  +{
  +   struct key_security_struct *ksec = key-security;
  +   char *context = NULL;
  +   unsigned len;
  +   int rc;
  +
  +   rc = security_sid_to_context(ksec-sid, context, len);
  +   if (!rc)
  +   rc = len;
  +   *_buffer = context;
  +   return rc;
  +}
  +
   #endif
   
   static struct security_operations selinux_ops = {
  @@ -4943,9 +4957,10 @@ static struct security_operations selinux_ops = {
   #endif
   
   #ifdef CONFIG_KEYS
  -   .key_alloc =selinux_key_alloc,
  -   .key_free = selinux_key_free,
  -   .key_permission =   selinux_key_permission,
  +   .key_alloc =selinux_key_alloc,
  +   .key_free = selinux_key_free,
  +   .key_permission =   selinux_key_permission,
  +   .key_getsecurity =  selinux_key_getsecurity,
   #endif
   };
   
  
  
  
 
 
 Casey Schaufler
 [EMAIL PROTECTED]
 
 --
 This message was distributed to subscribers of the selinux mailing list.
 If you no longer wish to subscribe, send mail to [EMAIL PROTECTED] with
 the words unsubscribe selinux without quotes as the message.
-- 
Stephen Smalley
National Security Agency

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/4] proc: fix NULL -i_fop oops

2007-11-20 Thread Stephen Smalley
On Mon, 2007-11-19 at 12:51 +, Christoph Hellwig wrote:
 On Fri, Nov 16, 2007 at 06:06:51PM +0300, Alexey Dobriyan wrote:
  proc_kill_inodes() can clear -i_fop in the middle of vfs_readdir resulting 
  in
  NULL dereference during file-f_op-readdir(file, buf, filler).
  
  The solution is to remove proc_kill_inodes() completely:
  a) we don't have tricky modules implementing their tricky readdir hooks 
  which
 could keeping this revoke from hell.
  b) In a situation when module is gone but PDE still alive, standard readdir
 will return only . and .., because pde-next was cleared by
 remove_proc_entry().
  c) the race proc_kill_inode() destined to prevent is not completely fixed, 
  just
 race window made smaller, because vfs_readdir() is run without sb_lock 
  held and
 without file_list_lock held. Effectively, -i_fop is cleared at random 
  moment,
 which can't fix properly anything.
 
 Nice, getting rid of this is a very good step formwards.  Unfortunately
 we have another copy of this junk in
 security/selinux/selinuxfs.c:sel_remove_entries() which would need the
 same treatment.

Can't just be dropped completely for selinux - we need a way to drop
obsolete entries from the prior policy when we load a new policy.

Is the only real problem here the clearing of f_op?  If so, we can
likely remove that from sel_remove_entries() without harm, and fix the
checks for it to use something more reliable.

-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/4] proc: fix NULL -i_fop oops

2007-11-20 Thread Stephen Smalley
On Tue, 2007-11-20 at 15:17 +, Christoph Hellwig wrote:
 On Tue, Nov 20, 2007 at 10:05:05AM -0500, Stephen Smalley wrote:
   Nice, getting rid of this is a very good step formwards.  Unfortunately
   we have another copy of this junk in
   security/selinux/selinuxfs.c:sel_remove_entries() which would need the
   same treatment.
  
  Can't just be dropped completely for selinux - we need a way to drop
  obsolete entries from the prior policy when we load a new policy.
  
  Is the only real problem here the clearing of f_op?  If so, we can
  likely remove that from sel_remove_entries() without harm, and fix the
  checks for it to use something more reliable.
 
 f_op removal is the biggest issue.  It can't really work and this is the
 last instance.  But in general having some half-backed attempts at revoke
 is never a good idea.

Yes, we're not trying to revoke per se, but just re-populate a set of
directories that represent elements of policy state on a policy
reload.  /selinux/booleans is one example - a directory with one entry
per policy boolean defined by the policy.  Old directory tree gets torn
down on each policy reload and replaced.

-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[patch 1/1] selinux: do not clear f_op when removing entries

2007-11-21 Thread Stephen Smalley
On Tue, 2007-11-20 at 15:17 +, Christoph Hellwig wrote:
 On Tue, Nov 20, 2007 at 10:05:05AM -0500, Stephen Smalley wrote:
   Nice, getting rid of this is a very good step formwards.  Unfortunately
   we have another copy of this junk in
   security/selinux/selinuxfs.c:sel_remove_entries() which would need the
   same treatment.
  
  Can't just be dropped completely for selinux - we need a way to drop
  obsolete entries from the prior policy when we load a new policy.
  
  Is the only real problem here the clearing of f_op?  If so, we can
  likely remove that from sel_remove_entries() without harm, and fix the
  checks for it to use something more reliable.
 
 f_op removal is the biggest issue.  It can't really work and this is the
 last instance.  But in general having some half-backed attempts at revoke
 is never a good idea.

Do not clear f_op when removing entries since it isn't safe to do.

Signed-off-by:  Stephen Smalley [EMAIL PROTECTED]

---

 security/selinux/selinuxfs.c |   28 +---
 1 file changed, 1 insertion(+), 27 deletions(-)

diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index f5f3e6d..ac6fe99 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -838,10 +838,6 @@ static ssize_t sel_read_bool(struct file *filep, char 
__user *buf,
 
ret = -EFAULT;
 
-   /* check to see if this file has been deleted */
-   if (!filep-f_op)
-   goto out;
-
if (count  PAGE_SIZE) {
ret = -EINVAL;
goto out;
@@ -882,10 +878,6 @@ static ssize_t sel_write_bool(struct file *filep, const 
char __user *buf,
if (length)
goto out;
 
-   /* check to see if this file has been deleted */
-   if (!filep-f_op)
-   goto out;
-
if (count = PAGE_SIZE) {
length = -ENOMEM;
goto out;
@@ -940,10 +932,6 @@ static ssize_t sel_commit_bools_write(struct file *filep,
if (length)
goto out;
 
-   /* check to see if this file has been deleted */
-   if (!filep-f_op)
-   goto out;
-
if (count = PAGE_SIZE) {
length = -ENOMEM;
goto out;
@@ -982,11 +970,9 @@ static const struct file_operations sel_commit_bools_ops = 
{
.write  = sel_commit_bools_write,
 };
 
-/* partial revoke() from fs/proc/generic.c proc_kill_inodes */
 static void sel_remove_entries(struct dentry *de)
 {
-   struct list_head *p, *node;
-   struct super_block *sb = de-d_sb;
+   struct list_head *node;
 
spin_lock(dcache_lock);
node = de-d_subdirs.next;
@@ -1006,18 +992,6 @@ static void sel_remove_entries(struct dentry *de)
}
 
spin_unlock(dcache_lock);
-
-   file_list_lock();
-   list_for_each(p, sb-s_files) {
-   struct file * filp = list_entry(p, struct file, f_u.fu_list);
-   struct dentry * dentry = filp-f_path.dentry;
-
-   if (dentry-d_parent != de) {
-   continue;
-   }
-   filp-f_op = NULL;
-   }
-   file_list_unlock();
 }
 
 #define BOOL_DIR_NAME booleans


-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: + smack-version-11c-simplified-mandatory-access-control-kernel.patch added to -mm tree

2007-11-21 Thread Stephen Smalley
On Wed, 2007-11-21 at 09:48 -0600, Serge E. Hallyn wrote:
 Quoting [EMAIL PROTECTED] ([EMAIL PROTECTED]):
  +/*
  + * There are not enough CAP bits available to make this
  + * real, so Casey borrowed the capability that looks to
  + * him like it has the best balance of similarity amd
  + * low use.
  + */
  +#define CAP_MAC_OVERRIDE CAP_LINUX_IMMUTABLE
 
 Hey Casey,
 
 note that 64-bit capabilities are now in -mm, so you could grab your own
 capability.

Which brings up an interesting question - what to do with
security-module-specific capabilities?  CAP_MAC_OVERRIDE is specific to
Smack - other MAC modules like SELinux won't honor it.  Maybe it should
be CAP_SMACK_OVERRIDE.

-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: + smack-version-11c-simplified-mandatory-access-control-kernel.patch added to -mm tree

2007-11-21 Thread Stephen Smalley
On Wed, 2007-11-21 at 09:21 -0800, Casey Schaufler wrote:
 --- Stephen Smalley [EMAIL PROTECTED] wrote:
 
  On Wed, 2007-11-21 at 09:48 -0600, Serge E. Hallyn wrote:
   Quoting [EMAIL PROTECTED] ([EMAIL PROTECTED]):
+/*
+ * There are not enough CAP bits available to make this
+ * real, so Casey borrowed the capability that looks to
+ * him like it has the best balance of similarity amd
+ * low use.
+ */
+#define CAP_MAC_OVERRIDE CAP_LINUX_IMMUTABLE
   
   Hey Casey,
   
   note that 64-bit capabilities are now in -mm, so you could grab your own
   capability.
 
 A proposal with merit. I'll do it.
 
  Which brings up an interesting question - what to do with
  security-module-specific capabilities? CAP_MAC_OVERRIDE is specific to
  Smack - other MAC modules like SELinux won't honor it.  Maybe it should
  be CAP_SMACK_OVERRIDE.
 
 But what about the MAC modules like Smack that do honor it?
 They shouldn't be using a module specific capability, so
 calling it CAP_SMACK_OVERRIDE isn't right because then the
 SecureWare* port would have to introduce CAP_SECUREWARE_OVERRIDE
 and bam, there go all your shiny new capability values. Further,
 Smack isn't what's overridden by CAP_MAC_OVERRIDE, it's the
 access control check, which is not the same thing at all at all.
 SELinux ignoring CAP_MAC_OVERRIDE is perfectly within the
 restrictive model of the LSM. If someone ported the Trusted
 Irix 4 MLS system as an LSM** having CAP_MAC_OVERRIDE wouldn't
 have to allow read of higher labeled files, as the policy on that
 system never allowed that, even for privileged processes.

Well, I'd be surprised to see that SecureWare port to Linux.  Or really
any other label-based MAC implementations at all - how many different
ones does Linux need?

Meanwhile, SELinux does implement MAC checks, and those checks do not
require an external privilege mechanism like capabilities and won't be
overridden by your shiny new capability.  Which makes me uneasy about
calling it CAP_MAC_OVERRIDE.  Explain it to a user - this capability is
for overriding MAC checks, except for SELinux.  Or AppArmor or TOMOYO
or ... if you also count them as MAC (depending on your definition of
it).

What the capability does is override the MAC checks in Smack.  And since
you don't seem to need more than one such capability for Smack, calling
it CAP_SMACK_OVERRIDE makes sense to me.  

 SELinux has every right to make it's own choices regarding how
 it behaves relative to any specific capability because it is
 allowed to restrict access in any way it sees fit. I think that
 you would get in trouble if you changed the value of a capability
 on a task within the LSM, because that will impact the usual
 access checks, but I wouldn't expect to see you doing that.

Given that the capability module is implemented via LSM, the above makes
no sense - a LSM is free to tweak the capability bits or change what
capable() returns already.  The fact that we don't presently do so is
another matter.

 SELinux has gone in a different direction on privileged operations
 than capabilities, so it's not surprising that the two schemes
 don't look like they're meshing very well in this case.
 
 
 *  SecureWare was an MLS kit. The MLS version of HP/UX was based
on it, and the Macintosh version got the first ever CMW evaluation.
SELinux took many of the same design approaches as SecureWare,
and in it's early versions bore a somewhat frightening resemblence
to the earlier system.
 
 ** Trix4 allowed a privileged process to write lower labeled files,
but not read higher labeled files. That way any files that got
created by accident were assured to be labeled at least as high
as the data they contained.
 
 
 Casey Schaufler
 [EMAIL PROTECTED]
-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 08/28] SECURITY: Allow kernel services to override LSM settings for task actions [try #2]

2007-12-10 Thread Stephen Smalley
);
 + set_to_dummy_if_null(ops, task_create_files_as);
   set_to_dummy_if_null(ops, task_setuid);
   set_to_dummy_if_null(ops, task_post_setuid);
   set_to_dummy_if_null(ops, task_setgid);
 diff --git a/security/security.c b/security/security.c
 index 92d66d6..86d94e5 100644
 --- a/security/security.c
 +++ b/security/security.c
 @@ -583,6 +583,19 @@ int security_task_dup(struct task_security *sec)
   return security_ops-task_dup_security(sec);
  }
  
 +int security_task_kernel_act_as(struct task_security *sec,
 + const char *service,
 + struct task_struct *daemon)
 +{
 + return security_ops-task_kernel_act_as(sec, service, daemon);
 +}
 +
 +int security_task_create_files_as(struct task_security *sec,
 +   struct inode *inode)
 +{
 + return security_ops-task_create_files_as(sec, inode);
 +}
 +
  int security_task_setuid(uid_t id0, uid_t id1, uid_t id2, int flags)
  {
   return security_ops-task_setuid(id0, id1, id2, flags);
 diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
 index 20a6b55..2416f54 100644
 --- a/security/selinux/hooks.c
 +++ b/security/selinux/hooks.c
 @@ -2846,6 +2846,49 @@ static int selinux_task_dup_security(struct 
 task_security *sec)
   return 0;
  }
  
 +/*
 + * get the security data for a kernel service, deriving the subjective 
 context
 + * from the security data of a userspace daemon if one supplied
 + * - all the creation contexts are set to unlabelled
 + */
 +static int selinux_task_kernel_act_as(struct task_security *sec,
 +   const char *service,
 +   struct task_struct *daemon)
 +{
 + struct task_security_struct *tsec, *dtsec;
 + u32 ksid;
 + int ret;
 +
 + tsec = sec-security;
 + dtsec = daemon ? daemon-sec-security : init_task.sec-security;
 +
 + ret = security_transition_sid(dtsec-sid, SECINITSID_KERNEL,
 +   SECCLASS_PROCESS, ksid);
 + if (ret  0)
 + return ret;
 +
 + tsec-sid = ksid;
 + tsec-create_sid = SECINITSID_UNLABELED;
 + tsec-keycreate_sid = SECINITSID_UNLABELED;
 + tsec-sockcreate_sid = SECINITSID_UNLABELED;

These SIDs need to be cleared rather than set to SECINITSID_UNLABELED.

 + sec-security = tsec;
 + return 0;
 +}
 +
 +/*
 + * set the file creation context in a security record to the same as the
 + * objective context of the specified inode
 + */
 +static int selinux_task_create_files_as(struct task_security *sec,
 + struct inode *inode)
 +{
 + struct task_security_struct *tsec = sec-security;
 + struct inode_security_struct *isec = inode-i_security;
 +
 + tsec-create_sid = isec-sid;
 + return 0;
 +}
 +
  static int selinux_task_setuid(uid_t id0, uid_t id1, uid_t id2, int flags)
  {
   /* Since setuid only affects the current process, and
 @@ -4884,6 +4927,8 @@ static struct security_operations selinux_ops = {
   .task_alloc_security =  selinux_task_alloc_security,
   .task_free_security =   selinux_task_free_security,
   .task_dup_security =selinux_task_dup_security,
 + .task_kernel_act_as =   selinux_task_kernel_act_as,
 + .task_create_files_as = selinux_task_create_files_as,
   .task_setuid =  selinux_task_setuid,
   .task_post_setuid = selinux_task_post_setuid,
   .task_setgid =  selinux_task_setgid,
 
 
 --
 This message was distributed to subscribers of the selinux mailing list.
 If you no longer wish to subscribe, send mail to [EMAIL PROTECTED] with
 the words unsubscribe selinux without quotes as the message.
-- 
Stephen Smalley
National Security Agency

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 08/28] SECURITY: Allow kernel services to override LSM settings for task actions [try #2]

2007-12-10 Thread Stephen Smalley
On Mon, 2007-12-10 at 17:07 +, David Howells wrote:
 Stephen Smalley [EMAIL PROTECTED] wrote:
 
   + tsec-create_sid = SECINITSID_UNLABELED;
   + tsec-keycreate_sid = SECINITSID_UNLABELED;
   + tsec-sockcreate_sid = SECINITSID_UNLABELED;
 
 Cleared means what?  Setting to 0?  Or is there some other constant I should
 use for that?

Yes, setting to 0.

Otherwise, only other issue I have with this interface is it won't
generalize to dealing with nfsd, where we want to set the acting context
to a context we obtain from or determine based upon the client.

Why can't cachefilesd just push a context into the kernel and pass that
into the hook as the acting context, and then nfsd can do likewise using
the context provided by the client or obtained locally from exports for
ordinary clients?  Avoids the transition SID computation altogether
within the kernel and makes this more generic.

-- 
Stephen Smalley
National Security Agency

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 08/28] SECURITY: Allow kernel services to override LSM settings for task actions [try #2]

2007-12-10 Thread Stephen Smalley
On Mon, 2007-12-10 at 21:08 +, David Howells wrote:
 Stephen Smalley [EMAIL PROTECTED] wrote:
 
  Otherwise, only other issue I have with this interface is it won't
  generalize to dealing with nfsd, where we want to set the acting context
  to a context we obtain from or determine based upon the client.
 
 Are you speaking of security_kernel_act_as() and security_create_files_as()
 specifically?  Or the task_struct::act_as override pointer in general?

security_kernel_act_as()

 I don't really know how nfsd wants to obtain and set its LSM context, so it's
 a bit difficult for me to make something that works for nfsd as well as
 cachefiles.

It would get a context from the client or from a local configuration
that would map security-unaware clients to a default context, and then
want to assume that context for the particular operation.  No transition
involved.

  Why can't cachefilesd just push a context into the kernel and pass that
  into the hook as the acting context,
 
 How does cachefilesd come up with such a context?  Grab it from
 /etc/cachefilesd.conf?

From a config file whose pathname would be provided by libselinux (ala
the way in which dbusd imports contexts), or directly as a context
returned by a libselinux function.  Has to be done that way so that it
can be set differently for different policy types (strict, targeted,
mls).

Naturally, cachefiles (the kernel module) would invoke a security hook
to check whether the daemon is allowed to set the specified context.

 I use to do that, but someone objected...  Possibly Karl MacMillan.

Yes, but I think I disagreed then too.

  and then nfsd can do likewise using the context provided by the client or
  obtained locally from exports for ordinary clients?  Avoids the transition
  SID computation altogether within the kernel and makes this more generic.
 
 I seem to remember that I was told that it should be done this way, possibly
 by Karl MacMillan, but I don't remember exactly.
 
 Now it's configured by cachefilesd.te:
 
   type_transition cachefilesd_t kernel_t : process cachefiles_kernel_t;

It doesn't fit with how other users of security_kernel_act_as() will
likely want to work (they will want to just set the context to a
specified value, whether one obtained from the client or from some local
source), nor with how type transitions normally work (exec, with the
program type as the second type field).  I think it will just cause
confusion and subtle breakage.

-- 
Stephen Smalley
National Security Agency

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 08/28] SECURITY: Allow kernel services to override LSM settings for task actions [try #2]

2007-12-11 Thread Stephen Smalley
On Mon, 2007-12-10 at 14:26 -0800, Casey Schaufler wrote:
 --- Stephen Smalley [EMAIL PROTECTED] wrote:
 
  On Mon, 2007-12-10 at 21:08 +, David Howells wrote:
   Stephen Smalley [EMAIL PROTECTED] wrote:
   
Otherwise, only other issue I have with this interface is it won't
generalize to dealing with nfsd, where we want to set the acting context
to a context we obtain from or determine based upon the client.
   
   Are you speaking of security_kernel_act_as() and 
   security_create_files_as()
   specifically?  Or the task_struct::act_as override pointer in general?
  
  security_kernel_act_as()
  
   I don't really know how nfsd wants to obtain and set its LSM context, so
  it's
   a bit difficult for me to make something that works for nfsd as well as
   cachefiles.
  
  It would get a context from the client or from a local configuration
  that would map security-unaware clients to a default context, and then
  want to assume that context for the particular operation.  No transition
  involved.
 
 I would expect that the operation would be more sophisticated
 than that. You certainly aren't going to use what comes from
 the other side without any processing, and I expect you'll have
 some sort of operation on anything you pull from a config file
 before you actually apply it.

Yes, that's true - the contexts would be subjected to a permission
check.  But that's separable from the act of setting it as the task's
acting security state (and needs to be separated, as the precise check
will vary depending on the situation - cachefiles is going to apply a
different sort of check than nfsd).

Why can't cachefilesd just push a context into the kernel and pass that
into the hook as the acting context,
   
   How does cachefilesd come up with such a context?  Grab it from
   /etc/cachefilesd.conf?
  
  From a config file whose pathname would be provided by libselinux (ala
  the way in which dbusd imports contexts), or directly as a context
  returned by a libselinux function.  Has to be done that way so that it
  can be set differently for different policy types (strict, targeted,
  mls).
 
 Unless you've got an LSM other than SELinux, of course. If
 cachefilesd is going to be responsible for maintaining this
 magic context there needs to be an LSM interface for it, not
 just an SELinux interface.

LSM is an in-kernel interface.  Here we are talking about a userspace
interface for obtaining the right security label to use.  There is no
equivalent to LSM in userspace as of yet.  Feel free to invent one, but
don't ask the rest of us to do it or wait for it to materialize.

  Naturally, cachefiles (the kernel module) would invoke a security hook
  to check whether the daemon is allowed to set the specified context.
  
   I use to do that, but someone objected...  Possibly Karl MacMillan.
  
  Yes, but I think I disagreed then too.
  
and then nfsd can do likewise using the context provided by the client 
or
obtained locally from exports for ordinary clients?  Avoids the
  transition
SID computation altogether within the kernel and makes this more 
generic.
   
   I seem to remember that I was told that it should be done this way,
  possibly
   by Karl MacMillan, but I don't remember exactly.
   
   Now it's configured by cachefilesd.te:
   
 type_transition cachefilesd_t kernel_t : process cachefiles_kernel_t;
  
  It doesn't fit with how other users of security_kernel_act_as() will
  likely want to work (they will want to just set the context to a
  specified value, whether one obtained from the client or from some local
  source), nor with how type transitions normally work (exec, with the
  program type as the second type field).  I think it will just cause
  confusion and subtle breakage.
 
 I think that I agree with Stephen, although I could be mirely confused.
 That happens to me when interfaces are described in SELinux terms. I
 still don't care much for multiple contexts, and I don't have a good
 grasp of how you'll deal with Smack, or any LSM other than SELinux.
 Just as Stephen mentions, I also don't see the generality that a change
 of this magnitude really ought to provide.

-- 
Stephen Smalley
National Security Agency

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 08/28] SECURITY: Allow kernel services to override LSM settings for task actions [try #2]

2007-12-11 Thread Stephen Smalley
On Mon, 2007-12-10 at 23:36 +, David Howells wrote:
 Stephen Smalley [EMAIL PROTECTED] wrote:
 
  From a config file whose pathname would be provided by libselinux (ala
  the way in which dbusd imports contexts), or directly as a context
  returned by a libselinux function.
 
 That sounds too SELinux specific.  How do I do it so that it works for any
 LSM?

You can't.  There is no LSM for userspace; LSM specifically disavowed
any common userspace API, and that was one of our original
objections/concerns about it.

 Is linking against libselinux is a viable option if it's not available under
 all LSM models?  Is it available under all LSM models?  Perhaps Casey can
 answer this one.

Nope, they would all have their own libraries, if they have a library at
all.  But that isn't your problem - your kernel interface should be
generic, and your LSM hooks should be generic, but your userspace isn't
required to be.  Have a look at how many programs in the distribution
currently link against libselinux, whether directly or by dlopen'ing it.

   I use to do that, but someone objected...  Possibly Karl MacMillan.
  
  Yes, but I think I disagreed then too.
 
 So, who's right?

Karl isn't a maintainer of the SELinux kernel code.  And I had thought
that even he had reconsidered this idea after further discussion.

  It doesn't fit with how other users of security_kernel_act_as() will
  likely want to work (they will want to just set the context to a
  specified value, whether one obtained from the client or from some local
  source), nor with how type transitions normally work (exec, with the
  program type as the second type field).  I think it will just cause
  confusion and subtle breakage.
 
 It's causing me lots of confusion as it is.  I have been / am being told by
 different people to do different things just in dealing with SELinux, and
 various people are raising extra requirements or restrictions beyond that.
 There doesn't seem to be a consensus.
 
 It sounds like the best option is just to have the kernel nick the userspace
 daemon's security context and use that as is, and junk all the restrictions on
 what the daemon can do so that the kernel isn't too restricted.

Well, you could do that, if that meets your needs, but it doesn't sound
very optimal either.  Why are you opposed to having userspace determine
the context and write it to a cachefiles interface, and just have the
kernel authorize it (invoke a hook to check permissions between the
daemon's context and the specified label), and make it the acting
context when appropriate (invoke a different hook to set it as the
acting context)?

-- 
Stephen Smalley
National Security Agency

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 08/28] SECURITY: Allow kernel services to override LSM settings for task actions [try #2]

2007-12-11 Thread Stephen Smalley
On Mon, 2007-12-10 at 15:46 -0800, Casey Schaufler wrote:
 --- David Howells [EMAIL PROTECTED] wrote:
 
  Stephen Smalley [EMAIL PROTECTED] wrote:
  
   From a config file whose pathname would be provided by libselinux (ala
   the way in which dbusd imports contexts), or directly as a context
   returned by a libselinux function.
  
  That sounds too SELinux specific.  How do I do it so that it works for any
  LSM?
  
  Is linking against libselinux is a viable option if it's not available under
  all LSM models?  Is it available under all LSM models?  Perhaps Casey can
  answer this one.
 
 Linking against libselinux is not now, nor will it ever be, a viable
 option. There's just too much sophistication contained in libselinux
 for us simple folk to deal with.
 
I use to do that, but someone objected...  Possibly Karl MacMillan.
   
   Yes, but I think I disagreed then too.
  
  So, who's right?
 
 Me! (smiley inserted here, for those in need)
 
   It doesn't fit with how other users of security_kernel_act_as() will
   likely want to work (they will want to just set the context to a
   specified value, whether one obtained from the client or from some local
   source), nor with how type transitions normally work (exec, with the
   program type as the second type field).  I think it will just cause
   confusion and subtle breakage.
  
  It's causing me lots of confusion as it is.  I have been / am being told by
  different people to do different things just in dealing with SELinux, and
  various people are raising extra requirements or restrictions beyond that.
  There doesn't seem to be a consensus.
  
  It sounds like the best option is just to have the kernel nick the userspace
  daemon's security context and use that as is, and junk all the restrictions
  on
  what the daemon can do so that the kernel isn't too restricted.
 
 That would be consistant with the (perhaps archaic now) behavior
 of nfsd on Unix, which did nothing but lend it's credential to the
 underlying kernel code. I think it's a rational approach, although I
 expect that in may have troubles under SELinux.

nfsd needs to able to set the acting label to a value determined based
on the client so that file operations performed on behalf of the client
are subjected to the right set of permission checks and new files are
labeled properly, just as it already does for uid and gid (via fsuid and
fsgid).  So merely inheriting the label from the nfsd daemon doesn't
help with that purpose.

Both nfsd and cachefiles need a way to set the acting label, so having a
common hook for both to do that makes sense.  The authorization of that
label will differ, so splitting the authorization into a separate hook
also makes sense.
 
-- 
Stephen Smalley
National Security Agency

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 08/28] SECURITY: Allow kernel services to override LSM settings for task actions [try #2]

2007-12-11 Thread Stephen Smalley
On Tue, 2007-12-11 at 11:26 -0800, Casey Schaufler wrote:
 --- Stephen Smalley [EMAIL PROTECTED] wrote:
 
  On Mon, 2007-12-10 at 14:26 -0800, Casey Schaufler wrote:
   --- Stephen Smalley [EMAIL PROTECTED] wrote:
   
On Mon, 2007-12-10 at 21:08 +, David Howells wrote:
 Stephen Smalley [EMAIL PROTECTED] wrote:
 
  Otherwise, only other issue I have with this interface is it won't
  generalize to dealing with nfsd, where we want to set the acting
  context
  to a context we obtain from or determine based upon the client.
 
 Are you speaking of security_kernel_act_as() and
  security_create_files_as()
 specifically?  Or the task_struct::act_as override pointer in general?

security_kernel_act_as()

 I don't really know how nfsd wants to obtain and set its LSM context,
  so
it's
 a bit difficult for me to make something that works for nfsd as well 
 as
 cachefiles.

It would get a context from the client or from a local configuration
that would map security-unaware clients to a default context, and then
want to assume that context for the particular operation.  No transition
involved.
   
   I would expect that the operation would be more sophisticated
   than that. You certainly aren't going to use what comes from
   the other side without any processing, and I expect you'll have
   some sort of operation on anything you pull from a config file
   before you actually apply it.
  
  Yes, that's true - the contexts would be subjected to a permission
  check.  But that's separable from the act of setting it as the task's
  acting security state (and needs to be separated, as the precise check
  will vary depending on the situation - cachefiles is going to apply a
  different sort of check than nfsd).
  
  Why can't cachefilesd just push a context into the kernel and pass
  that
  into the hook as the acting context,
 
 How does cachefilesd come up with such a context?  Grab it from
 /etc/cachefilesd.conf?

From a config file whose pathname would be provided by libselinux (ala
the way in which dbusd imports contexts), or directly as a context
returned by a libselinux function.  Has to be done that way so that it
can be set differently for different policy types (strict, targeted,
mls).
   
   Unless you've got an LSM other than SELinux, of course. If
   cachefilesd is going to be responsible for maintaining this
   magic context there needs to be an LSM interface for it, not
   just an SELinux interface.
  
  LSM is an in-kernel interface.  Here we are talking about a userspace
  interface for obtaining the right security label to use.  There is no
  equivalent to LSM in userspace as of yet.  Feel free to invent one, but
  don't ask the rest of us to do it or wait for it to materialize.
 
 I am much more concerned with the interfaces used to pass the
 information into the kernel. I would expect that to be LSM
 independent, not a call into libselinux that resolves into a
 selinuxfs operation, or it's netlink equivilant. It would be
 unfortunate if the userland/kernel interface became an obstacle
 to cachefiles being adopted.

That wasn't the issue.  The interface to the cachefiles module would
just consist of cachefilesd writing a string label to some pseudo file
tell cachefiles what label to apply as the acting label for operations
performed by cachefiles.  Which isn't SELinux-specific at all.

David was asking though how cachefilesd (the userspace agent) would
obtain such a label to use.  And that may very well be LSM-specific, and
as there is no LSM userspace API, it makes sense for him to invoke a
libselinux function at present.  If a liblsm is later created and
provides a common front-end API (internally dlopen'ing the right shared
library based on some configuration, whether libselinux or libsmack or
whatever), then cachefilesd can instead call the liblsm interface, but
that doesn't exist today.

-- 
Stephen Smalley
National Security Agency

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 08/28] SECURITY: Allow kernel services to override LSM settings for task actions [try #2]

2007-12-11 Thread Stephen Smalley
On Tue, 2007-12-11 at 20:42 +, David Howells wrote:
 Stephen Smalley [EMAIL PROTECTED] wrote:
 
   That sounds too SELinux specific.  How do I do it so that it works for any
   LSM?
  
  You can't.  There is no LSM for userspace; LSM specifically disavowed
  any common userspace API, and that was one of our original
  objections/concerns about it.
 
 So, basically, userspace programs (outside of security tools) aren't supposed
 to need access to the security infrastructure?

I didn't say that.  I said that LSM provides no standard way for
userspace to access the security infrastructure, at least beyond the
raw /proc/self/attr and xattr APIs, and I view that as a defect in LSM.
SELinux does have a userspace API, because we think there should be a
way for userspace to do that, and it is being used by userspace programs
outside of security tools.

   Is linking against libselinux is a viable option if it's not available 
   under
   all LSM models?  Is it available under all LSM models?  Perhaps Casey can
   answer this one.
  
  Nope, they would all have their own libraries, if they have a library at
  all.  But that isn't your problem
 
 It is if I have to maintain a special pieces of code for each possible LSM.
 One piece for SELinux, one piece for AppArmour, one piece for Smack, one piece
 for Casey's security system.  That sounds like a pain.

All your code has to do is invoke a function provided by libselinux.  If
at some later time a liblsm is introduced that provides a common
front-end to a libselinux, libsmack, ..., then you can use that.  But it
doesn't exist today.   But it all just becomes a simple function call
regardless.

  - your kernel interface should be generic, and your LSM hooks should be
  generic, but your userspace isn't required to be.  Have a look at how many
  programs in the distribution currently link against libselinux, whether
  directly or by dlopen'ing it.
 
 In /usr/bin ldd reports approximately 297 binaries link to libselinux, though
 I can't say how many of those linked against it directly rather than picking
 it up by contamination through a shared library.  Furthermore, I've no idea
 how many more find it by dlopen.

The point being that userspace is already using the SELinux API directly
- cachefilesd certainly won't be the first or most crucial application
to do so.

 I use to do that, but someone objected...  Possibly Karl MacMillan.

Yes, but I think I disagreed then too.
   
   So, who's right?
  
  Karl isn't a maintainer of the SELinux kernel code.  And I had thought
  that even he had reconsidered this idea after further discussion.
 
 Not that I know of.

Well, he can speak to that.  But regardless, he isn't the maintainer.

It doesn't fit with how other users of security_kernel_act_as() will
likely want to work (they will want to just set the context to a
specified value, whether one obtained from the client or from some local
source), nor with how type transitions normally work (exec, with the
program type as the second type field).  I think it will just cause
confusion and subtle breakage.
   
   It's causing me lots of confusion as it is.  I have been / am being told 
   by
   different people to do different things just in dealing with SELinux, and
   various people are raising extra requirements or restrictions beyond that.
   There doesn't seem to be a consensus.
   
   It sounds like the best option is just to have the kernel nick the 
   userspace
   daemon's security context and use that as is, and junk all the 
   restrictions on
   what the daemon can do so that the kernel isn't too restricted.
  
  Well, you could do that, if that meets your needs, but it doesn't sound
  very optimal either.
 
 True.  I'd rather have separate security for each.
 
  Why are you opposed to having userspace determine the context and write it
  to a cachefiles interface,
 
 Because, from what I gather, that means my userspace program needs to do
 something different, depending on the security model that's currently in force
 on a system.  Furthermore, I would have to have separate code, as far as I
 know, for each security model as there's no commonality in userspace.

 I can't just link against libselinux.  It might not be there.  I'm not going
 to tie my program to SELinux either.

You can certainly make the linking conditional at compile time based on
a build flag, and/or you can dlopen it at runtime and fall back
gracefully if not present.  It isn't tying your program to libselinux,
just using it if present, in this case to obtain an input to supply to
your kernel module so that the policy of deciding what context to use
is completely determined in userspace and not hardcoded at all into the
kernel.  If init and ls and ps can do it, cachefilesd can too.

 Furthermore, I worked out the right way to do this with some apparent
 SELinux person, and you seemed reasonably accepting of it.  Now I have to go
 and redo all the work

Re: [PATCH 08/28] SECURITY: Allow kernel services to override LSM settings for task actions [try #2]

2007-12-12 Thread Stephen Smalley
On Tue, 2007-12-11 at 15:04 -0800, Casey Schaufler wrote:
 --- David Howells [EMAIL PROTECTED] wrote:
 
  Stephen Smalley [EMAIL PROTECTED] wrote:
  
   All your code has to do is invoke a function provided by libselinux.
  
  Calling libselinux means it's a special case for a specific LSM.
  
  I think the best way to do this, then, has to be to dlopen the appropriate
  LSM
  library.  That way I don't need to do any conditional compilation or 
  linking,
  but can build all the bits in to cachefilesd and have the appropriate one
  selected by the /etc/cachefilesd.conf.
  
  So, what do I invoke in libselinux, how do I configure it, and how do I
  integrate the config into my RPM and install it?
  
  And then what does it give me that I can hand to the kernel (a context 
  string
  for SELinux, I presume), how do I get the kernel to make a check on it, how
  do
  I configure the check and how do I install that config from my RPM (I 
  presume
  I just need to modify the .fc, .if and .te files that I have already)?
 
 That seems like an awful lot of work. I suggest that what you
 put in /etc/cachefilesd.conf is a line like:
 
security_context:whatever
 
 and have your daemon pass whatever into the kernel using
 a cachefile mechanism. The kernel code can call
 security_secctx_to_secid(whatever) to determine if it's valid.
 No need to invoke LSM specific code in your daemon. You may need
 to have an application, say cachefileselinuxcontext, that will
 read the current policy and spit out an appropriate value of
 whatever, but that can be separate and LSM specific without
 mucking up your basic infrastructure applications. 

That sounds workable, although I think he will want a more specific hook
than security_secctx_to_secid(), or possibly a second hook call, that
would not only validate the context but authorize the use of it by the
cachefilesd process.  And then the security_task_kernel_act_as() hook
just takes the secid as input rather than the task struct of the daemon,
and applies it.  At that point, nfsd can use the same mechanism for
setting the acting SID based on the client process after doing its own
authorization.

   That mostly works, but it means that an update to policy may require an
   update to /etc/cachefilesd.conf, or that switching from one policy to
   another might likewise require changing that file.  Versus using a
   separate policy-provided config file for the label.
  
  Whilst that's a fair point, if it's in a config file somewhere, then someone
  may want to change it or someone may want to provide a second file for a
  second cache with a different security label.
 
   BTW, as should be obvious, some LSMs aren't label-based at all, so it
   would need to be optional.
  
  Aargh.  In which case it might not be possible to make the SELinux context
  passing from userspace - kernel generic for all LSMs:-(
 
 For LSM's that don't use labels what you will have to pass in
 won't be a label, it will be something else. But since any LSM
 that wants to do networking or audit will have to deal with
 secid's and secctx's the method outlined above ought to fit the
 bill.

-- 
Stephen Smalley
National Security Agency

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 08/28] SECURITY: Allow kernel services to override LSM settings for task actions [try #2]

2007-12-12 Thread Stephen Smalley
On Wed, 2007-12-12 at 08:51 -0800, Casey Schaufler wrote:
 --- Stephen Smalley [EMAIL PROTECTED] wrote:
 
  On Tue, 2007-12-11 at 15:04 -0800, Casey Schaufler wrote:
   --- David Howells [EMAIL PROTECTED] wrote:
   
Stephen Smalley [EMAIL PROTECTED] wrote:

 All your code has to do is invoke a function provided by libselinux.

Calling libselinux means it's a special case for a specific LSM.

I think the best way to do this, then, has to be to dlopen the
  appropriate
LSM
library.  That way I don't need to do any conditional compilation or
  linking,
but can build all the bits in to cachefilesd and have the appropriate 
one
selected by the /etc/cachefilesd.conf.

So, what do I invoke in libselinux, how do I configure it, and how do I
integrate the config into my RPM and install it?

And then what does it give me that I can hand to the kernel (a context
  string
for SELinux, I presume), how do I get the kernel to make a check on it,
  how
do
I configure the check and how do I install that config from my RPM (I
  presume
I just need to modify the .fc, .if and .te files that I have already)?
   
   That seems like an awful lot of work. I suggest that what you
   put in /etc/cachefilesd.conf is a line like:
   
  security_context:whatever
   
   and have your daemon pass whatever into the kernel using
   a cachefile mechanism. The kernel code can call
   security_secctx_to_secid(whatever) to determine if it's valid.
   No need to invoke LSM specific code in your daemon. You may need
   to have an application, say cachefileselinuxcontext, that will
   read the current policy and spit out an appropriate value of
   whatever, but that can be separate and LSM specific without
   mucking up your basic infrastructure applications. 
  
  That sounds workable, although I think he will want a more specific hook
  than security_secctx_to_secid(), or possibly a second hook call, that
  would not only validate the context but authorize the use of it by the
  cachefilesd process.
 
 What sort of authorization are you thinking of? I would expect
 that to have been done by cachefileselinuxcontext (or
 cachefilesspiffylsmcontext) up in userspace. If you're going to
 rely on userspace applications for policy enforcement they need
 to be good enough to count on after all.

In Smack, I'd expect that you'd want to apply a CAP_MAC_OVERRIDE check.
In SELinux, we'd apply a permission check between the task's security
context and the specified security context so that we can control the
pairwise relationship between them via allow rules and constraints.

The kernel has no way of knowing whether the context was determined by
cachefileselinuxcontext or not; it only knows that some task is trying
to write some value to /cachefiles/context or whatever the kernel
interface is, and it needs to apply some authorization check there,
where that check is security-module-specific.

  And then the security_task_kernel_act_as() hook
  just takes the secid as input rather than the task struct of the daemon,
  and applies it.  At that point, nfsd can use the same mechanism for
  setting the acting SID based on the client process after doing its own
  authorization.
 
 good points all, in spite of my personal distaste for secids.

-- 
Stephen Smalley
National Security Agency

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 08/28] SECURITY: Allow kernel services to override LSM settings for task actions [try #2]

2007-12-12 Thread Stephen Smalley
On Wed, 2007-12-12 at 18:29 +, David Howells wrote:
 Stephen Smalley [EMAIL PROTECTED] wrote:
 
  That sounds workable, although I think he will want a more specific hook
  than security_secctx_to_secid(), or possibly a second hook call, that
  would not only validate the context but authorize the use of it by the
  cachefilesd process.  And then the security_task_kernel_act_as() hook
  just takes the secid as input rather than the task struct of the daemon,
  and applies it.  At that point, nfsd can use the same mechanism for
  setting the acting SID based on the client process after doing its own
  authorization.
 
 I thought using secids was verboten as it made things too specific.

Well, that has been Casey's objection in the past to it, but he seems to
have accepted their use now for certain purposes, and they are already
entrenched in the audit and labeled networking interfaces.

 Have you example code for the security hook you mention?  I'm not sure I
 understand why security_secctx_to_secid() is not sufficient.

security_secctx_to_secid() would just validate and map a context string
to a secid.  It wouldn't perform any permission check, as the caller
might a kernel-internal user that is just mapping back and forth like
current users of security_secid_to_secctx, or it might be something that
ultimately originated from userspace but the hook has no way of knowing
why or what set of checks would be appropriate.  You'd need a more
specific hook for the authorization, one that would perform a permission
check, e.g. an avc_has_perm() call.  Which likely requires defining a
new class and permissions for your cachefiles kernel interface.

 Or is it that I need something that takes a secctx, converts it to a secid and
 authorises its use all in one go?  If it's this, why can't that be rolles into
 security_task_kernel_act_as()?  That sets up a task_security struct which is
 then switched in and out without consultation of the LSM.

I was under the impression that security_task_kernel_act_as() was being
used to switch the current task to an acting context, not to initially
set up a struct for later use.  If you go with the latter approach, then
what is the lifecycle on that struct?

BTW, it gets a little confusing with your use of task_security for the
full task security state vs our existing use of task_security_struct
within SELinux for the task's LSM security blob.  I suppose ours could
be renamed to task_selinux.

-- 
Stephen Smalley
National Security Agency

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 08/28] SECURITY: Allow kernel services to override LSM settings for task actions [try #2]

2007-12-12 Thread Stephen Smalley
On Wed, 2007-12-12 at 11:44 -0800, Casey Schaufler wrote:
 --- David Howells [EMAIL PROTECTED] wrote:
 
  Casey Schaufler [EMAIL PROTECTED] wrote:
  
   What sort of authorization are you thinking of? I would expect
   that to have been done by cachefileselinuxcontext (or
   cachefilesspiffylsmcontext) up in userspace. If you're going to
   rely on userspace applications for policy enforcement they need
   to be good enough to count on after all.
  
  It can't be done in userspace, otherwise someone using the cachefilesd
  interface can pass an arbitrary context up.
 
 Yes, but I would expect that interface to be protected (owned by root,
 mode 0400). If /dev/cachefiles has to be publicly accessable make it
 a privileged ioctl.

Uid 0 != CAP_MAC_OVERRIDE if you configure file caps and such.

  The security context has to be
  passed across the file descriptor attached to /dev/cachefiles along with the
  other configuration parameters as a text string.
 
 I got that.
 
  This fd selects the
  particular cache context that a particular instance of a running daemon is
  using.
 
 Yes, but forgive me being slow, I don't see the problem.
 
 
 Casey Schaufler
 [EMAIL PROTECTED]
-- 
Stephen Smalley
National Security Agency

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 08/28] SECURITY: Allow kernel services to override LSM settings for task actions [try #2]

2007-12-12 Thread Stephen Smalley
On Wed, 2007-12-12 at 11:20 -0800, Casey Schaufler wrote:
 --- David Howells [EMAIL PROTECTED] wrote:
 
  Casey Schaufler [EMAIL PROTECTED] wrote:
  
   You may need to have an application, say cachefileselinuxcontext, that 
   will
   read the current policy and spit out an appropriate value of whatever,
   but that can be separate and LSM specific without mucking up your basic
   infrastructure applications.
  
  What would I do with such a thing?  How would it get run?  Spat out to 
  where?
 
 Put it in /etc/init.d/cachefiles and run it at boot time. Put the
 result into /etc/cachefiles.conf. Have cachefilesd read it and pass
 it downward.

More likely, run it at build time in your .spec file to generate
cachefiles.conf, then run it again maybe upon a policy update or if the
user selects a different policy.

-- 
Stephen Smalley
National Security Agency

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 08/28] SECURITY: Allow kernel services to override LSM settings for task actions [try #2]

2007-12-13 Thread Stephen Smalley
On Wed, 2007-12-12 at 22:49 +, David Howells wrote:
 Stephen Smalley [EMAIL PROTECTED] wrote:
 
   Have you example code for the security hook you mention?  I'm not sure I
   understand why security_secctx_to_secid() is not sufficient.
  
  security_secctx_to_secid() would just validate and map a context string to a
  secid.
 
 Validate as in check it's a valid string?  Okay, I see that.

Yes.

  It wouldn't perform any permission check, as the caller might a
  kernel-internal user that is just mapping back and forth like current users
  of security_secid_to_secctx, or it might be something that ultimately
  originated from userspace but the hook has no way of knowing why or what set
  of checks would be appropriate.  You'd need a more specific hook for the
  authorization, one that would perform a permission check, e.g. an
  avc_has_perm() call.  Which likely requires defining a new class and
  permissions for your cachefiles kernel interface.
 
 Hmmm...  This is sounding very not-simple.  I don't know how to do this.  I
 can probably guess the kernel side by looking at how SECCLASS_KEY is done, but
 it sounds like it involves changes to the userspace policy processing tools
 too.

Not the tools, just the policy definitions.  Dan can help with that.
You add the definitions to the policy, then there is a script to
regenerate the SELinux kernel headers that #define the SECCLASS_ and
permission values.

 It also sounds a bit like overkill, but if it's the right way then I guess it
 has to be done.
 
 What does the security class represent in this case?  And can it be 
 generalised
 to apply to non-cachefiles stuff too?

It is just a way of carving up the permission space, typically based on
object type, but it can essentially be arbitrary.  The check in this
case seems specific to cachefiles since it is controlling an operation
on the /dev/cachefiles interface that only applies to cachefiles
internal operations, so making a cachefiles class seems reasonable.

If this was instead just a generic set my acting context to value
operation, then it could be a generic /proc/self/attr/active interface
with an generic implementation and permission check, but here we aren't
setting the active context of the cachefilesd daemon but rather of the
cachefiles kernel module.

The other approach that I suggested a long time ago is to exempt the
cachefiles kernel module internal operations from SELinux permission
checks altogether by setting some task flag when performing those
operations and checking for that flag either on entry to the security_
static inlines, similar to the inode private flag, or within SELinux
itself.  Rationale being that the cachefiles kernel module can already
do what it wants and the SELinux permission checks are really about
controlling what userspace can do.  Then we don't have to invent a
context for the kernel module at all or worry about subtle breakage when
some permission is missing from its policy.

   Or is it that I need something that takes a secctx, converts it to a secid
   and authorises its use all in one go?  If it's this, why can't that be
   rolles into security_task_kernel_act_as()?  That sets up a task_security
   struct which is then switched in and out without consultation of the LSM.
  
  I was under the impression that security_task_kernel_act_as() was being used
  to switch the current task to an acting context, not to initially set up a
  struct for later use.
 
 Definitely the latter.  I guess I wasn't very clear in the patch description.
 It also sounds like I need to adjust the naming of certain functions.
 
  If you go with the latter approach, then what is the lifecycle on that
  struct?
 
  (1) You create a new task_security struct
 
  (2) You fill in the fsuid, fsgid, etc.
 
  (3) You request that the LSM security pointer in it be set to point to the
  context you want (at the moment this is done by attempting a transition
  from the daemon's context):
 
   ret = security_transition_sid(dtsec-sid, SECINITSID_KERNEL,
 SECCLASS_PROCESS, ksid);
 
  and, in the current code, it returns an error if you're not allowed to do
  that.  But instead you'd ask it to set a specific context, and it'd set
  that if you're permitted to do that, and give you an error if you're not.
 
  (4) You then use the task_security at will to override the task-act_as
  pointer in whatever task(s) you're operating on behalf of at the moment.
 
  (5) When you cease operating on the behalf of a task, you revert its act_as
  pointer and drop a reference to your task_security struct.
 
  (6) When the last ref to the task_security struct goes away, the LSM data
  attached to it is released, as are its groups and keyrings.
 
  BTW, it gets a little confusing with your use of task_security for the full
  task security state vs our existing use of task_security_struct within
  SELinux for the task's LSM security blob.
 
 I know

Re: [PATCH 08/28] SECURITY: Allow kernel services to override LSM settings for task actions [try #2]

2007-12-13 Thread Stephen Smalley
On Wed, 2007-12-12 at 22:55 +, David Howells wrote:
 Stephen Smalley [EMAIL PROTECTED] wrote:
 
  More likely, run it at build time in your .spec file to generate
  cachefiles.conf,
 
 I don't think sticking it in cachefiles.conf is a good idea necessarily.
 That has to be an administrator modifiable file.  Is there a program I could
 make cachefiles run directly and capture the output of that could give me the
 info I want?

Yes, we could easily make a simple program that just invokes a
libselinux function that in turn grabs the proper context from some
context configuration file under /etc/selinux/$SELINUXTYPE/contexts/ and
outputs it.  Dan can help with that.

  then run it again maybe upon a policy update or if the user selects a
  different policy.
 
 How do I do that?

-- 
Stephen Smalley
National Security Agency

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 08/28] SECURITY: Allow kernel services to override LSM settings for task actions [try #2]

2007-12-13 Thread Stephen Smalley
On Thu, 2007-12-13 at 15:36 +, David Howells wrote:
 Stephen Smalley [EMAIL PROTECTED] wrote:
 
  It is just a way of carving up the permission space, typically based on
  object type, but it can essentially be arbitrary.  The check in this
  case seems specific to cachefiles since it is controlling an operation
  on the /dev/cachefiles interface that only applies to cachefiles
  internal operations, so making a cachefiles class seems reasonable.
 
 Can you specify what sort of permissions you're thinking of providing for
 tasks to operate on this class?

They would correspond with the operations provided by
the /dev/cachefiles interface, at the granularity you want to support
distinctions to be made.  Could just be a single 'setcontext' permission
if that is all you want to control distinctly, or could be a permission
per operation.

   Can an object of this class 'operate' on
 other objects, or can only process-class objects do that?

In this case, I wouldn't expect a cachefiles object to act on anything
else.  Some objects are also used as subjects, especially in the
networking arena.

 How does an object of this class acquire a label?  What is an object of this
 class?  Is it a cache?  Or were you thinking of a module?

I was thinking the latter since the only goal was to control what
contexts could be set by a given task, but you could support per-cache
objects with their own labels (in which case the label would likely be
determined from the creating task).

If the latter, you don't really need a label for the object, and can
just use the supplied context/secid as the object of the permission
check, ala:
rc = avc_has_perm(tsec-sid, secid, SECCLASS_CACHEFILES,
CACHEFILES__SETCONTEXT);

If the former, then you'd need more than one check, as you then have to
check whether the task can act on the cache in question, and then check
whether it can set the context for that cache to the specified value.

-- 
Stephen Smalley
National Security Agency

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 08/28] SECURITY: Allow kernel services to override LSM settings for task actions [try #2]

2007-12-13 Thread Stephen Smalley
On Thu, 2007-12-13 at 17:01 +, David Howells wrote:
 Stephen Smalley [EMAIL PROTECTED] wrote:
 
  They would correspond with the operations provided by the /dev/cachefiles
  interface, at the granularity you want to support distinctions to be made.
 
 Can this be made simpler by the fact that /dev/cachefiles has its own unique
 label (cachefiles_dev_t).

That lets you control who can use the interface at all, but not what
operations they can invoke on it.

  Could just be a single 'setcontext' permission if that is all you want to
  control distinctly, or could be a permission per operation.
 
 There is only one operation that makes sense to have a permission: set
 context and begin caching.
 
 All the other operations on a file descriptor attached to /dev/cachfiles are
 necessary for there to be a managed cache at all, and given that you've
 managed to open /dev/cachefiles that's sufficient access for those, I think.

Do any of the interfaces allow a task to act on a cache other than one
it has created?  How does the task identify the desired cache?  What if
there is a conflict between multiple tasks asking for the same cache?

  If the latter, you don't really need a label for the object, and can
  just use the supplied context/secid as the object of the permission
  check, ala:
  rc = avc_has_perm(tsec-sid, secid, SECCLASS_CACHEFILES,
  CACHEFILES__SETCONTEXT);
 
 Ummm.   I was under the impression that the target SID had to be a member of
 target class.

Not necessarily.

secid is being applied as the acting context for the cachefiles kernel
module, so the above makes sense, even though there isn't really any
object in view here.  Abstractly, the question we are asking above is:

Can this task set the context of the cachefiles kernel module to this
value?

-- 
Stephen Smalley
National Security Agency

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] Version 9 (2.6.24-rc1) Smack: Simplified Mandatory Access Control Kernel

2007-10-25 Thread Stephen Smalley
On Wed, 2007-10-24 at 20:46 -0700, Casey Schaufler wrote:
 From: Casey Schaufler [EMAIL PROTECTED]
 
 Smack is the Simplified Mandatory Access Control Kernel.
 
 Smack implements mandatory access control (MAC) using labels
 attached to tasks and data containers, including files, SVIPC,
 and other tasks. Smack is a kernel based scheme that requires
 an absolute minimum of application support and a very small
 amount of configuration data.
 
 Smack uses extended attributes and
 provides a set of general mount options, borrowing technics used
 elsewhere. Smack uses netlabel for CIPSO labeling. Smack provides
 a pseudo-filesystem smackfs that is used for manipulation of
 system Smack attributes.
 
 The patch, patches for ls and sshd, a README, a startup script,
 and x86 binaries for ls and sshd are also available on
 
 http://www.schaufler-ca.com
 
 This version has been tested with 2.6.23, various 2.6.23-gits
 and 2.6.24-rc1. Development has been done using Fedora Core 7
 in a virtual machine environment and on an old Sony laptop.
 
 Smack provides mandatory access controls based on the label attached
 to a task and the label attached to the object it is attempting to
 access. Smack labels are deliberately short (1-23 characters) text
 strings. Single character labels using special characters are reserved
 for system use. The only operation applied to Smack labels is equality
 comparison. No wildcards or expressions, regular or otherwise, are
 used. Smack labels are composed of printable characters and may not
 include /.
 
 A file always gets the Smack label of the task that created it.
 
 Smack defines and uses these labels:
 
 * - pronounced star
 _ - pronounced floor
 ^ - pronounced hat
 ? - pronounced huh
 
 The access rules enforced by Smack are, in order:
 
 1. Any access requested by a task labeled * is denied.
 2. A read or execute access requested by a task labeled ^
is permitted.
 3. A read or execute access requested on an object labeled _
is permitted.
 4. Any access requested on an object labeled * is permitted.
 5. Any access requested by a task on an object with the same
label is permitted.
 6. Any access requested that is explicitly defined in the loaded
rule set is permitted.
 7. Any other access is denied.
 
 Rules may be explicitly defined by writing subject,object,access
 triples to /smack/load.
 
 Smack rule sets can be easily defined that describe BellLaPadula
 sensitivity, Biba integrity, and a variety of interesting
 configurations. Smack rule sets can be modified on the fly to
 accomodate changes in the operating environment or even the time
 of day.
 
 Some practical use cases:
 
 Hierarchical levels. The less common of the two usual uses
 for MLS systems is to define hierarchical levels, often
 unclassified, confidential, secret, and so on. To set up smack
 to support this, these rules could be defined:
 
CUnclass rx
SC   rx
SUnclass rx
TS   S   rx
TS   C   rx
TS   Unclass rx
 
 A TS process can read S, C, and Unclass data, but cannot write it.
 An S process can read C and Unclass. Note that specifying that
 TS can read S and S can read C does not imply TS can read C, it
 has to be explicitly stated.
 
 Non-hierarchical categories. This is the more common of the
 usual uses for an MLS system. Since the default rule is that a
 subject cannot access an object with a different label no
 access rules are required to implement compartmentalization.
 
 A case that the Bell  LaPadula policy does not allow is demonstrated
 with this Smack access rule:
 
 A case that BellLaPadula does not allow that Smack does:
 
 ESPNABC   r
 ABC ESPN  r
 
 On my portable video device I have two applications, one that
 shows ABC programming and the other ESPN programming. ESPN wants
 to show me sport stories that show up as news, and ABC will
 only provide minimal information about a sports story if ESPN
 is covering it. Each side can look at the other's info, neither
 can change the other. Neither can see what FOX is up to, which
 is just as well all things considered.
 
 Another case that I especially like:
 
 SatData Guard   w
 Guard   Publish w
 
 A program running with the Guard label opens a UDP socket and
 accepts messages sent by a program running with a SatData label.
 The Guard program inspects the message to ensure it is wholesome
 and if it is sends it to a program running with the Publish label.
 This program then puts the information passed in an appropriate
 place. Note that the Guard program cannot write to a Publish
 file system object because file system semanitic require read as
 well as write.
 
 The four cases (categories, levels, mutual read, guardbox) here
 are all quite real, and problems I've been asked to solve over
 the years. The first two are easy to do with traditonal MLS systems
 while the last two you can't without invoking privilege, at least
 

Re: [AppArmor 35/45] Allow permission functions to tell between parent and leaf checks

2007-10-26 Thread Stephen Smalley
On Thu, 2007-10-25 at 23:40 -0700, [EMAIL PROTECTED] wrote:
 plain text document attachment (parent-permission.diff)
 Set the LOOKUP_CONTINUE flag when checking parent permissions. This allows
 permission functions to tell between parent and leaf checks.
 
 Signed-off-by: Andreas Gruenbacher [EMAIL PROTECTED]
 Signed-off-by: John Johansen [EMAIL PROTECTED]
 
 ---
  fs/namei.c |6 ++
  1 file changed, 6 insertions(+)
 
 --- a/fs/namei.c
 +++ b/fs/namei.c
 @@ -1472,6 +1472,10 @@ static int may_delete(struct inode *dir,
   BUG_ON(victim-d_parent-d_inode != dir);
   audit_inode_child(victim-d_name.name, victim, dir);
  
 +#if 0
 + if (nd)
 + nd-flags |= LOOKUP_CONTINUE;
 +#endif

#if 0?

   error = permission(dir,MAY_WRITE | MAY_EXEC, NULL);
   if (error)
   return error;
 @@ -1509,6 +1513,8 @@ static inline int may_create(struct inod
   return -EEXIST;
   if (IS_DEADDIR(dir))
   return -ENOENT;
 + if (nd)
 + nd-flags |= LOOKUP_CONTINUE;
   return permission(dir,MAY_WRITE | MAY_EXEC, nd);
  }
  
 
-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] Version 9 (2.6.24-rc1) Smack: Simplified Mandatory Access Control Kernel

2007-10-26 Thread Stephen Smalley
On Wed, 2007-10-24 at 20:46 -0700, Casey Schaufler wrote:
 diff -uprN -X linux-2.6.24-rc1-base/Documentation/dontdiff 
 linux-2.6.24-rc1-base/security/smack/smack_lsm.c 
 linux-2.6.24-rc1-smack/security/smack/smack_lsm.c
 --- linux-2.6.24-rc1-base/security/smack/smack_lsm.c  1969-12-31 
 16:00:00.0 -0800
 +++ linux-2.6.24-rc1-smack/security/smack/smack_lsm.c 2007-10-23 
 16:45:06.0 -0700
snip
 +/**
 + * smack_inode_getsecurity - get smack xattrs
 + * @inode: the object
 + * @name: attribute name
 + * @buffer: where to put the result
 + * @size: size of the buffer
 + * @err: unused
 + *
 + * Returns the size of the attribute or an error code
 + */
 +static int smack_inode_getsecurity(const struct inode *inode,
 +const char *name, void *buffer,
 +size_t size, int err)
 +{
 + struct socket_smack *ssp;
 + struct socket *sock;
 + struct super_block *sbp;
 + struct inode *ip = (struct inode *)inode;
 + char *bsp = buffer;
 + char *isp;
 +
 + if (size  SMK_LABELLEN || name == NULL || bsp == NULL ||
 + inode == NULL || inode-i_security == NULL)
 + return 0;
 +
 + if (strcmp(name, XATTR_SMACK_SUFFIX) == 0) {
 + isp = smk_of_inode(inode);
 + strncpy(buffer, isp, SMK_LABELLEN);
 + return strlen(isp) + 1;
 + }
 +
 + /*
 +  * The rest of the Smack xattrs are only on sockets.
 +  */
 + sbp = ip-i_sb;
 + if (sbp-s_magic != SOCKFS_MAGIC)
 + return -EOPNOTSUPP;
 +
 + sock = SOCKET_I(ip);
 + if (sock == NULL)
 + return -EOPNOTSUPP;
 +
 + ssp = sock-sk-sk_security;
 +
 + /*
 +  * Should the packet attribute be unavailable return the error.
 +  * This can happen if packets come in too fast.
 +  */
 + if (strcmp(name, XATTR_SMACK_PACKET) == 0) {
 + if (ssp-smk_packet[0] == '\0')
 + return -ENODATA;
 + isp = ssp-smk_packet;

Wrong strategy, racy.  Use getpeersec hooks, SO_PEERSEC for stream or
SCM_SECURITY for datagram.  They aren't just for labeled IPSEC - they
work fine for NetLabel too, see SELinux for an example.

 + } else if (strcmp(name, XATTR_SMACK_IPIN) == 0)
 + isp = ssp-smk_in;
 + else if (strcmp(name, XATTR_SMACK_IPOUT) == 0)
 + isp = ssp-smk_out;
 + else
 + return -EOPNOTSUPP;
 +
 + strncpy(buffer, isp, SMK_LABELLEN);
 + return strlen(isp) + 1;
 +}
 +
snip
 +static int smack_socket_recvmsg(struct socket *sock, struct msghdr *msg,
 + int size, int flags)
 +{
 + struct socket_smack *ssp = sock-sk-sk_security;
 +
 + /*
 +  * If the depth is 0 no packets are queued.
 +  * If the depth is  1 the current has been overwritten.
 +  */
 +
 + if (ssp-smk_depth != 1)
 + ssp-smk_packet[0] = '\0';
 + if (ssp-smk_depth != 0)
 + ssp-smk_depth--;
 +
 + return 0;
 +}

Same deal, use SCM_SECURITY and the getpeersec_dgram hook to do this in
a race-free way.

 +
 +/**
 + * smack_socket_sock_rcv_skb - Smack packet delivery access check
 + * @sk: socket
 + * @skb: packet
 + *
 + * Returns 0 if the packet should be delivered, an error code otherwise
 + */
 +static int smack_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
 +{
 + struct netlbl_lsm_secattr secattr;
 + struct socket_smack *ssp = sk-sk_security;
 + char smack[SMK_LABELLEN];
 + int rc;
 +
 + if (sk-sk_family != PF_INET  sk-sk_family != PF_INET6)
 + return 0;
 +
 + /*
 +  * Translate what netlabel gave us.
 +  */
 + memset(smack, '\0', SMK_LABELLEN);
 + netlbl_secattr_init(secattr);
 + rc = netlbl_skbuff_getattr(skb, secattr);
 + if (rc == 0)
 + smack_from_secattr(secattr, smack);
 + else
 + strncpy(smack, smack_net_ambient, SMK_MAXLEN);
 + netlbl_secattr_destroy(secattr);
 + /*
 +  * Receiving a packet requires that the other end
 +  * be able to write here. Read access is not required.
 +  * This is the simplist possible security model
 +  * for networking.
 +  */
 + rc = smk_access(smack, ssp-smk_in, MAY_WRITE);
 + if (rc != 0)
 + return rc;
 +
 + /*
 +  * If recv was called and there were no outstanding packets
 +  * this is the current Smack value to make available.
 +  */
 + if (ssp-smk_depth == 0)
 + strcpy(ssp-smk_packet, smack);
 + ssp-smk_depth++;

Ditto.

 +
 + return 0;
 +}
 +

-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] file capabilities: allow sigcont within session (v2)

2007-11-01 Thread Stephen Smalley
On Wed, 2007-10-31 at 18:49 -0500, Serge E. Hallyn wrote:
 From 5bff8967f45a35f858b96ca673d9bf98eac53d49 Mon Sep 17 00:00:00 2001
 From: Serge E. Hallyn [EMAIL PROTECTED]
 Date: Wed, 31 Oct 2007 11:22:04 -0500
 Subject: [PATCH 1/1] file capabilities: allow sigcont within session (v2)
 
 (This is a proposed fix to http://bugzilla.kernel.org/show_bug.cgi?id=9247)
 
 Allow sigcont to be sent to a process with greater capabilities
 if it is in the same session.  Otherwise, a shell from which
 I've started a root shell and done 'suspend' can't be restarted
 by the parent shell.
 
 Also don't do file-capabilities signaling checks when uids for
 the processes don't match, since the standard check_kill_permission
 will have done those checks.

Description doesn't match the code.  And in the non-matching uid case,
check_kill_permission typically returns an error before it reaches
cap_task_kill (modulo the special cases).

 
 Signed-off-by: Serge E. Hallyn [EMAIL PROTECTED]
 ---
  security/commoncap.c |9 +
  1 files changed, 9 insertions(+), 0 deletions(-)
 
 diff --git a/security/commoncap.c b/security/commoncap.c
 index bf67871..4de6857 100644
 --- a/security/commoncap.c
 +++ b/security/commoncap.c
 @@ -526,6 +526,15 @@ int cap_task_kill(struct task_struct *p, struct siginfo 
 *info,
   if (info != SEND_SIG_NOINFO  (is_si_special(info) || 
 SI_FROMKERNEL(info)))
   return 0;
  
 + /* if tasks have same uid, then check_kill_permission did check */
 + if (current-uid == p-uid || current-euid == p-uid ||
 + current-uid == p-suid || current-euid == p-suid)
 + return 0;

I'm confused - if you are allowing all signals within the same uid, then
what was the point of having a cap_task_kill at all?  cap_task_kill was
supposed to prevent a process with lesser capabilities from killing a
process with more capabilities, even if they have the same uid, so that
when you have a program marked with file capabilities instead of a
setuid-0 program, that program can't be sent arbitrary signals by the
caller.

 +
 + /* sigcont is permitted within same session */
 + if (sig == SIGCONT  (task_session_nr(current)==task_session_nr(p)))
 + return 0;
 +
   if (secid)
   /*
* Signal sent as a particular user.
-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 07/37] Security: De-embed task security record from task and use refcounting

2008-02-11 Thread Stephen Smalley

On Mon, 2008-02-11 at 17:30 +, David Howells wrote:
 James Morris [EMAIL PROTECTED] wrote:
 
   Remove the temporarily embedded task security record from task_struct.
   Instead it is made to dangle from the task_struct::sec and
   task_struct::act_as pointers with references counted for each.
  
  ...
  
  These patches are kind of huge.
 
 Yeah, I know.  The problem is that each patch must compile and run.  They
 can't be split up without violating that unfortunately.
 
  Why manually copy these fields after a kmemdup?
 
 Fair point.  Fixed.
 
  What about the task backpointer? (i.e. tsec2-task)
 
 The problem is that there can't be one with this patch as the task_security
 struct and the LSM security data attached to it may outlive the task it points
 back to.
 
 It seems that the backpointer can be dispensed with.  Nothing particularly
 seems to use it.  Do you know the reason for its existence?

Looks unused now.
Similarly for some of the other security structs.
Only inode, superblock, and sock back pointers still seem to be in use.

-- 
Stephen Smalley
National Security Agency

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/3] CRED: Split the task security data and move part of it into struct cred

2007-09-26 Thread Stephen Smalley
On Wed, 2007-09-26 at 14:30 +0100, David Howells wrote:
 Stephen Smalley [EMAIL PROTECTED] wrote:
 
  Precisely when to use one identity vs. the other though isn't always
  clear, and the potential for accidental divergence is also a concern.
 
 What should auditing use in audit_filter_rules() when dealing with
 AUDIT_SUBJ_* cases?  Should the SUBJ cases use the subjective SID and the
 AUDIT_OBJ_* cases use the objective SID?  On the other hand AUDIT_OBJ_* cases
 don't seem to have anything to do with tasks.

(cc'd linux-audit)

As you say, I don't think AUDIT_OBJ_* has anything to do with tasks,
just object labels (like inode labels).

I think you likely want the actor SID / subject SID or whatever you want
to call it for AUDIT_SUBJ_*.

-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Version 3 (2.6.23-rc8) Smack: Simplified Mandatory Access Control Kernel

2007-10-01 Thread Stephen Smalley
On Mon, 2007-10-01 at 08:07 -0700, Linus Torvalds wrote:
 
 On Mon, 1 Oct 2007, James Morris wrote:
  
  Merging Smack, however, would lock the kernel into the LSM API.  
  Presently, as SELinux is the only in-tree user, LSM can still be removed.
 
 Hell f*cking NO!
 
 You security people are insane. I'm tired of this only my version is 
 correct crap. The whole and only point of LSM was to get away from that.
 
 And anybody who claims that there is consensus on SELinux is just in 
 denial.
 
 People are arguing against other peoples security on totally bogus points. 
 First it was AppArmor, now this.
 
 I guess I have to merge AppArmor and SMACK just to get this *disease* off 
 the table. You're acting like a string theorist, claiming that t here is 
 no other viable theory out there. Stop it. It's been going on for too damn 
 long.

You argued against pluggable schedulers, right?  Why is security
different?

Do you really want to encourage people to roll their own security module
rather than working toward a common security architecture and a single
balanced solution (which doesn't necessarily mean SELinux, mind you, but
certainly could draw from parts of it)?   As with pluggable schedulers,
the LSM approach prevents cross pollination and forces users to make
poor choices.

Some have suggested that security modules are no different than
filesystem implementations, but filesystem implementations at least are
constrained by their need to present a common API and must conform with
and leverage the VFS infrastructure.  Different security modules present
very different interfaces and behaviors from one another and LSM doesn't
provide the same kind of common functionality and well-defined semantics
as the VFS.  The result of merging many wildly different security
modules will be chaos for application developers and users, likely
leading them to ignore everything but the least common denominator.
It almost makes more sense to merge no security modules at all than to
have LSM and many different security modules.

If Smack is mergeable despite likely being nothing more than a strict
subset of SELinux (MAC, label-based, should be easily emulated on top of
SELinux or via fairly simple extension to it to make such emulation
simpler or more optimal), then what isn't mergeable as a separate
security module?

-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Version 3 (2.6.23-rc8) Smack: Simplified Mandatory Access Control Kernel

2007-10-05 Thread Stephen Smalley
 directly in a simplified SELinux-type policy.
 
 But Kyle, it's already possible to compile out the part I don't
 want. I configure SELinux off and away I go.
 
 Smack is not a subset of SELinux, it behaves differently. SELinux
 has a policy that is program behavior oriented, Smack is strictly
 subjet/object oriented. Your 4 components (A-D) are meaningless to
 Smack.

To clarify, SELinux is also based on subjects and objects grouped into
equivalence classes (labels), and the granularity at which one applies
protection is configurable, so you can certainly have very
coarse-grained labels that don't require any specific knowledge of
application behavior.  A type is just a security equivalence class - it
doesn't have to map to an application at all.

Also, the idea behind SELinux was that its policy engine (security
server, security/selinux/ss/*) could be replaced with other
implementations without affecting the rest of SELinux if someone wanted
to try radically different logic.  The interface to that policy engine
is itself general and not tied to TE.

   I'm not yet annoyed enough to go implement an iptables like  
   interface to the LSM enhancing it with more generic mechanism to  
   make the problem simpler, but I'm getting there.  Perhaps next  
   time  I'm bored.
  
   I think a fair amount of what we need is already done in SELinux,  
   and efforts would be better spent in figuring out what seems too  
   complicated in SELinux and making it simpler.
 
 The granularity and consequently the size of the policy specificiation
 result in policies that are too complicated. Tieing the policy to the
 expected behavior of specific applications adds to the complexity.

Well, it reveals the complexity already present in the system, and gives
you the option of controlling it.  Your choice as to at what granularity
to apply it.

 SELinux is designed to increase in complexity as it evolves. Making
 it simpler would conflict with the design goal of finer granularity.
 
   Probably a fair amount  of that just means better tools.
 
 Now what kind of tools are you talking about? Static analysis?
 Data flow diagrammers for Java?
 
   How about thinking of it another way.
  
   Perform the split up you talked about above and move the table  
   matching into the LSM hooks.
  
   Use something like the iptables action and match to module mapping  
   code so we can have multiple modules compiled in and useable at the  
   same time with the LSM hooks.
  
   I think it is firmly established that selling SElinux to everyone  
   is politically untenable.  However enhancing the LSM (even if it is  
   mostly selinux code movement down a layer) I think can be sold.
 
 That would be silly. Smack uses a significantly smaller set of hooks
 than SELinux requires and still does interesting things. We went through
 the replace LSM with the SELinux interface exercise a couple years
 ago, I would hate to have to regurgitate all those discussions.

I don't think Eric is proposing replacing LSM with the SELinux interface
as it exists today, but rather making LSM more Netfilter-like and
radically refactoring SELinux (and any other security module) to consist
of a chain of smaller modules that are more general and reusable, and
that can be composed and applied in interesting ways via an
iptables-like interface.  I'm not sure what that would look like
exactly, but it seems reasonable to explore.

One of the things left unresolved with LSM is userland API, and it does
involve more than just returning EPERM or EACCES to applications.  You
already have patched ls and sshd programs, and have acknowledged the
need for more userland modifications to ultimately achieve your own
goals.  If LSM is going to succeed in the kernel, then ultimately you
need some common API for userland so that you don't need separate
versions of ls, ps, sshd, etc for Smack vs SELinux vs. whatever.

-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Version 3 (2.6.23-rc8) Smack: Simplified Mandatory Access Control Kernel

2007-10-09 Thread Stephen Smalley
On Mon, 2007-10-08 at 10:31 -0700, Casey Schaufler wrote:
 --- Serge E. Hallyn [EMAIL PROTECTED] wrote:
 
  Quoting Casey Schaufler ([EMAIL PROTECTED]):
   ...
   Good suggestion. In fact, that is exactly how I approached my
   first two attempts at the problem. What you get if you take that
   route is an imposing infrastructure that has virually nothing
   to do and that adds no value to the solution. Programming to the
   LSM interface, on the other hand, allowed me to drastically reduce
   the size and complexity of the implementation.
  
  (tongue-in-cheek)
  
  No no, everyone knows you don't build simpler things on top of more
  complicated ones, you go the other way around.  So what he was
  suggesting was that selinux be re-written on top of smack.
  
  :)
 
 I'm not sure how seriously anyone ought to take what I'm about to
 outline. Please feel free to treat it with as much or as little
 reverence as you choose.
 
 How to implement SELinux starting from Smack.
 
 You'll need to break up the current rwxa accesses into a set
 that matches the current SELinux set. Assign each a letter and
 a MAY_ACTION #define.
 
 You'll need a mapping for domain transitions, something like:
 
  subject-label program-label new-subject-label
 
 This will require bprm hooks that aren't there now.
 
 Additional hooks will need to be filled out as Smack does not
 add access control to things that aren't objects or actions that
 aren't accesses. Treat these as if they are accesses to objects
 and there shouldn't be too much trouble.
 
 Do something about the linear search for subject/object label pairs.
 With the larger label set searching will become an issue.
 
 Audit integration, too. The networking code will require some work
 for ipsec. The interfaces are pretty clean, Smack isn't using it
 because the CIPSO interface is simpler, not because there's any
 real problem with it.
 
 I wouldn't expect the whole thing to be more than a couple week's
 work for someone who really wanted to do it.

Note that Serge said SELinux re-written on top of Smack, not rewrite
Smack to be more like SELinux.  I don't believe the former is even
possible, given that Smack is strictly less expressive and granular by
design.  Rewriting Smack to be more like SELinux should be possible, but
seems like more work than emulating Smack on SELinux via policy, and to
what end?

-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC]selinux: Improving SELinux read/write performance

2007-09-06 Thread Stephen Smalley
On Thu, 2007-09-06 at 16:27 +0900, Yuichi Nakamura wrote:
 Hello.
 
 As I posted before in selinux list,
 I found big overhead of SELinux in read/write on some CPUs,
 and trying tuning.
 There were discussion in previous threads.
 Part 1:
 http://marc.info/?t=11884534341r=1w=2
 Part 2:
 http://marc.info/?t=11888074984r=1w=2
 
 I would like to RFC again about this topic.

Thanks for your work on this, as this is clearly an important area to
improve.

 1. Background
 Look at benchmark result below.
 lmbench simple read/write.
 Big overhead exists especially on SH(SuperH) arch.
 
 1) Result for x86(Pentium 4 2.6Ghz), kernel 2.6.22
 Base  SELinux  Overhead(%)
 Simple read 1.10  1.24 12.3
 Simple write1.00  1.14 14.0
 * Base: kernel compiled without SELinux support
 
 2) Result for SH(SH4, SH7751R), kernel 2.6.22
 BaseSELinux   Overhead(%)
 Simple read 2.395.49  130.5
 Simple write2.075.10  146.6
 # This result is a little different from previous threads, 
 # because I changed some kernel configs.
 
 Overhead more than 100%
 I also found about 70-90% overhead in ARM.
 
 2. About patch
 I found a overhead in selinux_file_permission function.
 This is a function that is called in read/write calls, 
 and does SELinux permission check.
 SELinux checks permission both in open and read/write time.
 Stephen Smalley sugessted that we can usually skip permission check 
 in selinux_file_permission.
 By this patch, 
 permission check in selinux_file_permssion is done only when
 - sid of task has changed after file open
 - sid of inode has changed after file open
 - policy load or boolean change happen after file open
 
 To detect these changes,
 I added entries in file_security struct and saving these values at file open.
 
 And to save sid of inode at the time of file open,
 I had to add new LSM hook in __dentry_open function.
 
 3. Benchmark after applying this patch
 1) Result for x86(Pentium 4 2.6Ghz), kernel 2.6.22
 Base  SELinux  Overhead(%)
 Simple read 1.10  1.12 1.6(Before 12.3)
 Simple write1.00  1.03 3.6(Before 14.0)
 
 2) Result for SH(SH4, SH7751R), kernel 2.6.22
 BaseSELinux   Overhead(%)
 Simple read 2.392.65  11.1(Before 130.5)
 Simple write2.072.28  10.5(Before 146.6)
 
 Performance has improved a lot.
 I want comments from community.
 
 Signed-off-by: Yuichi Nakamura[EMAIL PROTECTED]
 ---
  fs/open.c |5 +++
  include/linux/security.h  |   11 +++
  security/selinux/avc.c|5 ++-
  security/selinux/hooks.c  |   54 
 +-
  security/selinux/include/objsec.h |3 ++
  5 files changed, 76 insertions(+), 2 deletions(-)
 diff -purN -X linux-2.6.22/Documentation/dontdiff 
 linux-2.6.22.orig/security/selinux/avc.c linux-2.6.22/security/selinux/avc.c
 --- linux-2.6.22.orig/security/selinux/avc.c  2007-07-09 08:32:17.0 
 +0900
 +++ linux-2.6.22/security/selinux/avc.c   2007-09-06 14:33:35.0 
 +0900
 @@ -123,6 +123,7 @@ DEFINE_PER_CPU(struct avc_cache_stats, a
  #endif
  
  static struct avc_cache avc_cache;
 +u32 policy_seqno = 0;
  static struct avc_callback_node *avc_callbacks;
  static struct kmem_cache *avc_node_cachep;
  
 @@ -431,8 +432,10 @@ static int avc_latest_notif_update(int s
   ret = -EAGAIN;
   }
   } else {
 - if (seqno  avc_cache.latest_notif)
 + if (seqno  avc_cache.latest_notif) {
   avc_cache.latest_notif = seqno;
 + policy_seqno = seqno;
 + }

I would have just provided an avc interface for obtaining the seqno,
e.g.
u32 avc_policy_seqno(void)
{
return avc_cache.latest_notif;
}

   }
   spin_unlock_irqrestore(notif_lock, flag);
  
 diff -purN -X linux-2.6.22/Documentation/dontdiff 
 linux-2.6.22.orig/security/selinux/hooks.c 
 linux-2.6.22/security/selinux/hooks.c
 --- linux-2.6.22.orig/security/selinux/hooks.c2007-07-09 
 08:32:17.0 +0900
 +++ linux-2.6.22/security/selinux/hooks.c 2007-09-06 16:08:36.0 
 +0900
 @@ -84,6 +84,7 @@
  extern unsigned int policydb_loaded_version;
  extern int selinux_nlmsg_lookup(u16 sclass, u16 nlmsg_type, u32 *perm);
  extern int selinux_compat_net;
 +extern u32 policy_seqno;

I think that they frown upon extern declarations in .c files (versus
in .h files), so we don't want to add more - we ultimately should clean
up what we presently have.

  
  #ifdef CONFIG_SECURITY_SELINUX_DEVELOP
  int selinux_enforcing = 0;
 @@ -220,6 +221,8 @@ static int file_alloc_security(struct fi
  
   fsec-file = file;
   fsec-sid = tsec-sid;
 + fsec-tsid = tsec-sid;

I'm not sure why we need the separate field here, as fsec-sid already
holds the allocating task SID and doesn't change.

 + fsec-pseqno = policy_seqno;

Not sure that you want

Re: [RFC]selinux: Improving SELinux read/write performance

2007-09-10 Thread Stephen Smalley
  2
  #define AVC_CALLBACK_REVOKE  4
 diff -purN -X linux-2.6.22/Documentation/dontdiff 
 linux-2.6.22.orig/security/selinux/include/objsec.h 
 linux-2.6.22/security/selinux/include/objsec.h
 --- linux-2.6.22.orig/security/selinux/include/objsec.h   2007-07-09 
 08:32:17.0 +0900
 +++ linux-2.6.22/security/selinux/include/objsec.h2007-09-10 
 09:56:22.0 +0900
 @@ -53,6 +53,8 @@ struct file_security_struct {
   struct file *file;  /* back pointer to file object */
   u32 sid;  /* SID of open file description */
   u32 fown_sid; /* SID of file owner (for SIGIO) */
 + u32 isid; /* SID of inode at the time of file open */
 + u32 pseqno;   /* Policy seqno at the time of file open */
  };
  
  struct superblock_security_struct {
 diff -purN -X linux-2.6.22/Documentation/dontdiff linux-2.6.22.orig/fs/open.c 
 linux-2.6.22/fs/open.c
 --- linux-2.6.22.orig/fs/open.c   2007-07-09 08:32:17.0 +0900
 +++ linux-2.6.22/fs/open.c2007-09-10 09:56:22.0 +0900
 @@ -698,6 +698,11 @@ static struct file *__dentry_open(struct
  
   if (!open  f-f_op)
   open = f-f_op-open;
 +
 + error = security_dentry_open(f, flags);
 + if (error)
 + goto cleanup_all;
 +
   if (open) {
   error = open(inode, f);
   if (error)
 diff -purN -X linux-2.6.22/Documentation/dontdiff 
 linux-2.6.22.orig/include/linux/security.h 
 linux-2.6.22/include/linux/security.h
 --- linux-2.6.22.orig/include/linux/security.h2007-07-09 
 08:32:17.0 +0900
 +++ linux-2.6.22/include/linux/security.h 2007-09-10 09:56:22.0 
 +0900
 @@ -503,6 +503,11 @@ struct request_sock;
   *   @file contains the file structure being received.
   *   Return 0 if permission is granted.
   *
 + * Security hook for dentry
 + *
 + * @dentry_open
 + *   Check permission or get additional information before opening dentry.
 + *
   * Security hooks for task operations.
   *
   * @task_create:
 @@ -1253,6 +1258,7 @@ struct security_operations {
   int (*file_send_sigiotask) (struct task_struct * tsk,
   struct fown_struct * fown, int sig);
   int (*file_receive) (struct file * file);
 + int (*dentry_open)  (struct file *file, int flags);
  
   int (*task_create) (unsigned long clone_flags);
   int (*task_alloc_security) (struct task_struct * p);
 @@ -1854,6 +1860,11 @@ static inline int security_file_receive 
   return security_ops-file_receive (file);
  }
  
 +static inline int security_dentry_open (struct file *file, int flags)
 +{
 + return security_ops-dentry_open (file, flags);
 +}
 +
  static inline int security_task_create (unsigned long clone_flags)
  {
   return security_ops-task_create (clone_flags);
 @@ -2529,6 +2540,11 @@ static inline int security_file_receive 
   return 0;
  }
  
 +static inline int security_dentry_open (struct file *file, int flags)
 +{
 + return 0;
 +}
 +
  static inline int security_task_create (unsigned long clone_flags)
  {
   return 0;
 
 Regards,
-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [2.6.20.17 review 00/58] 2.6.20.17 -stable review

2007-08-22 Thread Stephen Smalley
On Wed, 2007-08-22 at 06:23 -0700, James Morris wrote:
 On Wed, 22 Aug 2007, Michal Piotrowski wrote:
 
  I got a problem with SELinux
  http://www.stardust.webpages.pl/files/tbf/bitis-gabonica/2.6.20.17-rc1/console.log
  http://www.stardust.webpages.pl/files/tbf/bitis-gabonica/2.6.20.17-rc1/stable-config
 
 Please set
 CONFIG_SECURITY_SELINUX_ENABLE_SECMARK_DEFAULT=n
 
 You don't have complete policy for the new network controls, which are not 
 enabled by default and not integreated fully into distros yet.

Still, that denial shouldn't be against kernel_t unless he has iptables
SECMARK rules that assign that value.

It's the change to the skb allocator - no longer clears up through
truesize and thus secmark is garbage initially.  That would apply to
mainline too.

-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [2.6.20.17 review 00/58] 2.6.20.17 -stable review

2007-08-22 Thread Stephen Smalley
On Wed, 2007-08-22 at 09:36 -0400, Stephen Smalley wrote:
 On Wed, 2007-08-22 at 06:23 -0700, James Morris wrote:
  On Wed, 22 Aug 2007, Michal Piotrowski wrote:
  
   I got a problem with SELinux
   http://www.stardust.webpages.pl/files/tbf/bitis-gabonica/2.6.20.17-rc1/console.log
   http://www.stardust.webpages.pl/files/tbf/bitis-gabonica/2.6.20.17-rc1/stable-config
  
  Please set
  CONFIG_SECURITY_SELINUX_ENABLE_SECMARK_DEFAULT=n
  
  You don't have complete policy for the new network controls, which are not 
  enabled by default and not integreated fully into distros yet.
 
 Still, that denial shouldn't be against kernel_t unless he has iptables
 SECMARK rules that assign that value.
 
 It's the change to the skb allocator - no longer clears up through
 truesize and thus secmark is garbage initially.  That would apply to
 mainline too.

Oops, never mind - tail still follows secmark, so that shouldn't matter.
So I'm not sure why we are getting a bad value for secmark here - should
be initialized to zero and never modified unless there is an iptables
secmark rule.

-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [2.6.20.17 review 00/58] 2.6.20.17 -stable review

2007-08-22 Thread Stephen Smalley
On Wed, 2007-08-22 at 16:29 +0200, Michal Piotrowski wrote:
 On 22/08/07, James Morris [EMAIL PROTECTED] wrote:
  On Wed, 22 Aug 2007, Stephen Smalley wrote:
 
   Oops, never mind - tail still follows secmark, so that shouldn't matter.
   So I'm not sure why we are getting a bad value for secmark here - should
   be initialized to zero and never modified unless there is an iptables
   secmark rule.
 
  Michal, do you see this in current git?
 
 No, I do not see this problem in 2.6.23. I had similar problem last
 month, but it is fixed now.
 
 http://lkml.org/lkml/2007/7/12/362

The difference being that there the denials were against unlabeled_t
(the expected default in the presence of no iptables SECMARK rules, and
allowed by current policies), while the denials against 2.6.20.17 were
against kernel_t.  Which shouldn't ever happen unless you have an
iptables SECMARK rule that assigns that value to a packet.  So this is a
different issue.  BTW, the fact that it is showing up as kernel_t means
that skb-secmark == SECINITSID_KERNEL == 1, FWIW.  Whereas it should be
zero in the absence of iptables rules that set it.

-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [2.6.20.17 review 00/58] 2.6.20.17 -stable review

2007-08-22 Thread Stephen Smalley
On Wed, 2007-08-22 at 19:50 +0200, Michal Piotrowski wrote:
 On 22/08/07, Michal Piotrowski [EMAIL PROTECTED] wrote:
  On 22/08/07, James Morris [EMAIL PROTECTED] wrote:
 [snip]
   The previous problem is theoretically unrelated.  It arose via a separate
   mechanism which can't be used at the same as the one you're seeing now in
   the logs.
  
   So this either looks like a problem which has gone unnoticed and was
   inadvertently fixed at some point, or is unique to the 2.6.20 stable
   series.
 
  Yup, it is very interesting why no one noticed it. Binary search in 
  progress:
  good - 2.6.20.4
  bad - 2.6.20.8
 
 Ok, I narrowed the problem to 2.6.20.7. There are a few net changes
 http://eu.kernel.org/pub/linux/kernel/v2.6/ChangeLog-2.6.20.7
 any suggestions?
 
 I also have seen this avc on 2.6.20.6 during reboot
 
 [ 2333.905944] audit(1187803699.273:271): avc:  denied  { send } for
 saddr=192.168.1.70 src=48591 daddr=72.14.217.189 dest=80 netif=eth0
 scontext=user_u:system_r:unconfined_t:s0
 tcontext=system_u:system_r:kernel_t:s0 tclass=packet
 [ 2334.420598] audit(1187803699.789:272): avc:  denied  { send } for
 saddr=192.168.1.70 src=47248 daddr=66.249.91.18 dest=80 netif=eth0
 scontext=user_u:system_r:unconfined_t:s0
 tcontext=system_u:system_r:kernel_t:s0 tclass=packet
 
 so the roots of the problem may lie between 2.6.20.4 and 2.6.20.6
 
 http://www.stardust.webpages.pl/files/tbf/bitis-gabonica/2.6.20.17-rc1/console2.log

Might be related to this:
http://marc.info/?l=git-commits-headm=118271540932264w=2

-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 00/16] Permit filesystem local caching [try #3]

2007-08-13 Thread Stephen Smalley
On Sat, 2007-08-11 at 08:56 -0700, Casey Schaufler wrote:
 --- David Howells [EMAIL PROTECTED] wrote:
 
  Casey Schaufler [EMAIL PROTECTED] wrote:
  
   How would you expect an LSM that is not SELinux to interface with
   CacheFiles?
  
  You have to understand that I didn't know that much about the LSM interface,
  so I asked advice of the Red Hat security people, who, naturally, pointed me
  at the SELinux mailing list.  I knew my stuff would have to work with 
  SELinux
  to be used with RH stuff.
 
 Sigh. So it's not only SELinux specific, but RedHat specific as well.
 
  Furthermore, as you pointed out, there aren't any other LSM modules upstream
  yet for me to work against.  I would like CacheFiles to work with all LSM
  modules in general, but I don't know how to do that yet.
 
 While neither is upstream you can certainly look at AppArmor and Smack,
 both of which have nefarious plans for getting upstream.
 
  I'm open to suggestion as to how to modify things to support any LSM.
 
 It's been a long time since I dealt with file system cacheing, and
 that was under Unix, and I don't claim to have a working understanding
 of the internals of CacheFiles, but ...
 
  Btw, do you understand the problems that CacheFiles has to deal with?  If I
  set this down clearly, this may help you or someone else suggest a better 
  way
  to do things.
  
(1) Some random process tries to access a file on a network filesystem
(NFS example).
  
(2) NFS goes to the cache to attempt to read the data from there prior to
going to the network.
  
(3) The cache driver wants to access the files in the cache, but it's
running in the security context of either the aforementioned random
process, or one of FS-Cache's thread pool.
 
 I think that this is the point you should attack. Control the security
 characteristics of the cache driver properly and you shouldn't need the
 complexity that you're asking to introduce.
  
This security context, however, doesn't necessarily give it the rights
to access what's in the cache, so the driver has to be permitted to 
  act
as a context appropriate to accessing the cache, without changing the
overall security context of the random process (which would impact
things trying to act on that process - kill() for example).
 
 Can you run the cache as an independent thread and send it messages
 rather than trying to do things in the context of the calling process?
 I know that that involves extra bookkeeppingg, but it's lots safer.
 
(4) Assuming the data is found in the cache, all well and good, but if it
isn't, the cache driver will have to create some files in the cache.
  
Now, if the cache driver just went ahead and created the files, they
could end up with their own security contexts being derived from the
random process's security context, thus potentially making it
  impossible
for other processes to access the cache.
 
 Yes, and the SELinux semantics for what label to give a file don't
 help much, either. The problem with the act_as interfaces is that
 I wouldn't expect them to be any more reliable than the old access()
 system call, which never really gave you a helpful answer.
  
So the file-creation part of the security context must also be
overridden temporarily, assuming that whatever LSM is in force has 
  such
a concept.
 
 Ideally you want to be running in the right context to create the
 new file so that no one can use it and then label it correctly
 and make it available.
 
  Part of the problem is that the VFS does not pass around the security 
  context
  as which the VFS routines act, but rather gets them from the task_struct. 
 
 That's by design. 
 
  For
  the most part, this is entirely sufficient, but in the cache driver case,
  it's
  a problem.
 
 The cache driver is a unique case with an unusual function. It's pretty
 obvious that the kernel architecture, the VFS architecture, LSM, SELinux,
 NFS and pretty much everyone else has given no thought whatever to the
 implications of their designs on file system cacheing. For all concerned,
 I'll say sorry 'bout that.

Parts of it are unique, but some of the same issues crop up in nfs - we
will need a way there as well for nfsd to assume the client process'
label for permission checking and new file labeling purposes, and the
act_as hook is not fundamentally different than what nfsd does today
with the fsuid/fsguid, just applied to the security label.

-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 00/16] Permit filesystem local caching [try #3]

2007-08-13 Thread Stephen Smalley
On Mon, 2007-08-13 at 11:54 +0100, David Howells wrote:
 Casey Schaufler [EMAIL PROTECTED] wrote:
 
  Sigh. So it's not only SELinux specific, but RedHat specific as well.
 
 *Blink*.  How did you come to that conclusion?
 
 (3) The cache driver wants to access the files in the cache, but it's
 running in the security context of either the aforementioned random
 process, or one of FS-Cache's thread pool.
  
  I think that this is the point you should attack. Control the security
  characteristics of the cache driver properly and you shouldn't need the
  complexity that you're asking to introduce.
 
 How?  The cache driver acts on behalf of someone else.  That someone else has
 one security context, but the cache itself has to have a different context so
 that the cache can be shared.
 
 Furthermore, the cache driver doesn't have a security context per se.
 
 This security context, however, doesn't necessarily give it the
 rights to access what's in the cache, so the driver has to be
 permitted to act as a context appropriate to accessing the cache,
 without changing the overall security context of the random process
 (which would impact things trying to act on that process - kill()
 for example).
  
  Can you run the cache as an independent thread and send it messages
  rather than trying to do things in the context of the calling process?
  I know that that involves extra bookkeeppingg, but it's lots safer.
 
 It introduces more complexity, which I believe you were just arguing against
 above...  It also incurs more kernel threads - which I really really want to
 avoid.
 
 I would rank the complexity and resource overhead of the act-as stuff in LSM
 (or at least in SELinux) as much less than what you're suggesting.
 
 As it stands, the FS-Cache layer has a pool of threads that CacheFiles makes
 use of, but this can't be bound to the security of a specific cache because
 there may be more than one cache of more than one cache driver type.
 
  Yes, and the SELinux semantics for what label to give a file don't
  help much, either. The problem with the act_as interfaces is that
  I wouldn't expect them to be any more reliable than the old access()
  system call, which never really gave you a helpful answer.
 
 I don't see how act_as compares to access().
 
  Ideally you want to be running in the right context to create the
  new file so that no one can use it and then label it correctly
  and make it available.
 
 That sounds like it'd be the complexity thing again...
 
   Part of the problem is that the VFS does not pass around the security
   context as which the VFS routines act, but rather gets them from the
   task_struct.
  
  That's by design. 
 
 I suspect that's more by the fact that security wasn't particularly thought
 about when these interfaces were first written.  As with everything in the
 kernel, it might be negotiable.
 
  The cache driver is a unique case with an unusual function. It's pretty
  obvious that the kernel architecture, the VFS architecture, LSM, SELinux,
  NFS and pretty much everyone else has given no thought whatever to the
  implications of their designs on file system cacheing. For all concerned,
  I'll say sorry 'bout that.
 
 Meaning you think I should just give up on this?
 
 How about I reduce the interface I'm proposing to two functions:
 
   (1) int security_act_as(struct task_struct *context)
 
   Temporarily make the current process act as the given task, including,
   for example, for SELinux, the security ID with which this task acts on
   things, and the security ID with which this task creates files.

I don't see how that helps with nfsd assuming the label of a remote
client process.

 
   (2) int security_act_as_self(void);
 
   Restore the context as which we're asking.
 
 This would mean that the task's security context would have to be able to 
 store
 acting security IDs for everything, but I don't think that's too much of a
 stretch resourcewise.

-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 00/16] Permit filesystem local caching [try #3]

2007-08-13 Thread Stephen Smalley
On Mon, 2007-08-13 at 15:51 +0100, David Howells wrote:
 Casey Schaufler [EMAIL PROTECTED] wrote:
 
  I haven't looked into the issues at all and I bet there are plenty,
  maybe in audit and places outside of the security realm, but this
  looks like a clean approach from the LSM interface standpoint. Do
  you want the entire task or just task-security?
 
 It would probably have to be the task struct, lest the security information
 (for which I've no refcount held) went away whilst I was trying to access it.
 
  I could see it either way, but I suspect the task is your best bet. If you
  call security_act_as() twice, then security_act_as_self() do you pop a
  stack, or return to the initial state?
 
 Good point.  I've pondered that.  What I have at the moment partly acts like a
 stack in that I store some of the shifted-out context on the machine stack (in
 struct cachefiles_secctx).  The act-as context should probably be shifted too,
 in addition to the old file-creation SID and the fsuid/fsgid.
 
  How about security_act_as(NULL) returning you to the initial state, and
  dropping security_act_as_self()?
 
 That would be fine.
 
 Actually, to address Stephen Smalley's requirements also, how about making
 things a bit more complex.  Have the following suite of functions:
 
  (1) int security_get_context(struct sec **_context);
 
   This allocates and gives the caller a blob that describes the current
   context of all the LSM module states attached to the current task and
   stores a pointer to it in *_context.
 
  (2) int security_push(struct sec *context, struct sec **_old_context)
 
   This causes all the LSM modules on the current task to switch to a new
   acting state, passing back the old state.  It does not change how
   other tasks do things to this one.
 
  (3) int security_pop(struct sec *context)
 
   This causes all the LSM modules on the current task to switch to a new
   acting state, deleting the old state.  It does not change how
   other tasks do things to this one.
 
  (4) int security_delete_context(struct sec *context)
 
   This deletes a context blob.
 
 The context blob could then be structured very simply.  Give each loaded LSM
 module an integer index as it is registered.  Having a limit to the number of
 LSM modules would make things simpler.  The blob would then be an array of
 void pointers, one per LSM module, indexed by the integer index for each one.
 It you don't have a limit on the number of LSM modules, you'd also need a
 count of slots in the blob.
 
 Any LSM module that wanted to implement the above three functions would fill
 in or otherwise use the slot that belongs to it.  Otherwise the slot would
 just be left NULL.
 
 For example:
 
   context ---+++-+
   | SLOT 0 |---| SELINUX |
   ++  +++-+
   | SLOT 1 |-| THINGY |
   ++  ++
   | ...|
   ++ +---+
   | SLOT N || AUDIT |
   ++ +---+
 
 For Stephen and NFS, he could then generate a context from NFS which nfsd
 could then put in place.  Perhaps any unfilled slot would be ignored by the
 LSM module to which it belonged.

Seems like over-design - we don't need to support LSM stacking, and we
don't need to support pushing/popping more than one level of context.

What was the objection again to the original interface, aside from
replacing u32 secids with void* security blobs?

-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 00/16] Permit filesystem local caching [try #3]

2007-08-14 Thread Stephen Smalley
On Tue, 2007-08-14 at 08:53 -0700, Casey Schaufler wrote:
 --- David Howells [EMAIL PROTECTED] wrote:
 
  Casey Schaufler [EMAIL PROTECTED] wrote:
  
   With Smack you can leave the label alone, raise CAP_MAC_OVERRIDE,
   do your business of setting the label correctly, and then drop
   the capability. No new hooks required.
  
  That sounds like a contradiction.  How can you both leave it alone and set
  it?
 
 Whoops, sorry. You leave the process label alone and explicitly
 set the file label using the xattr interfaces.

xattr interfaces don't help with the initial labeling of the file when
it is created.

-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 00/16] Permit filesystem local caching [try #3]

2007-08-14 Thread Stephen Smalley
On Mon, 2007-08-13 at 14:44 -0700, Casey Schaufler wrote:
 --- David Howells [EMAIL PROTECTED] wrote:
 
  Casey Schaufler [EMAIL PROTECTED] wrote:
  
   The specification of your push interface that the push operation
   not affect how others access the process is OK for SELinux, but
   not for any other MAC scheme that I've dealt with, and I think
   that's most of them. Nuts. Smack, for example, uses exactly one
   label on the process for all purposes.
  
  It's a fairly important concept.  The victimisation security context on a
  process must not change, even if the kernel overrides the security context
  that that process acts as so that it can transparently do work on its 
  behalf.
  
  IMO, the right way to do this is to pass the security context directly to
  vfs_mkdir() and co.
  
   Are you concerned about accesses other than signals? Signals
   could be staitforward to deal with in a pushed situation, but
   I'd hesitate to say that the solution would generalize without
   additional thought.
  
  There's also /proc and ptrace() for example.  ps -z must not show the
  overridden state.
  
(5) int security_xfrm_to_kernel_context(void *from, void **_to);
 
 Woof. What are you transforming from? 

In CacheFiles case, the cachefilesd daemon's security label into the
  label
the cache driver acts as on behalf of other processes.
   
   I'm not sure I understand what this is doing.
  
  CacheFiles consists of two parts: the kernel module which creates things in
  the cache and does accesses into the cache on behalf of processes that 
  access
  cached filesystems, and the userspace daemon that builds cull tables and
  deletes things.
  
  The reason there are two security labels is that the daemon's label gives it
  just enough rights to be able to do its job.  More or less all it can do is
  lookup, opendir, readdir, stat, rmdir, unlink and open the chardev for
  talking
  to the kernel module.  This means that the daemon can't, for example, be 
  made
  to read or modify cache storage objects.
  
  Thus means, however, that the daemon's label isn't sufficient for the kernel
  module to do its job.  But since there's no way for the kernel module to
  directly get a label (and indeed it doesn't know the label it needs), a
  transformation has to be applied that turns the process label used by the
  daemon into a process label that the kernel, and only the kernel, can use.
  
  The kernel's label gives it, amongst other things, the additional rights to
  do
  mkdir, creat, open, read, write, setxattr, getxattr, rename - things the
  daemon isn't allowed to do.
 
 With Smack you can leave the label alone, raise CAP_MAC_OVERRIDE,
 do your business of setting the label correctly, and then drop
 the capability. No new hooks required.

Except that CAP_MAC_OVERRIDE doesn't exist upstream, and if it did, it
would represent Smack-specific logic in the core kernel (when you're
complaining about SELinux-specific logic there).  So even that would
have to be encapsulated within a hook.

-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/1] file capabilities: clear fcaps on inode change (v2)

2007-08-07 Thread Stephen Smalley
On Mon, 2007-08-06 at 13:52 -0500, Serge E. Hallyn wrote:
 From 1376764cbb54243f088cf00c39000c4f4418f461 Mon Sep 17 00:00:00 2001
 From: Serge E. Hallyn [EMAIL PROTECTED]
 Date: Mon, 6 Aug 2007 14:20:06 -0400
 Subject: [PATCH 1/1] file capabilities: clear fcaps on inode change (v2)
 
 When a file with posix capabilities is overwritten, the
 file capabilities, like a setuid bit, should be removed.
 
 This patch introduces security_inode_killpriv().  This is
 currently only defined for capability, and is called when
 an inode is changed to inform the security module that
 it may want to clear out any privilege attached to that inode.
 The capability module checks whether any file capabilities
 are defined for the inode, and, if so, clears them.
 
 Signed-off-by: Serge E. Hallyn [EMAIL PROTECTED]
 ---
  fs/attr.c|7 +++
  fs/nfsd/vfs.c|4 ++--
  fs/open.c|3 ++-
  fs/splice.c  |4 
  include/linux/fs.h   |1 +
  include/linux/security.h |   18 ++
  mm/filemap.c |5 +
  security/capability.c|1 +
  security/commoncap.c |   27 +++
  security/dummy.c |6 ++
  security/security.c  |5 +
  11 files changed, 78 insertions(+), 3 deletions(-)
 

 diff --git a/security/capability.c b/security/capability.c
 index dc2b66c..e23864e 100644
 --- a/security/capability.c
 +++ b/security/capability.c
 @@ -37,6 +37,7 @@ static struct security_operations capability_ops = {
  
   .inode_setxattr =   cap_inode_setxattr,
   .inode_removexattr =cap_inode_removexattr,
 + .inode_removexattr =cap_inode_killpriv,

s/inode_removexattr/inode_killpriv/

Also, doesn't SELinux then need to define a corresponding hook function
to call the secondary module?  Otherwise, it will fall back to the dummy
implementation and stacking selinux + capabilities with file caps won't
yield the right behavior.

-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 11/14] CacheFiles: Permit an inode's security ID to be obtained [try #2]

2007-08-09 Thread Stephen Smalley
On Thu, 2007-08-09 at 10:07 -0700, Casey Schaufler wrote:
 --- David Howells [EMAIL PROTECTED] wrote:
 
  Permit an inode's security ID to be obtained by the CacheFiles module.  This
  is
  then used as the SID with which files and directories will be created in the
  cache.
 
 This is SELinux specific functionality. It should not be an LSM
 interface. 

Odd, you proposed exactly the same hook (aside from naming convention
and secid as argument vs. as retval) in recent postings on linux-audit
and selinux list for use by the audit system.

-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/6] SELinux: change Kconfig to use select instead of depends

2007-10-10 Thread Stephen Smalley
On Tue, 2007-10-09 at 16:28 -0700, Randy Dunlap wrote:
 On Wed, 10 Oct 2007 09:19:55 +1000 (EST) James Morris wrote:
 
  From: Eric Paris [EMAIL PROTECTED]
  
  Changes the security/selinux/Kconfig to use select instead of depends
  for most of the SELinux requirements.  This allows the SELinux option to
  show up when people do a make config without already knowing they had to
  enable audit and other non-obvious choices.  Added a depends on SECURITY
  (which previously existed through SECURITY_NETWORK) so that SELinux
  would not always show up, but would be easy and intuitive to find.
  
  Signed-off-by: Eric Paris [EMAIL PROTECTED]
  Acked-by: Stephen Smalley [EMAIL PROTECTED]
  Signed-off-by: James Morris [EMAIL PROTECTED]
  ---
   security/selinux/Kconfig |7 ++-
   1 files changed, 6 insertions(+), 1 deletions(-)
  
  diff --git a/security/selinux/Kconfig b/security/selinux/Kconfig
  index b32a459..40b97e6 100644
  --- a/security/selinux/Kconfig
  +++ b/security/selinux/Kconfig
  @@ -1,6 +1,10 @@
   config SECURITY_SELINUX
  bool NSA SELinux Support
  -   depends on SECURITY_NETWORK  AUDIT  NET  INET
  +   depends on SECURITY
  +   select SECURITY_NETWORK
  +   select AUDIT
  +   select NET
  +   select INET
  select NETWORK_SECMARK
  default n
  help
 
 I doth protest.  Enabling the entire NET subsystem thru a hidden
 select is awful.  Select should be used (sparingly) to enable
 library code only.  If someone wants NET enabled, they should
 enable it overtly, not covertly.

Does that apply to all the options, or only to NET (e.g. is it ok to
select AUDIT)?  I thought that this patch came out of earlier
discussions about proper use of select vs. depends.  It may have gone
too far, but I'm not sure it should be discarded entirely.

 
 
  @@ -9,6 +13,7 @@ config SECURITY_SELINUX
You can obtain the policy compiler (checkpolicy), the utility for
labeling filesystems (setfiles), and an example policy configuration
from http://www.nsa.gov/selinux/.
  +
If you are unsure how to answer this question, answer N.
   
   config SECURITY_SELINUX_BOOTPARAM
 
 
 
 ---
 ~Randy
-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Version 3 (2.6.23-rc8) Smack: Simplified Mandatory Access Control Kernel

2007-10-10 Thread Stephen Smalley
On Wed, 2007-10-10 at 07:48 -0600, Eric W. Biederman wrote:
 Alan Cox [EMAIL PROTECTED] writes:
 
  My very practical question:  How do I run selinux in one container,
  and SMACK in another?
 
  In the LSM model you don't because you could have the same container
  objects visible in different contains at the same time and subject to
  different LSMs. What does it mean to pass an SELinux protected object
  over an AppArmour protected unix domain socket into a SMACK protected
  container ?
 
 You raise a good point.  My intuitive definition would go something like
 this.  In the initial LSM space we would have whatever is the primary
 LSM and it would always be invoked about everything.   However it
 would view a single container (no matter what user in that container)
 as having a single set of permissions.  Then the LSM in the container
 be asked to further validate accesses, but it would distinguish
 between users in the container.

SELinux internally has a notion of a type hierarchy, where a type is
limited to a subset of its parent's permissions, and one can then
delegate the ability to manage sub-types via a policy daemon.  But this
is all handled in userspace; the kernel doesn't care about it.

Ditto for the modular policy support - that's a userspace construct that
is ultimately turned into a single coherent policy for the kernel to
enforce.

 At this point it looks like if I am going to be effective at doing
 anything I am going to need to step back watch SMACK get merged and
 then really look at what the LSM modules are implementing.  Then
 I can refactor the whole mess and move additional functionality into
 the LSM to help me achieve other things.
 
  Really its the same problem as I'd like to use different file permission
  systems on different process identifiers and it would be very hard to
  get right simply because objects can pass between two different security
  models.
 
 Yep.  Although the isolation of a container with a completely
 different set of namespaces is tight enough that except for people
 debugging a container from processes in the container from outside the
 container object exchange essentially doesn't happen.
 
 You do raise a very good question here.  Does an LSM implement a
 different file permission system?  Or does an LSM implement a firewall
 between processes?
 
 Certainly selinux seems too programmable to be considered just a
 different file permission system.  

A LSM implements a security model, where that model may encompass all
processes and objects.  SELinux (and Smack) in particular implement
mandatory access control and thus need to enforce consistent policy over
all processes and objects based on their security labels.

-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC] [PATCH 2/2] capabilities: implement 64-bit capabilities

2007-10-16 Thread Stephen Smalley
,
   int size)
  {
   __u32 magic_etc;
  
 - if (size != XATTR_CAPS_SZ)
 + if (size != XATTR_CAPS_SZ_1  size != XATTR_CAPS_SZ_2)
   return -EINVAL;
  
 - magic_etc = le32_to_cpu(caps-magic_etc);
 + magic_etc = le32_to_cpu(caps-v1.magic_etc);
  
   switch ((magic_etc  VFS_CAP_REVISION_MASK)) {
 - case VFS_CAP_REVISION:
 + case VFS_CAP_REVISION_1:
 + if (size != XATTR_CAPS_SZ_1)
 + return -EINVAL;
 + if (magic_etc  VFS_CAP_FLAGS_EFFECTIVE)
 + bprm-cap_effective = true;
 + else
 + bprm-cap_effective = false;
 + bprm-cap_permitted = to_cap_t(
 + (__u64) le32_to_cpu(caps-v1.permitted) );
 + bprm-cap_inheritable = to_cap_t(
 + (__u64) le32_to_cpu(caps-v1.inheritable) );
 + return 0;
 + case VFS_CAP_REVISION_2:
 + if (size != XATTR_CAPS_SZ_2)
 + return -EINVAL;
   if (magic_etc  VFS_CAP_FLAGS_EFFECTIVE)
   bprm-cap_effective = true;
   else
   bprm-cap_effective = false;
 - bprm-cap_permitted = to_cap_t( le32_to_cpu(caps-permitted) );
 - bprm-cap_inheritable = to_cap_t( 
 le32_to_cpu(caps-inheritable) );
 + bprm-cap_permitted = to_cap_t(
 + le64_to_cpu(caps-v2.permitted) );
 + bprm-cap_inheritable = to_cap_t(
 + le64_to_cpu(caps-v2.inheritable) );
   return 0;
   default:
   return -EINVAL;
 @@ -220,7 +241,7 @@ static int get_file_caps(struct linux_binprm *bprm)
  {
   struct dentry *dentry;
   int rc = 0;
 - struct vfs_cap_data incaps;
 + union vfs_cap_union incaps;
   struct inode *inode;
  
   if (bprm-file-f_vfsmnt-mnt_flags  MNT_NOSUID) {
 @@ -235,7 +256,7 @@ static int get_file_caps(struct linux_binprm *bprm)
  
   rc = inode-i_op-getxattr(dentry, XATTR_NAME_CAPS, NULL, 0);
   if (rc  0) {
 - if (rc == XATTR_CAPS_SZ)
 + if (rc == XATTR_CAPS_SZ_2 || rc == XATTR_CAPS_SZ_1)
   rc = inode-i_op-getxattr(dentry, XATTR_NAME_CAPS,
   incaps, XATTR_CAPS_SZ);
   else
-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] SELinux: cleanup ipc_has_perm

2005-04-12 Thread Stephen Smalley
This patch removes the sclass argument from ipc_has_perm in the
SELinux module, as it can be obtained from the ipc security structure.
The use of a separate argument was a legacy of the older precondition
function handling in SELinux and is obsolete.  Please apply.

Signed-off-by:  Stephen Smalley [EMAIL PROTECTED]
Signed-off-by:  James Morris [EMAIL PROTECTED]

---

 security/selinux/hooks.c |   21 -
 1 files changed, 8 insertions(+), 13 deletions(-)

= security/selinux/hooks.c 1.95 vs edited =
--- 1.95/security/selinux/hooks.c   2005-04-01 16:30:16 -05:00
+++ edited/security/selinux/hooks.c 2005-04-08 10:37:42 -04:00
@@ -3666,7 +3666,7 @@ static void msg_msg_free_security(struct
 }
 
 static int ipc_has_perm(struct kern_ipc_perm *ipc_perms,
-   u16 sclass, u32 perms)
+   u32 perms)
 {
struct task_security_struct *tsec;
struct ipc_security_struct *isec;
@@ -3678,7 +3678,7 @@ static int ipc_has_perm(struct kern_ipc_
AVC_AUDIT_DATA_INIT(ad, IPC);
ad.u.ipc_id = ipc_perms-key;
 
-   return avc_has_perm(tsec-sid, isec-sid, sclass, perms, ad);
+   return avc_has_perm(tsec-sid, isec-sid, isec-sclass, perms, ad);
 }
 
 static int selinux_msg_msg_alloc_security(struct msg_msg *msg)
@@ -3763,7 +3763,7 @@ static int selinux_msg_queue_msgctl(stru
return 0;
}
 
-   err = ipc_has_perm(msq-q_perm, SECCLASS_MSGQ, perms);
+   err = ipc_has_perm(msq-q_perm, perms);
return err;
 }
 
@@ -3915,7 +3915,7 @@ static int selinux_shm_shmctl(struct shm
return 0;
}
 
-   err = ipc_has_perm(shp-shm_perm, SECCLASS_SHM, perms);
+   err = ipc_has_perm(shp-shm_perm, perms);
return err;
 }
 
@@ -3934,7 +3934,7 @@ static int selinux_shm_shmat(struct shmi
else
perms = SHM__READ | SHM__WRITE;
 
-   return ipc_has_perm(shp-shm_perm, SECCLASS_SHM, perms);
+   return ipc_has_perm(shp-shm_perm, perms);
 }
 
 /* Semaphore security operations */
@@ -4023,7 +4023,7 @@ static int selinux_sem_semctl(struct sem
return 0;
}
 
-   err = ipc_has_perm(sma-sem_perm, SECCLASS_SEM, perms);
+   err = ipc_has_perm(sma-sem_perm, perms);
return err;
 }
 
@@ -4037,18 +4037,13 @@ static int selinux_sem_semop(struct sem_
else
perms = SEM__READ;
 
-   return ipc_has_perm(sma-sem_perm, SECCLASS_SEM, perms);
+   return ipc_has_perm(sma-sem_perm, perms);
 }
 
 static int selinux_ipc_permission(struct kern_ipc_perm *ipcp, short flag)
 {
-   struct ipc_security_struct *isec = ipcp-security;
-   u16 sclass = SECCLASS_IPC;
u32 av = 0;
 
-   if (isec  isec-magic == SELINUX_MAGIC)
-   sclass = isec-sclass;
-
av = 0;
if (flag  S_IRUGO)
av |= IPC__UNIX_READ;
@@ -4058,7 +4053,7 @@ static int selinux_ipc_permission(struct
if (av == 0)
return 0;
 
-   return ipc_has_perm(ipcp, sclass, av);
+   return ipc_has_perm(ipcp, av);
 }
 
 /* module stacking operations */

-- 
Stephen Smalley [EMAIL PROTECTED]
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] SELinux: fix deadlock on dcache lock

2005-04-15 Thread Stephen Smalley
This patch fixes a deadlock on the dcache lock detected during testing
at IBM by moving the logging of the current executable information
from the SELinux avc_audit function to audit_log_exit (via an
audit_log_task_info helper) for processing upon syscall exit.  For
consistency, the patch also removes the logging of other task-related
information from avc_audit, deferring handling to audit_log_exit
instead.  This allows simplification of the avc_audit code, allows the
exe information to be obtained more reliably, always includes the comm
information (useful for scripts), and avoids including bogus task
information for checks performed from irq or softirq.  Please apply.

Signed-off-by:  Stephen Smalley [EMAIL PROTECTED]
Signed-off-by:  James Morris [EMAIL PROTECTED]

--

 kernel/auditsc.c   |   28 
 security/selinux/avc.c |   34 --
 2 files changed, 28 insertions(+), 34 deletions(-)

= kernel/auditsc.c 1.10 vs edited =
--- 1.10/kernel/auditsc.c   2005-03-17 11:33:20 -05:00
+++ edited/kernel/auditsc.c 2005-04-15 09:22:15 -04:00
@@ -610,6 +610,33 @@
printk(KERN_ERR audit: freed %d contexts\n, count);
 }
 
+static void audit_log_task_info(struct audit_buffer *ab)
+{
+   char name[sizeof(current-comm)];
+   struct mm_struct *mm = current-mm;
+   struct vm_area_struct *vma;
+
+   get_task_comm(name, current);
+   audit_log_format(ab,  comm=%s, name);
+
+   if (!mm)
+   return;
+
+   down_read(mm-mmap_sem);
+   vma = mm-mmap;
+   while (vma) {
+   if ((vma-vm_flags  VM_EXECUTABLE) 
+   vma-vm_file) {
+   audit_log_d_path(ab, exe=,
+vma-vm_file-f_dentry,
+vma-vm_file-f_vfsmnt);
+   break;
+   }
+   vma = vma-vm_next;
+   }
+   up_read(mm-mmap_sem);
+}
+
 static void audit_log_exit(struct audit_context *context)
 {
int i;
@@ -639,6 +666,7 @@
  context-gid,
  context-euid, context-suid, context-fsuid,
  context-egid, context-sgid, context-fsgid);
+   audit_log_task_info(ab);
audit_log_end(ab);
while (context-aux) {
struct audit_aux_data *aux;
= security/selinux/avc.c 1.23 vs edited =
--- 1.23/security/selinux/avc.c 2005-03-28 17:21:18 -05:00
+++ edited/security/selinux/avc.c   2005-04-15 09:22:16 -04:00
@@ -532,7 +532,6 @@
u16 tclass, u32 requested,
struct av_decision *avd, int result, struct avc_audit_data *a)
 {
-   struct task_struct *tsk = current;
struct inode *inode = NULL;
u32 denied, audited;
struct audit_buffer *ab;
@@ -556,39 +555,6 @@
audit_log_format(ab, avc:  %s , denied ? denied : granted);
avc_dump_av(ab, tclass,audited);
audit_log_format(ab,  for );
-   if (a  a-tsk)
-   tsk = a-tsk;
-   if (tsk  tsk-pid) {
-   struct mm_struct *mm;
-   struct vm_area_struct *vma;
-   audit_log_format(ab,  pid=%d, tsk-pid);
-   if (tsk == current)
-   mm = current-mm;
-   else
-   mm = get_task_mm(tsk);
-   if (mm) {
-   if (down_read_trylock(mm-mmap_sem)) {
-   vma = mm-mmap;
-   while (vma) {
-   if ((vma-vm_flags  VM_EXECUTABLE) 
-   vma-vm_file) {
-   audit_log_d_path(ab, exe=,
-   vma-vm_file-f_dentry,
-   vma-vm_file-f_vfsmnt);
-   break;
-   }
-   vma = vma-vm_next;
-   }
-   up_read(mm-mmap_sem);
-   } else {
-   audit_log_format(ab,  comm=%s, tsk-comm);
-   }
-   if (tsk != current)
-   mmput(mm);
-   } else {
-   audit_log_format(ab,  comm=%s, tsk-comm);
-   }
-   }
if (a) {
switch (a-type) {
case AVC_AUDIT_DATA_IPC:


-- 
Stephen Smalley [EMAIL PROTECTED]
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 0/7] procfs privacy

2005-04-18 Thread Stephen Smalley
On Mon, 2005-04-18 at 22:18 +0200, Lorenzo Hernndez Garca-Hierro
wrote:
 For this purpose I (re)submitted a patch originally made by Serge E.
 Hallyn that adds a hook in order to catch task lookups, thus, providing
 an easy way to handle and determine when a task can lookup'ed.
 
 It's at:
 http://pearls.tuxedo-es.org/patches/lsm/lsm-task_lookup-hook.patch
 
 vSecurity currently provides support for it (optional).
 
 SELinux policy can handle in a much more fine-grained these
 restrictions, just that it's still something that not all people can
 deploy without some special effort and tweak up (if their system
 doesn't provide support for it, of course, currently Red Hat has done a
 great job in that terms).

To be precise, SELinux assigns security labels to /proc inodes
(/proc/pid inodes are labeled based on the associated task label, and
other /proc inodes are labeled based on the policy configuration), and
controls access based on the policy.  It can e.g. prevent a process in
one security domain from accessing anything under /proc/pid for a
process in a different domain, but not from seeing the top-level entry
in /proc itself (as it doesn't do any kind of directory filtering).

-- 
Stephen Smalley [EMAIL PROTECTED]
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] Real-Time Preemption, -RT-2.6.11-rc3-V0.7.38-01

2005-02-09 Thread Stephen Smalley
On Tue, 2005-02-08 at 16:58, William Weston wrote:
 Hi Ingo,
 
 Great work on the -RT kernel!  Here's a status report from my Athlon box
 w/ kernel -RT-2.6.11-rc3-V0.7.38-03, realtime-lsm-0.8.5, jack-0.99.48, 
 alsa-1.0.8, and latencytest-0.5.5:
snip
 A couple BUGs are being logged (see below), but without any ill effect
 other than taking up space on my /var.
snip
 Network interface (via rhine) startup triggers these two BUGs:
 
 BUG: sleeping function called from invalid context ksoftirqd/0(2) at 
 kernel/rt.c:1448
 in_atomic():1 [0001], irqs_disabled():0
  [c0103e77] dump_stack+0x17/0x20 (12)
  [c0119f89] __might_sleep+0xd9/0xf0 (40)
  [c0134816] __spin_lock+0x36/0x50 (24)
  [c0147914] kmem_cache_alloc+0x34/0x120 (44)
  [c01d3143] sel_netif_lookup+0x63/0x150 (28)
  [c01d32cd] sel_netif_sids+0x2d/0xb0 (28)
  [c01d01bc] selinux_socket_sock_rcv_skb+0xac/0x230 (144)

I'm not sure I understand, as sel_netif_lookup passes GFP_ATOMIC to
kmalloc.

  [c02fd248] udp_queue_rcv_skb+0xb8/0x280 (28)
  [c02fd8e2] udp_rcv+0x192/0x3e0 (100)
  [c02dc224] ip_local_deliver+0x64/0x1c0 (32)
  [c02dc595] ip_rcv+0x215/0x3f0 (56)
  [c02c201c] netif_receive_skb+0x12c/0x160 (40)
  [c02c20ce] process_backlog+0x7e/0x110 (32)
  [c02c21d2] net_rx_action+0x72/0x130 (24)
  [c0122428] ___do_softirq+0x48/0xd0 (40)
  [c012254b] _do_softirq+0x1b/0x30 (8)
  [c0122920] ksoftirqd+0xa0/0xf0 (28)
  [c01312fb] kthread+0x8b/0xc0 (36)
  [c01012f5] kernel_thread_helper+0x5/0x10 (537116692)
 ---
 | preempt count: 0002 ]
 | 2-level deep critical section nesting:
 
 .. [c013dd3f]  __do_IRQ+0xef/0x180
 .[c0105306] ..   ( = do_IRQ+0x56/0xa0)
 .. [c0135240]  print_traces+0x10/0x40
 .[c0103e77] ..   ( = dump_stack+0x17/0x20)

-- 
Stephen Smalley [EMAIL PROTECTED]
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Thoughts on the No Linux Security Modules framework old claims

2005-02-16 Thread Stephen Smalley
On Wed, 2005-02-16 at 08:29, Lorenzo Hernndez Garca-Hierro wrote:
 Yes, there are many cases that apply to such scenario and context, this
 may be worth to work on, but think it's main shortcoming is that it cuts
 performance and adds further overlapping to the DAC checks, that should
 be the first ones being called (as most times they do) and then apply
 the LSM basis, so, post-processing will be only required if the DAC
 checks get in override or passed, without adding too-much overhead to
 the current behavior.
 
 So, I just agree partially, but yes, maybe modifying the DAC checks
 themselves and add what-ever-else helper function to handle by-default
 auditing in certain operations could be interesting.

Audit is being handled by a separate audit framework, not by LSM.  There
is already support in the Linux 2.6 kernel for auditing at syscall exit
(thereby guaranteeing that you capture the final return value in all
cases), with the ability of an LSM to enable such auditing for a
particular event from its hook functions.  Further, there is ongoing
work (see the linux-audit mailing list) for a set of audit-related hooks
that will allow auditing based on object identity and the requested mode
separate from any particular LSM.

-- 
Stephen Smalley [EMAIL PROTECTED]
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] securityfs

2005-07-06 Thread Stephen Smalley
On Wed, 2005-07-06 at 02:52 -0400, James Morris wrote:
 On Sun, 3 Jul 2005, Greg KH wrote:
 
  Good idea.  Here's a patch to do just that (compile tested only...)
  
  Comments?
 
 Looks promising so far.
 
 I'm currently porting selinuxfs funtionality to securityfs, although I'm
 not sure if we'll be ok during early initialization.  selinuxfs is
 currently kern_mounted via an initcall.  We may need an initcall
 kern_mount() of securityfs before SELinux kicks in.
 
 Stephen: opinions on this?

The reason for creating a kernel mount of selinuxfs at that point is so
that the selinuxfs_mount vfsmount and selinux_null dentry are available
for flush_unauthorized_files to use.

 Otherwise, it looks like it'll allow SELinux to drop some code.  Generally 
 it will mean that other LSM components won't have to create their own 
 filesystems, and their subdirectories will be hanging off /security (or 
 wherever selinuxfs is mounted), rather than scattered across /
 
 Some of the SELinux code may be useful as part of securityfs later, as 
 well.

We need to be able to assign specific security labels to specific inodes
in selinuxfs, particularly for the booleans, but ideally for any of
them.

Userspace compatibility is obviously a concern for such a change.
libselinux determines where selinuxfs is mounted during library
initialization by checking /proc/mounts for selinuxfs, and rc.sysinit
does likewise.  /sbin/init performs the initial mount of selinuxfs prior
to initial policy load.  Further, the existence of selinuxfs
in /proc/filesystems is used as a test of whether SELinux was enabled in
the kernel (e.g. is_selinux_enabled in libselinux).

I'm not sure such a change is worthwhile for SELinux; large amount of
disruption for little real gain.

-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] securityfs

2005-07-06 Thread Stephen Smalley
On Wed, 2005-07-06 at 11:35 -0400, James Morris wrote:
 When exactly is this needed?  The securityfs mountpoint will be available 
 via a core_initcall, after which we can initialize the selinux subtree.

As long as it occurs prior to initial policy load, so that should be
fine.

 With securityfs, we'd have /sys/kernel/security/selinux configured during 
 kernel initialization.

It still has to be mounted by userspace, right?  So /sbin/init still
needs to know to mount securityfs, and where the selinux nodes live
under it, so you are still talking about changing it (and libselinux and
rc.sysinit), and risking compatibility breakage for existing systems
when they update their kernels.

 Could be a simple change to look for the presence of
 /sys/kernel/security/selinux

Slightly different.  That would correspond to checking for the presence
of selinuxfs in /proc/mounts (i.e. has it been mounted), vs. the current
check of selinuxfs in /proc/filesystems (i.e. has it been registered).
The latter allows testing whether SELinux is enabled at all in the
kernel, regardless of whether selinuxfs has been mounted in the process'
namespace.  In practice, the difference likely only matters for init and
you can deal with distinction there.  But again, this requires code
change to libselinux, /sbin/init, and rc.sysinit at least, with
coordinated update with the kernel.  Certainly possible, but experience
suggests it isn't likely to go smoothly.

 I think it should reduce and simplify the SELinux kernel code, with less
 filesystems in the kernel, consolidating several potential projects into
 the same security filesystem.

If there are several such projects in the first place...

-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 5/12] lsm stacking v0.2: actual stacker module

2005-07-11 Thread Stephen Smalley
On Thu, 2005-06-30 at 14:50 -0500, [EMAIL PROTECTED] wrote:
 Adds the actual stacker LSM.
snip
 +static int stacker_inode_getsecurity(struct inode *inode, const char *name, 
 void *buffer, size_t size)
 +{
 + 
 RETURN_ERROR_IF_ANY_ERROR(inode_getsecurity,inode_getsecurity(inode,name,buffer,size));
 +}
 +
 +static int stacker_inode_setsecurity(struct inode *inode, const char *name, 
 const void *value, size_t size, int flags)
 +{
 + 
 RETURN_ERROR_IF_ANY_ERROR(inode_setsecurity,inode_setsecurity(inode,name,value,size,flags));
 +}
 +
 +static int stacker_inode_listsecurity(struct inode *inode, char *buffer, 
 size_t buffer_size)
 +{
 + 
 RETURN_ERROR_IF_ANY_ERROR(inode_listsecurity,inode_listsecurity(inode,buffer, 
 buffer_size));
 +}

These hooks pose a similar problem for stacking as with the
[gs]etprocattr hooks, although [gs]etsecurity have the benefit of
already taking a distinguishing name suffix (the part after the
security. prefix).  Note also that inode_getsecurity returns the number
of bytes used/required on success.

The proposed inode_init_security hook will likewise have an issue for
stacking.

-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 5/12] lsm stacking v0.2: actual stacker module

2005-07-11 Thread Stephen Smalley
On Mon, 2005-07-11 at 12:51 -0500, [EMAIL PROTECTED] wrote:
 I can imagine a few ways of fixing this:
 
   1.  We simply expect that only one module use xattrs.  This
   is probably unacceptable, as we will want both EVM and selinux
   to store xattrs.

Note that these particular hooks are only used for filesystems like
devpts and tmpfs where there is no underlying storage for the security
xattrs but we still need a way to [gs]et the incore inode security label
from userspace.

   2.  A module registers an xattr name when it registers
   itself.  Then only the registered module is consulted on one of
   these calls.  If no module is registered, all are consulted as
   they are now.

SELinux already checks the name suffix in inode_getsecurity and
inode_setsecurity, and returns -EOPNOTSUPP if it isn't selinux.  Hence,
stacker could just iterate through the modules until it gets a result
other than -EOPNOTSUPP, relying on the modules to check the name.

listsecurity is different, as it is supposed to yield the list of
attribute names concatenated together, but that shouldn't be difficult
for stacker to construct, similar to your getprocattr logic but without
the need to add tags.

   This prevents a module like capability from deciding
   based on its own credentials whether another module's hook
   should be called.  Is that a good or bad thing?

These hooks aren't supposed to be doing permission checking; that is
handled by the separate security_inode_*xattr hooks.  They are just for
getting/setting the incore inode security label.

   This might have the added bonus of obviating the need
   for a separate cap_stack module.

I don't think so - different hooks are involved (inode_setxattr vs.
inode_setsecurity).

-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [2.6 patch] selinux: cleanups

2005-03-14 Thread Stephen Smalley
On Sun, 2005-03-13 at 04:01 +0100, Adrian Bunk wrote:
 The patch below contains the following possible cleanups:
 - make needlessly global code static
 - remove the following unused global functions:
   - avc.c: avc_ss_grant
   - avc.c: avc_ss_try_revoke
   - avc.c: avc_ss_revoke
   - avc.c: avc_ss_set_auditallow
   - avc.c: avc_ss_set_auditdeny
   - ss/avtab.c: avtab_map
   - ss/ebitmap.c: ebitmap_or
   - ss/hashtab.c: hashtab_remove
   - ss/hashtab.c: hashtab_replace
   - ss/hashtab.c: hashtab_map_remove_on_error
   - ss/sidtab.c: sidtab_remove
 - remove the following unused static functions:
   - avc.c: avc_update_cache
   - avc.c: avc_control
 
 
 Please review and comment on which of these changes are correct and 
 which conflict with pending patches for in-kernel users of the functions 
 affected.
 
 
 diffstat output:
  security/selinux/avc.c  |  174 
  security/selinux/hooks.c|   40 +++---
  security/selinux/include/avc.h  |7 -
  security/selinux/include/avc_ss.h   |   13 --
  security/selinux/include/objsec.h   |2 
  security/selinux/include/security.h |3 
  security/selinux/selinuxfs.c|4 
  security/selinux/ss/avtab.c |   27 
  security/selinux/ss/avtab.h |6 
  security/selinux/ss/conditional.c   |2 
  security/selinux/ss/ebitmap.c   |   43 --
  security/selinux/ss/ebitmap.h   |1 
  security/selinux/ss/hashtab.c   |  113 --
  security/selinux/ss/hashtab.h   |   38 --
  security/selinux/ss/mls.c   |2 
  security/selinux/ss/policydb.c  |   10 -
  security/selinux/ss/policydb.h  |3 
  security/selinux/ss/services.c  |   23 ---
  security/selinux/ss/sidtab.c|   36 -
  19 files changed, 34 insertions(+), 513 deletions(-)
 
 
 Signed-off-by: Adrian Bunk [EMAIL PROTECTED]

Looks fine to me (although your diffstat output is stale).  Re-diff
against 2.6.11-mm3 is below, feel free to send along to Andrew Morton.

Acked-by: Stephen Smalley [EMAIL PROTECTED]

 security/selinux/avc.c|  174 --
 security/selinux/hooks.c  |   40 
 security/selinux/include/avc.h|7 -
 security/selinux/include/avc_ss.h |   13 --
 security/selinux/include/objsec.h |2 
 security/selinux/selinuxfs.c  |4 
 security/selinux/ss/avtab.c   |   29 --
 security/selinux/ss/avtab.h   |6 -
 security/selinux/ss/conditional.c |2 
 security/selinux/ss/ebitmap.c |   43 -
 security/selinux/ss/ebitmap.h |1 
 security/selinux/ss/hashtab.c |  113 
 security/selinux/ss/hashtab.h |   38 
 security/selinux/ss/policydb.c|   10 +-
 security/selinux/ss/policydb.h|3 
 security/selinux/ss/services.c|   18 +--
 security/selinux/ss/services.h|6 -
 security/selinux/ss/sidtab.c  |   36 ---
 18 files changed, 42 insertions(+), 503 deletions(-)

diff -X /home/sds/dontdiff -rup linux-2.6.11-mm3/security/selinux/avc.c 
linux-2.6.11-mm3-adrian/security/selinux/avc.c
--- linux-2.6.11-mm3/security/selinux/avc.c 2005-03-02 02:38:17.0 
-0500
+++ linux-2.6.11-mm3-adrian/security/selinux/avc.c  2005-03-14 
14:08:28.0 -0500
@@ -139,7 +139,7 @@ static inline int avc_hash(u32 ssid, u32
  * @tclass: target security class
  * @av: access vector
  */
-void avc_dump_av(struct audit_buffer *ab, u16 tclass, u32 av)
+static void avc_dump_av(struct audit_buffer *ab, u16 tclass, u32 av)
 {
const char **common_pts = NULL;
u32 common_base = 0;
@@ -199,7 +199,7 @@ void avc_dump_av(struct audit_buffer *ab
  * @tsid: target security identifier
  * @tclass: target security class
  */
-void avc_dump_query(struct audit_buffer *ab, u32 ssid, u32 tsid, u16 tclass)
+static void avc_dump_query(struct audit_buffer *ab, u32 ssid, u32 tsid, u16 
tclass)
 {
int rc;
char *scontext;
@@ -828,136 +828,6 @@ out:
return rc;
 }
 
-static int avc_update_cache(u32 event, u32 ssid, u32 tsid,
-u16 tclass, u32 perms)
-{
-   struct avc_node *node;
-   int i;
-
-   rcu_read_lock();
-
-   if (ssid == SECSID_WILD || tsid == SECSID_WILD) {
-   /* apply to all matching nodes */
-   for (i = 0; i  AVC_CACHE_SLOTS; i++) {
-   list_for_each_entry_rcu(node, avc_cache.slots[i], 
list) {
-   if (avc_sidcmp(ssid, node-ae.ssid) 
-   avc_sidcmp(tsid, node-ae.tsid) 
-   tclass == node-ae.tclass ) {
-   avc_update_node(event, perms, 
node-ae.ssid,
-   node-ae.tsid, 
node-ae.tclass);
-   }
-   }
-   }
-   } else {
-   /* apply to one node

[PATCH][SELINUX] Allow mounting of filesystems with invalid root inode context

2005-03-21 Thread Stephen Smalley
This patch alters the SELinux handling of inodes with invalid security
contexts so that a filesystem with a root inode that has an invalid
security context can still be mounted for administrative recovery
without disabling SELinux altogether.  Please apply.

Signed-off-by:  Stephen Smalley [EMAIL PROTECTED]
Signed-off-by:  James Morris [EMAIL PROTECTED]

 security/selinux/hooks.c |4 +++-
 1 files changed, 3 insertions(+), 1 deletion(-)

Index: linux-2.6/security/selinux/hooks.c
===
RCS file: /nfshome/pal/CVS/linux-2.6/security/selinux/hooks.c,v
retrieving revision 1.157
diff -u -p -r1.157 hooks.c
--- linux-2.6/security/selinux/hooks.c  14 Mar 2005 19:56:52 -  1.157
+++ linux-2.6/security/selinux/hooks.c  18 Mar 2005 20:39:03 -
@@ -828,7 +828,9 @@ static int inode_doinit_with_dentry(stru
   __FUNCTION__, context, -rc,
   inode-i_sb-s_id, inode-i_ino);
kfree(context);
-   goto out;
+   /* Leave with the unlabeled SID */
+   rc = 0;
+   break;
}
}
kfree(context);

-- 
Stephen Smalley [EMAIL PROTECTED]
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH][SELINUX] Make code static and remove unused code

2005-03-21 Thread Stephen Smalley
This patch from Adrian Bunk makes needlessly global code static and
removes a number of unused global and static functions from SELinux.
Please apply.

Author:  Adrian Bunk [EMAIL PROTECTED]
Signed-off-by:  Stephen Smalley [EMAIL PROTECTED]

 security/selinux/avc.c|  174 --
 security/selinux/hooks.c  |   40 
 security/selinux/include/avc.h|7 -
 security/selinux/include/avc_ss.h |   13 --
 security/selinux/include/objsec.h |2 
 security/selinux/selinuxfs.c  |4 
 security/selinux/ss/avtab.c   |   29 --
 security/selinux/ss/avtab.h   |6 -
 security/selinux/ss/conditional.c |2 
 security/selinux/ss/ebitmap.c |   43 -
 security/selinux/ss/ebitmap.h |1 
 security/selinux/ss/hashtab.c |  113 
 security/selinux/ss/hashtab.h |   38 
 security/selinux/ss/policydb.c|   10 +-
 security/selinux/ss/policydb.h|3 
 security/selinux/ss/services.c|   18 +--
 security/selinux/ss/services.h|6 -
 security/selinux/ss/sidtab.c  |   36 ---
 18 files changed, 42 insertions(+), 503 deletions(-)

diff -X /home/sds/dontdiff -rup linux-2.6.11-mm3/security/selinux/avc.c 
linux-2.6.11-mm3-adrian/security/selinux/avc.c
--- linux-2.6.11-mm3/security/selinux/avc.c 2005-03-02 02:38:17.0 
-0500
+++ linux-2.6.11-mm3-adrian/security/selinux/avc.c  2005-03-14 
14:08:28.0 -0500
@@ -139,7 +139,7 @@ static inline int avc_hash(u32 ssid, u32
  * @tclass: target security class
  * @av: access vector
  */
-void avc_dump_av(struct audit_buffer *ab, u16 tclass, u32 av)
+static void avc_dump_av(struct audit_buffer *ab, u16 tclass, u32 av)
 {
const char **common_pts = NULL;
u32 common_base = 0;
@@ -199,7 +199,7 @@ void avc_dump_av(struct audit_buffer *ab
  * @tsid: target security identifier
  * @tclass: target security class
  */
-void avc_dump_query(struct audit_buffer *ab, u32 ssid, u32 tsid, u16 tclass)
+static void avc_dump_query(struct audit_buffer *ab, u32 ssid, u32 tsid, u16 
tclass)
 {
int rc;
char *scontext;
@@ -828,136 +828,6 @@ out:
return rc;
 }
 
-static int avc_update_cache(u32 event, u32 ssid, u32 tsid,
-u16 tclass, u32 perms)
-{
-   struct avc_node *node;
-   int i;
-
-   rcu_read_lock();
-
-   if (ssid == SECSID_WILD || tsid == SECSID_WILD) {
-   /* apply to all matching nodes */
-   for (i = 0; i  AVC_CACHE_SLOTS; i++) {
-   list_for_each_entry_rcu(node, avc_cache.slots[i], 
list) {
-   if (avc_sidcmp(ssid, node-ae.ssid) 
-   avc_sidcmp(tsid, node-ae.tsid) 
-   tclass == node-ae.tclass ) {
-   avc_update_node(event, perms, 
node-ae.ssid,
-   node-ae.tsid, 
node-ae.tclass);
-   }
-   }
-   }
-   } else {
-   /* apply to one node */
-   avc_update_node(event, perms, ssid, tsid, tclass);
-   }
-
-   rcu_read_unlock();
-
-   return 0;
-}
-
-static int avc_control(u32 event, u32 ssid, u32 tsid,
-   u16 tclass, u32 perms,
-   u32 seqno, u32 *out_retained)
-{
-   struct avc_callback_node *c;
-   u32 tretained = 0, cretained = 0;
-   int rc = 0;
-
-   /*
-* try_revoke only removes permissions from the cache
-* state if they are not retained by the object manager.
-* Hence, try_revoke must wait until after the callbacks have
-* been invoked to update the cache state.
-*/
-   if (event != AVC_CALLBACK_TRY_REVOKE)
-   avc_update_cache(event,ssid,tsid,tclass,perms);
-
-   for (c = avc_callbacks; c; c = c-next)
-   {
-   if ((c-events  event) 
-   avc_sidcmp(c-ssid, ssid) 
-   avc_sidcmp(c-tsid, tsid) 
-   c-tclass == tclass 
-   (c-perms  perms)) {
-   cretained = 0;
-   rc = c-callback(event, ssid, tsid, tclass,
-(c-perms  perms),
-cretained);
-   if (rc)
-   goto out;
-   tretained |= cretained;
-   }
-   }
-
-   if (event == AVC_CALLBACK_TRY_REVOKE) {
-   /* revoke any unretained permissions */
-   perms = ~tretained;
-   avc_update_cache(event,ssid,tsid,tclass,perms);
-   *out_retained = tretained;
-   }
-
-   avc_latest_notif_update(seqno, 0);
-
-out:
-   return rc;
-}
-
-/**
- * avc_ss_grant - Grant previously denied permissions.
- * @ssid: source security identifier

[PATCH][SELINUX] Audit unrecognized netlink messages

2005-03-21 Thread Stephen Smalley
This patch changes SELinux to audit any unrecognized netlink messages
in controlled classes rather than silently rejecting them, and to
allow them if in permissive mode.  Please apply.

Signed-off-by:  Stephen Smalley [EMAIL PROTECTED]
Signed-off-by:  James Morris [EMAIL PROTECTED]

 security/selinux/hooks.c |   10 ++
 1 files changed, 10 insertions(+)
 
Index: linux-2.6/security/selinux/hooks.c
===
RCS file: /nfshome/pal/CVS/linux-2.6/security/selinux/hooks.c,v
retrieving revision 1.158
diff -u -p -r1.158 hooks.c
--- linux-2.6/security/selinux/hooks.c  21 Mar 2005 15:56:37 -  1.158
+++ linux-2.6/security/selinux/hooks.c  21 Mar 2005 16:24:06 -
@@ -67,6 +67,7 @@
 #include linux/hugetlb.h
 #include linux/personality.h
 #include linux/sysctl.h
+#include linux/audit.h
 
 #include avc.h
 #include objsec.h
@@ -3377,6 +3378,15 @@ static int selinux_nlmsg_perm(struct soc

err = selinux_nlmsg_lookup(isec-sclass, nlh-nlmsg_type, perm);
if (err) {
+   if (err == -EINVAL) {
+   audit_log(current-audit_context,
+ SELinux:  unrecognized netlink message
+  type=%hu for sclass=%hu\n,
+ nlh-nlmsg_type, isec-sclass);
+   if (!selinux_enforcing)
+   err = 0;
+   }
+
/* Ignore */
if (err == -ENOENT)
err = 0;


-- 
Stephen Smalley [EMAIL PROTECTED]
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] don't do pointless NULL checks and casts before kfree() in security/

2005-03-22 Thread Stephen Smalley
On Sun, 2005-03-20 at 13:29 +0100, Jesper Juhl wrote:
 kfree() handles NULL pointers, so checking a pointer for NULL before 
 calling kfree() on it is pointless. kfree() takes a void* argument and 
 changing the type of a pointer before kfree()'ing it is equally pointless.
 This patch removes the pointless checks for NULL and needless mucking 
 about with the pointer types before kfree() for all files in security/* 
 where I could locate such code.
 
 The following files are modified by this patch:
   security/keys/key.c
   security/keys/user_defined.c
   security/selinux/hooks.c
   security/selinux/selinuxfs.c
   security/selinux/ss/conditional.c
   security/selinux/ss/policydb.c
   security/selinux/ss/services.c
 
 (please keep me on CC if you reply)
 
 
 Signed-off-by: Jesper Juhl [EMAIL PROTECTED]

The diffs to selinux look fine to me, and the resulting kernel seems to
be operating without problem.  Feel free to send along to Andrew Morton.

Acked-by: Stephen Smalley [EMAIL PROTECTED]


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4 with proper signed-off] security/selinux/ss/policydb.c: fix sparse warnings

2005-03-22 Thread Stephen Smalley
On Sun, 2005-03-20 at 12:59 +0100, Domen Puncer wrote:
 Signed-off-by: Alexey Dobriyan [EMAIL PROTECTED]
 Signed-off-by: Domen Puncer [EMAIL PROTECTED]
 ---
 
 
  kj-domen/security/selinux/ss/policydb.c |   35 
 ++--
  1 files changed, 20 insertions(+), 15 deletions(-)
 
 diff -puN security/selinux/ss/policydb.c~sparse-security_selinux_ss_policydb 
 security/selinux/ss/policydb.c
 --- kj/security/selinux/ss/policydb.c~sparse-security_selinux_ss_policydb 
 2005-03-20 12:11:25.0 +0100
 +++ kj-domen/security/selinux/ss/policydb.c   2005-03-20 12:11:25.0 
 +0100
 @@ -1494,9 +1497,11 @@ int policydb_read(struct policydb *p, vo
   goto bad;
   }
  
 - if (buf[2] != info-sym_num || buf[3] != info-ocon_num) {
 + if (le32_to_cpu(buf[2]) != info-sym_num ||
 + le32_to_cpu(buf[3]) != info-ocon_num) {
   printk(KERN_ERR security:  policydb table sizes (%d,%d) do 
 -not match mine (%d,%d)\n, buf[2], buf[3],
 +not match mine (%d,%d)\n,
 +le32_to_cpu(buf[2]), le32_to_cpu(buf[3]),
  info-sym_num, info-ocon_num);
   goto bad;
   }
 _

You didn't remove the loop that already converted these values to little
endian already (no that isn't the same as the earlier loop that you did
remove), so now you are converting them twice.  And why is this new code
better even if you fix this omission?  

-- 
Stephen Smalley [EMAIL PROTECTED]
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4 with proper signed-off] security/selinux/ss/policydb.c: fix sparse warnings

2005-03-22 Thread Stephen Smalley
On Tue, 2005-03-22 at 10:19 -0500, Stephen Smalley wrote:
 You didn't remove the loop that already converted these values to little

s/ to / from /

 endian already (no that isn't the same as the earlier loop that you did
 remove), so now you are converting them twice.  And why is this new code
 better even if you fix this omission?  

Note btw that you would also need to modify usage of buf[0] and buf[1]
if you do remove that loop.  But I'm still not clear on the benefit of
the change (silencing warnings generated by a checker doesn't count
unless they point to a real bug).

-- 
Stephen Smalley [EMAIL PROTECTED]
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH][SELINUX] Add name_connect permission check

2005-03-23 Thread Stephen Smalley
This patch adds a name_connect permission check to SELinux to provide
control over outbound TCP connections to particular ports distinct
from the general controls over sending and receiving packets.  Please
apply.

 security/selinux/hooks.c |   48 ++-
 security/selinux/include/av_perm_to_string.h |1 
 security/selinux/include/av_permissions.h|1 
 3 files changed, 49 insertions(+), 1 deletion(-)

Index: linux-2.6/security/selinux/hooks.c
===
RCS file: /nfshome/pal/CVS/linux-2.6/security/selinux/hooks.c,v
retrieving revision 1.160
diff -u -p -r1.160 hooks.c
--- linux-2.6/security/selinux/hooks.c  22 Mar 2005 17:30:12 -  1.160
+++ linux-2.6/security/selinux/hooks.c  23 Mar 2005 14:17:26 -
@@ -3085,7 +3085,53 @@ out:
 
 static int selinux_socket_connect(struct socket *sock, struct sockaddr 
*address, int addrlen)
 {
-   return socket_has_perm(current, sock, SOCKET__CONNECT);
+   struct inode_security_struct *isec;
+   int err;
+
+   err = socket_has_perm(current, sock, SOCKET__CONNECT);
+   if (err)
+   return err;
+
+   /*
+* If a TCP socket, check name_connect permission for the port.
+*/
+   isec = SOCK_INODE(sock)-i_security;
+   if (isec-sclass == SECCLASS_TCP_SOCKET) {
+   struct sock *sk = sock-sk;
+   struct avc_audit_data ad;
+   struct sockaddr_in *addr4 = NULL;
+   struct sockaddr_in6 *addr6 = NULL;
+   unsigned short snum;
+   u32 sid;
+
+   if (sk-sk_family == PF_INET) {
+   addr4 = (struct sockaddr_in *)address;
+   if (addrlen != sizeof(struct sockaddr_in))
+   return -EINVAL;
+   snum = ntohs(addr4-sin_port);
+   } else {
+   addr6 = (struct sockaddr_in6 *)address;
+   if (addrlen != sizeof(struct sockaddr_in6))
+   return -EINVAL;
+   snum = ntohs(addr6-sin6_port);
+   }
+
+   err = security_port_sid(sk-sk_family, sk-sk_type,
+   sk-sk_protocol, snum, sid);
+   if (err)
+   goto out;
+
+   AVC_AUDIT_DATA_INIT(ad,NET);
+   ad.u.net.dport = htons(snum);
+   ad.u.net.family = sk-sk_family;
+   err = avc_has_perm(isec-sid, sid, isec-sclass,
+  TCP_SOCKET__NAME_CONNECT, ad);
+   if (err)
+   goto out;
+   }
+
+out:
+   return err;
 }
 
 static int selinux_socket_listen(struct socket *sock, int backlog)
Index: linux-2.6/security/selinux/include/av_perm_to_string.h
===
RCS file: 
/nfshome/pal/CVS/linux-2.6/security/selinux/include/av_perm_to_string.h,v
retrieving revision 1.23
diff -u -p -r1.23 av_perm_to_string.h
--- linux-2.6/security/selinux/include/av_perm_to_string.h  23 Feb 2005 
20:26:54 -  1.23
+++ linux-2.6/security/selinux/include/av_perm_to_string.h  22 Mar 2005 
20:29:05 -
@@ -25,6 +25,7 @@
S_(SECCLASS_TCP_SOCKET, TCP_SOCKET__NEWCONN, newconn)
S_(SECCLASS_TCP_SOCKET, TCP_SOCKET__ACCEPTFROM, acceptfrom)
S_(SECCLASS_TCP_SOCKET, TCP_SOCKET__NODE_BIND, node_bind)
+   S_(SECCLASS_TCP_SOCKET, TCP_SOCKET__NAME_CONNECT, name_connect)
S_(SECCLASS_UDP_SOCKET, UDP_SOCKET__NODE_BIND, node_bind)
S_(SECCLASS_RAWIP_SOCKET, RAWIP_SOCKET__NODE_BIND, node_bind)
S_(SECCLASS_NODE, NODE__TCP_RECV, tcp_recv)
Index: linux-2.6/security/selinux/include/av_permissions.h
===
RCS file: /nfshome/pal/CVS/linux-2.6/security/selinux/include/av_permissions.h,v
retrieving revision 1.22
diff -u -p -r1.22 av_permissions.h
--- linux-2.6/security/selinux/include/av_permissions.h 23 Feb 2005 20:26:54 
-  1.22
+++ linux-2.6/security/selinux/include/av_permissions.h 22 Mar 2005 20:29:05 
-
@@ -253,6 +253,7 @@
 #define TCP_SOCKET__NEWCONN   0x0080UL
 #define TCP_SOCKET__ACCEPTFROM0x0100UL
 #define TCP_SOCKET__NODE_BIND 0x0200UL
+#define TCP_SOCKET__NAME_CONNECT  0x0400UL
 
 #define UDP_SOCKET__IOCTL 0x0001UL
 #define UDP_SOCKET__READ  0x0002UL

-- 
Stephen Smalley [EMAIL PROTECTED]
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH][SELINUX] Add name_connect permission check

2005-03-23 Thread Stephen Smalley
On Wed, 2005-03-23 at 09:40 -0500, Stephen Smalley wrote:
 This patch adds a name_connect permission check to SELinux to provide
 control over outbound TCP connections to particular ports distinct
 from the general controls over sending and receiving packets.  Please
 apply.
 
  security/selinux/hooks.c |   48 
 ++-
  security/selinux/include/av_perm_to_string.h |1 
  security/selinux/include/av_permissions.h|1 
  3 files changed, 49 insertions(+), 1 deletion(-)

Ah, sorry - forgot the Signed-off-by lines.

Signed-off-by:  Stephen Smalley [EMAIL PROTECTED]
Signed-off-by:  James Morris [EMAIL PROTECTED]

-- 
Stephen Smalley [EMAIL PROTECTED]
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/4] vfs: adds the S_PRIVATE flag and adds use to security

2005-03-07 Thread Stephen Smalley
On Fri, 2005-03-04 at 21:28 -0800, Andrew Morton wrote:
 Jeffrey Mahoney [EMAIL PROTECTED] wrote:
 
   This patch adds an S_PRIVATE flag to inode-i_flags to mark an inode as
   filesystem-internal. As such, it should be excepted from the security
   infrastructure to allow the filesystem to perform its own access control.
 
 OK, thanks.  I'll assume that the other three patches are unchanged.
 
 I don't think we've heard from the SELinux team regarding these patches?
 
 (See http://www.zip.com.au/~akpm/linux/patches/stuff/selinux-reiserfs/)

Acked-by:  Stephen Smalley [EMAIL PROTECTED]

-- 
Stephen Smalley [EMAIL PROTECTED]
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH][LSM/SELINUX] Pass requested protection to security_file_mmap/mprotect hooks

2005-03-07 Thread Stephen Smalley
This patch adds a reqprot parameter to the security_file_mmap and
security_file_mprotect hooks that is the original requested protection
value prior to any modification for read-implies-exec, and changes the
SELinux module to allow a mode of operation (controllable via a
checkreqprot setting) where it applies checks based on that protection
value rather than the protection that will be applied by the kernel,
effectively restoring SELinux's original behavior prior to the
introduction of the read-implies-exec logic in the mainline kernel.
The patch also disables execmem and execmod checking entirely on
PPC32, as the PPC32 ELF ABI presently requires RWE segments per Ulrich
Drepper.  The patch is relative to the enhanced MLS support patch submitted
against 2.6.11-mm1.

At present, the read-implies-exec logic causes SELinux to see every
mmap/mprotect read request by legacy binaries or binaries marked with
PT_GNU_STACK RWE as a read|execute request, which tends to distort
policy even if it reflects what is ultimately possible.  The
checkreqprot setting allows one to set the desired behavior for
SELinux, so either the current behavior or the original behavior is
possible.  The checkreqprot value has a compile-time configurable
default value and can also be set via boot parameter or at runtime via
/selinux/checkreqprot if allowed by policy.  Thanks to Chris Wright,
James Morris, and Colin Walters for comments on an earlier version of
the patch.

Signed-off-by: Stephen Smalley [EMAIL PROTECTED]
Signed-off-by: James Morris [EMAIL PROTECTED]

 include/linux/security.h |   25 +++
 mm/mmap.c|4 -
 mm/mprotect.c|6 +-
 security/dummy.c |7 ++-
 security/selinux/Kconfig |   19 
 security/selinux/hooks.c |   18 ++--
 security/selinux/include/av_perm_to_string.h |1 
 security/selinux/include/av_permissions.h|1 
 security/selinux/include/objsec.h|2 
 security/selinux/selinuxfs.c |   60 +++
 10 files changed, 126 insertions(+), 17 deletions(-)

diff -X /home/sds/dontdiff -rup linux-2.6.11-mm1-mls/include/linux/security.h 
linux-2.6.11-mm1-mls-reqprot/include/linux/security.h
--- linux-2.6.11-mm1-mls/include/linux/security.h   2005-03-07 
13:06:54.0 -0500
+++ linux-2.6.11-mm1-mls-reqprot/include/linux/security.h   2005-03-07 
12:21:48.0 -0500
@@ -458,13 +458,15 @@ struct swap_info_struct;
  * Check permissions for a mmap operation.  The @file may be NULL, e.g.
  * if mapping anonymous memory.
  * @file contains the file structure for file to map (may be NULL).
- * @prot contains the requested permissions.
+ * @reqprot contains the protection requested by the application.
+ * @prot contains the protection that will be applied by the kernel.
  * @flags contains the operational flags.
  * Return 0 if permission is granted.
  * @file_mprotect:
  * Check permissions before changing memory access permissions.
  * @vma contains the memory region to modify.
- * @prot contains the requested permissions.
+ * @reqprot contains the protection requested by the application.
+ * @prot contains the protection that will be applied by the kernel.
  * Return 0 if permission is granted.
  * @file_lock:
  * Check permission before performing file locking operations.
@@ -1128,9 +1130,12 @@ struct security_operations {
void (*file_free_security) (struct file * file);
int (*file_ioctl) (struct file * file, unsigned int cmd,
   unsigned long arg);
-   int (*file_mmap) (struct file * file,
+   int (*file_mmap) (struct file * file, 
+ unsigned long reqprot,
  unsigned long prot, unsigned long flags);
-   int (*file_mprotect) (struct vm_area_struct * vma, unsigned long prot);
+   int (*file_mprotect) (struct vm_area_struct * vma, 
+ unsigned long reqprot,
+ unsigned long prot);
int (*file_lock) (struct file * file, unsigned int cmd);
int (*file_fcntl) (struct file * file, unsigned int cmd,
   unsigned long arg);
@@ -1631,16 +1636,18 @@ static inline int security_file_ioctl (s
return security_ops-file_ioctl (file, cmd, arg);
 }
 
-static inline int security_file_mmap (struct file *file, unsigned long prot,
+static inline int security_file_mmap (struct file *file, unsigned long reqprot,
+ unsigned long prot,
  unsigned long flags)
 {
-   return security_ops-file_mmap (file, prot, flags);
+   return security_ops-file_mmap (file, reqprot, prot, flags);
 }
 
 static inline int security_file_mprotect (struct vm_area_struct *vma

Re: [PATCH][LSM/SELINUX] Pass requested protection to security_file_mmap/mprotect hooks

2005-03-08 Thread Stephen Smalley
On Mon, 2005-03-07 at 16:14 -0800, Andrew Morton wrote:
 Stephen Smalley [EMAIL PROTECTED] wrote:
 
  +__setup(checkreqprot=, checkreqprot_setup);
 
 Can we have an update to Documentation/kernel-parameters.txt, please?

Ok, how does the patch below look?  Includes descriptions of the other
two SELinux-related parameters as well.

Signed-off-by:  Stephen Smalley [EMAIL PROTECTED]

 Documentation/kernel-parameters.txt |   26 ++
 1 files changed, 26 insertions(+)

--- linux-2.6.11-mm2/Documentation/kernel-parameters.txt2005-03-08 
07:46:07.491966080 -0500
+++ linux-2.6.11-mm2-sel/Documentation/kernel-parameters.txt2005-03-08 
08:21:11.179157016 -0500
@@ -67,6 +67,7 @@
SCSIAppropriate SCSI support is enabled.
A lot of drivers has their options described inside of
Documentation/scsi/.
+   SELINUX SELinux support is enabled.
SERIAL  Serial support is enabled.
SMP The kernel is an SMP kernel.
SPARC   Sparc architecture is enabled.
@@ -296,6 +297,14 @@
See header of drivers/cdrom/cdu31a.c.
 
chandev=[HW,NET] Generic channel device initialisation
+
+   checkreqprot[SELINUX] Set initial checkreqprot flag value.
+   Format: { 0 | 1 }
+   See security/selinux/Kconfig help text.
+   0 -- check protection applied by kernel (includes any 
implied execute protection).
+   1 -- check protection requested by application.
+   Default value is set via a kernel config option.
+   Value can be changed at runtime via 
/selinux/checkreqprot.
  
clock=  [BUGS=IA-32, HW] gettimeofday timesource override. 
Forces specified timesource (if avaliable) to be used
@@ -444,6 +453,14 @@
See Documentation/block/as-iosched.txt
and Documentation/block/deadline-iosched.txt for 
details.
 
+   enforcing   [SELINUX] Set initial enforcing status.
+   Format: {0 | 1}
+   See security/selinux/Kconfig help text.
+   0 -- permissive (log only, no denials).
+   1 -- enforcing (deny and log).
+   Default value is 0.
+   Value can be changed at runtime via /selinux/enforce.
+
es1370= [HW,OSS]
Format: lineout[,micbias]
See also header of sound/oss/es1370.c.
@@ -1187,6 +1204,15 @@
 
scsi_logging=   [SCSI]
 
+   selinux [SELINUX] Disable or enable SELinux at boot time.
+   Format: { 0 | 1 }
+   See security/selinux/Kconfig help text.
+   0 -- disable.
+   1 -- enable.
+   Default value is set via kernel config option.
+   If enabled at boot time, /selinux/disable can be used
+   later to disable prior to initial policy load.
+
serialnumber[BUGS=IA-32]
 
sg_def_reserved_size=



-- 
Stephen Smalley [EMAIL PROTECTED]
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH][SELINUX] Fix selinux_setprocattr

2005-03-08 Thread Stephen Smalley
This patch against 2.6.11-mm2 changes the selinux_setprocattr hook function 
(which
handles writes to nodes in the /proc/pid/attr directory) to ignore an
optional terminating newline at the end of the value, and to handle a
value beginning with a newline or a null in the same manner as a zero
length value (clearing the attribute for the process and resetting it
to using the default policy behavior).  This change is to address the
divergence from POSIX in the existing API, as POSIX says that write(2)
with a zero count will return zero with no other effect, as well as to
simplify use of the API from scripts (although that isn't
recommended).  Please apply.

Signed-off-by:  Stephen Smalley [EMAIL PROTECTED]
Signed-off-by:  James Morris [EMAIL PROTECTED]

 security/selinux/hooks.c |8 ++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff -X /home/sds/dontdiff -ru linux-2.6.11-mm2/security/selinux/hooks.c 
linux-2.6.11-mm2-sel/security/selinux/hooks.c
--- linux-2.6.11-mm2/security/selinux/hooks.c   2005-03-08 08:43:52.867139656 
-0500
+++ linux-2.6.11-mm2-sel/security/selinux/hooks.c   2005-03-08 
08:44:02.733639720 -0500
@@ -4106,6 +4106,7 @@
struct task_security_struct *tsec;
u32 sid = 0;
int error;
+   char *str = value;
 
if (current != p) {
/* SELinux only allows a process to change its own
@@ -4130,8 +4131,11 @@
return error;
 
/* Obtain a SID for the context, if one was specified. */
-   if (size) {
-   int error;
+   if (size  str[1]  str[1] != '\n') {
+   if (str[size-1] == '\n') {
+   str[size-1] = 0;
+   size--;
+   }
error = security_context_to_sid(value, size, sid);
if (error)
return error;

-- 
Stephen Smalley [EMAIL PROTECTED]
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/1] SELinux AVC audit log ipaddr field support (for task_struct-curr_ip)

2005-03-10 Thread Stephen Smalley
On Thu, 2005-03-10 at 16:05 +0100, Lorenzo Hernndez Garca-Hierro
wrote:
 Provides support for a new field ipaddr within the SELinux
 AVC audit log, relying in task_struct-curr_ip (ipv4 only)
 provided by the task-curr_ip or grSecurity patch to be applied
 before.It was first implemented by Joshua Brindle (a.k.a Method)
 from the Hardened Gentoo project.
 
 An example of the audit messages with ipaddr field:
 audit(1110432234.161:0): avc:  denied  { search } for  pid=19057
 exe=/usr/bin/wget name=portage dev=hda3 ino=1024647 ipaddr=192.168.1.30
 scontext=root:sysadm_r:portage_fetch_t 
 tcontext=system_u:object_r:portage_tmp_t tclass=dir

Even if the basic idea were sound (doubtful), this would need to be
generalized (i.e. not ipv4-specific).  Also, I think I'd rather see
extensions to the audit data be incorporated into the audit framework,
not the AVC-specific audit code, and some of the existing avc_audit()
code migrated into the audit framework (e.g. the exe= information
currently generated by avc_audit could be done by audit_log_exit
instead).

-- 
Stephen Smalley [EMAIL PROTECTED]
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] Fix SELinux for removal of i_sock

2005-04-01 Thread Stephen Smalley
Hi,

This patch against -bk eliminates the use of i_sock by SELinux as it
appears to have been removed recently, breaking the build of SELinux in
-bk.  Simply replacing the i_sock test with an S_ISSOCK test would be
unsafe in the SELinux code, as the latter will also return true for the
inodes of socket files in the filesystem, not just the actual socket
objects IIUC.  Hence this patch reworks the SELinux code to avoid the
need to apply such a test in the first place, part of which was
obsoleted anyway by earlier changes to SELinux.  Please apply.

Signed-off-by:  Stephen Smalley [EMAIL PROTECTED]
Signed-off-by:  James Morris [EMAIL PROTECTED]

 security/selinux/hooks.c |   21 +++--
 1 files changed, 3 insertions(+), 18 deletions(-)

= security/selinux/hooks.c 1.93 vs edited =
--- 1.93/security/selinux/hooks.c   2005-03-28 17:21:19 -05:00
+++ edited/security/selinux/hooks.c 2005-04-01 15:01:58 -05:00
@@ -877,18 +877,8 @@ static int inode_doinit_with_dentry(stru
isec-initialized = 1;
 
 out:
-   if (inode-i_sock) {
-   struct socket *sock = SOCKET_I(inode);
-   if (sock-sk) {
-   isec-sclass = 
socket_type_to_security_class(sock-sk-sk_family,
-
sock-sk-sk_type,
-
sock-sk-sk_protocol);
-   } else {
-   isec-sclass = SECCLASS_SOCKET;
-   }
-   } else {
+   if (isec-sclass == SECCLASS_FILE)
isec-sclass = inode_mode_to_security_class(inode-i_mode);
-   }
 
if (hold_sem)
up(isec-sem);
@@ -2979,18 +2969,15 @@ out:
 static void selinux_socket_post_create(struct socket *sock, int family,
   int type, int protocol, int kern)
 {
-   int err;
struct inode_security_struct *isec;
struct task_security_struct *tsec;
 
-   err = inode_doinit(SOCK_INODE(sock));
-   if (err  0)
-   return;
isec = SOCK_INODE(sock)-i_security;
 
tsec = current-security;
isec-sclass = socket_type_to_security_class(family, type, protocol);
isec-sid = kern ? SECINITSID_KERNEL : tsec-sid;
+   isec-initialized = 1;
 
return;
 }
@@ -3158,14 +3145,12 @@ static int selinux_socket_accept(struct 
if (err)
return err;
 
-   err = inode_doinit(SOCK_INODE(newsock));
-   if (err  0)
-   return err;
newisec = SOCK_INODE(newsock)-i_security;
 
isec = SOCK_INODE(sock)-i_security;
newisec-sclass = isec-sclass;
newisec-sid = isec-sid;
+   newisec-initialized = 1;
 
return 0;
 }


-- 
Stephen Smalley [EMAIL PROTECTED]
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Fix SELinux for removal of i_sock

2005-04-04 Thread Stephen Smalley
On Fri, 2005-04-01 at 12:35 -0800, David S. Miller wrote:
 On Fri, 01 Apr 2005 15:06:37 -0500
 Stephen Smalley [EMAIL PROTECTED] wrote:
 
  This patch against -bk eliminates the use of i_sock by SELinux as it
  appears to have been removed recently, breaking the build of SELinux in
  -bk.  Simply replacing the i_sock test with an S_ISSOCK test would be
  unsafe in the SELinux code, as the latter will also return true for the
  inodes of socket files in the filesystem, not just the actual socket
  objects IIUC.  Hence this patch reworks the SELinux code to avoid the
  need to apply such a test in the first place, part of which was
  obsoleted anyway by earlier changes to SELinux.  Please apply.
  
  Signed-off-by:  Stephen Smalley [EMAIL PROTECTED]
  Signed-off-by:  James Morris [EMAIL PROTECTED]
 
 Applied, thanks Stephen.

So, just for clarification, since a S_ISSOCK test is not necessarily
equivalent to an i_sock test (in the case of inodes of socket files in
the filesystem), was removing i_sock truly the right choice?  It may not
be an issue for typical users of i_sock since you can't open a
descriptor to such a socket file, so any code that was acting on an open
file shouldn't have to deal with this ambiguity, but could possibly lead
to an erroneous use of SOCKET_I on the inode of a socket file in other
code (which is what would have happened in SELinux if we had just
changed the i_sock test to an ISSOCK test).  Thanks, just trying to
avoid confusion in the kernel in the future...
  
-- 
Stephen Smalley [EMAIL PROTECTED]
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: idr_remove

2005-02-22 Thread Stephen Smalley
On Tue, 2005-02-22 at 13:22 -0500, Jim Houston wrote:
 I spent time looking at the pty and selinux code yesterday.
 I had little luck finding where the selinux code hooks into 
 the pty code.

The call to lookup_one_len() by fs/devpts/inode.c:get_node() ultimately
calls permission(...,MAY_EXEC,...) on the devpts root directory, which
will call security_inode_permission() - selinux_inode_permission() to
check search access to the directory.  Hence, get_node() can fail if
SELinux is enabled and the process has no permission at all to
search /dev/pts.  If it can get that far, then the inode will ultimately
have its security label set upon the d_instantiate() call (via
security_d_instantiate() - selinux_d_instantiate()), and be
subsequently checked for opens/reads/writes via the
selinux_inode_permission() and selinux_file_permission() hook functions.

-- 
Stephen Smalley [EMAIL PROTECTED]
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Thoughts on the No Linux Security Modules framework old claims

2005-02-24 Thread Stephen Smalley
On Wed, 2005-02-23 at 13:37 -0800, Crispin Cowan wrote:
 You are confused. It is Secure Computing Corporation that holds patents 
 that threaten SELinux 
 http://www.securecomputing.com/pdf/Statement_of_Assurance.pdf

The NSA made a statement in response to that statement a long time ago,
and in any event, the patents in question have expired AFAICS.

-- 
Stephen Smalley [EMAIL PROTECTED]
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] SELinux: Leak in error path

2005-03-01 Thread Stephen Smalley
On Tue, 2005-03-01 at 01:32 +0100, Alexander Nyberg wrote:
 There's a leak here in the first error path.
 
 Found by the Coverity tool.
 
 Signed-off-by: Alexander Nyberg [EMAIL PROTECTED]

Acked-by:  Stephen Smalley [EMAIL PROTECTED]

-- 
Stephen Smalley [EMAIL PROTECTED]
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] SELinux: null dereference in error path

2005-03-01 Thread Stephen Smalley
On Tue, 2005-03-01 at 01:32 +0100, Alexander Nyberg wrote:
 The 'bad' label will call function that unconditionally dereferences
 the NULL pointer.
 
 Found by the Coverity tool
 
 Signed-off-by: Alexander Nyberg [EMAIL PROTECTED]

Acked-by:  Stephen Smalley [EMAIL PROTECTED]

-- 
Stephen Smalley [EMAIL PROTECTED]
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH][SELINUX] Define execmod permission for character devices

2005-02-01 Thread Stephen Smalley
This patch against 2.6.11-rc2-mm2 regenerates the SELinux module headers
to define the execmod permission for character device files in order to
provide proper auditing of such checks on /dev/zero.  Please apply.

Signed-off-by:  Stephen Smalley [EMAIL PROTECTED]
Signed-off-by:  James Morris [EMAIL PROTECTED]

 security/selinux/include/av_perm_to_string.h |3 +++
 security/selinux/include/av_permissions.h|4 
 2 files changed, 7 insertions(+)

Index: linux-2.6/security/selinux/include/av_perm_to_string.h
===
RCS file: 
/nfshome/pal/CVS/linux-2.6/security/selinux/include/av_perm_to_string.h,v
retrieving revision 1.19
diff -u -p -r1.19 av_perm_to_string.h
--- linux-2.6/security/selinux/include/av_perm_to_string.h  1 Dec 2004 
16:47:00 -   1.19
+++ linux-2.6/security/selinux/include/av_perm_to_string.h  31 Jan 2005 
19:40:08 -
@@ -17,6 +17,9 @@
S_(SECCLASS_FILE, FILE__EXECUTE_NO_TRANS, execute_no_trans)
S_(SECCLASS_FILE, FILE__ENTRYPOINT, entrypoint)
S_(SECCLASS_FILE, FILE__EXECMOD, execmod)
+   S_(SECCLASS_CHR_FILE, CHR_FILE__EXECUTE_NO_TRANS, execute_no_trans)
+   S_(SECCLASS_CHR_FILE, CHR_FILE__ENTRYPOINT, entrypoint)
+   S_(SECCLASS_CHR_FILE, CHR_FILE__EXECMOD, execmod)
S_(SECCLASS_FD, FD__USE, use)
S_(SECCLASS_TCP_SOCKET, TCP_SOCKET__CONNECTTO, connectto)
S_(SECCLASS_TCP_SOCKET, TCP_SOCKET__NEWCONN, newconn)
Index: linux-2.6/security/selinux/include/av_permissions.h
===
RCS file: /nfshome/pal/CVS/linux-2.6/security/selinux/include/av_permissions.h,v
retrieving revision 1.18
diff -u -p -r1.18 av_permissions.h
--- linux-2.6/security/selinux/include/av_permissions.h 1 Dec 2004 16:47:00 
-   1.18
+++ linux-2.6/security/selinux/include/av_permissions.h 31 Jan 2005 19:40:08 
-
@@ -143,6 +143,10 @@
 #define CHR_FILE__QUOTAON 0x8000UL
 #define CHR_FILE__MOUNTON 0x0001UL
 
+#define CHR_FILE__EXECUTE_NO_TRANS0x0002UL
+#define CHR_FILE__ENTRYPOINT  0x0004UL
+#define CHR_FILE__EXECMOD 0x0008UL
+
 #define BLK_FILE__IOCTL   0x0001UL
 #define BLK_FILE__READ0x0002UL
 #define BLK_FILE__WRITE   0x0004UL

-- 
Stephen Smalley [EMAIL PROTECTED]
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH][SELINUX] Audit any unmapped permissions

2005-02-01 Thread Stephen Smalley
This patch against 2.6.11-rc2-mm2 changes SELinux to display any
permission values that could not be mapped to names as a hex value when
generating an audit message.  Please apply.

Signed-off-by:  Stephen Smalley [EMAIL PROTECTED]
Signed-off-by:  James Morris [EMAIL PROTECTED]

 security/selinux/avc.c |   11 +--
 1 files changed, 9 insertions(+), 2 deletions(-)

Index: linux-2.6/security/selinux/avc.c
===
RCS file: /nfshome/pal/CVS/linux-2.6/security/selinux/avc.c,v
retrieving revision 1.50
diff -u -p -r1.50 avc.c
--- linux-2.6/security/selinux/avc.c10 Dec 2004 17:16:30 -  1.50
+++ linux-2.6/security/selinux/avc.c1 Feb 2005 12:50:14 -
@@ -162,8 +162,10 @@ void avc_dump_av(struct audit_buffer *ab
i = 0;
perm = 1;
while (perm  common_base) {
-   if (perm  av)
+   if (perm  av) {
audit_log_format(ab,  %s, common_pts[i]);
+   av = ~perm;
+   }
i++;
perm = 1;
}
@@ -175,14 +177,19 @@ void avc_dump_av(struct audit_buffer *ab
(av_perm_to_string[i2].value == perm))
break;
}
-   if (i2  ARRAY_SIZE(av_perm_to_string))
+   if (i2  ARRAY_SIZE(av_perm_to_string)) {
audit_log_format(ab,  %s,
 av_perm_to_string[i2].name);
+   av = ~perm;
+   }
}
i++;
perm = 1;
}
 
+   if (av)
+   audit_log_format(ab,  0x%x, av);
+
audit_log_format(ab,  });
 }
 

-- 
Stephen Smalley [EMAIL PROTECTED]
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH][SELINUX] Fix selinux_inode_setattr hook

2005-02-04 Thread Stephen Smalley
This patch against 2.6.11-rc3 fixes the selinux_inode_setattr hook
function to honor the ATTR_FORCE flag, skipping any permission checking
in that case.  Otherwise, it is possible though unlikely for a denial
from the hook to prevent proper updating, e.g. for remove_suid upon
writing to a file.  This would only occur if the process had write
permission to a suid file but lacked setattr permission to it.  Please
apply.

Signed-off-by: Stephen Smalley [EMAIL PROTECTED]
Signed-off-by: James Morris [EMAIL PROTECTED]

 security/selinux/hooks.c |3 +++
 1 files changed, 3 insertions(+)

Index: linux-2.6/security/selinux/hooks.c
===
RCS file: /nfshome/pal/CVS/linux-2.6/security/selinux/hooks.c,v
retrieving revision 1.150
diff -u -p -r1.150 hooks.c
--- linux-2.6/security/selinux/hooks.c  26 Jan 2005 21:20:59 -  1.150
+++ linux-2.6/security/selinux/hooks.c  4 Feb 2005 16:39:23 -
@@ -2142,6 +2142,9 @@ static int selinux_inode_setattr(struct 
if (rc)
return rc;
 
+   if (iattr-ia_valid  ATTR_FORCE)
+   return 0;
+
if (iattr-ia_valid  (ATTR_MODE | ATTR_UID | ATTR_GID |
   ATTR_ATIME_SET | ATTR_MTIME_SET))
return dentry_has_perm(current, NULL, dentry, FILE__SETATTR);


-- 
Stephen Smalley [EMAIL PROTECTED]
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH][SELINUX] Fix selinux_inode_setattr hook

2005-02-04 Thread Stephen Smalley
On Fri, 2005-02-04 at 13:14, Chris Wright wrote:
 * Stephen Smalley ([EMAIL PROTECTED]) wrote:
  This patch against 2.6.11-rc3 fixes the selinux_inode_setattr hook
  function to honor the ATTR_FORCE flag, skipping any permission checking
  in that case.  Otherwise, it is possible though unlikely for a denial
  from the hook to prevent proper updating, e.g. for remove_suid upon
  writing to a file.  This would only occur if the process had write
  permission to a suid file but lacked setattr permission to it.  Please
  apply.
 
 Is there any reason not to promote this to the framework?

I wasn't sure if a security module might still want to be notified of
forced changes (e.g. to adjust some state in its own security
structure), even if it skips permission checking on such changes.

-- 
Stephen Smalley [EMAIL PROTECTED]
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Fix audit control message checks

2005-01-18 Thread Stephen Smalley
On Sat, 2005-01-15 at 15:07, Serge E. Hallyn wrote:
 The audit control messages are sent over netlink.  Permission checks
 are done on the process receiving the message, which may not be the
 same as the process sending the message.  This patch switches the
 netlink_send security hooks to calculate the effective capabilities
 based on the sender.  Then audit_receive_msg performs capability checks
 based on that.
 
 It also introduces the CAP_AUDIT_WRITE and CAP_AUDIT_CONTROL capabilities,
 and replaces the previous CAP_SYS_ADMIN checks in audit code with the
 appropriate checks.
 
 Please apply.
 
 Changelog:
   1/15/2005: Simplified dummy_netlink_send given that dummy now
   keeps track of capabilities.
   1/14/2005: Many fixes based on feedback from [EMAIL PROTECTED]
   list.
   1/14/2005: Removed the netlink_msg_type helper function.
   1/07/2005: Swith to using CAP_AUDIT_WRITE and CAP_AUDIT_CONTROL.
 
 thanks,
 -serge
 
 Signed-off-by: Serge Hallyn [EMAIL PROTECTED]

Signed-off-by:  Stephen Smalley [EMAIL PROTECTED]

-- 
Stephen Smalley [EMAIL PROTECTED]
National Security Agency

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


  1   2   3   4   5   6   7   8   9   10   >