Re: [PATCH] security: commoncap: clean up kernel-doc comments

2021-04-15 Thread James Morris
On Sun, 11 Apr 2021, Randy Dunlap wrote:

> Fix kernel-doc notation in commoncap.c.
> 
> Use correct (matching) function name in comments as in code.
> Use correct function argument names in kernel-doc comments.
> Use kernel-doc's "Return:" format for function return values.
> 
> Fixes these kernel-doc warnings:
> 
> ../security/commoncap.c:1206: warning: expecting prototype for 
> cap_task_ioprio(). Prototype was for cap_task_setioprio() instead
> ../security/commoncap.c:1219: warning: expecting prototype for 
> cap_task_ioprio(). Prototype was for cap_task_setnice() instead
> 
> Signed-off-by: Randy Dunlap 
> Cc: Serge Hallyn 
> Cc: James Morris 
> Cc: linux-security-mod...@vger.kernel.org

Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git 
fixes-v5.12

-- 
James Morris




Re: [PATCH v33 00/12] Landlock LSM

2021-04-08 Thread James Morris
I've added this to my tree at:

git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git 
landlock_lsm_v33

and merged that into the next-testing branch which is pulled into Linux 
next.


-- 
James Morris




Re: [PATCH v33 07/12] landlock: Support filesystem access-control

2021-04-08 Thread James Morris
On Wed, 7 Apr 2021, Mickaël Salaün wrote:

> Changes since v31:
> * Gracefully forbid reparenting by returning EXDEV in hook_path_link()
>   and hook_path_rename() (hinted by Al Viro).
> * Replace excessive WARN_ON_ONCE() with unlikely() in
>   hook_path_rename() and use ENOENT instead of EACCES.
> * Improve comment in unmask_layers() (pointed out by Al Viro).  Also use
>   filesystem "topology" instead of "layout", which seems more
>   appropriate.
> * Add access(2) to the documented list of unsupported syscall families.
> * Replace "option" with "flag" in hook_sb_mount() comment.

Good to see these changes.

Al: any further comments now on this patch?

-- 
James Morris



Re: [PATCH] integrity/ima: Add declarations to init_once void arguments.

2021-04-05 Thread James Morris
On Tue, 6 Apr 2021, Jiele Zhao wrote:

> Ping.

Mimi Zohar is the maintainer for this code.

> 
> On 2021/3/23 9:33, Jiele Zhao wrote:
> > init_once is a callback to kmem_cache_create. The parameter
> > type of this function is void *, so it's better to give a
> > explicit cast here.
> >
> > Signed-off-by: Jiele Zhao 
> > ---
> >   security/integrity/iint.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/security/integrity/iint.c b/security/integrity/iint.c
> > index 1d20003243c3..5f3f2de997e1 100644
> > --- a/security/integrity/iint.c
> > +++ b/security/integrity/iint.c
> > @@ -152,7 +152,7 @@ void integrity_inode_free(struct inode *inode)
> >   
> >   static void init_once(void *foo)
> >   {
> > -   struct integrity_iint_cache *iint = foo;
> > +   struct integrity_iint_cache *iint = (struct integrity_iint_cache
> > *)foo;
> >   
> >memset(iint, 0, sizeof(*iint));
> >iint->ima_file_status = INTEGRITY_UNKNOWN;
> 

-- 
James Morris




Re: [PATCH 03/11] security: commoncap: fix -Wstringop-overread warning

2021-03-24 Thread James Morris
On Mon, 22 Mar 2021, Arnd Bergmann wrote:

> From: Arnd Bergmann 
> 
> gcc-11 introdces a harmless warning for cap_inode_getsecurity:
> 
> security/commoncap.c: In function ‘cap_inode_getsecurity’:
> security/commoncap.c:440:33: error: ‘memcpy’ reading 16 bytes from a region 
> of size 0 [-Werror=stringop-overread]
>   440 | memcpy(>data, >data, 
> sizeof(__le32) * 2 * VFS_CAP_U32);
>   | 
> ^~
> 
> The problem here is that tmpbuf is initialized to NULL, so gcc assumes
> it is not accessible unless it gets set by vfs_getxattr_alloc().  This is
> a legitimate warning as far as I can tell, but the code is correct since
> it correctly handles the error when that function fails.
> 
> Add a separate NULL check to tell gcc about it as well.
> 
> Signed-off-by: Arnd Bergmann 

Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git 
fixes-v5.12

-- 
James Morris



Re: [PATCH v30 02/12] landlock: Add ruleset and domain management

2021-03-24 Thread James Morris
On Fri, 19 Mar 2021, Mickaël Salaün wrote:

> 
> >> Cc: Kees Cook 
> >> Signed-off-by: Mickaël Salaün 
> >> Acked-by: Serge Hallyn 
> >> Link: https://lore.kernel.org/r/20210316204252.427806-3-...@digikod.net
> > 
> > (Aside: you appear to be self-adding your Link: tags -- AIUI, this is
> > normally done by whoever pulls your series. I've only seen Link: tags
> > added when needing to refer to something else not included in the
> > series.)
> 
> It is an insurance to not lose history. :)

How will history be lost? The code is in the repo and discussions can 
easily be found by searching for subjects or message IDs.

Is anyone else doing this self linking?

-- 
James Morris



Re: [PATCH v30 00/12] Landlock LSM

2021-03-18 Thread James Morris
I've queued this patchset here:

git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git 
landlock_lsm

and pulled it into next-testing, which will get it coverage in linux-next.

All going well, I'll aim to push this to Linus in the next merge window. 
More review and testing during that time will be helpful.


-- 
James Morris




Re: [PATCH v30 07/12] landlock: Support filesystem access-control

2021-03-18 Thread James Morris


> This commit adds a minimal set of supported filesystem access-control
> which doesn't enable to restrict all file-related actions.

It would be great to get some more review/acks on this patch, particularly 
from VFS/FS folk.


-- 
James Morris




[ANNOUNCE][CFP] Linux Security Summit 2021

2021-02-08 Thread James Morris
==
   ANNOUNCEMENT AND CALL FOR PARTICIPATION

 LINUX SECURITY SUMMIT 2021
 
  27-29 September
  Dublin, Ireland
==

DESCRIPTION
 
Linux Security Summit (LSS) is a technical forum for collaboration between
Linux developers, researchers, and end-users.  Its primary aim is to foster
community efforts in analyzing and solving Linux security challenges.

 The program committee currently seeks proposals for:
 
   * Refereed Presentations:
 45 minutes in length.
 
   * Panel Discussion Topics:
 45 minutes in length.
 
   * Short Topics:
 30 minutes in total, including at least 10 minutes discussion.
 
   * Tutorials
 90 minutes in length.
 
Tutorial sessions should be focused on advanced Linux security defense
topics within areas such as the kernel, compiler, and security-related
libraries.  Priority will be given to tutorials created for this conference,
and those where the presenter a leading subject matter expert on the topic.
 
Topic areas include, but are not limited to:
 
   * Kernel self-protection
   * Access control
   * Cryptography and key management
   * Integrity policy and enforcement
   * Hardware Security
   * IoT and embedded security
   * Virtualization and containers
   * System-specific system hardening
   * Case studies
   * Security tools
   * Security UX
   * Emerging technologies, threats & techniques

  Proposals should be submitted via:
https://events.linuxfoundation.org/linux-security-summit-europe/program/cfp/


** Note that for 2021, the North American and European events are combined into
a single event planned for Dublin, Ireland. **
 

DATES
 
  * CFP close:June 27
  * CFP notifications:July 20
  * Schedule announced:   July 22
  * Event:September 27-29

WHO SHOULD ATTEND
 
We're seeking a diverse range of attendees and welcome participation by
people involved in Linux security development, operations, and research.
 
LSS is a unique global event that provides the opportunity to present and
discuss your work or research with key Linux security community members and
maintainers.  It's also useful for those who wish to keep up with the latest
in Linux security development and to provide input to the development
process.

WEB SITE

https://events.linuxfoundation.org/linux-security-summit-europe/

TWITTER

  For event updates and announcements, follow:

https://twitter.com/LinuxSecSummit
  
#linuxsecuritysummit

PROGRAM COMMITTEE

  The program committee for LSS 2021 is:

    * James Morris, Microsoft
* Serge Hallyn, Cisco
* Paul Moore, Cisco
* Stephen Smalley, NSA
* Elena Reshetova, Intel
* John Johansen, Canonical
* Kees Cook, Google
* Casey Schaufler, Intel
* Mimi Zohar, IBM
* David A. Wheeler, Institute for Defense Analyses

  The program committee may be contacted as a group via email:
lss-pc () lists.linuxfoundation.org



Re: [PATCH v2 1/1] mm/madvise: replace ptrace attach requirement for process_madvise

2021-01-19 Thread James Morris
On Mon, 11 Jan 2021, Suren Baghdasaryan wrote:

> Replace PTRACE_MODE_ATTACH with a combination of PTRACE_MODE_READ
> and CAP_SYS_NICE. PTRACE_MODE_READ to prevent leaking ASLR metadata
> and CAP_SYS_NICE for influencing process performance.


Almost missed these -- please cc the LSM mailing list when modifying 
capabilities or other LSM-related things.

-- 
James Morris




[SECURITY] fix namespaced fscaps when !CONFIG_SECURITY

2020-12-10 Thread James Morris
Please apply this config bugfix from Serge Hallyn.


The following changes since commit bbf5c979011a099af5dc76498918ed7df445635b:

  Linux 5.9 (2020-10-11 14:15:50 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git 
tags/fixes-v5.10a

for you to fetch changes up to ed9b25d1970a4787ac6a39c2091e63b127ecbfc1:

  [SECURITY] fix namespaced fscaps when !CONFIG_SECURITY (2020-12-04 16:24:11 
-0800)


Fix namespaced fscaps when !CONFIG_SECURITY from Serge Hallyn.


Serge Hallyn (1):
  [SECURITY] fix namespaced fscaps when !CONFIG_SECURITY

 include/linux/security.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

commit ed9b25d1970a4787ac6a39c2091e63b127ecbfc1
Author: Serge Hallyn 
Date:   Sun Nov 15 21:55:31 2020 -0600

[SECURITY] fix namespaced fscaps when !CONFIG_SECURITY

Namespaced file capabilities were introduced in 8db6c34f1dbc .
When userspace reads an xattr for a namespaced capability, a
virtualized representation of it is returned if the caller is
in a user namespace owned by the capability's owning rootid.
The function which performs this virtualization was not hooked
up if CONFIG_SECURITY=n.  Therefore in that case the original
xattr was shown instead of the virtualized one.

To test this using libcap-bin (*1),

$ v=$(mktemp)
$ unshare -Ur setcap cap_sys_admin-eip $v
$ unshare -Ur setcap -v cap_sys_admin-eip $v
/tmp/tmp.lSiIFRvt8Y: OK

"setcap -v" verifies the values instead of setting them, and
will check whether the rootid value is set.  Therefore, with
this bug un-fixed, and with CONFIG_SECURITY=n, setcap -v will
fail:

$ v=$(mktemp)
$ unshare -Ur setcap cap_sys_admin=eip $v
$ unshare -Ur setcap -v cap_sys_admin=eip $v
nsowner[got=1000, want=0],/tmp/tmp.HHDiOOl9fY differs in []

Fix this bug by calling cap_inode_getsecurity() in
security_inode_getsecurity() instead of returning
-EOPNOTSUPP, when CONFIG_SECURITY=n.

*1 - note, if libcap is too old for getcap to have the '-n'
option, then use verify-caps instead.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=209689
Cc: Hervé Guillemet 
Acked-by: Casey Schaufler 
Signed-off-by: Serge Hallyn 
Signed-off-by: Andrew G. Morgan 
Signed-off-by: James Morris 

diff --git a/include/linux/security.h b/include/linux/security.h
index 0a0a03b36a3b..2befc0a25eb3 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -864,7 +864,7 @@ static inline int security_inode_killpriv(struct dentry 
*dentry)
 
 static inline int security_inode_getsecurity(struct inode *inode, const char 
*name, void **buffer, bool alloc)
 {
-   return -EOPNOTSUPP;
+   return cap_inode_getsecurity(inode, name, buffer, alloc);
 }
 
 static inline int security_inode_setsecurity(struct inode *inode, const char 
*name, const void *value, size_t size, int flags)

Re: [PATCH v2 04/10] ovl: make ioctl() safe

2020-12-08 Thread James Morris
On Mon, 7 Dec 2020, Miklos Szeredi wrote:

> ovl_ioctl_set_flags() does a capability check using flags, but then the
> real ioctl double-fetches flags and uses potentially different value.
> 
> The "Check the capability before cred override" comment misleading: user
> can skip this check by presenting benign flags first and then overwriting
> them to non-benign flags.

Is this a security bug which should be fixed in stable?

-- 
James Morris




Re: [PATCH v2 01/10] vfs: move cap_convert_nscap() call into vfs_setxattr()

2020-12-08 Thread James Morris
On Mon, 7 Dec 2020, Miklos Szeredi wrote:

> cap_convert_nscap() does permission checking as well as conversion of the
> xattr value conditionally based on fs's user-ns.
> 
> This is needed by overlayfs and probably other layered fs (ecryptfs) and is
> what vfs_foo() is supposed to do anyway.
> 
> Signed-off-by: Miklos Szeredi 


Acked-by: James Morris 


-- 
James Morris




Re: [PATCH] fix namespaced fscaps when !CONFIG_SECURITY

2020-12-04 Thread James Morris
On Fri, 4 Dec 2020, Andrew G. Morgan wrote:

> The correct bug reference for this patch is:
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=209689
> 
> Reviewed-by: Andrew G. Morgan 

Thanks.

Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git 
fixes-5.10
and next-testing


-- 
James Morris




Re: [PATCH] fix namespaced fscaps when !CONFIG_SECURITY

2020-11-30 Thread James Morris
On Sun, 29 Nov 2020, Serge E. Hallyn wrote:

> Hi James,
> 
> would you mind adding this to the security tree?  (You can cherrypick
> from 
> https://git.kernel.org/pub/scm/linux/kernel/git/sergeh/linux.git/commit/?h=2020-11-29/fix-nscaps
>  )

Sure.

> 
> thanks,
> -serge
> 
> On Tue, Nov 17, 2020 at 08:09:59AM -0800, Andrew G. Morgan wrote:
> > Signed-off-by: Andrew G. Morgan 
> > 
> > 
> > On Tue, Nov 17, 2020 at 7:09 AM Serge E. Hallyn  wrote:
> > 
> > > Namespaced file capabilities were introduced in 8db6c34f1dbc .
> > > When userspace reads an xattr for a namespaced capability, a
> > > virtualized representation of it is returned if the caller is
> > > in a user namespace owned by the capability's owning rootid.
> > > The function which performs this virtualization was not hooked
> > > up if CONFIG_SECURITY=n.  Therefore in that case the original
> > > xattr was shown instead of the virtualized one.
> > >
> > > To test this using libcap-bin (*1),
> > >
> > > $ v=$(mktemp)
> > > $ unshare -Ur setcap cap_sys_admin-eip $v
> > > $ unshare -Ur setcap -v cap_sys_admin-eip $v
> > > /tmp/tmp.lSiIFRvt8Y: OK
> > >
> > > "setcap -v" verifies the values instead of setting them, and
> > > will check whether the rootid value is set.  Therefore, with
> > > this bug un-fixed, and with CONFIG_SECURITY=n, setcap -v will
> > > fail:
> > >
> > > $ v=$(mktemp)
> > > $ unshare -Ur setcap cap_sys_admin=eip $v
> > > $ unshare -Ur setcap -v cap_sys_admin=eip $v
> > > nsowner[got=1000, want=0],/tmp/tmp.HHDiOOl9fY differs in []
> > >
> > > Fix this bug by calling cap_inode_getsecurity() in
> > > security_inode_getsecurity() instead of returning
> > > -EOPNOTSUPP, when CONFIG_SECURITY=n.
> > >
> > > *1 - note, if libcap is too old for getcap to have the '-n'
> > > option, then use verify-caps instead.
> > >
> > > Signed-off-by: Serge Hallyn 
> > > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1593431
> > > Cc: Hervé Guillemet 
> > > Cc: Andrew G. Morgan 
> > > Cc: Casey Schaufler 
> > > ---
> > >  include/linux/security.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/include/linux/security.h b/include/linux/security.h
> > > index bc2725491560..39642626a707 100644
> > > --- a/include/linux/security.h
> > > +++ b/include/linux/security.h
> > > @@ -869,7 +869,7 @@ static inline int security_inode_killpriv(struct
> > > dentry *dentry)
> > >
> > >  static inline int security_inode_getsecurity(struct inode *inode, const
> > > char *name, void **buffer, bool alloc)
> > >  {
> > > -   return -EOPNOTSUPP;
> > > +   return cap_inode_getsecurity(inode, name, buffer, alloc);
> > >  }
> > >
> > >  static inline int security_inode_setsecurity(struct inode *inode, const
> > > char *name, const void *value, size_t size, int flags)
> > > --
> > > 2.25.1
> > >
> > >
> 

-- 
James Morris



Re: [PATCH v24 12/12] landlock: Add user and kernel documentation

2020-11-23 Thread James Morris
On Sat, 21 Nov 2020, Jann Horn wrote:

> On Thu, Nov 12, 2020 at 9:52 PM Mickaël Salaün  wrote:
> > This documentation can be built with the Sphinx framework.
> >
> > Cc: James Morris 
> > Cc: Jann Horn 
> > Cc: Kees Cook 
> > Cc: Serge E. Hallyn 
> > Signed-off-by: Mickaël Salaün 
> > Reviewed-by: Vincent Dagonneau 
> 
> Reviewed-by: Jann Horn 
> 

Thanks, Jann!

-- 
James Morris



Re: [PATCH v6 8/8] selinux: measure state and hash of the policy using IMA

2020-11-20 Thread James Morris
On Thu, 19 Nov 2020, Tushar Sugandhi wrote:

> an impact on the security guarantees provided by SELinux. Measuring
> such in-memory data structures through IMA subsystem provides a secure
> way for a remote attestation service to know the state of the system
> and also the runtime changes in the state of the system.

I think we need better clarity on the security model here than just "a 
secure way...".  Secure how and against what threats?

This looks to me like configuration assurance, i.e. you just want to know 
that systems have been configured correctly, not to detect a competent 
attack. Is that correct?



-- 
James Morris




Re: [PATCH] fix namespaced fscaps when !CONFIG_SECURITY

2020-11-19 Thread James Morris
On Tue, 17 Nov 2020, Andrew G. Morgan wrote:

> Signed-off-by: Andrew G. Morgan 

This should be Acked-by or Reviewed-by, unless this is your patch, or it 
came via your tree.


-- 
James Morris




Re: [PATCH] fix namespaced fscaps when !CONFIG_SECURITY

2020-11-19 Thread James Morris
On Tue, 17 Nov 2020, Serge E. Hallyn wrote:

> *1 - note, if libcap is too old for getcap to have the '-n'
> option, then use verify-caps instead.
> 
> Signed-off-by: Serge Hallyn 
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1593431

"Perf fails to compile with python 3.7" 

Wrong bug ID?


-- 
James Morris




Re: [PATCH] fix namespaced fscaps when !CONFIG_SECURITY

2020-11-19 Thread James Morris
[Adding LSM list]

On Tue, 17 Nov 2020, Serge E. Hallyn wrote:

> Namespaced file capabilities were introduced in 8db6c34f1dbc .
> When userspace reads an xattr for a namespaced capability, a
> virtualized representation of it is returned if the caller is
> in a user namespace owned by the capability's owning rootid.
> The function which performs this virtualization was not hooked
> up if CONFIG_SECURITY=n.  Therefore in that case the original
> xattr was shown instead of the virtualized one.
> 
> To test this using libcap-bin (*1),
> 
> $ v=$(mktemp)
> $ unshare -Ur setcap cap_sys_admin-eip $v
> $ unshare -Ur setcap -v cap_sys_admin-eip $v
> /tmp/tmp.lSiIFRvt8Y: OK
> 
> "setcap -v" verifies the values instead of setting them, and
> will check whether the rootid value is set.  Therefore, with
> this bug un-fixed, and with CONFIG_SECURITY=n, setcap -v will
> fail:
> 
> $ v=$(mktemp)
> $ unshare -Ur setcap cap_sys_admin=eip $v
> $ unshare -Ur setcap -v cap_sys_admin=eip $v
> nsowner[got=1000, want=0],/tmp/tmp.HHDiOOl9fY differs in []
> 
> Fix this bug by calling cap_inode_getsecurity() in
> security_inode_getsecurity() instead of returning
> -EOPNOTSUPP, when CONFIG_SECURITY=n.
> 
> *1 - note, if libcap is too old for getcap to have the '-n'
> option, then use verify-caps instead.
> 
> Signed-off-by: Serge Hallyn 
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1593431
> Cc: Hervé Guillemet 
> Cc: Andrew G. Morgan 
> Cc: Casey Schaufler 
> ---
>  include/linux/security.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/security.h b/include/linux/security.h
> index bc2725491560..39642626a707 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -869,7 +869,7 @@ static inline int security_inode_killpriv(struct dentry 
> *dentry)
>  
>  static inline int security_inode_getsecurity(struct inode *inode, const char 
> *name, void **buffer, bool alloc)
>  {
> - return -EOPNOTSUPP;
> + return cap_inode_getsecurity(inode, name, buffer, alloc);
>  }
>  
>  static inline int security_inode_setsecurity(struct inode *inode, const char 
> *name, const void *value, size_t size, int flags)
> 

-- 
James Morris



Re: [PATCH v24 02/12] landlock: Add ruleset and domain management

2020-11-19 Thread James Morris
On Thu, 12 Nov 2020, Mickaël Salaün wrote:

> Cc: James Morris 
> Cc: Jann Horn 
> Cc: Kees Cook 
> Cc: Serge E. Hallyn 
> Signed-off-by: Mickaël Salaün 
> ---
> 
> Changes since v23:
> * Always intersect access rights.  Following the filesystem change
>   logic, make ruleset updates more consistent by always intersecting
>   access rights (boolean AND) instead of combining them (boolean OR) for
>   the same layer.  This defensive approach could also help avoid user
>   space to inadvertently allow multiple access rights for the same
>   object (e.g.  write and execute access on a path hierarchy) instead of
>   dealing with such inconsistency.  This can happen when there is no
>   deduplication of objects (e.g. paths and underlying inodes) whereas
>   they get different access rights with landlock_add_rule(2).
> * Add extra checks to make sure that:
>   - there is always an (allocated) object in each used rules;
>   - when updating a ruleset with a new rule (i.e. not merging two
> rulesets), the ruleset doesn't contain multiple layers.
> * Hide merge parameter from the public landlock_insert_rule() API.  This
>   helps avoid misuse of this function.
> * Replace a remaining hardcoded 1 with SINGLE_DEPTH_NESTING.

Jann: any chance you could review this patch again given the changes 
above?

Thanks.


-- 
James Morris



Re: [PATCH v1 2/9] landlock: Cosmetic fixes for filesystem management

2020-11-19 Thread James Morris
On Wed, 11 Nov 2020, Mickaël Salaün wrote:

> Improve comments and make get_inode_object() more readable.  The kfree()
> call is correct but we should mimimize as much as possible lock windows.
> 
> Cc: James Morris 
> Cc: Jann Horn 
> Cc: Serge E. Hallyn 
> Signed-off-by: Mickaël Salaün 


Reviewed-by: James Morris 

> ---
>  security/landlock/fs.c | 24 
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index b67c821bb40b..33fc7ae17c7f 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -85,8 +85,8 @@ static struct landlock_object *get_inode_object(struct 
> inode *const inode)
>   return object;
>   }
>   /*
> -  * We're racing with release_inode(), the object is going away.
> -  * Wait for release_inode(), then retry.
> +  * We are racing with release_inode(), the object is going
> +  * away.  Wait for release_inode(), then retry.
>*/
>   spin_lock(>lock);
>   spin_unlock(>lock);
> @@ -107,21 +107,21 @@ static struct landlock_object *get_inode_object(struct 
> inode *const inode)
>   lockdep_is_held(>i_lock));
>   if (unlikely(object)) {
>   /* Someone else just created the object, bail out and retry. */
> - kfree(new_object);
>   spin_unlock(>i_lock);
> + kfree(new_object);
>  
>   rcu_read_lock();
>   goto retry;
> - } else {
> - rcu_assign_pointer(inode_sec->object, new_object);
> - /*
> -  * @inode will be released by hook_sb_delete() on its
> -  * superblock shutdown.
> -  */
> - ihold(inode);
> - spin_unlock(>i_lock);
> - return new_object;
>   }
> +
> + rcu_assign_pointer(inode_sec->object, new_object);
> + /*
> +  * @inode will be released by hook_sb_delete() on its superblock
> +  * shutdown.
> +  */
> + ihold(inode);
> + spin_unlock(>i_lock);
> + return new_object;
>  }
>  
>  /* All access rights which can be tied to files. */
> 

-- 
James Morris



Re: [PATCH v24 00/12] Landlock LSM

2020-11-16 Thread James Morris
On Thu, 12 Nov 2020, Mickaël Salaün wrote:

> Hi,
> 
> This patch series simplifies the code, makes stacked access-control more
> consistent (from the user point of view), properly handles memory
> allocation errors, and adds more tests (covering layered ruleset corner
> cases).  Most of these changes were sent as a separate patch series:
> https://lore.kernel.org/lkml/2020213442.434639-1-...@digikod.net/
> 
> James and Jann, could you please take a look at the main changes
> (patches 2, 7 and 8)?

We definitely need more review on these changes that came in. I'll drop 
the previous patchset from my tree and wait until the latest code is fully 
reviewed.

Fundamental locking issues and similar should be worked out before 
submitting for mainline merge.

-- 
James Morris



Re: [PATCH v1 0/9] Landlock fixes

2020-11-11 Thread James Morris
On Wed, 11 Nov 2020, Mickaël Salaün wrote:

> Hi,
> 
> This patch series fixes some issues and makes the Landlock filesystem
> access-control more consistent and deterministic when stacking multiple
> rulesets.  This is checked by current and new tests.  I also extended
> documentation and example to help users.
> 
> This series can be applied on top of
> https://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git/log/?h=landlock_lsm

Actually, given the number of fixes here, please respin so we get a 
cleaner initial PR for Linus.

> 
> Regards,
> 
> Mickaël Salaün (9):
>   landlock: Fix memory allocation error handling
>   landlock: Cosmetic fixes for filesystem management
>   landlock: Enforce deterministic interleaved path rules
>   landlock: Always intersect access rights
>   landlock: Add extra checks when inserting a rule
>   selftests/landlock: Extend layout1.inherit_superset
>   landlock: Clean up get_ruleset_from_fd()
>   landlock: Add help to enable Landlock as a stacked LSM
>   landlock: Extend documentation about limitations
> 
>  Documentation/userspace-api/landlock.rst   |  17 +++
>  samples/landlock/sandboxer.c   |  21 +++-
>  security/landlock/Kconfig  |   4 +-
>  security/landlock/fs.c |  67 +-
>  security/landlock/object.c |   5 +-
>  security/landlock/ruleset.c|  34 ++---
>  security/landlock/syscall.c|  24 ++--
>  tools/testing/selftests/landlock/fs_test.c | 140 +++--
>  8 files changed, 239 insertions(+), 73 deletions(-)
> 
> 
> base-commit: 96b3198c4025c11347651700b77e45a686d78553
> 

-- 
James Morris



Re: [PATCH v22 16/23] LSM: security_secid_to_secctx in netlink netfilter

2020-11-10 Thread James Morris
On Tue, 10 Nov 2020, Pablo Neira Ayuso wrote:

> Hi Casey,
> 
> On Wed, Nov 04, 2020 at 04:49:17PM -0800, Casey Schaufler wrote:
> > Change netlink netfilter interfaces to use lsmcontext
> > pointers, and remove scaffolding.
> > 
> > Reviewed-by: Kees Cook 
> > Reviewed-by: John Johansen 
> > Acked-by: Stephen Smalley 
> > Signed-off-by: Casey Schaufler 
> > Cc: net...@vger.kernel.org
> > Cc: netfilter-de...@vger.kernel.org
> 
> You can carry this tag in your follow up patches.
> 
> Acked-by: Pablo Neira Ayuso 

Thanks for the review!

> 
> Thanks.
> 
> > ---
> >  net/netfilter/nfnetlink_queue.c | 37 +
> >  1 file changed, 14 insertions(+), 23 deletions(-)
> > 
> > diff --git a/net/netfilter/nfnetlink_queue.c 
> > b/net/netfilter/nfnetlink_queue.c
> > index 84be5a49a157..0d8b83d84422 100644
> > --- a/net/netfilter/nfnetlink_queue.c
> > +++ b/net/netfilter/nfnetlink_queue.c
> > @@ -301,15 +301,13 @@ static int nfqnl_put_sk_uidgid(struct sk_buff *skb, 
> > struct sock *sk)
> > return -1;
> >  }
> >  
> > -static u32 nfqnl_get_sk_secctx(struct sk_buff *skb, char **secdata)
> > +static void nfqnl_get_sk_secctx(struct sk_buff *skb, struct lsmcontext 
> > *context)
> >  {
> > -   u32 seclen = 0;
> >  #if IS_ENABLED(CONFIG_NETWORK_SECMARK)
> > struct lsmblob blob;
> > -   struct lsmcontext context = { };
> >  
> > if (!skb || !sk_fullsock(skb->sk))
> > -   return 0;
> > +   return;
> >  
> > read_lock_bh(>sk->sk_callback_lock);
> >  
> > @@ -318,14 +316,12 @@ static u32 nfqnl_get_sk_secctx(struct sk_buff *skb, 
> > char **secdata)
> >  * blob. security_secid_to_secctx() will know which security
> >  * module to use to create the secctx.  */
> > lsmblob_init(, skb->secmark);
> > -   security_secid_to_secctx(, );
> > -   *secdata = context.context;
> > +   security_secid_to_secctx(, context);
> > }
> >  
> > read_unlock_bh(>sk->sk_callback_lock);
> > -   seclen = context.len;
> >  #endif
> > -   return seclen;
> > +   return;
> >  }
> >  
> >  static u32 nfqnl_get_bridge_size(struct nf_queue_entry *entry)
> > @@ -398,12 +394,10 @@ nfqnl_build_packet_message(struct net *net, struct 
> > nfqnl_instance *queue,
> > struct net_device *indev;
> > struct net_device *outdev;
> > struct nf_conn *ct = NULL;
> > +   struct lsmcontext context = { };
> > enum ip_conntrack_info ctinfo;
> > struct nfnl_ct_hook *nfnl_ct;
> > bool csum_verify;
> > -   struct lsmcontext scaff; /* scaffolding */
> > -   char *secdata = NULL;
> > -   u32 seclen = 0;
> >  
> > size = nlmsg_total_size(sizeof(struct nfgenmsg))
> > + nla_total_size(sizeof(struct nfqnl_msg_packet_hdr))
> > @@ -469,9 +463,9 @@ nfqnl_build_packet_message(struct net *net, struct 
> > nfqnl_instance *queue,
> > }
> >  
> > if ((queue->flags & NFQA_CFG_F_SECCTX) && entskb->sk) {
> > -   seclen = nfqnl_get_sk_secctx(entskb, );
> > -   if (seclen)
> > -   size += nla_total_size(seclen);
> > +   nfqnl_get_sk_secctx(entskb, );
> > +   if (context.len)
> > +   size += nla_total_size(context.len);
> > }
> >  
> > skb = alloc_skb(size, GFP_ATOMIC);
> > @@ -604,7 +598,8 @@ nfqnl_build_packet_message(struct net *net, struct 
> > nfqnl_instance *queue,
> > nfqnl_put_sk_uidgid(skb, entskb->sk) < 0)
> > goto nla_put_failure;
> >  
> > -   if (seclen && nla_put(skb, NFQA_SECCTX, seclen, secdata))
> > +   if (context.len &&
> > +   nla_put(skb, NFQA_SECCTX, context.len, context.context))
> > goto nla_put_failure;
> >  
> > if (ct && nfnl_ct->build(skb, ct, ctinfo, NFQA_CT, NFQA_CT_INFO) < 0)
> > @@ -632,10 +627,8 @@ nfqnl_build_packet_message(struct net *net, struct 
> > nfqnl_instance *queue,
> > }
> >  
> > nlh->nlmsg_len = skb->len;
> > -   if (seclen) {
> > -   lsmcontext_init(, secdata, seclen, 0);
> > -   security_release_secctx();
> > -   }
> > +   if (context.len)
> > +   security_release_secctx();
> > return skb;
> >  
> >  nla_put_failure:
> > @@ -643,10 +636,8 @@ nfqnl_build_packet_message(struct net *net, struct 
> > nfqnl_instance *queue,
> > kfree_skb(skb);
> > net_err_ratelimited("nf_queue: error creating packet message\n");
> >  nlmsg_failure:
> > -   if (seclen) {
> > -   lsmcontext_init(, secdata, seclen, 0);
> > -   security_release_secctx();
> > -   }
> > +   if (context.len)
> > +   security_release_secctx();
> > return NULL;
> >  }
> >  
> > -- 
> > 2.24.1
> > 
> 

-- 
James Morris




Re: [PATCH v22 06/23] LSM: Use lsmblob in security_secid_to_secctx

2020-11-09 Thread James Morris
On Wed, 4 Nov 2020, Casey Schaufler wrote:

> Change security_secid_to_secctx() to take a lsmblob as input
> instead of a u32 secid. It will then call the LSM hooks
> using the lsmblob element allocated for that module. The
> callers have been updated as well. This allows for the
> possibility that more than one module may be called upon
> to translate a secid to a string, as can occur in the
> audit code.
> 
> Signed-off-by: Casey Schaufler 
> Cc: net...@vger.kernel.org
> Cc: linux-au...@redhat.com

Ditto with this, + audit. Also, you should put primary maintainers on the 
To: line or they may miss the email.

-- 
James Morris




Re: [PATCH v22 05/23] LSM: Use lsmblob in security_secctx_to_secid

2020-11-09 Thread James Morris
On Wed, 4 Nov 2020, Casey Schaufler wrote:

> Change the security_secctx_to_secid interface to use a lsmblob
> structure in place of the single u32 secid in support of
> module stacking. Change its callers to do the same.
> 
> The security module hook is unchanged, still passing back a secid.
> The infrastructure passes the correct entry from the lsmblob.
> 
> Signed-off-by: Casey Schaufler 
> Cc: net...@vger.kernel.org

You probably need to include Netfilter maintainers specifically for this 
(added them + the Netfilter list).

This also needs signoffs from LSM owners.

-- 
James Morris




Re: [PATCH v23 00/12] Landlock LSM

2020-11-09 Thread James Morris
On Tue, 3 Nov 2020, Mickaël Salaün wrote:

> Hi,
> 
> Can you please consider to merge this into the tree?
> 

I've added this to my tree:
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git 
landlock_lsm

and merged into next-testing (which is pulled into linux-next).


Please make any further changes against the branch in my tree.


-- 
James Morris



Re: [PATCH v3 0/2] security: add fault injection to LSM hooks

2020-11-09 Thread James Morris
On Mon, 9 Nov 2020, Aleksandr Nogikh wrote:

> On Thu, 29 Oct 2020 at 21:35, Aleksandr Nogikh
>  wrote:
> >
> > From: Aleksandr Nogikh 
> >
> > Fault injection 
> > capabilities[Documentation/fault-injection/fault-injection.rst]
> > facilitate testing of the stability of the Linux kernel by providing
> > means to force a number of kernel interfaces to return error
> > codes. This patch series proposes adding such fault injection
> > capability into LSM hooks.
> >
> > The intent is to make it possible to test whether the existing kernel
> > code properly handles negative return values of LSM hooks. Syzbot
> > [https://github.com/google/syzkaller/blob/master/docs/syzbot.md] will
> > automatically do that with the aid of instrumentation tools once these
> > changes are merged.
> > [...]
> 
> What tree should these changes go to?
> 

Mine, but more signoffs/acks are required.

> Is there anyone else who is not on the recipient list but still might
> be interested in the series?
> 

-- 
James Morris




Re: [PATCH v22 05/12] LSM: Infrastructure management of the superblock

2020-10-28 Thread James Morris
On Tue, 27 Oct 2020, Mickaël Salaün wrote:

> From: Casey Schaufler 
> 
> Move management of the superblock->sb_security blob out of the
> individual security modules and into the security infrastructure.
> Instead of allocating the blobs from within the modules, the modules
> tell the infrastructure how much space is required, and the space is
> allocated there.
> 
> Cc: Kees Cook 
> Cc: John Johansen 
> Signed-off-by: Casey Schaufler 
> Signed-off-by: Mickaël Salaün 
> Reviewed-by: Stephen Smalley 

It would be good to see review from JJ here.

-- 
James Morris



Re: [PATCH v22 06/12] fs,security: Add sb_delete hook

2020-10-28 Thread James Morris
On Tue, 27 Oct 2020, Mickaël Salaün wrote:

> From: Mickaël Salaün 
> 
> The sb_delete security hook is called when shutting down a superblock,
> which may be useful to release kernel objects tied to the superblock's
> lifetime (e.g. inodes).
> 
> This new hook is needed by Landlock to release (ephemerally) tagged
> struct inodes.  This comes from the unprivileged nature of Landlock
> described in the next commit.
> 
> Cc: Al Viro 
> Cc: James Morris 
> Cc: Jann Horn 
> Cc: Kees Cook 
> Cc: Serge E. Hallyn 
> Signed-off-by: Mickaël Salaün 

Al, Kees, JJ et al, any objections?

-- 
James Morris



Re: [PATCH v22 09/12] arch: Wire up Landlock syscalls

2020-10-28 Thread James Morris
On Tue, 27 Oct 2020, Mickaël Salaün wrote:

> From: Mickaël Salaün 
> 
> Wire up the following system calls for all architectures:
> * landlock_create_ruleset(2)
> * landlock_add_rule(2)
> * landlock_enforce_ruleset_current(2)
> 
> Cc: Arnd Bergmann 
> Cc: James Morris 
> Cc: Jann Horn 
> Cc: Kees Cook 
> Cc: Serge E. Hallyn 
> Signed-off-by: Mickaël Salaün 

Acks from arch maintainers here would be appreciated.

-- 
James Morris



Re: [PATCH] vsock: use ns_capable_noaudit() on socket create

2020-10-26 Thread James Morris
On Fri, 23 Oct 2020, Jeff Vander Stoep wrote:

> During __vsock_create() CAP_NET_ADMIN is used to determine if the
> vsock_sock->trusted should be set to true. This value is used later
> for determing if a remote connection should be allowed to connect
> to a restricted VM. Unfortunately, if the caller doesn't have
> CAP_NET_ADMIN, an audit message such as an selinux denial is
> generated even if the caller does not want a trusted socket.
> 
> Logging errors on success is confusing. To avoid this, switch the
> capable(CAP_NET_ADMIN) check to the noaudit version.
> 
> Reported-by: Roman Kiryanov 
> https://android-review.googlesource.com/c/device/generic/goldfish/+/1468545/
> Signed-off-by: Jeff Vander Stoep 


Reviewed-by: James Morris 


-- 
James Morris




Re: [PATCH] security: remove unneeded break

2020-10-19 Thread James Morris
On Mon, 19 Oct 2020, t...@redhat.com wrote:

> From: Tom Rix 
> 
> A break is not needed if it is preceded by a return
> 
> Signed-off-by: Tom Rix 


Acked-by: James Morris 



-- 
James Morris




Re: [PATCH v21 07/12] landlock: Support filesystem access-control

2020-10-14 Thread James Morris
On Wed, 14 Oct 2020, Mickaël Salaün wrote:

> 
> On 14/10/2020 20:52, Mickaël Salaün wrote:
> > 
> > On 14/10/2020 20:07, James Morris wrote:
> >> On Thu, 8 Oct 2020, Mickaël Salaün wrote:
> >>
> >>> +config ARCH_EPHEMERAL_STATES
> >>> + def_bool n
> >>> + help
> >>> +   An arch should select this symbol if it does not keep an internal 
> >>> kernel
> >>> +   state for kernel objects such as inodes, but instead relies on 
> >>> something
> >>> +   else (e.g. the host kernel for an UML kernel).
> >>> +
> >>
> >> This is used to disable Landlock for UML, correct?
> > 
> > Yes
> > 
> >> I wonder if it could be 
> >> more specific: "ephemeral states" is a very broad term.
> >>
> >> How about something like ARCH_OWN_INODES ?
> > 
> > Sounds good. We may need add new ones (e.g. for network socket, UID,
> > etc.) in the future though.
> > 
> 
> Because UML is the exception here, it would be more convenient to keep
> the inverted semantic. What about ARCH_NO_OWN_INODES or
> ARCH_EPHEMERAL_INODES?

The latter seems good.

-- 
James Morris



Re: [PATCH v21 07/12] landlock: Support filesystem access-control

2020-10-14 Thread James Morris
On Thu, 8 Oct 2020, Mickaël Salaün wrote:

> +config ARCH_EPHEMERAL_STATES
> + def_bool n
> + help
> +   An arch should select this symbol if it does not keep an internal 
> kernel
> +   state for kernel objects such as inodes, but instead relies on 
> something
> +   else (e.g. the host kernel for an UML kernel).
> +

This is used to disable Landlock for UML, correct? I wonder if it could be 
more specific: "ephemeral states" is a very broad term.

How about something like ARCH_OWN_INODES ?


-- 
James Morris



Re: [PATCH v9 0/3] SELinux support for anonymous inodes and UFFD

2020-10-08 Thread James Morris
On Wed, 7 Oct 2020, Lokesh Gidra wrote:

>  Is there anything else that needs to be done before merging this
> patch series? I urge the reviewers to please take a look.
> 

It looks generally fine to me from a security POV, we really need some 
feedback from VFS folk.


-- 
James Morris




Re: [PATCH v5 11/16] LSM: Add "contents" flag to kernel_read_file hook

2020-10-06 Thread James Morris
On Fri, 2 Oct 2020, Kees Cook wrote:

> Signed-off-by: Kees Cook 
> Reviewed-by: Mimi Zohar 


Reviewed-by: James Morris 


-- 
James Morris




Re: [PATCH v5 10/16] module: Call security_kernel_post_load_data()

2020-10-06 Thread James Morris
On Fri, 2 Oct 2020, Kees Cook wrote:

> Now that there is an API for checking loaded contents for modules
> loaded without a file, call into the LSM hooks.
> 
> Signed-off-by: Kees Cook 
> Reviewed-by: KP Singh 
> Acked-by: Jessica Yu 


Reviewed-by: James Morris 


-- 
James Morris




Re: [PATCH v5 04/16] fs/kernel_read_file: Split into separate source file

2020-10-06 Thread James Morris
On Fri, 2 Oct 2020, Kees Cook wrote:

> These routines are used in places outside of exec(2), so in preparation
> for refactoring them, move them into a separate source file,
> fs/kernel_read_file.c.
> 
> Signed-off-by: Kees Cook 
> Reviewed-by: Mimi Zohar 
> Reviewed-by: Luis Chamberlain 
> Acked-by: Scott Branden 


Reviewed-by: James Morris 


-- 
James Morris




Re: [PATCH v5 01/16] fs/kernel_read_file: Remove FIRMWARE_PREALLOC_BUFFER enum

2020-10-06 Thread James Morris
On Fri, 2 Oct 2020, Kees Cook wrote:

> FIRMWARE_PREALLOC_BUFFER is a "how", not a "what", and confuses the LSMs
> that are interested in filtering between types of things. The "how"
> should be an internal detail made uninteresting to the LSMs.
> 
> Fixes: a098ecd2fa7d ("firmware: support loading into a pre-allocated buffer")
> Fixes: fd90bc559bfb ("ima: based on policy verify firmware signatures 
> (pre-allocated buffer)")
> Fixes: 4f0496d8ffa3 ("ima: based on policy warn about loading firmware 
> (pre-allocated buffer)")
> Signed-off-by: Kees Cook 
> Reviewed-by: Mimi Zohar 
> Reviewed-by: Luis Chamberlain 
> Acked-by: Scott Branden 
> Cc: sta...@vger.kernel.org


Reviewed-by: James Morris 


-- 
James Morris




Re: [PATCH v2 0/4] [RFC] Implement Trampoline File Descriptor

2020-09-23 Thread James Morris
On Wed, 23 Sep 2020, Pavel Machek wrote:

> This is not first crazy patch from your company. Perhaps you should
> have a person with strong Unix/Linux experience performing "straight
> face test" on outgoing patches?

Just for the record: the author of the code has 30+ years experience in 
SunOS, Solaris, Unixware, Realtime, SVR4, and Linux.


-- 
James Morris




[GIT PULL] security: device_cgroup RCU warning fix

2020-09-15 Thread James Morris
This was posted a while back and been baking in -next for a while, please 
consider for 5.9.


The following changes since commit bcf876870b95592b52519ed4aafcf9d95999bc9c:

  Linux 5.8 (2020-08-02 14:21:45 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git 
tags/fixes-v5.9a

for you to fetch changes up to bc62d68e2a0a69fcdcf28aca8edb01abf306b698:

  device_cgroup: Fix RCU list debugging warning (2020-08-20 11:25:03 -0700)


device_cgroup RCU warning fix from Amol Grover 



Amol Grover (1):
  device_cgroup: Fix RCU list debugging warning

 security/device_cgroup.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)


commit bc62d68e2a0a69fcdcf28aca8edb01abf306b698
Author: Amol Grover 
Date:   Mon Apr 6 16:29:50 2020 +0530

device_cgroup: Fix RCU list debugging warning

exceptions may be traversed using list_for_each_entry_rcu()
outside of an RCU read side critical section BUT under the
protection of decgroup_mutex. Hence add the corresponding
lockdep expression to fix the following false-positive
warning:

[2.304417] =
[2.304418] WARNING: suspicious RCU usage
[2.304420] 5.5.4-stable #17 Tainted: GE
[2.304422] -
[2.304424] security/device_cgroup.c:355 RCU-list traversed in 
non-reader section!!

Signed-off-by: Amol Grover 
Signed-off-by: James Morris 

diff --git a/security/device_cgroup.c b/security/device_cgroup.c
index 43ab0ad45c1b..04375df52fc9 100644
--- a/security/device_cgroup.c
+++ b/security/device_cgroup.c
@@ -354,7 +354,8 @@ static bool match_exception_partial(struct list_head 
*exceptions, short type,
 {
struct dev_exception_item *ex;
 
-   list_for_each_entry_rcu(ex, exceptions, list) {
+   list_for_each_entry_rcu(ex, exceptions, list,
+   lockdep_is_held(_mutex)) {
if ((type & DEVCG_DEV_BLOCK) && !(ex->type & DEVCG_DEV_BLOCK))
continue;
if ((type & DEVCG_DEV_CHAR) && !(ex->type & DEVCG_DEV_CHAR))


Re: [RFC PATCH v9 0/3] Add introspect_access(2) (was O_MAYEXEC)

2020-09-11 Thread James Morris
On Thu, 10 Sep 2020, Matthew Wilcox wrote:

> On Thu, Sep 10, 2020 at 08:38:21PM +0200, Mickaël Salaün wrote:
> > There is also the use case of noexec mounts and file permissions. From
> > user space point of view, it doesn't matter which kernel component is in
> > charge of defining the policy. The syscall should then not be tied with
> > a verification/integrity/signature/appraisal vocabulary, but simply an
> > access control one.
> 
> permission()?
> 

The caller is not asking the kernel to grant permission, it's asking 
"SHOULD I access this file?"

The caller doesn't know, for example, if the script file it's about to 
execute has been signed, or if it's from a noexec mount. It's asking the 
kernel, which does know. (Note that this could also be extended to reading 
configuration files).

How about: should_faccessat ?

-- 
James Morris


Re: [RFC PATCH v8 0/3] Add support for AT_INTERPRETED (was O_MAYEXEC)

2020-09-11 Thread James Morris
On Wed, 9 Sep 2020, Al Viro wrote:

> On Wed, Sep 09, 2020 at 09:19:11AM +0200, Mickaël Salaün wrote:
> > 
> > On 08/09/2020 20:50, Al Viro wrote:
> > > On Tue, Sep 08, 2020 at 09:59:53AM +0200, Mickaël Salaün wrote:
> > >> Hi,
> > >>
> > >> This height patch series rework the previous O_MAYEXEC series by not
> > >> adding a new flag to openat2(2) but to faccessat2(2) instead.  As
> > >> suggested, this enables to perform the access check on a file descriptor
> > >> instead of on a file path (while opening it).  This may require two
> > >> checks (one on open and then with faccessat2) but it is a more generic
> > >> approach [8].
> > > 
> > > Again, why is that folded into lookup/open/whatnot, rather than being
> > > an operation applied to a file (e.g. O_PATH one)?
> > > 
> > 
> > I don't understand your question. AT_INTERPRETED can and should be used
> > with AT_EMPTY_PATH. The two checks I wrote about was for IMA.
> 
> Once more, with feeling: don't hide that behind existing syscalls.
> If you want to tell LSM have a look at given fs object in a special
> way, *add* *a* *new* *system* *call* *for* *doing* *just* *that*.

It's not just for LSM, though, and it has identical semantics from the 
caller's POV as faccessat().



-- 
James Morris



Re: [RESEND][RFC PATCH 0/6] Fork brute force attack mitigation (fbfam)

2020-09-11 Thread James Morris
On Thu, 10 Sep 2020, Kees Cook wrote:

> [kees: re-sending this series on behalf of John Wood 
>  also visible at https://github.com/johwood/linux fbfam]
> 
> From: John Wood 

Why are you resending this? The author of the code needs to be able to 
send and receive emails directly as part of development and maintenance.

-- 
James Morris




Re: [RFC] security: replace indirect calls with static calls

2020-08-20 Thread James Morris
On Thu, 20 Aug 2020, Brendan Jackman wrote:

> With this implementation, any overhead of the indirect call in the LSM
> framework is completely mitigated (performance results: [7]). This
> facilitates the adoption of "bpf" LSM on production machines and also
> benefits all other LSMs.

This looks like a potentially useful improvement, although I wonder if it 
would be overshadowed by an LSM hook doing real work.

Do you have any more benchmarking beyond eventfd_write() ?



> 
> [1]: https://lwn.net/ml/linux-kernel/20200710133831.943894...@infradead.org/
> [2]: https://lwn.net/Articles/798157/
> [3] measurements: 
> https://gist.githubusercontent.com/PaulRenauld/fe3ee7b51121556e03c181432c8b3dd5/raw/62437b1416829ca0e8a0ed9101530bc90fd42d69/lsm-performance.png
> protocol: 
> https://gist.github.com/PaulRenauld/fe3ee7b51121556e03c181432c8b3dd5#file-measurement-protocol-md
> [4]: https://lwn.net/Articles/813261/
> [5]: git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git 
> x86/static_call
> [6]: https://lwn.net/ml/linux-kernel/20200710133831.943894...@infradead.org/#t
> [7]: 
> https://gist.githubusercontent.com/PaulRenauld/fe3ee7b51121556e03c181432c8b3dd5/raw/00e414b73e0c38c2eae8f05d5363a745179ba285/faster-lsm-results.png
> 
> Cc: Alexei Starovoitov 
> Cc: Daniel Borkmann 
> Cc: James Morris 
> Cc: p...@google.com
> Cc: ja...@google.com
> Cc: pet...@infradead.org
> Cc: rafael.j.wyso...@intel.com
> Cc: keesc...@chromium.org
> Cc: thgar...@chromium.org
> Cc: kpsi...@google.com
> Cc: paul.renauld.e...@gmail.com
> 
> Signed-off-by: Paul Renauld 
> Signed-off-by: KP Singh 
> Signed-off-by: Brendan Jackman 
> ---
>  include/linux/lsm_hooks.h   |   1 +
>  include/linux/lsm_static_call.h | 134 
>  security/security.c | 217 
>  3 files changed, 331 insertions(+), 21 deletions(-)
>  create mode 100644 include/linux/lsm_static_call.h
> 
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 95b7c1d32062..d11e116b588e 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1524,6 +1524,7 @@ union security_list_options {
>   #define LSM_HOOK(RET, DEFAULT, NAME, ...) RET (*NAME)(__VA_ARGS__);
>   #include "lsm_hook_defs.h"
>   #undef LSM_HOOK
> + void *generic_func;
>  };
>  
>  struct security_hook_heads {
> diff --git a/include/linux/lsm_static_call.h b/include/linux/lsm_static_call.h
> new file mode 100644
> index ..f5f5698292e0
> --- /dev/null
> +++ b/include/linux/lsm_static_call.h
> @@ -0,0 +1,134 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +/*
> + * Copyright (C) 2020 Google LLC.
> + */
> +
> +#ifndef __LINUX_LSM_STATIC_CALL_H
> +#define __LINUX_LSM_STATIC_CALL_H
> +
> +/*
> + * Static slots are used in security/security.c to avoid costly
> + * indirect calls by replacing them with static calls.
> + * The number of static calls for each LSM hook is fixed.
> + */
> +#define SECURITY_STATIC_SLOT_COUNT 11
> +
> +/*
> + * Identifier for the LSM static slots.
> + * HOOK is an LSM hook as defined in linux/lsm_hookdefs.h
> + * IDX is the index of the slot. 0 <= NUM < SECURITY_STATIC_SLOT_COUNT
> + */
> +#define STATIC_SLOT(HOOK, IDX) security_static_slot_##HOOK##_##IDX
> +
> +/*
> + * Call the macro M for each LSM hook slot.
> + * M should take as first argument the index and then
> + * the same __VA_ARGS__
> + * Essentially, this will expand to:
> + *   M(0, ...)
> + *   M(1, ...)
> + *   M(2, ...)
> + *   ...
> + * Note that no trailing semicolon is placed so M should be defined
> + * accordingly.
> + * This adapts to a change to SECURITY_STATIC_SLOT_COUNT.
> + */
> +#define SECURITY_FOREACH_STATIC_SLOT(M, ...) \
> + UNROLL_MACRO_LOOP(SECURITY_STATIC_SLOT_COUNT, M, __VA_ARGS__)
> +
> +/*
> + * Intermediate macros to expand SECURITY_STATIC_SLOT_COUNT
> + */
> +#define UNROLL_MACRO_LOOP(N, MACRO, ...) \
> + _UNROLL_MACRO_LOOP(N, MACRO, __VA_ARGS__)
> +
> +#define _UNROLL_MACRO_LOOP(N, MACRO, ...)\
> + __UNROLL_MACRO_LOOP(N, MACRO, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP(N, MACRO, ...)   \
> + __UNROLL_MACRO_LOOP_##N(MACRO, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_0(MACRO, ...)
> +
> +#define __UNROLL_MACRO_LOOP_1(MACRO, ...)\
> + __UNROLL_MACRO_LOOP_0(MACRO, __VA_ARGS__)   \
> + MACRO(0, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_2(MACRO, ...)\
> + __UNROLL_MACRO_LOOP_1(MACRO, __VA_ARGS__)   \
> + MACRO(1, __VA_ARGS__)
> +
> +#define __UNROLL_

Re: [PATCH v6 0/3] SELinux support for anonymous inodes and UFFD

2020-08-20 Thread James Morris
On Fri, 7 Aug 2020, Lokesh Gidra wrote:

> Userfaultfd in unprivileged contexts could be potentially very
> useful. We'd like to harden userfaultfd to make such unprivileged use
> less risky. This patch series allows SELinux to manage userfaultfd
> file descriptors and in the future, other kinds of
> anonymous-inode-based file descriptor.  SELinux policy authors can
> apply policy types to anonymous inodes by providing name-based
> transition rules keyed off the anonymous inode internal name (
> "[userfaultfd]" in the case of userfaultfd(2) file descriptors) and
> applying policy to the new SIDs thus produced.

Can you expand more on why this would be useful, e.g. use-cases?


-- 
James Morris




Re: [PATCH RESEND] device_cgroup: Fix RCU list debugging warning

2020-08-20 Thread James Morris
On Mon, 17 Aug 2020, Stephen Rothwell wrote:

> > > mainline, so it can go up any tree.  I can take it if no one else will,
> > > but it might be better going in via the security tree.
> > 
> > James, do you mind pulling it in?
> 
> I am still carrying this patch.  Has it been superceded, or is it still
> necessary?

It appears to be necessary.

Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git 
fixes-v5.9

-- 
James Morris




Re: [dm-devel] [RFC PATCH v5 00/11] Integrity Policy Enforcement LSM (IPE)

2020-08-11 Thread James Morris
On Sat, 8 Aug 2020, Chuck Lever wrote:

> My interest is in code integrity enforcement for executables stored
> in NFS files.
> 
> My struggle with IPE is that due to its dependence on dm-verity, it
> does not seem to able to protect content that is stored separately
> from its execution environment and accessed via a file access
> protocol (FUSE, SMB, NFS, etc).

It's not dependent on DM-Verity, that's just one possible integrity 
verification mechanism, and one of two supported in this initial 
version. The other is 'boot_verified' for a verified or otherwise trusted 
rootfs. Future versions will support FS-Verity, at least.

IPE was designed to be extensible in this way, with a strong separation of 
mechanism and policy.

Whatever is implemented for NFS should be able to plug in to IPE pretty 
easily.


-- 
James Morris




Re: [dm-devel] [RFC PATCH v5 00/11] Integrity Policy Enforcement LSM (IPE)

2020-08-10 Thread James Morris
On Fri, 7 Aug 2020, Mimi Zohar wrote:

> > > Are you planning to attend Plumbers? Perhaps we could propose a BoF 
> > > session on this topic.
> > 
> > That sounds like a good idea.
> 
> Other than it is already sold out.

Mimi advised me off-list that she is able to attend, so I've submitted a 
BoF proposal:

https://www.linuxplumbersconf.org/event/7/abstracts/732/


-- 
James Morris




[GIT PULL] Security subsystem updates for v5.9

2020-08-10 Thread James Morris
A couple of minor documentation updates only for this release. Please 
pull.

---

The following changes since commit 3d77e6a8804abcc0504c904bd6e5cdf3a5cf8162:

  Linux 5.7 (2020-05-31 16:49:15 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git 
tags/for-v5.9

for you to fetch changes up to bb22d80b47d5dd641d09d31946c4be0f610f3f45:

  LSM: drop duplicated words in header file comments (2020-08-06 12:00:17 -0700)


Minor fixes for v5.9.


Alexander A. Klimov (1):
  Replace HTTP links with HTTPS ones: security

Randy Dunlap (1):
  LSM: drop duplicated words in header file comments

 include/linux/lsm_hook_defs.h| 2 +-
 include/linux/lsm_hooks.h| 2 +-
 security/Kconfig | 2 +-
 security/apparmor/Kconfig| 2 +-
 security/integrity/ima/Kconfig   | 2 +-
 security/integrity/ima/ima_template.c| 2 +-
 security/integrity/ima/ima_template_lib.c| 2 +-
 security/integrity/ima/ima_template_lib.h| 2 +-
 security/keys/encrypted-keys/ecryptfs_format.c   | 2 +-
 security/keys/encrypted-keys/ecryptfs_format.h   | 2 +-
 security/keys/encrypted-keys/encrypted.c | 2 +-
 security/keys/encrypted-keys/masterkey_trusted.c | 2 +-
 12 files changed, 12 insertions(+), 12 deletions(-)


Re: [dm-devel] [RFC PATCH v5 00/11] Integrity Policy Enforcement LSM (IPE)

2020-08-07 Thread James Morris
On Thu, 6 Aug 2020, Mimi Zohar wrote:

> On Thu, 2020-08-06 at 09:51 +1000, James Morris wrote:
> > On Wed, 5 Aug 2020, Mimi Zohar wrote:
> > 
> > > If block layer integrity was enough, there wouldn't have been a need
> > > for fs-verity.   Even fs-verity is limited to read only filesystems,
> > > which makes validating file integrity so much easier.  From the
> > > beginning, we've said that fs-verity signatures should be included in
> > > the measurement list.  (I thought someone signed on to add that support
> > > to IMA, but have not yet seen anything.)
> > > 
> > > Going forward I see a lot of what we've accomplished being incorporated
> > > into the filesystems.  When IMA will be limited to defining a system
> > > wide policy, I'll have completed my job.
> > 
> > What are your thoughts on IPE being a standalone LSM? Would you prefer to 
> > see its functionality integrated into IMA?
> 
> Improving the integrity subsystem would be preferred.
> 

Are you planning to attend Plumbers? Perhaps we could propose a BoF 
session on this topic.

-- 
James Morris




Re: [dm-devel] [RFC PATCH v5 00/11] Integrity Policy Enforcement LSM (IPE)

2020-08-05 Thread James Morris
On Wed, 5 Aug 2020, Mimi Zohar wrote:

> If block layer integrity was enough, there wouldn't have been a need
> for fs-verity.   Even fs-verity is limited to read only filesystems,
> which makes validating file integrity so much easier.  From the
> beginning, we've said that fs-verity signatures should be included in
> the measurement list.  (I thought someone signed on to add that support
> to IMA, but have not yet seen anything.)
> 
> Going forward I see a lot of what we've accomplished being incorporated
> into the filesystems.  When IMA will be limited to defining a system
> wide policy, I'll have completed my job.

What are your thoughts on IPE being a standalone LSM? Would you prefer to 
see its functionality integrated into IMA?



Re: [dm-devel] [RFC PATCH v5 00/11] Integrity Policy Enforcement LSM (IPE)

2020-08-05 Thread James Morris
On Wed, 5 Aug 2020, James Bottomley wrote:

> I'll leave Mimi to answer, but really this is exactly the question that
> should have been asked before writing IPE.  However, since we have the
> cart before the horse, let me break the above down into two specific
> questions.

The question is valid and it was asked. We decided to first prototype what 
we needed and then evaluate if it should be integrated with IMA. We 
discussed this plan in person with Mimi (at LSS-NA in 2019), and presented 
a more mature version of IPE to LSS-NA in 2020, with the expectation that 
such a discussion may come up (it did not).

These patches are still part of this process and 'RFC' status.

>1. Could we implement IPE in IMA (as in would extensions to IMA cover
>   everything).  I think the answers above indicate this is a "yes".

It could be done, if needed.

>2. Should we extend IMA to implement it?  This is really whether from a
>   usability standpoint two seperate LSMs would make sense to cover the
>   different use cases.

One issue here is that IMA is fundamentally a measurement & appraisal 
scheme which has been extended to include integrity enforcement. IPE was 
designed from scratch to only perform integrity enforcement. As such, it 
is a cleaner design -- "do one thing and do it well" is a good design 
pattern.

In our use-case, we utilize _both_ IMA and IPE, for attestation and code 
integrity respectively. It is useful to be able to separate these 
concepts. They really are different:

- Code integrity enforcement ensures that code running locally is of known 
provenance and has not been modified prior to execution.

- Attestation is about measuring the health of a system and having that 
measurement validated by a remote system. (Local attestation is useless).

I'm not sure there is value in continuing to shoe-horn both of these into 
IMA.


>  I've got to say the least attractive thing
>   about separation is the fact that you now both have a policy parser.
>You've tried to differentiate yours by making it more Kconfig
>   based, but policy has a way of becoming user space supplied because
>   the distros hate config options, so I think you're going to end up
>   with a policy parser very like IMAs.


-- 
James Morris




Re: linux-next: build failure after merge of the security tree

2020-08-03 Thread James Morris
On Thu, 30 Jul 2020, Stephen Rothwell wrote:

> Hi James,
> 
> On Thu, 30 Jul 2020 12:35:03 +1000 (AEST) James Morris  
> wrote:
> >
> > On Thu, 30 Jul 2020, Stephen Rothwell wrote:
> > 
> > > > I am still applying the above patch ...  
> > > 
> > > The merge window is coming up fast ... is anything happening about this
> > > failure?  
> > 
> > A new patch is coming, but I'm not sure this code has had enough review 
> > from the core VFS folk.
> > 
> > Please drop secure_uffd_v5.9 for the time being.
> 
> You just need to remove/revert it from your security tree
> (git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git#next-testing).

Done.

-- 
James Morris




Re: linux-next: build failure after merge of the security tree

2020-07-29 Thread James Morris
On Thu, 30 Jul 2020, Stephen Rothwell wrote:

> > I am still applying the above patch ...
> 
> The merge window is coming up fast ... is anything happening about this
> failure?

A new patch is coming, but I'm not sure this code has had enough review 
from the core VFS folk.

Please drop secure_uffd_v5.9 for the time being.


-- 
James Morris




Re: [PATCH v4 08/17] fs/kernel_read_file: Add file_size output argument

2020-07-29 Thread James Morris
On Wed, 29 Jul 2020, Kees Cook wrote:

> In preparation for adding partial read support, add an optional output
> argument to kernel_read_file*() that reports the file size so callers
> can reason more easily about their reading progress.
> 
> Acked-by: Scott Branden 
> Reviewed-by: Mimi Zohar 
> Reviewed-by: Luis Chamberlain 
> Signed-off-by: Kees Cook 


Reviewed-by: James Morris 


-- 
James Morris




Re: [PATCH v4 07/17] fs/kernel_read_file: Switch buffer size arg to size_t

2020-07-29 Thread James Morris
On Wed, 29 Jul 2020, Kees Cook wrote:

> In preparation for further refactoring of kernel_read_file*(), rename
> the "max_size" argument to the more accurate "buf_size", and correct
> its type to size_t. Add kerndoc to explain the specifics of how the
> arguments will be used. Note that with buf_size now size_t, it can no
> longer be negative (and was never called with a negative value). Adjust
> callers to use it as a "maximum size" when *buf is NULL.
> 
> Acked-by: Scott Branden 
> Reviewed-by: Mimi Zohar 
> Reviewed-by: Luis Chamberlain 
> Signed-off-by: Kees Cook 


Reviewed-by: James Morris 


-- 
James Morris




Re: [PATCH v4 06/17] fs/kernel_read_file: Remove redundant size argument

2020-07-29 Thread James Morris
On Wed, 29 Jul 2020, Kees Cook wrote:

> In preparation for refactoring kernel_read_file*(), remove the redundant
> "size" argument which is not needed: it can be included in the return
> code, with callers adjusted. (VFS reads already cannot be larger than
> INT_MAX.)
> 
> Acked-by: Scott Branden 
> Reviewed-by: Mimi Zohar 
> Reviewed-by: Luis Chamberlain 
> Signed-off-by: Kees Cook 

Reviewed-by: James Morris 

-- 
James Morris




Re: [PATCH v4 04/17] fs/kernel_read_file: Split into separate include file

2020-07-29 Thread James Morris
On Wed, 29 Jul 2020, Kees Cook wrote:

> From: Scott Branden 
> 
> Move kernel_read_file* out of linux/fs.h to its own linux/kernel_read_file.h
> include file. That header gets pulled in just about everywhere
> and doesn't really need functions not related to the general fs interface.
> 
> Suggested-by: Christoph Hellwig 
> Signed-off-by: Scott Branden 
> Reviewed-by: Christoph Hellwig 
> Acked-by: Greg Kroah-Hartman 
> Link: 
> https://lore.kernel.org/r/20200706232309.12010-2-scott.bran...@broadcom.com
> Reviewed-by: Mimi Zohar 
> Reviewed-by: Luis Chamberlain 
> Signed-off-by: Kees Cook 


Acked-by: James Morris 


-- 
James Morris




Re: [PATCH v1 0/4] [RFC] Implement Trampoline File Descriptor

2020-07-28 Thread James Morris
On Tue, 28 Jul 2020, Casey Schaufler wrote:

> You could make a separate LSM to do these checks instead of limiting
> it to SELinux. Your use case, your call, of course.

It's not limited to SELinux. This is hooked via the LSM API and 
implementable by any LSM (similar to execmem, execstack etc.)


-- 
James Morris




Re: [PATCH] LSM: drop duplicated words in header file comments

2020-07-27 Thread James Morris
On Fri, 17 Jul 2020, Randy Dunlap wrote:

> From: Randy Dunlap 
> 
> Drop the doubled words "the" and "and" in comments.
> 
> Signed-off-by: Randy Dunlap 

Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git 
next-general


-- 
James Morris




Re: [PATCH] integrity: remove redundant initialization of variable ret

2020-07-27 Thread James Morris
On Wed, 1 Jul 2020, Colin King wrote:

> From: Colin Ian King 
> 
> The variable ret is being initialized with a value that is never read
> and it is being updated later with a new value.  The initialization is
> redundant and can be removed.
> 
> Addresses-Coverity: ("Unused value")
> Signed-off-by: Colin Ian King 
> ---
>  security/integrity/digsig_asymmetric.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/security/integrity/digsig_asymmetric.c 
> b/security/integrity/digsig_asymmetric.c
> index 4e0d6778277e..cfa4127d0518 100644
> --- a/security/integrity/digsig_asymmetric.c
> +++ b/security/integrity/digsig_asymmetric.c
> @@ -79,7 +79,7 @@ int asymmetric_verify(struct key *keyring, const char *sig,
>   struct public_key_signature pks;
>   struct signature_v2_hdr *hdr = (struct signature_v2_hdr *)sig;
>   struct key *key;
> - int ret = -ENOMEM;
> +     int ret;

Assuming Mimi will grab this.


Acked-by: James Morris 

-- 
James Morris




Re: [PATCH] Replace HTTP links with HTTPS ones: security

2020-07-07 Thread James Morris
On Sun, 5 Jul 2020, Alexander A. Klimov wrote:

> Rationale:
> Reduces attack surface on kernel devs opening the links for MITM
> as HTTPS traffic is much harder to manipulate.
> 
> Deterministic algorithm:
> For each file:
>   If not .svg:
> For each line:
>   If doesn't contain `\bxmlns\b`:
> For each link, `\bhttp://[^# \t\r\n]*(?:\w|/)`:
>   If both the HTTP and HTTPS versions
>   return 200 OK and serve the same content:
> Replace HTTP with HTTPS.
> 
> Signed-off-by: Alexander A. Klimov 

Thanks.

Applied to 
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git 
next-general


-- 
James Morris




[GIT PULL] Security subsystem fixes for v5.8

2020-06-29 Thread James Morris
Please pull (now using signed tags).

The following changes since commit 48778464bb7d346b47157d21ffde2af6b2d39110:

  Linux 5.8-rc2 (2020-06-21 15:45:29 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git 
tags/fixes-v5.8-rc3-a

for you to fetch changes up to 23e390cdbe6f85827a43d38f9288dcd3066fa376:

  security: Fix hook iteration and default value for inode_copy_up_xattr 
(2020-06-23 16:39:23 -0700)


Two simple fixes for v5.8:

1) Fix hook iteration and default value for inode_copy_up_xattr
from KP Singh 

2) Fix the key_permission LSM hook function type
from Sami Tolvanen 


KP Singh (1):
  security: Fix hook iteration and default value for inode_copy_up_xattr

Sami Tolvanen (1):
  security: fix the key_permission LSM hook function type

 include/linux/lsm_hook_defs.h |  4 ++--
 security/security.c   | 17 -
 2 files changed, 18 insertions(+), 3 deletions(-)


Re: [PATCH] security: fix the key_permission LSM hook function type

2020-06-22 Thread James Morris
On Mon, 15 Jun 2020, Sami Tolvanen wrote:

> Commit 8c0637e950d6 ("keys: Make the KEY_NEED_* perms an enum rather than
> a mask") changed the type of the key_permission callback functions, but
> didn't change the type of the hook, which trips indirect call checking with
> Control-Flow Integrity (CFI). This change fixes the issue by changing the
> hook type to match the functions.
> 
> Fixes: 8c0637e950d6 ("keys: Make the KEY_NEED_* perms an enum rather than a 
> mask")
> Signed-off-by: Sami Tolvanen 

Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git 
fixes-v5.8


NOTE: please cc: the LSM list with patches such as these.



-- 
James Morris




Re: [GIT PULL] SafeSetID LSM changes for v5.8

2020-06-15 Thread James Morris
On Mon, 15 Jun 2020, Micah Morton wrote:

> On Sun, Jun 14, 2020 at 10:21 PM James Morris  wrote:
> >
> > On Sun, 14 Jun 2020, Micah Morton wrote:
> >
> > > This patch was sent to the security mailing list and there were no 
> > > objections.
> >
> > Standard practice for new or modified LSM hooks is that they are reviewed
> > and acked by maintainers of major LSMs (SELinux, AppArmor, and Smack, at
> > least).
> >
> > "No objections" should be considered "not reviewed".
> >
> > Can you add your tree to linux-next?
> > https://www.kernel.org/doc/man-pages/linux-next.html
> 
> Sure, I can do that. I should just send an email to Stephen Rothwell
> asking him to include the -next branch from my tree?

Yep, thanks.

> 
> >
> > --
> > James Morris
> > 
> >
> 

-- 
James Morris




Re: [GIT PULL] SafeSetID LSM changes for v5.8

2020-06-14 Thread James Morris
On Sun, 14 Jun 2020, Micah Morton wrote:

> This patch was sent to the security mailing list and there were no objections.

Standard practice for new or modified LSM hooks is that they are reviewed 
and acked by maintainers of major LSMs (SELinux, AppArmor, and Smack, at 
least).

"No objections" should be considered "not reviewed".

Can you add your tree to linux-next?
https://www.kernel.org/doc/man-pages/linux-next.html

-- 
James Morris




Re: [PATCH v5 0/3] SELinux support for anonymous inodes and UFFD

2020-06-03 Thread James Morris
On Wed, 1 Apr 2020, Daniel Colascione wrote:

> Daniel Colascione (3):
>   Add a new LSM-supporting anonymous inode interface
>   Teach SELinux about anonymous inodes
>   Wire UFFD up to SELinux
> 
>  fs/anon_inodes.c| 191 ++--
>  fs/userfaultfd.c|  30 -
>  include/linux/anon_inodes.h |  13 ++
>  include/linux/lsm_hooks.h   |  11 ++
>  include/linux/security.h|   3 +
>  security/security.c |   9 ++
>  security/selinux/hooks.c|  53 
>  security/selinux/include/classmap.h |   2 +
>  8 files changed, 267 insertions(+), 45 deletions(-)

Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git 
secure_uffd_v5.9
and next-testing.

This will provide test coverage in linux-next, as we aim to get this 
upstream for v5.9.

I had to make some minor fixups, please review.


-- 
James Morris




Re: [GIT PULL] SELinux patches for v5.8

2020-06-03 Thread James Morris
On Wed, 3 Jun 2020, Casey Schaufler wrote:

> On 6/3/2020 3:12 PM, James Morris wrote:
> > On Wed, 3 Jun 2020, Casey Schaufler wrote:
> >
> >> The use of security modules was expected to be rare.
> > This is not correct. Capabilities were ported to LSM and stacked from the 
> > beginning, and several major distros worked on LSM so they could ship 
> > their own security modules.
> 
> Capabilities has always been a special case.
> Until Android adopted SELinux the actual use of LSMs was rare.

Nope, it was enabled by default in several distros and very widely 
deployed in the govt space (at least).

-- 
James Morris




Re: [GIT PULL] SELinux patches for v5.8

2020-06-03 Thread James Morris
On Wed, 3 Jun 2020, Casey Schaufler wrote:

> The use of security modules was expected to be rare.

This is not correct. Capabilities were ported to LSM and stacked from the 
beginning, and several major distros worked on LSM so they could ship 
their own security modules.



-- 
James Morris




Re: [GIT PULL][Security] lockdown: Allow unprivileged users to see lockdown status

2020-06-03 Thread James Morris
On Tue, 2 Jun 2020, Linus Torvalds wrote:

> On Mon, Jun 1, 2020 at 7:15 PM James Morris  wrote:
> >
> > Just one update for the security subsystem: allows unprivileged users to
> > see the status of the lockdown feature. From Jeremy Cline.
> 
> Hmm.
> 
> That branch seems to have sprouted another commit just today.

Oops, sorry, I thought it was already pulled.

> 
> I ended up taking that too as trivial, but it shows how you seem to
> basically send me a pointer to a live branch. Please don't do that.
> When you make changes to that branch, I now get those changes that you
> may not have meant to send me (and that I get upset for being
> surprised by).
> 
> An easy solution to that is to send me a signed tag instead of a
> pointer to a branch. Then you can continue to update the branch, while
> the tag stays stable.
> 
> Plus we've been encouraging signed tags for pull requests anyway.

Ok.

-- 
James Morris




Re: [PATCH v2] capabilities: add description for CAP_SETFCAP

2020-06-02 Thread James Morris
On Tue, 2 Jun 2020, Stefan Hajnoczi wrote:

> Document the purpose of CAP_SETFCAP.  For some reason this capability
> had no description while the others did.
> 
> Signed-off-by: Stefan Hajnoczi 

Thanks.

Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git 
next-general


-- 
James Morris




[GIT PULL][Security] lockdown: Allow unprivileged users to see lockdown status

2020-06-01 Thread James Morris
Hi Linus,

Just one update for the security subsystem: allows unprivileged users to 
see the status of the lockdown feature. From Jeremy Cline.

Please pull.


The following changes since commit 3e27a33932df104f4f9ff811467b0b4ccebde773:

  security: remove duplicated include from security.h (2020-02-21 08:53:48 
-0800)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git 
next-general

for you to fetch changes up to 60cf7c5ed5f7087c4de87a7676b8c82d96fd166c:

  lockdown: Allow unprivileged users to see lockdown status (2020-05-14 
10:23:05 -0700)


Jeremy Cline (1):
  lockdown: Allow unprivileged users to see lockdown status

 security/lockdown/lockdown.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

---
commit 60cf7c5ed5f7087c4de87a7676b8c82d96fd166c
Author: Jeremy Cline 
Date:   Thu May 14 10:05:46 2020 -0400

lockdown: Allow unprivileged users to see lockdown status

A number of userspace tools, such as systemtap, need a way to see the
current lockdown state so they can gracefully deal with the kernel being
locked down. The state is already exposed in
/sys/kernel/security/lockdown, but is only readable by root. Adjust the
permissions so unprivileged users can read the state.

Fixes: 000d388ed3bb ("security: Add a static lockdown policy LSM")
Cc: Frank Ch. Eigler 
Signed-off-by: Jeremy Cline 
Signed-off-by: James Morris 

diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index 40b790536def..ae594c0a127f 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -175,7 +175,7 @@ static int __init lockdown_secfs_init(void)
 {
struct dentry *dentry;
 
-   dentry = securityfs_create_file("lockdown", 0600, NULL, NULL,
+   dentry = securityfs_create_file("lockdown", 0644, NULL, NULL,
_ops);
return PTR_ERR_OR_ZERO(dentry);
 }


Re: [PATCH bpf] security: Fix hook iteration for secid_to_secctx

2020-05-20 Thread James Morris
On Wed, 20 May 2020, Alexei Starovoitov wrote:

> On Wed, May 20, 2020 at 8:15 AM Casey Schaufler  
> wrote:
> >
> >
> > On 5/20/2020 5:56 AM, KP Singh wrote:
> > > From: KP Singh 
> > >
> > > secid_to_secctx is not stackable, and since the BPF LSM registers this
> > > hook by default, the call_int_hook logic is not suitable which
> > > "bails-on-fail" and casues issues when other LSMs register this hook and
> > > eventually breaks Audit.
> > >
> > > In order to fix this, directly iterate over the security hooks instead
> > > of using call_int_hook as suggested in:
> > >
> > > https: 
> > > //lore.kernel.org/bpf/9d0eb6c6-803a-ff3a-5603-9ad6d9edf...@schaufler-ca.com/#t
> > >
> > > Fixes: 98e828a0650f ("security: Refactor declaration of LSM hooks")
> > > Fixes: 625236ba3832 ("security: Fix the default value of secid_to_secctx 
> > > hook"
> > > Reported-by: Alexei Starovoitov 
> > > Signed-off-by: KP Singh 
> >
> > This looks fine.
> 
> Tested. audit works now.
> I fixed missing ')' in the commit log
> and applied to bpf tree.
> It will be on the way to Linus tree soon.

Please add:


Acked-by: James Morris 


-- 
James Morris




Re: [PATCH v2 3/8] exec: Convert security_bprm_set_creds into security_bprm_repopulate_creds

2020-05-19 Thread James Morris
On Mon, 18 May 2020, Eric W. Biederman wrote:

> diff --git a/fs/exec.c b/fs/exec.c
> index 9e70da47f8d9..8e3b93d51d31 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -1366,7 +1366,7 @@ int begin_new_exec(struct linux_binprm * bprm)
>* the final state of setuid/setgid/fscaps can be merged into the
>* secureexec flag.
>*/
> - bprm->secureexec |= bprm->cap_elevated;
> + bprm->secureexec |= bprm->active_secureexec;

Which kernel tree are these patches for? Seems like begin_new_exec() is 
from a prerequisite patchset.


-- 
James Morris




Re: [PATCH v2 5/8] exec: Move the call of prepare_binprm into search_binary_handler

2020-05-19 Thread James Morris
On Mon, 18 May 2020, Eric W. Biederman wrote:

> 
> The code in prepare_binary_handler needs to be run every time
> search_binary_handler is called so move the call into search_binary_handler
> itself to make the code simpler and easier to understand.
> 
> Signed-off-by: "Eric W. Biederman" 

Nice cleanup.

Reviewed-by: James Morris 

-- 
James Morris




Re: [PATCH v2 2/8] exec: Factor security_bprm_creds_for_exec out of security_bprm_set_creds

2020-05-19 Thread James Morris
On Tue, 19 May 2020, Kees Cook wrote:

> > /* SELinux context only depends on initial program or script and not
> >  * the script interpreter */
> > -   if (bprm->called_set_creds)
> > -   return 0;
> >  
> > old_tsec = selinux_cred(current_cred());
> > new_tsec = selinux_cred(bprm->cred);
> 
> As you've done in the other LSMs, I think this comment can be removed
> (or moved to the top of the function) too.

I'd prefer moved to top of the function.

-- 
James Morris




Re: [PATCH] security: fix the default value of secid_to_secctx hook

2020-05-14 Thread James Morris
1eb3381e54b 100644
> > --- a/include/linux/lsm_hook_defs.h
> > +++ b/include/linux/lsm_hook_defs.h
> > @@ -243,7 +243,7 @@ LSM_HOOK(int, -EINVAL, getprocattr, struct task_struct 
> > *p, char *name,
> >  char **value)
> >  LSM_HOOK(int, -EINVAL, setprocattr, const char *name, void *value, size_t 
> > size)
> >  LSM_HOOK(int, 0, ismaclabel, const char *name)
> > -LSM_HOOK(int, 0, secid_to_secctx, u32 secid, char **secdata,
> > +LSM_HOOK(int, -EOPNOTSUPP, secid_to_secctx, u32 secid, char **secdata,
> >  u32 *seclen)
> >  LSM_HOOK(int, 0, secctx_to_secid, const char *secdata, u32 seclen, u32 
> > *secid)
> >  LSM_HOOK(void, LSM_RET_VOID, release_secctx, char *secdata, u32 seclen)
> > --
> > 2.20.1
> >
> 


-- 
James Morris



Re: [PATCH] security: fix the default value of secid_to_secctx hook

2020-05-14 Thread James Morris
On Tue, 12 May 2020, Anders Roxell wrote:

> security_secid_to_secctx is called by the bpf_lsm hook and a successful
> return value (i.e 0) implies that the parameter will be consumed by the
> LSM framework. The current behaviour return success when the pointer
> isn't initialized when CONFIG_BPF_LSM is enabled, with the default
> return from kernel/bpf/bpf_lsm.c.
> 
> This is the internal error:
> 
> [ 1229.341488][ T2659] usercopy: Kernel memory exposure attempt detected from 
> null address (offset 0, size 280)!
> [ 1229.374977][ T2659] [ cut here ]
> [ 1229.376813][ T2659] kernel BUG at mm/usercopy.c:99!
> [ 1229.378398][ T2659] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
> [ 1229.380348][ T2659] Modules linked in:
> [ 1229.381654][ T2659] CPU: 0 PID: 2659 Comm: systemd-journal Tainted: GB 
>   W 5.7.0-rc5-next-20200511-00019-g864e0c6319b8-dirty #13
> [ 1229.385429][ T2659] Hardware name: linux,dummy-virt (DT)
> [ 1229.387143][ T2659] pstate: 8045 (Nzcv daif +PAN -UAO BTYPE=--)
> [ 1229.389165][ T2659] pc : usercopy_abort+0xc8/0xcc
> [ 1229.390705][ T2659] lr : usercopy_abort+0xc8/0xcc
> [ 1229.392225][ T2659] sp : 64247450
> [ 1229.393533][ T2659] x29: 64247460 x28: 
> [ 1229.395449][ T2659] x27: 0118 x26: 
> [ 1229.397384][ T2659] x25: a000127049e0 x24: a000127049e0
> [ 1229.399306][ T2659] x23: a000127048e0 x22: a000127048a0
> [ 1229.401241][ T2659] x21: a00012704b80 x20: a000127049e0
> [ 1229.403163][ T2659] x19: a00012704820 x18: 
> [ 1229.405094][ T2659] x17:  x16: 
> [ 1229.407008][ T2659] x15:  x14: 003d0900
> [ 1229.408942][ T2659] x13: 8d5b25b2 x12: 1fffed5b25b1
> [ 1229.410859][ T2659] x11: 1fffed5b25b1 x10: 8d5b25b1
> [ 1229.412791][ T2659] x9 : a0001034bee0 x8 : 6ad92d8f
> [ 1229.414707][ T2659] x7 :  x6 : a00015eacb20
> [ 1229.416642][ T2659] x5 : 693c8040 x4 : 
> [ 1229.418558][ T2659] x3 : a0001034befc x2 : d57a7483a01c6300
> [ 1229.420610][ T2659] x1 :  x0 : 0059
> [ 1229.422526][ T2659] Call trace:
> [ 1229.423631][ T2659]  usercopy_abort+0xc8/0xcc
> [ 1229.425091][ T2659]  __check_object_size+0xdc/0x7d4
> [ 1229.426729][ T2659]  put_cmsg+0xa30/0xa90
> [ 1229.428132][ T2659]  unix_dgram_recvmsg+0x80c/0x930
> [ 1229.429731][ T2659]  sock_recvmsg+0x9c/0xc0
> [ 1229.431123][ T2659]  sys_recvmsg+0x1cc/0x5f8
> [ 1229.432663][ T2659]  ___sys_recvmsg+0x100/0x160
> [ 1229.434151][ T2659]  __sys_recvmsg+0x110/0x1a8
> [ 1229.435623][ T2659]  __arm64_sys_recvmsg+0x58/0x70
> [ 1229.437218][ T2659]  el0_svc_common.constprop.1+0x29c/0x340
> [ 1229.438994][ T2659]  do_el0_svc+0xe8/0x108
> [ 1229.440587][ T2659]  el0_svc+0x74/0x88
> [ 1229.441917][ T2659]  el0_sync_handler+0xe4/0x8b4
> [ 1229.443464][ T2659]  el0_sync+0x17c/0x180
> [ 1229.444920][ T2659] Code: aa1703e2 aa1603e1 910a8260 97ecc860 (d421)
> [ 1229.447070][ T2659] ---[ end trace 400497d91baeaf51 ]---
> [ 1229.448791][ T2659] Kernel panic - not syncing: Fatal exception
> [ 1229.450692][ T2659] Kernel Offset: disabled
> [ 1229.452061][ T2659] CPU features: 0x240002,20002004
> [ 1229.453647][ T2659] Memory Limit: none
> [ 1229.455015][ T2659] ---[ end Kernel panic - not syncing: Fatal exception 
> ]---
> 
> Rework the so the default return value is -EOPNOTSUPP.
> 
> There are likely other callbacks such as security_inode_getsecctx() that
> may have the same problem, and that someone that understand the code
> better needs to audit them.
> 
> Thank you Arnd for helping me figure out what went wrong.
> 
> CC: Arnd Bergmann 
> Fixes: 98e828a0650f ("security: Refactor declaration of LSM hooks")
> Signed-off-by: Anders Roxell 

Note, this patch should have been sent to me and cc'd the LSM list.


Acked-by: James Morris 



-- 
James Morris




Re: [PATCH v17 05/10] fs,landlock: Support filesystem access-control

2020-05-14 Thread James Morris
On Thu, 14 May 2020, Mickaël Salaün wrote:

> > fsnotify is not an LSM.
> 
> Yes, so I'll need to add a new LSM hook for this (release) call, right?

Unless an existing one will work.

-- 
James Morris



Re: [PATCH v17 05/10] fs,landlock: Support filesystem access-control

2020-05-14 Thread James Morris
On Thu, 14 May 2020, Mickaël Salaün wrote:

> > This needs to be converted to the LSM API via superblock blob stacking.
> > 
> > See Casey's old patch: 
> > https://lore.kernel.org/linux-security-module/20190829232935.7099-2-ca...@schaufler-ca.com/
> 
> s_landlock_inode_refs is quite similar to s_fsnotify_inode_refs, but I
> can do it once the superblock security blob patch is upstream. Is it a
> blocker for now? What is the current status of lbs_superblock?

Yes it is a blocker. Landlock should not be adding its own functions in 
core code, it should be using the LSM API (and extending that as needed).

> Anyway, we also need to have a call to landlock_release_inodes() in
> generic_shutdown_super(), which does not fit the LSM framework, and I
> think it is not an issue. Landlock handling of inodes is quite similar
> to fsnotify.

fsnotify is not an LSM.

-- 
James Morris



Re: [PATCH RESEND] lockdown: Allow unprivileged users to see lockdown status

2020-05-14 Thread James Morris
On Thu, 14 May 2020, Jeremy Cline wrote:

> A number of userspace tools, such as systemtap, need a way to see the
> current lockdown state so they can gracefully deal with the kernel being
> locked down. The state is already exposed in
> /sys/kernel/security/lockdown, but is only readable by root. Adjust the
> permissions so unprivileged users can read the state.
> 
> Fixes: 000d388ed3bb ("security: Add a static lockdown policy LSM")
> Cc: Frank Ch. Eigler 
> Signed-off-by: Jeremy Cline 

Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git 
next-general


-- 
James Morris




Re: [PATCH v17 05/10] fs,landlock: Support filesystem access-control

2020-05-13 Thread James Morris
On Mon, 11 May 2020, Mickaël Salaün wrote:


> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 45cc10cdf6dd..2276642f8e05 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1517,6 +1517,11 @@ struct super_block {
>   /* Pending fsnotify inode refs */
>   atomic_long_t s_fsnotify_inode_refs;
>  
> +#ifdef CONFIG_SECURITY_LANDLOCK
> + /* References to Landlock underlying objects */
> + atomic_long_t s_landlock_inode_refs;
> +#endif
> +

This needs to be converted to the LSM API via superblock blob stacking.

See Casey's old patch: 
https://lore.kernel.org/linux-security-module/20190829232935.7099-2-ca...@schaufler-ca.com/



-- 
James Morris



Re: [PATCH v17 02/10] landlock: Add ruleset and domain management

2020-05-13 Thread James Morris
On Mon, 11 May 2020, Mickaël Salaün wrote:

> + * .. warning::
> + *
> + *   It is currently not possible to restrict some file-related actions
> + *   accessible through these syscall families: :manpage:`chdir(2)`,
> + *   :manpage:`truncate(2)`, :manpage:`stat(2)`, :manpage:`flock(2)`,
> + *   :manpage:`chmod(2)`, :manpage:`chown(2)`, :manpage:`setxattr(2)`,
> + *   :manpage:`ioctl(2)`, :manpage:`fcntl(2)`.
> + *   Future Landlock evolutions will enable to restrict them.

I have to wonder how useful Landlock will be without more coverage per 
the above.

It would be helpful if you could outline a threat model for this initial 
version, so people can get an idea of what kind of useful protection may
be gained from it.

Are there any distros or other major users who are planning on enabling or 
at least investigating Landlock?

Do you have any examples of a practical application of this scheme?



-- 
James Morris



Re: [PATCH] lockdown: Allow unprivileged users to see lockdown status

2020-05-13 Thread James Morris
On Mon, 11 May 2020, Jeremy Cline wrote:

> On Sat, Feb 22, 2020 at 03:51:24AM +1100, James Morris wrote:
> > On Thu, 20 Feb 2020, Jeremy Cline wrote:
> > 
> > > A number of userspace tools, such as systemtap, need a way to see the
> > > current lockdown state so they can gracefully deal with the kernel being
> > > locked down. The state is already exposed in
> > > /sys/kernel/security/lockdown, but is only readable by root. Adjust the
> > > permissions so unprivileged users can read the state.
> > > 
> > > Fixes: 000d388ed3bb ("security: Add a static lockdown policy LSM")
> > > Cc: Frank Ch. Eigler 
> > > Signed-off-by: Jeremy Cline 
> > 
> > Looks fine to me, any objection from Matthew or others?
> > 
> 
> Can we take resounding silence as no objections?

Please resend and I'll apply it to my tree.

> 
> - Jeremy
> 
> > > ---
> > >  security/lockdown/lockdown.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> > > index 5a952617a0eb..87cbdc64d272 100644
> > > --- a/security/lockdown/lockdown.c
> > > +++ b/security/lockdown/lockdown.c
> > > @@ -150,7 +150,7 @@ static int __init lockdown_secfs_init(void)
> > >  {
> > >   struct dentry *dentry;
> > >  
> > > - dentry = securityfs_create_file("lockdown", 0600, NULL, NULL,
> > > + dentry = securityfs_create_file("lockdown", 0644, NULL, NULL,
> > >   _ops);
> > >   return PTR_ERR_OR_ZERO(dentry);
> > >  }
> > > 
> > 
> > -- 
> > James Morris
> > 
> > 
> 

-- 
James Morris




[GIT PULL] security: Fix the default value of fs_context_parse_param hook

2020-05-07 Thread James Morris
Please pull this fix from KP Singh (several folks are reporting issues 
around this):

The following changes since commit c45e8bccecaf633480d378daff11e122dfd5e96d:

  Merge tag 'for-5.7/dm-fixes-2' of 
git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm 
(2020-04-30 16:45:08 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git 
for-v5.7

for you to fetch changes up to 54261af473be4c5481f6196064445d2945f2bdab:

  security: Fix the default value of fs_context_parse_param hook (2020-04-30 
20:29:34 -0700)


KP Singh (1):
  security: Fix the default value of fs_context_parse_param hook

 include/linux/lsm_hook_defs.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

---

commit 54261af473be4c5481f6196064445d2945f2bdab
Author: KP Singh 
Date:   Thu Apr 30 17:52:40 2020 +0200

security: Fix the default value of fs_context_parse_param hook

security_fs_context_parse_param is called by vfs_parse_fs_param and
a succussful return value (i.e 0) implies that a parameter will be
consumed by the LSM framework. This stops all further parsing of the
parmeter by VFS. Furthermore, if an LSM hook returns a success, the
remaining LSM hooks are not invoked for the parameter.

The current default behavior of returning success means that all the
parameters are expected to be parsed by the LSM hook and none of them
end up being populated by vfs in fs_context

This was noticed when lsm=bpf is supplied on the command line before any
other LSM. As the bpf lsm uses this default value to implement a default
hook, this resulted in a failure to parse any fs_context parameters and
a failure to mount the root filesystem.

Fixes: 98e828a0650f ("security: Refactor declaration of LSM hooks")
Reported-by: Mikko Ylinen 
Signed-off-by: KP Singh 
Signed-off-by: James Morris 

diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 9cd4455528e5..1bdd027766d4 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -55,7 +55,7 @@ LSM_HOOK(void, LSM_RET_VOID, bprm_committing_creds, struct 
linux_binprm *bprm)
 LSM_HOOK(void, LSM_RET_VOID, bprm_committed_creds, struct linux_binprm *bprm)
 LSM_HOOK(int, 0, fs_context_dup, struct fs_context *fc,
 struct fs_context *src_sc)
-LSM_HOOK(int, 0, fs_context_parse_param, struct fs_context *fc,
+LSM_HOOK(int, -ENOPARAM, fs_context_parse_param, struct fs_context *fc,
 struct fs_parameter *param)
 LSM_HOOK(int, 0, sb_alloc_security, struct super_block *sb)
 LSM_HOOK(void, LSM_RET_VOID, sb_free_security, struct super_block *sb)


Re: [PATCH v5 1/3] Add a new LSM-supporting anonymous inode interface

2020-05-07 Thread James Morris
gt; @@ -1552,6 +1559,9 @@ union security_list_options {
>   const struct qstr *qstr,
>   const char **name, void **value,
>   size_t *len);
> + int (*inode_init_security_anon)(struct inode *inode,
> + const struct qstr *name,
> + const struct inode *context_inode);
>   int (*inode_create)(struct inode *dir, struct dentry *dentry,
>   umode_t mode);
>   int (*inode_link)(struct dentry *old_dentry, struct inode *dir,
> @@ -1884,6 +1894,7 @@ struct security_hook_heads {
>   struct hlist_head inode_alloc_security;
>   struct hlist_head inode_free_security;
>   struct hlist_head inode_init_security;
> + struct hlist_head inode_init_security_anon;
>   struct hlist_head inode_create;
>   struct hlist_head inode_link;
>   struct hlist_head inode_unlink;
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 64b19f050343..2108c3ce0666 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -320,6 +320,9 @@ void security_inode_free(struct inode *inode);
>  int security_inode_init_security(struct inode *inode, struct inode *dir,
>const struct qstr *qstr,
>initxattrs initxattrs, void *fs_data);
> +int security_inode_init_security_anon(struct inode *inode,
> +   const struct qstr *name,
> +   const struct inode *context_inode);
>  int security_old_inode_init_security(struct inode *inode, struct inode *dir,
>const struct qstr *qstr, const char **name,
>void **value, size_t *len);
> diff --git a/security/security.c b/security/security.c
> index 565bc9b67276..70bfebada024 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1033,6 +1033,15 @@ int security_inode_init_security(struct inode *inode, 
> struct inode *dir,
>  }
>  EXPORT_SYMBOL(security_inode_init_security);
>  
> +int
> +security_inode_init_security_anon(struct inode *inode,
> +   const struct qstr *name,
> +   const struct inode *context_inode)
> +{
> + return call_int_hook(inode_init_security_anon, 0, inode, name,
> +  context_inode);
> +}
> +
>  int security_old_inode_init_security(struct inode *inode, struct inode *dir,
>const struct qstr *qstr, const char **name,
>void **value, size_t *len)
> 

-- 
James Morris




Re: [PATCH v3 3/5] fs: Enable to enforce noexec mounts or file exec through RESOLVE_MAYEXEC

2020-05-01 Thread James Morris
On Fri, 1 May 2020, Mickaël Salaün wrote:

> 
> However, for fully controlled distros such as CLIP OS, it make sense to
> enforce such restrictions at kernel build time. I can add an alternative
> kernel configuration to enforce a particular policy at boot and disable
> this sysctl.

Sounds good.

-- 
James Morris



Re: [PATCH v3 3/5] fs: Enable to enforce noexec mounts or file exec through RESOLVE_MAYEXEC

2020-04-30 Thread James Morris
On Tue, 28 Apr 2020, Mickaël Salaün wrote:

> Enable to either propagate the mount options from the underlying VFS
> mount to prevent execution, or to propagate the file execute permission.
> This may allow a script interpreter to check execution permissions
> before reading commands from a file.

I'm finding the description of this patch difficult to understand.

In the first case, this seems to mean: if you open a file with 
RESOLVE_MAYEXEC from a noexec mount, then it will fail. Correct?

In the second case, do you mean a RESOLVE_MAYEXEC open will fail if the 
file does not have +x set for the user?


> The main goal is to be able to protect the kernel by restricting
> arbitrary syscalls that an attacker could perform with a crafted binary
> or certain script languages.

This sounds like the job of seccomp. Why is this part of MAYEXEC?

>  It also improves multilevel isolation
> by reducing the ability of an attacker to use side channels with
> specific code.  These restrictions can natively be enforced for ELF
> binaries (with the noexec mount option) but require this kernel
> extension to properly handle scripts (e.g., Python, Perl).

Again, not sure why you're talking about side channels and MAYEXEC and 
mount options. Are you more generally talking about being able to prevent 
execution of arbitrary script files included by an interpreter?

> Add a new sysctl fs.open_mayexec_enforce to control this behavior.
> Indeed, because of compatibility with installed systems, only the system
> administrator is able to check that this new enforcement is in line with
> the system mount points and file permissions.  A following patch adds
> documentation.

I don't like the idea of any of this feature set being configurable. 
RESOLVE_MAYEXEC as a new flag should have well-defined, stable semantics.


-- 
James Morris


Re: [PATCH v3 1/5] fs: Add support for a RESOLVE_MAYEXEC flag on openat2(2)

2020-04-30 Thread James Morris
On Tue, 28 Apr 2020, Mickaël Salaün wrote:

> When the RESOLVE_MAYEXEC flag is passed, openat2(2) may be subject to
> additional restrictions depending on a security policy managed by the
> kernel through a sysctl or implemented by an LSM thanks to the
> inode_permission hook.


Reviewed-by: James Morris 


-- 
James Morris



Re: [PATCH v3 2/5] fs: Add a MAY_EXECMOUNT flag to infer the noexec mount property

2020-04-30 Thread James Morris
On Tue, 28 Apr 2020, Mickaël Salaün wrote:

> An LSM doesn't get path information related to an access request to open
> an inode.  This new (internal) MAY_EXECMOUNT flag enables an LSM to
> check if the underlying mount point of an inode is marked as executable.
> This is useful to implement a security policy taking advantage of the
> noexec mount option.
> 
> This flag is set according to path_noexec(), which checks if a mount
> point is mounted with MNT_NOEXEC or if the underlying superblock is
> SB_I_NOEXEC.
> 
> Signed-off-by: Mickaël Salaün 
> Reviewed-by: Philippe Trébuchet 
> Reviewed-by: Thibaut Sautereau 
> Cc: Aleksa Sarai 
> Cc: Al Viro 
> Cc: Kees Cook 

Are there any existing LSMs which plan to use this aspect?

-- 
James Morris


Re: [PATCH v3 0/5] Add support for RESOLVE_MAYEXEC

2020-04-30 Thread James Morris
On Tue, 28 Apr 2020, Mickaël Salaün wrote:

> Furthermore, the security policy can also be delegated to an LSM, either
> a MAC system or an integrity system.  For instance, the new kernel
> MAY_OPENEXEC flag closes a major IMA measurement/appraisal interpreter
> integrity gap by bringing the ability to check the use of scripts [1].
> Other uses are expected, such as for openat2(2) [2], SGX integration
> [3], bpffs [4] or IPE [5].

Confirming that this is a highly desirable feature for the proposed IPE 
LSM.

-- 
James Morris



Re: [PATCH] capabilities: add description for CAP_SETFCAP

2020-04-30 Thread James Morris
On Tue, 28 Apr 2020, Stefan Hajnoczi wrote:

> On Tue, Apr 14, 2020 at 04:49:45PM +0100, Stefan Hajnoczi wrote:
> > Document the purpose of CAP_SETFCAP.  For some reason this capability
> > had no description while the others did.
> > 
> > Signed-off-by: Stefan Hajnoczi 
> > ---
> >  include/uapi/linux/capability.h | 2 ++
> >  1 file changed, 2 insertions(+)
> 
> Ping?

Please resend and I'll add it to my tree.

> 
> > diff --git a/include/uapi/linux/capability.h 
> > b/include/uapi/linux/capability.h
> > index 272dc69fa080..7288f0ad44af 100644
> > --- a/include/uapi/linux/capability.h
> > +++ b/include/uapi/linux/capability.h
> > @@ -332,6 +332,8 @@ struct vfs_ns_cap_data {
> >  
> >  #define CAP_AUDIT_CONTROL30
> >  
> > +/* Set or remove capabilities on files */
> > +
> >  #define CAP_SETFCAP 31
> >  
> >  /* Override MAC access.
> > -- 
> > 2.25.1
> > 
> 

-- 
James Morris




Re: [PATCH bpf] security: Fix the default value of fs_context_parse_param hook

2020-04-30 Thread James Morris
On Thu, 30 Apr 2020, KP Singh wrote:

> From: KP Singh 
> 

Applied to:
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git 
for-v5.7


-- 
James Morris




Re: [PATCH] Documentation: LSM: Correct the basic LSM description

2020-04-29 Thread James Morris
On Tue, 28 Apr 2020, Jonathan Corbet wrote:

> On Tue, 21 Apr 2020 14:48:34 -0700
> Casey Schaufler  wrote:
> 
> > This is a first pass at updating the basic documentation on
> > Linux Security Modules (LSM), which is frighteningly out of date.
> > Remove untrue statements about the LSM framework. Replace them
> > with true statements where it is convenient to do so. This is
> > the beginnig of a larger effort to bring the LSM documentation
> > up to date.
> > 
> > Signed-off-by: Casey Schaufler 
> > ---
> >  Documentation/security/lsm.rst | 202 
> > ++---
> >  1 file changed, 66 insertions(+), 136 deletions(-)
> 
> James, are you planning to pick this up, or should I grab it?

You can grab it, but I don't think this patch ended up on the lsm list for 
review (I only caught it in the moderation queue for lss-pc).


-- 
James Morris




Re: [PATCH 0/7] Harden userfaultfd

2019-10-15 Thread James Morris
On Sat, 12 Oct 2019, Daniel Colascione wrote:

>  Documentation/admin-guide/sysctl/vm.rst | 19 +-
>  fs/anon_inodes.c| 89 +
>  fs/userfaultfd.c| 47 +++--
>  include/linux/anon_inodes.h | 27 ++--
>  include/linux/lsm_hooks.h   |  8 +++
>  include/linux/security.h|  2 +
>  include/linux/userfaultfd_k.h   |  3 +
>  include/uapi/linux/userfaultfd.h| 14 
>  kernel/sysctl.c |  9 +++
>  security/security.c |  8 +++
>  security/selinux/hooks.c| 68 +++
>  security/selinux/include/classmap.h |  2 +
>  12 files changed, 256 insertions(+), 40 deletions(-)

For any changes to security/ please include the linux-security-module 
list.

-- 
James Morris




Re: [PATCH] perf_event: Add support for LSM and SELinux checks

2019-10-11 Thread James Morris
On Fri, 11 Oct 2019, Joel Fernandes (Google) wrote:

> This patch adds LSM and SELinux access checking which will be used in
> Android to access perf_event_open(2) for the purposes of attaching BPF
> programs to tracepoints, perf profiling and other operations from
> userspace. These operations are intended for production systems.


Acked-by: James Morris 

-- 
James Morris




Re: [PATCH RFC] perf_event: Add support for LSM and SELinux checks

2019-10-10 Thread James Morris
On Thu, 10 Oct 2019, Casey Schaufler wrote:

> > Because it is not necessary.
> 
> The logic escapes me, but OK.

We should only extend the stacking infrastructure to what is concretely 
required. We don't yet have a use-case for stacking perf_event so we 
should keep the code as simple as possible. As soon as multiple LSMs 
determine they need to share the blob, we can convert the code to blob 
sharing.


-- 
James Morris




  1   2   3   4   5   6   7   8   9   10   >