dotnet/roslyn

Feature request: Support full-signing when running on CoreCLR.

Closed this issue ยท 35 comments

On corefx repo, we started using an Open key to fully sign some of our libraries. This key contains both public and private keys, and can be located here. This worked fine in Windows with the Full MSBuild and x-plat with Mono MSBuild, but it started failing x-plat once we moved to .Net Core MSBuild and the portable compilers. The exception we get is:

CSC : error CS7027: Error signing output with public key from file '.../corefx/Tools/Open.snk' -- Cannot marshal 'return value': Invalid managed [.../corefx/src/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj]

We would really like this issue to get addressed so that we are able to fully sign this libraries in non-Windows builds.

This may be an issue with my snk file parser, I'll take it.

Looks like this works in dotnet cli. Can you provide an isolated repro?

@agocke sorry it took so long to respond, sure there is an easy way to repro this exact scenario. If you remove this line from that file, and then try to build the latest corefx in Ubuntu or OSX you should get an exception like the one previously specified. If you need any more info, please let me know.

I looked through the corefx repo. As far as I can tell, you're not actually setting public sign anywhere in the repo, even after disabling that property. Checking the msbuild log, there is no /publicsign parameter being passed. The error message produced is thus the failure of the compiler to real sign on Linux/Mac.

I'm going to close this for now -- if new info is found then we can re-open.

@agocke I'm a little unclear what you are suggesting. Are you suggesting you cannot repro it or that we are just missing an extra flag we need to pass? Even if there is an extra property that isn't getting set it seems like the error message should be improved.

@weshaggard You're right about the error message -- I've filed #9288 to address that.

As to this bug, I understood this to be an issue with public sign -- full signing is not currently supported on coreclr. Did you want to re-categorize as a feature request?

Yes we should re-categorize it as a feature request because it is something I think we need to support.

@weshaggard Do you have a corefx bug we can follow? :) Roslyn currently PInvokes to the CLR for signing support and we'll continue to either use a CLR or CoreFX signing utility for full signing.

tmat commented

Re full signing - we have discussed this already. @jkotas wrote a prototype. I think we can productize that code in the new PE writer.

@agocke Is there any kind of ETA of when this feature might come online? We are hitting some issues in corefx given that this is not yet supported.

@joperezr No ETA, sorry.

@jaredpar Have we costed this at all?

@jaredpar - Any plan when this will be fixed? This works with project.json based projects, but doesn't for MSBuild projects.

@tmat is working on that. Will let him comment.

@jaredpar @tmat Any update on this? We still have some functionality disabled in our Unix builds for corefx which we'd like to turn back on.

Wondering if this will be in 2.0? i see a lot of milestone changes but nothing that specifies 2.0 rtm anymore

This will not be a part of the Net Core 2.0 work for non-Windows platforms. Plan is to have it in the release immediately following that.

Can we get a firm commitment for vnext? We get a lot of bugs on the sdk around it. I see advice all the time around switching to public signing on non-Windows without explaining the consequences. There's even been pressure to make public signing happen by default on non Windows. I'm growing tired of explaining why the only true fix is to implement the real feature in the compiler.

Re full signing - we have discussed this already. @jkotas wrote a prototype. I think we can productize that code in the new PE writer.

@tmat, @jkotas, where can I find the prototype? I can take a stab at a PR to 15.6.

tmat commented

@nguerrera We have fully functional signing in SRM already. What's missing is calling it from Roslyn, which we have never had a prototype of.

@tmat Ah, I see. I'll take a stab at making that call.

@nguerrera this is on my schedule for the post 15.3 release and work is assigned out.

@jaredpar OK. Even better, thanks! Looking forward to closure on this.

I have a solution with strong-singed assemblies. Assemblies FQ-names are used in InternalsVisibleTo attributes (to open internal types for test assemblies). It works fine on netcore2 on Windows. But on Linux it doesn't.

I cannot disable signing as InternalsVisibleTo requires only fully-qualified names. So assemblies should have strong names.

I build a test project with signing:

dotnet build /p:AssemblyOriginatorKeyFile=/home/..../Keys/Private.snk /p:SignAssembly=true

Build succeeded.

Then I run tests:

dotnet test --no-build

It fails:

Test run for /home/shrike/work/rnd/xfw3/src/Tests/bin/Debug/netcoreapp2.0/Croc.XFW3.Tests.dll(.NETCoreApp,Version=v2.0)
Microsoft (R) Test Execution Command Line Tool Version 15.3.0-preview-20170628-02
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...
[xUnit.net 00:00:00.9066978] Exception discovering tests from Croc.XFW3.Tests: System.BadImageFormatException: Could not load file or assembly 'Croc.XFW3.Tests, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null'. An attempt was made to load a program with an incorrect format.

File name: 'Croc.XFW3.Tests, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null'
   at System.Reflection.RuntimeAssembly._nLoad(AssemblyName fileName, String codeBase, Evidence assemblySecurity, RuntimeAssembly locationHint, StackCrawlMark& stackMark, IntPtr pPrivHostBinder, Boolean throwOnFileNotFound, Boolean forIntrospection, Boolean suppressSecurityChecks, IntPtr ptrLoadContextBinder)
   at System.Reflection.RuntimeAssembly.InternalLoadAssemblyName(AssemblyName assemblyRef, Evidence assemblySecurity, RuntimeAssembly reqAssembly, StackCrawlMark& stackMark, IntPtr pPrivHostBinder, Boolean throwOnFileNotFound, Boolean forIntrospection, IntPtr ptrLoadContextBinder)
   at System.Reflection.Assembly.Load(AssemblyName assemblyRef)
   at Xunit.Sdk.ReflectionAssemblyInfo..ctor(String assemblyFileName)

What is strange is that built assembly with key has no PublicKeyToken: "Could not load file or assembly 'Croc.XFW3.Tests, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null'". Why?

How should I build and test assemblies if I need strong-names?

How should I build and test assemblies if I need strong-names?

The best option is to use public signing when building in on CoreClr until we get full strong name signing enabled. A public signed assembly should run just fine on CoreClr.

Public signing is not an option for me, I maintain an opensource .net library which is published on nuget, linux is my primary dev environment and having to switch to a windows to build it is a pain.

Please don't make us linux users second class citizens.

jnm2 commented

How bad of a workaround would it be to override the build target with a custom task which makes use of Mono's implementation? https://github.com/mono/mono/blob/mono-5.4.1.6/mcs/class/Mono.Security/Mono.Security/StrongName.cs#L400

Will it be available this year or 2018 Q1? A brief status update would be highly appreciated.

Update on this: the work to do strong name signing on CoreClr is complete. It did not make the cut off for the Dev15.5 release but will be in the following release. It should be merged in one of the main branches in the next few days.

jnm2 commented

Thanks! Good to know because I was considering sinking some time into a workaround package.

strong name signing on CoreClr is complete

Couldn't find any reference to a pull-request, could you provide one?

It should be merged in one of the main branches in the next few days.

Reference to this issue in pull request would be great when that happens. Must be interesting to see work done that took about 23 months. :trollface:

Here is the PR that merged this into the post-dev15.5-contrib branch. This is "master" for our dev15.6 work.

#23061

๐Ÿ‘

For the record: the new cross-plat strong name signing provider works fine on Mono too ๐Ÿ˜„ (tested with Roslyn 2.7.0.62620)