-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Infrastructure: Disable full assembly signing by default on non-Windows platforms #123401
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
I think we want to change the default for all .NET runtime and SDK projects in https://github.com/dotnet/arcade/ . @dotnet/runtime-infrastructure Do you agree? |
|
I see the CI failures on Linux. It seems some tests/components still strictly expect a strong name. I agree with @jkotas that moving this to Arcade is the right way to go. It feels like a more robust architectural solution rather than just a local patch in the runtime. By fixing it in Arcade, we ensure a consistent 'build law' across all .NET products and handle Public Signing defaults more gracefully. That said, would you prefer I investigate a Public Signing middle-ground here to unblock the CI, or should we focus our efforts on the Arcade migration instead? |
|
The error is: Looks like a problem with F# compiler (F# compiler lives in http://github.com/dotnet/fsharp so the fix may need to be done there. Could you please investigate that? |
I’ve been looking into src/Compiler/AbstractIL/ilwrite.fs in the F# repo, and it appears that the logic in writeBinaryAux might be automatically setting up a signer if a PublicKey is present, even without an explicit signer. This likely leads to the call in signImage where s.SignStream is invoked, which potentially triggers the StrongNameSignatureSize error. |
|
It would be best to fix the F# compiler. To make progress, it is ok to do a temporary local workaround for tests until the fixed F# compiler propagates to the toolset that the repo is compiled with. |
|
There is an existing issue for the F# compiler here dotnet/fsharp#17451 This issue was found when the Red Hat folks were using The |
Disabling full-signing seems OK, but we need to make sure to replace it with public signing, not delay signing. There are various tools and utilities out there that balk at delay-signed binaries. We should also confirm that SHA-1 is only needed in the implementation, not in generating signatures. If it's needed for generating signatures then I don't think disabling full signing will actually remove all calls to SHA1. |
|
SHA1 alone is not blocked (yet?). RSA+SHA-1 is the blocked algorithm that needs to be avoided. |
| <PropertyGroup> | ||
| <!-- Default any assembly not specifying a key to use the Open Key --> | ||
| <StrongNameKeyId>Open</StrongNameKeyId> | ||
| <FullAssemblySigningSupported Condition="'$(FullAssemblySigningSupported)' == '' and '$(OS)' != 'Windows_NT'">false</FullAssemblySigningSupported> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be better to have this in the top-level Directory.Build.props file so that it applied to the whole repo?
|
@dotnet/runtime-infrastructure What is the best way to ensure that this does not break dotnet/dotnet official build? Or should we just give this a try and revert if it breaks the dotnet/dotnet official build? |
|
I have a request out to create the new VMR validation pipeline we talked about, but in the meantime the best way to validate would be with a PR into the VMR with the changes. |
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
|
If we decide to move forward with a VMR PR for validation as @jkoritzinsky suggested, I was wondering if it might make more sense to implement this fix in Arcade instead of a local override in the Runtime. |
|
Yes, I agree. It may be a good idea to fix the F# compiler first so that the global setting does not cause problems for repo-specific workflows. VMR validation will catch problems in end-to-end official builds, etc. I won't catch problems in repo-specific workflows. |
|
Exactly. |
|
Update: As discussed, I have opened a PR in the F# repository (dotnet/fsharp#19236) to explicitly opt-in to assembly signing. This ensures F# remains stable before we move the global default in Arcade. Once the F# PR is merged and the toolset is updated, I will proceed with the Arcade PR to change the default to false for all non-Windows platforms. I'll keep this PR as a reference for now, but the final fix will be delivered via Arcade. Thanks for the guidance @jkotas! |
|
Quick update: The fix for the F# compiler root cause is ready and all CI checks are passing in dotnet/fsharp#19242. Once that's merged and the toolset is updated, I'll proceed with the Arcade PR to change the global default as discussed. Keeping this PR open for reference until we transition to the Arcade/VMR phase |
Summary
This PR disables full assembly signing by default for non-Windows builds in src/libraries. This change addresses build failures on Linux distributions (like RHEL 9/10 and CentOS Stream) where SHA-1 is disabled for security reasons, preventing RSA+SHA-1 strong name signing.
Context
As reported in #123010, the runtime build fails on modern Linux environments because the build system attempts to use SHA-1 for assembly signing. Following the suggestion from @jkotas, instead of targeting specific Linux distros, we are disabling full signing by default on all non-Windows platforms. Full signing is typically only required for official Microsoft builds.
Changes
Modified src/libraries/Directory.Build.props to set FullAssemblySigningSupported to false when the OS is not Windows_NT.
Ensured that users can still manually override this property if needed by using a conditional check.
Impact
Linux/macOS: Improved developer experience by preventing "invalid digest" errors during local builds.
Windows: No change in behavior; full signing remains enabled by default.
Fixes #123010