Fix StrongNameSignatureSize failure on Linux when using full RSA keys#19242
Conversation
❗ Release notes required
|
|
All functional CI checks have passed, including the FsharpPlus regression tests. @T-Gro @jkotas I've updated the release notes with the PR link. Although the check_release_notes bot is currently failing due to an infrastructure authentication error (401), I have manually verified the changes are correct in the file. This PR is now ready for final review. It addresses the root cause in the compiler's signing logic for non-Windows platforms as discussed. Thank you! |
|
Hi @T-Gro, most CI checks are now green, including the long-running FsharpPlus regression tests and the VS release build. |
|
@aw0lid the failures seem consistent after a rerun. you can take a look at the details in the Azure DevOps test view: https://dev.azure.com/dnceng-public/public/_build/results?buildId=1265770&view=ms.vss-test-web.build-test-results-tab&runId=35321986&resultId=103476&paneView=attachments |
Thanks @akoeplinger for the logs. I've just force-pushed an update to address those failures. This should resolve the issues in the Desktop shards. Ready for another run |
|
The failing test was related (it tests compilation with --keyfile:sha512full.snk ). https://github.com/search?q=repo%3Adotnet%2Ffsharp%20signedtest&type=code |
Exactly! I noticed that signedtest-4 was failing specifically due to the sha512full.snk key file. |
|
Exactly! After reviewing the test matrix you shared, it's clear that signedtest-4 (sha512full.snk) and others like the SHA-1024 variants are failing because the RSA Magic number (RSA2) is shifted much further than the standard offsets due to extended metadata. |
|
Update Publishing build artifacts failed with an error: Error: Not found SourceFolder: D:\a_work\1\s\artifacts\log\Release Since this job failed due to missing artifacts rather than test or logic errors, could someone please trigger a re-run for fsharpqa_release? |
|
I checked that looks related to your change. |
|
Hi, References used in the implementation:
Desktop Shard 2 consistently fails on tests involving full RSA key pairs (e.g. Given that:
I’m unsure whether Desktop Shard 2 reflects legacy behavior that is still expected to be preserved, or a known limitation of the older signing pipeline. I’d appreciate guidance on how you’d like to proceed:
Happy to adjust the approach based on your recommendation |
|
Why would it be treated as legacy? Other legs pass because they do not have the same test coverage - the same test is not run on other OSes, this is the only one exercising this particular signing logic with a .snk argument. "Legacy desktop" is still a supported product. this compiler must work both on modern .NET as well as .NET Framework. |
Fair point, @T-Gro. If it's a supported target and was passing before, we must maintain that. I'll take a deeper look into the exact signature size calculation for full .snk files on .NET Framework to ensure we match the legacy SignFile requirements. I'll update the PR as soon as I align the logic with the expected padding/alignment for those cases. |
|
Can you please update PR commentary to explain what was done and why? |
|
Gentle ping in case this fell through the cracks |
|
@aw0lid : Just 2 things unclear to me related to conditioning this, otherwise this is good to go. |
#11887) Fix StrongNameSignatureSize failure on Linux This fix handles full RSA key pairs on non-Windows platforms by attempting both Public and KeyPair imports, mirroring Roslyn's behavior. Final code formatting fix for signatureSize Align signature size logic with Roslyn and fix cross-platform key import Fix cross-platform strong name signing by manually parsing RSA key blobs fix: move release notes to 300
|
@T-Gro You're right — the conditions were unnecessary. I verified this with a clean CI run after removing them, and all builds passed successfully. I've removed the conditions from both the .fsproj and the .fs file to simplify the project configuration. Thanks for pointing it out. |
* Type checker: recover on argument/overload checking (#19314) * [main] Update dependencies from dotnet/arcade (#19333) * Update dependencies from https://github.com/dotnet/arcade build 20260219.2 On relative base path root Microsoft.DotNet.Arcade.Sdk From Version 10.0.0-beta.26117.6 -> To Version 10.0.0-beta.26119.2 * Update dependencies from https://github.com/dotnet/arcade build 20260223.2 On relative base path root Microsoft.DotNet.Arcade.Sdk From Version 10.0.0-beta.26117.6 -> To Version 10.0.0-beta.26123.2 --------- Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> * [main] Source code updates from dotnet/dotnet (#19343) * Backflow from https://github.com/dotnet/dotnet / 854c152 build 302768 [[ commit created by automation ]] * Update dependencies from build 302768 No dependency updates to commit [[ commit created by automation ]] * Backflow from https://github.com/dotnet/dotnet / 51587e2 build 302820 [[ commit created by automation ]] * Update dependencies from build 302820 No dependency updates to commit [[ commit created by automation ]] --------- Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: Tomas Grosup <Tomas.Grosup@gmail.com> * Update image used for insert into VS step (#19357) * Fix :: FAR :: Remove corrupted .ctor symbol reference (#19358) * DotNetBuildUseMonoRuntime stackguard (#19360) * Fsharp.Core :: {Array;List;Set;Array.Parallel} partitionWith (taking Choice<T,U> partitioner) (#19335) * Sort out some outstanding issues after xUnit upgrade (#19363) * Improve collection comparison diagnostics in tests (#19365) Added shouldBeEqualCollections to Assert.fs for detailed collection comparison, including reporting missing/unexpected items and positional differences. Updated Project25 symbol uses test to use this helper and print actual/expected values for easier debugging. * Enhance the compiler to produce a FS0750 error on let! or use! outside a CE (#19347) * Update to latest .NET 10 SDK patch (#19350) * Feature: `#elif` preprocessor directive (#19323) * Support #exit;; as alias to #quit;; in fsi (#19329) * Checker: prevent reporting optional parameter rewritten tree symbols (#19353) * Improve static compilation of state machines (#19297) * Add FSC compiler options tests (#19348) * Update dependencies from https://dev.azure.com/dnceng/internal/_git/dotnet-optimization build 20260226.1 (#19366) On relative base path root optimization.linux-arm64.MIBC.Runtime , optimization.linux-x64.MIBC.Runtime , optimization.windows_nt-arm64.MIBC.Runtime , optimization.windows_nt-x64.MIBC.Runtime , optimization.windows_nt-x86.MIBC.Runtime From Version 1.0.0-prerelease.26117.2 -> To Version 1.0.0-prerelease.26126.1 Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> * Fix Get-PrBuildIds.ps1 reporting SUCCESS while builds in progress (#19313) * Fix flaky Project25 TP test by replacing FSharp.Data NuGet with TestTP (#19364) (#19373) Replace FSharp.Data (resolved via NuGet at runtime) with the built-in TestTP type provider for the Project25 symbol API tests. This fixes non-deterministic test failures on Linux CI caused by Directory.GetFiles returning DLLs in random inode order on ext4, which varied whether the FSharp.Data namespace was tagged as 'provided' or not. Changes: - Replace 70-line NuGet restore/staging setup with 2-line TestTP reference - Update source to use ErasedWithConstructor.Provided.MyType instead of FSharp.Data.XmlProvider - Replace brittle exact-match symbol list with targeted assertions for provided types, methods, and namespaces - Remove FactSkipOnSignedBuild (TestTP is always available after build) - Rename test variables to match the types being tested Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add automatic merge flow from main to feature/net11-scouting (#19283) * Optimize Set.intersect symmetry and add release notes (#19292) Co-authored-by: Ahmed <w0lf@192.168.1.8> * Fix strong name signature size to align with Roslyn for public signing (#11887) (#19242) * Use culture-independent `IndexOf` in interpolated string parsing (#19370) * Rename "inline hints" to "inlay hints" (#19318) * Rename "inline hints" to "inlay hints" for LSP consistency Aligns F#-owned terminology with LSP and VS Code conventions. The Roslyn ExternalAccess types (IFSharpInlineHintsService, etc.) are left unchanged as they are owned by Roslyn. Fixes #16608 * Add release note for inline-to-inlay hints rename --------- Co-authored-by: Tomas Grosup <Tomas.Grosup@gmail.com> * FCS: capture additional types during analysis (#19305) * remove duplicate FlatErrors file (#19383) * Update dependencies from https://github.com/dotnet/arcade build 20260302.1 (#19379) [main] Update dependencies from dotnet/arcade * [main] Update dependencies from dotnet/msbuild (#19021) * Update dependencies from https://github.com/dotnet/msbuild build 20251021.3 On relative base path root Microsoft.Build , Microsoft.Build.Framework , Microsoft.Build.Tasks.Core , Microsoft.Build.Utilities.Core From Version 18.1.0-preview-25515-01 -> To Version 18.1.0-preview-25521-03 * Update dependencies from https://github.com/dotnet/msbuild build 20251023.2 On relative base path root Microsoft.Build , Microsoft.Build.Framework , Microsoft.Build.Tasks.Core , Microsoft.Build.Utilities.Core From Version 18.1.0-preview-25515-01 -> To Version 18.1.0-preview-25523-02 * Update dependencies from https://github.com/dotnet/msbuild build 20251024.3 On relative base path root Microsoft.Build , Microsoft.Build.Framework , Microsoft.Build.Tasks.Core , Microsoft.Build.Utilities.Core From Version 18.1.0-preview-25515-01 -> To Version 18.1.0-preview-25524-03 * Update dependencies from https://github.com/dotnet/msbuild build 20251027.5 On relative base path root Microsoft.Build , Microsoft.Build.Framework , Microsoft.Build.Tasks.Core , Microsoft.Build.Utilities.Core From Version 18.1.0-preview-25515-01 -> To Version 18.1.0-preview-25527-05 * Update dependencies from https://github.com/dotnet/msbuild build 20251027.6 On relative base path root Microsoft.Build , Microsoft.Build.Framework , Microsoft.Build.Tasks.Core , Microsoft.Build.Utilities.Core From Version 18.1.0-preview-25515-01 -> To Version 18.1.0-preview-25527-06 * Update dependencies from https://github.com/dotnet/msbuild build 20251104.4 On relative base path root Microsoft.Build , Microsoft.Build.Framework , Microsoft.Build.Tasks.Core , Microsoft.Build.Utilities.Core From Version 18.1.0-preview-25515-01 -> To Version 18.1.0-preview-25554-04 * Bump dependency versions in Version.Details.props Updated package versions for several dependencies. --------- Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: Tomas Grosup <Tomas.Grosup@gmail.com> * Add fsharp-release-announcement agent (#19390) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix Seq.empty rendering as "EmptyEnumerable" in serializers (#19317) * Update FileContentMapping.fs (#19391) * Fix flaky help_options test: restore enableConsoleColoring after mutation (#19385) * Initial plan * Fix flaky help_options test by saving/restoring enableConsoleColoring global The `fsc --consolecolors switch` test mutates the global `enableConsoleColoring` via `--consolecolors-`. When help_options tests run after this test, the help output says "(off by default)" instead of "(on by default)", causing baseline mismatches. Fix: save and restore `enableConsoleColoring` around the test. Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com> Co-authored-by: Tomas Grosup <Tomas.Grosup@gmail.com> * isolate checker (#19393) Isolate type-provider tests with dedicated FSharpChecker Introduce Project25.checker to avoid shared state races in type-provider tests. All relevant test cases now use this dedicated instance, improving test isolation, reliability, and determinism without changing test functionality. * Introduce CallRelatedSymbolSink to avoid affect name resolution with related symbols (#19361) * Add RelatedSymbolUseKind flags enum and separate sink for related symbols Address auduchinok's review comments on PR #19252: related symbols (union case testers, copy-and-update record types) are now reported via a separate NotifyRelatedSymbolUse sink instead of abusing NotifyNameResolution. - Add [<Flags>] RelatedSymbolUseKind enum (None/UnionCaseTester/CopyAndUpdateRecord/All) - Add NotifyRelatedSymbolUse to ITypecheckResultsSink interface - Refactor RegisterUnionCaseTesterForProperty to use the new sink - Refactor copy-and-update in TcRecdExpr to use CallRelatedSymbolSink - Add ?relatedSymbolKinds parameter to GetUsesOfSymbolInFile (default: None) - Wire up VS FAR to pass relatedSymbolKinds=All - Write related symbols to ItemKeyStore for background FAR - Update semantic classification tests: tester properties now classified as Property (not UnionCase) since they're no longer in capturedNameResolutions * Add release notes for FSharp.Compiler.Service 11.0.100 --------- Co-authored-by: Eugene Auduchinok <eugene.auduchinok@gmail.com> Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: Tomas Grosup <Tomas.Grosup@gmail.com> Co-authored-by: Adam Boniecki <20281641+abonie@users.noreply.github.com> Co-authored-by: Jakub Majocha <1760221+majocha@users.noreply.github.com> Co-authored-by: Evgeny Tsvetkov <61620612+evgTSV@users.noreply.github.com> Co-authored-by: Youssef Victor <youssefvictor00@gmail.com> Co-authored-by: Bozhidar Batsov <bozhidar@batsov.dev> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Ahmed Waleed <ahmedwalidahmed.0@gmail.com> Co-authored-by: Ahmed <w0lf@192.168.1.8> Co-authored-by: Brian Rourke Boll <brianrourkeboll@users.noreply.github.com> Co-authored-by: Apoorv Darshan <ad13dtu@gmail.com> Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com>
The F# compiler produces assemblies with malformed public key blobs when public-signing with full key pairs on Unix (dotnet/fsharp#19441). Previously SignAssembly was only disabled for source-build, but the fix for dotnet/fsharp#19242 exposed this on all Unix legs. Since this is a non-shipping test project, disable signing unconditionally as a workaround. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
Fix strong-name signature size calculation to correctly handle full RSA key blobs (
.snk) across all supported signing paths.Background
The previous implementation of
signatureSizeassumed a fixed public-key-only RSA blob layout and advanced the reader by skipping headers at hardcoded positions.This worked for public-only keys but failed when a full RSA key pair was supplied, as the blob layout no longer matched the assumed structure, leading to an incorrect signature size and signing failures.
What changed
The
signatureSizeimplementation was updated to remove fixed layout assumptions and hard failures:RSAPUBKEYheader.RSAPUBKEYheader can be located, the historical default signature size (128 bytes) is used as a fallback.Important
No cryptographic operations were changed, and no new signing paths were introduced.
Why this works
The strong-name signature size depends only on the RSA modulus size, independent of whether the key blob contains public-only data or full private key material.
By locating and reading the modulus metadata directly—rather than assuming a single blob layout—the compiler now correctly handles both public-only keys and full RSA key pairs while preserving legacy behavior.
Scope
signatureSizecalculation logic.Fixes #17451