Re: [PATCH 1/2] security: Add a cred_getsecid hook

[Paul Moore]

On Mon, Oct 16, 2017 at 4:37 PM, Matthew Garrett  wrote:
> For IMA purposes, we want to be able to obtain the prepared secid in the
> bprm structure before the credentials are committed. Add a cred_getsecid
> hook that makes this possible.
>
> Signed-off-by: Matthew Garrett 
> Cc: Paul Moore 
> Cc: Stephen Smalley 
> Cc: Eric Paris 
> Cc: selinux@tycho.nsa.gov
> Cc: Casey Schaufler 
> Cc: linux-security-mod...@vger.kernel.org
> Cc: Mimi Zohar 
> Cc: Dmitry Kasatkin 
> Cc: linux-integr...@vger.kernel.org
> ---
>  include/linux/lsm_hooks.h  |  6 ++
>  include/linux/security.h   |  1 +
>  security/security.c|  7 +++
>  security/selinux/hooks.c   |  8 
>  security/smack/smack.h | 10 ++
>  security/smack/smack_lsm.c | 14 ++
>  6 files changed, 46 insertions(+)

No argument from me on the SELinux/LSM bits:

Acked-by: Paul Moore 

> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index ce02f76a6188..48a929fd47e6 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -556,6 +556,10 @@
>   * @new points to the new credentials.
>   * @old points to the original credentials.
>   * Transfer data from original creds to new creds
> + * @cred_getsecid:
> + * Retrieve the security identifier of the cred structure @c
> + * @p contains the credentials, secid will be placed into @secid.
> + * In case of failure, @secid will be set to zero.
>   * @kernel_act_as:
>   * Set the credentials for a kernel service to act as (subjective 
> context).
>   * @new points to the credentials to be modified.
> @@ -1510,6 +1514,7 @@ union security_list_options {
> int (*cred_prepare)(struct cred *new, const struct cred *old,
> gfp_t gfp);
> void (*cred_transfer)(struct cred *new, const struct cred *old);
> +   void (*cred_getsecid)(const struct cred *c, u32 *secid);
> int (*kernel_act_as)(struct cred *new, u32 secid);
> int (*kernel_create_files_as)(struct cred *new, struct inode *inode);
> int (*kernel_module_request)(char *kmod_name);
> @@ -1783,6 +1788,7 @@ struct security_hook_heads {
> struct list_head cred_free;
> struct list_head cred_prepare;
> struct list_head cred_transfer;
> +   struct list_head cred_getsecid;
> struct list_head kernel_act_as;
> struct list_head kernel_create_files_as;
> struct list_head kernel_read_file;
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 458e24bea2d4..8d969958c25e 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -324,6 +324,7 @@ int security_cred_alloc_blank(struct cred *cred, gfp_t 
> gfp);
>  void security_cred_free(struct cred *cred);
>  int security_prepare_creds(struct cred *new, const struct cred *old, gfp_t 
> gfp);
>  void security_transfer_creds(struct cred *new, const struct cred *old);
> +void security_cred_getsecid(const struct cred *c, u32 *secid);
>  int security_kernel_act_as(struct cred *new, u32 secid);
>  int security_kernel_create_files_as(struct cred *new, struct inode *inode);
>  int security_kernel_module_request(char *kmod_name);
> diff --git a/security/security.c b/security/security.c
> index 55b5997e4b72..0f5784880c94 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1009,6 +1009,13 @@ void security_transfer_creds(struct cred *new, const 
> struct cred *old)
> call_void_hook(cred_transfer, new, old);
>  }
>
> +void security_cred_getsecid(const struct cred *c, u32 *secid)
> +{
> +   *secid = 0;
> +   call_void_hook(cred_getsecid, c, secid);
> +}
> +EXPORT_SYMBOL(security_cred_getsecid);
> +
>  int security_kernel_act_as(struct cred *new, u32 secid)
>  {
> return call_int_hook(kernel_act_as, 0, new, secid);
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 33fd061305c4..e0828e9130c7 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -3829,6 +3829,13 @@ static void selinux_cred_transfer(struct cred *new, 
> const struct cred *old)
> *tsec = *old_tsec;
>  }
>
> +static void selinux_cred_getsecid(const struct cred *c, u32 *secid)
> +{
> +   rcu_read_lock();
> +   *secid = cred_sid(c);
> +   rcu_read_unlock();
> +}
> +
>  /*
>   * set the security data for a kernel service
>   * - all the creation contexts are set to unlabelled
> @@ -6332,6 +6339,7 @@ static struct security_hook_list selinux_hooks[] 
> __lsm_ro_after_init = {
> LSM_HOOK_INIT(cred_free, selinux_cred_free),
> LSM_HOOK_INIT(cred_prepare, selinux_cred_prepare),
> LSM_HOOK_INIT(cred_transfer, selinux_cred_transfer),
> +   LSM_HOOK_INIT(cred_getsecid, selinux_cred_getsecid),
>

Re: [RFC PATCH 1/1] selinux-testsuite: Add CALIPSO/IPv6 tests

[Stephen Smalley]

On Thu, 2017-10-19 at 16:57 +0100, Richard Haines wrote:
> Add CALIPSO tests to inet_socket.
> 
> Note the CALIPSO/IPv6 datagram tests check whether the kernel patch
> described in "Add SCM_SECURITY support to IPv6" [1] is installed.
> 
> [1] https://github.com/SELinuxProject/selinux-kernel/issues/24
> 
> Signed-off-by: Richard Haines 
> ---
>  tests/inet_socket/Makefile  |   3 +
>  tests/inet_socket/calipso-flush |   5 ++
>  tests/inet_socket/calipso-load  |   7 +++
>  tests/inet_socket/server.c  |  67 +--
>  tests/inet_socket/test  | 118
> 
>  5 files changed, 173 insertions(+), 27 deletions(-)
>  create mode 100644 tests/inet_socket/calipso-flush
>  create mode 100644 tests/inet_socket/calipso-load
> 
> diff --git a/tests/inet_socket/Makefile b/tests/inet_socket/Makefile
> index 5bfd561..a0a0d47 100644
> --- a/tests/inet_socket/Makefile
> +++ b/tests/inet_socket/Makefile
> @@ -3,5 +3,8 @@ TARGETS=client server bind connect
>  LDLIBS+= -lselinux
>  
>  all: $(TARGETS)
> + chmod +x *-load
> + chmod +x *-flush

Can't we just fix the created file mode in git and avoid running this
all the time?  Alternatively, run the scripts explicitly via /bin/sh to
remove the dependency on the file mode being set?

> +
>  clean:
>   rm -f $(TARGETS)
> diff --git a/tests/inet_socket/calipso-flush
> b/tests/inet_socket/calipso-flush
> new file mode 100644
> index 000..5143962
> --- /dev/null
> +++ b/tests/inet_socket/calipso-flush
> @@ -0,0 +1,5 @@
> +#!/bin/sh
> +# Reset NetLabel configuration to unlabeled after CALIPSO/IPv6
> tests.
> +netlabelctl map del default
> +netlabelctl calipso del doi:16
> +netlabelctl map add default protocol:unlbl
> diff --git a/tests/inet_socket/calipso-load
> b/tests/inet_socket/calipso-load
> new file mode 100644
> index 000..4bb9c7f
> --- /dev/null
> +++ b/tests/inet_socket/calipso-load
> @@ -0,0 +1,7 @@
> +#!/bin/sh
> +# Define a doi for testing loopback for CALIPSO/IPv6.
> +netlabelctl calipso add pass doi:16
> +netlabelctl map del default
> +netlabelctl map add default address:0.0.0.0/0 protocol:unlbl
> +netlabelctl map add default address:::/0 protocol:unlbl
> +netlabelctl map add default address:::1 protocol:calipso,16
> diff --git a/tests/inet_socket/server.c b/tests/inet_socket/server.c
> index 2801397..f7e38c8 100644
> --- a/tests/inet_socket/server.c
> +++ b/tests/inet_socket/server.c
> @@ -10,6 +10,9 @@
>  #include 
>  #include 
>  
> + /* Defines IPV6_PASSSEC if kernel supports IPV6 cmsg_type */
> +#include 
> +
>  #ifndef SO_PEERSEC
>  #define SO_PEERSEC 31
>  #endif
> @@ -79,11 +82,25 @@ int main(int argc, char **argv)
>   perror("socket");
>   exit(1);
>   }
> - result = setsockopt(sock, SOL_IP, IP_PASSSEC, ,
> sizeof(on));
> - if (result < 0) {
> - perror("setsockopt: SO_PASSSEC");
> - close(sock);
> - exit(1);
> +
> + /* Allow retrival of IPv4 and IPv6 UDP/Datagram security
> contexts */

retrieval

> + if (hints.ai_socktype == SOCK_DGRAM) {
> + result = setsockopt(sock, SOL_IP, IP_PASSSEC, ,
> sizeof(on));
> + if (result < 0) {
> + perror("setsockopt: IP_PASSSEC");
> + close(sock);
> + exit(1);
> + }
> +
> +#ifdef IPV6_PASSSEC
> + result = setsockopt(sock, SOL_IPV6, IPV6_PASSSEC,
> ,
> + sizeof(on));
> + if (result < 0) {
> + perror("setsockopt: IPV6_PASSSEC");
> + close(sock);
> + exit(1);
> + }
> +#endif

Let's minimize use of #ifdef's here and throughout.  Same model as the
kernel; wrap it up in a macro or static inline that can then be
unconditionally called.  Also not sure we want this at all until it is
in mainline or at least Paul's tree?

>   }
>  
>   result = setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, ,
> sizeof(on));
> @@ -176,18 +193,34 @@ int main(int argc, char **argv)
>   }
>   if (nopeer) {
>   strcpy(msglabel, "nopeer");
> - }
> - for (cmsg = CMSG_FIRSTHDR(); cmsg;
> -  cmsg = CMSG_NXTHDR(, cmsg)) {
> - if (cmsg->cmsg_level == SOL_IP &&
> - cmsg->cmsg_type == SCM_SECURITY)
> {
> - size_t len = cmsg->cmsg_len
> - CMSG_LEN(0);
> -
> - if (len > 0 && len <
> sizeof(msglabel)) {
> - memcpy(msglabel,
> CMSG_DATA(cmsg), len);
> - msglabel[len] = 0;
> - printf("%s: Got
> SCM_SECURITY=%s\n",
> - 

Re: [PATCH] selinux-testsuite: Stop Infiniband building if not enabled

[Stephen Smalley]

On Thu, 2017-10-19 at 16:56 +0100, Richard Haines wrote:
> The default is not to test, however it still tries to build
> create_modify_qp.c that requires a header and library that may not
> exist.
> 
> Signed-off-by: Richard Haines 

Thanks, applied.

> ---
>  tests/infiniband_pkey/Makefile | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/infiniband_pkey/Makefile
> b/tests/infiniband_pkey/Makefile
> index 60f0d24..4fa6fb2 100644
> --- a/tests/infiniband_pkey/Makefile
> +++ b/tests/infiniband_pkey/Makefile
> @@ -2,6 +2,14 @@ TARGETS=create_modify_qp
>  
>  LDLIBS+= -libverbs
>  
> -all: $(TARGETS)
> +RESULT=$(shell grep "SELINUX_INFINIBAND_PKEY_TEST=0"
> ./ibpkey_test.conf)
> +
> +ifeq  ($(RESULT), )
> +all: $(TARGETS)
> +else
> +all:
> + @echo "Infiniband test disabled"
> +endif
> +
>  clean:
>   rm -f $(TARGETS)

Re: [PATCH v3] selinux: libselinux: Enable multiple input files to selabel_open.

[William Roberts]

On Mon, Oct 23, 2017 at 9:12 AM, William Roberts
 wrote:
> On Mon, Oct 23, 2017 at 8:57 AM, Dan Cashman  wrote:
>> On 10/20/2017 09:09 AM, William Roberts wrote:
>>>
>>> On Thu, Oct 19, 2017 at 3:12 PM, Nicolas Iooss 
>>> wrote:

 On Thu, Oct 19, 2017 at 9:46 PM, Stephen Smalley 
 wrote:
>
> On Thu, 2017-10-19 at 14:27 -0400, Stephen Smalley wrote:
>>
>> On Thu, 2017-10-19 at 09:25 -0700, William Roberts wrote:
>>>
>>> On Thu, Oct 19, 2017 at 7:26 AM, Stephen Smalley >> wrote:

 On Tue, 2017-10-17 at 09:33 -0700, Daniel Cashman wrote:
>
> [...]
> diff --git a/libselinux/src/label_file.c
> b/libselinux/src/label_file.c
> index 560d8c3d..b3b36bc2 100644
> --- a/libselinux/src/label_file.c
> +++ b/libselinux/src/label_file.c
> @@ -709,28 +709,61 @@ static int init(struct selabel_handle
> *rec,
> const struct selinux_opt *opts,
>unsigned n)
>   {
>struct saved_data *data = (struct saved_data *)rec->data;
> - const char *path = NULL;
> + size_t num_paths = 0;
> + char **path = NULL;
>const char *prefix = NULL;
> - int status = -1, baseonly = 0;
> + int status = -1;
> + size_t i;
> + bool baseonly = false;
> + bool path_provided;
>
>/* Process arguments */
> - while (n--)
> - switch(opts[n].type) {
> + i = n;
> + while (i--)
> + switch(opts[i].type) {
>case SELABEL_OPT_PATH:
> - path = opts[n].value;
> + num_paths++;
>break;
>case SELABEL_OPT_SUBSET:
> - prefix = opts[n].value;
> + prefix = opts[i].value;
>break;
>case SELABEL_OPT_BASEONLY:
> - baseonly = !!opts[n].value;
> + baseonly = !!opts[i].value;
>break;
>}
>
> + if (!num_paths) {
> + num_paths = 1;
> + path_provided = false;
> + } else {
> + path_provided = true;
> + }
> +
> + path = calloc(num_paths, sizeof(*path));
> + if (path == NULL) {
> + goto finish;
> + }
> + rec->spec_files = path;
> + rec->spec_files_len = num_paths;
> +
> + if (path_provided) {
> + for (i = 0; i < n; i++) {
> + switch(opts[i].type) {
> + case SELABEL_OPT_PATH:
> + *path = strdup(opts[i].value);


 Perhaps surprisingly, opts[i].value can be NULL here and some
 callers
 rely on that.  After applying your patch, coreutils,
 selabel_lookup,
 and other userspace programs all seg fault as a result.  The use
 case
 is programs where the selinux_opt structure is declared with a
 SELABEL_OPT_PATH entry whose value is subsequently filled in, and
 left
 NULL if they want to use the default path for file_contexts.
 Internally, libselinux also does this from the
 matchpathcon_init_prefix() function.

 In any event, you need to handle this case.

>>>
>>> Poor Stephen has turned into your test bot :-)
>>>
>>> Does any of the test suite exercise this? Maybe make a PR
>>> on github first to get Travis to test these?
>>
>>
>> Unfortunately the travis-ci tests don't catch this one currently.
>>
>>> Stephen can you share with how your testing this so Dan can follow
>>> suit?
>>
>>
>> In this case, you could build and install to a private directory as
>> per
>> the README, then set your LD_LIBRARY_PATH=~/obj/lib or whatever and
>> run
>> selabel_lookup.
>
>
> Ala:
> $ make DESTDIR=~/obj install
> $ LD_LIBRARY_PATH=~/obj/lib selabel_lookup -b file -k /etc
> Segmentation fault (core dumped)
>
> BTW, I guess this exposes another issue with the patch; there is no
> support for exercising the new code included with it, e.g. an extension
> to utils/selabel_lookup.c to permit specifying multiple input files via
> multiple -f options.  Otherwise, we have no upstream way of testing it.


 For selabel_lookup, it would be possible to write a test script (or a

Re: [PATCH v3] selinux: libselinux: Enable multiple input files to selabel_open.

[William Roberts]

On Mon, Oct 23, 2017 at 8:57 AM, Dan Cashman  wrote:
> On 10/20/2017 09:09 AM, William Roberts wrote:
>>
>> On Thu, Oct 19, 2017 at 3:12 PM, Nicolas Iooss 
>> wrote:
>>>
>>> On Thu, Oct 19, 2017 at 9:46 PM, Stephen Smalley 
>>> wrote:

 On Thu, 2017-10-19 at 14:27 -0400, Stephen Smalley wrote:
>
> On Thu, 2017-10-19 at 09:25 -0700, William Roberts wrote:
>>
>> On Thu, Oct 19, 2017 at 7:26 AM, Stephen Smalley >>
>>>
>> wrote:
>>>
>>> On Tue, 2017-10-17 at 09:33 -0700, Daniel Cashman wrote:

 [...]
 diff --git a/libselinux/src/label_file.c
 b/libselinux/src/label_file.c
 index 560d8c3d..b3b36bc2 100644
 --- a/libselinux/src/label_file.c
 +++ b/libselinux/src/label_file.c
 @@ -709,28 +709,61 @@ static int init(struct selabel_handle
 *rec,
 const struct selinux_opt *opts,
unsigned n)
   {
struct saved_data *data = (struct saved_data *)rec->data;
 - const char *path = NULL;
 + size_t num_paths = 0;
 + char **path = NULL;
const char *prefix = NULL;
 - int status = -1, baseonly = 0;
 + int status = -1;
 + size_t i;
 + bool baseonly = false;
 + bool path_provided;

/* Process arguments */
 - while (n--)
 - switch(opts[n].type) {
 + i = n;
 + while (i--)
 + switch(opts[i].type) {
case SELABEL_OPT_PATH:
 - path = opts[n].value;
 + num_paths++;
break;
case SELABEL_OPT_SUBSET:
 - prefix = opts[n].value;
 + prefix = opts[i].value;
break;
case SELABEL_OPT_BASEONLY:
 - baseonly = !!opts[n].value;
 + baseonly = !!opts[i].value;
break;
}

 + if (!num_paths) {
 + num_paths = 1;
 + path_provided = false;
 + } else {
 + path_provided = true;
 + }
 +
 + path = calloc(num_paths, sizeof(*path));
 + if (path == NULL) {
 + goto finish;
 + }
 + rec->spec_files = path;
 + rec->spec_files_len = num_paths;
 +
 + if (path_provided) {
 + for (i = 0; i < n; i++) {
 + switch(opts[i].type) {
 + case SELABEL_OPT_PATH:
 + *path = strdup(opts[i].value);
>>>
>>>
>>> Perhaps surprisingly, opts[i].value can be NULL here and some
>>> callers
>>> rely on that.  After applying your patch, coreutils,
>>> selabel_lookup,
>>> and other userspace programs all seg fault as a result.  The use
>>> case
>>> is programs where the selinux_opt structure is declared with a
>>> SELABEL_OPT_PATH entry whose value is subsequently filled in, and
>>> left
>>> NULL if they want to use the default path for file_contexts.
>>> Internally, libselinux also does this from the
>>> matchpathcon_init_prefix() function.
>>>
>>> In any event, you need to handle this case.
>>>
>>
>> Poor Stephen has turned into your test bot :-)
>>
>> Does any of the test suite exercise this? Maybe make a PR
>> on github first to get Travis to test these?
>
>
> Unfortunately the travis-ci tests don't catch this one currently.
>
>> Stephen can you share with how your testing this so Dan can follow
>> suit?
>
>
> In this case, you could build and install to a private directory as
> per
> the README, then set your LD_LIBRARY_PATH=~/obj/lib or whatever and
> run
> selabel_lookup.


 Ala:
 $ make DESTDIR=~/obj install
 $ LD_LIBRARY_PATH=~/obj/lib selabel_lookup -b file -k /etc
 Segmentation fault (core dumped)

 BTW, I guess this exposes another issue with the patch; there is no
 support for exercising the new code included with it, e.g. an extension
 to utils/selabel_lookup.c to permit specifying multiple input files via
 multiple -f options.  Otherwise, we have no upstream way of testing it.
>>>
>>>
>>> For selabel_lookup, it would be possible to write a test script (or a
>>> CUnit-based test program like libsepol and libsemanage have) which
>>> does not need an SELinux policy to be installed on the system, thanks
>>> to option -f.
>>> Nevertheless other parts of libselinux (and 

Re: [PATCH v3] selinux: libselinux: Enable multiple input files to selabel_open.

[Dan Cashman]

On 10/23/2017 09:20 AM, William Roberts wrote:

On Mon, Oct 23, 2017 at 9:12 AM, William Roberts
 wrote:

On Mon, Oct 23, 2017 at 8:57 AM, Dan Cashman  wrote:

On 10/20/2017 09:09 AM, William Roberts wrote:


On Thu, Oct 19, 2017 at 3:12 PM, Nicolas Iooss 
wrote:


On Thu, Oct 19, 2017 at 9:46 PM, Stephen Smalley 
wrote:


On Thu, 2017-10-19 at 14:27 -0400, Stephen Smalley wrote:


On Thu, 2017-10-19 at 09:25 -0700, William Roberts wrote:


On Thu, Oct 19, 2017 at 7:26 AM, Stephen Smalley data;
- const char *path = NULL;
+ size_t num_paths = 0;
+ char **path = NULL;
const char *prefix = NULL;
- int status = -1, baseonly = 0;
+ int status = -1;
+ size_t i;
+ bool baseonly = false;
+ bool path_provided;

/* Process arguments */
- while (n--)
- switch(opts[n].type) {
+ i = n;
+ while (i--)
+ switch(opts[i].type) {
case SELABEL_OPT_PATH:
- path = opts[n].value;
+ num_paths++;
break;
case SELABEL_OPT_SUBSET:
- prefix = opts[n].value;
+ prefix = opts[i].value;
break;
case SELABEL_OPT_BASEONLY:
- baseonly = !!opts[n].value;
+ baseonly = !!opts[i].value;
break;
}

+ if (!num_paths) {
+ num_paths = 1;
+ path_provided = false;
+ } else {
+ path_provided = true;
+ }
+
+ path = calloc(num_paths, sizeof(*path));
+ if (path == NULL) {
+ goto finish;
+ }
+ rec->spec_files = path;
+ rec->spec_files_len = num_paths;
+
+ if (path_provided) {
+ for (i = 0; i < n; i++) {
+ switch(opts[i].type) {
+ case SELABEL_OPT_PATH:
+ *path = strdup(opts[i].value);



Perhaps surprisingly, opts[i].value can be NULL here and some
callers
rely on that.  After applying your patch, coreutils,
selabel_lookup,
and other userspace programs all seg fault as a result.  The use
case
is programs where the selinux_opt structure is declared with a
SELABEL_OPT_PATH entry whose value is subsequently filled in, and
left
NULL if they want to use the default path for file_contexts.
Internally, libselinux also does this from the
matchpathcon_init_prefix() function.

In any event, you need to handle this case.



Poor Stephen has turned into your test bot :-)

Does any of the test suite exercise this? Maybe make a PR
on github first to get Travis to test these?



Unfortunately the travis-ci tests don't catch this one currently.


Stephen can you share with how your testing this so Dan can follow
suit?



In this case, you could build and install to a private directory as
per
the README, then set your LD_LIBRARY_PATH=~/obj/lib or whatever and
run
selabel_lookup.



Ala:
$ make DESTDIR=~/obj install
$ LD_LIBRARY_PATH=~/obj/lib selabel_lookup -b file -k /etc
Segmentation fault (core dumped)

BTW, I guess this exposes another issue with the patch; there is no
support for exercising the new code included with it, e.g. an extension
to utils/selabel_lookup.c to permit specifying multiple input files via
multiple -f options.  Otherwise, we have no upstream way of testing it.



For selabel_lookup, it would be possible to write a test script (or a
CUnit-based test program like libsepol and libsemanage have) which
does not need an SELinux policy to be installed on the system, thanks
to option -f.
Nevertheless other parts of libselinux (and programs) can not
currently be tested on a system without SELinux, as /etc/selinux and
/sys/fs/selinux are needed. If we want to automatize the testing of
such parts in a CI like Travis-CI, I suppose we would need to run a
virtual machine with its own filesystem, a kernel with SELinux
enabled, and some programs like coreutils compiled with the tested
version of libselinux/libsepol/... As implementing such an idea may
take quite a long time, would there be simpler ways to perform
automatic testing of libselinux? Perhaps using another continuous
integration system?


We have travis up and running, but it's not excising this code path. Any PR
submitted to the selinux project on githib has travis "test" it via
the .travis.yml
file in the root of the tree.

If you have a fork of selinux on github you 

Re: [PATCH v3] selinux: libselinux: Enable multiple input files to selabel_open.

[Dan Cashman]

On 10/20/2017 09:09 AM, William Roberts wrote:

On Thu, Oct 19, 2017 at 3:12 PM, Nicolas Iooss  wrote:

On Thu, Oct 19, 2017 at 9:46 PM, Stephen Smalley  wrote:

On Thu, 2017-10-19 at 14:27 -0400, Stephen Smalley wrote:

On Thu, 2017-10-19 at 09:25 -0700, William Roberts wrote:

On Thu, Oct 19, 2017 at 7:26 AM, Stephen Smalley data;
- const char *path = NULL;
+ size_t num_paths = 0;
+ char **path = NULL;
   const char *prefix = NULL;
- int status = -1, baseonly = 0;
+ int status = -1;
+ size_t i;
+ bool baseonly = false;
+ bool path_provided;

   /* Process arguments */
- while (n--)
- switch(opts[n].type) {
+ i = n;
+ while (i--)
+ switch(opts[i].type) {
   case SELABEL_OPT_PATH:
- path = opts[n].value;
+ num_paths++;
   break;
   case SELABEL_OPT_SUBSET:
- prefix = opts[n].value;
+ prefix = opts[i].value;
   break;
   case SELABEL_OPT_BASEONLY:
- baseonly = !!opts[n].value;
+ baseonly = !!opts[i].value;
   break;
   }

+ if (!num_paths) {
+ num_paths = 1;
+ path_provided = false;
+ } else {
+ path_provided = true;
+ }
+
+ path = calloc(num_paths, sizeof(*path));
+ if (path == NULL) {
+ goto finish;
+ }
+ rec->spec_files = path;
+ rec->spec_files_len = num_paths;
+
+ if (path_provided) {
+ for (i = 0; i < n; i++) {
+ switch(opts[i].type) {
+ case SELABEL_OPT_PATH:
+ *path = strdup(opts[i].value);


Perhaps surprisingly, opts[i].value can be NULL here and some
callers
rely on that.  After applying your patch, coreutils,
selabel_lookup,
and other userspace programs all seg fault as a result.  The use
case
is programs where the selinux_opt structure is declared with a
SELABEL_OPT_PATH entry whose value is subsequently filled in, and
left
NULL if they want to use the default path for file_contexts.
Internally, libselinux also does this from the
matchpathcon_init_prefix() function.

In any event, you need to handle this case.



Poor Stephen has turned into your test bot :-)

Does any of the test suite exercise this? Maybe make a PR
on github first to get Travis to test these?


Unfortunately the travis-ci tests don't catch this one currently.


Stephen can you share with how your testing this so Dan can follow
suit?


In this case, you could build and install to a private directory as
per
the README, then set your LD_LIBRARY_PATH=~/obj/lib or whatever and
run
selabel_lookup.


Ala:
$ make DESTDIR=~/obj install
$ LD_LIBRARY_PATH=~/obj/lib selabel_lookup -b file -k /etc
Segmentation fault (core dumped)

BTW, I guess this exposes another issue with the patch; there is no
support for exercising the new code included with it, e.g. an extension
to utils/selabel_lookup.c to permit specifying multiple input files via
multiple -f options.  Otherwise, we have no upstream way of testing it.


For selabel_lookup, it would be possible to write a test script (or a
CUnit-based test program like libsepol and libsemanage have) which
does not need an SELinux policy to be installed on the system, thanks
to option -f.
Nevertheless other parts of libselinux (and programs) can not
currently be tested on a system without SELinux, as /etc/selinux and
/sys/fs/selinux are needed. If we want to automatize the testing of
such parts in a CI like Travis-CI, I suppose we would need to run a
virtual machine with its own filesystem, a kernel with SELinux
enabled, and some programs like coreutils compiled with the tested
version of libselinux/libsepol/... As implementing such an idea may
take quite a long time, would there be simpler ways to perform
automatic testing of libselinux? Perhaps using another continuous
integration system?


The free travis just provides an Ubuntu image if you use sudo: true
you get a full VM if you don't, you get a docker container.

With that said, we could try to hack up the VM image to support what we need,
or perhaps we can use CMockaand mock out the filesystem calls to proc/selinuxfs
with something that replicates it.(not sure if that's how mock objects work)




Cheers,
Nicolas





Is there selinux-specific travis documentation, or a pointer to 

Re: [RFC PATCH 1/2] security, capabilities: Add CAP_SYS_MOUNT

[Stephen Smalley]

On Sat, 2017-10-21 at 15:43 +0200, Nicolas Belouin wrote:
> With CAP_SYS_ADMIN being bloated and inapropriate for actions such
> as mounting/unmounting filesystems, the creation of a new capability
> is needed.
> CAP_SYS_MOUNT is meant to give a process the ability to call for
> mount,
> umount and umount2 syscalls.

If adding a new capability isn't deemed acceptable, then another option
would be to introduce LSM hooks where there isn't already coverage and
implement finer-grained permission checks there.  In some cases, that
already occurs for mount and umount*.  That also offers the possibility
of taking the object of the operation into account, unlike capabilities
which are only subject/process-based.


> 
> Signed-off-by: Nicolas Belouin 
> ---
>  include/uapi/linux/capability.h | 5 -
>  security/selinux/include/classmap.h | 4 ++--
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/include/uapi/linux/capability.h
> b/include/uapi/linux/capability.h
> index 230e05d35191..ce230aa6d928 100644
> --- a/include/uapi/linux/capability.h
> +++ b/include/uapi/linux/capability.h
> @@ -365,8 +365,11 @@ struct vfs_ns_cap_data {
>  
>  #define CAP_AUDIT_READ   37
>  
> +/* Allow mounting, unmounting filesystems */
>  
> -#define CAP_LAST_CAP CAP_AUDIT_READ
> +#define CAP_SYS_MOUNT38
> +
> +#define CAP_LAST_CAP CAP_SYS_MOUNT
>  
>  #define cap_valid(x) ((x) >= 0 && (x) <= CAP_LAST_CAP)
>  
> diff --git a/security/selinux/include/classmap.h
> b/security/selinux/include/classmap.h
> index 35ffb29a69cb..a873dce97fd5 100644
> --- a/security/selinux/include/classmap.h
> +++ b/security/selinux/include/classmap.h
> @@ -24,9 +24,9 @@
>   "audit_control", "setfcap"
>  
>  #define COMMON_CAP2_PERMS  "mac_override", "mac_admin", "syslog", \
> - "wake_alarm", "block_suspend", "audit_read"
> + "wake_alarm", "block_suspend", "audit_read",
> "sys_mount"
>  
> -#if CAP_LAST_CAP > CAP_AUDIT_READ
> +#if CAP_LAST_CAP > CAP_SYS_MOUNT
>  #error New capability defined, please update COMMON_CAP2_PERMS.
>  #endif
>  

Re: [PATCH] libselinux: android: support exact match for a property key

[Jaekyun Seok via Selinux]

Please see my inline comments.

Thanks,

On Sat, Oct 21, 2017 at 1:02 AM, William Roberts 
wrote:

> On Fri, Oct 20, 2017 at 7:54 AM, Jeffrey Vander Stoep via Selinux
>  wrote:
> > Please hold off on submission. We're discussing if this is really
> necessary.
>
> Yeah I'd like to hear about what issues the current longest match
> logic was causing
> in the commit message.
>

I am working to whitelist properties which should be restricted from being
accessed by some components.

To do that, exact match should be supported.


>
> >
> > On Thu, Oct 19, 2017 at 4:49 PM, Jaekyun Seok via Selinux
> >  wrote:
> >> Performs exact match if a property key of property contexts ends with
> '$'
> >> instead of prefix match.
>
> This seems like an overly verbose way to accomplish exact match. The
> property_contexts
> file has things like:
>
> *  <-- match everything
> foo.bar.   <- match prefix foo.bar. properties
> foo.bar.baz <-- currently matches foo.bar.baz, foo.bar.bazbaz, etc. No
> trailing .
> could be changed to mean exact match.
>
> Really what you would want is that if it doesn't end with a dot, don't
> do a prefix
> match. No need to add the $ semantic AFAICT.
>

Sounds good to me. I will discuss this way internally.


>
> >>
> >> This will enable to define an exact rule which can avoid unexpected
> >> context assignment.
> >>
> >> Signed-off-by: Jaekyun Seok 
> >> ---
> >>  libselinux/src/label_backends_android.c | 9 +++--
> >>  1 file changed, 7 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/libselinux/src/label_backends_android.c
> b/libselinux/src/label_backends_android.c
> >> index cb8aae26..4611d396 100644
> >> --- a/libselinux/src/label_backends_android.c
> >> +++ b/libselinux/src/label_backends_android.c
> >> @@ -258,8 +258,13 @@ static struct selabel_lookup_rec
> *property_lookup(struct selabel_handle *rec,
> >> }
> >>
> >> for (i = 0; i < data->nspec; i++) {
> >> -   if (strncmp(spec_arr[i].property_key, key,
> >> -   strlen(spec_arr[i].property_key)) == 0) {
> >> +   size_t property_key_len = strlen(spec_arr[i].property_
> key);
> >> +   if (spec_arr[i].property_key[property_key_len - 1] ==
> '$' &&
> >> +   strlen(key) == property_key_len - 1 &&
> >> +   strncmp(spec_arr[i].property_key, key,
> property_key_len - 1) == 0) {
> >> +   break;
> >> +   }
> >> +   if (strncmp(spec_arr[i].property_key, key,
> property_key_len) == 0) {
> >> break;
> >> }
> >> if (strncmp(spec_arr[i].property_key, "*", 1) == 0)
> >> --
> >> 2.15.0.rc0.271.g36b669edcc-goog
> >>
> >>
> >
>
>
>
> --
> Respectfully,
>
> William C Roberts
>



-- 
Jaekyun Seok | Software Engineer | jaek...@google.com | +82 2 531 9235

Re: [kernel-hardening] [RFC PATCH 1/2] security, capabilities: Add CAP_SYS_MOUNT

[Casey Schaufler]

On 10/21/2017 11:41 AM, Nicolas Belouin wrote:
>
> On October 21, 2017 7:31:24 PM GMT+02:00, Casey Schaufler 
>  wrote:
>> On 10/21/2017 6:43 AM, Nicolas Belouin wrote:
>>> With CAP_SYS_ADMIN being bloated and inapropriate for actions such
>>> as mounting/unmounting filesystems, the creation of a new capability
>>> is needed.
>>> CAP_SYS_MOUNT is meant to give a process the ability to call for
>> mount,
>>> umount and umount2 syscalls.
>> This is increased granularity for it's own sake. There is no
>> compelling reason to break out this capability in particular.
> Obviously there is a need to break CAP_SYS_ADMIN in pieces,

No. This is a baseless assumption. Granularity for the sake of
granularity is bad. Data General (a dead company) followed the
fine grained capability path and ended up with 330 capabilities.
Developers can't handle the granularity we already have (hell,
half of them don't know what mode bits are for) and making it
finer will only make it harder for them to make use of the ones
we have.

>  to do so, you have to start somewhere, so I chose to begin with this.
>
>> Can you identify existing use cases where you would have
>> CAP_SYS_MOUNT without also having CAP_SYS_ADMIN? I should think
>> that all the work that's gone into unprivileged mounts over
>> the past couple years would make this unnecessary.
> If you look at the udiskd deamon used by most desktop environments, it is 
> launched as root or at least with CAP_SYS_ADMIN. Here, you could use 
> CAP_SYS_MOUNT.

Does this demon do anything else that uses CAP_SYS_ADMIN?
If it has to have that anyway, it's not an argument for breaking
out the mount capability.
 

>  There might also be a use within containers as you don't want to give 
> CAP_SYS_ADMIN to a container if it just need to mount/unmount filesystems.

There is massive work going on elsewhere to allow containers to
mount without privilege. And I'm not at all interested in "might".

> If you go even further, it could be used to allow swapon/swapoff (maybe in 
> future patch set).

No, you'd be asking for CAP_SWAP for that. Swap control has nothing to do
with mounting filesystems.

>
>>> Signed-off-by: Nicolas Belouin 
>>> ---
>>>  include/uapi/linux/capability.h | 5 -
>>>  security/selinux/include/classmap.h | 4 ++--
>>>  2 files changed, 6 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/include/uapi/linux/capability.h
>> b/include/uapi/linux/capability.h
>>> index 230e05d35191..ce230aa6d928 100644
>>> --- a/include/uapi/linux/capability.h
>>> +++ b/include/uapi/linux/capability.h
>>> @@ -365,8 +365,11 @@ struct vfs_ns_cap_data {
>>>  
>>>  #define CAP_AUDIT_READ 37
>>>  
>>> +/* Allow mounting, unmounting filesystems */
>>>  
>>> -#define CAP_LAST_CAP CAP_AUDIT_READ
>>> +#define CAP_SYS_MOUNT  38
>>> +
>>> +#define CAP_LAST_CAP CAP_SYS_MOUNT
>>>  
>>>  #define cap_valid(x) ((x) >= 0 && (x) <= CAP_LAST_CAP)
>>>  
>>> diff --git a/security/selinux/include/classmap.h
>> b/security/selinux/include/classmap.h
>>> index 35ffb29a69cb..a873dce97fd5 100644
>>> --- a/security/selinux/include/classmap.h
>>> +++ b/security/selinux/include/classmap.h
>>> @@ -24,9 +24,9 @@
>>> "audit_control", "setfcap"
>>>  
>>>  #define COMMON_CAP2_PERMS  "mac_override", "mac_admin", "syslog", \
>>> -   "wake_alarm", "block_suspend", "audit_read"
>>> +   "wake_alarm", "block_suspend", "audit_read", "sys_mount"
>>>  
>>> -#if CAP_LAST_CAP > CAP_AUDIT_READ
>>> +#if CAP_LAST_CAP > CAP_SYS_MOUNT
>>>  #error New capability defined, please update COMMON_CAP2_PERMS.
>>>  #endif
>>>  
> Nicolas
>

Re: [RFC PATCH 1/2] security, capabilities: create CAP_TRUSTED

[nicolas]

,linux-e...@vger.kernel.org,linux-ker...@vger.kernel.org,linux-f2fs-de...@lists.sourceforge.net,linux-fsde...@vger.kernel.org,linux-...@lists.infradead.org,jfs-discuss...@lists.sourceforge.net,ocfs2-de...@oss.oracle.com,linux-unio...@vger.kernel.org,reiserfs-de...@vger.kernel.org,linux-security-mod...@vger.kernel.org,selinux@tycho.nsa.gov,linux-...@vger.kernel.org,kernel-harden...@lists.openwall.com
From: Nicolas Belouin 
Message-ID: <99179b10-4eae-4fab-9d14-b88515626...@belouin.fr>



On October 21, 2017 6:03:02 PM GMT+02:00, "Serge E. Hallyn"  
wrote:
>Quoting Nicolas Belouin (nico...@belouin.fr):
>> with CAP_SYS_ADMIN being bloated, the usefulness of using it to
>> flag a process to be entrusted for e.g reading and writing trusted
>> xattr is near zero.
>> CAP_TRUSTED aims to provide userland with a way to mark a process as
>> entrusted to do specific (not specially admin-centered) actions. It
>> would for example allow a process to red/write the trusted xattrs.
>
>You say "for example".  Are you intending to add more uses?  If so,
>what
>are they?  If not, how about renaming it CAP_TRUSTED_XATTR?
>

I don't see any other use for now, but I don't want it to be too narrow and non 
usable in a similar context in the future. So I believe the underlying purpose 
of marking a process as "trusted" (even if for now it only means rw permission 
on trusted xattr) is more meaningful.

>What all does allowing writes to trusted xattrs give you?  There are
>the overlayfs whiteouts, what else?

Nicolas

Re: [kernel-hardening] [RFC PATCH 1/2] security, capabilities: create CAP_TRUSTED

[nicolas]

,linux-e...@vger.kernel.org,linux-ker...@vger.kernel.org,linux-f2fs-de...@lists.sourceforge.net,linux-fsde...@vger.kernel.org,linux-...@lists.infradead.org,jfs-discuss...@lists.sourceforge.net,ocfs2-de...@oss.oracle.com,linux-unio...@vger.kernel.org,reiserfs-de...@vger.kernel.org,linux-security-mod...@vger.kernel.org,selinux@tycho.nsa.gov,linux-...@vger.kernel.org,kernel-harden...@lists.openwall.com
From: Nicolas Belouin 
Message-ID: 



On October 21, 2017 7:25:21 PM GMT+02:00, Casey Schaufler 
 wrote:
>On 10/21/2017 6:45 AM, Nicolas Belouin wrote:
>> with CAP_SYS_ADMIN being bloated, the usefulness of using it to
>> flag a process to be entrusted for e.g reading and writing trusted
>> xattr is near zero.
>> CAP_TRUSTED aims to provide userland with a way to mark a process as
>> entrusted to do specific (not specially admin-centered) actions. It
>> would for example allow a process to red/write the trusted xattrs.
>
>Please explain how this is different from CAP_MAC_ADMIN in
>any existing use case. If it is significantly different, how
>would the two interact?

>From my point of view, CAP_MAC_ADMIN allows one to read/write security xattrs, 
>those are meant to describe security policies. As far as I know of, trusted 
>xattrs are intended for a privileged process to read or write arbitrary data. 
>I don't have any real world example in mind that use trusted xattrs, but I'll 
>try to find one.

>
>> Signed-off-by: Nicolas Belouin 
>> ---
>>  include/uapi/linux/capability.h | 6 +-
>>  security/selinux/include/classmap.h | 5 +++--
>>  2 files changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/uapi/linux/capability.h
>b/include/uapi/linux/capability.h
>> index ce230aa6d928..27e457b93c84 100644
>> --- a/include/uapi/linux/capability.h
>> +++ b/include/uapi/linux/capability.h
>> @@ -369,7 +369,11 @@ struct vfs_ns_cap_data {
>>  
>>  #define CAP_SYS_MOUNT   38
>>  
>> -#define CAP_LAST_CAP CAP_SYS_MOUNT
>> +/* Allow read/write trusted xattr */
>> +
>> +#define CAP_TRUSTED 39
>> +
>> +#define CAP_LAST_CAP CAP_TRUSTED
>>  
>>  #define cap_valid(x) ((x) >= 0 && (x) <= CAP_LAST_CAP)
>>  
>> diff --git a/security/selinux/include/classmap.h
>b/security/selinux/include/classmap.h
>> index a873dce97fd5..f5dc8e109f5a 100644
>> --- a/security/selinux/include/classmap.h
>> +++ b/security/selinux/include/classmap.h
>> @@ -24,9 +24,10 @@
>>  "audit_control", "setfcap"
>>  
>>  #define COMMON_CAP2_PERMS  "mac_override", "mac_admin", "syslog", \
>> -"wake_alarm", "block_suspend", "audit_read", "sys_mount"
>> +"wake_alarm", "block_suspend", "audit_read", "sys_mount", \
>> +"trusted"
>>  
>> -#if CAP_LAST_CAP > CAP_SYS_MOUNT
>> +#if CAP_LAST_CAP > CAP_TRUSTED
>>  #error New capability defined, please update COMMON_CAP2_PERMS.
>>  #endif
>>  

Nicolas

Re: [kernel-hardening] [RFC PATCH 1/2] security, capabilities: Add CAP_SYS_MOUNT

[Nicolas Belouin]

On October 21, 2017 7:31:24 PM GMT+02:00, Casey Schaufler 
 wrote:
>On 10/21/2017 6:43 AM, Nicolas Belouin wrote:
>> With CAP_SYS_ADMIN being bloated and inapropriate for actions such
>> as mounting/unmounting filesystems, the creation of a new capability
>> is needed.
>> CAP_SYS_MOUNT is meant to give a process the ability to call for
>mount,
>> umount and umount2 syscalls.
>
>This is increased granularity for it's own sake. There is no
>compelling reason to break out this capability in particular.

Obviously there is a need to break CAP_SYS_ADMIN in pieces, to do so, you have 
to start somewhere, so I chose to begin with this.

>Can you identify existing use cases where you would have
>CAP_SYS_MOUNT without also having CAP_SYS_ADMIN? I should think
>that all the work that's gone into unprivileged mounts over
>the past couple years would make this unnecessary.

If you look at the udiskd deamon used by most desktop environments, it is 
launched as root or at least with CAP_SYS_ADMIN. Here, you could use 
CAP_SYS_MOUNT. There might also be a use within containers as you don't want to 
give CAP_SYS_ADMIN to a container if it just need to mount/unmount filesystems. 
If you go even further, it could be used to allow swapon/swapoff (maybe in 
future patch set).

>
>> Signed-off-by: Nicolas Belouin 
>> ---
>>  include/uapi/linux/capability.h | 5 -
>>  security/selinux/include/classmap.h | 4 ++--
>>  2 files changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/uapi/linux/capability.h
>b/include/uapi/linux/capability.h
>> index 230e05d35191..ce230aa6d928 100644
>> --- a/include/uapi/linux/capability.h
>> +++ b/include/uapi/linux/capability.h
>> @@ -365,8 +365,11 @@ struct vfs_ns_cap_data {
>>  
>>  #define CAP_AUDIT_READ  37
>>  
>> +/* Allow mounting, unmounting filesystems */
>>  
>> -#define CAP_LAST_CAP CAP_AUDIT_READ
>> +#define CAP_SYS_MOUNT   38
>> +
>> +#define CAP_LAST_CAP CAP_SYS_MOUNT
>>  
>>  #define cap_valid(x) ((x) >= 0 && (x) <= CAP_LAST_CAP)
>>  
>> diff --git a/security/selinux/include/classmap.h
>b/security/selinux/include/classmap.h
>> index 35ffb29a69cb..a873dce97fd5 100644
>> --- a/security/selinux/include/classmap.h
>> +++ b/security/selinux/include/classmap.h
>> @@ -24,9 +24,9 @@
>>  "audit_control", "setfcap"
>>  
>>  #define COMMON_CAP2_PERMS  "mac_override", "mac_admin", "syslog", \
>> -"wake_alarm", "block_suspend", "audit_read"
>> +"wake_alarm", "block_suspend", "audit_read", "sys_mount"
>>  
>> -#if CAP_LAST_CAP > CAP_AUDIT_READ
>> +#if CAP_LAST_CAP > CAP_SYS_MOUNT
>>  #error New capability defined, please update COMMON_CAP2_PERMS.
>>  #endif
>>  

Nicolas

Re: [kernel-hardening] [RFC PATCH 1/2] security, capabilities: Add CAP_SYS_MOUNT

[Casey Schaufler]

On 10/21/2017 6:43 AM, Nicolas Belouin wrote:
> With CAP_SYS_ADMIN being bloated and inapropriate for actions such
> as mounting/unmounting filesystems, the creation of a new capability
> is needed.
> CAP_SYS_MOUNT is meant to give a process the ability to call for mount,
> umount and umount2 syscalls.

This is increased granularity for it's own sake. There is no
compelling reason to break out this capability in particular.
Can you identify existing use cases where you would have
CAP_SYS_MOUNT without also having CAP_SYS_ADMIN? I should think
that all the work that's gone into unprivileged mounts over
the past couple years would make this unnecessary.

> Signed-off-by: Nicolas Belouin 
> ---
>  include/uapi/linux/capability.h | 5 -
>  security/selinux/include/classmap.h | 4 ++--
>  2 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/include/uapi/linux/capability.h b/include/uapi/linux/capability.h
> index 230e05d35191..ce230aa6d928 100644
> --- a/include/uapi/linux/capability.h
> +++ b/include/uapi/linux/capability.h
> @@ -365,8 +365,11 @@ struct vfs_ns_cap_data {
>  
>  #define CAP_AUDIT_READ   37
>  
> +/* Allow mounting, unmounting filesystems */
>  
> -#define CAP_LAST_CAP CAP_AUDIT_READ
> +#define CAP_SYS_MOUNT38
> +
> +#define CAP_LAST_CAP CAP_SYS_MOUNT
>  
>  #define cap_valid(x) ((x) >= 0 && (x) <= CAP_LAST_CAP)
>  
> diff --git a/security/selinux/include/classmap.h 
> b/security/selinux/include/classmap.h
> index 35ffb29a69cb..a873dce97fd5 100644
> --- a/security/selinux/include/classmap.h
> +++ b/security/selinux/include/classmap.h
> @@ -24,9 +24,9 @@
>   "audit_control", "setfcap"
>  
>  #define COMMON_CAP2_PERMS  "mac_override", "mac_admin", "syslog", \
> - "wake_alarm", "block_suspend", "audit_read"
> + "wake_alarm", "block_suspend", "audit_read", "sys_mount"
>  
> -#if CAP_LAST_CAP > CAP_AUDIT_READ
> +#if CAP_LAST_CAP > CAP_SYS_MOUNT
>  #error New capability defined, please update COMMON_CAP2_PERMS.
>  #endif
>  

Re: [RFC PATCH 1/2] security, capabilities: create CAP_TRUSTED

[Serge E. Hallyn]

Quoting Nicolas Belouin (nico...@belouin.fr):
> with CAP_SYS_ADMIN being bloated, the usefulness of using it to
> flag a process to be entrusted for e.g reading and writing trusted
> xattr is near zero.
> CAP_TRUSTED aims to provide userland with a way to mark a process as
> entrusted to do specific (not specially admin-centered) actions. It
> would for example allow a process to red/write the trusted xattrs.

You say "for example".  Are you intending to add more uses?  If so, what
are they?  If not, how about renaming it CAP_TRUSTED_XATTR?

What all does allowing writes to trusted xattrs give you?  There are
the overlayfs whiteouts, what else?

Re: [kernel-hardening] [RFC PATCH 1/2] security, capabilities: create CAP_TRUSTED

[Casey Schaufler]

On 10/21/2017 6:45 AM, Nicolas Belouin wrote:
> with CAP_SYS_ADMIN being bloated, the usefulness of using it to
> flag a process to be entrusted for e.g reading and writing trusted
> xattr is near zero.
> CAP_TRUSTED aims to provide userland with a way to mark a process as
> entrusted to do specific (not specially admin-centered) actions. It
> would for example allow a process to red/write the trusted xattrs.

Please explain how this is different from CAP_MAC_ADMIN in
any existing use case. If it is significantly different, how
would the two interact?

> Signed-off-by: Nicolas Belouin 
> ---
>  include/uapi/linux/capability.h | 6 +-
>  security/selinux/include/classmap.h | 5 +++--
>  2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/include/uapi/linux/capability.h b/include/uapi/linux/capability.h
> index ce230aa6d928..27e457b93c84 100644
> --- a/include/uapi/linux/capability.h
> +++ b/include/uapi/linux/capability.h
> @@ -369,7 +369,11 @@ struct vfs_ns_cap_data {
>  
>  #define CAP_SYS_MOUNT38
>  
> -#define CAP_LAST_CAP CAP_SYS_MOUNT
> +/* Allow read/write trusted xattr */
> +
> +#define CAP_TRUSTED  39
> +
> +#define CAP_LAST_CAP CAP_TRUSTED
>  
>  #define cap_valid(x) ((x) >= 0 && (x) <= CAP_LAST_CAP)
>  
> diff --git a/security/selinux/include/classmap.h 
> b/security/selinux/include/classmap.h
> index a873dce97fd5..f5dc8e109f5a 100644
> --- a/security/selinux/include/classmap.h
> +++ b/security/selinux/include/classmap.h
> @@ -24,9 +24,10 @@
>   "audit_control", "setfcap"
>  
>  #define COMMON_CAP2_PERMS  "mac_override", "mac_admin", "syslog", \
> - "wake_alarm", "block_suspend", "audit_read", "sys_mount"
> + "wake_alarm", "block_suspend", "audit_read", "sys_mount", \
> + "trusted"
>  
> -#if CAP_LAST_CAP > CAP_SYS_MOUNT
> +#if CAP_LAST_CAP > CAP_TRUSTED
>  #error New capability defined, please update COMMON_CAP2_PERMS.
>  #endif
>  

[RFC PATCH 1/2] security, capabilities: create CAP_TRUSTED

[Nicolas Belouin]

with CAP_SYS_ADMIN being bloated, the usefulness of using it to
flag a process to be entrusted for e.g reading and writing trusted
xattr is near zero.
CAP_TRUSTED aims to provide userland with a way to mark a process as
entrusted to do specific (not specially admin-centered) actions. It
would for example allow a process to red/write the trusted xattrs.

Signed-off-by: Nicolas Belouin 
---
 include/uapi/linux/capability.h | 6 +-
 security/selinux/include/classmap.h | 5 +++--
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/include/uapi/linux/capability.h b/include/uapi/linux/capability.h
index ce230aa6d928..27e457b93c84 100644
--- a/include/uapi/linux/capability.h
+++ b/include/uapi/linux/capability.h
@@ -369,7 +369,11 @@ struct vfs_ns_cap_data {
 
 #define CAP_SYS_MOUNT  38
 
-#define CAP_LAST_CAP CAP_SYS_MOUNT
+/* Allow read/write trusted xattr */
+
+#define CAP_TRUSTED39
+
+#define CAP_LAST_CAP CAP_TRUSTED
 
 #define cap_valid(x) ((x) >= 0 && (x) <= CAP_LAST_CAP)
 
diff --git a/security/selinux/include/classmap.h 
b/security/selinux/include/classmap.h
index a873dce97fd5..f5dc8e109f5a 100644
--- a/security/selinux/include/classmap.h
+++ b/security/selinux/include/classmap.h
@@ -24,9 +24,10 @@
"audit_control", "setfcap"
 
 #define COMMON_CAP2_PERMS  "mac_override", "mac_admin", "syslog", \
-   "wake_alarm", "block_suspend", "audit_read", "sys_mount"
+   "wake_alarm", "block_suspend", "audit_read", "sys_mount", \
+   "trusted"
 
-#if CAP_LAST_CAP > CAP_SYS_MOUNT
+#if CAP_LAST_CAP > CAP_TRUSTED
 #error New capability defined, please update COMMON_CAP2_PERMS.
 #endif
 
-- 
2.14.2

[RFC PATCH 2/2] fs: Grant CAP_TRUSTED rw access to trusted xattrs

[Nicolas Belouin]

The trusted xattrs read/write access is the perfect use case for
CAP_TRUSTED.

Signed-off-by: Nicolas Belouin 
---
 fs/ext2/xattr_trusted.c |  2 +-
 fs/ext4/xattr_trusted.c |  2 +-
 fs/f2fs/xattr.c |  6 +++---
 fs/hfsplus/xattr.c  |  2 +-
 fs/jffs2/xattr_trusted.c|  2 +-
 fs/jfs/xattr.c  |  3 ++-
 fs/ocfs2/xattr.c|  2 +-
 fs/overlayfs/inode.c|  3 ++-
 fs/reiserfs/xattr_trusted.c | 11 ---
 fs/squashfs/xattr.c |  2 +-
 fs/ubifs/xattr.c|  3 ++-
 fs/xattr.c  |  4 ++--
 12 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/fs/ext2/xattr_trusted.c b/fs/ext2/xattr_trusted.c
index 65049b71af13..8006c24f8ee0 100644
--- a/fs/ext2/xattr_trusted.c
+++ b/fs/ext2/xattr_trusted.c
@@ -11,7 +11,7 @@
 static bool
 ext2_xattr_trusted_list(struct dentry *dentry)
 {
-   return capable(CAP_SYS_ADMIN);
+   return capable(CAP_SYS_ADMIN) || capable(CAP_TRUSTED);
 }
 
 static int
diff --git a/fs/ext4/xattr_trusted.c b/fs/ext4/xattr_trusted.c
index c7765c735714..9849d1e0ebb9 100644
--- a/fs/ext4/xattr_trusted.c
+++ b/fs/ext4/xattr_trusted.c
@@ -15,7 +15,7 @@
 static bool
 ext4_xattr_trusted_list(struct dentry *dentry)
 {
-   return capable(CAP_SYS_ADMIN);
+   return capable(CAP_SYS_ADMIN) || capable(CAP_TRUSTED);
 }
 
 static int
diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
index 7c65540148f8..ef572464deca 100644
--- a/fs/f2fs/xattr.c
+++ b/fs/f2fs/xattr.c
@@ -37,7 +37,7 @@ static int f2fs_xattr_generic_get(const struct xattr_handler 
*handler,
return -EOPNOTSUPP;
break;
case F2FS_XATTR_INDEX_TRUSTED:
-   if (!capable(CAP_SYS_ADMIN))
+   if (!capable(CAP_SYS_ADMIN) && !capable(CAP_TRUSTED))
return -EPERM;
break;
case F2FS_XATTR_INDEX_SECURITY:
@@ -62,7 +62,7 @@ static int f2fs_xattr_generic_set(const struct xattr_handler 
*handler,
return -EOPNOTSUPP;
break;
case F2FS_XATTR_INDEX_TRUSTED:
-   if (!capable(CAP_SYS_ADMIN))
+   if (!capable(CAP_SYS_ADMIN) && !capable(CAP_TRUSTED))
return -EPERM;
break;
case F2FS_XATTR_INDEX_SECURITY:
@@ -83,7 +83,7 @@ static bool f2fs_xattr_user_list(struct dentry *dentry)
 
 static bool f2fs_xattr_trusted_list(struct dentry *dentry)
 {
-   return capable(CAP_SYS_ADMIN);
+   return capable(CAP_SYS_ADMIN) || capable(CAP_TRUSTED);
 }
 
 static int f2fs_xattr_advise_get(const struct xattr_handler *handler,
diff --git a/fs/hfsplus/xattr.c b/fs/hfsplus/xattr.c
index ae03a19196ef..0ec6d02ee7e9 100644
--- a/fs/hfsplus/xattr.c
+++ b/fs/hfsplus/xattr.c
@@ -606,7 +606,7 @@ static inline int can_list(const char *xattr_name)
 
return !strncmp(xattr_name, XATTR_TRUSTED_PREFIX,
XATTR_TRUSTED_PREFIX_LEN) ||
-   capable(CAP_SYS_ADMIN);
+   capable(CAP_SYS_ADMIN) || capable(CAP_TRUSTED);
 }
 
 static ssize_t hfsplus_listxattr_finder_info(struct dentry *dentry,
diff --git a/fs/jffs2/xattr_trusted.c b/fs/jffs2/xattr_trusted.c
index 5d6030826c52..3c7ae98e4525 100644
--- a/fs/jffs2/xattr_trusted.c
+++ b/fs/jffs2/xattr_trusted.c
@@ -35,7 +35,7 @@ static int jffs2_trusted_setxattr(const struct xattr_handler 
*handler,
 
 static bool jffs2_trusted_listxattr(struct dentry *dentry)
 {
-   return capable(CAP_SYS_ADMIN);
+   return capable(CAP_SYS_ADMIN) || capable(CAP_TRUSTED);
 }
 
 const struct xattr_handler jffs2_trusted_xattr_handler = {
diff --git a/fs/jfs/xattr.c b/fs/jfs/xattr.c
index 1c46573a96ed..e4f44f5133a1 100644
--- a/fs/jfs/xattr.c
+++ b/fs/jfs/xattr.c
@@ -859,7 +859,8 @@ ssize_t __jfs_getxattr(struct inode *inode, const char 
*name, void *data,
 static inline int can_list(struct jfs_ea *ea)
 {
return (!strncmp(ea->name, XATTR_TRUSTED_PREFIX,
-XATTR_TRUSTED_PREFIX_LEN) || capable(CAP_SYS_ADMIN));
+XATTR_TRUSTED_PREFIX_LEN) ||
+   capable(CAP_SYS_ADMIN) || capable(CAP_TRUSTED));
 }
 
 ssize_t jfs_listxattr(struct dentry * dentry, char *data, size_t buf_size)
diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index 5fdf269ba82e..2b3994d192c6 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -906,7 +906,7 @@ static int ocfs2_xattr_list_entry(struct super_block *sb,
break;
 
case OCFS2_XATTR_INDEX_TRUSTED:
-   if (!capable(CAP_SYS_ADMIN))
+   if (!capable(CAP_SYS_ADMIN) && !capable(CAP_TRUSTED))
return 0;
break;
}
diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index a619addecafc..c627003d3a74 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -264,7 +264,8 @@ static bool ovl_can_list(const char *s)
return true;
 

[RFC PATCH 2/2] fs: add the possibility to use CAP_SYS_MOUNT to (u)mount a fs

[Nicolas Belouin]

Fulfill the purpose of CAP_SYS_MOUNT by adding it as a sufficient
capability to mount and unmount filesystems.

Signed-off-by: Nicolas Belouin 
---
 fs/cachefiles/daemon.c |  2 +-
 fs/ext4/ioctl.c|  2 +-
 fs/namespace.c |  3 ++-
 fs/super.c | 14 +-
 4 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/fs/cachefiles/daemon.c b/fs/cachefiles/daemon.c
index 1ee54ffd3a24..fc53bdeacc8a 100644
--- a/fs/cachefiles/daemon.c
+++ b/fs/cachefiles/daemon.c
@@ -91,7 +91,7 @@ static int cachefiles_daemon_open(struct inode *inode, struct 
file *file)
_enter("");
 
/* only the superuser may do this */
-   if (!capable(CAP_SYS_ADMIN))
+   if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_MOUNT))
return -EPERM;
 
/* the cachefiles device may only be open once at a time */
diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index afb66d4ab5cf..19d838e558e2 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -458,7 +458,7 @@ static int ext4_shutdown(struct super_block *sb, unsigned 
long arg)
struct ext4_sb_info *sbi = EXT4_SB(sb);
__u32 flags;
 
-   if (!capable(CAP_SYS_ADMIN))
+   if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_MOUNT))
return -EPERM;
 
if (get_user(flags, (__u32 __user *)arg))
diff --git a/fs/namespace.c b/fs/namespace.c
index 3b601f115b6c..1eaa6a9f1631 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -1661,7 +1661,8 @@ void __detach_mounts(struct dentry *dentry)
  */
 static inline bool may_mount(void)
 {
-   return ns_capable(current->nsproxy->mnt_ns->user_ns, CAP_SYS_ADMIN);
+   return ns_capable(current->nsproxy->mnt_ns->user_ns, CAP_SYS_ADMIN) ||
+  ns_capable(current->nsproxy->mnt_ns->user_ns, CAP_SYS_MOUNT);
 }
 
 static inline bool may_mandlock(void)
diff --git a/fs/super.c b/fs/super.c
index 166c4ee0d0ed..1d84d8b87216 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -474,7 +474,7 @@ struct super_block *sget_userns(struct file_system_type 
*type,
 
if (!(flags & (SB_KERNMOUNT|SB_SUBMOUNT)) &&
!(type->fs_flags & FS_USERNS_MOUNT) &&
-   !capable(CAP_SYS_ADMIN))
+   !capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_MOUNT))
return ERR_PTR(-EPERM);
 retry:
spin_lock(_lock);
@@ -551,7 +551,9 @@ struct super_block *sget(struct file_system_type *type,
user_ns = _user_ns;
 
/* Ensure the requestor has permissions over the target filesystem */
-   if (!(flags & (SB_KERNMOUNT|SB_SUBMOUNT)) && !ns_capable(user_ns, 
CAP_SYS_ADMIN))
+   if (!(flags & (SB_KERNMOUNT | SB_SUBMOUNT)) &&
+   !ns_capable(user_ns, CAP_SYS_ADMIN) &&
+   !ns_capable(user_ns, CAP_SYS_MOUNT))
return ERR_PTR(-EPERM);
 
return sget_userns(type, test, set, flags, user_ns, data);
@@ -1020,10 +1022,12 @@ struct dentry *mount_ns(struct file_system_type 
*fs_type,
 {
struct super_block *sb;
 
-   /* Don't allow mounting unless the caller has CAP_SYS_ADMIN
-* over the namespace.
+   /* Don't allow mounting unless the caller has CAP_SYS_ADMIN (deprecated)
+* or CAP_SYS_MOUNT over the namespace.
 */
-   if (!(flags & SB_KERNMOUNT) && !ns_capable(user_ns, CAP_SYS_ADMIN))
+   if (!(flags & SB_KERNMOUNT) &&
+   !ns_capable(user_ns, CAP_SYS_ADMIN) &&
+   !ns_capable(user_ns, CAP_SYS_MOUNT))
return ERR_PTR(-EPERM);
 
sb = sget_userns(fs_type, ns_test_super, ns_set_super, flags,
-- 
2.14.2

[RFC PATCH 1/2] security, capabilities: Add CAP_SYS_MOUNT

[Nicolas Belouin]

With CAP_SYS_ADMIN being bloated and inapropriate for actions such
as mounting/unmounting filesystems, the creation of a new capability
is needed.
CAP_SYS_MOUNT is meant to give a process the ability to call for mount,
umount and umount2 syscalls.

Signed-off-by: Nicolas Belouin 
---
 include/uapi/linux/capability.h | 5 -
 security/selinux/include/classmap.h | 4 ++--
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/include/uapi/linux/capability.h b/include/uapi/linux/capability.h
index 230e05d35191..ce230aa6d928 100644
--- a/include/uapi/linux/capability.h
+++ b/include/uapi/linux/capability.h
@@ -365,8 +365,11 @@ struct vfs_ns_cap_data {
 
 #define CAP_AUDIT_READ 37
 
+/* Allow mounting, unmounting filesystems */
 
-#define CAP_LAST_CAP CAP_AUDIT_READ
+#define CAP_SYS_MOUNT  38
+
+#define CAP_LAST_CAP CAP_SYS_MOUNT
 
 #define cap_valid(x) ((x) >= 0 && (x) <= CAP_LAST_CAP)
 
diff --git a/security/selinux/include/classmap.h 
b/security/selinux/include/classmap.h
index 35ffb29a69cb..a873dce97fd5 100644
--- a/security/selinux/include/classmap.h
+++ b/security/selinux/include/classmap.h
@@ -24,9 +24,9 @@
"audit_control", "setfcap"
 
 #define COMMON_CAP2_PERMS  "mac_override", "mac_admin", "syslog", \
-   "wake_alarm", "block_suspend", "audit_read"
+   "wake_alarm", "block_suspend", "audit_read", "sys_mount"
 
-#if CAP_LAST_CAP > CAP_AUDIT_READ
+#if CAP_LAST_CAP > CAP_SYS_MOUNT
 #error New capability defined, please update COMMON_CAP2_PERMS.
 #endif
 
-- 
2.14.2

[PATCH] libsepol: free ibendport device names

[Jan Zarsky]

When reading policy, ibendport device names are allocated in
ocontext_read_selinux() but they are not freed when calling
sepol_policydb_free();

Fix this by freeing them in ocontext_selinux_free().

Signed-off-by: Jan Zarsky 
---
 libsepol/src/policydb.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
index 37788f36..c7521235 100644
--- a/libsepol/src/policydb.c
+++ b/libsepol/src/policydb.c
@@ -1420,6 +1420,8 @@ void ocontext_selinux_free(ocontext_t **ocontexts)
if (i == OCON_ISID || i == OCON_FS || i == OCON_NETIF
|| i == OCON_FSUSE)
free(ctmp->u.name);
+   else if (i == OCON_IBENDPORT)
+   free(ctmp->u.ibendport.dev_name);
free(ctmp);
}
}
-- 
2.14.2