dotnet / runtime

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

PublishAot brings in Microsoft.DotNet.ILCompiler and causes AppLocker issue #95174

Open Balkoth opened 11 months ago

Balkoth commented 11 months ago

Description

Publishing a project with aot tries to call findvcvarsall.bat in C:\Users\_USER_\.nuget\packages\microsoft.dotnet.ilcompiler\8.0.0\build which in turn is blocked in a corporate environment by AppLocker rules.

Reproduction Steps

Expected behavior

Project is successfully published.

Actual behavior

The build is cancelled, because at least one batch file is run from a location where, under default corporate AppLocker rules, script and exe execution is not allowed.

Regression?

No response

Known Workarounds

No response

Configuration

.NET SDK: Version: 8.0.100 Commit: 57efcf1350 Workload version: 8.0.100-manifests.8d38d0cc

OS Name: Windows OS Version: 10.0.19045 OS Platform: Windows RID: win-x64 Base Path: C:\Program Files\dotnet\sdk\8.0.100\

Other information

New workflows should be created in a way that developers can take advantage, without having administrative rights on their machines and without bothering it helpdesk and sysadmin people.

In a world where it security is a top priority it is a bad habit to expect developers in corporate environments to have excess rights on their machines just to develop software.

Wraith2 commented 11 months ago

In a world where it security is a top priority it is a bad habit to expect developers in corporate environments to have excess rights on their machines just to develop software.

Arguably it also a bad habit to block the location that is expected to contain user specific files on that machine, doing so is what forces the user to need permissions to find somewhere else to put files so they can do their job. I suggest you contact your security department and talk to them about their inappropriate configuration.

Balkoth commented 11 months ago

It's people like you who make corporate environments insecure. You might not care about security, but we here do. No one here is allowed to execute scripts or executables from untrusted locations.

Allowing users to run that stuff from a userdir, where users can put anything downloaded from the web is a nightmare. Thats why AppLocker and comparable solutions exist.

huoyaoyuan commented 11 months ago

No one here is allowed to execute scripts or executables from untrusted locations.

What's the definition of "executable" for managed application? Is it the launcher dotnet.exe, or the dll? What if a process load an unmanaged dll, or a managed assembly?

If you are able to run roslyn analyzer installed via NuGet package, you are already violating this rule.


The AOT compiler is shipped via an additional package. It's not shipped with the SDK which should be set to trusted.

Shipping development-time tools with package management system isn't unusual.

Balkoth commented 11 months ago

I am not allowed to give any more insight to our security concept here as this information could be used by a potential attacker.

In my experience its entirely unusual that nuget packages run batch scripts, or otherwise i would have already stumbled over it.

sylveon commented 11 months ago

What alternative do you suggest? Trying to find a VS install from a NuGet package is gonna need to execute some code one way or another.

Balkoth commented 11 months ago

If a Visual Studio installation is required, then why not just include this with it in ProgramFiles? Seems like a core feature to me.

sylveon commented 11 months ago

Visual Studio could be installed elsewhere than in Program Files, hence the need for a findvcvarsall.bat ;)

Balkoth commented 11 months ago

If this is included in the vsinstalldir where you are already allowed to execute, then there will be no problem. Does not matter if its programfiles or another directory. If it's essential to find a vs install, then include such a script in the sdk.

Not providing such a seemingly essential method, sparks things like this script an a bad location. And if another package needs to find a vs install they roll their own scirpt. Now you have two scripts, each with their own bugs.

ghost commented 11 months ago

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas See info in area-owners.md if you want to be subscribed.

Issue Details
### Description Publishing a project with aot tries to call `findvcvarsall.bat` in `C:\Users\_USER_\.nuget\packages\microsoft.dotnet.ilcompiler\8.0.0\build` which in turn is blocked in a corporate environment by AppLocker rules. ### Reproduction Steps - Create new console project with `dotnet new console --aot` - Publish the project with `dotnet publish /p:RuntimeIdentifier=win-x64 /p:SelfContained=true` ### Expected behavior Project is successfully published. ### Actual behavior The build is cancelled, because at least one batch file is run from a location where, under default corporate AppLocker rules, script and exe execution is not allowed. ### Regression? _No response_ ### Known Workarounds _No response_ ### Configuration .NET SDK: Version: 8.0.100 Commit: 57efcf1350 Workload version: 8.0.100-manifests.8d38d0cc OS Name: Windows OS Version: 10.0.19045 OS Platform: Windows RID: win-x64 Base Path: C:\Program Files\dotnet\sdk\8.0.100\ ### Other information New workflows should be created in a way that developers can take advantage, without having administrative rights on their machines and without bothering it helpdesk and sysadmin people. In a world where it security is a top priority it is a bad habit to expect developers in corporate environments to have excess rights on their machines just to develop software.
Author: Balkoth
Assignees: -
Labels: `untriaged`, `area-NativeAOT-coreclr`
Milestone: -
jkotas commented 11 months ago

You can set IlcUseEnvironmentalTools property in your .csproj file to true (this will disable invocation of findvcvarsall.bat) and setup the VS native tools build environment using alternative method that is compatible with your setup.

As others pointed out, it is very common for nuget packages to include code that executes during the build. I agree that .bat files in nuget packages may be unusual, but it is very common for nuget packages to include Powershell scripts that have same capabilities as .bat scripts and carry the same security risk.

If it's essential to find a vs install, then include such a script in the sdk.

It does not make sense to include the findvcvarsall.bat script in the sdk. findvcvarsall script is an implementation detail of the native AOT compiler package, so it should come with the package.

Balkoth commented 11 months ago

Going through hacks to trick the setup into this is not an option for me. If this cant work out of the box, then i can not use this feature.

If you are saying this is so common, then how comes that this is the first time this impacts me? Where are those very common nuget packages that do this? Or are you the special unicorn that does this?

I'm really questioning myself: As this is a Microsoft product, are you at Microsoft allowed to run random executables and scripts from untrusted locations? Is this in line with Microsoft company policies?

jkotas commented 11 months ago

Here is a random example of nuget package that contains Powershell scripts: https://nuget.info/packages/xunit.analyzers/1.6.0 . Look in the tools directory.

are you at Microsoft allowed to run random executables and scripts from untrusted locations?

We are not allowed to download random untrusted executables. Microsoft.DotNet.ILCompiler package is signed by Microsoft certificate.

From https://nuget.info/packages/Microsoft.DotNet.ILCompiler/8.0.0 : image

Thus the package content, including findvcvarsall.bat script, is trusted.

hamarb123 commented 11 months ago

Going through hacks to trick the setup into this is not an option for me. If this cant work out of the box, then i can not use this feature.

If it was a "hack", then I doubt it would be so easy to work around - it would probably require you to modify some .targets files or similar, or something much more atrocious, if support for this flag wasn't built in.

Is NativeAOT enabled out of the box? No. You have to set a flag for it in the csproj or when you publish; therefore you can't use it anyway since it "doesn't work out of the box" and requires "hacks" like settings a flag.

The above 2 statements are intended to point out the fallacy of your statement.

If you are saying this is so common, then how comes that this is the first time this impacts me? Where are those very common nuget packages that do this? Or are you the special unicorn that does this?

Did you know that nuget packages can actually download exes, dlls, and whatever else they want, and run them at compile time? They can even execute arbitrary C# code, which gets compiled and executed by msbuild. Hope that's in line with your company policy. One batch file is not an issue compared to all of this. Downloading an untrusted nuget package is the security issue, once you've downloaded it, you've already failed security.

Most nuget packages, which aren't just a simple C# project compiled into a nuget package, have at least target files (which can already basically do anything). For example, I went onto nuget.org, and the second top package has targets files, which, again, can run arbitrary code, so if you haven't checked every file in every nuget package before installing it, then you have no idea what code is being run.

If you want a non-microsoft one, then top ~4th non-Microsoft one (depending how you count), AWSSDK.Core, has a powershell script in it.

Balkoth commented 11 months ago

Here is a random example of nuget package that contains Powershell scripts: https://nuget.info/packages/xunit.analyzers/1.6.0 . Look in the tools directory.

are you at Microsoft allowed to run random executables and scripts from untrusted locations?

We are not allowed to download random untrusted executables. Microsoft.DotNet.ILCompiler package is signed by Microsoft certificate.

From https://nuget.info/packages/Microsoft.DotNet.ILCompiler/8.0.0 : image

Thus the package content, including findvcvarsall.bat script, is trusted.

How is this enforced once the nuget package is downloaded and in the .nuget folder? Or is this just a soft rule?

hamarb123 commented 11 months ago

@hamarb123 And when everybody jumps off the bridge, you jump too, because why not?

You're the one jumping off the bridge by not checking the packages are safe... If you're not validating the trustworthiness of every part of each nuget package, or deciding that you trust some providers, you have no idea what you're running on your computer.

If you had validated the nuget packages you installed, then you'd already have known there was a script before you installed it (which was done by enabling NAOT), if not, then you're the one deciding not to validate if they're safe thoroughly. The fact that you're surprised that lots of nuget packages utilise this feature means that either: you just happened to have not used any of them, which is not very likely, or you haven't been validating their safety, which would make your whole argument of "batch file unsafe if in user folder" "I no want batch file in that folder" meaningless for the reasons I've already described.

Its not an argument that someone else does something.

I don't quite know what you're referring to here, but if you're referring to me giving examples of things that specific nuget packages which do potentially run arbitrary code without you having a closer look, including 1 which uses a script file, then, I'll remind you that you said:

Where are those very common nuget packages that do this?

Balkoth commented 11 months ago

I said this, because this is my experience. The org runs AppLocker, and script execution is not allowed in that folder. I never had a problem in the past with this.

Just because you know everything inside out what Visual Studio does when loading nuget packages, does not mean that everyone else does.

I came here and posted the original bug report, because i thought this is a legitimate issue. I still think that this is problematic.

hamarb123 commented 11 months ago

Just because you know everything inside out what Visual Studio does when loading nuget packages, does not mean that everyone else does.

Sorry for informing you that the attack surface is wider than you thought it was. Next time I'll be sure to allow you to think you're secure when you're potentially running arbitrary code from anyone that can post a nuget package.

Did you also know that if you download an excel spreadsheet with macros, and then open it, it can run arbitrary code? Probably. Because you actually looked into that attack vector. It's not my fault if you don't spend the time to learn how to use things safely and determine their attack surface.

Don't try to blame me or anyone else for your ignorance of potential security issues, especially when we're literally trying to explain it to you so you can be secure.

The reason I brought up any nuget package being able to run any code is because your organisation think's it's protected from something by blocking batch scripts, but it's clearly not protected from arbitrary code execution in nuget packages, otherwise you'd already be aware of this and the issue wouldn't have arisen to begin with; because the only protection to it is to either block all code execution by nuget packages somehow (which would break a lot of nuget packages, and I'm not even sure how you'd achieve this frankly), or to verify their trustworthiness manually (which you clearly hadn't done, since you were unaware of the ability to run any code).

Balkoth commented 11 months ago

Thanks for creating an unwelcome environment and derailing the report. Have a nice day.

BitesizedLion commented 11 months ago

Thanks for creating an unwelcome environment and derailing the report. Have a nice day.

You've had a really rude tone from the start towards everybody. I suggest you stop, it's unnecessary.

huoyaoyuan commented 11 months ago

I agree that an extracted bat file is problematic. Executables, dlls and even powershell scripts can be signed with digital cert, but bat file can't.

The 8.0.100 SDK has been shipped in this form and we are finding workaround that works for you. In the future versions the design may be changed based on feedback. Please also be mindful that restrictions sometimes can provide a false sense of security.

MichalStrehovsky commented 11 months ago

Switching this to PowerShell is unlikely (and we don't know if somebody else's IT department wouldn't block .ps1 instead). What we could do is move this logic into the MSBuild .targets files - it's just a lot more work to do it that way.

MSBuild .targets are also completely unsigned and can contain arbitrary C# snippets, but usually run under the radar of IT department policies because there's no group policy they could configure to break MSBuild/NuGet in general.

I'm marking this Future and Up for grabs.

josephmoresena commented 9 months ago

Powershell is not allowed at the company where I work, so any script that calls PS is automatically blocked. findvcvarsall.bat has a PS call to send telemetry. So... I commented these lines and now I (and the whole team) can compile applications in NativeAOT.

I don't know if that can be useful to you right now @Balkoth