broadcast: add TALK access checks for broadcast messages

Add code to perform broadcast access checks, we split the shared code
into two functions:
kdbus_custom_ep_check_talk_access()
kdbus_ep_has_default_talk_access()

And add kdbus_ep_policy_check_broadcast() to do the broadcast access
checks.

To perform broadcast, these rules must be satisfied:

1) Check custom endpoint policies, if it allows the TALK continue,
   otherwise block.

2) If the sender connection is a privileged connection, allow
broadcast.

3) If the sender and receiver run under the same user, allow broadcast.

4) If the sender connection owns names on the bus and if
the destination connection do not own names, allow broadcast. Otherwise
fail check the bus policy rules for these two reasons:

   * anonymous connections should not signal to other connections.
   * receivers that own names may have policies that block the TALK
     access, so do not bypass this.

  This openes the case where connections that own names may gain TALK
  access to other connections on the bus through broadcast! Yes but
  since this is the intended behaviour of signals we can't do
  otherwise. Of course as stated above if the destination owns names
  then broadcasts are subject to policy rules (we do not bypass policy
  rules).

5) If the policy rules of the default endpoint block the TALK access,
then block broadcasts, otherwise allow it.

These are the same rules that apply to TALK access and unicast checks,
the only exception is rule 4) that was introduced to allow services to
signal on the bus.

Signed-off-by: Djalal Harouni <tixxdz@opendz.org>

diff --git a/connection.c b/connection.c
index ef71241155cc58746d192f654e49d5c3af3d138f..1b6be3bd0838c1f64ca061a41c616dea6eb55d61 100644 (file)
--- a/connection.c
+++ b/connection.c
@@ -606,6 +606,13 @@ static void kdbus_conn_broadcast(struct kdbus_ep *ep,
                 * data, even when they did not ask for it.
                 */
                if (conn_src) {
+                       /* Check if conn_src is allowed to signal */
+                       ret = kdbus_ep_policy_check_broadcast(conn_dst->ep,
+                                                             conn_src,
+                                                             conn_dst);
+                       if (ret < 0)
+                               continue;
+
                        ret = kdbus_ep_policy_check_src_names(conn_dst->ep,
                                                              conn_src,
                                                              conn_dst);

diff --git a/endpoint.c b/endpoint.c
index 6e8e361c77ed8d7a92bf5633cf98f4a39459320d..faf10497973d52ac5420cbe7358ab7be93e6d180 100644 (file)
--- a/endpoint.c
+++ b/endpoint.c
@@ -392,6 +392,43 @@ int kdbus_ep_policy_check_src_names(struct kdbus_ep *ep,
        return ret;
 }
 
+static int
+kdbus_custom_ep_check_talk_access(struct kdbus_ep *ep,
+                                 struct kdbus_conn *conn_src,
+                                 struct kdbus_conn *conn_dst)
+{
+       int ret;
+
+       if (!ep->has_policy)
+               return 0;
+
+       /* Custom endpoints have stricter policies */
+       ret = kdbus_policy_check_talk_access(&ep->policy_db,
+                                            conn_src, conn_dst);
+
+       /*
+        * Don't leak hints whether a name exists on a custom
+        * endpoint.
+        */
+       if (ret == -EPERM)
+               ret = -ENOENT;
+
+       return ret;
+}
+
+static bool
+kdbus_ep_has_default_talk_access(struct kdbus_conn *conn_src,
+                                struct kdbus_conn *conn_dst)
+{
+       if (kdbus_bus_cred_is_privileged(conn_src->bus, conn_src->cred))
+               return true;
+
+       if (uid_eq(conn_src->cred->fsuid, conn_dst->cred->uid))
+               return true;
+
+       return false;
+}
+
 /**
  * kdbus_ep_policy_check_talk_access() - verify a connection can talk to the
  *                                      the passed connection
@@ -410,26 +447,69 @@ int kdbus_ep_policy_check_talk_access(struct kdbus_ep *ep,
 {
        int ret;
 
-       if (ep->has_policy) {
-               ret = kdbus_policy_check_talk_access(&ep->policy_db,
-                                                    conn_src, conn_dst);
+       /* First check the custom endpoint with its policies */
+       ret = kdbus_custom_ep_check_talk_access(ep, conn_src, conn_dst);
+       if (ret < 0)
+               return ret;
 
-               /*
-                * Don't leak hints whether a name exists on a custom
-                * endpoint.
-                */
-               if (ret == -EPERM)
-                       return -ENOENT;
+       /* Then check if it satisfies the implicit policies */
+       if (kdbus_ep_has_default_talk_access(conn_src, conn_dst))
+               return 0;
 
-               if (ret < 0)
-                       return ret;
-       }
+       /* Fallback to the default endpoint policy */
+       ret = kdbus_policy_check_talk_access(&ep->bus->policy_db,
+                                            conn_src, conn_dst);
+       if (ret < 0)
+               return ret;
 
-       if (kdbus_bus_cred_is_privileged(conn_src->bus, conn_src->cred))
+       return 0;
+}
+
+/**
+ * kdbus_ep_policy_check_broadcast() - verify a connection can send
+ *                                    broadcast messages to the
+ *                                    passed connection
+ * @ep:                        Endpoint to operate on
+ * @conn_src:          Connection that tries to talk
+ * @conn_dst:          Connection that is talked to
+ *
+ * This verifies that @conn_src is allowed to send broadcast messages
+ * to @conn_dst via the endpoint @ep.
+ *
+ * Return: 0 if allowed, negative error code if not.
+ */
+int kdbus_ep_policy_check_broadcast(struct kdbus_ep *ep,
+                                   struct kdbus_conn *conn_src,
+                                   struct kdbus_conn *conn_dst)
+{
+       int ret;
+
+       /* First check the custom endpoint with its policies */
+       ret = kdbus_custom_ep_check_talk_access(ep, conn_src, conn_dst);
+       if (ret < 0)
+               return ret;
+
+       /* Then check if it satisfies the implicit policies */
+       if (kdbus_ep_has_default_talk_access(conn_src, conn_dst))
                return 0;
-       if (uid_eq(conn_src->cred->fsuid, conn_dst->cred->uid))
+
+       /*
+        * If conn_src owns names on the bus, and the conn_dst does
+        * not own any name, then allow conn_src to signal to
+        * conn_dst. Otherwise fallback and perform the bus policy
+        * check on conn_dst.
+        *
+        * This way we allow services to signal on the bus, and we
+        * block broadcasts directed to services that own names and
+        * do not want to receive these messages unless there is a
+        * policy entry to permit it. By this we try to follow the
+        * same logic used for unicat messages.
+        */
+       if (atomic_read(&conn_src->name_count) > 0 &&
+           atomic_read(&conn_dst->name_count) == 0)
                return 0;
 
+       /* Fallback to the default endpoint policy */
        ret = kdbus_policy_check_talk_access(&ep->bus->policy_db,
                                             conn_src, conn_dst);
        if (ret < 0)

diff --git a/endpoint.h b/endpoint.h
index 018c74ad177be3d34fdc674a110160f3cb73b56c..23ca8266fd4d2063b0142b5e643f58be7d020d2c 100644 (file)
--- a/endpoint.h
+++ b/endpoint.h
@@ -83,6 +83,9 @@ int kdbus_ep_policy_check_src_names(struct kdbus_ep *ep,
 int kdbus_ep_policy_check_talk_access(struct kdbus_ep *ep,
                                      struct kdbus_conn *conn_src,
                                      struct kdbus_conn *conn_dst);
+int kdbus_ep_policy_check_broadcast(struct kdbus_ep *ep,
+                                   struct kdbus_conn *conn_src,
+                                   struct kdbus_conn *conn_dst);
 int kdbus_ep_policy_check_own_access(struct kdbus_ep *ep,
                                     const struct kdbus_conn *conn,
                                     const char *name);