9:59 a.m.
On Thu, Apr 24, 2008 at 03:42:35PM +0100, John Levon wrote:
On Thu, Apr 24, 2008 at 03:25:05PM +0100, Daniel P. Berrange wrote:
> > --- libvirt-0.4.0/qemud/remote.c 2007-12-12 05:30:49.000000000 -0800
> > +++ libvirt-new/qemud/remote.c 2008-04-10 12:52:18.059618661 -0700
> > @@ -434,6 +434,15 @@ remoteDispatchOpen (struct qemud_server
> > flags = args->flags;
> > if (client->readonly) flags |= VIR_CONNECT_RO;
> >
> > +#ifdef __sun
> > + /*
> > + * On Solaris, all clients are forced to go via virtd. As a result,
> > + * virtd must indicate it really does want to connect to the
> > + * hypervisor.
> > + */
> > + name = "xen:///";
> > +#endif
> > +
>
> I'm pretty sure we can come up with a way todo this without having to
> have the #ifdef __sun here. AFAICT, you're trying to stop the remote
> driver in the daemon re-entrantly calling back into the daemon. If
> so, then we can make this more generic, by just having a global flag
> to disable the remote driver entirely when inside the daemon.
That's right. Such a flag sounds sensible to me, I wasn't sure over the
rules of what the daemon can actually use (i.e. private interfaces to
libvirt core).
It is fuzzy & full of trickery :-) If you need to detect whether you
are inside teh daemon or not, then you need to register a state driver
with the virRegisterStateDriver(). The 'init' method in that will be
run when inside the daemon thus enabling you to toggle the flag.
> After the 0.4.0 release I refactored the damon code to have a
generic function for
> getting socket peer credentials. So we can move this code into the qemud.c file
> in the qemudGetSocketIdentity() method. Its good to have a Solaris impl for this
> API.
But this only returns the UID right? Or are you saying there's a generic
function for "can connect"? If so, we could certainly use that. I don't
see how we could use qemudGetSocketIdentity() (though we could certainly
*implement* it).
I'd just like to see if have an impl for Solaris - whether you happen to
use it or not, is separate issue. Some people may be taking libvirt
directly & building/deploying it themselves on Solaris, outside the
context of your confined model, so it may turn out to be useful.
> We can probably use 0666 even on Linux - there's no
compelling reason why we
> need to have 0777 with execute permission - read & write should be sufficient
> I believe.
This hunk is enforcing 0666 even without Polkit on Solaris. So I can fix
the 0777 too, but we do need this hunk.
Ah yes, ignore me here
> > +#ifdef __sun
> > +static void
> > +qemudSetupPrivs (struct qemud_server *server)
> > +{
> > + chown ("/var/run/libvirt", 60, 60);
> > + chmod ("/var/run/libvirt", 0755);
> > + chown ("/var/run/libvirt/libvirt-sock", 60, 60);
> > + chmod ("/var/run/libvirt/libvirt-sock", 0666);
> > + chown (server->logDir, 60, 60);
>
> The thing that concerns me here, is that we're getting a bit of a disconnect
> between the config file and the impl. The user can already set these things
> via the cofnig file, so hardcoding specific values is not so pleasant. I'm
> wondering if its practical to have this privilege separation stuff be toggled
> via a config file setting and avoid hardcoding this soo much.
We don't even ship the config file, as there's nothing in it that we
want to make configurable on Solaris. We don't have the same issue that
Linux does, where it needs to define a single set of sources that
applies to whole bunch of disparate environments. We just have the
Solaris ecosystem, and it's always configured and locked down in the
same way.
When there's genuine configuration values that users need for something
enabled on Solaris, it will be in SMF anyway, so will need re-working on
the config backend in libvirt.
So you're not making use of any of the remote management features like
TLS and SASL/Kerberos ? I'd think at least users need the config to switch
between which auth scheme they'd like to use, because the choice of TLS
vs Kerberos is really a deployment question for admins.
> > +/**
> > + * xenHavePrivilege()
> > + *
> > + * Return true if the current process should be able to connect to Xen.
> > + */
> > +int
> > +xenHavePrivilege()
> > +{
> > +#ifdef __sun
> > + return priv_ineffect (PRIV_XVM_CONTROL);
> > +#else
> > + return getuid () == 0;
> > +#endif
> > +}
>
> As mentioned earlier, we probably want to move this into the util.c file
> and have the privilege name passed in as a parameter.
Could you explain further how you see this working?
Something like
int virHavePrivilege(const char *);
Pass in PRIV_XEN_CONTROL, and on Solaris have that check with your
prvilege code, and on Linux just have it ignore the param and check
the UID. It is likely we'll do more fine grained checks based on
named privileges on Linux too
> > +/*
> > + * Attempt to access the domain via 'xenconsole', which may have
> > + * additional privilege to reach the console.
> > + */
> > +static int
> > +vshXenConsole(int id)
>
> Clearly this has to die.
It's not at all clear to me. virsh console is already a wart, and I
don't see the problem in a pragmatic approach here, given it only ever
tries the direct approach as a fallback.
Well, xm console is the true wart since it is needlessly design to
only work with Xen, when it could have trivially been portable. Thus
we have virsh console which is portable - fetching the PTY from the
XML document instead.
> The virsh console command implements exactly
> the same functionality as xenconsole without being xen specific.
>
> I'm guessing you're doing this because you need the console in a
> separate process from virsh, so it can run with different privileges.
Yes. In particular it needs to run setuid root and:
- get the pts from xenstore (needs PRIV_XVM_CONTROL)
That's already obtainable from the XML doc & since you run the Xen
driver within the daemon, it should have the PRIV_XVM_CONTROL privilege
already.
- open the pty (requires all privileges)
- drop all privileges before continuing
> If so, we should probably split 'virsh console' out into a separate
> standlone binary/command 'virt-console'. This will let you get the
> privilege separation without relying on Xen specific commands.
This just adds a whole bunch of extra code to no clear advantage?
There's no extra code because virsh console already exists and isn't
going away because it offers a superset of what xenconsole does, by
virtue of being portable
Dan.
--
|: Red Hat, Engineering, Boston -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 :|