On Mon, Jul 29, 2019 at 02:32:31PM +0200, Andrea Bolognani wrote:
> On Tue, 2019-07-23 at 17:03 +0100, Daniel P. Berrangé wrote:
> [...]
> > If connecting to a remote host over any kind of ssh tunnel, for now we
> > must assume only the legacy socket exists. A future patch will introduce
> > a netcat replacement that is tailored for libvirt to make remote
> > tunnelling easier.
> >
> > The configure arg '--with-remote-default-mode=legacy|direct' allows
> > packagers to set a default at build time. If not given, it will default
> > to direct mode.
> >
> > In RPM builds this is overriden, because before we can default to the
> > new daemons, we must get SELinux policy written & the timeframe for that
> > is unclear at this stage.
> > @@ -758,21 +776,126 @@ remoteGetUNIXSocket(remoteDriverTransport transport,
> > if (!(userdir = virGetUserRuntimeDirectory()))
> > return NULL;
> >
> > - if (virAsprintf(&sockname,
> > - "%s/" LIBVIRTD_USER_UNIX_SOCKET, userdir) < 0)
> > + if (virAsprintf(&sockname, "%s/%s-sock",
> > + userdir, sock_prefix) < 0)
>
> I kinda just noticed, but don't we support R/O connections in
> session mode?
The client app is required to be the same user ID as the daemon.
As such there's no meaningful security separation between the
two from a DAC pov, so R/O socket was deemed to be a waste of
time.
If you had SELinux strictly locking things down it could be
considered slightly more secure, but no one has ever cared
enough to enable it.
> > + if (!direct_sock_name) {
> > + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> > + _("Cannot use direct socket mode if no URI is set"));
> > + return NULL;
> > + }
>
> Is the error message accurate? We should be way past making sure we
> have a URI to work with by now.
We'll only hit direct_sock_name == NULL, if driver == NULL.
We'll only hit driver == NULL, if the original URI was NULL.
> > +#ifndef WIN32
> > +static const char *
> > +remoteGetDaemonPathEnv(void)
> > +{
> > + /* We prefer a VIRTD_PATH env var to use for all daemons,
> > + * but if it is not set we will fallback to LIBVIRTD_PATH
> > + * for previous behaviour
> > + */
> > + if (virGetEnvBlockSUID("VIRTD_PATH") != NULL) {
> > + return "VIRTD_PATH";
> > + } else {
> > + return "LIBVIRTD_PATH";
> > + }
> > +}
> > +#endif /* WIN32 */
>
> I don't think this function needs to be guarded by 'ifndef WIN32':
> we already do so at the call site, and AFAICT there's nothing in the
> helper itself that warrants compiling it out on Windows.
It is a static function, so will trigger an unused function warning.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
