From: sven.vermeulen@siphos.be (Sven Vermeulen)
Date: Mon, 9 Jan 2012 22:35:47 +0100
Subject: [refpolicy] Contribute cachefilesd policy from Fedora to
 Refpolicy.
In-Reply-To: <4F072D2F.7000908@redhat.com>
References: <4F072D2F.7000908@redhat.com>
Message-ID: <20120109213546.GJ3416@siphos.be>
To: refpolicy@oss.tresys.com
List-Id: refpolicy.oss.tresys.com

On Fri, Jan 06, 2012 at 12:19:43PM -0500, Daniel J Walsh wrote:
> Originally written by David Howells and Karl MacMillan
> 
> Please Review and Ack.
[...]
> +###############################################################################
> +#
> +# Copyright (C) 2006 Red Hat, Inc. All Rights Reserved.
> +# Written by David Howells (dhowells at redhat.com)
> +#            Karl MacMillan (kmacmill at redhat.com)
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License
> +# as published by the Free Software Foundation; either version
> +# 2 of the License, or (at your option) any later version.
> +#
> +###############################################################################

Not sure how refpolicy wants to deal with copyright statements like these.
They aren't harmful and the license is okay, but looking at what is
currently in, this would be a first. Christopher?

> +#
> +# Define the contexts to be assigned to various files and directories of
> +# importance to the CacheFiles kernel module and userspace management daemon.
> +#
> +
> +# cachefilesd executable will have:
> +# label: system_u:object_r:cachefilesd_exec_t
> +# MLS sensitivity: s0
> +# MCS categories: <none>

Superfluous comments (here and later); refpolicy wants to keep comments only when it isn't
obvious from the code.

> +type cachefiles_var_t;
> +files_type(cachefiles_var_t)
> +
> +type cachefiles_dev_t;
> +dev_node(cachefiles_dev_t)
> +
> +type cachefilesd_t;
> +type cachefilesd_exec_t;
> +init_daemon_domain(cachefilesd_t, cachefilesd_exec_t)
> +
> +type cachefilesd_var_run_t;
> +files_pid_file(cachefilesd_var_run_t)
> +
> +type cachefiles_kernel_t;
> +domain_type(cachefiles_kernel_t)
> +domain_obj_id_change_exemption(cachefiles_kernel_t)
> +role system_r types cachefiles_kernel_t;

The order is a bit off. Probably best to start with
cachefiles_t/cachefiles_exec_t, then the rest in alphabetical order.

> +optional_policy(`
> +	rpm_use_script_fds(cachefilesd_t)
> +')

Should be moved downwards.

> +allow cachefilesd_t cachefilesd_var_run_t:file create_file_perms;
> +manage_files_pattern(cachefilesd_t, cachefilesd_var_run_t, cachefilesd_var_run_t)

manage_files_pattern contains manage_file_perms, which includes the
privileges from create_file_perms, so the first allow statement isn't
needed.

> +allow cachefilesd_t cachefiles_var_t:dir { rw_dir_perms delete_dir_perms };

Isn't manage_dir_pattern() more appropriate here?

> +files_read_etc_files(cachefilesd_t)
> +miscfiles_read_localization(cachefilesd_t)
> +logging_send_syslog_msg(cachefilesd_t)
> +init_dontaudit_use_script_ptys(cachefilesd_t)
> +term_dontaudit_use_generic_ptys(cachefilesd_t)
> +term_dontaudit_getattr_unallocated_ttys(cachefilesd_t)

Should be alphabetically ordered (first the ones from the kernel section,
then system).

Wkr,
	Sven Vermeulen