Bug 1527522

I believe this is a too broad change. It would be better to introduce a new type for foreman-proxy service.

Indeed this opens doors to denial attacks. The proper way of fixing this is to make a patch into systemd so kill signal is no longer recognized as start/stop hook.

Alternatively, you can introduce a boolean flag that can be turned off by default (opt-in only) so we can turn this on in our deployment but users are still locked down.

We can do boolean for this. It makes sense to me.

I believe a boolean would give a false sense of higher security. I did some policy mining and found that:

There is already a rule:

allow logrotate_t domain : process signal ;

which essentially allows logrotate to send signals other than SIGKILL, SIGSTOP or SIGCHLD to virtually any other process (since all domain types should have domain attribute). So, well, adding systemd stopping would not be that bad (in fact only adding SIGKILL, SIGSTOP and SIGCHLD signals for systemd-managed services). :-)

In other words - logrotate scripts are already highly privileged with root user and wide permissions from SELinux.

(In reply to Lukas Vrabec from comment #4)
> We can do boolean for this. It makes sense to me.

Lukas, I also think some additional boolean tuning for logrotate would be very welcome in the policy.

I, too, am experiencing SELinux denials because of the relationship between cron and logrotate. In my case, crond_t transitions to system_cronjob_t by executing the following bin_t file:

~] ll -Z /etc/cron.daily/logrotate 
-rwx------. root root system_u:object_r:bin_t:s0       /etc/cron.daily/logrotate

See:
   allow system_cronjob_t bin_t : file { ioctl read getattr lock execute execute_no_trans entrypoint open } ; 
   allow crond_t system_cronjob_t : process transition ; 
   allow crond_t bin_t : file { ioctl read getattr lock execute execute_no_trans open } ; 

However, /etc/cron.daily/logrotate does call explicitely "/usr/sbin/logrotate /etc/logrotate.conf", the only logrotate_exec_t on my system btw:

~] find / -context "*logrotate_exec_t*"
/usr/sbin/logrotate

Both crond_t and system_cronjob_t can execute that:
   allow system_cronjob_t logrotate_exec_t : file { read getattr execute open } ; 
   allow crond_t logrotate_exec_t : file { read getattr execute open } ; 

...to transition into logrotate_t:
   allow logrotate_t logrotate_exec_t : file { ioctl read getattr lock execute execute_no_trans entrypoint open } ; 
   allow system_cronjob_t logrotate_t : process transition ; 
   allow crond_t logrotate_t : process transition ; 

/etc/logrotate.conf in turn includes config found in /etc/logrotate.d, among which a file with a "postrotate" instruction calling another script in my system, which in turn had to execute something with "execstack" (I know, that's bad, but I have no control over this).

Ah, but that is one thing logrotate_t cannot do when system_cronjob_t (and crond_t) can, look:

~] sesearch -s crond_t -AC | grep execstack
ET allow crond_t crond_t : process execstack ; [ selinuxuser_execstack ]
~] sesearch -s system_cronjob_t -AC | grep execstack
ET allow system_cronjob_t system_cronjob_t : process execstack ; [ selinuxuser_execstack ]
~] sesearch -s logrotate_t -AC | grep execstack
~]


So a solution for me was to edit /etc/cron.daily/logrotate and use runcon by replacing:
"/usr/sbin/logrotate /etc/logrotate.conf" with:
"runcon -t system_cronjob_t /usr/sbin/logrotate /etc/logrotate.conf"

Obviously I don't like having to use runcon in an executable in my centralized config management tool.

It would have been nice to have a boolean such as cron_logrotate_transition_disabled to prevent system_cronjob_t and crond_t to transition to logrotate_t (execute_no_trans?). Combined with the selinuxuser_execstack boolean, that would provide enough flexibility to solve most SELinux problems between cron and logrotate.

For the record, we already have cron_userdomain_transition:

~] sesearch -b cron_userdomain_transition -p transition -AC
Found 6 semantic av rules:
ET allow crond_t sysadm_t : process transition ; [ cron_userdomain_transition ]
ET allow crond_t unconfined_t : process transition ; [ cron_userdomain_transition ]
DF allow crond_t unconfined_cronjob_t : process transition ; [ cron_userdomain_transition ]
ET allow crond_t openshift_domain : process transition ; [ cron_userdomain_transition ]
ET allow crond_t staff_t : process transition ; [ cron_userdomain_transition ]
ET allow crond_t user_t : process transition ; [ cron_userdomain_transition ]


But that is not quite what I want.

So yeah, a cron_logrotate_transition_disabled boolean would be welcome.

(In reply to Radosław Piliszek from comment #5)
> I believe a boolean would give a false sense of higher security. I did some
> policy mining and found that:
> 
> There is already a rule:
> 
> allow logrotate_t domain : process signal ;
> 
> which essentially allows logrotate to send signals other than SIGKILL,
> SIGSTOP or SIGCHLD to virtually any other process (since all domain types
> should have domain attribute). So, well, adding systemd stopping would not
> be that bad (in fact only adding SIGKILL, SIGSTOP and SIGCHLD signals for
> systemd-managed services). :-)
> 
> In other words - logrotate scripts are already highly privileged with root
> user and wide permissions from SELinux.

Agree here, I don't see any real value to have this in boolean. I'll allow this access.

(In reply to Benjamin Lefoul from comment #6)
> (In reply to Lukas Vrabec from comment #4)
> > We can do boolean for this. It makes sense to me.
> 
> Lukas, I also think some additional boolean tuning for logrotate would be
> very welcome in the policy.
> 
> I, too, am experiencing SELinux denials because of the relationship between
> cron and logrotate. In my case, crond_t transitions to system_cronjob_t by
> executing the following bin_t file:
> 
> ~] ll -Z /etc/cron.daily/logrotate 
> -rwx------. root root system_u:object_r:bin_t:s0      
> /etc/cron.daily/logrotate
> 
> See:
>    allow system_cronjob_t bin_t : file { ioctl read getattr lock execute
> execute_no_trans entrypoint open } ; 
>    allow crond_t system_cronjob_t : process transition ; 
>    allow crond_t bin_t : file { ioctl read getattr lock execute
> execute_no_trans open } ; 
> 
> However, /etc/cron.daily/logrotate does call explicitely
> "/usr/sbin/logrotate /etc/logrotate.conf", the only logrotate_exec_t on my
> system btw:
> 
> ~] find / -context "*logrotate_exec_t*"
> /usr/sbin/logrotate
> 
> Both crond_t and system_cronjob_t can execute that:
>    allow system_cronjob_t logrotate_exec_t : file { read getattr execute
> open } ; 
>    allow crond_t logrotate_exec_t : file { read getattr execute open } ; 
> 
> ...to transition into logrotate_t:
>    allow logrotate_t logrotate_exec_t : file { ioctl read getattr lock
> execute execute_no_trans entrypoint open } ; 
>    allow system_cronjob_t logrotate_t : process transition ; 
>    allow crond_t logrotate_t : process transition ; 
> 
> /etc/logrotate.conf in turn includes config found in /etc/logrotate.d, among
> which a file with a "postrotate" instruction calling another script in my
> system, which in turn had to execute something with "execstack" (I know,
> that's bad, but I have no control over this).
> 
> Ah, but that is one thing logrotate_t cannot do when system_cronjob_t (and
> crond_t) can, look:
> 
> ~] sesearch -s crond_t -AC | grep execstack
> ET allow crond_t crond_t : process execstack ; [ selinuxuser_execstack ]
> ~] sesearch -s system_cronjob_t -AC | grep execstack
> ET allow system_cronjob_t system_cronjob_t : process execstack ; [
> selinuxuser_execstack ]
> ~] sesearch -s logrotate_t -AC | grep execstack
> ~]
> 
> 
> So a solution for me was to edit /etc/cron.daily/logrotate and use runcon by
> replacing:
> "/usr/sbin/logrotate /etc/logrotate.conf" with:
> "runcon -t system_cronjob_t /usr/sbin/logrotate /etc/logrotate.conf"
> 
> Obviously I don't like having to use runcon in an executable in my
> centralized config management tool.
> 
> It would have been nice to have a boolean such as
> cron_logrotate_transition_disabled to prevent system_cronjob_t and crond_t
> to transition to logrotate_t (execute_no_trans?). Combined with the
> selinuxuser_execstack boolean, that would provide enough flexibility to
> solve most SELinux problems between cron and logrotate.
> 
> For the record, we already have cron_userdomain_transition:
> 
> ~] sesearch -b cron_userdomain_transition -p transition -AC
> Found 6 semantic av rules:
> ET allow crond_t sysadm_t : process transition ; [
> cron_userdomain_transition ]
> ET allow crond_t unconfined_t : process transition ; [
> cron_userdomain_transition ]
> DF allow crond_t unconfined_cronjob_t : process transition ; [
> cron_userdomain_transition ]
> ET allow crond_t openshift_domain : process transition ; [
> cron_userdomain_transition ]
> ET allow crond_t staff_t : process transition ; [ cron_userdomain_transition
> ]
> ET allow crond_t user_t : process transition ; [ cron_userdomain_transition ]
> 
> 
> But that is not quite what I want.
> 
> So yeah, a cron_logrotate_transition_disabled boolean would be welcome.

Could you please create new bug related your issues? 
Thanks.

(In reply to Lukas Vrabec from comment #8)
> (In reply to Benjamin Lefoul from comment #6)
> > (In reply to Lukas Vrabec from comment #4)
> > > We can do boolean for this. It makes sense to me.
> > 
> > Lukas, I also think some additional boolean tuning for logrotate would be
> > very welcome in the policy.
> 
> Could you please create new bug related your issues? 
> Thanks.

Okay: https://bugzilla.redhat.com/show_bug.cgi?id=1627075

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2018:3111