public inbox for [email protected]
help / color / mirror / Atom feedFrom: Andres Freund <[email protected]>
Subject: [PATCH v2] windows: Only consider us to be running as service if stderr is invalid.
Date: Tue, 2 Mar 2021 21:24:11 -0800
Previously pgwin32_is_service() would falsely return true when
postgres is started from somewhere within a service, but not as a
service. That is e.g. always the case with windows docker containers,
which some CI services use to run windows tests in.
In addition to this change, it likely would be a good idea to have
pg_ctl runservice pass down a flag indicating that postgres is running
as a service.
Author: Andres Freund <[email protected]>
Discussion: https://postgr.es/m/[email protected]
---
src/port/win32security.c | 13 +++++++++++--
src/bin/pg_ctl/pg_ctl.c | 33 +++++++++++++++++++++++++++++++++
2 files changed, 44 insertions(+), 2 deletions(-)
diff --git a/src/port/win32security.c b/src/port/win32security.c
index 4a673fde19a..b57ce61d752 100644
--- a/src/port/win32security.c
+++ b/src/port/win32security.c
@@ -95,8 +95,9 @@ pgwin32_is_admin(void)
* We consider ourselves running as a service if one of the following is
* true:
*
- * 1) We are running as LocalSystem (only used by services)
- * 2) Our token contains SECURITY_SERVICE_RID (automatically added to the
+ * 1) Standard error is not valid (always the case for services)
+ * 2) We are running as LocalSystem (only used by services)
+ * 3) Our token contains SECURITY_SERVICE_RID (automatically added to the
* process token by the SCM when starting a service)
*
* The check for LocalSystem is needed, because surprisingly, if a service
@@ -121,11 +122,19 @@ pgwin32_is_service(void)
PSID ServiceSid;
PSID LocalSystemSid;
SID_IDENTIFIER_AUTHORITY NtAuthority = {SECURITY_NT_AUTHORITY};
+ HANDLE stderr_handle;
/* Only check the first time */
if (_is_service != -1)
return _is_service;
+ stderr_handle = GetStdHandle(STD_ERROR_HANDLE);
+ if (stderr_handle != INVALID_HANDLE_VALUE && stderr_handle != NULL)
+ {
+ _is_service = 0;
+ return _is_service;
+ }
+
/* First check for LocalSystem */
if (!AllocateAndInitializeSid(&NtAuthority, 1,
SECURITY_LOCAL_SYSTEM_RID, 0, 0, 0, 0, 0, 0, 0,
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 7985da0a943..c99e3c507de 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -1737,6 +1737,31 @@ typedef BOOL (WINAPI * __SetInformationJobObject) (HANDLE, JOBOBJECTINFOCLASS, L
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.
+ *
+ * Process started as services appear to have "empty" handles (GetStdHandle()
+ * returns NULL) rather than invalid ones. But passing down NULL ourselves
+ * doesn't work, it's interpreted as STARTUPINFO->hStd* not being set. But we
+ * can pass down INVALID_HANDLE_VALUE - which makes GetStdHandle() in the new
+ * process (and its child processes!) return INVALID_HANDLE_VALUE. Which
+ * achieves the goal of postmaster running in a similar environment as pg_ctl.
+ */
+static void
+InheritStdHandles(STARTUPINFO* si)
+{
+ si->dwFlags |= STARTF_USESTDHANDLES;
+ si->hStdInput = GetStdHandle(STD_INPUT_HANDLE);
+ if (si->hStdInput == NULL)
+ si->hStdInput = INVALID_HANDLE_VALUE;
+ si->hStdOutput = GetStdHandle(STD_OUTPUT_HANDLE);
+ if (si->hStdOutput == NULL)
+ si->hStdOutput = INVALID_HANDLE_VALUE;
+ si->hStdError = GetStdHandle(STD_ERROR_HANDLE);
+ if (si->hStdError == NULL)
+ si->hStdError = INVALID_HANDLE_VALUE;
+}
+
/*
* Create a restricted token, a job object sandbox, and execute the specified
* process with it.
@@ -1774,6 +1799,14 @@ CreateRestrictedProcess(char *cmd, PROCESS_INFORMATION *processInfo, bool as_ser
ZeroMemory(&si, sizeof(si));
si.cb = sizeof(si);
+ /*
+ * Set stdin/stdout/stderr handles to be inherited in the child
+ * process. That allows postmaster and the processes it starts to perform
+ * additional checks to see if running in a service (otherwise they get
+ * the default console handles - which point to "somewhere").
+ */
+ InheritStdHandles(&si);
+
Advapi32Handle = LoadLibrary("ADVAPI32.DLL");
if (Advapi32Handle != NULL)
{
--
2.29.2.540.g3cf59784d4
--pm3lfdw7knjptu4k--
view thread (26+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected]
Subject: Re: [PATCH v2] windows: Only consider us to be running as service if stderr is invalid.
In-Reply-To: <no-message-id-1870438@localhost>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox