Re: [v2] [PATCH 1/1] XSELinux: When SELinux is enabled the xserver seg faults

2012-07-05 Thread Richard Haines
Ok - I'll send an updated XSELinux patch [V3] as that will fix the crash 
plus a separate patch for the dix grab.

I don't know the code that well but there are probably other places that
may need checking.

Richard

--- On Thu, 5/7/12, Peter Hutterer peter.hutte...@who-t.net wrote:

 
 thanks for the update. The second part I'm not quite happy
 yet, I think it's
 the wrong approach. With passive grabs on
 XIAll(Master)Devices, the grab
 will activate on whichever device matches the grab. So if
 you issue such a
 grab and device 2 clicks, you'll get a grab on device 2. If
 device 3 clicks,
 you get a grab on device 3.
 
 For the SELinux case where some devices may not be permitted
 I think we
 should leave open the possibility that _some_ device in the
 future may be
 permitted. This means always return success from a
 XIAll*Devices passive
 grab request but then check for permissions when the grab
 actually
 activates.
 
 So the example above, if device 2 is restricted you'll be
 able to register a
 passive grab but it won't trigger for device 2, only for
 device 3.
 This needs some extra code in the dix, CheckPassiveGrab to
 be precise to
 return false for restricted devices.
 
 Cheers,
   Peter
 ___
 xorg-devel@lists.x.org:
 X.Org development
 Archives: http://lists.x.org/archives/xorg-devel
 Info: http://lists.x.org/mailman/listinfo/xorg-devel
 
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[V3] [PATCH 1/1] XSELinux: When SELinux is enabled the xserver seg faults

2012-07-05 Thread Richard Haines
This patch was created using xorg-server-1.12.2 source.

When using Fedora 17 with xorg-server-1.12.2 and SELinux is enabled
('setsebool xserver_object_manager on') the xserver will not load. The 
Xlog file has a seg fault pointing to XACE/SELinux. Bug 50641 was raised
(https://bugs.freedesktop.org/show_bug.cgi?id=50641).

The bug is caused by X calling XaceHook(XACE_DEVICE_ACCESS, client, ...)
with a device ID of '1' that is XIAllMasterDevices. It would also happen if
the device ID = 0 (XIAllDevices).

The fix is to return if device is XIAll*Devices and let the permission be
determined when a real device ID is presented.

Signed-off-by: Richard Haines richard_c_hai...@btinternet.com
---
 Xext/xselinux_hooks.c |   10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/Xext/xselinux_hooks.c b/Xext/xselinux_hooks.c
index e9c7e93..82d3892 100644
--- a/Xext/xselinux_hooks.c
+++ b/Xext/xselinux_hooks.c
@@ -336,7 +336,15 @@ SELinuxDevice(CallbackListPtr *pcbl, pointer unused, 
pointer calldata)
 int rc;
 
 subj = dixLookupPrivate(rec-client-devPrivates, subjectKey);
-obj = dixLookupPrivate(rec-dev-devPrivates, objectKey);
+/*
+ * The XIAllMasterDevices or XIAllDevices do not have devPrivates
+ * entries. If they are requested we just return as each device access
+ * will be checked individually.
+ */
+if (rec-dev-id != XIAllDevices  rec-dev-id != XIAllMasterDevices)
+obj = dixLookupPrivate(rec-dev-devPrivates, objectKey);
+else 
+return;
 
 /* If this is a new object that needs labeling, do it now */
 if (rec-access_mode  DixCreateAccess) {
-- 
1.7.10.4

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PATCH 1/1] dix: Check whether grab allowed by security provider

2012-07-05 Thread Richard Haines
This patch was created using xorg-server-1.12.2 source.

Call XACE to verify if grab access is allowed.

Signed-off-by: Richard Haines richard_c_hai...@btinternet.com
---
 dix/events.c |6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/dix/events.c b/dix/events.c
index 86336fe..0ce2140 100644
--- a/dix/events.c
+++ b/dix/events.c
@@ -3807,7 +3807,11 @@ CheckPassiveGrab(DeviceIntPtr device, GrabPtr grab, 
InternalEvent *event,
 return FALSE;
 }
 
-return TRUE;
+/* Finally check whether grab allowed by security provider */
+if (XaceHook(XACE_DEVICE_ACCESS, rClient(grab), device, DixGrabAccess) == 
0)
+return TRUE;
+else
+return FALSE;
 }
 
 /**
-- 
1.7.10.4

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[v2] [PATCH 1/1] XSELinux: When SELinux is enabled the xserver seg faults

2012-06-15 Thread Richard Haines
Note: [v2] patch fixes indentation

This patch was created using xorg-server-1.12.0 source.

When using Fedora 17 with xorg-server-1.12.0 and SELinux is enabled
('setsebool xserver_object_manager on') the xserver will not load. The X
log file has a seg fault pointing to XACE/SELinux. Bug 50641 was raised
(https://bugs.freedesktop.org/show_bug.cgi?id=50641).

The bug is caused by X calling XaceHook(XACE_DEVICE_ACCESS, client, ...)
with a device ID of '1' that is XIAllMasterDevices. It would also happen
if the device ID = 0 (XIAllDevices).

The only places currently seen calling with a device id=1 are:
GrabKey - in Xi/exevents.c and AddPassiveGrabToList - in dix/grabs.c
These start life in ProcXIPassiveGrabDevice (in Xi/xipassivegrab.c) that
has been called by XIGrabKeycode.

The patch has been tested using the other XI calls that would also impact
this: XIGrabTouchBegin, XIGrabButton, XIGrabFocusIn and XIGrabEnter with
and without the correct permissions (grab and freeze) with no problems.

Both possible classes have to be checked (x_keyboard and x_pointer) as it
is not known whether it is a pointer or keyboard as this info is not
available. To get this info would require a change to the
XaceHook(XACE_DEVICE_ACCESS, client, ..) call to pass an additional
parameter stating the actual devices (that would defeat the objective of
the XIAllMasterDevices and XIAllDevices dev ids).

Note that there are other devices apart from the keyboard and pointer, for
example on the test system: DeviceID: 9 is the Integrated_Webcam_1.3M. As
it is classed as a slave keyboard it is checked.

Signed-off-by: Richard Haines richard_c_hai...@btinternet.com
---
 Xext/xselinux_hooks.c |   43 ++-
 1 file changed, 38 insertions(+), 5 deletions(-)

diff --git a/Xext/xselinux_hooks.c b/Xext/xselinux_hooks.c
index 0d4c9ab..583c884 100644
--- a/Xext/xselinux_hooks.c
+++ b/Xext/xselinux_hooks.c
@@ -336,9 +336,17 @@ SELinuxDevice(CallbackListPtr *pcbl, pointer unused, 
pointer calldata)
 SELinuxAuditRec auditdata = { .client = rec-client, .dev = rec-dev };
 security_class_t cls;
 int rc;
+DeviceIntPtr dev = NULL;
+int i = 0;
 
 subj = dixLookupPrivate(rec-client-devPrivates, subjectKey);
-obj = dixLookupPrivate(rec-dev-devPrivates, objectKey);
+/*
+ * The XIAllMasterDevices or XIAllDevices do not have devPrivates
+ * entries. Therefore dixLookupPrivate for the object is done later
+ * for these device IDs.
+ */
+if (rec-dev-id != XIAllDevices  rec-dev-id != XIAllMasterDevices)
+obj = dixLookupPrivate(rec-dev-devPrivates, objectKey);
 
 /* If this is a new object that needs labeling, do it now */
 if (rec-access_mode  DixCreateAccess) {
@@ -356,10 +364,35 @@ SELinuxDevice(CallbackListPtr *pcbl, pointer unused, 
pointer calldata)
}
 }
 
-cls = IsPointerDevice(rec-dev) ? SECCLASS_X_POINTER : SECCLASS_X_KEYBOARD;
-rc = SELinuxDoCheck(subj, obj, cls, rec-access_mode, auditdata);
-if (rc != Success)
-   rec-status = rc;
+if (rec-dev-id != XIAllDevices  rec-dev-id != XIAllMasterDevices) {
+cls = IsPointerDevice(rec-dev) ? SECCLASS_X_POINTER : 
SECCLASS_X_KEYBOARD;
+rc = SELinuxDoCheck(subj, obj, cls, rec-access_mode, auditdata);
+if (rc != Success)
+rec-status = rc;
+return;
+} else {
+/*
+ * Device ID must be 0 or 1
+ * We have to check both possible classes as we don't know whether it
+ * was a pointer or keyboard. Therefore all devices are checked for:
+ * rec-dev-id == XIAllDevices
+ * and only masters for:
+ *  rec-dev-id == XIAllMasterDevices
+ *
+ * An error is returned should any device fail SELinuxDoCheck
+ */
+for (dev = inputInfo.devices; dev; dev = dev-next, i++) {
+if (!IsMaster(dev)  rec-dev-id == XIAllMasterDevices)
+continue;
+cls = IsPointerDevice(dev) ? SECCLASS_X_POINTER : 
SECCLASS_X_KEYBOARD;
+obj = dixLookupPrivate(dev-devPrivates, objectKey);
+rc = SELinuxDoCheck(subj, obj, cls, rec-access_mode, auditdata);
+if (rc != Success) {
+rec-status = rc;
+return;
+}
+}
+}
 }
 
 static void
-- 
1.7.10.2

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Subject: [PATCH 1/1] XSELinux: When SELinux is enabled the xserver seg faults

2012-06-12 Thread Richard Haines
This patch was created using xorg-server-1.12.0 source.

When using Fedora 17 with xorg-server-1.12.0 and SELinux is enabled
('setsebool xserver_object_manager on') the xserver will not load. The X
log file has a seg fault pointing to XACE/SELinux. Bug 50641 was raised
(https://bugs.freedesktop.org/show_bug.cgi?id=50641). The patch below is a
possible fix.

The bug is caused by X calling XaceHook(XACE_DEVICE_ACCESS, client, ...)
with a device ID of '1' that is XIAllMasterDevices. It would also happen
if the device ID = 0 (XIAllDevices).

The only places currently seen calling with a device id=1 are:
GrabKey - in Xi/exevents.c and AddPassiveGrabToList - in dix/grabs.c
These start life in ProcXIPassiveGrabDevice (in Xi/xipassivegrab.c) that
has been called by XIGrabKeycode.

The patch has been tested using the other XI calls that would also impact
this: XIGrabTouchBegin, XIGrabButton, XIGrabFocusIn and XIGrabEnter with
and without the correct permissions (grab and freeze) with no problems.

Both possible classes have to be checked (x_keyboard and x_pointer) as it
is not known whether it is a pointer or keyboard as this info is not
available. To get this info would require a change to the
XaceHook(XACE_DEVICE_ACCESS, client, ..) call to pass an additional
parameter stating the actual devices (that would defeat the objective of
the XIAllMasterDevices and XIAllDevices dev ids).

Note that there are other devices apart from the keyboard and pointer, for
example on the test system: DeviceID: 9 is the Integrated_Webcam_1.3M. As
it is classed as a slave keyboard it is checked.

Signed-off-by: Richard Haines richard_c_hai...@btinternet.com
---
 Xext/xselinux_hooks.c |   44 +++-
 1 file changed, 39 insertions(+), 5 deletions(-)

diff --git a/Xext/xselinux_hooks.c b/Xext/xselinux_hooks.c
index 0d4c9ab..c2b21d6 100644
--- a/Xext/xselinux_hooks.c
+++ b/Xext/xselinux_hooks.c
@@ -336,9 +336,17 @@ SELinuxDevice(CallbackListPtr *pcbl, pointer unused, 
pointer calldata)
 SELinuxAuditRec auditdata = { .client = rec-client, .dev = rec-dev };
 security_class_t cls;
 int rc;
+DeviceIntPtr dev = NULL;
+int i = 0;
 
 subj = dixLookupPrivate(rec-client-devPrivates, subjectKey);
-obj = dixLookupPrivate(rec-dev-devPrivates, objectKey);
+   /*
+* The XIAllMasterDevices or XIAllDevices do not have devPrivates
+* entries. Therefore dixLookupPrivate for the object is done later
+* for these device IDs.
+*/
+   if (rec-dev-id != XIAllDevices  rec-dev-id != XIAllMasterDevices)
+   obj = dixLookupPrivate(rec-dev-devPrivates, objectKey);
 
 /* If this is a new object that needs labeling, do it now */
 if (rec-access_mode  DixCreateAccess) {
@@ -356,12 +364,38 @@ SELinuxDevice(CallbackListPtr *pcbl, pointer unused, 
pointer calldata)
}
 }
 
-cls = IsPointerDevice(rec-dev) ? SECCLASS_X_POINTER : SECCLASS_X_KEYBOARD;
-rc = SELinuxDoCheck(subj, obj, cls, rec-access_mode, auditdata);
-if (rc != Success)
-   rec-status = rc;
+   if (rec-dev-id != XIAllDevices  rec-dev-id != XIAllMasterDevices) 
{
+   cls = IsPointerDevice(rec-dev) ? SECCLASS_X_POINTER : 
SECCLASS_X_KEYBOARD;
+   rc = SELinuxDoCheck(subj, obj, cls, rec-access_mode, 
auditdata);
+   if (rc != Success)
+   rec-status = rc;
+   return;
+   } else {
+   /*
+* Device ID must be 0 or 1
+* We have to check both possible classes as we don't know 
whether it
+* was a pointer or keyboard. Therefore all devices are checked 
for:
+*  rec-dev-id == XIAllDevices
+* and only masters for:
+*   rec-dev-id == XIAllMasterDevices
+*
+* An error is returned should any device fail SELinuxDoCheck
+*/
+   for (dev = inputInfo.devices; dev; dev = dev-next, i++) {
+   if (!IsMaster(dev)  rec-dev-id == 
XIAllMasterDevices)
+   continue;
+   cls = IsPointerDevice(dev) ? SECCLASS_X_POINTER : 
SECCLASS_X_KEYBOARD;
+   obj = dixLookupPrivate(dev-devPrivates, objectKey);
+   rc = SELinuxDoCheck(subj, obj, cls, rec-access_mode, 
auditdata);
+   if (rc != Success) {
+   rec-status = rc;
+   return;
+   }
+   }
+   }
 }
 
+
 static void
 SELinuxSend(CallbackListPtr *pcbl, pointer unused, pointer calldata)
 {
-- 
1.7.10.2

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PATCH 1/1] XSELinux: Fix initialisation of polyinstantiated properties and selections

2012-04-02 Thread Richard Haines
Note: This patch has been generated and tested on xorg-server-1.11.4

The specfile (x_contexts) is currently checked for non-poly properties
or selections first. If an entry should be present that specifies
a poly entry and there is a default non-poly entry as a fallback, then
this will be used instead (this is standard practice so that there is
always a known label added). This patch checks for poly_selection or
poly_property entries first and then checks non-poly entries.

Example x_contexts entry for selections:
poly_selection PRIMARY  system_u:object_r:clipboard_xselection_t:s0
selection CLIPBOARD system_u:object_r:clipboard_xselection_t:s0
selection * system_u:object_r:xselection_t:s0 # default fallback

Signed-off-by: Richard Haines richard_c_hai...@btinternet.com
---
 Xext/xselinux_label.c |   12 ++--
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/Xext/xselinux_label.c b/Xext/xselinux_label.c
index e5929fa..ceb5a7e 100644
--- a/Xext/xselinux_label.c
+++ b/Xext/xselinux_label.c
@@ -97,16 +97,16 @@ SELinuxAtomToSIDLookup(Atom atom, SELinuxObjectRec *obj, 
int map, int polymap)
 security_context_t ctx;
 int rc = Success;
 
-obj-poly = 1;
+obj-poly = 0;
 
 /* Look in the mappings of names to contexts */
-if (selabel_lookup_raw(label_hnd, ctx, name, map) == 0) {
-   obj-poly = 0;
+if (selabel_lookup_raw(label_hnd, ctx, name, polymap) == 0) {
+   obj-poly = 1;
 } else if (errno != ENOENT) {
-   ErrorF(SELinux: a property label lookup failed!\n);
+   ErrorF(SELinux: a poly property or selection label lookup failed!\n);
return BadValue;
-} else if (selabel_lookup_raw(label_hnd, ctx, name, polymap)  0) {
-   ErrorF(SELinux: a property label lookup failed!\n);
+} else if (selabel_lookup_raw(label_hnd, ctx, name, map)  0) {
+   ErrorF(SELinux: a property or selection label lookup failed!\n);
return BadValue;
 }
 
-- 
1.7.7.1

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel