QSB #38: Qrexec policy bypass and possible information leak

151 views
Skip to first unread message

Marek Marczykowski-Górecki

unread,
Feb 19, 2018, 7:50:16 PM2/19/18
to qubes-announce, qubes-users, qubes-devel
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Dear Qubes Community,

We have just published Qubes Security Bulletin (QSB) #38:
Qrexec policy bypass and possible information leak.
The text of this QSB is reproduced below. This QSB and its accompanying
signatures will always be available in the Qubes Security Pack (qubes-secpack).

View QSB #38 in the qubes-secpack:

<https://github.com/QubesOS/qubes-secpack/blob/master/QSBs/qsb-038-2018.txt>

Learn about the qubes-secpack, including how to obtain, verify, and read it:

<https://www.qubes-os.org/security/pack/>

View all past QSBs:

<https://www.qubes-os.org/security/bulletins/>

```

---===[ Qubes Security Bulletin #38 ]===---

February 20, 2018


Qrexec policy bypass and possible information leak

Summary
========

One of our developers, Wojtek Porczyk, discovered a vulnerability in the way
qube names are handled, which can result in qrexec policies being bypassed, a
theoretical information leak, and possibly other vulnerabilities. The '$'
character, when part of a qrexec RPC name and/or destination
specification (like '$adminvm', '$default', or one of the variants of
'$dispvm') is expanded according to shell parameter expansion [1]
after evaluating the qrexec policy but before invoking the RPC handler
executable.

Impact
=======

1. Potential policy bypass. The qrexec argument value that is delivered to the
handler executable can be different from the value that is present in the
RPC policy at the time the policy is evaluated. This is especially
problematic when the policy is defined as a blacklist of arguments rather
than a whitelist, e.g. "permit any arguments to example.Call but
PROHIBITED". If an attacker were to call 'example.Call+PROHIBITED$invalid',
the argument would not match the blacklisted variable at the time of policy
evaluation, so it would be admitted. However, performing shell parameter
expansion on the argument results in the prohibited value, which is what the
actual handler receives.

2. Potential information leak. If the qrexec handler acts upon the argument,
the attacker could read or deduce the contents of those variables.

3. Other potential vulnerabilities. Some of the variables present in the
environment, like $HOME and $PATH, also contain characters that are not
permissible in qrexec names or arguments that could theoretically lead to
other classes of vulnerabilities, such as directory traversal.

Technical details
==================

The '$' character is used in several places in qrexec and is therefore an
allowed character in parameters to Qubes RPC calls. It is also allowed as part
of the RPC name. The validation code is as follows [2]:

static void sanitize_name(char * untrusted_s_signed, char *extra_allowed_chars)
{
unsigned char * untrusted_s;
for (untrusted_s=(unsigned char*)untrusted_s_signed; *untrusted_s; untrusted_s++) {
if (*untrusted_s >= 'a' && *untrusted_s <= 'z')
continue;
if (*untrusted_s >= 'A' && *untrusted_s <= 'Z')
continue;
if (*untrusted_s >= '0' && *untrusted_s <= '9')
continue;
if (*untrusted_s == '$' ||
*untrusted_s == '_' ||
*untrusted_s == '-' ||
*untrusted_s == '.')
continue;
if (extra_allowed_chars && strchr(extra_allowed_chars, *untrusted_s))
continue;
*untrusted_s = '_';
}
}

and is invoked as [3]:

sanitize_name(untrusted_params.service_name, "+");
sanitize_name(untrusted_params.target_domain, ":");

Those arguments are part of the basis of policy evaluation. If policy
evaluation was successful, the parameters are then forwarded to the destination
domain over qrexec, and the call is executed using the qubes-rpc-multiplexer
executable, which is invoked by a POSIX shell. The exact mechanism differs
between dom0 and other qubes [4]:

if self.target == 'dom0':
cmd = '{multiplexer} {service} {source} {original_target}'.format(
multiplexer=QUBES_RPC_MULTIPLEXER_PATH,
service=self.service,
source=self.source,
original_target=self.original_target)
else:
cmd = '{user}:QUBESRPC {service} {source}'.format(
user=(self.rule.override_user or 'DEFAULT'),
service=self.service,
source=self.source)

# ...

try:
subprocess.call([QREXEC_CLIENT] + qrexec_opts + [cmd])

For the dom0 case, these are the relevant parts from the executable referenced
as QREXEC_CLIENT above [5]:

/* called from do_fork_exec */
void do_exec(const char *prog)
{
execl("/bin/bash", "bash", "-c", prog, NULL);
}

/* ... */

static void prepare_local_fds(char *cmdline)
{
/* ... */
do_fork_exec(cmdline, &local_pid, &local_stdin_fd, &local_stdout_fd,
NULL);
}

/* ... */

int main(int argc, char **argv)
{
/* ... */

if (strcmp(domname, "dom0") == 0) {
/* ... */

prepare_local_fds(remote_cmdline);

For qubes other than dom0, the command line is reconstructed from the command
passed through qrexec [6]:

void do_exec(const char *cmd)
{
char buf[strlen(QUBES_RPC_MULTIPLEXER_PATH) + strlen(cmd) - RPC_REQUEST_COMMAND_LEN + 1];
char *realcmd = index(cmd, ':'), *user;

/* ... */

/* replace magic RPC cmd with RPC multiplexer path */
if (strncmp(realcmd, RPC_REQUEST_COMMAND " ", RPC_REQUEST_COMMAND_LEN+1)==0) {
strcpy(buf, QUBES_RPC_MULTIPLEXER_PATH);
strcpy(buf + strlen(QUBES_RPC_MULTIPLEXER_PATH), realcmd + RPC_REQUEST_COMMAND_LEN);
realcmd = buf;
}

/* ... */

#ifdef HAVE_PAM
/* ... */
shell_basename = basename (pw->pw_shell);
/* this process is going to die shortly, so don't care about freeing */
arg0 = malloc (strlen (shell_basename) + 2);

/* ... */

/* FORK HERE */
child = fork ();

switch (child) {
case -1:
goto error;
case 0:
/* child */

if (setgid (pw->pw_gid))
exit(126);
if (setuid (pw->pw_uid))
exit(126);
setsid();
/* This is a copy but don't care to free as we exec later anyways. */
env = pam_getenvlist (pamh);

execle(pw->pw_shell, arg0, "-c", realcmd, (char*)NULL, env);

/* ... */

#else
execl("/bin/su", "su", "-", user, "-c", realcmd, NULL);
perror("execl");
exit(1);
#endif

Notice that the '$' character is unescaped in all cases when it is passed to
the shell and is interpreted according to the rules of parameter expansion [1].

Mitigating factors
===================

Only the '$' shell special character character was allowed, so only the
corresponding simple form of parameter expansion is permitted [1]. The '{}'
characters are prohibited, so other forms of parameter expansion are not
possible. Had other characters like '()', been permitted, which is not the
case, this vulnerability would amount to code execution.

The qrexec calls that are present in a default Qubes OS installation and that
have, by default, a policy that would actually allow them to be called:

- do not contain the '$' character; and
- do not act upon differences in their arguments.

Therefore, this vulnerability is limited to custom RPCs and/or custom policies.

The attacker is constrained to preexisting environment variables and shell
special variables, which do not appear to contain very valuable information.

Since writing policies in the blacklist paradigm is a poor security practice in
general, it is perhaps less common among the security-conscious Qubes userbase.
All users who write custom RPCs or policies are henceforth advised to adopt the
whitelist paradigm.

Resolution
===========

We've decided to deprecate the '$' character from qrexec-related usage.
Instead, to denote special tokens, we will use the '@' character,
which we believe is less likely to be interpreted in a special way
by the relevant software.

This is a forward-incompatible change for existing systems, specifically in
policy syntax, remote domain parameters to the qrexec-client and
qrexec-client-vm tools, and the API exposed to the qrexec handler script. In
order to maintain backward compatibility, these tools will accept older
keywords while parsing policy and command line parameters, then translate them
to the new keywords before evaluating the policy or invoking the actual call,
respectively.

It will no longer be possible to define calls and receive arguments containing
the '$' character. However, we believe that no such calls exist. Had they
existed, this bug would have been disclosed earlier.

In addition, the shell will not be used to call qubes-rpc-multiplexer.

The environment variable specifying the original target qube will also be
specified differently for cases that, in the past, would have contained the '$'
character. However, this wasn't working as specified anyway, so we believe the
impact of this change to be minimal. The new variables will be as follows:

- - QREXEC_REQUESTED_TARGET_TYPE
with value of either 'name' or 'keyword'
- - QREXEC_REQUESTED_TARGET
set only when QREXEC_REQUESTED_TARGET_TYPE set to 'name'
- - QREXEC_REQUESTED_TARGET_KEYWORD
set only when QREXEC_REQUESTED_TARGET_TYPE set to 'keyword'

Patching
=========

The specific packages that resolve the problem discussed in this bulletin are
as follows:

For Qubes 3.2, dom0:
- qubes-utils 3.2.7
- qubes-core-dom0-linux 3.2.17

For Qubes 3.2, domUs:
- qubes-utils 3.2.7
- qubes-core-vm (Fedora) / qubes-core-agent (Debian) 3.2.24

For Qubes 4.0, dom0:
- qubes-utils 4.0.16
- qubes-core-dom0 4.0.23
- qubes-core-dom0-linux 4.0.11

For Qubes 4.0, domUs:
- qubes-utils 4.0.16
- qubes-core-agent 4.0.22

The packages for dom0 are to be installed in dom0 via the Qubes VM Manager or
via the qubes-dom0-update command as follows:

For updates from the stable repository (not immediately available):
$ sudo qubes-dom0-update

For updates from the security-testing repository:
$ sudo qubes-dom0-update --enablerepo=qubes-dom0-security-testing

The packages for domUs are to be installed in TemplateVMs and StandaloneVMs via
the Qubes VM Manager or via the respective package manager:

For updates to Fedora from the stable repository (not immediately available):
$ sudo dnf update

For updates to Fedora from the security-testing repository:
$ sudo dnf update --enablerepo=qubes-vm-*-security-testing

For updates to Debian from the stable repository (not immediately available):
$ sudo apt update && sudo apt dist-upgrade

For updates to Debian from the security-testing repository:
First, uncomment the line below "Qubes security updates testing repository" in
/etc/apt/sources.list.d/qubes-r*.list
Then:
$ sudo apt update && sudo apt dist-upgrade

A restart is required for these changes to take effect. In the case of dom0,
this entails a full system restart. In the case of TemplateVMs, this entails
shutting down the TemplateVM before restarting all the TemplateBasedVMs based
on that TemplateVM.

These packages will migrate from the security-testing repository to the current
(stable) repository over the next two weeks after being tested by the
community.

Timeline
=========

2011-07-22 Commit c23cc48 permits '$' character [7].
2016-03-27 Commit 0607d90 introduces qrexec arguments [8][9].
2018-02-14 The vulnerability is discovered and reported internally.
2018-02-20 The vulnerability is patched, and this bulletin is released.

Credits
========

The issue was discovered by Wojtek Porczyk.

References
===========

[1] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_02
[2] https://github.com/QubesOS/qubes-core-admin-linux/blob/v4.0.10/qrexec/qrexec-daemon.c#L643-L662
[3] https://github.com/QubesOS/qubes-core-admin-linux/blob/v4.0.10/qrexec/qrexec-daemon.c#L685-L686
[4] https://github.com/QubesOS/qubes-core-admin/blob/v4.0.22/qubespolicy/__init__.py#L452
[5] https://github.com/QubesOS/qubes-core-admin-linux/blob/v4.0.10/qrexec/qrexec-daemon.c
[6] https://github.com/QubesOS/qubes-core-agent-linux/blob/v4.0.21/qrexec/qrexec-agent.c#L136
[7] https://github.com/QubesOS/qubes-core-admin/commit/c23cc48#diff-3aa52ac2dd3e25700efd40e77b02b2d0
[8] https://github.com/QubesOS/qubes-core-admin-linux/commit/0607d90
[9] https://github.com/QubesOS/qubes-issues/issues/1876

- --
The Qubes Security Team
https://www.qubes-os.org/security/
```

- --
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-----

iQEzBAEBCAAdFiEEhrpukzGPukRmQqkK24/THMrX1ywFAlqLcJkACgkQ24/THMrX
1yz3vgf/Z+EGbCuLA/ZNH0KmDKZdtv1Zn1iO7lbVlsmN7vxdFiZaGX0m1oEj7czR
pNRCk6UE9w+ZSzD2UbKiq5RXBeZ/sMm4CsaDZmfOi0HdKEJbDvPa+S9jx7l8Zlu3
kVgtfBRgl+YNfjC8A1szAsuE3c2xuynvQmAjDTYLjsX/LekqORW+EL7xEkTyfH8a
Ijub5N3gyK97Wmu4MHuXqZwvMnirn9BzCL2r4aZKh24LIcXVA70qxQd4+Bg0qRgQ
6ZQttdR8dJVdlPbKrB+ob9uuOYX3syh2r6l54UBMzdTcqMMMSBv+5GJ8SCumVX5U
Lv5kbQjCKTlAR2c/s+kw2YmmdrSk1w==
=uOOl
-----END PGP SIGNATURE-----

Tom Zander

unread,
Feb 20, 2018, 7:21:37 AM2/20/18
to qubes-users, qubes-devel
On Tuesday, 20 February 2018 01:49:37 CET Marek Marczykowski-Górecki wrote:
> We've decided to deprecate the '$' character from qrexec-related usage.
> Instead, to denote special tokens, we will use the '@' character,
> which we believe is less likely to be interpreted in a special way
> by the relevant software.

I would argue against the @ sign on account that it is a special character
in bash as well.

Search for it here;
https://linux.die.net/man/1/bash
I don't immediately see a way to exploit it, but why risk it?

--
Tom Zander
Blog: https://zander.github.io
Vlog: https://vimeo.com/channels/tomscryptochannel


Wojtek Porczyk

unread,
Feb 20, 2018, 8:04:15 AM2/20/18
to Tom Zander, qubes-users, qubes-devel
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On Tue, Feb 20, 2018 at 01:21:30PM +0100, 'Tom Zander' via qubes-devel wrote:
> On Tuesday, 20 February 2018 01:49:37 CET Marek Marczykowski-Górecki wrote:
> > We've decided to deprecate the '$' character from qrexec-related usage.
> > Instead, to denote special tokens, we will use the '@' character,
> > which we believe is less likely to be interpreted in a special way
> > by the relevant software.
>
> I would argue against the @ sign on account that it is a special character
> in bash as well.
>
> Search for it here;
> https://linux.die.net/man/1/bash
> I don't immediately see a way to exploit it, but why risk it?

We absolutely need a special character that is not allowed in qube name to
make the special tokens immediately obvious in policy. The process I used was
to list available characters (POSIX Portable Character Set [1]) and substract
the characters that are special to some relevant things:
- - qube name: a-z A-Z 0-9 _ -
- - POSIX shell [2]: |&;<>()$`\\"'*?[#~=% and the space, tab and newline
- - POSIX shell reserved words [3]: ! { }
- - non-POSIX things [3]: [ ]
- - special qrexec character: +
- - path separators (POSIX and NT): / \ :
- - regular expressions: ^. (and other already excluded)

This leaves: '\0\a\b,@'. The point is, all characters are special to
something. I'm sure if I searched for more "special" characters, I'd find them
('\0' is special to C strings, '\a' and '\b' to terminal, '@' in emails, and
',' we use in other context in policy). So I stopped there and by consensus we
picked '@'.

If I missed something, could you please point out? I know shell just good
enough to know that it's not possible to know every shell quirk. :)

[1] http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap06.html
[2] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_02
[3] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_04


- --
pozdrawiam / best regards _.-._
Wojtek Porczyk .-^' '^-.
Invisible Things Lab |'-.-^-.-'|
| | | |
I do not fear computers, | '-.-' |
I fear lack of them. '-._ : ,-'
-- Isaac Asimov `^-^-_>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJajBzCAAoJEL9r2TIQOiNRCvUQALLk151eBxc4URbBvWMQhTFy
24T8WygKzNutv+cIW/YlY1FVrPUcwtlJMCbj99mX45ZanHZLDNTEMKC6J1WS4tlg
SOo0r0U0SnAbUiNvKTsMiydRZDt05gmgxITRxxpCK0FN9WvxEZDF2N67XIwyd0nm
0bPyj3cguN5FJlW6dWkUS6/XoLCn6fCX6hGd0wvJFaujGcQkrL6gKYncp8dS3VfO
72hZd04vraZFaBEIg2kPhtff5jGpZaB/gI6wfZVeZKzewlHimVz/Efneh8bdeU7f
MSTqwp+RJOCIHXPxPjs3ARZQYPQIsj6XDL+UFk47sZK8p5fNRlDu9tgsWYHpMPwQ
TrvEkzAdVXHJpb6prpStfI0gLdcI9TmR8D2hCMEyHYICc9cahET3TPyVmuyKoBuf
BzLfhaLhv1mC6/aNR+/kgEAfW0ZZM8o6Dedbccz8Tuu/fc8c2A3JqlOKbVcmBc3x
9wK8CTrY08dsse8YnywbaxhK5+o01miaL3Uhf9LTbyxsVNAlvavdUWnlrxl/1e3O
2f4bTIFXLJ4jLpsmy+e0Itg1/IKnPWZs9uCouDZ8G601pz9p4ORPZJwWa9Cykllb
/PAlVwx/GJm2qPGxP4lxX2+2T2QXRhoCJTHBp9zoFyzQ7xqTFALEZtC3CRWvT1dr
t9joU8uqhcS4Wt6nA9lh
=EN6G
-----END PGP SIGNATURE-----

Tom Zander

unread,
Feb 20, 2018, 10:27:23 AM2/20/18
to Wojtek Porczyk, qubes-users, qubes-devel
On Tuesday, 20 February 2018 14:04:03 CET Wojtek Porczyk wrote:
> On Tue, Feb 20, 2018 at 01:21:30PM +0100, 'Tom Zander' via qubes-devel
wrote:
> > On Tuesday, 20 February 2018 01:49:37 CET Marek Marczykowski-Górecki
wrote:
> > > We've decided to deprecate the '$' character from qrexec-related
> > > usage.
> > > Instead, to denote special tokens, we will use the '@' character,
> > > which we believe is less likely to be interpreted in a special way
> > > by the relevant software.
> >
> > I would argue against the @ sign on account that it is a special
> > character in bash as well.
> >
> > I don't immediately see a way to exploit it, but why risk it?
>
> We absolutely need a special character that is not allowed in qube name to
> make the special tokens immediately obvious in policy. The process I used
> was to list available characters (POSIX Portable Character Set [1])
[]
> If I missed something, could you please point out? I know shell just good
> enough to know that it's not possible to know every shell quirk. :)

The thing you have to rememeber is that the escape character never needs to
be typed by the user.
In QRexec you are defining an API, applications like qvm-run are using that
API. What the user passes into qvm-run and what is actually sent to dom0
does not have to be identical.
I guess you do the translation currently as well; '$' turns into '@' in your
new code.

The consequence of this is that you don't have to limit yourself to the
posix list.
Using the portable characters set for a non-character simply isn't needed.

So, knowing that your API is actually based on 8-bit characters and not 7
bits which you are limiting yourself to, my suggestion is to take something
above 127 and below 256 as a special char.
Most fun one would be “ÿ” which is a normal character you can pass on a
shell script if you must, its actual byte-value is 0xFF

Marek Marczykowski-Górecki

unread,
Feb 20, 2018, 10:55:17 AM2/20/18
to Tom Zander, Wojtek Porczyk, qubes-users, qubes-devel
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On Tue, Feb 20, 2018 at 04:27:16PM +0100, 'Tom Zander' via qubes-devel wrote:
> On Tuesday, 20 February 2018 14:04:03 CET Wojtek Porczyk wrote:
> > On Tue, Feb 20, 2018 at 01:21:30PM +0100, 'Tom Zander' via qubes-devel
> wrote:
> > > On Tuesday, 20 February 2018 01:49:37 CET Marek Marczykowski-Górecki
> wrote:
> > > > We've decided to deprecate the '$' character from qrexec-related
> > > > usage.
> > > > Instead, to denote special tokens, we will use the '@' character,
> > > > which we believe is less likely to be interpreted in a special way
> > > > by the relevant software.
> > >
> > > I would argue against the @ sign on account that it is a special
> > > character in bash as well.
> > >
> > > I don't immediately see a way to exploit it, but why risk it?
> >
> > We absolutely need a special character that is not allowed in qube name to
> > make the special tokens immediately obvious in policy. The process I used
> > was to list available characters (POSIX Portable Character Set [1])
> []
> > If I missed something, could you please point out? I know shell just good
> > enough to know that it's not possible to know every shell quirk. :)
>
> The thing you have to rememeber is that the escape character never needs to
> be typed by the user.
> In QRexec you are defining an API, applications like qvm-run are using that
> API. What the user passes into qvm-run and what is actually sent to dom0
> does not have to be identical.

In theory yes, but this would introduce more complexity to this code
(taking care where which encoding is used etc).

> I guess you do the translation currently as well; '$' turns into '@' in your
> new code.
>
> The consequence of this is that you don't have to limit yourself to the
> posix list.
> Using the portable characters set for a non-character simply isn't needed.
>
> So, knowing that your API is actually based on 8-bit characters and not 7
> bits which you are limiting yourself to, my suggestion is to take something
> above 127 and below 256 as a special char.
> Most fun one would be “ÿ” which is a normal character you can pass on a
> shell script if you must, its actual byte-value is 0xFF

Until some helpful application (shell or else) will try to interpret it
as UTF-8.

- --
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-----

iQEzBAEBCAAdFiEEhrpukzGPukRmQqkK24/THMrX1ywFAlqMRLUACgkQ24/THMrX
1yxCegf+Iii677oWd0CmJgoygfVfiQnmDl+a7XBX/i+tb8BMqO67AgwzoM6cWXq6
ZaA76a50qKSmcSjj6xSPtg4sPV0hqpgORsnxikAn5zg9vi7QJMJ0q+hKuKVxHAY1
TZSVFynTs6ci0JjgVRiB8QZCrl2eC9hQraGs46u6Zevvj80ZapaEqu0Sh0rowpDe
SZ+QbiKi/QD1IeSF03OjnlqtoEyAZtPJ4dMY9F8IpR0P/vzsPxnkx/493HVjSA1i
7Z7kutdCcrGAqCtROhQ9DnS7+GTfdNcDJ5zwZ5yz5fJWlrzFgDSjENuwrSUqU/13
W6HNQVwx/fW+RBseUkJ/o98GHVW8sg==
=af4O
-----END PGP SIGNATURE-----

Tom Zander

unread,
Feb 20, 2018, 12:57:42 PM2/20/18
to Marek Marczykowski-Górecki, Wojtek Porczyk, qubes-users, qubes-devel
On Tuesday, 20 February 2018 16:54:36 CET Marek Marczykowski-Górecki wrote:
> > The thing you have to rememeber is that the escape character never needs
> > to be typed by the user.
> > In QRexec you are defining an API, applications like qvm-run are using
> > that API. What the user passes into qvm-run and what is actually sent
> > to dom0 does not have to be identical.
>
> In theory yes, but this would introduce more complexity to this code
> (taking care where which encoding is used etc).

I read the code, there is no encoding.
You correctly used the POSIX Portable Character Set for text. So no need
for encoding.
When you use the qrexec API you just sent a struct with some arrays of bytes
for VM names.
In your qrexec code you use an array of unsigned chars. Also, no encoding.

The point is that you use encodings only when you have **text** with
characters > 127. Which you don't allow.

The problem you fear doesn't exist.

The reason is because when accepting user-input you use encodings.

When your app starts talking to qrexec/qubsed there is no longer any
encoding. Just an 8-bit bytearray. The text has been standardized.

On the 'other' side of qrexec (on dom0) you have perfect control over the
situation and you also don't have any need for recoding or encodings or
anything like that. It still is just 8 bits data, not encoded.

> > I guess you do the translation currently as well; '$' turns into '@' in
> > your new code.
> >
> > The consequence of this is that you don't have to limit yourself to the
> > posix list.
> > Using the portable characters set for a non-character simply isn't
> > needed.
> >
> > So, knowing that your API is actually based on 8-bit characters and not
> > 7
> > bits which you are limiting yourself to, my suggestion is to take
> > something above 127 and below 256 as a special char.
> > Most fun one would be “ÿ” which is a normal character you can pass on a
> > shell script if you must, its actual byte-value is 0xFF
>
> Until some helpful application (shell or else) will try to interpret it
> as UTF-8.

Ehm, how would “some helpful application” manage to get in your qrexec
policy-frameowork? If you fear that you have bigger issues as they could
replace anything with anything...

Anyway, to answer your fear.

No. UTF-8 doesn't allow 0xFF, it will just tell you the stream is broken.
(see attached example file) Or, more likely, it will just switch off utf-8.
escaped

Marek Marczykowski-Górecki

unread,
Feb 20, 2018, 1:42:03 PM2/20/18
to Tom Zander, Wojtek Porczyk, qubes-users, qubes-devel
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On Tue, Feb 20, 2018 at 06:57:34PM +0100, Tom Zander wrote:
> On Tuesday, 20 February 2018 16:54:36 CET Marek Marczykowski-Górecki wrote:
> > > The thing you have to rememeber is that the escape character never needs
> > > to be typed by the user.
> > > In QRexec you are defining an API, applications like qvm-run are using
> > > that API. What the user passes into qvm-run and what is actually sent
> > > to dom0 does not have to be identical.
> >
> > In theory yes, but this would introduce more complexity to this code
> > (taking care where which encoding is used etc).
>
> I read the code, there is no encoding.
> You correctly used the POSIX Portable Character Set for text. So no need
> for encoding.
> When you use the qrexec API you just sent a struct with some arrays of bytes
> for VM names.
> In your qrexec code you use an array of unsigned chars. Also, no encoding.
>
> The point is that you use encodings only when you have **text** with
> characters > 127. Which you don't allow.
>
> The problem you fear doesn't exist.
>
> The reason is because when accepting user-input you use encodings.
>
> When your app starts talking to qrexec/qubsed there is no longer any
> encoding. Just an 8-bit bytearray. The text has been standardized.
>
> On the 'other' side of qrexec (on dom0) you have perfect control over the
> situation and you also don't have any need for recoding or encodings or
> anything like that. It still is just 8 bits data, not encoded.

And then, after policy evaluation, you pass that data to actual service
to execute the operation (which may be in dom0 or another VM).

> > > I guess you do the translation currently as well; '$' turns into '@' in
> > > your new code.
> > >
> > > The consequence of this is that you don't have to limit yourself to the
> > > posix list.
> > > Using the portable characters set for a non-character simply isn't
> > > needed.
> > >
> > > So, knowing that your API is actually based on 8-bit characters and not
> > > 7
> > > bits which you are limiting yourself to, my suggestion is to take
> > > something above 127 and below 256 as a special char.
> > > Most fun one would be “ÿ” which is a normal character you can pass on a
> > > shell script if you must, its actual byte-value is 0xFF
> >
> > Until some helpful application (shell or else) will try to interpret it
> > as UTF-8.
>
> Ehm, how would “some helpful application” manage to get in your qrexec
> policy-frameowork? If you fear that you have bigger issues as they could
> replace anything with anything...

I'm not sure if you've read the QSB carefully. The problem is how the
thing executing the service handle arguments. Not policy evaluating part
- - there '$' or other potentially shell special characters are handled
correctly. Anyone can write a qrexec service, and we want to provide a
framework that make it hard to shoot yourself in the foot.

> Anyway, to answer your fear.
>
> No. UTF-8 doesn't allow 0xFF, it will just tell you the stream is broken.
> (see attached example file) Or, more likely, it will just switch off utf-8.
>



- --
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-----

iQEzBAEBCAAdFiEEhrpukzGPukRmQqkK24/THMrX1ywFAlqMa8cACgkQ24/THMrX
1yyE6gf+McyBJoK/NW4KAfYEL5xritven8tl35yXaWND674jddKz/WXMaaf9mdar
338xMDBLoeDMZvYVpujd2+lbbnwyMPOUvxGtDQ87R+mluWX5YkWcrzNgVWhrjbJu
GTczmlkCT+PaW5cYonXCvFMzGyB3Afu2T7BMBD947RU2SUpiaooAfUUqpS1dCGG8
cQF8xe2Ab3vCBF1e1ocOEF5KRYgsC8NsTc1KDqlDp9AORqfLOINHv0ZNFS/Gaksn
CLqvKO5VPr+oswHm8v5I4MLQPK5cQuFvn7oJOs3+pOzZqVk9y7xvjZ4xKiVNF4pB
tK0B3Tpp3F0VzI9fhAoYHi4pI2PSUg==
=mIGj
-----END PGP SIGNATURE-----

Tom Zander

unread,
Feb 20, 2018, 1:54:32 PM2/20/18
to Marek Marczykowski-Górecki, Wojtek Porczyk, qubes-users, qubes-devel
On Tuesday, 20 February 2018 19:41:19 CET Marek Marczykowski-Górecki wrote:
> > On the 'other' side of qrexec (on dom0) you have perfect control over
> > the
> > situation and you also don't have any need for recoding or encodings or
> > anything like that. It still is just 8 bits data, not encoded.
>
> And then, after policy evaluation, you pass that data to actual service
> to execute the operation (which may be in dom0 or another VM).

Yes, WITHOUT the escape character.

Remember, you escape the special names of VM names that dom0 will
substitute. “$adminvm” doesn't end up being the string you offer to qubesd,
the string “dom0” is.

Likewise; you don't start a service in Dispvm18431 and send it the text
“$dispvm”.

Wojtek Porczyk

unread,
Feb 21, 2018, 6:12:19 AM2/21/18
to Tom Zander, qubes-users, qubes-devel
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Thank you for the suggestion, but I don't think it's correct.

The character has to be input in at least two places: in /etc/qubes-rpc/policy
as the second token (destination) on the line and as argument to
qrexec-client[-vm] executable. Using any of the common editors, any
language-specific keyboard layout, and any common encoding. Most people have
UTF-8, or ISO-8859-*, but we don't exclude the possibility to have admin qube
on Windows -- there was at least one serious attempt -- so this brings UTF-16
and Windows-125*.

As and example, may I use ÿ character you provided:
1) You're right the codepoint is U+00FF, but UTF-8 encoding is actually
"\xc3\xbf", so no, we cannot use it.
2) I don't have it on my keyboard. So anytime I have to input one of those
characters, I search all the modifiers for the right one (ý? no. ŷ? neither.
ỹ? my font has trouble with that, is that even a letter? ý? tried this one
already...). I don't have real data, but I think most people don't even know
where to start looking for this and in the optimistic case will end up
sourcing it from gucharmap or equivalent. This is bad UX.

Maybe there is a character outside portable charset that is portable and
writable enough, but I don't know of any. I haven't thought there is hope
enough to actually find one, so I didn't bother searching. That's why I've
asked.


Again, thanks for your review. I think it's helpful, because this change was
made behind community's back (for obvious reasons), fast, and in very limited
group of people. I wasn't sure if we didn't make some mistake, so the best
what I could hope for was to explain myself and get ex post facto review,
which you provided.


- --
pozdrawiam / best regards _.-._
Wojtek Porczyk .-^' '^-.
Invisible Things Lab |'-.-^-.-'|
| | | |
I do not fear computers, | '-.-' |
I fear lack of them. '-._ : ,-'
-- Isaac Asimov `^-^-_>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJajVQEAAoJEL9r2TIQOiNRpbUQAJjEEAk+rKZOrFjjMkG3WQem
/KNCL9gfVt3T6/keBBuEwfX3XcOIiO/FBWNfcf6dxeBGGcMHHQn0pd4Ucj/HZw8b
0/s63gjXH+ru7m4x2VW/3uDI4igkic6UUYPVHDB0sQtbTvGGWsr5pPJxcx7JgbwX
+mJmDgt7i/9Y3lAGEva5ex+q7WG4hJd8ArgnJGAVnp7MrTgIduHW1/2QufC6uvvE
gRRc3gbZK5FkT5Yg38UumE4sNcmnV0Nvu3m+o/g/cBcEER7wO81XW6TKFj0Ok/Bg
Ostsov9NwO3iGv0usSUvMKfw7Aac3VK9SsW0r5sxA/QFe9jVvasVnmvIrxTRwwL+
W+gP5piagxgphLhUcR6LwyEhRPWzb06iDaaztXnLXyInWFEGdei1ATmlQNI0Rmno
pNh/QLQqS6YF+hAl8LxSkOj3tjcg7MTYl00y9Z6ePJRUDA84s1hlWT43agWNN4L3
SX55/UzTU8DlhcduL3WmY6DVIKKlfE2Q82VorkprY6i/u/d7fdCblYLOtatZh2JB
OK1ZFOprRlAJodYQMUws7o8cDgY3LxfgKX45PC23DJG6o5CDM+WoqmUw72uxMFft
jRE299HwUV3qfzxm9/bjLPJqgnP+nSFnY/4J02iHUQxkg3Xb9ibKxFmTWhnf78zd
REwcpFmrrjnrVVy14CZ2
=wkxM
-----END PGP SIGNATURE-----

Tom Zander

unread,
Feb 21, 2018, 7:06:53 AM2/21/18
to Wojtek Porczyk, qubes-users, qubes-devel
On Wednesday, 21 February 2018 12:12:06 CET Wojtek Porczyk wrote:
> This is bad UX.

This is frustrating, I spent too many emails making the point clear that
this is an API level escape token. Not a user-visible one, and then you
respond to the thread showing you still completely missed that.

So let me be blunt as this is likely the last email from me to qubes anyway;

Fact: Variables given to qrexec are going to be replaced with the actual
relevant value.
For instance bash takes`ls *` and replaces the star with the actual values
_before_ calling ls. Ls or any executable does not have to deal with things
like star or dollar sign etc.

Your and Marek complaints are that you need to escape the variables when you
pass them on to the target VM handler.
If you are indeed doing that, you are doing it wrong and you can wait for
the next security bulletin like the one we are discussing right now.

The point of a variable that is passed from a VM to the dom0 qrexec daemon
is that your source VM doesn't have to know about who is $adminVM or what is
the actually started dispVM's name.
QRexec daemon (in dom0) should do the variable replacement before the user
request leaves qrexec-daemon running in dom0.
Just like bash does the replacement before it forwards the command-line.


Again, if you do not do the variable replacement there, but instead pass it
through unvalidated and unrelated software, you are going to continue having
security flaws.

qubenix

unread,
Feb 21, 2018, 11:41:40 AM2/21/18
to Tom Zander, qubes-users, qubes-devel
> So let me be blunt as this is likely the last email from me to qubes
anyway;

Bye, I'm happy to see you go away finally. It was killing me inside that
you were working on the new gui controller. Fuck off back to bcash.

--
qubenix
GPG: B536812904D455B491DCDCDD04BE1E61A3C2E500

awokd

unread,
Feb 21, 2018, 11:49:34 AM2/21/18
to qubenix, Tom Zander, qubes-users, qubes-devel
On Wed, February 21, 2018 4:41 pm, qubenix wrote:
>> So let me be blunt as this is likely the last email from me to qubes
>>
> anyway;
>
> Bye, I'm happy to see you go away finally. It was killing me inside that
> you were working on the new gui controller. Fuck off back to bcash.

Tom had some constructive criticism to offer, which is healthy. I don't
think your response is particularly constructive or appropriate for the
mailing lists, however.


Pedro Martins

unread,
Feb 21, 2018, 3:59:35 PM2/21/18
to qubes...@googlegroups.com

On 20-02-2018 00:49, Marek Marczykowski-Górecki wrote:
> Resolution
> ===========
>
> We've decided to deprecate the '$' character from qrexec-related usage.
> Instead, to denote special tokens, we will use the '@' character,
> which we believe is less likely to be interpreted in a special way
> by the relevant software.
>

Maybe a bit late to the party but I would like to suggest that instead
of using another character, you can transform something you don't
control (special characters) into something you can (normal characters).

For example, by simple encoding (after sanitation) $adminVM to text
string 2461646D696E564D (hex values of corresponding ascii character)
and only decoding it when appropriate (and you control this), you avoid
interpretation of special characters outside of where it is meaningful
while still keeping forward compatibility.
--
Pedro Martins

Wojtek Porczyk

unread,
Feb 21, 2018, 5:13:01 PM2/21/18
to Pedro Martins, qubes...@googlegroups.com
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On Wed, Feb 21, 2018 at 08:58:52PM +0000, Pedro Martins wrote:
> On 20-02-2018 00:49, Marek Marczykowski-Górecki wrote:
> >Resolution
> >===========
> >
> >We've decided to deprecate the '$' character from qrexec-related usage.
> >Instead, to denote special tokens, we will use the '@' character,
> >which we believe is less likely to be interpreted in a special way
> >by the relevant software.
> >
>
> Maybe a bit late to the party

No, that's fine. By that measure, whole list is late to the party, since we
had to release QSB together with a working patchset for those who are not
interested in bikeshedding about the character. I think this is important
issue, because this QSB is result of our own mistakes and not some problem
with Xen, which was generally a reccuring theme in recent bulletins. We're
still before -rc5, so if someone came up with something really outstanding,
we are open, though I'd prefer not to change things twice.

> but I would like to suggest that instead of
> using another character, you can transform something you don't control
> (special characters) into something you can (normal characters).

> For example, by simple encoding (after sanitation) $adminVM to text string
> 2461646D696E564D (hex values of corresponding ascii character) and only
> decoding it when appropriate (and you control this), you avoid
> interpretation of special characters outside of where it is meaningful while
> still keeping forward compatibility.

This idea floated as part of internal deliberation and we decided against. The
proposal differed from yours, but the argument is the same: we'd like to avoid
any translation/interpretation/acting upon untrusted string as much as
possible. We aim for simple code paths, and just checking the character
against known value seems simpler that translating strings we don't control
back and forth. That's also reason why patch for sanitisation function differs
between R3.2 and R4.0: 3.2 is stable, released version and compatibility is
more important (so we have translation there), but 4.0 is technically
unsupported release candidate and the slight incompatibility is justified by
simplified logic.


- --
pozdrawiam / best regards _.-._
Wojtek Porczyk .-^' '^-.
Invisible Things Lab |'-.-^-.-'|
| | | |
I do not fear computers, | '-.-' |
I fear lack of them. '-._ : ,-'
-- Isaac Asimov `^-^-_>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJaje7fAAoJEL9r2TIQOiNRxtQP/RGWTjY+OefP1Gpa2mUkt2Sb
veWPPZyXrepaNkYCXOqOSCI+qr/x+JCzCz45otdf8wes63LfYqia1wcg87eByRB9
X79VYwRQNT93cd8MxhM94hHdHBThBHaQ+72DcSBXHGnmUYwmSIbJRkLypF8yM6/S
mhDooPOwc/nCNM+aDC7tAdFLyBolsgqmZ8jw28RcUHb48/IF53UUmcwixHWSm2rg
LwybNNO+dsuy7OwFjgwF6dSepQ/9Yox4rwNBNT59QQHXOFqxVZcZCr3zZPJGaqpq
V15/dSijLtydeNp9MMyLMZZmnlh5CQ0xkNChshnbwk7exHMROfV8wGedMUfpx5R3
+S6TknLXnZxsF9q0X07Bw5x6Kl1RSZTtAEA4kGwFmwNPOcusx1F3PhjTAGb8SGil
ufFJuA/lCUnbj+hCVTsvQ9u2lah4UT29ZoBT3jZVcBkHguVSw/UGuzemq0bT9dIb
ccDzFXcsaCdHKidj1NgOSv77sZutL8AFcdUvCHc6zzXjmu48PfhHGX/+mNHMRkGG
o4O++k/4RBN6Hfa5RiIxyPHP1LAipOcLgLHAmMN8cgzHx8SUIVDnL1jpwgzq+yRb
2R3x0weHILtyMA/ThwjQR/j18vcwXLPdWhpb3zUohfWDz1m9856bT81QInFzrp1h
8SQF3tsjmNM6rCm3CSCO
=xc94
-----END PGP SIGNATURE-----

awokd

unread,
Feb 23, 2018, 3:50:46 AM2/23/18
to qubes-devel
On Wed, February 21, 2018 11:35 am, 'Tom Zander' via qubes-devel wrote:

> The point of a variable that is passed from a VM to the dom0 qrexec
> daemon is that your source VM doesn't have to know about who is $adminVM
> or what is the actually started dispVM's name. QRexec daemon (in dom0)
> should do the variable replacement before the user request leaves
> qrexec-daemon running in dom0. Just like bash does the replacement before
> it forwards the command-line.

Not to beat a dead horse, but is there anything to this concept? Should
the variable expansion take place somewhere earlier in the pipeline
instead of at the target, or is that not something to worry about?



bow...@gmail.com

unread,
Feb 23, 2018, 10:44:07 PM2/23/18
to qubes-devel
I fail to grasp fully the problem. Could you maybe provide an example of an attack (sourceVM script and sourceVMname, policy and targetVM script).

After multiple rewording on my understanding and reading some doc I have the feeling that I am getting closer.

The policy validation service evaluate the policy file and does not interpolate the string with $. It compares $variable (as a string) with a set of strings (i.e. $dispvm, $tag, etc...). Are you not able to white-list this part?
Or to use non authorized character in the policy file but keep $ in the sourceVMscript (is it what you suggested). Or use a trailing character as well.
i.e. anon-whonix %dispvm% allow,target=%dispvm%:anon-whonix-dvm

Another option might be to play with the formatting of the policy files (multiple line per rule)?
i.e. 0 anon-whonix 1 dispvm allow,target=1 dispvm:0 anon-whonix-dvm
('1 ' is $ '0 ' no $).

Is there not also a problem after the policy evaluation with the string substitution itself in Dom0? policy should be more explicit when no argument is allowed.
i.e.:
/etc/qubes-rpc/policy/FileCopy+ (default argument policy)
/etc/qubes-rpc/policy/FileCopy (no argument policy)
/etc/qubes-rpc/policy/FileCopy+blah (argument blah policy)

bow...@gmail.com

unread,
Feb 24, 2018, 5:00:24 AM2/24/18
to qubes-devel
Please ignore. reading the commits was the for me. Apologies for the noise.

Marek Marczykowski-Górecki

unread,
Feb 24, 2018, 6:21:14 AM2/24/18
to aw...@danwin1210.me, qubes-devel
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

The problem is that '$' keywords in some places (like call argument, or
original target specification) are not meant to be expanded _at all_. And
since '$' is a special character in shell used for variables, it's
enough to forget escape it in just one place to have to cause problems.
And as this QSB shows - we did forget about one place. We don't want
take chances if that was the only place we forget about it (now or in
the future) and that's why we've decide to change that character to
something safe.

- --
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-----

iQEzBAEBCAAdFiEEhrpukzGPukRmQqkK24/THMrX1ywFAlqRSpAACgkQ24/THMrX
1yxiSwf/eZ6DusuOeMevRlGyPUiqK1Nq+skh/+4itFVUfchQ0ndXmzjm8Hjzd2RY
C/OZHLfTCNH3MJ6VeNO5eBEepZ5rkzeyuMua+GrvCpc/riAmLXRObc2YwoR93bpf
jm4Bn/qcSE/YugR6OceuzuL3SUHJOEr4nt1ZPlnogiuT0QXE7fKaf9c3Vy+OIiKD
XlBA2Bc65mBkA6v2MXFwygiUsk+jABWluU4TWYyjnwO9Nu+HfMD08iLaC+tvk3GL
CGw9YHgzVqG39MiC30wonwJR5d2GZIka224oKtKxa3hDzZmMtD9g5uh9NV0i77V8
Eg7fGEwyBU6ocSQc4QC8jmFZjxcqMw==
=Ptbw
-----END PGP SIGNATURE-----

awokd

unread,
Feb 24, 2018, 6:34:46 AM2/24/18
to qubes-devel
On Sat, February 24, 2018 11:20 am, Marek Marczykowski-Górecki wrote:

> The problem is that '$' keywords in some places (like call argument, or
> original target specification) are not meant to be expanded _at all_. And
> since '$' is a special character in shell used for variables, it's enough
> to forget escape it in just one place to have to cause problems. And as
> this QSB shows - we did forget about one place. We don't want take chances
> if that was the only place we forget about it (now or in the future) and
> that's why we've decide to change that character to something safe.

That sounds completely logical. Thank you for summarizing it for me.


Reply all
Reply to author
Forward
0 new messages