[RFA] fix gdb/901 - attach to process running as service on Windows

[RFA] fix gdb/901 - attach to process running as service on Windows

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 450 bytes --]

The following patch fixes gdb/901. I am submitting it on behalf of
Pascal Obry, who found the fix. No new regression on Windows XP.

2002-12-26  J. Brobecker  <brobecker@gnat.com>

        From Pascal Obry  <obry@act-europe.fr>
        * win32-nat.c (SetPrivilege): New routine.
        (Child_Attach): call SetPrivilege to correctly set child
        privileges to be able to debug applications running as services.

Ok to commit?

Thanks,
-- 
Joel

[-- Attachment #2: win32-nat.diff --]
[-- Type: text/plain, Size: 2609 bytes --]

Index: win32-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/win32-nat.c,v
retrieving revision 1.66
diff -c -3 -p -r1.66 win32-nat.c
*** win32-nat.c	23 Nov 2002 02:49:45 -0000	1.66
--- win32-nat.c	26 Dec 2002 12:32:02 -0000
*************** has_detach_ability (void)
*** 1374,1388 ****
--- 1374,1459 ----
    return 0;
  }
  
+ /* This routine is copied from the knowledge base of the SDK
+   see article PSS ID Number: Q131065 on http://support.microsoft.com.
+ 
+   This routine is used to gain the right privileges to be able to debug
+   Windows applications running as Services.  */
+ 
+ BOOL SetPrivilege (HANDLE hToken, LPCTSTR Privilege, BOOL bEnablePrivilege)
+ {
+   TOKEN_PRIVILEGES tp;
+   LUID luid;
+   TOKEN_PRIVILEGES tpPrevious;
+   DWORD cbPrevious = sizeof (TOKEN_PRIVILEGES);
+ 
+   if (!LookupPrivilegeValue (NULL, Privilege, &luid)) 
+     return FALSE;
+ 
+   /* First pass.  Get current privilege setting.  */
+   tp.PrivilegeCount           = 1;
+   tp.Privileges[0].Luid       = luid;
+   tp.Privileges[0].Attributes = 0;
+ 
+   AdjustTokenPrivileges (hToken,
+ 			 FALSE,
+ 			 &tp,
+ 			 sizeof (TOKEN_PRIVILEGES),
+ 			 &tpPrevious,
+ 			 &cbPrevious);
+ 
+   if (GetLastError() != ERROR_SUCCESS) 
+     return FALSE;
+   
+   /* Second pass.  Set privilege based on previous setting.  */
+   tpPrevious.PrivilegeCount     = 1;
+   tpPrevious.Privileges[0].Luid = luid;
+ 
+   if (bEnablePrivilege) 
+     {
+       tpPrevious.Privileges[0].Attributes |= (SE_PRIVILEGE_ENABLED);
+     }
+   else 
+     {
+       tpPrevious.Privileges[0].Attributes ^= 
+ 	(SE_PRIVILEGE_ENABLED & tpPrevious.Privileges[0].Attributes);
+     }
+ 
+   AdjustTokenPrivileges (hToken,
+ 			 FALSE,
+ 			 &tpPrevious,
+ 			 cbPrevious,
+ 			 NULL,
+ 			 NULL);
+ 
+   if (GetLastError() != ERROR_SUCCESS) 
+     return FALSE;
+   
+   return TRUE;
+ }
+ 
  /* Attach to process PID, then initialize for debugging it.  */
  static void
  child_attach (char *args, int from_tty)
  {
    BOOL ok;
    DWORD pid;
+   HANDLE hToken;
  
    if (!args)
      error_no_arg ("process-id to attach");
+ 
+   if (OpenProcessToken (GetCurrentProcess(),
+ 			TOKEN_ADJUST_PRIVILEGES | TOKEN_QUERY,
+ 			&hToken))
+     {
+       if (!SetPrivilege (hToken, SE_DEBUG_NAME, TRUE))
+         {
+           printf_unfiltered ("Failed to get SE_DEBUG_NAME priviledge");
+           printf_unfiltered ("This can cause attach to fail on Windows NT");
+         }
+       CloseHandle (hToken);
+     }
  
    pid = strtoul (args, 0, 0);
    ok = DebugActiveProcess (pid);

Re: [RFA] fix gdb/901 - attach to process running as service on Windows

[Christopher Faylor]

On Thu, Dec 26, 2002 at 04:38:17PM +0400, Joel Brobecker wrote:
>The following patch fixes gdb/901. I am submitting it on behalf of
>Pascal Obry, who found the fix. No new regression on Windows XP.
>
>2002-12-26  J. Brobecker  <brobecker@gnat.com>
>
>        From Pascal Obry  <obry@act-europe.fr>
>        * win32-nat.c (SetPrivilege): New routine.
>        (Child_Attach): call SetPrivilege to correctly set child
>        privileges to be able to debug applications running as services.
>
>Ok to commit?

The theory is ok.  I'd already looked at the patch submitted with the
bug report but I question the legality of including source code directly
from Microsoft into gdb.

I asked our NT security expert (Corinna Vinschen) to take a look at
this.  We're looking at donating some code from cygwin to accomplish the
same thing.  Expect a patch when the holiday season is over.

So, to answer the question, "No, it's not ok to commit" but we should have
a fix for the problem relatively soon.

cgf

Re: [RFA] fix gdb/901 - attach to process running as service on Windows

[Corinna Vinschen]

On Thu, Dec 26, 2002 at 01:12:32PM -0500, Chris Faylor wrote:
> On Thu, Dec 26, 2002 at 04:38:17PM +0400, Joel Brobecker wrote:
> >The following patch fixes gdb/901. I am submitting it on behalf of
> >Pascal Obry, who found the fix. No new regression on Windows XP.
> >
> >2002-12-26  J. Brobecker  <brobecker@gnat.com>
> >
> >        From Pascal Obry  <obry@act-europe.fr>
> >        * win32-nat.c (SetPrivilege): New routine.
> >        (Child_Attach): call SetPrivilege to correctly set child
> >        privileges to be able to debug applications running as services.
> >
> >Ok to commit?
> 
> The theory is ok.  I'd already looked at the patch submitted with the
> bug report but I question the legality of including source code directly
> from Microsoft into gdb.
> 
> I asked our NT security expert (Corinna Vinschen) to take a look at
> this.  We're looking at donating some code from cygwin to accomplish the
> same thing.  Expect a patch when the holiday season is over.
> 
> So, to answer the question, "No, it's not ok to commit" but we should have
> a fix for the problem relatively soon.

Ok, done.  I've checked in the below patch. 

The code to set the privilege is taken from Cygwin and tweaked to fit
into gdb.  Especially the used Win32 functions have to be loaded
dynamically to accomodate 9x/Me which don't have security functions.

The Warning message is only printed, if a *privileged* user can't enable
the SE_DEBUG_NAME privilege.  To reiterate what you probably already know:
The SE_DEBUG_NAME privilege is needed *only* when a user wants to debug
processes which (simplified) aren't owned by him/her.  For normal debugging
of own processes this privilege isn't needed.  Since the SE_DEBUG_NAME
privilege is given only to privileged users (only members of the
Administrators group by default), the codeonly warns, if the current user
has the privilege, but can't enable it.  If a user doesn't have the
privilege, attach is expected to fail anyway, the same way as on Posix
systems for non-root users trying to debug a process of another user.

Corinna

2003-01-07  Corinna Vinschen  <vinschen@redhat.com>

        * win32-nat.c (set_process_privilege): New function.
        (child_attach): Call set_process_privilege() to enable the
        SE_DEBUG_NAME user privilege if available in process token.

Index: win32-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/win32-nat.c,v
retrieving revision 1.66
diff -u -p -r1.66 win32-nat.c
--- win32-nat.c	23 Nov 2002 02:49:45 -0000	1.66
+++ win32-nat.c	10 Jan 2003 20:12:38 -0000
@@ -1374,6 +1374,83 @@ has_detach_ability (void)
   return 0;
 }
 
+/* Try to set or remove a user privilege to the current process.  Return -1
+   if that fails, the previous setting of that privilege otherwise.
+
+   This code is copied from the Cygwin source code and rearranged to allow
+   dynamically loading of the needed symbols from advapi32 which is only
+   available on NT/2K/XP. */
+static int
+set_process_privilege (const char *privilege, BOOL enable)
+{
+  static HMODULE advapi32 = NULL;
+  static BOOL WINAPI (*OpenProcessToken)(HANDLE, DWORD, PHANDLE);
+  static BOOL WINAPI (*LookupPrivilegeValue)(LPCSTR, LPCSTR, PLUID);
+  static BOOL WINAPI (*AdjustTokenPrivileges)(HANDLE, BOOL, PTOKEN_PRIVILEGES,
+					      DWORD, PTOKEN_PRIVILEGES, PDWORD);
+
+  HANDLE token_hdl = NULL;
+  LUID restore_priv;
+  TOKEN_PRIVILEGES new_priv, orig_priv;
+  int ret = -1;
+  DWORD size;
+
+  if (GetVersion () >= 0x80000000)  /* No security availbale on 9x/Me */
+    return 0;
+
+  if (!advapi32)
+    {
+      if (!(advapi32 = LoadLibrary ("advapi32.dll")))
+	goto out;
+      if (!OpenProcessToken)
+	OpenProcessToken = GetProcAddress (advapi32, "OpenProcessToken");
+      if (!LookupPrivilegeValue)
+	LookupPrivilegeValue = GetProcAddress (advapi32,
+					       "LookupPrivilegeValueA");
+      if (!AdjustTokenPrivileges)
+	AdjustTokenPrivileges = GetProcAddress (advapi32,
+						"AdjustTokenPrivileges");
+      if (!OpenProcessToken || !LookupPrivilegeValue || !AdjustTokenPrivileges)
+        {
+	  advapi32 = NULL;
+	  goto out;
+	}
+    }
+  
+  if (!OpenProcessToken (GetCurrentProcess (),
+			 TOKEN_QUERY | TOKEN_ADJUST_PRIVILEGES,
+			 &token_hdl))
+    goto out;
+
+  if (!LookupPrivilegeValue (NULL, privilege, &restore_priv))
+    goto out;
+
+  new_priv.PrivilegeCount = 1;
+  new_priv.Privileges[0].Luid = restore_priv;
+  new_priv.Privileges[0].Attributes = enable ? SE_PRIVILEGE_ENABLED : 0;
+
+  if (!AdjustTokenPrivileges (token_hdl, FALSE, &new_priv,
+                              sizeof orig_priv, &orig_priv, &size))
+    goto out;
+#if 0
+  /* Disabled, otherwise every `attach' in an unprivileged user session
+     would raise the "Failed to get SE_DEBUG_NAME privilege" warning in
+     child_attach(). */
+  /* AdjustTokenPrivileges returns TRUE even if the privilege could not
+     be enabled. GetLastError () returns an correct error code, though. */
+  if (enable && GetLastError () == ERROR_NOT_ALL_ASSIGNED)
+    goto out;
+#endif
+
+  ret = orig_priv.Privileges[0].Attributes == SE_PRIVILEGE_ENABLED ? 1 : 0;
+
+out:
+  if (token_hdl)
+    CloseHandle (token_hdl);
+
+  return ret;
+}
+
 /* Attach to process PID, then initialize for debugging it.  */
 static void
 child_attach (char *args, int from_tty)
@@ -1383,6 +1460,12 @@ child_attach (char *args, int from_tty)
 
   if (!args)
     error_no_arg ("process-id to attach");
+
+  if (set_process_privilege (SE_DEBUG_NAME, TRUE) < 0)
+    {
+      printf_unfiltered ("Warning: Failed to get SE_DEBUG_NAME privilege\n");
+      printf_unfiltered ("This can cause attach to fail on Windows NT/2K/XP\n");
+    }
 
   pid = strtoul (args, 0, 0);
   ok = DebugActiveProcess (pid);


-- 
Corinna Vinschen
Cygwin Developer
Red Hat, Inc.
mailto:vinschen@redhat.com