Re: [PATCH] ptrace: use fsuid, fsgid, effective creds for fs access checks

[Andrew Morton]

On Mon, 9 Nov 2015 22:12:09 +0100 Jann Horn  wrote:
> 
> > Can we do
> > 
> > #define PTRACE_foo (PTRACE_MODE_READ|PTRACE_MODE_FSCREDS)
> > 
> > to avoid all that?
> 
> Hm. All combinations of the PTRACE_MODE_*CREDS flags with
> PTRACE_MODE_{READ,ATTACH} plus optionally PTRACE_MODE_NOAUDIT
> make sense, I think. So your suggestion would be to create
> four new #defines
> PTRACE_MODE_{READ,ATTACH}_{FSCREDS,REALCREDS} and then let
> callers OR in the PTRACE_MODE_NOAUDIT flag if needed?

If these flag combinations have an identifiable concept behind them then
sure, it makes sense to capture that via a well-chosen identifier.


--
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] ptrace: use fsuid, fsgid, effective creds for fs access checks

[Andrew Morton]

On Sun,  8 Nov 2015 13:08:36 +0100 Jann Horn  wrote:

> By checking the effective credentials instead of the real UID /
> permitted capabilities, ensure that the calling process actually
> intended to use its credentials.
> 
> To ensure that all ptrace checks use the correct caller
> credentials (e.g. in case out-of-tree code or newly added code
> omits the PTRACE_MODE_*CREDS flag), use two new flags and
> require one of them to be set.
> 
> The problem was that when a privileged task had temporarily dropped
> its privileges, e.g. by calling setreuid(0, user_uid), with the
> intent to perform following syscalls with the credentials of
> a user, it still passed ptrace access checks that the user would
> not be able to pass.
> 
> While an attacker should not be able to convince the privileged
> task to perform a ptrace() syscall, this is a problem because the
> ptrace access check is reused for things in procfs.
> 
> In particular, the following somewhat interesting procfs entries
> only rely on ptrace access checks:
> 
>  /proc/$pid/stat - uses the check for determining whether pointers
>  should be visible, useful for bypassing ASLR
>  /proc/$pid/maps - also useful for bypassing ASLR
>  /proc/$pid/cwd - useful for gaining access to restricted
>  directories that contain files with lax permissions, e.g. in
>  this scenario:
>  lrwxrwxrwx root root /proc/13020/cwd -> /root/foobar
>  drwx-- root root /root
>  drwxr-xr-x root root /root/foobar
>  -rw-r--r-- root root /root/foobar/secret
> 
> Therefore, on a system where a root-owned mode 6755 binary
> changes its effective credentials as described and then dumps a
> user-specified file, this could be used by an attacker to reveal
> the memory layout of root's processes or reveal the contents of
> files he is not allowed to access (through /proc/$pid/cwd).

I'll await reviewer input on this one.  Meanwhile, a bunch of
minor(ish) things...

> --- a/fs/proc/array.c
> +++ b/fs/proc/array.c
> @@ -395,7 +395,8 @@ static int do_task_stat(struct seq_file *m, struct 
> pid_namespace *ns,
>  
>   state = *get_task_state(task);
>   vsize = eip = esp = 0;
> - permitted = ptrace_may_access(task, PTRACE_MODE_READ | 
> PTRACE_MODE_NOAUDIT);
> + permitted = ptrace_may_access(task,
> + PTRACE_MODE_READ | PTRACE_MODE_NOAUDIT | PTRACE_MODE_FSCREDS);

There's lots of ugliness in the patch to do with fitting code into 80 cols. 
Can we do

#define PTRACE_foo (PTRACE_MODE_READ|PTRACE_MODE_FSCREDS)

to avoid all that?

> --- a/include/linux/ptrace.h
> +++ b/include/linux/ptrace.h
> @@ -57,7 +57,22 @@ extern void exit_ptrace(struct task_struct *tracer, struct 
> list_head *dead);
>  #define PTRACE_MODE_READ 0x01
>  #define PTRACE_MODE_ATTACH   0x02
>  #define PTRACE_MODE_NOAUDIT  0x04
> -/* Returns true on success, false on denial. */
> +#define PTRACE_MODE_FSCREDS 0x08
> +#define PTRACE_MODE_REALCREDS 0x10
> +/**
> + * ptrace_may_access - check whether the caller is permitted to access
> + * a target task.
> + * @task: target task
> + * @mode: selects type of access and caller credentials
> + *
> + * Returns true on success, false on denial.
> + *
> + * One of the flags PTRACE_MODE_FSCREDS and PTRACE_MODE_REALCREDS must
> + * be set in @mode to specify whether the access was requested through
> + * a filesystem syscall (should use effective capabilities and fsuid
> + * of the caller) or through an explicit syscall such as
> + * process_vm_writev or ptrace (and should use the real credentials).
> + */
>  extern bool ptrace_may_access(struct task_struct *task, unsigned int mode);

It is unconventional to put the kernedoc in the header - people have
been trained to look for it in the .c file.

> +++ b/kernel/ptrace.c
> @@ -219,6 +219,13 @@ static int ptrace_has_cap(struct user_namespace *ns, 
> unsigned int mode)
>  static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
>  {
>   const struct cred *cred = current_cred(), *tcred;
> + kuid_t caller_uid;
> + kgid_t caller_gid;
> +
> + if (!(mode & PTRACE_MODE_FSCREDS) != !(mode & PTRACE_MODE_REALCREDS)) {

So setting either one of these and not the other is an error.  How
come?

> + WARN(1, "denying ptrace access check without 
> PTRACE_MODE_*CREDS\n");

This warning cannot be triggered by malicious userspace, I trust?
--
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] per-process securebits

[Andrew Morton]

On Mon, 4 Feb 2008 18:17:22 +
Pavel Machek [EMAIL PROTECTED] wrote:

 On Fri 2008-02-01 20:07:01, James Morris wrote:
  On Fri, 1 Feb 2008, Andrew Morton wrote:
  
   Really?  I'd feel a lot more comfortable if yesterday's version 1 had led
   to a stream of comments from suitably-knowledgeable kernel developers 
   which
   indicated that those developers had scrutinised this code from every
   conceivable angle and had declared themselves 100% happy with it.
  
  FWIW, I've reviewed the patch in detail a couple of times, and don't see 
  any issues with it that haven't already been raised by Serge.
  
  You can add my reviewed-by.
  
  I think it does need more eyes, and some time baking in -mm.  
 
 I don't thing -mm baking helps here. People playing with -mm are not
 the ones trying to hack your box.
 

That's for sure.  Whitebox testing and really really careful review
is our best shot with this stuff.
-
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] per-process securebits

[Andrew Morton]

On Wed, 30 Jan 2008 23:02:30 -0800 Andrew G. Morgan [EMAIL PROTECTED] wrote:

 With filesystem capabilities it is now possible to do away with
 (set)uid-0 based privilege and use capabilities instead.
 
 Historically, this was first attempted with a kernel-global set of
 securebits. That implementation, however, proved problematic, and has
 slowly whithered in the kernel. Prior to this patch, there remained no
 interface for manipulating the securebits - and thus no interface for
 suppressing root as all-capable.
 
 This patch reimplements securebits, with bit locking, as a per-process
 value. (To avoid increasing the per-task footprint of this change,
 I've merged the implementation of the per-process keep_capabilities
 bit with the per-process securebits value.)
 
 A process can now drop all legacy privilege (through uid=0), for
 itself and all of its fork()'d/exec()'d children with:
 
   prctl(PR_SET_SECUREBITS, 0x2f);
 

This is the sort of patch which strikes terror into many hearts.  Please,
it cannot be hidden over on the lsm list.  I'll assume that this version is
an rfc/rfr for now and will cheerily delete it.

For the next version, please do circulate it more widely.  It will need careful
explanation and review.

I think it would be useful for this patch's changelog to give us a little
recap of what went wrong with capabilities, if that's possible (and if it's
relevant).  What was the bug which caused us to cripple capability
inheritance (some sendmail thing?) and why was that bug considered unfixable
at the time and by what means does this new code avoid the same old bug?

A bit more changelog-for-dummies would be nice, too.  This particular dummy
doesn't understand why/how fs-caps made it possible for us to start using
capabilities properly.

And last, but by no means least: s/whither/wither/ ;)

Thanks.
-
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] mm.h, security.h, key.h and preventing namespace poisoning

[Andrew Morton]

On Thu, 20 Dec 2007 15:11:40 +1100 (EST) James Morris [EMAIL PROTECTED] wrote:

   +#ifdef CONFIG_SECURITY
   +extern unsigned long mmap_min_addr;
   +#endif
   +
#include asm/page.h
#include asm/pgtable.h
#include asm/processor.h
  
  Fine by me.
 
 I'll queue it for -mm  2.6.25.

I don't think we need the ifdef.  If someone incorrectly refers to this
then they'll get a link-time error rather than a compile-time error (bad). 
But we lose an ifdef (good).

-
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] -mm (2.4.26-rc3-mm1) v2 Smack using capabilities 32 and 33

[Andrew Morton]

On Mon, 26 Nov 2007 12:38:56 -0800
Casey Schaufler [EMAIL PROTECTED] wrote:

 From: Casey Schaufler [EMAIL PROTECTED]
 
 This patch takes advantage of the increase in capability bits
 to allocate capabilities for Mandatory Access Control. Whereas
 Smack was overloading a previously allocated capability it is
 now using a pair, one for overriding access control checks and
 the other for changes to the MAC configuration.
 
 The two capabilities allocated should be obvious in their intent.
 The comments in capability.h are intended to make it clear that
 there is no intention that implementations of MAC LSM modules
 be any more constrained by the presence of these capabilities
 than an implementation of DAC LSM modules are by the analogous
 DAC capabilities.
 
 
 + !__capable(current, CAP_MAC_ADMIN))
 + !__capable(current, CAP_MAC_ADMIN))

Is there any reason for not using plain old capable() here?
-
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] 64bit capability support (legacy support fix)

[Andrew Morton]

On Wed, 21 Nov 2007 11:10:51 -0600
Serge E. Hallyn [EMAIL PROTECTED] wrote:

 Quoting Andrew Morton ([EMAIL PROTECTED]):
  On Sat, 17 Nov 2007 21:25:27 -0800 Andrew Morgan [EMAIL PROTECTED] wrote:
  
   The attached patch (171282b3553fcec43b9ab615eb7daf6c2b494a87) applies
   against 2.6.24-rc2-mm1. It addresses the problem reported by Kevin and
   Andy - ultimately, the legacy support wasn't transparent. In particular,
   userspace 32-bit capability manipulations (when run by root) that used
   to work, without this patch, fail.
  
  My venerable FC1 machine says
  
  warning: process `zsh' gets w/ old libcap
  warning: process `zsh' gets w/ old libcap
  warning: process `zsh' gets w/ old libcap
  
  should I be scared?
 
 It should be safe as of Andrew's latest patch.  (Before that patch it
 was only unsafe because root's capabilities are just set to {~0,~0} so
 they include invalid capabilities.
 
 Agreed a better error message would be good.

yup

  Would it be inappropriate
 to include the URL for new libcap versions?

I doubt it, really.  Anyone who's running anything as old as FC1 won't be
upgrading (and probably couldn't find a package to upgrade to).

Or does old libcap here refer to all the versions whcih are deployed
today?  If so then we should jsut kill the message.  ot at least make it a
once-per-boot thing.


-
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.6.24-rc3 -mm only) Smack Version 11c Simplified Mandatory Access Control Kernel

[Andrew Morton]

On Tue, 20 Nov 2007 11:04:32 -0800 (PST)
Casey Schaufler [EMAIL PROTECTED] wrote:

 
 --- Casey Schaufler [EMAIL PROTECTED] wrote:
 
  From: Casey Schaufler [EMAIL PROTECTED]
  
  ...
 
 I have verified this version against broken-out-2007-11-20-01-45
 as well. Compiles, boots, and passes tests.
 

So is it time for me to wake up and start paying attention again?
-
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.6.24-rc3 -mm only) Smack Version 11c Simplified Mandatory Access Control Kernel

[Andrew Morton]

On Mon, 19 Nov 2007 21:54:37 -0800
Casey Schaufler [EMAIL PROTECTED] wrote:

 From: Casey Schaufler [EMAIL PROTECTED]
 
 Smack is the Simplified Mandatory Access Control Kernel.
 

This patch seems bigger than the first version ;)

random-trivial-comments-just-to-show-i-read-it:

 +static int smack_inode_setsecurity(struct inode *inode, const char *name,
 +const void *value, size_t size, int flags)
 +{
 + char *sp;
 + struct inode_smack *nsp = (struct inode_smack *)inode-i_security;

Please avoid casting when assigning to and from void* - it's unneeded and
defeats typechecking - if someone goes and turns inode.i_security into a
float or a struct capiloaddatapart* then we do want this code to warn about
it.

 + struct socket_smack *ssp;
 + struct socket *sock;
 +
 + if (value == NULL || size  SMK_LABELLEN)
 + return -EACCES;
 +
 + sp = smk_import(value, size);
 + if (sp == NULL)
 + return -EINVAL;
 +
 + if (strcmp(name, XATTR_SMACK_SUFFIX) == 0) {
 + mutex_lock(nsp-smk_lock);
 + nsp-smk_inode = sp;
 + mutex_unlock(nsp-smk_lock);

Does this locking actually do anything?  The only place where it makes
sense is if there is some code elsewhere which reads nsp-smk_inode twice,
both instances under the same taking of -smk_lock, and in which it expects
both reads to return the same value.

IOW: it's fishy.

 + return 0;
 + }
 + /*
 +  * The rest of the Smack xattrs are only on sockets.
 +  */
 + if (inode-i_sb-s_magic != SOCKFS_MAGIC)
 + return -EOPNOTSUPP;
 +
 + sock = SOCKET_I(inode);
 + if (sock == NULL)
 + return -EOPNOTSUPP;
 +
 + ssp = sock-sk-sk_security;
 +
 + if (strcmp(name, XATTR_SMACK_IPIN) == 0)
 + ssp-smk_in = sp;
 + else if (strcmp(name, XATTR_SMACK_IPOUT) == 0)
 + ssp-smk_out = sp;
 + else
 + return -EOPNOTSUPP;
 + return 0;
 +}
 +

 ...

 +
 +/**
 + * smack_file_set_fowner - set the file security blob value
 + * @file: object in question
 + *
 + * Returns 0
 + * Further research may be required on this one.
 + */
 +static int smack_file_set_fowner(struct file *file)
 +{
 + file-f_security = current-security;
 + return 0;
 +}

hm.  I'd have expected to see some refcounting when a ref to an object is
copied like this.

 +/**
 + * smack_file_send_sigiotask - Smack on sigio
 + * @tsk: The target task
 + * @fown: the object the signal come from
 + * @signum: unused
 + *
 + * Allow a privileged task to get signals even if it shouldn't
 + *
 + * Returns 0 if a subject with the object's smack could
 + * write to the task, an error code otherwise.
 + */
 +static int smack_file_send_sigiotask(struct task_struct *tsk,
 +  struct fown_struct *fown, int signum)
 +{
 + struct file *file;
 + int rc;
 +
 + /*
 +  * struct fown_struct is never outside the context of a struct file
 +  */
 + file = (struct file *)((long)fown - offsetof(struct file, f_owner));

Will container_of() work here?

 + rc = smk_access(file-f_security, tsk-security, MAY_WRITE);
 + if (rc != 0  __capable(tsk, CAP_MAC_OVERRIDE))
 + return 0;
 + return rc;
 +}
 +
 +/**
 + * smack_file_receive - Smack file receive check
 + * @file: the object
 + *
 + * Returns 0 if current has access, error code otherwise
 + */
 +static int smack_file_receive(struct file *file)
 +{
 + int may = 0;
 +
 + /*
 +  * This code relies on bitmasks.
 +  */
 + if (file-f_mode  FMODE_READ)
 + may = MAY_READ;
 + if (file-f_mode  FMODE_WRITE)
 + may |= MAY_WRITE;
 +
 + return smk_curacc(file-f_security, may);
 +}
 +
 +/*
 + * Task hooks
 + */
 +
 +/**
 + * smack_task_alloc_security - allocate a task blob
 + * @tsk: the task in need of a blob
 + *
 + * Smack isn't using copies of blobs. Everyone
 + * points to an immutible list. No alloc required.
 + * No data copy required.

I guess that answers my refcounting question above.

Spello: immutable.

 + * Always returns 0
 + */
 +static int smack_task_alloc_security(struct task_struct *tsk)
 +{
 + tsk-security = current-security;
 +
 + return 0;
 +}
 +
 +/**
 + * smack_task_free_security - free a task blob
 + * @task: the task with the blob
 + *
 + * Smack isn't using copies of blobs. Everyone
 + * points to an immutible list. The blobs never go away.
 + * There is no leak here.

Thoroughly answered.

Ditto on the spello tho.

 + */
 +static void smack_task_free_security(struct task_struct *task)
 +{
 + task-security = NULL;
 +}
 +

 ...

 +static int smack_task_kill(struct task_struct *p, struct siginfo *info,
 +int sig, u32 secid)
 +{
 + /*
 +  * Special cases where signals really ought to go through
 +  * in spite of policy. Stephen Smalley suggests it may
 +  * make sense to change the caller so 

Re: [PATCH] 64bit capability support (legacy support fix)

[Andrew Morton]

On Sat, 17 Nov 2007 21:25:27 -0800 Andrew Morgan [EMAIL PROTECTED] wrote:

 The attached patch (171282b3553fcec43b9ab615eb7daf6c2b494a87) applies
 against 2.6.24-rc2-mm1. It addresses the problem reported by Kevin and
 Andy - ultimately, the legacy support wasn't transparent. In particular,
 userspace 32-bit capability manipulations (when run by root) that used
 to work, without this patch, fail.

My venerable FC1 machine says

warning: process `zsh' gets w/ old libcap
warning: process `zsh' gets w/ old libcap
warning: process `zsh' gets w/ old libcap

should I be scared?
-
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/2] Version 11 (2.6.24-rc2) Smack: Simplified Mandatory Access Control Kernel

[Andrew Morton]

On Thu, 08 Nov 2007 20:48:52 -0800 Casey Schaufler [EMAIL PROTECTED] wrote:

 Smack is the Simplified Mandatory Access Control Kernel.

This ran afoul of
http://userweb.kernel.org/~akpm/mmotm/broken-out/vfs-security-rework-inode_getsecurity-and-callers-to.patch

Until that patch gets merged we'll need to run two smack patches: one
against mainline and one which updates smack to incorporate that patch's
API changes, please.

-
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] 64 bit capabilities

[Andrew Morton]

On Wed, 07 Nov 2007 23:44:49 -0800
Andrew Morgan [EMAIL PROTECTED] wrote:

 The attached patch (e3d27bcb07485a6c8927c8e4f5483d35a99680c3) adds
 64-bit capability support to the kernel. This version of the patch is
 designed to apply against the 2.6.23-mm1 tree.
 
 FWIW libcap-2.00 supports this change (and earlier capability formats)
 
  http://www.kernel.org/pub/linux/libs/security/linux-privs/kernel-2.6/
 
 Cheers
 
 Andrew
 
 Note: to apply this patch against Linus' upstream kernel, you will first
 have to undo this other patch from Serge:
 
  From b68680e4731abbd78863063aaa0dca2a6d8cc723 Mon Sep 17 00:00:00 2001
  From: Serge E. Hallyn [EMAIL PROTECTED]
  Date: Sun, 21 Oct 2007 16:41:38 -0700
  Subject: [PATCH] capabilities: clean up file capability reading
 
 It seems that this patch has made it into 2.6.24-rc1, but it is not

Well I did that reversion, but I don't understand why.  Was that patch
wrong, or did it make this new patch impractical, or...?
-
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

[Andrew Morton]

On Tue, 16 Oct 2007 16:41:59 -0500
Serge E. Hallyn [EMAIL PROTECTED] wrote:

 To properly test this the libcap code will need to be updated first,
 which I'm looking at now...

This seems fairly significant.  I asusme that this patch won't break
presently-deployed libcap?
-
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

[Andrew Morton]

On Wed, 17 Oct 2007 21:59:20 -0500 Serge E. Hallyn [EMAIL PROTECTED] wrote:

 Quoting Andrew Morton ([EMAIL PROTECTED]):
  On Tue, 16 Oct 2007 16:41:59 -0500
  Serge E. Hallyn [EMAIL PROTECTED] wrote:
  
   To properly test this the libcap code will need to be updated first,
   which I'm looking at now...
  
  This seems fairly significant.  I asusme that this patch won't break
  presently-deployed libcap?
 
 It will break libcap.

yikes, dropped!

  And I'm not sure of the right way to address it.
 So I was hoping to hear some ideas from Andrew Morgan, Chris Wright, and
 Kaigai.
 
 We can introduce new capget64() and capset64() calls, and have
 capget() return -EINVAL or -EAGAIN if a high bit would be needed to
 accurately get the task's capabilities.
 
 Or we can require a new libcap, since capget and capset aren't
 required for most day-to-day function anyway.
 
 I guess now that I've written this out, it seems pretty clear
 that capget64() and capget64() are the way to go.  Any objections?

Sounds sane.  New syscalls are cheap and it's clear separation.
-
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

[Andrew Morton]

On Sat, 29 Sep 2007 17:20:36 -0700 Casey Schaufler [EMAIL PROTECTED] wrote:

 
 Smack is the Simplified Mandatory Access Control Kernel.


I don't know enough about security even to be dangerous.  I went back and
reviewed the August thread from your version 1 submission and the message I
take away is that the code has been well-received and looks good when
considered on its own merits, but selinux could probably be configured to
do something sufficiently similar.

I'd have trouble declaring that but to be a reason to not merge smack.
I'm more thinking let's merge it and see if people use it.

 
  Documentation/Smack.txt   |  104 +
  security/Kconfig  |1 
  security/Makefile |2 
  security/smack/Kconfig|   10 
  security/smack/Makefile   |9 
  security/smack/smack.h|  207 ++
  security/smack/smack_access.c |  345 
  security/smack/smack_lsm.c| 2685 
  security/smack/smackfs.c  | 1201 ++
  9 files changed, 4564 insertions(+)

And that wonderful diffstat really is key to being able to do this.

My major non-technical concern is that Casey Schaufler might get hit by a
bus.  If this happens, we can remove the feature in three minutes (that
diffstat again), but that may not be feasible if people have come to rely
upon the feature.

otoh, if a significant number of people are using smack, presumably someone
else would step up to maintain smack post-bus.  The risk seems acceptable
to me.

My major technical concern is the apparent paucity of documentation.


So with the information which I presently have available to me, I'm
thinking that this should go into 2.6.24.

Is smack useful without a patched ls, sshd and init.d?  What is the status
of getting those userspace patches merged?  ie: do you know who to send the
diffs to, and are they likely to take them?

What other userspace tools are likely to need patching?



Notes on the code:


- Please run scripts/checkpatch.pl across the diff.  It generates 50-100
  warnings about minor stylistic matters, and those warnings all look legit
  to me.  (extern decls in C are my fave peeve).

- Smack.txt and the website seem a bit skimpy.  Is there enough
  documentation out there for someone to usefully (and, more importantly,
  safely) start using smack?

- In his review of version 1, Andi suggested that your ruleset traversal
  be protected by RCU.  But it seems that this wasn't done.  Were the races
  which he identified fixed by other means?  If so, what were they?

- hm, netlabels.  Who might be a suitable person to review that code? 
  Seems that Paul Moore is the man.  Maybe he'd be interested in taking a
  look over it (please?)

- some parts of the code use the smack_foo naming convention and other
  parts use smk_foo.  Seems odd.  Deliberate?

- According to git-log, you haven't merged any kernel code at all in at
  least 5.5 years.  This patch makes it look like you've been doing kernel
  full time for a decade.  That thing in my hand is a hat.


-
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 try #3] security: Convert LSM into a static interface

[Andrew Morton]

On Sat, 14 Jul 2007 12:37:01 -0400 (EDT) James Morris [EMAIL PROTECTED] wrote:

 Convert LSM into a static interface

allmodconfig broke

security/built-in.o: In function `rootplug_bprm_check_security':
security/root_plug.c:64: undefined reference to `usb_find_device'
security/root_plug.c:70: undefined reference to `usb_put_dev'

-
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 try #3] security: Convert LSM into a static interface

[Andrew Morton]

On Tue, 24 Jul 2007 01:53:58 -0700 Greg KH [EMAIL PROTECTED] wrote:

 On Tue, Jul 24, 2007 at 01:02:24AM -0700, Andrew Morton wrote:
  On Sat, 14 Jul 2007 12:37:01 -0400 (EDT) James Morris [EMAIL PROTECTED] 
  wrote:
  
   Convert LSM into a static interface
  
  allmodconfig broke
  
  security/built-in.o: In function `rootplug_bprm_check_security':
  security/root_plug.c:64: undefined reference to `usb_find_device'
  security/root_plug.c:70: undefined reference to `usb_put_dev'
 
 That's wierd, who would have disabled the exports of those functions or
 removed the #include linux/usb.h from this file?
 

root_plug is linked into vmlinux and usb is modular.  I did this:

--- a/security/Kconfig~security-convert-lsm-into-a-static-interface-fix-2
+++ a/security/Kconfig
@@ -82,7 +82,7 @@ config SECURITY_CAPABILITIES
 
 config SECURITY_ROOTPLUG
bool Root Plug Support
-   depends on USB  SECURITY
+   depends on USB=y  SECURITY
help
  This is a sample LSM module that should only be used as such.
  It prevents any programs running with egid == 0 if a specific
_


I suppose we could do SECURITY_ROOTPLUG=m if USB=m, but I thought the
whole point was to disallow modular LSM clients?
-
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 00/44] AppArmor security module overview

[Andrew Morton]

On Tue, 26 Jun 2007 16:07:56 -0700
[EMAIL PROTECTED] wrote:

 This post contains patches to include the AppArmor application security
 framework, with request for inclusion into -mm for wider testing.

Patches 24 and 31 didn't come through.

Rolled-up diffstat (excluding 2431):

 fs/attr.c|7 
 fs/dcache.c  |  181 ++-
 fs/ecryptfs/inode.c  |   41 
 fs/exec.c|3 
 fs/fat/file.c|2 
 fs/hpfs/namei.c  |2 
 fs/namei.c   |  115 +-
 fs/nfsd/nfs4recover.c|7 
 fs/nfsd/nfs4xdr.c|2 
 fs/nfsd/vfs.c|   89 +
 fs/ntfs/file.c   |2 
 fs/open.c|   50 
 fs/reiserfs/file.c   |2 
 fs/reiserfs/xattr.c  |8 
 fs/splice.c  |4 
 fs/stat.c|2 
 fs/sysfs/file.c  |2 
 fs/utimes.c  |   11 
 fs/xattr.c   |   75 -
 fs/xfs/linux-2.6/xfs_lrw.c   |2 
 include/linux/audit.h|   12 
 include/linux/fs.h   |   27 
 include/linux/nfsd/nfsd.h|3 
 include/linux/security.h |  182 ++-
 include/linux/sysctl.h   |2 
 include/linux/xattr.h|   11 
 ipc/mqueue.c |2 
 kernel/audit.c   |6 
 kernel/sysctl.c  |   27 
 mm/filemap.c |   12 
 mm/filemap_xip.c |2 
 mm/shmem.c   |2 
 mm/tiny-shmem.c  |2 
 net/unix/af_unix.c   |2 
 security/Kconfig |1 
 security/Makefile|1 
 security/apparmor/Kconfig|   10 
 security/apparmor/Makefile   |   13 
 security/apparmor/apparmor.h |  265 +
 security/apparmor/apparmorfs.c   |  252 +
 security/apparmor/inline.h   |  211 
 security/apparmor/list.c |   94 +
 security/apparmor/locking.txt|   68 +
 security/apparmor/lsm.c  |  817 
 security/apparmor/main.c | 1255 +
 security/apparmor/match.c|  248 
 security/apparmor/match.h|   83 +
 security/apparmor/module_interface.c |  589 +++
 security/apparmor/procattr.c |  155 +++
 security/commoncap.c |7 
 security/dummy.c |   43 
 security/selinux/hooks.c |   94 -
 52 files changed, 4701 insertions(+), 404 deletions(-)

which seems OK.


so...  where do we stand with this?  Fundamental, irreconcilable
differences over the use of pathname-based security?

Are there any other sticking points?


-
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 00/44] AppArmor security module overview

[Andrew Morton]

On Tue, 26 Jun 2007 19:24:03 -0700 John Johansen [EMAIL PROTECTED] wrote:

  
  so...  where do we stand with this?  Fundamental, irreconcilable
  differences over the use of pathname-based security?
  
 There certainly seems to be some differences of opinion over the use
 of pathname-based-security.

I was refreshed to have not been cc'ed on a lkml thread for once.  I guess
it couldn't last.

Do you agree with the irreconcilable part?  I think I do.

I suspect that we're at the stage of having to decide between

a) set aside the technical issues and grudgingly merge this stuff as a
   service to Suse and to their users (both of which entities are very
   important to us) and leave it all as an object lesson in
   how-not-to-develop-kernel-features.

   Minimisation of the impact on the rest of the kernel is of course
   very important here.

versus

b) leave it out and require that Suse wear the permanent cost and
   quality impact of maintaining it out-of-tree.  It will still be an
   object lesson in how-not-to-develop-kernel-features.

Sigh.  Please don't put us in this position again.  Get stuff upstream
before shipping it to customers, OK?  It ain't rocket science.

  Are there any other sticking points?
  
  
 The conditional passing of the vfsmnt mount in the vfs, as done in this
 patch series, has received a NAK.  This problem results from NFS passing
 a NULL nameidata into the vfs.  We have a second patch series that we
 have posted for discussion that addresses this by splitting the nameidata
 struct.
 Message-Id: [EMAIL PROTECTED]
 Subject: [RFD 0/4] AppArmor - Don't pass NULL nameidata to
 vfs_create/lookup/permission IOPs
 
 other issues that have been raised are:
 - AppArmor does not currently mediate IPC and network communications.
   Mediation of these is a wip
 - the use of d_path to generate the pathname used for mediation when a
   file is opened.
   - Generating the pathname using a reverse walk is considered ugly
   - A buffer is alloced to store the generated path name.
   - The  buffer size has a configurable upper limit which will cause
 opens to fail if the pathname length exceeds this limit.  This
 is a fail closed behavior.
   - there have been some concerns expressed about the performance
 of this approach
   We are evaluating our options on how best to address this issue.

OK, useful summary, thanks.  I'd encourage you to proceed apace.
-
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