10:03 p.m.
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 -=|
5:18 a.m.
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(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
8:08 a.m.
> /* 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
8:34 a.m.
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 -=|
8:51 a.m.
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
12:49 p.m.
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 -=|
2:14 p.m.
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 -=|
6:24 p.m.
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
6267
days inactive
6275
days old
7 comments
4 participants
participants (4)
-
Daniel P. Berrange -
Daniel Veillard -
Jim Fehlig -
Richard W.M. Jones