Re: [PATCH] selinux: Inode label revalidation performance fix

[Stephen Smalley]

On 01/05/2016 05:12 PM, Andreas Gruenbacher wrote:

Commit 5d226df4 has introduced a performance regression of about
10% in the UnixBench pipe benchmark.  It turns out that the call
to inode_security in selinux_file_permission can be moved below
the zero-mask test and that inode_security_revalidate can be
removed entirely, which brings us back to roughly the original
performance.

Signed-off-by: Andreas Gruenbacher <agrue...@redhat.com>


Acked-by:  Stephen Smalley <s...@tycho.nsa.gov>


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

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 40e071a..f8110cf 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -273,11 +273,6 @@ static int __inode_security_revalidate(struct inode *inode,
return 0;
  }

-static void inode_security_revalidate(struct inode *inode)
-{
-   __inode_security_revalidate(inode, NULL, true);
-}
-
  static struct inode_security_struct *inode_security_novalidate(struct inode 
*inode)
  {
return inode->i_security;
@@ -3277,19 +3272,19 @@ static int selinux_file_permission(struct file *file, 
int mask)
  {
struct inode *inode = file_inode(file);
struct file_security_struct *fsec = file->f_security;
-   struct inode_security_struct *isec = inode_security(inode);
+   struct inode_security_struct *isec;
u32 sid = current_sid();

if (!mask)
/* No permission to check.  Existence test. */
return 0;

+   isec = inode_security(inode);
if (sid == fsec->sid && fsec->isid == isec->sid &&
fsec->pseqno == avc_policy_seqno())
/* No change since file_open check. */
return 0;

-   inode_security_revalidate(inode);
return selinux_revalidate_file_permission(file, mask);
  }

@@ -3595,7 +3590,6 @@ static int selinux_file_open(struct file *file, const 
struct cred *cred)
 * new inode label or new policy.
 * This check is not redundant - do not remove.
 */
-   inode_security_revalidate(file_inode(file));
return file_path_has_perm(cred, file, open_file_to_av(file));
  }




--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Exposing secid to secctx mapping to user-space

[Stephen Smalley]

On 12/15/2015 11:06 AM, Casey Schaufler wrote:

On 12/15/2015 7:00 AM, Stephen Smalley wrote:

On 12/14/2015 05:57 PM, Roberts, William C wrote:




If I understand correctly, the goal here is to avoid the lookup from
pid to context. If we somehow Had the context or a token to a context
during the ipc transaction to userspace, we could just use that In
computing the access decision. If that is correct, then since we have
PID, why not just extend the SE Linux compute av decision interface to support

passing of PID and then it can do the lookup in the Kernel?

That's no less racy than getpidcon().



I got a bounce from when I sent this from gmail, resending.

True, but in this case the binder transaction would be dead...

Why not just pass ctx? It's less than ideal, but it might be good enough for 
now until contexts get unwieldy big.

grep -rn '^type ' * | grep domain | cut -d' ' -f 2-2 | sed s/','//g | awk ' {  
thislen=length($0); printf("%-5s %dn", NR, thislen); totlen+=thislen}
END { printf("average: %dn", totlen/NR); } '

The avg type length for domain types in external/sepolicy is 7. Add the full 
ctx:

u:r:xxx:s0(cat)

1. We're looking at like 18 or so bytes, how do we know this won't be "fast 
enough"?
2. What's the current perf numbers, and what's agreed upon on what you need to 
hit to be fast enough?
3. I'm assuming the use case is in service manager, but would a userspace cache 
of AVD's help? Then you can (possibly) avoid both kernel trips, and you can 
invalidate the cache on policy reload and on PID deaths? In the case of service 
manager would it always be a miss based on usage pattern? I'm assuming things 
would say hey resolve this once, and be done. However, you could only do the 
ctx lookup and do the avd based on the policy in user space, thus avoiding 1 of 
two trips.


1. I don't think it is the size of the context that is the concern but rather 
the fact that it is a variable-length string, whereas current binder commands 
use fixed-size arguments and encode the size in the command value (common for 
ioctls).  Supporting passing a variable-length string would be a change to the 
protocol and would complicate the code.  On the performance side, it means 
generating a context string from the secid and then copying it to userspace on 
each IPC (versus just getting the secid and putting that in the existing 
binder_transaction_data that is already copied to userspace).


I have long wondered why SELinux generates the context string
of the secid more than once. Audit performance alone would
justify keeping it around. The variable length issue isn't
so difficult as you make it out. As William pointed out earlier,
most SELinux contexts are short. Two protocols, one with a
fixed length of 16 chars (typical is 7) and one with a fixed
length of 256 (for abnormal contexts) solve the problem without
complicating the code hardly at all.

If it's such a problem, why do we have SO_PEERSEC return a
variable length string? That's been around forever and seems
to work just fine.


Generating an audit record means you are already on the slow path, so 
adding the cost of generating a context at that point shouldn't be 
significant (but if one has performance data to the contrary, that would 
always be of interest). Keeping complete context strings in kernel 
memory at all times wasn't viewed as desirable, particularly when you 
consider the next point.


There is no internal limit on SELinux context size and one can in fact 
generate a SELinux context string that exceeds 256 bytes (just create a 
category set with many different, non-contiguous categories, e.g. a 
category set with every other category), so you can't just pick a couple 
of sizes (16 and 256 in your example) and be done with it.  It may be 
unusual to exceed 256 but it isn't impossible, and users of MLS systems 
have in fact run up against even the page size limitations of the 
/proc/self/attr interfaces as a pain point.


getsockopt SO_PEERSEC, as I mentioned earlier, is for stream sockets, so 
you are dealing with a connection-oriented model (with its attendant 
baseline overhead), server applications opt into getting the security 
label after the connection is established via getsockopt (which means 
that the overhead is not imposed on all connections, and which provides 
a straightforward way of passing variable-length data already), and all 
of the payload data is typically being copied through the kernel (to and 
from socket buffers).  With binder, we are talking about a synchronous 
IPC mechanism where the kernel copies the sender information (currently 
PID and UID) to the receiver on every IPC, the protocol currently passes 
no variable-length data, and the data is copied directly from the 
sender's memory into kernel-managed buffers that are mapped read-only 
into the receiver.  So introducing variable-length string copy to binder 
is a more substantial change, and the baseline perform

Re: Exposing secid to secctx mapping to user-space

[Stephen Smalley]

On 12/15/2015 12:19 PM, Joe Nall wrote:



On Dec 15, 2015, at 10:06 AM, Casey Schaufler  wrote:

...
I have long wondered why SELinux generates the context string
of the secid more than once. Audit performance alone would
justify keeping it around. The variable length issue isn't
so difficult as you make it out. As William pointed out earlier,
most SELinux contexts are short. Two protocols, one with a
fixed length of 16 chars (typical is 7) and one with a fixed
length of 256 (for abnormal contexts) solve the problem without
complicating the code hardly at all.


We have 'abnormal' contexts over 1024 bytes in production MLS systems. It is 
possible, though unlikely, to see raw contexts over 5k bytes with 1024 category 
bits. Thinking like this broke the original RHEL 5 racoon and more recently 
RHEL 6 openswan for us.

joe

system_u:system_r:silly_test_t:s2:c0,c2,c4,c6,c8,c10,c12,c14,c16,c18,c20,c22,c24,c26,c28,c30,c32,c34,c36,c38,c40,c42,c44,c46,c48,c50,c52,c54,c56,c58,c60,c62,c64,c66,c68,c70,c72,c74,c76,c78,c80,c82,c84,c86,c88,c90,c92,c94,c96,c98,c100,c102,c104,c106,c108,c110,c112,c114,c116,c118,c120,c122,c124,c126,c128,c130,c132,c134,c136,c138,c140,c142,c144,c146,c148,c150,c152,c154,c156,c158,c160,c162,c164,c166,c168,c170,c172,c174,c176,c178,c180,c182,c184,c186,c188,c190,c192,c194,c196,c198,c200,c202,c204,c206,c208,c210,c212,c214,c216,c218,c220,c222,c224,c226,c228,c230,c232,c234,c236,c238,c240,c242,c244,c246,c248,c250,c252,c254,c256,c258,c260,c262,c264,c266,c268,c270,c272,c274,c276,c278,c280,c282,c284,c286,c288,c290,c292,c294,c296,c298,c300,c302,c304,c306,c308,c310,c312,c314,c316,c318,c320,c322,c324,c326,c328,c330,c332,c334,c336,c338,c340,c342,c344,c346,c348,c350,c352,c354,c356,c358,c360,c362,c364,c366,c368,c370,c372,c374,c376,c378,c380,c382,c384,c386,c388,c390,c392,c394,c396,c398,c400,c402,c404,c4

0
6,c408,c410,c412,c414,c416,c418,c420,c422,c424,c426,c428,c430,c432,c434,c436,c438,c440,c442,c444,c446,c448,c450,c452,c454,c456,c458,c460,c462,c464,c466,c468,c470,c472,c474,c476,c478,c480,c482,c484,c486,c488,c490,c492,c494,c496,c498,c500,c502,c504,c506,c508,c510,c512,c514,c516,c518,c520,c522,c524,c526,c528,c530,c532,c534,c536,c538,c540,c542,c544,c546,c548,c550,c552,c554,c556,c558,c560,c562,c564,c566,c568,c570,c572,c574,c576,c578,c580,c582,c584,c586,c588,c590,c592,c594,c596,c598,c600,c602,c604,c606,c608,c610,c612,c614,c616,c618,c620,c622,c624,c626,c628,c630,c632,c634,c636,c638,c640,c642,c644,c646,c648,c650,c652,c654,c656,c658,c660,c662,c664,c666,c668,c670,c672,c674,c676,c678,c680,c682,c684,c686,c688,c690,c692,c694,c696,c698,c700,c702,c704,c706,c708,c710,c712,c714,c716,c718,c720,c722,c724,c726,c728,c730,c732,c734,c736,c738,c740,c742,c744,c746,c748,c750,c752,c754,c756,c758,c760,c762,c764,c766,c768,c770,c772,c774,c776,c778,c780,c782,c784,c786,c788,c790,c792,c794,c796,c798,c800,c802,c804,c
8
06,c808,c810,c812,c814,c816,c818,c820,c822,c824,c826,c828,c830,c832,c834,c836,c838,c840,c842,c844,c846,c848,c850,c852,c854,c856,c858,c860,c862,c864,c866,c868,c870,c872,c874,c876,c878,c880,c882,c884,c886,c888,c890,c892,c894,c896,c898,c900,c902,c904,c906,c908,c910,c912,c914,c916,c918,c920,c922,c924,c926,c928,c930,c932,c934,c936,c938,c940,c942,c944,c946,c948,c950,c952,c954,c956,c958,c960,c962,c964,c966,c968,c970,c972,c974,c976,c978,c980,c982,c984,c986,c988,c990,c992,c994,c996,c998,c1000,c1002,c1004,c1006,c1008,c1010,c1012,c1014,c1016,c1018,c1020,c1022:s2:c0,c2,c4,c6,c8,c10,c12,c14,c16,c18,c20,c22,c24,c26,c28,c30,c32,c34,c36,c38,c40,c42,c44,c46,c48,c50,c52,c54,c56,c58,c60,c62,c64,c66,c68,c70,c72,c74,c76,c78,c80,c82,c84,c86,c88,c90,c92,c94,c96,c98,c100,c102,c104,c106,c108,c110,c112,c114,c116,c118,c120,c122,c124,c126,c128,c130,c132,c134,c136,c138,c140,c142,c144,c146,c148,c150,c152,c154,c156,c158,c160,c162,c164,c166,c168,c170,c172,c174,c176,c178,c180,c182,c184,c186,c188,c190,c192,c194,c196,
c
198,c200,c202,c204,c206,c208,c210,c212,c214,c216,c218,c220,c222,c224,c226,c228,c230,c232,c234,c236,c238,c240,c242,c244,c246,c248,c250,c252,c254,c256,c258,c260,c262,c264,c266,c268,c270,c272,c274,c276,c278,c280,c282,c284,c286,c288,c290,c292,c294,c296,c298,c300,c302,c304,c306,c308,c310,c312,c314,c316,c318,c320,c322,c324,c326,c328,c330,c332,c334,c336,c338,c340,c342,c344,c346,c348,c350,c352,c354,c356,c358,c360,c362,c364,c366,c368,c370,c372,c374,c376,c378,c380,c382,c384,c386,c388,c390,c392,c394,c396,c398,c400,c402,c404,c406,c408,c410,c412,c414,c416,c418,c420,c422,c424,c426,c428,c430,c432,c434,c436,c438,c440,c442,c444,c446,c448,c450,c452,c454,c456,c458,c460,c462,c464,c466,c468,c470,c472,c474,c476,c478,c480,c482,c484,c486,c488,c490,c492,c494,c496,c498,c500,c502,c504,c506,c508,c510,c512,c514,c516,c518,c520,c522,c524,c526,c528,c530,c532,c534,c536,c538,c540,c542,c544,c546,c548,c550,c552,c554,c556,c558,c560,c562,c564,c566,c568,c570,c572,c574,c576,c578,c580,c582,c584,c586,c588,c590,c592,c594,c596
,

Re: Exposing secid to secctx mapping to user-space

[Stephen Smalley]

On 12/14/2015 05:57 PM, Roberts, William C wrote:




If I understand correctly, the goal here is to avoid the lookup from
pid to context. If we somehow Had the context or a token to a context
during the ipc transaction to userspace, we could just use that In
computing the access decision. If that is correct, then since we have
PID, why not just extend the SE Linux compute av decision interface to support

passing of PID and then it can do the lookup in the Kernel?

That's no less racy than getpidcon().



I got a bounce from when I sent this from gmail, resending.

True, but in this case the binder transaction would be dead...

Why not just pass ctx? It's less than ideal, but it might be good enough for 
now until contexts get unwieldy big.

grep -rn '^type ' * | grep domain | cut -d' ' -f 2-2 | sed s/','//g | awk ' {  
thislen=length($0); printf("%-5s %dn", NR, thislen); totlen+=thislen}
END { printf("average: %dn", totlen/NR); } '

The avg type length for domain types in external/sepolicy is 7. Add the full 
ctx:

u:r:xxx:s0(cat)

1. We're looking at like 18 or so bytes, how do we know this won't be "fast 
enough"?
2. What's the current perf numbers, and what's agreed upon on what you need to 
hit to be fast enough?
3. I'm assuming the use case is in service manager, but would a userspace cache 
of AVD's help? Then you can (possibly) avoid both kernel trips, and you can 
invalidate the cache on policy reload and on PID deaths? In the case of service 
manager would it always be a miss based on usage pattern? I'm assuming things 
would say hey resolve this once, and be done. However, you could only do the 
ctx lookup and do the avd based on the policy in user space, thus avoiding 1 of 
two trips.


1. I don't think it is the size of the context that is the concern but 
rather the fact that it is a variable-length string, whereas current 
binder commands use fixed-size arguments and encode the size in the 
command value (common for ioctls).  Supporting passing a variable-length 
string would be a change to the protocol and would complicate the code. 
 On the performance side, it means generating a context string from the 
secid and then copying it to userspace on each IPC (versus just getting 
the secid and putting that in the existing binder_transaction_data that 
is already copied to userspace).


2. Don't know; deferring to Daniel to run whatever binder IPC benchmarks 
might exist with and without the current patch that copies the context 
string.


3. It is for any binder-based service that wants to apply SELinux access 
checks, which presently includes servicemanager and keystore.
We already have a userspace AVC (in libselinux) that gets used 
automatically when you use selinux_check_access(), but you still need to 
get the sender security context in some manner.


--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Exposing secid to secctx mapping to user-space

[Stephen Smalley]

On 12/14/2015 12:03 PM, Mike Palmiotto wrote:

On Sun, Dec 13, 2015 at 5:06 PM, Paul Moore <p...@paul-moore.com> wrote:

On Friday, December 11, 2015 05:14:38 PM Stephen Smalley wrote:

Perhaps we could provide a new fixed-size tokenized version of the
security context string for export to userspace that could be embedded
in the binder transaction structure?  This could avoid both the
limitations of the current secid (e.g. limited to 32 bits, no
stackability) and the overhead of copying context strings on every IPC.


On Friday, December 11, 2015 04:24:48 PM Casey Schaufler wrote:

How about this: Provide an alias mechanism for secctx. There would then
be a secid (32bits) a secctx (arbitrary text string) and a secalias which
could be a limited string of some length. You could use the alias in place
of the secctx anywhere you liked.


My initial reaction to the secalias idea isn't overly positive.  It seems like
a kludge with a lot of duplication, both in terms of code and concept, and a
lot of risk for confusion both by users and policy writers.  I think if we
really wanted to limit the security label string format to a small size we
should have done that from the start, it's too late now.

Assuming we see some binder performance numbers, and the numbers are bad, I'm
a little more open to doing something with the secid token.  Up to this point
we haven't made any guarantees about the token and we haven't exported it
outside the kernel so there is some ability to change it to fit our needs.
Granted, this isn't perfect solution either, and perhaps ultimately we would
need something else, but I think it is worth looking into this first before we
introduce another string label.


Agreed here. I can definitely see a use for security identifier tokens
in SE Postgres as well. Ideally these tokens would be 32 bit uints as
opposed to shorter string aliases.


The userspace AVC provides its own SID mapping (man avc_context_to_sid, 
avc_has_perm), but that mapping is process-local (unlike kernel SIDs, 
which are global) and non-persistent (like kernel SIDs, which also 
aren't guaranteed to remain stable across reboot).  That's what is 
conventionally used by userspace object managers like dbus-daemon, 
Xorg/XSELinux, and SE-Postgres (although IIRC SE-Postgres rolled their 
own custom AVC in order to optimize it specifically for their needs). 
Those SIDs can be used for in-core objects, but you still need to store 
the security context strings for persistent objects, although you can 
obviously store each unique one only once and maintain your own indices 
(ala the persistent SIDs of the Flask and original SELinux labeled 
filesystem implementation).


--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Exposing secid to secctx mapping to user-space

[Stephen Smalley]

On 12/11/2015 02:55 PM, Paul Moore wrote:

On Fri, Dec 11, 2015 at 1:37 PM, Daniel Cashman  wrote:

Hello,

I would like to write a patch that would expose, via selinuxfs, the
mapping between secids in the kernel and security contexts to
user-space, but before doing so wanted to get some feedback as to
whether or not such an endeavor could have any support upstream.  The
direct motivation for this is the desire to communicate calling security
ids/contexts over binder IPC on android for use in a user-space object
manager.  Passing the security ids themselves would be simpler and more
efficient in the critical kernel path, but they currently have no
user-space meaning.


In general we try to avoid exposing the secid tokens outside the
kernel, I view them as an implementation hack designed to make it
easier to manage and operate on the security labels in the kernel.  I
suspect you will hear something very similar from Casey and the other
Smack developers.  Another consideration is the long standing LSM
stacking effort, they have several good reasons for wanting to abolish
the secid token, propagating it to userspace would make that all but
impossible.

While I'm sympathetic to your desire for less complexity and better
performance in passing security labels, from a kernel perspective I
think we lose too much in exporting the secid tokens outside the LSM.


To clarify, security identifiers were by design provided in the Flask 
architecture and SELinux as lightweight, non-persistent handles to 
security contexts, and exposed to userspace by the original SELinux API 
(pre-2.6, of course).  They were only removed when we transitioned to 
using extended attributes and the xattr API for file security labels, 
and we made all of the other APIs consistent as passing context strings 
seemed acceptable for proc and selinuxfs as well.  There was some 
thought to turning SIDs into proper reference-counted objects or even 
wrapping them with descriptors so that their lifecycles could be fully 
managed by the kernel, but that never happened.


Passing a security context string on every binder IPC may be too costly 
from a performance point of view (numbers would be helpful here).  I 
think this situation differs from that of stream sockets (i.e. 
getsockopt SO_PEERSEC), since they are looking at enabling passing of 
sender security label for every binder IPC (not just specific 
applications) and since binder IPC is quite different from stream socket 
IPC in its semantics and its performance.


Perhaps we could provide a new fixed-size tokenized version of the 
security context string for export to userspace that could be embedded 
in the binder transaction structure?  This could avoid both the 
limitations of the current secid (e.g. limited to 32 bits, no 
stackability) and the overhead of copying context strings on every IPC.

--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 6/7] selinux: Revalidate invalid inode security labels

[Stephen Smalley]

On 11/02/2015 02:27 PM, Paul Moore wrote:

On Sunday, November 01, 2015 06:24:32 PM Andreas Gruenbacher wrote:

When fetching an inode's security label, check if it is still valid, and
try reloading it if it is not. Reloading will fail when we are in RCU
context which doesn't allow sleeping, or when we can't find a dentry for
the inode.  (Reloading happens via iop->getxattr which takes a dentry
parameter.)  When reloading fails, continue using the old, invalid
label.

Signed-off-by: Andreas Gruenbacher <agrue...@redhat.com>
Acked-by: Stephen Smalley <s...@tycho.nsa.gov>


Generally I would say that you made enough changes between v4 and v5 that
Stephen's ACK should have been dropped, but considering that he suggested the
changes I think's it's okay to leave it as-is.

Stephen, I'm reviewing/merging these changes now for the selinux#next-queue,
speak up if you have want your ACK removed.


Revised patch looks fine to me; no need to remove the Acked-by.




diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index dcac6dc..0f94c2d 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -241,11 +241,63 @@ static int inode_alloc_security(struct inode *inode)
return 0;
  }

+static int inode_doinit_with_dentry(struct inode *inode, struct dentry
*opt_dentry); +
+/*
+ * Try reloading inode security labels that have been marked as invalid.
The + * @may_sleep parameter indicates when sleeping and thus reloading
labels is + * allowed; when set to false, returns ERR_PTR(-ECHILD) when the
label is + * invalid.  The @opt_dentry parameter should be set to a dentry
of the inode; + * when no dentry is available, set it to NULL instead.
+ */
+static int __inode_security_revalidate(struct inode *inode,
+  struct dentry *opt_dentry,
+  bool may_sleep)
+{
+   struct inode_security_struct *isec = inode->i_security;
+
+   might_sleep_if(may_sleep);
+
+   if (isec->initialized == LABEL_INVALID) {
+   if (!may_sleep)
+   return -ECHILD;
+
+   /*
+* Try reloading the inode security label.  This will fail if
+* @opt_dentry is NULL and no dentry for this inode can be
+* found; in that case, continue using the old label.
+*/
+   inode_doinit_with_dentry(inode, opt_dentry);
+   }
+   return 0;
+}
+
+static void inode_security_revalidate(struct inode *inode)
+{
+   __inode_security_revalidate(inode, NULL, true);
+}
+
+static struct inode_security_struct *inode_security_novalidate(struct inode
*inode) +{
+   return inode->i_security;
+}
+
+static struct inode_security_struct *inode_security_rcu(struct inode
*inode, bool rcu) +{
+   int error;
+
+   error = __inode_security_revalidate(inode, NULL, !rcu);
+   if (error)
+   return ERR_PTR(error);
+   return inode->i_security;
+}
+
  /*
   * Get the security label of an inode.
   */
  static struct inode_security_struct *inode_security(struct inode *inode)
  {
+   __inode_security_revalidate(inode, NULL, true);
return inode->i_security;
  }

@@ -256,6 +308,7 @@ static struct inode_security_struct
*backing_inode_security(struct dentry *dentr {
struct inode *inode = d_backing_inode(dentry);

+   __inode_security_revalidate(inode, dentry, true);
return inode->i_security;
  }

@@ -362,8 +415,6 @@ static const char *labeling_behaviors[7] = {
"uses native labeling",
  };

-static int inode_doinit_with_dentry(struct inode *inode, struct dentry
*opt_dentry); -
  static inline int inode_doinit(struct inode *inode)
  {
return inode_doinit_with_dentry(inode, NULL);
@@ -1655,6 +1706,7 @@ static inline int dentry_has_perm(const struct cred
*cred,

ad.type = LSM_AUDIT_DATA_DENTRY;
ad.u.dentry = dentry;
+   __inode_security_revalidate(inode, dentry, true);
return inode_has_perm(cred, inode, av, );
  }

@@ -1670,6 +1722,7 @@ static inline int path_has_perm(const struct cred
*cred,

ad.type = LSM_AUDIT_DATA_PATH;
ad.u.path = *path;
+   __inode_security_revalidate(inode, path->dentry, true);
return inode_has_perm(cred, inode, av, );
  }

@@ -2874,7 +2927,9 @@ static int selinux_inode_follow_link(struct dentry
*dentry, struct inode *inode, ad.type = LSM_AUDIT_DATA_DENTRY;
ad.u.dentry = dentry;
sid = cred_sid(cred);
-   isec = inode_security(inode);
+   isec = inode_security_rcu(inode, rcu);
+   if (IS_ERR(isec))
+   return PTR_ERR(isec);

return avc_has_perm_flags(sid, isec->sid, isec->sclass, FILE__READ, ,
  rcu ? MAY_NOT_BLOCK : 0);
@@ -2926,7 +2981,9 @@ static int selinux_inode_permission(struct inode
*inode, int mask) perms = file_mask_to_av(inode->i_mode, mask);

si

Re: [PATCH v4 3/7] security: Make inode argument of inode_getsecid non-const

[Stephen Smalley]

On 10/28/2015 08:47 PM, Andreas Gruenbacher wrote:

Make the inode argument of the inode_getsecid hook non-const so that we
can use it to revalidate invalid security labels.

Signed-off-by: Andreas Gruenbacher <agrue...@redhat.com>


Acked-by: Stephen Smalley <s...@tycho.nsa.gov>


---
  include/linux/audit.h  | 8 
  include/linux/lsm_hooks.h  | 2 +-
  include/linux/security.h   | 4 ++--
  kernel/audit.c | 2 +-
  kernel/audit.h | 2 +-
  kernel/auditsc.c   | 6 +++---
  security/security.c| 2 +-
  security/selinux/hooks.c   | 2 +-
  security/smack/smack_lsm.c | 2 +-
  9 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index b2abc99..7a9e0d7 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -137,7 +137,7 @@ extern void __audit_getname(struct filename *name);
  extern void __audit_inode(struct filename *name, const struct dentry *dentry,
unsigned int flags);
  extern void __audit_file(const struct file *);
-extern void __audit_inode_child(const struct inode *parent,
+extern void __audit_inode_child(struct inode *parent,
const struct dentry *dentry,
const unsigned char type);
  extern void __audit_seccomp(unsigned long syscall, long signr, int code);
@@ -202,7 +202,7 @@ static inline void audit_inode_parent_hidden(struct 
filename *name,
__audit_inode(name, dentry,
AUDIT_INODE_PARENT | AUDIT_INODE_HIDDEN);
  }
-static inline void audit_inode_child(const struct inode *parent,
+static inline void audit_inode_child(struct inode *parent,
 const struct dentry *dentry,
 const unsigned char type) {
if (unlikely(!audit_dummy_context()))
@@ -359,7 +359,7 @@ static inline void __audit_inode(struct filename *name,
const struct dentry *dentry,
unsigned int flags)
  { }
-static inline void __audit_inode_child(const struct inode *parent,
+static inline void __audit_inode_child(struct inode *parent,
const struct dentry *dentry,
const unsigned char type)
  { }
@@ -373,7 +373,7 @@ static inline void audit_file(struct file *file)
  static inline void audit_inode_parent_hidden(struct filename *name,
const struct dentry *dentry)
  { }
-static inline void audit_inode_child(const struct inode *parent,
+static inline void audit_inode_child(struct inode *parent,
 const struct dentry *dentry,
 const unsigned char type)
  { }
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index bdd0a3a..4c48227 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1420,7 +1420,7 @@ union security_list_options {
int flags);
int (*inode_listsecurity)(struct inode *inode, char *buffer,
size_t buffer_size);
-   void (*inode_getsecid)(const struct inode *inode, u32 *secid);
+   void (*inode_getsecid)(struct inode *inode, u32 *secid);

int (*file_permission)(struct file *file, int mask);
int (*file_alloc_security)(struct file *file);
diff --git a/include/linux/security.h b/include/linux/security.h
index 9ee61b2..e79149a 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -273,7 +273,7 @@ int security_inode_killpriv(struct dentry *dentry);
  int security_inode_getsecurity(struct inode *inode, const char *name, void 
**buffer, bool alloc);
  int security_inode_setsecurity(struct inode *inode, const char *name, const 
void *value, size_t size, int flags);
  int security_inode_listsecurity(struct inode *inode, char *buffer, size_t 
buffer_size);
-void security_inode_getsecid(const struct inode *inode, u32 *secid);
+void security_inode_getsecid(struct inode *inode, u32 *secid);
  int security_file_permission(struct file *file, int mask);
  int security_file_alloc(struct file *file);
  void security_file_free(struct file *file);
@@ -734,7 +734,7 @@ static inline int security_inode_listsecurity(struct inode 
*inode, char *buffer,
return 0;
  }

-static inline void security_inode_getsecid(const struct inode *inode, u32 
*secid)
+static inline void security_inode_getsecid(struct inode *inode, u32 *secid)
  {
*secid = 0;
  }
diff --git a/kernel/audit.c b/kernel/audit.c
index 662c007..d20f674 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -1708,7 +1708,7 @@ static inline int audit_copy_fcaps(struct audit_names 
*name,

  /* Copy inode data into an audit_names. */
  void audit_copy_inode(struct audit_names *name, const struct dentry *dentry,
-  

Re: [PATCH v4 4/7] selinux: Add accessor functions for inode->i_security

[Stephen Smalley]

On 10/28/2015 08:47 PM, Andreas Gruenbacher wrote:

Add functions dentry_security and inode_security for accessing
inode->i_security.  These functions initially don't do much, but they
will later be used to revalidate the security labels when necessary.

Signed-off-by: Andreas Gruenbacher <agrue...@redhat.com>


Acked-by: Stephen Smalley <s...@tycho.nsa.gov>


---
  security/selinux/hooks.c | 97 
  1 file changed, 56 insertions(+), 41 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index a8f09af..48d1908 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -241,6 +241,24 @@ static int inode_alloc_security(struct inode *inode)
return 0;
  }

+/*
+ * Get the security label of an inode.
+ */
+static struct inode_security_struct *inode_security(struct inode *inode)
+{
+   return inode->i_security;
+}
+
+/*
+ * Get the security label of a dentry's backing inode.
+ */
+static struct inode_security_struct *backing_inode_security(struct dentry 
*dentry)
+{
+   struct inode *inode = d_backing_inode(dentry);
+
+   return inode->i_security;
+}
+
  static void inode_free_rcu(struct rcu_head *head)
  {
struct inode_security_struct *isec;
@@ -564,8 +582,8 @@ static int selinux_get_mnt_opts(const struct super_block 
*sb,
opts->mnt_opts_flags[i++] = DEFCONTEXT_MNT;
}
if (sbsec->flags & ROOTCONTEXT_MNT) {
-   struct inode *root = d_backing_inode(sbsec->sb->s_root);
-   struct inode_security_struct *isec = root->i_security;
+   struct dentry *root = sbsec->sb->s_root;
+   struct inode_security_struct *isec = 
backing_inode_security(root);

rc = security_sid_to_context(isec->sid, , );
if (rc)
@@ -620,8 +638,8 @@ static int selinux_set_mnt_opts(struct super_block *sb,
int rc = 0, i;
struct superblock_security_struct *sbsec = sb->s_security;
const char *name = sb->s_type->name;
-   struct inode *inode = d_backing_inode(sbsec->sb->s_root);
-   struct inode_security_struct *root_isec = inode->i_security;
+   struct dentry *root = sbsec->sb->s_root;
+   struct inode_security_struct *root_isec = backing_inode_security(root);
u32 fscontext_sid = 0, context_sid = 0, rootcontext_sid = 0;
u32 defcontext_sid = 0;
char **mount_options = opts->mnt_opts;
@@ -852,8 +870,8 @@ static int selinux_cmp_sb_context(const struct super_block 
*oldsb,
if ((oldflags & DEFCONTEXT_MNT) && old->def_sid != new->def_sid)
goto mismatch;
if (oldflags & ROOTCONTEXT_MNT) {
-   struct inode_security_struct *oldroot = 
d_backing_inode(oldsb->s_root)->i_security;
-   struct inode_security_struct *newroot = 
d_backing_inode(newsb->s_root)->i_security;
+   struct inode_security_struct *oldroot = 
backing_inode_security(oldsb->s_root);
+   struct inode_security_struct *newroot = 
backing_inode_security(newsb->s_root);
if (oldroot->sid != newroot->sid)
goto mismatch;
}
@@ -903,17 +921,14 @@ static int selinux_sb_clone_mnt_opts(const struct 
super_block *oldsb,
if (!set_fscontext)
newsbsec->sid = sid;
if (!set_rootcontext) {
-   struct inode *newinode = d_backing_inode(newsb->s_root);
-   struct inode_security_struct *newisec = 
newinode->i_security;
+   struct inode_security_struct *newisec = 
backing_inode_security(newsb->s_root);
newisec->sid = sid;
}
newsbsec->mntpoint_sid = sid;
}
if (set_rootcontext) {
-   const struct inode *oldinode = d_backing_inode(oldsb->s_root);
-   const struct inode_security_struct *oldisec = 
oldinode->i_security;
-   struct inode *newinode = d_backing_inode(newsb->s_root);
-   struct inode_security_struct *newisec = newinode->i_security;
+   const struct inode_security_struct *oldisec = 
backing_inode_security(oldsb->s_root);
+   struct inode_security_struct *newisec = 
backing_inode_security(newsb->s_root);

newisec->sid = oldisec->sid;
}
@@ -1712,13 +1727,13 @@ out:
  /*
   * Determine the label for an inode that might be unioned.
   */
-static int selinux_determine_inode_label(const struct inode *dir,
+static int selinux_determine_inode_label(struct inode *dir,
 const struct qstr *name,
 u16 tclass,
 u32 *_new_isid)
  {
const struct super

Re: [PATCH v4 6/7] selinux: Revalidate invalid inode security labels

[Stephen Smalley]

On 10/28/2015 08:47 PM, Andreas Gruenbacher wrote:

When fetching an inode's security label, check if it is still valid, and
try reloading it if it is not. Reloading will fail when we are in RCU
context which doesn't allow sleeping, or when we can't find a dentry for
the inode.  (Reloading happens via iop->getxattr which takes a dentry
parameter.)  When reloading fails, continue using the old, invalid
label.

Signed-off-by: Andreas Gruenbacher <agrue...@redhat.com>


Could probably use inode_security_novalidate() for all of the 
SOCK_INODE() cases, right?  Otherwise,

Acked-by: Stephen Smalley <s...@tycho.nsa.gov>


---
  security/selinux/hooks.c | 70 
  1 file changed, 65 insertions(+), 5 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index dcac6dc..47dc704 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -241,11 +241,63 @@ static int inode_alloc_security(struct inode *inode)
return 0;
  }

+static int inode_doinit_with_dentry(struct inode *inode, struct dentry 
*opt_dentry);
+
+/*
+ * Try reloading inode security labels that have been marked as invalid.  The
+ * @may_sleep parameter indicates when sleeping and thus reloading labels is
+ * allowed; when set to false, returns ERR_PTR(-ECHILD) when the label is
+ * invalid.  The @opt_dentry parameter should be set to a dentry of the inode;
+ * when no dentry is available, set it to NULL instead.
+ */
+static int __inode_security_revalidate(struct inode *inode,
+  struct dentry *opt_dentry,
+  bool may_sleep)
+{
+   struct inode_security_struct *isec = inode->i_security;
+
+   might_sleep_if(may_sleep);
+
+   if (isec->initialized == LABEL_INVALID) {
+   if (!may_sleep)
+   return -ECHILD;
+
+   /*
+* Try reloading the inode security label.  This will fail if
+* @opt_dentry is NULL and no dentry for this inode can be
+* found; in that case, continue using the old label.
+*/
+   inode_doinit_with_dentry(inode, opt_dentry);
+   }
+   return 0;
+}
+
+static void inode_security_revalidate(struct inode *inode)
+{
+   __inode_security_revalidate(inode, NULL, true);
+}
+
+static struct inode_security_struct *inode_security_novalidate(struct inode 
*inode)
+{
+   return inode->i_security;
+}
+
+static struct inode_security_struct *inode_security_rcu(struct inode *inode, 
bool rcu)
+{
+   int error;
+
+   error = __inode_security_revalidate(inode, NULL, !rcu);
+   if (error)
+   return ERR_PTR(error);
+   return inode->i_security;
+}
+
  /*
   * Get the security label of an inode.
   */
  static struct inode_security_struct *inode_security(struct inode *inode)
  {
+   __inode_security_revalidate(inode, NULL, true);
return inode->i_security;
  }

@@ -256,6 +308,7 @@ static struct inode_security_struct 
*backing_inode_security(struct dentry *dentr
  {
struct inode *inode = d_backing_inode(dentry);

+   __inode_security_revalidate(inode, dentry, true);
return inode->i_security;
  }

@@ -362,8 +415,6 @@ static const char *labeling_behaviors[7] = {
"uses native labeling",
  };

-static int inode_doinit_with_dentry(struct inode *inode, struct dentry 
*opt_dentry);
-
  static inline int inode_doinit(struct inode *inode)
  {
return inode_doinit_with_dentry(inode, NULL);
@@ -1655,6 +1706,7 @@ static inline int dentry_has_perm(const struct cred *cred,

ad.type = LSM_AUDIT_DATA_DENTRY;
ad.u.dentry = dentry;
+   __inode_security_revalidate(inode, dentry, true);
return inode_has_perm(cred, inode, av, );
  }

@@ -1670,6 +1722,7 @@ static inline int path_has_perm(const struct cred *cred,

ad.type = LSM_AUDIT_DATA_PATH;
ad.u.path = *path;
+   __inode_security_revalidate(inode, path->dentry, true);
return inode_has_perm(cred, inode, av, );
  }

@@ -2874,7 +2927,9 @@ static int selinux_inode_follow_link(struct dentry 
*dentry, struct inode *inode,
ad.type = LSM_AUDIT_DATA_DENTRY;
ad.u.dentry = dentry;
sid = cred_sid(cred);
-   isec = inode_security(inode);
+   isec = inode_security_rcu(inode, rcu);
+   if (IS_ERR(isec))
+   return PTR_ERR(isec);

return avc_has_perm_flags(sid, isec->sid, isec->sclass, FILE__READ, ,
  rcu ? MAY_NOT_BLOCK : 0);
@@ -2926,7 +2981,9 @@ static int selinux_inode_permission(struct inode *inode, 
int mask)
perms = file_mask_to_av(inode->i_mode, mask);

sid = cred_sid(cred);
-   isec = inode_security(inode);
+   isec = inode_security_rcu(inode, flags & MAY_NOT_BLOCK);
+   if (IS_ERR(isec))
+   return PTR_ERR(isec);

   

Re: [PATCH v4 5/7] security: Add hook to invalidate inode security labels

[Stephen Smalley]

On 10/28/2015 08:47 PM, Andreas Gruenbacher wrote:

Add a hook to invalidate an inode's security label when the cached
information becomes invalid.

Add the new hook in selinux: set a flag when a security label becomes
invalid.

Signed-off-by: Andreas Gruenbacher <agrue...@redhat.com>
Reviewed-by: James Morris <james.l.mor...@oracle.com>


Acked-by: Stephen Smalley <s...@tycho.nsa.gov>


---
  include/linux/lsm_hooks.h |  6 ++
  include/linux/security.h  |  5 +
  security/security.c   |  8 
  security/selinux/hooks.c  | 30 --
  security/selinux/include/objsec.h |  6 ++
  5 files changed, 45 insertions(+), 10 deletions(-)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 4c48227..71969de 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1261,6 +1261,10 @@
   *audit_rule_init.
   *@rule contains the allocated rule
   *
+ * @inode_invalidate_secctx:
+ * Notify the security module that it must revalidate the security context
+ * of an inode.
+ *
   * @inode_notifysecctx:
   *Notify the security module of what the security context of an inode
   *should be.  Initializes the incore security context managed by the
@@ -1516,6 +1520,7 @@ union security_list_options {
int (*secctx_to_secid)(const char *secdata, u32 seclen, u32 *secid);
void (*release_secctx)(char *secdata, u32 seclen);

+   void (*inode_invalidate_secctx)(struct inode *inode);
int (*inode_notifysecctx)(struct inode *inode, void *ctx, u32 ctxlen);
int (*inode_setsecctx)(struct dentry *dentry, void *ctx, u32 ctxlen);
int (*inode_getsecctx)(struct inode *inode, void **ctx, u32 *ctxlen);
@@ -1757,6 +1762,7 @@ struct security_hook_heads {
struct list_head secid_to_secctx;
struct list_head secctx_to_secid;
struct list_head release_secctx;
+   struct list_head inode_invalidate_secctx;
struct list_head inode_notifysecctx;
struct list_head inode_setsecctx;
struct list_head inode_getsecctx;
diff --git a/include/linux/security.h b/include/linux/security.h
index e79149a..4824a4c 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -353,6 +353,7 @@ int security_secid_to_secctx(u32 secid, char **secdata, u32 
*seclen);
  int security_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid);
  void security_release_secctx(char *secdata, u32 seclen);

+void security_inode_invalidate_secctx(struct inode *inode);
  int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen);
  int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen);
  int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen);
@@ -1093,6 +1094,10 @@ static inline void security_release_secctx(char 
*secdata, u32 seclen)
  {
  }

+static inline void security_inode_invalidate_secctx(struct inode *inode)
+{
+}
+
  static inline int security_inode_notifysecctx(struct inode *inode, void *ctx, 
u32 ctxlen)
  {
return -EOPNOTSUPP;
diff --git a/security/security.c b/security/security.c
index c5beb7e..e8ffd92 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1161,6 +1161,12 @@ void security_release_secctx(char *secdata, u32 seclen)
  }
  EXPORT_SYMBOL(security_release_secctx);

+void security_inode_invalidate_secctx(struct inode *inode)
+{
+   call_void_hook(inode_invalidate_secctx, inode);
+}
+EXPORT_SYMBOL(security_inode_invalidate_secctx);
+
  int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen)
  {
return call_int_hook(inode_notifysecctx, 0, inode, ctx, ctxlen);
@@ -1763,6 +1769,8 @@ struct security_hook_heads security_hook_heads = {
LIST_HEAD_INIT(security_hook_heads.secctx_to_secid),
.release_secctx =
LIST_HEAD_INIT(security_hook_heads.release_secctx),
+   .inode_invalidate_secctx =
+   LIST_HEAD_INIT(security_hook_heads.inode_invalidate_secctx),
.inode_notifysecctx =
LIST_HEAD_INIT(security_hook_heads.inode_notifysecctx),
.inode_setsecctx =
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 48d1908..dcac6dc 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -820,7 +820,7 @@ static int selinux_set_mnt_opts(struct super_block *sb,
goto out;

root_isec->sid = rootcontext_sid;
-   root_isec->initialized = 1;
+   root_isec->initialized = LABEL_INITIALIZED;
}

if (defcontext_sid) {
@@ -1308,11 +1308,11 @@ static int inode_doinit_with_dentry(struct inode 
*inode, struct dentry *opt_dent
unsigned len = 0;
int rc = 0;

-   if (isec->initialized)
+   if (isec->initialized == LABEL_INITIALIZED)
goto out;

mutex_lock(>lock);
-   if (isec->ini

Re: [PATCH v3 3/7] selinux: Get rid of file_path_has_perm

[Stephen Smalley]

On 10/28/2015 07:48 AM, Andreas Gruenbacher wrote:
> On Tue, Oct 27, 2015 at 5:40 PM, Stephen Smalley <s...@tycho.nsa.gov> wrote:
>> On 10/26/2015 05:15 PM, Andreas Gruenbacher wrote:
>>>
>>> Use path_has_perm directly instead.
>>
>>
>> This reverts:
>>
>> commit 13f8e9810bff12d01807b6f92329111f45218235
>> Author: David Howells <dhowe...@redhat.com>
>> Date:   Thu Jun 13 23:37:55 2013 +0100
>>
>> SELinux: Institute file_path_has_perm()
>>
>> Create a file_path_has_perm() function that is like path_has_perm() but
>> instead takes a file struct that is the source of both the path and the
>> inode (rather than getting the inode from the dentry in the path).  This
>> is then used where appropriate.
>>
>> This will be useful for situations like unionmount where it will be
>> possible to have an apparently-negative dentry (eg. a fallthrough) that
>> is
>> open with the file struct pointing to an inode on the lower fs.
>>
>> Signed-off-by: David Howells <dhowe...@redhat.com>
>> Signed-off-by: Al Viro <v...@zeniv.linux.org.uk>
>>
>> which I think David was intending to use as part of his SELinux/overlayfs
>> support.
> 
> Okay. As long as overlayfs support in SELinux is in half-finished
> state, let's leave this alone.

Also, the caller is holding a spinlock (tty_files_lock), so you can't call 
inode_doinit from
here.

Try stress testing your patch series by just always setting isec->initialized 
to LABEL_INVALID.
Previously the *has_perm functions could be called under essentially any 
condition, with the exception
of when in a RCU walk and needing to audit the dname (but they did not 
previously block/sleep).



--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 3/7] selinux: Get rid of file_path_has_perm

[Stephen Smalley]

On 10/28/2015 01:31 PM, Stephen Smalley wrote:

On 10/28/2015 07:48 AM, Andreas Gruenbacher wrote:

On Tue, Oct 27, 2015 at 5:40 PM, Stephen Smalley <s...@tycho.nsa.gov> wrote:

On 10/26/2015 05:15 PM, Andreas Gruenbacher wrote:


Use path_has_perm directly instead.



This reverts:

commit 13f8e9810bff12d01807b6f92329111f45218235
Author: David Howells <dhowe...@redhat.com>
Date:   Thu Jun 13 23:37:55 2013 +0100

 SELinux: Institute file_path_has_perm()

 Create a file_path_has_perm() function that is like path_has_perm() but
 instead takes a file struct that is the source of both the path and the
 inode (rather than getting the inode from the dentry in the path).  This
 is then used where appropriate.

 This will be useful for situations like unionmount where it will be
 possible to have an apparently-negative dentry (eg. a fallthrough) that
is
 open with the file struct pointing to an inode on the lower fs.

 Signed-off-by: David Howells <dhowe...@redhat.com>
 Signed-off-by: Al Viro <v...@zeniv.linux.org.uk>

which I think David was intending to use as part of his SELinux/overlayfs
support.


Okay. As long as overlayfs support in SELinux is in half-finished
state, let's leave this alone.


Also, the caller is holding a spinlock (tty_files_lock), so you can't call 
inode_doinit from
here.

Try stress testing your patch series by just always setting isec->initialized 
to LABEL_INVALID.
Previously the *has_perm functions could be called under essentially any 
condition, with the exception
of when in a RCU walk and needing to audit the dname (but they did not 
previously block/sleep).


file_has_perm() also gets called from match_file() callback to 
iterate_fd(), which holds files->file_lock.




--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] selinux: export validatetrans decisions

[Stephen Smalley]

On 10/27/2015 04:48 PM, Andrew Perepechko wrote:

Make validatetrans decisions available through selinuxfs.
"/validatetrans" is added to selinuxfs for this purpose.
This functionality is needed by file system servers
implemented in userspace or kernelspace without the VFS
layer.

Writing "$oldcontext $newcontext $tclass $taskcontext"
to /validatetrans is expected to return 0 if the transition
is allowed and -EPERM otherwise.

Signed-off-by: Andrew Perepechko 
CC: andrew.perepec...@seagate.com
---
  security/selinux/include/classmap.h |2 +-
  security/selinux/include/security.h |3 +
  security/selinux/selinuxfs.c|   79 +++
  security/selinux/ss/services.c  |   41 ++
  4 files changed, 114 insertions(+), 11 deletions(-)

diff --git a/security/selinux/include/classmap.h 
b/security/selinux/include/classmap.h
index 5a4eef5..ef83c4b 100644
--- a/security/selinux/include/classmap.h
+++ b/security/selinux/include/classmap.h
@@ -21,7 +21,7 @@ struct security_class_mapping secclass_map[] = {
  { "compute_av", "compute_create", "compute_member",
"check_context", "load_policy", "compute_relabel",
"compute_user", "setenforce", "setbool", "setsecparam",
-   "setcheckreqprot", "read_policy", NULL } },
+   "setcheckreqprot", "read_policy", "validate_trans", NULL } },
{ "process",
  { "fork", "transition", "sigchld", "sigkill",
"sigstop", "signull", "signal", "ptrace", "getsched", "setsched",
diff --git a/security/selinux/include/security.h 
b/security/selinux/include/security.h
index 223e9fd..38feb55 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -187,6 +187,9 @@ int security_node_sid(u16 domain, void *addr, u32 addrlen,
  int security_validate_transition(u32 oldsid, u32 newsid, u32 tasksid,
 u16 tclass);

+int security_validate_transition_user(u32 oldsid, u32 newsid, u32 tasksid,
+ u16 tclass);
+
  int security_bounded_transition(u32 oldsid, u32 newsid);

  int security_sid_mls_copy(u32 sid, u32 mls_sid, u32 *new_sid);
diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index c02da25..e460b4e 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -116,6 +116,7 @@ enum sel_inos {
SEL_DENY_UNKNOWN, /* export unknown deny handling to userspace */
SEL_STATUS, /* export current status using mmap() */
SEL_POLICY, /* allow userspace to read the in kernel policy */
+   SEL_VALIDATE_TRANS, /* compute validatetrans decision */
SEL_INO_NEXT,   /* The next inode number to use */
  };

@@ -653,6 +654,83 @@ static const struct file_operations sel_checkreqprot_ops = 
{
.llseek = generic_file_llseek,
  };

+static ssize_t sel_write_validatetrans(struct file *file,
+   const char __user *buf,
+   size_t count, loff_t *ppos)
+{
+   char *oldcon = NULL, *newcon = NULL, *taskcon = NULL;
+   char *req = NULL;
+   u32 osid, nsid, tsid;
+   u16 tclass;
+   int rc;
+
+   rc = task_has_security(current, SECURITY__VALIDATE_TRANS);
+   if (rc)
+   goto out;
+
+   rc = -ENOMEM;
+   if (count >= PAGE_SIZE)
+   goto out;
+
+   /* No partial writes. */
+   rc = -EINVAL;
+   if (*ppos != 0)
+   goto out;
+
+   rc = -ENOMEM;
+   req = kzalloc(count + 1, GFP_KERNEL);
+   if (!req)
+   goto out;
+
+   rc = -EFAULT;
+   if (copy_from_user(req, buf, count))
+   goto out;
+
+   rc = -ENOMEM;
+   oldcon = kzalloc(count + 1, GFP_KERNEL);
+   if (!oldcon)
+   goto out;
+
+   newcon = kzalloc(count + 1, GFP_KERNEL);
+   if (!newcon)
+   goto out;
+
+   taskcon = kzalloc(count + 1, GFP_KERNEL);
+   if (!taskcon)
+   goto out;
+
+   rc = -EINVAL;
+   if (sscanf(req, "%s %s %hu %s", oldcon, newcon, , taskcon) != 4)
+   goto out;
+
+   rc = security_context_str_to_sid(oldcon, , GFP_KERNEL);
+   if (rc)
+   goto out;
+
+   rc = security_context_str_to_sid(newcon, , GFP_KERNEL);
+   if (rc)
+   goto out;
+
+   rc = security_context_str_to_sid(taskcon, , GFP_KERNEL);
+   if (rc)
+   goto out;
+
+   rc = security_validate_transition_user(osid, nsid, tsid, tclass);
+   if (!rc)
+   rc = count;
+out:
+   kfree(req);
+   kfree(oldcon);
+   kfree(newcon);
+   kfree(taskcon);
+   return rc;
+}
+
+static const struct file_operations sel_validatetrans_ops = {
+   .write  = sel_write_validatetrans,
+   .llseek = generic_file_llseek,
+};
+
  /*
   * Remaining nodes use transaction based IO methods like 

Re: [PATCH v3 0/7] Inode security label invalidation

[Stephen Smalley]

On 10/26/2015 05:15 PM, Andreas Gruenbacher wrote:

Here is another version of the patch queue to make gfs2 and similar file
systems work with SELinux.  As suggested by Stephen Smalley [*], the relevant
uses of inode->security are wrapped in function calls that try to revalidate
invalid labels.

   [*] http://marc.info/?l=linux-kernel=144416710207686=2

The patches are looking good from my point of view; is there anything else that
needs addressing?

Does SELinux have test suites that these patches could be tested agains?


git clone https://github.com/SELinuxProject/selinux-testsuite
sudo yum install perl-Test perl-Test-Harness selinux-policy-devel gcc 
libselinux-devel net-tools netlabel_tools iptables

cd selinux-testsuite
sudo make test



Thanks,
Andreas

Andreas Gruenbacher (7):
   selinux: Remove unused variable in selinux_inode_init_security
   selinux: Add accessor functions for inode->i_security
   selinux: Get rid of file_path_has_perm
   selinux: Push dentry down from {dentry,path,file}_has_perm
   security: Add hook to invalidate inode security labels
   selinux: Revalidate invalid inode security labels
   gfs2: Invalide security labels of inodes when they go invalid

  fs/gfs2/glops.c   |   2 +
  include/linux/lsm_hooks.h |   6 ++
  include/linux/security.h  |   5 +
  security/security.c   |   8 ++
  security/selinux/hooks.c  | 213 ++
  security/selinux/include/objsec.h |   6 ++
  6 files changed, 152 insertions(+), 88 deletions(-)



--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 3/7] selinux: Get rid of file_path_has_perm

[Stephen Smalley]

On 10/26/2015 05:15 PM, Andreas Gruenbacher wrote:

Use path_has_perm directly instead.


This reverts:

commit 13f8e9810bff12d01807b6f92329111f45218235
Author: David Howells 
Date:   Thu Jun 13 23:37:55 2013 +0100

SELinux: Institute file_path_has_perm()

Create a file_path_has_perm() function that is like path_has_perm() but
instead takes a file struct that is the source of both the path and the
inode (rather than getting the inode from the dentry in the path). 
 This

is then used where appropriate.

This will be useful for situations like unionmount where it will be
possible to have an apparently-negative dentry (eg. a fallthrough) 
that is

open with the file struct pointing to an inode on the lower fs.

Signed-off-by: David Howells 
Signed-off-by: Al Viro 

which I think David was intending to use as part of his 
SELinux/overlayfs support.


path_has_perm() uses d_backing_inode(path->dentry), while 
file_path_has_perm() uses file_inode(file).




Signed-off-by: Andreas Gruenbacher 
---
  security/selinux/hooks.c | 18 +++---
  1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 65e8689..d6b4dc9 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -1673,18 +1673,6 @@ static inline int path_has_perm(const struct cred *cred,
return inode_has_perm(cred, inode, av, );
  }

-/* Same as path_has_perm, but uses the inode from the file struct. */
-static inline int file_path_has_perm(const struct cred *cred,
-struct file *file,
-u32 av)
-{
-   struct common_audit_data ad;
-
-   ad.type = LSM_AUDIT_DATA_PATH;
-   ad.u.path = file->f_path;
-   return inode_has_perm(cred, file_inode(file), av, );
-}
-
  /* Check whether a task can use an open file descriptor to
 access an inode in a given way.  Check access to the
 descriptor itself, and then use dentry_has_perm to
@@ -2371,14 +2359,14 @@ static inline void flush_unauthorized_files(const 
struct cred *cred,
struct tty_file_private *file_priv;

/* Revalidate access to controlling tty.
-  Use file_path_has_perm on the tty path directly
+  Use path_has_perm on the tty path directly
   rather than using file_has_perm, as this particular
   open file may belong to another process and we are
   only interested in the inode-based check here. */
file_priv = list_first_entry(>tty_files,
struct tty_file_private, list);
file = file_priv->file;
-   if (file_path_has_perm(cred, file, FILE__READ | 
FILE__WRITE))
+   if (path_has_perm(cred, >f_path, FILE__READ | 
FILE__WRITE))
drop_tty = 1;
}
spin_unlock(_files_lock);
@@ -3537,7 +3525,7 @@ static int selinux_file_open(struct file *file, const 
struct cred *cred)
 * new inode label or new policy.
 * This check is not redundant - do not remove.
 */
-   return file_path_has_perm(cred, file, open_file_to_av(file));
+   return path_has_perm(cred, >f_path, open_file_to_av(file));
  }

  /* task security operations */



--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] selinux: export validatetrans decisions

[Stephen Smalley]

On 10/27/2015 02:27 PM, Andrew Perepechko wrote:

+   if (rc)
+   goto out;
+
+   rc = -ENOMEM;
+   if (count >= PAGE_SIZE - 1)
+   goto out;


Why PAGE_SIZE-1?



This is to avoid allocation of more than a single page.


Yes, but you don't need PAGE_SIZE - 1 for that.  The check can just be 
>= PAGE_SIZE, as used elsewhere in selinuxfs.c.



kzalloc(count+1) will guarantee the string is zero-terminated.

The code below can be slightly optimized by modifying the
copied string in-place, but I tried to follow the style
used in neighbouring functions.



#next has security_context_str_to_sid() as a convenient helper for this.



OK.



Hmm...in what situation don't you want it to reflect the kernel
enforcing mode (i.e. when won't you just have your userspace file server
end up checking security_getenforce() and ignore the error in that
situation)?  Userspace AVC is different since it is caching decisions
from the kernel but still ends up honoring the kernel's enforcing status
(unless you explicitly set it to use its own private one).


We expected to have an option to be able to enforce the policy even
if the server itself is running in permissive, but this is not a critical
requirement, so I'll update this bit.



Beyond that, the things that you don't want to happen when called from
userspace include not unmapping the class value and not printk'ing on an
unrecognized class.  See security_transition_sid_user() ->
security_compute_sid() for example.



Sorry, you are right, indeed. :( This part somehow did not get in the final
patch.

I'll update the patch according to your comments.

--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/7] selinux: Add accessor functions for inode->i_security

[Stephen Smalley]

On 10/26/2015 05:15 PM, Andreas Gruenbacher wrote:

Add functions dentry_security and inode_security for accessing
inode->i_security.  These functions initially don't do much, but they
will later be used to revalidate the security labels when necessary.

Signed-off-by: Andreas Gruenbacher 
---
  security/selinux/hooks.c | 101 ++-
  1 file changed, 57 insertions(+), 44 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index fc8f626..65e8689 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -241,6 +241,24 @@ static int inode_alloc_security(struct inode *inode)
return 0;
  }

+/*
+ * Get the security label of a dentry's inode.
+ */
+static struct inode_security_struct *dentry_security(struct dentry *dentry)
+{
+   struct inode *inode = d_backing_inode(dentry);
+
+   return inode->i_security;
+}
+
+/*
+ * Get the security label of an inode.
+ */
+static struct inode_security_struct *inode_security(struct inode *inode)
+{
+   return inode->i_security;
+}
+
  static void inode_free_rcu(struct rcu_head *head)
  {
struct inode_security_struct *isec;



@@ -2207,7 +,6 @@ static int selinux_bprm_set_creds(struct linux_binprm 
*bprm)
struct task_security_struct *new_tsec;
struct inode_security_struct *isec;
struct common_audit_data ad;
-   struct inode *inode = file_inode(bprm->file);
int rc;

/* SELinux context only depends on initial program or script and not
@@ -2217,7 +2231,7 @@ static int selinux_bprm_set_creds(struct linux_binprm 
*bprm)

old_tsec = current_security();
new_tsec = bprm->cred->security;
-   isec = inode->i_security;
+   isec = dentry_security(bprm->file->f_path.dentry);


IIUC, this could change which inode label gets used when using overlayfs 
(the overlay inode or the underlying inode).  Not sure whether the 
current code is correct for overlayfs (overlayfs + SELinux support still 
in progress).



@@ -3154,7 +3168,7 @@ out_nofree:
  static int selinux_inode_setsecurity(struct inode *inode, const char *name,
 const void *value, size_t size, int flags)
  {
-   struct inode_security_struct *isec = inode->i_security;
+   struct inode_security_struct *isec = inode_security(inode);


Was it intentional to not do this for selinux_inode_getsecurity() and 
selinux_inode_getsecid()?



@@ -3241,8 +3254,8 @@ int ioctl_has_perm(const struct cred *cred, struct file 
*file,
  {
struct common_audit_data ad;
struct file_security_struct *fsec = file->f_security;
-   struct inode *inode = file_inode(file);
-   struct inode_security_struct *isec = inode->i_security;
+   struct dentry *dentry = file->f_path.dentry;
+   struct inode_security_struct *isec = dentry_security(dentry);
struct lsm_ioctlop_audit ioctl;
u32 ssid = cred_sid(cred);
int rc;
@@ -3263,7 +3276,7 @@ int ioctl_has_perm(const struct cred *cred, struct file 
*file,
goto out;
}

-   if (unlikely(IS_PRIVATE(inode)))
+   if (unlikely(IS_PRIVATE(dentry->d_inode)))
return 0;

rc = avc_has_extended_perms(ssid, isec->sid, isec->sclass,
@@ -3506,7 +3519,7 @@ static int selinux_file_open(struct file *file, const 
struct cred *cred)
struct inode_security_struct *isec;

fsec = file->f_security;
-   isec = file_inode(file)->i_security;
+   isec = dentry_security(file->f_path.dentry);


Similarly for these cases, switching from file_inode(file) to 
d_backing_inode(dentry) could affect overlayfs interaction IIUC.  cc'd 
David for clarification.


--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH v3 2/5] lsm: introduce hooks for kdbus

[Stephen Smalley]

On Mon, Oct 19, 2015 at 6:29 PM, Paul Moore <pmo...@redhat.com> wrote:
> On Friday, October 09, 2015 10:56:12 AM Stephen Smalley wrote:
>> On 10/07/2015 07:08 PM, Paul Moore wrote:
>> > diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
>> > index ef63d65..1cb87b3 100644
>> > --- a/ipc/kdbus/connection.c
>> > +++ b/ipc/kdbus/connection.c
>> > @@ -108,6 +109,14 @@ static struct kdbus_conn *kdbus_conn_new(struct
>> > kdbus_ep *ep,>
>> > if (!owner && (creds || pids || seclabel))
>> >
>> > return ERR_PTR(-EPERM);
>> >
>> > +   ret = security_kdbus_conn_new(get_cred(file->f_cred),
>>
>> You only need to use get_cred() if saving a reference; otherwise, you'll
>> leak one here.
>
> Yes, that was a typo on my part, thanks.
>
>> Also, do we want file->f_cred here or ep->bus->node.creds (the latter is
>> what is used by their own checks; the former is typically the same as
>> current cred IIUC).  For that matter, what about ep->node.creds vs ep->bus-
>> node.creds vs. ep->bus->domain->node.creds?  Can they differ?  Do we care?
>
> We don't want file->f_cred, per our previous discussions.  I was working on
> this patchset in small chunks and while I added credential storing in the
> nodes, I forgot to update the hooks before I hit send, my apologies.
>
> My current thinking is to pass both the endpoint and bus credentials, as I
> believe they can differ.  Both the bus and the endpoint inherit their security
> labels from their creator and while I don't have any specifics, I think it is
> reasonable to imagine those two processes having different security labels.
> Assuming we pass both credentials down to the LSM, I'm currently thinking of
> the following SELinux access controls for this hook:
>
>   allow  bus_t:kdbus { connect };
>   allow  ep_t:kdbus { use privileged activator monitor policy };

I think it would be simpler to apply an associate check when the
endpoint is created between the endpoint label and the bus label
(which will typically be the same), and then only check based on
endpoint label for all subsequent permission checks involving that
endpoint.  Then you don't have to worry about which label to use for
all the other permission checks. And you get finer-grained control -
per-endpoint rather than only per-bus.  In the common case, the labels
will be the same and it won't matter, but if you want to support some
MAC form of their 'custom endpoints' with further restrictions, you
can do that by labeling the endpoints uniquely and using those labels
in your permission checks.

Otherwise, I have to ask whether you need two different classes above
(bus vs endpoint), and whether the privileged/activator/monitor/policy
permissions properly belong with the endpoint label or the bus label.
At least some of those are bus-wide properties, not endpoint-specific,
but that's ok as long as we have established a relationship between
the endpoint label and bus label.

> ... besides the additional label, I added the kdbus:use permission and dropped
> the kdbus:owner permission.  Considering that the endpoint label, ep_t, in the
> examples above, could be different from the current process, it seemed
> reasonable to want to control that interaction and I felt the fd:use
> permission was the closest existing control so I reused the permission name.
> I decided to drop the "owner" permission as it really wasn't the useful for
> anything anymore, it simply indicates that the current task is the DAC owner
> of the endpoint.

Can you 'use' an endpoint in any way other than to connect via it?
If not, I'd just call that connect (won't conflict if you get rid of
the separate bus check above), or distinguish it via separate classes
or as connectthrough vs connectto.

conn->owner is used to determine whether the caller can fake
credentials, skip kdbus policy checking, create an activator, monitor,
or policy holder connection, etc.  Our options are:
1. Apply a SELinux check when it is set to see if the caller is
allowed to own the bus based on MAC labels and policy, and if not,
refuse to create the connection (that's what checking the owner
permission was doing).

2. Separately apply MAC checks over each of those abilities (fake
creds, override policy, create an activator, monitor, or policy
holder, etc) when there is an attempt to exercise them (not all during
connection creation), and selectively deny that ability.  More
invasive, more potential for breakage for applications that don't
expect failure if they could create the connection in the first place.

3. Treat faking of DAC credentials and skipping of kdbus policy
checking as not of interest to MAC, leaving it only controlled by the
existing uid match or C

Re: [RFC PATCH v3 1/5] kdbus: add creator credentials to the endpoints

[Stephen Smalley]

On 10/07/2015 07:08 PM, Paul Moore wrote:

In order to effectively enforce LSM based access controls we need to
have more information about the kdbus endpoint creator than the
uid/gid currently stored in the kdbus_node_type struct.  This patch
replaces the uid/gid values with a reference to the node creator's
credential struct which serves the needs of both the kdbus DAC access
controls as well as the LSM's access controls.

Two macros have also been created, kdbus_node_[uid,gid](), which can
be used to easily extract the euid/egid information from the new
credential reference.  The effective uid/gid is used as it was used
in all areas of the previous kdbus code except for areas where the
uid/gid was never set beyond the basic initialization to zero/root;
I expect this was a bug that was never caught as the node creator in
these cases was always expect to be root.

Signed-off-by: Paul Moore 

---
ChangeLog:
- v3
  * Ported to the 4.3-rc4 based kdbus tree
- v2
  * Initial draft
---
  ipc/kdbus/bus.c  |   13 +
  ipc/kdbus/endpoint.c |   14 --
  ipc/kdbus/endpoint.h |3 +--
  ipc/kdbus/fs.c   |4 ++--
  ipc/kdbus/node.c |   11 ---
  ipc/kdbus/node.h |5 +++--
  6 files changed, 19 insertions(+), 31 deletions(-)




diff --git a/ipc/kdbus/node.c b/ipc/kdbus/node.c
index 89f58bc..cd0c1a0 100644
--- a/ipc/kdbus/node.c
+++ b/ipc/kdbus/node.c
@@ -12,6 +12,7 @@
   */

  #include 
+#include 
  #include 
  #include 
  #include 
@@ -170,13 +171,7 @@
   * node initialization. They must remain constant. If
   * NULL, they're skipped.
   *
- * * node->mode: filesystem access modes


mode still remains
--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH v3 2/5] lsm: introduce hooks for kdbus

[Stephen Smalley]

On 10/07/2015 07:08 PM, Paul Moore wrote:

Add LSM access control hooks to kdbus; several new hooks are added and
the existing security_file_receive() hook is reused.  The new hooks
are listed below:

  * security_kdbus_conn_new
Check if the current task is allowed to create a new kdbus
connection.
  * security_kdbus_own_name
Check if a connection is allowed to own a kdbus service name.
  * security_kdbus_conn_talk
Check if a connection is allowed to talk to a kdbus peer.
  * security_kdbus_conn_see
Check if a connection can see a kdbus peer.
  * security_kdbus_conn_see_name
Check if a connection can see a kdbus service name.
  * security_kdbus_conn_see_notification
Check if a connection can receive notifications.
  * security_kdbus_proc_permission
Check if a connection can access another task's pid namespace info.
  * security_kdbus_init_inode
Set the security label on a kdbusfs inode

Signed-off-by: Paul Moore <pmo...@redhat.com>

---
ChangeLog:
- v3
  * Ported to the 4.3-rc4 based kdbus tree
- v2
  * Implemented suggestions by Stephen Smalley
* call security_kdbus_conn_new() sooner
* reworked hook inside kdbus_conn_policy_own_name()
* fixed if-conditional in kdbus_conn_policy_talk()
* reworked hook inside kdbus_conn_policy_see_name_unlocked()
* reworked hook inside kdbus_conn_policy_see()
* reworked hook inside kdbus_conn_policy_see_notification()
* added the security_kdbus_init_inode() hook
- v1
  * Initial draft
---
  include/linux/lsm_hooks.h |   63 +++
  include/linux/security.h  |   71 
  ipc/kdbus/connection.c|   73 +
  ipc/kdbus/fs.c|6 
  ipc/kdbus/message.c   |   19 +---
  ipc/kdbus/metadata.c  |6 +---
  security/security.c   |   62 ++
  7 files changed, 265 insertions(+), 35 deletions(-)


diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
index ef63d65..1cb87b3 100644
--- a/ipc/kdbus/connection.c
+++ b/ipc/kdbus/connection.c
@@ -26,6 +26,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -108,6 +109,14 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep 
*ep,
if (!owner && (creds || pids || seclabel))
return ERR_PTR(-EPERM);

+   ret = security_kdbus_conn_new(get_cred(file->f_cred),


You only need to use get_cred() if saving a reference; otherwise, you'll 
leak one here.  Also, do we want file->f_cred here or 
ep->bus->node.creds (the latter is what is used by their own checks; the 
former is typically the same as current cred IIUC).  For that matter, 
what about ep->node.creds vs ep->bus->node.creds vs. 
ep->bus->domain->node.creds?  Can they differ?  Do we care?



+ creds, pids, seclabel,
+ owner, privileged,
+ is_activator, is_monitor,
+ is_policy_holder);
+   if (ret < 0)
+   return ERR_PTR(ret);
+
ret = kdbus_sanitize_attach_flags(hello->attach_flags_send,
  _flags_send);
if (ret < 0)
@@ -1435,12 +1444,12 @@ bool kdbus_conn_policy_own_name(struct kdbus_conn *conn,
return false;
}

-   if (conn->owner)
-   return true;
+   if (!conn->owner &&
+   kdbus_policy_query(>ep->bus->policy_db, conn_creds, name,
+  hash) < KDBUS_POLICY_OWN)
+   return false;

-   res = kdbus_policy_query(>ep->bus->policy_db, conn_creds,
-name, hash);
-   return res >= KDBUS_POLICY_OWN;
+   return (security_kdbus_own_name(conn_creds, name) == 0);


Similar question here.  conn_creds is the credentials of the creator of 
the connection, typically the client/sender, right? 
conn->ep->bus->node.creds are the credentials of the bus owner, so don't 
we want to ask "Can I own this name on this bus?".  Note that their 
policy checks are based on conn->ep->policy_db, i.e. the policy 
associated with the endpoint, and conn->owner is only true if the 
connection creator has the same uid as the bus.



  }

  /**
@@ -1465,14 +1474,13 @@ bool kdbus_conn_policy_talk(struct kdbus_conn *conn,
 to, KDBUS_POLICY_TALK))
return false;

-   if (conn->owner)
-   return true;
-   if (uid_eq(conn_creds->euid, to->cred->uid))
-   return true;
+   if (!conn->owner && !uid_eq(conn_creds->euid, to->cred->uid) &&
+   !kdbus_conn_policy_query_all(conn, conn_creds,
+

Re: [RFC PATCH v3 3/5] lsm: add support for auditing kdbus service names

[Stephen Smalley]

On 10/07/2015 07:08 PM, Paul Moore wrote:

The kdbus service names will be recorded using 'service', similar to
the existing dbus audit records.

Signed-off-by: Paul Moore 

---
ChangeLog:
- v3
  * Ported to the 4.3-rc4 based kdbus tree
- v2
  * Initial draft
---
  include/linux/lsm_audit.h |2 ++
  security/lsm_audit.c  |4 
  2 files changed, 6 insertions(+)

diff --git a/include/linux/lsm_audit.h b/include/linux/lsm_audit.h
index ffb9c9d..d6a656f 100644
--- a/include/linux/lsm_audit.h
+++ b/include/linux/lsm_audit.h
@@ -59,6 +59,7 @@ struct common_audit_data {
  #define LSM_AUDIT_DATA_INODE  9
  #define LSM_AUDIT_DATA_DENTRY 10
  #define LSM_AUDIT_DATA_IOCTL_OP   11
+#define LSM_AUDIT_DATA_KDBUS   12
union   {
struct path path;
struct dentry *dentry;
@@ -75,6 +76,7 @@ struct common_audit_data {
  #endif
char *kmod_name;
struct lsm_ioctlop_audit *op;
+   const char *kdbus_name;
} u;
/* this union contains LSM specific data */
union {
diff --git a/security/lsm_audit.c b/security/lsm_audit.c
index cccbf30..0a3dc1b 100644
--- a/security/lsm_audit.c
+++ b/security/lsm_audit.c
@@ -397,6 +397,10 @@ static void dump_common_audit_data(struct audit_buffer *ab,
audit_log_format(ab, " kmod=");
audit_log_untrustedstring(ab, a->u.kmod_name);
break;
+   case LSM_AUDIT_DATA_KDBUS:
+   audit_log_format(ab, " service=");


Not a major issue to me, but just wondering if this needs to be further 
qualified to indicate it is a kdbus service.  service= is rather generic.



+   audit_log_untrustedstring(ab, a->u.kdbus_name);
+   break;
} /* switch (a->type) */
  }


___
Selinux mailing list
seli...@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.



--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH v3 5/5] selinux: introduce kdbus access controls

[Stephen Smalley]

On 10/07/2015 07:08 PM, Paul Moore wrote:

Add the SELinux access control implementation for the new kdbus LSM
hooks using the new kdbus object class and the following permissions:

  [NOTE: permissions below are based on kdbus code from Aug 2015]

  * kdbus:impersonate
Send a different security label to kdbus peers.
  * kdbus:fakecreds
Send different DAC credentials to kdbus peers.
  * kdbus:fakepids
Send a different PID to kdbus peers.
  * kdbus:owner
Act as a kdbus bus owner.
  * kdbus:privileged
Act as a privileged endpoint.
  * kdbus:activator
Act as a kdbus activator.
  * kdbus:monitor
Act as a kdbus monitor.
  * kdbus:policy_holder
Act as a kdbus policy holder.
  * kdbus:connect
Create a new kdbus connection.
  * kdbus:own
Own a kdbus service name.
  * kdbus:talk
Talk between two kdbus endpoints.
  * kdbus:see
See another kdbus endpoint.
  * kdbus:see_name
See a kdbus service name.
  * kdbus:see_notification
See a kdbus notification.

Signed-off-by: Paul Moore 

---
ChangeLog:
- v3
  * Ported to the 4.3-rc4 based kdbus tree
  * Fix the missing NULL terminator in the kdbus obj class definition
- v2
  * Add the selinux_kdbus_init_inode() hook
  * Add some very basic info on the permissions to the description
  * Add kdbus service name auditing in the AVC records
- v1
  * Initial draft
---
  security/selinux/hooks.c|  153 +++
  security/selinux/include/classmap.h |4 +
  2 files changed, 155 insertions(+), 2 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index e4369d8..5581990 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -9,8 +9,10 @@
   *  James Morris 
   *
   *  Copyright (C) 2001,2002 Networks Associates Technology, Inc.
- *  Copyright (C) 2003-2008 Red Hat, Inc., James Morris 
- *Eric Paris 
+ *  Copyright (C) 2003-2008,2015 Red Hat, Inc.
+ * James Morris 
+ * Eric Paris 
+ * Paul Moore 
   *  Copyright (C) 2004-2005 Trusted Computer Solutions, Inc.
   *
   *  Copyright (C) 2006, 2007, 2009 Hewlett-Packard Development Company, L.P.
@@ -2035,6 +2037,143 @@ static int selinux_binder_transfer_file(struct 
task_struct *from,
);
  }

+static int selinux_kdbus_conn_new(const struct cred *creds,
+ const struct kdbus_creds *fake_creds,
+ const struct kdbus_pids *fake_pids,
+ const char *fake_seclabel,
+ bool owner, bool privileged,
+ bool is_activator, bool is_monitor,
+ bool is_policy_holder)
+{
+   int rc;
+   u32 tsid = current_sid();
+   u32 av = KDBUS__CONNECT;
+
+   if (fake_creds)
+   av |= KDBUS__FAKECREDS;
+   if (fake_pids)
+   av |= KDBUS__FAKEPIDS;
+   if (owner)
+   av |= KDBUS__OWNER;
+   if (privileged)
+   av |= KDBUS__PRIVILEGED;
+   if (is_activator)
+   av |= KDBUS__ACTIVATOR;
+   if (is_monitor)
+   av |= KDBUS__MONITOR;
+   if (is_policy_holder)
+   av |= KDBUS__POLICY_HOLDER;
+
+   rc = avc_has_perm(tsid, cred_sid(creds), SECCLASS_KDBUS, av, NULL);
+   if (rc)
+   return rc;
+
+   if (fake_seclabel) {
+   u32 sid;
+   if (security_context_to_sid(fake_seclabel,
+   strlen(fake_seclabel),
+   , GFP_KERNEL))
+   return -EINVAL;
+
+   rc = avc_has_perm(tsid, sid,
+ SECCLASS_KDBUS, KDBUS__IMPERSONATE, NULL);
+   }
+
+   return rc;
+}
+
+static int selinux_kdbus_own_name(const struct cred *creds, const char *name)
+{
+   int rc;
+   u32 name_sid;
+   struct common_audit_data ad;
+
+   rc = security_kdbus_sid(name, _sid);
+   if (rc)
+   return rc;
+
+   ad.type = LSM_AUDIT_DATA_KDBUS;
+   ad.u.kdbus_name = name;
+
+   return avc_has_perm(cred_sid(creds), name_sid,
+   SECCLASS_KDBUS, KDBUS__OWN, );
+}
+
+static int selinux_kdbus_conn_talk(const struct cred *creds,
+  const struct cred *creds_to)
+{
+   return avc_has_perm(cred_sid(creds), cred_sid(creds_to),
+   SECCLASS_KDBUS, KDBUS__TALK, NULL);
+}
+
+static int selinux_kdbus_conn_see(const struct cred *creds,
+ const struct cred *creds_whom)
+{
+ 

Re: [PATCH] security: selinux: Use a kmem_cache for allocation struct file_security_struct

[Stephen Smalley]

On 10/05/2015 01:45 AM, Sangwoo wrote:
> The size of struct file_security_struct is 16byte at my setup.
> But, the real allocation size for per each file_security_struct
> is 64bytes in my setup that kmalloc min size is 64bytes
> because ARCH_DMA_MINALIGN is 64.
> 
> This allocation is called every times at file allocation(alloc_file()).
> So, the total slack memory size(allocated size - request size)
> is increased exponentially.
> 
> E.g) Min Kmalloc Size : 64bytes, Unit : bytes
>   Allocated Size | Request Size | Slack Size | Allocation Count
> ---
>  770048  |192512|   577536   |  12032
> 
> At the result, this change reduce memory usage 42bytes per each
> file_security_struct
> 
> Signed-off-by: Sangwoo <sangwoo2.p...@lge.com>

Acked-by:  Stephen Smalley <s...@tycho.nsa.gov>

> ---
>  security/selinux/hooks.c |8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 3f8d567..c20e082 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -126,6 +126,7 @@ int selinux_enabled = 1;
>  #endif
>  
>  static struct kmem_cache *sel_inode_cache;
> +static struct kmem_cache *file_security_cache;
>  
>  /**
>   * selinux_secmark_enabled - Check to see if SECMARK is currently enabled
> @@ -287,7 +288,7 @@ static int file_alloc_security(struct file *file)
>   struct file_security_struct *fsec;
>   u32 sid = current_sid();
>  
> - fsec = kzalloc(sizeof(struct file_security_struct), GFP_KERNEL);
> + fsec = kmem_cache_zalloc(file_security_cache, GFP_KERNEL);
>   if (!fsec)
>   return -ENOMEM;
>  
> @@ -302,7 +303,7 @@ static void file_free_security(struct file *file)
>  {
>   struct file_security_struct *fsec = file->f_security;
>   file->f_security = NULL;
> - kfree(fsec);
> + kmem_cache_free(file_security_cache, fsec);
>  }
>  
>  static int superblock_alloc_security(struct super_block *sb)
> @@ -6086,6 +6087,9 @@ static __init int selinux_init(void)
>   sel_inode_cache = kmem_cache_create("selinux_inode_security",
>   sizeof(struct 
> inode_security_struct),
>   0, SLAB_PANIC, NULL);
> + file_security_cache = kmem_cache_create("selinux_file_security",
> + sizeof(struct file_security_struct),
> + 0, SLAB_PANIC, NULL);
>   avc_init();
>  
>   security_add_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks));
> 

--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/5] Security: Provide unioned file support

[Stephen Smalley]

On 09/29/2015 05:03 PM, Stephen Smalley wrote:

On 09/28/2015 04:00 PM, David Howells wrote:


The attached patches provide security support for unioned files where the
security involves an object-label-based LSM (such as SELinux) rather
than a
path-based LSM.

[Note that a number of the bits that were in the original patch set
are now
upstream and I've rebased on Casey's changes to the security hook system]

The patches can be broken down into two sets:

  (1) A patch to add LSM hooks to handle copy up of a file, including
label
  determination/setting and xattr filtration and a patch to have
  overlayfs call the hooks during the copy-up procedure.

  (2) My SELinux implementations of these hooks.  I do three things:

  (a) Don't copy up SELinux xattrs from the lower file to the upper
   file.  It is assumed that the upper file will be created
with the
   attrs we want or the selinux_inode_copy_up() hook will set it
   appropriately.

 The reason there are two separate hooks here is that
 selinux_inode_copy_up_xattr() might not ever be called if there
 aren't actually any xattrs on the lower inode.

  (b) I try to derive a label to be used by file operations by, in
order
   of preference: using the label on the union inode if there
is one
   (the normal overlayfs case); using the override label set
on the
   superblock, if provided; or trying to derive a new label by
sid
   transition operation.

  (c) Using the label obtained in (b) in file_has_perm() rather than
   using the label on the lower inode.

Now the steps I have outlined in (b) and (c) seem to be at odds with what
Dan Walsh and Stephen Smalley want - but I'm not sure I follow what that
is, let alone how to do it:

Wanted to bring back the original proposal.  Stephen suggested that
we could change back to the MLS way of handling labels.

In MCS we base the MCS label of content created by a process on the
containing directory.  Which means that if a process running as
s0:c1,c2 creates content in a directory labeled s0, it will get
created as s0.

In MLS if a process running as s0:c1,c2 creates content in a
directory it labels it s0:c1,c2.  No matter what the label of the
directory is.  (Well actually if the directory is not ranged the
process will not be able to create the content.)

We changed the default for MCS in Rawhide for about a week, when I
realized this was a huge problem for containers sharing content.
Currently if you want two containers to share the same volume
mount, we label the content as svirt_sandbox_file_t:s0 If one
process running as s0:c1,c2 creates content it gets created as s0,
if the second container process is running as s0:c3,c4, it can
still read/write the content.  If we changed the default the object
would get created as s0:c1,c2 and process runing as s0:c3,c4 would
be blocked.

So I had it reverted back to the standard MCS Mode.

If we could get the default to be MLS mode on COW file systems and
MCS on Volumes, we would get the best of both worlds.


How are you testing this?
I tried as follows:

# Make sure we have a policy that is using xattrs to label overlay inodes.
$ seinfo --fs_use | grep overlay
fs_use_xattr overlay system_u:object_r:fs_t:s0

# Define some types for the different directories involved.
$ cat overlay.te
policy_module(overlay, 1.0)

type lower_t;
files_type(lower_t)

type upper_t;
files_type(upper_t)

type work_t;
files_type(work_t)

type merged_t;
files_type(merged_t)

$ make -f /usr/share/selinux/devel/Makefile overlay.pp
$ sudo semodule -i overlay.pp

# Create and label the different directories involved.
$ mkdir lower upper work merged
$ chcon -t lower_t lower
$ chcon -t upper_t upper
$ chcon -t work_t work
$ chcon -t merged_t merged

# Populate lower
$ echo "lower" > lower/a
$ mkdir lower/b

# Mount overlay
$ sudo mount -t overlay -o lowerdir=lower,upperdir=upper,workdir=work
merged

# Look at the merged dir and labels.
$ ls -Z merged
unconfined_u:object_r:lower_t:s0 a
unconfined_u:object_r:lower_t:s0 b

# Write/create some files/directories.
$ echo "foo" >> merged/a
$ mkdir merged/b/c
$ mkdir merged/c

# Look again.
$ ls -ZR merged
merged:
unconfined_u:object_r:lower_t:s0 a  unconfined_u:object_r:upper_t:s0 c
unconfined_u:object_r:lower_t:s0 b

merged/b:
unconfined_u:object_r:work_t:s0 c

merged/b/c:

$ ls -ZR upper
upper:
  unconfined_u:object_r:work_t:s0 a  unconfined_u:object_r:upper_t:s0 c
  unconfined_u:object_r:work_t:s0 b

upper/b:
unconfined_u:object_r:work_t:s0 c

upper/b/c:

Note that the copied-up file (a) and directory (b) are labeled lower_t
in the overlay, but work_t in the upper dir, and neither of those is
really what we want IIUC.

And that's just the labeling question.  Then there is the question of
what permission checks were applied during those c

Re: NFS/LSM: allow NFS to control all of its own mount options

[Stephen Smalley]

 to [EMAIL PROTECTED]
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
-- 
Stephen Smalley
National Security Agency

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

Re: [PATCH] [RFC] Smack update for file capabilities

[Stephen Smalley]

On Mon, 2008-02-18 at 15:58 -0800, Casey Schaufler wrote:
 From: Casey Schaufler [EMAIL PROTECTED]
 
 This patch assumes Smack unlabeled outgoing ambient packets - v4
 which is one reason it's RFC.
 
 Update the Smack LSM to allow the registration of the capability
 module as a secondary LSM. Integrate the new hooks required for
 file based capabilities.
 
 Signed-off-by: Casey Schaufler [EMAIL PROTECTED]
 
 ---
 
  security/smack/smack_lsm.c |   63 +--
  1 file changed, 60 insertions(+), 3 deletions(-)
 
 diff -uprN -X linux-2.6.25-g0216-precap//Documentation/dontdiff 
 linux-2.6.25-g0216-precap/security/smack/smack_lsm.c 
 linux-2.6.25-g0216/security/smack/smack_lsm.c
 --- linux-2.6.25-g0216-precap/security/smack/smack_lsm.c  2008-02-18 
 10:53:45.0 -0800
 +++ linux-2.6.25-g0216/security/smack/smack_lsm.c 2008-02-18 
 14:15:25.0 -0800
 @@ -584,6 +584,12 @@ static int smack_inode_getattr(struct vf
  static int smack_inode_setxattr(struct dentry *dentry, char *name,
   void *value, size_t size, int flags)
  {
 + int rc;
 +
 + rc = cap_inode_setxattr(dentry, name, value, size, flags);
 + if (rc != 0)
 + return rc;

You only want to do that if the attribute isn't one of the Smack
attributes.  Otherwise, the capability module will apply a CAP_SYS_ADMIN
check on setting anything in the security namespace, including the Smack
attributes.

 +
   if (!capable(CAP_MAC_ADMIN)) {
   if (strcmp(name, XATTR_NAME_SMACK) == 0 ||
   strcmp(name, XATTR_NAME_SMACKIPIN) == 0 ||
 @@ -658,6 +664,12 @@ static int smack_inode_getxattr(struct d
   */
  static int smack_inode_removexattr(struct dentry *dentry, char *name)
  {
 + int rc;
 +
 + rc = cap_inode_removexattr(dentry, name);
 + if (rc != 0)
 + return rc;

Ditto.

 +
   if (strcmp(name, XATTR_NAME_SMACK) == 0  !capable(CAP_MAC_ADMIN))
   return -EPERM;
  
 @@ -1016,7 +1028,12 @@ static void smack_task_getsecid(struct t
   */
  static int smack_task_setnice(struct task_struct *p, int nice)
  {
 - return smk_curacc(p-security, MAY_WRITE);
 + int rc;
 +
 + rc = cap_task_setnice(p, nice);
 + if (rc == 0)
 + rc = smk_curacc(p-security, MAY_WRITE);
 + return rc;
  }
  
  /**
 @@ -1028,7 +1045,12 @@ static int smack_task_setnice(struct tas
   */
  static int smack_task_setioprio(struct task_struct *p, int ioprio)
  {
 - return smk_curacc(p-security, MAY_WRITE);
 + int rc;
 +
 + rc = cap_task_setioprio(p, ioprio);
 + if (rc == 0)
 + rc = smk_curacc(p-security, MAY_WRITE);
 + return rc;
  }
  
  /**
 @@ -1053,7 +1075,12 @@ static int smack_task_getioprio(struct t
  static int smack_task_setscheduler(struct task_struct *p, int policy,
  struct sched_param *lp)
  {
 - return smk_curacc(p-security, MAY_WRITE);
 + int rc;
 +
 + rc = cap_task_setscheduler(p, policy, lp);
 + if (rc == 0)
 + rc = smk_curacc(p-security, MAY_WRITE);
 + return rc;
  }
  
  /**
 @@ -1093,6 +1120,11 @@ static int smack_task_movememory(struct 
  static int smack_task_kill(struct task_struct *p, struct siginfo *info,
  int sig, u32 secid)
  {
 + int rc;
 +
 + rc = cap_task_kill(p, info, sig, secid);
 + if (rc != 0)
 + return rc;
   /*
* Special cases where signals really ought to go through
* in spite of policy. Stephen Smalley suggests it may
 @@ -1778,6 +1810,27 @@ static int smack_ipc_permission(struct k
   return smk_curacc(isp, may);
  }
  
 +/* module stacking operations */
 +
 +/**
 + * smack_register_security - stack capability module
 + * @name: module name
 + * @ops: module operations - ignored
 + *
 + * Allow the capability module to register.
 + */
 +static int smack_register_security(const char *name,
 +struct security_operations *ops)
 +{
 + if (strcmp(name, capability) != 0)
 + return -EINVAL;
 +
 + printk(KERN_INFO %s:  Registering secondary module %s\n,
 +__func__, name);
 +
 + return 0;
 +}
 +
  /**
   * smack_d_instantiate - Make sure the blob is correct on an inode
   * @opt_dentry: unused
 @@ -2412,6 +2465,8 @@ static struct security_operations smack_
   .inode_post_setxattr =  smack_inode_post_setxattr,
   .inode_getxattr =   smack_inode_getxattr,
   .inode_removexattr =smack_inode_removexattr,
 + .inode_need_killpriv =  cap_inode_need_killpriv,
 + .inode_killpriv =   cap_inode_killpriv,
   .inode_getsecurity =smack_inode_getsecurity,
   .inode_setsecurity =smack_inode_setsecurity,
   .inode_listsecurity =   smack_inode_listsecurity,
 @@ -2471,6 +2526,8 @@ static struct security_operations smack_
   .netlink_send

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

[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-security-module in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Stephen Smalley]

On Mon, 2008-01-14 at 14:06 +, David Howells wrote:
 David Howells [EMAIL PROTECTED] wrote:
 
  Okay...  It looks like I want four security operations/hooks for cachefiles:
 
 FYI, I added the following vectors:
 
   # kernel services that need to override task security
   class kernel_service
   {
   use_as_override
   create_files_as
   }
 
 The first allows:
 
   avc_has_perm(daemon_tsec-sid, nominated_sid,
SECCLASS_KERNEL_SERVICE,
KERNEL_SERVICE__USE_AS_OVERRIDE,
NULL);
 
 And the second something like:
 
   avc_has_perm(tsec-sid, inode-sid,
SECCLASS_KERNEL_SERVICE,
KERNEL_SERVICE__CREATE_FILES_AS,
NULL);
 
 Rather than specifically dedicating them to the cache, I made them general.

Make sure that you or Dan submits a policy patch to register these
classes and permissions in the policy when the kernel patch is queued
for merge.

-- 
Stephen Smalley
National Security Agency

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

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

[Stephen Smalley]

On Tue, 2008-01-15 at 16:03 +, David Howells wrote:
 Stephen Smalley [EMAIL PROTECTED] wrote:
 
(3) Check that the kernel may create files as a particular secid (this
could be specified indirectly by specifying an inode, which would
hide the secid inside the LSM).
  
  I don't think this check is on the kernel per se but rather the ability
  of the daemon to nominate a secid for use on files created later by the
  kernel module.
 
 Hmmm...  At the moment the cachefiles module works out for itself what the
 file label should be by looking at the root directory it was given and
 assuming the label on that is what it's going to be using.  Are you suggesting
 this should be specified directly instead by the daemon?

No, just that however the secid is determined (whether indirectly via
specification of a directory or directly via specification of a secid),
the ability of the daemon to control what secid gets used ought to be
controlled.  Or, alternatively, the ability of the daemon to enable
caching in a given directory ought to be controlled.

-- 
Stephen Smalley
National Security Agency

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

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

[Stephen Smalley]

On Tue, 2008-01-15 at 10:10 -0800, Casey Schaufler wrote:
 --- David Howells [EMAIL PROTECTED] wrote:
 
  Stephen Smalley [EMAIL PROTECTED] wrote:
  
 (3) Check that the kernel may create files as a particular secid (this
 could be specified indirectly by specifying an inode, which would
 hide the secid inside the LSM).
   
   I don't think this check is on the kernel per se but rather the ability
   of the daemon to nominate a secid for use on files created later by the
   kernel module.
  
  Hmmm...  At the moment the cachefiles module works out for itself what the
  file label should be by looking at the root directory it was given and
  assuming the label on that is what it's going to be using.  Are you
  suggesting
  this should be specified directly instead by the daemon?
 
 Oh my. While there will be cases where the label of the file
 will match the label of the containing directory, and in fact
 for most label based LSMs that will usually be the case, you
 certainly can't count on it. The only place that you can find
 the correct label for a file with any confidence in from the
 xattr (assuming the LSM uses xattrs) on the file itself. I can
 imaging an LSM for which it would make sense to derive the
 label from the root directory, but I know Smack isn't one of
 them, and I don't think that SELinux is either, although I
 would defer a definitive answer on that to Stephen.

The cache files are created by the cachefiles kernel module, not by the
userspace daemon, and the userspace daemon doesn't need to directly
read/write them at all (but I think it does need to be able to unlink
them?).  The userspace daemon merely identifies the directory where the
cache should live as part of configuring the cache when enabling it.

Hence, it is fine to use a fixed label for the cache files (systemhigh
in a MLS world), and to let the directory's label serve as the basis for
it.  Only the cachefiles kernel module directly reads and writes the
files.
 
-- 
Stephen Smalley
National Security Agency

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

Re: [RFC PATCH] Adding prctl override support for LSMs

[Stephen Smalley]

On Wed, 2008-01-09 at 20:52 -0800, Andrew Morgan wrote:
 -BEGIN PGP SIGNED MESSAGE-
 Hash: SHA1
 
 [Replying to everyone at once.]
 
 Yes, this is how I wanted to implement the per-process securebits thing.
 This is also half of my original patch (from last year).
 
 The recent cap_bset changes have been implemented via prctl(), but in
 that case the bset setting is done in the main kernel code and only
 really honored in the capability LSM. This doesn't seem very modular to
 me, and this prctl patch has the potential for better containing the
 modularity for that code too.
 
 Since it is potentially useful for other LSMs, I thought I would break
 it out to give it more visibility/discussion and to get a better sense
 of whether it was acceptable or not.
 
 So far the objection (thanks Stephen for the historical context!) seems
 to be potential for abuse:
 
 ~  [EMAIL PROTECTED]
   [PATCH] remove sys_security
 
   I've been auditing the LSM stuff a bit more..
 
   They have registered an implemented a syscall, sys_security
   that does nothing but switch into the individual modules
   based on the first argument, i.e. it's ioctl() switching
   on the security module instead of device node.  Yuck.
 
   Patch below removes it (no intree users), maybe selinux/etc
   folks should send their actual syscall for review instead..
 
 Since SELinux is now 'in-tree', is this class of objection now moot?

I don't think so; it could still be abused by out-of-tree modules, and
they really want to be able to carefully scrutinize additions to the
kernel's syscall interface.

I think you are better off directly implementing what you want in
prctl() without worrying about LSM, as capabilities are still deeply
intertwined with the core kernel (e.g. the capability bitmaps weren't
pushed into LSM's security blobs).  Or add a node to /proc/pid/attr for
it.

-- 
Stephen Smalley
National Security Agency

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

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

[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-security-module in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[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-security-module in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH v9 12/18] SELinux: Add a new peer class and permissions to the Flask definitions

[Stephen Smalley]

On Fri, 2007-12-21 at 12:09 -0500, Paul Moore wrote:
 Add additional Flask definitions to support the new peer object class and
 additional permissions to the netif and node object classes.
 
 Signed-off-by: Paul Moore [EMAIL PROTECTED]

Not an obstacle to merging, but need to get this reserved in policy too.

 ---
 
  security/selinux/include/av_perm_to_string.h |5 +
  security/selinux/include/av_permissions.h|5 +
  security/selinux/include/class_to_string.h   |7 +++
  security/selinux/include/flask.h |1 +
  4 files changed, 18 insertions(+), 0 deletions(-)
 
 diff --git a/security/selinux/include/av_perm_to_string.h 
 b/security/selinux/include/av_perm_to_string.h
 index 049bf69..caa0634 100644
 --- a/security/selinux/include/av_perm_to_string.h
 +++ b/security/selinux/include/av_perm_to_string.h
 @@ -37,6 +37,8 @@
 S_(SECCLASS_NODE, NODE__ENFORCE_DEST, enforce_dest)
 S_(SECCLASS_NODE, NODE__DCCP_RECV, dccp_recv)
 S_(SECCLASS_NODE, NODE__DCCP_SEND, dccp_send)
 +   S_(SECCLASS_NODE, NODE__RECVFROM, recvfrom)
 +   S_(SECCLASS_NODE, NODE__SENDTO, sendto)
 S_(SECCLASS_NETIF, NETIF__TCP_RECV, tcp_recv)
 S_(SECCLASS_NETIF, NETIF__TCP_SEND, tcp_send)
 S_(SECCLASS_NETIF, NETIF__UDP_RECV, udp_recv)
 @@ -45,6 +47,8 @@
 S_(SECCLASS_NETIF, NETIF__RAWIP_SEND, rawip_send)
 S_(SECCLASS_NETIF, NETIF__DCCP_RECV, dccp_recv)
 S_(SECCLASS_NETIF, NETIF__DCCP_SEND, dccp_send)
 +   S_(SECCLASS_NETIF, NETIF__INGRESS, ingress)
 +   S_(SECCLASS_NETIF, NETIF__EGRESS, egress)
 S_(SECCLASS_UNIX_STREAM_SOCKET, UNIX_STREAM_SOCKET__CONNECTTO, 
 connectto)
 S_(SECCLASS_UNIX_STREAM_SOCKET, UNIX_STREAM_SOCKET__NEWCONN, newconn)
 S_(SECCLASS_UNIX_STREAM_SOCKET, UNIX_STREAM_SOCKET__ACCEPTFROM, 
 acceptfrom)
 @@ -159,3 +163,4 @@
 S_(SECCLASS_DCCP_SOCKET, DCCP_SOCKET__NODE_BIND, node_bind)
 S_(SECCLASS_DCCP_SOCKET, DCCP_SOCKET__NAME_CONNECT, name_connect)
 S_(SECCLASS_MEMPROTECT, MEMPROTECT__MMAP_ZERO, mmap_zero)
 +   S_(SECCLASS_PEER, PEER__RECV, recv)
 diff --git a/security/selinux/include/av_permissions.h 
 b/security/selinux/include/av_permissions.h
 index eda89a2..c2b5bb2 100644
 --- a/security/selinux/include/av_permissions.h
 +++ b/security/selinux/include/av_permissions.h
 @@ -292,6 +292,8 @@
  #define NODE__ENFORCE_DEST0x0040UL
  #define NODE__DCCP_RECV   0x0080UL
  #define NODE__DCCP_SEND   0x0100UL
 +#define NODE__RECVFROM0x0200UL
 +#define NODE__SENDTO  0x0400UL
  #define NETIF__TCP_RECV   0x0001UL
  #define NETIF__TCP_SEND   0x0002UL
  #define NETIF__UDP_RECV   0x0004UL
 @@ -300,6 +302,8 @@
  #define NETIF__RAWIP_SEND 0x0020UL
  #define NETIF__DCCP_RECV  0x0040UL
  #define NETIF__DCCP_SEND  0x0080UL
 +#define NETIF__INGRESS0x0100UL
 +#define NETIF__EGRESS 0x0200UL
  #define NETLINK_SOCKET__IOCTL 0x0001UL
  #define NETLINK_SOCKET__READ  0x0002UL
  #define NETLINK_SOCKET__WRITE 0x0004UL
 @@ -824,3 +828,4 @@
  #define DCCP_SOCKET__NODE_BIND0x0040UL
  #define DCCP_SOCKET__NAME_CONNECT 0x0080UL
  #define MEMPROTECT__MMAP_ZERO 0x0001UL
 +#define PEER__RECV0x0001UL
 diff --git a/security/selinux/include/class_to_string.h 
 b/security/selinux/include/class_to_string.h
 index e77de0e..b1b0d1d 100644
 --- a/security/selinux/include/class_to_string.h
 +++ b/security/selinux/include/class_to_string.h
 @@ -64,3 +64,10 @@
  S_(NULL)
  S_(dccp_socket)
  S_(memprotect)
 +S_(NULL)
 +S_(NULL)
 +S_(NULL)
 +S_(NULL)
 +S_(NULL)
 +S_(NULL)
 +S_(peer)
 diff --git a/security/selinux/include/flask.h 
 b/security/selinux/include/flask.h
 index a9c2b20..09e9dd2 100644
 --- a/security/selinux/include/flask.h
 +++ b/security/selinux/include/flask.h
 @@ -50,6 +50,7 @@
  #define SECCLASS_KEY 58
  #define SECCLASS_DCCP_SOCKET 60
  #define SECCLASS_MEMPROTECT  61
 +#define SECCLASS_PEER68
  
  /*
   * Security identifier indices for initial entities
 
 
 --
 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-security-module in
the body of a message to [EMAIL PROTECTED]
More majordomo

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

[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-security-module in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH v8 10/18] SELinux: Add a network node caching mechanism similar to the sel_netif_*() functions

[Stephen Smalley]

On Mon, 2007-12-17 at 15:56 -0500, Paul Moore wrote:
 On Monday 17 December 2007 3:35:28 pm Stephen Smalley wrote:
  On Fri, 2007-12-14 at 16:50 -0500, Paul Moore wrote:
   This patch adds a SELinux IP address/node SID caching mechanism similar
   to the sel_netif_*() functions.  The node SID queries in the SELinux
   hooks files are also modified to take advantage of this new
   functionality.  In addition, remove the address length information from
   the sk_buff parsing routines as it is redundant since we already have the
   address family.
 
  This is very nice - we also need the same kind of cache for port SIDs.
 
 Thanks.  Any problem if we wait until 2.6.26 for a port SID cache?  It 
 shouldn't be any worse than it is now (the new code is not concerned with 
 ports) and the current patchset is already large enough that it keeps me up 
 at night thinking about all the places it could go wrong ...

Yes, that's fine - just a note to file away for the future.  We'll still
want the cache eventually though since the name_bind and name_connect
checks are based on the port SIDs and will remain even when the compat
checks are obsoleted.

-- 
Stephen Smalley
National Security Agency

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

Re: [RFC PATCH v8 18/18] SELinux: Add network ingress and egress control permission checks

[Stephen Smalley]

On Tue, 2007-12-18 at 08:59 -0500, Paul Moore wrote:
 On Monday 17 December 2007 3:05:37 pm Stephen Smalley wrote:
  On Sun, 2007-12-16 at 11:47 -0500, Paul Moore wrote:
   We should probably have different permissions for the interface and node
   cases.  Take the example of an admin who is only interested in enforcing
   interface controls and not node controls.  They would most likely write
   the following policy rule to nullify the node check ...
  
allow unlabeled_t peer_t:peer egress;
  
   ... which would end up applying to both the interface and node checks
   because they use the same permission.  I'm thinking we should split the
   permissions like this:
  
allow netif_t peer_t:peer if_egress;
allow netnode_t peer_t: peer node_egress;
  
   ... and do something similar for the ingress side.  Thoughts?
 
  That starts to sound a lot like using netif and node classes instead of
  the peer class.
  allow peer_t netif_t:netif egress;
  allow peer_t netnode_t:node egress;
 
 Thinking about this some more ... egress/ingress make sense from an interface 
 point of view but they sound out of place from a node point of view.  After 
 all, you are not egressing to a node, to are sending to a node.  The same 
 thing applies in the opposite direction, you don't ingress from a node, 
 you receive from a node.  With that in mind I'm thinking of going with the 
 following:
 
  allow netif_t peer_t:peer { ingress egress };
  allow netnode_t peer_t:peer { recv_from send_to };

nit:  recvfrom and sendto don't really require an underscore to be
readable, and we've already set a precedent for them in the socket and
association classes.

 Thoughts?  Should I just forget all this and use the peer label as a subject 
 label?

I'm not certain what we gain by using the peer as the object and class
in these checks, and it seems to make their meaning less clear.  It
should be noted that we already use process labels as both subjects
(when the actor) and objects (when the target/recipient of an action, as
in signal delivery or IPC), and that process labels flow to sockets
they create and socket labels flow to packets they send, and socket
labels likewise serve dual roles as subjects (Can this socket send/recv
this packet?) and objects (Can this process send/recv on this socket?). 

In the case of locally generated or destined traffic, we always have a
local socket that we can use as the subject of the check, which I think
is why we end up not using packet/peer as the subject generally - we
essentially have two subjects to choose from, and we favor the local
one.  But in the forwarded case, the packet/peer is the only
subject/actor in view really.

OTOH, I'm not sure your original concern about unlabeled_t is
well-founded now that I think about it; the netif and node type space is
disjoint and they default to discrete initial SIDs and types (netif_t,
node_t), not to unlabeled_t, right?  So the types themselves encode the
class there.

-- 
Stephen Smalley
National Security Agency

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

Re: [RFC PATCH v8 06/18] LSM: Add inet_sys_snd_skb() LSM hook

[Stephen Smalley]

);
   set_to_dummy_if_null(ops, xfrm_policy_clone_security);
 diff --git a/security/security.c b/security/security.c
 index 3bdcada..7f55459 100644
 --- a/security/security.c
 +++ b/security/security.c
 @@ -961,6 +961,12 @@ void security_req_classify_flow(const struct 
 request_sock *req, struct flowi *fl
  }
  EXPORT_SYMBOL(security_req_classify_flow);
  
 +int security_inet_sys_snd_skb(struct sk_buff *skb, int family)
 +{
 + return security_ops-inet_sys_snd_skb(skb, family);
 +}
 +EXPORT_SYMBOL(security_inet_sys_snd_skb);
 +
  void security_sock_graft(struct sock *sk, struct socket *parent)
  {
   security_ops-sock_graft(sk, parent);
 
 
 --
 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-security-module in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH v8 05/18] LSM: Add secctx_to_secid() LSM hook

[Stephen Smalley]

On Fri, 2007-12-14 at 16:50 -0500, Paul Moore wrote:
 Add a secctx_to_secid() LSM hook to go along with the existing
 secid_to_secctx() LSM hook.  This patch also includes the SELinux
 implementation for this hook.

Acked-by:  Stephen Smalley [EMAIL PROTECTED]

This one can go up anytime, as we have other people wanting such a hook
too.

Or alternatively we need to rationalize the entire selinux/exports
interface with these hooks as used by the networking and audit
subsystems, as that issue will be coming up anyway for other LSMs.

One thing to note is that some of these interfaces treat the context as
an opaque byte array of a given length, while other ones depend on the
context to be a NUL-terminated string (e.g. audit).

 ---
 
  include/linux/security.h |   13 +
  security/dummy.c |6 ++
  security/security.c  |6 ++
  security/selinux/hooks.c |6 ++
  4 files changed, 31 insertions(+), 0 deletions(-)
 
 diff --git a/include/linux/security.h b/include/linux/security.h
 index ac05083..db19c92 100644
 --- a/include/linux/security.h
 +++ b/include/linux/security.h
 @@ -1183,6 +1183,10 @@ struct request_sock;
   *   Convert secid to security context.
   *   @secid contains the security ID.
   *   @secdata contains the pointer that stores the converted security 
 context.
 + * @secctx_to_secid:
 + *  Convert security context to secid.
 + *  @secid contains the pointer to the generated security ID.
 + *  @secdata contains the security context.
   *
   * @release_secctx:
   *   Release the security context.
 @@ -1371,6 +1375,7 @@ struct security_operations {
   int (*getprocattr)(struct task_struct *p, char *name, char **value);
   int (*setprocattr)(struct task_struct *p, char *name, void *value, 
 size_t size);
   int (*secid_to_secctx)(u32 secid, char **secdata, u32 *seclen);
 + int (*secctx_to_secid)(char *secdata, u32 seclen, u32 *secid);
   void (*release_secctx)(char *secdata, u32 seclen);
  
  #ifdef CONFIG_SECURITY_NETWORK
 @@ -1603,6 +1608,7 @@ int security_setprocattr(struct task_struct *p, char 
 *name, void *value, size_t
  int security_netlink_send(struct sock *sk, struct sk_buff *skb);
  int security_netlink_recv(struct sk_buff *skb, int cap);
  int security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen);
 +int security_secctx_to_secid(char *secdata, u32 seclen, u32 *secid);
  void security_release_secctx(char *secdata, u32 seclen);
  
  #else /* CONFIG_SECURITY */
 @@ -2280,6 +2286,13 @@ static inline int security_secid_to_secctx(u32 secid, 
 char **secdata, u32 *secle
   return -EOPNOTSUPP;
  }
  
 +static inline int security_secctx_to_secid(char *secdata,
 +u32 seclen,
 +u32 *secid)
 +{
 + return -EOPNOTSUPP;
 +}
 +
  static inline void security_release_secctx(char *secdata, u32 seclen)
  {
  }
 diff --git a/security/dummy.c b/security/dummy.c
 index 3ccfbbe..0b62f95 100644
 --- a/security/dummy.c
 +++ b/security/dummy.c
 @@ -928,6 +928,11 @@ static int dummy_secid_to_secctx(u32 secid, char 
 **secdata, u32 *seclen)
   return -EOPNOTSUPP;
  }
  
 +static int dummy_secctx_to_secid(char *secdata, u32 seclen, u32 *secid)
 +{
 + return -EOPNOTSUPP;
 +}
 +
  static void dummy_release_secctx(char *secdata, u32 seclen)
  {
  }
 @@ -1086,6 +1091,7 @@ void security_fixup_ops (struct security_operations 
 *ops)
   set_to_dummy_if_null(ops, getprocattr);
   set_to_dummy_if_null(ops, setprocattr);
   set_to_dummy_if_null(ops, secid_to_secctx);
 + set_to_dummy_if_null(ops, secctx_to_secid);
   set_to_dummy_if_null(ops, release_secctx);
  #ifdef CONFIG_SECURITY_NETWORK
   set_to_dummy_if_null(ops, unix_stream_connect);
 diff --git a/security/security.c b/security/security.c
 index 0e1f1f1..3bdcada 100644
 --- a/security/security.c
 +++ b/security/security.c
 @@ -816,6 +816,12 @@ int security_secid_to_secctx(u32 secid, char **secdata, 
 u32 *seclen)
  }
  EXPORT_SYMBOL(security_secid_to_secctx);
  
 +int security_secctx_to_secid(char *secdata, u32 seclen, u32 *secid)
 +{
 + return security_ops-secctx_to_secid(secdata, seclen, secid);
 +}
 +EXPORT_SYMBOL(security_secctx_to_secid);
 +
  void security_release_secctx(char *secdata, u32 seclen)
  {
   return security_ops-release_secctx(secdata, seclen);
 diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
 index 9f3124b..8bb673b 100644
 --- a/security/selinux/hooks.c
 +++ b/security/selinux/hooks.c
 @@ -4710,6 +4710,11 @@ static int selinux_secid_to_secctx(u32 secid, char 
 **secdata, u32 *seclen)
   return security_sid_to_context(secid, secdata, seclen);
  }
  
 +static int selinux_secctx_to_secid(char *secdata, u32 seclen, u32 *secid)
 +{
 + return security_context_to_sid(secdata, seclen, secid);
 +}
 +
  static void selinux_release_secctx(char *secdata, u32 seclen)
  {
   kfree(secdata);
 @@ -4898,6 +4903,7 @@ static

Re: [RFC PATCH v8 09/18] SELinux: Only store the network interface's ifindex

[Stephen Smalley]

On Fri, 2007-12-14 at 16:50 -0500, Paul Moore wrote:
 Instead of storing the packet's network interface name store the ifindex.  
 This
 allows us to defer the need to lookup the net_device structure until the audit
 record is generated meaning that in the majority of cases we never need to
 bother with this at all.

 ---
 
  security/selinux/avc.c |   15 ---
  security/selinux/hooks.c   |4 ++--
  security/selinux/include/avc.h |7 +--
  3 files changed, 19 insertions(+), 7 deletions(-)
 
 diff --git a/security/selinux/avc.c b/security/selinux/avc.c
 index 81b3dff..8ecfab9 100644
 --- a/security/selinux/avc.c
 +++ b/security/selinux/avc.c
 @@ -661,9 +661,18 @@ void avc_audit(u32 ssid, u32 tsid,
   daddr, dest);
   break;
   }
 - if (a-u.net.netif)
 - audit_log_format(ab,  netif=%s,
 - a-u.net.netif);
 + if (a-u.net.netif = 0) {
 + struct net_device *dev;
 +
 + /* NOTE: we always use init's namespace */
 + dev = dev_get_by_index(init_net,
 +a-u.net.netif);
 + if (dev) {
 + audit_log_format(ab,  netif=%s,
 +  dev-name);
 + dev_put(dev);
 + }
 + }
   break;
   }
   }
 diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
 index 2ca8dfb..e429a8c 100644
 --- a/security/selinux/hooks.c
 +++ b/security/selinux/hooks.c
 @@ -3691,7 +3691,7 @@ static int selinux_socket_sock_rcv_skb(struct sock *sk, 
 struct sk_buff *skb)
   family = PF_INET;
  
   AVC_AUDIT_DATA_INIT(ad, NET);
 - ad.u.net.netif = skb-dev ? skb-dev-name : [unknown];
 + ad.u.net.netif = skb-iif;
   ad.u.net.family = family;
  
   err = selinux_parse_skb(skb, ad, addrp, len, 1, NULL);
 @@ -4023,7 +4023,7 @@ static unsigned int selinux_ip_postroute_last(unsigned 
 int hooknum,
   sksec = sk-sk_security;
  
   AVC_AUDIT_DATA_INIT(ad, NET);
 - ad.u.net.netif = dev-name;
 + ad.u.net.netif = dev-ifindex;
   ad.u.net.family = family;
  
   err = selinux_parse_skb(skb, ad, addrp, len, 0, proto);
 diff --git a/security/selinux/include/avc.h b/security/selinux/include/avc.h
 index 553607a..5185152 100644
 --- a/security/selinux/include/avc.h
 +++ b/security/selinux/include/avc.h
 @@ -51,7 +51,7 @@ struct avc_audit_data {
   struct inode *inode;
   } fs;
   struct {
 - char *netif;
 + int netif;
   struct sock *sk;
   u16 family;
   __be16 dport;
 @@ -77,7 +77,10 @@ struct avc_audit_data {
  
  /* Initialize an AVC audit data structure. */
  #define AVC_AUDIT_DATA_INIT(_d,_t) \
 -{ memset((_d), 0, sizeof(struct avc_audit_data)); (_d)-type = 
 AVC_AUDIT_DATA_##_t; }
 +{ memset((_d), 0, sizeof(struct avc_audit_data)); \
 +   (_d)-type = AVC_AUDIT_DATA_##_t; \
 +   if ((_d)-type == AVC_AUDIT_DATA_NET) \
 +   (_d)-u.net.netif = -1; }

As a minor nit, at the same time you do this, turn this into a static
inline function please.

  
  /*
   * AVC statistics
 
 
 --
 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-security-module in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH v8 10/18] SELinux: Add a network node caching mechanism similar to the sel_netif_*() functions

[Stephen Smalley]

, 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-security-module in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[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]

[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-security-module in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[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-security-module in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[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-security-module in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[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-security-module in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[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-security-module in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[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-security-module in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[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-security-module in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[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-security-module in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[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-security-module in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[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-security-module in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[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-security-module in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[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-security-module in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] namespaces: introduce sys_hijack (v10)

[Stephen Smalley]

On Tue, 2007-11-27 at 16:38 -0600, Serge E. Hallyn wrote:
 Quoting Stephen Smalley ([EMAIL PROTECTED]):
  On Tue, 2007-11-27 at 10:11 -0600, Serge E. Hallyn wrote:
   Quoting Crispin Cowan ([EMAIL PROTECTED]):
Just the name sys_hijack makes me concerned.

This post describes a bunch of what, but doesn't tell us about why
we would want this. What is it for?
   
   Please see my response to Casey's email.
   
And I second Casey's concern about careful management of the privilege
required to hijack a process.
   
   Absolutely.  We're definately still in RFC territory.
   
   Note that there are currently several proposed (but no upstream) ways to
   accomplish entering a namespace:
   
 1. bind_ns() is a new pair of syscalls proposed by Cedric.  An
 nsproxy is given an integer id.  The id can be used to enter
 an nsproxy, basically a straight current-nsproxy = target_nsproxy;
   
 2. I had previously posted a patchset on top of the nsproxy
 cgroup which allowed entering a nsproxy through the ns cgroup
 interface.
   
   There are objections to both those patchsets because simply switching a
   task's nsproxy using a syscall or file write in the middle of running a
   binary is quite unsafe.  Eric Biederman had suggested using ptrace or
   something like it to accomplish the goal.
   
   Just using ptrace is however not safe either.  You are inheriting *all*
   of the target's context, so it shouldn't be difficult for a nefarious
   container/vserver admin to trick the host admin into running something
   which gives the container/vserver admin full access to the host.
  
  I don't follow the above - with ptrace, you are controlling a process
  already within the container (hence in theory already limited to its
  container), and it continues to execute within that container.  What's
  the issue there?
 
 Hmm, yeah, I may have overspoken - I'm not good at making up exploits
 but while I see it possible to confuse the host admin by setting bogus
 environment, I guess there may not be an actual exploit.
 
 Still after the fork induced through ptrace, we'll have to execute a
 file out of the hijacked process' namespaces and path (unless we get
 *really* 'exotic').  With hijack, execution continues under the caller's
 control, which I do much prefer.
 
 The remaining advantages of hijack over ptrace (beside using ptrace for
 that is crufty) are
 
   1. not subject to pid wraparound (when doing hijack_cgroup
  or hijack_ns)
   2. ability to enter a namespace which has no active processes

So possibly I'm missing something, but the situation with hijack seems
more exploitable than ptrace to me - you've created a hybrid task with
one foot in current's world (open files, tty, connection to parent,
executable) and one foot in the target's world (namespaces, uid/gid)
which can then be leveraged by other tasks within the target's
world/container as a way of breaking out of the container.  No?

 These also highlight selinux issues.  In the case of hijacking an
 empty cgroup, there is no security context (because there is no task) so
 the context of 'current' will be used.  In the case of hijacking a
 populated cgroup, a task is chosen at random to be the hijack source.

Seems like you might be better off with a single operation for creating
a new task within a given namespace set / cgroup rather than trying to
handle multiple situations with different semantics / inheritance
behavior.  IOW, forget about hijacking a specific pid or picking a task
at random from a populated cgroup - just always initialize the state of
the newly created task in the same manner based solely on elements of
the caller's state and the cgroup's state.

 So there are two ways to look at deciding which context to use.  Since
 control continues in the original acting process' context, we might
 want the child to continue in its context.  However if the process
 creates any objects in the virtual server, we don't want them
 mislabeled, so we might want the task in the hijacked task's context.

I suspect that we want to continue in the parent's context, and then the
program can always use setfscreatecon() or exec a helper in a different
context if it wants to create files with contexts tailored to the
target.

 Sigh.  So here's why I thought I'd punt on selinux at least until I had
 a working selinux-enforced container/vserver  :)

-- 
Stephen Smalley
National Security Agency

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

Re: Path of task via LSM

[Stephen Smalley]

On Tue, 2007-11-27 at 09:31 -0500, Andrew Blaich wrote:
 Is there a way to retrieve the full path of the currently running task
 via a task_struct or similar structure from the view of the LSM?
 Currently, the work I am doing with LSM design requires hooking into
 bprm_alloc_security in order to read the linux_binprm structure which
 contains the path of the application as it was run.  Is there a
 similar way to access the full path dynamically say if I'm checking a
 socket based LSM hook which does not receive the linux_binprm
 structure
 
 I have been unsuccessful in using dentry and vfsmnt from the current
 task_struct via the d_path() lookup function.

audit_log_task_info() is an example.

It isn't a perfect technique, but usually yields the expected answer.

But I wouldn't recommend doing that on every LSM hook call.

-- 
Stephen Smalley
National Security Agency

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

Re: [PATCH 1/2] namespaces: introduce sys_hijack (v10)

[Stephen Smalley]

On Tue, 2007-11-27 at 10:11 -0600, Serge E. Hallyn wrote:
 Quoting Crispin Cowan ([EMAIL PROTECTED]):
  Just the name sys_hijack makes me concerned.
  
  This post describes a bunch of what, but doesn't tell us about why
  we would want this. What is it for?
 
 Please see my response to Casey's email.
 
  And I second Casey's concern about careful management of the privilege
  required to hijack a process.
 
 Absolutely.  We're definately still in RFC territory.
 
 Note that there are currently several proposed (but no upstream) ways to
 accomplish entering a namespace:
 
   1. bind_ns() is a new pair of syscalls proposed by Cedric.  An
   nsproxy is given an integer id.  The id can be used to enter
   an nsproxy, basically a straight current-nsproxy = target_nsproxy;
 
   2. I had previously posted a patchset on top of the nsproxy
   cgroup which allowed entering a nsproxy through the ns cgroup
   interface.
 
 There are objections to both those patchsets because simply switching a
 task's nsproxy using a syscall or file write in the middle of running a
 binary is quite unsafe.  Eric Biederman had suggested using ptrace or
 something like it to accomplish the goal.
 
 Just using ptrace is however not safe either.  You are inheriting *all*
 of the target's context, so it shouldn't be difficult for a nefarious
 container/vserver admin to trick the host admin into running something
 which gives the container/vserver admin full access to the host.

I don't follow the above - with ptrace, you are controlling a process
already within the container (hence in theory already limited to its
container), and it continues to execute within that container.  What's
the issue there?

 That's where the hijack idea came from.  Yes, I called it hijack to make
 sure alarm bells went off :) bc it's definately still worrisome.  But at
 this point I believe it is the safest solution suggested so far.

-- 
Stephen Smalley
National Security Agency

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

Re: Missing security_file_permission() check from sys_splice()

[Stephen Smalley]

On Thu, 2007-11-08 at 23:20 -0600, Lin Tan wrote:
 Seems that an unauthorized user can send file through sockets due to  
 the following missing check errors.
 
 There is not security_file_permission() check from sys_splice(),  
 which can invoke sock_sendpage(). The call chain is as follows.
 sys_splice - do_splice - do_splice_from - generic_splice_sendpage  
 (via function pointer out-f_op-splice_write, which is set up in net/ 
 socket.c) - pipe_to_sendpage - sock_sendpage ( via file-f_op- 
  sendpage, in net/socket.c)
 
 I believe sock_sendpage() needs to be protected by  
 security_file_permission() for two reasons. First,  in the following  
 path it is protected.
 
 sys_sendfile - do_sendfile - file_send_actor - sock_sendpage
 
 Second, if it is not protected, then unauthorized user can send file  
 through sockets.  Adding the check in do_splice_from() should solve  
 the problem.
 
 Similar problems exit in do_splice_to() and probably in sys_vmspliace 
 () too.

What kernel version are you looking at?

Current kernel has security_file_permission() calls in do_splice_from()
and do_splice_to().

BTW, one might argue that for the socket case, these should be mediated
by the socket hooks, which does happen if the sendpage operation falls
back to sock_no_sendpage() - kernel_sendmsg() - sock_sendmsg().  But
that doesn't happen when the protocol implementation implements its own
sendpage operations, of course.  So possibly there should be a socket
security hook call in sock_sendpage().

-- 
Stephen Smalley
National Security Agency

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

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

[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-security-module in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC 1/2] capabilities: fix compilation with strict type checking (v2)

[Stephen Smalley]

On Thu, 2007-10-25 at 17:41 -0700, Chris Wright wrote:
 * Casey Schaufler ([EMAIL PROTECTED]) wrote:
  --- Chris Wright [EMAIL PROTECTED] wrote:
  
   * Serge E. Hallyn ([EMAIL PROTECTED]) wrote:
Here is a new version of the 64-bit capability patches I was supposed
to send last week I think.

This patch could stand alone without the 64-bit caps, but should
definately not be applied anywhere until it has been better
reviewed.  It is the alternative to the patch removing the
capability type checking code.
   
   How likely is  64?
  
  If the Granularity Gremlins get loose the answer is 100%.
  DG/UX ended up with over 330.
 
 Yeah, I think a few systems ended up with  64.

I think the current Solaris and FreeBSD implementations support
extensible privilege sets, and that Solaris already has  64.

 
  Fortunately the GGs have a playpen already in SELinux.
  I suggest that the capabilities maintainer be very stingy
  and refer anyone who's need isn't pretty obvious there.
  This means that the folks who want to divide CAP_SYSADMIN
  are going to be disappointed with what they get, but some
  level of restraint is important.
 
 Sure, I guess my point is, if we open up to 64, how quickly
 will we hit 65.  Perhaps a generic bitmask is better, and then
 we need a stricter type mode anyway.
-- 
Stephen Smalley
National Security Agency

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

Re: [PATCH 1/2] VFS/Security: Rework inode_getsecurity and callers to return resulting buffer

[Stephen Smalley]

On Fri, 2007-10-26 at 10:02 -0500, Serge E. Hallyn wrote:
 Quoting David P. Quigley ([EMAIL PROTECTED]):
  On Thu, 2007-10-25 at 19:02 -0500, Serge E. Hallyn wrote:
   Quoting David P. Quigley ([EMAIL PROTECTED]):
 static int task_alloc_security(struct task_struct *task)
@@ -2423,14 +2397,22 @@ static const char 
*selinux_inode_xattr_getsuffix(void)
  *
  * Permission check is handled by selinux_inode_getxattr hook.
  */
-static int selinux_inode_getsecurity(const struct inode *inode, const 
char *name, void *buffer, size_t size, int err)
+static int selinux_inode_getsecurity(const struct inode *inode,
+   const char *name,
+   void **buffer)
 {
+   u32 size;
+   int error;
struct inode_security_struct *isec = inode-i_security;
 
if (strcmp(name, XATTR_SELINUX_SUFFIX))
return -EOPNOTSUPP;
 
-   return selinux_getsecurity(isec-sid, buffer, size);
+   error = security_sid_to_context(isec-sid, (char **)buffer, 
size);
   
   The only other downside I see here is that when the user just passes in
   NULL for a buffer, security_sid_to_context() will still
   kmalloc the buffer only to have it immediately freed by
   xattr_getsecurity() through release_secctx().  I trust that isn't seen
   as any major performance impact?
  
  There is no way to avoid this in the SELinux case. SELinux doesn't store
  the sid to string mapping directly. Rather it takes the sid and then
  builds the string from fields in the related structure. So regardless
  this data is being allocated internally. The only issue I potentially
  see is that if someone passes in null expecting just to get the length
  we are actually returning a value. However we are changing the semantics
  of the function so the old semantics are no longer valid.
 
 Hmm?  Which semantics are no longer valid?
 
 You're changing the semantincs of the in-kernel API, but userspace can
 still send in NULL to query the length of the buffer needed.  So if
 userspace does two getxattrs, one to get the length, then another to get
 the value, selinux will be kmallocing twice.
 
 For a file manager doing a listing on a huge directory and wanting to
 list the selinux type, i could see that being a performance issue.  Of
 course they could get around that by sending in a 'reasonably large'
 buffer for a first try.

That's what current userland does. libselinux always tries with an
initial buffer first (and usually succeeds), thereby avoiding the second
call to getxattr in the common case.

-- 
Stephen Smalley
National Security Agency

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

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

[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-security-module in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[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-security-module in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[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-security-module in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[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-security-module in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[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: [RFC]selinux: Improving SELinux read/write performance

[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-security-module in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[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: [PATCH 00/16] Permit filesystem local caching [try #3]

[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-security-module in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[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-security-module in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[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-security-module in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: LSM: memory and user-space interaction

[Stephen Smalley]

On Tue, 2007-08-07 at 16:11 +0800, Cliffe wrote:
 G’day,
 
 I would really appreciate some advice.
 
 I realise the kernel has a small stack, and I imagine this will have a 
 greater impact on my LSM design than I originally thought. I would 
 really appreciate some input.
 
 My LSM has a hierarchical policy structure which is made up of a 
 (relatively) complex relationship of structs. A network of structs 
 represents policy and when a task is executed a tree of structs is 
 extracted and duplicated from this structure to create the task’s policy 
 (this separate tree is required for other reasons).
 
 I have written a user-space program which reads in policy and creates 
 the hierarchical policy structure. I was going to feed this policy into 
 the LSM via a securityfs and move the code which creates the policy 
 structure into the LSM; so that the entire system policy is represented 
 within the LSM. Then when a task is executed the LSM just makes the tree 
 which represents the task’s security context and associates it with the 
 task.
 
 My concern is that the whole hierarchical policy structure will be 
 loaded into kernel space and I do not really know if there will be 
 enough memory for it. Does this seem to be a problem?

Don't confuse kernel stack limitation with the ability to dynamically
allocate memory in the kernel.

How large is your policy?  I'm guessing that SELinux reference policy is
larger, e.g. from /proc/slabinfo:
# nameactive_objs num_objs objsize objperslab 
pagesperslab : tunables limit batchcount sharedfactor : slabdata 
active_slabs num_slabs sharedavail : globalstat listallocs maxobjs 
grown reaped error maxfreeable nodeallocs remotefrees 
alienoverflow : cpustat allochit allocmiss freehit freemiss
avtab_node261133 261188 40   921 : tunables   32   168 : 
slabdata   2839   2839  0 : globalstat  261144 261144  28390
 00000 : cpustat 244102  17031  0  0


 Would I be better off keeping the system-wide hierarchical policy 
 structure in user space and only somehow creating the task tree in the 
 LSM? This would require the LSM to pull information from user-space 
 (when a binary is executed) which seems to be a no-no.
 
 I also have a related question: my policy includes the option to specify 
 allowed one-way-hashes (such as SHA-1) of a binary. How can I (and am I 
 allowed to) pull this information (the hash of a specified binary) from 
 my user-space daemon? (the simplest way I can think of is to have the 
 user-space program monitor a securityfs file for changes - but this 
 could be a very slow way)
 
 Thanks again,
 
 Cliffe.
 
 -
 To unsubscribe from this list: send the line unsubscribe 
 linux-security-module in
 the body of a message to [EMAIL PROTECTED]
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
-- 
Stephen Smalley
National Security Agency

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

Re: [PATCH][RFC] V2 Remove SELinux dependencies from linux-audit via LSM

[Stephen Smalley]

On Sun, 2007-08-05 at 17:03 -0700, Casey Schaufler wrote:
 From: Casey Schaufler [EMAIL PROTECTED]
 
 This patch interposes LSM interfaces between the audit system
 and SELinux. This helps make SELinux a cleaner LSM and clarifies
 the interfaces provided by the audit system. The audit system
 no longer requires SELinux functions or data structures, making
 it available for use by other LSMs.
 
 The audit system interfaces should now be useful to any LSM that
 can provide secids and text string representations that match them.
 The audit system uses secids only to map to those strings and
 treats them as opaque data otherwise. Audit rule information that
 is specific to an LSM is maintained through a void *. 
 
 The SELinux code uses LSM interfaces to access the audit system,
 with the exception of audit_rule_update_callout(), which is
 intended to be called at the descretion of an LSM to update the
 LSM specific rules.
 
 The LSM interface includes six new entries, four for audit and two
 that supply secids from the LSM to networking and audit subsystems.
 Also, there were several cases where SELinux code was being called
 where LSM interfaces were more appropriate. These uses have been
 repaired and the SELinux interfaces are no longer exported.
 
 Signed-off-by: Casey Schaufler [EMAIL PROTECTED]

Looks sane, but the patch is malformed again, so I can't apply it.

There are still a few places where you appear to be making
sparse-induced cleanups of other code unrelated to this change, so make
those separate patches (and be sure you aren't duplicating what is
already upstream).

You've tested the resulting kernel?  Built with a variety of configs?

 ---
 
 I believe that I've addressed the comments from V1. Thank
 you for your contributions.
 
  diffstat -p1 -w 70 audit0805.patch
  include/linux/audit.h   |   55 +-
  include/linux/security.h|   80 +++
  include/linux/selinux.h |  138 --
  kernel/audit.c  |   43 +++-
  kernel/audit.h  |   41 ---
  kernel/auditfilter.c|   72 -
  kernel/auditsc.c|   45 
  net/netlink/af_netlink.c|3
  security/selinux/exports.c  |   42 ---
  security/selinux/hooks.c|   55 +-
  security/selinux/include/security.h |   38 +++
  security/selinux/ss/services.c  |   19 +--
  12 files changed, 293 insertions(+), 338 deletions(-)
 
 
 
 diff -uprN -X linux-2.6.22-base/Documentation/dontdiff
 linux-2.6.22-base/include/linux/audit.h
 linux-2.6.22-audit/include/linux/audit.h
 --- linux-2.6.22-base/include/linux/audit.h   2007-07-08 16:32:17.0 
 -0700
 +++ linux-2.6.22-audit/include/linux/audit.h  2007-08-02 09:37:26.0
 -0700
 @@ -321,20 +321,63 @@ struct audit_rule { /* for AUDIT_LIST, 
  #ifdef __KERNEL__
  #include linux/sched.h
  
 -struct audit_sig_info {
 - uid_t   uid;
 - pid_t   pid;
 - charctx[0];
 -};
 -
  struct audit_buffer;
  struct audit_context;
 +struct audit_parent;
  struct inode;
  struct netlink_skb_parms;
  struct linux_binprm;
  struct mq_attr;
  struct mqstat;
  
 +struct audit_sig_info {
 + uid_t   uid;
 + pid_t   pid;
 + charctx[0];
 +};
 +
 +struct audit_watch {
 + atomic_tcount;  /* reference count */
 + char*path;  /* insertion path */
 + dev_t   dev;/* associated superblock device */
 + unsigned long   ino;/* associated inode number */
 + struct audit_parent *parent; /* associated parent */
 + struct list_headwlist;  /* entry in parent-watches list */
 + struct list_headrules;  /* associated rules */
 +};
 +
 +struct audit_field {
 + u32 type;
 + u32 val;
 + u32 op;
 + char*se_str;
 + void*se_rule;
 +};
 +
 +struct audit_krule {
 + int vers_ops;
 + u32 flags;
 + u32 listnr;
 + u32 action;
 + u32 mask[AUDIT_BITMASK_SIZE];
 + u32 buflen; /* for data alloc on list rules */
 + u32 field_count;
 + char*filterkey; /* ties events to rules */
 + struct audit_field  *fields;
 + struct audit_field  *arch_f; /* quick access to arch field */
 + struct audit_field  *inode_f; /* quick access to an inode field */
 + struct audit_watch  *watch; /* associated watch */
 + struct list_headrlist;  /* entry in audit_watch.rules list */
 +};
 +
 +struct audit_entry {
 + struct list_headlist;
 + struct rcu_head rcu;
 + struct audit_krule  rule;
 +};
 +
 +extern int audit_rule_update_callout(void);
 +
  #define 

Re: [PATCH][RFC] V1 Remove SELinux dependencies from linux-audit via LSM

[Stephen Smalley]

;
 + struct nameidata *ndw = NULL;

Not related to your changes, and seems to already be present in git (but
without splitting into two lines).

 diff -uprN -X linux-2.6.22-base/Documentation/dontdiff
 linux-2.6.22-base/security/selinux/hooks.c
 linux-2.6.22-audit/security/selinux/hooks.c
 --- linux-2.6.22-base/security/selinux/hooks.c2007-07-08 
 16:32:17.0
 -0700
 +++ linux-2.6.22-audit/security/selinux/hooks.c   2007-08-01 
 21:42:32.0
 -0700
 @@ -4838,6 +4862,12 @@ static struct security_operations selinu
  
   .secid_to_secctx =  selinux_secid_to_secctx,
   .release_secctx =   selinux_release_secctx,
 + .inode_getsecid =   selinux_get_inode_sid,
 + .ipc_getsecid = selinux_get_ipc_sid,
 + .audit_rule_supplied =  selinux_audit_rule_supplied,
 + .audit_rule_match = selinux_audit_rule_match,
 + .audit_rule_init =  selinux_audit_rule_init,
 + .audit_rule_free =  selinux_audit_rule_free,

So I'd then expect you to also remove the function prototypes from
include/linux/selinux.h unless there is some other caller, and move the
functions from selinux/exports.c to selinux/hooks.c and make them
static.  And drop the selinux_enabled check from them, as the hook
function will only be registered if SELinux is enabled.

-- 
Stephen Smalley
National Security Agency

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

Re: [PATCH][RFC] V1 Remove SELinux dependencies from linux-audit via LSM

[Stephen Smalley]

On Fri, 2007-08-03 at 09:33 -0700, Casey Schaufler wrote:
 --- Casey Schaufler [EMAIL PROTECTED] wrote:
 
diff -uprN -X linux-2.6.22-base/Documentation/dontdiff
linux-2.6.22-base/include/linux/security.h
linux-2.6.22-audit/include/linux/security.h
--- linux-2.6.22-base/include/linux/security.h  2007-07-08
   16:32:17.0
-0700
+++ linux-2.6.22-audit/include/linux/security.h 2007-08-01
   20:14:18.0
-0700
@@ -35,6 +35,8 @@
 #include net/flow.h
 
 struct ctl_table;
+struct audit_krule;
+struct selinux_audit_rule;
   
   selinux_audit_rule in LSM interface?
  
  The structure needs a new name. Any objections to audit_rule_lsm?
  I'd suggest security_audit_rule, but that doesn't say anything about
  where to look to see how it gets used.
 
 Actually, it's worse than that because an selinux_audit_rule really
 is SELinux specific. Any problem with making the security_audit_rule
 interfaces use a void * ? The audit code appears to be accomodating.

The struct is already opaque outside of the security module, so you can
just rename it and implement your own version of the struct in your
module.

-- 
Stephen Smalley
National Security Agency

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

Re: [PATCH RFC] file capabilities: clear fcaps on inode change

[Stephen Smalley]

On Sun, 2007-07-29 at 08:48 -0700, Casey Schaufler wrote:
 --- Serge E. Hallyn [EMAIL PROTECTED] wrote:
 
  Quoting Andrew Morgan ([EMAIL PROTECTED]):
   -BEGIN PGP SIGNED MESSAGE-
   Hash: SHA1
   
   Is this the sort of change that should be abstracted into the security
   module API?
   
   To this point, everything about the fcap changes have been in headers
   and within the security module code.
  
  Yes.  I'd thought about adding a security_ops-inode_change() or
  somesuch hook, but there were two reasons I didn't.  First, this
  should be done whether or not the capability module is loaded in
  this kernel.  If we're testing a kernel with only the dummy
  module, we still want this to happen.  Second, only the capability
  module needs this so far.  If more modules end up needing this then
  we can make it more generic. But note that most security modules
  will likely label data the way selinux does, for classification for
  access control, rather than for granting privilege to unprivileged
  processes.
 
 Hum. I'm sure that my understanding is imperfect, but since the
 SELinux policy will depend on the implementation of an application
 (e.g. login, crontab) and the label on an application will depend
 on the policy (it's not a name based scheme, after all) I should
 think that overwriting a program binary on SELinux ought to result
 in a change to the label on the binary, and that the new label ought
 to be defined by the policy. That would argue strongly for an LSM
 inode_change() hook that allows SELinux to set the label according
 to the SELinux policy.

No, labels don't change when an existing file is overwritten; if the
process was allowed to overwrite the file by policy, then the label is
already appropriate.  One would prevent untrusted entities from writing
to the labels of trusted programs in the first place in one's policy.

-- 
Stephen Smalley
National Security Agency

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

Re: [xfs-masters] [RFC: 2.6 patch] make the *FS_SECURITY options no longer user visible

[Stephen Smalley]

On Mon, 2007-07-30 at 09:29 +1000, David Chinner wrote:
 On Sun, Jul 29, 2007 at 05:02:09PM +0200, Adrian Bunk wrote:
  Please correct me if any of the following assumptions is wrong:
  - SELinux is currently the only user of filesystem security labels
shipped with the Linux kernel
  - if a user has SELinux enabled he wants his filesystems to support
security labels
  
  Based on these assumption, it doesn't make sense to have the
  *FS_SECURITY user visible since we can perfectly determine automatically 
  when turning them on makes sense.
 
 Hmmm. The code in XFS is not dependent on selinux, but this change
 would mean that testing the security xattr namespace would require a
 selinux enabled kernel.
 
 I agree that the default for these should be y and selected if
 selinux is enabled, but forcing us to use selinux enabled kernels
 (on distro's that may not support selinux) just to test the
 security xattr namespace is a bit of a pain.

You can enable SECURITY_SELINUX in the kernel config but still have it
boot disabled by default via SECURITY_SELINUX_BOOTPARAM_VALUE=0.

-- 
Stephen Smalley
National Security Agency

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

Re: [RFC][PATCH] Version5 - Simplified mandatory access control kernel implementation

[Stephen Smalley]

snip
+
+/*
+ * I hope these are the hokeyist lines of code in the module. Casey.
+ */
+#define DEVPTS_SUPER_MAGIC 0x1cd1
+#define SOCKFS_MAGIC   0x534F434B
+#define PIPEFS_MAGIC   0x50495045
+#define TMPFS_MAGIC0x01021994
snip
+   /*
+* This is pretty hackish.
+* Casey says that we shouldn't have to do
+* file system specific code, but it does help
+* with keeping it simple.
+*/
+   switch (sbp-s_magic) {
+   case SMACK_MAGIC:
...

Rather than using the filesystem magic numbers, why not just map the
filesystem type string to a desired labeling behavior upon sb_kern_mount
and then use that behavior in d_instantiate, as is done in SELinux?  For
SELinux, the mapping is maintained in policy, but naturally you could
define a fixed mapping in your module if you prefer (and in fact,
SELinux originally did that too).

Or if you are going to use the magic numbers, I think you need separate
patches to move the definitions from their current locations to
include/linux/magic.h and then #include that file in both places rather
than replicating their definitions in your code.

Another option would be to push the responsibility to the filesystems,
e.g. define some new flags for file_system_type struct that indicate the
right behavior for labeling inodes, and have the security module only
use those flags rather than needing to know about individual filesystem
types.

-- 
Stephen Smalley
National Security Agency

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

Re: [RFC][PATCH] Simplified mandatory access control kernel implementation

[Stephen Smalley]

On Wed, 2007-07-18 at 18:15 -0700, Casey Schaufler wrote:
 --- Joshua Brindle [EMAIL PROTECTED] wrote:
 
  Casey Schaufler wrote:
   ... 
  
   I do have a hackish newsmack command, which I should probably include.
   All it does is write the new label to /proc/self/attr/current and
   exec the desired program. That's not good enough for a production
   system because of the well known pty, tty, and open files issues,
   but fine for development purposes.

  
  Right, I'd like to see how you solve those problems :)
 
 Me too. Especially with devpts out of bounds. I have some ideas,
 but I don't know whether they're good ones yet.

I'm not sure what you mean by the above.  In selinux, we label the ptys
with a label derived from the allocating task at creation time (handled
by d_instantiate hook), and then let userspace relabel them as
appropriate based for the user session label for login/sshd and newrole.

-- 
Stephen Smalley
National Security Agency

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

Re: [RFC][PATCH] Version3 - Simplified mandatory access control kernel implementation

[Stephen Smalley]

On Wed, 2007-07-18 at 20:46 -0700, Casey Schaufler wrote:
 --- Stephen Smalley [EMAIL PROTECTED] wrote:
 
  On Tue, 2007-07-17 at 19:59 -0700, Casey Schaufler wrote:
 - Speaking of which, are you ok with your MAC model being overridden 
 by
 all uid 0 processes?  Or do you plan to change securebits and use file
 caps?
   
   I've been tracking the file caps closely. I like file capabilities,
   but I have had success with uid 0 MAC systems as well. Long term, yes
   I want file caps, but the uid 0 world is important, too.
  
  So uid == 0 is still all powerful with your module, right?
  No restrictions on any uid 0 process, not only for authenticated root
  shells but for any system process running with uid 0 or any setuid root
  program.  They can override your MAC model at will, as well as
  indirectly subvert it via other capabilities.  Whereas most (all?) other
  security modules to date allow you to restrict what capabilities can be
  exercised by a uid 0 process.
 
 Smack is intended to be a MAC module. I want to leave the privilege
 model as an independent issue.

It can be a separate mechanism, but there is a dependency there
naturally (for protection of the MAC model).

Other question there is if you aren't going to recheck on use, then what
story do you tell wrt relabeling of files and/or tasks changing labels,
given that you permit both to happen?
   
   It requires privilege. Really, that's the answer that's been used
   on every B1/LSPP evaluation up until yours (Congratulations, BTW)
   and an important aspect of the system is that labels don't change
   very often, and only under controlled circumstances.
  
  The labels don't change very often bit is a good principle, albeit
  hard to achieve in an imperfect world.
 
 I think that the smack rules model will be helpful there, although
 it will bring up issues of it's own.

Hey, that's the selinux way of thinking - change the rules, not the
label.  The label describes the data (security properties thereof) and
doesn't need to change.

  The under controlled
  circumstances seems hard to guarantee since to change task or file
  labels at all in your model, you are required to have complete
  MAC_OVERRIDE anyway, so there are no restrictions from a MAC point of
  view at all, and you have no other mechanism to protect and limit the
  process, other than DAC, which is fairly weak in its ability to provide
  such guarantees.
 
 That's an argument in favor of a fine grain privilege mechanism.
 MAC_OVERRIDE is likely too coarse, where a capabilty for each
 individual check is likely finer than is helpful.
 
  That's one of the benefits of TE, being able to
  protect and limit MLS trusted subjects more effectively.  
 
 If you like that level of granularity it's hard to argue
 that TE is a poor choice.

It does let you apply MAC overrides at the granularity not only of
per-operation but per-object/label (i.e. trusted downgrader can
downgrade these objects but not those objects, and can further only
downgrade from X to Y), as well as help protect the integrity of the
trusted subject.  BTW, do you envision using this mechanism to express
composite BLP+Biba simultaneously?  If not, what's the plan for an
integrity mechanism to complement BLP?

  There is no locking evident in the patch at all, either for your global
  data (e.g. smack_list, smk_links) or for your per-object security blobs
  (e.g. look at how selinux does locking for setup of the superblock and
  inode security blobs).
 
 I'm having a look through the locking. I don't think it has to
 be as extensive as SELinux. Fewer lists to deal with.

Not sure it really differs substantially aside from the fact that you
don't have any security decision cache (AVC).

  Also make sure you don't re-init your inode
  security blob in d_instantiate after having already set it up once.
 
 The inode security blob is a single smack_t.

Yup, that's still larger than a word (which is a nice property of the
SELinux SIDs for manipulation), and you still don't want to clobber it
once it has been set (you don't want a subsequent d_instantiate on the
same inode to replace the smack label once it has been set; you don't
want to clobber a previous setxattr with a re-init for e.g. your devpts
labels).

  You'll eventually have to broach the issue of getting your own
  capability bit rather than reusing an existing one.
 
 When the expanded capability set goes in and bits become
 available the whole if and if so how many discussion awaits.

Sorry, what expanded capability set? File capabilities patch was still
limited to 32 bit capabilities last I looked, and increasingly less
forward compatible with future extensions.  Serge?

-- 
Stephen Smalley
National Security Agency

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

Re: [RFC][PATCH] Simplified mandatory access control kernel implementation

[Stephen Smalley]

On Thu, 2007-07-19 at 08:26 -0700, Casey Schaufler wrote:
 --- Stephen Smalley [EMAIL PROTECTED] wrote:
 
  On Wed, 2007-07-18 at 18:15 -0700, Casey Schaufler wrote:
   --- Joshua Brindle [EMAIL PROTECTED] wrote:
   
Casey Schaufler wrote:
 ... 

 I do have a hackish newsmack command, which I should probably include.
 All it does is write the new label to /proc/self/attr/current and
 exec the desired program. That's not good enough for a production
 system because of the well known pty, tty, and open files issues,
 but fine for development purposes.
  

Right, I'd like to see how you solve those problems :)
   
   Me too. Especially with devpts out of bounds. I have some ideas,
   but I don't know whether they're good ones yet.
  
  I'm not sure what you mean by the above.
 
 There was discussion some time back in which changes to devpts
 had been suggested but rejected due to the firm stand of the devpts
 maintainers. I can dredge the thread up if it's important.

Not sure which thread that was.

  In selinux, we label the ptys
  with a label derived from the allocating task at creation time (handled
  by d_instantiate hook),
 
 So far so good ...
 
  and then let userspace relabel them as
  appropriate based for the user session label for login/sshd and newrole.
 
 ... which may be the end solution. Another way to do it is to
 associate a label with the connection between the pseudo devices
 rather than the endpoints. That might require devpts changes.
 If there is a kernel based solution, and I don't have one in hand
 yet, it would be slick.

There is the known problem with newrole relabeling of the pty leaving
the master end unchanged.

-- 
Stephen Smalley
National Security Agency

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

Re: [RFC][PATCH] Version3 - Simplified mandatory access control kernel implementation

[Stephen Smalley]

On Tue, 2007-07-17 at 19:59 -0700, Casey Schaufler wrote:
   - Speaking of which, are you ok with your MAC model being overridden by
   all uid 0 processes?  Or do you plan to change securebits and use file
   caps?
 
 I've been tracking the file caps closely. I like file capabilities,
 but I have had success with uid 0 MAC systems as well. Long term, yes
 I want file caps, but the uid 0 world is important, too.

So uid == 0 is still all powerful with your module, right?
No restrictions on any uid 0 process, not only for authenticated root
shells but for any system process running with uid 0 or any setuid root
program.  They can override your MAC model at will, as well as
indirectly subvert it via other capabilities.  Whereas most (all?) other
security modules to date allow you to restrict what capabilities can be
exercised by a uid 0 process.

  Other question there is if you aren't going to recheck on use, then what
  story do you tell wrt relabeling of files and/or tasks changing labels,
  given that you permit both to happen?
 
 It requires privilege. Really, that's the answer that's been used
 on every B1/LSPP evaluation up until yours (Congratulations, BTW)
 and an important aspect of the system is that labels don't change
 very often, and only under controlled circumstances.

The labels don't change very often bit is a good principle, albeit
hard to achieve in an imperfect world.  The under controlled
circumstances seems hard to guarantee since to change task or file
labels at all in your model, you are required to have complete
MAC_OVERRIDE anyway, so there are no restrictions from a MAC point of
view at all, and you have no other mechanism to protect and limit the
process, other than DAC, which is fairly weak in its ability to provide
such guarantees.  That's one of the benefits of TE, being able to
protect and limit MLS trusted subjects more effectively.  

 Updated patch:
 
 http://www.schaufler-ca.com/data/smack-0717A-patch.tar

Ultimately, you have to actually post the patches inline if you want
more extensive comments and/or to actually submit anything.

Don't forget to follow the guidance in Documentation/SubmitChecklist and
run scripts/checkpatch.pl on it.  And check it via sparse.

There is no locking evident in the patch at all, either for your global
data (e.g. smack_list, smk_links) or for your per-object security blobs
(e.g. look at how selinux does locking for setup of the superblock and
inode security blobs).  Also make sure you don't re-init your inode
security blob in d_instantiate after having already set it up once.

You'll eventually have to broach the issue of getting your own
capability bit rather than reusing an existing one.

Obviously those key hooks need to be filled in (or dropped).

-- 
Stephen Smalley
National Security Agency

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

Re: [RFC] [PATCH 2/2] file capabilities: change fE to a bool

[Stephen Smalley]

On Wed, 2007-07-18 at 12:53 -0500, Serge E. Hallyn wrote:
 Quoting Andrew Morgan ([EMAIL PROTECTED]):
  -BEGIN PGP SIGNED MESSAGE-
  Hash: SHA1
  
  Serge,
  
  I spent the evening getting my local build of libcap (building the
  libcap/progs/old/setcap and getcap tools) working with the new kernel
  support.
  
  It seems there is a basic insecurity bug in the xattr support insofar as
  doing the following does not delete the capabilities on a file when I
  copy over it...:
  
  [EMAIL PROTECTED] progs]$ cd ~
  [EMAIL PROTECTED] progs]$ cp /bin/ping .
  [EMAIL PROTECTED] progs]$ ./ping localhost
  [EMAIL PROTECTED] progs]$ ping: icmp open socket: Operation not permitted
  [EMAIL PROTECTED] progs]$ sudo setcap cap_net_raw=ep ping
  [EMAIL PROTECTED] progs]$ getcap ping
  Capabilities for `ping':
  = cap_net_raw+ep
  [EMAIL PROTECTED] progs]$ ./ping localhost
  64 bytes from localhost.localdomain (127.0.0.1): icmp_seq=0 ttl=64
  time=0.056 ms64 bytes from localhost.localdomain (127.0.0.1): icmp_seq=1
  ttl=64 time=0.058 ms
  Ctrl-C
  - --- localhost.localdomain ping statistics ---
  2 packets transmitted, 2 received, 0% packet loss, time 1009ms
  rtt min/avg/max/mdev = 0.056/0.057/0.058/0.001 ms, pipe 2
  [EMAIL PROTECTED] progs]$ cp /bin/ping .
  [EMAIL PROTECTED] progs]$ ./ping localhost
  PING localhost.localdomain (127.0.0.1) 56(84) bytes of data.
  64 bytes from localhost.localdomain (127.0.0.1): icmp_seq=0 ttl=64
  time=0.057 ms64 bytes from localhost.localdomain (127.0.0.1): icmp_seq=1
  ttl=64 time=0.056 ms
  - --- localhost.localdomain ping statistics ---
  2 packets transmitted, 2 received, 0% packet loss, time 1008ms
  rtt min/avg/max/mdev = 0.056/0.056/0.057/0.007 ms, pipe 2
  
  This last operation should have failed (Operation not permitted).
 
 Boy, that is messed up.  The xattrs are attached to the inode, so no way
 should that happen.

Overwriting the existing file won't change the inode.
For suid, this is handled by remove_suid - notify_change with
ATTR_KILL_SUID/SGID.  No equivalent for security xattrs presently.

 Are you sure the cp succeeded?  Could you do an ls -i on /bin/ping and
 ~/ping before and after the copy to make sure?  Or just echo $? after
 the cp?

-- 
Stephen Smalley
National Security Agency

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

Re: [RFC] [PATCH 2/2] file capabilities: change fE to a bool

[Stephen Smalley]

On Wed, 2007-07-18 at 14:03 -0400, Stephen Smalley wrote:
 On Wed, 2007-07-18 at 12:53 -0500, Serge E. Hallyn wrote:
  Quoting Andrew Morgan ([EMAIL PROTECTED]):
   -BEGIN PGP SIGNED MESSAGE-
   Hash: SHA1
   
   Serge,
   
   I spent the evening getting my local build of libcap (building the
   libcap/progs/old/setcap and getcap tools) working with the new kernel
   support.
   
   It seems there is a basic insecurity bug in the xattr support insofar as
   doing the following does not delete the capabilities on a file when I
   copy over it...:
   
   [EMAIL PROTECTED] progs]$ cd ~
   [EMAIL PROTECTED] progs]$ cp /bin/ping .
   [EMAIL PROTECTED] progs]$ ./ping localhost
   [EMAIL PROTECTED] progs]$ ping: icmp open socket: Operation not permitted
   [EMAIL PROTECTED] progs]$ sudo setcap cap_net_raw=ep ping
   [EMAIL PROTECTED] progs]$ getcap ping
   Capabilities for `ping':
   = cap_net_raw+ep
   [EMAIL PROTECTED] progs]$ ./ping localhost
   64 bytes from localhost.localdomain (127.0.0.1): icmp_seq=0 ttl=64
   time=0.056 ms64 bytes from localhost.localdomain (127.0.0.1): icmp_seq=1
   ttl=64 time=0.058 ms
   Ctrl-C
   - --- localhost.localdomain ping statistics ---
   2 packets transmitted, 2 received, 0% packet loss, time 1009ms
   rtt min/avg/max/mdev = 0.056/0.057/0.058/0.001 ms, pipe 2
   [EMAIL PROTECTED] progs]$ cp /bin/ping .
   [EMAIL PROTECTED] progs]$ ./ping localhost
   PING localhost.localdomain (127.0.0.1) 56(84) bytes of data.
   64 bytes from localhost.localdomain (127.0.0.1): icmp_seq=0 ttl=64
   time=0.057 ms64 bytes from localhost.localdomain (127.0.0.1): icmp_seq=1
   ttl=64 time=0.056 ms
   - --- localhost.localdomain ping statistics ---
   2 packets transmitted, 2 received, 0% packet loss, time 1008ms
   rtt min/avg/max/mdev = 0.056/0.056/0.057/0.007 ms, pipe 2
   
   This last operation should have failed (Operation not permitted).
  
  Boy, that is messed up.  The xattrs are attached to the inode, so no way
  should that happen.
 
 Overwriting the existing file won't change the inode.
 For suid, this is handled by remove_suid - notify_change with
 ATTR_KILL_SUID/SGID.  No equivalent for security xattrs presently.

Oh, and you don't want to just remove security.selinux in that case, so
if you do that for security.cap, don't make it apply to all security.
attributes.

-- 
Stephen Smalley
National Security Agency

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

Re: [RFC][PATCH] Simplified mandatory access control kernel implementation

[Stephen Smalley]

On Sat, 2007-07-14 at 14:47 -0700, Casey Schaufler wrote:
 The patch exceeds the 40k size rule, coming in at about 100k.
 I would be happy to send the patch to anyone who has trouble
 with the project site. The patch can be found under:
 
 http:/www.schaufler-ca.com/data/smack-0710A-patch.tar

+/*
+ * In smack sockets are not security policy elements.
+ * The task associated with the socket is the policy
+ * element, so point the socket security blob at the
+ * task blob.
+ */
+static int smack_sk_alloc_security(struct sock *sk, int family, gfp_t
priority)
+{
+   sk-sk_security = current-security;
+   return 0;
+}

And if the socket outlives the task?

-- 
Stephen Smalley
National Security Agency

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

Re: [RFC][PATCH] Simplified mandatory access control kernel implementation

[Stephen Smalley]

On Mon, 2007-07-16 at 07:41 -0700, Casey Schaufler wrote:
 --- Stephen Smalley [EMAIL PROTECTED] wrote:
 
  On Sat, 2007-07-14 at 14:47 -0700, Casey Schaufler wrote:
   The patch exceeds the 40k size rule, coming in at about 100k.
   I would be happy to send the patch to anyone who has trouble
   with the project site. The patch can be found under:
   
   http:/www.schaufler-ca.com/data/smack-0710A-patch.tar
  
  +/*
  + * In smack sockets are not security policy elements.
  + * The task associated with the socket is the policy
  + * element, so point the socket security blob at the
  + * task blob.
  + */
  +static int smack_sk_alloc_security(struct sock *sk, int family, gfp_t
  priority)
  +{
  +   sk-sk_security = current-security;
  +   return 0;
  +}
  
  And if the socket outlives the task?
 
 An error condition that the code isn't handling correctly.
 
 Unless I'm missing something (happens from time to time) a
 socket that is not associated with a task is not a useful
 entity as it can not be read, written, or reassociated with
 another task. At least, I don't think that there is a way to
 do any of these things. I'm reasonably confident on the first
 two, but less certain about the reassociation bit. I'd be
 happy to learn how my understanding is imperfect should that
 be the case.

The socket can be inherited by a child task or passed via local IPC to
an unrelated task, and then used by those other tasks.  It isn't tied to
the original allocating task's lifecycle in any way, nor is it
guaranteed to only be used by that original allocating task.  It can
exist in zero, one or many tasks' file tables and still be receiving
data, and can go from completely disassociated (i.e. not present in any
tasks' file tables) to being associated.

-- 
Stephen Smalley
National Security Agency

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

Re: [RFC][PATCH] Simplified mandatory access control kernel implementation

[Stephen Smalley]

On Mon, 2007-07-16 at 08:32 -0700, Casey Schaufler wrote:
 --- Stephen Smalley [EMAIL PROTECTED] wrote:
 
  On Mon, 2007-07-16 at 07:41 -0700, Casey Schaufler wrote:
   --- Stephen Smalley [EMAIL PROTECTED] wrote:
   
On Sat, 2007-07-14 at 14:47 -0700, Casey Schaufler wrote:
 The patch exceeds the 40k size rule, coming in at about 100k.
 I would be happy to send the patch to anyone who has trouble
 with the project site. The patch can be found under:
 
 http:/www.schaufler-ca.com/data/smack-0710A-patch.tar

+/*
+ * In smack sockets are not security policy elements.
+ * The task associated with the socket is the policy
+ * element, so point the socket security blob at the
+ * task blob.
+ */
+static int smack_sk_alloc_security(struct sock *sk, int family, gfp_t
priority)
+{
+   sk-sk_security = current-security;
+   return 0;
+}

And if the socket outlives the task?
   
   An error condition that the code isn't handling correctly.
   
   Unless I'm missing something (happens from time to time) a
   socket that is not associated with a task is not a useful
   entity as it can not be read, written, or reassociated with
   another task. At least, I don't think that there is a way to
   do any of these things. I'm reasonably confident on the first
   two, but less certain about the reassociation bit. I'd be
   happy to learn how my understanding is imperfect should that
   be the case.
  
  The socket can be inherited by a child task or passed via local IPC to
  an unrelated task, and then used by those other tasks.  It isn't tied to
  the original allocating task's lifecycle in any way, nor is it
  guaranteed to only be used by that original allocating task.  It can
  exist in zero, one or many tasks' file tables and still be receiving
  data, and can go from completely disassociated (i.e. not present in any
  tasks' file tables) to being associated.
 
 Great, thank you. I will address all these scenarios, although it may
 take a bit to gather the required evidence, or change the code as
 necessary. Your claim that the life of the socket isn't tied to that
 of the allocating task is correct, however in most cases a socket
 spends its entire existence associated with the task that created it
 and goes away when that task exits. Even the simple fork case is rare
 due to the behavior of multiple processes sharing a socket. In any
 case, all possible scenarios need to be addressed.

xinetd?

Look through SELinux policy for all cases where permission is allowed to
a socket with a different domain/type (and even that doesn't tell you
the full story, as multiple tasks/processes may run in the same
domain/type).

 Just curious, what mechanism do you see for associating a socket that
 has no associations with a task?

Task 1 sends a socket S1 to a local IPC socket S2, then closes its own
descriptor to S1.  Socket S1 remains alive, can receive traffic.  Later
task 2 receives from local IPC socket S2, gets socket S1, then receives
the already queued data from S1.
   
-- 
Stephen Smalley
National Security Agency

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

Re: What kind of feature does New LSM security model need?

[Stephen Smalley]

On Wed, 2007-07-11 at 08:54 +0900, Kazuki Omo(Company) wrote:
 Dear, Sir,
 
 Sorry for my poorly English. I've just wanted to make sure the process
 how can we put other security model to mainline.
 I guess the steps are
 
 1. put patch to lsm-ml/lkm-ml and related ml.
 2. debate(I guess AppArmor is now on this stage)
 3. .I don't know
 
 Form last year, I saw some of patches were put to lsm-ml(stage1),
 and there were so many debate(stage2). But just debate... and
 not included to mainline. I want to know what do they need to
 put new-model to stage3.

In the discussion of the bsdjail security module back in 2004, Andrew
Morton indicated that acceptance of any new code into mainline requires
that it have a real user base:
http://marc.info/?l=linux-kernelm=109717928411882w=2

-- 
Stephen Smalley
National Security Agency

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

Re: What kind of feature does New LSM security model need?

[Stephen Smalley]

On Wed, 2007-07-11 at 10:30 -0700, Casey Schaufler wrote:
 --- Stephen Smalley [EMAIL PROTECTED] wrote:
 
  On Wed, 2007-07-11 at 08:54 +0900, Kazuki Omo(Company) wrote:
   Dear, Sir,
   
   Sorry for my poorly English. I've just wanted to make sure the process
   how can we put other security model to mainline.
   I guess the steps are
   
   1. put patch to lsm-ml/lkm-ml and related ml.
   2. debate(I guess AppArmor is now on this stage)
   3. .I don't know
   
   Form last year, I saw some of patches were put to lsm-ml(stage1),
   and there were so many debate(stage2). But just debate... and
   not included to mainline. I want to know what do they need to
   put new-model to stage3.
  
  In the discussion of the bsdjail security module back in 2004, Andrew
  Morton indicated that acceptance of any new code into mainline requires
  that it have a real user base:
  http://marc.info/?l=linux-kernelm=109717928411882w=2
 
 Stephen, you have got to keep up with your email. I didn't
 think you were that far behind!
 
 Andrew's more current position, from Tue, 26 Jun 2007 19:47:00:
 
 Sigh.  Please don't put us in this position again.  Get stuff upstream
  before shipping it to customers, OK?  It ain't rocket science.

Hi Casey,

As I understood it, sufficiently large user demand / vendor pull is
required (but not sufficient) for mainline inclusion.  That doesn't mean
that a major distro has to ship it before it hits mainline (which is
what Andrew complained about above); it usually seems to mean that a
major distro has identified the functionality as being important to
their users and wants it upstreamed so that they can ship it.  So
random-security-modules-of-the-day with no broad user demand / vendor
pull don't seem likely to go into mainline.  Any more than a
random-filesystem-of-the-day would.

Of course, mind you, it isn't my decision to make.

-- 
Stephen Smalley
National Security Agency

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

Re: [AppArmor 32/44] Enable LSM hooks to distinguish operations on file descriptors from operations on pathnames

[Stephen Smalley]

On Thu, 2007-06-28 at 20:15 +0200, Andreas Gruenbacher wrote:
 On Thursday 28 June 2007 18:12, James Morris wrote:
  Are you trying to cater for the case where you're holding an open fd for a 
  file which has been deleted, and thus has no pathname?
 
 Yes, see the AA_CHECK_FD flag in security/apparmor/main.c:aa_perm_dentry(). 
 We 
 want to distinguish between the following two cases:
 
  - process performs an operation on an open file descriptor,
 
  - process performs an operation on a pathname, and between the dentry
lookup and the LSM permission check, the file gets deleted.
 
 In the former case, we obviously want to continue giving the process access 
 to 
 his fd (the classical pattern: open temporary file; delete it so that it will 
 self-recycle, continue using the open file descriptor).
 
 In the latter case, The file still existed at the time of the lookup but not 
 anymore at the time of the permission check. The file obviously doesn't have 
 a filename anymore, so we cannot check permissions. If we granted access in 
 that case, processes could bypass their profile permissions in that race 
 window. We close the race by returning -ENOENT in that case, the same result 
 as if the file had already been deleted before the lookup.

So you don't actually need/use the struct file pointer; you just need a
flag indicating whether or not access was by open file descriptor or by
pathname?

And what does this mean for a process that has changed hats?  Which
might not be authorized to access the file anymore, even via an already
opened descriptor.

-- 
Stephen Smalley
National Security Agency

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

Re: [AppArmor 39/45] AppArmor: Profile loading and manipulation, pathname matching

[Stephen Smalley]

On Thu, 2007-06-21 at 23:17 +0200, Lars Marowsky-Bree wrote:
 On 2007-06-21T16:59:54, Stephen Smalley [EMAIL PROTECTED] wrote:
 
  Or can access the data under a different path to which their profile
  does give them access, whether in its final destination or in some
  temporary file processed along the way.
 
 Well, yes. That is intentional.
 
 Your point is?

It may very well be unintentional access, especially when taking into
account wildcards in profiles and user-writable directories.

  The emphasis on never modifying applications for security in AA likewise
  has an adverse impact here, as you will ultimately have to deal with
  application mediation of access to their own objects and operations not
  directly visible to the kernel (as we have already done in SELinux for
  D-BUS and others and are doing for X).  Otherwise, your protection of
  desktop applications is easily subverted.
 
 That is an interesting argument, but not what we're discussing here.
 We're arguing filesystem access mediation.

IOW, anything that AA cannot protect against is out of scope.  An easy
escape from any criticism.

  Um, no.  It might not be able to directly open files via that path, but
  showing that it can never read or write your mail is a rather different
  matter.
 
 Yes. Your use case is different than mine.

My use case is being able to protect data reliably.  Yours?

-- 
Stephen Smalley
National Security Agency

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

Re: [AppArmor 39/45] AppArmor: Profile loading and manipulation, pathname matching

[Stephen Smalley]

On Fri, 2007-06-22 at 21:34 +1000, Neil Brown wrote:
 On Friday June 22, [EMAIL PROTECTED] wrote:
   
   Yes. Your use case is different than mine.
  
  My use case is being able to protect data reliably.  Yours?
 
 Saying protect data is nearly meaningless without a threat model.
 I bet you don't try to protect data from a direct nuclear hit, or a
 court order.
 
 
 AA has a fairly clear threat model.  It involves a flaw in a
 program being used by an external agent to cause it to use
 privileges it would not normally exercise to subvert privacy or
 integrity.
 I think this model matches a lot of real threats that real sysadmins
 have real concerns about.  I believe that the design of AA addresses
 this model quite well.
  
 
 What is your threat model?  Maybe it is just different.

The threat model you describe above is a subset of what SELinux
addresses.  And our argument is that AA does not meet that model well,
because it relies upon ambiguous and unstable identifiers for subjects
and objects (a violation of the fundamental requirements for access
control) and because it provides very incomplete mediation.

From http://www.nsa.gov/selinux/info/faq.cfm:
The Security-enhanced Linux's new features are designed to enforce the
separation of information based on confidentiality and integrity
requirements. They are designed for preventing processes from reading
data and programs, tampering with data and programs, bypassing
application security mechanisms, executing untrustworthy programs, or
interfering with other processes in violation of the system security
policy. They also help to confine the potential damage that can be
caused by malicious or flawed programs. They should also be useful for
enabling a single system to be used by users with differing security
authorizations to access multiple kinds of information with differing
security requirements without compromising those security requirements.

-- 
Stephen Smalley
National Security Agency

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

Re: [AppArmor 39/45] AppArmor: Profile loading and manipulation, pathname matching

[Stephen Smalley]

On Fri, 2007-06-22 at 01:06 -0700, John Johansen wrote:
 On Thu, Jun 21, 2007 at 04:59:54PM -0400, Stephen Smalley wrote:
  On Thu, 2007-06-21 at 21:54 +0200, Lars Marowsky-Bree wrote:
   On 2007-06-21T15:42:28, James Morris [EMAIL PROTECTED] wrote:
   
  
   And now, yes, I know AA doesn't mediate IPC or networking (yet), but
   that's a missing feature, not broken by design.
  
  The incomplete mediation flows from the design, since the pathname-based
  mediation doesn't generalize to cover all objects unlike label- or
  attribute-based mediation.  And the use the natural abstraction for
  each object type approach likewise doesn't yield any general model or
  anything that you can analyze systematically for data flow.
  
 No the incomplete mediation does not flow from the design.  We have
 deliberately focused on doing the necessary modifications for pathname
 based mediation.  The IPC and network mediation are a wip.

The fact that you have to go back to the drawing board for them is that
you didn't get the abstraction right in the first place.

 We have never claimed to be using pathname-based mediation IPC or networking.
 The natural abstraction approach does generize well enough and will
 be analyzable.

I think we must have different understandings of the words generalize
and analyzable.  Look, if I want to be able to state properties about
data flow in the system for confidentiality or integrity goals (my
secret data can never leak to unauthorized entities, my critical data
can never be corrupted/tainted by unauthorized entities - directly or
indirectly), then I need to be able to have a common reference point for
my policy.  When my policy is based on different abstractions
(pathnames, IP addresses, window ids, whatever) for different objects,
then I can no longer identify how data can flow throughout the system in
a system-wide way. 

  Um, no.  It might not be able to directly open files via that path, but
  showing that it can never read or write your mail is a rather different
  matter.
  
 Actually it can be analyzed and shown.  It is ugly to do and we
 currently don't have a tool capable of doing it, but it is possible.

No, it isn't possible when using ambiguous and unstable identifiers for
the subjects and objects, nor when mediation is incomplete.

-- 
Stephen Smalley
National Security Agency

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

Re: [AppArmor 39/45] AppArmor: Profile loading and manipulation, pathname matching

[Stephen Smalley]

On Fri, 2007-06-22 at 13:37 +0200, Lars Marowsky-Bree wrote:
 On 2007-06-22T07:19:39, Stephen Smalley [EMAIL PROTECTED] wrote:
 
Or can access the data under a different path to which their profile
does give them access, whether in its final destination or in some
temporary file processed along the way.
   Well, yes. That is intentional.
   
   Your point is?
  
  It may very well be unintentional access, especially when taking into
  account wildcards in profiles and user-writable directories.
 
 Again, you're saying that AA is not confining unconfined processes.
 That's a given. If unconfined processes assist confined processes in
 breeching their confinement, yes, that is not mediated.

The issue arises even for a collection of collaborating confined
processes with different profiles, and the collaboration may be
intentional or unintentional (in the latter case, one of the confined
processes may be taking advantage of known behavior of another process
and shared access by both to some resource in order to induce a
particular behavior in that process).

And remember that confinement isn't just about limiting untrusted
processes but also about protecting trusted processes; limiting the
inputs and outputs of a trusted process can be just as important to
preventing exploitation.

 You're basically saying that anything but system-wide mandatory access
 control is pointless.

Mandatory access control as historically understood has always meant
system-wide.  As well as always being based on the security properties
of the data (so that global and persistent protection of the data is
possible).  You can't actually use the terms 'mandatory access control'
or 'confinement' for AppArmor unless you redefine them.

 If you want to go down that route, what is your reply to me saying that
 SELinux cannot mediate NFS mounts - if the server is not confined using
 SELinux as well? The argument is really, really moot and pointless. Yes,
 unconfined actions can affect confined processes. 

Sorry, do you mean the server as in the server system or as in the
server daemon?  For the former, I'd agree - we would want SELinux
policy applied on the server as well as the client to ensure that the
data is being protected consistently throughout and that the server is
not misrepresenting the security guarantees expected by the clients.
Providing an illusion of confinement on each client without any
corresponding protection on the server system would be very prone to
bypass.  For the latter, the kernel can only truly confine application
code, not in-kernel threads, although we can subject the in-kernel nfsd
to permission checking as a robustness check.  We've always noted that
SELinux does depend on the correctness of the kernel.

   That is an interesting argument, but not what we're discussing here.
   We're arguing filesystem access mediation.
  IOW, anything that AA cannot protect against is out of scope.  An easy
  escape from any criticism.
 
 I'm quite sure that this reply is not AA specific as you try to make it
 appear.

Every time we've noted an issue with AA, the answer has been that it is
out of scope.  Yet the public documentation for AA misrepresents the
situation and its comparisons with SELinux conveniently ignore its
limitations.

   Yes. Your use case is different than mine.
  My use case is being able to protect data reliably.  Yours?
 
 I want to restrict certain possibly untrusted applications and
 network-facing services from accessing certain file patterns, because as
 a user and admin, that's the mindset I'm used to. I might be interested
 in mediating other channels too, but the files are what I really care
 about. I'm inclined to trust the other processes.
 
 Your use case mandates complete system-wide mediation, because you want
 full data flow analysis. Mine doesn't.

Then yours isn't mandatory access control, nor is it confinement.  

-- 
Stephen Smalley
National Security Agency

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

Re: [AppArmor 39/45] AppArmor: Profile loading and manipulation, pathname matching

[Stephen Smalley]

On Fri, 2007-06-22 at 14:42 +0200, Lars Marowsky-Bree wrote:
 On 2007-06-22T07:53:47, Stephen Smalley [EMAIL PROTECTED] wrote:
 
   No the incomplete mediation does not flow from the design.  We have
   deliberately focused on doing the necessary modifications for pathname
   based mediation.  The IPC and network mediation are a wip.
  The fact that you have to go back to the drawing board for them is that
  you didn't get the abstraction right in the first place.
 
 That's an interesting claim, however I don't think it holds. AA was
 designed to mediate file access in a form which is intuitive to admins.
 
 It's to be expected that it doesn't directly apply to mediating other
 forms of access.
 
  I think we must have different understandings of the words generalize
  and analyzable.  Look, if I want to be able to state properties about
  data flow in the system for confidentiality or integrity goals (my
  secret data can never leak to unauthorized entities, my critical data
  can never be corrupted/tainted by unauthorized entities - directly or
  indirectly),
 
 I seem to think that this is not what AA is trying to do, so evaluating
 it in that context doesn't seem useful. It's like saying a screw driver
 isn't a hammer, so it is useless because you have a nail.

Again, in that case, please remove all uses of the terms mandatory
access control, confinement and integrity protection from AA
documentation and code.

-- 
Stephen Smalley
National Security Agency

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

Re: [AppArmor 39/45] AppArmor: Profile loading and manipulation, pathname matching

[Stephen Smalley]

On Thu, 2007-06-21 at 21:54 +0200, Lars Marowsky-Bree wrote:
 On 2007-06-21T15:42:28, James Morris [EMAIL PROTECTED] wrote:
 
   A veto is not a technical argument. All technical arguments (except for
   path name is ugly, yuk yuk!) have been addressed, have they not?
  AppArmor doesn't actually provide confinement, because it only operates on 
  filesystem objects.
  
  What you define in AppArmor policy does _not_ reflect the actual 
  confinement properties of the policy.  Applications can simply use other 
  mechanisms to access objects, and the policy is effectively meaningless.
 
 Only if they have access to another process which provides them with
 that data.

Or can access the data under a different path to which their profile
does give them access, whether in its final destination or in some
temporary file processed along the way.

 And now, yes, I know AA doesn't mediate IPC or networking (yet), but
 that's a missing feature, not broken by design.

The incomplete mediation flows from the design, since the pathname-based
mediation doesn't generalize to cover all objects unlike label- or
attribute-based mediation.  And the use the natural abstraction for
each object type approach likewise doesn't yield any general model or
anything that you can analyze systematically for data flow.

The emphasis on never modifying applications for security in AA likewise
has an adverse impact here, as you will ultimately have to deal with
application mediation of access to their own objects and operations not
directly visible to the kernel (as we have already done in SELinux for
D-BUS and others and are doing for X).  Otherwise, your protection of
desktop applications is easily subverted.

  You might define this as a non-technical issue, but the fact that AppArmor 
  simply does not and can not work is a fairly significant consideration, I 
  would imagine.
 
 If I restrict my Mozilla to not access my on-disk mail folder, it can't
 get there. (Barring bugs in programs which Mozilla is allowed to run
 unconfined, sure.)

Um, no.  It might not be able to directly open files via that path, but
showing that it can never read or write your mail is a rather different
matter.

-- 
Stephen Smalley
National Security Agency

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

Re: [RFC] TOMOYO Linux

[Stephen Smalley]

On Wed, 2007-06-13 at 16:32 -0700, william(at)elan.net wrote:
 On Thu, 14 Jun 2007, Toshiharu Harada wrote:
 
  2007/6/14, Rik van Riel [EMAIL PROTECTED]:
  Toshiharu Harada wrote:
   2007/6/14, Rik van Riel [EMAIL PROTECTED]:
   SELinux has a well designed robust and flexible functions.
   So it should be used for everywhere.  I understand it.
   As you mentioned one can analyze the system (process)
   behaviors from AVC logs. But the maintenance cost is not trivial.
  
   If logging with process context is the only purpose,
   current TOMOYO Linux can do it with no hustle at all.
  
  Yes, but so does standard SELinux.
  
  You are making me curious: what does TOMOYO do that is
  not done by regular SELinux?
  
  Logging with process name, path name and contexts is
  already done.  I must have missed some other TOMOYO
  feature in your initial email...
 
  I see SELinux can log with process name, path name and
  contexts, but contexts must be defined beforehand.
  TOMOYO Linux kernel does that with pathname, so no
  label definitions needed.
  You can confirm the process (domain) transitions any time
  and access occurred are clarified per domain basis automatically.
  Security context in TOMOYO Linux is represented and stored
  as a call chain and very intuitive.
 
  TOMOYO Linux has a mode called learning
  in addition to permissive and enforce. You can easily
  get the TOMOYO Linux policy with learning mode that
  SELinux does not have. In addition, access control mode of
  TOMOYO Linux can be managed for every difference domain.
 
 This sounds a like like feature differences compared at:
   http://www.novell.com/linux/security/apparmor/selinux_comparison.html

Amazing, per that table, AppArmor is better in every way than SELinux.
Nothing like an unbiased and fair-minded comparison.

-- 
Stephen Smalley
National Security Agency

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

Re: [AppArmor 39/45] AppArmor: Profile loading and manipulation, pathname matching

[Stephen Smalley]

On Mon, 2007-06-04 at 23:03 +0200, Andreas Gruenbacher wrote:
 On Tuesday 15 May 2007 11:20, Pavel Machek wrote:
  Hi!
  
   Pathname matching, transition table loading, profile loading and
   manipulation.
  
  So we get small interpretter of state machines, and reason we need is
  is 'apparmor is misdesigned and works with paths when it should have
  worked with handles'.
 
 I assume you mean labels instead of handles.
 
 AppArmor's design is around paths not labels, and independent of whether or 
 not you like AppArmor, this design leads to a useful security model distinct 
 from the SELinux security model (which is useful in its own ways). The 
 differences between those models cannot be argued away, neither is a subset 
 of the other, and neither is a misdesign. I would be thankful if you could 
 stop spreading this lie.

I have a hard time distinguishing AppArmor's model from its
implementation; every time we suggest that one might emulate much of
AppArmor's functionality on SELinux (as in SEEdit), someone points to a
specific characteristic of the AppArmor implementation that cannot be
emulated in this manner.  But is that implementation characteristic an
actual requirement or just how it happens to have been done to date in
AA?  And I get the impression that even if we extended SELinux in
certain ways to ease such emulation, the AA folks would never be
satisfied because the implementation would still differ.  Can we
separate the desired functionality and actual requirements from the
implementation specifics?

  If you solve the 'new file problem', aa becomes subset of selinux.
  And I'm pretty sure patch will be nicer than this. 
 
 You are quite mistaken. SELinux turns pathnames into labels when it initially 
 labels all files (when a policy is rolled out), whereas AppArmor computes 
 the label of each file when a file is opened. The two models start to 
 diverge as soon as files are renamed: in SELinux, labels stick with the 
 files. In AppArmor, labels stick with the names.

I'd argue a bit with that characterization, given that:
- in the case of SELinux, the pathname is never used as a basis for
decisions by the kernel,
- under AA, each file may have an arbitrary set of labels or
policies applied to it depending on what programs are accessing it and
what names are being used to reference it - there is no system view of
the subjects and objects and thus no way to identify the overall system
policy for a given file.
- names are far less tranquil than labels.
 
 So what you advocate for is a hybrid between the SELinux and the AppArmor 
 model, not a superset.
 
 It could be that the SELinux folks will solve the issues they are having with 
 new files using something better than restorecond in the future, perhaps even 
 an in-kernel mechanism (although I somewhat doubt it). But then again, their 
 basic model makes sense even without any live file relabeling, and so that's 
 probably not very high up on the priority list.

Live file relabeling (non-tranquility) tends to break one's ability to
show anything about preservation of confidentiality or integrity
(particularly in the absence of complete revocation support).

On the new files issue, it wouldn't be difficult or even a real
divergence from our existing model to introduce the component name (not
a full pathname, but the last component) as an additional input to the
decision for labeling new files (along with the existing use of the
creating process' label, the parent directory label, and the kind of new
file) at creation time, and that would reduce the need somewhat to
modify some applications that create files of multiple security contexts
in the same directory.  That would further help the SEEdit folks in
emulating AA on top of SELinux, but as before, I don't get the
impression that the AA folks will ever be satisfied with such an
emulation, not because of any real requirement but merely because they
are tied to their implementation specifics.

-- 
Stephen Smalley
National Security Agency

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

Re: New to LSM list. A few questions.

[Stephen Smalley]

On Thu, 2007-04-26 at 16:46 +0800, Cliffe wrote:
 G’day,
 
 I am a PhD candidate. My research project will involve implementing an 
 experimental access control model as a LSM. I have some programming 
 background (I teach intro to C and Java); however, I am new to kernel 
 programming.
 
 So I thought I would introduce myself. Is this mailing list an 
 appropriate place to ask a few questions (and later discuss the 
 resulting LSM)?
 
 I have read two papers about LSM [1, 2] which give a good foundation of 
 LSM, and the Linux Journal root plug example article, and I have started 
 reading through some LSM sources. Are any of these sources outdated? Do 
 you recommend any other must-read sources?
 
 With my [very] limited exposure to kernel-level code I am still not sure 
 how to go about writing information to disk. I know that generally it is 
 forbidden (and there is usually no need to) and there seems to be a 
 number of ways to communicate with user-land processes. But I basically 
 just want to log the arguments to a LSM hook call into a file (to poke 
 around and see exactly what is happening and what LSM operations 
 specific applications invoke). Is this possible or do I need a user-land 
 application to read/accept the data? For example, how does AppArmor (or 
 other LSMs with learning-modes) log application behaviour?

You can use the audit subsystem; kernel interface is defined by
include/linux/audit.h, and you'll see examples of usage in existing
modules (like SELinux, e.g. security/selinux/avc.c:avc_audit()).  You
definitely don't want to log every LSM hook call though, as you'll
quickly fill your disk.  The audit data will go to syslog (and end up
in /var/log/messages) if not running auditd or
to /var/log/audit/audit.log if running auditd.

 I want to recursively apply the same decision logic to enforce multiple 
 policies (concurrently on the same subjects). Would it be practical to 
 have a primary security module which loads and stacks copies of a 
 secondary module initialised using module parameters to enforce separate 
 policies?

No, see prior discussions of module stacking.  SELinux implements
several policies internally, such as RBAC, TE, and MLS, and handles the
composition directly.  Limited forms of module stacking can be used in
trivial cases, like stacking with the native Linux capabilities module;
again, you'll see an example of that in SELinux.

An alternative approach to implementing your own LSM would be to look at
modifying the SELinux security server (code under security/selinux/ss,
interface defined by security/selinux/include/security.h), as that is
the policy engine for SELinux and has a general interface for security
decisions that isn't tied to a specific policy model.  Then you would be
able to reuse the rest of SELinux, including both the rest of the kernel
code, the userland support, etc.

Or possibly you could even represent your policies as configurations
of SELinux, and avoid having to modify kernel code at all.  The
configuration language for SELinux is quite flexible.

-- 
Stephen Smalley
National Security Agency

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