From: dsugar@tresys.com (David Sugar) Date: Mon, 18 Sep 2017 12:20:32 +0000 Subject: [refpolicy] [PATCH 1/1] Fix init_inherit_rlimit interface In-Reply-To: References: <1B50C12ACFF4CB42B90D2581155DF50205B72B50@Exchange10.columbia.tresys.com> <20170915154654.GC4441@julius.enp8s0.d30> <1B50C12ACFF4CB42B90D2581155DF50205B74B76@Exchange10.columbia.tresys.com> <20170915162819.GE4441@julius.enp8s0.d30> Message-ID: <1B50C12ACFF4CB42B90D2581155DF50205B78EEF@Exchange10.columbia.tresys.com> To: refpolicy@oss.tresys.com List-Id: refpolicy.oss.tresys.com > -----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. 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. Opinions? Dave > > -- > Chris PeBenito > _______________________________________________ > refpolicy mailing list > refpolicy at oss.tresys.com > http://oss.tresys.com/mailman/listinfo/refpolicy