diff --git a/src/System.Management.Automation/security/SecurityManager.cs b/src/System.Management.Automation/security/SecurityManager.cs index 137daecc5b4..0611c520fdd 100644 --- a/src/System.Management.Automation/security/SecurityManager.cs +++ b/src/System.Management.Automation/security/SecurityManager.cs @@ -3,6 +3,7 @@ using System; using System.Collections.ObjectModel; +using System.Diagnostics.CodeAnalysis; using System.IO; using System.Management.Automation; using System.Management.Automation.Host; @@ -10,6 +11,7 @@ using System.Management.Automation.Language; using System.Management.Automation.Security; using System.Security; +using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; using System.Text; @@ -65,6 +67,22 @@ internal enum RunPromptDecision #region constructor + /// + /// The EKU OID that identifies a certificate is from Azure Trusted Signing. + /// + private const string _azureTrustedSigningIdentifier = "1.3.6.1.4.1.311.97.1.0"; + + /// + /// The OID prefix that uniquely identifies a certificate issued by Azure Trusted Signing. + /// + private const string _azureTrustedSigningIdPrefix = "1.3.6.1.4.1.311.97."; + + [TraceSource("SecurityManager", "Security Manager Script Trust Checks.")] + private static readonly PSTraceSource s_tracer = PSTraceSource.GetTracer( + "SecurityManager", + "Security Manager Script Trust Checks.", + false); + // execution policy that dictates what can run in msh private ExecutionPolicy _executionPolicy; @@ -217,7 +235,7 @@ private bool CheckPolicy(ExternalScriptInfo script, PSHost host, out Exception r if (signature.Status == SignatureStatus.Valid) { // The file is signed by a trusted publisher - if (IsTrustedPublisher(signature, path)) + if (IsTrustedPublisher(signature)) { policyCheckPassed = true; } @@ -287,7 +305,7 @@ private bool CheckPolicy(ExternalScriptInfo script, PSHost host, out Exception r if (signature.Status == SignatureStatus.Valid) { // The file is signed by a trusted publisher - if (IsTrustedPublisher(signature, path)) + if (IsTrustedPublisher(signature)) { policyCheckPassed = true; } @@ -350,7 +368,7 @@ private bool CheckPolicy(ExternalScriptInfo script, PSHost host, out Exception r // The file is signed by a trusted publisher if (signature.Status == SignatureStatus.Valid) { - if (IsTrustedPublisher(signature, path)) + if (IsTrustedPublisher(signature)) { policyCheckPassed = true; } @@ -431,51 +449,173 @@ private static bool IsLocalFile(string filename) #endif } - // Checks that a publisher is trusted by the system or is one of - // the signed product binaries - private static bool IsTrustedPublisher(Signature signature, string file) +#nullable enable + /// + /// Checks if the publisher is trusted by checking whether the + /// certificate thumbprint is in the "Trusted Publishers" store or + /// the Azure Trusted Signer Publisher ID is present in the + /// "Trusted Publishers" store. + /// + /// The signature to check. + /// True if the publisher is trusted. + private static bool IsTrustedPublisher(Signature signature) { // Get the thumbprint of the current signature X509Certificate2 signerCertificate = signature.SignerCertificate; string thumbprint = signerCertificate.Thumbprint; + s_tracer.WriteLine("Checking if publisher with thumbprint {0} is trusted.", thumbprint); + + TryGetAzureTrustedSignerPublisherId(signerCertificate, out string? azurePublisherId); // See if it matches any in the list of trusted publishers X509Store trustedPublishers = new X509Store(StoreName.TrustedPublisher, StoreLocation.CurrentUser); trustedPublishers.Open(OpenFlags.ReadOnly); + bool isTrusted = false; foreach (X509Certificate2 trustedCertificate in trustedPublishers.Certificates) { + s_tracer.WriteLine("Checking publisher against certificate '{0}' and thumbprint {1}.", + trustedCertificate.FriendlyName, + trustedCertificate.Thumbprint); + if (string.Equals(trustedCertificate.Thumbprint, thumbprint, StringComparison.OrdinalIgnoreCase)) { - if (!IsUntrustedPublisher(signature, file)) - { - return true; - } + isTrusted = true; + } + else if (azurePublisherId is not null && + TryGetAzureTrustedSignerPublisherId(trustedCertificate, out string? trustedIdentifier) && + azurePublisherId == trustedIdentifier) + { + isTrusted = true; + break; } } + // Do a final check to verify that the certificate has not been + // explicitly added to the "Disallowed" store. + if (isTrusted && !IsUntrustedPublisher(signerCertificate)) + { + return true; + } + return false; } - private static bool IsUntrustedPublisher(Signature signature, string file) + /// + /// Checks if the publisher is untrusted by checking whether the same + /// certificate thumbprint is in the "Disallowed" store. + /// + /// The certificate to check by thumbprint. + /// True when the publisher is untrusted. + private static bool IsUntrustedPublisher(X509Certificate2 signerCertificate) { // Get the thumbprint of the current signature - X509Certificate2 signerCertificate = signature.SignerCertificate; string thumbprint = signerCertificate.Thumbprint; + s_tracer.WriteLine("Checking if certificate {0} is untrusted.", + thumbprint); // See if it matches any in the list of trusted publishers - X509Store trustedPublishers = new X509Store(StoreName.Disallowed, StoreLocation.CurrentUser); - trustedPublishers.Open(OpenFlags.ReadOnly); + X509Store untrustedPublishers = new X509Store(StoreName.Disallowed, StoreLocation.CurrentUser); + untrustedPublishers.Open(OpenFlags.ReadOnly); - foreach (X509Certificate2 trustedCertificate in trustedPublishers.Certificates) + foreach (X509Certificate2 untrustedCertificate in untrustedPublishers.Certificates) { - if (string.Equals(trustedCertificate.Thumbprint, thumbprint, StringComparison.OrdinalIgnoreCase)) + s_tracer.WriteLine("Checking publisher against untrusted certificate '{0}' and thumbprint {1}.", + untrustedCertificate.FriendlyName, + untrustedCertificate.Thumbprint); + + if (string.Equals(untrustedCertificate.Thumbprint, thumbprint, StringComparison.OrdinalIgnoreCase)) + { return true; + } } return false; } + /// + /// Checks if the certificate has the Azure Trusted Signer Publisher ID + /// EKU present and sets publisherId to that unique identifier. + /// + /// The certificate to check. + /// An opaque blog that uniquely identifies the publisher if present. + /// True when the certificate has the Azure Trusted Signer Publisher ID EKU. + private static bool TryGetAzureTrustedSignerPublisherId( + X509Certificate2 certificate, + [NotNullWhen(true)] out string? publisherId) + { + bool containsAzTSIdentifier = false; + string? azurePubOid = null; + + foreach (X509Extension ext in certificate.Extensions) + { + if (ext is X509EnhancedKeyUsageExtension ekuExt) + { + // The EKU OIDs need to contain the Azure Trusted Signing Identifier + // and have one that starts with the Azure Trusted Signing ID Prefix. + foreach (Oid oid in ekuExt.EnhancedKeyUsages) + { + if (oid.Value == _azureTrustedSigningIdentifier) + { + containsAzTSIdentifier = true; + } + else if (oid.Value?.StartsWith(_azureTrustedSigningIdPrefix) == true) + { + azurePubOid = oid.Value; + } + } + + break; // No need to check other extensions. + } + } + + string? caThumbprint = null; + if (containsAzTSIdentifier && azurePubOid is not null) + { + s_tracer.WriteLine("Certificate {0} has Azure Trusted Signer EKU OID {1}.", + certificate.Thumbprint, + azurePubOid); + + // To avoid matching on certs that have the same EKU OID added + // we add the thumbprint of the root CA to the unique + // identifier. This means someone can't manually create a + // cert with the same OID as one already trusted as it needs to + // come from the same CA. We don't do a revocation check as we + // aren't checking the validity of the certificate, just getting + // the thumbprint of the root CA. + using X509Chain chain = new X509Chain(); + chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; + if (chain.Build(certificate)) + { + // Remarks state that the last element in the chain in the + // root CA on all platforms. + caThumbprint = chain.ChainElements[^1].Certificate.Thumbprint; + } + else + { + s_tracer.WriteLine("Failed to find root CA for certificate {0}: {1}", + certificate.Thumbprint, + chain.ChainStatus[0].StatusInformation); + } + } + + if (caThumbprint is not null) + { + publisherId = $"{azurePubOid}.{caThumbprint}"; + + s_tracer.WriteLine("Publisher ID for certificate {0} is {1}.", + certificate.Thumbprint, + publisherId); + return true; + } + else + { + publisherId = null; + return false; + } + } +#nullable disable + /// /// Trust a publisher by adding it to the "Trusted Publishers" store. /// diff --git a/test/powershell/Modules/Microsoft.PowerShell.Security/ExecutionPolicy.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Security/ExecutionPolicy.Tests.ps1 index 9bcc0143d34..bc1c232d4d2 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Security/ExecutionPolicy.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Security/ExecutionPolicy.Tests.ps1 @@ -31,6 +31,267 @@ Describe "ExecutionPolicy" -Tags "CI" { } } +Describe "Trusted Publisher Checks" -Tags 'CI', 'RequireAdminOnWindows' { + BeforeAll { + $originalDefaultParameterValues = $PSDefaultParameterValues.Clone() + if (-not $IsWindows) { + $PSDefaultParameterValues["It:Skip"] = $true + return + } + + Function New-X509Certificate { + [OutputType([System.Security.Cryptography.X509Certificates.X509Certificate2])] + [CmdletBinding()] + param ( + [Parameter(Mandatory)] + [string]$Subject, + + [Parameter()] + [ValidateSet('CA', 'CodeSigning')] + [string] + $Purpose, + + [Parameter()] + [string[]] + $EkuOid = @(), + + [Parameter()] + [System.Security.Cryptography.X509Certificates.X509Certificate2] + $Issuer + ) + + $key = [System.Security.Cryptography.RSA]::Create(2048) + $request = [System.Security.Cryptography.X509Certificates.CertificateRequest]::new( + $Subject, + $key, + "SHA256", + [System.Security.Cryptography.RSASignaturePadding]::Pkcs1) + + if ($Purpose -eq 'CA') { + $request.CertificateExtensions.Add( + [System.Security.Cryptography.X509Certificates.X509BasicConstraintsExtension]::new($true, $false, 0, $true)) + } + else { + $request.CertificateExtensions.Add( + [System.Security.Cryptography.X509Certificates.X509KeyUsageExtension]::new( + [System.Security.Cryptography.X509Certificates.X509KeyUsageFlags]::DigitalSignature, + $true)) + + $enhancedKeyUsageOids = [System.Security.Cryptography.OidCollection]::new() + $null = $enhancedKeyUsageOids.Add( + [System.Security.Cryptography.Oid]::FromFriendlyName('Code Signing', 'EnhancedKeyUsage')) + foreach ($oid in $EkuOid) { + $null = $enhancedKeyUsageOids.Add([System.Security.Cryptography.Oid]::new($oid)) + } + + $request.CertificateExtensions.Add( + [System.Security.Cryptography.X509Certificates.X509EnhancedKeyUsageExtension]::new( + $enhancedKeyUsageOids, $true)) + } + + $request.CertificateExtensions.Add( + [System.Security.Cryptography.X509Certificates.X509SubjectKeyIdentifierExtension]::new( + $request.PublicKey, $false)) + + if ($Issuer) { + $request.CertificateExtensions.Add( + [System.Security.Cryptography.X509Certificates.X509AuthorityKeyIdentifierExtension]::CreateFromCertificate( + $Issuer, $true, $false)) + + $notBefore = $Issuer.NotBefore + $notAfter = $Issuer.NotAfter + $serialNumber = [byte[]]::new(9) + [System.Random]::new().NextBytes($serialNumber) + + $cert = $request.Create($Issuer, $notBefore, $notAfter, $serialNumber) + + # Create does not create an X509Certificate2 object with the + # associated private key so we need to manually do it. + [System.Security.Cryptography.X509Certificates.RSACertificateExtensions]::CopyWithPrivateKey($cert, $key) + } + else { + $notBefore = [DateTimeOffset]::UtcNow.AddDays(-1) + $notAfter = [DateTimeOffset]::UtcNow.AddDays(30) + $request.CreateSelfSigned($notBefore, $notAfter) + } + } + + Function Invoke-SignedScript { + [CmdletBinding()] + param ( + [Parameter(Mandatory)] + [System.Security.Cryptography.X509Certificates.X509Certificate2] + $Certificate + ) + + $ErrorActionPreference = 'Stop' + + $scriptPath = Join-Path -Path Temp: -ChildPath "pwsh-signed-test.ps1" + Set-Content -LiteralPath $scriptPath -Value '"ran"' + try { + $null = Set-AuthenticodeSignature -FilePath $scriptPath -Certificate $Certificate -HashAlgorithm SHA256 + + # We use a job to avoid polluting the current process execution policy. + Start-Job -ScriptBlock { + Set-ExecutionPolicy -ExecutionPolicy AllSigned -Scope Process -Force + + # Use PowerShell to run without a PSHost so prompts fail. + $ps = [PowerShell]::Create() + $ps.AddCommand($args[0]).Invoke() + foreach ($e in $ps.Streams.Error) { + Write-Error $e + } + } -ArgumentList $scriptPath | Receive-Job -Wait -AutoRemoveJob + } + finally { + Remove-Item -LiteralPath $scriptPath -ErrorAction SilentlyContinue + } + } + + $azureIdMarker = '1.3.6.1.4.1.311.97.1.0' + $azureIdPrefix = '1.3.6.1.4.1.311.97.' + $publisherOneEku = @($azureIdMarker, "${azureIdPrefix}123.456.789") + $publisherTwoEku = @($azureIdMarker, "${azureIdPrefix}987.654.321") + $publisherNoMarker = @("${azureIdPrefix}123.456.789") + + $ca = New-X509Certificate -Subject "CN=Pwsh Test Signing CA" -Purpose CA + $otherCa = New-X509Certificate -Subject "CN=Pwsh Test Signing CA 2" -Purpose CA + $signingParams = @{ + Purpose = 'CodeSigning' + Issuer = $ca + } + + $publisherOneCerts = @( + # 3 certs for publisher one to test matching by EKU OID + New-X509Certificate -Subject "CN=Pwsh Test Signing Certificate 1" -EkuOid $publisherOneEku @signingParams + New-X509Certificate -Subject "CN=Pwsh Test Signing Certificate 1" -EkuOid $publisherOneEku @signingParams + New-X509Certificate -Subject "CN=Pwsh Test Signing Certificate 1" -EkuOid $publisherOneEku @signingParams + ) + $publisherTwoCerts = @( + # 1 cert for another publisher with a different EKU OID + New-X509Certificate -Subject "CN=Pwsh Test Signing Certificate 2" -EkuOid $publisherTwoEku @signingParams + ) + $publisherNoMarkerCerts = @( + # 2 certs for verifying that EKU OID is only checked when the marker is present as well + New-X509Certificate -Subject "CN=Pwsh Test Signing Certificate No Marker" -EkuOid $publisherNoMarker @signingParams + New-X509Certificate -Subject "CN=Pwsh Test Signing Certificate No Marker" -EkuOid $publisherNoMarker @signingParams + ) + $otherCaCerts = @( + # Used to verify that we can't spoof the EKU by issuing from another CA + New-X509Certificate -Subject "CN=Pwsh Test Signing Certificate 1" -EkuOid $publisherOneEku -Issuer $otherCa -Purpose CodeSigning + ) + + # We cannot use CurrentUser as always prompts with an interactive + # dialog Window. This is why the tests need to run as Administrator. + $rootStore = Get-Item Cert:\LocalMachine\Root + $rootStore.Open('ReadWrite') + $rootStore.Add($ca) + $rootStore.Add($otherCa) + + $trustedStore = Get-Item Cert:\CurrentUser\TrustedPublisher + $trustedStore.Open('ReadWrite') + + $disallowedStore = Get-Item Cert:\CurrentUser\Disallowed + $disallowedStore.Open('ReadWrite') + } + + AfterEach { + if (-not $IsWindows) { + return + } + + $certs = [System.Security.Cryptography.X509Certificates.X509Certificate2Collection]::new( + [System.Security.Cryptography.X509Certificates.X509Certificate2[]]@( + $publisherOneCerts; $publisherTwoCerts; $publisherNoMarkerCerts; $otherCaCerts)) + $trustedStore.RemoveRange($certs) + $disallowedStore.RemoveRange($certs) + } + + AfterAll { + $global:PSDefaultParameterValues = $originalDefaultParameterValues + if (-not $IsWindows) { + return + } + + $rootStore.Remove($ca) + $rootStore.Remove($otherCa) + $rootStore.Dispose() + $trustedStore.Dispose() + $disallowedStore.Dispose() + } + + It "Trusts publisher by thumbprint match" { + $trustedStore.Add($publisherOneCerts[0]) + Invoke-SignedScript -Certificate $publisherOneCerts[0] | Should -Be ran + } + + It "Trusts publisher by EKU OID in another trusted cert" { + $trustedStore.Add($publisherOneCerts[0]) + Invoke-SignedScript -Certificate $publisherOneCerts[1] | Should -Be ran + } + + It "Ignores disallow cert with EKU OID match when CA root is different" { + $disallowedStore.Add($otherCaCerts[0]) + $trustedStore.Add($publisherOneCerts[0]) + Invoke-SignedScript -Certificate $publisherOneCerts[0] | Should -Be ran + } + + It "Ignores disallow cert with EKU OID match but different thumbprint - allow " -TestCases @( + @{ AllowType = 'ByThumbprint'} + @{ AllowType = 'ByEku' } + ) { + param ($AllowType) + + $trustedCert = $AllowType -eq 'ByThumbprint' ? $publisherOneCerts[0] : $publisherOneCerts[1] + $trustedStore.Add($trustedCert) + $disallowedStore.Add($publisherOneCerts[2]) + + Invoke-SignedScript -Certificate $publisherOneCerts[0] | Should -Be ran + } + + It "Distrusts publisher not in TrustedPublisher store" { + { + Invoke-SignedScript -Certificate $publisherOneCerts[0] + } | Should -Throw + } + + It "Distrusts publisher when EKU OID has no marker present" { + $trustedStore.Add($publisherNoMarkerCerts[0]) + { + Invoke-SignedScript -Certificate $publisherNoMarkerCerts[1] + } | Should -Throw + } + + It "Distrusts publisher when EKU OID matches but from another CA" { + $trustedStore.Add($publisherOneCerts[0]) + { + Invoke-SignedScript -Certificate $otherCaCerts[0] + } | Should -Throw + } + + It "Distrusts publisher when EKU OID is not the same but from same CA" { + $trustedStore.Add($publisherOneCerts[0]) + { + Invoke-SignedScript -Certificate $publisherTwoCerts[0] + } | Should -Throw + } + + It "Distrusts publisher when trusted but is disallowed - allow " -TestCases @( + @{ AllowType = 'ByThumbprint' } + @{ AllowType = 'ByEku' } + ) { + param ($AllowType) + + $trustedCert = $AllowType -eq 'ByThumbprint' ? $publisherOneCerts[0] : $publisherOneCerts[1] + $trustedStore.Add($trustedCert) + $disallowedStore.Add($publisherOneCerts[1]) + + { + Invoke-SignedScript -Certificate $publisherOneCerts[1] + } | Should -Throw + } +} + # # Ported from MultiMachine Tests # Tests\Engine\HelpSystem\Pester.Engine.HelpSystem.BugFix.Tests.ps1