Replace load of functions by direct calls for some WIN32
authorMichael Paquier <michael@paquier.xyz>
Fri, 9 Sep 2022 01:52:17 +0000 (10:52 +0900)
committerMichael Paquier <michael@paquier.xyz>
Fri, 9 Sep 2022 01:52:17 +0000 (10:52 +0900)
This commit changes the following code paths to do direct system calls
to some WIN32 functions rather than loading them from an external
library, shaving some code in the process:
- Creation of restricted tokens in pg_ctl.c, introduced by a25cd81.
- QuerySecurityContextToken() in auth.c for SSPI authentication in the
backend, introduced in d602592.
- CreateRestrictedToken() in src/common/.  This change is similar to the
case of pg_ctl.c.

Most of these functions were loaded rather than directly called because,
as mentioned in the code comments, MinGW headers were not declaring
them.  I have double-checked the recent MinGW code, and all the
functions changed here are declared in its headers, so this change
should be safe.  Note that I do not have a MinGW environment at hand so
I have not tested it directly, but that MSVC was fine with the change.
The buildfarm will tell soon enough if this change is appropriate or not
for a much broader set of environments.

A few code paths still use GetProcAddress() to load some functions:
- LDAP authentication for ldap_start_tls_sA(), where I am not confident
that this change would work.
- win32env.c and win32ntdll.c where we have a per-MSVC version
dependency for the name of the library loaded.
- crashdump.c for MiniDumpWriteDump() and EnumDirTree(), where direct
calls were not able to work after testing.

Reported-by: Thomas Munro
Reviewed-by: Justin Prysby
Discussion: https://postgr.es/m/CA+hUKG+BMdcaCe=P-EjMoLTCr3zrrzqbcVE=8h5LyNsSVHKXZA@mail.gmail.com

src/backend/libpq/auth.c
src/bin/pg_ctl/pg_ctl.c
src/common/restricted_token.c

index b2b0b83a97b0dc118c450d5f15cac16b3ed70f89..b3e51698dccc7c28b49bd5311f69212bb39bcd5e 100644 (file)
@@ -1201,11 +1201,8 @@ pg_SSPI_recvauth(Port *port)
    DWORD       accountnamesize = sizeof(accountname);
    DWORD       domainnamesize = sizeof(domainname);
    SID_NAME_USE accountnameuse;
-   HMODULE     secur32;
    char       *authn_id;
 
-   QUERY_SECURITY_CONTEXT_TOKEN_FN _QuerySecurityContextToken;
-
    /*
     * Acquire a handle to the server credentials.
     */
@@ -1358,36 +1355,12 @@ pg_SSPI_recvauth(Port *port)
     *
     * Get the name of the user that authenticated, and compare it to the pg
     * username that was specified for the connection.
-    *
-    * MingW is missing the export for QuerySecurityContextToken in the
-    * secur32 library, so we have to load it dynamically.
     */
 
-   secur32 = LoadLibrary("SECUR32.DLL");
-   if (secur32 == NULL)
-       ereport(ERROR,
-               (errmsg("could not load library \"%s\": error code %lu",
-                       "SECUR32.DLL", GetLastError())));
-
-   _QuerySecurityContextToken = (QUERY_SECURITY_CONTEXT_TOKEN_FN) (pg_funcptr_t)
-       GetProcAddress(secur32, "QuerySecurityContextToken");
-   if (_QuerySecurityContextToken == NULL)
-   {
-       FreeLibrary(secur32);
-       ereport(ERROR,
-               (errmsg_internal("could not locate QuerySecurityContextToken in secur32.dll: error code %lu",
-                                GetLastError())));
-   }
-
-   r = (_QuerySecurityContextToken) (sspictx, &token);
+   r = QuerySecurityContextToken(sspictx, &token);
    if (r != SEC_E_OK)
-   {
-       FreeLibrary(secur32);
        pg_SSPI_error(ERROR,
                      _("could not get token from SSPI security context"), r);
-   }
-
-   FreeLibrary(secur32);
 
    /*
     * No longer need the security context, everything from here on uses the
index bea2470d866caaffaa90817137e1ba11d7ff726a..bba056f94fe2c577795ceaa146e17a271a9f0e3f 100644 (file)
@@ -1724,17 +1724,6 @@ pgwin32_doRunAsService(void)
 }
 
 
-/*
- * Mingw headers are incomplete, and so are the libraries. So we have to load
- * a whole lot of API functions dynamically.
- */
-typedef BOOL (WINAPI * __CreateRestrictedToken) (HANDLE, DWORD, DWORD, PSID_AND_ATTRIBUTES, DWORD, PLUID_AND_ATTRIBUTES, DWORD, PSID_AND_ATTRIBUTES, PHANDLE);
-typedef BOOL (WINAPI * __IsProcessInJob) (HANDLE, HANDLE, PBOOL);
-typedef HANDLE (WINAPI * __CreateJobObject) (LPSECURITY_ATTRIBUTES, LPCTSTR);
-typedef BOOL (WINAPI * __SetInformationJobObject) (HANDLE, JOBOBJECTINFOCLASS, LPVOID, DWORD);
-typedef BOOL (WINAPI * __AssignProcessToJobObject) (HANDLE, HANDLE);
-typedef BOOL (WINAPI * __QueryInformationJobObject) (HANDLE, JOBOBJECTINFOCLASS, LPVOID, DWORD, LPDWORD);
-
 /*
  * Set up STARTUPINFO for the new process to inherit this process' handles.
  *
@@ -1777,20 +1766,11 @@ CreateRestrictedProcess(char *cmd, PROCESS_INFORMATION *processInfo, bool as_ser
    STARTUPINFO si;
    HANDLE      origToken;
    HANDLE      restrictedToken;
+   BOOL        inJob;
    SID_IDENTIFIER_AUTHORITY NtAuthority = {SECURITY_NT_AUTHORITY};
    SID_AND_ATTRIBUTES dropSids[2];
    PTOKEN_PRIVILEGES delPrivs;
 
-   /* Functions loaded dynamically */
-   __CreateRestrictedToken _CreateRestrictedToken = NULL;
-   __IsProcessInJob _IsProcessInJob = NULL;
-   __CreateJobObject _CreateJobObject = NULL;
-   __SetInformationJobObject _SetInformationJobObject = NULL;
-   __AssignProcessToJobObject _AssignProcessToJobObject = NULL;
-   __QueryInformationJobObject _QueryInformationJobObject = NULL;
-   HANDLE      Kernel32Handle;
-   HANDLE      Advapi32Handle;
-
    ZeroMemory(&si, sizeof(si));
    si.cb = sizeof(si);
 
@@ -1802,20 +1782,6 @@ CreateRestrictedProcess(char *cmd, PROCESS_INFORMATION *processInfo, bool as_ser
     */
    InheritStdHandles(&si);
 
-   Advapi32Handle = LoadLibrary("ADVAPI32.DLL");
-   if (Advapi32Handle != NULL)
-   {
-       _CreateRestrictedToken = (__CreateRestrictedToken) (pg_funcptr_t) GetProcAddress(Advapi32Handle, "CreateRestrictedToken");
-   }
-
-   if (_CreateRestrictedToken == NULL)
-   {
-       /* Log error if we cannot get the function */
-       write_stderr(_("%s: could not locate object function to create restricted token: error code %lu\n"),
-                    progname, (unsigned long) GetLastError());
-       return 0;
-   }
-
    /* Open the current token to use as a base for the restricted one */
    if (!OpenProcessToken(GetCurrentProcess(), TOKEN_ALL_ACCESS, &origToken))
    {
@@ -1848,19 +1814,18 @@ CreateRestrictedProcess(char *cmd, PROCESS_INFORMATION *processInfo, bool as_ser
        /* Error message already printed */
        return 0;
 
-   b = _CreateRestrictedToken(origToken,
-                              0,
-                              sizeof(dropSids) / sizeof(dropSids[0]),
-                              dropSids,
-                              delPrivs->PrivilegeCount, delPrivs->Privileges,
-                              0, NULL,
-                              &restrictedToken);
+   b = CreateRestrictedToken(origToken,
+                             0,
+                             sizeof(dropSids) / sizeof(dropSids[0]),
+                             dropSids,
+                             delPrivs->PrivilegeCount, delPrivs->Privileges,
+                             0, NULL,
+                             &restrictedToken);
 
    free(delPrivs);
    FreeSid(dropSids[1].Sid);
    FreeSid(dropSids[0].Sid);
    CloseHandle(origToken);
-   FreeLibrary(Advapi32Handle);
 
    if (!b)
    {
@@ -1872,79 +1837,55 @@ CreateRestrictedProcess(char *cmd, PROCESS_INFORMATION *processInfo, bool as_ser
    AddUserToTokenDacl(restrictedToken);
    r = CreateProcessAsUser(restrictedToken, NULL, cmd, NULL, NULL, TRUE, CREATE_SUSPENDED, NULL, NULL, &si, processInfo);
 
-   Kernel32Handle = LoadLibrary("KERNEL32.DLL");
-   if (Kernel32Handle != NULL)
-   {
-       _IsProcessInJob = (__IsProcessInJob) (pg_funcptr_t) GetProcAddress(Kernel32Handle, "IsProcessInJob");
-       _CreateJobObject = (__CreateJobObject) (pg_funcptr_t) GetProcAddress(Kernel32Handle, "CreateJobObjectA");
-       _SetInformationJobObject = (__SetInformationJobObject) (pg_funcptr_t) GetProcAddress(Kernel32Handle, "SetInformationJobObject");
-       _AssignProcessToJobObject = (__AssignProcessToJobObject) (pg_funcptr_t) GetProcAddress(Kernel32Handle, "AssignProcessToJobObject");
-       _QueryInformationJobObject = (__QueryInformationJobObject) (pg_funcptr_t) GetProcAddress(Kernel32Handle, "QueryInformationJobObject");
-   }
-
-   /* Verify that we found all functions */
-   if (_IsProcessInJob == NULL || _CreateJobObject == NULL || _SetInformationJobObject == NULL || _AssignProcessToJobObject == NULL || _QueryInformationJobObject == NULL)
+   if (IsProcessInJob(processInfo->hProcess, NULL, &inJob))
    {
-       /* Log error if we can't get version */
-       write_stderr(_("%s: WARNING: could not locate all job object functions in system API\n"), progname);
-   }
-   else
-   {
-       BOOL        inJob;
-
-       if (_IsProcessInJob(processInfo->hProcess, NULL, &inJob))
+       if (!inJob)
        {
-           if (!inJob)
-           {
-               /*
-                * Job objects are working, and the new process isn't in one,
-                * so we can create one safely. If any problems show up when
-                * setting it, we're going to ignore them.
-                */
-               HANDLE      job;
-               char        jobname[128];
+           /*
+            * Job objects are working, and the new process isn't in one, so
+            * we can create one safely. If any problems show up when setting
+            * it, we're going to ignore them.
+            */
+           HANDLE      job;
+           char        jobname[128];
 
-               sprintf(jobname, "PostgreSQL_%lu",
-                       (unsigned long) processInfo->dwProcessId);
+           sprintf(jobname, "PostgreSQL_%lu",
+                   (unsigned long) processInfo->dwProcessId);
 
-               job = _CreateJobObject(NULL, jobname);
-               if (job)
-               {
-                   JOBOBJECT_BASIC_LIMIT_INFORMATION basicLimit;
-                   JOBOBJECT_BASIC_UI_RESTRICTIONS uiRestrictions;
-                   JOBOBJECT_SECURITY_LIMIT_INFORMATION securityLimit;
+           job = CreateJobObject(NULL, jobname);
+           if (job)
+           {
+               JOBOBJECT_BASIC_LIMIT_INFORMATION basicLimit;
+               JOBOBJECT_BASIC_UI_RESTRICTIONS uiRestrictions;
+               JOBOBJECT_SECURITY_LIMIT_INFORMATION securityLimit;
 
-                   ZeroMemory(&basicLimit, sizeof(basicLimit));
-                   ZeroMemory(&uiRestrictions, sizeof(uiRestrictions));
-                   ZeroMemory(&securityLimit, sizeof(securityLimit));
+               ZeroMemory(&basicLimit, sizeof(basicLimit));
+               ZeroMemory(&uiRestrictions, sizeof(uiRestrictions));
+               ZeroMemory(&securityLimit, sizeof(securityLimit));
 
-                   basicLimit.LimitFlags = JOB_OBJECT_LIMIT_DIE_ON_UNHANDLED_EXCEPTION | JOB_OBJECT_LIMIT_PRIORITY_CLASS;
-                   basicLimit.PriorityClass = NORMAL_PRIORITY_CLASS;
-                   _SetInformationJobObject(job, JobObjectBasicLimitInformation, &basicLimit, sizeof(basicLimit));
+               basicLimit.LimitFlags = JOB_OBJECT_LIMIT_DIE_ON_UNHANDLED_EXCEPTION | JOB_OBJECT_LIMIT_PRIORITY_CLASS;
+               basicLimit.PriorityClass = NORMAL_PRIORITY_CLASS;
+               SetInformationJobObject(job, JobObjectBasicLimitInformation, &basicLimit, sizeof(basicLimit));
 
-                   uiRestrictions.UIRestrictionsClass = JOB_OBJECT_UILIMIT_DESKTOP | JOB_OBJECT_UILIMIT_DISPLAYSETTINGS |
-                       JOB_OBJECT_UILIMIT_EXITWINDOWS | JOB_OBJECT_UILIMIT_READCLIPBOARD |
-                       JOB_OBJECT_UILIMIT_SYSTEMPARAMETERS | JOB_OBJECT_UILIMIT_WRITECLIPBOARD;
+               uiRestrictions.UIRestrictionsClass = JOB_OBJECT_UILIMIT_DESKTOP | JOB_OBJECT_UILIMIT_DISPLAYSETTINGS |
+                   JOB_OBJECT_UILIMIT_EXITWINDOWS | JOB_OBJECT_UILIMIT_READCLIPBOARD |
+                   JOB_OBJECT_UILIMIT_SYSTEMPARAMETERS | JOB_OBJECT_UILIMIT_WRITECLIPBOARD;
 
-                   _SetInformationJobObject(job, JobObjectBasicUIRestrictions, &uiRestrictions, sizeof(uiRestrictions));
+               SetInformationJobObject(job, JobObjectBasicUIRestrictions, &uiRestrictions, sizeof(uiRestrictions));
 
-                   securityLimit.SecurityLimitFlags = JOB_OBJECT_SECURITY_NO_ADMIN | JOB_OBJECT_SECURITY_ONLY_TOKEN;
-                   securityLimit.JobToken = restrictedToken;
-                   _SetInformationJobObject(job, JobObjectSecurityLimitInformation, &securityLimit, sizeof(securityLimit));
+               securityLimit.SecurityLimitFlags = JOB_OBJECT_SECURITY_NO_ADMIN | JOB_OBJECT_SECURITY_ONLY_TOKEN;
+               securityLimit.JobToken = restrictedToken;
+               SetInformationJobObject(job, JobObjectSecurityLimitInformation, &securityLimit, sizeof(securityLimit));
 
-                   _AssignProcessToJobObject(job, processInfo->hProcess);
-               }
+               AssignProcessToJobObject(job, processInfo->hProcess);
            }
        }
    }
 
-
    CloseHandle(restrictedToken);
 
    ResumeThread(processInfo->hThread);
 
-   FreeLibrary(Kernel32Handle);
-
    /*
     * We intentionally don't close the job object handle, because we want the
     * object to live on until pg_ctl shuts down.
index 82b74b565ebed7db929db18b73ec3754dae67d49..8f4b76b32958f9e1031080b4f129e9f944c37edf 100644 (file)
@@ -28,8 +28,6 @@
 /* internal vars */
 char      *restrict_env;
 
-typedef BOOL (WINAPI * __CreateRestrictedToken) (HANDLE, DWORD, DWORD, PSID_AND_ATTRIBUTES, DWORD, PLUID_AND_ATTRIBUTES, DWORD, PSID_AND_ATTRIBUTES, PHANDLE);
-
 /* Windows API define missing from some versions of MingW headers */
 #ifndef  DISABLE_MAX_PRIVILEGE
 #define DISABLE_MAX_PRIVILEGE  0x1
@@ -52,36 +50,15 @@ CreateRestrictedProcess(char *cmd, PROCESS_INFORMATION *processInfo)
    HANDLE      restrictedToken;
    SID_IDENTIFIER_AUTHORITY NtAuthority = {SECURITY_NT_AUTHORITY};
    SID_AND_ATTRIBUTES dropSids[2];
-   __CreateRestrictedToken _CreateRestrictedToken;
-   HANDLE      Advapi32Handle;
 
    ZeroMemory(&si, sizeof(si));
    si.cb = sizeof(si);
 
-   Advapi32Handle = LoadLibrary("ADVAPI32.DLL");
-   if (Advapi32Handle == NULL)
-   {
-       pg_log_error("could not load library \"%s\": error code %lu",
-                    "ADVAPI32.DLL", GetLastError());
-       return 0;
-   }
-
-   _CreateRestrictedToken = (__CreateRestrictedToken) (pg_funcptr_t) GetProcAddress(Advapi32Handle, "CreateRestrictedToken");
-
-   if (_CreateRestrictedToken == NULL)
-   {
-       pg_log_error("cannot create restricted tokens on this platform: error code %lu",
-                    GetLastError());
-       FreeLibrary(Advapi32Handle);
-       return 0;
-   }
-
    /* Open the current token to use as a base for the restricted one */
    if (!OpenProcessToken(GetCurrentProcess(), TOKEN_ALL_ACCESS, &origToken))
    {
        pg_log_error("could not open process token: error code %lu",
                     GetLastError());
-       FreeLibrary(Advapi32Handle);
        return 0;
    }
 
@@ -97,22 +74,20 @@ CreateRestrictedProcess(char *cmd, PROCESS_INFORMATION *processInfo)
        pg_log_error("could not allocate SIDs: error code %lu",
                     GetLastError());
        CloseHandle(origToken);
-       FreeLibrary(Advapi32Handle);
        return 0;
    }
 
-   b = _CreateRestrictedToken(origToken,
-                              DISABLE_MAX_PRIVILEGE,
-                              sizeof(dropSids) / sizeof(dropSids[0]),
-                              dropSids,
-                              0, NULL,
-                              0, NULL,
-                              &restrictedToken);
+   b = CreateRestrictedToken(origToken,
+                             DISABLE_MAX_PRIVILEGE,
+                             sizeof(dropSids) / sizeof(dropSids[0]),
+                             dropSids,
+                             0, NULL,
+                             0, NULL,
+                             &restrictedToken);
 
    FreeSid(dropSids[1].Sid);
    FreeSid(dropSids[0].Sid);
    CloseHandle(origToken);
-   FreeLibrary(Advapi32Handle);
 
    if (!b)
    {