Attachment 139075 Details for Bug 83499 – On windows lock client process to avoid pid reusage

[patch] On windows lock client process to avoid pid reusage

On-windows-lock-client-process-to-avoid-pid-reusag.patch (text/plain), 5.66 KB, created by Ralf Habacker on 2018-04-24 21:03:07 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Ralf Habacker

Created: 2018-04-24 21:03:07 UTC

Size: 5.66 KB

patch

obsolete

>From c003cfdaa8e5cd44467f902ab28795850055f923 Mon Sep 17 00:00:00 2001
>From: Ralf Habacker <ralf.habacker@freenet.de>
>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

Actions: View | Splinter Review

Attachments on bug 83499: 106126 | 106127 | 139047 | 139049 | 139075

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.