dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.12k stars 4.7k forks source link

Unable to embed compatibility section of application manifest in .exe generated by .NET Core 3.0.100-preview-009817 #3409

Closed livarcocc closed 4 years ago

livarcocc commented 5 years ago

@SteveL-MSFT commented on Thu Jan 10 2019

For PowerShell Core 6.0, we embed an application manifest to enable .NET Core compatibility settings. This works for our exe built against .NET Core 2.1, but fails for the same code retargeting netcoreapp30:

This is the exe built as netcoreapp30:

c:\Program Files (x86)\Windows Kits\10\bin\x64>mt -manifest C:\Users\slee\repos\PowerShell\assets\pwsh.manifest -updateresource:C:\Users\slee\repos\PowerShell\src\powershell-win-core\bin\Release\netcoreapp3.0\win7-x64\publish\pwsh.exe;#1
Microsoft (R) Manifest Tool
Copyright (c) Microsoft Corporation.
All rights reserved.

mt : general error c101008d: Failed to write the updated manifest to the resource of file "C:\Users\slee\repos\PowerShell\src\powershell-win-core\bin\Release\netcoreapp3.0\win7-x64\publish\pwsh.exe". The parameter is incorrect.

Using same manifest for same exe built as netcoreapp21:

c:\Program Files (x86)\Windows Kits\10\bin\x64>mt -manifest C:\Users\slee\repos\PowerShell\assets\pwsh.manifest -updateresource:C:\Users\slee\desktop\pwsh.exe;#1
Microsoft (R) Manifest Tool
Copyright (c) Microsoft Corporation.
All rights reserved.

sigcheck from sysinternals confirms the application manifest is embedded correctly in the second case. The manifest file is from our repo. The trustInfo section is able to be embedded, just not the compatibility section.


@livarcocc commented on Fri Jan 11 2019

@jeffschwMSFT can you take a look at this? I am not sure the SDK is playing a role here.


@jeffschwMSFT commented on Fri Jan 11 2019

@livarcocc can you help move this issue to core-setup? I do not have sufficient write permissions in this repo.

jeffschwMSFT commented 5 years ago

@sbomer can you take a look?

@SteveL-MSFT can you check to see if pwsh.exe already has a manifest which is leading to the error?

SteveL-MSFT commented 5 years ago

@jeffschwMSFT In the above repro steps using mt.exe, both the successful and unsuccessful cases have a manifest that is added during our build process. However, the resulting manifest is missing the compatibility section when using .Net Core 3.0 (vs 2.1).

git clone git@github.com:bergmeister/PowerShell.git
cd powershell
git checkout netcore3.0
ipmo .\build.psm1
start-psbootstrap
start-psbuild -clean -crossgen -configuration release
sigcheck -m (get-psoutput)

Expected:

        Verified:       Signed
        Signing date:   2:59 PM 12/6/2018
        Publisher:      Microsoft Corporation
        Company:        n/a
        Description:    n/a
        Product:        PowerShell Core 6
        Prod version:   6.2.0-preview.3
        File version:   6.2.0
        MachineType:    64-bit
        Manifest:
<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<assembly manifestVersion="1.0" xmlns="urn:schemas-microsoft-com:asm.v1">
    <description> PowerShell Core 6 </description>
    <trustInfo xmlns="urn:schemas-microsoft-com:asm.v3">
        <security>
            <requestedPrivileges>
                <requestedExecutionLevel
                    level="asInvoker"
                    uiAccess="false"
                />
            </requestedPrivileges>
        </security>
    </trustInfo>
    <compatibility xmlns="urn:schemas-microsoft-com:compatibility.v1">
        <application>
            <!-- Windows 10 -->
            <supportedOS Id="{8e0f7a12-bfb3-4fe8-b9a5-48fd50a15a9a}"/>
            <!-- Windows 8.1 -->
            <supportedOS Id="{1f676c76-80e1-4239-95bb-83d0f6d0da78}"/>
            <!-- Windows 8 -->
            <supportedOS Id="{4a2f28e3-53b9-4441-ba9c-d69d4a4a6e38}"/>
            <!-- Windows 7 -->
            <supportedOS Id="{35138b9a-5d96-4fbd-8e2d-a2440225f93a}"/>
        </application>
    </compatibility>
</assembly>

<!--Padding to make filesize even multiple of 4 X -->

Actual:

        Verified:       Unsigned
        Link date:      5:20 PM 11/21/2018
        Publisher:      n/a
        Company:        Microsoft Corporation
        Description:    pwsh
        Product:        PowerShell Core 6
        Prod version:   6.2.0-preview.3-123-ge9ef32ee8e9ec3311a1facd4d30ed4677eabeeb4
        File version:   6.2.0
        MachineType:    64-bit
        Manifest:
<?xml version="1.0" encoding="UTF-8" standalone="yes"?>

<assembly xmlns="urn:schemas-microsoft-com:asm.v1" manifestVersion="1.0">
  <assemblyIdentity version="1.0.0.0" name="MyApplication.app"/>
  <trustInfo xmlns="urn:schemas-microsoft-com:asm.v2">
    <security>
      <requestedPrivileges xmlns="urn:schemas-microsoft-com:asm.v3">
        <requestedExecutionLevel level="asInvoker" uiAccess="false"/>
      </requestedPrivileges>
    </security>
  </trustInfo>
</assembly>
vitek-karas commented 5 years ago

I haven't tried the repro (thanks for it!), but there have been changes to this area in .NET Core 3.0. The toolchain will now embed any native resources found in the managed .exe over to the apphost native .exe. So there should be no need for workarounds like yours where you add native resources after the build. You should be able to specify all native resources, including the manifest to the compiler (in the csproj file) and they should seamlessly propagate to the final .exe.

SteveL-MSFT commented 5 years ago

@vitek-karas I modified our csproj to include the application manifest and it has the same behavior as using the eternal tool. The TrustInfo section is there, but the Compatibility section is missing.

vitek-karas commented 5 years ago

Thanks @SteveL-MSFT somebody on our side will have to take a look and try to repro this.

vitek-karas commented 5 years ago

I tried this using your branch (netcore3.0):

Can you please try this on your end to see if this works as well. Let me know if this would work for you then.

Note: I also could not run sigcheck as it complained it can't find the tool - not sure if it's part of the repo or I need it installed from something else.

SteveL-MSFT commented 5 years ago

@vitek-karas thanks for looking into this. I tried moving the <ApplicationManfest> element from PowerShell.Common.Props which is imported by powershell-win-core.csproj and it still didn't work for me. What version of the .Net Core 3.0 SDK are you using? Don't worry about crossgen, it's not necessary for the repro. sigcheck is a SysInternals tool. You can just start the built pwsh (run: &(get-psoutput) assuming you're using build.psm1) and use [Environment]::OSVersion to see if it reports 10.0 or 6.2 (expecting 10.0 due to compatibility flags). If you're using a fork, can you share that branch so I can try building yours?

vitek-karas commented 5 years ago

I have a branch with the fix here: FixWinManifest (it's a fork from the netcore3.0 PR branch).

I verified that it works with the sigcheck as described above.

Per dotnet --info which I run from the powershel directory it says it's using:

.NET Core SDK (reflecting any global.json):
 Version:   3.0.100-preview-009817
 Commit:    79838fe30c

I do have a slightly older 3.0 preview installed in the global location, but that should not have any effect really as the SDK above should be used for the build.

SteveL-MSFT commented 5 years ago

@vitek-karas Checking out your branch, I was able to build and verify that it works as expected. A diff shows that the only difference is that one line which I tried previously and didn't work. I notice that sigcheck has slightly different results from your build for the manifest:

<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<assembly manifestVersion="1.0" xmlns="urn:schemas-microsoft-com:asm.v1">
    <description> PowerShell Core 6 </description>
    <trustInfo xmlns="urn:schemas-microsoft-com:asm.v3">
        <security>
            <requestedPrivileges>
                <requestedExecutionLevel
                    level="asInvoker"
                    uiAccess="false"
                />
            </requestedPrivileges>
        </security>
    </trustInfo>
    <compatibility xmlns="urn:schemas-microsoft-com:compatibility.v1">
        <application>
            <!-- Windows 10 -->
            <supportedOS Id="{8e0f7a12-bfb3-4fe8-b9a5-48fd50a15a9a}"/>
            <!-- Windows 8.1 -->
            <supportedOS Id="{1f676c76-80e1-4239-95bb-83d0f6d0da78}"/>
            <!-- Windows 8 -->
            <supportedOS Id="{4a2f28e3-53b9-4441-ba9c-d69d4a4a6e38}"/>
            <!-- Windows 7 -->
            <supportedOS Id="{35138b9a-5d96-4fbd-8e2d-a2440225f93a}"/>
        </application>
    </compatibility>
</assembly>

In the non-working case, there's some extra garbage in front which looks like maybe a BOM? Even though pwsh.manifest doesn't have a BOM.

<?xml version="1.0" encoding="UTF-8" standalone="yes"?>

<assembly xmlns="urn:schemas-microsoft-com:asm.v1" manifestVersion="1.0">
  <assemblyIdentity version="1.0.0.0" name="MyApplication.app"/>
  <trustInfo xmlns="urn:schemas-microsoft-com:asm.v2">
    <security>
      <requestedPrivileges xmlns="urn:schemas-microsoft-com:asm.v3">
        <requestedExecutionLevel level="asInvoker" uiAccess="false"/>
      </requestedPrivileges>
    </security>
  </trustInfo>
</assembly>

At this point, since it works with your branch, you can close this issue. Trying to figure out why building with yours works but the working branch we're using still doesn't. Thanks for spending time on this.

vitek-karas commented 5 years ago

Glad to help... The difference does look like a BOM, but it should not make a difference. The OS should be reading this with a conformant XML parser and it should be able to handle the BOM just fine (sigcheck won't since it displays it as text I assume).

SteveL-MSFT commented 5 years ago

Alright, this is embarrassing. Looks like it was a simple typo of the word ApplicationManifest that was causing all the problems.

vitek-karas commented 5 years ago

Seems resolved as far as I can tell. Please reopen if this is still an issue.