From:   Dominick Grift <dac.override@gmail.com>
To:     "Sugar\, David" <dsugar@tresys.com>
Cc:     "selinux-refpolicy\@vger.kernel.org" 
        <selinux-refpolicy@vger.kernel.org>
Subject: Re: [PATCH] Setup attribute for fixed_disk_device and removable_device
References: <20190305223402.19263-1-dsugar@tresys.com>
Date:   Wed, 06 Mar 2019 09:18:12 +0100
In-Reply-To: <20190305223402.19263-1-dsugar@tresys.com> (David Sugar's message
        of "Tue, 5 Mar 2019 22:34:37 +0000")
Message-ID: <871s3kza9n.fsf@gmail.com>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Sender: selinux-refpolicy-owner@vger.kernel.org
Precedence: bulk

"Sugar, David" <dsugar@tresys.com> writes:

> I am having trouble with some denials due to the fact I am setting
> up specific private types for media attached to my system.  This
> changes to use an attribute for media and interfaces to add types
> to the newly created attribute. I'm seeing denials from lvm_t which
> this helps to resolve.
>
> If the names of the new attributes are not OK, please suggest others.
> I will update as needed.

You probably should create new interfaces for operations that apply to
all fixed disk types instead of modifying the ones that apply to generic
fixed disk

Example: storage_getattr_all_fixed_disk_dev()

>
> Signed-off-by: Dave Sugar <dsugar@tresys.com>
> ---
>  policy/modules/kernel/storage.if | 139 +++++++++++++++++++------------
>  policy/modules/kernel/storage.te |  11 ++-
>  2 files changed, 96 insertions(+), 54 deletions(-)
>
> diff --git a/policy/modules/kernel/storage.if b/policy/modules/kernel/storage.if
> index 0292eee4..c8d9bfcc 100644
> --- a/policy/modules/kernel/storage.if
> +++ b/policy/modules/kernel/storage.if
> @@ -13,11 +13,11 @@
>  #
>  interface(`storage_getattr_fixed_disk_dev',`
>  	gen_require(`
> -		type fixed_disk_device_t;
> +		attribute fixed_disk_device;
>  	')
>  
>  	dev_list_all_dev_nodes($1)
> -	allow $1 fixed_disk_device_t:blk_file getattr;
> +	allow $1 fixed_disk_device:blk_file getattr;
>  ')
>  
>  ########################################
> @@ -33,11 +33,11 @@ interface(`storage_getattr_fixed_disk_dev',`
>  #
>  interface(`storage_dontaudit_getattr_fixed_disk_dev',`
>  	gen_require(`
> -		type fixed_disk_device_t;
> +		attribute fixed_disk_device;
>  	')
>  
> -	dontaudit $1 fixed_disk_device_t:blk_file getattr;
> -	dontaudit $1 fixed_disk_device_t:chr_file getattr; # /dev/rawctl
> +	dontaudit $1 fixed_disk_device:blk_file getattr;
> +	dontaudit $1 fixed_disk_device:chr_file getattr; # /dev/rawctl
>  ')
>  
>  ########################################
> @@ -53,11 +53,11 @@ interface(`storage_dontaudit_getattr_fixed_disk_dev',`
>  #
>  interface(`storage_setattr_fixed_disk_dev',`
>  	gen_require(`
> -		type fixed_disk_device_t;
> +		attribute fixed_disk_device;
>  	')
>  
>  	dev_list_all_dev_nodes($1)
> -	allow $1 fixed_disk_device_t:blk_file setattr;
> +	allow $1 fixed_disk_device:blk_file setattr;
>  ')
>  
>  ########################################
> @@ -73,10 +73,10 @@ interface(`storage_setattr_fixed_disk_dev',`
>  #
>  interface(`storage_dontaudit_setattr_fixed_disk_dev',`
>  	gen_require(`
> -		type fixed_disk_device_t;
> +		attribute fixed_disk_device;
>  	')
>  
> -	dontaudit $1 fixed_disk_device_t:blk_file setattr;
> +	dontaudit $1 fixed_disk_device:blk_file setattr;
>  ')
>  
>  ########################################
> @@ -95,12 +95,12 @@ interface(`storage_dontaudit_setattr_fixed_disk_dev',`
>  interface(`storage_raw_read_fixed_disk',`
>  	gen_require(`
>  		attribute fixed_disk_raw_read;
> -		type fixed_disk_device_t;
> +		attribute fixed_disk_device;
>  	')
>  
>  	dev_list_all_dev_nodes($1)
> -	allow $1 fixed_disk_device_t:blk_file read_blk_file_perms;
> -	allow $1 fixed_disk_device_t:chr_file read_chr_file_perms;
> +	allow $1 fixed_disk_device:blk_file read_blk_file_perms;
> +	allow $1 fixed_disk_device:chr_file read_chr_file_perms;
>  	typeattribute $1 fixed_disk_raw_read;
>  ')
>  
> @@ -117,12 +117,12 @@ interface(`storage_raw_read_fixed_disk',`
>  #
>  interface(`storage_dontaudit_read_fixed_disk',`
>  	gen_require(`
> -		type fixed_disk_device_t;
> +		attribute fixed_disk_device;
>  
>  	')
>  
> -	dontaudit $1 fixed_disk_device_t:blk_file read_blk_file_perms;
> -	dontaudit $1 fixed_disk_device_t:chr_file read_chr_file_perms;
> +	dontaudit $1 fixed_disk_device:blk_file read_blk_file_perms;
> +	dontaudit $1 fixed_disk_device:chr_file read_chr_file_perms;
>  ')
>  
>  ########################################
> @@ -141,12 +141,12 @@ interface(`storage_dontaudit_read_fixed_disk',`
>  interface(`storage_raw_write_fixed_disk',`
>  	gen_require(`
>  		attribute fixed_disk_raw_write;
> -		type fixed_disk_device_t;
> +		attribute fixed_disk_device;
>  	')
>  
>  	dev_list_all_dev_nodes($1)
> -	allow $1 fixed_disk_device_t:blk_file write_blk_file_perms;
> -	allow $1 fixed_disk_device_t:chr_file write_chr_file_perms;
> +	allow $1 fixed_disk_device:blk_file write_blk_file_perms;
> +	allow $1 fixed_disk_device:chr_file write_chr_file_perms;
>  	typeattribute $1 fixed_disk_raw_write;
>  ')
>  
> @@ -163,11 +163,11 @@ interface(`storage_raw_write_fixed_disk',`
>  #
>  interface(`storage_dontaudit_write_fixed_disk',`
>  	gen_require(`
> -		type fixed_disk_device_t;
> +		attribute fixed_disk_device;
>  
>  	')
>  
> -	dontaudit $1 fixed_disk_device_t:blk_file write_blk_file_perms;
> +	dontaudit $1 fixed_disk_device:blk_file write_blk_file_perms;
>  ')
>  
>  ########################################
> @@ -200,11 +200,11 @@ interface(`storage_raw_rw_fixed_disk',`
>  #
>  interface(`storage_create_fixed_disk_dev',`
>  	gen_require(`
> -		type fixed_disk_device_t;
> +		attribute fixed_disk_device;
>  	')
>  
>  	allow $1 self:capability mknod;
> -	allow $1 fixed_disk_device_t:blk_file create_blk_file_perms;
> +	allow $1 fixed_disk_device:blk_file create_blk_file_perms;
>  	dev_add_entry_generic_dirs($1)
>  ')
>  
> @@ -220,10 +220,10 @@ interface(`storage_create_fixed_disk_dev',`
>  #
>  interface(`storage_delete_fixed_disk_dev',`
>  	gen_require(`
> -		type fixed_disk_device_t;
> +		attribute fixed_disk_device;
>  	')
>  
> -	allow $1 fixed_disk_device_t:blk_file delete_blk_file_perms;
> +	allow $1 fixed_disk_device:blk_file delete_blk_file_perms;
>  	dev_remove_entry_generic_dirs($1)
>  ')
>  
> @@ -240,13 +240,13 @@ interface(`storage_delete_fixed_disk_dev',`
>  interface(`storage_manage_fixed_disk',`
>  	gen_require(`
>  		attribute fixed_disk_raw_read, fixed_disk_raw_write;
> -		type fixed_disk_device_t;
> +		attribute fixed_disk_device;
>  	')
>  
>  	dev_list_all_dev_nodes($1)
>  	allow $1 self:capability mknod;
> -	allow $1 fixed_disk_device_t:blk_file manage_blk_file_perms;
> -	allow $1 fixed_disk_device_t:chr_file manage_chr_file_perms;
> +	allow $1 fixed_disk_device:blk_file manage_blk_file_perms;
> +	allow $1 fixed_disk_device:chr_file manage_chr_file_perms;
>  	typeattribute $1 fixed_disk_raw_read, fixed_disk_raw_write;
>  ')
>  
> @@ -305,11 +305,11 @@ interface(`storage_tmpfs_filetrans_fixed_disk',`
>  #
>  interface(`storage_relabel_fixed_disk',`
>  	gen_require(`
> -		type fixed_disk_device_t;
> +		attribute fixed_disk_device;
>  	')
>  
>  	dev_list_all_dev_nodes($1)
> -	allow $1 fixed_disk_device_t:blk_file relabel_blk_file_perms;
> +	allow $1 fixed_disk_device:blk_file relabel_blk_file_perms;
>  ')
>  
>  ########################################
> @@ -324,11 +324,11 @@ interface(`storage_relabel_fixed_disk',`
>  #
>  interface(`storage_swapon_fixed_disk',`
>  	gen_require(`
> -		type fixed_disk_device_t;
> +		attribute fixed_disk_device;
>  	')
>  
>  	dev_list_all_dev_nodes($1)
> -	allow $1 fixed_disk_device_t:blk_file { getattr swapon };
> +	allow $1 fixed_disk_device:blk_file { getattr swapon };
>  ')
>  
>  ########################################
> @@ -530,11 +530,11 @@ interface(`storage_dontaudit_rw_scsi_generic',`
>  #
>  interface(`storage_getattr_removable_dev',`
>  	gen_require(`
> -		type removable_device_t;
> +		attribute removable_device;
>  	')
>  
>  	dev_list_all_dev_nodes($1)
> -	allow $1 removable_device_t:blk_file getattr;
> +	allow $1 removable_device:blk_file getattr;
>  ')
>  
>  ########################################
> @@ -550,10 +550,10 @@ interface(`storage_getattr_removable_dev',`
>  #
>  interface(`storage_dontaudit_getattr_removable_dev',`
>  	gen_require(`
> -		type removable_device_t;
> +		attribute removable_device;
>  	')
>  
> -	dontaudit $1 removable_device_t:blk_file getattr;
> +	dontaudit $1 removable_device:blk_file getattr;
>  ')
>  
>  ########################################
> @@ -569,11 +569,11 @@ interface(`storage_dontaudit_getattr_removable_dev',`
>  #
>  interface(`storage_dontaudit_read_removable_device',`
>  	gen_require(`
> -		type removable_device_t;
> +		attribute removable_device;
>  
>  	')
>  
> -	dontaudit $1 removable_device_t:blk_file read_blk_file_perms;
> +	dontaudit $1 removable_device:blk_file read_blk_file_perms;
>  ')
>  
>  ########################################
> @@ -589,10 +589,10 @@ interface(`storage_dontaudit_read_removable_device',`
>  #
>  interface(`storage_dontaudit_write_removable_device',`
>  	gen_require(`
> -		type removable_device_t;
> +		attribute removable_device;
>  	')
>  
> -	dontaudit $1 removable_device_t:blk_file write_blk_file_perms;
> +	dontaudit $1 removable_device:blk_file write_blk_file_perms;
>  ')
>  
>  ########################################
> @@ -608,11 +608,11 @@ interface(`storage_dontaudit_write_removable_device',`
>  #
>  interface(`storage_setattr_removable_dev',`
>  	gen_require(`
> -		type removable_device_t;
> +		attribute removable_device;
>  	')
>  
>  	dev_list_all_dev_nodes($1)
> -	allow $1 removable_device_t:blk_file setattr;
> +	allow $1 removable_device:blk_file setattr;
>  ')
>  
>  ########################################
> @@ -628,10 +628,10 @@ interface(`storage_setattr_removable_dev',`
>  #
>  interface(`storage_dontaudit_setattr_removable_dev',`
>  	gen_require(`
> -		type removable_device_t;
> +		attribute removable_device;
>  	')
>  
> -	dontaudit $1 removable_device_t:blk_file setattr;
> +	dontaudit $1 removable_device:blk_file setattr;
>  ')
>  
>  ########################################
> @@ -650,11 +650,11 @@ interface(`storage_dontaudit_setattr_removable_dev',`
>  #
>  interface(`storage_raw_read_removable_device',`
>  	gen_require(`
> -		type removable_device_t;
> +		attribute removable_device;
>  	')
>  
>  	dev_list_all_dev_nodes($1)
> -	allow $1 removable_device_t:blk_file read_blk_file_perms;
> +	allow $1 removable_device:blk_file read_blk_file_perms;
>  ')
>  
>  ########################################
> @@ -669,10 +669,10 @@ interface(`storage_raw_read_removable_device',`
>  #
>  interface(`storage_dontaudit_raw_read_removable_device',`
>  	gen_require(`
> -		type removable_device_t;
> +		attribute removable_device;
>  	')
>  
> -	dontaudit $1 removable_device_t:blk_file read_blk_file_perms;
> +	dontaudit $1 removable_device:blk_file read_blk_file_perms;
>  ')
>  
>  ########################################
> @@ -691,11 +691,11 @@ interface(`storage_dontaudit_raw_read_removable_device',`
>  #
>  interface(`storage_raw_write_removable_device',`
>  	gen_require(`
> -		type removable_device_t;
> +		attribute removable_device;
>  	')
>  
>  	dev_list_all_dev_nodes($1)
> -	allow $1 removable_device_t:blk_file write_blk_file_perms;
> +	allow $1 removable_device:blk_file write_blk_file_perms;
>  ')
>  
>  ########################################
> @@ -710,10 +710,10 @@ interface(`storage_raw_write_removable_device',`
>  #
>  interface(`storage_dontaudit_raw_write_removable_device',`
>  	gen_require(`
> -		type removable_device_t;
> +		attribute removable_device;
>  	')
>  
> -	dontaudit $1 removable_device_t:blk_file write_blk_file_perms;
> +	dontaudit $1 removable_device:blk_file write_blk_file_perms;
>  ')
>  
>  ########################################
> @@ -813,3 +813,40 @@ interface(`storage_unconfined',`
>  
>  	typeattribute $1 storage_unconfined_type;
>  ')
> +
> +########################################
> +## <summary>
> +##	Mark a type as a removable device type.
> +## </summary>
> +## <param name="domain">
> +##	<summary>
> +##	Type to associate.
> +##	</summary>
> +## </param>
> +#
> +interface(`storage_removable_device_type',`
> +	gen_require(`
> +		attribute removable_device;
> +	')
> +
> +	typeattribute $1 removable_device;
> +')
> +
> +########################################
> +## <summary>
> +##	Mark a type as a fixed disk device type.
> +## </summary>
> +## <param name="domain">
> +##	<summary>
> +##	Type to associate
> +##	</summary>
> +## </param>
> +#
> +interface(`storage_fixed_disk_device_type',`
> +	gen_require(`
> +		attribute fixed_disk_device;
> +	')
> +
> +	typeattribute $1 fixed_disk_device;
> +')
> +
> diff --git a/policy/modules/kernel/storage.te b/policy/modules/kernel/storage.te
> index c10290c0..7a358290 100644
> --- a/policy/modules/kernel/storage.te
> +++ b/policy/modules/kernel/storage.te
> @@ -11,15 +11,19 @@ attribute scsi_generic_read;
>  attribute scsi_generic_write;
>  attribute storage_unconfined_type;
>  
> +attribute removable_device;
> +attribute fixed_disk_device;
> +
>  #
>  # fixed_disk_device_t is the type of
>  # /dev/hd* and /dev/sd*.
>  #
>  type fixed_disk_device_t;
>  dev_node(fixed_disk_device_t)
> +storage_fixed_disk_device_type(fixed_disk_device_t)
>  
> -neverallow ~{ fixed_disk_raw_read storage_unconfined_type } fixed_disk_device_t:{ chr_file blk_file } read;
> -neverallow ~{ fixed_disk_raw_write storage_unconfined_type } fixed_disk_device_t:{ chr_file blk_file } { append write };
> +neverallow ~{ fixed_disk_raw_read storage_unconfined_type } fixed_disk_device:{ chr_file blk_file } read;
> +neverallow ~{ fixed_disk_raw_write storage_unconfined_type } fixed_disk_device:{ chr_file blk_file } { append write };
>  
>  #
>  # fuse_device_t is the type of /dev/fuse
> @@ -43,6 +47,7 @@ neverallow ~{ scsi_generic_write storage_unconfined_type } scsi_generic_device_t
>  #
>  type removable_device_t;
>  dev_node(removable_device_t)
> +storage_removable_device_type(removable_device_t)
>  
>  #
>  # tape_device_t is the type of
> @@ -55,5 +60,5 @@ dev_node(tape_device_t)
>  # Unconfined access to this module
>  #
>  
> -allow storage_unconfined_type { fixed_disk_device_t removable_device_t }:blk_file { manage_blk_file_perms relabelfrom relabelto map execute swapon quotaon mounton audit_access execmod };
> +allow storage_unconfined_type { fixed_disk_device removable_device }:blk_file { manage_blk_file_perms relabelfrom relabelto map execute swapon quotaon mounton audit_access execmod };
>  allow storage_unconfined_type { scsi_generic_device_t tape_device_t }:chr_file { manage_chr_file_perms relabelfrom relabelto map execute swapon quotaon mounton execute_no_trans entrypoint execmod audit_access };

-- 
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift