Feedback-ID: i0fe9450f:Fastmail
Date: Thu, 8 Sep 2022 17:55:40 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Thomas Munro <thomas.munro@gmail.com>
Cc: Justin Pryzby <pryzby@telsasoft.com>,
	Andrew Dunstan <andrew@dunslane.net>,
	Daniel Gustafsson <daniel@yesql.se>,
	Juan =?iso-8859-1?Q?Jos=E9_Santamar=EDa?= Flecha
 <juanjo.santamaria@gmail.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Bump MIN_WINNT to 0x0600 (Vista) as minimal runtime in 16~
Message-ID: <YxmuDClnF90gAxxW@paquier.xyz>
References: <YqrKOPvi/JtS5gzR@paquier.xyz>
 <YsU5fb0ceDADr2Ti@paquier.xyz>
 <CA+hUKGL1p7BHdZc_GEBYkLarPzFEAYKd_HXA_NJ_ct4dJCD-iw@mail.gmail.com>
 <503cc622-ea4d-0a78-ff0c-2c757b97149e@dunslane.net>
 <YsYkzVnFpV79ZTXU@paquier.xyz>
 <YsZnhwERsDv/LtC7@paquier.xyz>
 <20220826112637.GD2342@telsasoft.com>
 <YwmtHRRoA5gQi7lZ@paquier.xyz>
 <Yw1gZjJjS3BNTlim@paquier.xyz>
 <CA+hUKG+BMdcaCe=P-EjMoLTCr3zrrzqbcVE=8h5LyNsSVHKXZA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="YsNoccMKm7pRZKoi"
Content-Disposition: inline
In-Reply-To: 
 <CA+hUKG+BMdcaCe=P-EjMoLTCr3zrrzqbcVE=8h5LyNsSVHKXZA@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/YxmuDClnF90gAxxW%40paquier.xyz>
Precedence: bulk


--YsNoccMKm7pRZKoi
Content-Type: multipart/mixed; boundary="DR0kpOiIfHMVriwO"
Content-Disposition: inline


--DR0kpOiIfHMVriwO
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Tue, Aug 30, 2022 at 01:29:24PM +1200, Thomas Munro wrote:
> This reminds me of 24c3ce8f1, which replaced a dlopen()-ish thing with
> a direct function call.  Can you just call all these functions
> directly these days?

Hmm.  Some tests in the CI show that attempting to call directly
MiniDumpWriteDump() causes a linking failure at compilation.  Anyway,
in the same fashion, I can get some simplifications done right for
pg_ctl.c, auth.c and restricted_token.c.  And I am seeing all these
functions listed in the headers of MinGW, meaning that all these
should work out of the box in this case, no?

The others are library-dependent, and I not really confident about
ldap_start_tls_sA().  So, at the end, I am finishing with the
attached, what do you think?  This cuts some code, which is nice:
 3 files changed, 48 insertions(+), 159 deletions(-)
--
Michael

--DR0kpOiIfHMVriwO
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="win32_direct_funccalls.patch"
Content-Transfer-Encoding: quoted-printable

diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index b2b0b83a97..b3e51698dc 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -1201,11 +1201,8 @@ pg_SSPI_recvauth(Port *port)
 	DWORD		accountnamesize =3D sizeof(accountname);
 	DWORD		domainnamesize =3D sizeof(domainname);
 	SID_NAME_USE accountnameuse;
-	HMODULE		secur32;
 	char	   *authn_id;
=20
-	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.
 	 */
=20
-	secur32 =3D LoadLibrary("SECUR32.DLL");
-	if (secur32 =3D=3D NULL)
-		ereport(ERROR,
-				(errmsg("could not load library \"%s\": error code %lu",
-						"SECUR32.DLL", GetLastError())));
-
-	_QuerySecurityContextToken =3D (QUERY_SECURITY_CONTEXT_TOKEN_FN) (pg_func=
ptr_t)
-		GetProcAddress(secur32, "QuerySecurityContextToken");
-	if (_QuerySecurityContextToken =3D=3D NULL)
-	{
-		FreeLibrary(secur32);
-		ereport(ERROR,
-				(errmsg_internal("could not locate QuerySecurityContextToken in secur3=
2.dll: error code %lu",
-								 GetLastError())));
-	}
-
-	r =3D (_QuerySecurityContextToken) (sspictx, &token);
+	r =3D QuerySecurityContextToken(sspictx, &token);
 	if (r !=3D SEC_E_OK)
-	{
-		FreeLibrary(secur32);
 		pg_SSPI_error(ERROR,
 					  _("could not get token from SSPI security context"), r);
-	}
-
-	FreeLibrary(secur32);
=20
 	/*
 	 * No longer need the security context, everything from here on uses the
diff --git a/src/common/restricted_token.c b/src/common/restricted_token.c
index 82b74b565e..8f4b76b329 100644
--- a/src/common/restricted_token.c
+++ b/src/common/restricted_token.c
@@ -28,8 +28,6 @@
 /* internal vars */
 char	   *restrict_env;
=20
-typedef BOOL (WINAPI * __CreateRestrictedToken) (HANDLE, DWORD, DWORD, PSI=
D_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 =3D {SECURITY_NT_AUTHORITY};
 	SID_AND_ATTRIBUTES dropSids[2];
-	__CreateRestrictedToken _CreateRestrictedToken;
-	HANDLE		Advapi32Handle;
=20
 	ZeroMemory(&si, sizeof(si));
 	si.cb =3D sizeof(si);
=20
-	Advapi32Handle =3D LoadLibrary("ADVAPI32.DLL");
-	if (Advapi32Handle =3D=3D NULL)
-	{
-		pg_log_error("could not load library \"%s\": error code %lu",
-					 "ADVAPI32.DLL", GetLastError());
-		return 0;
-	}
-
-	_CreateRestrictedToken =3D (__CreateRestrictedToken) (pg_funcptr_t) GetPr=
ocAddress(Advapi32Handle, "CreateRestrictedToken");
-
-	if (_CreateRestrictedToken =3D=3D NULL)
-	{
-		pg_log_error("cannot create restricted tokens on this platform: error co=
de %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;
 	}
=20
@@ -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;
 	}
=20
-	b =3D _CreateRestrictedToken(origToken,
-							   DISABLE_MAX_PRIVILEGE,
-							   sizeof(dropSids) / sizeof(dropSids[0]),
-							   dropSids,
-							   0, NULL,
-							   0, NULL,
-							   &restrictedToken);
+	b =3D CreateRestrictedToken(origToken,
+							  DISABLE_MAX_PRIVILEGE,
+							  sizeof(dropSids) / sizeof(dropSids[0]),
+							  dropSids,
+							  0, NULL,
+							  0, NULL,
+							  &restrictedToken);
=20
 	FreeSid(dropSids[1].Sid);
 	FreeSid(dropSids[0].Sid);
 	CloseHandle(origToken);
-	FreeLibrary(Advapi32Handle);
=20
 	if (!b)
 	{
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index bea2470d86..4fb24c8a39 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -1724,17 +1724,6 @@ pgwin32_doRunAsService(void)
 }
=20
=20
-/*
- * Mingw headers are incomplete, and so are the libraries. So we have to l=
oad
- * a whole lot of API functions dynamically.
- */
-typedef BOOL (WINAPI * __CreateRestrictedToken) (HANDLE, DWORD, DWORD, PSI=
D_AND_ATTRIBUTES, DWORD, PLUID_AND_ATTRIBUTES, DWORD, PSID_AND_ATTRIBUTES, =
PHANDLE);
-typedef BOOL (WINAPI * __IsProcessInJob) (HANDLE, HANDLE, PBOOL);
-typedef HANDLE (WINAPI * __CreateJobObject) (LPSECURITY_ATTRIBUTES, LPCTST=
R);
-typedef BOOL (WINAPI * __SetInformationJobObject) (HANDLE, JOBOBJECTINFOCL=
ASS, LPVOID, DWORD);
-typedef BOOL (WINAPI * __AssignProcessToJobObject) (HANDLE, HANDLE);
-typedef BOOL (WINAPI * __QueryInformationJobObject) (HANDLE, JOBOBJECTINFO=
CLASS, LPVOID, DWORD, LPDWORD);
-
 /*
  * Set up STARTUPINFO for the new process to inherit this process' handles.
  *
@@ -1777,20 +1766,11 @@ CreateRestrictedProcess(char *cmd, PROCESS_INFORMAT=
ION *processInfo, bool as_ser
 	STARTUPINFO si;
 	HANDLE		origToken;
 	HANDLE		restrictedToken;
+	BOOL		inJob;
 	SID_IDENTIFIER_AUTHORITY NtAuthority =3D {SECURITY_NT_AUTHORITY};
 	SID_AND_ATTRIBUTES dropSids[2];
 	PTOKEN_PRIVILEGES delPrivs;
=20
-	/* Functions loaded dynamically */
-	__CreateRestrictedToken _CreateRestrictedToken =3D NULL;
-	__IsProcessInJob _IsProcessInJob =3D NULL;
-	__CreateJobObject _CreateJobObject =3D NULL;
-	__SetInformationJobObject _SetInformationJobObject =3D NULL;
-	__AssignProcessToJobObject _AssignProcessToJobObject =3D NULL;
-	__QueryInformationJobObject _QueryInformationJobObject =3D NULL;
-	HANDLE		Kernel32Handle;
-	HANDLE		Advapi32Handle;
-
 	ZeroMemory(&si, sizeof(si));
 	si.cb =3D sizeof(si);
=20
@@ -1802,20 +1782,6 @@ CreateRestrictedProcess(char *cmd, PROCESS_INFORMATI=
ON *processInfo, bool as_ser
 	 */
 	InheritStdHandles(&si);
=20
-	Advapi32Handle =3D LoadLibrary("ADVAPI32.DLL");
-	if (Advapi32Handle !=3D NULL)
-	{
-		_CreateRestrictedToken =3D (__CreateRestrictedToken) (pg_funcptr_t) GetP=
rocAddress(Advapi32Handle, "CreateRestrictedToken");
-	}
-
-	if (_CreateRestrictedToken =3D=3D NULL)
-	{
-		/* Log error if we cannot get the function */
-		write_stderr(_("%s: could not locate object function to create restricte=
d 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_INFORMAT=
ION *processInfo, bool as_ser
 		/* Error message already printed */
 		return 0;
=20
-	b =3D _CreateRestrictedToken(origToken,
-							   0,
-							   sizeof(dropSids) / sizeof(dropSids[0]),
-							   dropSids,
-							   delPrivs->PrivilegeCount, delPrivs->Privileges,
-							   0, NULL,
-							   &restrictedToken);
+	b =3D CreateRestrictedToken(origToken,
+							  0,
+							  sizeof(dropSids) / sizeof(dropSids[0]),
+							  dropSids,
+							  delPrivs->PrivilegeCount, delPrivs->Privileges,
+							  0, NULL,
+							  &restrictedToken);
=20
 	free(delPrivs);
 	FreeSid(dropSids[1].Sid);
 	FreeSid(dropSids[0].Sid);
 	CloseHandle(origToken);
-	FreeLibrary(Advapi32Handle);
=20
 	if (!b)
 	{
@@ -1872,79 +1837,55 @@ CreateRestrictedProcess(char *cmd, PROCESS_INFORMAT=
ION *processInfo, bool as_ser
 	AddUserToTokenDacl(restrictedToken);
 	r =3D CreateProcessAsUser(restrictedToken, NULL, cmd, NULL, NULL, TRUE, C=
REATE_SUSPENDED, NULL, NULL, &si, processInfo);
=20
-	Kernel32Handle =3D LoadLibrary("KERNEL32.DLL");
-	if (Kernel32Handle !=3D NULL)
+	if (IsProcessInJob(processInfo->hProcess, NULL, &inJob))
 	{
-		_IsProcessInJob =3D (__IsProcessInJob) (pg_funcptr_t) GetProcAddress(Ker=
nel32Handle, "IsProcessInJob");
-		_CreateJobObject =3D (__CreateJobObject) (pg_funcptr_t) GetProcAddress(K=
ernel32Handle, "CreateJobObjectA");
-		_SetInformationJobObject =3D (__SetInformationJobObject) (pg_funcptr_t) =
GetProcAddress(Kernel32Handle, "SetInformationJobObject");
-		_AssignProcessToJobObject =3D (__AssignProcessToJobObject) (pg_funcptr_t=
) GetProcAddress(Kernel32Handle, "AssignProcessToJobObject");
-		_QueryInformationJobObject =3D (__QueryInformationJobObject) (pg_funcptr=
_t) GetProcAddress(Kernel32Handle, "QueryInformationJobObject");
-	}
-
-	/* Verify that we found all functions */
-	if (_IsProcessInJob =3D=3D NULL || _CreateJobObject =3D=3D NULL || _SetIn=
formationJobObject =3D=3D NULL || _AssignProcessToJobObject =3D=3D NULL || =
_QueryInformationJobObject =3D=3D NULL)
-	{
-		/* Log error if we can't get version */
-		write_stderr(_("%s: WARNING: could not locate all job object functions i=
n 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];
+
+			sprintf(jobname, "PostgreSQL_%lu",
+					(unsigned long) processInfo->dwProcessId);
+
+			job =3D CreateJobObject(NULL, jobname);
+			if (job)
 			{
-				/*
-				 * 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];
+				JOBOBJECT_BASIC_LIMIT_INFORMATION basicLimit;
+				JOBOBJECT_BASIC_UI_RESTRICTIONS uiRestrictions;
+				JOBOBJECT_SECURITY_LIMIT_INFORMATION securityLimit;
=20
-				sprintf(jobname, "PostgreSQL_%lu",
-						(unsigned long) processInfo->dwProcessId);
+				ZeroMemory(&basicLimit, sizeof(basicLimit));
+				ZeroMemory(&uiRestrictions, sizeof(uiRestrictions));
+				ZeroMemory(&securityLimit, sizeof(securityLimit));
=20
-				job =3D _CreateJobObject(NULL, jobname);
-				if (job)
-				{
-					JOBOBJECT_BASIC_LIMIT_INFORMATION basicLimit;
-					JOBOBJECT_BASIC_UI_RESTRICTIONS uiRestrictions;
-					JOBOBJECT_SECURITY_LIMIT_INFORMATION securityLimit;
+				basicLimit.LimitFlags =3D JOB_OBJECT_LIMIT_DIE_ON_UNHANDLED_EXCEPTION =
| JOB_OBJECT_LIMIT_PRIORITY_CLASS;
+				basicLimit.PriorityClass =3D NORMAL_PRIORITY_CLASS;
+				SetInformationJobObject(job, JobObjectBasicLimitInformation, &basicLim=
it, sizeof(basicLimit));
=20
-					ZeroMemory(&basicLimit, sizeof(basicLimit));
-					ZeroMemory(&uiRestrictions, sizeof(uiRestrictions));
-					ZeroMemory(&securityLimit, sizeof(securityLimit));
+				uiRestrictions.UIRestrictionsClass =3D JOB_OBJECT_UILIMIT_DESKTOP | JO=
B_OBJECT_UILIMIT_DISPLAYSETTINGS |
+					JOB_OBJECT_UILIMIT_EXITWINDOWS | JOB_OBJECT_UILIMIT_READCLIPBOARD |
+					JOB_OBJECT_UILIMIT_SYSTEMPARAMETERS | JOB_OBJECT_UILIMIT_WRITECLIPBOA=
RD;
=20
-					basicLimit.LimitFlags =3D JOB_OBJECT_LIMIT_DIE_ON_UNHANDLED_EXCEPTION=
 | JOB_OBJECT_LIMIT_PRIORITY_CLASS;
-					basicLimit.PriorityClass =3D NORMAL_PRIORITY_CLASS;
-					_SetInformationJobObject(job, JobObjectBasicLimitInformation, &basicL=
imit, sizeof(basicLimit));
+				SetInformationJobObject(job, JobObjectBasicUIRestrictions, &uiRestrict=
ions, sizeof(uiRestrictions));
=20
-					uiRestrictions.UIRestrictionsClass =3D JOB_OBJECT_UILIMIT_DESKTOP | J=
OB_OBJECT_UILIMIT_DISPLAYSETTINGS |
-						JOB_OBJECT_UILIMIT_EXITWINDOWS | JOB_OBJECT_UILIMIT_READCLIPBOARD |
-						JOB_OBJECT_UILIMIT_SYSTEMPARAMETERS | JOB_OBJECT_UILIMIT_WRITECLIPBO=
ARD;
+				securityLimit.SecurityLimitFlags =3D JOB_OBJECT_SECURITY_NO_ADMIN | JO=
B_OBJECT_SECURITY_ONLY_TOKEN;
+				securityLimit.JobToken =3D restrictedToken;
+				SetInformationJobObject(job, JobObjectSecurityLimitInformation, &secur=
ityLimit, sizeof(securityLimit));
=20
-					_SetInformationJobObject(job, JobObjectBasicUIRestrictions, &uiRestri=
ctions, sizeof(uiRestrictions));
-
-					securityLimit.SecurityLimitFlags =3D JOB_OBJECT_SECURITY_NO_ADMIN | J=
OB_OBJECT_SECURITY_ONLY_TOKEN;
-					securityLimit.JobToken =3D restrictedToken;
-					_SetInformationJobObject(job, JobObjectSecurityLimitInformation, &sec=
urityLimit, sizeof(securityLimit));
-
-					_AssignProcessToJobObject(job, processInfo->hProcess);
-				}
+				AssignProcessToJobObject(job, processInfo->hProcess);
 			}
 		}
 	}
=20
-
 	CloseHandle(restrictedToken);
=20
 	ResumeThread(processInfo->hThread);
=20
-	FreeLibrary(Kernel32Handle);
-
 	/*
 	 * We intentionally don't close the job object handle, because we want the
 	 * object to live on until pg_ctl shuts down.

--DR0kpOiIfHMVriwO--

--YsNoccMKm7pRZKoi
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmMZrgwACgkQnvQgOdby
QH0fqRAAhNAbcERuqIEFXCzugf/yFpHVVIleOGL2oiOb1y+xY7BfNcDHASVDU5ap
qbVgkTWpDLdCrbYpTTnIk08fktkhVNue9MQLRvjLGLRDGegsjhztz31exiNVvBIb
t9DjsciO7tfVjxEXAdOhL7lUQuYSgR/d9z+wQTJio1bWTNv9xCVzYjNMGgFotuNH
PPWth+v+jfUEHzFMJjL4IQBn91RtVP1/iW/aCFvQQypa7UjtHFHGsa3u3qku4HXq
vUzlxLgDcyacTpwao9B70UVNapjcQC0SFmrs5MYCqMGnkb6SXHca/tjGbzR4nUP0
brSZtH/FcD9CSjfqgYpFkvACUH6s69PNi35gX4BadZUQ2ajm2l5P1znF4HOesLdh
gYPcQqd7pOMdYocQuRAXBPr80Xq/Dj4MlclI7nn6cwfGCdLYcoOdyjvwA/4rc/Zm
qE/22sOC77MEDwKB3nNWppU0IqjMC7qrGVVOis/fwfVToHsWxeeDbupdTJ/27Aql
ugfZqeqkkwwCvI2JA3DE+f2kVsO/I00RR7dYw4T5RQ6LNav0tCTuLpV+59AbByQd
NWWVyLo+QluwtgFIEjI0HGmX7VtSihrJz3zoSUtveVxWScZFcjkjwNW40hOTfxFF
FKXdhYI2CYq5yjhm6BHU8Y6BEd9TU/um4zACNbxfMZMNUyNU3Ec=
=oolk
-----END PGP SIGNATURE-----

--YsNoccMKm7pRZKoi--