Provide a way to disable using subtle crypto on WebAssembly for the .NET crypto APIs #73386
Comments
|
Tagging subscribers to this area: @dotnet/area-system-security, @vcsjones Issue Details.NET 7 provides a new implementation of the .NET crypto APIs on WebAssembly based on the browser's subtle crypto API. This new implementation relies on support for shared array buffers, otherwise it falls back to the existing managed crypto implementations. The subtle crypto based implementation is coming in a bit late in the release. Given the risk of regression, we should a switch somewhere to disable using the subtle crypto implementation and force using the managed implementation in case there are unforeseen issues. At some future point we may also want a way to enforce that the subte crypto implementations are used for security critical scenarios. While this isn't a requirement for .NET 7, we should account for this future possibility in the solution design for this issue.
|
|
I fundamentally disagree with having a way to opt-in to the managed implementations, especially because we added new algorithms as a part of this effort. (A switch to say "it's on, or fail" I think is fine... and if it's a build switch then it could trim out the managed fallback code) As with all feature work, if it broke something that used to work, we'll fix it in servicing. |
|
@bartonjs I appreciate your point, and do agree in the long term that's where we want to get to. The short-term concern here is that the new unmanaged code paths haven't had the level of real-world customer testing that we'd expect, since they aren't shipping until RC1, and we know the implementation is unusually sophisticated (going through SharedArrayBuffer and a WebWorker). We're concerned about the risk of unexpected breakage when people have complex deployment styles. Fixing in servicing would be essential, of course, but relying on that is still an unwelcome level of risk. There might even be cases we haven't understood where an app can't rely on web workers or something - we just wouldn't know because the feature wasn't in earlier previews. Of course, it does ultimately come down to the level of confidence about this new implementation having been rigorously tested enough in a wide-enough range of complex real-world deployment scenarios. |
|
@bartonjs It is not our intent that disabling the subtle crypto implementations be a common configuration. It is purely a risk mitigation in the event of regression. Note that we're not proposing to change the default behavior, which starting in RC1 will use the new subtle crypto implementation by default if it is available. The risk here also isn't just about functional issues that can be serviced. There may be other functional disparities between the browser and managed implementations that we simply don't know about yet and might not be able to address due to browser limitations. It's worth noting that even with the current default many users will likely still end up with the managed crypto implementation. They might not have setup the required headers for shared array buffers to work, or they might be in an older browser version without shared array buffers enabled. For sake of argument, there are a couple of other mitigations worth considering. If a user hits an issue with the subtle crypto implementation after deploying with .NET 7 they could: 1. Roll back to .NET 6, 2. Disable the HTTP headers required for shared array buffers to work. But there is still risk these mitigations aren't sufficient. A user might require new functionality in .NET 7, the HTTP headers might not be under their control, or they might need shared array buffers or other functionality enabled by the HTTP headers for other reasons. |
Note that the only crypto API that worked in WebAssembly in .NET 6 was the SHA family of hashing algorithms. That used the managed/C# algorithm in .NET 6. In .NET 7, we added the subtle crypto implementation for the SHA APIs, and enabled HMAC, AES, Rfc2898DeriveBytes, and HKDF. The algorithms that were enabled didn't work in .NET 6, so there is no risk of regression there. So the only risk of regression is in the SHA 1/256/384/512 APIs. |
|
I thought the risk was more general than that. Let’s say that in some particular deployment scenario, the act of trying to load the web worker JS file causes an error. Won’t that then happen regardless of which crypto algorithm (or none) you are using? I thought it goes through this new process during app startup, not just when you start using a particular crypto API. It’s totally possible we’re overthinking this risk. It’s just that these kinds of new deployment scenarios have caused complications in the past and we don’t have much proof yet that this works in a wide range of scenarios. For example, has anyone tried this in a PWA yet? And what combination of mobile browsers? What about when we’re being loaded inside an iframe or a web worker? What about with custom service workers? What about when using Blazor for a Chrome or VS Code extension? Maybe they all work, but normally the preview process uncovers things. |
|
Tagging subscribers to 'arch-wasm': @lewing Issue Details.NET 7 provides a new implementation of the .NET crypto APIs on WebAssembly based on the browser's subtle crypto API. This new implementation relies on support for shared array buffers, otherwise it falls back to the existing managed crypto implementations. The subtle crypto based implementation is coming in a bit late in the release. Given the risk of regression, we should provide a switch somewhere to disable using the subtle crypto implementation and force using the managed implementation in case there are unforeseen issues. At some future point we may also want a way to enforce that the subte crypto implementations are used for security critical scenarios. While this isn't a requirement for .NET 7, we should account for this future possibility in the solution design for this issue.
|
|
We can always add the switch later (even in a servicing release after GA) if we get real-world reports that the subtle crypto infrastructure is causing problems. Would it work to wait until we get data that there is an issue? Rather than being aggressive about putting in safety switches preemptively? |
|
@eerhardt It's unusually YOLOish (unless you're saying someone has tested the worker loading process in wide range of deployment scenarios, like those listed above). However as long as this does land in RC1 then we at least get some possibility of reacting to developer feedback. I don't fully understand the level of pushback to having something like an undocumented switch that we could use if it turns out there are troubles, and quietly remove later if not. Is it a nontrivial implementation cost? Altogether I think we've made as much of a point as we have now, and as you're the feature owner, I'd defer to you to decide on the right way to roll it out. |
The pushback is because we don't want people using the managed fallback crypto implementation. from the design doc:
So adding a switch that forces apps down this path (as opposed to its original intention of being a "fallback") isn't desirable, and we'd rather not do it unless there is concrete evidence that it is necessary. |
|
In addition to the scenarios already noted where an application could fail to use SubtleCrypto, customers are also at the mercy of browser behavior here. I can imagine a scenario where a browser update causes our integration with SubtleCrypto to fail wholesale and cause the application to crash. Without an escape hatch to bypass that integration and use the managed implementations, they could face service interruptions without any remedies readily available. I recognize and agree with the position that we do not want to encourage the usage of the managed implementations, but I support the approach of providing an ability to disable SubtleCrypto for such scenarios. |
|
Maybe we can even encode that in the property name like (bad example:) |
Sure. Same thing could happen with OpenSSL servicing or Windows servicing (and happens more often than we'd like with macOS versioning). We don't carry managed implementations in those cases. I am very, very strenuously behind "we should not have ANY sort of special opt-out here until evidence proves it necessary". If any of:
We would not have managed implementations at all. They are very much a last ditch effort, and there shouldn't be any shortcuts to the last ditch. |
An issue is that they aren't really a last ditch effort today. Today they are the default because no one is setting the headers. (see dotnet/aspnetcore#42114) So by default, WASM is using the managed implementation today, and a developer will need to opt in to using the subtle crypto implementation by setting those headers. Even when dotnet/aspnetcore#42114 is fixed, it will only be fixed when the WASM app is hosted in an ASP.NET server that sets the headers. For other cases (like static site hosting), the headers won't be set by default. So in reality, the managed implementations will be used quite often - much more than a "last ditch effort". IMO - The real risk here is that WASM apps that aren't even using crypto are affected by the subtle crypto web worker, since we are trying to initialize the web worker at startup time, even if crypto isn't being used. @danroth27 @SteveSandersonMS - would adding a switch that skips the web worker init, and causes all crypto APIs to throw (instead of falling back to the managed implementation) meet your ask? It seems like a compromise between your position and @bartonjs's position. @bartonjs doesn't want to use the managed implemenation, and your ask is for a switch that disables subtle crypto. Having a switch that causes both not to happen seems to meet both requirements. Pinging @blowdart as well, in case he has any thoughts here. |
I'm sad now. |
|
So, what's the plan here? |
I booked a meeting for Thursday so we can discuss our options. |
Good points there, @bartonjs. Thanks. |
|
Given #73858 is there anything to do here? |
|
No. Closing in favor of #73858. |
ghost
locked as resolved and limited conversation to collaborators
.NET 7 provides a new implementation of the .NET crypto APIs on WebAssembly based on the browser's subtle crypto API. This new implementation relies on support for shared array buffers, otherwise it falls back to the existing managed crypto implementations. The subtle crypto based implementation is coming in a bit late in the release. Given the risk of regression, we should provide a switch somewhere to disable using the subtle crypto implementation and force using the managed implementation in case there are unforeseen issues.
At some future point we may also want a way to enforce that the subte crypto implementations are used for security critical scenarios. While this isn't a requirement for .NET 7, we should account for this future possibility in the solution design for this issue.
The text was updated successfully, but these errors were encountered: