From: harrytaurus2002@hotmail.com (HarryCiao)
Date: Fri, 12 Nov 2010 10:27:12 +0000
Subject: [refpolicy] FW:  Add support for the samhain program
In-Reply-To: <20101111121804.GA17316@localhost.localdomain>
References: <SNT139-w647DF2426616B71E050D74AB300@phx.gbl>,
	<SNT139-w4060518B7D42FDC9CB94CEAB320@phx.gbl>,
	<20101111121804.GA17316@localhost.localdomain>
Message-ID: <SNT139-w213FD19EAD76DC3550A145AB330@phx.gbl>
To: refpolicy@oss.tresys.com
List-Id: refpolicy.oss.tresys.com


Hi Dom,

So many thanks for your patent reply! I have enhanced samhain.pp based on your comments, please see the attached v1 version. But I did have some related pending questions I would like to further ask you about, please see my inline replies(prefixed by "[H]"), thanks a lot!


Date: Thu, 11 Nov 2010 13:18:05 +0100
From: domg472@gmail.com
To: refpolicy at oss.tresys.com
Subject: Re: [refpolicy] FW:  Add support for the samhain program

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?
 
[H]: Ok, I have changed to call samhain_run() for the sysadm, but then the samhain_admin() interface is defined but not currently used anywhere(same as aide_admin()). If samhain daemon is in a bad condition, then no administrator role could clean up its log/log.lock files or database files when SELinux is in the Enforcing mode.

What's the point of defining some so-far-not-used interface? I guess one possible answer is that they are likely to be used in the future, for example, the logadm may want to be able to manage the samhain log files in the future.

What you think?

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

[H]: Right, I should have put a "\" in the front of the "." character here.
 
> +
> +/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.

[H]: Gotcha, thanks!

> +')
> +
> +#######################################
> +## <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

[H]: Gotcha, thanks!

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

[H]: Ok, I have removed this interface but preserving the role allow and role transition rules above into the samhain_admin() interface, in the hope that the samhain administrator could use its rc init script file directly and the samhain domain would run in the system_r role, the same role as when it is started by init during system starting up.
 
> +')
> +
> +#######################################
> +## <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)
 
[H]: I see your points, and I'd noticed the usage of admin_pattern in aide.if.

Same concern as above: if these samhain_manage_xxxx_files() interfaces not used(in neither .te nor .if here), should we preserve them or discard them?

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

[H]: Gotcha, thanks!

 
> +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_*

[H]: Yes, turns out can_exec is a macro indeed and should be placed before any interface called.
 
> +
> +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?
 
[H]: Right, this interface has nothing to do with /dev/urandom or /dev/random

Have a nice weekend!

Best regards,
Harry

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

_______________________________________________
refpolicy mailing list
refpolicy at oss.tresys.com
http://oss.tresys.com/mailman/listinfo/refpolicy 		 	   		  
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://oss.tresys.com/pipermail/refpolicy/attachments/20101112/7685dbb4/attachment-0001.html 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: v1-Add-support-for-the-samhain-program.patch
Type: text/x-patch
Size: 10255 bytes
Desc: not available
Url : http://oss.tresys.com/pipermail/refpolicy/attachments/20101112/7685dbb4/attachment-0001.bin