Skip to content

Use key services and data keys where possible for faster EC operations. - #38101

Closed
vcsjones wants to merge 1 commit into
dotnet:masterfrom
vcsjones:fix-36107-crypto-perf
Closed

Use key services and data keys where possible for faster EC operations.#38101
vcsjones wants to merge 1 commit into
dotnet:masterfrom
vcsjones:fix-36107-crypto-perf

Conversation

@vcsjones

Copy link
Copy Markdown
Member

This is an attempt at improving asymmetric cryptographic operations on macOS, starting with ECDSA.

The makes use of Apple's newer Trust / Key Services APIs instead of using security transforms. In order to gain performance increases, two things needed to happen. One, the key needed to be a "data" key, not attached to any keychain, not even an ephemeral one. Second, use the SecKey* APIs.

Though the APIs still work off of SecKeyRefs, many of the legacy APIs cannot operate on keys that are not in a keychain, ephemeral or not. An example of this is SecItemExport for ECDSA data keys. To keep the existing code paths working and the changes more minimal, and allow for more gradual adoption of key services where possible, both key handle types are used. When data keys are present, those are preferred for signing and verification operations.

In this PR data keys exist in two scenarios. The first is Create or GenerateKey. The second is ImportParameters and any other import methods that defer to it. Other means of obtaining a key, like from a certificate in a keychain or importing a PFX, will not have data keys and will use security transforms.

The data keys are obtained by exporting the keys to ECParameters and importing them again through keychain services APIs. This means that although signing and verifying ECDSA operations are much faster, importing and key generation take a performance hit since they have more work to do.

Some native support for RSA exists here. Though none of the managed implementation uses it yet, as this is solely for ECDSA so far.

Contributes to #36107

Benchmarks:

All sign / verify benches use a generated key.

MethodJobToolchainConfigMeanErrorStdDevRatio
SignHashJob-FCMYUVfast-ecdsanistP256, SHA256283.1 μs3.70 μs3.46 μs0.05
SignHashJob-CJQWDKmasternistP256, SHA2565,303.6 μs22.00 μs19.50 μs1.00
VerifyHashJob-FCMYUVfast-ecdsanistP256, SHA256235.1 μs3.09 μs2.89 μs0.02
VerifyHashJob-CJQWDKmasternistP256, SHA25610,176.4 μs42.18 μs37.40 μs1.00
SignHashJob-FCMYUVfast-ecdsanistP384, SHA384784.0 μs9.87 μs9.23 μs0.08
SignHashJob-CJQWDKmasternistP384, SHA3849,659.1 μs30.62 μs28.65 μs1.00
VerifyHashJob-FCMYUVfast-ecdsanistP384, SHA384643.1 μs8.74 μs8.18 μs0.03
VerifyHashJob-CJQWDKmasternistP384, SHA38418,932.7 μs39.28 μs32.80 μs1.00
SignHashJob-FCMYUVfast-ecdsanistP521, SHA5121,104.3 μs13.61 μs12.73 μs0.06
SignHashJob-CJQWDKmasternistP521, SHA51217,658.8 μs59.31 μs55.48 μs1.00
VerifyHashJob-FCMYUVfast-ecdsanistP521, SHA512841.2 μs5.21 μs4.35 μs0.02
VerifyHashJob-CJQWDKmasternistP521, SHA51234,376.4 μs113.74 μs106.40 μs1.00
MethodJobToolchaincurveMeanErrorStdDevRatio
KeyGenJob-FCMYUVfast-ecdsanistP25665.20 ms0.528 ms0.494 ms1.09
KeyGenJob-CJQWDKmasternistP25660.09 ms0.368 ms0.326 ms1.00
KeyGenJob-FCMYUVfast-ecdsanistP38496.29 ms0.595 ms0.557 ms1.04
KeyGenJob-CJQWDKmasternistP38492.89 ms0.737 ms0.689 ms1.00
KeyGenJob-FCMYUVfast-ecdsanistP521151.86 ms0.676 ms0.632 ms1.04
KeyGenJob-CJQWDKmasternistP521146.65 ms0.874 ms0.817 ms1.00
MethodJobToolchaincurveMeanErrorStdDevRatioRatioSD
ImportPrivateParametersJob-FCMYUVfast-ecdsanistP25624,156.73 μs208.931 μs195.434 μs1.320.01
ImportPrivateParametersJob-CJQWDKmasternistP25618,260.68 μs126.533 μs118.359 μs1.000.00
ImportPublicParametersJob-FCMYUVfast-ecdsanistP256500.94 μs6.068 μs5.676 μs2.360.03
ImportPublicParametersJob-CJQWDKmasternistP256212.29 μs0.567 μs0.503 μs1.000.00
ExportPrivateParametersJob-FCMYUVfast-ecdsanistP2564,785.72 μs20.711 μs19.373 μs0.990.00
ExportPrivateParametersJob-CJQWDKmasternistP2564,813.65 μs19.685 μs17.450 μs1.000.00
ExportPublicParametersJob-FCMYUVfast-ecdsanistP25663.79 μs0.260 μs0.243 μs1.020.01
ExportPublicParametersJob-CJQWDKmasternistP25662.81 μs0.387 μs0.362 μs1.000.00
ImportPrivateParametersJob-FCMYUVfast-ecdsanistP38440,911.78 μs189.020 μs157.841 μs1.120.01
ImportPrivateParametersJob-CJQWDKmasternistP38436,636.35 μs334.361 μs296.402 μs1.000.00
ImportPublicParametersJob-FCMYUVfast-ecdsanistP384494.84 μs1.100 μs0.859 μs2.350.01
ImportPublicParametersJob-CJQWDKmasternistP384210.55 μs0.663 μs0.620 μs1.000.00
ExportPrivateParametersJob-FCMYUVfast-ecdsanistP3844,793.74 μs17.751 μs13.859 μs0.990.01
ExportPrivateParametersJob-CJQWDKmasternistP3844,820.73 μs26.000 μs23.049 μs1.000.00
ExportPublicParametersJob-FCMYUVfast-ecdsanistP38462.36 μs0.344 μs0.322 μs0.980.01
ExportPublicParametersJob-CJQWDKmasternistP38463.75 μs0.275 μs0.215 μs1.000.00
ImportPrivateParametersJob-FCMYUVfast-ecdsanistP52173,257.64 μs930.959 μs870.820 μs1.070.02
ImportPrivateParametersJob-CJQWDKmasternistP52168,366.20 μs874.672 μs818.169 μs1.000.00
ImportPublicParametersJob-FCMYUVfast-ecdsanistP521504.41 μs1.454 μs1.360 μs2.260.10
ImportPublicParametersJob-CJQWDKmasternistP521220.97 μs4.129 μs7.551 μs1.000.00
ExportPrivateParametersJob-FCMYUVfast-ecdsanistP5214,808.95 μs18.758 μs16.628 μs0.960.01
ExportPrivateParametersJob-CJQWDKmasternistP5215,005.39 μs37.495 μs35.073 μs1.000.00
ExportPublicParametersJob-FCMYUVfast-ecdsanistP52163.73 μs0.298 μs0.279 μs0.970.02
ExportPublicParametersJob-CJQWDKmasternistP52165.62 μs1.293 μs1.681 μs1.000.00

Code is here: https://github.com/vcsjones/DotNetCryptoBenches

@ghost

Copy link
Copy Markdown

Tagging subscribers to this area: @bartonjs, @vcsjones, @krwq
Notify danmosemsft if you want to be subscribed.

@vcsjones

Copy link
Copy Markdown
MemberAuthor

I guess what I am looking to get input on:

  1. Are the performance gains worth the introduced complexity of essentially supporting two different sign / verify paths on macOS?
  2. Are the performance gains relevant to where data keys exist? e.g. generated and imported? Or do we expect most keys to come from PFX / keychains?
    1. This may be possible for PFX and in some cases keychain keys. The limiting factor is that the private key needs to exportable. If we can't do that, we can't key it in to key services.
  3. Are the performance losses on generate and import acceptable?

@vcsjones
vcsjonesforce-pushed the fix-36107-crypto-perf branch from 0f9cca1 to ad615e6CompareJune 18, 2020 17:19

@bartonjsbartonjs left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Things seem.... OK... though I'm confused now about what the difference is between new and old, and am curious to see if things can collapse (when I looked previously I thought they were completely parallel key types, not both SecKeyRef).

If the big difference is that now (vs when I wrote all of this) keys work without being stored in the keychain, but PFX export fails unless it's in a keychain, then I'd hope for an approach like:

  • KeyGen makes only the purely ephemeral keys ("data keys"?)
  • I assume creating a SecIdentityRef still requires a keychain, so CopyWithPrivateKey would need to find out that the private key doesn't have a keychain and deal with that then.
  • The Get*PublicKey methods on a cert should just export the keychain-based key and return a new ephemeral key, it looks like that's always guaranteed to make the first public key usage faster, by a couple orders of magnitude.
  • The Get*PrivateKey methods can opportunistically export the keychain-based key and make it into an ephemeral keypair. If export fails, use an ephemeral public key along with a keychain private key.

Assuming that all keys can go through the simpler-looking API, we'd just route them all there. If the new API isn't available on iOS, but the old one is, we can hopefully use the one signature and make it use the new API when available and the old API when required. Then all the complexities are around key->cert and cert->key, but once we have a key everything makes sense.

{
Success => dataKey,
kErrorSeeError => throw CreateExceptionForCFError(errorHandle),
_ => throw new CryptographicException { HResult = result }

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The unexpected error codes should Debug.Fail, then probably just throw a default CryptographicException (consistency with the existing paths).

SecKeyPair keys = GetKeys();
byte[] formattedSignature = AsymmetricAlgorithmHelpers.ConvertIeee1363ToDer(signature);

if (keys.PublicDataKey != null)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would there ever be a situation where we can't get a public data key?

internal SafeSecKeyRefHandle? PrivateDataKey { get; private set; }

private SecKeyPair(SafeSecKeyRefHandle publicKey, SafeSecKeyRefHandle? privateKey)
private SecKeyPair(SafeSecKeyRefHandle publicKey, SafeSecKeyRefHandle? privateKey, SafeSecKeyRefHandle? publicDataKey, SafeSecKeyRefHandle? privateDataKey)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the SecKeyRef version of the public key needed for (abstractly)? It seems like all public keys are exportable, all exportable keys can be made into data keys, and data keys are faster.

Can this pair type be a triplet instead of a quadret? PublicDataKey / PrivateDataKey / PrivateKey

internal SafeSecKeyRefHandle? PrivateDataKey { get; private set; }

private SecKeyPair(SafeSecKeyRefHandle publicKey, SafeSecKeyRefHandle? privateKey)
private SecKeyPair(SafeSecKeyRefHandle publicKey, SafeSecKeyRefHandle? privateKey, SafeSecKeyRefHandle? publicDataKey, SafeSecKeyRefHandle? privateDataKey)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, now I'm confused. If the existing keys were SecKeyRefs, and the new keys are SecKeyRefs, what's the difference, and why don't they interoperate?

I'm not finding the data in the docs

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's... hard to articulate and compounded by I think there is at least one bug in MacOS that I am trying to get chased down. I'll experiment a bit and come back with a more thorough explanation of things.

Now that I have Big Sur working and runtime building on it (surprise, nothing in S.S.C broke so far) I wonder if I will still hit quirks.

@vcsjones

Copy link
Copy Markdown
MemberAuthor

Going to close this for now, will make a new PR when it makes sense.

@ghostghost locked as resolved and limited conversation to collaborators Dec 8, 2020
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@vcsjones@bartonjs@Dotnet-GitSync-Bot