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

apphost has a permission of -rwxr--r-- #3669

Closed
omajid opened this issue Jun 28, 2019 · 21 comments · Fixed by dotnet/core-setup#8510
Closed

apphost has a permission of -rwxr--r-- #3669

omajid opened this issue Jun 28, 2019 · 21 comments · Fixed by dotnet/core-setup#8510
Labels
area-HostModel Microsoft.NET.HostModel issues

Comments

@omajid
Copy link
Member

omajid commented Jun 28, 2019

I built .NET Core 3.0 preview 6. It includes an apphost executible that can only be executed by owner, no one else.

$ tar xf ../bin/x64/Release/dotnet-sdk-3.0.100-preview6-012264-fedora.30-x64.tar.gz
$ find -name 'apphost'
./packs/Microsoft.NETCore.App.Host.fedora.30-x64/3.0.0-preview6-27804-01/runtimes/fedora.30-x64/native/apphost
./sdk/3.0.100-preview6-012264/AppHostTemplate/apphost
$ ls -l ./sdk/3.0.100-preview6-012264/AppHostTemplate/apphost ./packs/Microsoft.NETCore.App.Host.fedora.30-x64/3.0.0-preview6-27804-01/runtimes/fedora.30-x64/native/apphost
-rwxr--r--. 1 omajid omajid 962184 Jun 28 14:39 ./sdk/3.0.100-preview6-012264/AppHostTemplate/apphost
-rwxr--r--. 1 omajid omajid 962184 Jun 28 14:38 ./packs/Microsoft.NETCore.App.Host.fedora.30-x64/3.0.0-preview6-27804-01/runtimes/fedora.30-x64/native/apphost

That looks pretty weird. Is this a known issue?

Other executibles look okay:

$ ls -l ./dotnet ./shared/Microsoft.NETCore.App/3.0.0-preview6-27804-01/createdump 
-rwxr-xr-x. 1 omajid omajid 744264 Jun 28 14:27 ./dotnet
-rwxr-xr-x. 1 omajid omajid 611560 Jun 28 14:27 ./shared/Microsoft.NETCore.App/3.0.0-preview6-27804-01/createdump
@dagood
Copy link
Member

dagood commented Jul 3, 2019

It looks like this is the case for the Microsoft build tarball too--transferring to Core-Setup because that's where the apphost pack originates. This causes the apphost in build output to have limited permissions:

#> dotnet new console
#> dotnet build
#> ls -la bin/Debug/netcoreapp3.0/
total 116
drwxr-xr-x. 2 root root  4096 Jul  3 21:08 .
drwxr-xr-x. 3 root root  4096 Jul  3 21:08 ..
-rwxr--r--. 1 root root 82520 Jul  3 21:08 proj                <= The apphost shim
-rw-r--r--. 1 root root   382 Jul  3 21:08 proj.deps.json
-rw-r--r--. 1 root root  4608 Jul  3 21:08 proj.dll
-rw-r--r--. 1 root root   408 Jul  3 21:08 proj.pdb
-rw-r--r--. 1 root root   139 Jul  3 21:08 proj.runtimeconfig.dev.json
-rw-r--r--. 1 root root   164 Jul  3 21:08 proj.runtimeconfig.json

@dagood dagood transferred this issue from dotnet/source-build Jul 3, 2019
@dagood
Copy link
Member

dagood commented Jul 3, 2019

@wli3 @peterhuene do we expect everyone to have execute permission for the apphost?

An execute permission issue was fixed a while back for the macOS installers (https://github.com/dotnet/cli/issues/11231), but I believe the tarball is assembled from the apphost pack nupkg which didn't get that fix.

(This doesn't appear to be a regression in particular:)

#> ls -la */**/apphost

-rwxrw-r--. 1 dagood dagood 86552 Apr 26 23:11 preview5/packs/Microsoft.NETCore.App.Host.linux-x64/3.0.0-preview5-27626-15/runtimes/linux-x64/native/apphost
-rwxrw-r--. 1 dagood dagood 86552 Apr 26 23:11 preview5/sdk/3.0.100-preview5-011568/AppHostTemplate/apphost

-rwxr--r--. 1 dagood dagood 82520 Jun  4 09:36 preview6/packs/Microsoft.NETCore.App.Host.linux-x64/3.0.0-preview6-27804-01/runtimes/linux-x64/native/apphost
-rwxr--r--. 1 dagood dagood 82520 Jun  4 09:36 preview6/sdk/3.0.100-preview6-012264/AppHostTemplate/apphost

@vitek-karas
Copy link
Member

The interesting question is if it should have execute permissions at all. The apphost.exe itself is not usable, it is only usable after it goes through the patching process in the SDK. If that process sets the right permissions, then it should be just fine.

@dagood
Copy link
Member

dagood commented Jul 6, 2019

The SDK doesn't set permissions, hence the need for that earlier macOS-focused fix. (https://github.com/dotnet/cli/issues/11231.) The publish output shows this in practice:

#> ls -la bin/Debug/netcoreapp3.0/
...
-rwxr--r--. 1 root root 82520 Jul  3 21:08 proj

Setting apphost permissions could be implemented in the SDK, and yeah, giving this apphost template execute permissions seems weird to me in principle. The reasoning for putting the fix in Core-Setup is that making every dev machine run chmod (or something like it?) is considered less reliable and more difficult to implement than a build infra patch in Core-Setup, which only has to work once on machines with more well-known configs.

That macOS fix was a last-minute change for an earlier 3.0 preview, so maybe the approach can be revisited outside that context.

@dagood
Copy link
Member

dagood commented Jul 31, 2019

@wli3 @peterhuene @livarcocc, what do you think is the right resolution for this problem? Fixing the permissions on the setup side or in the SDK when it produces a host based on the templates?

@peterhuene
Copy link

peterhuene commented Jul 31, 2019

Personally I think the template should be r--r--r-- and the HostWriter (now in core-setup) should set +x when creating the customized apphost, as it is the one that copies from the template.

@am11
Copy link
Member

am11 commented Oct 9, 2019

Moved from dotnet/core#3559.

From the perspective of dotnet-build, dotnet-publish and publishing as a single file (bundle), is core-setup the correct (kitchen sink) place to fix it? Something like this: c785bf0. If it is, then I just need to figure out how to test it with dotnet-publish command in localhost to validate the change. Any pointers? :)

@vitek-karas
Copy link
Member

@am11 that looks pretty good - but what about Windows - will it work there? I would expect we want this to only happen on Mac/Linux.

core-setup does not have the infra to run full dotnet-publish in tests. Typically we add only a targeted test in core-setup (basically run the code and validate that it marked the file correctly). End-to-end tests can the be added to dotnet/sdk repo. It has the "Caller" of the HostModel package and its tests do have the infra to run full dotnet-publish.

@vitek-karas
Copy link
Member

/cc @swaroop-sridhar

@tmds
Copy link
Member

tmds commented Oct 10, 2019

// 754₈

I'd set the x bit also for other. If someone wants something stricter, they can do it themselves.

RetryUtil.RetryOnIOError

This isn't needed. chmod won't throw IOException.
You can check the return value of chmod. In case it is -1 you can throw Win32Exception which will give more info (from errno).

I would expect we want this to only happen on Mac/Linux.

You can use !RuntimeInformation.IsOSPlatform(OSPlatform.Windows).

@am11
Copy link
Member

am11 commented Oct 10, 2019

Thanks! I did not notice that this lib is shared with Windows. Agree with @tmds, that this could be skipped in Windows, as the file mode (attribute) seems correct:

(on a computer which is not a member of domain)

#!/usr/bin/env powershell

~\Source\Repos\myApp> dotnet publish
~\Source\Repos\myApp> ls $(gcm .\bin\Debug\netcoreapp3.0\publish\myApp.exe).Path | select `
    Name,Mode,Attributes,@{name="Access";expression={(get-acl $_).AccessToString}} | ft -wrap

Name      Mode   Attributes Access
----      ----   ---------- ------
myApp.exe -a----    Archive NT AUTHORITY\SYSTEM Allow  FullControl
                            BUILTIN\Administrators Allow  FullControl
                            ADEEL-PC\adeel Allow  FullControl


# compared to other installed software executable for the current user, for example, `git`
~\Source\Repos\myApp> ls $(gcm git).Path | select `
    Name,Mode,Attributes,@{name="Access";expression={(get-acl $_).AccessToString}} | ft -wrap

Name    Mode   Attributes Access
----    ----   ---------- ------
git.exe -a----    Archive NT AUTHORITY\SYSTEM Allow  FullControl
                          BUILTIN\Administrators Allow  FullControl
                          BUILTIN\Users Allow  ReadAndExecute, Synchronize
                          APPLICATION PACKAGE AUTHORITY\ALL APPLICATION PACKAGES Allow  ReadAndExecute, Synchronize
                          APPLICATION PACKAGE AUTHORITY\ALL RESTRICTED APPLICATION PACKAGES Allow  ReadAndExecute, Synchronize

They vary by Access level, but something which probably users can change by themselves.

@am11
Copy link
Member

am11 commented Oct 10, 2019

chmod won't throw IOException.

Agree and if something bad happens, it would be like SIGSEGV, SIGABRT etc. In the same vein, perhaps, this existing code could be revised: https://github.com/dotnet/core-setup/blob/18780678da576d8c629066f50af30159a8859b2f/tools-local/setuptools/dotnet-deb-tool/Program.cs#L55

@tmds
Copy link
Member

tmds commented Oct 10, 2019

@am11 can you create a PR?

@am11
Copy link
Member

am11 commented Oct 10, 2019

@tmds, sure, I am trying to figure out how to test the end-to-end flow. Should I copy over produced binaries (from ./build.sh's output) to ~/.dotnet or some environment variable so dotnet will pick up the custom build?

@tmds
Copy link
Member

tmds commented Oct 10, 2019

binaries (from ./build.sh's output) to ~/.dotnet

This is the sdk that gets used during the build. You shouldn't patch it. Maybe try to patch a fresh 3.0.100 sdk from dot.net/download. Other people will chime in and suggest some things.

@am11
Copy link
Member

am11 commented Oct 10, 2019

Yup, as @vitek-karas mentioned that E2E unit tests are not available in core-setup right now, I was thinking about patching a local/temp copy obtained by:

curl -sSL https://dot.net/v1/dotnet-install.sh | \
  bash /dev/stdin --install-dir /tmp/dotnet_experimental --channel 3.0

and test it at least once in my localhost.

Seems like there used to be a unit test checking file permission: dotnet/sdk@b98c28e, but the checks were removed in 2.1 for some reason.

Meanwhile, I have issued a PR: dotnet/core-setup#8510. PTAL. :)

swaroop-sridhar referenced this issue in dotnet/core-setup Oct 17, 2019
Set apphost and bundle file permission to 755₈

When building a .net core 3 app, the SDK currently simply copies the apphost template (including its permissions) from the install-location. 

This caused two problems in Unix systems:
* If the dotnet install location is write-protected, the build fails when SDK tries update the apphost (to set the app-path, etc.)  (#8511)
* The built apphost can only be run by the owner (#7062)

This change explicitly sets the file permissions of the Apphost in the SDK to fix the above issues.
@swaroop-sridhar
Copy link
Contributor

Keeping the issue open until the change is in 3.1

swaroop-sridhar referenced this issue in swaroop-sridhar/core-setup Oct 22, 2019
Set apphost and bundle file permission to 755₈

When building a .net core 3 app, the SDK currently simply copies the apphost template (including its permissions) from the install-location.

This caused two problems in Unix systems:
* If the dotnet install location is write-protected, the build fails when SDK tries update the apphost (to set the app-path, etc.)  (#8511)
* The built apphost can only be run by the owner (#7062)

This change explicitly sets the file permissions of the Apphost in the SDK to fix the above issues.
swaroop-sridhar referenced this issue in swaroop-sridhar/core-setup Oct 22, 2019
Set apphost and bundle file permission to 755₈

When building a .net core 3 app, the SDK currently simply copies the apphost template (including its permissions) from the install-location.

This caused two problems in Unix systems:
* If the dotnet install location is write-protected, the build fails when SDK tries update the apphost (to set the app-path, etc.)  (#8511)
* The built apphost can only be run by the owner (#7062)

This change explicitly sets the file permissions of the Apphost in the SDK to fix the above issues.
swaroop-sridhar referenced this issue in swaroop-sridhar/core-setup Oct 22, 2019
When building a .net core 3 app, the SDK currently simply copies the apphost template (including its permissions) from the install-location.

This caused two problems in Unix systems:
* If the dotnet install location is write-protected, the build fails when SDK tries update the apphost (to set the app-path, etc.)  (#8511)
* The built apphost can only be run by the owner (#7062)

This change explicitly sets the file permissions of the Apphost in the SDK to fix the above issues.
swaroop-sridhar referenced this issue in swaroop-sridhar/core-setup Oct 22, 2019
Set apphost and bundle file permission to 755₈

When building a .net core 3 app, the SDK currently simply copies the apphost template (including its permissions) from the install-location.

This caused two problems in Unix systems:
* If the dotnet install location is write-protected, the build fails when SDK tries update the apphost (to set the app-path, etc.)  (#8511)
* The built apphost can only be run by the owner (#7062)

This change explicitly sets the file permissions of the Apphost in the SDK to fix the above issues.
swaroop-sridhar referenced this issue in dotnet/core-setup Oct 23, 2019
When building a .net core 3 app, the SDK currently simply copies the apphost template (including its permissions) from the install-location.

This caused two problems in Unix systems:
* If the dotnet install location is write-protected, the build fails when SDK tries update the apphost (to set the app-path, etc.)  (#8511)
* The built apphost can only be run by the owner (#7062)

This change explicitly sets the file permissions of the Apphost in the SDK to fix the above issues.
@swaroop-sridhar
Copy link
Contributor

Fixed by 10d8d09

@omajid
Copy link
Member Author

omajid commented Nov 18, 2019

Has this really been fixed? I couldn't verify it on my end. Here's what I did:

$ wget https://download.visualstudio.microsoft.com/download/pr/941853c3-98c6-44ff-b11f-3892e4f91814/14e8f22c7a1d95dd6fe9a53296d19073/dotnet-sdk-3.1.100-preview3-014645-linux-x64.tar.gz
$ mkdir dotnet-sdk-3.1.100-preview3-014645-linux-x64
$ cd dotnet-sdk-3.1.100-preview3-014645-linux-x64
$ tar xf ../dotnet-sdk-3.1.100-preview3-014645-linux-x64.tar.gz 
$ ls -l ./sdk/3.1.100-preview3-014645/AppHostTemplate/apphost
-rwxr--r--. 1 omajid omajid 86584 Nov  3 06:50 ./sdk/3.1.100-preview3-014645/AppHostTemplate/apphost

@dagood
Copy link
Member

dagood commented Nov 18, 2019

I believe this issue was focused on setting the generated apphost to -rwxr-xr-x. Reducing the apphost template's permission to r--r--r-- was mentioned, but I think not nearly as big a problem.

Were you able to validate the dotnet publish side of it?

@omajid
Copy link
Member Author

omajid commented Nov 18, 2019

The generated apphost looks correct:

$ mkdir console
$ cd console/
$ dotnet new console
The template "Console Application" was created successfully.

Processing post-creation actions...
Running 'dotnet restore' on /home/omajid/temp/console/console.csproj...
  Restore completed in 54.95 ms for /home/omajid/temp/console/console.csproj.

Restore succeeded.

$ dotnet publish
Microsoft (R) Build Engine version 16.4.0-preview-19529-02+0c507a29b for .NET Core
Copyright (C) Microsoft Corporation. All rights reserved.

  Restore completed in 19.08 ms for /home/omajid/temp/console/console.csproj.
  You are using a preview version of .NET Core. See: https://aka.ms/dotnet-core-preview
  console -> /home/omajid/temp/console/bin/Debug/netcoreapp3.1/console.dll
  console -> /home/omajid/temp/console/bin/Debug/netcoreapp3.1/publish/

$ find -iname console -exec ls -l {} \;
-rwxr-xr-x. 1 omajid omajid 86584 Nov 18 15:19 ./obj/Debug/netcoreapp3.1/console
-rwxr-xr-x. 1 omajid omajid 86584 Nov 18 15:19 ./bin/Debug/netcoreapp3.1/publish/console
-rwxr-xr-x. 1 omajid omajid 86584 Nov 18 15:19 ./bin/Debug/netcoreapp3.1/console

@msftgits msftgits transferred this issue from dotnet/core-setup Jan 30, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-HostModel Microsoft.NET.HostModel issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants