Qubes GUI daemon overwriting window class

[Daniel Schoepe]

Hi,

I'm trying to make xmonad [1], a Haskell-based tiling window manager,
play nice with Qubes. Running xmonad and adding colored window
decorations based on the VM name worked as expected.

However, one useful feature of xmonad is to automatically move windows
around based on the window class (i.e. the WM_CLASS property of a
window). For example, this allows to automatically move Firefox
instances to another workspace. As far as I can tell though, the
WM_CLASS property is overwritten with the name of the VM [2].

Since window titles are often less useful for this kind of automatic
window management, it would be helpful to still make use of the window
class.

Is the original window class still available in some place that is
accessible to the window manager?

On a more general note, is overwriting the window class intentional?
Since the VM name is already present in _QUBES_VMNAME, this may not be
necessary and more window manager configurations might work on Qubes
without modification if this were left unchanged.

Best regards,
Daniel

[1] http://xmonad.org/
[2] https://github.com/QubesOS/qubes-gui-daemon/blob/d799e8a24c991ca58edab255781d541181d060ca/gui-daemon/xside.c#L405

[Marek Marczykowski-Górecki]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On Wed, Oct 21, 2015 at 01:18:17AM +0200, Daniel Schoepe wrote:
> Hi,
>
> I'm trying to make xmonad [1], a Haskell-based tiling window manager,
> play nice with Qubes. Running xmonad and adding colored window
> decorations based on the VM name worked as expected.

Do you want to share a configuration for that? I think some more people
will be interested :)

> However, one useful feature of xmonad is to automatically move windows
> around based on the window class (i.e. the WM_CLASS property of a
> window). For example, this allows to automatically move Firefox
> instances to another workspace. As far as I can tell though, the
> WM_CLASS property is overwritten with the name of the VM [2].
>
> Since window titles are often less useful for this kind of automatic
> window management, it would be helpful to still make use of the window
> class.
>
> Is the original window class still available in some place that is
> accessible to the window manager?

No, it isn't even retrieved from the VM.

> On a more general note, is overwriting the window class intentional?
> Since the VM name is already present in _QUBES_VMNAME, this may not be
> necessary and more window manager configurations might work on Qubes
> without modification if this were left unchanged.

Yes, setting WM_CLASS to the VM name is (theoretically) unnecessary now.
Previously it was used for having the right icon on the window, not now
the icon is set independently (_NET_WM_ICON).

So currently the alternative for having VM name in WM_CLASS is... not
having it at all. Which would be bad for example for window grouping in
KDE.

> [1] http://xmonad.org/
> [2] https://github.com/QubesOS/qubes-gui-daemon/blob/d799e8a24c991ca58edab255781d541181d060ca/gui-daemon/xside.c#L405

- --
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQEcBAEBCAAGBQJWJudNAAoJENuP0xzK19csfNQH/R6L+N2cs2vYxgTJCYTjn+af
fG172c/j6vAJM/FQsFpThyetVtqZZrpJBNs4FuKla547byJjbUEyo6Gsb69/CcWY
rXK8JNRjzwWYooMVDaGk1E3xUCFAyJIc0N+ecaxYkgZtPC5MSeMzK5XRLWCbsSpx
nYgfVjM+btGcf6gOdH0yom5DuU1WCA2hvKjZm0icVoUqWnBIXiea+uZeV70g/HrW
dTem6WGOLKBCSjEEtTJL18KvKlBRLyUzxEo1L9iozWq86otnUPFxg3ectrDcmRrc
UZyueq6vBPma/17eokwfSjafGqTJ3EYhjy57n6WckDcmHCTswpVtbe95AxnvikE=
=j1lJ
-----END PGP SIGNATURE-----

[Daniel Schoepe]

On Wed, 21 Oct 2015 03:15 +0200, Marek Marczykowski-Górecki wrote:
> Do you want to share a configuration for that? I think some more people
> will be interested :)

Sure, my plan is to put the patches on github once I'm finished with
all the required changes.

> Yes, setting WM_CLASS to the VM name is (theoretically) unnecessary now.
> Previously it was used for having the right icon on the window, not now
> the icon is set independently (_NET_WM_ICON).
>
> So currently the alternative for having VM name in WM_CLASS is... not
> having it at all. Which would be bad for example for window grouping in
> KDE.

Given that qubes already retrieves WM_NAME from the VM, I guess also
retrieving the window class may be something to consider, if this data
is used by many window managers. In the short term, I noticed that it's
possible to retrieve the window class from dom0 by running the following
command:

qvm-run --pass-io $VM "xprop -id $window_id WM_CLASS"

Where $window_id is the in _QUBES_VMWINDOWID.

I guess this might create a (minor) possibility to screw up the
code that parses the result, but that sounds a bit far-fetched.

Best regards,
Daniel

[Marek Marczykowski-Górecki]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Take a look at window-icon-updater (in master branch - planned for R3.1):
https://github.com/QubesOS/qubes-gui-daemon/tree/master/window-icon-updater
https://github.com/QubesOS/qubes-gui-agent-linux/tree/master/window-icon-updater

It uses similar approach to retrieve application icon. This is done in
separate qrexec connection, because icons are quite complex, require
additional processing (coloring) etc. So it's better to not put this
into GUI protocol, which should remain as simple as possible (better to
have two simple protocols, than one complex).

You can use similar approach for WM_CLASS. It will be an overkill, but
at least it doesn't require modifying GUI protocol, carrying custom
patch etc.

That said, I think adding WM_CLASS should not introduce any more
complexity to the GUI protocol. For backward compatibility it should be
separate message. This leads to some design questions:

Background:
- --------------
http://tronche.com/gui/x/icccm/sec-4.html
4.1.2.5. WM_CLASS Property

The WM_CLASS property (of type STRING without control characters)
contains two consecutive null-terminated strings. These specify the
Instance and Class names to be used by both the client and the window
manager for looking up resources for the application or as identifying
information. This property must be present when the window leaves the
Withdrawn state and may be changed only while the window is in the
Withdrawn state. Window managers may examine the property only when they
start up and when the window leaves the Withdrawn state, but there
should be no need for a client to change its state dynamically.

The two strings, respectively, are:

A string that names the particular instance of the application to
which the client that owns this window belongs. Resources that are
specified by instance name override any resources that are specified by
class name. Instance names can be specified by the user in an
operating-system specific manner. On POSIX-conformant systems, the
following conventions are used:
If "-name NAME" is given on the command line, NAME is used as
the instance name.
Otherwise, if the environment variable RESOURCE_NAME is set, its
value will be used as the instance name.
Otherwise, the trailing part of the name used to invoke the
program (argv[0] stripped of any directory names) is used as the
instance name.
A string that names the general class of applications to which the
client that owns this window belongs. Resources that are specified by
class apply to all applications that have the same class name. Class
names are specified by the application writer. Examples of commonly used
class names include: "Emacs", "XTerm", "XClock", "XLoad", and so on.

Note that WM_CLASS strings are null-terminated and, thus, differ from
the general conventions that STRING properties are null-separated. This
inconsistency is necessary for backwards compatibility.

Questions:
- -----------
1. Should the VM name be prepended to the class name? Or instance name? Or
both?
2. If any of above "yes" - what should be the separator? ":"?
3. Should the application be able to change class/instance later, or
just set it once (some stronger than "may be changed only while the
window is in the Withdrawn state").
4. What should be set if VM application did not provided the
class/instance name? Set VM name there as it is done currently?
5. What should be general constraints on those two strings? Especially
maximum length and allowed characters.

And when when we settle on some design, if you want this feature sooner
than later, feel free to provide a patches :)

- --
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQEcBAEBCAAGBQJWJ7MgAAoJENuP0xzK19csxNQIAJXbIo7nQ1A+58nXJK32J8vn
VhtitkvG4XJ8bS2sk9HIhUtQAH5aQslBqruYwzeFpyC6ultx44aI6yDdnA9T01rb
Z1S/6WMStLwcIkc//2ebNP3pEvOFxywgyqiZ5NXDqNohnHFI2GCYKSwD1ptwudIC
AF/N8sSU1g7e5K1JDMhpgqu4Iqa5hJFGEh/yevNRw4Gt3L8P0saMeVXPfBj0dsMe
Jv6DxokqxoGOIkZCyoGQxGg/EUrTNh4Ps0U9Qld5vmYIT2UeRqo6ZXyhSUk7EgxA
rg4DITqoH/1pQoOBd+xsOtSOv2ZNL0Mt0dtVJewhni4pChF33kV+2YH/YDHHBB4=
=f9dS
-----END PGP SIGNATURE-----

[Daniel Schoepe]

On Wed, 21 Oct 2015 17:45 +0200, Marek Marczykowski-Górecki wrote:
> 1. Should the VM name be prepended to the class name? Or instance name? Or
> both?

As far as I know, the window class is not used in a directly
user-visible way. It might be enough to leave it as it is and don't
prepend anything. If the WM pays attention to _QUBES_VMNAME, this
shouldn't lead to the application fooling the user in some way.

> 2. If any of above "yes" - what should be the separator? ":"?
> 3. Should the application be able to change class/instance later, or
> just set it once (some stronger than "may be changed only while the
> window is in the Withdrawn state").

Given that things work pretty well without the original class name at
all, it might be enough to go for the easy solution and only allowing
setting it once. If that isn't enough for some use case, we could still
make it more flexible later on.

> 4. What should be set if VM application did not provided the
> class/instance name? Set VM name there as it is done currently?

That sounds like a good default. Another idea is to keep it
empty, like in the client window.

> 5. What should be general constraints on those two strings? Especially
> maximum length and allowed characters.

How about handling it in the same way as WM_NAME (whatever that way is)?
If the VM can already cause damage through overly-long, badly-encoded
strings there, then this would already be a problem. If not, then using
the same approach for WM_CLASS shouldn't create problems either.

> And when when we settle on some design, if you want this feature sooner
> than later, feel free to provide a patches :)

I'll have a go at this when we decide these issues, but it might take a
bit until I have some free time to spend on it.

Best regards,
Daniel

[Marek Marczykowski-Górecki]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On Wed, Oct 21, 2015 at 09:15:11PM +0200, Daniel Schoepe wrote:
> On Wed, 21 Oct 2015 17:45 +0200, Marek Marczykowski-Górecki wrote:
> > 1. Should the VM name be prepended to the class name? Or instance name? Or
> > both?
>
> As far as I know, the window class is not used in a directly
> user-visible way. It might be enough to leave it as it is and don't
> prepend anything. If the WM pays attention to _QUBES_VMNAME, this
> shouldn't lead to the application fooling the user in some way.

*If* the WM pays attention. Currently it is handled only to prefix window
title with VM name. If we're going to expose WM_CLASS for additional
tools (like automatic window positioning, or applying other properties),
it is probably also about another tools. In case of scripted WMs like
awesome (or xmonad if I understand correctly) probably not a problem.
But what about some "standard" tools like built in configuration of KDE
and/or Xfce?

> > 2. If any of above "yes" - what should be the separator? ":"?
> > 3. Should the application be able to change class/instance later, or
> > just set it once (some stronger than "may be changed only while the
> > window is in the Withdrawn state").
>
> Given that things work pretty well without the original class name at
> all, it might be enough to go for the easy solution and only allowing
> setting it once. If that isn't enough for some use case, we could still
> make it more flexible later on.

Fair enough.

> > 4. What should be set if VM application did not provided the
> > class/instance name? Set VM name there as it is done currently?
>
> That sounds like a good default. Another idea is to keep it
> empty, like in the client window.
>
> > 5. What should be general constraints on those two strings? Especially
> > maximum length and allowed characters.
>
> How about handling it in the same way as WM_NAME (whatever that way is)?
> If the VM can already cause damage through overly-long, badly-encoded
> strings there, then this would already be a problem. If not, then using
> the same approach for WM_CLASS shouldn't create problems either.

https://github.com/QubesOS/qubes-gui-common/blob/master/include/qubes-gui-protocol.h#L176-L179
struct msg_wmname {
char data[128];
};

Then only ASCII characters allowed. Or full UTF-8 if specifically
enabled in guid.conf. I think plain ASCII should be enough for WM_CLASS.

The code:
https://github.com/QubesOS/qubes-gui-daemon/blob/master/gui-daemon/xside.c#L1995-L2014

> > And when when we settle on some design, if you want this feature sooner
> > than later, feel free to provide a patches :)
>
> I'll have a go at this when we decide these issues, but it might take a
> bit until I have some free time to spend on it.
>
> Best regards,
> Daniel
>

- --
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQEcBAEBCAAGBQJWJ+qMAAoJENuP0xzK19csFi8IAIyCEE/faI+P9Zna8Ar1ynxI
CkzlJHjJvTWVJ0xZOa7kdHqbkxBp4vRp/I7CQCFszedjXBq7CIXISZunuGq/DQeS
3kULkrh/DPxH2sylQ0Q0aPfyvseTu07sofWkuxUVl7T/rfvWz06LwBTjyGVxs3s3
csUPIMuPTyeJDoPjwcJ7vDdMhRXfh9uBYki4mvZ5rjc08mOMWcEfVcjEBb+zkWgG
9QCaS0MpmNWI4NAoRdlOFN5gQHK34QKjAunnw8sLunvVFqeMkVIIvV7KO28yqAXB
T+Hn71WEyaKOaPnIapJ4zFVuJoCZiWRWi5/DvWUZbz1p3EGv377bKVHzVar6W8A=
=gAqE
-----END PGP SIGNATURE-----

[Daniel Schoepe]

On Wed, 21 Oct 2015 21:42 +0200, Marek Marczykowski-Górecki wrote:
> *If* the WM pays attention. Currently it is handled only to prefix window
> title with VM name. If we're going to expose WM_CLASS for additional
> tools (like automatic window positioning, or applying other properties),
> it is probably also about another tools. In case of scripted WMs like
> awesome (or xmonad if I understand correctly) probably not a problem.
> But what about some "standard" tools like built in configuration of KDE
> and/or Xfce?

As far as I understand, this would not really be a security issue, but
more about convenience features, like grouping windows by VM name. In
that case it may be reasonable to assume that the WM looks up
_QUBES_VMNAME for that purpose (users can do this easily in xmonad for
instance, so extending something like kwin should also not pose a big
problem). Since the prefix still appears in the title, users should get
visual feedback at least that way.

> https://github.com/QubesOS/qubes-gui-common/blob/master/include/qubes-gui-protocol.h#L176-L179
> struct msg_wmname {
> char data[128];
> };
>
> Then only ASCII characters allowed. Or full UTF-8 if specifically
> enabled in guid.conf. I think plain ASCII should be enough for WM_CLASS.
>
> The code:
> https://github.com/QubesOS/qubes-gui-daemon/blob/master/gui-daemon/xside.c#L1995-L2014

Thanks for the pointers, this seems like the right way to deal with
WM_CLASS as well then. If anything, plain ASCII should be less of a
problem in this case than it is for WM_NAME.

Best regards,
Daniel

[Marek Marczykowski-Górecki]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On Fri, Oct 23, 2015 at 04:25:54PM +0200, Daniel Schoepe wrote:
> On Wed, 21 Oct 2015 21:42 +0200, Marek Marczykowski-Górecki wrote:
> > *If* the WM pays attention. Currently it is handled only to prefix window
> > title with VM name. If we're going to expose WM_CLASS for additional
> > tools (like automatic window positioning, or applying other properties),
> > it is probably also about another tools. In case of scripted WMs like
> > awesome (or xmonad if I understand correctly) probably not a problem.
> > But what about some "standard" tools like built in configuration of KDE
> > and/or Xfce?
>
> As far as I understand, this would not really be a security issue, but
> more about convenience features, like grouping windows by VM name.

In most cases - yes. But I can think of some cases when user assigns
some special properties (fullscreen? move to desktop with only trusted
apps?) to a window based on its origin. Such thing would require some
user cooperation for exploitation, but not unthinkable. For example take
a look here:
https://github.com/QubesOS/qubes-issues/issues/1166

So, I think we should prefix VM name to at least one of class/instance
name. Not sure which one. Or maybe both?

> In
> that case it may be reasonable to assume that the WM looks up
> _QUBES_VMNAME for that purpose (users can do this easily in xmonad for
> instance, so extending something like kwin should also not pose a big
> problem). Since the prefix still appears in the title, users should get
> visual feedback at least that way.

We try hard to avoid any unnecessary modification of 3rd-party packages.
If possible, it should work with upstream package version. If kwin can
be _configured_ to use _QUBES_VMNAME for grouping and such, that's fine.
But if not, we should think about how to make that work with the
upstream version.

- --
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQEcBAEBCAAGBQJWKteMAAoJENuP0xzK19csXO4H/jN1DmfzY57prUKLslb8bu3G
AEvFGNZEK2z4ULqbzvfKkdAxUCtRb+bUwMGJGb8Ge4NsJDqlmHQd/5uCSMUpWwjl
/ldVggdf+g+rsnXROkAPh06crVDnQXGebXNx/n4h54bVTCa9+Th9w4GIqta/K/+N
yL5woaeX0FGhNsy6RjH7Rrzy4XLrRsWxIHBMk8LOF+gYTmZ9DoTOvzOfuCworUP2
HdS2VJI/qW04m/mbnR628a+HPbOgRaYiq8vokvPhKNgMoDwsXK55gSJxbRHhmHd5
0/2fRc0MDCva7J6yclFOI3kqLKYCfYTSCH2ALlR+9OQfSgRhGBSv5zVqe2SDxjQ=
=eewq
-----END PGP SIGNATURE-----

[Daniel Schoepe]

On Sat, 24 Oct 2015 02:57 +0200, Marek Marczykowski-Górecki wrote:
> On Fri, Oct 23, 2015 at 04:25:54PM +0200, Daniel Schoepe wrote:
>> On Wed, 21 Oct 2015 21:42 +0200, Marek Marczykowski-Górecki wrote:
>> > *If* the WM pays attention. Currently it is handled only to prefix window
>> > title with VM name. If we're going to expose WM_CLASS for additional
>> > tools (like automatic window positioning, or applying other properties),
>> > it is probably also about another tools. In case of scripted WMs like
>> > awesome (or xmonad if I understand correctly) probably not a problem.
>> > But what about some "standard" tools like built in configuration of KDE
>> > and/or Xfce?
>>
>> As far as I understand, this would not really be a security issue, but
>> more about convenience features, like grouping windows by VM name.
>
> In most cases - yes. But I can think of some cases when user assigns
> some special properties (fullscreen? move to desktop with only trusted
> apps?) to a window based on its origin. Such thing would require some
> user cooperation for exploitation, but not unthinkable. For example take
> a look here:
> https://github.com/QubesOS/qubes-issues/issues/1166
>
> So, I think we should prefix VM name to at least one of class/instance
> name. Not sure which one. Or maybe both?

Fair enough, I don't think there is any harm in just prefixing both of
them. Regarding the implementation, I will take a look at the current
way GUI information is handled and try to come up with a tentative patch
for this.

Best regards,
Daniel

[Manuel Amador (Rudd-O)]

On 10/24/2015 12:57 AM, Marek Marczykowski-Górecki wrote:
> We try hard to avoid any unnecessary modification of 3rd-party packages. > If possible, it should work with upstream package version. If kwin
can > be _configured_ to use _QUBES_VMNAME for grouping and such, that's
fine. > But if not, we should think about how to make that work with the
> upstream version.

Kwin can be told to do a wildly wide array of things based on X
properties. No need to do anything fancy like modifying things in Kwin.

--
Rudd-O
http://rudd-o.com/

[Marek Marczykowski-Górecki]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

It is now tracked here:
https://github.com/QubesOS/qubes-issues/issues/1953

- --
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
-----BEGIN PGP SIGNATURE-----

Version: GnuPG v2

iQEcBAEBCAAGBQJXKzlaAAoJENuP0xzK19csEgkH/iLeWnF/1mNwTlE0NCOjUWM8
et0z9YOJfD+kt3PlD954OaYG1qTUHvfBmW5w3Amu0/cApGns/L2Lr683Lzz9blEA
/pgLAk2cE9/FbC7rDSs627iJxPbDS/4fCCSl66hQ4NHb2M3F9Fk9VPQj2WKgNV8a
u9iOpXrEyi56Fb1/uSVJuJlhR4xYKWPTgfZHEnD2ippqAtbUWywJ5JFunERPB5uF
FVT7By5DeT43jMBD0wLGsWc+dDHWQuiEmGAnusSW4KE6AJmDcoA04ylOI+rkXxnM
DY39fhJYan9v/pBx3NyIKFH0AptNaBEscrgg4o0Krk+mJGmGu6p1yXjYFMcJQJU=
=uh7A
-----END PGP SIGNATURE-----