OpenSslCryptographicException: error:03000098:digital envelope routines::invalid digest on CentOS Stream 9

omajid commented 2 years ago

Description

While building .NET 6 using .NET 6 in in CentOS Stream 9 (aka RHEL 9 in-development), I am running into error : Unhandled exception. Interop+Crypto+OpenSslCryptographicException: error:03000098:digital envelope routines::invalid digest

error : Unhandled exception. Interop+Crypto+OpenSslCryptographicException: error:03000098:digital envelope routines::invalid digest 
error :    at Interop.Crypto.RsaSignHash(SafeEvpPKeyHandle pkey, RSASignaturePaddingMode paddingMode, IntPtr digestAlgorithm, ReadOnlySpan`1 hash, Span`1 destination) 
error :    at System.Security.Cryptography.RSAImplementation.RSAOpenSsl.TrySignHash(ReadOnlySpan`1 hash, Span`1 destination, HashAlgorithmName hashAlgorithm, RSASignaturePadding padding, Boolean allocateSignature, Int32& bytesWritten, Byte[]& signature) 
error :    at System.Security.Cryptography.RSAImplementation.RSAOpenSsl.SignHash(Byte[] hash, HashAlgorithmName hashAlgorithm, RSASignaturePadding padding) 
error :    at Microsoft.CodeAnalysis.SigningUtilities.CalculateRsaSignature(IEnumerable`1 content, RSAParameters privateKey) 
error :    at Microsoft.CodeAnalysis.DesktopStrongNameProvider.<>c__DisplayClass12_0.<SignBuilder>b__0(IEnumerable`1 content) 
error :    at System.Reflection.PortableExecutable.PEBuilder.Sign(BlobBuilder peImage, Blob strongNameSignatureFixup, Func`2 signatureProvider) 
error :    at System.Reflection.PortableExecutable.ManagedPEBuilder.Sign(BlobBuilder peImage, Func`2 signatureProvider) 
error :    at Microsoft.CodeAnalysis.DesktopStrongNameProvider.SignBuilder(ExtendedPEBuilder peBuilder, BlobBuilder peBlob, RSAParameters privateKey) 
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) 
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) 
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) 
error :    at Microsoft.CodeAnalysis.CommonCompiler.CompileAndEmit(TouchedFileLogger touchedFilesLogger, Compilation& compilation, ImmutableArray`1 analyzers, ImmutableArray`1 generators, ImmutableArray`1 additionalTextFiles, AnalyzerConfigSet analyzerConfigSet, ImmutableArray`1 sourceFileAnalyzerConfigOptions, ImmutableArray`1 embeddedTexts, DiagnosticBag diagnostics, CancellationToken cancellationToken, CancellationTokenSource& analyzerCts, Boolean& reportAnalyzer, AnalyzerDriver& analyzerDriver) 
error :    at Microsoft.CodeAnalysis.CommonCompiler.RunCore(TextWriter consoleOutput, ErrorLogger errorLogger, CancellationToken cancellationToken) 
error :    at Microsoft.CodeAnalysis.CommonCompiler.Run(TextWriter consoleOutput, CancellationToken cancellationToken) 
error :    at Microsoft.CodeAnalysis.CSharp.CommandLine.Csc.<>c__DisplayClass1_0.<Run>b__0(TextWriter tw) 
error :    at Microsoft.CodeAnalysis.CommandLine.ConsoleUtil.RunWithUtf8Output[T](Boolean utf8Output, TextWriter textWriter, Func`2 func) 
error :    at Microsoft.CodeAnalysis.CSharp.CommandLine.Csc.Run(String[] args, BuildPaths buildPaths, TextWriter textWriter, IAnalyzerAssemblyLoader analyzerLoader) 
error :    at Microsoft.CodeAnalysis.CommandLine.BuildClient.RunCompilation(IEnumerable`1 originalArguments, BuildPaths buildPaths, TextWriter textWriter, String pipeName) 
error :    at Microsoft.CodeAnalysis.CommandLine.BuildClient.Run(IEnumerable`1 arguments, RequestLanguage language, CompileFunc compileFunc, CompileOnServerFunc compileOnServerFunc) 
error :    at Microsoft.CodeAnalysis.CSharp.CommandLine.Program.MainCore(String[] args)

Full log is here: https://centos.softwarefactory-project.io/zuul/t/centos/build/b00c0fe1895c4e1487350108a41214da

Could this be caused by Disable SHA1 signature creation and verification by default?

Reproduction Steps

It seems like building runtime in a CentOS Stream 9 container should be enough to trigger the bug. Will test and update this step later.

Expected behavior

I can build .NET itself.

Actual behavior

I can't build .NET, because the compiler needs signing (via OpenSSL) to work.

Regression?

It's a regression somewhere. Most likely it's not .NET itself that's to blame, because this same source code (no changes) built a few days ago.

Known Workarounds

I am testing if an export OPENSSL_ENABLE_SHA1_SIGNATURES=1 will work around the issue.

Edit: Confirmed. It does make the build move past this particular error.

Configuration

.NET 6 source-build using the 6.0.102 tag of dotnet/installer
CentOS Stream 9 on x86_64

Other information

No response

ghost commented 2 years ago

Tagging subscribers to this area: @dotnet/area-system-security, @vcsjones See info in area-owners.md if you want to be subscribed.

Issue Details

### Description While building .NET 6 using .NET 6 in in CentOS Stream 9 (aka RHEL 9 in-development), I am running into `error : Unhandled exception. Interop+Crypto+OpenSslCryptographicException: error:03000098:digital envelope routines::invalid digest` ``` error : Unhandled exception. Interop+Crypto+OpenSslCryptographicException: error:03000098:digital envelope routines::invalid digest error : at Interop.Crypto.RsaSignHash(SafeEvpPKeyHandle pkey, RSASignaturePaddingMode paddingMode, IntPtr digestAlgorithm, ReadOnlySpan`1 hash, Span`1 destination) error : at System.Security.Cryptography.RSAImplementation.RSAOpenSsl.TrySignHash(ReadOnlySpan`1 hash, Span`1 destination, HashAlgorithmName hashAlgorithm, RSASignaturePadding padding, Boolean allocateSignature, Int32& bytesWritten, Byte[]& signature) error : at System.Security.Cryptography.RSAImplementation.RSAOpenSsl.SignHash(Byte[] hash, HashAlgorithmName hashAlgorithm, RSASignaturePadding padding) error : at Microsoft.CodeAnalysis.SigningUtilities.CalculateRsaSignature(IEnumerable`1 content, RSAParameters privateKey) error : at Microsoft.CodeAnalysis.DesktopStrongNameProvider.<>c__DisplayClass12_0.b__0(IEnumerable`1 content) error : at System.Reflection.PortableExecutable.PEBuilder.Sign(BlobBuilder peImage, Blob strongNameSignatureFixup, Func`2 signatureProvider) error : at System.Reflection.PortableExecutable.ManagedPEBuilder.Sign(BlobBuilder peImage, Func`2 signatureProvider) error : at Microsoft.CodeAnalysis.DesktopStrongNameProvider.SignBuilder(ExtendedPEBuilder peBuilder, BlobBuilder peBlob, RSAParameters privateKey) 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) 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) 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) error : at Microsoft.CodeAnalysis.CommonCompiler.CompileAndEmit(TouchedFileLogger touchedFilesLogger, Compilation& compilation, ImmutableArray`1 analyzers, ImmutableArray`1 generators, ImmutableArray`1 additionalTextFiles, AnalyzerConfigSet analyzerConfigSet, ImmutableArray`1 sourceFileAnalyzerConfigOptions, ImmutableArray`1 embeddedTexts, DiagnosticBag diagnostics, CancellationToken cancellationToken, CancellationTokenSource& analyzerCts, Boolean& reportAnalyzer, AnalyzerDriver& analyzerDriver) error : at Microsoft.CodeAnalysis.CommonCompiler.RunCore(TextWriter consoleOutput, ErrorLogger errorLogger, CancellationToken cancellationToken) error : at Microsoft.CodeAnalysis.CommonCompiler.Run(TextWriter consoleOutput, CancellationToken cancellationToken) error : at Microsoft.CodeAnalysis.CSharp.CommandLine.Csc.<>c__DisplayClass1_0.b__0(TextWriter tw) error : at Microsoft.CodeAnalysis.CommandLine.ConsoleUtil.RunWithUtf8Output[T](Boolean utf8Output, TextWriter textWriter, Func`2 func) error : at Microsoft.CodeAnalysis.CSharp.CommandLine.Csc.Run(String[] args, BuildPaths buildPaths, TextWriter textWriter, IAnalyzerAssemblyLoader analyzerLoader) error : at Microsoft.CodeAnalysis.CommandLine.BuildClient.RunCompilation(IEnumerable`1 originalArguments, BuildPaths buildPaths, TextWriter textWriter, String pipeName) error : at Microsoft.CodeAnalysis.CommandLine.BuildClient.Run(IEnumerable`1 arguments, RequestLanguage language, CompileFunc compileFunc, CompileOnServerFunc compileOnServerFunc) error : at Microsoft.CodeAnalysis.CSharp.CommandLine.Program.MainCore(String[] args) ``` Full log is here: https://centos.softwarefactory-project.io/zuul/t/centos/build/b00c0fe1895c4e1487350108a41214da Could this be caused by [Disable SHA1 signature creation and verification by default](https://gitlab.com/redhat/centos-stream/rpms/openssl/-/commit/78fb78d30755ae18fdaef28ef392f4e67c662ff6)? ### Reproduction Steps It seems like building runtime in a CentOS Stream 9 container should be enough to trigger the bug. Will test and update this step later. ### Expected behavior I can build .NET itself. ### Actual behavior I can't build .NET, because the compiler needs signing (via OpenSSL) to work. ### Regression? It's a regression somewhere. Most likely it's not .NET itself that's to blame, because this same source code (no changes) built a few days ago. ### Known Workarounds I am testing if an `export OPENSSL_ENABLE_SHA1_SIGNATURES=1` will work around the issue. ### Configuration - .NET 6 source-build using the 6.0.102 tag of dotnet/installer - CentOS Stream 9 on x86_64 - ### Other information _No response_

Author:	omajid
Assignees:	-
Labels:	`area-System.Security`, `untriaged`
Milestone:	-

vcsjones commented 2 years ago

Could this be caused by Disable SHA1 signature creation and verification by default?

Seems likely. It's trying to produce a strong name signature and that uses RSA-SHA1:

https://github.com/dotnet/roslyn/blob/315c2e149ba7889b0937d872274c33fcbfe9af5f/src/Compilers/Core/Portable/PEWriter/SigningUtilities.cs#L22

omajid commented 2 years ago

Thanks for the pointer.

I tried a build with a workaround to enable SHA1 and it seems to be working. It does seem to be down to SHA1 signature creation being disabled.

How difficult would be be to switch to something slightly better for a future release of .NET? I have no knowledge of Windows, but I found a blog post that suggests SHA256 and SHA512 are also supported for signing?

vcsjones commented 2 years ago

How difficult would be be to switch to something slightly better for a future release of .NET? I have no knowledge of Windows,

My understanding is that this RSA-SHA1 is being used for .NET's strong naming functionality, not Windows Authenticode. The Authenticode signatures are likely already SHA256.

I don't know what it would take to move the ecosystem past RSA-SHA1 for the purpose that it is being used. Perhaps @bartonjs or @GrabYourPitchforks know, directly or indirectly.

vcsjones commented 2 years ago

For what it's worth, this breaks any .NET project that uses strong name signing, not just building .NET itself.

To reproduce, create a new C# project. Add this somewhere:

[assembly:System.Reflection.AssemblyKeyFile("testkey.snk")]

Grab a strong name key. You can use testkey.snk in this repository. Put it next to the csproj.

It will fail to build with the same error.

jaredpar commented 2 years ago

My understanding is that this RSA-SHA1 is being used for .NET's strong naming functionality, not Windows Authenticode

That is correct for this scenario / stack trace

bartonjs commented 2 years ago

ECMA-335 only describes RSA(SSA)-SHA1(-PKCS1v1_5):

The Strong Name (SN) signing process uses standard hash and cipher algorithms for Strong name signing. An SHA-1 hash over most of the PE file is generated. That hash value is RSA-signed with the SN private key. For verification purposes the public key is stored into the PE file as well as the signed hash value.

There's a concept of "extended strong name", which is a SHA256 countersignature on the SHA-1 signature (similar to how Authenticode bootstrapped SHA256); but I don't think that there's a Strong Name scenario that avoids SHA-1.

While Strong Name isn't important for .NET Core / .NET 5+, I can imagine RHEL users building components (such as the Roslyn C# compiler) that need to support Strong Name for .NET Framework-based customers (either a .NET Standard library or even building directly for a .NET Framework TFM).

I don't know how dotnet build works (e.g. does it launch a second process?), but perhaps there's a way for the RHEL build of the SDK to always assert OPENSSL_ENABLE_SHA1_SIGNATURES=1 in the environment when using dotnet build?

vcsjones commented 2 years ago

Additionally, I did a quick search to see if there are other things in the dotnet org that use RSA+SHA1 and strong name signing is the only meaningful result.

https://cs.github.com/?scopeName=All+repos&scope=&q=org%3Adotnet+%2F%28Sign%7CVerify%29%28Hash%7CData%29%5C%28.*HashAlgorithmName.SHA1%2F+NOT+path%3A%2Ftests%3F%2F

omajid commented 2 years ago

I reported this to the roslyn folks directly as well, at https://github.com/dotnet/roslyn/issues/59880, particularly because I am not sure if roslyn's usage of SHA1 qualifies as "security" or not.

jaredpar commented 2 years ago

@omajid I responded to the issue in dotnet/roslyn. In summary though this isn't a decision the roslyn compiler is making. The .NET platform requires SHA-1 be used for strong naming. The roslyn compiler, and every other .NET compiler that supports strong naming, is simply implementing the specification here. There are no other options besides SHA-1 unless the underlying format is changed.

omajid commented 2 years ago

Thanks, @jaredpar

The .NET platform requires SHA-1 be used for strong naming

What should the platform do when (RSA+)SHA1 is not available on the OS (because of policies like FIPS, or general system hardening)? Should it hard-fail like it does now? Should it have a separate this-is-not-security implementation of RSA+SHA1?

Is there a way to tell OpenSSL "we want to use RSA+SHA1, but not for security, we promise"?

There are no other options besides SHA-1 unless the underlying format is changed.

Given how SHA1 is being phased out everywhere, what would it take to update the underlying format?

omajid commented 2 years ago

Perhaps there's a way for the RHEL build of the SDK to always assert OPENSSL_ENABLE_SHA1_SIGNATURES=1 in the environment when using dotnet build?

I slept over this one, and I wonder if this will allow using RSA+SHA1 even in contexts where the OS really doesn't want it to be used, eg for TLS when downloading nuget packages (that happens as part of dotnet build, right?).

And it wont help when FIPS enablement permanently disables RSA+SHA1.

vcsjones commented 2 years ago

Is there a way to tell OpenSSL "we want to use RSA+SHA1, but not for security, we promise"?

Correct me if I am wrong, but this is a patch on top of OpenSSL developed by RedHat, not something that is in OpenSSL upstream, correct? It would then be up to RedHat to implement a work around, if one needs to be provided.

Based on the changeset, it looks like developers can:

Set rh-allow-sha1-signatures in openssl.cnf to permanently re-allow SHA1 signatures.
Use the environment variable, as you have already described.

Even if RedHat provided an option for "pinky swear it's not for security", it would require the runtime detect this custom API from RedHat, and somehow expose this option to roslyn and fsharp compilers.

Alternatively, the roslyn and fsharp compilers could consider using a private implementation, say one written in managed code, of RSA and not use OpenSSL at all. I don't know if that is at all feasible or not.

jaredpar commented 2 years ago

Alternatively, the roslyn and fsharp compilers could consider using a private implementation, say one written in managed code, of RSA and not use OpenSSL at all. I don't know if that is at all feasible or not.

I don't think having a private implementation in Roslyn / F# is the right answer. That means every compiler team has to own their own copy of SHA-1, we all have to individually justify it to the security bugs that will come every 6 months (even justifying the enum value of SHA1 is painful). It also means 3rd party .NET compilers have to do the same.

If we go the private implementation route I think the best approach is put into the PEBuilder type directly, make it private and have a Sign overload that uses this implementation. That makes it clear it is only used for ECMA-335 compliance for which there is a known SHA-1 exception.

Given how SHA1 is being phased out everywhere, what would it take to update the underlying format?

@jkotas and @davidwrighton are the best to answer that question authoritatively. But my understanding is that the answer is a lot. Consider that it's not just updating for .NET Core. In order for the update to solve this problem we also need to update it for .NET Framework. Further it probably needs to be updated for all versions of .NET Framework that netstandard2.0 maps to. That is a huge cost.

bartonjs commented 2 years ago

Given how SHA1 is being phased out everywhere, what would it take to update the underlying format?

Personally, I think it's one of those "there's not enough money on the planet" problems. My understanding is that the main reason that Roslyn still has Strong Name support is for .NET Framework compatibility. .NET Framework can't support anything other than RSASSA-SHA1-PKCS1-v1_5 without first making the change (which seems obvious... but seems against our current level of support for it) and then waiting for it to be deployed pretty much everywhere in the world (beautiful in-place upgrade SxS protocol delays)... and then could finally start using it. Oh, and it may or may not require first adding a revision to ECMA-335 (not sure if that spec lags or precedes implementation).

Should it have a separate this-is-not-security implementation of RSA+SHA1?

It's certainly... possible. But, personally, I think that the answer is "operating environments that do not allow RSA+SHA1 can not be used with scenarios that need to produce assemblies with strong names". Further information over time could, of course, change our position here.

And it wont help when FIPS enablement permanently disables RSA+SHA1.

In Windows, at least, FIPSyness has never blocked algorithms. The OS team's view is that the application can make the decision as to whether their attempt to use a non-FIPS-approved algorithm is valid. As far as I understand things, FIPS only applies when cryptography is used for a security decision... e.g. MD5 is not FIPS-Approved, but you don't lose government contracts for using the Content-MD5 HTTP header, since it's not security. .NET's position is that strong name is not a security feature... ergo, no FIPS implication.

I'm personally not sure what OpenSSL 3's FIPS module is going to do here (fall back to default/legacy if you ask for non-FIPS modes of operation, or fail).

That means every compiler team has to own their own copy of SHA-1

Actually, it'd be their own copy of RSA (specifically, RSASSA-PKCS1-v1_5 signing)... the SHA-1 by itself works fine.

jaredpar commented 2 years ago

My understanding is that the main reason that Roslyn still has Strong Name support is for .NET Framework compatibility

Prefer we stop referring to this as a Roslyn problem because it keeps leading to Roslyn based solutions, Roslyn issues, etc ... This is a .NET Compiler problem. It impacts F# just as much as Roslyn.

That means every compiler team has to own their own copy of SHA-1

Actually, it'd be their own copy of RSA (specifically, RSASSA-PKCS1-v1_5 signing)... the SHA-1 by itself works fine.

Gotcha

jkotas commented 2 years ago

operating environments that do not allow RSA+SHA1 can not be used with scenarios that need to produce assemblies with strong names

I agree. If you need to produce assembly that is fully compatible with .NET Framework, you need to have RSA1+SHA1. If you do not need it to be fully compatible with .NET Framework, use public signing. Note that the public signed binaries still work on .NET Framework in most scenarios.

Maybe the resolution for this is to change the defaults somewhere? Public sign by default, require opt-in into full signing?

jaredpar commented 2 years ago

Maybe the resolution for this is to change the defaults somewhere? Public sign by default, require opt-in into full signing?

I think changing the default for .NET Core target frameworks is reasonable. It should not have any impact.

For netstandard2.0 and .NET Framework target frameworks though it seems a lot dicier. To address this problem we'd need to flip the defaults for these target frameworks to. That is necessary at a fundamental level because even our own source build, which must work on RedHat, uses these target frameworks. It's also reasonable for customers to build these target frameworks on RedHat. So seems like we have to include them in the change.

So say for example in .NET 7 we made the call to do public sign by default and force people who want real sign to opt into it. That seems like a reasonable stance to take. Problem is how do we communicate this to customers? Worry we could quickly end up in a place where people upgrade to .NET 7, everything passes tests but fails in deployment because that is the place which was most likely to rely on strong named binaries.

jkotas commented 2 years ago

For netstandard2.0 and .NET Framework target frameworks though it seems a lot dicier.

Agree. I do not think it is worth the pain to be changing default for these. If you need to target legacy systems, you need to have legacy build machine configuration.

omajid commented 2 years ago

Thanks, everyone for the discussion.

For now, I see two paths forward for users, and no action needed on the .NET side:

Users should use PublicSigning, through something like this in their project file

<PropertyGroup>
<SignAssembly>True</SignAssembly>
<PublicSign>True</PublicSign> 
<AssemblyOriginatorKeyFile>key.snk</AssemblyOriginatorKeyFile>
</PropertyGroup>

Users should re-enable SHA1+RSA, through some OpenSSL configuration. This wont work in FIPS mode.

This might be through something like this shell command:
```
# update-crypto-policies --set LEGACY
```
Or it might be through a special environment variable and/or configuration option (which are less well supported/stable than this LEGACY switch).

Is there anything else that needs to be done? Shall I close this issue? Is this worth documenting somewhere?

bartonjs commented 2 years ago

Is this worth documenting somewhere?

I'm not sure where/what we'd document.

From the RSA perspective, it only applies when on Fedora/CentOS/RHEL when using RSAOpenSsl or RSA.Create() (a custom RSA implementation, such as RSAKeyVault, wouldn't necessarily have the problem), and at best we'd just link to the RedHat crypto docs for it. (From an API perspective, it's covered under the blanket docs of "if something goes wrong: CryptographicException"... and this is just "the underlying provider doesn't support the hash algorithm". I think RSACryptoServiceProvider -might- still support RSA+MD4, but RSAOpenSsl doesn't).

From the assembly strong name perspective, we already recommend public sign for .NET Core projects, and don't know where in that doc stream anyone would find "if building for .NET Framework or .NET Standard (for .NET Framework) from [a RedHat-influenced distro], things get complicated".

So, I'm not opposed to documentation, just can't think of where it'd go and how to explain the problem or solution in an approachable manner :smile:.

akoeplinger commented 2 years ago

I'm curious which part of the .NET 6 build requires real signing (the log file linked in the original post isn't available anymore). Presumably we should still fix that one at least to use public sign?

bartonjs commented 2 years ago

Any library that can encounter a strongname check on .NET Framework probably needs to be strong name signed, so that's potentially anything in the project tree that compiles for .NET Framework or .NET Standard (and excluding anything that compiles against a .NET Core/5+ TFM).

But that's really only true for an official distribution channel that can realistically end up on .NET Framework where it might encounter an SN check. I don't think there's anything that builds out of dotnet/* that expects to end up in a realistic SN-relevant scenario where Microsoft doesn't have the official build and distribution channel. So that means that (e.g.) Red Hat building dotnet/* using public signing, since the output of those builds is generally expected only to be for the shared runtime for Red Hat-influenced Linux distros.

The main SN-relevant scenario on .NET Framework is installing an assembly in the GAC. Apps using local copies of DLLs from NuGet or whatever aren't SN-relevant (OK, I think there's a regkey to make .NET Framework SN-check all assembly loads, but it's almost definitely not the default). So "we" (Microsoft builds) have to do real-signing (particularly for assemblies that have in the past), but it shouldn't come up much for anyone else.

omajid commented 2 years ago

I'm curious which part of the .NET 6 build requires real signing (the log file linked in the original post isn't available anymore). Presumably we should still fix that one at least to use public sign?

I am hitting that in source-build-reference-packages:

    /builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets(75,5): error : Unhandled exception. Interop+Crypto+OpenSslCryptographicException: error:03000098:digital envelope routines::invalid digest [/builddir/build/BUILD/dotnet-v6.0.102/src/source-build-reference-packages.71a327c277a022d15af3d103c0796620ecd40471/artifacts/source-build/self/src/src/referencePackages/src/microsoft.bcl.asyncinterfaces/5.0.0/Microsoft.Bcl.AsyncInterfaces.5.0.0.csproj]
    ##vso[task.logissue type=error;sourcepath=/builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets;linenumber=75;columnnumber=5;code=;](NETCORE_ENGINEERING_TELEMETRY=Build) Unhandled exception. Interop+Crypto+OpenSslCryptographicException: error:03000098:digital envelope routines::invalid digest
    /builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets(75,5): error :    at Interop.Crypto.RsaSignHash(SafeEvpPKeyHandle pkey, RSASignaturePaddingMode paddingMode, IntPtr digestAlgorithm, ReadOnlySpan`1 hash, Span`1 destination) [/builddir/build/BUILD/dotnet-v6.0.102/src/source-build-reference-packages.71a327c277a022d15af3d103c0796620ecd40471/artifacts/source-build/self/src/src/referencePackages/src/microsoft.bcl.asyncinterfaces/5.0.0/Microsoft.Bcl.AsyncInterfaces.5.0.0.csproj]
    ##vso[task.logissue type=error;sourcepath=/builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets;linenumber=75;columnnumber=5;code=;](NETCORE_ENGINEERING_TELEMETRY=Build)    at Interop.Crypto.RsaSignHash(SafeEvpPKeyHandle pkey, RSASignaturePaddingMode paddingMode, IntPtr digestAlgorithm, ReadOnlySpan`1 hash, Span`1 destination)
    /builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets(75,5): error :    at System.Security.Cryptography.RSAImplementation.RSAOpenSsl.TrySignHash(ReadOnlySpan`1 hash, Span`1 destination, HashAlgorithmName hashAlgorithm, RSASignaturePadding padding, Boolean allocateSignature, Int32& bytesWritten, Byte[]& signature) [/builddir/build/BUILD/dotnet-v6.0.102/src/source-build-reference-packages.71a327c277a022d15af3d103c0796620ecd40471/artifacts/source-build/self/src/src/referencePackages/src/microsoft.bcl.asyncinterfaces/5.0.0/Microsoft.Bcl.AsyncInterfaces.5.0.0.csproj]
    ##vso[task.logissue type=error;sourcepath=/builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets;linenumber=75;columnnumber=5;code=;](NETCORE_ENGINEERING_TELEMETRY=Build)    at System.Security.Cryptography.RSAImplementation.RSAOpenSsl.TrySignHash(ReadOnlySpan`1 hash, Span`1 destination, HashAlgorithmName hashAlgorithm, RSASignaturePadding padding, Boolean allocateSignature, Int32& bytesWritten, Byte[%5D& signature)
    /builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets(75,5): error :    at System.Security.Cryptography.RSAImplementation.RSAOpenSsl.SignHash(Byte[] hash, HashAlgorithmName hashAlgorithm, RSASignaturePadding padding) [/builddir/build/BUILD/dotnet-v6.0.102/src/source-build-reference-packages.71a327c277a022d15af3d103c0796620ecd40471/artifacts/source-build/self/src/src/referencePackages/src/microsoft.bcl.asyncinterfaces/5.0.0/Microsoft.Bcl.AsyncInterfaces.5.0.0.csproj]
    ##vso[task.logissue type=error;sourcepath=/builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets;linenumber=75;columnnumber=5;code=;](NETCORE_ENGINEERING_TELEMETRY=Build)    at System.Security.Cryptography.RSAImplementation.RSAOpenSsl.SignHash(Byte[%5D hash, HashAlgorithmName hashAlgorithm, RSASignaturePadding padding)
    /builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets(75,5): error :    at Microsoft.CodeAnalysis.SigningUtilities.CalculateRsaSignature(IEnumerable`1 content, RSAParameters privateKey) [/builddir/build/BUILD/dotnet-v6.0.102/src/source-build-reference-packages.71a327c277a022d15af3d103c0796620ecd40471/artifacts/source-build/self/src/src/referencePackages/src/microsoft.bcl.asyncinterfaces/5.0.0/Microsoft.Bcl.AsyncInterfaces.5.0.0.csproj]
    ##vso[task.logissue type=error;sourcepath=/builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets;linenumber=75;columnnumber=5;code=;](NETCORE_ENGINEERING_TELEMETRY=Build)    at Microsoft.CodeAnalysis.SigningUtilities.CalculateRsaSignature(IEnumerable`1 content, RSAParameters privateKey)
    /builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets(75,5): error :    at Microsoft.CodeAnalysis.DesktopStrongNameProvider.<>c__DisplayClass12_0.<SignBuilder>b__0(IEnumerable`1 content) [/builddir/build/BUILD/dotnet-v6.0.102/src/source-build-reference-packages.71a327c277a022d15af3d103c0796620ecd40471/artifacts/source-build/self/src/src/referencePackages/src/microsoft.bcl.asyncinterfaces/5.0.0/Microsoft.Bcl.AsyncInterfaces.5.0.0.csproj]
    ##vso[task.logissue type=error;sourcepath=/builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets;linenumber=75;columnnumber=5;code=;](NETCORE_ENGINEERING_TELEMETRY=Build)    at Microsoft.CodeAnalysis.DesktopStrongNameProvider.<>c__DisplayClass12_0.<SignBuilder>b__0(IEnumerable`1 content)
    /builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets(75,5): error :    at System.Reflection.PortableExecutable.PEBuilder.Sign(BlobBuilder peImage, Blob strongNameSignatureFixup, Func`2 signatureProvider) [/builddir/build/BUILD/dotnet-v6.0.102/src/source-build-reference-packages.71a327c277a022d15af3d103c0796620ecd40471/artifacts/source-build/self/src/src/referencePackages/src/microsoft.bcl.asyncinterfaces/5.0.0/Microsoft.Bcl.AsyncInterfaces.5.0.0.csproj]
    ##vso[task.logissue type=error;sourcepath=/builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets;linenumber=75;columnnumber=5;code=;](NETCORE_ENGINEERING_TELEMETRY=Build)    at System.Reflection.PortableExecutable.PEBuilder.Sign(BlobBuilder peImage, Blob strongNameSignatureFixup, Func`2 signatureProvider)
    /builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets(75,5): error :    at System.Reflection.PortableExecutable.ManagedPEBuilder.Sign(BlobBuilder peImage, Func`2 signatureProvider) [/builddir/build/BUILD/dotnet-v6.0.102/src/source-build-reference-packages.71a327c277a022d15af3d103c0796620ecd40471/artifacts/source-build/self/src/src/referencePackages/src/microsoft.bcl.asyncinterfaces/5.0.0/Microsoft.Bcl.AsyncInterfaces.5.0.0.csproj]
    ##vso[task.logissue type=error;sourcepath=/builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets;linenumber=75;columnnumber=5;code=;](NETCORE_ENGINEERING_TELEMETRY=Build)    at System.Reflection.PortableExecutable.ManagedPEBuilder.Sign(BlobBuilder peImage, Func`2 signatureProvider)
    /builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets(75,5): error :    at Microsoft.CodeAnalysis.DesktopStrongNameProvider.SignBuilder(ExtendedPEBuilder peBuilder, BlobBuilder peBlob, RSAParameters privateKey) [/builddir/build/BUILD/dotnet-v6.0.102/src/source-build-reference-packages.71a327c277a022d15af3d103c0796620ecd40471/artifacts/source-build/self/src/src/referencePackages/src/microsoft.bcl.asyncinterfaces/5.0.0/Microsoft.Bcl.AsyncInterfaces.5.0.0.csproj]
    ##vso[task.logissue type=error;sourcepath=/builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets;linenumber=75;columnnumber=5;code=;](NETCORE_ENGINEERING_TELEMETRY=Build)    at Microsoft.CodeAnalysis.DesktopStrongNameProvider.SignBuilder(ExtendedPEBuilder peBuilder, BlobBuilder peBlob, RSAParameters privateKey)
    /builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets(75,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) [/builddir/build/BUILD/dotnet-v6.0.102/src/source-build-reference-packages.71a327c277a022d15af3d103c0796620ecd40471/artifacts/source-build/self/src/src/referencePackages/src/microsoft.bcl.asyncinterfaces/5.0.0/Microsoft.Bcl.AsyncInterfaces.5.0.0.csproj]
    ##vso[task.logissue type=error;sourcepath=/builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets;linenumber=75;columnnumber=5;code=;](NETCORE_ENGINEERING_TELEMETRY=Build)    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)
    /builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets(75,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) [/builddir/build/BUILD/dotnet-v6.0.102/src/source-build-reference-packages.71a327c277a022d15af3d103c0796620ecd40471/artifacts/source-build/self/src/src/referencePackages/src/microsoft.bcl.asyncinterfaces/5.0.0/Microsoft.Bcl.AsyncInterfaces.5.0.0.csproj]
    ##vso[task.logissue type=error;sourcepath=/builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets;linenumber=75;columnnumber=5;code=;](NETCORE_ENGINEERING_TELEMETRY=Build)    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)
    /builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets(75,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) [/builddir/build/BUILD/dotnet-v6.0.102/src/source-build-reference-packages.71a327c277a022d15af3d103c0796620ecd40471/artifacts/source-build/self/src/src/referencePackages/src/microsoft.bcl.asyncinterfaces/5.0.0/Microsoft.Bcl.AsyncInterfaces.5.0.0.csproj]
    ##vso[task.logissue type=error;sourcepath=/builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets;linenumber=75;columnnumber=5;code=;](NETCORE_ENGINEERING_TELEMETRY=Build)    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)
    /builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets(75,5): error :    at Microsoft.CodeAnalysis.CommonCompiler.CompileAndEmit(TouchedFileLogger touchedFilesLogger, Compilation& compilation, ImmutableArray`1 analyzers, ImmutableArray`1 generators, ImmutableArray`1 additionalTextFiles, AnalyzerConfigSet analyzerConfigSet, ImmutableArray`1 sourceFileAnalyzerConfigOptions, ImmutableArray`1 embeddedTexts, DiagnosticBag diagnostics, CancellationToken cancellationToken, CancellationTokenSource& analyzerCts, Boolean& reportAnalyzer, AnalyzerDriver& analyzerDriver) [/builddir/build/BUILD/dotnet-v6.0.102/src/source-build-reference-packages.71a327c277a022d15af3d103c0796620ecd40471/artifacts/source-build/self/src/src/referencePackages/src/microsoft.bcl.asyncinterfaces/5.0.0/Microsoft.Bcl.AsyncInterfaces.5.0.0.csproj]
    ##vso[task.logissue type=error;sourcepath=/builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets;linenumber=75;columnnumber=5;code=;](NETCORE_ENGINEERING_TELEMETRY=Build)    at Microsoft.CodeAnalysis.CommonCompiler.CompileAndEmit(TouchedFileLogger touchedFilesLogger, Compilation& compilation, ImmutableArray`1 analyzers, ImmutableArray`1 generators, ImmutableArray`1 additionalTextFiles, AnalyzerConfigSet analyzerConfigSet, ImmutableArray`1 sourceFileAnalyzerConfigOptions, ImmutableArray`1 embeddedTexts, DiagnosticBag diagnostics, CancellationToken cancellationToken, CancellationTokenSource& analyzerCts, Boolean& reportAnalyzer, AnalyzerDriver& analyzerDriver)
    /builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets(75,5): error :    at Microsoft.CodeAnalysis.CommonCompiler.RunCore(TextWriter consoleOutput, ErrorLogger errorLogger, CancellationToken cancellationToken) [/builddir/build/BUILD/dotnet-v6.0.102/src/source-build-reference-packages.71a327c277a022d15af3d103c0796620ecd40471/artifacts/source-build/self/src/src/referencePackages/src/microsoft.bcl.asyncinterfaces/5.0.0/Microsoft.Bcl.AsyncInterfaces.5.0.0.csproj]
    ##vso[task.logissue type=error;sourcepath=/builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets;linenumber=75;columnnumber=5;code=;](NETCORE_ENGINEERING_TELEMETRY=Build)    at Microsoft.CodeAnalysis.CommonCompiler.RunCore(TextWriter consoleOutput, ErrorLogger errorLogger, CancellationToken cancellationToken)
    /builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets(75,5): error :    at Microsoft.CodeAnalysis.CommonCompiler.Run(TextWriter consoleOutput, CancellationToken cancellationToken) [/builddir/build/BUILD/dotnet-v6.0.102/src/source-build-reference-packages.71a327c277a022d15af3d103c0796620ecd40471/artifacts/source-build/self/src/src/referencePackages/src/microsoft.bcl.asyncinterfaces/5.0.0/Microsoft.Bcl.AsyncInterfaces.5.0.0.csproj]
    ##vso[task.logissue type=error;sourcepath=/builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets;linenumber=75;columnnumber=5;code=;](NETCORE_ENGINEERING_TELEMETRY=Build)    at Microsoft.CodeAnalysis.CommonCompiler.Run(TextWriter consoleOutput, CancellationToken cancellationToken)
    /builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets(75,5): error :    at Microsoft.CodeAnalysis.CSharp.CommandLine.Csc.<>c__DisplayClass1_0.<Run>b__0(TextWriter tw) [/builddir/build/BUILD/dotnet-v6.0.102/src/source-build-reference-packages.71a327c277a022d15af3d103c0796620ecd40471/artifacts/source-build/self/src/src/referencePackages/src/microsoft.bcl.asyncinterfaces/5.0.0/Microsoft.Bcl.AsyncInterfaces.5.0.0.csproj]
    ##vso[task.logissue type=error;sourcepath=/builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets;linenumber=75;columnnumber=5;code=;](NETCORE_ENGINEERING_TELEMETRY=Build)    at Microsoft.CodeAnalysis.CSharp.CommandLine.Csc.<>c__DisplayClass1_0.<Run>b__0(TextWriter tw)
    /builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets(75,5): error :    at Microsoft.CodeAnalysis.CommandLine.ConsoleUtil.RunWithUtf8Output[T](Boolean utf8Output, TextWriter textWriter, Func`2 func) [/builddir/build/BUILD/dotnet-v6.0.102/src/source-build-reference-packages.71a327c277a022d15af3d103c0796620ecd40471/artifacts/source-build/self/src/src/referencePackages/src/microsoft.bcl.asyncinterfaces/5.0.0/Microsoft.Bcl.AsyncInterfaces.5.0.0.csproj]
    ##vso[task.logissue type=error;sourcepath=/builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets;linenumber=75;columnnumber=5;code=;](NETCORE_ENGINEERING_TELEMETRY=Build)    at Microsoft.CodeAnalysis.CommandLine.ConsoleUtil.RunWithUtf8Output[T%5D(Boolean utf8Output, TextWriter textWriter, Func`2 func)
    /builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets(75,5): error :    at Microsoft.CodeAnalysis.CSharp.CommandLine.Csc.Run(String[] args, BuildPaths buildPaths, TextWriter textWriter, IAnalyzerAssemblyLoader analyzerLoader) [/builddir/build/BUILD/dotnet-v6.0.102/src/source-build-reference-packages.71a327c277a022d15af3d103c0796620ecd40471/artifacts/source-build/self/src/src/referencePackages/src/microsoft.bcl.asyncinterfaces/5.0.0/Microsoft.Bcl.AsyncInterfaces.5.0.0.csproj]
    ##vso[task.logissue type=error;sourcepath=/builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets;linenumber=75;columnnumber=5;code=;](NETCORE_ENGINEERING_TELEMETRY=Build)    at Microsoft.CodeAnalysis.CSharp.CommandLine.Csc.Run(String[%5D args, BuildPaths buildPaths, TextWriter textWriter, IAnalyzerAssemblyLoader analyzerLoader)
    /builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets(75,5): error :    at Microsoft.CodeAnalysis.CommandLine.BuildClient.RunCompilation(IEnumerable`1 originalArguments, BuildPaths buildPaths, TextWriter textWriter, String pipeName) [/builddir/build/BUILD/dotnet-v6.0.102/src/source-build-reference-packages.71a327c277a022d15af3d103c0796620ecd40471/artifacts/source-build/self/src/src/referencePackages/src/microsoft.bcl.asyncinterfaces/5.0.0/Microsoft.Bcl.AsyncInterfaces.5.0.0.csproj]
    ##vso[task.logissue type=error;sourcepath=/builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets;linenumber=75;columnnumber=5;code=;](NETCORE_ENGINEERING_TELEMETRY=Build)    at Microsoft.CodeAnalysis.CommandLine.BuildClient.RunCompilation(IEnumerable`1 originalArguments, BuildPaths buildPaths, TextWriter textWriter, String pipeName)
    /builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets(75,5): error :    at Microsoft.CodeAnalysis.CommandLine.BuildClient.Run(IEnumerable`1 arguments, RequestLanguage language, CompileFunc compileFunc, CompileOnServerFunc compileOnServerFunc) [/builddir/build/BUILD/dotnet-v6.0.102/src/source-build-reference-packages.71a327c277a022d15af3d103c0796620ecd40471/artifacts/source-build/self/src/src/referencePackages/src/microsoft.bcl.asyncinterfaces/5.0.0/Microsoft.Bcl.AsyncInterfaces.5.0.0.csproj]
    ##vso[task.logissue type=error;sourcepath=/builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets;linenumber=75;columnnumber=5;code=;](NETCORE_ENGINEERING_TELEMETRY=Build)    at Microsoft.CodeAnalysis.CommandLine.BuildClient.Run(IEnumerable`1 arguments, RequestLanguage language, CompileFunc compileFunc, CompileOnServerFunc compileOnServerFunc)
    /builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets(75,5): error :    at Microsoft.CodeAnalysis.CSharp.CommandLine.Program.MainCore(String[] args) [/builddir/build/BUILD/dotnet-v6.0.102/src/source-build-reference-packages.71a327c277a022d15af3d103c0796620ecd40471/artifacts/source-build/self/src/src/referencePackages/src/microsoft.bcl.asyncinterfaces/5.0.0/Microsoft.Bcl.AsyncInterfaces.5.0.0.csproj]
    ##vso[task.logissue type=error;sourcepath=/builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets;linenumber=75;columnnumber=5;code=;](NETCORE_ENGINEERING_TELEMETRY=Build)    at Microsoft.CodeAnalysis.CSharp.CommandLine.Program.MainCore(String[%5D args)
    /builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets(75,5): error :    at Microsoft.CodeAnalysis.CSharp.CommandLine.Program.Main(String[] args) [/builddir/build/BUILD/dotnet-v6.0.102/src/source-build-reference-packages.71a327c277a022d15af3d103c0796620ecd40471/artifacts/source-build/self/src/src/referencePackages/src/microsoft.bcl.asyncinterfaces/5.0.0/Microsoft.Bcl.AsyncInterfaces.5.0.0.csproj]
    ##vso[task.logissue type=error;sourcepath=/builddir/build/BUILD/dotnet-v6.0.102/previously-built-dotnet/sdk/6.0.102/Roslyn/Microsoft.CSharp.Core.targets;linenumber=75;columnnumber=5;code=;](NETCORE_ENGINEERING_TELEMETRY=Build)    at Microsoft.CodeAnalysis.CSharp.CommandLine.Program.Main(String[%5D args)

tmds commented 2 years ago

@bartonjs @vcsjones when we run dotnet/runtime tests there is no failure on RHEL 9. Does that mean this isn't covered by the tests?

vcsjones commented 2 years ago

We almost certainly have tests that would hit RSA+PKCS1+SHA1 signature condition, such as this one.

https://github.com/dotnet/runtime/blob/91da4ac9fd3470fe4798f0151e36f1769a9e9c2c/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/SignVerify.cs#L508

Can you confirm if those tests ran?

omajid commented 2 years ago

In the xml results, I see:

<test name="System.Security.Cryptography.Rsa.Tests.SignVerify_Array.SignAndVerify_Roundtrip(hashAlgorithm: \&quot;MD5\&quot;, rsaParameters: System.Security.Cryptography.RSAParameters)" type="System.Security.Cryptography.Rsa.Tests.SignVerify_Array" method="SignAndVerify_Roundtrip" time="0.0071895" result="Pass" />
<test name="System.Security.Cryptography.Rsa.Tests.SignVerify_Array.SignAndVerify_Roundtrip(hashAlgorithm: \&quot;SHA1\&quot;, rsaParameters: System.Security.Cryptography.RSAParameters)" type="System.Security.Cryptography.Rsa.Tests.SignVerify_Array" method="SignAndVerify_Roundtrip" time="0.0068557" result="Pass" />
<test name="System.Security.Cryptography.Rsa.Tests.SignVerify_Array.SignAndVerify_Roundtrip(hashAlgorithm: \&quot;SHA256\&quot;, rsaParameters: System.Security.Cryptography.RSAParameters)" type="System.Security.Cryptography.Rsa.Tests.SignVerify_Array" method="SignAndVerify_Roundtrip" time="0.0072911" result="Pass" />
<test name="System.Security.Cryptography.Rsa.Tests.SignVerify_Array.SignAndVerify_Roundtrip(hashAlgorithm: \&quot;MD5\&quot;, rsaParameters: System.Security.Cryptography.RSAParameters)" type="System.Security.Cryptography.Rsa.Tests.SignVerify_Array" method="SignAndVerify_Roundtrip" time="0.0436004" result="Pass" />
<test name="System.Security.Cryptography.Rsa.Tests.SignVerify_Array.SignAndVerify_Roundtrip(hashAlgorithm: \&quot;SHA1\&quot;, rsaParameters: System.Security.Cryptography.RSAParameters)" type="System.Security.Cryptography.Rsa.Tests.SignVerify_Array" method="SignAndVerify_Roundtrip" time="0.0420614" result="Pass" />
<test name="System.Security.Cryptography.Rsa.Tests.SignVerify_Array.SignAndVerify_Roundtrip(hashAlgorithm: \&quot;SHA256\&quot;, rsaParameters: System.Security.Cryptography.RSAParameters)" type="System.Security.Cryptography.Rsa.Tests.SignVerify_Array" method="SignAndVerify_Roundtrip" time="0.042372" result="Pass" />
<test name="System.Security.Cryptography.Rsa.Tests.SignVerify_Array.SignAndVerify_Roundtrip(hashAlgorithm: \&quot;SHA384\&quot;, rsaParameters: System.Security.Cryptography.RSAParameters)" type="System.Security.Cryptography.Rsa.Tests.SignVerify_Array" method="SignAndVerify_Roundtrip" time="0.043085" result="Pass" />
<test name="System.Security.Cryptography.Rsa.Tests.SignVerify_Array.SignAndVerify_Roundtrip(hashAlgorithm: \&quot;SHA512\&quot;, rsaParameters: System.Security.Cryptography.RSAParameters)" type="System.Security.Cryptography.Rsa.Tests.SignVerify_Array" method="SignAndVerify_Roundtrip" time="0.0424903" result="Pass" />

:confused:

vcsjones commented 2 years ago

Is it possible that the environment changes you made to get it building are getting picked up by the tests?

I am testing if an export OPENSSL_ENABLE_SHA1_SIGNATURES=1 will work around the issue.

If that is still in effect during test execution, then I would expect the tests to pass.

vcsjones commented 2 years ago

@tmds @omajid I can get the tests to fail in CentOS 9 Stream. Here is a Docker file which does so.

This was done with the base image sha256:c498ab29be98c552440487f06e78ba22b3892f4b29d37aa208fe7ded95280f17. Note: this hard-codes some paths to assume ARM64. If you are running this docker image on x64, you will need to adjust some paths in the CMD.

I think this probably means somewhere in your build / test infrastructure, SHA1 is being enabled.

Note that with SHA1 signatures disabled, quite a number of things do not actually work. I can't even get it to build without enabling SHA1 signatures in a few places.

FROM quay.io/centos/centos:stream9

RUN dnf -y groupinstall 'Development Tools' && \
    dnf install -y dnf-plugins-core epel-release && \
    dnf repolist --all && \
    dnf config-manager --set-enabled crb && \
    dnf install -y \
        clang \
        cmake \
        git \
        glibc-langpack-en \
        hostname \
        krb5-devel \
        libicu-devel \
        lld \
        llvm \
        make \
        openssl-devel \
        python3 \
        tar \
        util-linux \
        zlib-devel \
        lttng-ust-devel \
        libunwind-devel

RUN mkdir /projects
WORKDIR /projects
RUN git clone https://github.com/dotnet/runtime.git
WORKDIR /projects/runtime

# OPENSSL_ENABLE_SHA1_SIGNATURES needs to be set for project restore to work
# since ADO is doing RSA SHA1 signatures somewhere in TLS.
# We don't want to export it though so that unit tests don't pick this up.
RUN OPENSSL_ENABLE_SHA1_SIGNATURES=1 ./build.sh -rc release -s clr+libs

# Build tests with OPENSSL_ENABLE_SHA1_SIGNATURES so that everything
# gets restored and built.
RUN OPENSSL_ENABLE_SHA1_SIGNATURES=1 \
    ./dotnet.sh build src/libraries/System.Security.Cryptography/tests

# Actual run does NOT have OPENSSL_ENABLE_SHA1_SIGNATURES.
CMD artifacts/bin/testhost/net7.0-Linux-Debug-arm64/dotnet exec \
    --runtimeconfig artifacts/bin/System.Security.Cryptography.Tests/Debug/net7.0-unix/System.Security.Cryptography.Tests.runtimeconfig.json \
    artifacts/bin/System.Security.Cryptography.Tests/Debug/net7.0-unix/xunit.console.dll \
    artifacts/bin/System.Security.Cryptography.Tests/Debug/net7.0-unix/System.Security.Cryptography.Tests.dll \
    -notrait category=OuterLoop -notrait category=failing

I get the following test results:

=== TEST EXECUTION SUMMARY ===
   System.Security.Cryptography.Tests  Total: 6205, Errors: 0, Failed: 39, Skipped: 17, Time: 192.750s

An example failure:

Interop+Crypto+OpenSslCryptographicException : error:03000098:digital envelope routines::invalid digest
      Stack Trace:
    System.Security.Cryptography.Rsa.Tests.SignVerify_Array.InvalidKeySize_DoesNotInvalidateKey [FAIL]
        /projects/runtime/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EvpPkey.Rsa.cs(147,0): at Interop.Crypto.RsaSignHash(SafeEvpPKeyHandle pkey, RSASignaturePaddingMode paddingMode, IntPtr digestAlgorithm, ReadOnlySpan`1 hash, Span`1 destination)
        /projects/runtime/src/libraries/Common/src/System/Security/Cryptography/RSAOpenSsl.cs(796,0): at System.Security.Cryptography.RSAOpenSsl.TrySignHash(ReadOnlySpan`1 hash, Span`1 destination, HashAlgorithmName hashAlgorithm, RSASignaturePadding padding, Boolean allocateSignature, Int32& bytesWritten, Byte[]& signature)
        /projects/runtime/src/libraries/Common/src/System/Security/Cryptography/RSAOpenSsl.cs(726,0): at System.Security.Cryptography.RSAOpenSsl.SignHash(Byte[] hash, HashAlgorithmName hashAlgorithm, RSASignaturePadding padding)
<snip>

tmds commented 2 years ago

I think this probably means somewhere in your build / test infrastructure, SHA1 is being enabled.

@vcsjones you're right. Our rhel9 builds were not using the default policy. Using the default, it doesn't work.

tmds commented 2 years ago

@vcsjones probably we want to update the test suite so it skips these tests rather than fail? Is that something you can tackle?

tmds commented 2 years ago

For the broader issue, source-build .NET should no longer use the RSA+SHA1 signing. @omajid, maybe we should create a source-build ticket for tracking that across the different repositories?

vcsjones commented 2 years ago

Is that something you can tackle?

I suppose that is the best thing we can do here. Yeah I can get a some pull requests in for this over next few days.

My bigger concern is CI protection. Similar to issues with OpenSSL 3, tests will probably be introduced with SHA1 signing unless we have a CI path that catches it.

tmds commented 2 years ago

My bigger concern is CI protection. Similar to issues with OpenSSL 3, tests will probably be introduced with SHA1 signing unless we have a CI path that catches it.

Our internal CI now runs the build with sha1 signature enabled, and then run tests with it disabled. We see the test failures now.

Until there is a public CI, I'll let you know when our internal CI fails, or try to make a PR to fix it.

omajid commented 2 years ago

Until there is a public CI

I tried adding it (https://github.com/dotnet/runtime/pull/65785) but it's blocked by https://github.com/dotnet/dotnet-buildtools-prereqs-docker/pull/581

vcsjones commented 2 years ago

@tmds @omajid one other thing I wanted to call out about the SHA1 signature disabling in RHEL9 and .NET is how this interacts with TLS.

Currently, this trivial .NET program will fail in RHEL9.

using System.Net.Http;

using HttpClient client = new HttpClient();
using HttpResponseMessage response = await client.GetAsync("https://pkgs.dev.azure.com");

The remote certificate is invalid because of errors in the certificate chain: NotSignatureValid

I suspect this is because .NET validates signatures on self-signed in trust stores, whereas OpenSSL does not normally do this. OpenSSL does support this with -check_ss_sig. You can reproduce this with the command line:

openssl s_client -check_ss_sig -strict -debug -connect pkgs.dev.azure.com:443

This will connect, but the trace will indicate the same error:

Verify return code: 7 (certificate signature failure)

I don't know if this is a "problem" or not, but since .NET opts in to this strictness check whereas OpenSSL doesn't normally do this, you might see additional reports about .NET Failing to connect to HTTPS endpoints where the root certificate is RSA-SHA1 signed.

omajid commented 2 years ago

Thanks for calling it out. We were aware of SHA-1 breaking microsoft.com and pkgs.dev.azure.com (https://github.com/dotnet/core/issues/6830). I see that there's a plan to replace with SHA-256: https://github.com/dotnet/core/issues/6830#issuecomment-1080919309, which might solve that specific issue, but the general issue still remains.

Is there a flag (env var?) to make .NET opt out of this strict checking?

vcsjones commented 2 years ago

@omajid

Hmmm. There are a couple of things at play here.

First, regarding OCSP. We always use SHA1 for OCSP. Even if CA/B servers add support for SHA1, we will continue to request SHA1, and I imagine most CAs will need to continue to support SHA1.

I don't know if we can easily switch to SHA256 here. For CA/B adhering trust stores, that would work. But there is probably a lot of enterprise PKI out there that has OCSP that may not be ready to switch to SHA256, and does not fall under the scope of CA/B.

We may want to open a separate issue to consider this migration.

The OCSP issue is separate from the "root" certificate check, but I will see if there are plans to switch to a certificate chain where the root in the trust store is SHA256 signed.

tmds commented 2 years ago

I don't know if this is a "problem" or not, but since .NET opts in to this strictness check whereas OpenSSL doesn't normally do this, you might see additional reports about .NET Failing to connect to HTTPS endpoints where the root certificate is RSA-SHA1 signed.

What happens when you use the url with wget/curl/...? Do they also use the 'strict' mode?

If they are happy with the certificate, .NET should be made less strict, and accept it certificate too.

vcsjones commented 2 years ago

What happens when you use the url with wget/curl/...? Do they also use the 'strict' mode?

They do not appear to, no. We explicitly enable it here:

https://github.com/dotnet/runtime/blob/424a09cb81c678fb1ba1c27211b80aba2de070ad/src/native/libs/System.Security.Cryptography.Native/pal_x509.c#L277

.NET should be made less strict, and accept it certificate too.

I think the reason why this was added is because this is what Windows does. Windows checks the signature on self-signed certificates. So this was done so that X509 chain building is more consistent across platforms, as I understand it.

@bartonjs may want to weigh in on this one when he returns.

tmds commented 2 years ago

X509_STORE_CTX_set_flags(ctx, X509_V_FLAG_CHECK_SS_SIGNATURE);

@bartonjs can we drop this strict mode? It causes .NET to reject certificates on RHEL9 which are accepted by system tools like curl and wget.

bartonjs commented 2 years ago

can we drop this strict mode?

No. It was added specifically because .NET on Windows was rejecting things that .NET on Linux wasn't. This is one of those places where I feel like ".NET is .NET" is the better compat answer.

tmds commented 2 years ago

No. It was added specifically because .NET on Windows was rejecting things that .NET on Linux wasn't. This is one of those places where I feel like ".NET is .NET" is the better compat answer.

That breaks .NET on RHEL9. If the system considers the certificate secure, .NET shouldn't reject it for Windows-compatibilty sake.

bartonjs commented 2 years ago

That breaks .NET on RHEL9.

To be brutally honest: no, it doesn't. It reveals that RHEL9's change to OpenSSL is based on incorrect assumptions. RHEL9 broke experiences .NET on RHEL9, not this line of code. (RHEL made a change to this area, .NET didn't, ergo the active verb "break" can only be attributed to RHEL).

If the system considers the certificate secure, .NET shouldn't reject it for Windows-compatibilty sake.

That's a reasonable statement, and I think I'm willing to accept a PR that makes it true. However, that same PR cannot let corrupted self-signed certificates that were provided via chain.ChainPolicy.ExtraStore complete a chain build without reporting them as NotSignatureValid, or a chain build where a corrupted self-signed certificate is provided as the target certificate of the chain.

Chain build up to a system cert, cert is corrupted or uses a signature algorithm that the platform itself no longer considers valid yet the cert is still considered trustworthy for some reason: Pass is acceptable
Chain build up to a cert provided in chain.ChainPolicy.CustomTrustStore with the CustomRootTrust trust mode: I like this less. But, since that could be being done with the anchor selected from existing system trust (paring it down for pinning), I think "matches above" is the correct answer (and means we don't have to distinguish types of trust in this scenario).
Chain build up to a cert provided by ExtraStore, that isn't unified with a trusted certificate, that has a corrupted signature: NotSignatureValid.
Chain build up to a cert provided by ExtraStore, that isn't unified with a trusted certificate, that has an unsupported algorithm: NotSignatureValid.
Chain build of a corrupted self-issued cert, not trusted by the current chain context: Must report NotSignatureValid.
Chain build of a self-issued cert that uses an algorithm (etc), not trusted by the current chain context: Must report NotSignatureValid.
Chain builds against self-issued certs that are part of the trust context: I'm ambivalent here. On the one hand, I want to report "this is broken, yo"; on the other it's a little weird when popping up the chain goes from success to failure.

tmds commented 2 years ago

That's a reasonable statement, and I think I'm willing to accept a PR that makes it true. However, that same PR cannot let corrupted self-signed certificates that were provided via chain.ChainPolicy.ExtraStore complete a chain build without reporting them as NotSignatureValid, or a chain build where a corrupted self-signed certificate is provided as the target certificate of the chain.

Personally, I'd like .NET to be as close to validating certificates as the other tools on the system. Not more strict, and not less strict.

@bartonjs or @vcsjones, can one of you look into making this change? I can take a shot at it, but being largely unfamiliar with the subject, it will take me more time to understand the code, and figure out what changes are needed to match these requirements.

omajid commented 2 years ago

Fedora is also planning to disable SHA1 soon: https://fedoraproject.org/wiki/Changes/StrongCryptoSettings3Forewarning1

bartonjs commented 2 years ago

Right now I don't have much availability... I'm focused on promised feature work before our "feature complete" preview (my portions are already a bit at-risk).

What's the urgency here / is it terrible if we don't fix it up before .NET 8?

Off the top of my head, what the PR needs to do, aside from make sure we have tests that cover the truth table above, is at/around https://github.com/dotnet/runtime/blob/9c34a588eb3d614a9ac4918141800dedda981f9a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/OpenSslX509ChainProcessor.cs#L693 (where we have a WorkingChain but haven't built the elements yet), add something like

if (the last item has the OpenSSL equivalent to NotSignatureValid set)
{
    if (the chain didn't finish with PartialChain or UntrustedRootAuthority)
    {
        Clear(lastFlag, NotSignatureValid);
    }
}

That way we're describing under what situations we don't report the error, rather than trying to figure out the conditions where we need to circle back and decide if we need to perform the check.

(It may make more sense in BuildElements, since there the difference between "last" and "last error" stand out)

tmds commented 2 years ago

What's the urgency here / is it terrible if we don't fix it up before .NET 8?

We'd like this fixed before users are adopting .NET 6 on RHEL 9. That means a fix+backport in the coming months.

Based on @vcsjones's input, I understand that as a workaround: we can source-build with OPENSSL_ENABLE_SHA1_SIGNATURES=1, and apply a single-line patch that removes the setting of X509_V_FLAG_CHECK_SS_SIGNATURE. This workaround would not meet the requirements set earlier.

We prefer an upstream fix over a workaround. @vcsjones unless you are available to pick this up, I will take a shot at it.

vcsjones commented 2 years ago

@vcsjones unless you are available to pick this up, I will take a shot at it.

I will make an attempt but time box it. If it turns out to be more work than expected I may need to defer (my time spent in this repo is purely voluntary).

tmds commented 2 years ago

Thanks. Even if you can't finish it, your effort will give me a head start.

dotnet / runtime