Re: [Qemu-devel] [PATCH] Bind VNC to localhost unless otherwise specified to increase security

[Attila-Mihaly Balazs]

Hello,

I think that QEMU is a great project (in fact I've written by BS
thesis about it many years back - creating a sandbox to analyze
suspicious program behavior). Sadly I don't have the bandwidth to
follow up on this any further. I'm still convinced that binding any
listening socked to localhost by default is much better since most
people don't change the defaults (and - as you pointed out - this
should be done for other services that listen on TCP not just VNC).

I also take that this is a breaking change and I don't have any great
data to substantiate that it should be a priority. My worry comes from
sites like vncroulette.com which scan the entire IPv4 address space
for open VNC servers and anecdotally what I tended to observe was that
there were many QEMU instances in these scans.

Unfortunately vncroulette.com is now defunct but here is an outline if
someone wants to investigate this:

- buy from shodan.io the list of VNC servers (it currently knows about
~330k) - it would cost arount 100USD
- use a small Perl script for example to iterate over them and detect
ones which (a) don't have any authentication and (b) run QEMU.

Ie. something like:

use Net::VNC;

my $vnc = Net::VNC->new({hostname => '127.0.0.1', port => 5901});
$vnc->login;

if ($vnc->name =~ /^QEMU/) {
  print "Vulnerable QEMU VNC server at: ..."
};

This could lead to some more quantified data which hopefully can
convince people about the importance of this and similar changes.

Kind regards!
Attila Balazs (Grey Panther)

On Tue, Jun 7, 2016 at 11:42 PM, Gerd Hoffmann <address@hidden> wrote:
> On Di, 2016-06-07 at 20:51 +0300, Attila-Mihaly Balazs wrote:
>> >
>> > I understand the reason you want to do this change, but I don't really
>> > like the fact that this is making "empty hostname" semantics for the -vnc
>> > option, diverge from the "empty hostname" semantics for other QEMU args
>> > like -chardev.  The main point of having all of QEMU use the same code
>> > for sockets listen/connect setup via the InetSocketAddress struct is that
>> > we gain consistent semantics across the whole codebase. This change to
>> > VNC code is throwing away that consistency, so I'm against this change
>> > really.
>> >
>>
>> Daniel, thank you for taking the time to consider this patch. You
>> raised a good point and I agree that it's better to have consistency.
>> I probably could submit a patch to change the meaning of "empty host"
>> to "bind to localhost" (instead of "bind to all interfaces") globally
>> (ie. for all code which uses InetSocketAddress) - and in my mind it
>> makes good security sense to do so.
>>
>> Do you think such a change is likely to be accepted?
>
> See other mail.  Changing defaults is always problematic.
>
> And in this specific case the upper management layers (libvirt,
> virt-manager, ...) already configure things strict by default (vnc) or
> prefer to not use tcp sockets in the first place (chardevs).
>
> So I'd tend to not change the qemu default here.
>
> cheers,
>   Gerd
>