chiark / gitweb /
bus-proxy: fix policy for expected/non-expected reply tags
[elogind.git] / src / bus-proxyd / bus-proxyd.c
index 64c11ffd169760d21866f3c76ba25f679232a460..3cbbab718bd139bed498e67ce2c5857e606d6998 100644 (file)
@@ -139,9 +139,9 @@ static int parse_argv(int argc, char *argv[]) {
                                 return log_oom();
 
 #ifdef ENABLE_KDBUS
-                        a = strjoin("x-container-kernel:machine=", e, ";x-container-unix:machine=", e, NULL);
+                        a = strjoin("x-machine-kernel:machine=", e, ";x-machine-unix:machine=", e, NULL);
 #else
-                        a = strjoin("x-container-unix:machine=", e, NULL);
+                        a = strjoin("x-machine-unix:machine=", e, NULL);
 #endif
                         if (!a)
                                 return log_oom();
@@ -189,7 +189,7 @@ static int rename_service(sd_bus *a, sd_bus *b) {
         assert(a);
         assert(b);
 
-        r = sd_bus_get_owner_creds(b, SD_BUS_CREDS_UID|SD_BUS_CREDS_PID|SD_BUS_CREDS_CMDLINE|SD_BUS_CREDS_COMM, &creds);
+        r = sd_bus_get_owner_creds(b, SD_BUS_CREDS_UID|SD_BUS_CREDS_PID|SD_BUS_CREDS_CMDLINE|SD_BUS_CREDS_COMM|SD_BUS_CREDS_AUGMENT, &creds);
         if (r < 0)
                 return r;
 
@@ -345,6 +345,17 @@ static int synthetic_reply_method_error(sd_bus_message *call, const sd_bus_error
         return synthetic_driver_send(call->bus, m);
 }
 
+static int synthetic_reply_method_errorf(sd_bus_message *call, const char *name, const char *format, ...) {
+        _cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL;
+        va_list ap;
+
+        va_start(ap, format);
+        bus_error_setfv(&error, name, format, ap);
+        va_end(ap);
+
+        return synthetic_reply_method_error(call, &error);
+}
+
 static int synthetic_reply_method_errno(sd_bus_message *call, int error, const sd_bus_error *p) {
 
         _cleanup_bus_error_free_ sd_bus_error berror = SD_BUS_ERROR_NULL;
@@ -394,6 +405,9 @@ static int synthetic_reply_return_strv(sd_bus_message *call, char **l) {
 
         assert(call);
 
+        if (call->header->flags & BUS_MESSAGE_NO_REPLY_EXPECTED)
+                return 0;
+
         r = sd_bus_message_new_method_return(call, &m);
         if (r < 0)
                 return synthetic_reply_method_errno(call, r, NULL);
@@ -963,7 +977,7 @@ static int process_driver(sd_bus *a, sd_bus *b, sd_bus_message *m, Policy *polic
 
 static int handle_policy_error(sd_bus_message *m, int r) {
         if (r == -ESRCH || r == -ENXIO)
-                return sd_bus_reply_method_errorf(m, SD_BUS_ERROR_NAME_HAS_NO_OWNER, "Name %s is currently not owned by anyone.", m->destination);
+                return synthetic_reply_method_errorf(m, SD_BUS_ERROR_NAME_HAS_NO_OWNER, "Name %s is currently not owned by anyone.", m->destination);
 
         return r;
 }
@@ -978,6 +992,22 @@ static int process_policy(sd_bus *from, sd_bus *to, sd_bus_message *m, Policy *p
         if (!policy)
                 return 0;
 
+        /*
+         * dbus-1 distinguishes expected and non-expected replies by tracking
+         * method-calls and timeouts. By default, DENY rules are *NEVER* applied
+         * on expected replies, unless explicitly specified. But we dont track
+         * method-calls, thus, we cannot know whether a reply is expected.
+         * Fortunately, the kdbus forbids non-expected replies, so we can safely
+         * ignore any policy on those and let the kernel deal with it.
+         *
+         * TODO: To be correct, we should only ignore policy-tags that are
+         * applied on non-expected replies. However, so far we don't parse those
+         * tags so we let everything pass. I haven't seen a DENY policy tag on
+         * expected-replies, ever, so don't bother..
+         */
+        if (m->reply_cookie > 0)
+                return 0;
+
         if (from->is_kernel) {
                 uid_t sender_uid = UID_INVALID;
                 gid_t sender_gid = GID_INVALID;
@@ -994,6 +1024,24 @@ static int process_policy(sd_bus *from, sd_bus *to, sd_bus_message *m, Policy *p
                 (void) sd_bus_creds_get_uid(&m->creds, &sender_uid);
                 (void) sd_bus_creds_get_gid(&m->creds, &sender_gid);
 
+                if (sender_uid == UID_INVALID || sender_gid == GID_INVALID) {
+                        _cleanup_bus_creds_unref_ sd_bus_creds *sender_creds = NULL;
+
+                        /* If the message came from another legacy
+                         * client, then the message creds will be
+                         * missing, simply because on legacy clients
+                         * per-message creds were unknown. In this
+                         * case, query the creds of the peer
+                         * instead. */
+
+                        r = bus_get_name_creds_kdbus(from, m->sender, SD_BUS_CREDS_UID|SD_BUS_CREDS_GID, true, &sender_creds);
+                        if (r < 0)
+                                return handle_policy_error(m, r);
+
+                        (void) sd_bus_creds_get_uid(sender_creds, &sender_uid);
+                        (void) sd_bus_creds_get_gid(sender_creds, &sender_gid);
+                }
+
                 /* First check whether the sender can send the message to our name */
                 if (set_isempty(owned_names)) {
                         if (policy_check_send(policy, sender_uid, sender_gid, m->header->type, NULL, m->path, m->interface, m->member))
@@ -1010,7 +1058,7 @@ static int process_policy(sd_bus *from, sd_bus *to, sd_bus_message *m, Policy *p
                 }
 
                 if (granted) {
-                        /* Then check whether us (the recipient) can recieve from the sender's name */
+                        /* Then check whether us (the recipient) can receive from the sender's name */
                         if (strv_isempty(sender_names)) {
                                 if (policy_check_recv(policy, our_ucred->uid, our_ucred->gid, m->header->type, NULL, m->path, m->interface, m->member))
                                         return 0;
@@ -1026,7 +1074,7 @@ static int process_policy(sd_bus *from, sd_bus *to, sd_bus_message *m, Policy *p
 
                 /* Return an error back to the caller */
                 if (m->header->type == SD_BUS_MESSAGE_METHOD_CALL)
-                        return sd_bus_reply_method_errorf(m, SD_BUS_ERROR_ACCESS_DENIED, "Access prohibited by XML receiver policy.");
+                        return synthetic_reply_method_errorf(m, SD_BUS_ERROR_ACCESS_DENIED, "Access prohibited by XML receiver policy.");
 
                 /* Return 1, indicating that the message shall not be processed any further */
                 return 1;
@@ -1115,7 +1163,7 @@ static int process_policy(sd_bus *from, sd_bus *to, sd_bus_message *m, Policy *p
 
                 /* Return an error back to the caller */
                 if (m->header->type == SD_BUS_MESSAGE_METHOD_CALL)
-                        return sd_bus_reply_method_errorf(m, SD_BUS_ERROR_ACCESS_DENIED, "Access prohibited by XML sender policy.");
+                        return synthetic_reply_method_errorf(m, SD_BUS_ERROR_ACCESS_DENIED, "Access prohibited by XML sender policy.");
 
                 /* Return 1, indicating that the message shall not be processed any further */
                 return 1;
@@ -1268,6 +1316,7 @@ int main(int argc, char *argv[]) {
         _cleanup_free_ char *peersec = NULL;
         Policy policy_buffer = {}, *policy = NULL;
         _cleanup_set_free_free_ Set *owned_names = NULL;
+        uid_t original_uid;
 
         log_set_target(LOG_TARGET_JOURNAL_OR_KMSG);
         log_parse_environment();
@@ -1289,6 +1338,8 @@ int main(int argc, char *argv[]) {
                 goto finish;
         }
 
+        original_uid = getuid();
+
         is_unix =
                 sd_is_socket(in_fd, AF_UNIX, 0, 0) > 0 &&
                 sd_is_socket(out_fd, AF_UNIX, 0, 0) > 0;
@@ -1430,7 +1481,11 @@ int main(int argc, char *argv[]) {
                 policy = &policy_buffer;
                 /* policy_dump(policy); */
 
-                if (!policy_check_hello(policy, ucred.uid, ucred.gid)) {
+                if (ucred.uid == original_uid)
+                        log_debug("Permitting access, since bus owner matches bus client.");
+                else if (policy_check_hello(policy, ucred.uid, ucred.gid))
+                        log_debug("Permitting access due to XML policy.");
+                else {
                         r = log_error_errno(EPERM, "Policy denied connection.");
                         goto finish;
                 }
@@ -1591,14 +1646,26 @@ int main(int argc, char *argv[]) {
                                 if (!processed) {
                                         k = sd_bus_send(b, m, NULL);
                                         if (k < 0) {
-                                                if (k == -ECONNRESET)
+                                                if (k == -ECONNRESET) {
                                                         r = 0;
-                                                else {
+                                                        goto finish;
+                                                } else if (k == -EPERM && m->reply_cookie > 0) {
+                                                        /* If the peer tries to send a reply and it is rejected with EPERM
+                                                         * by the kernel, we ignore the error. This catches cases where the
+                                                         * original method-call didn't had EXPECT_REPLY set, but the proxy-peer
+                                                         * still sends a reply. This is allowed in dbus1, but not in kdbus. We
+                                                         * don't want to track reply-windows in the proxy, so we simply ignore
+                                                         * EPERM for all replies. The only downside is, that callers are no
+                                                         * longer notified if their replies are dropped. However, this is
+                                                         * equivalent to the caller's timeout to expire, so this should be
+                                                         * acceptable. Nobody sane sends replies without a matching method-call,
+                                                         * so nobody should care. */
+                                                        r = 1;
+                                                } else {
                                                         r = k;
                                                         log_error_errno(r, "Failed to send message to client: %m");
+                                                        goto finish;
                                                 }
-
-                                                goto finish;
                                         } else
                                                 r = 1;
                                 }
@@ -1668,17 +1735,20 @@ int main(int argc, char *argv[]) {
 
                                                 k = sd_bus_send(a, m, NULL);
                                                 if (k < 0) {
-                                                        if (k == -EREMCHG)
+                                                        if (k == -EREMCHG) {
                                                                 /* The name database changed since the policy check, hence let's check again */
                                                                 continue;
-                                                        else if (k == -ECONNRESET)
+                                                        } else if (k == -ECONNRESET) {
                                                                 r = 0;
-                                                        else {
+                                                                goto finish;
+                                                        } else if (k == -EPERM && m->reply_cookie > 0) {
+                                                                /* see above why EPERM is ignored for replies */
+                                                                r = 1;
+                                                        } else {
                                                                 r = k;
                                                                 log_error_errno(r, "Failed to send message to bus: %m");
+                                                                goto finish;
                                                         }
-
-                                                        goto finish;
                                                 } else
                                                         r = 1;