diff mbox

[1/1] genhomedircon: support policies using RBACSEP

Message ID 1062ebe32922aec79a0232acfdd0005e9ce124da.1474639773.git.gary.tierney@gmx.com (mailing list archive)
State Not Applicable
Headers show

Commit Message

Gary Tierney Sept. 23, 2016, 2:28 p.m. UTC
Introduces support for generating homedir/user contexts for policies
that implement RBACSEP.  The support works by taking the prefix of a
logins seuser and replacing the role field in their context
specifications with the prefix.  A new option "genhomedircon-rbacsep"
was added to /etc/selinux/semanage.conf to allow toggling this behavior.

The user prefix can be set from both standard kernel policy and CIL:

CIL:
    (user user_u)
    (role user_r)
    (userrole user_u user_r)
    (userprefix user_u user_r)

kernel policy language:
    role user_r;
    user user_u roles { user_r } prefix user_r;

Signed-off-by: Gary Tierney <gary.tierney@gmx.com>
---
 libsemanage/src/conf-parse.y    | 14 +++++++++++++-
 libsemanage/src/conf-scan.l     |  1 +
 libsemanage/src/genhomedircon.c | 30 +++++++++++++++++++++++++++++-
 libsemanage/src/semanage_conf.h |  1 +
 4 files changed, 44 insertions(+), 2 deletions(-)

Comments

Gary Tierney Sept. 23, 2016, 3:43 p.m. UTC | #1
On Fri, Sep 23, 2016 at 03:28:44PM +0100, Gary Tierney wrote:
> Introduces support for generating homedir/user contexts for policies
> that implement RBACSEP.  The support works by taking the prefix of a
> logins seuser and replacing the role field in their context
> specifications with the prefix.  A new option "genhomedircon-rbacsep"
> was added to /etc/selinux/semanage.conf to allow toggling this behavior.
> 
> The user prefix can be set from both standard kernel policy and CIL:
> 
> CIL:
>     (user user_u)
>     (role user_r)
>     (userrole user_u user_r)
>     (userprefix user_u user_r)
> 
> kernel policy language:
>     role user_r;
>     user user_u roles { user_r } prefix user_r;
> 
> Signed-off-by: Gary Tierney <gary.tierney@gmx.com>
> ---
>  libsemanage/src/conf-parse.y    | 14 +++++++++++++-
>  libsemanage/src/conf-scan.l     |  1 +
>  libsemanage/src/genhomedircon.c | 30 +++++++++++++++++++++++++++++-
>  libsemanage/src/semanage_conf.h |  1 +
>  4 files changed, 44 insertions(+), 2 deletions(-)
> 
> diff --git a/libsemanage/src/conf-parse.y b/libsemanage/src/conf-parse.y
> index b527e89..d2112d2 100644
> --- a/libsemanage/src/conf-parse.y
> +++ b/libsemanage/src/conf-parse.y
> @@ -61,7 +61,7 @@ static int parse_errors;
>  
>  %token MODULE_STORE VERSION EXPAND_CHECK FILE_MODE SAVE_PREVIOUS SAVE_LINKED TARGET_PLATFORM COMPILER_DIR IGNORE_MODULE_CACHE STORE_ROOT
>  %token LOAD_POLICY_START SETFILES_START SEFCONTEXT_COMPILE_START DISABLE_GENHOMEDIRCON HANDLE_UNKNOWN USEPASSWD IGNOREDIRS
> -%token BZIP_BLOCKSIZE BZIP_SMALL REMOVE_HLL
> +%token BZIP_BLOCKSIZE BZIP_SMALL REMOVE_HLL GENHOMEDIRCON_RBACSEP
>  %token VERIFY_MOD_START VERIFY_LINKED_START VERIFY_KERNEL_START BLOCK_END
>  %token PROG_PATH PROG_ARGS
>  %token <s> ARG
> @@ -95,6 +95,7 @@ single_opt:     module_store
>  	|	bzip_blocksize
>  	|	bzip_small
>  	|	remove_hll
> +	|	genhomedircon_rbacsep
>          ;
>  
>  module_store:   MODULE_STORE '=' ARG {
> @@ -268,6 +269,17 @@ remove_hll:  REMOVE_HLL'=' ARG {
>  	free($3);
>  }
>  
> +genhomedircon_rbacsep:  GENHOMEDIRCON_RBACSEP'=' ARG {
> +	if (strcasecmp($3, "false") == 0) {
> +		current_conf->genhomedircon_rbacsep = 0;
> +	} else if (strcasecmp($3, "true") == 0) {
> +		current_conf->genhomedircon_rbacsep = 1;
> +	} else {
> +		yyerror("genhomedircon-rbacsep can only be 'true' or 'false'");
> +	}
> +	free($3);
> +}
> +
>  command_block: 
>                  command_start external_opts BLOCK_END  {
>                          if (new_external->path == NULL) {
> diff --git a/libsemanage/src/conf-scan.l b/libsemanage/src/conf-scan.l
> index 607bbf0..114098c 100644
> --- a/libsemanage/src/conf-scan.l
> +++ b/libsemanage/src/conf-scan.l
> @@ -54,6 +54,7 @@ handle-unknown    return HANDLE_UNKNOWN;
>  bzip-blocksize	return BZIP_BLOCKSIZE;
>  bzip-small	return BZIP_SMALL;
>  remove-hll	return REMOVE_HLL;
> +genhomedircon-rbacsep	return GENHOMEDIRCON_RBACSEP;
>  "[load_policy]"   return LOAD_POLICY_START;
>  "[setfiles]"      return SETFILES_START;
>  "[sefcontext_compile]"      return SEFCONTEXT_COMPILE_START;
> diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c
> index 3fc9e7a..98f9ebd 100644
> --- a/libsemanage/src/genhomedircon.c
> +++ b/libsemanage/src/genhomedircon.c
> @@ -71,6 +71,10 @@
>  #define COMMENT_USER_HOME_CONTEXT "\n\n#\n# Home Context for user %s" \
>  			"\n#\n\n"
>  
> +#define WARNING_RBACSEP_INVALID_ROLE  "genhomedircon-rbacsep is enabled, " \
> +			"but the user prefix of " \
> +			"'%s' for %s is not a valid role.  Skipping user."
> +
>  /* placeholders used in the template file
>     which are searched for and replaced */
>  #define TEMPLATE_HOME_ROOT "HOME_ROOT"
> @@ -638,6 +642,11 @@ static int write_contexts(genhomedircon_settings_t *s, FILE *out,
>  			goto fail;
>  		}
>  
> +		if (s->h_semanage->conf->genhomedircon_rbacsep &&
> +		    sepol_context_set_role(sepolh, context, user->prefix) < 0) {
> +		    goto fail;
> +		}
> +
>  		if (sepol_context_to_string(sepolh, context,
>  					    &new_context_str) < 0) {
>  			goto fail;
> @@ -857,7 +866,7 @@ static int setup_fallback_user(genhomedircon_settings_t * s)
>  	int errors = 0;
>  
>  	retval = semanage_seuser_list(s->h_semanage, &seuser_list, &nseusers);
> -	if (retval < 0 || (nseusers < 1)) {
> +	if (retval < 0 || (nseusers < 2)) {
>  		/* if there are no users, this function can't do any other work */
>  		return errors;
>  	}
> @@ -886,6 +895,17 @@ static int setup_fallback_user(genhomedircon_settings_t * s)
>  					level = FALLBACK_LEVEL;
>  			}
>  
> +			if (u && s->h_semanage->conf->genhomedircon_rbacsep &&
> +			    !semanage_user_has_role(u, prefix)) {
> +				WARN(s->h_semanage, WARNING_RBACSEP_INVALID_ROLE,
> +				     prefix, seuname);
> +
> +				errors = STATUS_ERR;
> +				semanage_user_key_free(key);
> +				semanage_user_free(u);
> +				break;
> +			}
> +
>  			if (push_user_entry(&(s->fallback), FALLBACK_NAME,
>  					    FALLBACK_UIDGID, FALLBACK_UIDGID,
>  					    seuname, prefix, "", level,
> @@ -969,6 +989,14 @@ static int add_user(genhomedircon_settings_t * s,
>  		level = FALLBACK_LEVEL;
>  	}
>  
> +	if (s->h_semanage->conf->genhomedircon_rbacsep &&
> +	    !semanage_user_has_role(user, prefix)) {
> +		WARN(s->h_semanage, WARNING_RBACSEP_INVALID_ROLE, prefix, sename);
> +
> +		retval = STATUS_SUCCESS;
> +		goto cleanup;
> +	}
> +
>  	retval = getpwnam_r(name, &pwstorage, rbuf, rbuflen, &pwent);
>  	if (retval != 0 || pwent == NULL) {
>  		if (retval != 0 && retval != ENOENT) {
> diff --git a/libsemanage/src/semanage_conf.h b/libsemanage/src/semanage_conf.h
> index c99ac8c..2c968da 100644
> --- a/libsemanage/src/semanage_conf.h
> +++ b/libsemanage/src/semanage_conf.h
> @@ -46,6 +46,7 @@ typedef struct semanage_conf {
>  	int bzip_blocksize;
>  	int bzip_small;
>  	int remove_hll;
> +	int genhomedircon_rbacsep;
>  	int ignore_module_cache;
>  	char *ignoredirs;	/* ";" separated of list for genhomedircon to ignore */
>  	struct external_prog *load_policy;
> -- 
> 2.4.11
> 
> _______________________________________________
> Selinux mailing list
> Selinux@tycho.nsa.gov
> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
> To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.

perfinion at #selinux on freenode IRC suggested that the genhomedircon-rbacsep
option should be dropped, and instead a RBACSEP context should be chosen first
in all cases.  If validation of this context fails, then it should fall back
to whatever the existing role is.

Anyone have thoughts on this?  This seems to me like a much better solution than
using a new genhomedircon-rbacsep option, but the problem of using
"userprefix" still remains.
Stephen Smalley Sept. 23, 2016, 7:36 p.m. UTC | #2
On 09/23/2016 10:28 AM, Gary Tierney wrote:
> Introduces support for generating homedir/user contexts for policies
> that implement RBACSEP.  The support works by taking the prefix of a
> logins seuser and replacing the role field in their context
> specifications with the prefix.  A new option "genhomedircon-rbacsep"
> was added to /etc/selinux/semanage.conf to allow toggling this behavior.

The user prefix was previously used as a prefix for types, e.g. you
could have:
HOME_DIR/\.gnupg(/.+)?  system_u:object_r:ROLE_gpg_secret_t
and get it replaced with:
/home/[^/]+/\.gnupg(/.+)?       system_u:object_r:user_gpg_secret_t
/root/\.gnupg(/.+)?             system_u:object_r:sysadm_gpg_secret_t

So I guess you could use it for the role field too, but for consistency
you would want it to be:
HOME_DIR/\.gnupg(/.+)?  system_u:ROLE_r:ROLE_gpg_secret_t

and the prefix would still just be "user".

> 
> The user prefix can be set from both standard kernel policy and CIL:
> 
> CIL:
>     (user user_u)
>     (role user_r)
>     (userrole user_u user_r)
>     (userprefix user_u user_r)
> 
> kernel policy language:
>     role user_r;
>     user user_u roles { user_r } prefix user_r;
> 
> Signed-off-by: Gary Tierney <gary.tierney@gmx.com>
> ---
>  libsemanage/src/conf-parse.y    | 14 +++++++++++++-
>  libsemanage/src/conf-scan.l     |  1 +
>  libsemanage/src/genhomedircon.c | 30 +++++++++++++++++++++++++++++-
>  libsemanage/src/semanage_conf.h |  1 +
>  4 files changed, 44 insertions(+), 2 deletions(-)
> 
>
> diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c
> index 3fc9e7a..98f9ebd 100644
> --- a/libsemanage/src/genhomedircon.c
> +++ b/libsemanage/src/genhomedircon.c
> @@ -857,7 +866,7 @@ static int setup_fallback_user(genhomedircon_settings_t * s)
>  	int errors = 0;
>  
>  	retval = semanage_seuser_list(s->h_semanage, &seuser_list, &nseusers);
> -	if (retval < 0 || (nseusers < 1)) {
> +	if (retval < 0 || (nseusers < 2)) {

Why did this test change?

>  		/* if there are no users, this function can't do any other work */
>  		return errors;
>  	}
> @@ -886,6 +895,17 @@ static int setup_fallback_user(genhomedircon_settings_t * s)
>  					level = FALLBACK_LEVEL;
>  			}
>  
> +			if (u && s->h_semanage->conf->genhomedircon_rbacsep &&
> +			    !semanage_user_has_role(u, prefix)) {

I don't think you want to use prefix alone here, since it may be a
prefix rather than a role name.

The kernel policy contains the list of authorized roles for the user, so
libsepol could export that, but that won't tell you anything about a
default.

libselinux get_default_context() and friends are context-sensitive (the
result depends on the caller's context, such that it may differ for
login vs sshd vs gdm and even among multiple distinct instances of any
of these, e.g. if they have different levels), so I don't think you can
use those.

I don't think we presently provide a good way to find this information,
which is why we added the user prefix in the first place.  But it is
intended to be a prefix, not a role.
Gary Tierney Sept. 23, 2016, 8:51 p.m. UTC | #3
On Fri, Sep 23, 2016 at 03:36:47PM -0400, Stephen Smalley wrote:
>On 09/23/2016 10:28 AM, Gary Tierney wrote:
>> Introduces support for generating homedir/user contexts for policies
>> that implement RBACSEP.  The support works by taking the prefix of a
>> logins seuser and replacing the role field in their context
>> specifications with the prefix.  A new option "genhomedircon-rbacsep"
>> was added to /etc/selinux/semanage.conf to allow toggling this behavior.
>
>The user prefix was previously used as a prefix for types, e.g. you
>could have:
>HOME_DIR/\.gnupg(/.+)?  system_u:object_r:ROLE_gpg_secret_t
>and get it replaced with:
>/home/[^/]+/\.gnupg(/.+)?       system_u:object_r:user_gpg_secret_t
>/root/\.gnupg(/.+)?             system_u:object_r:sysadm_gpg_secret_t
>
>So I guess you could use it for the role field too, but for consistency
>you would want it to be:
>HOME_DIR/\.gnupg(/.+)?  system_u:ROLE_r:ROLE_gpg_secret_t
>
>and the prefix would still just be "user".
>

That would work for us currently with refpolicy, but if I write a 
similar CIL statement:

(filecon "HOME_DIR/\.gnupg(/.+)?" (system_u ROLE_r ROLE_gpg_secret_t))

Then I get a compile error because secilc thinks ROLE_r is the name of 
the role.  I don't think there's any way to work around this in CIL.

>>
>> The user prefix can be set from both standard kernel policy and CIL:
>>
>> CIL:
>>     (user user_u)
>>     (role user_r)
>>     (userrole user_u user_r)
>>     (userprefix user_u user_r)
>>
>> kernel policy language:
>>     role user_r;
>>     user user_u roles { user_r } prefix user_r;
>>
>> Signed-off-by: Gary Tierney <gary.tierney@gmx.com>
>> ---
>>  libsemanage/src/conf-parse.y    | 14 +++++++++++++-
>>  libsemanage/src/conf-scan.l     |  1 +
>>  libsemanage/src/genhomedircon.c | 30 +++++++++++++++++++++++++++++-
>>  libsemanage/src/semanage_conf.h |  1 +
>>  4 files changed, 44 insertions(+), 2 deletions(-)
>>
>>
>> diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c
>> index 3fc9e7a..98f9ebd 100644
>> --- a/libsemanage/src/genhomedircon.c
>> +++ b/libsemanage/src/genhomedircon.c
>> @@ -857,7 +866,7 @@ static int setup_fallback_user(genhomedircon_settings_t * s)
>>  	int errors = 0;
>>
>>  	retval = semanage_seuser_list(s->h_semanage, &seuser_list, &nseusers);
>> -	if (retval < 0 || (nseusers < 1)) {
>> +	if (retval < 0 || (nseusers < 2)) {
>
>Why did this test change?
>

My mistake, didn't intend to include that in this patch.

>>  		/* if there are no users, this function can't do any other 
>>  		work */
>>  		return errors;
>>  	}
>> @@ -886,6 +895,17 @@ static int setup_fallback_user(genhomedircon_settings_t * s)
>>  					level = FALLBACK_LEVEL;
>>  			}
>>
>> +			if (u && s->h_semanage->conf->genhomedircon_rbacsep &&
>> +			    !semanage_user_has_role(u, prefix)) {
>
>I don't think you want to use prefix alone here, since it may be a
>prefix rather than a role name.
>
>The kernel policy contains the list of authorized roles for the user, so
>libsepol could export that, but that won't tell you anything about a
>default.
>
>libselinux get_default_context() and friends are context-sensitive (the
>result depends on the caller's context, such that it may differ for
>login vs sshd vs gdm and even among multiple distinct instances of any
>of these, e.g. if they have different levels), so I don't think you can
>use those.
>
>I don't think we presently provide a good way to find this information,
>which is why we added the user prefix in the first place.  But it is
>intended to be a prefix, not a role.

Do you have any suggestions on how this information could be configured? 
I toyed with extending the policy language to support something like a 
"userhomedirrole" statement, but didn't look too much into what the 
implications of that would be.  That doesn't seem too suitable either 
since it'd only be used by genhomedircon.

I agree that the prefix isn't really suitable and this is more of a hack 
than a good solution (I forgot to include RFC in the subject!), though 
I'm unsure what the next step would be in creating a good solution for 
this.  Any input would be appreciated.
Dac Override Sept. 24, 2016, 8:26 a.m. UTC | #4
On 09/23/2016 09:36 PM, Stephen Smalley wrote:
> On 09/23/2016 10:28 AM, Gary Tierney wrote:
>> Introduces support for generating homedir/user contexts for policies
>> that implement RBACSEP.  The support works by taking the prefix of a
>> logins seuser and replacing the role field in their context
>> specifications with the prefix.  A new option "genhomedircon-rbacsep"
>> was added to /etc/selinux/semanage.conf to allow toggling this behavior.
> 
> The user prefix was previously used as a prefix for types, e.g. you
> could have:
> HOME_DIR/\.gnupg(/.+)?  system_u:object_r:ROLE_gpg_secret_t
> and get it replaced with:
> /home/[^/]+/\.gnupg(/.+)?       system_u:object_r:user_gpg_secret_t
> /root/\.gnupg(/.+)?             system_u:object_r:sysadm_gpg_secret_t
> 
> So I guess you could use it for the role field too, but for consistency
> you would want it to be:
> HOME_DIR/\.gnupg(/.+)?  system_u:ROLE_r:ROLE_gpg_secret_t
> 
> and the prefix would still just be "user".

No one is actually using that privsep functionality anymore. Reference
policy removed support for it.

Can we not instead just re-use that code for rbacsep?

The alternative would be to add code similar to the privsep code but
then for rbacsep.

Then we will have the issue that we can't reasonably rely on the
userprefix and prefix statements for rbacsep, because they might be used
for privsep (in theory at least)

I other words if we were to implement rbacsep code similar to the
privsep code, then we would need a new policy statement similar to
userprefix and prefix.

It seems much easier to me to just re-use the privsep code.

rbacsep is the successor to privsep after all.

> 
>>
>> The user prefix can be set from both standard kernel policy and CIL:
>>
>> CIL:
>>     (user user_u)
>>     (role user_r)
>>     (userrole user_u user_r)
>>     (userprefix user_u user_r)
>>
>> kernel policy language:
>>     role user_r;
>>     user user_u roles { user_r } prefix user_r;
>>
>> Signed-off-by: Gary Tierney <gary.tierney@gmx.com>
>> ---
>>  libsemanage/src/conf-parse.y    | 14 +++++++++++++-
>>  libsemanage/src/conf-scan.l     |  1 +
>>  libsemanage/src/genhomedircon.c | 30 +++++++++++++++++++++++++++++-
>>  libsemanage/src/semanage_conf.h |  1 +
>>  4 files changed, 44 insertions(+), 2 deletions(-)
>>
>>
>> diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c
>> index 3fc9e7a..98f9ebd 100644
>> --- a/libsemanage/src/genhomedircon.c
>> +++ b/libsemanage/src/genhomedircon.c
>> @@ -857,7 +866,7 @@ static int setup_fallback_user(genhomedircon_settings_t * s)
>>  	int errors = 0;
>>  
>>  	retval = semanage_seuser_list(s->h_semanage, &seuser_list, &nseusers);
>> -	if (retval < 0 || (nseusers < 1)) {
>> +	if (retval < 0 || (nseusers < 2)) {
> 
> Why did this test change?
> 
>>  		/* if there are no users, this function can't do any other work */
>>  		return errors;
>>  	}
>> @@ -886,6 +895,17 @@ static int setup_fallback_user(genhomedircon_settings_t * s)
>>  					level = FALLBACK_LEVEL;
>>  			}
>>  
>> +			if (u && s->h_semanage->conf->genhomedircon_rbacsep &&
>> +			    !semanage_user_has_role(u, prefix)) {
> 
> I don't think you want to use prefix alone here, since it may be a
> prefix rather than a role name.
> 
> The kernel policy contains the list of authorized roles for the user, so
> libsepol could export that, but that won't tell you anything about a
> default.
> 
> libselinux get_default_context() and friends are context-sensitive (the
> result depends on the caller's context, such that it may differ for
> login vs sshd vs gdm and even among multiple distinct instances of any
> of these, e.g. if they have different levels), so I don't think you can
> use those.
> 
> I don't think we presently provide a good way to find this information,
> which is why we added the user prefix in the first place.  But it is
> intended to be a prefix, not a role.

Adding a rbacsep=y option to semanage.conf might not be the prettiest
solution but it works, plus is that not what semanage.conf is for in the
first place?

> _______________________________________________
> Selinux mailing list
> Selinux@tycho.nsa.gov
> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
> To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.
>
Stephen Smalley Sept. 26, 2016, 1:41 p.m. UTC | #5
On 09/23/2016 11:43 AM, Gary Tierney wrote:
> On Fri, Sep 23, 2016 at 03:28:44PM +0100, Gary Tierney wrote:
>> Introduces support for generating homedir/user contexts for
>> policies that implement RBACSEP.  The support works by taking the
>> prefix of a logins seuser and replacing the role field in their
>> context specifications with the prefix.  A new option
>> "genhomedircon-rbacsep" was added to /etc/selinux/semanage.conf
>> to allow toggling this behavior.
>> 
>> The user prefix can be set from both standard kernel policy and
>> CIL:
>> 
>> CIL: (user user_u) (role user_r) (userrole user_u user_r) 
>> (userprefix user_u user_r)
>> 
>> kernel policy language: role user_r; user user_u roles { user_r }
>> prefix user_r;
>> 
>> Signed-off-by: Gary Tierney <gary.tierney@gmx.com>
<snip>
> perfinion at #selinux on freenode IRC suggested that the
> genhomedircon-rbacsep option should be dropped, and instead a
> RBACSEP context should be chosen first in all cases.  If validation
> of this context fails, then it should fall back to whatever the
> existing role is.
> 
> Anyone have thoughts on this?  This seems to me like a much better
> solution than using a new genhomedircon-rbacsep option, but the
> problem of using "userprefix" still remains.

Does a valid RBACSEP context necessarily mean that RBACSEP was
desired/intended? (likely: otherwise the role-type association
wouldn't be authorized)

Does an invalid RBACSEP context necessarily mean that RBACSEP was not
desired/intended? (unclear: what if we just forgot the role-type
statement for a particular type)

Also, we base it on context validation, then it could differ for
different entries - some could end up with RBACSEP applied and others not.
Stephen Smalley Sept. 26, 2016, 2:20 p.m. UTC | #6
On 09/24/2016 04:26 AM, Dominick Grift wrote:
> On 09/23/2016 09:36 PM, Stephen Smalley wrote:
>> On 09/23/2016 10:28 AM, Gary Tierney wrote:
>>> Introduces support for generating homedir/user contexts for
>>> policies that implement RBACSEP.  The support works by taking
>>> the prefix of a logins seuser and replacing the role field in
>>> their context specifications with the prefix.  A new option
>>> "genhomedircon-rbacsep" was added to /etc/selinux/semanage.conf
>>> to allow toggling this behavior.
>> 
>> The user prefix was previously used as a prefix for types, e.g.
>> you could have: HOME_DIR/\.gnupg(/.+)?
>> system_u:object_r:ROLE_gpg_secret_t and get it replaced with: 
>> /home/[^/]+/\.gnupg(/.+)?
>> system_u:object_r:user_gpg_secret_t /root/\.gnupg(/.+)?
>> system_u:object_r:sysadm_gpg_secret_t
>> 
>> So I guess you could use it for the role field too, but for
>> consistency you would want it to be: HOME_DIR/\.gnupg(/.+)?
>> system_u:ROLE_r:ROLE_gpg_secret_t
>> 
>> and the prefix would still just be "user".
> 
> No one is actually using that privsep functionality anymore.
> Reference policy removed support for it.
> 
> Can we not instead just re-use that code for rbacsep?
> 
> The alternative would be to add code similar to the privsep code
> but then for rbacsep.
> 
> Then we will have the issue that we can't reasonably rely on the 
> userprefix and prefix statements for rbacsep, because they might be
> used for privsep (in theory at least)
> 
> I other words if we were to implement rbacsep code similar to the 
> privsep code, then we would need a new policy statement similar to 
> userprefix and prefix.
> 
> It seems much easier to me to just re-use the privsep code.
> 
> rbacsep is the successor to privsep after all.

First, I'm not sure what you mean by privsep; usually that term refers
to privilege separation as in openssh.

There are at least three ways of implementing "role" separation for
objects in SELinux:
(1) via TE and the use of derived types on objects e.g. ROLE_home_t,
ROLE_devpts_t, etc,
(2) via RBAC and the use of roles on objects (originally problematic
because it required a set of changes to the kernel to support roles on
objects, but that's all history now),
(3) via UBAC and the use of SELinux user identities on objects to
represent roles.

refpolicy started with (1), experimented with (2) and seems to have
settled on (3), likely because (2) wasn't fully supported in the
kernel or userspace for a long time.  I guess libsemanage /
genhomedircon already support (3) adequately.

CIL apparently doesn't support (1), so that's broken regardless.

So I guess reusing the prefix for RBACSEP won't break any existing
users.  That said, it is clearly confusing to use something identified
in the policy language and documentation as a "prefix" for the purpose
of a "default role".  So maybe we should look to rename it in the
language and code, with backward compatibility.  That can be done as a
separate set of changes.  That might also help us with a different
problem - obsoleting security_compute_user() aka /sys/fs/selinux/user
and taking the get_default_context() logic to userspace.

Has anyone compared UBAC vs RBAC now that the kernel and policy
support roles on objects?  Is there a strong reason for refpolicy to
stay with (3) other than compatibility with older distributions and
this genhomedircon issue?
Dac Override Sept. 26, 2016, 2:34 p.m. UTC | #7
On 09/26/2016 04:20 PM, Stephen Smalley wrote:
> On 09/24/2016 04:26 AM, Dominick Grift wrote:
>> On 09/23/2016 09:36 PM, Stephen Smalley wrote:
>>> On 09/23/2016 10:28 AM, Gary Tierney wrote:
>>>> Introduces support for generating homedir/user contexts for
>>>> policies that implement RBACSEP.  The support works by taking
>>>> the prefix of a logins seuser and replacing the role field in
>>>> their context specifications with the prefix.  A new option
>>>> "genhomedircon-rbacsep" was added to /etc/selinux/semanage.conf
>>>> to allow toggling this behavior.
>>>
>>> The user prefix was previously used as a prefix for types, e.g.
>>> you could have: HOME_DIR/\.gnupg(/.+)?
>>> system_u:object_r:ROLE_gpg_secret_t and get it replaced with: 
>>> /home/[^/]+/\.gnupg(/.+)?
>>> system_u:object_r:user_gpg_secret_t /root/\.gnupg(/.+)?
>>> system_u:object_r:sysadm_gpg_secret_t
>>>
>>> So I guess you could use it for the role field too, but for
>>> consistency you would want it to be: HOME_DIR/\.gnupg(/.+)?
>>> system_u:ROLE_r:ROLE_gpg_secret_t
>>>
>>> and the prefix would still just be "user".
>>
>> No one is actually using that privsep functionality anymore.
>> Reference policy removed support for it.
>>
>> Can we not instead just re-use that code for rbacsep?
>>
>> The alternative would be to add code similar to the privsep code
>> but then for rbacsep.
>>
>> Then we will have the issue that we can't reasonably rely on the 
>> userprefix and prefix statements for rbacsep, because they might be
>> used for privsep (in theory at least)
>>
>> I other words if we were to implement rbacsep code similar to the 
>> privsep code, then we would need a new policy statement similar to 
>> userprefix and prefix.
>>
>> It seems much easier to me to just re-use the privsep code.
>>
>> rbacsep is the successor to privsep after all.
> 
> First, I'm not sure what you mean by privsep; usually that term refers
> to privilege separation as in openssh.

with privsep i mean prefixed user (home) content file contexts generated
by genhomedircon.

> 
> There are at least three ways of implementing "role" separation for
> objects in SELinux:
> (1) via TE and the use of derived types on objects e.g. ROLE_home_t,
> ROLE_devpts_t, etc,

I wouldn't compare the two as ROLE_home_t is generated by genhomedircon
and ROLE_devpts_t is not

> (2) via RBAC and the use of roles on objects (originally problematic
> because it required a set of changes to the kernel to support roles on
> objects, but that's all history now),

This is the way forward IMHO

> (3) via UBAC and the use of SELinux user identities on objects to
> represent roles.

UBAC or more accurately IBACSEP is inadequate because you cannot
implement automatic identity transitions as you can automatic role
transitions.

> 
> refpolicy started with (1), experimented with (2) and seems to have
> settled on (3), likely because (2) wasn't fully supported in the
> kernel or userspace for a long time.  I guess libsemanage /
> genhomedircon already support (3) adequately.

1. was removed (i think due to redhat objecting to it). 2. refpolicy
settled on 3 because at that time 2 was not an option (defaultrole did
not exist). libsemanage supports 3 out of the box yes but we dont have
automatic identity transitions and this makes RBACSEP a more flexible
alternative. we need to be able to deal with instances where a automatic
role transition is desired.

> 
> CIL apparently doesn't support (1), so that's broken regardless.

(1) is currently "broken", yes or legacy

> 
> So I guess reusing the prefix for RBACSEP won't break any existing
> users.  That said, it is clearly confusing to use something identified
> in the policy language and documentation as a "prefix" for the purpose
> of a "default role".  So maybe we should look to rename it in the
> language and code, with backward compatibility.  That can be done as a
> separate set of changes.  That might also help us with a different
> problem - obsoleting security_compute_user() aka /sys/fs/selinux/user
> and taking the get_default_context() logic to userspace.
> 

The name prefix, and userprefix would indeed ideally need to be changed
by I would say that this would be a second step.

> Has anyone compared UBAC vs RBAC now that the kernel and policy
> support roles on objects?  Is there a strong reason for refpolicy to
> stay with (3) other than compatibility with older distributions and
> this genhomedircon issue?

IBACSEP is not flexible enough mainly because we cannot define automatic
identity transitions.

Also until recently we did not have user attributes. but now we do so
that is no longer an issue.

>
Dac Override Sept. 26, 2016, 3:06 p.m. UTC | #8
On 09/26/2016 04:34 PM, Dominick Grift wrote:
> On 09/26/2016 04:20 PM, Stephen Smalley wrote:
>> On 09/24/2016 04:26 AM, Dominick Grift wrote:
>>> On 09/23/2016 09:36 PM, Stephen Smalley wrote:
>>>> On 09/23/2016 10:28 AM, Gary Tierney wrote:
>>>>> Introduces support for generating homedir/user contexts for
>>>>> policies that implement RBACSEP.  The support works by taking
>>>>> the prefix of a logins seuser and replacing the role field in
>>>>> their context specifications with the prefix.  A new option
>>>>> "genhomedircon-rbacsep" was added to /etc/selinux/semanage.conf
>>>>> to allow toggling this behavior.
>>>>
>>>> The user prefix was previously used as a prefix for types, e.g.
>>>> you could have: HOME_DIR/\.gnupg(/.+)?
>>>> system_u:object_r:ROLE_gpg_secret_t and get it replaced with: 
>>>> /home/[^/]+/\.gnupg(/.+)?
>>>> system_u:object_r:user_gpg_secret_t /root/\.gnupg(/.+)?
>>>> system_u:object_r:sysadm_gpg_secret_t
>>>>
>>>> So I guess you could use it for the role field too, but for
>>>> consistency you would want it to be: HOME_DIR/\.gnupg(/.+)?
>>>> system_u:ROLE_r:ROLE_gpg_secret_t
>>>>
>>>> and the prefix would still just be "user".
>>>
>>> No one is actually using that privsep functionality anymore.
>>> Reference policy removed support for it.
>>>
>>> Can we not instead just re-use that code for rbacsep?
>>>
>>> The alternative would be to add code similar to the privsep code
>>> but then for rbacsep.
>>>
>>> Then we will have the issue that we can't reasonably rely on the 
>>> userprefix and prefix statements for rbacsep, because they might be
>>> used for privsep (in theory at least)
>>>
>>> I other words if we were to implement rbacsep code similar to the 
>>> privsep code, then we would need a new policy statement similar to 
>>> userprefix and prefix.
>>>
>>> It seems much easier to me to just re-use the privsep code.
>>>
>>> rbacsep is the successor to privsep after all.
>>
>> First, I'm not sure what you mean by privsep; usually that term refers
>> to privilege separation as in openssh.
> 
> with privsep i mean prefixed user (home) content file contexts generated
> by genhomedircon.
> 
>>
>> There are at least three ways of implementing "role" separation for
>> objects in SELinux:
>> (1) via TE and the use of derived types on objects e.g. ROLE_home_t,
>> ROLE_devpts_t, etc,
> 
> I wouldn't compare the two as ROLE_home_t is generated by genhomedircon
> and ROLE_devpts_t is not
> 
>> (2) via RBAC and the use of roles on objects (originally problematic
>> because it required a set of changes to the kernel to support roles on
>> objects, but that's all history now),
> 
> This is the way forward IMHO
> 
>> (3) via UBAC and the use of SELinux user identities on objects to
>> represent roles.
> 
> UBAC or more accurately IBACSEP is inadequate because you cannot
> implement automatic identity transitions as you can automatic role
> transitions.
> 
>>
>> refpolicy started with (1), experimented with (2) and seems to have
>> settled on (3), likely because (2) wasn't fully supported in the
>> kernel or userspace for a long time.  I guess libsemanage /
>> genhomedircon already support (3) adequately.
> 
> 1. was removed (i think due to redhat objecting to it). 2. refpolicy
> settled on 3 because at that time 2 was not an option (defaultrole did
> not exist). libsemanage supports 3 out of the box yes but we dont have
> automatic identity transitions and this makes RBACSEP a more flexible
> alternative. we need to be able to deal with instances where a automatic
> role transition is desired.
> 
>>
>> CIL apparently doesn't support (1), so that's broken regardless.
> 
> (1) is currently "broken", yes or legacy
> 
>>
>> So I guess reusing the prefix for RBACSEP won't break any existing
>> users.  That said, it is clearly confusing to use something identified
>> in the policy language and documentation as a "prefix" for the purpose
>> of a "default role".  So maybe we should look to rename it in the
>> language and code, with backward compatibility.  That can be done as a
>> separate set of changes.  That might also help us with a different
>> problem - obsoleting security_compute_user() aka /sys/fs/selinux/user
>> and taking the get_default_context() logic to userspace.
>>
> 
> The name prefix, and userprefix would indeed ideally need to be changed
> by I would say that this would be a second step.
> 
>> Has anyone compared UBAC vs RBAC now that the kernel and policy
>> support roles on objects?  Is there a strong reason for refpolicy to
>> stay with (3) other than compatibility with older distributions and
>> this genhomedircon issue?
> 
> IBACSEP is not flexible enough mainly because we cannot define automatic
> identity transitions.
> 
> Also until recently we did not have user attributes. but now we do so
> that is no longer an issue.
> 
>>
> 
> 

Here is one example of how i think automatic role transitions help a little

Example: sudo

Whoever runs sudo first on the the system gets to create
/var/run/sudo/lectured

Imagine two selinux users associated with types that need to be able to
run sudo (joe_u/jane_u)

A user associated with joe_u runs sudo first, thus
/var/run/sudo/lectured gets associated with the joe_u identity. Now the
user associated with jane_u gets lectured all the time because the users
sudo instance cannot access /var/run/sudo/leactured.

With a automatic role transition we can tell SELinux to automatically
role transition to system_r on sudo_var_run_t type dirs. This way
RBACSEP constrained roles can access /var/run/sudo/lectured

An alternative approach would be to lift the RBACSEP constraint from
sudo_var_run_t, but that does not take away the argument that automatic
role transitions make RBACSEP a more flexible alternative

It makes RBACSEP just a small fraction more flexible than IBACSEP.

(ideally the issue would be resolved with a tempfiles.d snippet for
/run/sudo and /run/sudo/lectured (which is implemented on my request)
Dac Override Sept. 27, 2016, 7:44 a.m. UTC | #9
On Mon, Sep 26, 2016 at 10:20:12AM -0400, Stephen Smalley wrote:
> On 09/24/2016 04:26 AM, Dominick Grift wrote:
> > On 09/23/2016 09:36 PM, Stephen Smalley wrote:
> >> On 09/23/2016 10:28 AM, Gary Tierney wrote:
> >>> Introduces support for generating homedir/user contexts for
> >>> policies that implement RBACSEP.  The support works by taking
> >>> the prefix of a logins seuser and replacing the role field in
> >>> their context specifications with the prefix.  A new option
> >>> "genhomedircon-rbacsep" was added to /etc/selinux/semanage.conf
> >>> to allow toggling this behavior.
> >> 
> >> The user prefix was previously used as a prefix for types, e.g.
> >> you could have: HOME_DIR/\.gnupg(/.+)?
> >> system_u:object_r:ROLE_gpg_secret_t and get it replaced with: 
> >> /home/[^/]+/\.gnupg(/.+)?
> >> system_u:object_r:user_gpg_secret_t /root/\.gnupg(/.+)?
> >> system_u:object_r:sysadm_gpg_secret_t
> >> 
> >> So I guess you could use it for the role field too, but for
> >> consistency you would want it to be: HOME_DIR/\.gnupg(/.+)?
> >> system_u:ROLE_r:ROLE_gpg_secret_t
> >> 
> >> and the prefix would still just be "user".
> > 
> > No one is actually using that privsep functionality anymore.
> > Reference policy removed support for it.
> > 
> > Can we not instead just re-use that code for rbacsep?
> > 
> > The alternative would be to add code similar to the privsep code
> > but then for rbacsep.
> > 
> > Then we will have the issue that we can't reasonably rely on the 
> > userprefix and prefix statements for rbacsep, because they might be
> > used for privsep (in theory at least)
> > 
> > I other words if we were to implement rbacsep code similar to the 
> > privsep code, then we would need a new policy statement similar to 
> > userprefix and prefix.
> > 
> > It seems much easier to me to just re-use the privsep code.
> > 
> > rbacsep is the successor to privsep after all.
> 
> First, I'm not sure what you mean by privsep; usually that term refers
> to privilege separation as in openssh.
> 
> There are at least three ways of implementing "role" separation for
> objects in SELinux:
> (1) via TE and the use of derived types on objects e.g. ROLE_home_t,
> ROLE_devpts_t, etc,
> (2) via RBAC and the use of roles on objects (originally problematic
> because it required a set of changes to the kernel to support roles on
> objects, but that's all history now),
> (3) via UBAC and the use of SELinux user identities on objects to
> represent roles.
> 
> refpolicy started with (1), experimented with (2) and seems to have
> settled on (3), likely because (2) wasn't fully supported in the
> kernel or userspace for a long time.  I guess libsemanage /
> genhomedircon already support (3) adequately.
> 
> CIL apparently doesn't support (1), so that's broken regardless.
> 
> So I guess reusing the prefix for RBACSEP won't break any existing
> users.  That said, it is clearly confusing to use something identified
> in the policy language and documentation as a "prefix" for the purpose
> of a "default role".  So maybe we should look to rename it in the
> language and code, with backward compatibility.  That can be done as a
> separate set of changes.  That might also help us with a different
> problem - obsoleting security_compute_user() aka /sys/fs/selinux/user
> and taking the get_default_context() logic to userspace.
> 
> Has anyone compared UBAC vs RBAC now that the kernel and policy
> support roles on objects?  Is there a strong reason for refpolicy to
> stay with (3) other than compatibility with older distributions and
> this genhomedircon issue?
> 

Another likely issue is also with sudo.

An unprivileged user (staff_u) runs sudo to change to root/sysadm_r in order to create a file that should be accessible system wide.

The file he created will end up with staff_u identity and IBACSEP constrained processes might not be able to access it.

This wouldnt be an issue with RBACSEP because the role is changed to sysadm_r, which is not RBACSEP constrained. (no "privileged" roles are constrained)

This IBACSEP sudo issue can be avoided by using: `machinectl shell .host` instead. which will change the context fully to sysadm_u:sysadm_r:sysadm_t

However that is more difficult to configure and depends on systemd
Stephen Smalley Sept. 27, 2016, 1:39 p.m. UTC | #10
On 09/27/2016 03:44 AM, Dominick Grift wrote:
> On Mon, Sep 26, 2016 at 10:20:12AM -0400, Stephen Smalley wrote:
>> On 09/24/2016 04:26 AM, Dominick Grift wrote:
>>> On 09/23/2016 09:36 PM, Stephen Smalley wrote:
>>>> On 09/23/2016 10:28 AM, Gary Tierney wrote:
>>>>> Introduces support for generating homedir/user contexts
>>>>> for policies that implement RBACSEP.  The support works by
>>>>> taking the prefix of a logins seuser and replacing the role
>>>>> field in their context specifications with the prefix.  A
>>>>> new option "genhomedircon-rbacsep" was added to
>>>>> /etc/selinux/semanage.conf to allow toggling this
>>>>> behavior.
>>>> 
>>>> The user prefix was previously used as a prefix for types,
>>>> e.g. you could have: HOME_DIR/\.gnupg(/.+)? 
>>>> system_u:object_r:ROLE_gpg_secret_t and get it replaced with:
>>>>  /home/[^/]+/\.gnupg(/.+)? 
>>>> system_u:object_r:user_gpg_secret_t /root/\.gnupg(/.+)? 
>>>> system_u:object_r:sysadm_gpg_secret_t
>>>> 
>>>> So I guess you could use it for the role field too, but for 
>>>> consistency you would want it to be: HOME_DIR/\.gnupg(/.+)? 
>>>> system_u:ROLE_r:ROLE_gpg_secret_t
>>>> 
>>>> and the prefix would still just be "user".
>>> 
>>> No one is actually using that privsep functionality anymore. 
>>> Reference policy removed support for it.
>>> 
>>> Can we not instead just re-use that code for rbacsep?
>>> 
>>> The alternative would be to add code similar to the privsep
>>> code but then for rbacsep.
>>> 
>>> Then we will have the issue that we can't reasonably rely on
>>> the userprefix and prefix statements for rbacsep, because they
>>> might be used for privsep (in theory at least)
>>> 
>>> I other words if we were to implement rbacsep code similar to
>>> the privsep code, then we would need a new policy statement
>>> similar to userprefix and prefix.
>>> 
>>> It seems much easier to me to just re-use the privsep code.
>>> 
>>> rbacsep is the successor to privsep after all.
>> 
>> First, I'm not sure what you mean by privsep; usually that term
>> refers to privilege separation as in openssh.
>> 
>> There are at least three ways of implementing "role" separation
>> for objects in SELinux: (1) via TE and the use of derived types
>> on objects e.g. ROLE_home_t, ROLE_devpts_t, etc, (2) via RBAC and
>> the use of roles on objects (originally problematic because it
>> required a set of changes to the kernel to support roles on 
>> objects, but that's all history now), (3) via UBAC and the use of
>> SELinux user identities on objects to represent roles.
>> 
>> refpolicy started with (1), experimented with (2) and seems to
>> have settled on (3), likely because (2) wasn't fully supported in
>> the kernel or userspace for a long time.  I guess libsemanage / 
>> genhomedircon already support (3) adequately.
>> 
>> CIL apparently doesn't support (1), so that's broken regardless.
>> 
>> So I guess reusing the prefix for RBACSEP won't break any
>> existing users.  That said, it is clearly confusing to use
>> something identified in the policy language and documentation as
>> a "prefix" for the purpose of a "default role".  So maybe we
>> should look to rename it in the language and code, with backward
>> compatibility.  That can be done as a separate set of changes.
>> That might also help us with a different problem - obsoleting
>> security_compute_user() aka /sys/fs/selinux/user and taking the
>> get_default_context() logic to userspace.
>> 
>> Has anyone compared UBAC vs RBAC now that the kernel and policy 
>> support roles on objects?  Is there a strong reason for refpolicy
>> to stay with (3) other than compatibility with older
>> distributions and this genhomedircon issue?
>> 
> 
> Another likely issue is also with sudo.
> 
> An unprivileged user (staff_u) runs sudo to change to root/sysadm_r
> in order to create a file that should be accessible system wide.
> 
> The file he created will end up with staff_u identity and IBACSEP
> constrained processes might not be able to access it.
> 
> This wouldnt be an issue with RBACSEP because the role is changed
> to sysadm_r, which is not RBACSEP constrained. (no "privileged"
> roles are constrained)
> 
> This IBACSEP sudo issue can be avoided by using: `machinectl shell
> .host` instead. which will change the context fully to
> sysadm_u:sysadm_r:sysadm_t
> 
> However that is more difficult to configure and depends on systemd

Ok, so let's drop the unrelated change from the patch, resolve whether
we want this to be enabled through semanage.conf or some other means,
and get a final patch for it.  And then look in the future toward
renaming things in a backward-compatible manner.
Chris PeBenito Sept. 27, 2016, 10:19 p.m. UTC | #11
On 09/26/16 10:20, Stephen Smalley wrote:
> On 09/24/2016 04:26 AM, Dominick Grift wrote:
>> On 09/23/2016 09:36 PM, Stephen Smalley wrote:
>>> On 09/23/2016 10:28 AM, Gary Tierney wrote:
>>>> Introduces support for generating homedir/user contexts for
>>>> policies that implement RBACSEP.  The support works by taking
>>>> the prefix of a logins seuser and replacing the role field in
>>>> their context specifications with the prefix.  A new option
>>>> "genhomedircon-rbacsep" was added to /etc/selinux/semanage.conf
>>>> to allow toggling this behavior.
>>>
>>> The user prefix was previously used as a prefix for types, e.g.
>>> you could have: HOME_DIR/\.gnupg(/.+)?
>>> system_u:object_r:ROLE_gpg_secret_t and get it replaced with:
>>> /home/[^/]+/\.gnupg(/.+)?
>>> system_u:object_r:user_gpg_secret_t /root/\.gnupg(/.+)?
>>> system_u:object_r:sysadm_gpg_secret_t
>>>
>>> So I guess you could use it for the role field too, but for
>>> consistency you would want it to be: HOME_DIR/\.gnupg(/.+)?
>>> system_u:ROLE_r:ROLE_gpg_secret_t
>>>
>>> and the prefix would still just be "user".
>>
>> No one is actually using that privsep functionality anymore.
>> Reference policy removed support for it.
>>
>> Can we not instead just re-use that code for rbacsep?
>>
>> The alternative would be to add code similar to the privsep code
>> but then for rbacsep.
>>
>> Then we will have the issue that we can't reasonably rely on the
>> userprefix and prefix statements for rbacsep, because they might be
>> used for privsep (in theory at least)
>>
>> I other words if we were to implement rbacsep code similar to the
>> privsep code, then we would need a new policy statement similar to
>> userprefix and prefix.
>>
>> It seems much easier to me to just re-use the privsep code.
>>
>> rbacsep is the successor to privsep after all.
>
> First, I'm not sure what you mean by privsep; usually that term refers
> to privilege separation as in openssh.
>
> There are at least three ways of implementing "role" separation for
> objects in SELinux:
> (1) via TE and the use of derived types on objects e.g. ROLE_home_t,
> ROLE_devpts_t, etc,
> (2) via RBAC and the use of roles on objects (originally problematic
> because it required a set of changes to the kernel to support roles on
> objects, but that's all history now),
> (3) via UBAC and the use of SELinux user identities on objects to
> represent roles.
>
> refpolicy started with (1), experimented with (2) and seems to have
> settled on (3), likely because (2) wasn't fully supported in the
> kernel or userspace for a long time.  I guess libsemanage /
> genhomedircon already support (3) adequately.
>
> CIL apparently doesn't support (1), so that's broken regardless.
>
> So I guess reusing the prefix for RBACSEP won't break any existing
> users.  That said, it is clearly confusing to use something identified
> in the policy language and documentation as a "prefix" for the purpose
> of a "default role".  So maybe we should look to rename it in the
> language and code, with backward compatibility.  That can be done as a
> separate set of changes.  That might also help us with a different
> problem - obsoleting security_compute_user() aka /sys/fs/selinux/user
> and taking the get_default_context() logic to userspace.
>
> Has anyone compared UBAC vs RBAC now that the kernel and policy
> support roles on objects?  Is there a strong reason for refpolicy to
> stay with (3) other than compatibility with older distributions and
> this genhomedircon issue?

I'm open to reimplementing RBAC-based separations in refpolicy.  I 
started working on it but couldn't decide how best to structure the 
role-type associations and if role_transitions or default_role was the 
cleaner way to go to get roles on objects (e.g. consider how to handle 
non-separated types).  These days I'm more interested in getting a 
refpolicy high-level language implemented, now that CIL is generally 
available.  The problem is compilers isn't my thing, so I haven't really 
gotten anywhere yet.
Gary Tierney Sept. 29, 2016, 1:06 a.m. UTC | #12
On Tue, Sep 27, 2016 at 09:39:49AM -0400, Stephen Smalley wrote:
> On 09/27/2016 03:44 AM, Dominick Grift wrote:
> > On Mon, Sep 26, 2016 at 10:20:12AM -0400, Stephen Smalley wrote:
> >> On 09/24/2016 04:26 AM, Dominick Grift wrote:
> >>> On 09/23/2016 09:36 PM, Stephen Smalley wrote:
> >>>> On 09/23/2016 10:28 AM, Gary Tierney wrote:
> >>>>> Introduces support for generating homedir/user contexts
> >>>>> for policies that implement RBACSEP.  The support works by
> >>>>> taking the prefix of a logins seuser and replacing the role
> >>>>> field in their context specifications with the prefix.  A
> >>>>> new option "genhomedircon-rbacsep" was added to
> >>>>> /etc/selinux/semanage.conf to allow toggling this
> >>>>> behavior.
> >>>> 
> >>>> The user prefix was previously used as a prefix for types,
> >>>> e.g. you could have: HOME_DIR/\.gnupg(/.+)? 
> >>>> system_u:object_r:ROLE_gpg_secret_t and get it replaced with:
> >>>>  /home/[^/]+/\.gnupg(/.+)? 
> >>>> system_u:object_r:user_gpg_secret_t /root/\.gnupg(/.+)? 
> >>>> system_u:object_r:sysadm_gpg_secret_t
> >>>> 
> >>>> So I guess you could use it for the role field too, but for 
> >>>> consistency you would want it to be: HOME_DIR/\.gnupg(/.+)? 
> >>>> system_u:ROLE_r:ROLE_gpg_secret_t
> >>>> 
> >>>> and the prefix would still just be "user".
> >>> 
> >>> No one is actually using that privsep functionality anymore. 
> >>> Reference policy removed support for it.
> >>> 
> >>> Can we not instead just re-use that code for rbacsep?
> >>> 
> >>> The alternative would be to add code similar to the privsep
> >>> code but then for rbacsep.
> >>> 
> >>> Then we will have the issue that we can't reasonably rely on
> >>> the userprefix and prefix statements for rbacsep, because they
> >>> might be used for privsep (in theory at least)
> >>> 
> >>> I other words if we were to implement rbacsep code similar to
> >>> the privsep code, then we would need a new policy statement
> >>> similar to userprefix and prefix.
> >>> 
> >>> It seems much easier to me to just re-use the privsep code.
> >>> 
> >>> rbacsep is the successor to privsep after all.
> >> 
> >> First, I'm not sure what you mean by privsep; usually that term
> >> refers to privilege separation as in openssh.
> >> 
> >> There are at least three ways of implementing "role" separation
> >> for objects in SELinux: (1) via TE and the use of derived types
> >> on objects e.g. ROLE_home_t, ROLE_devpts_t, etc, (2) via RBAC and
> >> the use of roles on objects (originally problematic because it
> >> required a set of changes to the kernel to support roles on 
> >> objects, but that's all history now), (3) via UBAC and the use of
> >> SELinux user identities on objects to represent roles.
> >> 
> >> refpolicy started with (1), experimented with (2) and seems to
> >> have settled on (3), likely because (2) wasn't fully supported in
> >> the kernel or userspace for a long time.  I guess libsemanage / 
> >> genhomedircon already support (3) adequately.
> >> 
> >> CIL apparently doesn't support (1), so that's broken regardless.
> >> 
> >> So I guess reusing the prefix for RBACSEP won't break any
> >> existing users.  That said, it is clearly confusing to use
> >> something identified in the policy language and documentation as
> >> a "prefix" for the purpose of a "default role".  So maybe we
> >> should look to rename it in the language and code, with backward
> >> compatibility.  That can be done as a separate set of changes.
> >> That might also help us with a different problem - obsoleting
> >> security_compute_user() aka /sys/fs/selinux/user and taking the
> >> get_default_context() logic to userspace.
> >> 
> >> Has anyone compared UBAC vs RBAC now that the kernel and policy 
> >> support roles on objects?  Is there a strong reason for refpolicy
> >> to stay with (3) other than compatibility with older
> >> distributions and this genhomedircon issue?
> >> 
> > 
> > Another likely issue is also with sudo.
> > 
> > An unprivileged user (staff_u) runs sudo to change to root/sysadm_r
> > in order to create a file that should be accessible system wide.
> > 
> > The file he created will end up with staff_u identity and IBACSEP
> > constrained processes might not be able to access it.
> > 
> > This wouldnt be an issue with RBACSEP because the role is changed
> > to sysadm_r, which is not RBACSEP constrained. (no "privileged"
> > roles are constrained)
> > 
> > This IBACSEP sudo issue can be avoided by using: `machinectl shell
> > .host` instead. which will change the context fully to
> > sysadm_u:sysadm_r:sysadm_t
> > 
> > However that is more difficult to configure and depends on systemd
> 
> Ok, so let's drop the unrelated change from the patch, resolve whether
> we want this to be enabled through semanage.conf or some other means,
> and get a final patch for it.  And then look in the future toward
> renaming things in a backward-compatible manner.

An issue with the semanage.conf option is that switching from an RBACSEP 
policy to a non-RBACSEP policy (or vice versa) could result in no 
homedir contexts being generated if semanage.conf isn't updated to 
reflect that.  This was raised by one of the Gentoo guys r.e. users 
installing policy packages.

So, thinking about the semanage.conf option with refactoring prefix to 
something more suitable in the future in mind I came up with the 
following:

We can drop the option and treat the "prefix" as a homedir role, 
regardless of whether the policy implements RBACSEP or not.  In the 
future this could either be object_r, or a RBACSEP constrained role (or 
even an object role defined in policy with an identifier other than 
object_r).  This way we can do the following checks before trying to 
write out contexts to preserve backwards compatability:

1. Check if the given prefix is a valid role in the policy.
2. Check that the users list of roles contains the prefix, or that the 
role is object_r.
3. Start writing contexts out as normal but with fixed roles.  If an 
error is encountered with a generated context we can issue a warning 
about it and skip that entry, preserving genhomedircons current behavior 
(it's unlikely that we did not intend to use RBACSEP if we got this far, 
since a users prefix typically wouldn't be the identifier of a valid 
role).

If any of the checks leading up to 3 fail, we just write out the 
contexts as we usually would without fixing the role.  This would 
preserve backwards compatability and allow the old privsep model to work 
just based on string substitution.

This approach also makes toggling RBACSEP support for homedirs from the 
policy very easy with just a tunable:

(block user
     (user id)
     (role role)
     (userrole id role)

     (tunableif enable_rbacsep
         (true
             (userprefix id role)) # (userhomerole id role) ?
         (false
             (userprefix id object_r))))

I'm respinning my patch to see how that goes and will get it up later 
today.
diff mbox

Patch

diff --git a/libsemanage/src/conf-parse.y b/libsemanage/src/conf-parse.y
index b527e89..d2112d2 100644
--- a/libsemanage/src/conf-parse.y
+++ b/libsemanage/src/conf-parse.y
@@ -61,7 +61,7 @@  static int parse_errors;
 
 %token MODULE_STORE VERSION EXPAND_CHECK FILE_MODE SAVE_PREVIOUS SAVE_LINKED TARGET_PLATFORM COMPILER_DIR IGNORE_MODULE_CACHE STORE_ROOT
 %token LOAD_POLICY_START SETFILES_START SEFCONTEXT_COMPILE_START DISABLE_GENHOMEDIRCON HANDLE_UNKNOWN USEPASSWD IGNOREDIRS
-%token BZIP_BLOCKSIZE BZIP_SMALL REMOVE_HLL
+%token BZIP_BLOCKSIZE BZIP_SMALL REMOVE_HLL GENHOMEDIRCON_RBACSEP
 %token VERIFY_MOD_START VERIFY_LINKED_START VERIFY_KERNEL_START BLOCK_END
 %token PROG_PATH PROG_ARGS
 %token <s> ARG
@@ -95,6 +95,7 @@  single_opt:     module_store
 	|	bzip_blocksize
 	|	bzip_small
 	|	remove_hll
+	|	genhomedircon_rbacsep
         ;
 
 module_store:   MODULE_STORE '=' ARG {
@@ -268,6 +269,17 @@  remove_hll:  REMOVE_HLL'=' ARG {
 	free($3);
 }
 
+genhomedircon_rbacsep:  GENHOMEDIRCON_RBACSEP'=' ARG {
+	if (strcasecmp($3, "false") == 0) {
+		current_conf->genhomedircon_rbacsep = 0;
+	} else if (strcasecmp($3, "true") == 0) {
+		current_conf->genhomedircon_rbacsep = 1;
+	} else {
+		yyerror("genhomedircon-rbacsep can only be 'true' or 'false'");
+	}
+	free($3);
+}
+
 command_block: 
                 command_start external_opts BLOCK_END  {
                         if (new_external->path == NULL) {
diff --git a/libsemanage/src/conf-scan.l b/libsemanage/src/conf-scan.l
index 607bbf0..114098c 100644
--- a/libsemanage/src/conf-scan.l
+++ b/libsemanage/src/conf-scan.l
@@ -54,6 +54,7 @@  handle-unknown    return HANDLE_UNKNOWN;
 bzip-blocksize	return BZIP_BLOCKSIZE;
 bzip-small	return BZIP_SMALL;
 remove-hll	return REMOVE_HLL;
+genhomedircon-rbacsep	return GENHOMEDIRCON_RBACSEP;
 "[load_policy]"   return LOAD_POLICY_START;
 "[setfiles]"      return SETFILES_START;
 "[sefcontext_compile]"      return SEFCONTEXT_COMPILE_START;
diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c
index 3fc9e7a..98f9ebd 100644
--- a/libsemanage/src/genhomedircon.c
+++ b/libsemanage/src/genhomedircon.c
@@ -71,6 +71,10 @@ 
 #define COMMENT_USER_HOME_CONTEXT "\n\n#\n# Home Context for user %s" \
 			"\n#\n\n"
 
+#define WARNING_RBACSEP_INVALID_ROLE  "genhomedircon-rbacsep is enabled, " \
+			"but the user prefix of " \
+			"'%s' for %s is not a valid role.  Skipping user."
+
 /* placeholders used in the template file
    which are searched for and replaced */
 #define TEMPLATE_HOME_ROOT "HOME_ROOT"
@@ -638,6 +642,11 @@  static int write_contexts(genhomedircon_settings_t *s, FILE *out,
 			goto fail;
 		}
 
+		if (s->h_semanage->conf->genhomedircon_rbacsep &&
+		    sepol_context_set_role(sepolh, context, user->prefix) < 0) {
+		    goto fail;
+		}
+
 		if (sepol_context_to_string(sepolh, context,
 					    &new_context_str) < 0) {
 			goto fail;
@@ -857,7 +866,7 @@  static int setup_fallback_user(genhomedircon_settings_t * s)
 	int errors = 0;
 
 	retval = semanage_seuser_list(s->h_semanage, &seuser_list, &nseusers);
-	if (retval < 0 || (nseusers < 1)) {
+	if (retval < 0 || (nseusers < 2)) {
 		/* if there are no users, this function can't do any other work */
 		return errors;
 	}
@@ -886,6 +895,17 @@  static int setup_fallback_user(genhomedircon_settings_t * s)
 					level = FALLBACK_LEVEL;
 			}
 
+			if (u && s->h_semanage->conf->genhomedircon_rbacsep &&
+			    !semanage_user_has_role(u, prefix)) {
+				WARN(s->h_semanage, WARNING_RBACSEP_INVALID_ROLE,
+				     prefix, seuname);
+
+				errors = STATUS_ERR;
+				semanage_user_key_free(key);
+				semanage_user_free(u);
+				break;
+			}
+
 			if (push_user_entry(&(s->fallback), FALLBACK_NAME,
 					    FALLBACK_UIDGID, FALLBACK_UIDGID,
 					    seuname, prefix, "", level,
@@ -969,6 +989,14 @@  static int add_user(genhomedircon_settings_t * s,
 		level = FALLBACK_LEVEL;
 	}
 
+	if (s->h_semanage->conf->genhomedircon_rbacsep &&
+	    !semanage_user_has_role(user, prefix)) {
+		WARN(s->h_semanage, WARNING_RBACSEP_INVALID_ROLE, prefix, sename);
+
+		retval = STATUS_SUCCESS;
+		goto cleanup;
+	}
+
 	retval = getpwnam_r(name, &pwstorage, rbuf, rbuflen, &pwent);
 	if (retval != 0 || pwent == NULL) {
 		if (retval != 0 && retval != ENOENT) {
diff --git a/libsemanage/src/semanage_conf.h b/libsemanage/src/semanage_conf.h
index c99ac8c..2c968da 100644
--- a/libsemanage/src/semanage_conf.h
+++ b/libsemanage/src/semanage_conf.h
@@ -46,6 +46,7 @@  typedef struct semanage_conf {
 	int bzip_blocksize;
 	int bzip_small;
 	int remove_hll;
+	int genhomedircon_rbacsep;
 	int ignore_module_cache;
 	char *ignoredirs;	/* ";" separated of list for genhomedircon to ignore */
 	struct external_prog *load_policy;