diff --git a/lib/train/transports/local.rb b/lib/train/transports/local.rb index d2b32b136..a534799ee 100644 --- a/lib/train/transports/local.rb +++ b/lib/train/transports/local.rb @@ -230,6 +230,12 @@ def acquire_pipe pipe = nil + # Verify ownership before connecting + owner, current_user, is_owner = pipe_owned_by_current_user?(pipe_name) + unless is_owner + raise PipeError, "Unauthorized user '#{current_user}' tried to connect to pipe '#{pipe_name}'. Pipe is owned by '#{owner}'." + end + # PowerShell needs time to create pipe. 100.times do pipe = open("//./pipe/#{pipe_name}", "r+") @@ -246,8 +252,11 @@ def start_pipe_server(pipe_name) script = <<-EOF $ErrorActionPreference = 'Stop' - - $pipeServer = New-Object System.IO.Pipes.NamedPipeServerStream('#{pipe_name}') + $user = [System.Security.Principal.WindowsIdentity]::GetCurrent().Name + $pipeSecurity = New-Object System.IO.Pipes.PipeSecurity + $rule = New-Object System.IO.Pipes.PipeAccessRule($user, "FullControl", "Allow") + $pipeSecurity.AddAccessRule($rule) + $pipeServer = New-Object System.IO.Pipes.NamedPipeServerStream('#{pipe_name}', [System.IO.Pipes.PipeDirection]::InOut, 1, [System.IO.Pipes.PipeTransmissionMode]::Byte, [System.IO.Pipes.PipeOptions]::None, 4096, 4096, $pipeSecurity) $pipeReader = New-Object System.IO.StreamReader($pipeServer) $pipeWriter = New-Object System.IO.StreamWriter($pipeServer) @@ -288,6 +297,29 @@ def start_pipe_server(pipe_name) cmd = "#{@powershell_cmd} -NoProfile -ExecutionPolicy bypass -NonInteractive -EncodedCommand #{base64_script}" Process.create(command_line: cmd).process_id end + + def current_windows_user + user = `powershell -Command "[System.Security.Principal.WindowsIdentity]::GetCurrent().Name"`.strip + if user.nil? || user.empty? + user = `whoami`.strip + end + if user.nil? || user.empty? + raise "Unable to determine current Windows user" + end + + user + end + + # Verify pipe ownership before connecting + def pipe_owned_by_current_user?(pipe_name) + exists = `powershell -Command "Test-Path \\\\.\\pipe\\#{pipe_name}"`.strip.downcase == "true" + current_user = current_windows_user + return [nil, current_user, false] unless exists + + owner = `powershell -Command "(Get-Acl \\\\.\\pipe\\#{pipe_name}).Owner" 2>&1`.strip + is_owner = !owner.nil? && !current_user.nil? && owner.casecmp(current_user) == 0 + [owner, current_user, is_owner] + end end end end diff --git a/test/windows/local_test.rb b/test/windows/local_test.rb index ff9940dfb..480e08bac 100644 --- a/test/windows/local_test.rb +++ b/test/windows/local_test.rb @@ -3,7 +3,7 @@ require "minitest/autorun" require "minitest/spec" -require "mocha/setup" +require "mocha/minitest" require "train" require "tempfile" unless defined?(Tempfile) require "logger" @@ -56,6 +56,54 @@ _(cmd.exit_status).must_equal 1 end + describe "WindowsPipeRunner security features" do + let(:runner) { Train::Transports::Local::Connection::WindowsPipeRunner.allocate } + let(:pipe_name) { "test_pipe" } + let(:domain_user) { "DOMAIN\\User" } + let(:other_user) { "OTHERDOMAIN\\OtherUser" } + + before do + runner.stubs(:`).with(regexp_matches(/WindowsIdentity/)).returns(domain_user) + runner.stubs(:`).with("whoami").returns(domain_user.downcase) + end + + it "returns true when current user owns the pipe" do + runner.stubs(:`).with(regexp_matches(/Test-Path/)).returns("true") + runner.stubs(:`).with(regexp_matches(/Get-Acl/)).returns(domain_user) + owner, _, is_owner = runner.send(:pipe_owned_by_current_user?, pipe_name) + _(is_owner).must_equal true + _(owner).must_equal domain_user + end + + it "returns false when current user does not own the pipe" do + runner.stubs(:`).with(regexp_matches(/Test-Path/)).returns("true") + runner.stubs(:`).with(regexp_matches(/Get-Acl/)).returns(other_user) + owner, _, is_owner = runner.send(:pipe_owned_by_current_user?, pipe_name) + _(is_owner).must_equal false + _(owner).must_equal other_user + end + + it "returns false when pipe does not exist" do + runner.stubs(:`).with(regexp_matches(/Test-Path/)).returns("false") + owner, _, is_owner = runner.send(:pipe_owned_by_current_user?, pipe_name) + _(is_owner).must_equal false + _(owner).must_be_nil + end + + it "falls back to whoami if PowerShell user detection fails" do + runner.stubs(:`).with(regexp_matches(/WindowsIdentity/)).returns("") + runner.stubs(:`).with("whoami").returns(domain_user.downcase) + user = runner.send(:current_windows_user) + _(user).must_equal domain_user.downcase + end + + it "raises if both PowerShell and whoami fail" do + runner.stubs(:`).with(regexp_matches(/WindowsIdentity/)).returns("") + runner.stubs(:`).with("whoami").returns("") + _ { runner.send(:current_windows_user) }.must_raise RuntimeError + end + end + describe "force 64 bit powershell command" do let(:runner) { conn.instance_variable_get(:@runner) } let(:powershell) { runner.instance_variable_get(:@powershell_cmd) }