Replace load of functions by direct calls for some WIN32
authorMichael Paquier <[email protected]>
Fri, 9 Sep 2022 01:52:17 +0000 (10:52 +0900)
committerMichael Paquier <[email protected]>
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)
        {