Make it possible to opt out of `PublicSign=false` when using some full keys

[omajid]

Arcade forces anyone using the Open and MicrosoftAspNetCore strong name keys to use PublicSign=false.

MicrosoftAspNetcore:

arcade/src/Microsoft.DotNet.Arcade.Sdk/tools/StrongName.targets

Lines 40 to 46 in 179e63f

	<When Condition="'$(StrongNameKeyId)' == 'MicrosoftAspNetCore'">
	<PropertyGroup>
	<AssemblyOriginatorKeyFile>$(MSBuildThisFileDirectory)snk/AspNetCore.snk</AssemblyOriginatorKeyFile>
	<PublicKey>$(MicrosoftAspNetCorePublicKey)</PublicKey>
	<PublicKeyToken>adb9793829ddae60</PublicKeyToken>
	<PublicSign>false</PublicSign> <!-- The MicrosoftAspNetCore strong name key is a full key -->
	</PropertyGroup>

Open:

arcade/src/Microsoft.DotNet.Arcade.Sdk/tools/StrongName.targets

Lines 60 to 67 in 179e63f

	<When Condition="'$(StrongNameKeyId)' == 'Open'">
	<PropertyGroup>
	<AssemblyOriginatorKeyFile>$(MSBuildThisFileDirectory)snk/Open.snk</AssemblyOriginatorKeyFile>
	<PublicKey>$(OpenPublicKey)</PublicKey>
	<PublicKeyToken>cc7b13ffcd2ddd51</PublicKeyToken>
	<DelaySign>false</DelaySign>
	<PublicSign>false</PublicSign> <!-- The Open strong name key is a full key -->
	</PropertyGroup>

That sounds like a good idea, in general. Unfortunately, there are some environments where full signing (which requires SHA1) is not available. One example is CentOS Stream 9: dotnet/runtime#65874. Forcing full signing just cause build to fail with a stack trace that looks like this:

/home/dotnet/source-build-reference-packages/.dotnet/sdk/8.0.100-alpha.1.23061.8/Roslyn/Microsoft.CSharp.Core.targets(80,5): error : Unhandled exception. Interop+Crypto+OpenSslCryptographicException: error:03000098:digital envelope routines::invalid digest [/home/dotnet/source-build-reference-packages/src/referencePackages/src/microsoft.extensions.configuration.abstractions/2.1.1/Microsoft.Extensions.Configuration.Abstractions.2.1.1.csproj::TargetFramework=netstandard2.0]                                                                                                                                                            
/home/dotnet/source-build-reference-packages/.dotnet/sdk/8.0.100-alpha.1.23061.8/Roslyn/Microsoft.CSharp.Core.targets(80,5): error :    at Interop.Crypto.RsaSignHash(SafeEvpPKeyHandle pkey, RSASignaturePaddingMode paddingMode, IntPtr digestAlgorithm, ReadOnlySpan`1 hash, Span`1 destination) [/home/dotnet/source-build-reference-packages/src/referencePackages/src/microsoft.extensions.configuration.abstractions/2.1.1/Microsoft.Extensions.Configuration.Abstractions.2.1.1.csproj::TargetFramework=netstandard2.0]                                                                                                                         
/home/dotnet/source-build-reference-packages/.dotnet/sdk/8.0.100-alpha.1.23061.8/Roslyn/Microsoft.CSharp.Core.targets(80,5): error :    at System.Security.Cryptography.RSAOpenSsl.TrySignHash(ReadOnlySpan`1 hash, Span`1 destination, HashAlgorithmName hashAlgorithm, RSASignaturePadding padding, Boolean allocateSignature, Int32& bytesWritten, Byte[]& signature) [/home/dotnet/source-build-reference-packages/src/referencePackages/src/microsoft.extensions.configuration.abstractions/2.1.1/Microsoft.Extensions.Configuration.Abstractions.2.1.1.csproj::TargetFramework=netstandard2.0]                                                    
/home/dotnet/source-build-reference-packages/.dotnet/sdk/8.0.100-alpha.1.23061.8/Roslyn/Microsoft.CSharp.Core.targets(80,5): error :    at System.Security.Cryptography.RSAOpenSsl.SignHash(Byte[] hash, HashAlgorithmName hashAlgorithm, RSASignaturePadding padding) [/home/dotnet/source-build-reference-packages/src/referencePackages/src/microsoft.extensions.configuration.abstractions/2.1.1/Microsoft.Extensions.Configuration.Abstractions.2.1.1.csproj::TargetFramework=netstandard2.0]                                                                                                                                                      
/home/dotnet/source-build-reference-packages/.dotnet/sdk/8.0.100-alpha.1.23061.8/Roslyn/Microsoft.CSharp.Core.targets(80,5): error :    at Microsoft.CodeAnalysis.SigningUtilities.CalculateRsaSignature(IEnumerable`1 content, RSAParameters privateKey) [/home/dotnet/source-build-reference-packages/src/referencePackages/src/microsoft.extensions.configuration.abstractions/2.1.1/Microsoft.Extensions.Configuration.Abstractions.2.1.1.csproj::TargetFramework=netstandard2.0]     
/home/dotnet/source-build-reference-packages/.dotnet/sdk/8.0.100-alpha.1.23061.8/Roslyn/Microsoft.CSharp.Core.targets(80,5): error :    at Microsoft.CodeAnalysis.DesktopStrongNameProvider.<>c__DisplayClass12_0.<SignBuilder>b__0(IEnumerable`1 content) [/home/dotnet/source-build-reference-packages/src/referencePackages/src/microsoft.extensions.configuration.abstractions/2.1.1/Microsoft.Extensions.Configuration.Abstractions.2.1.1.csproj::TargetFramework=netstandard2.0]    
/home/dotnet/source-build-reference-packages/.dotnet/sdk/8.0.100-alpha.1.23061.8/Roslyn/Microsoft.CSharp.Core.targets(80,5): error :    at System.Reflection.PortableExecutable.PEBuilder.Sign(BlobBuilder peImage, Blob strongNameSignatureFixup, Func`2 signatureProvider) [/home/dotnet/source-build-reference-packages/src/referencePackages/src/microsoft.extensions.configuration.abstractions/2.1.1/Microsoft.Extensions.Configuration.Abstractions.2.1.1.csproj::TargetFramework=netstandard2.0]                                                                                                                                                
/home/dotnet/source-build-reference-packages/.dotnet/sdk/8.0.100-alpha.1.23061.8/Roslyn/Microsoft.CSharp.Core.targets(80,5): error :    at Microsoft.Cci.PeWriter.WritePeToStream(EmitContext context, CommonMessageProvider messageProvider, Func`1 getPeStream, Func`1 getPortablePdbStreamOpt, PdbWriter nativePdbWriterOpt, String pdbPathOpt, Boolean metadataOnly, Boolean isDeterministic, Boolean emitTestCoverageData, Nullable`1 privateKeyOpt, CancellationToken cancellationToken) [/home/dotnet/source-build-reference-packages/src/referencePackages/src/microsoft.extensions.configuration.abstractions/2.1.1/Microsoft.Extensions.Configuration.Abstractions.2.1.1.csproj::TargetFramework=netstandard2.0]                                                                                            
/home/dotnet/source-build-reference-packages/.dotnet/sdk/8.0.100-alpha.1.23061.8/Roslyn/Microsoft.CSharp.Core.targets(80,5): error :    at Microsoft.CodeAnalysis.Compilation.SerializePeToStream(CommonPEModuleBuilder moduleBeingBuilt, DiagnosticBag metadataDiagnostics, CommonMessageProvider messageProvider, Func`1 getPeStream, Func`1 getMetadataPeStreamOpt, Func`1 getPortablePdbStreamOpt, PdbWriter nativePdbWriterOpt, String pdbPathOpt, RebuildData rebuildData, Boolean metadataOnly, Boolean includePrivateMembers, Boolean isDeterministic, Boolean emitTestCoverageData, Nullable`1 privateKeyOpt, CancellationToken cancellationToken) [/home/dotnet/source-build-reference-packages/src/referencePackages/src/microsoft.extensions.configuration.abstractions/2.1.1/Microsoft.Extensions.Configuration.Abstractions.2.1.1.csproj::TargetFramework=netstandard2.0]                                                                                             
/home/dotnet/source-build-reference-packages/.dotnet/sdk/8.0.100-alpha.1.23061.8/Roslyn/Microsoft.CSharp.Core.targets(80,5): error :    at Microsoft.CodeAnalysis.Compilation.SerializeToPeStream(CommonPEModuleBuilder moduleBeingBuilt, EmitStreamProvider peStreamProvider, EmitStreamProvider metadataPEStreamProvider, EmitStreamProvider pdbStreamProvider, RebuildData rebuildData, Func`2 testSymWriterFactory, DiagnosticBag diagnostics, EmitOptions emitOptions, Nullable`1 privateKeyOpt, CancellationToken cancellationToken) [/home/dotnet/source-build-reference-packages/src/referencePackages/src/microsoft.extensions.configuration.abstractions/2.1.1/Microsoft.Extensions.Configuration.Abstractions.2.1.1.csproj::TargetFramework=netstandard2.0]                                                
/home/dotnet/source-build-reference-packages/.dotnet/sdk/8.0.100-alpha.1.23061.8/Roslyn/Microsoft.CSharp.Core.targets(80,5): error :    at Microsoft.CodeAnalysis.CommonCompiler.CompileAndEmit(TouchedFileLoggertouchedFilesLogger, Compilation& compilation, ImmutableArray`1 analyzers, ImmutableArray`1 generators, ImmutableArray`1 additionalTextFiles, AnalyzerConfigSet analyzerConfigSet, ImmutableArray`1 sourceFileAnalyzerConfigOptions, ImmutableArray`1 embeddedTexts, DiagnosticBag diagnostics, ErrorLogger errorLogger, CancellationToken cancellationToken, CancellationTokenSource& analyzerCts, AnalyzerDriver& analyzerDriver, Nullable`1& generatorTimingInfo) [/home/dotnet/source-build-reference-packages/src/referencePackages/src/microsoft.extensions.configuration.abstractions/2.1.1/Microsoft.Extensions.Configuration.Abstractions.2.1.1.csproj::TargetFramework=netstandard2.0]                                                                   
/home/dotnet/source-build-reference-packages/.dotnet/sdk/8.0.100-alpha.1.23061.8/Roslyn/Microsoft.CSharp.Core.targets(80,5): error :    at Microsoft.CodeAnalysis.CommonCompiler.RunCore(TextWriter consoleOutput, ErrorLogger errorLogger, CancellationToken cancellationToken) [/home/dotnet/source-build-reference-packages/src/referencePackages/src/microsoft.extensions.configuration.abstractions/2.1.1/Microsoft.Extensions.Configuration.Abstractions.2.1.1.csproj::TargetFramework=netstandard2.0]   
/home/dotnet/source-build-reference-packages/.dotnet/sdk/8.0.100-alpha.1.23061.8/Roslyn/Microsoft.CSharp.Core.targets(80,5): error :    at Microsoft[576/1833]sis.CommonCompiler.Run(TextWriter consoleOutput, CancellationToken cancellationToken) [/home/dotnet/source-build-reference-packages/src/referencePackages/src/microsoft.extensions.configuration.abstractions/2.1.1/Microsoft.Extensions.Configuration.Abstractions.2.1.1.csproj::TargetFramework=netstandard2.0]
/home/dotnet/source-build-reference-packages/.dotnet/sdk/8.0.100-alpha.1.23061.8/Roslyn/Microsoft.CSharp.Core.targets(80,5): error :    at Microsoft.CodeAnalysis.CSharp.CommandLine.Csc.<>c__DisplayClass1_0.<Run>b__0(TextWriter tw) [/home/dotnet/source-build-reference-packages/src/referencePackages/src/microsoft.extensions.configuration.abstractions/2.1.1/Microsoft.Extensions.Configuration.Abstractions.2.1.1.csproj::TargetFramework=netstandard2.0]
/home/dotnet/source-build-reference-packages/.dotnet/sdk/8.0.100-alpha.1.23061.8/Roslyn/Microsoft.CSharp.Core.targets(80,5): error :    at Microsoft.CodeAnalysis.CSharp.CommandLine.Csc.Run(String[] args, BuildPaths buildPaths, TextWriter textWriter, IAnalyzerAssemblyLoaderanalyzerLoader) [/home/dotnet/source-build-reference-packages/src/referencePackages/src/microsoft.extensions.configuration.abstractions/2.1.1/Microsoft.Extensions.Configuration.Abstractions.2.1.1.csproj::TargetFramework=netstandard2.0]
/home/dotnet/source-build-reference-packages/.dotnet/sdk/8.0.100-alpha.1.23061.8/Roslyn/Microsoft.CSharp.Core.targets(80,5): error :    at Microsoft.CodeAnalysis.CommandLine.BuildClient.RunCompilation(IEnumerable`1 originalArguments, BuildPaths buildPaths, TextWriter textWriter, String pipeName) [/home/dotnet/source-build-reference-packages/src/referencePackages/src/microsoft.extensions.configuration.abstractions/2.1.1/Microsoft.Extensions.Configuration.Abstractions.2.1.1.csproj::TargetFramework=netstandard2.0]
/home/dotnet/source-build-reference-packages/.dotnet/sdk/8.0.100-alpha.1.23061.8/Roslyn/Microsoft.CSharp.Core.targets(80,5): error :    at Microsoft.CodeAnalysis.CommandLine.BuildClient.Run(IEnumerable`1 arguments, RequestLanguage language, CompileFunc compileFunc, CompileOnServerFunc compileOnServerFunc) [/home/dotnet/source-build-reference-packages/src/referencePackages/src/microsoft.extensions.configuration.abstractions/2.1.1/Microsoft.Extensions.Configuration.Abstractions.2.1.1.csproj::TargetFramework=netstandard2.0]
/home/dotnet/source-build-reference-packages/.dotnet/sdk/8.0.100-alpha.1.23061.8/Roslyn/Microsoft.CSharp.Core.targets(80,5): error :    at Microsoft.CodeAnalysis.CSharp.CommandLine.Program.MainCore(String[] args) [/home/dotnet/source-build-reference-packages/src/referencePackages/src/microsoft.extensions.configuration.abstractions/2.1.1/Microsoft.Extensions.Configuration.Abstractions.2.1.1.csproj::TargetFramework=netstandard2.0]
/home/dotnet/source-build-reference-packages/.dotnet/sdk/8.0.100-alpha.1.23061.8/Roslyn/Microsoft.CSharp.Core.targets(80,5): error :    at Microsoft.CodeAnalysis.CSharp.CommandLine.Program.Main(String[] args) [/home/dotnet/source-build-reference-packages/src/referencePackages/src/microsoft.extensions.configuration.abstractions/2.1.1/Microsoft.Extensions.Configuration.Abstractions.2.1.1.csproj::TargetFramework=netstandard2.0]

I think we should allow making this pluggable and support using PublicSign=true even for the Open key.

What would be a good idea to do this? Would this be acceptable?

<PublicSign Condition="$(PublicSign) = ''">false</PublicSign>

If so, I can do a PR for this.

See dotnet/source-build#2907

cc @crummel @MichaelSimons @mthalman

[michellemcdaniel]

/cc @mmitche

[mmitche]

This change sounds fine to me. It will need to be a bit different though. https://github.com/dotnet/arcade/blob/main/src/Microsoft.DotNet.Arcade.Sdk/tools/StrongName.targets#L20 sets the PublicSIgn variable already, so only setting to false if unset won't work.

Perhaps we introduce a new variable which indicates the level of available signing, and then PublicSign is conditionally set by that?

[omajid]

That sounds sensible. What component would then become responsible for setting that variable? Would it be up to each repo that's using arcade?

So something like this?

diff --git a/src/Microsoft.DotNet.Arcade.Sdk/tools/StrongName.targets b/src/Microsoft.DotNet.Arcade.Sdk/tools/StrongName.targets
index bc69424a..9b09eb78 100644
--- a/src/Microsoft.DotNet.Arcade.Sdk/tools/StrongName.targets
+++ b/src/Microsoft.DotNet.Arcade.Sdk/tools/StrongName.targets
@@ -5,6 +5,7 @@
   <!--
     Reads variables:
       SignAssembly    "true" to sign the output assembly of the current project
+      SignAssemblyLevel    "Public" to use public signing even when full signing is possible
       StrongNameKeyId The id of the key used for strong name generation
 
     Writes variables:
@@ -42,7 +43,7 @@
         <AssemblyOriginatorKeyFile>$(MSBuildThisFileDirectory)snk/AspNetCore.snk</AssemblyOriginatorKeyFile>
         <PublicKey>$(MicrosoftAspNetCorePublicKey)</PublicKey>
         <PublicKeyToken>adb9793829ddae60</PublicKeyToken>
-        <PublicSign>false</PublicSign> <!-- The MicrosoftAspNetCore strong name key is a full key -->
+        <PublicSign Condition="'$(SignAssemblyLevel)' != 'Public'">false</PublicSign> <!-- The MicrosoftAspNetCore strong name key is a full key -->
       </PropertyGroup>
     </When>
     <When Condition="'$(StrongNameKeyId)' == 'ECMA'">
@@ -63,7 +64,7 @@
         <PublicKey>$(OpenPublicKey)</PublicKey>
         <PublicKeyToken>cc7b13ffcd2ddd51</PublicKeyToken>
         <DelaySign>false</DelaySign>
-        <PublicSign>false</PublicSign> <!-- The Open strong name key is a full key -->
+        <PublicSign Condition="'$(SignAssembly)' != 'Public'">false</PublicSign> <!-- The Open strong name key is a full key -->
       </PropertyGroup>
     </When>
     <When Condition="'$(StrongNameKeyId)' == 'SilverlightPlatform'">

Any recommendations on a name?

[mmitche]

I think the source build scripts would set it on a per-platform basis. Maybe msbuild could detect it? Not sure.

Anyways, I think we go with a simpler variable FullAssemblySigningSupported. Defaults to true.