cloudfoundry / dotnet-core-buildpack

Cloud Foundry buildpack for .NET Core on Linux
http://docs.cloudfoundry.org/buildpacks/
Apache License 2.0
91 stars 90 forks source link

.Net SDK brings Vulnerable Older Version of system/dependent library #786

Open egmanoharan opened 1 year ago

egmanoharan commented 1 year ago

What version of Cloud Foundry and CF CLI are you using? (i.e. What is the output of running cf curl /v2/info && cf version?

cf cli v8.6.1

What version of the buildpack you are using? dotnet-core-buildpack v2.4.12

If you were attempting to accomplish a task, what was it you were attempting to do?

Normal Deployment of of self-contained .Net Core Application.

Is your dotnet app unpublished, platform-dependant, or self-contained?

Self-Contained

What did you expect to happen?

.Net SDK 6.0.408 and 7.0.5 brings the vulnerable older version of the following system/dependent library

Severity | CVE | Type | PackageName | PackageVersion critical | CVE-2021-24112 | image | system.drawing.common | 4.7.0 high | CVE-2018-8292 | image | system.net.http | 4.3.0 high | CVE-2019-0820 | image | system.text.regularexpressions | 4.3.0 high | GHSA-5crp-9r3c-p9vr | image | newtonsoft.json | 9.0.1

What was the actual behavior? Net SDK 6.0.408 and 7.0.5 should bring the latest version of the above system libraries.

image

ForestEckhardt commented 1 year ago

We are, for the most part, using these dependency as they are released by Microsoft. Any patching of these dependencies would have to be done by them as we do not patch anything ourselves in the down stream.

egmanoharan commented 1 year ago

Thanks for your Update.

egmanoharan commented 1 year ago

Hi Team,

We have created a Vulnerability case with Microsoft for these vulnerabilities and we got a response

Hello Manoharan,

Thank you again for submitting this issue to Microsoft. We determined that a fix will not be released for the reported behavior.

This is a false positive and the dotnetcore build pack is not under .NET. It belongs to Cloud Foundry. This is a misunderstanding of how .NET builds work, probably caused by scanning deps.json, which does not indicate what build outputs, and was never meant to indicate what resolved dependencies are in a build, rather it’s minimum required versions.

As an example System.Drawing.Common is shipped in the .NET SDK, so if you have .NET 6 installed you get v6._, NET brings in v7.__.

Same for System.Net.Http, System.Text.RegularExpressions and System.Security.Cryptography Xml

Netwonsoft.Json isn’t a Microsoft product, and additional information would be needed from Cloud Foundry. This is not a fix for .NET but within the scanning tool.

We have closed this case.

If you have any questions, or additional information related to this report, please reply on this case thread.

While checking app container we found that the following json files contains the respective library filename entry Path: /proc/1098999/root/home/vcap/deps/0/dotnet-sdk/sdk/6.0.408/Sdks/Microsoft.NET.ILLink.Tasks/tools/net6.0/ILLink.Tasks.deps.json Path: /proc/1098999/root/home/vcap/deps/0/dotnet-sdk/sdk/6.0.408/DotnetTools/dotnet-format/dotnet-format.deps.json Path: /proc/1098999/root/home/vcap/deps/0/dotnet-sdk/sdk/6.0.408/dotnet-watch.deps.json Path: /proc/1098999/root/home/vcap/deps/0/dotnet-sdk/sdk/6.0.408/datacollector.deps.json

How these json files are populated? and in which location these dependent libraries are installed?

image

regards Mano

ForestEckhardt commented 1 year ago

All of these deps.json files are present as is in a fresh download .NET SDK 6.0.408. We do not add any of these projects to the .NET SDK installation. The most processing we do is taking an installation of .NET and breaking it into separate SDK and Runtime components. I am not sure what else to say other than we are not adding libraries to the .NET SDK.

MaikiGirardi commented 1 year ago

@ForestEckhardt is there a way to choose using Runtime instead of SDK in the buildpack? In our DevOps process we build the code before deploy and run security scanners in the code before publish into Cloud Foundry.

ForestEckhardt commented 1 year ago

My understanding is that the SDK does not end up in the final droplet and is only downloaded when building the app in case we need to build the application for the user.

MaikiGirardi commented 1 year ago

@ForestEckhardt unfortunately this is not what is happening, take a look in the logs below:

   -----> Dotnet-Core Buildpack version 2.4.16
   -----> Supplying Dotnet Core
   -----> Installing libunwind 1.7.2
   Copy [/tmp/buildpacks/c6ebd021cdd0f9;0/dependencies/781dcd4df4c/libunwind_1.7.2_linux_noarch_cflinuxfs4_894a2eb5.tgz]
   using the default SDK
   -----> Installing dotnet-sdk 6.0.413
   Copy [/tmp/buildpacks/c6ebd021cdd0f9;0/dependencies/2602cfe470ecc01694fda/dotnet-sdk_6.0.413_linux_x64_any-stack_c9a9eadd.tar.xz]
   -----> Installing dotnet-runtime 6.0.21
   Copy [/tmp/buildpacks/c6ebd021cdd0f9;0/dependencies/6ac4cf2ce265/dotnet-runtime_6.0.21_linux_x64_any-stack_53a68385.tar.xz]
   -----> Finalizing Dotnet Core
   ....

What we expect is that even for Framework Dependent it removes the SDK and keep only the DotNet Runtime and if we use self-contained it removes everything (SDK and runtime) because in that case that is embedded within our code.

In the example below is a connection into the container using cf ssh and checking the deps folder, you will we see a symlink created pointing to the SDK instead of point into Runtime:

$ ls -ls
lrwxrwxrwx 1 vcap vcap 20 Sep  8 15:04 dotnet -> ../dotnet-sdk/dotnet

This is why this vulnerabilities are shown in the scanners, SDK still present instead of being removed keeping only the runtime.

Here, we never build the app in PCF and we also see SDK being installed. That's why I asked you if there is any option only have the Runtime to avoid installing that version with vulnerabilities in the tools that are mainly used only for building apps and we do not use it.

Now, reading your comment, sounds like this is a possible BUG, right? Because looks like it must exists for build and after that be removed from the system and start using Runtime if I get it right.

ForestEckhardt commented 1 year ago

Hmmm interesting. I have taken a look at this. I think that part of the issue is that the .NET CLI is part of the SDK installation and the CLI needs to be colocated with the SDK bits in order to build projects properly but the CLI is also needed to run the application .dll in a Framework Dependent Deployment. I will need to take a closer look at this to see if there is anything that can be done about that.

ForestEckhardt commented 1 year ago

Looking at the workflow of the buildpack, it is currently working as intended. I think that there are a couple of solutions that we could do to make this work more smoothly but I would like to get some feedback from other maintainers as well as you.

The easiest solution would be to expect that all FDDs contain an executable (which I believe has been the default outcome since .NET5) and therefore we do not need the .NET CLI to run any FDD. I am not super thrilled with this idea as I am not sure how robust that assumption of an executable is.

The next easiest solution but the one that perpetuates some of the hacky nature of the existing buildpack is that during finalize we could copy the .NET CLI out of the dotnet-sdk folder and collocate it with the dotnet-runtime library that ends up in the final droplet.

The most difficult solution would be to update our dependency structure to reflect the dependencies that are provided by Microsoft themselves. Currently we try and do some optimization to split apart the dependency and only install the parts that are needed but this does lead to complications and difficulties. This would involve installing an installation of the SDK during the staging phase (if needed) and then in finalize we would remove that and install the proper runtime (if needed). This would be a large amount of work and would cause the speed of builds to slow on systems that have slower internet but I think it might be the "right way" to do it as Microsoft offers little to no flexibility in how .NET is installed. This would also have the side affect of the changing the dependencies themselves which feels like a difficult thing to approve. Overall I think that this option might be interesting in the long run (which is why I wanted to get it down) but might no be right for this issue.

sophiewigmore commented 1 year ago

One question I have is - if we now know from Microsoft that the CVEs picked up for deps.json files are false positives, is there still a strong need to get rid of SDK on the final app image? Is this necessary or a nice to have?

@ForestEckhardt I think if we were going to invest the time into resolving this, I would advocate for option # 3, because it most closely mirrors what we do in the Paketo .NET Core buildpack. I agree it's a lot of work, which is why I'm trying to understand how necessary this change actually is. In general I'm in favour of 1. making less assumptions about builds (argument against option 1) and 2. using dependencies as provided by Microsoft as much as possible (argument against option 2)

MaikiGirardi commented 1 year ago

@ForestEckhardt, looking at the options, I agree with @sophiewigmore that option 3 is the right one indeed. Just rephrasing you to make sure I got it right, the idea is simulate a container multistage deployment, that means, for teams who needs PCF to build their apps, it will download/install SDK and after that, remove it and download/install runtime to run the app. I got the right way?

This, for me, is the right option, because in cases that we do not send source code to be built by PCF, the SDK will not be installed at all (if you process test it), and runtime will take place since the beginning, this means that the download is even faster because SDK is much bigger than Runtime. And also, in cases like this that runtime is used in the buildpack instead of .EXE or SCD, restage works beautifully, that means, update OS and Runtime as it should.

I definitely vouch for this idea, because sounds better than the others.

MaikiGirardi commented 1 year ago

Hi @ForestEckhardt, is there any plan on your team roadmap to work on this issue?

ForestEckhardt commented 1 year ago

@johnnyr0x what do you think roadmap wise?

MaikiGirardi commented 1 year ago

Hi all, any news about this?

MaikiGirardi commented 1 year ago

Hi!, any update about this roadmap?

brayanhenao commented 1 year ago

C.C @ForestEckhardt @johnnyr0x

MaikiGirardi commented 1 year ago

Hi!, any update about this roadmap?

Sorry to be so insistent, nowadays to fix DotNet vulnerabilities we are forced to deploy apps in self contained mode. Since we are forced to do this now restage is not affective anymore, it only helps in terms of OS vulnerabilities, but does nothing in terms of .NET.