elastic / logstash

Logstash - transport and process your logs, events, or other data
https://www.elastic.co/products/logstash
Other
62 stars 3.5k forks source link

Superuser check is triggered on Windows when it shouldn't #14674

Open wallrik opened 2 years ago

wallrik commented 2 years ago

Issue

The following warning is triggered on Windows, running as a a user with admin rights, even when the process is not elevated: https://github.com/elastic/logstash/blob/86a18e6e3f37c0e1e78a7bffa8479e6a031cb113/logstash-core/lib/logstash/runner.rb#L458-L460 Before flipping it to a hard stop, please consider this use-case. πŸ™

Use-case

My current workflow when I need to test new pipeline configs is that I run a local Logstash instance on my Windows desktop before I transfer over the config files.

Suggestion

Please check the process integrity level on Windows and don't assume that it's running with administrative privileges based on the user.

I unfortunately do not write ruby, but just based on a quick Google search, someone suggested this ugly check:

require 'win32/registry'

is_admin = false
begin
  Win32::Registry::HKEY_USERS.open('S-1-5-19') {|reg| }
  is_admin = true
rescue
end

The code looks to be querying the registry key for the user LocalService. If I try that in a normal command-prompt with reg query HKEY_USERS\S-1-5-19 I get the answer ERROR: Access is denied.. However, if I elevate the command-prompt from the very same user I can successfully query the key. This means that it successfully detects if the process is elevated or not. βœ”

But I'm sure there's someone out there that can write a proper check that's not based on reg key permissions. πŸ˜…

In PR #14019 I appreciate the comment from @yaauie mentioning:

We also run into problems with the terminology root when Logstash is run on Windows (where we may want to limit being run-as Administrator in a future release). ... This setting could be documented as known working on POSIX platforms, and could be extended to include Windows at a point in the future.

The terminology was changed to superuser in #14089, but limiting run-as admin is exactly what I agree needs to happen, and not limit running Logstash normally on Windows. πŸ˜‡

wallrik commented 2 years ago

To follow up on this, and maybe give some inspiration on a fix. Check for this in PowerShell can be done as such:

$Identity = [Security.Principal.WindowsIdentity]::GetCurrent()
$Principal = [Security.Principal.WindowsPrincipal]::new($Identity)
$Admin = [Security.Principal.WindowsBuiltInRole]::Administrator

$Principal.IsInRole($Admin)

That will return true or false based on if the process is elevated or not, even if the user is a local administrator.

Another option could be to bypass the logic in runner.rb entirely for Windows, and move the check to the .bat-script. Over there you could have an endless number of different checks.

One method that would exit if the process is elevated:

WHOAMI.EXE /GROUPS | FIND "S-1-16-12288" >NUL && IF NOT ERRORLEVEL 1 EXIT /B 1

Another method:

NET.EXE SESSION >NUL 2>&1 && EXIT /B 1