[Libvir] Remote patch, 2007/02/19

[Daniel P. Berrange]

On Mon, Feb 19, 2007 at 03:30:25PM +0000, Richard W.M. Jones wrote:
> Just for everyone's information.
> 
> http://annexia.org/tmp/libvirt-tls-20070219.patch

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  -=|