From 30fa2e1ace062314e9624b29239c2c7e9519e6c2 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 23 Aug 2013 11:09:35 +0100 Subject: [PATCH] Revert "Factor out DBusAuthorization from DBusTransport" This reverts commit 600621dbc8073527a958091316eddfbb490c1032. --- bus/driver.c | 11 -- cmake/dbus/CMakeLists.txt | 2 - dbus/Makefile.am | 10 +- dbus/dbus-transport-protected.h | 13 +- dbus/dbus-transport.c | 267 ++++++++++++++++++++++++++++++++-------- doc/dbus-specification.xml | 8 -- 6 files changed, 229 insertions(+), 82 deletions(-) diff --git a/bus/driver.c b/bus/driver.c index 564cecb..23197e4 100644 --- a/bus/driver.c +++ b/bus/driver.c @@ -1535,7 +1535,6 @@ bus_driver_handle_get_connection_credentials (DBusConnection *connection, DBusMessageIter reply_iter; DBusMessageIter array_iter; unsigned long ulong_val; - char *windows_sid; const char *service; _DBUS_ASSERT_ERROR_IS_CLEAR (error); @@ -1570,16 +1569,6 @@ bus_driver_handle_get_connection_credentials (DBusConnection *connection, goto oom; } - if (dbus_connection_get_windows_user (conn, &windows_sid)) - { - if (!_dbus_asv_add_string (&array_iter, "WindowsSID", windows_sid)) - { - dbus_free(windows_sid); - goto oom; - } - dbus_free(windows_sid); - } - if (!_dbus_asv_close (&reply_iter, &array_iter)) goto oom; diff --git a/cmake/dbus/CMakeLists.txt b/cmake/dbus/CMakeLists.txt index bb7278c..0205f85 100644 --- a/cmake/dbus/CMakeLists.txt +++ b/cmake/dbus/CMakeLists.txt @@ -32,7 +32,6 @@ set (dbusinclude_HEADERS set (DBUS_LIB_SOURCES ${DBUS_DIR}/dbus-address.c ${DBUS_DIR}/dbus-auth.c - ${DBUS_DIR}/dbus-authorization.c ${DBUS_DIR}/dbus-bus.c ${DBUS_DIR}/dbus-connection.c ${DBUS_DIR}/dbus-credentials.c @@ -76,7 +75,6 @@ endif(UNIX) set (DBUS_LIB_HEADERS ${DBUS_DIR}/dbus-auth.h - ${DBUS_DIR}/dbus-authorization.h ${DBUS_DIR}/dbus-connection-internal.h ${DBUS_DIR}/dbus-credentials.h ${DBUS_DIR}/dbus-keyring.h diff --git a/dbus/Makefile.am b/dbus/Makefile.am index 180a44e..e118cbb 100644 --- a/dbus/Makefile.am +++ b/dbus/Makefile.am @@ -150,8 +150,6 @@ DBUS_LIB_SOURCES= \ dbus-address.c \ dbus-auth.c \ dbus-auth.h \ - dbus-authorization.c \ - dbus-authorization.h \ dbus-bus.c \ dbus-connection.c \ dbus-connection-internal.h \ @@ -210,8 +208,8 @@ DBUS_LIB_SOURCES= \ DBUS_SHARED_SOURCES= \ dbus-dataslot.c \ dbus-dataslot.h \ - dbus-file.c \ - dbus-file.h \ + dbus-file.c \ + dbus-file.h \ dbus-hash.c \ dbus-hash.h \ dbus-internals.c \ @@ -223,8 +221,8 @@ DBUS_SHARED_SOURCES= \ dbus-memory.c \ dbus-mempool.c \ dbus-mempool.h \ - dbus-pipe.c \ - dbus-pipe.h \ + dbus-pipe.c \ + dbus-pipe.h \ dbus-string.c \ dbus-string.h \ dbus-string-private.h \ diff --git a/dbus/dbus-transport-protected.h b/dbus/dbus-transport-protected.h index 93380ab..396f0ff 100644 --- a/dbus/dbus-transport-protected.h +++ b/dbus/dbus-transport-protected.h @@ -27,7 +27,6 @@ #include #include #include -#include #include #include @@ -89,7 +88,6 @@ struct DBusTransport DBusMessageLoader *loader; /**< Message-loading buffer. */ DBusAuth *auth; /**< Authentication conversation */ - DBusAuthorization *authorization; /**< Authorization conversation */ DBusCredentials *credentials; /**< Credentials of other end read from the socket */ @@ -102,12 +100,23 @@ struct DBusTransport char *expected_guid; /**< GUID we expect the server to have, #NULL on server side or if we don't have an expectation */ + DBusAllowUnixUserFunction unix_user_function; /**< Function for checking whether a user is authorized. */ + void *unix_user_data; /**< Data for unix_user_function */ + + DBusFreeFunction free_unix_user_data; /**< Function to free unix_user_data */ + + DBusAllowWindowsUserFunction windows_user_function; /**< Function for checking whether a user is authorized. */ + void *windows_user_data; /**< Data for windows_user_function */ + + DBusFreeFunction free_windows_user_data; /**< Function to free windows_user_data */ + unsigned int disconnected : 1; /**< #TRUE if we are disconnected. */ unsigned int authenticated : 1; /**< Cache of auth state; use _dbus_transport_peek_is_authenticated() to query value */ unsigned int send_credentials_pending : 1; /**< #TRUE if we need to send credentials */ unsigned int receive_credentials_pending : 1; /**< #TRUE if we need to receive credentials */ unsigned int is_server : 1; /**< #TRUE if on the server side */ unsigned int unused_bytes_recovered : 1; /**< #TRUE if we've recovered unused bytes from auth */ + unsigned int allow_anonymous : 1; /**< #TRUE if an anonymous client can connect */ }; dbus_bool_t _dbus_transport_init_base (DBusTransport *transport, diff --git a/dbus/dbus-transport.c b/dbus/dbus-transport.c index 661b54f..e68a9f0 100644 --- a/dbus/dbus-transport.c +++ b/dbus/dbus-transport.c @@ -28,7 +28,6 @@ #include "dbus-connection-internal.h" #include "dbus-watch.h" #include "dbus-auth.h" -#include "dbus-authorization.h" #include "dbus-address.h" #include "dbus-credentials.h" #include "dbus-mainloop.h" @@ -107,7 +106,6 @@ _dbus_transport_init_base (DBusTransport *transport, { DBusMessageLoader *loader; DBusAuth *auth; - DBusAuthorization *authorization = NULL; /* non-NULL only if is_server=TRUE */ DBusCounter *counter; char *address_copy; DBusCredentials *creds; @@ -115,26 +113,13 @@ _dbus_transport_init_base (DBusTransport *transport, loader = _dbus_message_loader_new (); if (loader == NULL) return FALSE; - - if (server_guid != NULL) - { - authorization = _dbus_authorization_new (); - if (authorization == NULL) - { - _dbus_message_loader_unref (loader); - return FALSE; /* OOM */ - } - - auth = _dbus_auth_server_new (server_guid); - } + + if (server_guid) + auth = _dbus_auth_server_new (server_guid); else - { - auth = _dbus_auth_client_new (); - } + auth = _dbus_auth_client_new (); if (auth == NULL) { - if (authorization != NULL) - _dbus_authorization_unref (authorization); _dbus_message_loader_unref (loader); return FALSE; } @@ -143,8 +128,6 @@ _dbus_transport_init_base (DBusTransport *transport, if (counter == NULL) { _dbus_auth_unref (auth); - if (authorization != NULL) - _dbus_authorization_unref (authorization); _dbus_message_loader_unref (loader); return FALSE; } @@ -154,13 +137,11 @@ _dbus_transport_init_base (DBusTransport *transport, { _dbus_counter_unref (counter); _dbus_auth_unref (auth); - if (authorization != NULL) - _dbus_authorization_unref (authorization); _dbus_message_loader_unref (loader); return FALSE; } - if (server_guid != NULL) + if (server_guid) { _dbus_assert (address == NULL); address_copy = NULL; @@ -174,8 +155,6 @@ _dbus_transport_init_base (DBusTransport *transport, _dbus_credentials_unref (creds); _dbus_counter_unref (counter); _dbus_auth_unref (auth); - if (authorization != NULL) - _dbus_authorization_unref (authorization); _dbus_message_loader_unref (loader); return FALSE; } @@ -184,7 +163,6 @@ _dbus_transport_init_base (DBusTransport *transport, transport->refcount = 1; transport->vtable = vtable; transport->loader = loader; - transport->authorization = authorization; transport->auth = auth; transport->live_messages = counter; transport->authenticated = FALSE; @@ -193,7 +171,15 @@ _dbus_transport_init_base (DBusTransport *transport, transport->send_credentials_pending = !transport->is_server; transport->receive_credentials_pending = transport->is_server; transport->address = address_copy; + + transport->unix_user_function = NULL; + transport->unix_user_data = NULL; + transport->free_unix_user_data = NULL; + transport->windows_user_function = NULL; + transport->windows_user_data = NULL; + transport->free_windows_user_data = NULL; + transport->expected_guid = NULL; /* Try to default to something that won't totally hose the system, @@ -217,10 +203,6 @@ _dbus_transport_init_base (DBusTransport *transport, if (transport->address) _dbus_verbose ("Initialized transport on address %s\n", transport->address); - /* we can have authorization data set only in server mode */ - _dbus_assert ((transport->is_server && transport->authorization != NULL) || - (!transport->is_server && transport->authorization == NULL)); - return TRUE; } @@ -236,10 +218,14 @@ _dbus_transport_finalize_base (DBusTransport *transport) if (!transport->disconnected) _dbus_transport_disconnect (transport); + if (transport->free_unix_user_data != NULL) + (* transport->free_unix_user_data) (transport->unix_user_data); + + if (transport->free_windows_user_data != NULL) + (* transport->free_windows_user_data) (transport->windows_user_data); + _dbus_message_loader_unref (transport->loader); _dbus_auth_unref (transport->auth); - if (transport->authorization) - _dbus_authorization_unref (transport->authorization); _dbus_counter_set_notify (transport->live_messages, 0, 0, NULL, NULL); _dbus_counter_unref (transport->live_messages); @@ -543,6 +529,163 @@ _dbus_transport_get_is_connected (DBusTransport *transport) return !transport->disconnected; } +static dbus_bool_t +auth_via_unix_user_function (DBusTransport *transport) +{ + DBusCredentials *auth_identity; + dbus_bool_t allow; + DBusConnection *connection; + DBusAllowUnixUserFunction unix_user_function; + void *unix_user_data; + dbus_uid_t uid; + + /* Dropping the lock here probably isn't that safe. */ + + auth_identity = _dbus_auth_get_identity (transport->auth); + _dbus_assert (auth_identity != NULL); + + connection = transport->connection; + unix_user_function = transport->unix_user_function; + unix_user_data = transport->unix_user_data; + uid = _dbus_credentials_get_unix_uid (auth_identity); + + _dbus_verbose ("unlock\n"); + _dbus_connection_unlock (connection); + + allow = (* unix_user_function) (connection, + uid, + unix_user_data); + + _dbus_verbose ("lock post unix user function\n"); + _dbus_connection_lock (connection); + + if (allow) + { + _dbus_verbose ("Client UID "DBUS_UID_FORMAT" authorized\n", uid); + } + else + { + _dbus_verbose ("Client UID "DBUS_UID_FORMAT + " was rejected, disconnecting\n", + _dbus_credentials_get_unix_uid (auth_identity)); + _dbus_transport_disconnect (transport); + } + + return allow; +} + +static dbus_bool_t +auth_via_windows_user_function (DBusTransport *transport) +{ + DBusCredentials *auth_identity; + dbus_bool_t allow; + DBusConnection *connection; + DBusAllowWindowsUserFunction windows_user_function; + void *windows_user_data; + char *windows_sid; + + /* Dropping the lock here probably isn't that safe. */ + + auth_identity = _dbus_auth_get_identity (transport->auth); + _dbus_assert (auth_identity != NULL); + + connection = transport->connection; + windows_user_function = transport->windows_user_function; + windows_user_data = transport->unix_user_data; + windows_sid = _dbus_strdup (_dbus_credentials_get_windows_sid (auth_identity)); + + if (windows_sid == NULL) + { + /* OOM */ + return FALSE; + } + + _dbus_verbose ("unlock\n"); + _dbus_connection_unlock (connection); + + allow = (* windows_user_function) (connection, + windows_sid, + windows_user_data); + + _dbus_verbose ("lock post windows user function\n"); + _dbus_connection_lock (connection); + + if (allow) + { + _dbus_verbose ("Client SID '%s' authorized\n", windows_sid); + } + else + { + _dbus_verbose ("Client SID '%s' was rejected, disconnecting\n", + _dbus_credentials_get_windows_sid (auth_identity)); + _dbus_transport_disconnect (transport); + } + + return allow; +} + +static dbus_bool_t +auth_via_default_rules (DBusTransport *transport) +{ + DBusCredentials *auth_identity; + DBusCredentials *our_identity; + dbus_bool_t allow; + + auth_identity = _dbus_auth_get_identity (transport->auth); + _dbus_assert (auth_identity != NULL); + + /* By default, connection is allowed if the client is 1) root or 2) + * has the same UID as us or 3) anonymous is allowed. + */ + + our_identity = _dbus_credentials_new_from_current_process (); + if (our_identity == NULL) + { + /* OOM */ + return FALSE; + } + + if (transport->allow_anonymous || + _dbus_credentials_get_unix_uid (auth_identity) == 0 || + _dbus_credentials_same_user (our_identity, + auth_identity)) + { + if (_dbus_credentials_include(our_identity,DBUS_CREDENTIAL_WINDOWS_SID)) + _dbus_verbose ("Client authorized as SID '%s'" + "matching our SID '%s'\n", + _dbus_credentials_get_windows_sid(auth_identity), + _dbus_credentials_get_windows_sid(our_identity)); + else + _dbus_verbose ("Client authorized as UID "DBUS_UID_FORMAT + " matching our UID "DBUS_UID_FORMAT"\n", + _dbus_credentials_get_unix_uid(auth_identity), + _dbus_credentials_get_unix_uid(our_identity)); + /* We have authenticated! */ + allow = TRUE; + } + else + { + if (_dbus_credentials_include(our_identity,DBUS_CREDENTIAL_WINDOWS_SID)) + _dbus_verbose ("Client authorized as SID '%s'" + " but our SID is '%s', disconnecting\n", + (_dbus_credentials_get_windows_sid(auth_identity) ? + _dbus_credentials_get_windows_sid(auth_identity) : ""), + (_dbus_credentials_get_windows_sid(our_identity) ? + _dbus_credentials_get_windows_sid(our_identity) : "")); + else + _dbus_verbose ("Client authorized as UID "DBUS_UID_FORMAT + " but our UID is "DBUS_UID_FORMAT", disconnecting\n", + _dbus_credentials_get_unix_uid(auth_identity), + _dbus_credentials_get_unix_uid(our_identity)); + _dbus_transport_disconnect (transport); + allow = FALSE; + } + + _dbus_credentials_unref (our_identity); + + return allow; +} + /** * Returns #TRUE if we have been authenticated. It will return #TRUE even if * the transport is now disconnected, but was ever authenticated before @@ -632,18 +775,33 @@ _dbus_transport_try_to_authenticate (DBusTransport *transport) */ if (maybe_authenticated && transport->is_server) { + dbus_bool_t allow; DBusCredentials *auth_identity; - + auth_identity = _dbus_auth_get_identity (transport->auth); _dbus_assert (auth_identity != NULL); - /* If we have an authenticated user, delegate deciding whether auth - * credentials are good enough to the app */ - if (!_dbus_authorization_do_authorization (transport->authorization, auth_identity)) + /* If we have an auth'd user and a user function, delegate + * deciding whether auth credentials are good enough to the + * app; otherwise, use our default decision process. + */ + if (transport->unix_user_function != NULL && + _dbus_credentials_include (auth_identity, DBUS_CREDENTIAL_UNIX_USER_ID)) { - _dbus_transport_disconnect (transport); - maybe_authenticated = FALSE; + allow = auth_via_unix_user_function (transport); + } + else if (transport->windows_user_function != NULL && + _dbus_credentials_include (auth_identity, DBUS_CREDENTIAL_WINDOWS_SID)) + { + allow = auth_via_windows_user_function (transport); + } + else + { + allow = auth_via_default_rules (transport); } + + if (!allow) + maybe_authenticated = FALSE; } transport->authenticated = maybe_authenticated; @@ -773,8 +931,6 @@ _dbus_transport_set_connection (DBusTransport *transport, _dbus_assert (transport->connection == NULL); transport->connection = connection; - if (transport->is_server) - _dbus_authorization_set_connection (transport->authorization, connection); _dbus_transport_ref (transport); if (!(* transport->vtable->connection_set) (transport)) @@ -1257,17 +1413,20 @@ _dbus_transport_get_adt_audit_session_data (DBusTransport *transport, * @param old_data the old user data to be freed * @param old_free_data_function old free data function to free it with */ -inline void +void _dbus_transport_set_unix_user_function (DBusTransport *transport, DBusAllowUnixUserFunction function, void *data, DBusFreeFunction free_data_function, void **old_data, DBusFreeFunction *old_free_data_function) -{ - if (transport->is_server) - _dbus_authorization_set_unix_authorization_callback (transport->authorization, function, - data, free_data_function, old_data, old_free_data_function); +{ + *old_data = transport->unix_user_data; + *old_free_data_function = transport->free_unix_user_data; + + transport->unix_user_function = function; + transport->unix_user_data = data; + transport->free_unix_user_data = free_data_function; } /** @@ -1313,7 +1472,7 @@ _dbus_transport_get_windows_user (DBusTransport *transport, * @param old_free_data_function old free data function to free it with */ -inline void +void _dbus_transport_set_windows_user_function (DBusTransport *transport, DBusAllowWindowsUserFunction function, void *data, @@ -1321,9 +1480,12 @@ _dbus_transport_set_windows_user_function (DBusTransport *transport void **old_data, DBusFreeFunction *old_free_data_function) { - if (transport->is_server) - _dbus_authorization_set_windows_authorization_callback (transport->authorization, function, - data, free_data_function, old_data, old_free_data_function); + *old_data = transport->windows_user_data; + *old_free_data_function = transport->free_windows_user_data; + + transport->windows_user_function = function; + transport->windows_user_data = data; + transport->free_windows_user_data = free_data_function; } /** @@ -1347,12 +1509,11 @@ _dbus_transport_set_auth_mechanisms (DBusTransport *transport, * @param transport the transport * @param value #TRUE to allow anonymous connection */ -inline void +void _dbus_transport_set_allow_anonymous (DBusTransport *transport, dbus_bool_t value) { - if (transport->is_server) - _dbus_authorization_set_allow_anonymous (transport->authorization, value); + transport->allow_anonymous = value != FALSE; } #ifdef DBUS_ENABLE_STATS diff --git a/doc/dbus-specification.xml b/doc/dbus-specification.xml index eca494e..65abd29 100644 --- a/doc/dbus-specification.xml +++ b/doc/dbus-specification.xml @@ -5666,14 +5666,6 @@ this concept. On Unix, this is the process ID defined by POSIX. - - WindowsSID - STRING - The Windows security identifier in its string form, - e.g. "S-1-5-21-3623811015-3361044348-30300820-1013" for - a domain or local computer user or "S-1-5-18" for the - LOCAL_SYSTEM user - -- 2.7.4