Closed foxxcomm closed 2 years ago
Thanks for the kind commends and suggestion. Regarding using a custom service account, I'm not sure any benefit outweighs the level of effort that would be needed to code and support it. What specific problem(s) would it solve?
What specific problem(s) would it solve?
Security. If SyncThing (in an ALL USERS installation) is compromised by a security hole, the Local Service account it runs under has far reaching permissions because it is used by dozens of other services in Windows including several security sensitive services which have access to system crypto keys and Windows user credentials. Placing SyncThing in it's own non-privileged service account isolates any possible compromise to only SyncThing and Syncthing's shared folders.
Joe
According to the documentation, the LocalService
has only minimal permissions on the local computer. Maybe you are thinking of the LocalSystem
account?
Bill - The idea is to completely isolate the SyncThing service with explicit permissions to only those files being synced by running it under a unique service account without membership in any groups. SyncThing accepts inbound connections from the Internet so is considered high risk for attack. LocalService not only runs in the group users which has quite a bit of access itself, but also runs dozens of services in Windows such as the Workfolders service which maintains read/write access to user files as well as access to encrypting file system certificates in-process. If this is too much work to change we can simply write a script that runs your installer and then creates a unique local service account without any group membership and permissions the SyncThing service account and program directory. No problem. Thanks.
It would help if you could clarify the specifics of what "has quite a bit of access itself" means, as the documentation states the account has only limited permissions.
Bill -- First, because Local Service is permissioned for the Users group, that in and of itself is too much access as the Users group has quite a bit of default file system access in NTFS. Best practice would be to limit SyncThing access to only the files/folders which are synced. Second Local Service is used for dozens of security sensitive services in Windows. One example as I mentioned is the Work Folders service and it's associated NTFS SyncShareState folder (to which Local Service has full permissions) which is highly sensitive for sites which deploy Work Folders to all users. Basically using any service account that is used by other services would not be best practice as it provides a vector for privilege escalation if an attack on the code is successful. Since we are talking about an application which accepts inbound connects from the Internet, best practice isolation of account rights is critical.
Thanks for sticking with me on this. I agree that the fact that LocalService
is a member of Users
could be a problem in case of a compromise. What do you think of the installer creating a custom local user account, setting for this account a very long, randomized password, and using that to run the service?
What do you think of the installer creating a custom local user account, setting for this account a very long, randomized password, and using that to run the service?
Would be a perfect solution. Critical that the service name is NOT random so the user can remember it in order to add directory permissions as needed later. Suggest "ServiceSyncthing" as it can't be mistaken for anything else. Be happy to perform testing.
This will be some work but I think it will be worth it. I have some PowerShell code already from another installer project that I can repurpose for this. It will probably be a month or so before I can get started on this, but thanks for the suggestion!
Welcome. Give me a heads up when it's testing time.
If we use a local account, it will need to be granted the SeNetworkLogonRight
user right in order for it to be able to start the service. In a domain, a GPO might override this setting (which would prevent the service from starting). We probably just need to document a recommendation to use non-administrative installation mode if the computer is joined to a domain.
Bill -- Took a look at this and SeNetworkLogonRight is only required if the local SyncThing service account was being logged into remotely. As far as I can tell SyncThing would not require that right and in fact we would NOT want the SyncThing service account to have that right. Looks like best practice here would be to make the local SyncThing service account a member of the local Guests group since by default members of the Guests group are blocked from logging into the service account over the network by inclusion in the User Rights Assignment "Deny access to this computer from the network" (secpol.msc). I'll do some testing here to confirm and report back.
Correction: I meant to write SeServiceLogonRight
(not SeNetworkLogonRight
).
No problem. Writing a PowerShell Migration Script to move from using Local Service to an isolated ServiceSyncThing local account with long random password, Includes setting and SeNetworkLogonRight permission and proper deny rights as well as modifying the service entry. Also does the database move - will share when done. You can rip the parts you need from it as desired.
Not sure what "proper deny rights" you're referring to (please clarify).
I have the PowerShell script for creating/resetting the service account, applying SeServiceLogonRight
, and creating the service (and uninstall as well).
I am interested in your code for migrating the LocalService
Syncthing configuration to the local service account. Can you post it in a gist or at least describe your technique for doing it?
Bill --
Here you go:
https://gist.github.com/foxxcomm/f56a5a9b3ed990bab373b7bbf34965f5
Regarding the proper lock down question, the deny rights set by the script are:
SeDenyInteractiveLogonRight, SeDenyNetworkLogonRight and SeDenyRemoteInteractiveLogonRight. These prevent local, Remote Desktop and network logins since they are not required by Syncthing.
Joe
I'm normally a bit reluctant to apply explicit deny entries to DACLs. Since these deny entries would only apply to RDP and CIFS connections, I'm wondering if there's added value in explicitly denying them (seeing as a compromise would not use those protocols anyway)?
Regarding the script: Thanks for the effort and ideas. The script will need to have a lower "entrance fee" (we'll need to avoid the PowerShell 6+ and Carbon module download requirements). The script I am currently working on uses P/Invoke code to accomplish the local user and user rights modifications.
My current idea in the installer is to check if the current admin install is using LocalService
to run the service and to offer to migrate the data if it doesn't already exist.
I'm wondering if there's added value in explicitly denying them (seeing as a compromise would not use those protocols anyway)?
Best practice for us security folks is always to apply least privilege and access rights on ALL objects created. Just as an example, let's say Syncthing app under the new isolated service account is compromised and service account password hash is able to be dumped. Without SeDenyNetworkLogonRight being set, we can now use the isolated service account hash to access the machine remotely as an authenticated user and attempt escalation. Hard but not impossible. See sections Vulnerability and Countermeasure:
The script will need to have a lower "entrance fee" (we'll need to avoid the PowerShell 6+ and Carbon module download requirements).
Understood. We use PowerShell and Carbon for system deployments so it's what I used rather than writing those functions from scratch.
My current idea in the installer is to check if the current admin install is using LocalService to run the service and to offer to migrate the data if it doesn't already exist.
Nice, but you will need to consider how to handle the case of an unattended upgrade installation.
Anyway, my continuing kudos for working this Bill! I'd really like to see Syncthing distribute your installer and want to help make it as secure as possible.
Joe
I don't think an attacker can get the service account password hash without local administrative permissions (if the service account user somehow gets elevated, then the game is over anyway).
As far as an unattended upgrade: For sure, that would be important.
After thinking about this a bit more, I think the best approach for an admin install is to continue to use the SystemRoot\ServiceProfiles\LocalService\AppData\Local\Syncthing
configuration directory (rather than worry about migrating the config). The service install can simply grant the service account permission to this directory and use the --home
parameter in the service configuration. The upgrade will then be (mostly) seamless: The user will need to give the service account access to folders they're syncing, but this will only need to be done once.
Bill -- Two Concerns about this: 1) I don't know what will happen to file permission on that LocalService profile (SystemRoot\ServiceProfiles\LocalService...) during O/S upgrades. My guess is that file permissions are reset to O/S defaults since this is a system owned directory which would will SyncThing under the new service account from accessing its data. 2) This makes it more likely that users will overlook saving and migrating the SyncThing data if the computer is replaced or a fresh O/S install is required since the data is buried under the Windows system directory and none of the profile migration tools that I know of include the LocalService profile.
If this happens, run setup again (reinstall) to fix it.
The path is documented.
I still think the benefit of a fixed path in a known location (even if located in a system directory) is a better solution than "migrating" the data.
Or: Move the configuration from SystemRoot\ServiceProfiles\LocalService\AppData\Local\Syncthing
to CommonAppData\Syncthing
(e.g., C:\ProgramData\Syncthing
). This would seem to make more sense from a Windows application perspective.
Bill -- You don't recommend ProgramData as the default permissions allow read to all users. Security sensitive files should not be installed there.
That's not a blocker. For this specific directory we can simply disable permission inheritance, remove existing ACEs, and set SYSTEM
:F, Administrators
:F, and serviceaccount:M.
OK, here's how it works now (currently testing). The issue is, for admin install mode, we need to support migration from the legacy configuration (Synchthing config files found in the Syncthing
folder inside the LocalService
user profile) to the new location (Syncthing
folder inside CommonAppData).
\ServiceProfiles\LocalService\AppData\Local\Syncthing\config.xml
exists\Syncthing\config.xml
and SystemRoot\ServiceProfiles\AppData\Local\Syncthing\CONFIGURATION_HAS_BEEN_MIGRATED.txt
do NOT existLocalService
user profile) path to the new (CommonAppData) pathCONFIGURATION_HAS_BEEN_MIGRATED.txt
file in the legacy folder and 2) prompt the user whether to remove the legacy configuration folderIf the user says "no" to the legacy configuration folder deletion prompt, reinstalling will not cause the prompt again because the conditions won't be met.
Regarding silent installs: If folder migration is needed, the installer will automatically delete the legacy folder after a successful migration.
I think this should work seamlessly.
Bill -- Sounds good. Let me know when a beta is ready and I'll test. I want to focus on confirming the custom permissions in your folder in the ProgramData tree are kept when O/S major upgrades are done so will be testing going from Windows 10->Windows 11 and Windows Server 2019->Windows Server 2022. Will also test silent installations as well.
Joe
Permissions in directories under CommonAppData shouldn't get overwritten during an upgrade. Directories under CommonAppData (e.g., C:\ProgramData
) are for applications to store data, and Windows (should!) respect the ACLs on directories under there and not change or reset them. I would be rather surprised (not in a good way) if this is not the case.
Just published 1.19.2 with the changes mentioned in this issue. Thanks for the feedback!
Bill -- Tested under Windows 10, Windows 11 and Windows 2022 Server. No issues found so far. Great work. Thank You.
Thanks for the feedback!
Update and validation: I performed an OS upgrade on a Windows 10 machine from 1909 (I think that was the exact build, but it shouldn't matter) to 21H2, and the permissions of CommonAppData\Syncthing
were not affected.
Bill - You should sign your releases to prevent false positives. Sophos firewall is blocking download (see attached).
That's a false positive, of course. I'm not signing because I don't have a code-signing certificate, nor am I willing to pay for one. My recommendation would be to whitelist the executable or submit it to the vendor as a false positive.
Bill --
First let me congratulate you on an EXTREMELY well done installer for Syncthing. I've been writing software installers for years for our product and was very impressed with your work. I hope the SyncThing team considers adding your installer to their Windows releases.
Now, getting to my suggestion:
I'm a security professional and noticed you are installing the service under the Windows Local Service account. Given that Local Service is also used by dozens of other services in Windows including several security sensitive services:
I would suggest allowing the user (during an ALL USERS install) to select a non-privileged service account of their choice and using it as the login for the service as well as permissions on C:\Program Files\SyncThing directory (For Auto Update). Knowing the service account name the user can manually add the service account to the permissions on the folders they sync. Perhaps as simple as a checkbox "Custom Service Account" during install (if not checked use Local Service) and they simply fill in the Service Account Name and Password. Optionally a checkbox on the Password field could be "Generate Random Password" which would grey out the Password field. You could use a random Password generator or simply execute net user /random:36 and capture the output) to use when you create the NSSM service.
You could optionally create the service account as well locking it down with no group memberships (users/guests), OR simply require the user to have already created the service account using compmgmt..msc first.
Make sure to mirror the new options on the silent install command line arguments so us Admins can deploy your installer with our own deployment tools with all options specified.
Thank You for a great IWindows nstaller for SyncThing!