From: pebenito@ieee.org (Chris PeBenito) Date: Sat, 16 Sep 2017 13:01:08 -0400 Subject: [refpolicy] [PATCH 1/1] Fix init_inherit_rlimit interface In-Reply-To: <20170915162819.GE4441@julius.enp8s0.d30> References: <1B50C12ACFF4CB42B90D2581155DF50205B72B50@Exchange10.columbia.tresys.com> <20170915154654.GC4441@julius.enp8s0.d30> <1B50C12ACFF4CB42B90D2581155DF50205B74B76@Exchange10.columbia.tresys.com> <20170915162819.GE4441@julius.enp8s0.d30> Message-ID: To: refpolicy@oss.tresys.com List-Id: refpolicy.oss.tresys.com On 09/15/2017 12:28 PM, Dominick Grift via refpolicy wrote: > On Fri, Sep 15, 2017 at 04:11:30PM +0000, David Sugar via refpolicy wrote: >> >> >>> -----Original Message----- >>> From: refpolicy-bounces at oss.tresys.com [mailto:refpolicy- >>> bounces at oss.tresys.com] On Behalf Of Dominick Grift via refpolicy >>> Sent: Friday, September 15, 2017 11:47 AM >>> To: refpolicy at oss.tresys.com >>> Subject: Re: [refpolicy] [PATCH 1/1] Fix init_inherit_rlimit interface >>> >>> On Fri, Sep 15, 2017 at 03:35:28PM +0000, David Sugar via refpolicy >>> wrote: >>>> My previous patch to add this interface got the order of arguments in >>> allow rule backwards. This fixes that problem. >>>> >>>> Signed-off-by: Dave Sugar >>>> --- >>>> policy/modules/system/init.if | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/policy/modules/system/init.if >>>> b/policy/modules/system/init.if index 303bd067..cb0fabcd 100644 >>>> --- a/policy/modules/system/init.if >>>> +++ b/policy/modules/system/init.if >>>> @@ -732,7 +732,7 @@ interface(`init_inherit_rlimit',` >>>> type init_t; >>>> ') >>>> >>>> - allow $1 init_t:process rlimitinh; >>>> + allow init_t $1:process rlimitinh; >>>> ') >>>> >>>> ######################################## >>> >>> Have you considered instead adding this to the >>> init_daemon/system_domain() interfaces (and probably that >>> init_spec_domtrans interfae you added recently? >>> >>> If youre going to require that this be allowed on a case-by-case then >>> this will be very hard to indentify for a lot of people >>> >>> Also this should probably be added for initrc_t: "allow init_t >>> initrc_t:process rlimitinh;", by the way probably also setsched for >>> Nice= >>> >>> By the way: and i am not sure about this but: we need to make sure that >>> crond_t can set rlimitinh of user shells if it runs the cronjobs in the >>> user domain. >>> because we wouldnt want users to be able to bypass their resource limits >>> by running a cronjob. (ie. are the resource limits properly set for user >>> cronjobs?) >>> >>> to test that: >>> >>> echo "joe soft nproc 500" >> /etc/security/limits.conf echo "joe hard >>> nproc 1000" >> /etc/security/limits.conf crontab -e >>> *\1 * * * * ulimit -S -u >>> *\1 * * * * ulimit -H -u >>> :wq! >>> >> >> This is a good point. I hadn't really thought about it too much. In the past this permission was dontaudit'ed and I'm guessing most people didn't have the need to actually inherit resource limits. Knowing that systemd has an easy way to set these resource limits for the process being launched it might make sense to include this in the init_daemon/system_domain/init_spec_domtrans () interfaces so anyone can use them without the pain of figuring out why things are not working. >> >> Is that too much permission for most processes? Am I writing something here that is a bit of an exception? > > I think it would be an reasonable compromise. Especially compared to what Fedora has where it allows init_t to rlimitinh all domains instead of just domains actually started by it > > Maybe Chris can chime in on this When the patch was first posted, I had thought of this, but didn't conclude if it was ok. I'm open to this, since it's not unreasonable for the init program to set the limits of its direct children. -- Chris PeBenito