From 7065358fda387da8fbdb3a822ff8707c32556faf Mon Sep 17 00:00:00 2001
From: Matt Mitchell <mmitche@microsoft.com>
Date: Fri, 11 Oct 2024 16:22:57 -0700
Subject: [PATCH] Fix up strong name signing for Unified Build (#15166)

---
 .../tools/Sign.proj                           |  5 +-
 .../tools/Sign.props                          |  3 +-
 .../FakeSignTool.cs                           |  3 +
 .../src/RealSignTool.cs                       | 14 ++++
 src/Microsoft.DotNet.SignTool/src/SignTool.cs | 68 +++++++++----------
 .../src/SignToolTask.cs                       | 11 ---
 .../src/ValidationOnlySignTool.cs             | 24 +++++++
 7 files changed, 79 insertions(+), 49 deletions(-)

diff --git a/src/Microsoft.DotNet.Arcade.Sdk/tools/Sign.proj b/src/Microsoft.DotNet.Arcade.Sdk/tools/Sign.proj
index b6ad1f86338..e41d22a7793 100644
--- a/src/Microsoft.DotNet.Arcade.Sdk/tools/Sign.proj
+++ b/src/Microsoft.DotNet.Arcade.Sdk/tools/Sign.proj
@@ -55,6 +55,9 @@
 
       <_DotNetCoreRequired>false</_DotNetCoreRequired>
       <_DotNetCoreRequired Condition="'$(_DryRun)' != 'true' and '$(OS)' != 'Windows_NT'">true</_DotNetCoreRequired>
+
+      <!-- SN is only available on Windows -->
+      <SNBinaryPath Condition="$([MSBuild]::IsOSPlatform('Windows'))">$(NuGetPackageRoot)sn\$(SNVersion)\sn.exe</SNBinaryPath>
     </PropertyGroup>
 
     <Error Condition="'$(AllowEmptySignList)' != 'true' AND '@(ItemsToSign)' == ''" 
@@ -91,7 +94,7 @@
         LogDir="$(ArtifactsLogDir)"
         MSBuildPath="$(_DesktopMSBuildPath)"
         DotNetPath="$(_DotNetCorePath)"
-        SNBinaryPath="$(NuGetPackageRoot)sn\$(SNVersion)\sn.exe"
+        SNBinaryPath="$(SNBinaryPath)"
         MicroBuildCorePath="$(NuGetPackageRoot)microbuild.core\$(MicroBuildCoreVersion)"
         WixToolsPath="$(WixInstallPath)"
         TarToolPath="$(NuGetPackageRoot)microsoft.dotnet.tar\$(MicrosoftDotNetSignToolVersion)\tools\net9.0\any\Microsoft.Dotnet.Tar.dll"
diff --git a/src/Microsoft.DotNet.Arcade.Sdk/tools/Sign.props b/src/Microsoft.DotNet.Arcade.Sdk/tools/Sign.props
index 095a4b8e14d..fa601f085e1 100644
--- a/src/Microsoft.DotNet.Arcade.Sdk/tools/Sign.props
+++ b/src/Microsoft.DotNet.Arcade.Sdk/tools/Sign.props
@@ -41,7 +41,8 @@
     <StrongNameSignInfo Include="SilverlightCert121" PublicKeyToken="7cec85d7bea7798e" CertificateName="Microsoft400" />
     <StrongNameSignInfo Include="StrongName" PublicKeyToken="b77a5c561934e089" CertificateName="Microsoft400" />
     <StrongNameSignInfo Include="StrongName" PublicKeyToken="b03f5f7f11d50a3a" CertificateName="Microsoft400" />
-    <StrongNameSignInfo Include="$(MSBuildThisFileDirectory)snk\Open.snk" PublicKeyToken="cc7b13ffcd2ddd51" CertificateName="Microsoft400" />
+    <!-- Do not include specific key files if full assembly signing is not supported, as they will not be usable when we sign. -->
+    <StrongNameSignInfo Include="$(MSBuildThisFileDirectory)snk\Open.snk" PublicKeyToken="cc7b13ffcd2ddd51" CertificateName="Microsoft400" Condition="'$(FullAssemblySigningSupported)' != 'false'" />
 
     <!--
       Map of file extensions to default certificate name. Files with these extensions are
diff --git a/src/Microsoft.DotNet.SignTool.Tests/FakeSignTool.cs b/src/Microsoft.DotNet.SignTool.Tests/FakeSignTool.cs
index 8bfa4870af6..0e0179b0264 100644
--- a/src/Microsoft.DotNet.SignTool.Tests/FakeSignTool.cs
+++ b/src/Microsoft.DotNet.SignTool.Tests/FakeSignTool.cs
@@ -7,6 +7,7 @@
 using System.IO.Packaging;
 using Microsoft.Build.Framework;
 using Microsoft.Build.Utilities;
+using System.Collections.Generic;
 
 namespace Microsoft.DotNet.SignTool
 {
@@ -23,6 +24,8 @@ public override void RemovePublicSign(string assemblyPath)
         {
         }
 
+        public override bool LocalStrongNameSign(IBuildEngine buildEngine, int round, IEnumerable<FileSignInfo> files) => true;
+
         public override bool VerifySignedPEFile(Stream stream)
         {
             var buffer = new byte[_stamp.Length];
diff --git a/src/Microsoft.DotNet.SignTool/src/RealSignTool.cs b/src/Microsoft.DotNet.SignTool/src/RealSignTool.cs
index 92415f6225c..a2fd51992a3 100644
--- a/src/Microsoft.DotNet.SignTool/src/RealSignTool.cs
+++ b/src/Microsoft.DotNet.SignTool/src/RealSignTool.cs
@@ -9,6 +9,7 @@
 using System.Linq;
 using System.Reflection.PortableExecutable;
 using System.Runtime.InteropServices;
+using System.Collections.Generic;
 
 namespace Microsoft.DotNet.SignTool
 {
@@ -142,5 +143,18 @@ public override bool VerifySignedVSIXFileMarker(string filePath)
         {
             return VerifySignatures.VerifySignedVSIXByFileMarker(filePath);
         }
+        
+        public override bool LocalStrongNameSign(IBuildEngine buildEngine, int round, IEnumerable<FileSignInfo> files)
+        {
+            foreach (var file in files)
+            {
+                if (file.SignInfo.ShouldLocallyStrongNameSign)
+                {
+                    return LocalStrongNameSign(file);
+                }
+            }
+
+            return true;
+        }
     }
 }
diff --git a/src/Microsoft.DotNet.SignTool/src/SignTool.cs b/src/Microsoft.DotNet.SignTool/src/SignTool.cs
index cf210abc821..cc7859f3005 100644
--- a/src/Microsoft.DotNet.SignTool/src/SignTool.cs
+++ b/src/Microsoft.DotNet.SignTool/src/SignTool.cs
@@ -30,6 +30,8 @@ internal SignTool(SignToolArgs args, TaskLoggingHelper log)
 
         public abstract void RemovePublicSign(string assemblyPath);
 
+        public abstract bool LocalStrongNameSign(IBuildEngine buildEngine, int round, IEnumerable<FileSignInfo> files);
+
         public abstract bool VerifySignedPEFile(Stream stream);
         public abstract bool VerifySignedPowerShellFile(string filePath);
         public abstract bool VerifySignedNugetFileMarker(string filePath);
@@ -45,42 +47,6 @@ public bool Sign(IBuildEngine buildEngine, int round, IEnumerable<FileSignInfo>
                 && AuthenticodeSign(buildEngine, round, files);
         }
 
-        private bool LocalStrongNameSign(IBuildEngine buildEngine, int round, IEnumerable<FileSignInfo> files)
-        {
-            foreach (var file in files)
-            {
-                if (file.SignInfo.ShouldLocallyStrongNameSign)
-                {
-                    if (!File.Exists(_args.SNBinaryPath) || !_args.SNBinaryPath.EndsWith("sn.exe"))
-                    {
-                        _log.LogError($"Found file that need to be strong-name sign ({file.FullPath}) but path to 'sn.exe' wasn't specified.");
-                        return false;
-                    }
-
-                    _log.LogMessage($"Strong-name signing {file.FullPath} locally.");
-
-                    // sn -R <path_to_file> <path_to_snk>
-                    var process = Process.Start(new ProcessStartInfo()
-                    {
-                        FileName = _args.SNBinaryPath,
-                        Arguments = $@"-R ""{file.FullPath}"" ""{file.SignInfo.StrongName}""",
-                        UseShellExecute = false,
-                        WorkingDirectory = TempDir,
-                    });
-
-                    process.WaitForExit();
-
-                    if (process.ExitCode != 0)
-                    {
-                        _log.LogError($"Failed to strong-name sign file {file.FullPath}");
-                        return false;
-                    }
-                }
-            }
-
-            return true;
-        }
-
         private bool AuthenticodeSign(IBuildEngine buildEngine, int round, IEnumerable<FileSignInfo> filesToSign)
         {
             var signingDir = Path.Combine(_args.TempDir, "Signing");
@@ -217,5 +183,35 @@ private static void AppendLine(StringBuilder builder, int depth, string text)
 
             builder.AppendLine(text);
         }
+
+        protected bool LocalStrongNameSign(FileSignInfo file)
+        {
+            if (!File.Exists(_args.SNBinaryPath) || !_args.SNBinaryPath.EndsWith("sn.exe"))
+            {
+                _log.LogError($"Found file that needs to be strong-name sign ({file.FullPath}), but path to 'sn.exe' wasn't specified.");
+                return false;
+            }
+
+            _log.LogMessage($"Strong-name signing {file.FullPath} locally.");
+
+            // sn -R <path_to_file> <path_to_snk>
+            var process = Process.Start(new ProcessStartInfo()
+            {
+                FileName = _args.SNBinaryPath,
+                Arguments = $@"-R ""{file.FullPath}"" ""{file.SignInfo.StrongName}""",
+                UseShellExecute = false,
+                WorkingDirectory = TempDir,
+            });
+
+            process.WaitForExit();
+
+            if (process.ExitCode != 0)
+            {
+                _log.LogError($"Failed to strong-name sign file {file.FullPath}");
+                return false;
+            }
+
+            return true;
+        }
     }
 }
diff --git a/src/Microsoft.DotNet.SignTool/src/SignToolTask.cs b/src/Microsoft.DotNet.SignTool/src/SignToolTask.cs
index 6b5524c4100..d6951cb78cc 100644
--- a/src/Microsoft.DotNet.SignTool/src/SignToolTask.cs
+++ b/src/Microsoft.DotNet.SignTool/src/SignToolTask.cs
@@ -226,17 +226,6 @@ public void ExecuteImpl()
                     Log.LogError($"An incorrect full path to 'sn.exe' was specified: {SNBinaryPath}");
                     return;
                 }
-
-                var strongNameLocally = StrongNameSignInfo != null
-                    && StrongNameSignInfo
-                        .Where(ti => !string.IsNullOrEmpty(ti.ItemSpec) && ti.ItemSpec.EndsWith(".snk", StringComparison.OrdinalIgnoreCase))
-                        .Any();
-
-                if (!isValidSNPath && strongNameLocally)
-                {
-                    Log.LogError($"An incorrect full path to 'sn.exe' was specified: {SNBinaryPath}");
-                    return;
-                }
             }
             if(WixToolsPath != null && !Directory.Exists(WixToolsPath))
             {
diff --git a/src/Microsoft.DotNet.SignTool/src/ValidationOnlySignTool.cs b/src/Microsoft.DotNet.SignTool/src/ValidationOnlySignTool.cs
index 951e4f3369c..2124d604e1c 100644
--- a/src/Microsoft.DotNet.SignTool/src/ValidationOnlySignTool.cs
+++ b/src/Microsoft.DotNet.SignTool/src/ValidationOnlySignTool.cs
@@ -4,6 +4,8 @@
 using System.IO;
 using Microsoft.Build.Framework;
 using Microsoft.Build.Utilities;
+using System.Collections.Generic;
+using System.Runtime.InteropServices;
 
 namespace Microsoft.DotNet.SignTool
 {
@@ -21,6 +23,28 @@ internal ValidationOnlySignTool(SignToolArgs args, TaskLoggingHelper log)
             TestSign = args.TestSign;
         }
 
+        public override bool LocalStrongNameSign(IBuildEngine buildEngine, int round, IEnumerable<FileSignInfo> files)
+        {
+            // On non-Windows, we skip strong name signing because sn.exe is not available.
+            // We could skip it always in the validation sign tool, but it is useful to
+            // get some level of validation.
+
+            if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
+            {
+                return true;
+            }
+
+            foreach (var file in files)
+            {
+                if (file.SignInfo.ShouldLocallyStrongNameSign)
+                {
+                    return LocalStrongNameSign(file);
+                }
+            }
+
+            return true;
+        }
+
         public override void RemovePublicSign(string assemblyPath)
         {
         }