-
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
App-level security policy for certificate revocation check #112145
Comments
Tagging subscribers to this area: @dotnet/ncl, @bartonjs, @vcsjones |
Any thoughts on this @bartonjs @GrabYourPitchforks @blowdart ? |
Of course a policy that can be set globally can be unset globally by code lurking in a nuget package, so it's 50% a security improvement and 50% a security vulnerability. If you're going to do it you probably need to have a "soft fail" option as well, where if the call times out it's taken as success. Partly this is needed is due to it being incredibly hard for library authors to know what to do about http clients, or client factories, or configuration, heck, I've hit that myself and had a two page word doc rant I never sent 🤷♂️, and so library devs just don't give any extensibility points. There's a lack of written guidance and samples for that. |
I think mutable static options are always a terrible idea. If there's a desire for forcing something in a process, it should be AppContext or Environment Variable. Read once, never can change again. |
Like we did with binary formatter. But then can you override it on a per client basis? And again we're back to having to audit all your dependencies |
The goal is not to protect against rogue code running in your process -- if you have that then nothing can help you (as the failure of the security model of .NET Framework shows.) The goal is to strengthen the security policy of innocent libraries that do not stand up to the stricter security policy of the application. If you want to protect against rogue libraries that try to downgrade a security policy, we can modify the policy set functionality to ignore downgrade requests. |
I'm not against an app switch similar to the new-and-improved timer switch that was added to .NET Framework timers (for example). Note that controlling such a setting via an environment variable adds complexity to the app's author (who now needs to change the launch process to set the variable instead of controlling the setting in the app's package.) |
No, you can't override it per client; that's the whole point. |
Then I am absolutely against such a feature. It breaks the promises HttpFactory makes and stops developers making informed, such as self-signed certs common in container scenarios or performance-based decisions. |
A major change from the .NET Framework networking libraries to .NET is the elimination of an app-level service point management. This is usually a good thing. One aspect in which it's lacking, however, is that there's no option for applications that host multiple 3rd-party libraries to enforce security policies on outgoing connections, such as checking the TLS certificate revocation list before agreeing to communicate with a web service.
Some library authors (for example, Azure SDK for .NET) provide the means for the hosting application to inject such policies into the library (for example, by injecting
IHttpClientFactory
.) Unfortunately, many libraries do not have this ability, leaving the application developer having to choose between security and functionality.We need a way for applications to force a specific policy on CRL checks. The least-disruptive way I can think of to do this is as follows:
SslClientAuthenticationOptions.CertificateRevocationCheckMode.get
so that it will returnOverrideCertificateRevocationCheckMode
, if that value has been set, if the current value ofCertificateRevocationCheckMode
isNoCheck
. This prevents "downgrade attacks" by libraries.(As far as I understand, this will have another benefit -- allowing users of
HttpClient
/HttpClientHandler
to benefit fromX509RevocationMode.Offline
.)The text was updated successfully, but these errors were encountered: