Re: [qubes-devel] QSB #38: Qrexec policy bypass and possible information leak

2018-02-24 Thread 'awokd' via 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.


-- 
You received this message because you are subscribed to the Google Groups 
"qubes-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to qubes-devel+unsubscr...@googlegroups.com.
To post to this group, send email to qubes-devel@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/qubes-devel/f002120a209dbf50b01c42ebdda3a722.squirrel%40tt3j2x4k5ycaa5zt.onion.
For more options, visit https://groups.google.com/d/optout.

Re: [qubes-devel] QSB #38: Qrexec policy bypass and possible information leak

2018-02-24 Thread Marek Marczykowski-Górecki 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

On Fri, Feb 23, 2018 at 08:50:09AM -, 'awokd' via qubes-devel wrote:
> 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?

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-

-- 
You received this message because you are subscribed to the Google Groups 
"qubes-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to qubes-devel+unsubscr...@googlegroups.com.
To post to this group, send email to qubes-devel@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/qubes-devel/20180224112024.GM2023%40mail-itl.
For more options, visit https://groups.google.com/d/optout.

Re: [qubes-devel] QSB #38: Qrexec policy bypass and possible information leak

2018-02-23 Thread 'awokd' via 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?



-- 
You received this message because you are subscribed to the Google Groups 
"qubes-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to qubes-devel+unsubscr...@googlegroups.com.
To post to this group, send email to qubes-devel@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/qubes-devel/eff4a6e79834a62d061b4fad52b278a7.squirrel%40tt3j2x4k5ycaa5zt.onion.
For more options, visit https://groups.google.com/d/optout.

Re: [qubes-devel] QSB #38: Qrexec policy bypass and possible information leak

2018-02-21 Thread Wojtek Porczyk 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

On Wed, Feb 21, 2018 at 08:58:52PM +, 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-

-- 
You received this message because you are subscribed to the Google Groups 
"qubes-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to qubes-devel+unsubscr...@googlegroups.com.
To post to this group, send email to qubes-devel@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/qubes-devel/20180221221248.GO1198%40invisiblethingslab.com.
For more options, visit https://groups.google.com/d/optout.

Re: [qubes-devel] QSB #38: Qrexec policy bypass and possible information leak

2018-02-21 Thread Pedro Martins 


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

--
You received this message because you are subscribed to the Google Groups 
"qubes-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to qubes-devel+unsubscr...@googlegroups.com.
To post to this group, send email to qubes-devel@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/qubes-devel/b9ac801c-4888-83d2-7181-4d86342c2059%40gmail.com.
For more options, visit https://groups.google.com/d/optout.

[qubes-devel] QSB #38: Qrexec policy bypass and possible information leak

2018-02-19 Thread Marek Marczykowski-Górecki 
-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:



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



View all past QSBs:



```

 ---===[ 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, _pid, _stdin_fd,