Arcade: SignTool: StrongNameInfo should not imply delay signing by default

[natemcmaster]

Found as a result of trying to onboard ASP.NET Core to Azure Pipelines.

SignTool currently assumes that when a strong name key is specified via StrongNameInfo, that this means assemblies where delay-signed. As a result, it adds the <StrongName> metadata to the generated project it runs via MicroBuild.

Not all strong name keys we use are available in ESRP. For example, the MicrosoftAspNetCore key. This key is fully available in open source, so it should be unnecessary to have ESRP re-apply a strong name since C# will fully-sign assemblies locally.

Talking with @JohnTortugo about this, he proposed one workaround, which is take advantage of the 'resign' capability by changing StrongNameInfo to the path to a .snk file. This is not desirable either because this is unnecessary in most cases. The 'resign' capability is only necessary for .dll's that need to modify the IL after the C# compiler is done, which IMO is the exception not the rule to strong-name signing. Re-signing every aspnet .dll would be inefficient.

[natemcmaster]

FYI- this error surfaces in aspnet builds like this:

F:\vsagent\60\a\MicroBuild\Plugins\MicroBuild.Plugins.Signing.1.1.74\build\MicroBuild.Plugins.Signing.targets(43,5): error : Could not find the specified StrongName cert AspNetCore [F:\vsagent\60\s\obj\Signing\Round0.proj]

[tmat]

I'm not sure I understand the scenario - can you just not specify StrongNameSignInfo for this key?

[natemcmaster]

We did use StrongNameSignInfo, that's the problem. We'd like to use

<StrongNameSignInfo Include="AspNetCore" PublicKeyToken="adb9793829ddae60" CertificateName="Microsoft400" />

When we do, signing fails with the error I mentioned above.

[markwilkie]

cc/ @JohnTortugo

[JohnTortugo]

What I understand from @natemcmaster is that he wants to use PKT to specify authenticode certificate. Moreover, the files are already strong name signed, the strong name certificate isn't available in ESRP and also strong-name signing again would be inefficient.

[tmat]

That can be achieved by using FileSignInfo with PublicKeyToken attribute.

The purpose of StrongNameSignInfo is to strong-name real-sign the assembly. If it already has real-signed strong-name then it shouldn't be listed in StrongNameSignInfo.

[tmat]

BTW, I understand there might be a confusion with StrongNameSignInfo having CertificateName attribute. I specifically proposed removing that but that hasn't been done yet.

[natemcmaster]

Yeah, this is confusing. When I was reading code, it appeared as if StrongNameSignInfo purpose was to say "sign all PE files with a given PKT with this Authenticode cert". FileSignInfo is close, but not exactly the same because it also requires specifying the file name.

[JohnTortugo]

Yeah, AFAIU FileSignInfo will require the file name to be specified. What I understand nate wants is PKT -> AuthenticodeCertificate; independent of name, etc.

I also don't like the StrongNameSignInfo name.. still, the change that I was going to propose to meet @natemcmaster scenario was to let StrongNameSignInfo->StrongName (ItemSpec) accepts "None" which would mean don't strong name this file. So essentially StrongNameSignInfo will be used to configure information for strong name & authenticode interchangeably.

[natemcmaster]

That would work for me.

[tmat]

I thought ASP.NET is listing all files explicitly via FileSignInfo, isn't that the case?

I'd then suggest StrongNameSignInfo to be changed to <PublicKeyTokenSignInfo Include="{PKT}" StrongName="{cert}|None|empty" CertificateName="{cert}|None|empty">

[JohnTortugo]

Only thing that I'm worried is about changing the name of the parameter at this point - there are a few people using the sign tool package directly. Any suggestion how should we approach that?

[natemcmaster]

I thought ASP.NET is listing all files explicitly via FileSignInfo, isn't that the case?

We were doing that and decided to try using the PKT/StrongNameSignInfo approach. If that's not going to work, we'll go back to listing files explicitly by name.

[JohnTortugo]

I'm going to setup a PR with the changes; excluding the property rename.

[tmat]

Yes, that needs to be done carefully.

I think the best way to fix up existing repos is to report an error in the Sign.proj if <StrongNameSignInfo> is item group is non-empty. Something like:

<Error Text="StrongNameSignInfo is obsolete. Please use PublicKeyTokenSignInfo. See documentation: http://..." Condition="'@(StrongNameSignInfo)' != ''"/>

Then the PR that updates repos to new Arcade SDK will fail and we will fix them up to use PublicKeyTokenSignInfo.

[JohnTortugo]

What about packages that call the SignToolTask directly? Maybe we should add a warning to the signtool saying that they should switch to use the new parameter and that StrongNameSignInfo will be soon removed.

[tmat]

If we rename the parameter they will fail (which is what we want).

[tmat]

What we don't want is silently stop signing stuff or changing the cert we use.

[JohnTortugo]

If you believe that this approach is good I'm also OK with it. I'll created another issue to address this, though: https://github.com/dotnet/arcade/issues/1919