[Libvir] PATCH: Allow remote driver to handle any connection URI
We currently have logic in the remote driver so that it handles the local QEMU driver URIs, so they get re-directed to the daemon. It also handles networking APIs for Xen driver. For normal APIs, Xen has the auto-spawned setuid proxy daemon. This was very useful at the time we wrote it, but it only supports a handful of operations, and only in read-only mode. One other factor is that SUSE, for example, do not ship it because it is setuid. I don't know whether this is just a general policy, or just because they've not had time to audit it, but that's not very good for their users. With the development of the remote driver & the flexible UNIX socket perms & group ownership, or with policykit support it is possible to replace the proxy with calls straight to the remote daemon. So this patch is the first step by allowing the remote driver to handle any hypervisor connection URI. If it doesn't have a hostname or transport specified, then it automatically tries to connect to the local libvirt daemon over UNIX sockets. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
On Wed, Sep 19, 2007 at 04:03:40AM +0100, Daniel P. Berrange wrote:
We currently have logic in the remote driver so that it handles the local QEMU driver URIs, so they get re-directed to the daemon. It also handles networking APIs for Xen driver. For normal APIs, Xen has the auto-spawned setuid proxy daemon. This was very useful at the time we wrote it, but it only supports a handful of operations, and only in read-only mode. One other factor is that SUSE, for example, do not ship it because it is setuid. I don't know whether this is just a general policy, or just because they've not had time to audit it, but that's not very good for their users.
With the development of the remote driver & the flexible UNIX socket perms & group ownership, or with policykit support it is possible to replace the proxy with calls straight to the remote daemon. So this patch is the first step by allowing the remote driver to handle any hypervisor connection URI. If it doesn't have a hostname or transport specified, then it automatically tries to connect to the local libvirt daemon over UNIX sockets.
Okay, I think I understand. I assume this is dependant logically on having the PolicyKit patch applied first to be able to filter the accesses, right ?
Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
diff -r bc9c1ba80870 src/remote_internal.c --- a/src/remote_internal.c Tue Sep 18 14:13:29 2007 -0400 +++ b/src/remote_internal.c Tue Sep 18 14:23:22 2007 -0400 @@ -232,9 +232,8 @@ remoteForkDaemon(virConnectPtr conn) /* Must not overlap with virDrvOpenFlags */ enum virDrvOpenRemoteFlags { VIR_DRV_OPEN_REMOTE_RO = (1 << 0), - VIR_DRV_OPEN_REMOTE_UNIX = (1 << 1), - VIR_DRV_OPEN_REMOTE_USER = (1 << 2), - VIR_DRV_OPEN_REMOTE_AUTOSTART = (1 << 3), + VIR_DRV_OPEN_REMOTE_USER = (1 << 1), + VIR_DRV_OPEN_REMOTE_AUTOSTART = (1 << 2), };
I'm just a bit worried about changing those if they end up on the wire in some ways. If that's the case then just keep he enum as-is. Looks fine to me, +1, Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
/* Must not overlap with virDrvOpenFlags */ enum virDrvOpenRemoteFlags { VIR_DRV_OPEN_REMOTE_RO = (1 << 0), - VIR_DRV_OPEN_REMOTE_UNIX = (1 << 1), - VIR_DRV_OPEN_REMOTE_USER = (1 << 2), - VIR_DRV_OPEN_REMOTE_AUTOSTART = (1 << 3), + VIR_DRV_OPEN_REMOTE_USER = (1 << 1), + VIR_DRV_OPEN_REMOTE_AUTOSTART = (1 << 2), };
I'm just a bit worried about changing those if they end up on the wire in some ways. If that's the case then just keep he enum as-is.
I don't really understand the purpose of these flags. They _do_ go over the wire in both the current and proposed implementation, but the remote end doesn't interpret them (unless I'm missing something ...?) Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
On Wed, Sep 19, 2007 at 02:08:33PM +0100, Richard W.M. Jones wrote:
/* Must not overlap with virDrvOpenFlags */ enum virDrvOpenRemoteFlags { VIR_DRV_OPEN_REMOTE_RO = (1 << 0), - VIR_DRV_OPEN_REMOTE_UNIX = (1 << 1), - VIR_DRV_OPEN_REMOTE_USER = (1 << 2), - VIR_DRV_OPEN_REMOTE_AUTOSTART = (1 << 3), + VIR_DRV_OPEN_REMOTE_USER = (1 << 1), + VIR_DRV_OPEN_REMOTE_AUTOSTART = (1 << 2), };
I'm just a bit worried about changing those if they end up on the wire in some ways. If that's the case then just keep he enum as-is.
I don't really understand the purpose of these flags. They _do_ go over the wire in both the current and proposed implementation, but the remote end doesn't interpret them (unless I'm missing something ...?)
These flags are only used by remote_internal.c between the methods remoteOpen, remoteNetworkOpen and the doRemoteOpen call. They don't go over the wire. NB, not to be confused with 'enum virDrvOpenFlags' which does go over the wire. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
Daniel P. Berrange wrote:
On Wed, Sep 19, 2007 at 02:08:33PM +0100, Richard W.M. Jones wrote:
/* Must not overlap with virDrvOpenFlags */ enum virDrvOpenRemoteFlags { VIR_DRV_OPEN_REMOTE_RO = (1 << 0), - VIR_DRV_OPEN_REMOTE_UNIX = (1 << 1), - VIR_DRV_OPEN_REMOTE_USER = (1 << 2), - VIR_DRV_OPEN_REMOTE_AUTOSTART = (1 << 3), + VIR_DRV_OPEN_REMOTE_USER = (1 << 1), + VIR_DRV_OPEN_REMOTE_AUTOSTART = (1 << 2), }; I'm just a bit worried about changing those if they end up on the wire in some ways. If that's the case then just keep he enum as-is. I don't really understand the purpose of these flags. They _do_ go over the wire in both the current and proposed implementation, but the remote end doesn't interpret them (unless I'm missing something ...?)
These flags are only used by remote_internal.c between the methods remoteOpen, remoteNetworkOpen and the doRemoteOpen call. They don't go over the wire.
NB, not to be confused with 'enum virDrvOpenFlags' which does go over the wire.
Are you sure they don't go over the wire? My reading of doRemoteOpen suggests that they do: static int doRemoteOpen (virConnectPtr conn, struct private_data *priv, const char *uri_str, int flags) { // bunch of code which reads 'flags' but never modifies it /* Finally we can call the remote side's open function. */ remote_open_args args = { &name, flags }; if (call (conn, priv, 1, REMOTE_PROC_OPEN, (xdrproc_t) xdr_remote_open_args, (char *) &args, (xdrproc_t) xdr_void, (char *) NULL) == -1) goto failed; and the 'call' function just serialises whatever is in the args array. But at this point flags could contain VIR_DRV_OPEN_REMOTE_* flags. Unless I'm reading this code wrong ... I checked at the other side and it doesn't use any flag except the standard RO flag, so we should be OK anyhow. Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
On Wed, Sep 19, 2007 at 02:51:42PM +0100, Richard W.M. Jones wrote:
Daniel P. Berrange wrote:
On Wed, Sep 19, 2007 at 02:08:33PM +0100, Richard W.M. Jones wrote:
/* Must not overlap with virDrvOpenFlags */ enum virDrvOpenRemoteFlags { VIR_DRV_OPEN_REMOTE_RO = (1 << 0), - VIR_DRV_OPEN_REMOTE_UNIX = (1 << 1), - VIR_DRV_OPEN_REMOTE_USER = (1 << 2), - VIR_DRV_OPEN_REMOTE_AUTOSTART = (1 << 3), + VIR_DRV_OPEN_REMOTE_USER = (1 << 1), + VIR_DRV_OPEN_REMOTE_AUTOSTART = (1 << 2), }; I'm just a bit worried about changing those if they end up on the wire in some ways. If that's the case then just keep he enum as-is. I don't really understand the purpose of these flags. They _do_ go over the wire in both the current and proposed implementation, but the remote end doesn't interpret them (unless I'm missing something ...?)
These flags are only used by remote_internal.c between the methods remoteOpen, remoteNetworkOpen and the doRemoteOpen call. They don't go over the wire.
NB, not to be confused with 'enum virDrvOpenFlags' which does go over the wire.
Are you sure they don't go over the wire? My reading of doRemoteOpen suggests that they do:
static int doRemoteOpen (virConnectPtr conn, struct private_data *priv, const char *uri_str, int flags) { // bunch of code which reads 'flags' but never modifies it
/* Finally we can call the remote side's open function. */ remote_open_args args = { &name, flags };
if (call (conn, priv, 1, REMOTE_PROC_OPEN, (xdrproc_t) xdr_remote_open_args, (char *) &args, (xdrproc_t) xdr_void, (char *) NULL) == -1) goto failed;
and the 'call' function just serialises whatever is in the args array. But at this point flags could contain VIR_DRV_OPEN_REMOTE_* flags.
Unless I'm reading this code wrong ...
Yep that's a bug - we're passing bogus data to the server. Fortunately its not checking any flag except _RO, but I'll fix this & repost. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
On Wed, Sep 19, 2007 at 06:49:48PM +0100, Daniel P. Berrange wrote:
On Wed, Sep 19, 2007 at 02:51:42PM +0100, Richard W.M. Jones wrote:
Are you sure they don't go over the wire? My reading of doRemoteOpen suggests that they do:
static int doRemoteOpen (virConnectPtr conn, struct private_data *priv, const char *uri_str, int flags) { // bunch of code which reads 'flags' but never modifies it
/* Finally we can call the remote side's open function. */ remote_open_args args = { &name, flags };
if (call (conn, priv, 1, REMOTE_PROC_OPEN, (xdrproc_t) xdr_remote_open_args, (char *) &args, (xdrproc_t) xdr_void, (char *) NULL) == -1) goto failed;
and the 'call' function just serialises whatever is in the args array. But at this point flags could contain VIR_DRV_OPEN_REMOTE_* flags.
Unless I'm reading this code wrong ...
Yep that's a bug - we're passing bogus data to the server. Fortunately its not checking any flag except _RO, but I'll fix this & repost.
Here's a tweaked patch which explicitly only passes through the _RO flag to the remote_open_args. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
Daniel P. Berrange wrote:
We currently have logic in the remote driver so that it handles the local QEMU driver URIs, so they get re-directed to the daemon. It also handles networking APIs for Xen driver. For normal APIs, Xen has the auto-spawned setuid proxy daemon. This was very useful at the time we wrote it, but it only supports a handful of operations, and only in read-only mode. One other factor is that SUSE, for example, do not ship it because it is setuid. I don't know whether this is just a general policy, or just because they've not had time to audit it, but that's not very good for their users.
Yep. Reason is the former. But this can be overridden (followed by an audit) if there is a good case. Apparently my case wasn't strong enough. Too be fair though, I didn't push hard. And now that I've seen this mail I'm reminded that I wanted to push this for openSUSE 10.3 -- which went GM today :-(. Jim
participants (4)
-
Daniel P. Berrange -
Daniel Veillard -
Jim Fehlig -
Richard W.M. Jones