CHEF-19255 Fix named pipes to be more secure

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

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) }