diff mbox

seobject: Handle python error returns correctly

Message ID 1480520874-22034-1-git-send-email-kwalker@redhat.com (mailing list archive)
State Not Applicable
Headers show

Commit Message

Kyle Walker Nov. 30, 2016, 3:47 p.m. UTC
After 9406ace8 ("libsemanage: throw exceptions in python rather than
return NULL"), calls to libsemanage functions return Python exceptions
instead of returning negative error return codes. For systems that did not
have the applicable headers installed prior to build, the difference was
not seen. Following commit 9792099f ("Properly build the swig exception
file even if the headers are missing"), that issue has been resolved and
the underlying semanage_fcontext_query_local and semanage_fcontext_query
calls now result in an OSError return. This results in the following error
when attempting to modify a fcontext defined in the systems base policy.

    libsemanage.dbase_llist_query: could not query record value (No such file or directory).
    OSError: No such file or directory

To resolve the error, handle the OSError exception, but retain the
previous query operation.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1398427

Signed-off-by: Kyle Walker <kwalker@redhat.com>
---
 python/semanage/seobject.py | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Comments

Stephen Smalley Nov. 30, 2016, 4:05 p.m. UTC | #1
On 11/30/2016 10:47 AM, Kyle Walker wrote:
> After 9406ace8 ("libsemanage: throw exceptions in python rather than
> return NULL"), calls to libsemanage functions return Python exceptions
> instead of returning negative error return codes. For systems that did not
> have the applicable headers installed prior to build, the difference was
> not seen. Following commit 9792099f ("Properly build the swig exception
> file even if the headers are missing"), that issue has been resolved and
> the underlying semanage_fcontext_query_local and semanage_fcontext_query
> calls now result in an OSError return. This results in the following error
> when attempting to modify a fcontext defined in the systems base policy.
> 
>     libsemanage.dbase_llist_query: could not query record value (No such file or directory).
>     OSError: No such file or directory
> 
> To resolve the error, handle the OSError exception, but retain the
> previous query operation.
> 
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1398427

Hmm...even after this patch, we still get the error message from
dbase_llist_query.  Wondering if we should drop that or if that would
hide anything useful for other errors.  Doesn't seem overly useful at
present.

> 
> Signed-off-by: Kyle Walker <kwalker@redhat.com>
> ---
>  python/semanage/seobject.py | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/python/semanage/seobject.py b/python/semanage/seobject.py
> index bb049c0..5f5fdec 100644
> --- a/python/semanage/seobject.py
> +++ b/python/semanage/seobject.py
> @@ -1953,10 +1953,12 @@ class fcontextRecords(semanageRecords):
>              if not exists:
>                  raise ValueError(_("File context for %s is not defined") % target)
>  
> -        (rc, fcontext) = semanage_fcontext_query_local(self.sh, k)
> -        if rc < 0:
> -            (rc, fcontext) = semanage_fcontext_query(self.sh, k)
> -            if rc < 0:
> +        try:
> +            (rc, fcontext) = semanage_fcontext_query_local(self.sh, k)
> +        except OSError:
> +            try:
> +                (rc, fcontext) = semanage_fcontext_query(self.sh, k)
> +            except OSError:
>                  raise ValueError(_("Could not query file context for %s") % target)
>  
>          if setype != "<<none>>":
>
Kyle Walker Nov. 30, 2016, 4:48 p.m. UTC | #2
On Wed, Nov 30, 2016 at 11:05 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> Hmm...even after this patch, we still get the error message from
> dbase_llist_query.  Wondering if we should drop that or if that would
> hide anything useful for other errors.  Doesn't seem overly useful at
> present.

It does still emit that warning, it stems from the dbase_llist_query_local
not returning any entries and falling back to the dbase_llist_query. It is
chatty, and not necessarily helpful. I can't really think of a use-case for
it myself. Outside of being able to verify that a fcontext is not in the
file_contexts.local, but in the underlying file_contexts database on
modification.

I.E. If you see that message, you are modifying from the defaults. If you
don't, you are modifying a fcontext that has already been modified on the
system previously.
Stephen Smalley Nov. 30, 2016, 5:26 p.m. UTC | #3
On 11/30/2016 11:48 AM, Kyle Walker wrote:
> On Wed, Nov 30, 2016 at 11:05 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
>> Hmm...even after this patch, we still get the error message from
>> dbase_llist_query.  Wondering if we should drop that or if that would
>> hide anything useful for other errors.  Doesn't seem overly useful at
>> present.
> 
> It does still emit that warning, it stems from the dbase_llist_query_local
> not returning any entries and falling back to the dbase_llist_query. It is
> chatty, and not necessarily helpful. I can't really think of a use-case for
> it myself. Outside of being able to verify that a fcontext is not in the
> file_contexts.local, but in the underlying file_contexts database on
> modification.
> 
> I.E. If you see that message, you are modifying from the defaults. If you
> don't, you are modifying a fcontext that has already been modified on the
> system previously.

Yes, so I applied your patch, but I think it would make sense to drop
that message from libsemanage too.  Otherwise, users are likely to be
confused by the error message and think the change did not take effect.
Kyle Walker Nov. 30, 2016, 6:26 p.m. UTC | #4
On Wed, Nov 30, 2016 at 12:26 PM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> Yes, so I applied your patch, but I think it would make sense to drop
> that message from libsemanage too.  Otherwise, users are likely to be
> confused by the error message and think the change did not take effect.

Makes sense to me! I'll go ahead and put pulling that error it on my TODO
list for today or tomorrow.

Thanks!
diff mbox

Patch

diff --git a/python/semanage/seobject.py b/python/semanage/seobject.py
index bb049c0..5f5fdec 100644
--- a/python/semanage/seobject.py
+++ b/python/semanage/seobject.py
@@ -1953,10 +1953,12 @@  class fcontextRecords(semanageRecords):
             if not exists:
                 raise ValueError(_("File context for %s is not defined") % target)
 
-        (rc, fcontext) = semanage_fcontext_query_local(self.sh, k)
-        if rc < 0:
-            (rc, fcontext) = semanage_fcontext_query(self.sh, k)
-            if rc < 0:
+        try:
+            (rc, fcontext) = semanage_fcontext_query_local(self.sh, k)
+        except OSError:
+            try:
+                (rc, fcontext) = semanage_fcontext_query(self.sh, k)
+            except OSError:
                 raise ValueError(_("Could not query file context for %s") % target)
 
         if setype != "<<none>>":