Just for everyone's information. http://annexia.org/tmp/libvirt-tls-20070219.patch -- Emerging Technologies, Red Hat http://et.redhat.com/~rjones/ 64 Baker Street, London, W1U 7DF Mobile: +44 7866 314 421 "[Negative numbers] darken the very whole doctrines of the equations and make dark of the things which are in their nature excessively obvious and simple" (Francis Maseres FRS, mathematician, 1759)
On Mon, Feb 19, 2007 at 03:30:25PM +0000, Richard W.M. Jones wrote:
Just for everyone's information.
Can you split out the internal virt/network driver API refactoring to a separate patch so we can apply & test it independantly of the main remote transport. In src/remote_internal.c: + case trans_unix: { + sockname = sockname ? : strdup (LIBVIRTD_UNIX_SOCKET); + + // 108 is hard-coded into the header files as well. +#define UNIX_PATH_MAX 108 + struct sockaddr_un addr; + memset (&addr, 0, sizeof addr); + addr.sun_family = AF_UNIX; + strncpy (addr.sun_path, sockname, UNIX_PATH_MAX); Should really use sizeof(addr.sun_path) - while 108 is hardcoded in the Linux headers, there's no guarentee Solaris hardcodes it to 108 too. In src/remote_internal.h: +//#define LIBVIRTD_UNIX_SOCKET "/var/run/libvirtd/socket" +#define LIBVIRTD_UNIX_SOCKET "/tmp/socket" // Just for testing +#define LIBVIRTD_CONFIGURATION_FILE "/etc/libvirtd.conf" Instead of using /var/run and /etc we should pass in the values of the sysconfdir & localstatedir variables from autoconf. That way, during testing & development people can run autogen.sh --prefix=$HOME/usr and these will automatically point to $HOME/usr/var/run/libvirtd and $HOME/usr/etc/libvirtd.conf Regards, 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 Mon, Feb 19, 2007 at 03:30:25PM +0000, Richard W.M. Jones wrote:
Just for everyone's information.
Can you split out the internal virt/network driver API refactoring to a separate patch so we can apply & test it independantly of the main remote transport.
Yup, it's a completely separate patch.
In src/remote_internal.c:
+ case trans_unix: { + sockname = sockname ? : strdup (LIBVIRTD_UNIX_SOCKET); + + // 108 is hard-coded into the header files as well. +#define UNIX_PATH_MAX 108 + struct sockaddr_un addr; + memset (&addr, 0, sizeof addr); + addr.sun_family = AF_UNIX; + strncpy (addr.sun_path, sockname, UNIX_PATH_MAX);
Should really use sizeof(addr.sun_path) - while 108 is hardcoded in the Linux headers, there's no guarentee Solaris hardcodes it to 108 too.
Yes, you're right -- fixed.
In src/remote_internal.h:
+//#define LIBVIRTD_UNIX_SOCKET "/var/run/libvirtd/socket" +#define LIBVIRTD_UNIX_SOCKET "/tmp/socket" // Just for testing +#define LIBVIRTD_CONFIGURATION_FILE "/etc/libvirtd.conf"
Instead of using /var/run and /etc we should pass in the values of the sysconfdir & localstatedir variables from autoconf. That way, during testing & development people can run autogen.sh --prefix=$HOME/usr and these will automatically point to $HOME/usr/var/run/libvirtd and $HOME/usr/etc/libvirtd.conf
Yes, also fixed. Rich. -- Emerging Technologies, Red Hat http://et.redhat.com/~rjones/ 64 Baker Street, London, W1U 7DF Mobile: +44 7866 314 421 "[Negative numbers] darken the very whole doctrines of the equations and make dark of the things which are in their nature excessively obvious and simple" (Francis Maseres FRS, mathematician, 1759)
On Mon, Feb 19, 2007 at 03:30:25PM +0000, Richard W.M. Jones wrote:
Just for everyone's information.
I'm really not at all a fan of the magic cookie being passed around all the time on the wire +/* Keep handles on the server side associated with cookies handed + * back to the client. We will store virConnectPtr's (amongst other + * things) here. + * + * Not using the functions in <hash.h> for two reasons: + * (1) They're not exported from libvirt. + * (2) Want to leave the implementation open to using libmm + * in future, so we can persist the cookies and/or share them + * between preforked server instances. + * + * Notes: + * (a) Cookies are never (and can never be) garbage collected. + * This is because SunRPC is stateless so a connection never really + * goes away. Actually this is not 100% true - if the connection + * is properly close()d by the client, then we garbage collect the + * cookie. + * (b) Should cookies be more secure? The problem here is an + * untrusted client (perhaps a R/O client) "stealing" the connection + * of a trusted client (perhaps a R/W client). You probably + * shouldn't be letting untrusted clients connect to libvirtd + * at all though. Anyway, the cookies implemented here are + * random 64 bit integers, so offer a modicum of security. + * (c) Current implementation is a simple linked list which should + * scale OK to a few dozen handles. After that it's time to use + * some sort of hashing or a tree. Note to future implementors + * that you need to be able to look up in both directions. + * (d) Equality is currently by reference. We make no attempt + * to look inside two structures to determine if they represent + * the same thing. + */ I understand that SunRPC is at its core a stateless protocol, because it is intended to be able to run apps over both unreliable datagram & reliable stream sockets. For libvirt's purposes though I don't see us ever caring about running over anything other than TCP / UNIX domain sockets which are stateful and reliable. With that in mind I'd venture to suggest we ditch the whole idea of cookies completely. Every method on the server end is already given a 'struct svc_req *req' This struct contains a field ' SVCXPRT *rq_xprt;' Which represents the data transport of the client. And the SVCXPRT struct has as its first member the ' int xp_sock' which is the socket associated with the client. So we can trivially & securely map from a client's TCP connetion to the virConnectPtr without needing any magic cookies. If we have our own svc_run() impl - which we will need if we are to integrate the main QEMU daemon into the general libvirtd, we will be able to detect when a client goes away & thus cleanup virConnectPtr sockets, by iterating over the list of server file descriptors exported: extern struct pollfd *svc_pollfd; extern int svc_max_pollfd; This deals with the magic limitations (a) and (b) of the cookie based approach above. (c) is made much more efficient - iterating & matching over a list of 'int fd' is not a major bottleneck, and (d) is now trivial since we can assume '1 client connection == 1 file descriptor' Regards, 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 Mon, Feb 19, 2007 at 03:30:25PM +0000, Richard W.M. Jones wrote:
Just for everyone's information.
I'm really not at all a fan of the magic cookie being passed around all the time on the wire
[cookies]
I understand that SunRPC is at its core a stateless protocol, because it is intended to be able to run apps over both unreliable datagram & reliable stream sockets. For libvirt's purposes though I don't see us ever caring about running over anything other than TCP / UNIX domain sockets which are stateful and reliable.
Right, and the other thing to observe is that in fact SunRPC over TCP / Unix domain sockets doesn't automatically reconnect anyway, as I once thought it did. We shouldn't try to do manual reconnection because that has all sorts of other interesting ways to fail and in any case would make the code really too complicated. [Gory details and test code in case anyone is interested: http://et.redhat.com/~rjones/sunrpc_reconnection/ ]
With that in mind I'd venture to suggest we ditch the whole idea of cookies completely.
Every method on the server end is already given a
'struct svc_req *req'
This struct contains a field
' SVCXPRT *rq_xprt;'
Which represents the data transport of the client. And the SVCXPRT struct has as its first member the ' int xp_sock' which is the socket associated with the client.
So we can trivially & securely map from a client's TCP connetion to the virConnectPtr without needing any magic cookies.
What concerns me here is that xp_sock is just a file descriptor and fds can be reused. It is also an fd that could be any of: * a TCPv6 socket * a TCPv4 socket * a Unix domain socket * on the client side, a socketpair (which on Linux is a funny type of Unix domain socket) So finding something unique about it may be tricky. What happens if two clients connect in succession over the local Unix domain socket? I need to think about this some more, so watch this space ... Also worth noting is that cookies may represent other server-side objects, in particular domains and networks. We can have multiple domains per connection. The relationship between networks and connections is complicated (and I don't pretend to understand it at the moment either). I will be thinking about this too ... Rich. -- Emerging Technologies, Red Hat http://et.redhat.com/~rjones/ 64 Baker Street, London, W1U 7DF Mobile: +44 7866 314 421 "[Negative numbers] darken the very whole doctrines of the equations and make dark of the things which are in their nature excessively obvious and simple" (Francis Maseres FRS, mathematician, 1759)
On Wed, Feb 21, 2007 at 06:13:40PM +0000, Richard W.M. Jones wrote:
Daniel P. Berrange wrote:
With that in mind I'd venture to suggest we ditch the whole idea of cookies completely.
Every method on the server end is already given a
'struct svc_req *req'
This struct contains a field
' SVCXPRT *rq_xprt;'
Which represents the data transport of the client. And the SVCXPRT struct has as its first member the ' int xp_sock' which is the socket associated with the client.
So we can trivially & securely map from a client's TCP connetion to the virConnectPtr without needing any magic cookies.
What concerns me here is that xp_sock is just a file descriptor and fds can be reused. It is also an fd that could be any of: * a TCPv6 socket * a TCPv4 socket * a Unix domain socket * on the client side, a socketpair (which on Linux is a funny type of Unix domain socket) So finding something unique about it may be tricky. What happens if two clients connect in succession over the local Unix domain socket?
I've just noticed that since we are providing our own server side transport implementations in sunrpc/svc_{tcp,ext,gnutls}.c we already have a place where we can safely put in cleanup hooks. In the 'svctcp_destroy' just after we call 'xprt_unregister' we can call out to purge client state associated with that FD. This ensures that we cleanup state before any new connection can re-use the same FD number. So there's actually no need to replace the svc_run() method in this case after all.
Also worth noting is that cookies may represent other server-side objects, in particular domains and networks. We can have multiple domains per connection. The relationship between networks and connections is complicated (and I don't pretend to understand it at the moment either). I will be thinking about this too ...
For domains & networks I imagine we'd just be passing a UUID / Name / ID across the wire to identify the object in question, as we do with the existing libvirt_proxy / qemu daemon ? 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, Feb 21, 2007 at 06:13:40PM +0000, Richard W.M. Jones wrote:
>> Daniel P. Berrange wrote:
>>
>>> With that in mind I'd venture to suggest we ditch the whole idea of cookies
>>> completely.
>>>
>>> Every method on the server end is already given a
>>>
>>> 'struct svc_req *req'
>>>
>>> This struct contains a field
>>>
>>> ' SVCXPRT *rq_xprt;'
>>>
>>> Which represents the data transport of the client. And the SVCXPRT struct
>>> has as its first member the ' int xp_sock' which is the socket associated
>>> with the client.
>>>
>>> So we can trivially & securely map from a client's TCP connetion to the
>>> virConnectPtr without needing any magic cookies.
>> What concerns me here is that xp_sock is just a file descriptor and fds
>> can be reused. It is also an fd that could be any of:
>> * a TCPv6 socket
>> * a TCPv4 socket
>> * a Unix domain socket
>> * on the client side, a socketpair (which on Linux is a funny type of
>> Unix domain socket)
>> So finding something unique about it may be tricky. What happens if two
>> clients connect in succession over the local Unix domain socket?
>
> I've just noticed that since we are providing our own server side transport
> implementations in sunrpc/svc_{tcp,ext,gnutls}.c we already have a place
> where we can safely put in cleanup hooks. In the 'svctcp_destroy' just
> after we call 'xprt_unregister' we can call out to purge client state
> associated with that FD. This ensures that we cleanup state before any new
> connection can re-use the same FD number. So there's actually no need to
> replace the svc_run() method in this case after all.
I was hoping to avoid this. Note that the contents of the sunrpc/
subdirectory (the client and server transports) are independent of
libvirt and I hoped to publish them separately so they could be reused
in other places. The other problem is the Unix transport, where we use
the transport from glibc directly. Anyway, still thinking about this
(the last few days have been a dead loss because of new laptop and
wireless problems).
>> Also worth noting is that cookies may represent other server-side
>> objects, in particular domains and networks. We can have multiple
>> domains per connection. The relationship between networks and
>> connections is complicated (and I don't pretend to understand it at the
>> moment either). I will be thinking about this too ...
>
> For domains & networks I imagine we'd just be passing a UUID / Name /
> ID across the wire to identify the object in question, as we do with the
> existing libvirt_proxy / qemu daemon ?
I'll take a look at this.
Rich.
--
Emerging Technologies, Red Hat http://et.redhat.com/~rjones/
64 Baker Street, London, W1U 7DF Mobile: +44 7866 314 421
"[Negative numbers] darken the very whole doctrines of the equations
and make dark of the things which are in their nature excessively
obvious and simple" (Francis Maseres FRS, mathematician, 1759)
On Fri, Feb 23, 2007 at 10:37:32AM +0000, Richard W.M. Jones wrote:
> Daniel P. Berrange wrote:
> >On Wed, Feb 21, 2007 at 06:13:40PM +0000, Richard W.M. Jones wrote:
> >>Daniel P. Berrange wrote:
> >>
> >>>With that in mind I'd venture to suggest we ditch the whole idea of
> >>>cookies
> >>>completely.
> >>>
> >>>Every method on the server end is already given a
> >>>
> >>> 'struct svc_req *req'
> >>>
> >>>This struct contains a field
> >>>
> >>> ' SVCXPRT *rq_xprt;'
> >>>
> >>>Which represents the data transport of the client. And the SVCXPRT struct
> >>>has as its first member the ' int xp_sock' which is the socket
> >>>associated
> >>>with the client.
> >>>
> >>>So we can trivially & securely map from a client's TCP connetion to the
> >>>virConnectPtr without needing any magic cookies.
> >>What concerns me here is that xp_sock is just a file descriptor and fds
> >>can be reused. It is also an fd that could be any of:
> >> * a TCPv6 socket
> >> * a TCPv4 socket
> >> * a Unix domain socket
> >> * on the client side, a socketpair (which on Linux is a funny type of
> >>Unix domain socket)
> >>So finding something unique about it may be tricky. What happens if two
> >>clients connect in succession over the local Unix domain socket?
> >
> >I've just noticed that since we are providing our own server side transport
> >implementations in sunrpc/svc_{tcp,ext,gnutls}.c we already have a place
> >where we can safely put in cleanup hooks. In the 'svctcp_destroy' just
> >after we call 'xprt_unregister' we can call out to purge client state
> >associated with that FD. This ensures that we cleanup state before any new
> >connection can re-use the same FD number. So there's actually no need to
> >replace the svc_run() method in this case after all.
>
> I was hoping to avoid this. Note that the contents of the sunrpc/
> subdirectory (the client and server transports) are independent of
> libvirt and I hoped to publish them separately so they could be reused
> in other places. The other problem is the Unix transport, where we use
> the transport from glibc directly.
To keep them untied from libvirt one could allow a generic 'void (*cleanup)(void *)'
callback to be passed into the create methods for each transport, and simply
invoke that from the descructor. We'd need to provide an alternate impl for
hte Unix transport too.
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 Fri, Feb 23, 2007 at 10:37:32AM +0000, Richard W.M. Jones wrote:
>> Daniel P. Berrange wrote:
>>> On Wed, Feb 21, 2007 at 06:13:40PM +0000, Richard W.M. Jones wrote:
>>>> Daniel P. Berrange wrote:
>>>>
>>>>> With that in mind I'd venture to suggest we ditch the whole idea of
>>>>> cookies
>>>>> completely.
>>>>>
>>>>> Every method on the server end is already given a
>>>>>
>>>>> 'struct svc_req *req'
>>>>>
>>>>> This struct contains a field
>>>>>
>>>>> ' SVCXPRT *rq_xprt;'
>>>>>
>>>>> Which represents the data transport of the client. And the SVCXPRT struct
>>>>> has as its first member the ' int xp_sock' which is the socket
>>>>> associated
>>>>> with the client.
>>>>>
>>>>> So we can trivially & securely map from a client's TCP connetion to the
>>>>> virConnectPtr without needing any magic cookies.
>>>> What concerns me here is that xp_sock is just a file descriptor and fds
>>>> can be reused. It is also an fd that could be any of:
>>>> * a TCPv6 socket
>>>> * a TCPv4 socket
>>>> * a Unix domain socket
>>>> * on the client side, a socketpair (which on Linux is a funny type of
>>>> Unix domain socket)
>>>> So finding something unique about it may be tricky. What happens if two
>>>> clients connect in succession over the local Unix domain socket?
>>> I've just noticed that since we are providing our own server side transport
>>> implementations in sunrpc/svc_{tcp,ext,gnutls}.c we already have a place
>>> where we can safely put in cleanup hooks. In the 'svctcp_destroy' just
>>> after we call 'xprt_unregister' we can call out to purge client state
>>> associated with that FD. This ensures that we cleanup state before any new
>>> connection can re-use the same FD number. So there's actually no need to
>>> replace the svc_run() method in this case after all.
>> I was hoping to avoid this. Note that the contents of the sunrpc/
>> subdirectory (the client and server transports) are independent of
>> libvirt and I hoped to publish them separately so they could be reused
>> in other places. The other problem is the Unix transport, where we use
>> the transport from glibc directly.
>
> To keep them untied from libvirt one could allow a generic 'void (*cleanup)(void *)'
> callback to be passed into the create methods for each transport, and simply
> invoke that from the descructor. We'd need to provide an alternate impl for
> hte Unix transport too.
Yes! Implemented!!
Rich.
--
Emerging Technologies, Red Hat http://et.redhat.com/~rjones/
64 Baker Street, London, W1U 7DF Mobile: +44 7866 314 421
"[Negative numbers] darken the very whole doctrines of the equations
and make dark of the things which are in their nature excessively
obvious and simple" (Francis Maseres FRS, mathematician, 1759)
participants (2)
-
Daniel P. Berrange -
Richard W.M. Jones