Re: [Xen-devel] [PATCH 11/15] flask: improve unknown permission handling

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Fri, Jun 17, 2016 at 01:02:58PM -0400, Daniel De Graaf wrote:
> On 06/17/2016 11:45 AM, Konrad Rzeszutek Wilk wrote:
> >On Thu, Jun 09, 2016 at 10:47:14AM -0400, Daniel De Graaf wrote:
> >>When an unknown domctl, sysctl, or other operation is encountered in the
> >>FLASK security server, use the allow_unknown bit in the security policy
> >>to decide if the permission should be allowed or denied.  This bit is
> >>off by default, but it can be set by using checkpolicy -U allow when
> >>compiling the policy.  This allows new operations to be tested without
> >>needing to immediately add security checks; however, it is not flexible
> >>enough to avoid adding the actual permission checks.  An error message
> >>is printed to the hypervisor console when this fallback is encountered.
> >
> >.. and the operation is permitted.
> 
> The error message is printed either way (with a different priority).  Were

correct.
> you suggesting I expand this explanation to include both the error and
> warning messages separately?

It just that the patch changes the behavior. That is in the past if
you had created an policy using checkpolicy -U allow it would print an
error and return -EPERM.

But now it will print an error and return 0 and pass the XSM check 
(aka operation ends being permitted).

> 
> >>
> >>Signed-off-by: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
> >>---
> >> xen/xsm/flask/hooks.c            | 44 
> >> +++++++++++++++++++++++++---------------
> >> xen/xsm/flask/include/security.h |  2 ++
> >> xen/xsm/flask/ss/policydb.c      |  1 +
> >> xen/xsm/flask/ss/policydb.h      |  6 ++++++
> >> xen/xsm/flask/ss/services.c      |  5 +++++
> >> 5 files changed, 42 insertions(+), 16 deletions(-)
> >>
> >>diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
> >>index a8d45e7..3ab3fbf 100644
> >>--- a/xen/xsm/flask/hooks.c
> >>+++ b/xen/xsm/flask/hooks.c
> >>@@ -136,6 +136,23 @@ static int get_irq_sid(int irq, u32 *sid, struct 
> >>avc_audit_data *ad)
> >>     return 0;
> >> }
> >>
> >>+static int avc_unknown_permission(const char *name, int id)
> >>+{
> >>+    int rc;
> >
> >I would add a new line here.
> 
> OK
> 
> >>+    if ( !flask_enforcing || security_get_allow_unknown() )
> >>+    {
> >>+        printk(XENLOG_G_WARNING "FLASK: Allowing unknown %s: %d.\n", name, 
> >>id);
> >>+        rc = 0;
> >>+    }
> >>+    else
> >>+    {
> >>+        printk(XENLOG_G_ERR "FLASK: Denying unknown %s: %d.\n", name, id);
> >>+        rc = -EPERM;
> >>+    }
> >>+
> >>+    return rc;
> >>+}
> >>+
> >
> >The rest looks OK, but I have a question: Is this how Linux operates?
> 
> Yes; selinux_nlmsg_perm for an unknown netlink message seems to be an
> example there.
> 
> -- 
> Daniel De Graaf
> National Security Agency

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel