SELinux-Refpolicy Archive on lore.kernel.org
 help / color / Atom feed
From: Chris PeBenito <pebenito@ieee.org>
To: "Sugar, David" <dsugar@tresys.com>,
	"selinux-refpolicy@vger.kernel.org" 
	<selinux-refpolicy@vger.kernel.org>
Subject: Re: [PATCH v2] Setup attribute for fixed_disk_device and removable_device
Date: Thu, 14 Mar 2019 18:06:32 -0400
Message-ID: <06720853-7f0a-7ed4-b180-7d6b6b4d4a3f@ieee.org> (raw)
In-Reply-To: <20190313181804.10224-2-dsugar@tresys.com>

On 3/13/19 2:18 PM, Sugar, David wrote:
> 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

What you implemented doesn't seem consistent with what you have in the 
commit message.  sr0 is in your example denials, so these aren't all 
fixed disk devices, so the interface names and the attribute names 
should be related to all storage devices, it would seem.


> Updates based on feedback to create 'all' interfaces rather than
> updating existing interfaces.
> 
> I'm trying to resolve issues with denials like the following:
> type=AVC msg=audit(1551461957.121:9050): avc:  denied  { open } for  pid=30877 comm="cryptsetup" path="/dev/sdb" dev="devtmpfs" ino=110450 scontext=system_u:system_r:lvm_t:s0-s0:c0.c1023 tcontext=system_u:object_r:private_device_t:s0 tclass=blk_file permissive=1
> type=AVC msg=audit(1551461957.121:9051): avc:  denied  { ioctl } for  pid=30877 comm="cryptsetup" path="/dev/sdb" dev="devtmpfs" ino=110450 ioctlcmd=1268 scontext=system_u:system_r:lvm_t:s0-s0:c0.c1023 tcontext=system_u:object_r:private_device_t:s0 tclass=blk_file permissive=1
> type=AVC msg=audit(1551461973.149:9101): avc:  denied  { getattr } for  pid=28 comm="kdevtmpfs" path="/sdb1" dev="devtmpfs" ino=110461 scontext=system_u:system_r:kernel_t:s0 tcontext=system_u:object_r:private_device_t:s0 tclass=blk_file permissive=1
> type=AVC msg=audit(1551461973.149:9102): avc:  denied  { setattr } for  pid=28 comm="kdevtmpfs" name="sdb1" dev="devtmpfs" ino=110461 scontext=system_u:system_r:kernel_t:s0 tcontext=system_u:object_r:private_device_t:s0 tclass=blk_file permissive=1
> type=AVC msg=audit(1551461973.149:9103): avc:  denied  { unlink } for  pid=28 comm="kdevtmpfs" name="sdb1" dev="devtmpfs" ino=110461 scontext=system_u:system_r:kernel_t:s0 tcontext=system_u:object_r:private_device_t:s0 tclass=blk_file permissive=1
> 
> type=AVC msg=audit(1552228893.128:472): avc:  denied  { getattr } for  pid=8141 comm="systemd-logind" name="sg1" dev="devtmpfs" ino=31873 scontext=system_u:system_r:systemd_logind_t:s0 tcontext=system_u:object_r:private_device_t:s0 tclass=chr_file permissive=1
> type=AVC msg=audit(1552228893.128:473): avc:  denied  { setattr } for  pid=8141 comm="systemd-logind" name="sg1" dev="devtmpfs" ino=31873 scontext=system_u:system_r:systemd_logind_t:s0 tcontext=system_u:object_r:private_device_t:s0 tclass=chr_file permissive=1
> type=AVC msg=audit(1552228893.128:474): avc:  denied  { getattr } for  pid=8141 comm="systemd-logind" name="sr0" dev="devtmpfs" ino=14570 scontext=system_u:system_r:systemd_logind_t:s0 tcontext=system_u:object_r:private_device_t:s0 tclass=blk_file permissive=1
> type=AVC msg=audit(1552228893.128:475): avc:  denied  { setattr } for  pid=8141 comm="systemd-logind" name="sr0" dev="devtmpfs" ino=14570 scontext=system_u:system_r:systemd_logind_t:s0 tcontext=system_u:object_r:private_device_t:s0 tclass=blk_file permissive=1
> 
> Signed-off-by: Dave Sugar <dsugar@tresys.com>
> ---
>   policy/modules/kernel/kernel.te  |   7 +-
>   policy/modules/kernel/storage.if | 320 +++++++++++++++++++++++++++++++
>   policy/modules/kernel/storage.te |   7 +
>   policy/modules/system/lvm.te     |   2 +-
>   policy/modules/system/systemd.te |   8 +-
>   5 files changed, 336 insertions(+), 8 deletions(-)
> 
> diff --git a/policy/modules/kernel/kernel.te b/policy/modules/kernel/kernel.te
> index e971c533..acc8a88c 100644
> --- a/policy/modules/kernel/kernel.te
> +++ b/policy/modules/kernel/kernel.te
> @@ -352,9 +352,10 @@ ifdef(`init_systemd',`
>   
>   	optional_policy(`
>   		storage_dev_filetrans_fixed_disk(kernel_t)
> -		storage_setattr_fixed_disk_dev(kernel_t)
> -		storage_create_fixed_disk_dev(kernel_t)
> -		storage_delete_fixed_disk_dev(kernel_t)
> +		storage_getattr_all_scsi_generic_dev(kernel_t)
> +		storage_setattr_all_fixed_disk_dev(kernel_t)
> +		storage_create_all_fixed_disk_dev(kernel_t)
> +		storage_delete_all_fixed_disk_dev(kernel_t)
>   	')
>   ')
>   
> diff --git a/policy/modules/kernel/storage.if b/policy/modules/kernel/storage.if
> index 0292eee4..93208835 100644
> --- a/policy/modules/kernel/storage.if
> +++ b/policy/modules/kernel/storage.if
> @@ -1,5 +1,25 @@
>   ## <summary>Policy controlling access to storage devices</summary>
>   
> +########################################
> +## <summary>
> +##	Allow the caller to get the attributes of
> +##	all fixed disk device nodes.
> +## </summary>
> +## <param name="domain">
> +##	<summary>
> +##	Domain allowed access.
> +##	</summary>
> +## </param>
> +#
> +interface(`storage_getattr_all_fixed_disk_dev',`
> +	gen_require(`
> +		attribute fixed_disk_device;
> +	')
> +
> +	dev_list_all_dev_nodes($1)
> +	allow $1 fixed_disk_device:blk_file getattr;
> +')
> +
>   ########################################
>   ## <summary>
>   ##	Allow the caller to get the attributes of fixed disk
> @@ -20,6 +40,26 @@ interface(`storage_getattr_fixed_disk_dev',`
>   	allow $1 fixed_disk_device_t:blk_file getattr;
>   ')
>   
> +########################################
> +## <summary>
> +##	Do not audit attempts made by the caller to get
> +##	the attributes of all fixed disk device nodes.
> +## </summary>
> +## <param name="domain">
> +##	<summary>
> +##	Domain to not audit.
> +##	</summary>
> +## </param>
> +#
> +interface(`storage_dontaudit_all_getattr_fixed_disk_dev',`
> +	gen_require(`
> +		attribute fixed_disk_device;
> +	')
> +
> +	dontaudit $1 fixed_disk_device:blk_file getattr;
> +	dontaudit $1 fixed_disk_device:chr_file getattr; # /dev/rawctl
> +')
> +
>   ########################################
>   ## <summary>
>   ##	Do not audit attempts made by the caller to get
> @@ -40,6 +80,26 @@ interface(`storage_dontaudit_getattr_fixed_disk_dev',`
>   	dontaudit $1 fixed_disk_device_t:chr_file getattr; # /dev/rawctl
>   ')
>   
> +########################################
> +## <summary>
> +##	Allow the caller to set the attributes of all
> +##	fixed disk device nodes.
> +## </summary>
> +## <param name="domain">
> +##	<summary>
> +##	Domain allowed access.
> +##	</summary>
> +## </param>
> +#
> +interface(`storage_setattr_all_fixed_disk_dev',`
> +	gen_require(`
> +		attribute fixed_disk_device;
> +	')
> +
> +	dev_list_all_dev_nodes($1)
> +	allow $1 fixed_disk_device:blk_file setattr;
> +')
> +
>   ########################################
>   ## <summary>
>   ##	Allow the caller to set the attributes of fixed disk
> @@ -79,6 +139,31 @@ interface(`storage_dontaudit_setattr_fixed_disk_dev',`
>   	dontaudit $1 fixed_disk_device_t:blk_file setattr;
>   ')
>   
> +########################################
> +## <summary>
> +##	Allow the caller to directly read from all fixed disks.
> +##	This is extremely dangerous as it can bypass the
> +##	SELinux protections for filesystem objects, and
> +##	should only be used by trusted domains.
> +## </summary>
> +## <param name="domain">
> +##	<summary>
> +##	Domain allowed access.
> +##	</summary>
> +## </param>
> +#
> +interface(`storage_raw_read_all_fixed_disk',`
> +	gen_require(`
> +		attribute fixed_disk_raw_read;
> +		attribute fixed_disk_device;
> +	')
> +
> +	dev_list_all_dev_nodes($1)
> +	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;
> +')
> +
>   ########################################
>   ## <summary>
>   ##	Allow the caller to directly read from a fixed disk.
> @@ -188,6 +273,26 @@ interface(`storage_raw_rw_fixed_disk',`
>   	storage_raw_write_fixed_disk($1)
>   ')
>   
> +########################################
> +## <summary>
> +##	Allow the caller to create all fixed disk device nodes.
> +## </summary>
> +## <param name="domain">
> +##	<summary>
> +##	Domain allowed access.
> +##	</summary>
> +## </param>
> +#
> +interface(`storage_create_all_fixed_disk_dev',`
> +	gen_require(`
> +		attribute fixed_disk_device;
> +	')
> +
> +	allow $1 self:capability mknod;
> +	allow $1 fixed_disk_device:blk_file create_blk_file_perms;
> +	dev_add_entry_generic_dirs($1)
> +')
> +
>   ########################################
>   ## <summary>
>   ##	Allow the caller to create fixed disk device nodes.
> @@ -208,6 +313,25 @@ interface(`storage_create_fixed_disk_dev',`
>   	dev_add_entry_generic_dirs($1)
>   ')
>   
> +########################################
> +## <summary>
> +##	Allow the caller to delete all fixed disk device nodes.
> +## </summary>
> +## <param name="domain">
> +##	<summary>
> +##	Domain allowed access.
> +##	</summary>
> +## </param>
> +#
> +interface(`storage_delete_all_fixed_disk_dev',`
> +	gen_require(`
> +		attribute fixed_disk_device;
> +	')
> +
> +	allow $1 fixed_disk_device:blk_file delete_blk_file_perms;
> +	dev_remove_entry_generic_dirs($1)
> +')
> +
>   ########################################
>   ## <summary>
>   ##	Allow the caller to delete fixed disk device nodes.
> @@ -227,6 +351,29 @@ interface(`storage_delete_fixed_disk_dev',`
>   	dev_remove_entry_generic_dirs($1)
>   ')
>   
> +########################################
> +## <summary>
> +##	Create, read, write, and delete fixed disk device nodes.
> +## </summary>
> +## <param name="domain">
> +##	<summary>
> +##	Domain allowed access.
> +##	</summary>
> +## </param>
> +#
> +interface(`storage_manage_all_fixed_disk',`
> +	gen_require(`
> +		attribute fixed_disk_raw_read, fixed_disk_raw_write;
> +		attribute fixed_disk_device;
> +	')
> +
> +	dev_list_all_dev_nodes($1)
> +	allow $1 self:capability mknod;
> +	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;
> +')
> +
>   ########################################
>   ## <summary>
>   ##	Create, read, write, and delete fixed disk device nodes.
> @@ -293,6 +440,25 @@ interface(`storage_tmpfs_filetrans_fixed_disk',`
>   	fs_tmpfs_filetrans($1, fixed_disk_device_t, blk_file)
>   ')
>   
> +########################################
> +## <summary>
> +##	Relabel all fixed disk device nodes.
> +## </summary>
> +## <param name="domain">
> +##	<summary>
> +##	Domain allowed access.
> +##	</summary>
> +## </param>
> +#
> +interface(`storage_relabel_all_fixed_disk',`
> +	gen_require(`
> +		attribute fixed_disk_device;
> +	')
> +
> +	dev_list_all_dev_nodes($1)
> +	allow $1 fixed_disk_device:blk_file relabel_blk_file_perms;
> +')
> +
>   ########################################
>   ## <summary>
>   ##	Relabel fixed disk device nodes.
> @@ -388,6 +554,26 @@ interface(`storage_dontaudit_rw_fuse',`
>   	dontaudit $1 fuse_device_t:chr_file rw_file_perms;
>   ')
>   
> +########################################
> +## <summary>
> +##	Allow the caller to get the attributes of
> +##	all generic SCSI interface device nodes.
> +## </summary>
> +## <param name="domain">
> +##	<summary>
> +##	Domain allowed access.
> +##	</summary>
> +## </param>
> +#
> +interface(`storage_getattr_all_scsi_generic_dev',`
> +	gen_require(`
> +		attribute scsi_generic_device;
> +	')
> +
> +	dev_list_all_dev_nodes($1)
> +	allow $1 scsi_generic_device:chr_file getattr;
> +')
> +
>   ########################################
>   ## <summary>
>   ##	Allow the caller to get the attributes of
> @@ -408,6 +594,26 @@ interface(`storage_getattr_scsi_generic_dev',`
>   	allow $1 scsi_generic_device_t:chr_file getattr;
>   ')
>   
> +########################################
> +## <summary>
> +##	Allow the caller to set the attributes of
> +##	all generic SCSI interface device nodes.
> +## </summary>
> +## <param name="domain">
> +##	<summary>
> +##	Domain allowed access.
> +##	</summary>
> +## </param>
> +#
> +interface(`storage_setattr_all_scsi_generic_dev',`
> +	gen_require(`
> +		attribute scsi_generic_device;
> +	')
> +
> +	dev_list_all_dev_nodes($1)
> +	allow $1 scsi_generic_device:chr_file setattr;
> +')
> +
>   ########################################
>   ## <summary>
>   ##	Allow the caller to set the attributes of
> @@ -517,6 +723,26 @@ interface(`storage_dontaudit_rw_scsi_generic',`
>   	dontaudit $1 scsi_generic_device_t:chr_file rw_file_perms;
>   ')
>   
> +########################################
> +## <summary>
> +##	Allow the caller to get the attributes of all removable
> +##	devices device nodes.
> +## </summary>
> +## <param name="domain">
> +##	<summary>
> +##	Domain allowed access.
> +##	</summary>
> +## </param>
> +#
> +interface(`storage_getattr_all_removable_dev',`
> +	gen_require(`
> +		attribute removable_device;
> +	')
> +
> +	dev_list_all_dev_nodes($1)
> +	allow $1 removable_device:blk_file getattr;
> +')
> +
>   ########################################
>   ## <summary>
>   ##	Allow the caller to get the attributes of removable
> @@ -556,6 +782,25 @@ interface(`storage_dontaudit_getattr_removable_dev',`
>   	dontaudit $1 removable_device_t:blk_file getattr;
>   ')
>   
> +########################################
> +## <summary>
> +##	Do not audit attempts made by the caller to get
> +##	the attributes of all removable devices device nodes.
> +## </summary>
> +## <param name="domain">
> +##	<summary>
> +##	Domain to not audit.
> +##	</summary>
> +## </param>
> +#
> +interface(`storage_dontaudit_getattr_all_removable_dev',`
> +	gen_require(`
> +		attribute removable_device;
> +	')
> +
> +	dontaudit $1 removable_device:blk_file getattr;
> +')
> +
>   ########################################
>   ## <summary>
>   ##	Do not audit attempts made by the caller to read
> @@ -595,6 +840,26 @@ interface(`storage_dontaudit_write_removable_device',`
>   	dontaudit $1 removable_device_t:blk_file write_blk_file_perms;
>   ')
>   
> +########################################
> +## <summary>
> +##	Allow the caller to set the attributes of all removable
> +##	devices device nodes.
> +## </summary>
> +## <param name="domain">
> +##	<summary>
> +##	Domain allowed access.
> +##	</summary>
> +## </param>
> +#
> +interface(`storage_setattr_all_removable_dev',`
> +	gen_require(`
> +		attribute removable_device;
> +	')
> +
> +	dev_list_all_dev_nodes($1)
> +	allow $1 removable_device:blk_file setattr;
> +')
> +
>   ########################################
>   ## <summary>
>   ##	Allow the caller to set the attributes of removable
> @@ -796,6 +1061,61 @@ interface(`storage_setattr_tape_dev',`
>   	allow $1 tape_device_t:chr_file setattr;
>   ')
>   
> +########################################
> +## <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;
> +')
> +
> +########################################
> +## <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 scsi generic device type.
> +## </summary>
> +## <param name="domain">
> +##	<summary>
> +##	Type to associate.
> +##	</summary>
> +## </param>
> +#
> +interface(`storage_scsi_generic_device_type',`
> +	gen_require(`
> +		attribute scsi_generic_device;
> +	')
> +
> +	typeattribute $1 scsi_generic_device;
> +')
> +
>   ########################################
>   ## <summary>
>   ##	Unconfined access to storage devices.
> diff --git a/policy/modules/kernel/storage.te b/policy/modules/kernel/storage.te
> index c10290c0..cc59380d 100644
> --- a/policy/modules/kernel/storage.te
> +++ b/policy/modules/kernel/storage.te
> @@ -11,12 +11,17 @@ attribute scsi_generic_read;
>   attribute scsi_generic_write;
>   attribute storage_unconfined_type;
>   
> +attribute fixed_disk_device;
> +attribute removable_device;
> +attribute scsi_generic_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 };
> @@ -33,6 +38,7 @@ dev_node(fuse_device_t)
>   #
>   type scsi_generic_device_t;
>   dev_node(scsi_generic_device_t)
> +storage_scsi_generic_device_type(scsi_generic_device_t)
>   
>   neverallow ~{ scsi_generic_read storage_unconfined_type } scsi_generic_device_t:{ chr_file blk_file } read;
>   neverallow ~{ scsi_generic_write storage_unconfined_type } scsi_generic_device_t:{ chr_file blk_file } { append write };
> @@ -43,6 +49,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
> diff --git a/policy/modules/system/lvm.te b/policy/modules/system/lvm.te
> index ba64c39d..0bb7177c 100644
> --- a/policy/modules/system/lvm.te
> +++ b/policy/modules/system/lvm.te
> @@ -300,7 +300,7 @@ storage_dontaudit_read_removable_device(lvm_t)
>   # cjp: needs to create an interface here for fixed disk create
>   storage_dev_filetrans_fixed_disk(lvm_t)
>   # Access raw devices and old /dev/lvm (c 109,0).  Is this needed?
> -storage_manage_fixed_disk(lvm_t)
> +storage_manage_all_fixed_disk(lvm_t)
>   
>   term_use_all_terms(lvm_t)
>   
> diff --git a/policy/modules/system/systemd.te b/policy/modules/system/systemd.te
> index 07529a5d..83f85a9d 100644
> --- a/policy/modules/system/systemd.te
> +++ b/policy/modules/system/systemd.te
> @@ -472,10 +472,10 @@ fs_unmount_tmpfs(systemd_logind_t)
>   
>   selinux_get_enforce_mode(systemd_logind_t)
>   
> -storage_getattr_removable_dev(systemd_logind_t)
> -storage_getattr_scsi_generic_dev(systemd_logind_t)
> -storage_setattr_removable_dev(systemd_logind_t)
> -storage_setattr_scsi_generic_dev(systemd_logind_t)
> +storage_getattr_all_removable_dev(systemd_logind_t)
> +storage_getattr_all_scsi_generic_dev(systemd_logind_t)
> +storage_setattr_all_removable_dev(systemd_logind_t)
> +storage_setattr_all_scsi_generic_dev(systemd_logind_t)
>   
>   term_setattr_unallocated_ttys(systemd_logind_t)
>   term_use_unallocated_ttys(systemd_logind_t)
> 


-- 
Chris PeBenito

  reply index

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-13 18:18 [PATCH v3] Separate out udevadm into a new domain Sugar, David
2019-03-13 18:18 ` [PATCH v2] Setup attribute for fixed_disk_device and removable_device Sugar, David
2019-03-14 22:06   ` Chris PeBenito [this message]
2019-03-15  2:22     ` Sugar, David
2019-03-17 20:14       ` Chris PeBenito
2019-03-14 22:05 ` [PATCH v3] Separate out udevadm into a new domain Chris PeBenito
2019-03-15  2:27   ` [PATCH v4] " Sugar, David
2019-03-17 20:15     ` Chris PeBenito

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=06720853-7f0a-7ed4-b180-7d6b6b4d4a3f@ieee.org \
    --to=pebenito@ieee.org \
    --cc=dsugar@tresys.com \
    --cc=selinux-refpolicy@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

SELinux-Refpolicy Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/selinux-refpolicy/0 selinux-refpolicy/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 selinux-refpolicy selinux-refpolicy/ https://lore.kernel.org/selinux-refpolicy \
		selinux-refpolicy@vger.kernel.org
	public-inbox-index selinux-refpolicy

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.selinux-refpolicy


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git