[SSSD] [PATCH] Two sbus fixes

Jakub Hrozek jhrozek at redhat.com
Tue Sep 1 08:05:46 UTC 2015


On Wed, Aug 26, 2015 at 02:26:06PM +0200, Pavel Březina wrote:
> On 08/10/2015 03:31 PM, Jakub Hrozek wrote:
> >Hi,
> >
> >I found two issues in the sbus code while testing the by-certificate
> >lookups. One was that if parsing the reply with client UID failed, then
> >errno would have been garbage.
> >
> >The second is that since we added signal handling, we always receive
> >NameOwnerChanged after someone binds to the bus. But NameOwnerChanged
> >sender is the bus itself, which doesn't reveal the sender ID -- the
> >request to find the sender ID failed and we didn't process the signal at
> >all.
> >
> >The only related discussion I found was:
> >     http://permalink.gmane.org/gmane.comp.freedesktop.dbus/7404
> >which would indicate that for the bus itself, the UID can't be
> >retrieved. So I modified the request that reads the UID to return a
> >special error in this case and lets the caller handle it -- for signals,
> >we proceed, but for messages, we don't because we can't check the UID
> >against the ACL.
> 
> Makes sense.
> 
> Ack, but it needs to be rebased.

Rebased patches are attached.
-------------- next part --------------
>From 70c8542ec28650da1217ef242cc848db03d1ba7d Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Fri, 7 Aug 2015 16:57:46 +0200
Subject: [PATCH 1/2] sbus: Initialize errno if constructing message fails and
 add debug messages

---
 src/sbus/sssd_dbus_request.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/sbus/sssd_dbus_request.c b/src/sbus/sssd_dbus_request.c
index 6a012f1c669bb093b6934b13d9f4688c178d1dc2..888a0eeb53ed62f45ab4f95eec4508e960a6ca40 100644
--- a/src/sbus/sssd_dbus_request.c
+++ b/src/sbus/sssd_dbus_request.c
@@ -333,6 +333,9 @@ struct tevent_req *sbus_get_sender_id_send(TALLOC_CTX *mem_ctx,
         goto immediate;
     }
 
+    DEBUG(SSSDBG_TRACE_INTERNAL,
+          "Looking for identity of sender [%s]\n", sender);
+
     key.type = HASH_KEY_STRING;
     key.str = discard_const(sender);
     ret = hash_lookup(conn->clients, &key, &value);
@@ -379,6 +382,7 @@ struct tevent_req *sbus_get_sender_id_send(TALLOC_CTX *mem_ctx,
                                      DBUS_TYPE_STRING, &sender,
                                      DBUS_TYPE_INVALID);
     if (!dbret) {
+        ret = ERR_INTERNAL;
         goto immediate;
     }
 
@@ -445,6 +449,7 @@ static void sbus_get_sender_id_done(DBusPendingCall *pending, void *ptr)
                                   DBUS_TYPE_UINT32, &uid,
                                   DBUS_TYPE_INVALID);
     if (!dbret) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "Could not parse reply!\n");
         ret = EIO;
         goto done;
     }
@@ -457,6 +462,7 @@ static void sbus_get_sender_id_done(DBusPendingCall *pending, void *ptr)
     value.ul = state->uid;
     ret = hash_enter(state->conn->clients, &key, &value);
     if (ret != HASH_SUCCESS) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "Could not add key to hash table!\n");
         ret = EIO;
         goto done;
     }
-- 
2.4.3

-------------- next part --------------
>From 7209bd420f5ee2053ae9f404477cacea77491af8 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Fri, 7 Aug 2015 17:13:02 +0200
Subject: [PATCH 2/2] sbus: Add a special error code for messages sent by the
 bus itself

---
 src/sbus/sssd_dbus_request.c | 5 +++++
 src/sbus/sssd_dbus_signals.c | 4 +++-
 src/util/util_errors.c       | 1 +
 src/util/util_errors.h       | 1 +
 4 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/src/sbus/sssd_dbus_request.c b/src/sbus/sssd_dbus_request.c
index 888a0eeb53ed62f45ab4f95eec4508e960a6ca40..552bb5da86668f4e94c0fb7699753706a0698c43 100644
--- a/src/sbus/sssd_dbus_request.c
+++ b/src/sbus/sssd_dbus_request.c
@@ -327,6 +327,11 @@ struct tevent_req *sbus_get_sender_id_send(TALLOC_CTX *mem_ctx,
         goto immediate;
     }
 
+    if (strcmp(sender, "org.freedesktop.DBus") == 0) {
+        ret = ERR_SBUS_SENDER_BUS;
+        goto immediate;
+    }
+
     state->sender = talloc_strdup(state, sender);
     if (state->sender == NULL) {
         ret = ENOMEM;
diff --git a/src/sbus/sssd_dbus_signals.c b/src/sbus/sssd_dbus_signals.c
index 5ecc9f1f60ab00cf69da3fa7791a0c15a9cc7038..1dc08ae25761881f23a81e7871a8af626497a9a3 100644
--- a/src/sbus/sssd_dbus_signals.c
+++ b/src/sbus/sssd_dbus_signals.c
@@ -267,7 +267,9 @@ sbus_signal_handler_got_caller_id(struct tevent_req *req)
     signal = tevent_req_callback_data(req, struct sbus_incoming_signal);
 
     ret = sbus_get_sender_id_recv(req, &signal->client);
-    if (ret != EOK) {
+    if (ret == ERR_SBUS_SENDER_BUS) {
+        DEBUG(SSSDBG_TRACE_FUNC, "Got a signal from the bus..\n");
+    } else if (ret != EOK) {
         DEBUG(SSSDBG_CRIT_FAILURE,
               "Failed to resolve caller's ID: %s\n", sss_strerror(ret));
         goto done;
diff --git a/src/util/util_errors.c b/src/util/util_errors.c
index ba61630c3b4b5b9b5d15324154b7dbbfa34e620f..fd6b9fbfe32eb8bb456e42cacacfebe439091754 100644
--- a/src/util/util_errors.c
+++ b/src/util/util_errors.c
@@ -80,6 +80,7 @@ struct err_string error_to_str[] = {
     { "Trusted forest root unknown" }, /* ERR_TRUST_FOREST_UNKNOWN */
     { "p11_child failed" }, /* ERR_P11_CHILD */
     { "Address family not supported" }, /* ERR_ADDR_FAMILY_NOT_SUPPORTED */
+    { "Message sender is the bus" }, /* ERR_SBUS_SENDER_BUS */
     { "ERR_LAST" } /* ERR_LAST */
 };
 
diff --git a/src/util/util_errors.h b/src/util/util_errors.h
index aa00cc592b38e7bbf6f95cfb6348f0bdc6ae2ed0..bda0c9b7d330d8e7a15bc460fc8ed97c80510ea8 100644
--- a/src/util/util_errors.h
+++ b/src/util/util_errors.h
@@ -102,6 +102,7 @@ enum sssd_errors {
     ERR_TRUST_FOREST_UNKNOWN,
     ERR_P11_CHILD,
     ERR_ADDR_FAMILY_NOT_SUPPORTED,
+    ERR_SBUS_SENDER_BUS,
     ERR_LAST            /* ALWAYS LAST */
 };
 
-- 
2.4.3



More information about the sssd-devel mailing list