* [PATCH] chcat: don't crash if access to binary policy is prohibited
@ 2020-05-09 14:06 bauen1
2020-05-10 17:25 ` Nicolas Iooss
0 siblings, 1 reply; 6+ messages in thread
From: bauen1 @ 2020-05-09 14:06 UTC (permalink / raw)
To: selinux
sobject will crash if access to the binary policy is prohibited by
selinux, e.g. refpolicy
this also breaks file operations that don't require seobject.
Signed-off-by: bauen1 <j2468h@gmail.com>
---
python/chcat/chcat | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/python/chcat/chcat b/python/chcat/chcat
index fdd2e46e..55408577 100755
--- a/python/chcat/chcat
+++ b/python/chcat/chcat
@@ -28,7 +28,6 @@ import os
import pwd
import getopt
import selinux
-import seobject
PROGNAME = "policycoreutils"
try:
@@ -65,6 +64,7 @@ def verify_users(users):
def chcat_user_add(newcat, users):
+ import seobject
errors = 0
logins = seobject.loginRecords()
seusers = logins.get_all()
@@ -144,6 +144,7 @@ def chcat_add(orig, newcat, objects, login_ind):
def chcat_user_remove(newcat, users):
+ import seobject
errors = 0
logins = seobject.loginRecords()
seusers = logins.get_all()
@@ -233,6 +234,7 @@ def chcat_remove(orig, newcat, objects, login_ind):
def chcat_user_replace(newcat, users):
+ import seobject
errors = 0
logins = seobject.loginRecords()
seusers = logins.get_all()
@@ -376,6 +378,7 @@ def listcats():
def listusercats(users):
+ import seobject
if len(users) == 0:
try:
users.append(os.getlogin())
--
2.26.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] chcat: don't crash if access to binary policy is prohibited
2020-05-09 14:06 [PATCH] chcat: don't crash if access to binary policy is prohibited bauen1
@ 2020-05-10 17:25 ` Nicolas Iooss
2020-05-29 13:16 ` Stephen Smalley
0 siblings, 1 reply; 6+ messages in thread
From: Nicolas Iooss @ 2020-05-10 17:25 UTC (permalink / raw)
To: bauen1, SElinux list
On Sat, May 9, 2020 at 4:06 PM bauen1 <j2468h@googlemail.com> wrote:
>
> sobject will crash if access to the binary policy is prohibited by
> selinux, e.g. refpolicy
> this also breaks file operations that don't require seobject.
>
> Signed-off-by: bauen1 <j2468h@gmail.com>
Hello,
This patch looks very hackish. In fact, an underlying issue that
exists with seobject is that "import seobject" raises an exception
when it is used from an environment that is not allowed to read the
policy:
>>> import seobject
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/usr/lib/python3.8/site-packages/seobject.py", line 33, in <module>
import sepolicy
File "/usr/lib/python3.8/site-packages/sepolicy/__init__.py", line
186, in <module>
raise e
File "/usr/lib/python3.8/site-packages/sepolicy/__init__.py", line
182, in <module>
policy_file = get_installed_policy()
File "/usr/lib/python3.8/site-packages/sepolicy/__init__.py", line
137, in get_installed_policy
raise ValueError(_("No SELinux Policy installed"))
ValueError: No SELinux Policy installed
Is this the issue you encountered when you write "seobject will crash"?
In my humble opinion, trying to hide such an issue by moving "import
seobject" makes maintaining the project more difficult. I would prefer
seeing a way to allow using "import seobject" without raising
exceptions, but working on this is unfortunately quite time-consuming
(I have not seen a straightforward way to deal with this, and there
exist several ways to solve this in not-very-direct ways, for example
with lazy loading of the policy when needed or with replacing some API
with stub functions if the policy cannot be loaded).
Therefore I will not ack this patch, but I will not block ("Nack") it
if another maintainer wants to include it.
Thanks,
Nicolas
> ---
> python/chcat/chcat | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/python/chcat/chcat b/python/chcat/chcat
> index fdd2e46e..55408577 100755
> --- a/python/chcat/chcat
> +++ b/python/chcat/chcat
> @@ -28,7 +28,6 @@ import os
> import pwd
> import getopt
> import selinux
> -import seobject
>
> PROGNAME = "policycoreutils"
> try:
> @@ -65,6 +64,7 @@ def verify_users(users):
>
>
> def chcat_user_add(newcat, users):
> + import seobject
> errors = 0
> logins = seobject.loginRecords()
> seusers = logins.get_all()
> @@ -144,6 +144,7 @@ def chcat_add(orig, newcat, objects, login_ind):
>
>
> def chcat_user_remove(newcat, users):
> + import seobject
> errors = 0
> logins = seobject.loginRecords()
> seusers = logins.get_all()
> @@ -233,6 +234,7 @@ def chcat_remove(orig, newcat, objects, login_ind):
>
>
> def chcat_user_replace(newcat, users):
> + import seobject
> errors = 0
> logins = seobject.loginRecords()
> seusers = logins.get_all()
> @@ -376,6 +378,7 @@ def listcats():
>
>
> def listusercats(users):
> + import seobject
> if len(users) == 0:
> try:
> users.append(os.getlogin())
> --
> 2.26.2
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] chcat: don't crash if access to binary policy is prohibited
2020-05-10 17:25 ` Nicolas Iooss
@ 2020-05-29 13:16 ` Stephen Smalley
2021-02-17 21:16 ` [PATCH v2] chcat: allow usage if binary policy is inaccessible bauen1
0 siblings, 1 reply; 6+ messages in thread
From: Stephen Smalley @ 2020-05-29 13:16 UTC (permalink / raw)
To: Nicolas Iooss; +Cc: bauen1, SElinux list
On Sun, May 10, 2020 at 1:26 PM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
>
> On Sat, May 9, 2020 at 4:06 PM bauen1 <j2468h@googlemail.com> wrote:
> >
> > sobject will crash if access to the binary policy is prohibited by
> > selinux, e.g. refpolicy
> > this also breaks file operations that don't require seobject.
> >
> > Signed-off-by: bauen1 <j2468h@gmail.com>
>
> Hello,
> This patch looks very hackish. In fact, an underlying issue that
> exists with seobject is that "import seobject" raises an exception
> when it is used from an environment that is not allowed to read the
> policy:
>
> >>> import seobject
> Traceback (most recent call last):
> File "<stdin>", line 1, in <module>
> File "/usr/lib/python3.8/site-packages/seobject.py", line 33, in <module>
> import sepolicy
> File "/usr/lib/python3.8/site-packages/sepolicy/__init__.py", line
> 186, in <module>
> raise e
> File "/usr/lib/python3.8/site-packages/sepolicy/__init__.py", line
> 182, in <module>
> policy_file = get_installed_policy()
> File "/usr/lib/python3.8/site-packages/sepolicy/__init__.py", line
> 137, in get_installed_policy
> raise ValueError(_("No SELinux Policy installed"))
> ValueError: No SELinux Policy installed
>
> Is this the issue you encountered when you write "seobject will crash"?
>
> In my humble opinion, trying to hide such an issue by moving "import
> seobject" makes maintaining the project more difficult. I would prefer
> seeing a way to allow using "import seobject" without raising
> exceptions, but working on this is unfortunately quite time-consuming
> (I have not seen a straightforward way to deal with this, and there
> exist several ways to solve this in not-very-direct ways, for example
> with lazy loading of the policy when needed or with replacing some API
> with stub functions if the policy cannot be loaded).
>
> Therefore I will not ack this patch, but I will not block ("Nack") it
> if another maintainer wants to include it.
I'm not opposed to the patch itself (I assume the current code breaks
usage of chcat under MLS policy by regular users who lack access to
the policy file), but your Signed-off-by line ought to be revised to
contain your real name. Otherwise, it doesn't really serve its
purpose. See the discussion of Signed-off-by in
https://www.kernel.org/doc/html/latest/process/submitting-patches.html.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] chcat: allow usage if binary policy is inaccessible
2020-05-29 13:16 ` Stephen Smalley
@ 2021-02-17 21:16 ` bauen1
2021-02-22 18:27 ` Petr Lautrbach
0 siblings, 1 reply; 6+ messages in thread
From: bauen1 @ 2021-02-17 21:16 UTC (permalink / raw)
To: selinux; +Cc: bauen1
Currently, chcat will crash when run as regular user, because import
sepolicy throws an Exception when failing to access the binary policy
under /etc/selinux/${POLICYNAME}/policy/ which is inaccessible to
regular users.
Signed-off-by: Jonathan Hettwer <j2468h@gmail.com>
---
v2:
Fix signed-off-by, improve commit message, but otherwise unchanged
python/chcat/chcat | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/python/chcat/chcat b/python/chcat/chcat
index fdd2e46e..55408577 100755
--- a/python/chcat/chcat
+++ b/python/chcat/chcat
@@ -28,7 +28,6 @@ import os
import pwd
import getopt
import selinux
-import seobject
PROGNAME = "policycoreutils"
try:
@@ -65,6 +64,7 @@ def verify_users(users):
def chcat_user_add(newcat, users):
+ import seobject
errors = 0
logins = seobject.loginRecords()
seusers = logins.get_all()
@@ -144,6 +144,7 @@ def chcat_add(orig, newcat, objects, login_ind):
def chcat_user_remove(newcat, users):
+ import seobject
errors = 0
logins = seobject.loginRecords()
seusers = logins.get_all()
@@ -233,6 +234,7 @@ def chcat_remove(orig, newcat, objects, login_ind):
def chcat_user_replace(newcat, users):
+ import seobject
errors = 0
logins = seobject.loginRecords()
seusers = logins.get_all()
@@ -376,6 +378,7 @@ def listcats():
def listusercats(users):
+ import seobject
if len(users) == 0:
try:
users.append(os.getlogin())
--
2.30.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] chcat: allow usage if binary policy is inaccessible
2021-02-17 21:16 ` [PATCH v2] chcat: allow usage if binary policy is inaccessible bauen1
@ 2021-02-22 18:27 ` Petr Lautrbach
2021-02-22 20:33 ` bauen1
0 siblings, 1 reply; 6+ messages in thread
From: Petr Lautrbach @ 2021-02-22 18:27 UTC (permalink / raw)
To: bauen1, selinux
bauen1 <j2468h@googlemail.com> writes:
> Currently, chcat will crash when run as regular user, because import
> sepolicy throws an Exception when failing to access the binary policy
> under /etc/selinux/${POLICYNAME}/policy/ which is inaccessible to
> regular users.
>
I'd rather follow Nicolas suggestion so I've prepared a patch, see
below, which moves the policy initialization in sepolicy module before
it's used for the first time. It seems to solve the same problem in more
generic way. I need to run some tests on that and then they pass I'll
propose it here on the mailing list.
https://github.com/bachradsusi/SELinuxProject-selinux/commit/6a12939b613b273a6e96e1cc4cc096cdf7db5ac6
--- a/python/sepolicy/sepolicy/__init__.py
+++ b/python/sepolicy/sepolicy/__init__.py
@@ -178,15 +178,14 @@ def load_store_policy(store):
return None
policy(policy_file)
-try:
+def init_policy():
policy_file = get_installed_policy()
policy(policy_file)
-except ValueError as e:
- if selinux.is_selinux_enabled() == 1:
- raise e
-
def info(setype, name=None):
+ if not _pol:
+ init_policy()
+
if setype == TYPE:
q = setools.TypeQuery(_pol)
q.name = name
@@ -337,6 +336,8 @@ def _setools_rule_to_dict(rule):
def search(types, seinfo=None):
+ if not _pol:
+ init_policy()
if not seinfo:
seinfo = {}
valid_types = set([ALLOW, AUDITALLOW, NEVERALLOW, DONTAUDIT, TRANSITION, ROLE_ALLOW])
> Signed-off-by: Jonathan Hettwer <j2468h@gmail.com>
> ---
>
> v2:
> Fix signed-off-by, improve commit message, but otherwise unchanged
>
> python/chcat/chcat | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/python/chcat/chcat b/python/chcat/chcat
> index fdd2e46e..55408577 100755
> --- a/python/chcat/chcat
> +++ b/python/chcat/chcat
> @@ -28,7 +28,6 @@ import os
> import pwd
> import getopt
> import selinux
> -import seobject
>
> PROGNAME = "policycoreutils"
> try:
> @@ -65,6 +64,7 @@ def verify_users(users):
>
>
> def chcat_user_add(newcat, users):
> + import seobject
> errors = 0
> logins = seobject.loginRecords()
> seusers = logins.get_all()
> @@ -144,6 +144,7 @@ def chcat_add(orig, newcat, objects, login_ind):
>
>
> def chcat_user_remove(newcat, users):
> + import seobject
> errors = 0
> logins = seobject.loginRecords()
> seusers = logins.get_all()
> @@ -233,6 +234,7 @@ def chcat_remove(orig, newcat, objects, login_ind):
>
>
> def chcat_user_replace(newcat, users):
> + import seobject
> errors = 0
> logins = seobject.loginRecords()
> seusers = logins.get_all()
> @@ -376,6 +378,7 @@ def listcats():
>
>
> def listusercats(users):
> + import seobject
> if len(users) == 0:
> try:
> users.append(os.getlogin())
> --
> 2.30.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] chcat: allow usage if binary policy is inaccessible
2021-02-22 18:27 ` Petr Lautrbach
@ 2021-02-22 20:33 ` bauen1
0 siblings, 0 replies; 6+ messages in thread
From: bauen1 @ 2021-02-22 20:33 UTC (permalink / raw)
To: Petr Lautrbach, bauen1, selinux
On 2/22/21 7:27 PM, Petr Lautrbach wrote:
> bauen1 <j2468h@googlemail.com> writes:
>
>> Currently, chcat will crash when run as regular user, because import
>> sepolicy throws an Exception when failing to access the binary policy
>> under /etc/selinux/${POLICYNAME}/policy/ which is inaccessible to
>> regular users.
>>
>
> I'd rather follow Nicolas suggestion so I've prepared a patch, see
> below, which moves the policy initialization in sepolicy module before
> it's used for the first time. It seems to solve the same problem in more
> generic way. I need to run some tests on that and then they pass I'll
> propose it here on the mailing list.
>
Yes, this is a much better approach.
--
bauen1
https://dn42.bauen1.xyz/
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-02-22 20:34 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-09 14:06 [PATCH] chcat: don't crash if access to binary policy is prohibited bauen1
2020-05-10 17:25 ` Nicolas Iooss
2020-05-29 13:16 ` Stephen Smalley
2021-02-17 21:16 ` [PATCH v2] chcat: allow usage if binary policy is inaccessible bauen1
2021-02-22 18:27 ` Petr Lautrbach
2021-02-22 20:33 ` bauen1
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.