-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
System.Security.Cryptography.Pkcs 8.0.1 package needed to resolve System.Formats.Asn1 vulnerability #105028
Comments
/cc @ViktorHofer and @ericstj. This looks like duplicate of #98827 but with different packages. |
@vcsjones: Thank you for drawing my attention to #98827. I had missed that when I searched for an existing issue. Having read that issue, and the pushback against updating these packages, I thought I should share some additional perspective. When a customer like myself takes a dependency on one of the packages that internally takes a dependency on System.Formats.Asn1, and when you use Visual Studio to build your solution, here is what the end-user experience currently looks like:
The proposed solution seems to be for customers to take on an additional dependency, even though they don't use it directly. This is taking something that could be fixed in one location, at the root, by simply publishing a System.Security.Cryptography.Pkcs 8.0.1 package with a System.Format.Asn1 >= 8.0.1 dependency, and pushing that work out to every project, current and new, that uses this package. How that is acceptable as a proposed solution is baffling. You're literally telling your customers to fix your problem for you, individually, per project. It's a remarkably irresponsible way to handle a security vulnerability. I'm going to wait for a reply to see if this can be changed, but if the policy is to do nothing in this scenario, that policy needs to be revisited, full stop. Your tooling (Visual Studio and the Nuget Package Manager within it) doesn't properly help customers understand the problem, and your proposed solution doesn't address the problem in such a way that customers can easily get the vulnerability repaired. How is doing nothing in this scenario, with inadequate tooling leaving customers to have to dig around to figure out what is really going on, considered an acceptable solution? |
Over the past couple of years, our csproj files have become littered with comments like these (it is our solution to say, we don't really want this direct dependency, but we have too, so don't delete it) <!-- We take a direct dependency on System.Formats.Asn1 because the transitive version used by other dependencies is marked as vulnerable #{issue-number} --> In terms of addressing these errors, we have found that the dotnet cli is the easiest to figure things out (though it will not report vulnerable versions which have been unlisted (which is another issue)) dotnet list package --vulnerable --include-transitive This is how we deal with the tools givens (though not ideal indeed) |
I find that so unfortunate, for something that could be so easily resolved at the root via a simple NuGet package update.
I use the same command, but that just tells me which packages are vulnerable. It doesn't identify why I'm seeing the vulnerability (i.e. if it's transitive, what is the source of the problem so that I can ping the source if they're not addressing it properly). Running the same command without |
Maybe the proper solution for Microsoft is to use floating version dependencies for Microsoft-managed packages so that the latest appropriate version is installed instead of the minimum version, which may contain vulnerabilities. i.e. Instead of having a dependency in System.Security.Cryptography.Pkcs on System.Format.Asn1 >= 8.0.0, set that dependency up as System.Format.Asn1 >= 8.*. If you take that approach, the Nuget Package resolver should pull down the latest 8.* version that is available, allowing for packages containing vulnerabilities to be flagged accordingly and automatically superseded as those vulnerabilities are fixed. That means you would need to publish a single System.Security.Cryptography.Pkcs package update, bringing it up to 8.0.1, but from that point forward if you don't need to make direct updates, indirect updates would automatically be picked up. As long as you limit your floating version updates to the packages that you (Microsoft) controls, and as long as no breaking changes are published within the same major version number, that would solve this problem. You could use >= 8.0.* instead of >= 8.* if you are more risk averse, because there should definitely not be any breaking changes in a 8.0.* update. If all of that makes you too uncomfortable due to no direct testing of the updates, then I don't see another solution other than to stand behind your packages by doing the right thing and updating them when there are vulnerabilities in your dependencies. The testing argument doesn't stand up though because by not updating you're telling customers to take on these updates themselves, and expecting that to work for them, so floating version dependencies seems like the right way to go since that's essentially what you're counting on working in this situation anyway. |
I suspect this is what would be very useful to have: NuGet/Home#11553 (or something similar to npm ls) |
hey folks, we recently blogged on this topic and how nuget can help you further on understanding transitive vulnerabilities: |
Thanks @JonDouglas, sounds like some great improvements are on their way that will make this much easier. Based on what I read, it seems like the Supplied By Platform would make for smart elimination of some of the recent package vulnerabilities (System.Text.Json, System.Format.Asn1), as long as you have the appropriate SDK version with the fixes installed, correct? It looks like that's been in progress since 2018, so maybe there's still some opportunity to address vulnerability issues in the meantime by using floating version dependencies if that future update won't see the light of day for some time yet? It's great if it will be resolved in the future, but it seems like a bad decision to leave things the way they are in the meantime. |
Instead of issuing a minor update to the package, you are requiring your customers to find a workaround and hope for the eventual delivery of the Supplied By Platform feature. Could you please explain why it is challenging for you to release a minor package update? |
@jeffhandley: I'd like to know more about the decision to tag this with the Future milestone as well. You have a package that you offer to customers. Your package is dependent on another package that has a known vulnerability/exploit. Any of your customers that use your package automatically pick up that exploit. Solutions already exist to solve this at the lowest level, so that your customers get the exploit fixed/corrected without needing to take on additional package dependencies that they shouldn't have to take on. One of those solutions (floating version dependencies) also allows for future exploits that show up in the packages that you are dependent on to be fixed, such that a customer would simply need to rebuild their project to get fixes to those packages as well. Yet instead of addressing this fixable issue with an update to your package, Microsoft's recommended solution is for each customer to fix this issue themselves, because of an internal policy that seems like it should be revisited and revised, with a plan in place to fix this at some time in the future once some new functionality that has been in progress since 2018 is released and adopted. Is that the outcome of this issue? And that's ok? If so, I'll deal with it simply because I don't have a choice, but it certainly doesn't seem like a very responsible way to handle vulnerabilities in packages that have fixes available. |
We just stumbled upon this now recently too. I am quite honestly baffled with what I am reading, at least according to my current understanding. I understand that the fix for us (the customer) is in theory super simple, add a direct dependency on a version where the vulnerability is fixed. However, to re-iterate what other's have said, I don't like this solution one bit as in our case we are referencing I do realize though that in our case, quite a few packages in the chain need to react and update their dependency before we "automatically" are covered by the fix. The maintainer for all however, is Microsoft, so I don't see it as a big challenge... Lastly, I agree that some kind of floating version system or "roll forward on patch" would be better for dependency resolution as this would mean that this issue, at least in our case, would not even exist. After the new 8.0.1 release of Asn1, we would just have had to re-run |
FWIW, if you use NuGet Central Package Version Management and <Project>
<ItemGroup>
<PackageVersion Include="Foo" Version="1.2.3" />
+ <PackageVersion Include="System.Formats.Asn1" Version="8.0.1" />
<PackageVersion Include="Bar" Version="4.5.6" />
</ItemGroup>
</Project> |
@martincostello Thanks for the tip! Unfortunately I don't see the benefit in the case where you would either way only reference the package directly once (if only needed in a single project). The problem is still that you need to maintain the version yourself and/or remember to remove it once you have confirmed the direct dependency chain no longer contains the vulnerable package. I do like the idea of the CPM though and I see how it does make the situation more managable in large projects where multiple direct references could be needed (on top of the benefit of managing all dependencies in one place of course 😃 ). |
It's great for dependabot because it only ever touches one file, regardless of how many times a package is used or what project(s). |
Yes, certainly. But in the case where I, either way, need to manually reference a transitive dependency, in a single .NET project in a solution, it does not relieve me of the "burden" of manually maintaining the reference. If anything, in that case I need to remember I have it defined both in EDIT: I believe my 2v1 statement above is probably incorrect when it comes to overriding transitive dependencies with |
The problem with CPM is twofold:
Saying that, we use CPM in some of our solutions, and it does reduce the burden a little once you learn the shortcomings of the Nuget Package Manager UI when it comes to CPM, but this issue is still something that shouldn't be pushed downstream so I still question the stance Microsoft has taken on this. |
I also ran into this and had quite a few of our images getting flagged by trivy scan, and with the stance microsoft has on this matter i doubt it will be resolved in a timely manner. I agree with the other participants that CPM might not be ideal - in my case I would need to set it up on 30 projs in as many repositories. So, i took a quite different approach: This tool does performs a record PackageReplacement(string package, string oldVersion, string newVersion, string reason, string severity); If a replacement is found i just do a So now all our builds are fixed in the trivy scans - but I don't have added asn1 as a permanent dependency that I need to maintain. |
This project should be able to automate it for you: https://github.com/Webreaper/CentralisedPackageConverter |
I like the idea behind this. Do you have the tool publicly available? I guess replicating it myself would not be too much work either way, but no work is less than something :) Could potentially be improved with accepting a range of versions? The non-restrictiveness of version strings is a problem though, so maybe a list of old versions instead of a single one? |
it's on our internal git server, but here are the main parts |
Cool! I had some free time and interest so I decided to implement my own version before even looking at your gist. I hope you don't mind. My version (probably full of issues) is available here: https://github.com/Matheos96/dependency-overrider One of the biggest differences is that I make use of a separate config.json file for defining overrides, rather than having it directly in code. But of course, the principle is the same. |
This is kind of low effort and doesn't add to the issue, but I just want to express appreciation for this discussion. It helped me get around this problem using the less-than-satisfactory methods suggested by MS. As is mentioned, this issue is not easy to sort out if you're not an expert at package management, which I am not. I write code... I don't care about Nuget and I feel like I shouldn't have to. |
Hi All - I authored #107342 and we'd like to add one of our conclusions in the hours of digging through version numbers, .csproj files and nuget.org dependencies. The nuget resolution goes for minimal version that fulfills all the dependencies in the project tree, which - in our case - upgrades 4.7.2 (not affected) to 8.0.0 (affected) but not to 8.0.4 which is available but does not match minimal version requirements. Suppose, we thought, that nuget keeps on doing what it does today but takes known vulnerabilities into account. Any thoughts? |
Reading this
I was already planning on commenting pretty much exactly this
In other words, I personally like this idea 😄 Though I do think it should be restricted to not walk over Major boundaries to ensure no breaking changes are suddenly introduced with new package upgrades. This also kind of assumes that Package authors are respecting the Semver 2.0 standards, not creating breaking changes in minor or patch version bumps.. |
The NuGet team isn't active in the runtime repo. (someone had to point out this issue to me, at which time I subscribed, otherwise I wouldn't have known about this NuGet related discussion here). Please create a feature request in https://github.com/NuGet/Home Note the feature will be incompatible with any project using a lock file, but hopefully that's obvious to anyone who uses lock files. In the past we did have someone internally suggest the same idea, but I can't find an issue for it. |
@zivkan Created a brief feature request in the NuGet repo NuGet/Home#13771. I based it largely on @ibruynin's comment. |
Coming from: #107342 Additional complexity for upcoming .net 9.0 transition. Dependency vulnerabilities are being reported as warnings during build since version 9. How are we seeing that is going to work if the policy remains that no new versions will be released to fix transient vulnerabilities? If everyone adheres to that policy all builds will fail due to warnings that are treated as errors (everyone enabled that, right?) |
Ever since PackageReference came out in 2016, the way to upgrade transient packages is to make them a direct reference in your project, so then you can control the version. Now Central Package Management with Transitive Pinning enabled can automate this, but if you're not already using CPM, there's some on-boarding effort required. I documented this in a blog post a few weeks ago: https://devblogs.microsoft.com/nuget/nugetaudit-2-0-elevating-security-and-trust-in-package-management/ The blog post also talks about other options like I'm not making a statement about .NET's new version policy. I'm just responding to the implicit question about what to do about the warnings that NuGet reports. And no, not everyone treats warnings as errors. |
This seems to be causing a lot of warnings on build and is slowing down pipeline in .NET9 significantly, sometimes causing memory issues |
Description
If you take a dependency on System.Security.Cryptography.Pkcs 8.0, you will receive a package vulnerability warning due to the vulnerability identified in #104622. That warning occurs because System.Security.Cryptography.Pkcs 8.0 has a dependency on System.Formats.Asn1 >= 8.0.0, and System.Formats.Asn1 8.0.0 is the vulnerable package. System.Formats.Asn1 8.0.1 is available, and no longer contains the vulnerability.
To fix this without requiring customers to take a dependency on a nested package that isn't directly referenced in their code (customers should not have to do that), Microsoft should publish a System.Security.Cryptography.Pkcs 8.0.1 package that has a dependency on System.Formats.Asn1 >= 8.0.1.
Reproduction Steps
Expected behavior
The project will compile without any package vulnerability warnings.
Actual behavior
The project compiles, and Visual Studio identifies that you have vulnerable packages.
Regression?
No
Known Workarounds
Take a hard dependency on System.Formats.Asn1 8.0.1.
NOTE: Taking a dependency on a nested package that you do not reference in your code really isn't a viable workaround, because it adds something else to manage that may very well not be cleaned up later, and it's a transient dependency, so really the package that does reference it and use it directly needs to be updated. That's the proper fix.
Configuration
.NET 8.0
Windows 11
x64
This issue is not specific to Windows or x86/x64.
Other information
The text was updated successfully, but these errors were encountered: