Creating and packaging a new policy module

[Juan Orti Alcaine]

Hello, I'm the package mantainer of gogoc, and I'm creating my first policy 
module for it following the instructions of this draft in the wiki [1].

It says you must build your module for three policies: mls, scritct and 
targeted, but I don't see any strict policy, is this information still 
correct? Must I build it also for minimum?

Also I have doubts if the module will always live in the gogoc package or it 
will be migrated sometime to the main selinux-policy-targeted package.

If you can take a look at the policy to find any possible error it would be 
great. It's already in the git repository of gogoc [2]

Kind regards.

[1] https://fedoraproject.org/wiki/SELinux_Policy_Modules_Packaging_Draft
[2] http://pkgs.fedoraproject.org/cgit/gogoc.git/tree/

[Dominick Grift]

On Sun, 2013-08-18 at 20:10 +0200, Juan Orti Alcaine wrote:
>  Hello, I'm the package mantainer of gogoc, and I'm creating
> my first policy 
>  module for it following the instructions of this draft in the wiki [1].
>  
>  It says you must build your module for three policies: mls, scritct and 
>  targeted, but I don't see any strict policy, is this information still 
>  correct? Must I build it also for minimum? 
Yes strict no longer exists, so remove any reference to it

>  
>  Also I have doubts if the module will always live in the gogoc package or it 
>  will be migrated sometime to the main selinux-policy-targeted package.
>  
>  If you can take a look at the policy to find any possible error it would be 
>  great. It's already in the git repository of gogoc [2]
>   
You policy should have no require{} in the .te file, everything should
have an api that you can use instead

Only type transition on what you need to type transition on, instead of
everything (you type transition on everything)

corecmd_bin_entry_type(gogoc_t) <- this doesnt make sense as you do not
domain type transition on bin_t anywhere

radvd_admin(gogoc_t, system_r) <- this one isnt appropriate here

systemd_exec_systemctl(gogoc_t)  <- why is this needed?

allow gogoc_t radvd_exec_t:file { read execute open execute_no_trans };
<-- depending on why gogoc runs dadvd you may want to run radvd with a
domain transition instead. If it turns out that you should have ran
radvd with a domain transition ,then it is encouraged you start over
with your policy because, one should always take care of type
transitions first before adding any other rules. because type
transitions can greatly impact access your process needs

There are duplicate rules in your policy

For example:
sysnet_dns_name_resolve(gogoc_t)
and
files_read_etc_files(gogoc_t)

are already enclosed with:
auth_use_nsswitch(gogoc_t)

Theres probably a bit moreroom for improvement other than above but this
is a start


>  Kind regards.
>  
>  [1] https://fedoraproject.org/wiki/SELinux_Policy_Modules_Packaging_Draft
>  [2] http://pkgs.fedoraproject.org/cgit/gogoc.git/tree/
>  --
>  selinux mailing list
>  selinux(a)lists.fedoraproject.org
>  https://admin.fedoraproject.org/mailman/listinfo/selinux

[Juan Orti Alcaine]

El 2013-08-19 13:27, Dominick Grift escribió:
>  On Sun, 2013-08-18 at 20:10 +0200, Juan Orti Alcaine wrote:
> > Hello, I'm the package mantainer of gogoc, and I'm creating my first 
> > policy
> > module for it following the instructions of this draft in the wiki 
> > [1].
> > 
> > It says you must build your module for three policies: mls, scritct 
> > and
> > targeted, but I don't see any strict policy, is this information still
> > correct? Must I build it also for minimum?
>  
>  Yes strict no longer exists, so remove any reference to it 
And what about the minimum policy?
Also, I see some packages dropping their policies to 
/usr/share/selinux/packages/ and others to 
/usr/share/selinux/{targeted,mls}. What's better?

>  
> > 
> > Also I have doubts if the module will always live in the gogoc package 
> > or it
> > will be migrated sometime to the main selinux-policy-targeted package.
> > 
> > If you can take a look at the policy to find any possible error it 
> > would be
> > great. It's already in the git repository of gogoc [2]
> > 
>  
>  You policy should have no require{} in the .te file, everything should
>  have an api that you can use instead
>  
>  Only type transition on what you need to type transition on, instead of
>  everything (you type transition on everything)
>  
>  corecmd_bin_entry_type(gogoc_t) <- this doesnt make sense as you do not
>  domain type transition on bin_t anywhere
>  
>  radvd_admin(gogoc_t, system_r) <- this one isnt appropriate here
>  
>  systemd_exec_systemctl(gogoc_t)  <- why is this needed?
>  
>  allow gogoc_t radvd_exec_t:file { read execute open execute_no_trans };
>  <-- depending on why gogoc runs dadvd you may want to run radvd with a
>  domain transition instead. If it turns out that you should have ran
>  radvd with a domain transition ,then it is encouraged you start over
>  with your policy because, one should always take care of type
>  transitions first before adding any other rules. because type
>  transitions can greatly impact access your process needs
>  
>  There are duplicate rules in your policy
>  
>  For example:
>  sysnet_dns_name_resolve(gogoc_t)
>  and
>  files_read_etc_files(gogoc_t)
>  
>  are already enclosed with:
>  auth_use_nsswitch(gogoc_t)
>  
>  Theres probably a bit moreroom for improvement other than above but 
>  this
>  is a start 
Thank you for your comments, I'm newbie at policy writing.

What gogoc does is to negotiate the tunnel and execute a shell script to 
configure the tun interface and launch the radvd with a custom config to 
advertise the prefix in the net.

I'm rewriting the policy and have doubts about how to transition to the 
radvd_t domain. I miss a interface like radvd_domtrans(gogoc_t), which I 
think it's the way to do the transition, another way I'm thinking it's 
adding a section require{ type radvd_exec_t;} and grant access to 
execute and domain transition. Which api function should I use?

Regards,
Juan.

[Dominick Grift]

On Tue, 2013-08-20 at 10:51 +0000, Juan Orti Alcaine wrote:
>  El 2013-08-19 13:27, Dominick Grift escribió:
>  > On Sun, 2013-08-18 at 20:10 +0200, Juan Orti Alcaine wrote:
>  >> Hello, I'm the package mantainer of gogoc, and I'm creating my first 
>  >> policy
>  >> module for it following the instructions of this draft in the wiki 
>  >> [1].
>  >> 
>  >> It says you must build your module for three policies: mls, scritct 
>  >> and
>  >> targeted, but I don't see any strict policy, is this information still
>  >> correct? Must I build it also for minimum?
>  > 
>  > Yes strict no longer exists, so remove any reference to it
>  
>  And what about the minimum policy? 
Probably should ignore minimal as well. Since that policy model aims to
be minimal

>  Also, I see some packages dropping their policies to 
>  /usr/share/selinux/packages/ and others to 
>  /usr/share/selinux/{targeted,mls}. What's better? 
Not sure but probably the latter

>  
>  > 
>  >> 
>  >> Also I have doubts if the module will always live in the gogoc package 
>  >> or it
>  >> will be migrated sometime to the main selinux-policy-targeted package.
>  >> 
>  >> If you can take a look at the policy to find any possible error it 
>  >> would be
>  >> great. It's already in the git repository of gogoc [2]
>  >> 
>  > 
>  > You policy should have no require{} in the .te file, everything should
>  > have an api that you can use instead
>  > 
>  > Only type transition on what you need to type transition on, instead of
>  > everything (you type transition on everything)
>  > 
>  > corecmd_bin_entry_type(gogoc_t) <- this doesnt make sense as you do not
>  > domain type transition on bin_t anywhere
>  > 
>  > radvd_admin(gogoc_t, system_r) <- this one isnt appropriate here
>  > 
>  > systemd_exec_systemctl(gogoc_t)  <- why is this needed?
>  > 
>  > allow gogoc_t radvd_exec_t:file { read execute open execute_no_trans };
>  > <-- depending on why gogoc runs dadvd you may want to run radvd with a
>  > domain transition instead. If it turns out that you should have ran
>  > radvd with a domain transition ,then it is encouraged you start over
>  > with your policy because, one should always take care of type
>  > transitions first before adding any other rules. because type
>  > transitions can greatly impact access your process needs
>  > 
>  > There are duplicate rules in your policy
>  > 
>  > For example:
>  > sysnet_dns_name_resolve(gogoc_t)
>  > and
>  > files_read_etc_files(gogoc_t)
>  > 
>  > are already enclosed with:
>  > auth_use_nsswitch(gogoc_t)
>  > 
>  > Theres probably a bit moreroom for improvement other than above but 
>  > this
>  > is a start
>  
>  Thank you for your comments, I'm newbie at policy writing.
>  
>  What gogoc does is to negotiate the tunnel and execute a shell script to 
>  configure the tun interface and launch the radvd with a custom config to 
>  advertise the prefix in the net.
>  
>  I'm rewriting the policy and have doubts about how to transition to the 
>  radvd_t domain. I miss a interface like radvd_domtrans(gogoc_t), which I 
>  think it's the way to do the transition, another way I'm thinking it's 
>  adding a section require{ type radvd_exec_t;} and grant access to 
>  execute and domain transition. Which api function should I use? 
upstream will probably only accept it with the use of a dadvd_domtrans()
but for your solution for now you could do something like this:

optional_policy(`
gen_require(`
	type radvd_exec_t, radvd_t;
')
domtrans_pattern(gogoc_t, radvd_exec_t, radvd_t)
')

>  
>  Regards,
>  Juan.

[Juan Orti Alcaine]

El 2013-08-20 11:13, Dominick Grift escribió:
>  upstream will probably only accept it with the use of a 
>  dadvd_domtrans()
>  but for your solution for now you could do something like this:
>  
>  optional_policy(`
>  gen_require(`
>  	type radvd_exec_t, radvd_t;
>  ')
>  domtrans_pattern(gogoc_t, radvd_exec_t, radvd_t)
>  ')
>   
I have updated the policy, could you please take a look at it and give 
me your oppinion?

http://pkgs.fedoraproject.org/cgit/gogoc.git/tree/gogoc.te
http://pkgs.fedoraproject.org/cgit/gogoc.git/tree/gogoc.if
http://pkgs.fedoraproject.org/cgit/gogoc.git/tree/gogoc.fc

Thank you,
Juan.

[Dominick Grift]

On Thu, 2013-08-22 at 06:33 +0000, Juan Orti Alcaine wrote:
>  El 2013-08-20 11:13, Dominick Grift escribió:
>  > upstream will probably only accept it with the use of a 
>  > dadvd_domtrans()
>  > but for your solution for now you could do something like this:
>  > 
>  > optional_policy(`
>  > gen_require(`
>  > 	type radvd_exec_t, radvd_t;
>  > ')
>  > domtrans_pattern(gogoc_t, radvd_exec_t, radvd_t)
>  > ')
>  > 
>  
>  I have updated the policy, could you please take a look at it and give 
>  me your oppinion?
>   
sysnet_read_config(gogoc_t) is duplicate since it is already called in
auth_use_nsswitch(gogoc_t)

allow radvd_t gogoc_var_run_t:file rw_file_perms; can be changed to:
allow radvd_t gogoc_var_run_t:file write_file_perms; since:
gogoc_read_pid_files(radvd_t) already allows radvd_t to read
gogoc_var_run_t files

Not sure but:
files_tmp_filetrans(gogoc_t, gogoc_tmp_t, { file dir }) can probably be
changed to:
files_tmp_filetrans(gogoc_t, gogoc_tmp_t, dir) since the type transition
probably is only needed for the dir (the file is probably created inside
this dir)

allow gogoc_t radvd_etc_t:file manage_file_perms; if this file gets
created by gogoc_t, then this probably needs a file type transition rule
as well, since the config file is located in /etc/ so without a type
transition rule the file would be created with type etc_t instead of
type radvd_etc_t

allow gogoc_t gogoc_tmp_t:file manage_file_perms; this is a duplicate
rule and can be removed

allow gogoc_t gogoc_log_t:file manage_file_perms insufficient, and may
be improved:

create_files_pattern(gogoc_t, gogoc_log_t, gogoc_log_t)
allow gogoc_t gogoc_log_t:file { append_file_perms read_file_perms
setattr_file_perms };

This will remove the write permission which gogoc_t shouldnt need ( log
files should be opened for append only)

gogoc_t probably needs to be able to create log file which means it
needs to be able to write/add directory entries to parent
dir /var/log/gogoc

allow gogoc_t gogoc_var_lib_t:file manage_file_perms;
allow gogoc_t gogoc_var_lib_t:dir rw_dir_perms;
allow gogoc_t gogoc_var_run_t:file manage_file_perms;
allow gogoc_t gogoc_var_run_t:dir rw_dir_perms;
allow gogoc_t gogoc_etc_t:file read_file_perms;
allow gogoc_t gogoc_etc_t:dir list_dir_perms;

These can be improved a bit by user patterns instead:
example:
manage_files_pattern(gogoc_t, gogpc_var_lib_t, gogoc_var_lib_t)
manage_files_pattern(gogoc_t, gogpc_var_run_t, gogoc_var_run_t)
read_files_pattern(gogoc_t, gogpc_etc_t, gogoc_etc_t)

This might shave off some unneeded permissions as well

type gogoc_etc_t;
files_config_file(gogoc_etc_t)

I would probably name this type "gogoc_conf_t" instead since
"gogoc_etc_t" refers to a path instead of a property of a file ( nitpick
but in light of consistent and self-documenting policy better to get
used to the best choices)

allow gogoc_t self:unix_dgram_socket create_socket_perms; Duplicate
rule: allowed included with: logging_send_syslog_msg(gogoc_t)

allow gogoc_t self:udp_socket create_socket_perms; duplicate rule:
already incuded with 
auth_use_nsswitch(gogoc_t)

>  http://pkgs.fedoraproject.org/cgit/gogoc.git/tree/gogoc.te
>  http://pkgs.fedoraproject.org/cgit/gogoc.git/tree/gogoc.if
>  http://pkgs.fedoraproject.org/cgit/gogoc.git/tree/gogoc.fc
>  
>  Thank you,
>  Juan.
>  --
>  selinux mailing list
>  selinux(a)lists.fedoraproject.org
>  https://admin.fedoraproject.org/mailman/listinfo/selinux

[Dominick Grift]

On Thu, 2013-08-22 at 09:09 +0200, Dominick Grift wrote:

>  
>  allow gogoc_t radvd_etc_t:file manage_file_perms; if this file gets
>  created by gogoc_t, then this probably needs a file type transition rule
>  as well, since the config file is located in /etc/ so without a type
>  transition rule the file would be created with type etc_t instead of
>  type radvd_etc_t 
Actually, now i see what you are trying to do:

> /var/run/gogoc/gogoc-rtadvd.conf		gen_context(system_u:object_r:radvd_etc_t,s0) 
So the config file is in /var/run/gogoc/gogoc-rtadvd.conf instead of /etc

remove this fc spec and remove these rules:

   gogoc_read_pid_files(radvd_t) # For radvd to read the generated config file
   allow gogoc_t radvd_etc_t:file manage_file_perms; # Create config file for radvd
   allow radvd_t gogoc_var_run_t:file rw_file_perms;

instead just allow radvd_t to manage gogoc_var_run_t files:

manage_files_pattern(radvd_t, gpgpc_var_run_t, gogoc_var_run_t)

[Dominick Grift]

On Thu, 2013-08-22 at 06:33 +0000, Juan Orti Alcaine wrote:
>  El 2013-08-20 11:13, Dominick Grift escribió:
>  > upstream will probably only accept it with the use of a 
>  > dadvd_domtrans()
>  > but for your solution for now you could do something like this:
>  > 
>  > optional_policy(`
>  > gen_require(`
>  > 	type radvd_exec_t, radvd_t;
>  > ')
>  > domtrans_pattern(gogoc_t, radvd_exec_t, radvd_t)
>  > ')
>  > 
>  
>  I have updated the policy, could you please take a look at it and give 
>  me your oppinion? 
sysnet_exec_ifconfig(gogoc_t)

its probably worth considering a domain transition to ifconfig instead
because:

allow gogoc_t self:capability { net_admin net_raw kill };

Are probably needed by ifconfig, and by running ifconfig in the ifconfig
domain, you might be able to remove these permissions from gogoc_t

However if you do decide to domain transition to ifconfig then its
probably a good idea to start all over, since other permissions you
added for gogoc_t might no longer be needed because they were added for
ifconfig

>  
>  http://pkgs.fedoraproject.org/cgit/gogoc.git/tree/gogoc.te
>  http://pkgs.fedoraproject.org/cgit/gogoc.git/tree/gogoc.if
>  http://pkgs.fedoraproject.org/cgit/gogoc.git/tree/gogoc.fc
>  
>  Thank you,
>  Juan.
>  --
>  selinux mailing list
>  selinux(a)lists.fedoraproject.org
>  https://admin.fedoraproject.org/mailman/listinfo/selinux

[Miroslav Grepl]

On 08/22/2013 09:25 AM, Dominick Grift wrote:
>  On Thu, 2013-08-22 at 06:33 +0000, Juan Orti Alcaine wrote:
> > El 2013-08-20 11:13, Dominick Grift escribió:
> >> upstream will probably only accept it with the use of a
> >> dadvd_domtrans()
> >> but for your solution for now you could do something like this:
> >>
> >> optional_policy(`
> >> gen_require(`
> >> 	type radvd_exec_t, radvd_t;
> >> ')
> >> domtrans_pattern(gogoc_t, radvd_exec_t, radvd_t)
> >> ')
> >>
> > I have updated the policy, could you please take a look at it and give
> > me your oppinion?
>  sysnet_exec_ifconfig(gogoc_t)
>
>  its probably worth considering a domain transition to ifconfig instead
>  because:
>
>  allow gogoc_t self:capability { net_admin net_raw kill };
>
>  Are probably needed by ifconfig, and by running ifconfig in the ifconfig
>  domain, you might be able to remove these permissions from gogoc_t
>
>  However if you do decide to domain transition to ifconfig then its
>  probably a good idea to start all over, since other permissions you
>  added for gogoc_t might no longer be needed because they were added for
>  ifconfig 
Yes, basically it could be decided from AVC msgs which you were getting.
> > http://pkgs.fedoraproject.org/cgit/gogoc.git/tree/gogoc.te
> > http://pkgs.fedoraproject.org/cgit/gogoc.git/tree/gogoc.if
> > http://pkgs.fedoraproject.org/cgit/gogoc.git/tree/gogoc.fc
> >
> > Thank you,
> > Juan.
> > --
> > selinux mailing list
> > selinux(a)lists.fedoraproject.org
> > https://admin.fedoraproject.org/mailman/listinfo/selinux
>
>  --
>  selinux mailing list
>  selinux(a)lists.fedoraproject.org
>  https://admin.fedoraproject.org/mailman/listinfo/selinux