Richard W.M. Jones | 8 Aug 15:32
Favicon

[PATCH 0/5] virt-viewer: browser plugin rewritten as lightweight virt-viewer process launcher

This series of patches (re-)implements the virt-viewer browser plugin
as a lightweight, hidden plugin which just launches an external
virt-viewer process.  The theory behind this is explained in more
detail in these two postings:

https://www.redhat.com/archives/ovirt-devel/2008-August/msg00004.html
http://sourceforge.net/mailarchive/forum.php?thread_name=20080808093541.GA31923%40amd.home.annexia.org&forum_name=gtk-vnc-devel

Rich.

--

-- 
Richard Jones, Emerging Technologies, Red Hat  http://et.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://et.redhat.com/~rjones/virt-df/
Richard W.M. Jones | 8 Aug 15:37
Favicon

[PATCH 1/5] Rename plugin/ directory to oldplugin/


Rename plugin/ directory to oldplugin/
 - Also fix .hgignore to ignore generated files in oldplugin directory.

Note that most of this patch is simply renaming files, which stay
identical.

Rich.

-- 
Richard Jones, Emerging Technologies, Red Hat  http://et.redhat.com/~rjones
Read my OCaml programming blog: http://camltastic.blogspot.com/
Fedora now supports 60 OCaml packages (the OPEN alternative to F#)
http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora
# HG changeset patch
# User "Richard W.M. Jones <rjones-H+wXaHxf7aLQT0dZR+AlfA <at> public.gmane.org>"
# Date 1218189313 -3600
# Node ID e268ed66e658bc364dea15f3d3f2541b2e28039e
# Parent  57d1fb020d5706a4bb1c9556a6a3a576c627decf
Rename plugin/ directory to oldplugin/
 - Also fix .hgignore to ignore generated files in oldplugin directory.

diff -r 57d1fb020d57 -r e268ed66e658 .hgignore
--- a/.hgignore	Tue Jun 17 04:02:50 2008 -0400
+++ b/.hgignore	Fri Aug 08 10:55:13 2008 +0100
 <at>  <at>  -20,3 +20,10  <at>  <at> 
 ^config.h.in$
 ^man/Makefile$
(Continue reading)

Daniel P. Berrange | 14 Aug 16:05
Favicon

Re: [PATCH 1/5] Rename plugin/ directory to oldplugin/

On Fri, Aug 08, 2008 at 02:37:48PM +0100, Richard W.M. Jones wrote:
> 
> Rename plugin/ directory to oldplugin/
>  - Also fix .hgignore to ignore generated files in oldplugin directory.

I don't like this - I don't see why we can't just keep both plugins
available in one directory. Just give them different names. eg call
the new one  virt-viewer-command-plugin.so and have the source for
both in the plugins/ directory. In the binary build, we can have
a separate sub-RPM for each plugin, so the user can choose which
they prefer to use.

Daniel
--

-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
Richard W.M. Jones | 14 Aug 18:03
Favicon

Re: [PATCH 1/5] Rename plugin/ directory to oldplugin/

On Thu, Aug 14, 2008 at 03:05:29PM +0100, Daniel P. Berrange wrote:
> On Fri, Aug 08, 2008 at 02:37:48PM +0100, Richard W.M. Jones wrote:
> > 
> > Rename plugin/ directory to oldplugin/
> >  - Also fix .hgignore to ignore generated files in oldplugin directory.
> 
> I don't like this - I don't see why we can't just keep both plugins
> available in one directory. Just give them different names. eg call
> the new one  virt-viewer-command-plugin.so and have the source for
> both in the plugins/ directory. In the binary build, we can have
> a separate sub-RPM for each plugin, so the user can choose which
> they prefer to use.

I think I felt we should deprecate the old plugin.  Keep it around for
a while just in case anyone can make it work, otherwise delete it.
What do you think?

Rich.

--

-- 
Richard Jones, Emerging Technologies, Red Hat  http://et.redhat.com/~rjones
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top
Daniel P. Berrange | 15 Aug 10:58
Favicon

Re: [PATCH 1/5] Rename plugin/ directory to oldplugin/

On Thu, Aug 14, 2008 at 05:03:50PM +0100, Richard W.M. Jones wrote:
> On Thu, Aug 14, 2008 at 03:05:29PM +0100, Daniel P. Berrange wrote:
> > On Fri, Aug 08, 2008 at 02:37:48PM +0100, Richard W.M. Jones wrote:
> > > 
> > > Rename plugin/ directory to oldplugin/
> > >  - Also fix .hgignore to ignore generated files in oldplugin directory.
> > 
> > I don't like this - I don't see why we can't just keep both plugins
> > available in one directory. Just give them different names. eg call
> > the new one  virt-viewer-command-plugin.so and have the source for
> > both in the plugins/ directory. In the binary build, we can have
> > a separate sub-RPM for each plugin, so the user can choose which
> > they prefer to use.
> 
> I think I felt we should deprecate the old plugin.  Keep it around for
> a while just in case anyone can make it work, otherwise delete it.
> What do you think?

I'm not so sure - I can see both plugins being potentially desirable.
All existing VNC plugins for example, use the fully embedded style
so they may be people who prefer that approach, even though for oVirt
we'd like to go the standalone approach.

Daniel
--

-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
(Continue reading)

Richard W.M. Jones | 8 Aug 15:38
Favicon

[PATCH 2/5] Change --enable-plugin => --enable-oldplugin, and add a new --enable-plugin.


Change --enable-plugin => --enable-oldplugin, and add a new --enable-plugin.
 - Ignore generated files in the (new) plugin/ directory.
 - -DPLUGIN=1 => -DOLDPLUGIN=1 in src/main.c.

-- 
Richard Jones, Emerging Technologies, Red Hat  http://et.redhat.com/~rjones
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top
# HG changeset patch
# User "Richard W.M. Jones <rjones@...>"
# Date 1218190993 -3600
# Node ID 319f0a7da787989ab8a4304984b0970a3270c5f4
# Parent  e268ed66e658bc364dea15f3d3f2541b2e28039e
Change --enable-plugin => --enable-oldplugin, and add a new --enable-plugin.
 - Ignore generated files in the (new) plugin/ directory.
 - -DPLUGIN=1 => -DOLDPLUGIN=1 in src/main.c.

diff -r e268ed66e658 -r 319f0a7da787 .hgignore
--- a/.hgignore	Fri Aug 08 10:55:13 2008 +0100
+++ b/.hgignore	Fri Aug 08 11:23:13 2008 +0100
@@ -27,3 +27,10 @@
 ^oldplugin/virt-viewer-plugin\.la$
 ^oldplugin/virt-viewer-plugin\.so$
 ^oldplugin/virt_viewer_plugin_la-.*\.lo$
+^plugin/\.deps
+^plugin/.*\.o
(Continue reading)

Daniel P. Berrange | 14 Aug 16:07
Favicon

Re: [PATCH 2/5] Change --enable-plugin => --enable-oldplugin, and add a new --enable-plugin.

On Fri, Aug 08, 2008 at 02:38:27PM +0100, Richard W.M. Jones wrote:
> 
> Change --enable-plugin => --enable-oldplugin, and add a new --enable-plugin.
>  - Ignore generated files in the (new) plugin/ directory.
>  - -DPLUGIN=1 => -DOLDPLUGIN=1 in src/main.c.

I'd like to see just a single  '--enable-plugins' which turns on
both plugins.  If we want to distinguish then perhaps allow an
optional argument    --enable-plugins=internal for the inline
plugin and --enable-plugins=external for the command launching
variant

Daniel
--

-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
Richard W.M. Jones | 8 Aug 15:39
Favicon

[PATCH 3/5] Document --enable-plugin and --enable-oldplugin, and how to use the plugins.


Document --enable-plugin and --enable-oldplugin, and how to use the plugins.

-- 
Richard Jones, Emerging Technologies, Red Hat  http://et.redhat.com/~rjones
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top
# HG changeset patch
# User "Richard W.M. Jones <rjones@...>"
# Date 1218191011 -3600
# Node ID b8a8da640ff33ce6771bec489260a8860fd49061
# Parent  319f0a7da787989ab8a4304984b0970a3270c5f4
Document --enable-plugin and --enable-oldplugin, and how to use the plugins.

diff -r 319f0a7da787 -r b8a8da640ff3 README
--- a/README	Fri Aug 08 11:23:13 2008 +0100
+++ b/README	Fri Aug 08 11:23:31 2008 +0100
@@ -27,4 +27,28 @@

   http://virt-manager.org/mailinglist.html

+Browser plugin
+--------------
+
+There are two browser plugins supplied, called "oldplugin" and
+"plugin".  They are not built by default, so to build them you have to
+enable them during configure:
(Continue reading)

Richard W.M. Jones | 8 Aug 15:39
Favicon

[PATCH 4/5] Add 'test.html' to EXTRA_DIST.


Add 'test.html' to EXTRA_DIST.

('test.html' file isn't being distributed in the tarball at
present, which is a bug).

-- 
Richard Jones, Emerging Technologies, Red Hat  http://et.redhat.com/~rjones
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top
# HG changeset patch
# User "Richard W.M. Jones <rjones@...>"
# Date 1218191391 -3600
# Node ID dc12fc3a78696ea36adb442ed0aa3304cbf06596
# Parent  b8a8da640ff33ce6771bec489260a8860fd49061
Add 'test.html' to EXTRA_DIST.

diff -r b8a8da640ff3 -r dc12fc3a7869 oldplugin/Makefile.am
--- a/oldplugin/Makefile.am	Fri Aug 08 11:23:31 2008 +0100
+++ b/oldplugin/Makefile.am	Fri Aug 08 11:29:51 2008 +0100
@@ -30,4 +30,6 @@

 CLEANFILES = virt-viewer-plugin.so

+EXTRA_DIST = test.html
+
 endif
(Continue reading)

Richard W.M. Jones | 8 Aug 15:43
Favicon

[PATCH 5/5] "Launch virt-viewer" (new) browser plugin.


"Launch virt-viewer" (new) browser plugin.
 - Based on npshell from xulrunner 1.9.0.
 - Includes test page.

The file 'npunix.c' is pretty much identical to what is in xulrunner
1.9.0, except that I made a few const-correctness fixes.

The file 'npshell.c' is only minimally modified from what is in
xulrunner, just changed so that it calls the new plugin entry points.

The interesting stuff happens in 'launcher_plugin.c' and 'test.html'.

The easiest way to test this is:

  ./autogen.sh --enable-plugin
  make
  cp plugin/virt-viewer-plugin.so ~/.mozilla/plugins/
  firefox `pwd`/plugin/test.html

Rich.

--

-- 
Richard Jones, Emerging Technologies, Red Hat  http://et.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://et.redhat.com/~rjones/virt-df/
# HG changeset patch
(Continue reading)

Daniel P. Berrange | 14 Aug 16:15
Favicon

Re: [PATCH 5/5] "Launch virt-viewer" (new) browser plugin.

On Fri, Aug 08, 2008 at 02:43:14PM +0100, Richard W.M. Jones wrote:
> +NPError
> +launcherRun (PluginInstance *self,
> +	     int16 argc, char* argn[], char* argv[])
> +{
> +  int i;
> +  const char *uri = NULL;
> +  const char *name = NULL;
> +  int direct = 0;
> +  int waitvnc = 0;
> +  int pid;
> +
> +  /* Protect against being called multiple times. */
> +  if (!self->launched) {
> +    self->launched = TRUE;
> +
> +    /* Parse the parameters given in <embed ...>. */
> +    i = argc;
> +    while (i > 0) {
> +      i --;
> +      if (argv[i] != NULL) {
> +        if (!PL_strcasecmp (argn[i], "URI"))
> +	  uri = argv[i];
> +        else if (!PL_strcasecmp (argn[i], "NAME"))
> +	  name = argv[i];
> +	else if (!PL_strcasecmp (argn[i], "DIRECT"))
> +	  direct = strcmp (argv[i], "1") == 0;
> +	else if (!PL_strcasecmp (argn[i], "WAITVNC"))
> +	  waitvnc = strcmp (argv[i], "1") == 0;
> +      }
(Continue reading)

Richard W.M. Jones | 14 Aug 18:07
Favicon

Re: [PATCH 5/5] "Launch virt-viewer" (new) browser plugin.

On Thu, Aug 14, 2008 at 03:15:19PM +0100, Daniel P. Berrange wrote:
> Am I understanding this correctly, that it'll launch the virt-viewer
> program immediately upon loading the HTML page containing the plugin
> <embed> snippet ?  If so that's a huge security problem - you are
> spawning a program which is allowed to connect to any host on the
> internet. It is also a denial-of-service - malicous  javascript
> could write a page containing thousands of <embed> snippets which
> would spawn thousands of processes.
> 
> I'd rather expect the plugin to have a small embedded area in the
> HTML page showing the details of what host will be connected to,
> what port, and then a button which has to be explicitly pressed
> to launch the external viewer.

Yes ... The trouble is if we do this, we end up needing to embed Gtk
widgets in the browser, which takes us back to square one.

I'll raise this on #virt, see if we can talk through the issues again.

Rich.

--

-- 
Richard Jones, Emerging Technologies, Red Hat  http://et.redhat.com/~rjones
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top
Daniel P. Berrange | 15 Aug 10:56
Favicon

Re: [PATCH 5/5] "Launch virt-viewer" (new) browser plugin.

On Thu, Aug 14, 2008 at 05:07:09PM +0100, Richard W.M. Jones wrote:
> On Thu, Aug 14, 2008 at 03:15:19PM +0100, Daniel P. Berrange wrote:
> > Am I understanding this correctly, that it'll launch the virt-viewer
> > program immediately upon loading the HTML page containing the plugin
> > <embed> snippet ?  If so that's a huge security problem - you are
> > spawning a program which is allowed to connect to any host on the
> > internet. It is also a denial-of-service - malicous  javascript
> > could write a page containing thousands of <embed> snippets which
> > would spawn thousands of processes.
> > 
> > I'd rather expect the plugin to have a small embedded area in the
> > HTML page showing the details of what host will be connected to,
> > what port, and then a button which has to be explicitly pressed
> > to launch the external viewer.
> 
> Yes ... The trouble is if we do this, we end up needing to embed Gtk
> widgets in the browser, which takes us back to square one.

Yeah I guess that does really :-( I must be possible to get GTK reliably
embedded though because I use Totem for movie playback and its embeding
GTK ok

Daniel
--

-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Gmane