From c003cfdaa8e5cd44467f902ab28795850055f923 Mon Sep 17 00:00:00 2001 From: Ralf Habacker Date: Tue, 24 Apr 2018 10:37:15 +0200 Subject: [PATCH] On windows lock client process to avoid pid reusage This is performed by calling GetExtendedTcpTable() and scan it for a client coming from 127.0.0.1 with a given server port. Then we call OpenProcess() to get a HANDLE for the related process. Now the related process is pinned in memory, and the process ID cannot be reused, because we have a handle on it. Then we call GetExtendedTcpTable() again, and scan it for a client coming from 127.0.0.1 with the same port. If the new TCP table *still* says 127.0.0.1 with the requested port is the same process then *maybe* we can assume that our HANDLE points to the process that is responsible for that client connection. It has been said 'maybe', because it is required to prove that pattern is race-free: does Windows guarantee that when process exits, all connections that it initiated are removed from the TCP table before the process ID can be reused? https://en.wikipedia.org/wiki/Berkeley_sockets#Terminating_sockets mentions that on closing sockets they may be in TIME_WAIT state for a specific time, but fortunally entries fetched from GetExtendedTcpTable() are already limited to an 'established' state (see https://cgit.freedesktop.org/dbus/dbus/tree/dbus/dbus-sysdeps-win.c#n174) Bug: https://bugs.freedesktop.org/show_bug.cgi?id=83499 --- dbus/dbus-credentials.c | 31 +++++++++++++++++++++++++++++++ dbus/dbus-credentials.h | 5 +++++ dbus/dbus-sysdeps-win.c | 27 +++++++++++++++++++++++++++ 3 files changed, 63 insertions(+) diff --git a/dbus/dbus-credentials.c b/dbus/dbus-credentials.c index 4a7bafd1..e0cd251b 100644 --- a/dbus/dbus-credentials.c +++ b/dbus/dbus-credentials.c @@ -56,6 +56,10 @@ struct DBusCredentials { char *linux_security_label; void *adt_audit_data; dbus_int32_t adt_audit_data_size; +#ifdef DBUS_WIN + /** handle to lock client pid */ + HANDLE process_handle; +#endif }; /** @} */ @@ -88,6 +92,9 @@ _dbus_credentials_new (void) creds->linux_security_label = NULL; creds->adt_audit_data = NULL; creds->adt_audit_data_size = 0; +#ifdef DBUS_WIN + creds->process_handle = 0; +#endif return creds; } @@ -143,6 +150,10 @@ _dbus_credentials_unref (DBusCredentials *credentials) dbus_free (credentials->windows_sid); dbus_free (credentials->linux_security_label); dbus_free (credentials->adt_audit_data); +#ifdef DBUS_WIN + if (credentials->process_handle != 0) + CloseHandle (credentials->process_handle); +#endif dbus_free (credentials); } } @@ -163,6 +174,21 @@ _dbus_credentials_add_pid (DBusCredentials *credentials, } /** + * Add a Windows process handle to the credentials. + * + * @param credentials the object + * @param process_handle the process handle + * @returns #FALSE if no memory + */ +dbus_bool_t +_dbus_credentials_add_process_handle (DBusCredentials *credentials, + HANDLE process_handle) +{ + credentials->process_handle = process_handle; + return TRUE; +} + +/** * Add a UNIX user ID to the credentials. * * @param credentials the object @@ -604,6 +630,11 @@ _dbus_credentials_clear (DBusCredentials *credentials) dbus_free (credentials->adt_audit_data); credentials->adt_audit_data = NULL; credentials->adt_audit_data_size = 0; +#ifdef DBUS_WIN + if (credentials->process_handle != 0) + CloseHandle (credentials->process_handle); + credentials->process_handle = 0; +#endif } /** diff --git a/dbus/dbus-credentials.h b/dbus/dbus-credentials.h index 3407b725..d7e07fea 100644 --- a/dbus/dbus-credentials.h +++ b/dbus/dbus-credentials.h @@ -50,6 +50,11 @@ void _dbus_credentials_unref (DBusCredentials DBUS_PRIVATE_EXPORT dbus_bool_t _dbus_credentials_add_pid (DBusCredentials *credentials, dbus_pid_t pid); +#ifdef DBUS_WIN +DBUS_PRIVATE_EXPORT +dbus_bool_t _dbus_credentials_add_process_handle (DBusCredentials *credentials, + HANDLE process_handle); +#endif DBUS_PRIVATE_EXPORT dbus_bool_t _dbus_credentials_add_unix_uid (DBusCredentials *credentials, dbus_uid_t uid); diff --git a/dbus/dbus-sysdeps-win.c b/dbus/dbus-sysdeps-win.c index bc41ca83..754b5b9a 100644 --- a/dbus/dbus-sysdeps-win.c +++ b/dbus/dbus-sysdeps-win.c @@ -2093,6 +2093,33 @@ _dbus_read_credentials_socket (DBusSocket handle, if (pid == 0) return TRUE; +#ifdef DBUS_WIN + { + dbus_pid_t _pid = 0; + HANDLE process_handle = OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, FALSE, pid); + if (process_handle == NULL) + { + dbus_set_error (error, DBUS_ERROR_IO_ERROR, + "could not get peer's process handle"); + goto out; + } + _pid = _dbus_get_peer_pid_from_tcp_handle (handle.sock); + if (pid == 0) + { + _dbus_verbose ("lost peer's process id\n"); + CloseHandle (process_handle); + return TRUE; + } + if (_pid != pid) + { + dbus_set_error (error, DBUS_ERROR_IO_ERROR, + "peer's process id has been changed from '%ld' to '%ld'", pid, _pid); + CloseHandle (process_handle); + goto out; + } + _dbus_credentials_add_process_handle (credentials, process_handle); + } +#endif _dbus_credentials_add_pid (credentials, pid); /* FIXME: time of check / time of use error: can we guarantee -- 2.13.6