qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrange" <berrange@redhat.com>
To: Gerd Hoffmann <kraxel@redhat.com>
Cc: qemu-devel@nongnu.org, Anthony Liguori <aliguori@amazon.com>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 6/6] vnc: track & limit connections
Date: Wed, 15 Oct 2014 15:39:15 +0100	[thread overview]
Message-ID: <20141015143915.GE3741@redhat.com> (raw)
In-Reply-To: <1413382769.4213.5.camel@nilsson.home.kraxel.org>

On Wed, Oct 15, 2014 at 04:19:29PM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> > > The logic to apply the limit to connections in "connecting" state (this
> > > is the state you are in *before* successfull authentication) is
> > > slightly different:  A new connect kicks out the oldest client which is
> > > still in "connecting" state.  This avoids a easy DoS by unauthenticated
> > > users by simply opening connections until the limit is reached.
> > 
> > I'd suggest that rather than kicking off the oldest client QEMU
> > should simply stop calling accept() when it reaches the limit
> > of active unauthenticated client connections.
> 
> Looks like I need to be a bit more verbose.  The DoS I try to prevent is
> that anybody can open $limit connections to the vnc server, let them sit
> around idle, thereby blocking further connects.

Ah, so the DoS I'm thinking of is that you can open connections
to VNC until the max file descriptor limit is exhausted and thus
break anything in QEMU that needs to open a new FD (eg migration).
The key to fixing that particular DoS is simply having a small
finite limit so we cannot exhaust file descriptors. Stopping
calling accept() works in that scenario.

> Whenever you stop calling accept or drop the new connection doesn't make
> much of a difference.
> 
> I try to prevent that by dropping the *oldest* connection, so you have a
> chance to connect even if a unprivileged attacker tries to use up all
> connection slots.

Lets say the limit is 5. The bad guy has 5 open idle connections.
The good guy opens a new one and pushes off one of the bad guy's
connections. Fine so far. The bad guy though can simply open 5 more
connections and he'll push the good guy's connection off again. This
fix would only address the scenario you describe if the good guy can
connect & complete auth before the bad guy opens more connections.
With TLS and/or SASL there's a non-negligible time window for auth
to take place.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

  reply	other threads:[~2014-10-15 14:39 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-15 12:19 [Qemu-devel] [PATCH 0/6] vnc: add support for multiple vnc server instances Gerd Hoffmann
2014-10-15 12:19 ` [Qemu-devel] [PATCH 1/6] vnc: remove vnc_display global Gerd Hoffmann
2014-10-15 12:19 ` [Qemu-devel] [PATCH 2/6] vnc: remove unused DisplayState parameter, add id instead Gerd Hoffmann
2014-10-15 12:19 ` [Qemu-devel] [PATCH 3/6] vnc: switch to QemuOpts, allow multiple servers Gerd Hoffmann
2014-10-15 12:19 ` [Qemu-devel] [PATCH 4/6] vnc: allow binding servers to qemu consoles Gerd Hoffmann
2014-10-15 12:19 ` [Qemu-devel] [PATCH 5/6] vnc: update docs/multiseat.txt Gerd Hoffmann
2014-10-15 12:19 ` [Qemu-devel] [PATCH 6/6] vnc: track & limit connections Gerd Hoffmann
2014-10-15 12:31   ` Daniel P. Berrange
2014-10-15 14:19     ` Gerd Hoffmann
2014-10-15 14:39       ` Daniel P. Berrange [this message]
2014-10-16 10:46         ` Gerd Hoffmann
2014-10-17  6:34           ` Gonglei
2014-10-17  6:38             ` Daniel P. Berrange
2014-10-17  6:54               ` Gonglei
2014-10-20  7:02             ` Gerd Hoffmann
2014-10-21  6:06               ` Gonglei
2014-10-21  8:57                 ` Gerd Hoffmann
2014-10-21  9:10                   ` Gonglei
2014-10-21  9:35                     ` Gerd Hoffmann
2014-10-21 10:32                       ` Gonglei
2014-10-15 14:51   ` Eric Blake
2014-10-15 12:32 ` [Qemu-devel] [PATCH 0/6] vnc: add support for multiple vnc server instances Daniel P. Berrange
2014-10-15 14:29   ` Gerd Hoffmann
2014-10-15 14:41     ` Daniel P. Berrange
2014-10-15 12:51 ` Daniel P. Berrange
2014-10-15 14:30   ` Gerd Hoffmann
2014-10-15 14:48 ` Eric Blake

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141015143915.GE3741@redhat.com \
    --to=berrange@redhat.com \
    --cc=aliguori@amazon.com \
    --cc=dgilbert@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).