Why does Python want to read /proc/meminfo

2017-05-05 Thread Ian Pilcher

I am trying to write an SELinux policy to confine a simple service that
I have written in Python, and I'm trying to decide whether to allow or
dontaudit various denials.

To start, I've reduced my service to the simplest case:

  #!/usr/bin/python

  import sys

  sys.exit()

Running this program in a confined domain generated the following
denial:

avc:  denied  { read } for  pid=2024 comm="denatc" name="meminfo" 
dev="proc" ino=4026532028 scontext=system_u:system_r:denatc_t:s0 
tcontext=system_u:object_r:proc_t:s0 tclass=file


The program does continue on and exit cleanly, so it doesn't seem to
strictly require the access.

Does anyone know why Python is trying to access this file, or what
functionality I might be missing if I don't allow the access?

--

Ian Pilcher arequip...@gmail.com
 "I grew up before Mark Zuckerberg invented friendship" 



Re: [PATCH] Add attribute expansion options

2017-05-05 Thread Jeffrey Vander Stoep via Selinux
Yes, we are generating CIL from policy.conf files.

On Fri, May 5, 2017 at 1:28 PM James Carter  wrote:

> On 05/04/2017 05:36 PM, Jeff Vander Stoep wrote:
> > This commit adds attribute expansion statements to the policy
> > language allowing compiler defaults to be overridden.
> >
> > Always expands an attribute example:
> > expandattribute { foo } true;
> > CIL example:
> > (expandtypeattribute (foo) true)
> >
> > Never expand an attribute example:
> > expandattribute { bar } false;
> > CIL example:
> > (expandtypeattribute (bar) false)
> >
>
> It works for secilc and for checkpolicy -C (which outputs CIL), but the
> expandattribute rules are ignored when building the kernel policy from a
> policy.conf. Are you generating CIL from policy.conf files? If not, then it
> might make sense to only have this feature in CIL.
>
> Jim
>
> > Adding the annotations directly to policy was chosen over other
> > methods as it is consistent with how targeted runtime optimizations
> > are specified in other languages. For example, in C the "inline"
> > command.
> >
> > Motivation
> >
> > expandattribute true:
> > Android has been moving away from a monolithic policy binary to
> > a two part split policy representing the Android platform and the
> > underlying vendor-provided hardware interface. The goal is a stable
> > API allowing these two parts to be updated independently of each
> > other. Attributes provide an important mechanism for compatibility.
> > For example, when the vendor provides a HAL for the platform,
> > permissions needed by clients of the HAL can be granted to an
> > attribute. Clients need only be assigned the attribute and do not
> > need to be aware of the underlying types and permissions being
> > granted.
> >
> > Inheriting permissions via attribute creates a convenient mechanism
> > for independence between vendor and platform policy, but results
> > in the creation of many attributes, and the potential for performance
> > issues when processes are clients of many HALs. [1] Annotating these
> > attributes for expansion at compile time allows us to retain the
> > compatibility benefits of using attributes without the performance
> > costs. [2]
> >
> > expandattribute false:
> > Commit 0be23c3f15fd added the capability to aggresively remove unused
> > attributes. This is generally useful as too many attributes assigned
> > to a type results in lengthy policy look up times when there is a
> > cache miss. However, removing attributes can also result in loss of
> > information used in external tests. On Android, we're considering
> > stripping neverallow rules from on-device policy. This is consistent
> > with the kernel policy binary which also did not contain neverallows.
> > Removing neverallow rules results in a 5-10% decrease in on-device
> > policy build and load and a policy size decrease of ~250k. Neverallow
> > rules are still asserted at build time and during device
> > certification (CTS). If neverallow rules are absent when secilc is
> > run, some attributes are being stripped from policy and neverallow
> > tests in CTS may be violated. [3] This change retains the aggressive
> > attribute stripping behavior but adds an override mechanism to
> > preserve attributes marked as necessary.
> >
> > [1] https://github.com/SELinuxProject/cil/issues/9
> > [2] Annotating all HAL client attributes for expansion resulted in
> >  system_server's dropping from 19 attributes to 8. Because these
> >  attributes were not widely applied to other types, the final
> >  policy size change was negligible.
> > [3] data_file_type and service_manager_type are stripped from AOSP
> >  policy when using secilc's -G option. This impacts 11 neverallow
> >  tests in CTS.
> >
> > Test: Build and boot Marlin with all hal_*_client attributes marked
> >  for expansion. Verify (using seinfo and sesearch) that permissions
> >  are correctly expanded from attributes to types.
> > Test: Mark types being stripped by secilc with "preserve" and verify
> >  that they are retained in policy and applied to the same types.
> >
> > Signed-off-by: Jeff Vander Stoep 
> > ---
> >   checkpolicy/policy_define.c| 82
> ++
> >   checkpolicy/policy_define.h|  1 +
> >   checkpolicy/policy_parse.y |  5 ++
> >   checkpolicy/policy_scan.l  |  2 +
> >   libsepol/cil/src/cil.c | 15 ++
> >   libsepol/cil/src/cil_build_ast.c   | 72
> ++
> >   libsepol/cil/src/cil_build_ast.h   |  2 +
> >   libsepol/cil/src/cil_copy_ast.c| 26 ++
> >   libsepol/cil/src/cil_flavor.h  |  1 +
> >   libsepol/cil/src/cil_internal.h| 16 --
> >   libsepol/cil/src/cil_post.c|  8 +++
> >   libsepol/cil/src/cil_reset_ast.c   |  1 +
> >   libsepol/cil/src/cil_resolve_ast.c | 

Re: [PATCH] Add attribute expansion options

2017-05-05 Thread James Carter

On 05/04/2017 05:36 PM, Jeff Vander Stoep wrote:

This commit adds attribute expansion statements to the policy
language allowing compiler defaults to be overridden.

Always expands an attribute example:
expandattribute { foo } true;
CIL example:
(expandtypeattribute (foo) true)

Never expand an attribute example:
expandattribute { bar } false;
CIL example:
(expandtypeattribute (bar) false)



It works for secilc and for checkpolicy -C (which outputs CIL), but the 
expandattribute rules are ignored when building the kernel policy from a 
policy.conf. Are you generating CIL from policy.conf files? If not, then it 
might make sense to only have this feature in CIL.


Jim


Adding the annotations directly to policy was chosen over other
methods as it is consistent with how targeted runtime optimizations
are specified in other languages. For example, in C the "inline"
command.

Motivation

expandattribute true:
Android has been moving away from a monolithic policy binary to
a two part split policy representing the Android platform and the
underlying vendor-provided hardware interface. The goal is a stable
API allowing these two parts to be updated independently of each
other. Attributes provide an important mechanism for compatibility.
For example, when the vendor provides a HAL for the platform,
permissions needed by clients of the HAL can be granted to an
attribute. Clients need only be assigned the attribute and do not
need to be aware of the underlying types and permissions being
granted.

Inheriting permissions via attribute creates a convenient mechanism
for independence between vendor and platform policy, but results
in the creation of many attributes, and the potential for performance
issues when processes are clients of many HALs. [1] Annotating these
attributes for expansion at compile time allows us to retain the
compatibility benefits of using attributes without the performance
costs. [2]

expandattribute false:
Commit 0be23c3f15fd added the capability to aggresively remove unused
attributes. This is generally useful as too many attributes assigned
to a type results in lengthy policy look up times when there is a
cache miss. However, removing attributes can also result in loss of
information used in external tests. On Android, we're considering
stripping neverallow rules from on-device policy. This is consistent
with the kernel policy binary which also did not contain neverallows.
Removing neverallow rules results in a 5-10% decrease in on-device
policy build and load and a policy size decrease of ~250k. Neverallow
rules are still asserted at build time and during device
certification (CTS). If neverallow rules are absent when secilc is
run, some attributes are being stripped from policy and neverallow
tests in CTS may be violated. [3] This change retains the aggressive
attribute stripping behavior but adds an override mechanism to
preserve attributes marked as necessary.

[1] https://github.com/SELinuxProject/cil/issues/9
[2] Annotating all HAL client attributes for expansion resulted in
 system_server's dropping from 19 attributes to 8. Because these
 attributes were not widely applied to other types, the final
 policy size change was negligible.
[3] data_file_type and service_manager_type are stripped from AOSP
 policy when using secilc's -G option. This impacts 11 neverallow
 tests in CTS.

Test: Build and boot Marlin with all hal_*_client attributes marked
 for expansion. Verify (using seinfo and sesearch) that permissions
 are correctly expanded from attributes to types.
Test: Mark types being stripped by secilc with "preserve" and verify
 that they are retained in policy and applied to the same types.

Signed-off-by: Jeff Vander Stoep 
---
  checkpolicy/policy_define.c| 82 ++
  checkpolicy/policy_define.h|  1 +
  checkpolicy/policy_parse.y |  5 ++
  checkpolicy/policy_scan.l  |  2 +
  libsepol/cil/src/cil.c | 15 ++
  libsepol/cil/src/cil_build_ast.c   | 72 ++
  libsepol/cil/src/cil_build_ast.h   |  2 +
  libsepol/cil/src/cil_copy_ast.c| 26 ++
  libsepol/cil/src/cil_flavor.h  |  1 +
  libsepol/cil/src/cil_internal.h| 16 --
  libsepol/cil/src/cil_post.c|  8 +++
  libsepol/cil/src/cil_reset_ast.c   |  1 +
  libsepol/cil/src/cil_resolve_ast.c | 53 ++-
  libsepol/cil/src/cil_tree.c| 11 
  libsepol/include/sepol/policydb/policydb.h |  6 ++-
  libsepol/src/module_to_cil.c   | 11 
  16 files changed, 307 insertions(+), 5 deletions(-)

diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
index 949ca711..63e3c53f 100644
--- a/checkpolicy/policy_define.c
+++ b/checkpolicy/policy_define.c
@@ -1139,6 +1139,88 @@ int define_attrib(void)
return 0;

Re: [PATCH 3/3] libsemanage: replace access(, F_OK) checks to make setuid programs work

2017-05-05 Thread Stephen Smalley
On Fri, 2017-05-05 at 14:49 +0200, Vit Mojzis wrote:
> access() uses real UID instead of effective UID which causes false
> negative checks in setuid programs.
> Replace access(,F_OK) (i.e. tests for file existence) by stat().
> 
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1186431
> 
> Signed-off-by: Vit Mojzis 
> ---
>  libsemanage/src/direct_api.c | 36 +-
> --
>  1 file changed, 21 insertions(+), 15 deletions(-)
> 
> diff --git a/libsemanage/src/direct_api.c
> b/libsemanage/src/direct_api.c
> index 508277d..35ca1b0 100644
> --- a/libsemanage/src/direct_api.c
> +++ b/libsemanage/src/direct_api.c
> @@ -272,7 +272,9 @@ int semanage_direct_connect(semanage_handle_t *
> sh)
>  
>   /* set the disable dontaudit value */
>   path = semanage_path(SEMANAGE_ACTIVE,
> SEMANAGE_DISABLE_DONTAUDIT);
> - if (access(path, F_OK) == 0)
> +
> + struct stat sb;
> + if (stat(path, ) == 0)

Need to check errno as well to ensure that it is just ENOENT when
stat() returns non-zero; anything else is an error.

stat() can produce a SELinux getattr denial, whereas access(F_OK)
doesn't perform any SELinux check beyond directory search access.

>   sepol_set_disable_dontaudit(sh->sepolh, 1);
>   else
>   sepol_set_disable_dontaudit(sh->sepolh, 0);
> @@ -1101,8 +1103,9 @@ static int
> semanage_compile_hll_modules(semanage_handle_t *sh,
>   goto cleanup;
>   }
>  
> + struct stat sb;
>   if (semanage_get_ignore_module_cache(sh) == 0 &&
> - access(cil_path, F_OK) == 0) {
> + stat(cil_path, ) == 0) {
>   continue;
>   }
>  
> @@ -1165,7 +1168,8 @@ static int
> semanage_direct_commit(semanage_handle_t * sh)
>  
>   /* Create or remove the disable_dontaudit flag file. */
>   path = semanage_path(SEMANAGE_TMP,
> SEMANAGE_DISABLE_DONTAUDIT);
> - if (access(path, F_OK) == 0)
> + struct stat sb;
> + if (stat(path, ) == 0)
>   do_rebuild |= !(sepol_get_disable_dontaudit(sh-
> >sepolh) == 1);
>   else
>   do_rebuild |= (sepol_get_disable_dontaudit(sh-
> >sepolh) == 1);
> @@ -1190,7 +1194,7 @@ static int
> semanage_direct_commit(semanage_handle_t * sh)
>  
>   /* Create or remove the preserve_tunables flag file. */
>   path = semanage_path(SEMANAGE_TMP,
> SEMANAGE_PRESERVE_TUNABLES);
> - if (access(path, F_OK) == 0)
> + if (stat(path, ) == 0)
>   do_rebuild |= !(sepol_get_preserve_tunables(sh-
> >sepolh) == 1);
>   else
>   do_rebuild |= (sepol_get_preserve_tunables(sh-
> >sepolh) == 1);
> @@ -1231,37 +1235,37 @@ static int
> semanage_direct_commit(semanage_handle_t * sh)
>    */
>   if (!do_rebuild) {
>   path = semanage_path(SEMANAGE_TMP,
> SEMANAGE_STORE_KERNEL);
> - if (access(path, F_OK) != 0) {
> + if (stat(path, ) != 0) {
>   do_rebuild = 1;
>   goto rebuild;
>   }
>  
>   path = semanage_path(SEMANAGE_TMP,
> SEMANAGE_STORE_FC);
> - if (access(path, F_OK) != 0) {
> + if (stat(path, ) != 0) {
>   do_rebuild = 1;
>   goto rebuild;
>   }
>  
>   path = semanage_path(SEMANAGE_TMP,
> SEMANAGE_STORE_SEUSERS);
> - if (access(path, F_OK) != 0) {
> + if (stat(path, ) != 0) {
>   do_rebuild = 1;
>   goto rebuild;
>   }
>  
>   path = semanage_path(SEMANAGE_TMP, SEMANAGE_LINKED);
> - if (access(path, F_OK) != 0) {
> + if (stat(path, ) != 0) {
>   do_rebuild = 1;
>   goto rebuild;
>   }
>  
>   path = semanage_path(SEMANAGE_TMP,
> SEMANAGE_SEUSERS_LINKED);
> - if (access(path, F_OK) != 0) {
> + if (stat(path, ) != 0) {
>   do_rebuild = 1;
>   goto rebuild;
>   }
>  
>   path = semanage_path(SEMANAGE_TMP,
> SEMANAGE_USERS_EXTRA_LINKED);
> - if (access(path, F_OK) != 0) {
> + if (stat(path, ) != 0) {
>   do_rebuild = 1;
>   goto rebuild;
>   }
> @@ -1395,7 +1399,7 @@ rebuild:
>   goto cleanup;
>  
>   path = semanage_path(SEMANAGE_TMP,
> SEMANAGE_SEUSERS_LINKED);
> - if (access(path, F_OK) == 0) {
> + if (stat(path, ) == 0) {
>   retval = semanage_copy_file(path,
>   semanage_path(SE
> MANAGE_TMP,
>     SE
> MANAGE_STORE_SEUSERS),
> @@ -1408,7 +1412,7 @@ rebuild:
>   }
>  
>   path = 

Re: [PATCH 2/3] libsemanage: remove access() check to make setuid programs work

2017-05-05 Thread Stephen Smalley
On Fri, 2017-05-05 at 14:49 +0200, Vit Mojzis wrote:
> F_OK access checks only work properly as long as all directories
> along
> the path are accessible to real user running the program.
> Replace F_OK access checks by testing return value of open, write,
> etc.

The patch description talks about F_OK tests, but...

> 
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1186431
> ---
>  libsemanage/src/direct_api.c | 40 +++---
> --
>  1 file changed, 15 insertions(+), 25 deletions(-)
> 
> diff --git a/libsemanage/src/direct_api.c
> b/libsemanage/src/direct_api.c
> index dc46f8f..508277d 100644
> --- a/libsemanage/src/direct_api.c
> +++ b/libsemanage/src/direct_api.c
> @@ -342,10 +342,6 @@ static int
> semanage_direct_disconnect(semanage_handle_t * sh)
>  
>  static int semanage_direct_begintrans(semanage_handle_t * sh)
>  {
> -
> - if (semanage_access_check(sh) != SEMANAGE_CAN_WRITE) {
> - return -1;
> - }

The patch has an unrelated change (possibly belonging in the first
patch?).

>   if (semanage_get_trans_lock(sh) < 0) {
>   return -1;
>   }
> @@ -1491,33 +1487,27 @@ rebuild:
>   }
>  
>   path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_FC_LOCAL);
> - if (access(path, F_OK) == 0) {
> - retval =
> semanage_copy_file(semanage_path(SEMANAGE_TMP,
> SEMANAGE_STORE_FC_LOCAL),
> - semanage_fin
> al_path(SEMANAGE_FINAL_TMP, SEMANAGE_FC_LOCAL),
> - sh->conf-
> >file_mode);
> - if (retval < 0) {
> - goto cleanup;
> - }
> + retval = semanage_copy_file(semanage_path(SEMANAGE_TMP,
> SEMANAGE_STORE_FC_LOCAL),
> + semanage_final_path(
> SEMANAGE_FINAL_TMP, SEMANAGE_FC_LOCAL),
> + sh->conf-
> >file_mode);
> + if (retval < 0 && errno != ENOENT) {
> + goto cleanup;
>   }
>  
>   path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_FC);
> - if (access(path, F_OK) == 0) {
> - retval =
> semanage_copy_file(semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_FC),
> - semanage_fin
> al_path(SEMANAGE_FINAL_TMP, SEMANAGE_FC),
> - sh->conf-
> >file_mode);
> - if (retval < 0) {
> - goto cleanup;
> - }
> + retval = semanage_copy_file(semanage_path(SEMANAGE_TMP,
> SEMANAGE_STORE_FC),
> + semanage_final_path(
> SEMANAGE_FINAL_TMP, SEMANAGE_FC),
> + sh->conf-
> >file_mode);
> + if (retval < 0 && errno != ENOENT) {
> + goto cleanup;
>   }
>  
>   path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_SEUSERS);
> - if (access(path, F_OK) == 0) {
> - retval =
> semanage_copy_file(semanage_path(SEMANAGE_TMP,
> SEMANAGE_STORE_SEUSERS),
> - semanage_fin
> al_path(SEMANAGE_FINAL_TMP, SEMANAGE_SEUSERS),
> - sh->conf-
> >file_mode);
> - if (retval < 0) {
> - goto cleanup;
> - }
> + retval = semanage_copy_file(semanage_path(SEMANAGE_TMP,
> SEMANAGE_STORE_SEUSERS),
> + semanage_final_path(
> SEMANAGE_FINAL_TMP, SEMANAGE_SEUSERS),
> + sh->conf-
> >file_mode);
> + if (retval < 0 && errno != ENOENT) {
> + goto cleanup;
>   }
>  
>   /* run genhomedircon if its enabled, this should be the last
> operation


Re: [PATCH 1/3] libsemanage: remove access() check to make setuid programs work

2017-05-05 Thread Stephen Smalley
On Fri, 2017-05-05 at 14:49 +0200, Vit Mojzis wrote:
> access() uses real UID instead of effective UID which causes false
> negative checks in setuid programs (except for F_OK which works
> properly). fopen() return values are always checked, which makes
> access() checks redundant.
> 
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1186431
> ---
>  libsemanage/src/direct_api.c |  3 ---
>  libsemanage/src/semanage_store.c | 17 -
>  2 files changed, 8 insertions(+), 12 deletions(-)
> 
> diff --git a/libsemanage/src/direct_api.c
> b/libsemanage/src/direct_api.c
> index f4b0416..dc46f8f 100644
> --- a/libsemanage/src/direct_api.c
> +++ b/libsemanage/src/direct_api.c
> @@ -146,9 +146,6 @@ int semanage_direct_connect(semanage_handle_t *
> sh)
>   if (semanage_create_store(sh, 1))
>   goto err;
>  
> - if (semanage_access_check(sh) < SEMANAGE_CAN_READ)
> - goto err;
> -
>   sh->u.direct.translock_file_fd = -1;
>   sh->u.direct.activelock_file_fd = -1;
>  
> diff --git a/libsemanage/src/semanage_store.c
> b/libsemanage/src/semanage_store.c
> index 6b75002..0a10032 100644
> --- a/libsemanage/src/semanage_store.c
> +++ b/libsemanage/src/semanage_store.c
> @@ -535,7 +535,6 @@ char *semanage_conf_path(void)
>  int semanage_create_store(semanage_handle_t * sh, int create)
>  {
>   struct stat sb;
> - int mode_mask = R_OK | W_OK | X_OK;
>   const char *path = semanage_files[SEMANAGE_ROOT];
>   int fd;
>  
> @@ -554,9 +553,9 @@ int semanage_create_store(semanage_handle_t * sh,
> int create)
>   return -1;
>   }
>   } else {
> - if (!S_ISDIR(sb.st_mode) || access(path, mode_mask)
> == -1) {
> + if (!S_ISDIR(sb.st_mode)) {
>   ERR(sh,
> - "Could not access module store at %s, or
> it is not a directory.",
> + "Module store at %s is not a
> directory.",
>   path);
>   return -1;
>   }
> @@ -577,9 +576,9 @@ int semanage_create_store(semanage_handle_t * sh,
> int create)
>   return -1;
>   }
>   } else {
> - if (!S_ISDIR(sb.st_mode) || access(path, mode_mask)
> == -1) {
> + if (!S_ISDIR(sb.st_mode)) {
>   ERR(sh,
> - "Could not access module store active
> subdirectory at %s, or it is not a directory.",
> + "Module store active subdirectory at %s
> is not a directory.",
>   path);
>   return -1;
>   }
> @@ -600,9 +599,9 @@ int semanage_create_store(semanage_handle_t * sh,
> int create)
>   return -1;
>   }
>   } else {
> - if (!S_ISDIR(sb.st_mode) || access(path, mode_mask)
> == -1) {
> + if (!S_ISDIR(sb.st_mode)) {
>   ERR(sh,
> - "Could not access module store active
> modules subdirectory at %s, or it is not a directory.",
> + "Module store active modules
> subdirectory at %s is not a directory.",
>   path);
>   return -1;
>   }
> @@ -621,8 +620,8 @@ int semanage_create_store(semanage_handle_t * sh,
> int create)
>   return -1;
>   }
>   } else {
> - if (!S_ISREG(sb.st_mode) || access(path, R_OK |
> W_OK) == -1) {
> - ERR(sh, "Could not access lock file at %s.",
> path);
> + if (!S_ISREG(sb.st_mode)) {
> + ERR(sh, "Lock file at %s missing.", path);

It can't be missing or the stat() would have failed.
Lock file is not a regular file?
Not sure how useful these tests are though...

>   return -1;
>   }
>   }


Re: [PATCH] libsepol/utils: Fix build without system sepol.h

2017-05-05 Thread Stephen Smalley
On Fri, 2017-05-05 at 12:12 +0200, Petr Lautrbach wrote:
> fcb5d5c removed ../include from CFLAGS from libsepol/utils/Makefile
> so
> that a build tool can't find sepol/sepol.h when only libsepol is
> built
> and a system is without sepol.h in standard paths. It should use its
> own
> sepol.h file during build. `oveeride` needs to be used in order not
> to
> be overridden by values provided on a command line. Same problem
> applies
> to LDFLAGS.
> 
> Fixes:
> $ make CFLAGS="" LDFLAGS=""
> make[1]: Entering directory '/root/selinux/libsepol/utils'
> cc chkcon.c  -lsepol -o chkcon
> chkcon.c:1:25: fatal error: sepol/sepol.h: No such file or directory
>  #include 
> 
> $ make CFLAGS="" LDFLAGS=""
> ...
> make -C utils
> make[1]: Entering directory '/root/selinux/libsepol/utils'
> cc  -I../includechkcon.c  -lsepol -o chkcon
> /usr/bin/ld: cannot find -lsepol
> collect2: error: ld returned 1 exit status

Thanks, applied

> 
> Signed-off-by: Petr Lautrbach 
> ---
>  libsepol/utils/Makefile | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/libsepol/utils/Makefile b/libsepol/utils/Makefile
> index a13164e1..3b2fb771 100644
> --- a/libsepol/utils/Makefile
> +++ b/libsepol/utils/Makefile
> @@ -3,7 +3,8 @@ PREFIX ?= $(DESTDIR)/usr
>  BINDIR ?= $(PREFIX)/bin
>  
>  CFLAGS ?= -Wall -Werror
> -LDFLAGS += -L../src
> +override CFLAGS += -I../include
> +override LDFLAGS += -L../src
>  LDLIBS += -lsepol
>  
>  TARGETS=$(patsubst %.c,%,$(wildcard *.c))


Re: [PATCH v2 1/2] selinux: add brief info to policydb

2017-05-05 Thread Casey Schaufler
On 5/5/2017 3:10 AM, Sebastien Buisson wrote:
> Add policybrief field to struct policydb. It holds a brief info
> of the policydb, in the following form:
> <0 or 1 for enforce>:<0 or 1 for checkreqprot>:=
> Policy brief is computed every time the policy is loaded, and when
> enforce or checkreqprot are changed.

You need to add a description of policy_brief in lsm_hooks.h.
I would like you to describe the expectations Lustre has for
the content of a policy brief in general. That will make it
easier for AppArmor and Smack to support Lustre when the time
comes. How do you see policy_brief being used by a modules
with dynamic policy?


>
> Add security_policy_brief hook to give access to policy brief to
> the rest of the kernel. Lustre client makes use of this information.
>
> Signed-off-by: Sebastien Buisson 
> ---
>  include/linux/lsm_hooks.h   |  2 +
>  include/linux/security.h|  7 
>  security/security.c |  6 +++
>  security/selinux/hooks.c| 11 +-
>  security/selinux/include/security.h |  2 +
>  security/selinux/selinuxfs.c|  6 ++-
>  security/selinux/ss/policydb.c  | 70 +++
>  security/selinux/ss/policydb.h  |  3 ++
>  security/selinux/ss/services.c  | 73 
> +
>  9 files changed, 178 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 080f34e..7139a07 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1568,6 +1568,7 @@
>   int (*inode_setsecctx)(struct dentry *dentry, void *ctx, u32 ctxlen);
>   int (*inode_getsecctx)(struct inode *inode, void **ctx, u32 *ctxlen);
>  
> + int (*policy_brief)(char **brief, size_t *len, bool alloc);
>  #ifdef CONFIG_SECURITY_NETWORK
>   int (*unix_stream_connect)(struct sock *sock, struct sock *other,
>   struct sock *newsk);
> @@ -1813,6 +1814,7 @@ struct security_hook_heads {
>   struct list_head inode_notifysecctx;
>   struct list_head inode_setsecctx;
>   struct list_head inode_getsecctx;
> + struct list_head policy_brief;
>  #ifdef CONFIG_SECURITY_NETWORK
>   struct list_head unix_stream_connect;
>   struct list_head unix_may_send;
> diff --git a/include/linux/security.h b/include/linux/security.h
> index af675b5..3b72053 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -377,6 +377,8 @@ int security_sem_semop(struct sem_array *sma, struct 
> sembuf *sops,
>  int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen);
>  int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen);
>  int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen);
> +
> +int security_policy_brief(char **brief, size_t *len, bool alloc);
>  #else /* CONFIG_SECURITY */
>  struct security_mnt_opts {
>  };
> @@ -1166,6 +1168,11 @@ static inline int security_inode_getsecctx(struct 
> inode *inode, void **ctx, u32
>  {
>   return -EOPNOTSUPP;
>  }
> +
> +static inline int security_policy_brief(char **brief, size_t *len, bool 
> alloc)
> +{
> + return -EOPNOTSUPP;
> +}
>  #endif   /* CONFIG_SECURITY */
>  
>  #ifdef CONFIG_SECURITY_NETWORK
> diff --git a/security/security.c b/security/security.c
> index b9fea39..954b391 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1285,6 +1285,12 @@ int security_inode_getsecctx(struct inode *inode, void 
> **ctx, u32 *ctxlen)
>  }
>  EXPORT_SYMBOL(security_inode_getsecctx);
>  
> +int security_policy_brief(char **brief, size_t *len, bool alloc)
> +{
> + return call_int_hook(policy_brief, -EOPNOTSUPP, brief, len, alloc);
> +}
> +EXPORT_SYMBOL(security_policy_brief);
> +
>  #ifdef CONFIG_SECURITY_NETWORK
>  
>  int security_unix_stream_connect(struct sock *sock, struct sock *other, 
> struct sock *newsk)
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index e67a526..b4dd605 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -104,8 +104,10 @@
>  static int __init enforcing_setup(char *str)
>  {
>   unsigned long enforcing;
> - if (!kstrtoul(str, 0, ))
> + if (!kstrtoul(str, 0, )) {
>   selinux_enforcing = enforcing ? 1 : 0;
> + security_policydb_update_info(NULL);
> + }
>   return 1;
>  }
>  __setup("enforcing=", enforcing_setup);
> @@ -6063,6 +6065,11 @@ static int selinux_inode_getsecctx(struct inode 
> *inode, void **ctx, u32 *ctxlen)
>   *ctxlen = len;
>   return 0;
>  }
> +
> +static int selinux_policy_brief(char **brief, size_t *len, bool alloc)
> +{
> + return security_policydb_brief(brief, len, alloc);
> +}
>  #ifdef CONFIG_KEYS
>  
>  static int selinux_key_alloc(struct key *k, const struct cred *cred,
> @@ -6277,6 +6284,8 @@ static int selinux_key_getsecurity(struct key *key, 
> char **_buffer)
>   

Announcing SPAN: SELinux Policy Analysis Notebook

2017-05-05 Thread Karl MacMillan
I’d like to announce SPAN - SELinux Policy Analysis Notebook 
(https://github.com/QuarkSecurity/SPAN/ 
). This is a Jupyter notebook based 
environment for SELinux policy analysis that let’s you mix queries, Python 
code, and Markdown formatted notes into an executable document. It’s an 
extension of SETools 4.

Using SPAN within Jupyter notebook is an amazingly productive way to do policy 
analysis. I really think that this is the most productive environment that I’ve 
seen for real policy analysis (and I’ve been working on SELinux policy analysis 
and tools for almost 15 years). The ability to quickly create custom tools to 
answer hard questions combined inline with well-formatted documentation makes a 
huge difference.

SPAN has been used so far to analyze 3 large, complex, custom systems with very 
large policies (hundreds of custom domains). The analysis was of much better 
quality and it took much less time because of SPAN.

If you just want to see what this looks like, you can see an example online 
(though the code is not executable):

https://nbviewer.jupyter.org/github/QuarkSecurity/SPAN/blob/master/examples/Span%20Example.ipynb#
 


If you’ve not seen Jupyter notebooks, they are a very popular tool for data 
science. Jupyter notebooks are an interactive environment that let you write 
text (in Markdown) and code together. You can get a feel for what's possible in 
this awesome notebook on Regex Golf from XKCD: 
http://nbviewer.jupyter.org/url/norvig.com/ipython/xkcd1313.ipynb 
. There is 
also the more official (and boring) introduction: 
https://jupyter-notebook-beginner-guide.readthedocs.io/en/latest/ 
.

SPAN was written by me (Karl MacMillan) along with Spencer Shimko and Brandon 
Whalen from Quark Security. And, of course, this is built on SETools 4 which is 
maintained by Chris PeBinito.

Thanks - Karl

Re: [PATCH 2/2] sestatus: show checkreqprot status

2017-05-05 Thread Stephen Smalley
On Thu, 2017-05-04 at 22:45 +0200, Christian Göttsche via Selinux
wrote:
> Show the current active checkreqprot state in sestatus
> ---
>  policycoreutils/sestatus/sestatus.c | 14 ++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/policycoreutils/sestatus/sestatus.c
> b/policycoreutils/sestatus/sestatus.c
> index 2111b15d..a461251d 100644
> --- a/policycoreutils/sestatus/sestatus.c
> +++ b/policycoreutils/sestatus/sestatus.c
> @@ -330,6 +330,20 @@ int main(int argc, char **argv)
>   break;
>   }
>  
> + printf_tab("Current checkreqprot mode:");
> + rc = security_get_checkreqprot();
> + switch (rc) {
> + case 0:
> + printf("Kernel preset\n");

As with the other one, "kernel preset" isn't very clear.  The
difference is between the actual protection applied by the kernel and
the protection requested by the application.

> + break;
> + case 1:
> + printf("Application requested\n");
> + break;
> + default:
> + printf("error (%s)\n", strerror(errno));
> + break;
> + }
> +
>   rc = security_policyvers();
>   printf_tab("Max kernel policy version:");
>   if (rc < 0)

Don't think this really conveys the right meaning or will be
understandable to users. My previous suggestions were:
Check requested protection: false/true
or
Memory protection checking: actual/requested
or
Memory protection checking: secure/insecure

Even if you really want to stick with "Current checkreqprot mode:", the
values (false/true, actual/requested, secure/insecure) still make
sense.




Re: [PATCH 1/2] libselinux: add security_get_checkreqprot

2017-05-05 Thread Stephen Smalley
On Thu, 2017-05-04 at 22:45 +0200, Christian Göttsche via Selinux
wrote:
> Add security_get_checkreqprot() function, returning the current
> active
> checkreqprot value
> ---
>  libselinux/include/selinux/selinux.h  |  3 +++
>  libselinux/man/man3/security_getenforce.3 |  9 ++-
>  libselinux/src/checkreqprot.c | 40
> +++
>  libselinux/src/selinux_internal.h |  1 +
>  4 files changed, 52 insertions(+), 1 deletion(-)
>  create mode 100644 libselinux/src/checkreqprot.c
> 
> diff --git a/libselinux/include/selinux/selinux.h
> b/libselinux/include/selinux/selinux.h
> index 45dd6ca5..01201eee 100644
> --- a/libselinux/include/selinux/selinux.h
> +++ b/libselinux/include/selinux/selinux.h
> @@ -331,6 +331,9 @@ extern int security_setenforce(int value);
>  /* Get the behavior for undefined classes/permissions */
>  extern int security_deny_unknown(void);
>  
> +/* Get the checkreqprot value */
> +extern int security_get_checkreqprot(void);
> +
>  /* Disable SELinux at runtime (must be done prior to initial policy
> load). */
>  extern int security_disable(void);
>  
> diff --git a/libselinux/man/man3/security_getenforce.3
> b/libselinux/man/man3/security_getenforce.3
> index 7658014a..346b2cbd 100644
> --- a/libselinux/man/man3/security_getenforce.3
> +++ b/libselinux/man/man3/security_getenforce.3
> @@ -1,6 +1,6 @@
>  .TH "security_getenforce" "3" "1 January 2004" "russ...@coker.com.au
> " "SELinux API documentation"
>  .SH "NAME"
> -security_getenforce, security_setenforce, security_deny_unknown \-
> get or set the enforcing state of SELinux
> +security_getenforce, security_setenforce, security_deny_unknown,
> security_get_checkreqprot\- get or set the enforcing state of SELinux
>  .
>  .SH "SYNOPSIS"
>  .B #include 
> @@ -10,6 +10,8 @@ security_getenforce, security_setenforce,
> security_deny_unknown \- get or set th
>  .BI "int security_setenforce(int "value );
>  .sp
>  .B int security_deny_unknown(void);
> +.sp
> +.B int security_get_checkreqprot(void);
>  .
>  .SH "DESCRIPTION"
>  .BR security_getenforce ()
> @@ -24,6 +26,11 @@ returned.
>  .BR security_deny_unknown ()
>  returns 0 if SELinux treats policy queries on undefined object
> classes or
>  permissions as being allowed, 1 if such queries are denied, and \-1
> on error.
> +
> +.BR security_get_checkreqprot ()
> +returns 0 if SELinux checks protection on mmap and mprotect calls
> preset by
> +the kernel, 1 if SELinux checks the protection on mmap and mprotect
> calls
> +requested by the application, and \-1 on error.

I don't think this is very clear.  How about:
security_get_checkreqprot() can be used to determine whether SELinux is
configured to check the protection requested by the application or the
actual protection that will be applied by the kernel (including the
effects of READ_IMPLIES_EXEC) on mmap and mprotect calls.  It returns 0
if SELinux checks the actual protection, 1 if it checks the requested
protection, and \-1 on error.

>  .
>  .SH "SEE ALSO"
>  .BR selinux "(8)"
> diff --git a/libselinux/src/checkreqprot.c
> b/libselinux/src/checkreqprot.c
> new file mode 100644
> index ..9b4b12d7
> --- /dev/null
> +++ b/libselinux/src/checkreqprot.c
> @@ -0,0 +1,40 @@
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include "selinux_internal.h"
> +#include "policy.h"
> +#include 
> +#include 
> +
> +int security_get_checkreqprot(void)
> +{
> + int fd, ret, checkreqprot = 0;
> + char path[PATH_MAX];
> + char buf[20];
> +
> + if (!selinux_mnt) {
> + errno = ENOENT;
> + return -1;
> + }
> +
> + snprintf(path, sizeof(path), "%s/checkreqprot",
> selinux_mnt);
> + fd = open(path, O_RDONLY | O_CLOEXEC);
> + if (fd < 0)
> + return -1;
> +
> + memset(buf, 0, sizeof(buf));
> + ret = read(fd, buf, sizeof(buf) - 1);
> + close(fd);
> + if (ret < 0)
> + return -1;
> +
> + if (sscanf(buf, "%d", ) != 1)
> + return -1;
> +
> + return checkreqprot;
> +}
> +
> +hidden_def(security_get_checkreqprot);
> diff --git a/libselinux/src/selinux_internal.h
> b/libselinux/src/selinux_internal.h
> index 3d5c9fb4..54949c13 100644
> --- a/libselinux/src/selinux_internal.h
> +++ b/libselinux/src/selinux_internal.h
> @@ -59,6 +59,7 @@ hidden_proto(selinux_mkload_policy)
>  hidden_proto(security_getenforce)
>  hidden_proto(security_setenforce)
>  hidden_proto(security_deny_unknown)
> +hidden_proto(security_get_checkreqprot)
>  hidden_proto(selinux_boolean_sub)
>  hidden_proto(selinux_current_policy_path)
>  hidden_proto(selinux_binary_policy_path)


Re: [PATCH 1/6] Revert "policycoreutils: let output of `fixfiles` be redirected (as normal)"

2017-05-05 Thread James Carter

On 05/04/2017 01:01 PM, Alan Jenkins wrote:

This reverts commit ac7899fc3ad6221e195dd13cdf14b346897314ae,
which is not yet part of an officially tagged release
(or release candidate).

`LOGFILE=/proc/self/fd/1` was wrong.

`LOGFILE=$(tty)` was being relied on in one case (exclude_dirs),
to log messages from a function run specifically with stdout redirected
(captured into a variable).

Having `logit "message"` break inside redirected functions
is a nasty leaky abstraction.

This caused e.g. `fixfiles restore` to terminate early with the error

 skipping: No such file or directory

if the user had configured any excluded paths in
/etc/selinux/fixfiles_exclude_dirs


These six patches have been applied.

Thanks,
Jim


---
  policycoreutils/scripts/fixfiles | 10 +++---
  1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/policycoreutils/scripts/fixfiles b/policycoreutils/scripts/fixfiles
index bc74d69..75d7762 100755
--- a/policycoreutils/scripts/fixfiles
+++ b/policycoreutils/scripts/fixfiles
@@ -119,7 +119,11 @@ VERBOSE="-p"
  FORCEFLAG=""
  DIRS=""
  RPMILES=""
-LOGFILE=/proc/self/fd/1
+LOGFILE=`tty`
+if [ $? != 0 ]; then
+LOGFILE="/dev/null"
+fi
+LOGGER=/usr/sbin/logger
  SETFILES=/sbin/setfiles
  RESTORECON=/sbin/restorecon
  FILESYSTEMSRW=`get_rw_labeled_mounts`
@@ -134,11 +138,11 @@ else
  fi
  
  #

-# Write to LOGFILE
+# Log to either syslog or a LOGFILE
  #
  logit () {
  if [ -n $LOGFILE ]; then
-echo $1 >> "$LOGFILE"
+echo $1 >> $LOGFILE
  fi
  }
  #




--
James Carter 
National Security Agency


Re: [PATCH 19/19] sepolicy/gui: Update text strings to use better gettext templates

2017-05-05 Thread Stephen Smalley
On Wed, 2017-05-03 at 12:30 +0200, Petr Lautrbach wrote:
> Signed-off-by: Petr Lautrbach 

Thanks, applied all 19 patches.

> ---
>  python/sepolicy/sepolicy/gui.py | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/python/sepolicy/sepolicy/gui.py
> b/python/sepolicy/sepolicy/gui.py
> index 7f84b6f9..007c94a7 100644
> --- a/python/sepolicy/sepolicy/gui.py
> +++ b/python/sepolicy/sepolicy/gui.py
> @@ -1378,8 +1378,8 @@ class SELinuxGui():
>  self.treeview = self.network_in_treeview
>  category = _("listen for inbound connections")
>  
> -self.add_button.set_tooltip_text(_("Add new port
> definition to which the '%(APP)s' domain is allowed to %s.") %
> {"APP": self.application, "PERM": category})
> -self.delete_button.set_tooltip_text(_("Delete modified
> port definitions to which the '%(APP)s' domain is allowed to %s.") %
> {"APP": self.application, "PERM": category})
> +self.add_button.set_tooltip_text(_("Add new port
> definition to which the '%(APP)s' domain is allowed to %(PERM)s.") %
> {"APP": self.application, "PERM": category})
> +self.delete_button.set_tooltip_text(_("Delete modified
> port definitions to which the '%(APP)s' domain is allowed to
> %(PERM)s.") % {"APP": self.application, "PERM": category})
>  self.modify_button.set_tooltip_text(_("Modify port
> definitions to which the '%(APP)s' domain is allowed to %(PERM)s.") %
> {"APP": self.application, "PERM": category})
>  
>  if self.transitions_radio_button.get_active():
> @@ -1599,8 +1599,8 @@ class SELinuxGui():
>  self.show_popup(self.login_popup_window)
>  
>  if self.opage == FILE_EQUIV_PAGE:
> -self.file_equiv_source_entry.set_text(self.file_equiv_li
> ststore.get_value(iter, 0))
> -self.file_equiv_dest_entry.set_text(self.file_equiv_list
> store.get_value(iter, 1))
> +self.file_equiv_source_entry.set_text(self.unmarkup(self
> .file_equiv_liststore.get_value(iter, 0)))
> +self.file_equiv_dest_entry.set_text(self.unmarkup(self.f
> ile_equiv_liststore.get_value(iter, 1)))
>  self.file_equiv_label.set_text((_("Modify File
> Equivalency Mapping. Mapping will be created when update is
> applied.")))
>  self.file_equiv_popup_window.set_title(_("Modify SELinux
> File Equivalency"))
>  self.clear_entry = True


Re: [RFC][PATCH] selinux: add a map permission check for mmap

2017-05-05 Thread Stephen Smalley
On Fri, 2017-05-05 at 09:14 -0400, Stephen Smalley wrote:
> Add a map permission check on mmap so that we can distinguish memory
> mapped
> access (since it has different implications for revocation). When a
> file
> is opened and then read or written via syscalls like
> read(2)/write(2),
> we revalidate access on each read/write operation via
> selinux_file_permission() and therefore can revoke access if the
> process context, the file context, or the policy changes in such a
> manner that access is no longer allowed. When a file is opened and
> then
> memory mapped via mmap(2) and then subsequently read or written
> directly
> in memory, we presently have no way to revalidate or revoke access.
> The purpose of a separate map permission check on mmap(2) is to
> permit
> policy to prohibit memory mapping of specific files for which we need
> to ensure that every access is revalidated, particularly useful for
> scenarios where we expect the file to be relabeled at runtime in
> order
> to reflect state changes (e.g. cross-domain solution, assured
> pipeline
> without data copying).
> 
> Signed-off-by: Stephen Smalley 
> ---
> NB I chose not to define a new policy capability for this permission,
> since it is adequately covered by handle_unknown for compatibility
> and
> others seemed to agree that this does not fall into the category of
> changes requiring a new policy capability.  I also chose to define
> the
> permission for socket classes in addition to file classes and let it
> be checked for both.
> 
>  security/selinux/hooks.c| 12 
>  security/selinux/include/classmap.h |  2 +-
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index e67a526..5432628 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -3550,6 +3550,18 @@ static int selinux_mmap_addr(unsigned long
> addr)
>  static int selinux_mmap_file(struct file *file, unsigned long
> reqprot,
>    unsigned long prot, unsigned long
> flags)
>  {
> + struct common_audit_data ad;
> + int rc;
> +
> + if (file) {
> + ad.type = LSM_AUDIT_DATA_FILE;
> + ad.u.file = file;
> + rc = inode_has_perm(current_cred(),
> file_inode(file),
> + FILE__MAP, );
> + if (rc)
> + return rc;
> + }
> +
>   if (selinux_checkreqprot)
>   prot = reqprot;
>  
> diff --git a/security/selinux/include/classmap.h
> b/security/selinux/include/classmap.h
> index 1e0cc9b..3e49a78 100644
> --- a/security/selinux/include/classmap.h
> +++ b/security/selinux/include/classmap.h
> @@ -1,7 +1,7 @@
>  #include 
>  
>  #define COMMON_FILE_SOCK_PERMS "ioctl", "read", "write", "create", \
> -"getattr", "setattr", "lock", "relabelfrom", "relabelto",
> "append"
> +"getattr", "setattr", "lock", "relabelfrom", "relabelto",
> "append", "map"
>  
>  #define COMMON_FILE_PERMS COMMON_FILE_SOCK_PERMS, "unlink", "link",
> \
>  "rename", "execute", "quotaon", "mounton", "audit_access", \

Below is the refpolicy patch (relative to the Fedora rawhide policy
source RPM) I used for testing.  I simply added the map permission
declaration to the common file and common socket definitions since I
wasn't concerned with compatibility for kernels that predate the
dynamic class/perm mapping support (< 2.6.33, RHEL < 6).  Not sure
whether that's acceptable for upstream refpolicy or not; if not, it
will need to be added at the end of the existing lists for all file and
socket classes.  With regard to allow rules, I initially tried just
adding map permission to mmap_file_perms and exec_file_perms, which
covered most shared libraries and executables, but quickly ran into a
number of cases where programs and libraries required it just for
mmap'ing data files.  I addressed several of those but more kept
popping up.  Abstractly, any domain that previously was allowed read
permission could have used mmap, so if the goal is to ensure 100%
backward compatibility in refpolicy, we would need to add it to every
rule that allows read permission.  In my limited testing, it seemed to
suffice to add it to the _file_perms and _chr_file_perms macros that
already allowed open (although technically a process that
inherits/receives a descriptor could mmap it as well).  I think it is
ok to allow map permission widely in refpolicy (as with open
permission), and leave restricting it to specialized domains (as with
sandbox domains for open) given its narrow use case.

diff -ru serefpolicy-3.13.1.orig/policy/flask/access_vectors 
serefpolicy-3.13.1/policy/flask/access_vectors
--- serefpolicy-3.13.1.orig/policy/flask/access_vectors 2017-05-03 
16:25:32.750029182 -0400
+++ serefpolicy-3.13.1/policy/flask/access_vectors  2017-05-04 
09:18:58.999737761 -0400
@@ -20,6 +20,7 @@
relabelfrom
relabelto
append
+   map

Re: [RFC][PATCH] selinux: add a map permission check for mmap

2017-05-05 Thread Joshua Brindle

Stephen Smalley wrote:

Add a map permission check on mmap so that we can distinguish memory mapped
access (since it has different implications for revocation). When a file
is opened and then read or written via syscalls like read(2)/write(2),
we revalidate access on each read/write operation via
selinux_file_permission() and therefore can revoke access if the
process context, the file context, or the policy changes in such a
manner that access is no longer allowed. When a file is opened and then
memory mapped via mmap(2) and then subsequently read or written directly
in memory, we presently have no way to revalidate or revoke access.
The purpose of a separate map permission check on mmap(2) is to permit
policy to prohibit memory mapping of specific files for which we need
to ensure that every access is revalidated, particularly useful for
scenarios where we expect the file to be relabeled at runtime in order
to reflect state changes (e.g. cross-domain solution, assured pipeline
without data copying).

Signed-off-by: Stephen Smalley
---
NB I chose not to define a new policy capability for this permission,
since it is adequately covered by handle_unknown for compatibility and
others seemed to agree that this does not fall into the category of
changes requiring a new policy capability.  I also chose to define the
permission for socket classes in addition to file classes and let it
be checked for both.


Thank you. This is very helpful. This fully closes the relabel 
revocation issue, right?


Is it actually safe to tell people that relabel+move is sufficient in an 
assured pipeline now?




  security/selinux/hooks.c| 12 
  security/selinux/include/classmap.h |  2 +-
  2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index e67a526..5432628 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3550,6 +3550,18 @@ static int selinux_mmap_addr(unsigned long addr)
  static int selinux_mmap_file(struct file *file, unsigned long reqprot,
 unsigned long prot, unsigned long flags)
  {
+   struct common_audit_data ad;
+   int rc;
+
+   if (file) {
+   ad.type = LSM_AUDIT_DATA_FILE;
+   ad.u.file = file;
+   rc = inode_has_perm(current_cred(), file_inode(file),
+   FILE__MAP,);
+   if (rc)
+   return rc;
+   }
+
if (selinux_checkreqprot)
prot = reqprot;

diff --git a/security/selinux/include/classmap.h 
b/security/selinux/include/classmap.h
index 1e0cc9b..3e49a78 100644
--- a/security/selinux/include/classmap.h
+++ b/security/selinux/include/classmap.h
@@ -1,7 +1,7 @@
  #include

  #define COMMON_FILE_SOCK_PERMS "ioctl", "read", "write", "create", \
-"getattr", "setattr", "lock", "relabelfrom", "relabelto", "append"
+"getattr", "setattr", "lock", "relabelfrom", "relabelto", "append", "map"

  #define COMMON_FILE_PERMS COMMON_FILE_SOCK_PERMS, "unlink", "link", \
  "rename", "execute", "quotaon", "mounton", "audit_access", \




[PATCH 3/3] libsemanage: replace access(, F_OK) checks to make setuid programs work

2017-05-05 Thread Vit Mojzis
access() uses real UID instead of effective UID which causes false
negative checks in setuid programs.
Replace access(,F_OK) (i.e. tests for file existence) by stat().

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1186431

Signed-off-by: Vit Mojzis 
---
 libsemanage/src/direct_api.c | 36 +---
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c
index 508277d..35ca1b0 100644
--- a/libsemanage/src/direct_api.c
+++ b/libsemanage/src/direct_api.c
@@ -272,7 +272,9 @@ int semanage_direct_connect(semanage_handle_t * sh)
 
/* set the disable dontaudit value */
path = semanage_path(SEMANAGE_ACTIVE, SEMANAGE_DISABLE_DONTAUDIT);
-   if (access(path, F_OK) == 0)
+
+   struct stat sb;
+   if (stat(path, ) == 0)
sepol_set_disable_dontaudit(sh->sepolh, 1);
else
sepol_set_disable_dontaudit(sh->sepolh, 0);
@@ -1101,8 +1103,9 @@ static int semanage_compile_hll_modules(semanage_handle_t 
*sh,
goto cleanup;
}
 
+   struct stat sb;
if (semanage_get_ignore_module_cache(sh) == 0 &&
-   access(cil_path, F_OK) == 0) {
+   stat(cil_path, ) == 0) {
continue;
}
 
@@ -1165,7 +1168,8 @@ static int semanage_direct_commit(semanage_handle_t * sh)
 
/* Create or remove the disable_dontaudit flag file. */
path = semanage_path(SEMANAGE_TMP, SEMANAGE_DISABLE_DONTAUDIT);
-   if (access(path, F_OK) == 0)
+   struct stat sb;
+   if (stat(path, ) == 0)
do_rebuild |= !(sepol_get_disable_dontaudit(sh->sepolh) == 1);
else
do_rebuild |= (sepol_get_disable_dontaudit(sh->sepolh) == 1);
@@ -1190,7 +1194,7 @@ static int semanage_direct_commit(semanage_handle_t * sh)
 
/* Create or remove the preserve_tunables flag file. */
path = semanage_path(SEMANAGE_TMP, SEMANAGE_PRESERVE_TUNABLES);
-   if (access(path, F_OK) == 0)
+   if (stat(path, ) == 0)
do_rebuild |= !(sepol_get_preserve_tunables(sh->sepolh) == 1);
else
do_rebuild |= (sepol_get_preserve_tunables(sh->sepolh) == 1);
@@ -1231,37 +1235,37 @@ static int semanage_direct_commit(semanage_handle_t * 
sh)
 */
if (!do_rebuild) {
path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_KERNEL);
-   if (access(path, F_OK) != 0) {
+   if (stat(path, ) != 0) {
do_rebuild = 1;
goto rebuild;
}
 
path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_FC);
-   if (access(path, F_OK) != 0) {
+   if (stat(path, ) != 0) {
do_rebuild = 1;
goto rebuild;
}
 
path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_SEUSERS);
-   if (access(path, F_OK) != 0) {
+   if (stat(path, ) != 0) {
do_rebuild = 1;
goto rebuild;
}
 
path = semanage_path(SEMANAGE_TMP, SEMANAGE_LINKED);
-   if (access(path, F_OK) != 0) {
+   if (stat(path, ) != 0) {
do_rebuild = 1;
goto rebuild;
}
 
path = semanage_path(SEMANAGE_TMP, SEMANAGE_SEUSERS_LINKED);
-   if (access(path, F_OK) != 0) {
+   if (stat(path, ) != 0) {
do_rebuild = 1;
goto rebuild;
}
 
path = semanage_path(SEMANAGE_TMP, SEMANAGE_USERS_EXTRA_LINKED);
-   if (access(path, F_OK) != 0) {
+   if (stat(path, ) != 0) {
do_rebuild = 1;
goto rebuild;
}
@@ -1395,7 +1399,7 @@ rebuild:
goto cleanup;
 
path = semanage_path(SEMANAGE_TMP, SEMANAGE_SEUSERS_LINKED);
-   if (access(path, F_OK) == 0) {
+   if (stat(path, ) == 0) {
retval = semanage_copy_file(path,
semanage_path(SEMANAGE_TMP,
  
SEMANAGE_STORE_SEUSERS),
@@ -1408,7 +1412,7 @@ rebuild:
}
 
path = semanage_path(SEMANAGE_TMP, SEMANAGE_USERS_EXTRA_LINKED);
-   if (access(path, F_OK) == 0) {
+   if (stat(path, ) == 0) {
retval = semanage_copy_file(path,
semanage_path(SEMANAGE_TMP,
  
SEMANAGE_USERS_EXTRA),
@@ -1732,7 +1736,8 @@ static int 

[PATCH 2/3] libsemanage: remove access() check to make setuid programs work

2017-05-05 Thread Vit Mojzis
F_OK access checks only work properly as long as all directories along
the path are accessible to real user running the program.
Replace F_OK access checks by testing return value of open, write, etc.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1186431
---
 libsemanage/src/direct_api.c | 40 +++-
 1 file changed, 15 insertions(+), 25 deletions(-)

diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c
index dc46f8f..508277d 100644
--- a/libsemanage/src/direct_api.c
+++ b/libsemanage/src/direct_api.c
@@ -342,10 +342,6 @@ static int semanage_direct_disconnect(semanage_handle_t * 
sh)
 
 static int semanage_direct_begintrans(semanage_handle_t * sh)
 {
-
-   if (semanage_access_check(sh) != SEMANAGE_CAN_WRITE) {
-   return -1;
-   }
if (semanage_get_trans_lock(sh) < 0) {
return -1;
}
@@ -1491,33 +1487,27 @@ rebuild:
}
 
path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_FC_LOCAL);
-   if (access(path, F_OK) == 0) {
-   retval = semanage_copy_file(semanage_path(SEMANAGE_TMP, 
SEMANAGE_STORE_FC_LOCAL),
-   
semanage_final_path(SEMANAGE_FINAL_TMP, SEMANAGE_FC_LOCAL),
-   sh->conf->file_mode);
-   if (retval < 0) {
-   goto cleanup;
-   }
+   retval = semanage_copy_file(semanage_path(SEMANAGE_TMP, 
SEMANAGE_STORE_FC_LOCAL),
+   
semanage_final_path(SEMANAGE_FINAL_TMP, SEMANAGE_FC_LOCAL),
+   sh->conf->file_mode);
+   if (retval < 0 && errno != ENOENT) {
+   goto cleanup;
}
 
path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_FC);
-   if (access(path, F_OK) == 0) {
-   retval = semanage_copy_file(semanage_path(SEMANAGE_TMP, 
SEMANAGE_STORE_FC),
-   
semanage_final_path(SEMANAGE_FINAL_TMP, SEMANAGE_FC),
-   sh->conf->file_mode);
-   if (retval < 0) {
-   goto cleanup;
-   }
+   retval = semanage_copy_file(semanage_path(SEMANAGE_TMP, 
SEMANAGE_STORE_FC),
+   
semanage_final_path(SEMANAGE_FINAL_TMP, SEMANAGE_FC),
+   sh->conf->file_mode);
+   if (retval < 0 && errno != ENOENT) {
+   goto cleanup;
}
 
path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_SEUSERS);
-   if (access(path, F_OK) == 0) {
-   retval = semanage_copy_file(semanage_path(SEMANAGE_TMP, 
SEMANAGE_STORE_SEUSERS),
-   
semanage_final_path(SEMANAGE_FINAL_TMP, SEMANAGE_SEUSERS),
-   sh->conf->file_mode);
-   if (retval < 0) {
-   goto cleanup;
-   }
+   retval = semanage_copy_file(semanage_path(SEMANAGE_TMP, 
SEMANAGE_STORE_SEUSERS),
+   
semanage_final_path(SEMANAGE_FINAL_TMP, SEMANAGE_SEUSERS),
+   sh->conf->file_mode);
+   if (retval < 0 && errno != ENOENT) {
+   goto cleanup;
}
 
/* run genhomedircon if its enabled, this should be the last operation
-- 
2.9.3



libsemanage: remove/replace access() checks to make setuid programs work

2017-05-05 Thread Vit Mojzis
access() uses real UID instead of effective UID which causes false
negative checks in setuid programs.

Following patches remove redundant access checks (where the access check was
followed by open, write,etc. call and the return value is checked), and replace
necessary "access(, F_OK)" checks by "stats()" (e.g. in case where existence of
a file determines if hll module compilation is necessary, or represents some
setting - such as preserve_tunables).

RHBZ #1186431

libsemanage/src/direct_api.c | 79 
---
libsemanage/src/semanage_store.c | 17 -
2 files changed, 44 insertions(+), 52 deletions(-)




Re: [PATCH 1/1] libselinux: add selinuxenforced tool

2017-05-05 Thread Petr Lautrbach
Dne 4.5.2017 v 23:12 Christian Göttsche via Selinux napsal(a):
> Add command line tool selinuxenforced to determine the current SELinux 
> enforced via exit code.
> Useful for script usage or monitoring.

Could the following script do the work?

case $(getenforce) in
 "Permissive") exit 1
  ;;
  "Enforcing") exit 0
  ;;
  "Disabled") exit 2
  ;;
esac


> ---
>  libselinux/man/man8/selinuxenforced.8 | 24 
>  libselinux/utils/.gitignore   |  1 +
>  libselinux/utils/selinuxenforced.c| 33 +
>  3 files changed, 58 insertions(+)
>  create mode 100644 libselinux/man/man8/selinuxenforced.8
>  create mode 100644 libselinux/utils/selinuxenforced.c
> 
> diff --git a/libselinux/man/man8/selinuxenforced.8 
> b/libselinux/man/man8/selinuxenforced.8
> new file mode 100644
> index ..5ef746e5
> --- /dev/null
> +++ b/libselinux/man/man8/selinuxenforced.8
> @@ -0,0 +1,24 @@
> +.TH "selinuxenforced" "8" "4 May 2017" "Security Enhanced Linux" "SELinux 
> Command Line documentation"
> +.SH "NAME"
> +selinuxenforced \- tool to be used within shell scripts to determine if 
> SELinux is in enforced mode
> +.
> +.SH "SYNOPSIS"
> +.B selinuxenforced
> +.
> +.SH "DESCRIPTION"
> +Indicates whether SELinux is in enforced mode or not.
> +.
> +.SH "EXIT STATUS"
> +It exits with status 0 if SELinux is in enforced mode,
> +1 if SELinux is in permissive mode,
> +2 if SELinux is disabled,
> +and 10 if a library call fails.
> +.
> +.SH AUTHOR
> +Christian Göttsche, 
> +.
> +.SH "SEE ALSO"
> +.BR selinux (8),
> +.BR setenforce (8),
> +.BR getenforce (8),
> +.BR selinuxenabled (8)
> diff --git a/libselinux/utils/.gitignore b/libselinux/utils/.gitignore
> index 5cd01025..bc1f4327 100644
> --- a/libselinux/utils/.gitignore
> +++ b/libselinux/utils/.gitignore
> @@ -21,6 +21,7 @@ selabel_partial_match
>  selinux_check_securetty_context
>  selinuxenabled
>  selinuxexeccon
> +selinuxenforced
>  setenforce
>  setfilecon
>  togglesebool
> diff --git a/libselinux/utils/selinuxenforced.c 
> b/libselinux/utils/selinuxenforced.c
> new file mode 100644
> index ..b5e1c8e8
> --- /dev/null
> +++ b/libselinux/utils/selinuxenforced.c
> @@ -0,0 +1,33 @@
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +int main(void)
> +{
> + int rc;
> +
> + rc = is_selinux_enabled();
> + if (rc < 0) {
> + fputs("selinuxenforced:  is_selinux_enabled() failed", stderr);
> + return 10;
> + }
> + if (rc == 1) {
> + rc = security_getenforce();
> + if (rc < 0) {
> + fputs("selinuxenforced:  security_getenforce() failed", 
> stderr);
> + return 10;
> + }
> +
> + if (rc) {
> + // enforced mode
> + return 0;
> + }
> +
> + // permissive mode
> + return 1;
> + }
> +
> + // SELinux disabled
> + return 2;
> +}
> 





Re: [PATCH] libsepol: Add INCLUDEDIR to utils/Makefile

2017-05-05 Thread Petr Lautrbach
Dne 4.5.2017 v 22:49 Stephen Smalley napsal(a):
> On Thu, 2017-05-04 at 16:22 +0200, Petr Lautrbach wrote:
>> The patch is wrong, please disregard.
>>
>> I'm not sure about the right fix in order not to break gentoo use
>> case.
>> I'd just revert fcb5d5c change in libsepol/utils/Makefile for now.
>
> Can't you just specify CFLAGS to make, similar to what is done in the
> top-level Makefile for building with DESTDIR set?
>

I believe that ../include and ../src paths should be always added to CFLAGS
and LDFLAGS in this case to prevent cases when LDFLAGS and CFLAGS are overriden
on a command line without setting paths containing sepol.h and sepol.so.

This way it's already used in libsepol/src/Makefile

Petr


[PATCH] libsepol/utils: Fix build without system sepol.h

2017-05-05 Thread Petr Lautrbach
fcb5d5c removed ../include from CFLAGS from libsepol/utils/Makefile so
that a build tool can't find sepol/sepol.h when only libsepol is built
and a system is without sepol.h in standard paths. It should use its own
sepol.h file during build. `oveeride` needs to be used in order not to
be overridden by values provided on a command line. Same problem applies
to LDFLAGS.

Fixes:
$ make CFLAGS="" LDFLAGS=""
make[1]: Entering directory '/root/selinux/libsepol/utils'
cc chkcon.c  -lsepol -o chkcon
chkcon.c:1:25: fatal error: sepol/sepol.h: No such file or directory
 #include 

$ make CFLAGS="" LDFLAGS=""
...
make -C utils
make[1]: Entering directory '/root/selinux/libsepol/utils'
cc  -I../includechkcon.c  -lsepol -o chkcon
/usr/bin/ld: cannot find -lsepol
collect2: error: ld returned 1 exit status

Signed-off-by: Petr Lautrbach 
---
 libsepol/utils/Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libsepol/utils/Makefile b/libsepol/utils/Makefile
index a13164e1..3b2fb771 100644
--- a/libsepol/utils/Makefile
+++ b/libsepol/utils/Makefile
@@ -3,7 +3,8 @@ PREFIX ?= $(DESTDIR)/usr
 BINDIR ?= $(PREFIX)/bin
 
 CFLAGS ?= -Wall -Werror
-LDFLAGS += -L../src
+override CFLAGS += -I../include
+override LDFLAGS += -L../src
 LDLIBS += -lsepol
 
 TARGETS=$(patsubst %.c,%,$(wildcard *.c))
-- 
2.12.2