Working qrexec-policy-daemon

Rewritten qrexec-daemon to use policy daemon instead of running
policy-exec separately for each call. If daemon fails, falls back
to old solution.

fixes QubesOS/qubes-issues#5125

.travis.yml

@@ -24,7 +24,7 @@ jobs:
     - python: '3.5'
       script: pylint qrexec
     - python: '3.5'
-      script: python -m coverage run -m unittest discover -s qrexec/tests -t . -p '*.py' -v
+      script: python -m coverage run -m pytest qrexec/tests -o python_files=*.py -v
 #    - python: '3.6'
 #      script: python -m coverage run -m unittest discover -s qrexec/tests -t . -p '*.py' -v
 #    - python: '3.7'

Makefile

@@ -38,6 +38,8 @@ install-dom0:
 	install -d $(DESTDIR)/etc/qubes/policy.d -m 775
 	install -d $(DESTDIR)/etc/qubes/policy.d/include -m 775
 	install -t $(DESTDIR)/etc/qubes/policy.d -m 664 policy.d/*
+	install -d $(DESTDIR)/lib/systemd/system -m 755
+	install -t $(DESTDIR)/lib/systemd/system -m 644 systemd/qubes-qrexec-policy-daemon.service
 .PHONY: install-dom0
 
 
@@ -48,7 +50,7 @@ all-vm:
 install-vm:
 	$(MAKE) install -C agent
 	install -d $(DESTDIR)/lib/systemd/system -m 755
-	install -t $(DESTDIR)/lib/systemd/system -m 644 systemd/*
+	install -t $(DESTDIR)/lib/systemd/system -m 644 systemd/qubes-qrexec-agent.service
 	install -m 0644 -D qubes-rpc-config/README $(DESTDIR)/etc/qubes/rpc-config/README
 #	install -d $(DESTDIR)/etc/qubes-rpc -m 755
 #	install -t $(DESTDIR)/etc/qubes-rpc -m 755 qubes-rpc/*

ci/requirements.txt

@@ -5,3 +5,4 @@ pylint
 sphinx
 codecov
 pydbus
+pytest-asyncio

daemon/qrexec-daemon.c

@@ -18,6 +18,7 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  *
  */
+#define _GNU_SOURCE
 
 #include <sys/select.h>
 #include <stdio.h>
@@ -28,12 +29,17 @@
 #include <fcntl.h>
 #include <sys/stat.h>
 #include <sys/wait.h>
+#include <sys/socket.h>
+#include <sys/un.h>
+#include <err.h>
 #include <string.h>
 #include <assert.h>
 #include "qrexec.h"
 #include "libqrexec-utils.h"
 
 #define QREXEC_MIN_VERSION QREXEC_PROTOCOL_V2
+#define QREXEC_SOCKET_PATH "/var/run/qubes/policy.sock"
+
 
 enum client_state {
     CLIENT_INVALID = 0,	// table slot not used
@@ -678,6 +684,73 @@ static void sanitize_name(char * untrusted_s_signed, char *extra_allowed_chars)
  * Called when agent sends a message asking to execute a predefined command.
  */
 
+static int connect_daemon_socket(
+        const int remote_domain_id,
+        const char *remote_domain_name,
+        const char *target_domain,
+        const char *service_name,
+        const struct service_params *request_id
+) {
+    int result;
+    int command_size;
+    char response[32];
+    char *command;
+    int daemon_socket;
+    struct sockaddr_un daemon_socket_address = {
+        .sun_family = AF_UNIX,
+        .sun_path = QREXEC_SOCKET_PATH
+    };
+
+    daemon_socket = socket(AF_UNIX, SOCK_STREAM, 0);
+    if (daemon_socket < 0) {
+         perror("socket creation failed");
+         return -1;
+    }
+
+    result = connect(daemon_socket, (struct sockaddr *) &daemon_socket_address,
+            sizeof(daemon_socket_address));
+    if (result < 0) {
+         perror("connection to socket failed");
+         return -1;
+    }
+
+    command_size = asprintf(&command, "domain_id=%d\n"
+        "source=%s\n"
+        "intended_target=%s\n"
+        "service_and_arg=%s\n"
+        "process_ident=%s\n\n",
+        remote_domain_id, remote_domain_name, target_domain,
+        service_name, request_id->ident);
+    if (command_size < 0) {
+         perror("failed to construct request");
+         return -1;
+    }
+
+    result = send(daemon_socket, command, command_size, 0);
+    free(command);
+      if (result < 0) {
+         perror("send to socket failed");
+         return -1;
+      }
+
+    result = recv(daemon_socket, response, sizeof(response), 0);
+    if (result < 0) {
+         perror("error reading from socket");
+         return -1;
+    }
+    else {
+        if (!strncmp(response, "result=allow\n", sizeof("result=allow\n")-1)) {
+            return 0;
+        } else if (!strncmp(response, "result=deny\n", sizeof("result=deny\n")-1)) {
+            return 1;
+        } else {
+            warnx("invalid response");
+            return -1;
+        }
+    }
+}
+
+
 static void handle_execute_service(
         const int remote_domain_id,
         const char *remote_domain_name,
@@ -686,6 +759,7 @@ static void handle_execute_service(
         const struct service_params *request_id)
 {
     int i;
+    int result;
     int policy_pending_slot;
     pid_t pid;
     char remote_domain_id_str[10];
@@ -708,8 +782,17 @@ static void handle_execute_service(
             policy_pending[policy_pending_slot].params = *request_id;
             return;
     }
+
+    result = connect_daemon_socket(remote_domain_id, remote_domain_name,
+                                   target_domain, service_name, request_id);
+    if (result >= 0) {
+        _exit(result);
+    } else {
+        warnx("invalid response");
+    }
+
     for (i = 3; i < MAX_FDS; i++)
-        close(i);
+        close(i); /// should I do that?
     signal(SIGCHLD, SIG_DFL);
     signal(SIGPIPE, SIG_DFL);
     snprintf(remote_domain_id_str, sizeof(remote_domain_id_str), "%d",
@@ -725,6 +808,7 @@ static void handle_execute_service(
     _exit(1);
 }
 
+
 static void handle_connection_terminated()
 {
     struct exec_params untrusted_params, params;

debian/qubes-core-qrexec.install

@@ -6,6 +6,7 @@ usr/bin/qrexec-fork-server
 usr/bin/qrexec-policy-graph
 usr/bin/qrexec-policy-restore
 usr/bin/qrexec-policy-exec
+usr/bin/qrexec-policy-daemon
 usr/bin/qrexec-policy
 usr/bin/qrexec-policy-agent
 usr/bin/qubes-policy

qrexec/tools/qrexec_policy_daemon.py

@@ -37,13 +37,13 @@
     type=pathlib.Path, default=POLICYSOCKET,
     help='Use alternative policy socket path')
 
-required_request_arguments = ('domain_id', 'source', 'intended_target',
+REQUIRED_REQUEST_ARGUMENTS = ('domain_id', 'source', 'intended_target',
                               'service_and_arg', 'process_ident')
 
-optional_request_arguments = ('assume_yes_for_ask', 'just_evaluate')
+OPTIONAL_REQUEST_ARGUMENTS = ('assume_yes_for_ask', 'just_evaluate')
 
-allowed_request_arguments = required_request_arguments + \
-                            optional_request_arguments
+ALLOWED_REQUEST_ARGUMENTS = REQUIRED_REQUEST_ARGUMENTS + \
+                            OPTIONAL_REQUEST_ARGUMENTS
 
 
 async def handle_client_connection(log, policy_path, reader, writer):
@@ -64,7 +64,7 @@ async def handle_client_connection(log, policy_path, reader, writer):
                     'error parsing policy request: '
                     'duplicate argument {}'.format(argument))
                 return
-            if argument not in allowed_request_arguments:
+            if argument not in ALLOWED_REQUEST_ARGUMENTS:
                 log.error(
                     'error parsing policy request: unknown argument {}'.format(
                         argument))
@@ -83,22 +83,22 @@ async def handle_client_connection(log, policy_path, reader, writer):
 
             args[argument] = value
 
-        if not all(arg in args for arg in required_request_arguments):
+        if not all(arg in args for arg in REQUIRED_REQUEST_ARGUMENTS):
             log.error(
                 'error parsing policy request: required argument missing')
             return
 
         result = handle_request(**args, log=log, path=policy_path)
 
-        writer.write(b"result=allow\n" if result else b"result=deny\n")
+        writer.write(b"result=deny\n" if result else b"result=allow\n")
 
         await writer.drain()
 
     finally:
         writer.close()
 
 
-async def main(args=None):
+async def start_serving(args=None):
     args = argparser.parse_args(args)
 
     log = logging.getLogger('policy')
@@ -114,8 +114,11 @@ async def main(args=None):
     await server.serve_forever()
 
 
-if __name__ == '__main__':
-    asyncio.run(main())
-
+def main(args=None):
+    # pylint: disable=no-member
+    # due to travis' limitations we have to use python 3.5 in pylint
+    asyncio.run(start_serving(args))
 
 
+if __name__ == '__main__':
+    main()

qrexec/tools/qrexec_policy_exec.py

@@ -129,7 +129,7 @@ def main(args=None):
         just_evaluate=args.just_evaluate,
         assume_yes_for_ask=args.assume_yes_for_ask)
 
-
+# pylint: disable=too-many-arguments
 def handle_request(domain_id, source, intended_target, service_and_arg,
                    process_ident, log, path=POLICYPATH, just_evaluate=False,
                    assume_yes_for_ask=False):

rpm_spec/qubes-qrexec-dom0.spec.in

@@ -61,6 +61,21 @@ export BACKEND_VMM=@BACKEND_VMM@
 make all-dom0
 #make -C doc PYTHON=%{__python3} SPHINXBUILD=sphinx-build-%{python3_version} man
 
+%post
+%systemd_post qubes-qrexec-policy-daemon.service
+
+%preun
+%systemd_preun qubes-qrexec-policy-daemon.service
+
+%posttrans
+# when upgrading from R4.0, %%postun of qubes-qrexec-policy-daemon will revert
+# %%post of this package. Redo the action in %%posttrans
+%systemd_post qubes-qrexec-policy-daemon.service
+# and then start it back
+if systemctl is-enabled qubes-qrexec-policy-daemon.service >/dev/null 2>&1; then
+    systemctl start qubes-qrexec-policy-daemon.service
+fi
+
 %install
 make install-dom0 \
     DESTDIR=$RPM_BUILD_ROOT \
@@ -96,5 +111,7 @@ rm -f %{name}-%{version}
 
 %{_sysconfdir}/qubes-rpc/policy.RegisterArgument
 
+/lib/systemd/system/qubes-qrexec-policy-daemon.service
+
 %changelog
 @CHANGELOG@

rpm_spec/qubes-qrexec.spec.in

@@ -97,6 +97,7 @@ rm -f %{name}-%{version}
 %{_bindir}/qrexec-policy-agent
 %{_bindir}/qrexec-policy-graph
 %{_bindir}/qrexec-policy-restore
+%{_bindir}/qrexec-policy-daemon
 %{_bindir}/qubes-policy
 %{_bindir}/qrexec-policy
 
@@ -129,6 +130,7 @@ rm -f %{name}-%{version}
 %{python3_sitelib}/qrexec/tools/qubes_policy.py
 %{python3_sitelib}/qrexec/tools/qrexec_policy_agent.py
 %{python3_sitelib}/qrexec/tools/qrexec_policy_exec.py
+%{python3_sitelib}/qrexec/tools/qrexec_policy_daemon.py
 %{python3_sitelib}/qrexec/tools/qrexec_policy_graph.py
 %{python3_sitelib}/qrexec/tools/qrexec_policy_restore.py
 
@@ -141,6 +143,7 @@ rm -f %{name}-%{version}
 %{python3_sitelib}/qrexec/tests/rpcconfirmation.py
 %{python3_sitelib}/qrexec/tests/policy_api.py
 %{python3_sitelib}/qrexec/tests/policy_parser.py
+%{python3_sitelib}/qrexec/tests/qrexec_policy_daemon.py
 
 %dir %{python3_sitelib}/qrexec/glade
 %{python3_sitelib}/qrexec/glade/PolicyCreateConfirmationWindow.glade

systemd/qubes-qrexec-policy-daemon.service

@@ -0,0 +1,7 @@
+[Unit]
+Description=Qubes remote exec policy daemon
+After=qubesd.service
+
+[Service]
+Type=simple
+ExecStart=/usr/bin/qrexec-policy-daemon