Re: [PATCH] selinux: Add __GFP_NOWARN to allocation at str_read()

2018-09-12 Thread Paul Moore 
On Fri, Sep 7, 2018 at 12:43 PM Tetsuo Handa
 wrote:
> syzbot is hitting warning at str_read() [1] because len parameter can
> become larger than KMALLOC_MAX_SIZE. We don't need to emit warning for
> this case.
>
> [1] 
> https://syzkaller.appspot.com/bug?id=7f2f5aad79ea8663c296a2eedb81978401a908f0
>
> Signed-off-by: Tetsuo Handa 
> Reported-by: syzbot 
> ---
>  security/selinux/ss/policydb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> index e9394e7..f4eadd3 100644
> --- a/security/selinux/ss/policydb.c
> +++ b/security/selinux/ss/policydb.c
> @@ -1101,7 +1101,7 @@ static int str_read(char **strp, gfp_t flags, void *fp, 
> u32 len)
> if ((len == 0) || (len == (u32)-1))
> return -EINVAL;
>
> -   str = kmalloc(len + 1, flags);
> +   str = kmalloc(len + 1, flags | __GFP_NOWARN);
> if (!str)
> return -ENOMEM;

Thanks for the patch.

My eyes are starting to glaze over a bit chasing down all of the
different kmalloc() code paths trying to ensure that this always does
the right thing based on size of the allocation and the different slab
allocators ... are we sure that this will always return NULL when (len
+ 1) is greater than KMALLOC_MAX_SIZE for the different slab allocator
configurations?

-- 
paul moore
www.paul-moore.com
___
Selinux mailing list
Selinux@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.

Re: [PATCH 3/6] selinux: convert to kvmalloc

2018-09-12 Thread Paul Moore 
On Fri, Sep 7, 2018 at 1:50 PM Kent Overstreet
 wrote:
> On Sat, Sep 08, 2018 at 02:08:03AM +0900, Tetsuo Handa wrote:
> > On 2018/09/08 1:56, Kent Overstreet wrote:
> > > @@ -329,8 +328,7 @@ int avtab_alloc(struct avtab *h, u32 nrules)
> > > nslot = MAX_AVTAB_HASH_BUCKETS;
> > > mask = nslot - 1;
> > >
> > > -   h->htable = flex_array_alloc(sizeof(struct avtab_node *), nslot,
> > > -GFP_KERNEL | __GFP_ZERO);
> > > +   h->htable = kvmalloc_array(nslot, sizeof(void *), GFP_KERNEL);
> > > if (!h->htable)
> > > return -ENOMEM;
> > >
> >
> > kvmalloc_array() does not imply __GFP_ZERO.
>
> Thanks, fixed

When you resubmit this patch, please make sure you submit it to the
SELinux list (added to the To line above) so that it can be properly
reviewed and merged.

Thanks.

-- 
paul moore
www.paul-moore.com
___
Selinux mailing list
Selinux@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.

Re: MLS dominance check behavior on el7

2018-09-12 Thread Ted Toth 
On Wed, Sep 12, 2018 at 9:36 AM Dominick Grift 
wrote:

> On Wed, Sep 12, 2018 at 09:57:20AM -0400, Stephen Smalley wrote:
> > On 09/12/2018 09:26 AM, Ted Toth wrote:
> > >
> > >
> > > On Wed, Sep 12, 2018 at 8:04 AM Stephen Smalley  > > > wrote:
> > >
> > > On 09/11/2018 04:59 PM, Ted Toth wrote:
> > >  > That's awesome and now it's got me thinking about other
> > >  > classes/permissions that we could implement. Can cil macros can
> be
> > >  > referenced in .te/.if files?
> > >
> > > Not sure I understand your question.  You can't directly embed cil
> > > statements in .te/.if files.  However, if you define a
> class/permission
> > > in a .cil module, you can certainly specify a require on it and
> use it
> > > from a conventional .te/.if module, ala:
> > > $ cat > usemcstrans.te < > > policy_module(usemcstrans, 1.0)
> > >
> > > require {
> > >  class mcstrans { color_use };
> > >  attribute domain;
> > > }
> > >
> > > allow domain self:mcstrans color_use;
> > > EOF
> > >
> > > $ make -f /usr/share/selinux/devel/Makefile usemcstrans.pp
> > > $ sudo semodule -i usemcstrans.pp
> > >
> > >
> > > If the cil contained:
> > >
> > > (macro use_color (type caller) (allow caller self mcstrans
> (color_use)))
> > >
> > > then in x.te can I use the macro:
> > >
> > > type x_t;
> > > use_color(x_t)
> >
> > Sorry, no.  The macros used in .te/.if files are just m4 definitions
> handled
> > at the preprocessing stage, not a feature of the module language.  The
> CIL
> > macros are directly supported by the CIL compiler, but they won't be
> visible
> > to the module compiler.  Also, you are missing several parentheses above
> > (I'm not fond of the lisp-like syntax myself).  In a CIL module, I think
> the
> > correct syntax would be:
> >
> > (macro use_color ((type caller)) (allow caller self (mcstrans
> (color_use
> >
> > (call use_color(x_t))
> >
> > Or you could define a m4 macro in an .if file and use that in a .te file.
> > Or both.
> >
>
> Ideally you would have all of your policy written in CIL or in a
> high-level language that was designed to leverage CIL.
>

Unfortunately I/we don't live in an ideal world :( but thanks for the
pointers.


>
> My DSSP2 policy is a CIL-only policy. In there I also leverage unordered
> classes, Meaning that for example if you remove or disable the mcstrans
> module then you automatically also remove or disable  the access vectors
> that mcstrans manages.
>
> minimal:
>
> https://github.com/DefenSec/dssp2-minimal
>
> standard (my personal policy based on top of minimal):
>
> https://github.com/DefenSec/dssp2-standard/commits/master
>
> DSSP2 does not support enforcement of confidentiality though
>
> > ___
> > Selinux mailing list
> > Selinux@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.
>
> --
> Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
> https://sks-keyservers.net/pks/lookup?op=get=0x3B6C5F1D2C7B6B02
> Dominick Grift
>
___
Selinux mailing list
Selinux@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.

Re: MLS dominance check behavior on el7

2018-09-12 Thread Dominick Grift 
On Wed, Sep 12, 2018 at 09:57:20AM -0400, Stephen Smalley wrote:
> On 09/12/2018 09:26 AM, Ted Toth wrote:
> > 
> > 
> > On Wed, Sep 12, 2018 at 8:04 AM Stephen Smalley  > > wrote:
> > 
> > On 09/11/2018 04:59 PM, Ted Toth wrote:
> >  > That's awesome and now it's got me thinking about other
> >  > classes/permissions that we could implement. Can cil macros can be
> >  > referenced in .te/.if files?
> > 
> > Not sure I understand your question.  You can't directly embed cil
> > statements in .te/.if files.  However, if you define a class/permission
> > in a .cil module, you can certainly specify a require on it and use it
> > from a conventional .te/.if module, ala:
> > $ cat > usemcstrans.te < > policy_module(usemcstrans, 1.0)
> > 
> > require {
> >          class mcstrans { color_use };
> >          attribute domain;
> > }
> > 
> > allow domain self:mcstrans color_use;
> > EOF
> > 
> > $ make -f /usr/share/selinux/devel/Makefile usemcstrans.pp
> > $ sudo semodule -i usemcstrans.pp
> > 
> > 
> > If the cil contained:
> > 
> > (macro use_color (type caller) (allow caller self mcstrans (color_use)))
> > 
> > then in x.te can I use the macro:
> > 
> > type x_t;
> > use_color(x_t)
> 
> Sorry, no.  The macros used in .te/.if files are just m4 definitions handled
> at the preprocessing stage, not a feature of the module language.  The CIL
> macros are directly supported by the CIL compiler, but they won't be visible
> to the module compiler.  Also, you are missing several parentheses above
> (I'm not fond of the lisp-like syntax myself).  In a CIL module, I think the
> correct syntax would be:
> 
> (macro use_color ((type caller)) (allow caller self (mcstrans (color_use
> 
> (call use_color(x_t))
> 
> Or you could define a m4 macro in an .if file and use that in a .te file.
> Or both.
> 

Ideally you would have all of your policy written in CIL or in a high-level 
language that was designed to leverage CIL.

My DSSP2 policy is a CIL-only policy. In there I also leverage unordered 
classes, Meaning that for example if you remove or disable the mcstrans module 
then you automatically also remove or disable  the access vectors that mcstrans 
manages.

minimal:

https://github.com/DefenSec/dssp2-minimal

standard (my personal policy based on top of minimal):

https://github.com/DefenSec/dssp2-standard/commits/master

DSSP2 does not support enforcement of confidentiality though

> ___
> Selinux mailing list
> Selinux@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.

-- 
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get=0x3B6C5F1D2C7B6B02
Dominick Grift


signature.asc
Description: PGP signature
___
Selinux mailing list
Selinux@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.

Re: MLS dominance check behavior on el7

2018-09-12 Thread Ted Toth 
On Wed, Sep 12, 2018 at 8:04 AM Stephen Smalley  wrote:

> On 09/11/2018 04:59 PM, Ted Toth wrote:
> > That's awesome and now it's got me thinking about other
> > classes/permissions that we could implement. Can cil macros can be
> > referenced in .te/.if files?
>
> Not sure I understand your question.  You can't directly embed cil
> statements in .te/.if files.  However, if you define a class/permission
> in a .cil module, you can certainly specify a require on it and use it
> from a conventional .te/.if module, ala:
> $ cat > usemcstrans.te < policy_module(usemcstrans, 1.0)
>
> require {
> class mcstrans { color_use };
> attribute domain;
> }
>
> allow domain self:mcstrans color_use;
> EOF
>
> $ make -f /usr/share/selinux/devel/Makefile usemcstrans.pp
> $ sudo semodule -i usemcstrans.pp
>
>
If the cil contained:

(macro use_color (type caller) (allow caller self mcstrans (color_use)))

then in x.te can I use the macro:

type x_t;
use_color(x_t)


> On Tue, Sep 11, 2018 at 2:27 PM Stephen Smalley  > > wrote:
> >
> > On 09/11/2018 02:49 PM, Ted Toth wrote:
> >  > Yes I too noticed the translate permission but couldn't find any
> > info
> >  > related to it intended purpose. Regarding CIL unfortunately I
> > have zero
> >  > experience with it but I've installed the compiler and started
> > reading
> >  > through https://github.com/SELinuxProject/cil/wiki (any other
> > pointers
> >  > to useful info would be appreciated). I have written lots of
> policy
> >  > would it be possible to add a class/permissions/mlsconstraints in
> an
> >  > old-fashion policy module?
> >
> > The older binary modules didn't support those kinds of statements
> > outside of the base module.  Try this:
> > $ cat > mcstrans.cil < > ; define a mcstrans class with one permission color_use
> > (class mcstrans (color_use))
> > ; allow all domains mcstrans color_use permission to themselves
> > (allow domain self (mcstrans (color_use)))
> > ; only allow mcstrans color_use permission when h1 dominates h2
> > (mlsconstrain (mcstrans (color_use)) (dom h1 h2))
> > ; append the new mcstrans class to the end after all others
> > (classorder (unordered mcstrans))
> > EOF
> >
> > $ sudo semodule -i mcstrans.cil
> >
> > Then try performing permission checks with "mcstrans" as your class
> and
> > "color_use" as your permission, between a domain and itself, with
> > different levels.
> >
> >  >
> >  > On Tue, Sep 11, 2018 at 1:27 PM Stephen Smalley
> > mailto:s...@tycho.nsa.gov>
> >  > >> wrote:
> >  >
> >  > On 09/11/2018 10:41 AM, Stephen Smalley wrote:
> >  >  > On 09/10/2018 06:30 PM, Ted Toth wrote:
> >  >  >> mcstrans mcscolor.c also uses the same logic I'd been
> > using to
> >  > check
> >  >  >> dominance so this too will no longer function as expected
> on
> >  > el7. Do
> >  >  >> you any suggestions for doing a 'generic' (one not tied
> to a
> >  > specific
> >  >  >> resource class) dominance check in lieu of context
> contains?
> >  >  >
> >  >  > You should probably define your own permission with its own
> >  > constraint
> >  >  > to avoid depending on the base policy's particular
> constraint
> >  >  > definitions.  Certainly for your own code.  For mcstrans,
> > mcscolor
> >  >  > probably ought to be switched to using at least a separate
> >  > permission in
> >  >  > the context class if not its own class to avoid
> > overloading the
> >  > meaning
> >  >  > with pam_selinux's usage (or vice versa, but likely harder
> > to change
> >  >  > pam_selinux at this point).
> >  >  >
> >  >  > It is possible to define an entirely new class, its
> > permissions,
> >  > and its
> >  >  > mls constraints via a CIL module IIUC, without needing to
> > change the
> >  >  > base policy.
> >  >  >
> >  >  > I don't think you can add a permission to an existing
> > class via a
> >  > CIL
> >  >  > module currently, unfortunately, so you can't just extend
> the
> >  > context
> >  >  > class without modifying the base policy.  So it may be
> > easier to
> >  > define
> >  >  > an entirely new class.
> >  >  >
> >  >  > The class and permission ought to be specific to the
> > usage.  For
> >  >  > example, mcstrans could have its own class (mcstrans) with
> > its own
> >  >  > permissions (e.g. color_match or color_use or ...) that
> > abstract
> >  > away
> >  >  > the logical check being performed.  Dominance checks
> > performed for
> >  >  > different

Re: MLS dominance check behavior on el7

2018-09-12 Thread Stephen Smalley 

On 09/11/2018 04:59 PM, Ted Toth wrote:
That's awesome and now it's got me thinking about other 
classes/permissions that we could implement. Can cil macros can be 
referenced in .te/.if files?


Not sure I understand your question.  You can't directly embed cil 
statements in .te/.if files.  However, if you define a class/permission 
in a .cil module, you can certainly specify a require on it and use it 
from a conventional .te/.if module, ala:

$ cat > usemcstrans.te > wrote:


On 09/11/2018 02:49 PM, Ted Toth wrote:
 > Yes I too noticed the translate permission but couldn't find any
info
 > related to it intended purpose. Regarding CIL unfortunately I
have zero
 > experience with it but I've installed the compiler and started
reading
 > through https://github.com/SELinuxProject/cil/wiki (any other
pointers
 > to useful info would be appreciated). I have written lots of policy
 > would it be possible to add a class/permissions/mlsconstraints in an
 > old-fashion policy module?

The older binary modules didn't support those kinds of statements
outside of the base module.  Try this:
$ cat > mcstrans.cil <
 > On Tue, Sep 11, 2018 at 1:27 PM Stephen Smalley
mailto:s...@tycho.nsa.gov>
 > >> wrote:
 >
 >     On 09/11/2018 10:41 AM, Stephen Smalley wrote:
 >      > On 09/10/2018 06:30 PM, Ted Toth wrote:
 >      >> mcstrans mcscolor.c also uses the same logic I'd been
using to
 >     check
 >      >> dominance so this too will no longer function as expected on
 >     el7. Do
 >      >> you any suggestions for doing a 'generic' (one not tied to a
 >     specific
 >      >> resource class) dominance check in lieu of context contains?
 >      >
 >      > You should probably define your own permission with its own
 >     constraint
 >      > to avoid depending on the base policy's particular constraint
 >      > definitions.  Certainly for your own code.  For mcstrans,
mcscolor
 >      > probably ought to be switched to using at least a separate
 >     permission in
 >      > the context class if not its own class to avoid
overloading the
 >     meaning
 >      > with pam_selinux's usage (or vice versa, but likely harder
to change
 >      > pam_selinux at this point).
 >      >
 >      > It is possible to define an entirely new class, its
permissions,
 >     and its
 >      > mls constraints via a CIL module IIUC, without needing to
change the
 >      > base policy.
 >      >
 >      > I don't think you can add a permission to an existing
class via a
 >     CIL
 >      > module currently, unfortunately, so you can't just extend the
 >     context
 >      > class without modifying the base policy.  So it may be
easier to
 >     define
 >      > an entirely new class.
 >      >
 >      > The class and permission ought to be specific to the
usage.  For
 >      > example, mcstrans could have its own class (mcstrans) with
its own
 >      > permissions (e.g. color_match or color_use or ...) that
abstract
 >     away
 >      > the logical check being performed.  Dominance checks
performed for
 >      > different reasons ought to use different permissions so
that one can
 >      > distinguish what TE pairs are allowed them.
 >      >
 >      > Your code could likewise define and use its own class and
permission.
 >      >
 >      > Does that make sense?
 >
 >     BTW, I noticed there is another permission ("translate")
defined in the
 >     context class and its constraint is ((h1 dom h2) or (t1 ==
 >     mlstranslate)).  I would have guessed that it was intended as a
 >     front-end service check over what processes could request context
 >     translations from mcstrans or what contexts they could
translate, but I
 >     don't see it being used in mcstrans anywhere.  Is this a
legacy thing
 >     from early setransd/mcstransd days?  There is a TODO comment in
 >     mcstrans
 >     process_request() that suggests there was an intent to perform a
 >     dominance check between the requester context and the specified
 >     context,
 >     but that's not implemented.  Appears to be allowed in current
policy
 >     for
 >     all domains to the setrans_t domain itself.
 >
 >      >
 >      >>
 >      >> Ted
 >      >>
 >      >> On Mon, Sep 10, 2018 at 1:19 PM Ted Toth
mailto:txt...@gmail.com>
 >     >
 >      >>

Re: [PATCH v2 00/10] LSM: Module stacking in support of S.A.R.A and Landlock

2018-09-12 Thread James Morris 
On Tue, 11 Sep 2018, Casey Schaufler wrote:

> LSM: Module stacking in support of S.A.R.A and Landlock

Please help prevent RSI and shorten this to SARA.


-- 
James Morris


___
Selinux mailing list
Selinux@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.