This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/commons-daemon.git
commit f3c7e3a4ac4fdaf056f58757e81a5133b7477c12 Author: Administrator <[email protected]> AuthorDate: Thu Mar 10 17:16:27 2022 +0000 Fix DAEMON-437. Grant service user access to log directory. https://issues.apache.org/jira/browse/DAEMON-437 --- src/changes/changes.xml | 6 +- src/native/windows/apps/prunmgr/Makefile | 1 + src/native/windows/apps/prunmgr/prunmgr.c | 43 +++++++++++-- src/native/windows/apps/prunsrv/Makefile | 1 + src/native/windows/apps/prunsrv/prunsrv.c | 13 ++++ src/native/windows/include/apxwin.h | 1 + src/native/windows/include/security.h | 37 +++++++++++ src/native/windows/src/security.c | 102 ++++++++++++++++++++++++++++++ 8 files changed, 199 insertions(+), 5 deletions(-) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 61d2e96..f4b7bfb 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -66,7 +66,11 @@ Procrun. Only redirect stderr and stdout to files for the running service. Output from commands executed on the command line will not be redirected. - </action> + </action> + <action issue="DAEMON-437" type="fix" dev="markt"> + Procrun. Ensure that the user configured to run the service is also + granted access to the log file directory. + </action> <!-- ADD --> <action type="add" dev="ggregory" due-to="John Patrick"> Enable Dependabot #20. diff --git a/src/native/windows/apps/prunmgr/Makefile b/src/native/windows/apps/prunmgr/Makefile index 92828ed..cf3986c 100644 --- a/src/native/windows/apps/prunmgr/Makefile +++ b/src/native/windows/apps/prunmgr/Makefile @@ -43,6 +43,7 @@ OBJECTS = \ $(WORKDIR)\mclib.obj \ $(WORKDIR)\registry.obj \ $(WORKDIR)\rprocess.obj \ + $(WORKDIR)\security.obj \ $(WORKDIR)\service.obj \ $(WORKDIR)\utils.obj \ $(WORKDIR)\prunmgr.obj diff --git a/src/native/windows/apps/prunmgr/prunmgr.c b/src/native/windows/apps/prunmgr/prunmgr.c index e2bcd65..0851639 100644 --- a/src/native/windows/apps/prunmgr/prunmgr.c +++ b/src/native/windows/apps/prunmgr/prunmgr.c @@ -42,10 +42,6 @@ LPAPXGUISTORE _gui_store = NULL; #define STAT_DISABLED TEXT("Disabled") #define STAT_NONE TEXT("") -#define STAT_SERVICE L"NT AUTHORITY\\LocalService" -#define STAT_NET_SERVICE L"NT AUTHORITY\\NetworkService" -#define STAT_SYSTEM L"LocalSystem" - #define LOGL_ERROR L"Error" #define LOGL_INFO L"Info" #define LOGL_WARN L"Warn" @@ -338,6 +334,9 @@ BOOL __generalLogonSave(HWND hDlg) WCHAR szU[SIZ_RESLEN]; WCHAR szP[SIZ_RESLEN]; WCHAR szC[SIZ_RESLEN]; + WCHAR szB[SIZ_DESLEN]; + HWND hDlgLogging = NULL; + DWORD dwResult; if (!(TST_BIT_FLAG(_propertyChanged, 2))) return TRUE; @@ -348,17 +347,47 @@ BOOL __generalLogonSave(HWND hDlg) GetDlgItemTextW(hDlg, IDC_PPSLUSER, szU, SIZ_RESMAX); GetDlgItemTextW(hDlg, IDC_PPSLPASS, szP, SIZ_RESMAX); GetDlgItemTextW(hDlg, IDC_PPSLCPASS, szC, SIZ_RESMAX); + + // Logging page will only be accessible if previously viewed + hDlgLogging = PropSheet_IndexToHwnd(GetParent(hDlg), 2); + if (hDlgLogging) { + GetDlgItemTextW(hDlgLogging, IDC_PPLGPATH, szB, SIZ_DESMAX); + } else { + LPWSTR b; + // Logging page not viewed so value in registry must be current. + b = apxRegistryGetStringW(hRegserv, APXREG_PARAMSOFTWARE, _s_log, L"Path"); + lstrlcpyW(szB, SIZ_DESLEN, b); + apxFree(b); + } if (lstrlenW(szU) == 0 || lstrcmpiW(szU, STAT_SERVICE) == 0) { apxServiceSetNames(hService, NULL, NULL, NULL, STAT_SERVICE, L""); + dwResult = apxSecurityGrantFileAccessToUser(szB, STAT_SERVICE); + if (dwResult) { + apxDisplayError(TRUE, NULL, 0, + "Failed to grant service user '%S' write permissions to log path '%S' due to error '%d'", + STAT_SERVICE, szB, dwResult); + } lstrlcpyW(_currentEntry->szObjectName, SIZ_RESLEN, STAT_SERVICE); } else if (lstrcmpiW(szU, STAT_NET_SERVICE) == 0) { apxServiceSetNames(hService, NULL, NULL, NULL, STAT_NET_SERVICE, L""); + dwResult = apxSecurityGrantFileAccessToUser(szB, STAT_NET_SERVICE); + if (dwResult) { + apxDisplayError(TRUE, NULL, 0, + "Failed to grant service user '%S' write permissions to log path '%S' due to error '%d'", + STAT_NET_SERVICE, szB, dwResult); + } lstrlcpyW(_currentEntry->szObjectName, SIZ_RESLEN, STAT_NET_SERVICE); } else if (lstrcmpiW(szU, STAT_SYSTEM) == 0) { apxServiceSetNames(hService, NULL, NULL, NULL, STAT_SYSTEM, L""); + dwResult = apxSecurityGrantFileAccessToUser(szB, STAT_SYSTEM); + if (dwResult) { + apxDisplayError(TRUE, NULL, 0, + "Failed to grant service user '%S' write permissions to log path '%S' due to error '%d'", + STAT_SYSTEM, szB, dwResult); + } lstrlcpyW(_currentEntry->szObjectName, SIZ_RESLEN, STAT_SYSTEM); if (IsDlgButtonChecked(hDlg, IDC_PPSLID) == BST_CHECKED) { apxServiceSetOptions(hService, @@ -373,6 +402,12 @@ BOOL __generalLogonSave(HWND hDlg) } else { if (szP[0] != L' ' && szC[0] != L' ' && !lstrcmpW(szP, szC)) { apxServiceSetNames(hService, NULL, NULL, NULL, szU, szP); + dwResult = apxSecurityGrantFileAccessToUser(szB, szU); + if (dwResult) { + apxDisplayError(TRUE, NULL, 0, + "Failed to grant service user '%S' write permissions to log path '%S' due to error '%d'", + szU, szB, dwResult); + } lstrlcpyW(_currentEntry->szObjectName, SIZ_RESLEN, szU); } else { diff --git a/src/native/windows/apps/prunsrv/Makefile b/src/native/windows/apps/prunsrv/Makefile index b6880c7..013d597 100644 --- a/src/native/windows/apps/prunsrv/Makefile +++ b/src/native/windows/apps/prunsrv/Makefile @@ -43,6 +43,7 @@ OBJECTS = \ $(WORKDIR)\mclib.obj \ $(WORKDIR)\registry.obj \ $(WORKDIR)\rprocess.obj \ + $(WORKDIR)\security.obj \ $(WORKDIR)\service.obj \ $(WORKDIR)\utils.obj \ $(WORKDIR)\prunsrv.obj diff --git a/src/native/windows/apps/prunsrv/prunsrv.c b/src/native/windows/apps/prunsrv/prunsrv.c index 472ce14..1f607be 100644 --- a/src/native/windows/apps/prunsrv/prunsrv.c +++ b/src/native/windows/apps/prunsrv/prunsrv.c @@ -796,6 +796,8 @@ static BOOL docmdInstallService(LPAPXCMDLINE lpCmdline) LPCWSTR sd = NULL; LPCWSTR su = NULL; LPCWSTR sp = NULL; + DWORD dwResult; + if (ST_DESCRIPTION & APXCMDOPT_FOUND) { sd = SO_DESCRIPTION; apxLogWrite(APXLOG_MARK_DEBUG "Setting service description '%S'.", @@ -812,6 +814,11 @@ static BOOL docmdInstallService(LPAPXCMDLINE lpCmdline) SO_SPASSWORD); } apxServiceSetNames(hService, NULL, NULL, sd, su, sp); + dwResult = apxSecurityGrantFileAccessToUser(SO_LOGPATH, su); + if (dwResult) { + apxLogWrite(APXLOG_MARK_WARN "Failed to grant service user '%S' write permissions to log path '%S' due to error '%d'", + su, SO_LOGPATH, dwResult); + } } apxCloseHandle(hService); if (rv) { @@ -978,6 +985,7 @@ static BOOL docmdUpdateService(LPAPXCMDLINE lpCmdline) } else { DWORD dwStart = SERVICE_NO_CHANGE; + DWORD dwResult; BOOL bDelayedStart = FALSE; DWORD dwType = SERVICE_NO_CHANGE; LPCWSTR su = NULL; @@ -998,6 +1006,11 @@ static BOOL docmdUpdateService(LPAPXCMDLINE lpCmdline) SO_DESCRIPTION, su, sp)); + dwResult = apxSecurityGrantFileAccessToUser(SO_LOGPATH, su); + if (dwResult) { + apxLogWrite(APXLOG_MARK_WARN "Failed to grant service user '%S' write permissions to log path '%S' due to error '%d'", + su, SO_LOGPATH, dwResult); + } /* Update the --Startup mode */ if (ST_STARTUP & APXCMDOPT_FOUND) { if (!lstrcmpiW(SO_STARTUP, PRSRV_DELAYED)) { diff --git a/src/native/windows/include/apxwin.h b/src/native/windows/include/apxwin.h index 1d70063..012cf30 100644 --- a/src/native/windows/include/apxwin.h +++ b/src/native/windows/include/apxwin.h @@ -107,6 +107,7 @@ typedef _W64 int intptr_t; #include "console.h" #include "rprocess.h" #include "registry.h" +#include "security.h" #include "service.h" #include "javajni.h" #include "gui.h" diff --git a/src/native/windows/include/security.h b/src/native/windows/include/security.h new file mode 100644 index 0000000..02c7d81 --- /dev/null +++ b/src/native/windows/include/security.h @@ -0,0 +1,37 @@ +/* Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef _SECURITY_H_INCLUDED_ +#define _SECURITY_H_INCLUDED_ + +__APXBEGIN_DECLS + +#define DEFAULT_SERVICE_USER L"NT AUTHORITY\\LocalService" + +#define STAT_SERVICE L"NT AUTHORITY\\LocalService" +#define STAT_NET_SERVICE L"NT AUTHORITY\\NetworkService" +#define STAT_SYSTEM L"LocalSystem" + +#define STAT_SYSTEM_WITH_DOMAIN L"NT AUTHORITY\\System" + +DWORD +apxLogGrantFileAccessToUser( + LPCWSTR szPath, + LPCWSTR szUser); + +__APXEND_DECLS + +#endif /* _SECURITY_H_INCLUDED_ */ diff --git a/src/native/windows/src/security.c b/src/native/windows/src/security.c new file mode 100644 index 0000000..5cfedff --- /dev/null +++ b/src/native/windows/src/security.c @@ -0,0 +1,102 @@ +/* Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "apxwin.h" +#include "private.h" +#include <stdio.h> +#include <accctrl.h> +#include <aclapi.h> + +DWORD +apxSecurityGrantFileAccessToUser( + LPCWSTR szPath, + LPCWSTR szUser) +{ + WCHAR sPath[SIZ_PATHLEN]; + WCHAR sUser[SIZ_RESLEN]; + DWORD dwResult; + PACL pOldDACL; + PACL pNewDACL; + PSECURITY_DESCRIPTOR pSD; + EXPLICIT_ACCESS ea; + + lstrlcpyW(sPath, SIZ_PATHLEN, szPath); + + if (szUser) { + /* The API used to set file permissions doesn't always recognised the + * same users as the API used to configured services. We do any + * necessary conversion here. The known issues are: + * LocalSystem is not recognised. It needs to be converted to + * "NT Authority\System" + * User names for the local machine that use the ".\username" form need + * to have the leading ".\" removed. + */ + if (!StrCmpW(STAT_SYSTEM, szUser)) { + lstrlcpyW(sUser, SIZ_RESLEN, STAT_SYSTEM_WITH_DOMAIN); + } else { + if (StrStrW(szUser, L".\\") == szUser) { + szUser +=2; + } + lstrlcpyW(sUser, SIZ_RESLEN, szUser); + } + } else { + lstrlcpyW(sUser, SIZ_RESLEN, DEFAULT_SERVICE_USER); + } + + /* Old (current) ACL. */ + dwResult = GetNamedSecurityInfoW( + sPath, + SE_FILE_OBJECT, + DACL_SECURITY_INFORMATION, + NULL, + NULL, + &pOldDACL, + NULL, + &pSD); + if (dwResult) { + return dwResult; + } + + /* Additional access. */ + ZeroMemory(&ea, sizeof(EXPLICIT_ACCESS)); + ea.grfAccessPermissions = GENERIC_EXECUTE + GENERIC_READ + GENERIC_WRITE; + ea.grfAccessMode = GRANT_ACCESS; + ea.grfInheritance = CONTAINER_INHERIT_ACE + OBJECT_INHERIT_ACE; + ea.Trustee.TrusteeForm = TRUSTEE_IS_NAME; + ea.Trustee.ptstrName = sUser; + + /* Merge old and additional into new ACL. */ + dwResult = SetEntriesInAcl(1, &ea, pOldDACL, &pNewDACL); + if (dwResult) { + return dwResult; + } + + /* Set the new ACL. */ + dwResult = SetNamedSecurityInfoW( + sPath, + SE_FILE_OBJECT, + DACL_SECURITY_INFORMATION, + NULL, + NULL, + pNewDACL, + NULL); + if (dwResult) { + return dwResult; + } + + return 0; +} + \ No newline at end of file
