Skip to content
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

SignFile task using dotnet cli tries to use signtool.exe from wrong location #6788

Open
Zastai opened this issue Aug 26, 2021 · 18 comments
Open
Labels
bug ClickOnce Issues related to ClickOnce, including the SignFile task.
Milestone

Comments

@Zastai
Copy link
Contributor

Zastai commented Aug 26, 2021

Issue Description

Thanks to issue #6098, the SignFile task is now enabled in dotnet msbuild 16.11.

However, it does not actually seem to work.

I was under the impression that SignFile did the signing itself, using core framework functionality.; however, it turns out it wants to use signtool.exe, but does not actually locate it correctly.

So when using SignFile as part of dotnet build, I get:

error MSB3482: An error occurred while signing: SignTool.exe was not found at path xxx\signtool.exe.

where xxx is the project directory. When building a solution it wants signtool.exe to be present in every project's directory separately.

MSBuild should be able to locate it properly; I had my own lookup in place before, using

    <SignToolPath Condition=" '$(SignToolPath)' == '' and '$(WindowsSdkVerBinPath)' != '' and '$(PROCESSOR_ARCHITECTURE)' == 'AMD64' and Exists('$(WindowsSdkVerBinPath)x64\signtool.exe') ">$(WindowsSdkVerBinPath)x64\signtool.exe</SignToolPath>
    <SignToolPath Condition=" '$(SignToolPath)' == '' and '$(WindowsSdkVerBinPath)' != '' and '$(PROCESSOR_ARCHITECTURE)' == 'AMD64' and Exists('$(WindowsSdkVerBinPath)x86\signtool.exe') ">$(WindowsSdkVerBinPath)x86\signtool.exe</SignToolPath>
    <SignToolPath Condition=" '$(SignToolPath)' == '' and '$(WindowsSdkVerBinPath)' != '' and '$(PROCESSOR_ARCHITECTURE)' == 'x86'   and Exists('$(WindowsSdkVerBinPath)x86\signtool.exe') ">$(WindowsSdkVerBinPath)x86\signtool.exe</SignToolPath>

and that would have led to using C:\Program Files (x86)\Windows Kits\10\bin\10.0.19041.0\x64\signtool.exe just fine.

Steps to Reproduce

  • create a simple project:
    dotnet new classlib -n CodeSigning
    cd CodeSigning
  • generate a self-signed certificate
    New-SelfSignedCertificate -Type CodeSigningCert -Subject CN=CodeSigning -CertStoreLocation Cert:\CurrentUser\My
    
    This will produce output like:
       PSParentPath: Microsoft.PowerShell.Security\Certificate::CurrentUser\My
    
    Thumbprint                                Subject              EnhancedKeyUsageList
    ----------                                -------              --------------------
    B77064C7175EF732F534B8D28C337CA2FB87E9D2  CN=CodeSigning       Code Signing
    
    make a note of that thumbprint value; it's needed in the next step.
  • Set up code signing in the project by adding this target to CodeSigning.csproj:
    <Target Name="_SignAssemblies" AfterTargets="Compile" DependsOnTargets="CreateSatelliteAssemblies;$(RazorCompileDependsOn)">
      <PropertyGroup>
        <SigningCertificate>B77064C7175EF732F534B8D28C337CA2FB87E9D2</SigningCertificate>
      </PropertyGroup>
      <ItemGroup>
        <_AssembliesToSign Include="$(IntermediateOutputPath)$(TargetFileName)" />
        <_AssembliesToSign Include="@(IntermediateSatelliteAssembliesWithTargetPath)" />
        <_AssembliesToSign Include="@(RazorIntermediateAssembly)" />
      </ItemGroup>
      <Message Importance="high" Text="Signing assemblies: @(_AssembliesToSign)" />
      <SignFile SigningTarget="%(_AssembliesToSign.Identity)" CertificateThumbprint="$(SigningCertificate)" />
    </Target>
    
    making sure that the value of $(SigningCertificate) is the thumbprint of the certificate you generated.
  • build using msbuild; this should succeed, with output including
    _SignAssemblies:
      Signing assemblies: obj\Debug\net5.0\CodeSigning.dll
    CopyFilesToOutputDirectory:
    
  • build using dotnet build -v:n; this will fail with an error like
           _SignAssemblies:
           Signing assemblies: obj\Debug\net5.0\CodeSigning.dll
       1>...\CodeSigning\CodeSigning.csproj(17,5): error MSB3482: An error occurred while signing: SignTool.exe was not found at path ...\CodeSigning\signtool.exe.
    

Expected Behavior

The SignFile task works, signing the assemblies, when using either dotnet build or msbuild.

Actual Behavior

The SignFile task works only when using msbuild.

Analysis

The SignFile task implementation does not seem to locate SignTool correctly when using dotnet build.
(And in addition, I thought that it was doing the signing itself (which would potentially make it work on Linux as well, which would be very convenient for CI/CD scenarios), using corefx functionality, not using an external utility from a Windows Kit.)

Versions & Configurations

Tested using VS2019 16.11.2 and .NET SDK 5.0.400, i.e. MSBuild 16.11.0.36601 on x64 Windows.

I'm not sure how to set up a certificate on Linux (no New-SelfSignedCertificate in pwsh there), but that would only matter if SignFile did the signing itself and not via SignTool.

@Zastai Zastai added bug needs-triage Have yet to determine what bucket this goes in. labels Aug 26, 2021
@Zastai
Copy link
Contributor Author

Zastai commented Aug 26, 2021

When I have time, I'll jump in the code and file a PR if needed.
Build.Tasks.Core uses the exact same source for SignFile as Build.Tasks, so this should be easy to resolve.
I'm annoyed at myself for not spotting this issue when enabling SignFile in the first place.

@benvillalobos
Copy link
Member

Thanks for filing the issue!

This looks to be the culprit:

https://github.com/dotnet/msbuild/blob/main/src/Tasks/ManifestUtil/SecurityUtil.cs#L784


After trying the repro, I can't actually reproduce it?

msbuild --version
16.11.0-preview-21351-03+1d7ed8e3d

dotnet --version
6.0.100-preview.7.21379.14

Oooh interesting. I see the issue when I global.json myself down to 5.0.400.

$(SignToolPath) has no value in both binlogs, and if I set it manually then it still fails in 5.0.400.

@sujitnayak any ideas what could be happening here?

@sujitnayak
Copy link
Contributor

sujitnayak commented Aug 27, 2021

signtool.exe resolves to the path of the tool installed by the ClickOnceBootStrapper MSI.

It will be at %ProgramFiles(x86)%\Microsoft SDKs\ClickOnce\SignTool.

This assumes you have VS installed on the machine since the CO bootstrapper MSI is installed by VS. I think the VS Build Tools SKU also install this MSI so at the minimum you need that installed.

@Zastai
Copy link
Contributor Author

Zastai commented Aug 27, 2021

(SignToolPath is my own variable, which I used because SignFile did not exist in core msbuild before 16.11. I only mentioned it to indicate that signtool is present and can be found using standard msbuild properties).

I do have VS installed - it was the VS update that gave me SDK 5.0.400 to begin with.
If the intent is for it to resolve to a VS component, then that seems to be failing. A diagnostic message for that case might be in order ("could not find SignTool as part of the ClickOnce bootstrapper; assuming it is provided by the project" perhaps?).
I'm also wondering why it would not (also) resolve to the one from the Windows Kit, if present, given that MSBuild provides properties pointing to the kit.

@sujitnayak
Copy link
Contributor

sujitnayak commented Aug 27, 2021

Can you clarify what version of Visual Studio you have installed?

On x64 Windows, the task reads the Path value from under the HKEY_LOCAL_MACHINE\SOFTWARE\WOW6432Node\Microsoft\ClickOnce\SignTool key to get the location of the tool. Could you confirm what you have there?

There was a bug in the task where x64 processes could not read the value becuase they looked in the non-WOW6432 location of the registry. We've fixed the bug in VS 2022 Preview but if you using a x64 dotnet, then you could be hitting this issue.

We could improve the error message but this is the first case where someone outside of the VS team is using this task and the task is failing in an unexpected way.

@Zastai
Copy link
Contributor Author

Zastai commented Aug 28, 2021

Visual Studio Enterprise 2019 16.11.2

HKEY_LOCAL_MACHINE\SOFTWARE\WOW6432Node\Microsoft\ClickOnce\SignTool exists and contains C:\Program Files (x86)\Microsoft SDKs\ClickOnce\SignTool\. That path exists and does contain signtool.exe.

So it looks like it should find it, but as you say, it'll likely be because dotnet.exe is running in 64-bit mode and does not see that registry key.

Note that if SignFile requires Visual Studio (Build Tools or otherwise) to be installed, that kind of defeats the purpose of enabling it. The whole point for me was to enable builds using CI without requiring installs, i.e. just doing a zip-based deployment of a .NET SDK. Any reason why signtool can't simply ship with Windows versions of the .NET SDK?

@sujitnayak
Copy link
Contributor

sujitnayak commented Aug 30, 2021

Note that if SignFile requires Visual Studio (Build Tools or otherwise) to be installed, that kind of defeats the purpose of enabling it.

SignFile task as it is implemented right now is meant for signing ClickOnce binaries/manifest and was not really designed to be used as a generic task for signing ad-hoc files.

@sujitnayak
Copy link
Contributor

Any reason why signtool can't simply ship with Windows versions of the .NET SDK?

signtool.exe is part of the Windows SDK and not the .NET SDK. So it ships with Windows SDKs that are installed under C:\Program Files (x86)\Windows Kits.

Currently SignFile task cannot find it in the Windows SDK so it falls back to the one installed by the ClickOnce MSI. The reason it cannot find signtool.exe in the Windows SDK is because we look for it in the Windows 8.1 SDK folder instead of Windows 10 SDK location and VS 2019 does not install Windows 8.1 SDK.

@benvillalobos do you know why we have VS version set to 150 (instead of 160) for .NET version 4.8 here:

CreateDotNetFrameworkSpecForV4(dotNetFrameworkVersion48, visualStudioVersion150),

@benvillalobos
Copy link
Member

benvillalobos commented Sep 1, 2021

I'm not sure. @Forgind do you remember the context of choosing 150 specifically here?

I see the intention, that 160 spec is new VisualStudioSpec(visualStudioVersion160, "NETFXSDK\\{0}", "v10.0", "InstallationFolder", new [] and specifically searches w10.

Wouldn't this have broken any other tools we search for in that Windows SDK path? Or are we just happening to find the tools and they work despite being from a previous version?

Edit: Looking at git blame, that line was updated, 160 didn't exist as a variable. Looks like an oversight and we should update it.

@Forgind
Copy link
Member

Forgind commented Sep 1, 2021

I think it needs to be 15 because that's the version of our assembly (15.1.0.0). We intend that to be constant going forward.

@benvillalobos
Copy link
Member

benvillalobos commented Sep 2, 2021

Team Triage: Since SignFile is working for its intended scenarios, we don't intend to investigate this further at this time.

--

Have you tried installing the Windows 8.1 SDK and running this scenario again? Presumably it worked in my case because I have it installed.

@benvillalobos benvillalobos removed the needs-triage Have yet to determine what bucket this goes in. label Sep 2, 2021
@benvillalobos benvillalobos added this to the Backlog milestone Sep 2, 2021
@Zastai
Copy link
Contributor Author

Zastai commented Sep 2, 2021

I suppose it would be nice to document the task better then. It is not described as "an internal task intended for signing click-once artefacts only. Requires either Visual Studio and/or a Windows 8.1 SDK to be installed". Instead, it suggests it performs general authenticode signing, but then does not, and has additional dependencies that make sense for msbuild.exe but not dotnet msbuild.

It certainly feels like there is little to no support for Authenticode in a .NET context - if that's the intent, fine.

@benvillalobos It thought it worked in your case because it found the ClickOnce install by looking in the right registry key when run in 64-bit mode, which is apparently a fix in 17 but not 16.11.

It makes no sense to me to have to install an obsolete SDK to make this work. We're signing all built assemblies, not just netfx, so I'm not sure I follow the reasoning that because .NET Framework 4.8 may be associated with VS15, that's all that needs to be looked at.

I guess I'll just look at including signtool.exe in our build support package, so we can have it available that way without needing a VS/WindowsKit installed.

@airbreather
Copy link

I suppose it would be nice to document the task better then. It is not described as "an internal task intended for signing click-once artefacts only. Requires either Visual Studio and/or a Windows 8.1 SDK to be installed".

https://docs.microsoft.com/en-us/visualstudio/msbuild/signfile-task now says this (MicrosoftDocs/visualstudio-docs#8038).

@bitslasher
Copy link

Is it really a valid option here to say that this task requires an SDK to be installed for an obsolete and unsupported version of Windows for it to work?

@rainersigwald
Copy link
Contributor

@bitslasher that's not what we're saying. What we're saying is "no scenario that uses the SignFile task is supported when building using the .NET SDK". If you can make it work via some series of changes and you're comfortable with that, ok, but the supported way to build and sign ClickOnce projects is with MSBuild.exe through Visual Studio or Visual Studio Build Tools.

@bitslasher
Copy link

@rainersigwald Thanks for the reply.

So is there a plan for making signed binaries a first-class concept in the .NET SDK? So this task was just all about ClickOnce. To me though, since all operating systems have signed binaries, in this day an age, and where the .NET SDK is and going in the future, it'd seem prudent to have a universal way of signing binaries with the SDK.

Outside of ClickOnce, Windows has just good old Authenticode, signed DLLs, EXEs, MSIs, etc. It seems like Linux also has signed ELF binaries. It's odd that in 2023 we're still having to hunt for a copy of signtool on our build machines, and it's limited to just Windows.

It'd be amazing if this task could find new life as a generic cross-platform signing task. :)

@Zastai
Copy link
Contributor Author

Zastai commented Feb 28, 2023

That would indeed be good; it's why I attempted to get the task active in the .NET MSBuild. But the focus seems to be on signing NuGet packages rather than individual binaries.

@bitslasher
Copy link

But the focus seems to be on signing NuGet packages rather than individual binaries.

Hi @Zastai, that's a shame too-- since all these NuGet packages end up needing to be used by an executable eventually! :)

Which makes me think-- what of when someone runs an app with dotnet run? If the DLL isn't signed, does dotnet care? The policy on say Windows that enforces signed EXEs only-- does dotnet bypass this because it's signed by Microsoft? That doesn't mean the DLL it's about to run is friendly... I need to go read some more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug ClickOnce Issues related to ClickOnce, including the SignFile task.
Projects
None yet
Development

No branches or pull requests

7 participants