Since feb83c1e710b9ea8044a89346f4868d03b31b0f1 libvirtd will abort on
startup if run as non-root

  2020-07-01 16:30:30.738+0000: 1647444: error : virDirOpenInternal:2869 : cannot open directory '/etc/libvirt/hooks/daemon.d': Permission denied

The root cause flaw is that non-root libvirtd is using /etc/libvirt for
its hooks. Traditionally that has been harmless though since we checked
whether we could access the hook file and degraded gracefully. We need
the same access check for iterating over the hook directory.

Long term we should make it possible to have an unprivileged hook dir
under $HOME.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/util/virhook.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/src/util/virhook.c b/src/util/virhook.c
index 3e025fd3a6..a7ab98560c 100644
--- a/src/util/virhook.c
+++ b/src/util/virhook.c
@@ -171,6 +171,12 @@ virHookCheck(int no, const char *driver)
     }
 
     dir_path = g_strdup_printf("%s.d", path);
+
+    if (access(dir_path, X_OK | R_OK) < 0) {
+        VIR_DEBUG("Hook dir %s is not accessible", dir_path);
+        return 1;
+    }
+
     if ((ret = virDirOpenIfExists(&dir, dir_path)) < 0)
         return -1;
 
@@ -415,6 +421,10 @@ virHookCall(int driver,
     }
 
     dir_path = g_strdup_printf("%s.d", path);
+
+    if (access(dir_path, X_OK | R_OK) < 0)
+        return script_ret;
+
     if ((ret = virDirOpenIfExists(&dir, dir_path)) < 0)
         return -1;
 
-- 
2.26.2

On 7/1/20 1:39 PM, Daniel P. Berrangé wrote:
> Since feb83c1e710b9ea8044a89346f4868d03b31b0f1 libvirtd will abort on
> startup if run as non-root
> 
>    2020-07-01 16:30:30.738+0000: 1647444: error : virDirOpenInternal:2869 : cannot open directory '/etc/libvirt/hooks/daemon.d': Permission denied
> 
> The root cause flaw is that non-root libvirtd is using /etc/libvirt for
> its hooks. Traditionally that has been harmless though since we checked
> whether we could access the hook file and degraded gracefully. We need
> the same access check for iterating over the hook directory.
> 
> Long term we should make it possible to have an unprivileged hook dir
> under $HOME.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---


This failed syntax-check:


../src/util/virhook.c:175:    if (access(dir_path, X_OK | R_OK) < 0) {
../src/util/virhook.c:425:    if (access(dir_path, X_OK | R_OK) < 0)
build-aux/syntax-check.mk: use virFileIsExecutable instead of access(,X_OK)
make: *** [../build-aux/syntax-check.mk:400: sc_prohibit_access_xok] Error 1
make: *** Waiting for unfinished jobs....


Given that this didn't break any tests and I believe we want this for the
upcoming release, feel free to change it to make 'syntax-check' happy and
add my r-b:


Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>