Replace strong name verification and signing with custom implementation

Directory.Packages.props

@@ -97,6 +97,7 @@
     <PackageVersion Include="Newtonsoft.Json" Version="$(NewtonsoftJsonVersion)" />
     <PackageVersion Include="Octokit" Version="12.0.0" />
     <PackageVersion Include="Polly.Core" Version="8.4.1" />
+    <PackageVersion Include="sn" Version="1.0.0" />
     <PackageVersion Include="xunit" Version="$(XUnitVersion)" />
     <PackageVersion Include="xunit.abstractions" Version="2.0.3" />
     <PackageVersion Include="xunit.extensibility.core" Version="$(XUnitVersion)" />

src/Microsoft.DotNet.SignTool.Tests/FakeSignTool.cs

@@ -20,7 +20,7 @@ internal FakeSignTool(SignToolArgs args, TaskLoggingHelper log)
         {
         }
 
-        public override void RemovePublicSign(string assemblyPath)
+        public override void RemoveStrongNameSign(string assemblyPath)
         {
         }
 

src/Microsoft.DotNet.SignTool.Tests/Microsoft.DotNet.SignTool.Tests.csproj

@@ -8,6 +8,7 @@
     <PackageReference Include="FluentAssertions" />
     <PackageReference Include="Microsoft.Build.Utilities.Core" />
     <PackageReference Include="Microsoft.Build.Framework" />
+    <PackageReference Include="sn" GeneratePathProperty="true" />
   </ItemGroup>
 
   <ItemGroup>
@@ -47,4 +48,13 @@
     </ItemGroup>
   </Target>
 
+  <Target Name="_CopySnTool" AfterTargets="ResolveProjectReferences">
+    <ItemGroup>
+      <_SnToolFiles Include="$(Pkgsn)\sn.exe.*"/>
+      <_SnToolFiles Include="$(Pkgsn)\1033\*"/>
+    </ItemGroup>
+    <ItemGroup>
+      <Content Include="@(_SnToolFiles)" CopyToOutputDirectory="PreserveNewest" Link="tools\sn\%(RecursiveDir)%(Filename)%(Extension)" />
+    </ItemGroup>
+  </Target>
 </Project>

src/Microsoft.DotNet.SignTool/Microsoft.DotNet.SignTool.csproj

@@ -8,7 +8,7 @@
     <Description>Build artifact signing tool</Description>
     <PackageTags>Arcade Build Tool Signing</PackageTags>
     <DevelopmentDependency>false</DevelopmentDependency>
-    <NoWarn>$(NoWarn);NU5128</NoWarn>
+    <NoWarn>$(NoWarn);NU5128;CA1416</NoWarn>
     <!-- Copy assemblies into lib folder so they can be used as a development dependency -->
     <BuildTaskTargetFolder>lib</BuildTaskTargetFolder>
   </PropertyGroup>

src/Microsoft.DotNet.SignTool/src/BatchSignUtil.cs

@@ -60,8 +60,10 @@ internal void Go(bool doStrongNameCheck)
                 return;
             }
 
-            // Next remove public signing from all of the assemblies; it can interfere with the signing process.
-            RemovePublicSign();
+            // Remove strong name signing, as sn.exe would choke on already signed binaries.
+            // Our new signing infra is more resilient, but sn.exe may be used as a backup
+            // or when not signing locally.
+            RemoveStrongNameSigning();
 
             // Next sign all of the files
             if (!SignFiles())
@@ -96,14 +98,14 @@ internal void Go(bool doStrongNameCheck)
             _log.LogMessage(MessageImportance.High, "Build artifacts signed and validated.");
         }
 
-        private void RemovePublicSign()
+        private void RemoveStrongNameSigning()
         {
             foreach (var fileSignInfo in _batchData.FilesToSign.Where(x => x.IsPEFile()))
             {
-                if (fileSignInfo.SignInfo.StrongName != null && fileSignInfo.SignInfo.ShouldSign)
+                if (fileSignInfo.SignInfo.ShouldStrongName)
                 {
-                    _log.LogMessage($"Removing public sign: '{fileSignInfo.FileName}'");
-                    _signTool.RemovePublicSign(fileSignInfo.FullPath);
+                    _log.LogMessage($"Removing strong name signing from: '{fileSignInfo.FileName}'");
+                    _signTool.RemoveStrongNameSign(fileSignInfo.FullPath);
                 }
             }
         }

src/Microsoft.DotNet.SignTool/src/Configuration.cs

@@ -12,6 +12,7 @@
 using System.Linq;
 using System.Reflection.Metadata;
 using System.Reflection.PortableExecutable;
+using System.Runtime.ConstrainedExecution;
 using System.Runtime.Versioning;
 
 namespace Microsoft.DotNet.SignTool
@@ -281,7 +282,8 @@ private FileSignInfo ExtractSignInfo(
             var extension = Path.GetExtension(file.FileName);
             string explicitCertificateName = null;
             var fileSpec = string.Empty;
-            var isAlreadySigned = false;
+            var isAlreadyAuthenticodeSigned = false;
+            var isAlreadyStrongNamed = false;
             var matchedNameTokenFramework = false;
             var matchedNameToken = false;
             var matchedName = false;
@@ -356,9 +358,33 @@ private FileSignInfo ExtractSignInfo(
 
             if (FileSignInfo.IsPEFile(file.FullPath))
             {
-                using (var stream = File.OpenRead(file.FullPath))
+                isAlreadyAuthenticodeSigned = ContentUtil.IsAuthenticodeSigned(file.FullPath);
+
+/* Unmerged change from project 'Microsoft.DotNet.SignTool (net472)'
+Before:
+                isAlreadyStrongNamed = ContentUtil.IsStrongNameSigned(file.FullPath);
+After:
+                isAlreadyStrongNamed = DotNet.SignTool.StrongName.IsStrongNameSigned(file.FullPath);
+*/
+                isAlreadyStrongNamed = StrongName.IsSigned(file.FullPath);
+
+
+                if (!isAlreadyAuthenticodeSigned)
+                {
+                    _log.LogMessage(MessageImportance.Low, $"PE file {file.FullPath} does not have a signature marker.");
+                }
+                else
+                {
+                    _log.LogMessage(MessageImportance.Low, $"PE file {file.FullPath} has a signature marker.");
+                }
+
+                if (!isAlreadyStrongNamed)
+                {
+                    _log.LogMessage(MessageImportance.Low, $"PE file {file.FullPath} does not have a valid strong name signature.");
+                }
+                else
                 {
-                    isAlreadySigned = ContentUtil.IsAuthenticodeSigned(stream);
+                    _log.LogMessage(MessageImportance.Low, $"PE file {file.FullPath} has a valid strong name signature.");
                 }
 
                 peInfo = GetPEInfo(file.FullPath);
@@ -379,13 +405,17 @@ private FileSignInfo ExtractSignInfo(
                         pktBasedSignInfo = pktBasedSignInfos.FirstOrDefault();
                     }
 
+                    // If crossgenned, we cannot strong name sign this binary.
+                    // If the file is already strong name signed, then there is no need to re-sign it. The current signing infra
+                    // does not support replacing the SN sig with a new one so if we got here, we can avoid strong name signing
+                    // altogether.
                     if (peInfo.IsCrossgened)
                     {
                         signInfo = new SignInfo(pktBasedSignInfo.Certificate, collisionPriorityId: _hashToCollisionIdMap[signedFileContentKey]);
                     }
                     else
                     {
-                        signInfo = pktBasedSignInfo;
+                        signInfo = pktBasedSignInfo.WithIsAlreadyStrongNamed(isAlreadyStrongNamed);
                     }
 
                     hasSignInfo = true;
@@ -404,8 +434,8 @@ private FileSignInfo ExtractSignInfo(
             }
             else if (FileSignInfo.IsPackage(file.FullPath))
             {
-                isAlreadySigned = VerifySignatures.IsSignedContainer(file.FullPath, _pathToContainerUnpackingDirectory, _tarToolPath);
-                if(!isAlreadySigned)
+                isAlreadyAuthenticodeSigned = VerifySignatures.IsSignedContainer(file.FullPath, _pathToContainerUnpackingDirectory, _tarToolPath);
+                if(!isAlreadyAuthenticodeSigned)
                 {
                     _log.LogMessage(MessageImportance.Low, $"Container {file.FullPath} does not have a signature marker.");
                 }
@@ -416,8 +446,8 @@ private FileSignInfo ExtractSignInfo(
             }
             else if (FileSignInfo.IsWix(file.FullPath))
             {
-                isAlreadySigned = VerifySignatures.IsDigitallySigned(file.FullPath);
-                if (!isAlreadySigned)
+                isAlreadyAuthenticodeSigned = VerifySignatures.IsDigitallySigned(file.FullPath);
+                if (!isAlreadyAuthenticodeSigned)
                 {
                     _log.LogMessage(MessageImportance.Low, $"File {file.FullPath} is not digitally signed.");
                 }
@@ -428,8 +458,8 @@ private FileSignInfo ExtractSignInfo(
             }
             else if(FileSignInfo.IsDeb(file.FullPath))
             {
-                isAlreadySigned = VerifySignatures.VerifySignedDeb(_log, file.FullPath);
-                if (!isAlreadySigned)
+                isAlreadyAuthenticodeSigned = VerifySignatures.VerifySignedDeb(_log, file.FullPath);
+                if (!isAlreadyAuthenticodeSigned)
                 {
                     _log.LogMessage(MessageImportance.Low, $"File {file.FullPath} is not signed.");
                 }
@@ -440,8 +470,8 @@ private FileSignInfo ExtractSignInfo(
             }
             else if(FileSignInfo.IsPowerShellScript(file.FullPath))
             {
-                isAlreadySigned = VerifySignatures.VerifySignedPowerShellFile(file.FullPath);
-                if (!isAlreadySigned)
+                isAlreadyAuthenticodeSigned = VerifySignatures.VerifySignedPowerShellFile(file.FullPath);
+                if (!isAlreadyAuthenticodeSigned)
                 {
                     _log.LogMessage(MessageImportance.Low, $"File {file.FullPath} does not have a signature block.");
                 }
@@ -479,9 +509,9 @@ private FileSignInfo ExtractSignInfo(
                         (d.GetMetadata(SignToolConstants.CollisionPriorityId) == "" ||
                         d.GetMetadata(SignToolConstants.CollisionPriorityId) == _hashToCollisionIdMap[signedFileContentKey])).Any();
 
-                if (isAlreadySigned && !dualCerts)
+                if (isAlreadyAuthenticodeSigned && !dualCerts)
                 {
-                    return new FileSignInfo(file, signInfo.WithIsAlreadySigned(isAlreadySigned), wixContentFilePath: wixContentFilePath);
+                    return new FileSignInfo(file, signInfo.WithIsAlreadySigned(isAlreadyAuthenticodeSigned), wixContentFilePath: wixContentFilePath);
                 }
 
                 if (signInfo.ShouldSign && peInfo != null)

src/Microsoft.DotNet.SignTool/src/ContentUtil.cs

@@ -11,6 +11,9 @@
 using System.Collections.Immutable;
 using System.Linq;
 using System.Reflection;
+using System.Diagnostics;
+using Microsoft.Build.Framework;
+using Microsoft.Build.Utilities;
 
 namespace Microsoft.DotNet.SignTool
 {
@@ -116,6 +119,14 @@ public static bool IsAuthenticodeSigned(Stream assemblyStream)
             }
         }
 
+        public static bool IsAuthenticodeSigned(string filePath)
+        {
+            using (var stream = new FileStream(filePath, FileMode.Open))
+            {
+                return IsAuthenticodeSigned(stream);
+            }
+        }
+
         public static string GetPublicKeyToken(string fullPath)
         {
             try

src/Microsoft.DotNet.SignTool/src/FileSignInfo.cs

@@ -125,8 +125,8 @@ internal FileSignInfo(PathWithHash pathWithHash, SignInfo signInfo, string targe
         public override string ToString()
             => $"File '{FileName}'" +
                (TargetFramework != null ? $" TargetFramework='{TargetFramework}'" : "") +
-               $" Certificate='{SignInfo.Certificate}'" +
-               (SignInfo.StrongName != null ? $" StrongName='{SignInfo.StrongName}'" : "");
+               (SignInfo.ShouldSign ? $" Certificate='{SignInfo.Certificate}'" : "") +
+               (SignInfo.ShouldStrongName ? $" StrongName='{SignInfo.StrongName}'" : "");
 
         internal FileSignInfo WithSignableParts()
             => new FileSignInfo(File, SignInfo.WithIsAlreadySigned(false), TargetFramework, WixContentFilePath, true);

src/Microsoft.DotNet.SignTool/src/RealSignTool.cs

@@ -79,20 +79,9 @@ public override bool RunMSBuild(IBuildEngine buildEngine, string projectFilePath
             return true;
         }
 
-        public override void RemovePublicSign(string assemblyPath)
+        public override void RemoveStrongNameSign(string assemblyPath)
         {
-            using (var stream = new FileStream(assemblyPath, FileMode.Open, FileAccess.ReadWrite, FileShare.Read))
-            using (var peReader = new PEReader(stream))
-            using (var writer = new BinaryWriter(stream))
-            {
-                if (!ContentUtil.IsPublicSigned(peReader))
-                {
-                    return;
-                }
-
-                stream.Position = peReader.PEHeaders.CorHeaderStartOffset + OffsetFromStartOfCorHeaderToFlags;
-                writer.Write((UInt32)(peReader.PEHeaders.CorHeader.Flags & ~CorFlags.StrongNameSigned));
-            }
+            StrongName.ClearStrongNameSignedBit(assemblyPath);
         }
 
         public override bool VerifySignedPEFile(Stream assemblyStream)
@@ -105,7 +94,6 @@ public override bool VerifySignedPEFile(Stream assemblyStream)
 
             return ContentUtil.IsAuthenticodeSigned(assemblyStream);
         }
-
         public override bool VerifyStrongNameSign(string fileFullPath)
         {
             // The assembly won't verify by design when doing test signing.
@@ -114,19 +102,7 @@ public override bool VerifyStrongNameSign(string fileFullPath)
                 return true;
             }
 
-            var process = Process.Start(new ProcessStartInfo()
-            {
-                FileName = _snPath,
-                Arguments = $@"-vf ""{fileFullPath}"" > nul",
-                UseShellExecute = false,
-                CreateNoWindow = true,
-                RedirectStandardError = false,
-                RedirectStandardOutput = false
-            });
-
-            process.WaitForExit();
-
-            return process.ExitCode == 0;
+            return StrongName.IsSigned(fileFullPath);
         }
 
         public override bool VerifySignedDeb(TaskLoggingHelper log, string filePath)

src/Microsoft.DotNet.SignTool/src/SignInfo.cs

@@ -11,12 +11,12 @@ internal readonly struct SignInfo
         /// <summary>
         /// Used to flag that the signing for the file is not necessary.
         /// </summary>
-        public static readonly SignInfo Ignore = new SignInfo(ignoreThisFile: true, alreadySigned: false);
+        public static readonly SignInfo Ignore = new SignInfo(ignoreThisFile: true, alreadySigned: false, isAlreadyStrongNamed: false);
 
         /// <summary>
         /// Used to flag that the file is already signed.
         /// </summary>
-        public static readonly SignInfo AlreadySigned = new SignInfo(ignoreThisFile: false, alreadySigned: true);
+        public static readonly SignInfo AlreadySigned = new SignInfo(ignoreThisFile: false, alreadySigned: true, isAlreadyStrongNamed: false);
 
         /// <summary>
         /// The authenticode certificate which should be used to sign the binary. This can be null
@@ -32,8 +32,9 @@ internal readonly struct SignInfo
 
         internal bool ShouldIgnore { get; }
 
-        internal bool IsAlreadySigned { get; }
+        internal bool IsAlreadyStrongNamed { get; }
 
+        internal bool IsAlreadySigned { get; }
 
         /// <summary>
         /// This is used to decide what SignInfos to use in the case of a collision. In case of a collision
@@ -42,38 +43,44 @@ internal readonly struct SignInfo
         /// </summary>
         internal string CollisionPriorityId { get; }
 
-        public bool ShouldLocallyStrongNameSign => !string.IsNullOrEmpty(StrongName) && StrongName.EndsWith(".snk", StringComparison.OrdinalIgnoreCase);
+        public bool ShouldLocallyStrongNameSign => ShouldStrongName && StrongName.EndsWith(".snk", StringComparison.OrdinalIgnoreCase);
 
         public bool ShouldSign => !IsAlreadySigned && !ShouldIgnore;
 
-        public SignInfo(string certificate, string strongName, string collisionPriorityId, bool shouldIgnore, bool isAlreadySigned)
+        public bool ShouldStrongName => !IsAlreadyStrongNamed && !string.IsNullOrEmpty(StrongName);
+
+        public SignInfo(string certificate, string strongName, string collisionPriorityId, bool shouldIgnore, bool isAlreadySigned, bool isAlreadyStrongNamed)
         {
             ShouldIgnore = shouldIgnore;
             IsAlreadySigned = isAlreadySigned;
             Certificate = certificate;
             StrongName = strongName;
             CollisionPriorityId = collisionPriorityId;
+            IsAlreadyStrongNamed = isAlreadyStrongNamed;
         }
 
-        private SignInfo(bool ignoreThisFile, bool alreadySigned) 
-            : this(certificate: null, strongName: null, collisionPriorityId: null, ignoreThisFile, alreadySigned)
+        private SignInfo(bool ignoreThisFile, bool alreadySigned, bool isAlreadyStrongNamed)
+            : this(certificate: null, strongName: null, collisionPriorityId: null, ignoreThisFile, alreadySigned, isAlreadyStrongNamed)
         {
         }
 
         internal SignInfo(string certificate, string strongName = null, string collisionPriorityId = null)
-            : this(certificate, strongName, collisionPriorityId, shouldIgnore: false, isAlreadySigned: false)
+            : this(certificate, strongName, collisionPriorityId, shouldIgnore: false, isAlreadySigned: false, isAlreadyStrongNamed: false)
         {
         }
 
         internal SignInfo WithCertificateName(string value, string collisionPriorityId)
-            => new SignInfo(value, StrongName, collisionPriorityId, ShouldIgnore, IsAlreadySigned);
+            => new SignInfo(value, StrongName, collisionPriorityId, ShouldIgnore, IsAlreadySigned, IsAlreadyStrongNamed);
 
         internal SignInfo WithCollisionPriorityId(string collisionPriorityId)
-            => new SignInfo(Certificate, StrongName, collisionPriorityId, ShouldIgnore, IsAlreadySigned);
+            => new SignInfo(Certificate, StrongName, collisionPriorityId, ShouldIgnore, IsAlreadySigned, IsAlreadyStrongNamed);
 
         internal SignInfo WithIsAlreadySigned(bool value = false)
             => Certificate != null ? 
-              new SignInfo(Certificate, StrongName, CollisionPriorityId, value, value) :
-              new SignInfo(Certificate, StrongName, CollisionPriorityId, true, value);
+              new SignInfo(Certificate, StrongName, CollisionPriorityId, value, value, IsAlreadyStrongNamed) :
+              new SignInfo(Certificate, StrongName, CollisionPriorityId, true, value, IsAlreadyStrongNamed);
+
+        internal SignInfo WithIsAlreadyStrongNamed(bool value = false) =>
+              new SignInfo(Certificate, StrongName, CollisionPriorityId, ShouldIgnore, IsAlreadySigned, value);
     }
 }