Skip to content

Conversation

@aw0lid
Copy link

@aw0lid aw0lid commented Jan 22, 2026

Summary
Following the discussion in dotnet/runtime#123401, this PR explicitly opts-in to assembly signing for the F# repository on non-Windows platforms.

Context
We are planning a global change in dotnet/arcade to disable full assembly signing by default on non-Windows systems to address RSA+SHA-1 signing issues on modern Linux distributions (e.g., RHEL 9/10).

As pointed out by @jkotas, F# requires strong-name signing for its bootstrapping process and repo-specific workflows. This change ensures that F# remains stable and continues to use the OpenSSL-based signing logic provided by Arcade, even after the global default is switched to off.

Changes
Modified Directory.Build.props to set FullAssemblySigningSupported to true when the OS is not Windows.

This acts as a local override that takes precedence over the upcoming Arcade SDK global defaults.

Related Issues
Foundational work for dotnet/runtime#123010

Referenced in dotnet/runtime#123401

Addresses concerns in #17451

/cc @jkotas @jkoritzinsky

Ensures that F# maintains assembly signing on Linux/macOS by explicitly opting in. This prevents potential breaks in repo-specific workflows (like bootstrapping) before the global Arcade SDK default is changed to 'off' for non-Windows platforms. Relates to dotnet/runtime#123010.
@github-actions
Copy link
Contributor

github-actions bot commented Jan 22, 2026

❗ Release notes required

@aw0lid,

Caution

No release notes found for the changed paths (see table below).

Please make sure to add an entry with an informative description of the change as well as link to this pull request, issue and language suggestion if applicable. Release notes for this repository are based on Keep A Changelog format.

The following format is recommended for this repository:

* <Informative description>. ([PR #XXXXX](https://github.com/dotnet/fsharp/pull/XXXXX))

See examples in the files, listed in the table below or in th full documentation at https://fsharp.github.io/fsharp-compiler-docs/release-notes/About.html.

If you believe that release notes are not necessary for this PR, please add NO_RELEASE_NOTES label to the pull request.

You can open this PR in browser to add release notes: open in github.dev

Change path Release notes path Description
src/Compiler docs/release-notes/.FSharp.Compiler.Service/10.0.200.md No release notes found or release notes format is not correct

All settings below can be overridden via CLI switches if needed. -->

<PropertyGroup>
<FullAssemblySigningSupported Condition="'$(OS)' != 'Windows_NT'">true</FullAssemblySigningSupported>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think that this is the right change.

Instead, it should be a fix in the F# compiler, so that project listed in the repro steps of #17451 compiles fine on RH with locked down crypto.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood, @jkotas. You're right—fixing the compiler logic to handle locked-down crypto gracefully is a much cleaner approach than a property override.

I’ve been looking into src/Compiler/AbstractIL/ilwrite.fs as we discussed earlier. It seems the issue arises when StrongNameSignatureSize is called.

Could you provide some guidance on the preferred way to fix this in the compiler? Should we ensure it skips the SHA-1 dependent signing calls entirely when certain conditions are met, or is there a specific abstraction in the F# codebase I should leverage for this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation in F# compiler should mirror the implementation in Roslyn. Check https://github.com/dotnet/roslyn/blob/main/src/Compilers/Core/Portable/PEWriter/SigningUtilities.cs and the callers of these methods.

Decoupled signer creation from CreateILModule to support modern RSA-based signature calculations, following the logic used in Microsoft.CodeAnalysis.
…peline

Updated AbstractIL.StrongNameSign.fsi by replacing legacy Open* methods with unified CreatePublicSigner and CreateKeyPairSigner factories. This aligns the interface with the modern Roslyn-based signing approach and prepares for improved RSA signature size calculations.
- Update SignatureSize calculation in ILStrongNameSigner to match Roslyn's logic.
- Use (keySize - 32) for public/delay signing to prevent PE file corruption.
- Maintain legacy signature size for full key-pair signing to ensure backward compatibility.
aw0lid added a commit to aw0lid/fsharp that referenced this pull request Jan 22, 2026
* Fix strong name signature size to align with Roslyn for public signing, addressing issues on Linux distributions with restricted crypto. ([PR dotnet#19236](dotnet#19236))
@aw0lid
Copy link
Author

aw0lid commented Jan 23, 2026

Closing this PR in favor of #19242.

Apologies for the noise here—I was initially focused on a build-property workaround. Following the guidance from @jkotas, I have now implemented the fix directly in the compiler's signing logic to ensure cross-platform compatibility, mirroring Roslyn's behavior.

All consolidated changes and the verified fix can be found in the new PR. Thanks for the patience and the guidance!

@aw0lid aw0lid closed this Jan 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants