Skip to content

Commit e2fc8d5

Browse files
agockeagocke
authored
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
1 parent 59f570e commit e2fc8d5

12 files changed

Lines changed: 44 additions & 76 deletions

File tree

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5864,7 +5864,7 @@ public void EmittedSubsystemVersion()
58645864
Assert.Equal(1, peHeaders.PEHeader.MinorSubsystemVersion);
58655865
}
58665866

5867-
[Fact(Skip = "https://github.com/dotnet/roslyn/pull/23529")]
5867+
[Fact]
58685868
public void CreateCompilationWithKeyFile()
58695869
{
58705870
string source = @"
@@ -5883,7 +5883,7 @@ public static void Main()
58835883
var cmd = new MockCSharpCompiler(null, dir.Path, new[] { "/nologo", "a.cs", "/keyfile:key.snk", });
58845884
var comp = cmd.CreateCompilation(TextWriter.Null, new TouchedFileLogger(), NullErrorLogger.Instance);
58855885

5886-
Assert.Equal(comp.Options.StrongNameProvider.GetType(), typeof(PortableStrongNameProvider));
5886+
Assert.IsType<DesktopStrongNameProvider>(comp.Options.StrongNameProvider);
58875887
}
58885888

58895889
[Fact]

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

Lines changed: 31 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1145,16 +1145,8 @@ private void ConfirmModuleAttributePresentAndAddingToAssemblyResultsInSignedOutp
11451145
using (var metadata = ModuleMetadata.CreateFromStream(moduleContents))
11461146
{
11471147
var flags = metadata.Module.PEReaderOpt.PEHeaders.CorHeader.Flags;
1148-
if (legacyStrongName)
1149-
{
1150-
//confirm file does not claim to be signed
1151-
Assert.Equal(0, (int)(flags & CorFlags.StrongNameSigned));
1152-
}
1153-
else
1154-
{
1155-
// It should have been signed in the stream
1156-
Assert.NotEqual(0, (int)(flags & CorFlags.StrongNameSigned));
1157-
}
1148+
//confirm file does not claim to be signed
1149+
Assert.Equal(0, (int)(flags & CorFlags.StrongNameSigned));
11581150

11591151
EntityHandle token = metadata.Module.GetTypeRef(metadata.Module.GetAssemblyRef("mscorlib"), "System.Runtime.CompilerServices", "AssemblyAttributesGoHere");
11601152
Assert.False(token.IsNil); //could the type ref be located? If not then the attribute's not there.
@@ -1599,52 +1591,49 @@ static void Goo() {}
15991591
Assert.True(emitResult.Success);
16001592
}
16011593

1602-
private void DelaySignWithAssemblySignatureKeyHelper(StrongNameProvider provider)
1594+
[Fact]
1595+
public void DelaySignWithAssemblySignatureKey()
16031596
{
1604-
//Note that this SignatureKey is some random one that I found in the devdiv build.
1605-
//It is not related to the other keys we use in these tests.
1597+
DelaySignWithAssemblySignatureKeyHelper(s_defaultPortableProvider);
1598+
DelaySignWithAssemblySignatureKeyHelper(s_defaultDesktopProvider);
16061599

1607-
//In the native compiler, when the AssemblySignatureKey attribute is present, and
1608-
//the binary is configured for delay signing, the contents of the assemblySignatureKey attribute
1609-
//(rather than the contents of the keyfile or container) are used to compute the size needed to
1610-
//reserve in the binary for its signature. Signing using this key is only supported via sn.exe
1600+
void DelaySignWithAssemblySignatureKeyHelper(StrongNameProvider provider)
1601+
{
1602+
//Note that this SignatureKey is some random one that I found in the devdiv build.
1603+
//It is not related to the other keys we use in these tests.
16111604

1612-
var options = TestOptions.ReleaseDll
1613-
.WithDelaySign(true)
1614-
.WithCryptoKeyFile(s_keyPairFile)
1615-
.WithStrongNameProvider(provider);
1605+
//In the native compiler, when the AssemblySignatureKey attribute is present, and
1606+
//the binary is configured for delay signing, the contents of the assemblySignatureKey attribute
1607+
//(rather than the contents of the keyfile or container) are used to compute the size needed to
1608+
//reserve in the binary for its signature. Signing using this key is only supported via sn.exe
16161609

1617-
var other = CreateCompilation(
1618-
@"
1610+
var options = TestOptions.ReleaseDll
1611+
.WithDelaySign(true)
1612+
.WithCryptoKeyFile(s_keyPairFile)
1613+
.WithStrongNameProvider(provider);
1614+
1615+
var other = CreateCompilation(
1616+
@"
16191617
[assembly: System.Reflection.AssemblyDelaySign(true)]
16201618
[assembly: System.Reflection.AssemblySignatureKey(""002400000c800000140100000602000000240000525341310008000001000100613399aff18ef1a2c2514a273a42d9042b72321f1757102df9ebada69923e2738406c21e5b801552ab8d200a65a235e001ac9adc25f2d811eb09496a4c6a59d4619589c69f5baf0c4179a47311d92555cd006acc8b5959f2bd6e10e360c34537a1d266da8085856583c85d81da7f3ec01ed9564c58d93d713cd0172c8e23a10f0239b80c96b07736f5d8b022542a4e74251a5f432824318b3539a5a087f8e53d2f135f9ca47f3bb2e10aff0af0849504fb7cea3ff192dc8de0edad64c68efde34c56d302ad55fd6e80f302d5efcdeae953658d3452561b5f36c542efdbdd9f888538d374cef106acf7d93a4445c3c73cd911f0571aaf3d54da12b11ddec375b3"", ""a5a866e1ee186f807668209f3b11236ace5e21f117803a3143abb126dd035d7d2f876b6938aaf2ee3414d5420d753621400db44a49c486ce134300a2106adb6bdb433590fef8ad5c43cba82290dc49530effd86523d9483c00f458af46890036b0e2c61d077d7fbac467a506eba29e467a87198b053c749aa2a4d2840c784e6d"")]
16211619
public class C
16221620
{
16231621
static void Goo() {}
16241622
}",
1625-
new MetadataReference[] { MscorlibRef_v4_0_30316_17626 },
1626-
options: options);
1623+
new MetadataReference[] { MscorlibRef_v4_0_30316_17626 },
1624+
options: options);
16271625

1628-
using (var metadata = ModuleMetadata.CreateFromImage(other.EmitToArray()))
1629-
{
1630-
var header = metadata.Module.PEReaderOpt.PEHeaders.CorHeader;
1631-
//confirm header has expected SN signature size
1632-
Assert.Equal(256, header.StrongNameSignatureDirectory.Size);
1626+
using (var metadata = ModuleMetadata.CreateFromImage(other.EmitToArray()))
1627+
{
1628+
var header = metadata.Module.PEReaderOpt.PEHeaders.CorHeader;
1629+
//confirm header has expected SN signature size
1630+
Assert.Equal(256, header.StrongNameSignatureDirectory.Size);
1631+
// Delay sign should not have the strong name flag set
1632+
Assert.Equal(CorFlags.ILOnly, header.Flags);
1633+
}
16331634
}
16341635
}
16351636

1636-
[Fact]
1637-
public void DelaySignWithAssemblySignatureKey()
1638-
{
1639-
DelaySignWithAssemblySignatureKeyHelper(s_defaultPortableProvider);
1640-
}
1641-
1642-
[Fact]
1643-
public void DelaySignWithAssemblySignatureKey_Legacy()
1644-
{
1645-
DelaySignWithAssemblySignatureKeyHelper(s_defaultDesktopProvider);
1646-
}
1647-
16481637
[WorkItem(545720, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/545720")]
16491638
[WorkItem(530050, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/530050")]
16501639
[Fact]

‎src/Compilers/CSharp/csc/csc.csproj‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,6 @@
3535
<Compile Include="..\..\Shared\CoreClrAnalyzerAssemblyLoader.cs">
3636
<Link>CoreClrAnalyzerAssemblyLoader.cs</Link>
3737
</Compile>
38-
<Compile Include="..\..\Shared\CoreClrShim.cs">
39-
<Link>CoreClrShim.cs</Link>
40-
</Compile>
4138
<Compile Include="..\..\Shared\DesktopBuildClient.cs">
4239
<Link>DesktopBuildClient.cs</Link>
4340
</Compile>

‎src/Compilers/Core/Portable/CodeAnalysis.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
</Content>
2828
</ItemGroup>
2929
<ItemGroup>
30+
<Compile Include="..\..\Shared\CoreClrShim.cs" Link="InternalUtilities\CoreClrShim.cs" />
3031
<Compile Include="..\..\Shared\DesktopShim.cs">
3132
<Link>DesktopShim.cs</Link>
3233
</Compile>

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,9 @@ internal StrongNameProvider GetStrongNameProvider(
286286
StrongNameFileSystem fileSystem,
287287
string tempDirectory)
288288
{
289-
bool fallback = ParseOptionsCore.Features.ContainsKey("UseLegacyStrongNameProvider") ||
289+
bool fallback =
290+
!CoreClrShim.IsRunningOnCoreClr ||
291+
ParseOptionsCore.Features.ContainsKey("UseLegacyStrongNameProvider") ||
290292
CompilationOptionsCore.CryptoKeyContainer != null;
291293
return fallback ?
292294
new DesktopStrongNameProvider(KeyFileSearchPaths, tempDirectory, fileSystem) :

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

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -201,13 +201,7 @@ internal static bool WritePeToStream(
201201
}
202202

203203
var strongNameProvider = context.Module.CommonCompilation.Options.StrongNameProvider;
204-
205204
var corFlags = properties.CorFlags;
206-
if (privateKeyOpt != null)
207-
{
208-
Debug.Assert(strongNameProvider.Capability == SigningCapability.SignsPeBuilder);
209-
corFlags |= CorFlags.StrongNameSigned;
210-
}
211205

212206
var peBuilder = new ExtendedPEBuilder(
213207
peHeaderBuilder,
@@ -228,8 +222,9 @@ internal static bool WritePeToStream(
228222

229223
PatchModuleVersionIds(mvidFixup, mvidSectionFixup, mvidStringFixup, peContentId.Guid);
230224

231-
if (privateKeyOpt != null)
225+
if (privateKeyOpt != null && corFlags.HasFlag(CorFlags.StrongNameSigned))
232226
{
227+
Debug.Assert(strongNameProvider.Capability == SigningCapability.SignsPeBuilder);
233228
strongNameProvider.SignPeBuilder(peBuilder, peBlob, privateKeyOpt.Value);
234229
FixupChecksum(peBuilder, peBlob);
235230
}

‎src/Compilers/Server/VBCSCompiler/VBCSCompiler.csproj‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,6 @@
3636
<Compile Include="..\..\Shared\CoreClrAnalyzerAssemblyLoader.cs">
3737
<Link>CoreClrAnalyzerAssemblyLoader.cs</Link>
3838
</Compile>
39-
<Compile Include="..\..\Shared\CoreClrShim.cs">
40-
<Link>CoreClrShim.cs</Link>
41-
</Compile>
4239
<Compile Include="..\..\Shared\DesktopAnalyzerAssemblyLoader.cs">
4340
<Link>DesktopAnalyzerAssemblyLoader.cs</Link>
4441
</Compile>

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ End Module
120120
Assert.Equal("", output.ToString().Trim())
121121
End Sub
122122

123-
<Fact(Skip:= "https://github.com/dotnet/roslyn/pull/23529")>
123+
<Fact>
124124
Public Sub CreateCompilationWithKeyFile()
125125
Dim source = "
126126
Public Class C
@@ -136,7 +136,7 @@ End Class"
136136
Dim cmd = New MockVisualBasicCompiler(dir.Path, {"/nologo", "a.vb", "/keyfile:key.snk"})
137137
Dim comp = cmd.CreateCompilation(TextWriter.Null, New TouchedFileLogger(), NullErrorLogger.Instance)
138138

139-
Assert.True(TypeOf comp.Options.StrongNameProvider Is PortableStrongNameProvider)
139+
Assert.IsType(Of DesktopStrongNameProvider)(comp.Options.StrongNameProvider)
140140
End Sub
141141

142142
<Fact>

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

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1214,6 +1214,7 @@ End Class
12141214
' confirm header has expected SN signature size
12151215
Dim peHeaders = New PEHeaders(other.EmitToStream())
12161216
Assert.Equal(256, peHeaders.CorHeader.StrongNameSignatureDirectory.Size)
1217+
Assert.Equal(CorFlags.ILOnly, peHeaders.CorHeader.Flags)
12171218
End Sub
12181219

12191220
''' <summary>
@@ -1260,13 +1261,8 @@ End Class
12601261

12611262
Using metadata = ModuleMetadata.CreateFromStream(moduleContents)
12621263
Dim flags = metadata.Module.PEReaderOpt.PEHeaders.CorHeader.Flags
1263-
If legacyStrongName Then
1264-
' confirm file does not claim to be signed
1265-
Assert.Equal(0, CInt(flags And CorFlags.StrongNameSigned))
1266-
Else
1267-
' portable signing should sign the module
1268-
Assert.Equal(CorFlags.StrongNameSigned, CInt(flags And CorFlags.StrongNameSigned))
1269-
End If
1264+
' confirm file does not claim to be signed
1265+
Assert.Equal(0, CInt(flags And CorFlags.StrongNameSigned))
12701266

12711267
Dim token As EntityHandle = metadata.Module.GetTypeRef(metadata.Module.GetAssemblyRef("mscorlib"), "System.Runtime.CompilerServices", "AssemblyAttributesGoHere")
12721268
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‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,6 @@
3434
<Compile Include="..\..\Shared\CoreClrAnalyzerAssemblyLoader.cs">
3535
<Link>CoreClrAnalyzerAssemblyLoader.cs</Link>
3636
</Compile>
37-
<Compile Include="..\..\Shared\CoreClrShim.cs">
38-
<Link>CoreClrShim.cs</Link>
39-
</Compile>
4037
<Compile Include="..\..\Shared\DesktopBuildClient.cs">
4138
<Link>DesktopBuildClient.cs</Link>
4239
</Compile>

0 commit comments

Comments
 (0)