The server-side APPGROUP code fails to compile when BuildXCSecurity is set to NO. Forthcoming patch disables the calls into XC-SECURITY; this is arguably wrong if APPGROUP really hard-depends on XC-SECURITY, in which case build-time logic is needed.
Created attachment 1968 [details] [review] no-security-1.patch
I think the patch is bad for two reasons: 1. Disabling the SECURITY extension will break "xauth", almost every ssh implementaion under the moon (except OpenSSH which may surive this condition, ssh.com's ssh doesn't handle that case), the XC-APPGROUP extension (see [2]) and a couple of other things... 2. AFAIK the XC-APPGROUP extension needs the SECURITY extension to provide "tags" for display connections - and right now the only implemented way to make such tags is to generate a MIT-MAGIC-COOKIE-1 which marks all applications which use that cookie with the "tag" (kaleb may correct me if I am wrong...).
(In reply to comment #2) > I think the patch is bad for two reasons: > 1. Disabling the SECURITY extension will break "xauth", almost every ssh > implementaion under the moon (except OpenSSH which may surive this condition, > ssh.com's ssh doesn't handle that case), the XC-APPGROUP extension (see [2]) and > a couple of other things... xauth only needs the Security extension for the 'generate' command, which is not used in the default startx, and which anyway only makes sense in the context of the server supporting the Security extension in the first place. BuildXCSecurity does not prevent xauth from supporting this command; it only means the built server will not support it. so it does not break xauth. openssh's Security integration has made a lot of people unhappy and is generally regarded as a bad move. it silently changed the semantics of the -X option which breaks most modern apps. as far as i can tell ssh generates MIT-MAGIC-COOKIE-1 tokens using its own internal PRNG so it wouldn't use the xauth generate command in any case. (this appears to be the way ssh behaves back to the 1.2.27 branchpoint for openssh from Tatu Ylonen's original release; i would be surprised if ssh.com's implementation behaved differently.) the point is, 'ssh -X' is broken anyway, disabling the Security extension in the server does not change that. it should be noted that openssh's X11 handling is done entirely without including any X11 headers or linking against any X11 libraries. not supporting the Security extension in the server is clearly not fatal, since kdrive doesn't support it and works just fine with xauth and ssh. > 2. AFAIK the XC-APPGROUP extension needs the SECURITY extension to provide > "tags" for display connections - and right now the only implemented way to make > such tags is to generate a MIT-MAGIC-COOKIE-1 which marks all applications > which use that cookie with the "tag" (kaleb may correct me if I am wrong...). if this is true, then build-time logic is required to do one of two things: a) if BuildAppgroup && !BuildXCSecurity, turn BuildXCSecurity on b) if BuildAppgroup && !BuildXCSecurity, turn BuildAppgroup off either one, i'm not picky. my reading of the appgroup code is that the Security extension is used to find an authId on client state change (and then do something magic with it when the Security extension exists), but that client state change notification only matters if the server supports Security anyway. so i don't know that this patch necessarily breaks XC-APPGROUP. i'll defer to expert opinion though.
(In reply to comment #3) > (In reply to comment #2) > > I think the patch is bad for two reasons: > > 1. Disabling the SECURITY extension will break "xauth", almost every ssh > > implementaion under the moon (except OpenSSH which may surive this condition, > > ssh.com's ssh doesn't handle that case), the XC-APPGROUP extension (see [2]) and > > a couple of other things... > > xauth only needs the Security extension for the 'generate' command, which is not > used in the default startx, and which anyway only makes sense in the context of > the server supporting the Security extension in the first place. > BuildXCSecurity does not prevent xauth from supporting this command; it only > means the built server will not support it. so it does not break xauth. It will break the "generate" command which is needed to grant applications access to the display when there is no cookie yet - the usual usecase for this is when the Xserver uses the SUN-DES-1 or MIT-KERBEROS-5 auth. and need to grant a remote application access to the display where the remote machine isn't part of the SecureRPC domain or part of the Kerberos system. Feel free to kill the SECRUITY extension but don't forget to disable the user-to-user auth. schemes as well. > openssh's Security integration has made a lot of people unhappy and is generally > regarded as a bad move. it silently changed the semantics of the -X option > which breaks most modern apps. Well, that's cleary the fault of the toolkits. AFAIK they were informed about the exploits around a _year_ before the first ssh version implemented the usage of the SECURITY extension. And this is no fatal condition for toolkits, they can easily trap the "offending" X calls and handle them correctly (for example CDE2.x applications don't have the problem and somewhere in Mozilla.org's bugzilla is a patch for the Gecko Xlib port to let it run happily in X11 "untrusted" mode). [snip] > i would be surprised if ssh.com's implementation behaved differently.) the > point is, 'ssh -X' is broken anyway, disabling the Security extension in the > server does not change that. That implementation works completely different (and even handles the SECURITY extension "timeout" correctly (openssh doesn't). > it should be noted that openssh's X11 handling is done entirely without > including any X11 headers or linking against any X11 libraries. > > not supporting the Security extension in the server is clearly not fatal, since > kdrive doesn't support it and works just fine with xauth and ssh. This is FATAL for most ssh implementations _except_ openssh. I have excplitly listed that case... it's the only ssh implementation which can recover from that case AFAIK. > > 2. AFAIK the XC-APPGROUP extension needs the SECURITY extension to provide > > "tags" for display connections - and right now the only implemented way to make > > such tags is to generate a MIT-MAGIC-COOKIE-1 which marks all applications > > which use that cookie with the "tag" (kaleb may correct me if I am wrong...). > > if this is true, then build-time logic is required to do one of two things: > > a) if BuildAppgroup && !BuildXCSecurity, turn BuildXCSecurity on > b) if BuildAppgroup && !BuildXCSecurity, turn BuildAppgroup off > > either one, i'm not picky. Kaleb can answer that...
(In reply to comment #4) > It will break the "generate" command which is needed to grant applications > access to the display when there is no cookie yet - the usual usecase for this > is when the Xserver uses the SUN-DES-1 or MIT-KERBEROS-5 auth. and need to grant > a remote application access to the display where the remote machine isn't part > of the SecureRPC domain or part of the Kerberos system. Feel free to kill the > SECRUITY extension but don't forget to disable the user-to-user auth. schemes as > well. So when the user asks to build without Security, they get a server that lacks support for the Security extension, and all that that entails including broken SUN-DES-1. Seems pretty straightforward to me. > Well, that's cleary the fault of the toolkits. AFAIK they were informed about > the exploits around a _year_ before the first ssh version implemented the usage > of the SECURITY extension. Granted. > This is FATAL for most ssh implementations _except_ openssh. I have excplitly > listed that case... it's the only ssh implementation which can recover from that > case AFAIK. It is fatal iff: - the ssh implementation implements Security integration (ie, is not PuTTY, TeraTerm, lsh, or most of the Java ssh implementations i've seen), AND - the user asks for X forwarding using the -X flag to openssh or the +x flag to ssh.com's ssh. The failure mode here is exactly what the user asks for: They built a server without the Security extension, and then required the ssh client to use the Security extension. There's one other failure mode I suppose: - The ssh implementation requires the Security extension even for normal (ie, trusted) forwarding. (ssh.com?) This in my mind would be a regression in the ssh client, as it would no longer be able to interoperate with X servers from before 6.3, when the Security extension was introduced. It is in any case giving the user what they asked for. There is a BuildXCSecurity option in imake. As a user I expect that if I turn off any particular option in imake it will build to completion and make a reasonable attempt at giving me what I asked for. So, either the option needs to disappear (and we just build with the Security extension all the time), or the code or Imakefiles need to be fixed. I am absolutely not suggesting turning off Security by default; I'm just asking that my build not fail when I ask to not have it.
Created attachment 2298 [details] [review] appgroup-needs-security.patch Easy fix: if the user requests (appgroup && !security), force security on.
applied to HEAD, closing.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.