-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
ECDsa.VerifyData Performance Slow on Mac #36107
Comments
I couldn't figure out the best area label to add to this issue. Please help me learn by adding exactly one area label. |
cc @bartonjs for area. |
I took a look at this and I don't see any easy wins for performance. In .NET Core, we don't implement any of the underlying cryptographic algorithms. It defers to functionality built in to the OS. In macOS, this is called SecurityTransforms. The performance issue seems to be that macOS's implementation is simply not that fast. The bulk of the verification time is spent in I made a Swift Playground that uses your key, signature, and message and used the same Cryptographic APIs that .NET is using. I did a little pre-processing of the data that .NET uses, like pre-computing the hash of the message and formatting the signature, but otherwise it's the same. Example output:
This is on macOS 10.15.4. Since your original program did 1,000 iterations, we can multiply that by 1000 and then divide by 1000 again to go back to seconds, so the total time for 1,000 invocations is approximately 10 seconds, so the Swift code demonstrates comparable time as the .NET code. Just for fun, I looked at the new CryptoKit API introduced in macOS 10.15, and I didn't see any improvement. |
@vcsjones Thanks for explaining. I run your code on my Mac, and find something interesting. I run CryptoKit API version first then The |
Interesting, it appears that CryptoKit has a bit of a "start up" time. If I run SecurityTransforms and CryptoKit in a loop 1000 times, the performance difference is very noticeable. CryptoKit is quite fast. I don't know how feasible it is to use CryptoKit for this off the top of my head. There are a number of barriers with using CryptoKit. The biggest one is the lack of a C-like API that .NET can call. .NET can work with SecurityTransforms because it does offer a C API. Since the native parts of .NET are written in C, thus SecurityTransforms is easy enough to use. CrytpoKit does not have a C API - it offers a Swift-only API/ABI that .NET doesn't understand. In order to use it, there would need to be a Swift -> Objective-C -> C indirection. This was explored while researching AES-GCM but it's only been mildly explored so far. The other issue is that CryptoKit is 10.15+ on macOS. I can't say with any certainty but I suspect .NET 5 will continue to support at least 10.14 as well. The CryptoKit times are encouraging though that CoreCrypto (Apple's truly low-level implementation of cryptographic functions that powers CommonCrypto and CryptoKit) is fast, and that something in SecurityTransforms is holding it back. I will continue to investigate other possibilities of SecurityTransforms - perhaps there is a way to get it closer to CryptoKit's performance. |
It looks like @bartonjs given the significant performance improvement with |
Does it require the original data (VerifyData) or does it also work with the hash (VerifyHash)? It'd be weird if VerifyHash ended up being slower. |
@bartonjs both. Can use There are similar options for RSA, (sign and encrypt). |
Cool, I'm up for the experiment, deferring to the existing function when we need to (DSA, RSA+MD5) |
I spent a little time on this to plumb this in to ECDSA verification and the results were not as good as I had hoped they would be. The branch for the changes is here: https://github.com/vcsjones/runtime/tree/ecdsa-perf I put together a benchmark project here: https://github.com/vcsjones/DotNetCryptoBenches The gist is that there was no to small improvement depending on the curve and data length. Small being at most a millisecond, usually half a millisecond. This is not the order of magnitude improvement I was hoping for. However there is one more avenue left for me to explore, which is how the key gets imported. The current approach uses |
@bartonjs Out of curiosity, what is the history behind keygen on macOS roundtripping through import/export? runtime/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_ecc.c Lines 32 to 39 in cacebda
It looks like keys are generated in a temporary keychain, then shuffled off in to the ephemeral keychain. Why generate them in a temporary keychain at all, then export them to the ephemeral one? |
Something obviously didn't work :). I think it's that SecGenerateKeyPair won't (or wouldn't in 10.12) generate to the ephemeral keychain; or something broke when it did so. So a temporary keychain gets created to make it happy, but then when we delete said keychain it gets angry, so we then export/import to the ephemeral keychain so we can delete the temporary keychain. All so that |
Okay, after switching key generating to use Key Services instead of Security Transforms, I see much more substantial results. I updated the branch and the benchmark's results directory, but to give an idea: .NET 5 Preview 4
CoreRun (Branch)
@bartonjs There is a performance win here I think, but it is a fairly substantial amount of work and changes. I made enough changes to get keygen, sign, verify on key services. Import / export is broken in the branch for ECDSA. There is some compatibility between key services and keychain / transforms, but not enough. For example, I'm also not sure right now what this means up the stack, like Since there is little interop between key services and keychain, I don't see a great piecemeal approach, either. This is on the order of "rewrite the macOS crypto PAL". More annoyingly, I don't see full DSA support here, which means in order to keep supporting DSA we would need to keep all of the existing code as well. All that said, if it works, it's a performance improvement of 10x to 50x. |
My gut tells me this will also improve RSA performance, but I don't have numbers to back that feeling up, yet. |
If import/export work then the semi-piecemeal approach would be to move ephemeral keys to key services while leaving cert+key keys on the current stack (import/export would be required so that CopyWithPrivateKey can succeed... in addition to all the other obvious needs of import/export) |
@bartonjs i think that is reasonable to try. |
Here are some numbers for RSA. Signing improves, however verify stays the same, (All tests are using PKCS1 padding). .NET 5 Preview 4
CoreRun
|
@bartonjs continuing to play with this, I think I figured out that if this were to be done the best solution mabe to carry a duplicate set of keys... the original keys in This makes import and key gen slightly more expensive since any time the key mutates we need to shove the key through a few places to get a data key, with the benefit of substantially improved perf for sign / verify. I put together a rough idea of what it would look like here: dotnet:7ef49af...vcsjones:58e5a11. There's some missing error checking and TODOs in there, but it should illustrate what a complete solution would look like. As to "why both"? It's for a few various reasons, mainly that data keys can't be pushed through The latter will keep performance consistent regardless of where the key is coming from without needing to tell people to export / import it again to get better performance. Is this sensible? |
It seems like a reasonable approach. I'd try to feed the SecKeyPair methods a delegate and the key type, rather than have so much repeated-looking code, but that's just an initial reaction to seeing very similar blocks multiple times. |
I believe this may be worth revisiting with benchmarks on Most of the work done by @vcsjones has been now merged in slightly modified form. There may still be a scenario or two where generating the keys and directly using them falls back to the legacy CSSM EC implementation. That would be easy to solve with carefully injecting |
I can do that much at least, I still have the benchmarks. Thanks for getting all that code in to a mergeable state. |
Benchmarks taken from: b8dbea7
This seems to be the case. Fresh keys note no performance improvement, round tripping the keys through export/import gets good performance. Ignore the allocated column. New keys (no improvement):
Imported keys (improvement) Same benchmark, just did
|
I can take a look at this since you are likely busy with iOS things. Unfortunately this attribute is new in macOS 10.15, but we can continue to use the CSSM path for 10.14. |
I believe the |
@vcsjones I have the patch ready, just waiting for other stuff to be merged first: https://github.com/dotnet/runtime/compare/d74211c...filipnavara:sscan-avail?expand=1 |
Hi, I have got performance issue when use ECDsa.VerifyData() on Mac, it's almost 10 times slower than on windows, which cause my app running very slow on Mac.
The runtime is dotnet core 3.1.
Here my test code:
on Win10:

on Mac:

The text was updated successfully, but these errors were encountered: