From: pebenito@ieee.org (Chris PeBenito) Date: Tue, 19 Sep 2017 18:22:16 -0400 Subject: [refpolicy] [PATCH 1/1] Fix init_inherit_rlimit interface In-Reply-To: <1B50C12ACFF4CB42B90D2581155DF50205B78EEF@Exchange10.columbia.tresys.com> References: <1B50C12ACFF4CB42B90D2581155DF50205B72B50@Exchange10.columbia.tresys.com> <20170915154654.GC4441@julius.enp8s0.d30> <1B50C12ACFF4CB42B90D2581155DF50205B74B76@Exchange10.columbia.tresys.com> <20170915162819.GE4441@julius.enp8s0.d30> <1B50C12ACFF4CB42B90D2581155DF50205B78EEF@Exchange10.columbia.tresys.com> Message-ID: To: refpolicy@oss.tresys.com List-Id: refpolicy.oss.tresys.com On 09/18/2017 08:20 AM, David Sugar via refpolicy wrote: > > >> -----Original Message----- >> From: refpolicy-bounces at oss.tresys.com [mailto:refpolicy- >> bounces at oss.tresys.com] On Behalf Of Chris PeBenito via refpolicy >> Sent: Saturday, September 16, 2017 1:01 PM >> To: refpolicy at oss.tresys.com >> Subject: Re: [refpolicy] [PATCH 1/1] Fix init_inherit_rlimit interface >> >> 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. > > I'm happy to submit a different patch removing the interface I added and altering init_daemon/system_domain/init_spec_domtrans () to grant this permission by default. I can also alter the interfaces for initrc_t in the same manner. That works. > There are other things that have been discussed (changes to cron) which I wouldn't put in as I don't have anything to verify those changes against right now. These aren't as clear, so I'll pass for now. -- Chris PeBenito