[libvirt] [patch]make rundir permission equals to socket permission to support unprivileged access
Libvirt-socket-rw and libvirt-socket-ro are not used only for libvirt or root user, but also for unprivileged application such as vdsm, Restrain the rundir only read/search for libvirt prevent comunication with unprivileged client,change rundir the permission equals to the sockets permission. See bug: https://bugzilla.redhat.com/show_bug.cgi?id=828073 Signed-off-by: lvroyce <lvroyce@linux.vnet.ibm.com> --- daemon/libvirtd.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index c74cd43..6095072 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -293,7 +293,7 @@ daemonUnixSocketPaths(struct daemonConfig *config, if (!(rundir = virGetUserRuntimeDirectory())) goto error; - old_umask = umask(077); + old_umask = umask(022); if (virFileMakePath(rundir) < 0) { umask(old_umask); goto error;
On Tue, Jun 05, 2012 at 02:21:10PM +0800, Royce Lv wrote:
Libvirt-socket-rw and libvirt-socket-ro are not used only for libvirt or root user, but also for unprivileged application such as vdsm, Restrain the rundir only read/search for libvirt prevent comunication with unprivileged client,change rundir the permission equals to the sockets permission. See bug: https://bugzilla.redhat.com/show_bug.cgi?id=828073
That BZ is marked as a Regression; do you know if that's right, and if so, what commit caused it? Dave
Signed-off-by: lvroyce <lvroyce@linux.vnet.ibm.com> --- daemon/libvirtd.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index c74cd43..6095072 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -293,7 +293,7 @@ daemonUnixSocketPaths(struct daemonConfig *config, if (!(rundir = virGetUserRuntimeDirectory())) goto error;
- old_umask = umask(077); + old_umask = umask(022); if (virFileMakePath(rundir) < 0) { umask(old_umask); goto error;
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
2012/6/5 Dave Allan <dallan@redhat.com>
On Tue, Jun 05, 2012 at 02:21:10PM +0800, Royce Lv wrote:
Libvirt-socket-rw and libvirt-socket-ro are not used only for libvirt or root user, but also for unprivileged application such as vdsm, Restrain the rundir only read/search for libvirt prevent comunication with unprivileged client,change rundir the permission equals to the sockets permission. See bug: https://bugzilla.redhat.com/show_bug.cgi?id=828073
That BZ is marked as a Regression; do you know if that's right, and if so, what commit caused it?
2012/6/5 Dave Allan <dallan@redhat.com>
On Tue, Jun 05, 2012 at 02:21:10PM +0800, Royce Lv wrote:
Libvirt-socket-rw and libvirt-socket-ro are not used only for libvirt or root user, but also for unprivileged application such as vdsm, Restrain the rundir only read/search for libvirt prevent comunication with unprivileged client,change rundir the permission equals to the sockets permission. See bug: https://bugzilla.redhat.com/show_bug.cgi?id=828073
That BZ is marked as a Regression; do you know if that's right, and if so, what commit caused it?
I guess it should be this one: commit: 32a9aac2e04c991340b66c855a1095e4e6445e54
Dave
Signed-off-by: lvroyce <lvroyce@linux.vnet.ibm.com> --- daemon/libvirtd.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index c74cd43..6095072 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -293,7 +293,7 @@ daemonUnixSocketPaths(struct daemonConfig *config, if (!(rundir = virGetUserRuntimeDirectory())) goto error;
- old_umask = umask(077); + old_umask = umask(022); if (virFileMakePath(rundir) < 0) { umask(old_umask); goto error;
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Jun 05, 2012 at 02:21:10PM +0800, Royce Lv wrote:
Libvirt-socket-rw and libvirt-socket-ro are not used only for libvirt or root user, but also for unprivileged application such as vdsm, Restrain the rundir only read/search for libvirt prevent comunication with unprivileged client,change rundir the permission equals to the sockets permission. See bug: https://bugzilla.redhat.com/show_bug.cgi?id=828073
Signed-off-by: lvroyce <lvroyce@linux.vnet.ibm.com> --- daemon/libvirtd.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index c74cd43..6095072 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -293,7 +293,7 @@ daemonUnixSocketPaths(struct daemonConfig *config, if (!(rundir = virGetUserRuntimeDirectory())) goto error;
- old_umask = umask(077); + old_umask = umask(022); if (virFileMakePath(rundir) < 0) { umask(old_umask); goto error;
The bug you quote above talks about being unable to connect to /var/run/libvirt/libvirt-sock for the privileged libvirtd. The change you are proposing here only touches unprivileged libvirtd when it creates $XDG_RUNTIME_DIR/libvirt. Furthermore the change you are proposing is a security flaw, since the $XDG_RUNTIME_DIR/libvirt directory is *only* intended to be accessed by the current unprivileged user. Using a umask of 022 lets it be accessible to any user. So NACK to this Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Sorry for misunderstood the code ,test and submit the patch mistakenly, will submit a new one. 2012/6/8 Daniel P. Berrange <berrange@redhat.com>
On Tue, Jun 05, 2012 at 02:21:10PM +0800, Royce Lv wrote:
Libvirt-socket-rw and libvirt-socket-ro are not used only for libvirt or root user, but also for unprivileged application such as vdsm, Restrain the rundir only read/search for libvirt prevent comunication with unprivileged client,change rundir the permission equals to the sockets permission. See bug: https://bugzilla.redhat.com/show_bug.cgi?id=828073
Signed-off-by: lvroyce <lvroyce@linux.vnet.ibm.com> --- daemon/libvirtd.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index c74cd43..6095072 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -293,7 +293,7 @@ daemonUnixSocketPaths(struct daemonConfig *config, if (!(rundir = virGetUserRuntimeDirectory())) goto error;
- old_umask = umask(077); + old_umask = umask(022); if (virFileMakePath(rundir) < 0) { umask(old_umask); goto error;
The bug you quote above talks about being unable to connect to /var/run/libvirt/libvirt-sock for the privileged libvirtd.
The change you are proposing here only touches unprivileged libvirtd when it creates $XDG_RUNTIME_DIR/libvirt.
Furthermore the change you are proposing is a security flaw, since the $XDG_RUNTIME_DIR/libvirt directory is *only* intended to be accessed by the current unprivileged user. Using a umask of 022 lets it be accessible to any user.
So NACK to this
Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/:| |: http://libvirt.org -o- http://virt-manager.org:| |: http://autobuild.org -o- http://search.cpan.org/~danberr/:| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc:|
participants (3)
-
Daniel P. Berrange -
Dave Allan -
Royce Lv