Key container signing fix (#25426)

This change makes the legacy behavior contingent on the runtime of the
compiler. When the compiler is running on the desktop runtime, the
legacy signing code will be used. When running on CoreCLR, the new
portable signing code will be used. This fixes key container signing for
almost all scenarios (signing using a key container when the compiler is
running on the desktop framework). Signing using a key container on
Windows using a compiler running on CoreCLR is still broken, but that is
a far rarer case.

This change also fixes delaysign, which would full sign the file if the
keyfile passed to the compiler contains a private key.

Fixes #25340
Fixes #25424

src/Compilers/CSharp/Test/CommandLine/CommandLineTests.cs

@@ -5864,7 +5864,7 @@ public void EmittedSubsystemVersion()
             Assert.Equal(1, peHeaders.PEHeader.MinorSubsystemVersion);
         }
 
-        [Fact(Skip = "https://github.com/dotnet/roslyn/pull/23529")]
+        [Fact]
         public void CreateCompilationWithKeyFile()
         {
             string source = @"
@@ -5883,7 +5883,7 @@ public static void Main()
             var cmd = new MockCSharpCompiler(null, dir.Path, new[] { "/nologo", "a.cs", "/keyfile:key.snk", });
             var comp = cmd.CreateCompilation(TextWriter.Null, new TouchedFileLogger(), NullErrorLogger.Instance);
 
-            Assert.Equal(comp.Options.StrongNameProvider.GetType(), typeof(PortableStrongNameProvider));
+            Assert.IsType<DesktopStrongNameProvider>(comp.Options.StrongNameProvider);
         }
 
         [Fact]

src/Compilers/CSharp/Test/Emit/Attributes/InternalsVisibleToAndStrongNameTests.cs

@@ -1145,16 +1145,8 @@ private void ConfirmModuleAttributePresentAndAddingToAssemblyResultsInSignedOutp
             using (var metadata = ModuleMetadata.CreateFromStream(moduleContents))
             {
                 var flags = metadata.Module.PEReaderOpt.PEHeaders.CorHeader.Flags;
-                if (legacyStrongName)
-                {
-                    //confirm file does not claim to be signed
-                    Assert.Equal(0, (int)(flags & CorFlags.StrongNameSigned));
-                }
-                else
-                {
-                    // It should have been signed in the stream
-                    Assert.NotEqual(0, (int)(flags & CorFlags.StrongNameSigned));
-                }
+                //confirm file does not claim to be signed
+                Assert.Equal(0, (int)(flags & CorFlags.StrongNameSigned));
 
                 EntityHandle token = metadata.Module.GetTypeRef(metadata.Module.GetAssemblyRef("mscorlib"), "System.Runtime.CompilerServices", "AssemblyAttributesGoHere");
                 Assert.False(token.IsNil);   //could the type ref be located? If not then the attribute's not there.
@@ -1599,52 +1591,49 @@ public class C
             Assert.True(emitResult.Success);
         }
 
-        private void DelaySignWithAssemblySignatureKeyHelper(StrongNameProvider provider)
+        [Fact]
+        public void DelaySignWithAssemblySignatureKey()
         {
-            //Note that this SignatureKey is some random one that I found in the devdiv build.
-            //It is not related to the other keys we use in these tests.
+            DelaySignWithAssemblySignatureKeyHelper(s_defaultPortableProvider);
+            DelaySignWithAssemblySignatureKeyHelper(s_defaultDesktopProvider);
 
-            //In the native compiler, when the AssemblySignatureKey attribute is present, and
-            //the binary is configured for delay signing, the contents of the assemblySignatureKey attribute
-            //(rather than the contents of the keyfile or container) are used to compute the size needed to
-            //reserve in the binary for its signature. Signing using this key is only supported via sn.exe
+            void DelaySignWithAssemblySignatureKeyHelper(StrongNameProvider provider)
+            {
+                //Note that this SignatureKey is some random one that I found in the devdiv build.
+                //It is not related to the other keys we use in these tests.
 
-            var options = TestOptions.ReleaseDll
-                .WithDelaySign(true)
-                .WithCryptoKeyFile(s_keyPairFile)
-                .WithStrongNameProvider(provider);
+                //In the native compiler, when the AssemblySignatureKey attribute is present, and
+                //the binary is configured for delay signing, the contents of the assemblySignatureKey attribute
+                //(rather than the contents of the keyfile or container) are used to compute the size needed to
+                //reserve in the binary for its signature. Signing using this key is only supported via sn.exe
 
-            var other = CreateCompilation(
-                @"
+                var options = TestOptions.ReleaseDll
+                    .WithDelaySign(true)
+                    .WithCryptoKeyFile(s_keyPairFile)
+                    .WithStrongNameProvider(provider);
+
+                var other = CreateCompilation(
+                    @"
 [assembly: System.Reflection.AssemblyDelaySign(true)]
 [assembly: System.Reflection.AssemblySignatureKey(""002400000c800000140100000602000000240000525341310008000001000100613399aff18ef1a2c2514a273a42d9042b72321f1757102df9ebada69923e2738406c21e5b801552ab8d200a65a235e001ac9adc25f2d811eb09496a4c6a59d4619589c69f5baf0c4179a47311d92555cd006acc8b5959f2bd6e10e360c34537a1d266da8085856583c85d81da7f3ec01ed9564c58d93d713cd0172c8e23a10f0239b80c96b07736f5d8b022542a4e74251a5f432824318b3539a5a087f8e53d2f135f9ca47f3bb2e10aff0af0849504fb7cea3ff192dc8de0edad64c68efde34c56d302ad55fd6e80f302d5efcdeae953658d3452561b5f36c542efdbdd9f888538d374cef106acf7d93a4445c3c73cd911f0571aaf3d54da12b11ddec375b3"", ""a5a866e1ee186f807668209f3b11236ace5e21f117803a3143abb126dd035d7d2f876b6938aaf2ee3414d5420d753621400db44a49c486ce134300a2106adb6bdb433590fef8ad5c43cba82290dc49530effd86523d9483c00f458af46890036b0e2c61d077d7fbac467a506eba29e467a87198b053c749aa2a4d2840c784e6d"")]
 public class C
 {
   static void Goo() {}
 }",
-                new MetadataReference[] { MscorlibRef_v4_0_30316_17626 },
-                options: options);
+                    new MetadataReference[] { MscorlibRef_v4_0_30316_17626 },
+                    options: options);
 
-            using (var metadata = ModuleMetadata.CreateFromImage(other.EmitToArray()))
-            {
-                var header = metadata.Module.PEReaderOpt.PEHeaders.CorHeader;
-                //confirm header has expected SN signature size
-                Assert.Equal(256, header.StrongNameSignatureDirectory.Size);
+                using (var metadata = ModuleMetadata.CreateFromImage(other.EmitToArray()))
+                {
+                    var header = metadata.Module.PEReaderOpt.PEHeaders.CorHeader;
+                    //confirm header has expected SN signature size
+                    Assert.Equal(256, header.StrongNameSignatureDirectory.Size);
+                    // Delay sign should not have the strong name flag set
+                    Assert.Equal(CorFlags.ILOnly, header.Flags);
+                }
             }
         }
 
-        [Fact]
-        public void DelaySignWithAssemblySignatureKey()
-        {
-            DelaySignWithAssemblySignatureKeyHelper(s_defaultPortableProvider);
-        }
-
-        [Fact]
-        public void DelaySignWithAssemblySignatureKey_Legacy()
-        {
-            DelaySignWithAssemblySignatureKeyHelper(s_defaultDesktopProvider);
-        }
-
         [WorkItem(545720, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/545720")]
         [WorkItem(530050, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/530050")]
         [Fact]

src/Compilers/CSharp/csc/csc.csproj

@@ -35,9 +35,6 @@
     <Compile Include="..\..\Shared\CoreClrAnalyzerAssemblyLoader.cs">
       <Link>CoreClrAnalyzerAssemblyLoader.cs</Link>
     </Compile>
-    <Compile Include="..\..\Shared\CoreClrShim.cs">
-      <Link>CoreClrShim.cs</Link>
-    </Compile>
     <Compile Include="..\..\Shared\DesktopBuildClient.cs">
       <Link>DesktopBuildClient.cs</Link>
     </Compile>

src/Compilers/Core/Portable/CodeAnalysis.csproj

@@ -27,6 +27,7 @@
     </Content>
   </ItemGroup>
   <ItemGroup>
+    <Compile Include="..\..\Shared\CoreClrShim.cs" Link="InternalUtilities\CoreClrShim.cs" />
     <Compile Include="..\..\Shared\DesktopShim.cs">
       <Link>DesktopShim.cs</Link>
     </Compile>

src/Compilers/Core/Portable/CommandLine/CommonCommandLineArguments.cs

@@ -286,7 +286,9 @@ public CompilationOptions CompilationOptions
             StrongNameFileSystem fileSystem,
             string tempDirectory)
         {
-            bool fallback = ParseOptionsCore.Features.ContainsKey("UseLegacyStrongNameProvider") ||
+            bool fallback =
+                !CoreClrShim.IsRunningOnCoreClr ||
+                ParseOptionsCore.Features.ContainsKey("UseLegacyStrongNameProvider") ||
                 CompilationOptionsCore.CryptoKeyContainer != null;
             return fallback ?
                 new DesktopStrongNameProvider(KeyFileSearchPaths, tempDirectory, fileSystem) :

src/Compilers/Core/Portable/PEWriter/PeWriter.cs

@@ -201,13 +201,7 @@ internal static class PeWriter
             }
 
             var strongNameProvider = context.Module.CommonCompilation.Options.StrongNameProvider;
-
             var corFlags = properties.CorFlags;
-            if (privateKeyOpt != null)
-            {
-                Debug.Assert(strongNameProvider.Capability == SigningCapability.SignsPeBuilder);
-                corFlags |= CorFlags.StrongNameSigned;
-            }
 
             var peBuilder = new ExtendedPEBuilder(
                 peHeaderBuilder,
@@ -228,8 +222,9 @@ internal static class PeWriter
 
             PatchModuleVersionIds(mvidFixup, mvidSectionFixup, mvidStringFixup, peContentId.Guid);
 
-            if (privateKeyOpt != null)
+            if (privateKeyOpt != null && corFlags.HasFlag(CorFlags.StrongNameSigned))
             {
+                Debug.Assert(strongNameProvider.Capability == SigningCapability.SignsPeBuilder);
                 strongNameProvider.SignPeBuilder(peBuilder, peBlob, privateKeyOpt.Value);
                 FixupChecksum(peBuilder, peBlob);
             }

src/Compilers/Server/VBCSCompiler/VBCSCompiler.csproj

@@ -36,9 +36,6 @@
     <Compile Include="..\..\Shared\CoreClrAnalyzerAssemblyLoader.cs">
       <Link>CoreClrAnalyzerAssemblyLoader.cs</Link>
     </Compile>
-    <Compile Include="..\..\Shared\CoreClrShim.cs">
-      <Link>CoreClrShim.cs</Link>
-    </Compile>
     <Compile Include="..\..\Shared\DesktopAnalyzerAssemblyLoader.cs">
       <Link>DesktopAnalyzerAssemblyLoader.cs</Link>
     </Compile>

src/Compilers/VisualBasic/Test/CommandLine/CommandLineTests.vb

@@ -120,7 +120,7 @@ End Module
             Assert.Equal("", output.ToString().Trim())
         End Sub
 
-        <Fact(Skip:= "https://github.com/dotnet/roslyn/pull/23529")>
+        <Fact>
         Public Sub CreateCompilationWithKeyFile()
             Dim source = "
 Public Class C
@@ -136,7 +136,7 @@ End Class"
             Dim cmd = New MockVisualBasicCompiler(dir.Path, {"/nologo", "a.vb", "/keyfile:key.snk"})
             Dim comp = cmd.CreateCompilation(TextWriter.Null, New TouchedFileLogger(), NullErrorLogger.Instance)
 
-            Assert.True(TypeOf comp.Options.StrongNameProvider Is PortableStrongNameProvider)
+            Assert.IsType(Of DesktopStrongNameProvider)(comp.Options.StrongNameProvider)
         End Sub
 
         <Fact>

src/Compilers/VisualBasic/Test/Emit/Attributes/InternalsVisibleToAndStrongNameTests.vb

@@ -1214,6 +1214,7 @@ End Class
         ' confirm header has expected SN signature size
         Dim peHeaders = New PEHeaders(other.EmitToStream())
         Assert.Equal(256, peHeaders.CorHeader.StrongNameSignatureDirectory.Size)
+        Assert.Equal(CorFlags.ILOnly, peHeaders.CorHeader.Flags)
     End Sub
 
     ''' <summary>
@@ -1260,13 +1261,8 @@ End Class
 
         Using metadata = ModuleMetadata.CreateFromStream(moduleContents)
             Dim flags = metadata.Module.PEReaderOpt.PEHeaders.CorHeader.Flags
-            If legacyStrongName Then
-                ' confirm file does not claim to be signed
-                Assert.Equal(0, CInt(flags And CorFlags.StrongNameSigned))
-            Else
-                ' portable signing should sign the module
-                Assert.Equal(CorFlags.StrongNameSigned, CInt(flags And CorFlags.StrongNameSigned))
-            End If
+            ' confirm file does not claim to be signed
+            Assert.Equal(0, CInt(flags And CorFlags.StrongNameSigned))
 
             Dim token As EntityHandle = metadata.Module.GetTypeRef(metadata.Module.GetAssemblyRef("mscorlib"), "System.Runtime.CompilerServices", "AssemblyAttributesGoHere")
             Assert.False(token.IsNil)   ' could the magic type ref be located? If not then the attribute's not there.

src/Compilers/VisualBasic/vbc/vbc.csproj

@@ -34,9 +34,6 @@
     <Compile Include="..\..\Shared\CoreClrAnalyzerAssemblyLoader.cs">
       <Link>CoreClrAnalyzerAssemblyLoader.cs</Link>
     </Compile>
-    <Compile Include="..\..\Shared\CoreClrShim.cs">
-      <Link>CoreClrShim.cs</Link>
-    </Compile>
     <Compile Include="..\..\Shared\DesktopBuildClient.cs">
       <Link>DesktopBuildClient.cs</Link>
     </Compile>

src/Scripting/Core/Scripting.csproj

@@ -45,9 +45,6 @@
     <Compile Include="..\..\Compilers\Core\Portable\FileSystem\RelativePathResolver.cs">
       <Link>Hosting\Resolvers\RelativePathResolver.cs</Link>
     </Compile>
-    <Compile Include="..\..\Compilers\Shared\CoreClrShim.cs">
-      <Link>CoreClrShim.cs</Link>
-    </Compile>
     <Compile Include="..\..\Compilers\Shared\GlobalAssemblyCacheHelpers\ClrGlobalAssemblyCache.cs">
       <Link>Hosting\Resolvers\ClrGlobalAssemblyCache.cs</Link>
     </Compile>

src/Test/Utilities/Portable/TestUtilities.csproj

@@ -67,9 +67,6 @@
     <InternalsVisibleToTest Include="Roslyn.Services.UnitTests" />
   </ItemGroup>
   <ItemGroup>
-    <Compile Include="..\..\..\Compilers\Shared\CoreClrShim.cs">
-      <Link>Assert\CoreClrShim.cs</Link>
-    </Compile>
     <Compile Include="..\..\..\Compilers\Shared\DesktopAnalyzerAssemblyLoader.cs">
       <Link>DesktopAnalyzerAssemblyLoader.cs</Link>
     </Compile>