From: domg472@gmail.com (Dominick Grift)
Date: Thu, 11 Nov 2010 13:18:05 +0100
Subject: [refpolicy] FW:  Add support for the samhain program
In-Reply-To: <SNT139-w4060518B7D42FDC9CB94CEAB320@phx.gbl>
References: <SNT139-w647DF2426616B71E050D74AB300@phx.gbl>
	<SNT139-w4060518B7D42FDC9CB94CEAB320@phx.gbl>
Message-ID: <20101111121804.GA17316@localhost.localdomain>
To: refpolicy@oss.tresys.com
List-Id: refpolicy.oss.tresys.com

On Thu, Nov 11, 2010 at 11:01:37AM +0000, HarryCiao wrote:
> 
> Hi Chris and Dom,

some comments inline
> 
> Sorry to bother you again.
> 
> I'd tried to develop a .pp for the samhain program(filesystem integrity checker). I am wondering if you could help give some comments on it so that I could do it in the right way :-)
> 
> Many thanks!
> 
> Best regards,
> Harry
> 
> From: harrytaurus2002 at hotmail.com
> To: refpolicy at oss.tresys.com
> Date: Tue, 9 Nov 2010 03:33:24 +0000
> Subject: [refpolicy] Add support for the samhain program
> 
> 
> 
> 
> 
> 
> 
> 
> Hi SELinux experts:
> 
> I have tried to add support for the samhain program, which is used to check filesystem integrity. Please help comment on the attached implementation of the samhain.pp, many thanks! 
> 
> I have tested it on the samhain-2.5.5 package with the default configuration by the follow commands, and samhain could detect changes as to be monitored in its configuration file (/etc/samhainrc):
> 
> (In sysadm_r role, install samhain.pp and update sysadm.pp)
> 1. Initialize database:
> newrole -l s15:c0.c1023 -- -c "samhain -t init"
> 
> 2. Check samhain daemon status:
> run_init /etc/init.d/samhain status
> 
> 3. Start samhain in daemon mode:
> run_init /etc/init.d/samhain start
> or,
> newrole -l s15:c0.c1023 -- -c "samhain -t check -D"
> 
> 4. Stop samhain daemon:
> run_init /etc/init.d/samhain stop
> 
> 
> Tow more questions:
> 1. sysadm or secadm, who is a better choice to call samhain_admin() for? sysadm could manage /var/log/, /var/lib
>  / already but doesn't belong to the mlsfilewrite attribute, well secadm has the opposite abilities.
> 
> Or some other better solution?
> 
> 2. Would the samhain_run_init_script() make sense if the samhain_admin() is called for secadm?
> 
> Thanks a lot!
> 
> Best regards,
> Harry 
>  		 	   		  
> 
> _______________________________________________
> refpolicy mailing list
> refpolicy at oss.tresys.com
> http://oss.tresys.com/mailman/listinfo/refpolicy 		 	   		  

> From 78c86950bcbc5ff4eb78c03cf8e0a59f11c542b7 Mon Sep 17 00:00:00 2001
> From: Harry Ciao <qingtao.cao@windriver.com>
> Date: Mon, 8 Nov 2010 14:42:38 +0800
> Subject: [v0 PATCH 1/1] Add support for the samhain program.
> 
> Add support for the samhain program.
> 
> Note, extra privileges may need to be granted to the samhain domain
> if its configuration file(/etc/samhainrc) is changed.
> 
> Signed-off-by: Harry Ciao <qingtao.cao@windriver.com>
> ---
>  policy/modules/roles/sysadm.te     |    4 +
>  policy/modules/services/samhain.fc |   13 +++
>  policy/modules/services/samhain.if |  210 ++++++++++++++++++++++++++++++++++++
>  policy/modules/services/samhain.te |  118 ++++++++++++++++++++
>  4 files changed, 345 insertions(+), 0 deletions(-)
>  create mode 100644 policy/modules/services/samhain.fc
>  create mode 100644 policy/modules/services/samhain.if
>  create mode 100644 policy/modules/services/samhain.te
> 
> diff --git a/policy/modules/roles/sysadm.te b/policy/modules/roles/sysadm.te
> index d5e88be..69cb358 100644
> --- a/policy/modules/roles/sysadm.te
> +++ b/policy/modules/roles/sysadm.te
> @@ -290,6 +290,10 @@ optional_policy(`
>  ')
>  
>  optional_policy(`
> +	samhain_admin(sysadm_t, sysadm_r)
> +')

why not samhain_run?

> +
> +optional_policy(`
>  	screen_role_template(sysadm, sysadm_r, sysadm_t)
>  ')
>  
> diff --git a/policy/modules/services/samhain.fc b/policy/modules/services/samhain.fc
> new file mode 100644
> index 0000000..5f78098
> --- /dev/null
> +++ b/policy/modules/services/samhain.fc
> @@ -0,0 +1,13 @@
> +/etc/rc\.d/init\.d/samhain	--	gen_context(system_u:object_r:samhain_initrc_exec_t,mls_systemhigh)
> +
> +/etc/samhainrc	--	gen_context(system_u:object_r:samhain_etc_t,mls_systemhigh)
> +
> +/usr/sbin/samhain	--	gen_context(system_u:object_r:samhain_exec_t,mls_systemhigh)
> +/usr/sbin/samhain_setpwd	--	gen_context(system_u:object_r:samhain_exec_t,mls_systemhigh)
> +
> +/var/lib/samhain(/.*)?	gen_context(system_u:object_r:samhain_db_t,mls_systemhigh)
> +
> +/var/log/samhain_log	--	gen_context(system_u:object_r:samhain_log_t,mls_systemhigh)
> +/var/log/samhain_log.lock	--	gen_context(system_u:object_r:samhain_log_t,mls_systemhigh)

I think refpolicy wants you to escape the dot (although i dont think it matters for file extensions

> +
> +/var/run/samhain\.pid	--	gen_context(system_u:object_r:samhain_var_run_t,mls_systemhigh)
> diff --git a/policy/modules/services/samhain.if b/policy/modules/services/samhain.if
> new file mode 100644
> index 0000000..2db64f8
> --- /dev/null
> +++ b/policy/modules/services/samhain.if
> @@ -0,0 +1,210 @@
> +## <summary>Samhain - check file integrity</summary>
> +
> +########################################
> +## <summary>
> +##	Execute samhain in the samhain domain
> +## </summary>
> +## <param name="domain">
> +##	<summary>
> +##	Domain allowed to transition.
> +##	</summary>
> +## </param>
> +#
> +interface(`samhain_domtrans',`
> +	gen_require(`
> +		type samhain_t, samhain_exec_t;
> +	')
> +
> +	corecmd_search_bin($1)
> +	domtrans_pattern($1, samhain_exec_t, samhain_t)
> +')
> +
> +########################################
> +## <summary>
> +##	Execute samhain in the samhain domain, and
> +##	allow the specifiled role the samhain domain
> +## </summary>
> +## <param name="domain">
> +##	<summary>
> +##	Domain allowed to transition.
> +##	</summary>
> +## </param>
> +## <param name="role">
> +##	<summary>
> +##	Role allowed to access.
> +##	</summary>
> +## </param>
> +## <rolecap/>
> +#
> +interface(`samhain_run',`
> +	gen_require(`
> +		type samhain_t;
> +	')
> +
> +	samhain_domtrans($1)
> +	role $2 types samhain_t;
> +')
> +
> +########################################
> +## <summary>
> +##	Manage the samhain configuration file.

personal nickpick but i usually use plural:

Manage samhain configuration files.

> +## </summary>
> +## <param name="domain">
> +##	<summary>
> +##	Domain allowed access.
> +##      </summary>

Watch your tabs and whitespaces.

> +## </param>
> +#
> +interface(`samhain_manage_config',`

I would use samhain_manage_config_files because thats what it does.

> +	gen_require(`
> +		type samhain_etc_t;
> +	')
> +
> +	files_search_etc($1)
> +	manage_files_pattern($1, samhain_etc_t, samhain_etc_t)
> +')
> +
> +########################################
> +## <summary>
> +##	Manage the samhain database directory and file.

plural: Manage samhain databases or Manage samhain database content.


> +## </summary>
> +## <param name="domain">
> +##	<summary>
> +##	Domain allowed access.
> +##      </summary>

tabs and spaces.

> +## </param>
> +#
> +interface(`samhain_manage_db',`
> +	gen_require(`
> +		type samhain_db_t;
> +	')
> +
> +	files_search_var_lib($1)
> +	manage_files_pattern($1, samhain_db_t, samhain_db_t)

In the description you say manage directories and files but this only allows management of files. if this is correct use: samhain_manage_db_files.
> +')
> +
> +#######################################
> +## <summary>
> +##	Manage the samhain init script.

plural: Manage samhain rc script files.

> +## </summary>
> +## <param name="domain">
> +##	<summary>
> +##	Domain allowed access.
> +##      </summary>

tabs and spaces. When you copy and paste interfaces: double check that all is correct and applicable
> +## </param>
> +#
> +interface(`samhain_manage_init_script',`

samhain_manage_rc_script_files.
> +	gen_require(`
> +		type samhain_initrc_exec_t;
> +	')
> +
> +	files_search_etc($1)
> +	manage_files_pattern($1, samhain_initrc_exec_t, samhain_initrc_exec_t)
> +')
> +
> +########################################
> +## <summary>
> +##	Manage the samhain log and log.lock files.

Manage samhain log files.
> +## </summary>
> +## <param name="domain">
> +##	<summary>
> +##	Domain allowed access.
> +##      </summary>

tabs and whitespaces
> +## </param>
> +#
> +interface(`samhain_manage_log',`

samhain_manage_log_files

> +	gen_require(`
> +		type samhain_log_t;
> +	')
> +
> +	logging_search_logs($1)
> +	manage_files_pattern($1, samhain_log_t, samhain_log_t)
> +')
> +
> +########################################
> +## <summary>
> +##	Manage the samhain pid file.

Manage samhain pid files.

> +## </summary>
> +## <param name="domain">
> +##	<summary>
> +##	Domain allowed access.
> +##      </summary>

tabs

> +## </param>
> +#
> +interface(`samhain_manage_pid',`

samhain_manage_pid_files.

> +	gen_require(`
> +		type samhain_var_run_t;
> +	')
> +
> +	files_search_pids($1)
> +	manage_files_pattern($1, samhain_var_run_t, samhain_var_run_t)

files_search_pids()

> +')
> +
> +#######################################
> +## <summary>
> +##	Enable executing the samhain init script directly
> +##	(thus bypassing the run_init tool)

Use a generic description, Extra comments go in <desc> ... </desc>

> +## </summary>
> +## <param name="domain">
> +##	<summary>
> +##	Domain allowed access.
> +##      </summary>

Domain allowed to transition.

watch your tabs

> +## </param>
> +## <param name="role">
> +## 	<summary>
> +## 	User role allowed access.

Role allowed access.

> +## 	</summary>
> +## </param>
> +## <rolecap/>
> +#
> +interface(`samhain_run_init_script',`
> +	gen_require(`
> +		type samhain_initrc_exec_t;
> +		role system_r;
> +	')
> +
> +	init_labeled_script_domtrans($1, samhain_initrc_exec_t)
> +	domain_system_change_exemption($1)
> +	role_transition $2 samhain_initrc_exec_t system_r;
> +	allow $2 system_r;

This doesnt look right. look in refpolicy for other examples of how this is done properly.

> +')
> +
> +#######################################
> +## <summary>
> +##	All of the rules required to administrate
> +##	the samhain environment.
> +## </summary>
> +## <param name="domain">
> +##	<summary>
> +##	Domain allowed access.
> +##      </summary>
> +## </param>
> +## <param name="role">
> +## 	<summary>
> +## 	Role allowed access.
> +## 	</summary>
> +## </param>
> +## <rolecap/>
> +#
> +interface(`samhain_admin',`
> +	gen_require(`
> +		type samhain_t;
> +	')
> +
> +	allow $1 samhain_t:process { ptrace signal_perms };
> +	ps_process_pattern($1, samhain_t)
> +
> +	samhain_manage_config($1)
> +	samhain_manage_db($1)
> +	samhain_manage_init_script($1)
> +	samhain_manage_log($1)
> +	samhain_manage_pid($1)

Use admin_patterns() instead of this interface calls.
Example:

logging_list_logs($1)
admin_pattern($1, samhain_log_t)

> +
> +	# The parent directories of samhain's database, log/log.lock files,
> +	# pid files are of s0, while these files are of the clearance level.
> +	mls_file_write_all_levels($1)
> +
> +	samhain_run($1, $2)
> +
> +	#samhain_run_init_script($1, $2)
Look to other similar *_admin() interfaces to see how its done properly. If samhain does not have an rc script then just remove it.

> +')
> diff --git a/policy/modules/services/samhain.te b/policy/modules/services/samhain.te
> new file mode 100644
> index 0000000..aaf9d23
> --- /dev/null
> +++ b/policy/modules/services/samhain.te
> @@ -0,0 +1,118 @@
> +policy_module(samhain, 1.0.0)
> +
> +########################################
> +#
> +# Declarations
> +#
> +
> +# Configuration file

These comments are redundant.

> +type samhain_etc_t;
> +files_security_file(samhain_etc_t)

Not sure about this but looks wrong. i would use files_config_file()

> +
> +# Log files

comment redundant.

> +type samhain_log_t;
> +logging_log_file(samhain_log_t)
> +
> +# File signature database
> +type samhain_db_t;
> +files_type(samhain_db_t)
> +
> +# init script

comment redundant.

> +type samhain_initrc_exec_t;
> +init_script_file(samhain_initrc_exec_t)
> +
> +# PID file

comment redundant.

> +type samhain_var_run_t;
> +files_pid_file(samhain_var_run_t)
> +
> +# Samhain domain
> +type samhain_t;
> +type samhain_exec_t;
> +init_ranged_daemon_domain(samhain_t, samhain_exec_t, mls_systemhigh)
> +
> +########################################
> +#
> +# Local policy
> +#
> +
> +allow samhain_t self:capability { dac_override dac_read_search fowner ipc_lock };
> +dontaudit samhain_t self:capability { sys_resource };

brace not needed (nothing to expand)

> +allow samhain_t self:fd { use };

Brace not needed. (nothing to expand)

> +# Need signal_perms to send SIGABRT/SIGKILL to termiate the samhain daemon
> +allow samhain_t self:process { setsched setrlimit signal_perms };

new line

> +allow samhain_t samhain_etc_t:file read_file_perms;
> +
> +# Samhain needs to get the attribute of /proc/kcore.
> +kernel_getattr_core_if(samhain_t)
> +
> +# Only needed when starting samhain daemon from its init script.
> +can_exec(samhain_t, samhain_exec_t)

I think this goes above kernel_*

> +
> +corecmd_list_bin(samhain_t)
> +corecmd_read_bin_symlinks(samhain_t)
> +
> +# To get entropy
> +dev_read_urand(samhain_t)
> +dev_dontaudit_read_rand(samhain_t)

Why dontaudit this?

> +
> +domain_use_interactive_fds(samhain_t)
> +
> +# To read configuration file
> +files_search_etc(samhain_t)
> +
> +# Manage PID file

comment redundant. This goes above the interface calls.

> +manage_files_pattern(samhain_t, samhain_var_run_t, samhain_var_run_t)
> +files_pid_filetrans(samhain_t, samhain_var_run_t, file)
> +
> +# Manage database directory and file
> +manage_files_pattern(samhain_t, samhain_db_t, samhain_db_t)
> +files_var_lib_filetrans(samhain_t, samhain_db_t, { file dir })
> +
> +# Manage log and log.lock files

comment redundant. This goes above the interface calls

> +manage_files_pattern(samhain_t, samhain_log_t, samhain_log_t)
> +logging_log_filetrans(samhain_t, samhain_log_t, file)
> +
> +# Samhain's PID and log.lock files are in directories of s0,
> +# while samhain daemon is running with the clearance level
> +mls_file_write_all_levels(samhain_t)
> +
> +# Get the attribute of all kinds of files in the rootfs
> +dev_getattr_all_blk_files(samhain_t)
> +dev_getattr_all_chr_files(samhain_t)
> +dev_getattr_generic_blk_files(samhain_t)
> +dev_getattr_generic_chr_files(samhain_t)

newline

> +files_getattr_all_dirs(samhain_t)
> +files_getattr_all_files(samhain_t)
> +files_getattr_all_symlinks(samhain_t)
> +files_getattr_all_pipes(samhain_t)
> +files_getattr_all_sockets(samhain_t)
> +files_getattr_all_mountpoints(samhain_t)
> +# Read from the file_type attribute and the lnk_file class
> +files_read_all_files(samhain_t)
> +files_read_all_symlinks(samhain_t)
> +
> +# Read from utmp and wtmp when monitoring login/logout events
> +init_read_utmp(samhain_t)
> +auth_read_login_records(samhain_t)

Use proper ordering. see style guide and refpolicy examples.

> +
> +# Get the attribute of other filesystem's mountpoint, such as /selinux
> +# /proc, /sys and /tmp, but not the contents inside, which suggests
> +# that following rules should be set in samhain configuration file:
> +# [Attributes]
> +#    file = /tmp
> +#    file = /proc
> +#    file = /sys
> +#    file = /selinux
> +# [IgnoreALL]
> +#    dir = -1/tmp
> +#    dir = -1/proc
> +#    dir = -1/sys
> +#    dir = -1/selinux
> +fs_getattr_all_dirs(samhain_t)
> +
> +# Get attribute of files with variant sensitivity levels
> +mls_file_read_all_levels(samhain_t)
> +
> +logging_send_syslog_msg(samhain_t)
> +
> +userdom_use_user_terminals(samhain_t)
> -- 
> 1.7.0.4
> 

> _______________________________________________
> refpolicy mailing list
> refpolicy at oss.tresys.com
> http://oss.tresys.com/mailman/listinfo/refpolicy

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
Url : http://oss.tresys.com/pipermail/refpolicy/attachments/20101111/504f282c/attachment.bin