Eamon, Thanks for the selection patch, I will review it next week and send my comments directly to you. I have implemented a much less clean version (hack) of selection redirection for a trusted X server that I am working on, and have not had trouble with clients rejecting synthetic requests -- but I have not done much testing with modern X clients so I may be blissfully ignorant of problems. One problem, outside the scope of your patch, that I have had with redirecting selections is caused by the hardcoded default Xt selection timeout. If the selection manager causes the selection reply to be delayed by more that 5000 milliseconds the Xt toolkit will ignore the selection. This is the code from lib/Xt/Seletion.c that causes the trouble: void _XtSetDefaultSelectionTimeout( unsigned long *timeout) { *timeout = 5000; /* default to 5 seconds */ } I do not have a good solution for the timeout. My workaround was to add a similar timeout to the selection manager so it can let the user know that the selection might be ignored. Pat Kane -------- <div>On 3/21/07, Eamon Walsh <<a href="mailto:ewalsh@tycho.nsa.gov">ewalsh@tycho.nsa.gov</a>> wrote:<blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"> This patch implements support for dealing with selections in XACE. The requirements are to support the usual access check, and also to support redirecting ConvertSelection requests to another client application. The patch introduces new fields into the Selection structure, destclient and destwindow, and adds an XACE hook in the selection request procs. The destclient field is where the SelectionRequest message actually gets sent when doing a ConvertSelection. This could be a selection manager application that can send the SelectionRequest on to the real selection owner, actively serve as a middleman, or send a SelectionNotify None back to the requestor to cancel the transfer. The destwindow field is what gets reported as the selection owner from GetSelectionOwner requests. This could be used to further redirect things to the selection manager. I'm aware of the XFixes selection events; the behavior of those is not changed. XACE module authors can register for this hook, use the hook arguments to decide whether to redirect, and change the dest field accordingly. Returning FALSE from the hook is a third option, which ignores the request (sends None back to requestor). This patch is based on the description of how selections work from the ICCCM. There are some details I'm not sure of, such as whether applications will accept a synthetic SelectionRequest as in the example above. Comments? -- Xext/xace.c | 11 +++++++++++ Xext/xace.h | 17 +++++++++-------- Xext/xacestr.h | 9 +++++++++ dix/dispatch.c | 25 +++++++++++++++++-------- include/selection.h | 4 ++++ 5 files changed, 50 insertions(+), 16 deletions(-) -- diff --git a/Xext/xace.c b/Xext/xace.c index ee0f39c..9502b5d 100644 --- a/Xext/xace.c +++ b/Xext/xace.c @@ -147,6 +147,17 @@ int XaceHook(int hook, ...) prv = &rec.rval; break; } + case XACE_SELECTION_ACCESS: { + XaceSelectionAccessRec rec = { + va_arg(ap, ClientPtr), + va_arg(ap, Selection*), + va_arg(ap, Mask), + TRUE /* default allow */ + }; + calldata = &rec; + prv = &rec.rval; + break; + } case XACE_SITE_POLICY: { XaceSitePolicyRec rec = { va_arg(ap, char*), diff --git a/Xext/xace.h b/Xext/xace.h index 7360dae..d3d5a84 100644 --- a/Xext/xace.h +++ b/Xext/xace.h @@ -28,7 +28,7 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. #ifdef XACE #define XACE_EXTENSION_NAME "XAccessControlExtension" -#define XACE_MAJOR_VERSION 1 +#define XACE_MAJOR_VERSION 2 #define XACE_MINOR_VERSION 0 #include "pixmap.h" /* for DrawablePtr */ @@ -50,13 +50,14 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. #define XACE_BACKGRND_ACCESS 7 #define XACE_EXT_ACCESS 8 #define XACE_HOSTLIST_ACCESS 9 -#define XACE_SITE_POLICY 10 -#define XACE_DECLARE_EXT_SECURE 11 -#define XACE_AUTH_AVAIL 12 -#define XACE_KEY_AVAIL 13 -#define XACE_AUDIT_BEGIN 14 -#define XACE_AUDIT_END 15 -#define XACE_NUM_HOOKS 16 +#define XACE_SELECTION_ACCESS 10 +#define XACE_SITE_POLICY 11 +#define XACE_DECLARE_EXT_SECURE 12 +#define XACE_AUTH_AVAIL 13 +#define XACE_KEY_AVAIL 14 +#define XACE_AUDIT_BEGIN 15 +#define XACE_AUDIT_END 16 +#define XACE_NUM_HOOKS 17 extern CallbackListPtr XaceHooks[XACE_NUM_HOOKS]; diff --git a/Xext/xacestr.h b/Xext/xacestr.h index bd30883..edf7b66 100644 --- a/Xext/xacestr.h +++ b/Xext/xacestr.h @@ -27,6 +27,7 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. #include "gcstruct.h" #include "windowstr.h" #include "inputstr.h" +#include " selection.h" #include "xace.h" /* XACE_CORE_DISPATCH */ @@ -93,6 +94,14 @@ typedef struct { int rval; } XaceHostlistAccessRec; +/* XACE_SELECTION_ACCESS */ +typedef struct { + ClientPtr client; + Selection *selection; + Mask access_mode; + int rval; +} XaceSelectionAccessRec; + /* XACE_SITE_POLICY */ typedef struct { char *policyString; diff --git a/dix/dispatch.c b/dix/dispatch.c index d44687e..e4bc937 100644 --- a/dix/dispatch.c +++ b/dix/dispatch.c @@ -1074,11 +1074,16 @@ ProcSetSelectionOwner(register ClientPtr client) NumCurrentSelections++; CurrentSelections = newsels; CurrentSelections[i].selection = stuff->selection; + CurrentSelections[i].devPrivates = NULL; } + dixFreePrivates(CurrentSelections[i].devPrivates); CurrentSelections[i].lastTimeChanged = time; CurrentSelections[i].window = stuff->window; + CurrentSelections[i].destwindow = stuff->window; CurrentSelections[i].pWin = pWin; CurrentSelections[i].client = (pWin ? client : NullClient); + CurrentSelections[i].destclient = (pWin ? client : NullClient); + CurrentSelections[i].devPrivates = NULL; if (SelectionCallback) { SelectionInfoRec info; @@ -1113,8 +1118,10 @@ ProcGetSelectionOwner(register ClientPtr client) reply.type = X_Reply; reply.length = 0; reply.sequenceNumber = client->sequence; - if (i < NumCurrentSelections) - reply.owner = CurrentSelections[i].window; + if (i < NumCurrentSelections && + XaceHook(XACE_SELECTION_ACCESS, client, &CurrentSelections[i], + DixReadAccess)) + reply.owner = CurrentSelections[i].destwindow; else reply.owner = None; WriteReplyToClient(client, sizeof(xGetSelectionOwnerReply), &reply); @@ -1153,20 +1160,18 @@ ProcConvertSelection(register ClientPtr client) CurrentSelections[i].selection != stuff->selection) i++; if ((i < NumCurrentSelections) && (CurrentSelections[i].window != None) && - XaceHook(XACE_RESOURCE_ACCESS, client, - CurrentSelections[i].window, RT_WINDOW, - DixReadAccess, CurrentSelections[i].pWin)) + XaceHook(XACE_SELECTION_ACCESS, client, &CurrentSelections[i], + DixReadAccess)) { event.u.u.type = SelectionRequest; event.u.selectionRequest.time = stuff->time; - event.u.selectionRequest.owner = - CurrentSelections[i].window; + event.u.selectionRequest.owner = CurrentSelections[i].window; event.u.selectionRequest.requestor = stuff->requestor; event.u.selectionRequest.selection = stuff->selection; event.u.selectionRequest.target = stuff->target; event.u.selectionRequest.property = stuff->property; if (TryClientEvents( - CurrentSelections[i].client, &event, 1, NoEventMask, + CurrentSelections[i].destclient, &event, 1, NoEventMask, NoEventMask /* CantBeFiltered */, NullGrab)) return (client->noClientException); } @@ -4021,9 +4026,11 @@ DeleteWindowFromAnySelections(WindowPtr pWin) info.kind = SelectionWindowDestroy; CallCallbacks(&SelectionCallback, &info); } + dixFreePrivates(CurrentSelections[i].devPrivates); CurrentSelections[i].pWin = (WindowPtr)NULL; CurrentSelections[i].window = None; CurrentSelections[i].client = NullClient; + CurrentSelections[i].devPrivates = NULL; } } @@ -4043,9 +4050,11 @@ DeleteClientFromAnySelections(ClientPtr client) info.kind = SelectionWindowDestroy; CallCallbacks(&SelectionCallback, &info); } + dixFreePrivates(CurrentSelections[i].devPrivates); CurrentSelections[i].pWin = (WindowPtr)NULL; CurrentSelections[i].window = None; CurrentSelections[i].client = NullClient; + CurrentSelections[i].devPrivates = NULL; } } diff --git a/include/selection.h b/include/selection.h index fbe7cfc..9347376 100644 --- a/include/selection.h +++ b/include/selection.h @@ -50,6 +50,7 @@ SOFTWARE. ******************************************************************/ #include "dixstruct.h " +#include "privates.h" /* * * Selection data structures @@ -61,6 +62,9 @@ typedef struct _Selection { Window window; WindowPtr pWin; ClientPtr client; + ClientPtr destclient; /* support for redirection */ + Window destwindow; /* support for redirection */ + PrivateRec *devPrivates; } Selection; #endif /* SELECTION_H */ _______________________________________________ xorg mailing list <a href="mailto:xorg@lists.freedesktop.org">xorg@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/xorg">http://lists.freedesktop.org/mailman/listinfo/xorg</a> </blockquote></div>