search

dotnet / msbuild

The Microsoft Build Engine (MSBuild) is the build platform for .NET and Visual Studio.
https://docs.microsoft.com/visualstudio/msbuild/msbuild
MIT License
5.21k stars 1.35k forks source link

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

Open Zastai opened 3 years ago

Zastai commented 3 years ago

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

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

(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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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:

https://github.com/dotnet/msbuild/blob/dec13b16c7bbe9086b25f98d661645f05d5c29ba/src/Shared/FrameworkLocationHelper.cs#L217

benvillalobos commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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.

Zastai commented 3 years ago

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 commented 2 years ago

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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.

NiKiZe commented 9 months ago

Oftopic, sorry!

Since this issue is top search result when trying to get info on how to Sign your binaries, here is and updated msbuild Target that does the right thing based on already given options in configuration.

  <Target Name="_SignAssemblies" AfterTargets="Compile" DependsOnTargets="CreateSatelliteAssemblies;$(RazorCompileDependsOn)" Condition="'$(CertificateThumbprint)' != ''">
    <ItemGroup>
      <_AssembliesToSign Include="$(IntermediateOutputPath)$(TargetFileName)" />
      <_AssembliesToSign Include="@(IntermediateSatelliteAssembliesWithTargetPath)" />
      <_AssembliesToSign Include="@(RazorIntermediateAssembly)" />
    </ItemGroup>
    <Message Importance="high" Text="Signing assemblies: @(_AssembliesToSign)" />
    <SignFile SigningTarget="%(_AssembliesToSign.Identity)" CertificateThumbprint="$(CertificateThumbprint)" TimestampUrl="$(TimestampUrl)"/>
  </Target>