mbox series

[00/58] LSM: Module stacking for AppArmor

Message ID 20190602165101.25079-1-casey@schaufler-ca.com (mailing list archive)
Headers show
Series LSM: Module stacking for AppArmor | expand

Message

Casey Schaufler June 2, 2019, 4:50 p.m. UTC
This patchset provides the changes required for
the AppArmor security module to stack safely with any other.

A new process attribute identifies which security module
information should be reported by SO_PEERSEC and the
/proc/.../attr/current interface. This is provided by
/proc/.../attr/display. Writing the name of the security
module desired to this interface will set which LSM hooks
will be called for this information. The first security
module providing the hooks will be used by default.

The use of integer based security tokens (secids) is
generally (but not completely) replaced by a structure
lsm_export. The lsm_export structure can contain information
for each of the security modules that export information
outside the LSM layer.

The LSM interfaces that provide "secctx" text strings
have been changed to use a structure "lsm_context"
instead of a pointer/length pair. In some cases the
interfaces used a "char *" pointer and in others a
"void *". This was necessary to ensure that the correct
release mechanism for the text is used. It also makes
many of the interfaces cleaner.

https://github.com/cschaufler/lsm-stacking.git#stack-5.2-v1-apparmor

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 drivers/android/binder.c                |  25 ++-
 fs/kernfs/dir.c                         |   6 +-
 fs/kernfs/inode.c                       |  31 ++-
 fs/kernfs/kernfs-internal.h             |   3 +-
 fs/nfs/inode.c                          |  13 +-
 fs/nfs/internal.h                       |   8 +-
 fs/nfs/nfs4proc.c                       |  17 +-
 fs/nfs/nfs4xdr.c                        |  16 +-
 fs/nfsd/nfs4proc.c                      |   8 +-
 fs/nfsd/nfs4xdr.c                       |  14 +-
 fs/nfsd/vfs.c                           |   7 +-
 fs/proc/base.c                          |   1 +
 include/linux/cred.h                    |   3 +-
 include/linux/lsm_hooks.h               |  91 +++++----
 include/linux/nfs4.h                    |   8 +-
 include/linux/security.h                | 133 +++++++++----
 include/net/af_unix.h                   |   2 +-
 include/net/netlabel.h                  |  10 +-
 include/net/scm.h                       |  14 +-
 kernel/audit.c                          |  43 ++--
 kernel/audit.h                          |   9 +-
 kernel/auditfilter.c                    |   6 +-
 kernel/auditsc.c                        |  77 ++++----
 kernel/cred.c                           |  15 +-
 net/ipv4/cipso_ipv4.c                   |  13 +-
 net/ipv4/ip_sockglue.c                  |  12 +-
 net/netfilter/nf_conntrack_netlink.c    |  29 ++-
 net/netfilter/nf_conntrack_standalone.c |  16 +-
 net/netfilter/nfnetlink_queue.c         |  38 ++--
 net/netfilter/nft_meta.c                |  13 +-
 net/netfilter/xt_SECMARK.c              |  14 +-
 net/netlabel/netlabel_kapi.c            |   5 +-
 net/netlabel/netlabel_unlabeled.c       | 101 +++++-----
 net/netlabel/netlabel_unlabeled.h       |   2 +-
 net/netlabel/netlabel_user.c            |  13 +-
 net/netlabel/netlabel_user.h            |   2 +-
 net/unix/af_unix.c                      |   6 +-
 security/apparmor/audit.c               |   4 +-
 security/apparmor/include/audit.h       |   2 +-
 security/apparmor/include/net.h         |   6 +-
 security/apparmor/include/secid.h       |   9 +-
 security/apparmor/lsm.c                 |  64 +++---
 security/apparmor/secid.c               |  42 ++--
 security/integrity/ima/ima.h            |  14 +-
 security/integrity/ima/ima_api.c        |   9 +-
 security/integrity/ima/ima_appraise.c   |   6 +-
 security/integrity/ima/ima_main.c       |  34 ++--
 security/integrity/ima/ima_policy.c     |  19 +-
 security/security.c                     | 338 +++++++++++++++++++++++++++-----
 security/selinux/hooks.c                | 259 ++++++++++++------------
 security/selinux/include/audit.h        |   5 +-
 security/selinux/include/objsec.h       |  42 +++-
 security/selinux/netlabel.c             |  25 +--
 security/selinux/ss/services.c          |  18 +-
 security/smack/smack.h                  |  18 ++
 security/smack/smack_lsm.c              | 238 +++++++++++-----------
 security/smack/smack_netfilter.c        |   8 +-
 security/smack/smackfs.c                |  12 +-
 58 files changed, 1217 insertions(+), 779 deletions(-)

Comments

Stephen Smalley June 4, 2019, 12:29 p.m. UTC | #1
On 6/2/19 12:50 PM, Casey Schaufler wrote:
> This patchset provides the changes required for
> the AppArmor security module to stack safely with any other.

Please explain the motivation - why do we want to allow AppArmor to 
stack with other modules, who would use it, how would it be used, what 
does it provide that isn't already possible in the absence of it. Also, 
Ubuntu fully upstreamed all of their changes to AppArmor, would this 
still suffice to enable stacking of AppArmor or do they rely on hooks 
that are not handled here?

Please explain the cost of the change - what do we pay in terms of 
memory, runtime, or other overheads in order to support this change?

> 
> A new process attribute identifies which security module
> information should be reported by SO_PEERSEC and the
> /proc/.../attr/current interface. This is provided by
> /proc/.../attr/display. Writing the name of the security
> module desired to this interface will set which LSM hooks
> will be called for this information. The first security
> module providing the hooks will be used by default.

Doesn't this effectively undo making the hooks read-only after init, at 
least for the subset involved?  What are the security implications thereof?

> The use of integer based security tokens (secids) is
> generally (but not completely) replaced by a structure
> lsm_export. The lsm_export structure can contain information
> for each of the security modules that export information
> outside the LSM layer.
> 
> The LSM interfaces that provide "secctx" text strings
> have been changed to use a structure "lsm_context"
> instead of a pointer/length pair. In some cases the
> interfaces used a "char *" pointer and in others a
> "void *". This was necessary to ensure that the correct
> release mechanism for the text is used. It also makes
> many of the interfaces cleaner.
> 
> https://github.com/cschaufler/lsm-stacking.git#stack-5.2-v1-apparmor
> 
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
>   drivers/android/binder.c                |  25 ++-
>   fs/kernfs/dir.c                         |   6 +-
>   fs/kernfs/inode.c                       |  31 ++-
>   fs/kernfs/kernfs-internal.h             |   3 +-
>   fs/nfs/inode.c                          |  13 +-
>   fs/nfs/internal.h                       |   8 +-
>   fs/nfs/nfs4proc.c                       |  17 +-
>   fs/nfs/nfs4xdr.c                        |  16 +-
>   fs/nfsd/nfs4proc.c                      |   8 +-
>   fs/nfsd/nfs4xdr.c                       |  14 +-
>   fs/nfsd/vfs.c                           |   7 +-
>   fs/proc/base.c                          |   1 +
>   include/linux/cred.h                    |   3 +-
>   include/linux/lsm_hooks.h               |  91 +++++----
>   include/linux/nfs4.h                    |   8 +-
>   include/linux/security.h                | 133 +++++++++----
>   include/net/af_unix.h                   |   2 +-
>   include/net/netlabel.h                  |  10 +-
>   include/net/scm.h                       |  14 +-
>   kernel/audit.c                          |  43 ++--
>   kernel/audit.h                          |   9 +-
>   kernel/auditfilter.c                    |   6 +-
>   kernel/auditsc.c                        |  77 ++++----
>   kernel/cred.c                           |  15 +-
>   net/ipv4/cipso_ipv4.c                   |  13 +-
>   net/ipv4/ip_sockglue.c                  |  12 +-
>   net/netfilter/nf_conntrack_netlink.c    |  29 ++-
>   net/netfilter/nf_conntrack_standalone.c |  16 +-
>   net/netfilter/nfnetlink_queue.c         |  38 ++--
>   net/netfilter/nft_meta.c                |  13 +-
>   net/netfilter/xt_SECMARK.c              |  14 +-
>   net/netlabel/netlabel_kapi.c            |   5 +-
>   net/netlabel/netlabel_unlabeled.c       | 101 +++++-----
>   net/netlabel/netlabel_unlabeled.h       |   2 +-
>   net/netlabel/netlabel_user.c            |  13 +-
>   net/netlabel/netlabel_user.h            |   2 +-
>   net/unix/af_unix.c                      |   6 +-
>   security/apparmor/audit.c               |   4 +-
>   security/apparmor/include/audit.h       |   2 +-
>   security/apparmor/include/net.h         |   6 +-
>   security/apparmor/include/secid.h       |   9 +-
>   security/apparmor/lsm.c                 |  64 +++---
>   security/apparmor/secid.c               |  42 ++--
>   security/integrity/ima/ima.h            |  14 +-
>   security/integrity/ima/ima_api.c        |   9 +-
>   security/integrity/ima/ima_appraise.c   |   6 +-
>   security/integrity/ima/ima_main.c       |  34 ++--
>   security/integrity/ima/ima_policy.c     |  19 +-
>   security/security.c                     | 338 +++++++++++++++++++++++++++-----
>   security/selinux/hooks.c                | 259 ++++++++++++------------
>   security/selinux/include/audit.h        |   5 +-
>   security/selinux/include/objsec.h       |  42 +++-
>   security/selinux/netlabel.c             |  25 +--
>   security/selinux/ss/services.c          |  18 +-
>   security/smack/smack.h                  |  18 ++
>   security/smack/smack_lsm.c              | 238 +++++++++++-----------
>   security/smack/smack_netfilter.c        |   8 +-
>   security/smack/smackfs.c                |  12 +-
>   58 files changed, 1217 insertions(+), 779 deletions(-)
>
Casey Schaufler June 4, 2019, 4:14 p.m. UTC | #2
On 6/4/2019 5:29 AM, Stephen Smalley wrote:
> On 6/2/19 12:50 PM, Casey Schaufler wrote:
>> This patchset provides the changes required for
>> the AppArmor security module to stack safely with any other.
>
> Please explain the motivation

I'll add some explanation for the next revision.
It won't be anything that I haven't posted many times
before, but you're right that it belongs in the log.

> - why do we want to allow AppArmor to stack with other modules,

First, is there a reason not to? Sure, you can confuse
administrators by implementing complex security policies,
but there are lots of ways to do that already.

AppArmor provides a different security model than SELinux,
TOMOYO or Smack. Smack is better at system component
separation, while AppArmor is better at application isolation.
It's a win to use each to its strength rather than trying to
stretch either to the edge of what it can do.

> who would use it,

Can't name names, but there have been multiple requests.

> how would it be used,

As mentioned above, Smack for system separation, AppArmor for
application isolation.

> what does it provide that isn't already possible in the absence of it.

It's not necessary that something be impossible to do any
other way. The question should be whether this provides for
a better way to achieve the goals, and this does that.
If I tried the come up with something that's impossible I
would expect the usual "you can do that with SELinux policy"
argument. We know we can do things. We want to have the tools
to do them better.

> Also, Ubuntu fully upstreamed all of their changes to AppArmor, would this still suffice to enable stacking of AppArmor or do they rely on hooks that are not handled here?

Some amount of merging will likely be required. But that's
always going to be true with parallel development tracks.
That's why we have git!

> Please explain the cost of the change - what do we pay in terms of memory, runtime, or other overheads in order to support this change?

Do you have particular benchmarks you want to see?
When I've supplied numbers in the past they have not
been remarked on.

>
>>
>> A new process attribute identifies which security module
>> information should be reported by SO_PEERSEC and the
>> /proc/.../attr/current interface. This is provided by
>> /proc/.../attr/display. Writing the name of the security
>> module desired to this interface will set which LSM hooks
>> will be called for this information. The first security
>> module providing the hooks will be used by default.
>
> Doesn't this effectively undo making the hooks read-only after init, at least for the subset involved?  What are the security implications thereof?

Any mechanism, be it a separate set of hooks, a name used to
do list look ups, or an sophisticated hash scheme will have that
impact for the processes that use it. This scheme has the best
performance profile of the mechanisms I experimented with and
avoids all sorts of special cases.

>
>> The use of integer based security tokens (secids) is
>> generally (but not completely) replaced by a structure
>> lsm_export. The lsm_export structure can contain information
>> for each of the security modules that export information
>> outside the LSM layer.
>>
>> The LSM interfaces that provide "secctx" text strings
>> have been changed to use a structure "lsm_context"
>> instead of a pointer/length pair. In some cases the
>> interfaces used a "char *" pointer and in others a
>> "void *". This was necessary to ensure that the correct
>> release mechanism for the text is used. It also makes
>> many of the interfaces cleaner.
>>
>> https://github.com/cschaufler/lsm-stacking.git#stack-5.2-v1-apparmor
>>
>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>> ---
>>   drivers/android/binder.c                |  25 ++-
>>   fs/kernfs/dir.c                         |   6 +-
>>   fs/kernfs/inode.c                       |  31 ++-
>>   fs/kernfs/kernfs-internal.h             |   3 +-
>>   fs/nfs/inode.c                          |  13 +-
>>   fs/nfs/internal.h                       |   8 +-
>>   fs/nfs/nfs4proc.c                       |  17 +-
>>   fs/nfs/nfs4xdr.c                        |  16 +-
>>   fs/nfsd/nfs4proc.c                      |   8 +-
>>   fs/nfsd/nfs4xdr.c                       |  14 +-
>>   fs/nfsd/vfs.c                           |   7 +-
>>   fs/proc/base.c                          |   1 +
>>   include/linux/cred.h                    |   3 +-
>>   include/linux/lsm_hooks.h               |  91 +++++----
>>   include/linux/nfs4.h                    |   8 +-
>>   include/linux/security.h                | 133 +++++++++----
>>   include/net/af_unix.h                   |   2 +-
>>   include/net/netlabel.h                  |  10 +-
>>   include/net/scm.h                       |  14 +-
>>   kernel/audit.c                          |  43 ++--
>>   kernel/audit.h                          |   9 +-
>>   kernel/auditfilter.c                    |   6 +-
>>   kernel/auditsc.c                        |  77 ++++----
>>   kernel/cred.c                           |  15 +-
>>   net/ipv4/cipso_ipv4.c                   |  13 +-
>>   net/ipv4/ip_sockglue.c                  |  12 +-
>>   net/netfilter/nf_conntrack_netlink.c    |  29 ++-
>>   net/netfilter/nf_conntrack_standalone.c |  16 +-
>>   net/netfilter/nfnetlink_queue.c         |  38 ++--
>>   net/netfilter/nft_meta.c                |  13 +-
>>   net/netfilter/xt_SECMARK.c              |  14 +-
>>   net/netlabel/netlabel_kapi.c            |   5 +-
>>   net/netlabel/netlabel_unlabeled.c       | 101 +++++-----
>>   net/netlabel/netlabel_unlabeled.h       |   2 +-
>>   net/netlabel/netlabel_user.c            |  13 +-
>>   net/netlabel/netlabel_user.h            |   2 +-
>>   net/unix/af_unix.c                      |   6 +-
>>   security/apparmor/audit.c               |   4 +-
>>   security/apparmor/include/audit.h       |   2 +-
>>   security/apparmor/include/net.h         |   6 +-
>>   security/apparmor/include/secid.h       |   9 +-
>>   security/apparmor/lsm.c                 |  64 +++---
>>   security/apparmor/secid.c               |  42 ++--
>>   security/integrity/ima/ima.h            |  14 +-
>>   security/integrity/ima/ima_api.c        |   9 +-
>>   security/integrity/ima/ima_appraise.c   |   6 +-
>>   security/integrity/ima/ima_main.c       |  34 ++--
>>   security/integrity/ima/ima_policy.c     |  19 +-
>>   security/security.c                     | 338 +++++++++++++++++++++++++++-----
>>   security/selinux/hooks.c                | 259 ++++++++++++------------
>>   security/selinux/include/audit.h        |   5 +-
>>   security/selinux/include/objsec.h       |  42 +++-
>>   security/selinux/netlabel.c             |  25 +--
>>   security/selinux/ss/services.c          |  18 +-
>>   security/smack/smack.h                  |  18 ++
>>   security/smack/smack_lsm.c              | 238 +++++++++++-----------
>>   security/smack/smack_netfilter.c        |   8 +-
>>   security/smack/smackfs.c                |  12 +-
>>   58 files changed, 1217 insertions(+), 779 deletions(-)
>>
>
Stephen Smalley June 4, 2019, 5:11 p.m. UTC | #3
On 6/4/19 12:14 PM, Casey Schaufler wrote:
> On 6/4/2019 5:29 AM, Stephen Smalley wrote:
>> On 6/2/19 12:50 PM, Casey Schaufler wrote:
>>> This patchset provides the changes required for
>>> the AppArmor security module to stack safely with any other.
>>
>> Please explain the motivation
> 
> I'll add some explanation for the next revision.
> It won't be anything that I haven't posted many times
> before, but you're right that it belongs in the log.
> 
>> - why do we want to allow AppArmor to stack with other modules,
> 
> First, is there a reason not to? Sure, you can confuse
> administrators by implementing complex security policies,
> but there are lots of ways to do that already.

There are costs to doing so, e.g.
- greater complexity in the security framework,
- possibly greater memory and runtime overheads,
- potential user confusion (which security module(s) caused a given 
failure?)
- potential distro maintainer burden (similar to above, but performing 
triage when any given permission denial can have multiple causes beyond 
just DAC + one module, weird interactions among modules, etc)

It isn't free so there should be a cost/benefit analysis.

> 
> AppArmor provides a different security model than SELinux,
> TOMOYO or Smack. Smack is better at system component
> separation, while AppArmor is better at application isolation.
> It's a win to use each to its strength rather than trying to
> stretch either to the edge of what it can do.
> 
>> who would use it,
> 
> Can't name names, but there have been multiple requests.
> 
>> how would it be used,
> 
> As mentioned above, Smack for system separation, AppArmor for
> application isolation.

Can you provide a concrete example of how combining the two yields a 
smaller, simpler configuration overall than using them individually?

> 
>> what does it provide that isn't already possible in the absence of it.
> 
> It's not necessary that something be impossible to do any
> other way. The question should be whether this provides for
> a better way to achieve the goals, and this does that.
> If I tried the come up with something that's impossible I
> would expect the usual "you can do that with SELinux policy"
> argument. We know we can do things. We want to have the tools
> to do them better.
> 
>> Also, Ubuntu fully upstreamed all of their changes to AppArmor, would this still suffice to enable stacking of AppArmor or do they rely on hooks that are not handled here?
> 
> Some amount of merging will likely be required. But that's
> always going to be true with parallel development tracks.
> That's why we have git!
> 
>> Please explain the cost of the change - what do we pay in terms of memory, runtime, or other overheads in order to support this change?
> 
> Do you have particular benchmarks you want to see?
> When I've supplied numbers in the past they have not
> been remarked on.

A combination of micro and macro benchmarks exercising multiple kernel 
subsystems would be good.  Kernel build time isn't sufficient.

> 
>>
>>>
>>> A new process attribute identifies which security module
>>> information should be reported by SO_PEERSEC and the
>>> /proc/.../attr/current interface. This is provided by
>>> /proc/.../attr/display. Writing the name of the security
>>> module desired to this interface will set which LSM hooks
>>> will be called for this information. The first security
>>> module providing the hooks will be used by default.
>>
>> Doesn't this effectively undo making the hooks read-only after init, at least for the subset involved?  What are the security implications thereof?
> 
> Any mechanism, be it a separate set of hooks, a name used to
> do list look ups, or an sophisticated hash scheme will have that
> impact for the processes that use it. This scheme has the best
> performance profile of the mechanisms I experimented with and
> avoids all sorts of special cases.
> 
>>
>>> The use of integer based security tokens (secids) is
>>> generally (but not completely) replaced by a structure
>>> lsm_export. The lsm_export structure can contain information
>>> for each of the security modules that export information
>>> outside the LSM layer.
>>>
>>> The LSM interfaces that provide "secctx" text strings
>>> have been changed to use a structure "lsm_context"
>>> instead of a pointer/length pair. In some cases the
>>> interfaces used a "char *" pointer and in others a
>>> "void *". This was necessary to ensure that the correct
>>> release mechanism for the text is used. It also makes
>>> many of the interfaces cleaner.
>>>
>>> https://github.com/cschaufler/lsm-stacking.git#stack-5.2-v1-apparmor
>>>
>>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>>> ---
>>>    drivers/android/binder.c                |  25 ++-
>>>    fs/kernfs/dir.c                         |   6 +-
>>>    fs/kernfs/inode.c                       |  31 ++-
>>>    fs/kernfs/kernfs-internal.h             |   3 +-
>>>    fs/nfs/inode.c                          |  13 +-
>>>    fs/nfs/internal.h                       |   8 +-
>>>    fs/nfs/nfs4proc.c                       |  17 +-
>>>    fs/nfs/nfs4xdr.c                        |  16 +-
>>>    fs/nfsd/nfs4proc.c                      |   8 +-
>>>    fs/nfsd/nfs4xdr.c                       |  14 +-
>>>    fs/nfsd/vfs.c                           |   7 +-
>>>    fs/proc/base.c                          |   1 +
>>>    include/linux/cred.h                    |   3 +-
>>>    include/linux/lsm_hooks.h               |  91 +++++----
>>>    include/linux/nfs4.h                    |   8 +-
>>>    include/linux/security.h                | 133 +++++++++----
>>>    include/net/af_unix.h                   |   2 +-
>>>    include/net/netlabel.h                  |  10 +-
>>>    include/net/scm.h                       |  14 +-
>>>    kernel/audit.c                          |  43 ++--
>>>    kernel/audit.h                          |   9 +-
>>>    kernel/auditfilter.c                    |   6 +-
>>>    kernel/auditsc.c                        |  77 ++++----
>>>    kernel/cred.c                           |  15 +-
>>>    net/ipv4/cipso_ipv4.c                   |  13 +-
>>>    net/ipv4/ip_sockglue.c                  |  12 +-
>>>    net/netfilter/nf_conntrack_netlink.c    |  29 ++-
>>>    net/netfilter/nf_conntrack_standalone.c |  16 +-
>>>    net/netfilter/nfnetlink_queue.c         |  38 ++--
>>>    net/netfilter/nft_meta.c                |  13 +-
>>>    net/netfilter/xt_SECMARK.c              |  14 +-
>>>    net/netlabel/netlabel_kapi.c            |   5 +-
>>>    net/netlabel/netlabel_unlabeled.c       | 101 +++++-----
>>>    net/netlabel/netlabel_unlabeled.h       |   2 +-
>>>    net/netlabel/netlabel_user.c            |  13 +-
>>>    net/netlabel/netlabel_user.h            |   2 +-
>>>    net/unix/af_unix.c                      |   6 +-
>>>    security/apparmor/audit.c               |   4 +-
>>>    security/apparmor/include/audit.h       |   2 +-
>>>    security/apparmor/include/net.h         |   6 +-
>>>    security/apparmor/include/secid.h       |   9 +-
>>>    security/apparmor/lsm.c                 |  64 +++---
>>>    security/apparmor/secid.c               |  42 ++--
>>>    security/integrity/ima/ima.h            |  14 +-
>>>    security/integrity/ima/ima_api.c        |   9 +-
>>>    security/integrity/ima/ima_appraise.c   |   6 +-
>>>    security/integrity/ima/ima_main.c       |  34 ++--
>>>    security/integrity/ima/ima_policy.c     |  19 +-
>>>    security/security.c                     | 338 +++++++++++++++++++++++++++-----
>>>    security/selinux/hooks.c                | 259 ++++++++++++------------
>>>    security/selinux/include/audit.h        |   5 +-
>>>    security/selinux/include/objsec.h       |  42 +++-
>>>    security/selinux/netlabel.c             |  25 +--
>>>    security/selinux/ss/services.c          |  18 +-
>>>    security/smack/smack.h                  |  18 ++
>>>    security/smack/smack_lsm.c              | 238 +++++++++++-----------
>>>    security/smack/smack_netfilter.c        |   8 +-
>>>    security/smack/smackfs.c                |  12 +-
>>>    58 files changed, 1217 insertions(+), 779 deletions(-)
>>>
>>
Casey Schaufler June 4, 2019, 7:58 p.m. UTC | #4
On 6/4/2019 10:11 AM, Stephen Smalley wrote:
> On 6/4/19 12:14 PM, Casey Schaufler wrote:
>> On 6/4/2019 5:29 AM, Stephen Smalley wrote:
>>> On 6/2/19 12:50 PM, Casey Schaufler wrote:
>>>> This patchset provides the changes required for
>>>> the AppArmor security module to stack safely with any other.
>>>
>>> Please explain the motivation
>>
>> I'll add some explanation for the next revision.
>> It won't be anything that I haven't posted many times
>> before, but you're right that it belongs in the log.
>>
>>> - why do we want to allow AppArmor to stack with other modules,
>>
>> First, is there a reason not to? Sure, you can confuse
>> administrators by implementing complex security policies,
>> but there are lots of ways to do that already.
>
> There are costs to doing so, e.g.
> - greater complexity in the security framework,

Taking blob management out of the modules and
into the framework makes simplifies the modules.

> - possibly greater memory and runtime overheads,

Possibly reduced memory and runtime overheads, as well.

> - potential user confusion (which security module(s) caused a given failure?)

That's not new. I've seen countless cases where users blame
SELinux or Smack when the problem is with mode bits and/or
capabilities. Not to mention that a good 50% of current Linux
users don't understand any of the security mechanisms to
begin with.

> - potential distro maintainer burden

Selection of security modules and how they are configured
has always been a burden for distro developers and maintainers.
Nothing new here.
And, they knew the job was dangerous when they took it.


> (similar to above, but performing triage when any given permission denial can have multiple causes beyond just DAC + one module, weird interactions among modules, etc)

Yama has been widely accepted by distros, and civilization
has yet to have officially been declared ended.

> It isn't free so there should be a cost/benefit analysis.

Some benchmarking is definitely in order, but most
of what's you're calling out as downside is hypothetical
or based on assumption. 

>
>>
>> AppArmor provides a different security model than SELinux,
>> TOMOYO or Smack. Smack is better at system component
>> separation, while AppArmor is better at application isolation.
>> It's a win to use each to its strength rather than trying to
>> stretch either to the edge of what it can do.
>>
>>> who would use it,
>>
>> Can't name names, but there have been multiple requests.
>>
>>> how would it be used,
>>
>> As mentioned above, Smack for system separation, AppArmor for
>> application isolation.
>
> Can you provide a concrete example of how combining the two yields a smaller, simpler configuration overall than using them individually?

Smack + AppArmor is a simpler, smaller model than the Smack policy
used in Tizen 2.

>
>>
>>> what does it provide that isn't already possible in the absence of it.
>>
>> It's not necessary that something be impossible to do any
>> other way. The question should be whether this provides for
>> a better way to achieve the goals, and this does that.
>> If I tried the come up with something that's impossible I
>> would expect the usual "you can do that with SELinux policy"
>> argument. We know we can do things. We want to have the tools
>> to do them better.
>>
>>> Also, Ubuntu fully upstreamed all of their changes to AppArmor, would this still suffice to enable stacking of AppArmor or do they rely on hooks that are not handled here?
>>
>> Some amount of merging will likely be required. But that's
>> always going to be true with parallel development tracks.
>> That's why we have git!
>>
>>> Please explain the cost of the change - what do we pay in terms of memory, runtime, or other overheads in order to support this change?
>>
>> Do you have particular benchmarks you want to see?
>> When I've supplied numbers in the past they have not
>> been remarked on.
>
> A combination of micro and macro benchmarks exercising multiple kernel subsystems would be good.  Kernel build time isn't sufficient.

Do you have preferences, or better yet, facilities
for running them? I am, alas, running on finite resources
and benchmark contributions, especially in areas where you
have specific concerns, would be most welcome.

>
>>
>>>
>>>>
>>>> A new process attribute identifies which security module
>>>> information should be reported by SO_PEERSEC and the
>>>> /proc/.../attr/current interface. This is provided by
>>>> /proc/.../attr/display. Writing the name of the security
>>>> module desired to this interface will set which LSM hooks
>>>> will be called for this information. The first security
>>>> module providing the hooks will be used by default.
>>>
>>> Doesn't this effectively undo making the hooks read-only after init, at least for the subset involved?  What are the security implications thereof?
>>
>> Any mechanism, be it a separate set of hooks, a name used to
>> do list look ups, or an sophisticated hash scheme will have that
>> impact for the processes that use it. This scheme has the best
>> performance profile of the mechanisms I experimented with and
>> avoids all sorts of special cases.
>>
>>>
>>>> The use of integer based security tokens (secids) is
>>>> generally (but not completely) replaced by a structure
>>>> lsm_export. The lsm_export structure can contain information
>>>> for each of the security modules that export information
>>>> outside the LSM layer.
>>>>
>>>> The LSM interfaces that provide "secctx" text strings
>>>> have been changed to use a structure "lsm_context"
>>>> instead of a pointer/length pair. In some cases the
>>>> interfaces used a "char *" pointer and in others a
>>>> "void *". This was necessary to ensure that the correct
>>>> release mechanism for the text is used. It also makes
>>>> many of the interfaces cleaner.
>>>>
>>>> https://github.com/cschaufler/lsm-stacking.git#stack-5.2-v1-apparmor
>>>>
>>>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>>>> ---
>>>>    drivers/android/binder.c                |  25 ++-
>>>>    fs/kernfs/dir.c                         |   6 +-
>>>>    fs/kernfs/inode.c                       |  31 ++-
>>>>    fs/kernfs/kernfs-internal.h             |   3 +-
>>>>    fs/nfs/inode.c                          |  13 +-
>>>>    fs/nfs/internal.h                       |   8 +-
>>>>    fs/nfs/nfs4proc.c                       |  17 +-
>>>>    fs/nfs/nfs4xdr.c                        |  16 +-
>>>>    fs/nfsd/nfs4proc.c                      |   8 +-
>>>>    fs/nfsd/nfs4xdr.c                       |  14 +-
>>>>    fs/nfsd/vfs.c                           |   7 +-
>>>>    fs/proc/base.c                          |   1 +
>>>>    include/linux/cred.h                    |   3 +-
>>>>    include/linux/lsm_hooks.h               |  91 +++++----
>>>>    include/linux/nfs4.h                    |   8 +-
>>>>    include/linux/security.h                | 133 +++++++++----
>>>>    include/net/af_unix.h                   |   2 +-
>>>>    include/net/netlabel.h                  |  10 +-
>>>>    include/net/scm.h                       |  14 +-
>>>>    kernel/audit.c                          |  43 ++--
>>>>    kernel/audit.h                          |   9 +-
>>>>    kernel/auditfilter.c                    |   6 +-
>>>>    kernel/auditsc.c                        |  77 ++++----
>>>>    kernel/cred.c                           |  15 +-
>>>>    net/ipv4/cipso_ipv4.c                   |  13 +-
>>>>    net/ipv4/ip_sockglue.c                  |  12 +-
>>>>    net/netfilter/nf_conntrack_netlink.c    |  29 ++-
>>>>    net/netfilter/nf_conntrack_standalone.c |  16 +-
>>>>    net/netfilter/nfnetlink_queue.c         |  38 ++--
>>>>    net/netfilter/nft_meta.c                |  13 +-
>>>>    net/netfilter/xt_SECMARK.c              |  14 +-
>>>>    net/netlabel/netlabel_kapi.c            |   5 +-
>>>>    net/netlabel/netlabel_unlabeled.c       | 101 +++++-----
>>>>    net/netlabel/netlabel_unlabeled.h       |   2 +-
>>>>    net/netlabel/netlabel_user.c            |  13 +-
>>>>    net/netlabel/netlabel_user.h            |   2 +-
>>>>    net/unix/af_unix.c                      |   6 +-
>>>>    security/apparmor/audit.c               |   4 +-
>>>>    security/apparmor/include/audit.h       |   2 +-
>>>>    security/apparmor/include/net.h         |   6 +-
>>>>    security/apparmor/include/secid.h       |   9 +-
>>>>    security/apparmor/lsm.c                 |  64 +++---
>>>>    security/apparmor/secid.c               |  42 ++--
>>>>    security/integrity/ima/ima.h            |  14 +-
>>>>    security/integrity/ima/ima_api.c        |   9 +-
>>>>    security/integrity/ima/ima_appraise.c   |   6 +-
>>>>    security/integrity/ima/ima_main.c       |  34 ++--
>>>>    security/integrity/ima/ima_policy.c     |  19 +-
>>>>    security/security.c                     | 338 +++++++++++++++++++++++++++-----
>>>>    security/selinux/hooks.c                | 259 ++++++++++++------------
>>>>    security/selinux/include/audit.h        |   5 +-
>>>>    security/selinux/include/objsec.h       |  42 +++-
>>>>    security/selinux/netlabel.c             |  25 +--
>>>>    security/selinux/ss/services.c          |  18 +-
>>>>    security/smack/smack.h                  |  18 ++
>>>>    security/smack/smack_lsm.c              | 238 +++++++++++-----------
>>>>    security/smack/smack_netfilter.c        |   8 +-
>>>>    security/smack/smackfs.c                |  12 +-
>>>>    58 files changed, 1217 insertions(+), 779 deletions(-)
>>>>
>>>
>
Stephen Smalley June 4, 2019, 8:34 p.m. UTC | #5
On 6/4/19 3:58 PM, Casey Schaufler wrote:
> On 6/4/2019 10:11 AM, Stephen Smalley wrote:
>> On 6/4/19 12:14 PM, Casey Schaufler wrote:
>>> On 6/4/2019 5:29 AM, Stephen Smalley wrote:
>>>> On 6/2/19 12:50 PM, Casey Schaufler wrote:
>>>>> This patchset provides the changes required for
>>>>> the AppArmor security module to stack safely with any other.
>>>>
>>>> Please explain the motivation
>>>
>>> I'll add some explanation for the next revision.
>>> It won't be anything that I haven't posted many times
>>> before, but you're right that it belongs in the log.
>>>
>>>> - why do we want to allow AppArmor to stack with other modules,
>>>
>>> First, is there a reason not to? Sure, you can confuse
>>> administrators by implementing complex security policies,
>>> but there are lots of ways to do that already.
>>
>> There are costs to doing so, e.g.
>> - greater complexity in the security framework,
> 
> Taking blob management out of the modules and
> into the framework makes simplifies the modules.
> 
>> - possibly greater memory and runtime overheads,
> 
> Possibly reduced memory and runtime overheads, as well.
> 
>> - potential user confusion (which security module(s) caused a given failure?)
> 
> That's not new. I've seen countless cases where users blame
> SELinux or Smack when the problem is with mode bits and/or
> capabilities. Not to mention that a good 50% of current Linux
> users don't understand any of the security mechanisms to
> begin with.
> 
>> - potential distro maintainer burden
> 
> Selection of security modules and how they are configured
> has always been a burden for distro developers and maintainers.
> Nothing new here.
> And, they knew the job was dangerous when they took it.
> 
> 
>> (similar to above, but performing triage when any given permission denial can have multiple causes beyond just DAC + one module, weird interactions among modules, etc)
> 
> Yama has been widely accepted by distros, and civilization
> has yet to have officially been declared ended.
> 
>> It isn't free so there should be a cost/benefit analysis.
> 
> Some benchmarking is definitely in order, but most
> of what's you're calling out as downside is hypothetical
> or based on assumption.
> 
>>
>>>
>>> AppArmor provides a different security model than SELinux,
>>> TOMOYO or Smack. Smack is better at system component
>>> separation, while AppArmor is better at application isolation.
>>> It's a win to use each to its strength rather than trying to
>>> stretch either to the edge of what it can do.
>>>
>>>> who would use it,
>>>
>>> Can't name names, but there have been multiple requests.
>>>
>>>> how would it be used,
>>>
>>> As mentioned above, Smack for system separation, AppArmor for
>>> application isolation.
>>
>> Can you provide a concrete example of how combining the two yields a smaller, simpler configuration overall than using them individually?
> 
> Smack + AppArmor is a simpler, smaller model than the Smack policy
> used in Tizen 2.

Not very compelling given that not even Tizen is using that policy 
anymore.  But even taking that as an example, have you actually done the 
work to craft a working configuration of Smack + AppArmor that meets the 
same security goals as the Tizen 2 policy and confirmed that it is in 
fact smaller and simpler?  Won't it just move the size/complexity from 
Smack rules to AppArmor profiles?  And those might be larger...

> 
>>
>>>
>>>> what does it provide that isn't already possible in the absence of it.
>>>
>>> It's not necessary that something be impossible to do any
>>> other way. The question should be whether this provides for
>>> a better way to achieve the goals, and this does that.
>>> If I tried the come up with something that's impossible I
>>> would expect the usual "you can do that with SELinux policy"
>>> argument. We know we can do things. We want to have the tools
>>> to do them better.
>>>
>>>> Also, Ubuntu fully upstreamed all of their changes to AppArmor, would this still suffice to enable stacking of AppArmor or do they rely on hooks that are not handled here?
>>>
>>> Some amount of merging will likely be required. But that's
>>> always going to be true with parallel development tracks.
>>> That's why we have git!
>>>
>>>> Please explain the cost of the change - what do we pay in terms of memory, runtime, or other overheads in order to support this change?
>>>
>>> Do you have particular benchmarks you want to see?
>>> When I've supplied numbers in the past they have not
>>> been remarked on.
>>
>> A combination of micro and macro benchmarks exercising multiple kernel subsystems would be good.  Kernel build time isn't sufficient.
> 
> Do you have preferences, or better yet, facilities
> for running them? I am, alas, running on finite resources
> and benchmark contributions, especially in areas where you
> have specific concerns, would be most welcome.
> 
>>
>>>
>>>>
>>>>>
>>>>> A new process attribute identifies which security module
>>>>> information should be reported by SO_PEERSEC and the
>>>>> /proc/.../attr/current interface. This is provided by
>>>>> /proc/.../attr/display. Writing the name of the security
>>>>> module desired to this interface will set which LSM hooks
>>>>> will be called for this information. The first security
>>>>> module providing the hooks will be used by default.
>>>>
>>>> Doesn't this effectively undo making the hooks read-only after init, at least for the subset involved?  What are the security implications thereof?
>>>
>>> Any mechanism, be it a separate set of hooks, a name used to
>>> do list look ups, or an sophisticated hash scheme will have that
>>> impact for the processes that use it. This scheme has the best
>>> performance profile of the mechanisms I experimented with and
>>> avoids all sorts of special cases.
>>>
>>>>
>>>>> The use of integer based security tokens (secids) is
>>>>> generally (but not completely) replaced by a structure
>>>>> lsm_export. The lsm_export structure can contain information
>>>>> for each of the security modules that export information
>>>>> outside the LSM layer.
>>>>>
>>>>> The LSM interfaces that provide "secctx" text strings
>>>>> have been changed to use a structure "lsm_context"
>>>>> instead of a pointer/length pair. In some cases the
>>>>> interfaces used a "char *" pointer and in others a
>>>>> "void *". This was necessary to ensure that the correct
>>>>> release mechanism for the text is used. It also makes
>>>>> many of the interfaces cleaner.
>>>>>
>>>>> https://github.com/cschaufler/lsm-stacking.git#stack-5.2-v1-apparmor
>>>>>
>>>>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>>>>> ---
>>>>>     drivers/android/binder.c                |  25 ++-
>>>>>     fs/kernfs/dir.c                         |   6 +-
>>>>>     fs/kernfs/inode.c                       |  31 ++-
>>>>>     fs/kernfs/kernfs-internal.h             |   3 +-
>>>>>     fs/nfs/inode.c                          |  13 +-
>>>>>     fs/nfs/internal.h                       |   8 +-
>>>>>     fs/nfs/nfs4proc.c                       |  17 +-
>>>>>     fs/nfs/nfs4xdr.c                        |  16 +-
>>>>>     fs/nfsd/nfs4proc.c                      |   8 +-
>>>>>     fs/nfsd/nfs4xdr.c                       |  14 +-
>>>>>     fs/nfsd/vfs.c                           |   7 +-
>>>>>     fs/proc/base.c                          |   1 +
>>>>>     include/linux/cred.h                    |   3 +-
>>>>>     include/linux/lsm_hooks.h               |  91 +++++----
>>>>>     include/linux/nfs4.h                    |   8 +-
>>>>>     include/linux/security.h                | 133 +++++++++----
>>>>>     include/net/af_unix.h                   |   2 +-
>>>>>     include/net/netlabel.h                  |  10 +-
>>>>>     include/net/scm.h                       |  14 +-
>>>>>     kernel/audit.c                          |  43 ++--
>>>>>     kernel/audit.h                          |   9 +-
>>>>>     kernel/auditfilter.c                    |   6 +-
>>>>>     kernel/auditsc.c                        |  77 ++++----
>>>>>     kernel/cred.c                           |  15 +-
>>>>>     net/ipv4/cipso_ipv4.c                   |  13 +-
>>>>>     net/ipv4/ip_sockglue.c                  |  12 +-
>>>>>     net/netfilter/nf_conntrack_netlink.c    |  29 ++-
>>>>>     net/netfilter/nf_conntrack_standalone.c |  16 +-
>>>>>     net/netfilter/nfnetlink_queue.c         |  38 ++--
>>>>>     net/netfilter/nft_meta.c                |  13 +-
>>>>>     net/netfilter/xt_SECMARK.c              |  14 +-
>>>>>     net/netlabel/netlabel_kapi.c            |   5 +-
>>>>>     net/netlabel/netlabel_unlabeled.c       | 101 +++++-----
>>>>>     net/netlabel/netlabel_unlabeled.h       |   2 +-
>>>>>     net/netlabel/netlabel_user.c            |  13 +-
>>>>>     net/netlabel/netlabel_user.h            |   2 +-
>>>>>     net/unix/af_unix.c                      |   6 +-
>>>>>     security/apparmor/audit.c               |   4 +-
>>>>>     security/apparmor/include/audit.h       |   2 +-
>>>>>     security/apparmor/include/net.h         |   6 +-
>>>>>     security/apparmor/include/secid.h       |   9 +-
>>>>>     security/apparmor/lsm.c                 |  64 +++---
>>>>>     security/apparmor/secid.c               |  42 ++--
>>>>>     security/integrity/ima/ima.h            |  14 +-
>>>>>     security/integrity/ima/ima_api.c        |   9 +-
>>>>>     security/integrity/ima/ima_appraise.c   |   6 +-
>>>>>     security/integrity/ima/ima_main.c       |  34 ++--
>>>>>     security/integrity/ima/ima_policy.c     |  19 +-
>>>>>     security/security.c                     | 338 +++++++++++++++++++++++++++-----
>>>>>     security/selinux/hooks.c                | 259 ++++++++++++------------
>>>>>     security/selinux/include/audit.h        |   5 +-
>>>>>     security/selinux/include/objsec.h       |  42 +++-
>>>>>     security/selinux/netlabel.c             |  25 +--
>>>>>     security/selinux/ss/services.c          |  18 +-
>>>>>     security/smack/smack.h                  |  18 ++
>>>>>     security/smack/smack_lsm.c              | 238 +++++++++++-----------
>>>>>     security/smack/smack_netfilter.c        |   8 +-
>>>>>     security/smack/smackfs.c                |  12 +-
>>>>>     58 files changed, 1217 insertions(+), 779 deletions(-)
>>>>>
>>>>
>>
James Morris June 4, 2019, 8:42 p.m. UTC | #6
On Tue, 4 Jun 2019, Casey Schaufler wrote:

> > It isn't free so there should be a cost/benefit analysis.
> 
> Some benchmarking is definitely in order, but most
> of what's you're calling out as downside is hypothetical
> or based on assumption. 

When you're proposing changes such as these, which make fundamental and 
far-reaching changes, the burden is on you to present the cost/benefit 
analysis.

You can't just say "Here are some changes and here are the benefits, and 
any possible costs are merely hypothetical".
Casey Schaufler June 4, 2019, 9:19 p.m. UTC | #7
On 6/4/2019 1:42 PM, James Morris wrote:
> On Tue, 4 Jun 2019, Casey Schaufler wrote:
>
>>> It isn't free so there should be a cost/benefit analysis.
>> Some benchmarking is definitely in order, but most
>> of what's you're calling out as downside is hypothetical
>> or based on assumption. 
> When you're proposing changes such as these, which make fundamental and 
> far-reaching changes, the burden is on you to present the cost/benefit 
> analysis.

Granted. There has been substantial conversation about it
over the years, but I have not done well including it in
this discussion.

> You can't just say "Here are some changes and here are the benefits, and 
> any possible costs are merely hypothetical".

Of course. Nonetheless, no evidence for performance impact has
been provided, while it has been asserted.
John Johansen June 5, 2019, 1:50 a.m. UTC | #8
On 6/4/19 5:29 AM, Stephen Smalley wrote:
> On 6/2/19 12:50 PM, Casey Schaufler wrote:
>> This patchset provides the changes required for
>> the AppArmor security module to stack safely with any other.
> 
> Please explain the motivation - why do we want to allow AppArmor to stack with other modules, who would use it, how would it be used, what does it provide that isn't already possible in the absence of it.
> 

Its another step towards making stacking generic. The current stacking in 5.2 only allows for a subset of blobs and limits what can be done by new security modules. This is another step towards achieving generic stacking. Whether it makes sense to stacking a given set of security modules is a different discussion. I am fairly sure if landlock/sara get in upstream they will at some point want access to parts of the LSM that are currently limited to a major LSM.

On the apparmor front stacking with other modules has been asked for in the context of containers, both application and system. For example snapd would like to be able to stack apparmor on an selinux system so the snap container can enforce its apparmor set of policy while the system as a whole is still being protected by selinux. Similar requests have been made for lxd doing system containers. lxd currently supports nested apparmor, so on an ubuntu system you can run suse container, where the ubuntu host is enforcing policy and the suse container is loading and enforcing its policy as well. In this case the policy of the container is bounded by the policy of the host. The goal is to be able to the same with selinux and smack based systems, LSM stacking is of course only part of what is required to make this work.

Currently with the stacking patches we can boot a fedora system, and run an ubuntu container with apparmor enforcing its policy inside the container and selinux enforcing its policy on the host.


> Also, Ubuntu fully upstreamed all of their changes to AppArmor, would this still suffice to enable stacking of AppArmor or do they rely on hooks that are not handled here?

Ubuntu actually has a very small apparmor delta these days, and we are working on eliminating it entirely. There are no patches in Ubuntu that require new hooks. As for the delta wrt to the stacking work, Ubuntu has pulled in a subset of this delta and has been shipping kernels with stacking enabled for 4 releases now and apparmor development is done with LSM stacking in mind.
James Morris June 5, 2019, 3:08 a.m. UTC | #9
On Tue, 4 Jun 2019, John Johansen wrote:

> system as a whole is still being protected by selinux. Similar requests 
> have been made for lxd doing system containers. lxd currently supports 
> nested apparmor, so on an ubuntu system you can run suse container, 
> where the ubuntu host is enforcing policy and the suse container is 
> loading and enforcing its policy as well. In this case the policy of the 
> container is bounded by the policy of the host. The goal is to be able 
> to the same with selinux and smack based systems, LSM stacking is of 
> course only part of what is required to make this work.

Interesting. So you're stacking apparmor with itself, and one is the 
container instance? And you add another stacked apparmor for a 2nd 
container etc. ?

> Ubuntu actually has a very small apparmor delta these days, and we are 
> working on eliminating it entirely. There are no patches in Ubuntu that 
> require new hooks. As for the delta wrt to the stacking work, Ubuntu has 
> pulled in a subset of this delta and has been shipping kernels with 
> stacking enabled for 4 releases now and apparmor development is done 
> with LSM stacking in mind.

A subset of these patches from Casey?
John Johansen June 5, 2019, 5:03 a.m. UTC | #10
On 6/4/19 8:08 PM, James Morris wrote:
> On Tue, 4 Jun 2019, John Johansen wrote:
> 
>> system as a whole is still being protected by selinux. Similar requests 
>> have been made for lxd doing system containers. lxd currently supports 
>> nested apparmor, so on an ubuntu system you can run suse container, 
>> where the ubuntu host is enforcing policy and the suse container is 
>> loading and enforcing its policy as well. In this case the policy of the 
>> container is bounded by the policy of the host. The goal is to be able 
>> to the same with selinux and smack based systems, LSM stacking is of 
>> course only part of what is required to make this work.
> 
> Interesting. So you're stacking apparmor with itself, and one is the 
> container instance? And you add another stacked apparmor for a 2nd 
> container etc. ?

Yes, on Ubuntu & suse you can lauch lxd system containers with the
container having a system policy bounding the container, and the container
having its own apparmor policy namespace. So it loads and has its own
policy that is enforced.

This allows for us to run older versions of ubuntu (say 16.04) on an
18.04 host, and have the 16.04 policy behave just as if it was the host.

> 
>> Ubuntu actually has a very small apparmor delta these days, and we are 
>> working on eliminating it entirely. There are no patches in Ubuntu that 
>> require new hooks. As for the delta wrt to the stacking work, Ubuntu has 
>> pulled in a subset of this delta and has been shipping kernels with 
>> stacking enabled for 4 releases now and apparmor development is done 
>> with LSM stacking in mind.
> 
> A subset of these patches from Casey?
> 

Yes, we have been testing the and using Casey's patches. The set has
changed from release to release, and we don't usually take all of them.
As we are trying to find the right balance.

For 19.04 we did test the full stack and the decision was to hold off
and give it more testing. The big concern was around the secid changes
which needed more review and testing before we could commit to them.
Instead we cherry-picked the stacking patches from 5.2 and a subset of
the set under review, and reverted the upstream apparmor changes that
require secids.

This approach won't be an option for the 19.10 release and we will be
needing the full patchset. I should be able to provide some benchmark
and testing data soon.
James Morris June 5, 2019, 8:53 p.m. UTC | #11
On Tue, 4 Jun 2019, John Johansen wrote:

> Yes, on Ubuntu & suse you can lauch lxd system containers with the
> container having a system policy bounding the container, and the container
> having its own apparmor policy namespace. So it loads and has its own
> policy that is enforced.
> 
> This allows for us to run older versions of ubuntu (say 16.04) on an
> 18.04 host, and have the 16.04 policy behave just as if it was the host.

How well does the LSM stacking scale to 100s or more containers?

> This approach won't be an option for the 19.10 release and we will be
> needing the full patchset. I should be able to provide some benchmark
> and testing data soon.

Great.
John Johansen June 5, 2019, 9:43 p.m. UTC | #12
On 6/5/19 1:53 PM, James Morris wrote:
> On Tue, 4 Jun 2019, John Johansen wrote:
> 
>> Yes, on Ubuntu & suse you can lauch lxd system containers with the
>> container having a system policy bounding the container, and the container
>> having its own apparmor policy namespace. So it loads and has its own
>> policy that is enforced.
>>
>> This allows for us to run older versions of ubuntu (say 16.04) on an
>> 18.04 host, and have the 16.04 policy behave just as if it was the host.
> 
> How well does the LSM stacking scale to 100s or more containers?
> 

Actually really well,

The cost isn't really based on how many containers but how many LSMs
are registered and how nested we are.

How we are currently handling it is apparmor is registered once, and
it is responsible for looping on its bounding. So for tasks that are
not in the container there is no additional cost.

For tasks in the first container, there is an extra cost of enforcing
the extra layer of apparmor policy loaded in the container. If you do
container in container there are two extra levels of apparmor policy.

This does rely on apparmor doing its own namespacing and bounding. LSM
stacking just allows us to start doing this with apparmor containers
on smack and selinux based systems.


>> This approach won't be an option for the 19.10 release and we will be
>> needing the full patchset. I should be able to provide some benchmark
>> and testing data soon.
> 
> Great.
>
James Morris June 5, 2019, 10:28 p.m. UTC | #13
On Wed, 5 Jun 2019, John Johansen wrote:

> This does rely on apparmor doing its own namespacing and bounding. LSM
> stacking just allows us to start doing this with apparmor containers
> on smack and selinux based systems.

Ahh, ok, I thought you were using an LSM stack for each container.
José Bollo June 7, 2019, 1:03 p.m. UTC | #14
On Tue, 4 Jun 2019 09:14:42 -0700
Casey Schaufler <casey@schaufler-ca.com> wrote:

> On 6/4/2019 5:29 AM, Stephen Smalley wrote:
> > On 6/2/19 12:50 PM, Casey Schaufler wrote:  
> >> This patchset provides the changes required for
> >> the AppArmor security module to stack safely with any other.  
> >
> > Please explain the motivation  
> 
> I'll add some explanation for the next revision.
> It won't be anything that I haven't posted many times
> before, but you're right that it belongs in the log.
> 
> > - why do we want to allow AppArmor to stack with other modules,  
> 
> First, is there a reason not to? Sure, you can confuse
> administrators by implementing complex security policies,
> but there are lots of ways to do that already.
> 
> AppArmor provides a different security model than SELinux,
> TOMOYO or Smack. Smack is better at system component
> separation, while AppArmor is better at application isolation.
> It's a win to use each to its strength rather than trying to
> stretch either to the edge of what it can do.
> 
> > who would use it,  

Hi all,

I would like to expose a potential use of interest for me: being able
to have containers running Smack on Ubuntu or Fedora platforms.

But it could also be interesting for running a container having fedora
on ubuntu or suse or the opposite.

How it will work? Will it work? Ask Casey.

just my 2 pennies
José Bollo

> Can't name names, but there have been multiple requests.
> 
> > how would it be used,  
> 
> As mentioned above, Smack for system separation, AppArmor for
> application isolation.
> 
> > what does it provide that isn't already possible in the absence of
> > it.  
> 
> It's not necessary that something be impossible to do any
> other way. The question should be whether this provides for
> a better way to achieve the goals, and this does that.
> If I tried the come up with something that's impossible I
> would expect the usual "you can do that with SELinux policy"
> argument. We know we can do things. We want to have the tools
> to do them better.
> 
> > Also, Ubuntu fully upstreamed all of their changes to AppArmor,
> > would this still suffice to enable stacking of AppArmor or do they
> > rely on hooks that are not handled here?  
> 
> Some amount of merging will likely be required. But that's
> always going to be true with parallel development tracks.
> That's why we have git!
> 
> > Please explain the cost of the change - what do we pay in terms of
> > memory, runtime, or other overheads in order to support this
> > change?  
> 
> Do you have particular benchmarks you want to see?
> When I've supplied numbers in the past they have not
> been remarked on.
> 
> >  
> >>
> >> A new process attribute identifies which security module
> >> information should be reported by SO_PEERSEC and the
> >> /proc/.../attr/current interface. This is provided by
> >> /proc/.../attr/display. Writing the name of the security
> >> module desired to this interface will set which LSM hooks
> >> will be called for this information. The first security
> >> module providing the hooks will be used by default.  
> >
> > Doesn't this effectively undo making the hooks read-only after
> > init, at least for the subset involved?  What are the security
> > implications thereof?  
> 
> Any mechanism, be it a separate set of hooks, a name used to
> do list look ups, or an sophisticated hash scheme will have that
> impact for the processes that use it. This scheme has the best
> performance profile of the mechanisms I experimented with and
> avoids all sorts of special cases.
> 
> >  
> >> The use of integer based security tokens (secids) is
> >> generally (but not completely) replaced by a structure
> >> lsm_export. The lsm_export structure can contain information
> >> for each of the security modules that export information
> >> outside the LSM layer.
> >>
> >> The LSM interfaces that provide "secctx" text strings
> >> have been changed to use a structure "lsm_context"
> >> instead of a pointer/length pair. In some cases the
> >> interfaces used a "char *" pointer and in others a
> >> "void *". This was necessary to ensure that the correct
> >> release mechanism for the text is used. It also makes
> >> many of the interfaces cleaner.
> >>
> >> https://github.com/cschaufler/lsm-stacking.git#stack-5.2-v1-apparmor
> >>
> >> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> >> ---
> >>   drivers/android/binder.c                |  25 ++-
> >>   fs/kernfs/dir.c                         |   6 +-
> >>   fs/kernfs/inode.c                       |  31 ++-
> >>   fs/kernfs/kernfs-internal.h             |   3 +-
> >>   fs/nfs/inode.c                          |  13 +-
> >>   fs/nfs/internal.h                       |   8 +-
> >>   fs/nfs/nfs4proc.c                       |  17 +-
> >>   fs/nfs/nfs4xdr.c                        |  16 +-
> >>   fs/nfsd/nfs4proc.c                      |   8 +-
> >>   fs/nfsd/nfs4xdr.c                       |  14 +-
> >>   fs/nfsd/vfs.c                           |   7 +-
> >>   fs/proc/base.c                          |   1 +
> >>   include/linux/cred.h                    |   3 +-
> >>   include/linux/lsm_hooks.h               |  91 +++++----
> >>   include/linux/nfs4.h                    |   8 +-
> >>   include/linux/security.h                | 133 +++++++++----
> >>   include/net/af_unix.h                   |   2 +-
> >>   include/net/netlabel.h                  |  10 +-
> >>   include/net/scm.h                       |  14 +-
> >>   kernel/audit.c                          |  43 ++--
> >>   kernel/audit.h                          |   9 +-
> >>   kernel/auditfilter.c                    |   6 +-
> >>   kernel/auditsc.c                        |  77 ++++----
> >>   kernel/cred.c                           |  15 +-
> >>   net/ipv4/cipso_ipv4.c                   |  13 +-
> >>   net/ipv4/ip_sockglue.c                  |  12 +-
> >>   net/netfilter/nf_conntrack_netlink.c    |  29 ++-
> >>   net/netfilter/nf_conntrack_standalone.c |  16 +-
> >>   net/netfilter/nfnetlink_queue.c         |  38 ++--
> >>   net/netfilter/nft_meta.c                |  13 +-
> >>   net/netfilter/xt_SECMARK.c              |  14 +-
> >>   net/netlabel/netlabel_kapi.c            |   5 +-
> >>   net/netlabel/netlabel_unlabeled.c       | 101 +++++-----
> >>   net/netlabel/netlabel_unlabeled.h       |   2 +-
> >>   net/netlabel/netlabel_user.c            |  13 +-
> >>   net/netlabel/netlabel_user.h            |   2 +-
> >>   net/unix/af_unix.c                      |   6 +-
> >>   security/apparmor/audit.c               |   4 +-
> >>   security/apparmor/include/audit.h       |   2 +-
> >>   security/apparmor/include/net.h         |   6 +-
> >>   security/apparmor/include/secid.h       |   9 +-
> >>   security/apparmor/lsm.c                 |  64 +++---
> >>   security/apparmor/secid.c               |  42 ++--
> >>   security/integrity/ima/ima.h            |  14 +-
> >>   security/integrity/ima/ima_api.c        |   9 +-
> >>   security/integrity/ima/ima_appraise.c   |   6 +-
> >>   security/integrity/ima/ima_main.c       |  34 ++--
> >>   security/integrity/ima/ima_policy.c     |  19 +-
> >>   security/security.c                     | 338
> >> +++++++++++++++++++++++++++-----
> >> security/selinux/hooks.c                | 259
> >> ++++++++++++------------ security/selinux/include/audit.h
> >> |   5 +- security/selinux/include/objsec.h       |  42 +++-
> >> security/selinux/netlabel.c             |  25 +--
> >> security/selinux/ss/services.c          |  18 +-
> >> security/smack/smack.h                  |  18 ++
> >> security/smack/smack_lsm.c              | 238
> >> +++++++++++----------- security/smack/smack_netfilter.c        |
> >> 8 +- security/smack/smackfs.c                |  12 +- 58 files
> >> changed, 1217 insertions(+), 779 deletions(-) 
> >