From: jwcart2@tycho.nsa.gov (James Carter)
Date: Mon, 23 Apr 2012 16:16:45 -0400
Subject: [refpolicy] [PATCH 1/3] Create non_auth_file_type attribute and
 interfaces
In-Reply-To: <4F9554CB.2040104@tresys.com>
References: <1333656877.4703.40.camel@moss-lions.epoch.ncsc.mil>
	<4F9554CB.2040104@tresys.com>
Message-ID: <1335212205.1708.27.camel@moss-lions.epoch.ncsc.mil>
To: refpolicy@oss.tresys.com
List-Id: refpolicy.oss.tresys.com

On Mon, 2012-04-23 at 09:10 -0400, Christopher J. PeBenito wrote:
> 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.
> 

Drat, I hate when I miss these.

> > +##	</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).
> 

Not a problem. There didn't seem to be any logic to how things were
ordered, so I expected that you would give direction.

> >  ########################################
> >  ## <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.
> 

That name makes more sense.

> > +	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.
> 

I wasn't sure what you would want done with authlogin.if, so I decided
that it was easier to do nothing. ;)

I will update the interfaces in authlogin.if to mark them as deprecated
and point them to the new interfaces.

Thanks,
-- 
James Carter <jwcart2@tycho.nsa.gov>
National Security Agency