From: cpebenito@tresys.com (Christopher J. PeBenito)
Date: Mon, 23 Apr 2012 09:10:35 -0400
Subject: [refpolicy] [PATCH 1/3] Create non_auth_file_type attribute and
 interfaces
In-Reply-To: <1333656877.4703.40.camel@moss-lions.epoch.ncsc.mil>
References: <1333656877.4703.40.camel@moss-lions.epoch.ncsc.mil>
Message-ID: <4F9554CB.2040104@tresys.com>
To: refpolicy@oss.tresys.com
List-Id: refpolicy.oss.tresys.com

On 04/05/12 16:14, James Carter wrote:
> - Creates a new attribute called non_auth_file_type.
> - Moves auth_file_type attribute declaration from authlogin to files.
> - Creates new interfaces to allow file accesses on non_auth_file_type files.

I'm fine with the changes, though there are a couple things; see inline.

> Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
> ---
>  policy/modules/kernel/files.if     |  163 +++++++++++++++++++++++++++++++++++-
>  policy/modules/kernel/files.te     |    6 ++
>  policy/modules/system/authlogin.te |    3 +-
>  3 files changed, 166 insertions(+), 6 deletions(-)
> 
> diff --git a/policy/modules/kernel/files.if b/policy/modules/kernel/files.if
> index deb24b4..4570d1a 100644
> --- a/policy/modules/kernel/files.if
> +++ b/policy/modules/kernel/files.if
> @@ -78,10 +78,30 @@
>  #
>  interface(`files_type',`
>  	gen_require(`
> -		attribute file_type, non_security_file_type;
> +		attribute file_type, non_security_file_type, non_auth_file_type;
>  	')
>  
> -	typeattribute $1 file_type, non_security_file_type;
> +	typeattribute $1 file_type, non_security_file_type, non_auth_file_type;
> +')
> +
> +########################################
> +## <summary>
> +##	Mark the specified type as a file
> +##  that is related to authentication.
> +## </summary>
> +## <param name="file_type">
> +##	<summary>
> +##	Type of the authentication-related
> +##  file.

There are some whitespace errors here and later in the patch.

> +##	</summary>
> +## </param>
> +#
> +interface(`files_auth_file',`
> +    gen_require(`
> +		attribute file_type, security_file_type, auth_file_type;
> +	')
> +
> +	typeattribute $1 file_type, security_file_type, auth_file_type;
>  ')
>  
>  ########################################
> @@ -99,10 +119,10 @@ interface(`files_type',`
>  #
>  interface(`files_security_file',`
>  	gen_require(`
> -		attribute file_type, security_file_type;
> +		attribute file_type, security_file_type, non_auth_file_type;
>  	')
>  
> -	typeattribute $1 file_type, security_file_type;
> +	typeattribute $1 file_type, security_file_type, non_auth_file_type;
>  ')
>  
>  ########################################
> @@ -669,6 +689,63 @@ interface(`files_read_non_security_files',`

The ordering in this file is messed up, so please don't follow it.  Please collect all the interfaces your adding, and put the auth interfaces first, and then non_auth interfaces.  Put all this after the interfaces that use the file_type attribute.  In my checkout, thats line 1277 (above config file interfaces).

>  ########################################
>  ## <summary>
> +##	Read all non-authentication related
> +##  directories.
> +## </summary>
> +## <param name="domain">
> +##	<summary>
> +##	Domain allowed access.
> +##	</summary>
> +## </param>
> +#
> +interface(`files_read_non_auth_dirs',`

The verb should be list, not read.

> +	gen_require(`
> +		attribute non_auth_file_type;
> +	')
> +
> +	allow $1 non_auth_file_type:dir list_dir_perms;
> +')
> +
> +########################################
> +## <summary>
> +##	Read all non-authentication related
> +##  files.
> +## </summary>
> +## <param name="domain">
> +##	<summary>
> +##	Domain allowed access.
> +##	</summary>
> +## </param>
> +#
> +interface(`files_read_non_auth_files',`
> +	gen_require(`
> +		attribute non_auth_file_type;
> +	')
> +
> +	read_files_pattern($1, non_auth_file_type, non_auth_file_type)
> +')
> +
> +########################################
> +## <summary>
> +##	Read all non-authentication related
> +## symbolic links.
> +## </summary>
> +## <param name="domain">
> +##	<summary>
> +##	Domain allowed access.
> +##	</summary>
> +## </param>
> +#
> +interface(`files_read_non_auth_symlinks',`
> +	gen_require(`
> +		attribute non_auth_file_type;
> +	')
> +
> +	read_lnk_files_pattern($1, non_auth_file_type, non_auth_file_type)
> +')
> +
> +########################################
> +## <summary>
>  ##	Read all directories on the filesystem, except
>  ##	the listed exceptions.
>  ## </summary>
[...]
> diff --git a/policy/modules/system/authlogin.te b/policy/modules/system/authlogin.te
> index 01c7331..6a96393 100644
> --- a/policy/modules/system/authlogin.te
> +++ b/policy/modules/system/authlogin.te
> @@ -5,7 +5,6 @@ policy_module(authlogin, 2.3.0)
>  # Declarations
>  #
>  
> -attribute auth_file_type;
>  attribute can_read_shadow_passwords;
>  attribute can_write_shadow_passwords;
>  attribute can_relabelto_shadow_passwords;
> @@ -51,7 +50,7 @@ type pam_var_run_t;
>  files_pid_file(pam_var_run_t)
>  
>  type shadow_t;
> -auth_file(shadow_t)
> +files_auth_file(shadow_t)
>  neverallow ~can_read_shadow_passwords shadow_t:file read;
>  neverallow ~can_write_shadow_passwords shadow_t:file { create write };
>  neverallow ~can_relabelto_shadow_passwords shadow_t:file relabelto;

There needs to be some work in the authlogin.if.  The interfaces that you're swapping in latter patches need to be deprecated (including auth_file()).  Additionally, all the currently existing authlogin deprecated interfaces point to the interfaces you're deprecated (eg auth_read_all_files_except_shadow), so they need to be updated too.

The interfaces in files don't need to be deprecated now, though I may do it in the future.

-- 
Chris PeBenito
Tresys Technology, LLC
www.tresys.com | oss.tresys.com