From b912500448748f61e4114bcffbe651f8ce558a0d Mon Sep 17 00:00:00 2001 From: Jordan Borean Date: Tue, 13 Dec 2022 10:00:45 +1000 Subject: [PATCH 1/2] Preserve content with Get-AuthenticodeSignature Do not try and roundtrip the -Content bytes to a Unicode encoded string and then back to bytes. Instead just use the raw input bytes when validating the content signature. --- .../security/SignatureCommands.cs | 2 +- .../security/Authenticode.cs | 27 ++-- .../security/SecurityManager.cs | 4 +- .../engine/Security/FileSignature.Tests.ps1 | 148 ++++++++++++++++++ 4 files changed, 166 insertions(+), 15 deletions(-) diff --git a/src/Microsoft.PowerShell.Security/security/SignatureCommands.cs b/src/Microsoft.PowerShell.Security/security/SignatureCommands.cs index 411c7952ea8..09fefdfb8e9 100644 --- a/src/Microsoft.PowerShell.Security/security/SignatureCommands.cs +++ b/src/Microsoft.PowerShell.Security/security/SignatureCommands.cs @@ -294,7 +294,7 @@ protected override Signature PerformAction(string filePath) /// protected override Signature PerformAction(string sourcePathOrExtension, byte[] content) { - return SignatureHelper.GetSignature(sourcePathOrExtension, System.Text.Encoding.Unicode.GetString(content)); + return SignatureHelper.GetSignature(sourcePathOrExtension, content); } } diff --git a/src/System.Management.Automation/security/Authenticode.cs b/src/System.Management.Automation/security/Authenticode.cs index e732420b188..591f64d6298 100644 --- a/src/System.Management.Automation/security/Authenticode.cs +++ b/src/System.Management.Automation/security/Authenticode.cs @@ -275,7 +275,7 @@ internal static Signature SignFile(SigningOption option, /// /// Thrown if the file specified by argument fileName is not found. /// - internal static Signature GetSignature(string fileName, string fileContent) + internal static Signature GetSignature(string fileName, byte[] fileContent) { Signature signature = null; @@ -398,7 +398,7 @@ private static uint GetErrorFromSignatureState(SignatureState signatureState) } #endif - private static Signature GetSignatureFromWinVerifyTrust(string fileName, string fileContent) + private static Signature GetSignatureFromWinVerifyTrust(string fileName, byte[] fileContent) { Signature signature = null; @@ -409,6 +409,7 @@ private static Signature GetSignatureFromWinVerifyTrust(string fileName, string { Utils.CheckArgForNullOrEmpty(fileName, "fileName"); SecuritySupport.CheckIfFileExists(fileName); + // SecurityUtils.CheckIfFileSmallerThan4Bytes(fileName); } @@ -424,7 +425,8 @@ private static Signature GetSignatureFromWinVerifyTrust(string fileName, string signature = GetSignatureFromWintrustData(fileName, error, wtd); wtd.dwStateAction = WinTrustAction.WTD_STATEACTION_CLOSE; - error = WinTrustMethods.WinVerifyTrust(IntPtr.Zero, + error = WinTrustMethods.WinVerifyTrust( + IntPtr.Zero, ref WINTRUST_ACTION_GENERIC_VERIFY_V2, ref wtd); @@ -441,8 +443,10 @@ private static Signature GetSignatureFromWinVerifyTrust(string fileName, string return signature; } - private static uint GetWinTrustData(string fileName, string fileContent, - out WinTrustMethods.WINTRUST_DATA wtData) + private static uint GetWinTrustData( + string fileName, + byte[] fileContent, + out WinTrustMethods.WINTRUST_DATA wtData) { wtData = new() { @@ -451,9 +455,6 @@ private static uint GetWinTrustData(string fileName, string fileContent, dwStateAction = WinTrustAction.WTD_STATEACTION_VERIFY, }; - byte[] contentBytes = fileContent == null - ? Array.Empty() - : System.Text.Encoding.Unicode.GetBytes(fileContent); unsafe { fixed (char* fileNamePtr = fileName) @@ -468,12 +469,13 @@ private static uint GetWinTrustData(string fileName, string fileContent, wtData.dwUnionChoice = WinTrustUnionChoice.WTD_CHOICE_FILE; wtData.pChoice = &wfi; - return WinTrustMethods.WinVerifyTrust(IntPtr.Zero, + return WinTrustMethods.WinVerifyTrust( + IntPtr.Zero, ref WINTRUST_ACTION_GENERIC_VERIFY_V2, ref wtData); } - fixed (byte* contentPtr = contentBytes) + fixed (byte* contentPtr = fileContent) { Guid pwshSIP = new("603BCC1F-4B59-4E08-B724-D2C6297EF351"); WinTrustMethods.WINTRUST_BLOB_INFO wbi = new() @@ -481,13 +483,14 @@ private static uint GetWinTrustData(string fileName, string fileContent, cbStruct = (uint)Marshal.SizeOf(), gSubject = pwshSIP, pcwszDisplayName = fileNamePtr, - cbMemObject = (uint)contentBytes.Length, + cbMemObject = (uint)fileContent.Length, pbMemObject = contentPtr, }; wtData.dwUnionChoice = WinTrustUnionChoice.WTD_CHOICE_BLOB; wtData.pChoice = &wbi; - return WinTrustMethods.WinVerifyTrust(IntPtr.Zero, + return WinTrustMethods.WinVerifyTrust( + IntPtr.Zero, ref WINTRUST_ACTION_GENERIC_VERIFY_V2, ref wtData); } diff --git a/src/System.Management.Automation/security/SecurityManager.cs b/src/System.Management.Automation/security/SecurityManager.cs index 2baf1560a38..b9bb5329a2d 100644 --- a/src/System.Management.Automation/security/SecurityManager.cs +++ b/src/System.Management.Automation/security/SecurityManager.cs @@ -535,7 +535,7 @@ private static Signature GetSignatureWithEncodingRetry(string path, ExternalScri // try harder to validate the signature by being explicit about encoding // and providing the script contents string verificationContents = Encoding.Unicode.GetString(script.OriginalEncoding.GetPreamble()) + script.ScriptContents; - signature = SignatureHelper.GetSignature(path, verificationContents); + signature = SignatureHelper.GetSignature(path, Encoding.Unicode.GetBytes(verificationContents)); // A last ditch effort - // If the file was originally ASCII or UTF8, the SIP may have added the Unicode BOM @@ -543,7 +543,7 @@ private static Signature GetSignatureWithEncodingRetry(string path, ExternalScri && script.OriginalEncoding != Encoding.Unicode) { verificationContents = Encoding.Unicode.GetString(Encoding.Unicode.GetPreamble()) + script.ScriptContents; - Signature fallbackSignature = SignatureHelper.GetSignature(path, verificationContents); + Signature fallbackSignature = SignatureHelper.GetSignature(path, Encoding.Unicode.GetBytes(verificationContents)); if (fallbackSignature.Status == SignatureStatus.Valid) signature = fallbackSignature; diff --git a/test/powershell/engine/Security/FileSignature.Tests.ps1 b/test/powershell/engine/Security/FileSignature.Tests.ps1 index 892aa0e40ea..948ff4d1a8b 100644 --- a/test/powershell/engine/Security/FileSignature.Tests.ps1 +++ b/test/powershell/engine/Security/FileSignature.Tests.ps1 @@ -20,3 +20,151 @@ Describe "Windows platform file signatures" -Tags 'Feature' { $signature.SignatureType | Should -BeExactly 'Catalog' } } + +Describe "Windows file content signatures" -Tags @('Feature', 'RequireAdminOnWindows') { + $PSDefaultParameterValues = @{ "It:Skip" = (-not $IsWindows) } + + BeforeAll { + $session = New-PSSession -UseWindowsPowerShell + try { + # New-SelfSignedCertificate runs in implicit remoting so do all the + # setup work over there + $caRootThumbprint, $signingThumbprint = Invoke-Command -Session $session -ScriptBlock { + $testPrefix = 'SelfSignedTest' + + $enhancedKeyUsage = [Security.Cryptography.OidCollection]::new() + $null = $enhancedKeyUsage.Add('1.3.6.1.5.5.7.3.3') # Code Signing + + $caParams = @{ + Extension = @( + [Security.Cryptography.X509Certificates.X509BasicConstraintsExtension]::new($true, $false, 0, $true), + [Security.Cryptography.X509Certificates.X509KeyUsageExtension]::new('KeyCertSign', $false), + [Security.Cryptography.X509Certificates.X509EnhancedKeyUsageExtension ]::new($enhancedKeyUsage, $false) + ) + CertStoreLocation = 'Cert:\CurrentUser\My' + NotAfter = (Get-Date).AddDays(1) + Type = 'Custom' + } + $caRoot = PKI\New-SelfSignedCertificate @caParams -Subject "CN=$testPrefix-CA" + + $rootStore = Get-Item -Path Cert:\LocalMachine\Root + $rootStore.Open([System.Security.Cryptography.X509Certificates.OpenFlags]::ReadWrite) + try { + $rootStore.Add([System.Security.Cryptography.X509Certificates.X509Certificate2]::new($caRoot.RawData)) + } finally { + $rootStore.Close() + } + + $certParams = @{ + CertStoreLocation = 'Cert:\CurrentUser\My' + KeyUsage = 'DigitalSignature' + TextExtension = @("2.5.29.37={text}1.3.6.1.5.5.7.3.3", "2.5.29.19={text}") + Type = 'Custom' + } + $certificate = PKI\New-SelfSignedCertificate @certParams -Subject "CN=$testPrefix-Signed" -Signer $caRoot + + $publisherStore = Get-Item -Path Cert:\LocalMachine\TrustedPublisher + $publisherStore.Open([System.Security.Cryptography.X509Certificates.OpenFlags]::ReadWrite) + try { + $publisherStore.Add([System.Security.Cryptography.X509Certificates.X509Certificate2]::new($certificate.RawData)) + } finally { + $publisherStore.Close() + } + + $caRoot | Remove-Item + + $caRoot.Thumbprint, $certificate.Thumbprint + } + } finally { + $session | Remove-PSSession + } + + $certificate = Get-Item -Path Cert:\CurrentUser\My\$signingThumbprint + } + + AfterAll { + Remove-Item -Path Cert:\LocalMachine\Root\$caRootThumbprint -Force + Remove-Item -Path Cert:\LocalMachine\TrustedPublisher\$signingThumbprint -Force + Remove-Item -Path Cert:\CurrentUser\My\$signingThumbprint -Force + } + + It "Validates signature using path on even char count with Encoding " -TestCases @( + @{ Encoding = 'ASCII' } + @{ Encoding = 'Unicode' } + @{ Encoding = 'UTF8BOM' } + @{ Encoding = 'UTF8NoBOM' } + ) { + param ($Encoding) + + Set-Content -Path testdrive:\test.ps1 -Value 'Write-Output "Hello World"' -Encoding $Encoding + + $scriptPath = Join-Path $TestDrive test.ps1 + $status = Set-AuthenticodeSignature -FilePath $scriptPath -Certificate $certificate + $status.Status | Should -Be 'Valid' + + $actual = Get-AuthenticodeSignature -FilePath $scriptPath + $actual.SignerCertificate.Thumbprint | Should -Be $certificate.Thumbprint + $actual.Status | Should -Be 'Valid' + } + + It "Validates signature using path on odd char count with Encoding " -TestCases @( + @{ Encoding = 'ASCII' } + @{ Encoding = 'Unicode' } + @{ Encoding = 'UTF8BOM' } + @{ Encoding = 'UTF8NoBOM' } + ) { + param ($Encoding) + + Set-Content -Path testdrive:\test.ps1 -Value 'Write-Output "Hello World!"' -Encoding $Encoding + + $scriptPath = Join-Path $TestDrive test.ps1 + $status = Set-AuthenticodeSignature -FilePath $scriptPath -Certificate $certificate + $status.Status | Should -Be 'Valid' + + $actual = Get-AuthenticodeSignature -FilePath $scriptPath + $actual.SignerCertificate.Thumbprint | Should -Be $certificate.Thumbprint + $actual.Status | Should -Be 'Valid' + } + + It "Validates signature using content on even char count with Encoding " -TestCases @( + @{ Encoding = 'ASCII' } + @{ Encoding = 'Unicode' } + @{ Encoding = 'UTF8BOM' } + @{ Encoding = 'UTF8NoBOM' } + ) { + param ($Encoding) + + Set-Content -Path testdrive:\test.ps1 -Value 'Write-Output "Hello World"' -Encoding $Encoding + + $scriptPath = Join-Path $TestDrive test.ps1 + $status = Set-AuthenticodeSignature -FilePath $scriptPath -Certificate $certificate + $status.Status | Should -Be 'Valid' + + $fileBytes = Get-Content -Path testdrive:\test.ps1 -AsByteStream + + $actual = Get-AuthenticodeSignature -Content $fileBytes -SourcePathOrExtension .ps1 + $actual.SignerCertificate.Thumbprint | Should -Be $certificate.Thumbprint + $actual.Status | Should -Be 'Valid' + } + + It "Validates signature using content on odd char count with Encoding " -TestCases @( + @{ Encoding = 'ASCII' } + @{ Encoding = 'Unicode' } + @{ Encoding = 'UTF8BOM' } + @{ Encoding = 'UTF8NoBOM' } + ) { + param ($Encoding) + + Set-Content -Path testdrive:\test.ps1 -Value 'Write-Output "Hello World!"' -Encoding $Encoding + + $scriptPath = Join-Path $TestDrive test.ps1 + $status = Set-AuthenticodeSignature -FilePath $scriptPath -Certificate $certificate + $status.Status | Should -Be 'Valid' + + $fileBytes = Get-Content -Path testdrive:\test.ps1 -AsByteStream + + $actual = Get-AuthenticodeSignature -Content $fileBytes -SourcePathOrExtension .ps1 + $actual.SignerCertificate.Thumbprint | Should -Be $certificate.Thumbprint + $actual.Status | Should -Be 'Valid' + } +} From 9e6b06ecaf5aba00761becf17cbe475abf8be310 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Thu, 20 Apr 2023 17:29:19 -0700 Subject: [PATCH 2/2] Try to get the exact raw bytes of a script plus BOM --- .../security/SecurityManager.cs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/System.Management.Automation/security/SecurityManager.cs b/src/System.Management.Automation/security/SecurityManager.cs index b9bb5329a2d..ff259a55a05 100644 --- a/src/System.Management.Automation/security/SecurityManager.cs +++ b/src/System.Management.Automation/security/SecurityManager.cs @@ -534,16 +534,16 @@ private static Signature GetSignatureWithEncodingRetry(string path, ExternalScri // try harder to validate the signature by being explicit about encoding // and providing the script contents - string verificationContents = Encoding.Unicode.GetString(script.OriginalEncoding.GetPreamble()) + script.ScriptContents; - signature = SignatureHelper.GetSignature(path, Encoding.Unicode.GetBytes(verificationContents)); + byte[] bytesWithBom = GetContentBytesWithBom(script.OriginalEncoding, script.ScriptContents); + signature = SignatureHelper.GetSignature(path, bytesWithBom); // A last ditch effort - // If the file was originally ASCII or UTF8, the SIP may have added the Unicode BOM if (signature.Status != SignatureStatus.Valid && script.OriginalEncoding != Encoding.Unicode) { - verificationContents = Encoding.Unicode.GetString(Encoding.Unicode.GetPreamble()) + script.ScriptContents; - Signature fallbackSignature = SignatureHelper.GetSignature(path, Encoding.Unicode.GetBytes(verificationContents)); + bytesWithBom = GetContentBytesWithBom(Encoding.Unicode, script.ScriptContents); + Signature fallbackSignature = SignatureHelper.GetSignature(path, bytesWithBom); if (fallbackSignature.Status == SignatureStatus.Valid) signature = fallbackSignature; @@ -552,6 +552,17 @@ private static Signature GetSignatureWithEncodingRetry(string path, ExternalScri return signature; } + private static byte[] GetContentBytesWithBom(Encoding encoding, string scriptContent) + { + ReadOnlySpan bomBytes = encoding.Preamble; + byte[] contentBytes = encoding.GetBytes(scriptContent); + byte[] bytesWithBom = new byte[bomBytes.Length + contentBytes.Length]; + + bomBytes.CopyTo(bytesWithBom); + contentBytes.CopyTo(bytesWithBom, index: bomBytes.Length); + return bytesWithBom; + } + #endregion signing check ///