* [refpolicy] [PATCH 1/1] Fix init_inherit_rlimit interface
@ 2017-09-15 15:35 David Sugar
2017-09-15 15:46 ` Dominick Grift
0 siblings, 1 reply; 9+ messages in thread
From: David Sugar @ 2017-09-15 15:35 UTC (permalink / raw)
To: refpolicy
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 <dsugar@tresys.com>
---
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;
')
########################################
--
2.13.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [refpolicy] [PATCH 1/1] Fix init_inherit_rlimit interface
2017-09-15 15:35 [refpolicy] [PATCH 1/1] Fix init_inherit_rlimit interface David Sugar
@ 2017-09-15 15:46 ` Dominick Grift
2017-09-15 16:11 ` David Sugar
0 siblings, 1 reply; 9+ messages in thread
From: Dominick Grift @ 2017-09-15 15:46 UTC (permalink / raw)
To: refpolicy
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 <dsugar@tresys.com>
> ---
> 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!
> --
> 2.13.5
> _______________________________________________
> refpolicy mailing list
> refpolicy at oss.tresys.com
> http://oss.tresys.com/mailman/listinfo/refpolicy
--
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
Url : http://oss.tresys.com/pipermail/refpolicy/attachments/20170915/a39b1c6a/attachment.bin
^ permalink raw reply [flat|nested] 9+ messages in thread
* [refpolicy] [PATCH 1/1] Fix init_inherit_rlimit interface
2017-09-15 15:46 ` Dominick Grift
@ 2017-09-15 16:11 ` David Sugar
2017-09-15 16:28 ` Dominick Grift
2017-09-15 17:16 ` Dominick Grift
0 siblings, 2 replies; 9+ messages in thread
From: David Sugar @ 2017-09-15 16:11 UTC (permalink / raw)
To: refpolicy
> -----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 <dsugar@tresys.com>
> > ---
> > 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?
> > --
> > 2.13.5
> > _______________________________________________
> > refpolicy mailing list
> > refpolicy at oss.tresys.com
> > http://oss.tresys.com/mailman/listinfo/refpolicy
>
> --
> Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
> https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
> Dominick Grift
^ permalink raw reply [flat|nested] 9+ messages in thread
* [refpolicy] [PATCH 1/1] Fix init_inherit_rlimit interface
2017-09-15 16:11 ` David Sugar
@ 2017-09-15 16:28 ` Dominick Grift
2017-09-16 17:01 ` Chris PeBenito
2017-09-15 17:16 ` Dominick Grift
1 sibling, 1 reply; 9+ messages in thread
From: Dominick Grift @ 2017-09-15 16:28 UTC (permalink / raw)
To: refpolicy
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 <dsugar@tresys.com>
> > > ---
> > > 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
>
> > > --
> > > 2.13.5
> > > _______________________________________________
> > > refpolicy mailing list
> > > refpolicy at oss.tresys.com
> > > http://oss.tresys.com/mailman/listinfo/refpolicy
> >
> > --
> > Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
> > https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
> > Dominick Grift
> _______________________________________________
> refpolicy mailing list
> refpolicy at oss.tresys.com
> http://oss.tresys.com/mailman/listinfo/refpolicy
--
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
Url : http://oss.tresys.com/pipermail/refpolicy/attachments/20170915/603a8182/attachment.bin
^ permalink raw reply [flat|nested] 9+ messages in thread
* [refpolicy] [PATCH 1/1] Fix init_inherit_rlimit interface
2017-09-15 16:11 ` David Sugar
2017-09-15 16:28 ` Dominick Grift
@ 2017-09-15 17:16 ` Dominick Grift
1 sibling, 0 replies; 9+ messages in thread
From: Dominick Grift @ 2017-09-15 17:16 UTC (permalink / raw)
To: refpolicy
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 <dsugar@tresys.com>
> > > ---
> > > 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!
https://github.com/TresysTechnology/refpolicy-contrib/blob/master/cron.if#L80
that should probably instead be:
dontaudit crond_t $2:process { noatsecure siginh };
allow crond_t $2:process rlimitinh;
https://github.com/TresysTechnology/refpolicy-contrib/blob/master/cron.te#L261
that should probably be:
dontaudit crond_t system_cronjob_t:process { noatsecure siginh rlimitinh };
dontaudit crond_t cronjob_t:process { noatsecure siginh };
allow crond_t cronjob_t:process rlimitinh;
https://github.com/TresysTechnology/refpolicy-contrib/blob/master/cron.te#L224
> >
>
> 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?
>
> > > --
> > > 2.13.5
> > > _______________________________________________
> > > refpolicy mailing list
> > > refpolicy at oss.tresys.com
> > > http://oss.tresys.com/mailman/listinfo/refpolicy
> >
> > --
> > Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
> > https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
> > Dominick Grift
> _______________________________________________
> refpolicy mailing list
> refpolicy at oss.tresys.com
> http://oss.tresys.com/mailman/listinfo/refpolicy
--
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
Url : http://oss.tresys.com/pipermail/refpolicy/attachments/20170915/c5f357c6/attachment.bin
^ permalink raw reply [flat|nested] 9+ messages in thread
* [refpolicy] [PATCH 1/1] Fix init_inherit_rlimit interface
2017-09-15 16:28 ` Dominick Grift
@ 2017-09-16 17:01 ` Chris PeBenito
2017-09-18 12:20 ` David Sugar
0 siblings, 1 reply; 9+ messages in thread
From: Chris PeBenito @ 2017-09-16 17:01 UTC (permalink / raw)
To: refpolicy
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 <dsugar@tresys.com>
>>>> ---
>>>> 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [refpolicy] [PATCH 1/1] Fix init_inherit_rlimit interface
2017-09-16 17:01 ` Chris PeBenito
@ 2017-09-18 12:20 ` David Sugar
2017-09-18 12:33 ` Dominick Grift
2017-09-19 22:22 ` Chris PeBenito
0 siblings, 2 replies; 9+ messages in thread
From: David Sugar @ 2017-09-18 12:20 UTC (permalink / raw)
To: refpolicy
> -----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 <dsugar@tresys.com>
> >>>> ---
> >>>> 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [refpolicy] [PATCH 1/1] Fix init_inherit_rlimit interface
2017-09-18 12:20 ` David Sugar
@ 2017-09-18 12:33 ` Dominick Grift
2017-09-19 22:22 ` Chris PeBenito
1 sibling, 0 replies; 9+ messages in thread
From: Dominick Grift @ 2017-09-18 12:33 UTC (permalink / raw)
To: refpolicy
On Mon, Sep 18, 2017 at 12:20:32PM +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 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 <dsugar@tresys.com>
> > >>>> ---
> > >>>> 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?
that cron stuff was just a "BTW", besides it is kind of unrelated so just ignore that part (maybe some one else feels up to that task and has a stake in it?)
>
> Dave
> >
> > --
> > Chris PeBenito
> > _______________________________________________
> > refpolicy mailing list
> > refpolicy at oss.tresys.com
> > http://oss.tresys.com/mailman/listinfo/refpolicy
> _______________________________________________
> refpolicy mailing list
> refpolicy at oss.tresys.com
> http://oss.tresys.com/mailman/listinfo/refpolicy
--
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
Url : http://oss.tresys.com/pipermail/refpolicy/attachments/20170918/2f21c7b3/attachment.bin
^ permalink raw reply [flat|nested] 9+ messages in thread
* [refpolicy] [PATCH 1/1] Fix init_inherit_rlimit interface
2017-09-18 12:20 ` David Sugar
2017-09-18 12:33 ` Dominick Grift
@ 2017-09-19 22:22 ` Chris PeBenito
1 sibling, 0 replies; 9+ messages in thread
From: Chris PeBenito @ 2017-09-19 22:22 UTC (permalink / raw)
To: refpolicy
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 <dsugar@tresys.com>
>>>>>> ---
>>>>>> 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
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-09-19 22:22 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-15 15:35 [refpolicy] [PATCH 1/1] Fix init_inherit_rlimit interface David Sugar
2017-09-15 15:46 ` Dominick Grift
2017-09-15 16:11 ` David Sugar
2017-09-15 16:28 ` Dominick Grift
2017-09-16 17:01 ` Chris PeBenito
2017-09-18 12:20 ` David Sugar
2017-09-18 12:33 ` Dominick Grift
2017-09-19 22:22 ` Chris PeBenito
2017-09-15 17:16 ` Dominick Grift
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.