From f239c8711e60d2ec354f56897e7cdf0763bd5f47 Mon Sep 17 00:00:00 2001 From: Heath Stewart Date: Fri, 18 Oct 2019 18:42:04 -0700 Subject: [PATCH 1/3] Add EventSource logging to CryptographyClient Fixes Azure #7548 --- .../src/Shared/EventSourceEventFormatting.cs | 2 +- sdk/core/Azure.Core/tests/TestFramework.props | 1 + .../{ => TestFramework}/TestEventListener.cs | 0 .../TestEventListenerExtensions.cs | 0 ...zure.Security.KeyVault.Certificates.csproj | 2 +- .../src/Azure.Security.KeyVault.Keys.csproj | 2 +- .../Cryptography/AesCryptographyProvider.cs | 4 +- .../src/Cryptography/CryptographyClient.cs | 149 ++++---- .../Cryptography/EcCryptographyProvider.cs | 24 +- .../Cryptography/RemoteCryptographyClient.cs | 1 - .../Cryptography/RsaCryptographyProvider.cs | 71 ++-- .../src/KeysEventSource.cs | 96 ++++++ .../Azure.Security.KeyVault.Keys.Tests.csproj | 2 + .../tests/EcCryptographyProviderTests.cs | 3 +- .../tests/KeysEventSourceTests.cs | 320 ++++++++++++++++++ .../tests/ThrowingCryptographyProvider.cs | 43 +++ .../Azure.Security.KeyVault.Secrets.csproj | 2 +- .../Azure.Security.KeyVault.Shared.projitems | 0 .../Azure.Security.KeyVault.Shared.shproj | 0 .../{ => src}/Base64Url.cs | 0 .../ChallengeBasedAuthenticationPolicy.cs | 0 .../{ => src}/ClientOptionsExtensions.cs | 0 .../{ => src}/IJsonSerializable.cs | 0 .../{ => src}/KeyVaultIdentifier.cs | 0 .../{ => src}/KeyVaultPage.cs | 0 .../{ => src}/KeyVaultPipeline.cs | 0 ...e.Security.KeyVault.Shared.Tests.projitems | 14 + ...zure.Security.KeyVault.Shared.Tests.shproj | 13 + .../TestExtensions.cs | 20 ++ sdk/keyvault/Azure.Security.KeyVault.sln | 7 +- 30 files changed, 666 insertions(+), 110 deletions(-) rename sdk/core/Azure.Core/tests/{ => TestFramework}/TestEventListener.cs (100%) rename sdk/core/Azure.Core/tests/{ => TestFramework}/TestEventListenerExtensions.cs (100%) create mode 100644 sdk/keyvault/Azure.Security.KeyVault.Keys/src/KeysEventSource.cs create mode 100644 sdk/keyvault/Azure.Security.KeyVault.Keys/tests/KeysEventSourceTests.cs create mode 100644 sdk/keyvault/Azure.Security.KeyVault.Keys/tests/ThrowingCryptographyProvider.cs rename sdk/keyvault/Azure.Security.KeyVault.Shared/{ => src}/Azure.Security.KeyVault.Shared.projitems (100%) rename sdk/keyvault/Azure.Security.KeyVault.Shared/{ => src}/Azure.Security.KeyVault.Shared.shproj (100%) rename sdk/keyvault/Azure.Security.KeyVault.Shared/{ => src}/Base64Url.cs (100%) rename sdk/keyvault/Azure.Security.KeyVault.Shared/{ => src}/ChallengeBasedAuthenticationPolicy.cs (100%) rename sdk/keyvault/Azure.Security.KeyVault.Shared/{ => src}/ClientOptionsExtensions.cs (100%) rename sdk/keyvault/Azure.Security.KeyVault.Shared/{ => src}/IJsonSerializable.cs (100%) rename sdk/keyvault/Azure.Security.KeyVault.Shared/{ => src}/KeyVaultIdentifier.cs (100%) rename sdk/keyvault/Azure.Security.KeyVault.Shared/{ => src}/KeyVaultPage.cs (100%) rename sdk/keyvault/Azure.Security.KeyVault.Shared/{ => src}/KeyVaultPipeline.cs (100%) create mode 100644 sdk/keyvault/Azure.Security.KeyVault.Shared/tests/Azure.Security.KeyVault.Shared.Tests/Azure.Security.KeyVault.Shared.Tests.projitems create mode 100644 sdk/keyvault/Azure.Security.KeyVault.Shared/tests/Azure.Security.KeyVault.Shared.Tests/Azure.Security.KeyVault.Shared.Tests.shproj create mode 100644 sdk/keyvault/Azure.Security.KeyVault.Shared/tests/Azure.Security.KeyVault.Shared.Tests/TestExtensions.cs diff --git a/sdk/core/Azure.Core/src/Shared/EventSourceEventFormatting.cs b/sdk/core/Azure.Core/src/Shared/EventSourceEventFormatting.cs index 216543087f78..e5d7f71d7853 100644 --- a/sdk/core/Azure.Core/src/Shared/EventSourceEventFormatting.cs +++ b/sdk/core/Azure.Core/src/Shared/EventSourceEventFormatting.cs @@ -8,7 +8,7 @@ namespace Azure.Core.Shared { - internal class EventSourceEventFormatting + internal static class EventSourceEventFormatting { public static string Format(EventWrittenEventArgs eventData) { diff --git a/sdk/core/Azure.Core/tests/TestFramework.props b/sdk/core/Azure.Core/tests/TestFramework.props index b9ebf0a7dfa9..3580a15f73eb 100644 --- a/sdk/core/Azure.Core/tests/TestFramework.props +++ b/sdk/core/Azure.Core/tests/TestFramework.props @@ -7,6 +7,7 @@ + diff --git a/sdk/core/Azure.Core/tests/TestEventListener.cs b/sdk/core/Azure.Core/tests/TestFramework/TestEventListener.cs similarity index 100% rename from sdk/core/Azure.Core/tests/TestEventListener.cs rename to sdk/core/Azure.Core/tests/TestFramework/TestEventListener.cs diff --git a/sdk/core/Azure.Core/tests/TestEventListenerExtensions.cs b/sdk/core/Azure.Core/tests/TestFramework/TestEventListenerExtensions.cs similarity index 100% rename from sdk/core/Azure.Core/tests/TestEventListenerExtensions.cs rename to sdk/core/Azure.Core/tests/TestFramework/TestEventListenerExtensions.cs diff --git a/sdk/keyvault/Azure.Security.KeyVault.Certificates/src/Azure.Security.KeyVault.Certificates.csproj b/sdk/keyvault/Azure.Security.KeyVault.Certificates/src/Azure.Security.KeyVault.Certificates.csproj index f018fca8822e..fb23040c6d53 100644 --- a/sdk/keyvault/Azure.Security.KeyVault.Certificates/src/Azure.Security.KeyVault.Certificates.csproj +++ b/sdk/keyvault/Azure.Security.KeyVault.Certificates/src/Azure.Security.KeyVault.Certificates.csproj @@ -28,7 +28,7 @@ - + diff --git a/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Azure.Security.KeyVault.Keys.csproj b/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Azure.Security.KeyVault.Keys.csproj index 7e36a2692634..a0b44ed8b756 100644 --- a/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Azure.Security.KeyVault.Keys.csproj +++ b/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Azure.Security.KeyVault.Keys.csproj @@ -23,7 +23,7 @@ - + diff --git a/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Cryptography/AesCryptographyProvider.cs b/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Cryptography/AesCryptographyProvider.cs index 5e70d49f3a20..ce527bae06b7 100644 --- a/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Cryptography/AesCryptographyProvider.cs +++ b/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Cryptography/AesCryptographyProvider.cs @@ -34,7 +34,7 @@ public override UnwrapResult UnwrapKey(KeyWrapAlgorithm algorithm, byte[] encryp int algorithmKeySizeBytes = algorithm.GetKeySizeInBytes(); if (algorithmKeySizeBytes == 0) { - // TODO: Log that we don't support the algorithm locally. + KeysEventSource.Singleton.AlgorithmNotSupported(nameof(UnwrapKey), ref algorithm); return null; } @@ -66,7 +66,7 @@ public override WrapResult WrapKey(KeyWrapAlgorithm algorithm, byte[] key, Cance int algorithmKeySizeBytes = algorithm.GetKeySizeInBytes(); if (algorithmKeySizeBytes == 0) { - // TODO: Log that we don't support the algorithm locally. + KeysEventSource.Singleton.AlgorithmNotSupported(nameof(WrapKey), ref algorithm); return null; } diff --git a/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Cryptography/CryptographyClient.cs b/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Cryptography/CryptographyClient.cs index 6fc99eb7b501..d99c1ec36752 100644 --- a/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Cryptography/CryptographyClient.cs +++ b/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Cryptography/CryptographyClient.cs @@ -72,7 +72,7 @@ internal CryptographyClient(Uri keyId, TokenCredential credential, CryptographyC } } - internal CryptographyClient(KeyVaultKey key, TokenCredential credential, CryptographyClientOptions options) + internal CryptographyClient(KeyVaultKey key, TokenCredential credential, CryptographyClientOptions options, ICryptographyProvider provider = null) { Argument.AssertNotNull(key, nameof(key)); Argument.AssertNotNull(credential, nameof(credential)); @@ -90,7 +90,7 @@ internal CryptographyClient(KeyVaultKey key, TokenCredential credential, Cryptog _pipeline = remoteClient.Pipeline; _remoteProvider = remoteClient; - _provider = LocalCryptographyProviderFactory.Create(key); + _provider = provider ?? LocalCryptographyProviderFactory.Create(key); } internal CryptographyClient(KeyVaultKey key, KeyVaultPipeline pipeline) @@ -155,7 +155,7 @@ public virtual async Task EncryptAsync(EncryptionAlgorithm algori { if (_provider is null) { - await InitializeAsync(cancellationToken).ConfigureAwait(false); + await InitializeAsync(nameof(Encrypt), cancellationToken).ConfigureAwait(false); } EncryptResult result = null; @@ -165,9 +165,10 @@ public virtual async Task EncryptAsync(EncryptionAlgorithm algori { result = await _provider.EncryptAsync(algorithm, plaintext, cancellationToken).ConfigureAwait(false); } - catch (CryptographicException) when (_provider.ShouldRemote) + catch (CryptographicException ex) when (_provider.ShouldRemote) { - // TODO: Log that a cryptographic exception occured and we'll try remotely. + // Use the non-async name as we do for scope. + KeysEventSource.Singleton.CryptographicException(nameof(Encrypt), ex); } } @@ -208,7 +209,7 @@ public virtual EncryptResult Encrypt(EncryptionAlgorithm algorithm, byte[] plain { if (_provider is null) { - Initialize(cancellationToken); + Initialize(nameof(Encrypt), cancellationToken); } EncryptResult result = null; @@ -218,9 +219,9 @@ public virtual EncryptResult Encrypt(EncryptionAlgorithm algorithm, byte[] plain { result = _provider.Encrypt(algorithm, plaintext, cancellationToken); } - catch (CryptographicException) when (_provider.ShouldRemote) + catch (CryptographicException ex) when (_provider.ShouldRemote) { - // TODO: Log that a cryptographic exception occured and we'll try remotely. + KeysEventSource.Singleton.CryptographicException(nameof(Encrypt), ex); } } @@ -261,7 +262,7 @@ public virtual async Task DecryptAsync(EncryptionAlgorithm algori { if (_provider is null) { - await InitializeAsync(cancellationToken).ConfigureAwait(false); + await InitializeAsync(nameof(Decrypt), cancellationToken).ConfigureAwait(false); } DecryptResult result = null; @@ -271,9 +272,10 @@ public virtual async Task DecryptAsync(EncryptionAlgorithm algori { result = await _provider.DecryptAsync(algorithm, ciphertext, cancellationToken).ConfigureAwait(false); } - catch (CryptographicException) when (_provider.ShouldRemote) + catch (CryptographicException ex) when (_provider.ShouldRemote) { - // TODO: Log that a cryptographic exception occured and we'll try remotely. + // Use the non-async name as we do for scope. + KeysEventSource.Singleton.CryptographicException(nameof(Decrypt), ex); } } @@ -314,7 +316,7 @@ public virtual DecryptResult Decrypt(EncryptionAlgorithm algorithm, byte[] ciphe { if (_provider is null) { - Initialize(cancellationToken); + Initialize(nameof(Decrypt), cancellationToken); } DecryptResult result = null; @@ -324,9 +326,9 @@ public virtual DecryptResult Decrypt(EncryptionAlgorithm algorithm, byte[] ciphe { result = _provider.Decrypt(algorithm, ciphertext, cancellationToken); } - catch (CryptographicException) when (_provider.ShouldRemote) + catch (CryptographicException ex) when (_provider.ShouldRemote) { - // TODO: Log that a cryptographic exception occured and we'll try remotely. + KeysEventSource.Singleton.CryptographicException(nameof(Decrypt), ex); } } @@ -367,7 +369,7 @@ public virtual async Task WrapKeyAsync(KeyWrapAlgorithm algorithm, b { if (_provider is null) { - await InitializeAsync(cancellationToken).ConfigureAwait(false); + await InitializeAsync(nameof(WrapKey), cancellationToken).ConfigureAwait(false); } WrapResult result = null; @@ -377,9 +379,10 @@ public virtual async Task WrapKeyAsync(KeyWrapAlgorithm algorithm, b { result = await _provider.WrapKeyAsync(algorithm, key, cancellationToken).ConfigureAwait(false); } - catch (CryptographicException) when (_provider.ShouldRemote) + catch (CryptographicException ex) when (_provider.ShouldRemote) { - // TODO: Log that a cryptographic exception occured and we'll try remotely. + // Use the non-async name as we do for scope. + KeysEventSource.Singleton.CryptographicException(nameof(WrapKey), ex); } } @@ -420,7 +423,7 @@ public virtual WrapResult WrapKey(KeyWrapAlgorithm algorithm, byte[] key, Cancel { if (_provider is null) { - Initialize(cancellationToken); + Initialize(nameof(WrapKey), cancellationToken); } WrapResult result = null; @@ -430,9 +433,9 @@ public virtual WrapResult WrapKey(KeyWrapAlgorithm algorithm, byte[] key, Cancel { result = _provider.WrapKey(algorithm, key, cancellationToken); } - catch (CryptographicException) when (_provider.ShouldRemote) + catch (CryptographicException ex) when (_provider.ShouldRemote) { - // TODO: Log that a cryptographic exception occured and we'll try remotely. + KeysEventSource.Singleton.CryptographicException(nameof(WrapKey), ex); } } @@ -473,7 +476,7 @@ public virtual async Task UnwrapKeyAsync(KeyWrapAlgorithm algorith { if (_provider is null) { - await InitializeAsync(cancellationToken).ConfigureAwait(false); + await InitializeAsync(nameof(UnwrapKey), cancellationToken).ConfigureAwait(false); } UnwrapResult result = null; @@ -483,9 +486,10 @@ public virtual async Task UnwrapKeyAsync(KeyWrapAlgorithm algorith { result = await _provider.UnwrapKeyAsync(algorithm, encryptedKey, cancellationToken).ConfigureAwait(false); } - catch (CryptographicException) when (_provider.ShouldRemote) + catch (CryptographicException ex) when (_provider.ShouldRemote) { - // TODO: Log that a cryptographic exception occured and we'll try remotely. + // Use the non-async name as we do for scope. + KeysEventSource.Singleton.CryptographicException(nameof(UnwrapKey), ex); } } @@ -526,7 +530,7 @@ public virtual UnwrapResult UnwrapKey(KeyWrapAlgorithm algorithm, byte[] encrypt { if (_provider is null) { - Initialize(cancellationToken); + Initialize(nameof(UnwrapKey), cancellationToken); } UnwrapResult result = null; @@ -536,9 +540,9 @@ public virtual UnwrapResult UnwrapKey(KeyWrapAlgorithm algorithm, byte[] encrypt { result = _provider.UnwrapKey(algorithm, encryptedKey, cancellationToken); } - catch (CryptographicException) when (_provider.ShouldRemote) + catch (CryptographicException ex) when (_provider.ShouldRemote) { - // TODO: Log that a cryptographic exception occured and we'll try remotely. + KeysEventSource.Singleton.CryptographicException(nameof(UnwrapKey), ex); } } @@ -579,7 +583,7 @@ public virtual async Task SignAsync(SignatureAlgorithm algorithm, by { if (_provider is null) { - await InitializeAsync(cancellationToken).ConfigureAwait(false); + await InitializeAsync(nameof(Sign), cancellationToken).ConfigureAwait(false); } SignResult result = null; @@ -589,9 +593,10 @@ public virtual async Task SignAsync(SignatureAlgorithm algorithm, by { result = await _provider.SignAsync(algorithm, digest, cancellationToken).ConfigureAwait(false); } - catch (CryptographicException) when (_provider.ShouldRemote) + catch (CryptographicException ex) when (_provider.ShouldRemote) { - // TODO: Log that a cryptographic exception occured and we'll try remotely. + // Use the non-async name as we do for scope. + KeysEventSource.Singleton.CryptographicException(nameof(Sign), ex); } } @@ -632,7 +637,7 @@ public virtual SignResult Sign(SignatureAlgorithm algorithm, byte[] digest, Canc { if (_provider is null) { - Initialize(cancellationToken); + Initialize(nameof(Sign), cancellationToken); } SignResult result = null; @@ -642,9 +647,9 @@ public virtual SignResult Sign(SignatureAlgorithm algorithm, byte[] digest, Canc { result = _provider.Sign(algorithm, digest, cancellationToken); } - catch (CryptographicException) when (_provider.ShouldRemote) + catch (CryptographicException ex) when (_provider.ShouldRemote) { - // TODO: Log that a cryptographic exception occured and we'll try remotely. + KeysEventSource.Singleton.CryptographicException(nameof(Sign), ex); } } @@ -685,7 +690,7 @@ public virtual async Task VerifyAsync(SignatureAlgorithm algorithm { if (_provider is null) { - await InitializeAsync(cancellationToken).ConfigureAwait(false); + await InitializeAsync(nameof(Verify), cancellationToken).ConfigureAwait(false); } VerifyResult result = null; @@ -695,9 +700,10 @@ public virtual async Task VerifyAsync(SignatureAlgorithm algorithm { result = await _provider.VerifyAsync(algorithm, digest, signature, cancellationToken).ConfigureAwait(false); } - catch (CryptographicException) when (_provider.ShouldRemote) + catch (CryptographicException ex) when (_provider.ShouldRemote) { - // TODO: Log that a cryptographic exception occured and we'll try remotely. + // Use the non-async name as we do for scope. + KeysEventSource.Singleton.CryptographicException(nameof(Verify), ex); } } @@ -738,7 +744,7 @@ public virtual VerifyResult Verify(SignatureAlgorithm algorithm, byte[] digest, { if (_provider is null) { - Initialize(cancellationToken); + Initialize(nameof(Verify), cancellationToken); } VerifyResult result = null; @@ -748,9 +754,9 @@ public virtual VerifyResult Verify(SignatureAlgorithm algorithm, byte[] digest, { result = _provider.Verify(algorithm, digest, signature, cancellationToken); } - catch (CryptographicException) when (_provider.ShouldRemote) + catch (CryptographicException ex) when (_provider.ShouldRemote) { - // TODO: Log that a cryptographic exception occured and we'll try remotely. + KeysEventSource.Singleton.CryptographicException(nameof(Verify), ex); } } @@ -795,7 +801,7 @@ public virtual async Task SignDataAsync(SignatureAlgorithm algorithm if (_provider is null) { - await InitializeAsync(cancellationToken).ConfigureAwait(false); + await InitializeAsync(nameof(SignData), cancellationToken).ConfigureAwait(false); } SignResult result = null; @@ -805,9 +811,10 @@ public virtual async Task SignDataAsync(SignatureAlgorithm algorithm { result = await _provider.SignAsync(algorithm, digest, cancellationToken).ConfigureAwait(false); } - catch (CryptographicException) when (_provider.ShouldRemote) + catch (CryptographicException ex) when (_provider.ShouldRemote) { - // TODO: Log that a cryptographic exception occured and we'll try remotely. + // Use the non-async name as we do for scope. + KeysEventSource.Singleton.CryptographicException(nameof(SignData), ex); } } @@ -852,7 +859,7 @@ public virtual SignResult SignData(SignatureAlgorithm algorithm, byte[] data, Ca if (_provider is null) { - Initialize(cancellationToken); + Initialize(nameof(SignData), cancellationToken); } SignResult result = null; @@ -862,9 +869,10 @@ public virtual SignResult SignData(SignatureAlgorithm algorithm, byte[] data, Ca { result = _provider.Sign(algorithm, digest, cancellationToken); } - catch (CryptographicException) when (_provider.ShouldRemote) + catch (CryptographicException ex) when (_provider.ShouldRemote) { - // TODO: Log that a cryptographic exception occured and we'll try remotely. + // Use the non-async name as we do for scope. + KeysEventSource.Singleton.CryptographicException(nameof(SignData), ex); } } @@ -910,7 +918,7 @@ public virtual async Task SignDataAsync(SignatureAlgorithm algorithm if (_provider is null) { - await InitializeAsync(cancellationToken).ConfigureAwait(false); + await InitializeAsync(nameof(SignData), cancellationToken).ConfigureAwait(false); } SignResult result = null; @@ -920,9 +928,10 @@ public virtual async Task SignDataAsync(SignatureAlgorithm algorithm { result = await _provider.SignAsync(algorithm, digest, cancellationToken).ConfigureAwait(false); } - catch (CryptographicException) when (_provider.ShouldRemote) + catch (CryptographicException ex) when (_provider.ShouldRemote) { - // TODO: Log that a cryptographic exception occured and we'll try remotely. + // Use the non-async name as we do for scope. + KeysEventSource.Singleton.CryptographicException(nameof(SignData), ex); } } @@ -968,7 +977,7 @@ public virtual SignResult SignData(SignatureAlgorithm algorithm, Stream data, Ca if (_provider is null) { - Initialize(cancellationToken); + Initialize(nameof(SignData), cancellationToken); } SignResult result = null; @@ -978,9 +987,9 @@ public virtual SignResult SignData(SignatureAlgorithm algorithm, Stream data, Ca { result = _provider.Sign(algorithm, digest, cancellationToken); } - catch (CryptographicException) when (_provider.ShouldRemote) + catch (CryptographicException ex) when (_provider.ShouldRemote) { - // TODO: Log that a cryptographic exception occured and we'll try remotely. + KeysEventSource.Singleton.CryptographicException(nameof(SignData), ex); } } @@ -1026,7 +1035,7 @@ public virtual async Task VerifyDataAsync(SignatureAlgorithm algor if (_provider is null) { - await InitializeAsync(cancellationToken).ConfigureAwait(false); + await InitializeAsync(nameof(VerifyData), cancellationToken).ConfigureAwait(false); } VerifyResult result = null; @@ -1036,9 +1045,10 @@ public virtual async Task VerifyDataAsync(SignatureAlgorithm algor { result = await _provider.VerifyAsync(algorithm, digest, signature, cancellationToken).ConfigureAwait(false); } - catch (CryptographicException) when (_provider.ShouldRemote) + catch (CryptographicException ex) when (_provider.ShouldRemote) { - // TODO: Log that a cryptographic exception occured and we'll try remotely. + // Use the non-async name as we do for scope. + KeysEventSource.Singleton.CryptographicException(nameof(VerifyData), ex); } } @@ -1084,7 +1094,7 @@ public virtual VerifyResult VerifyData(SignatureAlgorithm algorithm, byte[] data if (_provider is null) { - Initialize(cancellationToken); + Initialize(nameof(VerifyData), cancellationToken); } VerifyResult result = null; @@ -1094,9 +1104,9 @@ public virtual VerifyResult VerifyData(SignatureAlgorithm algorithm, byte[] data { result = _provider.Verify(algorithm, digest, signature, cancellationToken); } - catch (CryptographicException) when (_provider.ShouldRemote) + catch (CryptographicException ex) when (_provider.ShouldRemote) { - // TODO: Log that a cryptographic exception occured and we'll try remotely. + KeysEventSource.Singleton.CryptographicException(nameof(VerifyData), ex); } } @@ -1142,7 +1152,7 @@ public virtual async Task VerifyDataAsync(SignatureAlgorithm algor if (_provider is null) { - await InitializeAsync(cancellationToken).ConfigureAwait(false); + await InitializeAsync(nameof(VerifyData), cancellationToken).ConfigureAwait(false); } VerifyResult result = null; @@ -1152,9 +1162,10 @@ public virtual async Task VerifyDataAsync(SignatureAlgorithm algor { result = await _provider.VerifyAsync(algorithm, digest, signature, cancellationToken).ConfigureAwait(false); } - catch (CryptographicException) when (_provider.ShouldRemote) + catch (CryptographicException ex) when (_provider.ShouldRemote) { - // TODO: Log that a cryptographic exception occured and we'll try remotely. + // Use the non-async name as we do for scope. + KeysEventSource.Singleton.CryptographicException(nameof(VerifyData), ex); } } @@ -1200,7 +1211,7 @@ public virtual VerifyResult VerifyData(SignatureAlgorithm algorithm, Stream data if (_provider is null) { - Initialize(cancellationToken); + Initialize(nameof(VerifyData), cancellationToken); } VerifyResult result = null; @@ -1210,9 +1221,9 @@ public virtual VerifyResult VerifyData(SignatureAlgorithm algorithm, Stream data { result = _provider.Verify(algorithm, digest, signature, cancellationToken); } - catch (CryptographicException) when (_provider.ShouldRemote) + catch (CryptographicException ex) when (_provider.ShouldRemote) { - // TODO: Log that a cryptographic exception occured and we'll try remotely. + KeysEventSource.Singleton.CryptographicException(nameof(VerifyData), ex); } } @@ -1276,7 +1287,7 @@ private static byte[] CreateDigest(SignatureAlgorithm algorithm, Stream data) return hashAlgo.ComputeHash(data); } - private async Task InitializeAsync(CancellationToken cancellationToken) + private async Task InitializeAsync(string operation, CancellationToken cancellationToken) { if (_provider != null) { @@ -1290,7 +1301,15 @@ private async Task InitializeAsync(CancellationToken cancellationToken) try { Response key = await _remoteProvider.GetKeyAsync(cancellationToken).ConfigureAwait(false); + _provider = LocalCryptographyProviderFactory.Create(key.Value); + if (_provider is null) + { + KeysEventSource.Singleton.KeyTypeNotSupported(operation, key.Value); + + _provider = _remoteProvider; + return; + } } catch (RequestFailedException e) when (e.Status == 403) { @@ -1305,7 +1324,7 @@ private async Task InitializeAsync(CancellationToken cancellationToken) } } - private void Initialize(CancellationToken cancellationToken) + private void Initialize(string operation, CancellationToken cancellationToken) { if (_provider != null) { @@ -1323,7 +1342,7 @@ private void Initialize(CancellationToken cancellationToken) _provider = LocalCryptographyProviderFactory.Create(key.Value); if (_provider is null) { - // TODO: Log that the key type is unsupported locally. + KeysEventSource.Singleton.KeyTypeNotSupported(operation, key.Value); _provider = _remoteProvider; return; diff --git a/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Cryptography/EcCryptographyProvider.cs b/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Cryptography/EcCryptographyProvider.cs index cf7249eb4aa2..6a69cfd9b74f 100644 --- a/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Cryptography/EcCryptographyProvider.cs +++ b/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Cryptography/EcCryptographyProvider.cs @@ -11,6 +11,7 @@ namespace Azure.Security.KeyVault.Keys.Cryptography internal class EcCryptographyProvider : LocalCryptographyProvider { private readonly KeyCurveName _curve; + private readonly JsonWebKey _keyMaterial; internal EcCryptographyProvider(KeyVaultKey key) : base(key) { @@ -21,25 +22,24 @@ internal EcCryptographyProvider(KeyVaultKey key) : base(key) JsonWebKey keyMaterial = key.Key; if (keyMaterial != null && keyMaterial.CurveName.HasValue) { + // Save the key material to use for operational support validation. + _keyMaterial = keyMaterial; + _curve = keyMaterial.CurveName.Value; if (_curve.IsSupported) { KeyMaterial = keyMaterial; } - else - { - // TODO: Log that we don't support the algorithm locally. - } } } public override bool SupportsOperation(KeyOperation operation) { - if (KeyMaterial != null) + if (_keyMaterial != null) { if (operation == KeyOperation.Sign || operation == KeyOperation.Verify) { - return KeyMaterial.SupportsOperation(operation); + return _keyMaterial.SupportsOperation(operation); } } @@ -55,13 +55,18 @@ public override SignResult Sign(SignatureAlgorithm algorithm, byte[] digest, Can // The JWK is not supported by this client. Send to the server. if (KeyMaterial is null) { + if (KeysEventSource.Singleton.IsEnabled()) + { + KeysEventSource.Singleton.AlgorithmNotSupported(nameof(Sign), _curve.ToString()); + } + return null; } // A private key is required to sign. Send to the server. if (MustRemote) { - // TODO: Log that we need a private key. + KeysEventSource.Singleton.PrivateKeyRequired(nameof(Sign)); return null; } @@ -100,6 +105,11 @@ public override VerifyResult Verify(SignatureAlgorithm algorithm, byte[] digest, // The JWK is not supported by this client. Send to the server. if (KeyMaterial is null) { + if (KeysEventSource.Singleton.IsEnabled()) + { + KeysEventSource.Singleton.AlgorithmNotSupported(nameof(Verify), _curve.ToString()); + } + return null; } diff --git a/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Cryptography/RemoteCryptographyClient.cs b/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Cryptography/RemoteCryptographyClient.cs index 773486bf0ba4..872785cecf2e 100644 --- a/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Cryptography/RemoteCryptographyClient.cs +++ b/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Cryptography/RemoteCryptographyClient.cs @@ -6,7 +6,6 @@ using System; using System.Threading; using System.Threading.Tasks; -using Azure.Core.Diagnostics; namespace Azure.Security.KeyVault.Keys.Cryptography { diff --git a/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Cryptography/RsaCryptographyProvider.cs b/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Cryptography/RsaCryptographyProvider.cs index cb716e1c950b..c5faa891abfb 100644 --- a/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Cryptography/RsaCryptographyProvider.cs +++ b/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Cryptography/RsaCryptographyProvider.cs @@ -33,8 +33,13 @@ public override EncryptResult Encrypt(EncryptionAlgorithm algorithm, byte[] plai ThrowIfTimeInvalid(); RSAEncryptionPadding padding = algorithm.GetRsaEncryptionPadding(); - byte[] ciphertext = Encrypt(plaintext, padding); + if (padding is null) + { + KeysEventSource.Singleton.AlgorithmNotSupported(nameof(Encrypt), ref algorithm); + return null; + } + byte[] ciphertext = Encrypt(plaintext, padding); EncryptResult result = null; if (ciphertext != null) @@ -54,9 +59,21 @@ public override DecryptResult Decrypt(EncryptionAlgorithm algorithm, byte[] ciph { Argument.AssertNotNull(ciphertext, nameof(ciphertext)); + if (MustRemote) + { + // A private key is required to decrypt. Send to the server. + KeysEventSource.Singleton.PrivateKeyRequired(nameof(Decrypt)); + return null; + } + RSAEncryptionPadding padding = algorithm.GetRsaEncryptionPadding(); - byte[] plaintext = Decrypt(ciphertext, padding); + if (padding is null) + { + KeysEventSource.Singleton.AlgorithmNotSupported(nameof(Decrypt), ref algorithm); + return null; + } + byte[] plaintext = Decrypt(ciphertext, padding); DecryptResult result = null; if (plaintext != null) @@ -81,21 +98,21 @@ public override SignResult Sign(SignatureAlgorithm algorithm, byte[] digest, Can // A private key is required to sign. Send to the server. if (MustRemote) { - // TODO: Log that we need a private key. + KeysEventSource.Singleton.PrivateKeyRequired(nameof(Sign)); return null; } HashAlgorithmName hashAlgorithm = algorithm.GetHashAlgorithmName(); if (hashAlgorithm == default) { - // TODO: Log that we don't support the given algorithm. + KeysEventSource.Singleton.AlgorithmNotSupported(nameof(Sign), ref algorithm); return null; } RSASignaturePadding padding = algorithm.GetRsaSignaturePadding(); if (padding is null) { - // TODO: Log that we don't support the given algorithm. + KeysEventSource.Singleton.AlgorithmNotSupported(nameof(Sign), ref algorithm); return null; } @@ -118,14 +135,14 @@ public override VerifyResult Verify(SignatureAlgorithm algorithm, byte[] digest, HashAlgorithmName hashAlgorithm = algorithm.GetHashAlgorithmName(); if (hashAlgorithm == default) { - // TODO: Log that we don't support the given algorithm. + KeysEventSource.Singleton.AlgorithmNotSupported(nameof(Verify), ref algorithm); return null; } RSASignaturePadding padding = algorithm.GetRsaSignaturePadding(); if (padding is null) { - // TODO: Log that we don't support the given algorithm. + KeysEventSource.Singleton.AlgorithmNotSupported(nameof(Verify), ref algorithm); return null; } @@ -147,8 +164,13 @@ public override WrapResult WrapKey(KeyWrapAlgorithm algorithm, byte[] key, Cance ThrowIfTimeInvalid(); RSAEncryptionPadding padding = algorithm.GetRsaEncryptionPadding(); - byte[] encryptedKey = Encrypt(key, padding); + if (padding is null) + { + KeysEventSource.Singleton.AlgorithmNotSupported(nameof(WrapKey), ref algorithm); + return null; + } + byte[] encryptedKey = Encrypt(key, padding); WrapResult result = null; if (encryptedKey != null) @@ -168,9 +190,21 @@ public override UnwrapResult UnwrapKey(KeyWrapAlgorithm algorithm, byte[] encryp { Argument.AssertNotNull(encryptedKey, nameof(encryptedKey)); + if (MustRemote) + { + // A private key is required to decrypt. Send to the server. + KeysEventSource.Singleton.PrivateKeyRequired(nameof(UnwrapKey)); + return null; + } + RSAEncryptionPadding padding = algorithm.GetRsaEncryptionPadding(); - byte[] key = Decrypt(encryptedKey, padding); + if (padding is null) + { + KeysEventSource.Singleton.AlgorithmNotSupported(nameof(UnwrapKey), ref algorithm); + return null; + } + byte[] key = Decrypt(encryptedKey, padding); UnwrapResult result = null; if (key != null) @@ -188,31 +222,12 @@ public override UnwrapResult UnwrapKey(KeyWrapAlgorithm algorithm, byte[] encryp private byte[] Encrypt(byte[] data, RSAEncryptionPadding padding) { - if (padding is null) - { - // TODO: Log that we don't support the given algorithm. - return null; - } - using RSA rsa = KeyMaterial.ToRSA(true); return rsa.Encrypt(data, padding); } private byte[] Decrypt(byte[] data, RSAEncryptionPadding padding) { - // A private key is required to decrypt. Send to the server. - if (MustRemote) - { - // TODO: Log that we need a private key. - return null; - } - - if (padding is null) - { - // TODO: Log that we don't support the given algorithm. - return null; - } - using RSA rsa = KeyMaterial.ToRSA(); return rsa.Decrypt(data, padding); } diff --git a/sdk/keyvault/Azure.Security.KeyVault.Keys/src/KeysEventSource.cs b/sdk/keyvault/Azure.Security.KeyVault.Keys/src/KeysEventSource.cs new file mode 100644 index 000000000000..2eeed4b932de --- /dev/null +++ b/sdk/keyvault/Azure.Security.KeyVault.Keys/src/KeysEventSource.cs @@ -0,0 +1,96 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Diagnostics.Tracing; +using System.Globalization; +using System.Text; +using Azure.Core.Diagnostics; + +namespace Azure.Security.KeyVault.Keys +{ + [EventSource(Name = EventSourceName)] + internal sealed class KeysEventSource : EventSource + { + internal const int AlgorithmNotSupportedEvent = 1; + internal const int KeyTypeNotSupportedEvent = 2; + internal const int PrivateKeyRequiredEvent = 3; + internal const int CryptographicExceptionEvent = 4; + + private const string EventSourceName = "Azure-Security-KeyVault-Keys"; + + private KeysEventSource() : base(EventSourceName, EventSourceSettings.Default, AzureEventSourceListener.TraitName, AzureEventSourceListener.TraitValue) { } + + public static KeysEventSource Singleton { get; } = new KeysEventSource(); + + [NonEvent] + public void AlgorithmNotSupported(string operation, ref T algorithm) where T : struct + { + if (IsEnabled()) + { + AlgorithmNotSupported(operation, algorithm.ToString()); + } + } + + [Event(AlgorithmNotSupportedEvent, Level = EventLevel.Verbose, Message = "The algorithm {1} is not supported on this machine. Will try the {0} operation in the service.")] + public void AlgorithmNotSupported(string operation, string algorithm) => WriteEvent(AlgorithmNotSupportedEvent, operation, algorithm); + + [NonEvent] + public void KeyTypeNotSupported(string operation, KeyVaultKey key) + { + if (IsEnabled()) + { + string keyType = key?.KeyType.ToString() ?? "(null)"; + KeyTypeNotSupported(operation, keyType); + } + } + + [Event(KeyTypeNotSupportedEvent, Level = EventLevel.Verbose, Message = "The key type {1} is not supported on this machine. Will try the {0} operation in the service.")] + public void KeyTypeNotSupported(string operation, string keyType) => WriteEvent(KeyTypeNotSupportedEvent, operation, keyType); + + [Event(PrivateKeyRequiredEvent, Level = EventLevel.Verbose, Message = "A private key is required for a {0} operation")] + public void PrivateKeyRequired(string operation) => WriteEvent(PrivateKeyRequiredEvent, operation); + + [NonEvent] + public void CryptographicException(string operation, Exception ex) + { + if (IsEnabled()) + { + string message = FormatException(ex); + CryptographicException(operation, message); + } + } + + [Event(CryptographicExceptionEvent, Level = EventLevel.Informational, Message = "A cryptographic exception occured: {1}.\r\nWill try the {0} operation in the service.")] + public void CryptographicException(string operation, string message) => WriteEvent(CryptographicExceptionEvent, operation, message); + + private static string FormatException(Exception ex) + { + StringBuilder sb = new StringBuilder(); + bool nest = false; + + do + { + if (nest) + { + // Format how Exception.ToString() would. + sb.AppendLine() + .Append(" ---> "); + } + + // Do not include StackTrace, but do include HResult (often useful for CryptographicExceptions or IOExceptions). + sb.Append(ex.GetType().FullName) + .Append(" (0x") + .Append(ex.HResult.ToString("x", CultureInfo.InvariantCulture)) + .Append("): ") + .Append(ex.Message); + + ex = ex.InnerException; + nest = true; + } + while (ex != null); + + return sb.ToString(); + } + } +} diff --git a/sdk/keyvault/Azure.Security.KeyVault.Keys/tests/Azure.Security.KeyVault.Keys.Tests.csproj b/sdk/keyvault/Azure.Security.KeyVault.Keys/tests/Azure.Security.KeyVault.Keys.Tests.csproj index 7933807ea7b6..2863fead4e57 100644 --- a/sdk/keyvault/Azure.Security.KeyVault.Keys/tests/Azure.Security.KeyVault.Keys.Tests.csproj +++ b/sdk/keyvault/Azure.Security.KeyVault.Keys/tests/Azure.Security.KeyVault.Keys.Tests.csproj @@ -5,6 +5,8 @@ $(RequiredTargetFrameworks);net461;net47 + + diff --git a/sdk/keyvault/Azure.Security.KeyVault.Keys/tests/EcCryptographyProviderTests.cs b/sdk/keyvault/Azure.Security.KeyVault.Keys/tests/EcCryptographyProviderTests.cs index 3495e6c3f45f..a71109d9bd00 100644 --- a/sdk/keyvault/Azure.Security.KeyVault.Keys/tests/EcCryptographyProviderTests.cs +++ b/sdk/keyvault/Azure.Security.KeyVault.Keys/tests/EcCryptographyProviderTests.cs @@ -37,7 +37,8 @@ public void SupportsOperationUnsupportedCurve() EcCryptographyProvider client = new EcCryptographyProvider(new KeyVaultKey { Key = jwk }); - Assert.IsFalse(client.SupportsOperation(KeyOperation.Sign)); + // The provider caches the original allow key operations to facilitate tracing. Operation will still be sent to the service. + Assert.IsTrue(client.SupportsOperation(KeyOperation.Sign)); } [Test] diff --git a/sdk/keyvault/Azure.Security.KeyVault.Keys/tests/KeysEventSourceTests.cs b/sdk/keyvault/Azure.Security.KeyVault.Keys/tests/KeysEventSourceTests.cs new file mode 100644 index 000000000000..d170c0e5de08 --- /dev/null +++ b/sdk/keyvault/Azure.Security.KeyVault.Keys/tests/KeysEventSourceTests.cs @@ -0,0 +1,320 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections; +using System.Diagnostics.Tracing; +using System.Security.Cryptography; +using System.Threading.Tasks; +using Azure.Core.Pipeline; +using Azure.Core.Testing; +using Azure.Identity; +using Azure.Security.KeyVault.Keys.Cryptography; +using Azure.Security.KeyVault.Tests; +using NUnit.Framework; + +namespace Azure.Security.KeyVault.Keys.Tests +{ + [NonParallelizable] + public class KeysEventSourceTests : ClientTestBase + { + private const string KeyId = "https://localhost/keys/test"; + private TestEventListener _listener; + + public KeysEventSourceTests(bool isAsync) : base(isAsync) + { + } + + [SetUp] + public void Setup() + { + _listener = new TestEventListener(); + _listener.EnableEvents(KeysEventSource.Singleton, EventLevel.Verbose); + } + + [TearDown] + public void TearDown() + { + _listener.Dispose(); + } + + [Test] + public void MatchesNameAndGuid() + { + Type eventSourceType = typeof(KeysEventSource); + + Assert.NotNull(eventSourceType); + Assert.AreEqual("Azure-Security-KeyVault-Keys", EventSource.GetName(eventSourceType)); + Assert.AreEqual(Guid.Parse("657a121e-762e-50da-b233-05d7cdb24eb8"), EventSource.GetGuid(eventSourceType)); + Assert.IsNotEmpty(EventSource.GenerateManifest(eventSourceType, "assemblyPathToIncludeInManifest")); + } + + [TestCaseSource(nameof(GetAesOperations))] + public async Task AlgorithmNotSupportedAes(string operation, Func> thunk) + { + using Aes aes = Aes.Create(); + KeyVaultKey key = new KeyVaultKey + { + Key = new JsonWebKey(aes) + { + Id = KeyId, + }, + }; + + MockResponse response = new MockResponse(200); + response.SetContent(@$"{{""kid"":""{KeyId}"",""value"":""test""}}"); + + MockTransport transport = new MockTransport(response); + CryptographyClient client = CreateClient(key, transport); + + object result = await thunk(client, "invalid"); + Assert.IsNotNull(result); + + EventWrittenEventArgs e = _listener.SingleEventById(KeysEventSource.AlgorithmNotSupportedEvent); + Assert.AreEqual(EventLevel.Verbose, e.Level); + Assert.AreEqual("AlgorithmNotSupported", e.EventName); + Assert.AreEqual("invalid", e.GetProperty("algorithm")); + } + + [TestCaseSource(nameof(GetEcOperations), new object[] { true, true })] + public async Task AlgorithmNotSupportedEc(string operation, string value, Func> thunk) + { + KeyVaultKey key = new KeyVaultKey + { + Key = new JsonWebKey(new[] { KeyOperation.Sign, KeyOperation.Verify }) + { + Id = KeyId, + KeyType = KeyType.Ec, + CurveName = "invalid", + }, + }; + + MockResponse response = new MockResponse(200); + response.SetContent(@$"{{""kid"":""{KeyId}"",""value"":{value ?? @"""test"""}}}"); + + CryptographyClient client = CreateClient(key, new MockTransport(response)); + + object result = await thunk(client, "invalid"); + Assert.IsNotNull(result); + + EventWrittenEventArgs e = _listener.SingleEventById(KeysEventSource.AlgorithmNotSupportedEvent); + Assert.AreEqual(EventLevel.Verbose, e.Level); + Assert.AreEqual("AlgorithmNotSupported", e.EventName); + Assert.AreEqual(operation, e.GetProperty("operation")); + Assert.AreEqual("invalid", e.GetProperty("algorithm")); + } + + [TestCaseSource(nameof(GetRsaOperations), new object[] { true, true })] + public async Task AlgorithmNotSupportedRsa(string operation, string value, Func> thunk) + { + using RSA rsa = RSA.Create(); + KeyVaultKey key = new KeyVaultKey + { + Key = new JsonWebKey(rsa, includePrivateParameters: true) + { + Id = KeyId, + }, + }; + + MockResponse response = new MockResponse(200); + response.SetContent(@$"{{""kid"":""{KeyId}"",""value"":{value ?? @"""test"""}}}"); + + CryptographyClient client = CreateClient(key, new MockTransport(response)); + + object result = await thunk(client, "invalid"); + Assert.IsNotNull(result); + + EventWrittenEventArgs e = _listener.SingleEventById(KeysEventSource.AlgorithmNotSupportedEvent); + Assert.AreEqual(EventLevel.Verbose, e.Level); + Assert.AreEqual("AlgorithmNotSupported", e.EventName); + Assert.AreEqual(operation, e.GetProperty("operation")); + Assert.AreEqual("invalid", e.GetProperty("algorithm")); + } + + [TestCaseSource(nameof(GetRsaOperations), new object[] { true, true })] + public async Task KeyTypeNotSupported(string operation, string value, Func> thunk) + { + MockResponse keyResponse = new MockResponse(200); + keyResponse.SetContent($@"{{""key"":{{""kid"":""{KeyId}"",""kty"":""invalid""}}}}"); + + // Get the same key as above and reset the stream. + KeyVaultKey key = new KeyVaultKey(); + key.Deserialize(keyResponse.ContentStream); + keyResponse.ContentStream.Position = 0; + + MockResponse resultResponse = new MockResponse(200); + resultResponse.SetContent(@$"{{""kid"":""{KeyId}"",""value"":{value ?? @"""test"""}}}"); + + CryptographyClient client = CreateClient(key, new MockTransport(keyResponse, resultResponse)); + + object result = await thunk(client, "invalid"); + Assert.IsNotNull(result); + + EventWrittenEventArgs e = _listener.SingleEventById(KeysEventSource.KeyTypeNotSupportedEvent); + Assert.AreEqual(EventLevel.Verbose, e.Level); + Assert.AreEqual("KeyTypeNotSupported", e.EventName); + Assert.AreEqual(operation, e.GetProperty("operation")); + Assert.AreEqual("invalid", e.GetProperty("keyType")); + } + + [TestCaseSource(nameof(GetEcOperations), new object[] { false, true })] + public async Task PrivateKeyRequiredEc(string operation, string value, Func> thunk) + { +#if NET461 + Assert.Ignore("Creating JsonWebKey with ECDsa is not supported on net461."); +#endif + + KeyOperation[] keyOps = new[] + { + KeyOperation.Sign, + KeyOperation.Verify, + }; + + using ECDsa ecdsa = ECDsa.Create(); + KeyVaultKey key = new KeyVaultKey + { + Key = new JsonWebKey(ecdsa, includePrivateParameters: false, keyOps) + { + Id = KeyId, + }, + }; + + MockResponse response = new MockResponse(200); + response.SetContent(@$"{{""kid"":""{KeyId}"",""value"":{value ?? @"""test"""}}}"); + + CryptographyClient client = CreateClient(key, new MockTransport(response)); + + object result = await thunk(client, "invalid"); + Assert.IsNotNull(result); + + EventWrittenEventArgs e = _listener.SingleEventById(KeysEventSource.PrivateKeyRequiredEvent); + Assert.AreEqual(EventLevel.Verbose, e.Level); + Assert.AreEqual("PrivateKeyRequired", e.EventName); + Assert.AreEqual(operation, e.GetProperty("operation")); + } + + [TestCaseSource(nameof(GetRsaOperations), new object[] { false, true })] + public async Task PrivateKeyRequiredRsa(string operation, string value, Func> thunk) + { + KeyOperation[] keyOps = new[] + { + KeyOperation.Encrypt, + KeyOperation.Decrypt, + KeyOperation.Sign, + KeyOperation.Verify, + KeyOperation.WrapKey, + KeyOperation.UnwrapKey, + }; + + using RSA rsa = RSA.Create(); + KeyVaultKey key = new KeyVaultKey + { + Key = new JsonWebKey(rsa, includePrivateParameters: false, keyOps) + { + Id = KeyId, + }, + }; + + MockResponse response = new MockResponse(200); + response.SetContent(@$"{{""kid"":""{KeyId}"",""value"":{value ?? @"""test"""}}}"); + + CryptographyClient client = CreateClient(key, new MockTransport(response)); + + object result = await thunk(client, "invalid"); + Assert.IsNotNull(result); + + EventWrittenEventArgs e = _listener.SingleEventById(KeysEventSource.PrivateKeyRequiredEvent); + Assert.AreEqual(EventLevel.Verbose, e.Level); + Assert.AreEqual("PrivateKeyRequired", e.EventName); + Assert.AreEqual(operation, e.GetProperty("operation")); + } + + [TestCaseSource(nameof(GetRsaOperations), new object[] { true, true })] + public async Task CryptographicException(string operation, string value, Func> thunk) + { + using RSA rsa = RSA.Create(); + KeyVaultKey key = new KeyVaultKey + { + Key = new JsonWebKey(rsa, includePrivateParameters: true) + { + Id = KeyId, + }, + }; + + Random rand = new Random(); + rand.NextBytes(key.Key.N); + + MockResponse response = new MockResponse(200); + response.SetContent(@$"{{""kid"":""{KeyId}"",""value"":{value ?? @"""test"""}}}"); + + CryptographyClient client = CreateClient(key, new MockTransport(response), new ThrowingCryptographyProvider()); + + object result = await thunk(client, null); + Assert.IsNotNull(result); + + EventWrittenEventArgs e = _listener.SingleEventById(KeysEventSource.CryptographicExceptionEvent); + Assert.AreEqual(EventLevel.Informational, e.Level); + Assert.AreEqual("CryptographicException", e.EventName); + Assert.AreEqual(operation, e.GetProperty("operation")); + StringAssert.StartsWith("System.Security.Cryptography.CryptographicException (0x80092006):", e.GetProperty("message")); + } + + private CryptographyClient CreateClient(KeyVaultKey key, HttpPipelineTransport transport, ICryptographyProvider provider = null) + { + CryptographyClientOptions options = new CryptographyClientOptions + { + Transport = transport, + }; + + return InstrumentClient(new CryptographyClient(key, new DefaultAzureCredential(), options, provider)); + } + + private static IEnumerable GetAesOperations() + { + byte[] buffer = new byte[32]; + new Random().NextBytes(buffer); + + yield return new TestCaseData("WrapKey", new Func>(async (client, algorithm) => await client.WrapKeyAsync(algorithm, buffer))); + yield return new TestCaseData("UnwrapKey", new Func>(async (client, algorithm) => await client.UnwrapKeyAsync(algorithm, buffer))); + } + + private static IEnumerable GetEcOperations(bool includePublicKeyMethods, bool ignoreHashingMethods) + { + byte[] buffer = new byte[32]; + new Random().NextBytes(buffer); + + yield return new TestCaseData("Sign", null, new Func>(async (client, algorithm) => await client.SignAsync(algorithm, buffer))); + yield return new TestCaseData("SignData", null, new Func>(async (client, algorithm) => await client.SignDataAsync(algorithm, buffer))) + .ConditionalIgnore(ignoreHashingMethods, "Cannot hash locally with invalid algorithm"); + + if (includePublicKeyMethods) + { + yield return new TestCaseData("Verify", "true", new Func>(async (client, algorithm) => await client.VerifyAsync(algorithm, buffer, buffer))); + yield return new TestCaseData("VerifyData", "true", new Func>(async (client, algorithm) => await client.VerifyDataAsync(algorithm, buffer, buffer))) + .ConditionalIgnore(ignoreHashingMethods, "Cannot create hash locally with invalid algorithm"); + } + } + + // Use RSA operations as a proxy for cryptographic operations since it currently supports them all. + private static IEnumerable GetRsaOperations(bool includePublicKeyMethods, bool ignoreHashingMethods) + { + byte[] buffer = new byte[32]; + new Random().NextBytes(buffer); + + yield return new TestCaseData("Decrypt", null, new Func>(async (client, algorithm) => await client.DecryptAsync(algorithm ?? EncryptionAlgorithm.RsaOaep, buffer))); + yield return new TestCaseData("Sign", null, new Func>(async (client, algorithm) => await client.SignAsync(algorithm ?? SignatureAlgorithm.RS256, buffer))); + yield return new TestCaseData("SignData", null, new Func>(async (client, algorithm) => await client.SignDataAsync(algorithm ?? SignatureAlgorithm.RS256, buffer))) + .ConditionalIgnore(ignoreHashingMethods, "Cannot hash locally with invalid algorithm"); + yield return new TestCaseData("UnwrapKey", null, new Func>(async (client, algorithm) => await client.UnwrapKeyAsync(algorithm ?? KeyWrapAlgorithm.RsaOaep, buffer))); + + if (includePublicKeyMethods) + { + yield return new TestCaseData("Encrypt", null, new Func>(async (client, algorithm) => await client.EncryptAsync(algorithm ?? EncryptionAlgorithm.RsaOaep, buffer))); + yield return new TestCaseData("Verify", "true", new Func>(async (client, algorithm) => await client.VerifyAsync(algorithm ?? SignatureAlgorithm.RS256, buffer, buffer))); + yield return new TestCaseData("VerifyData", "true", new Func>(async (client, algorithm) => await client.VerifyDataAsync(algorithm ?? SignatureAlgorithm.RS256, buffer, buffer))) + .ConditionalIgnore(ignoreHashingMethods, "Cannot hash locally with invalid algorithm"); + yield return new TestCaseData("WrapKey", null, new Func>(async (client, algorithm) => await client.WrapKeyAsync(algorithm ?? KeyWrapAlgorithm.RsaOaep, buffer))); + } + } + } +} diff --git a/sdk/keyvault/Azure.Security.KeyVault.Keys/tests/ThrowingCryptographyProvider.cs b/sdk/keyvault/Azure.Security.KeyVault.Keys/tests/ThrowingCryptographyProvider.cs new file mode 100644 index 000000000000..5e6a28fefd82 --- /dev/null +++ b/sdk/keyvault/Azure.Security.KeyVault.Keys/tests/ThrowingCryptographyProvider.cs @@ -0,0 +1,43 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System.Security.Cryptography; +using System.Threading; +using System.Threading.Tasks; +using Azure.Security.KeyVault.Keys.Cryptography; + +namespace Azure.Security.KeyVault.Keys.Tests +{ + internal class ThrowingCryptographyProvider : ICryptographyProvider + { + internal const int CRYPT_E_NO_PROVIDER = unchecked((int)0x80092006); + + public bool ShouldRemote => true; + + public bool SupportsOperation(KeyOperation operation) => true; + + public DecryptResult Decrypt(EncryptionAlgorithm algorithm, byte[] ciphertext, CancellationToken cancellationToken = default) => throw new CryptographicException(CRYPT_E_NO_PROVIDER); + + public Task DecryptAsync(EncryptionAlgorithm algorithm, byte[] ciphertext, CancellationToken cancellationToken = default) => throw new CryptographicException(CRYPT_E_NO_PROVIDER); + + public EncryptResult Encrypt(EncryptionAlgorithm algorithm, byte[] plaintext, CancellationToken cancellationToken = default) => throw new CryptographicException(CRYPT_E_NO_PROVIDER); + + public Task EncryptAsync(EncryptionAlgorithm algorithm, byte[] plaintext, CancellationToken cancellationToken = default) => throw new CryptographicException(CRYPT_E_NO_PROVIDER); + + public SignResult Sign(SignatureAlgorithm algorithm, byte[] digest, CancellationToken cancellationToken = default) => throw new CryptographicException(CRYPT_E_NO_PROVIDER); + + public Task SignAsync(SignatureAlgorithm algorithm, byte[] digest, CancellationToken cancellationToken = default) => throw new CryptographicException(CRYPT_E_NO_PROVIDER); + + public UnwrapResult UnwrapKey(KeyWrapAlgorithm algorithm, byte[] encryptedKey, CancellationToken cancellationToken = default) => throw new CryptographicException(CRYPT_E_NO_PROVIDER); + + public Task UnwrapKeyAsync(KeyWrapAlgorithm algorithm, byte[] encryptedKey, CancellationToken cancellationToken = default) => throw new CryptographicException(CRYPT_E_NO_PROVIDER); + + public VerifyResult Verify(SignatureAlgorithm algorithm, byte[] digest, byte[] signature, CancellationToken cancellationToken = default) => throw new CryptographicException(CRYPT_E_NO_PROVIDER); + + public Task VerifyAsync(SignatureAlgorithm algorithm, byte[] digest, byte[] signature, CancellationToken cancellationToken = default) => throw new CryptographicException(CRYPT_E_NO_PROVIDER); + + public WrapResult WrapKey(KeyWrapAlgorithm algorithm, byte[] key, CancellationToken cancellationToken = default) => throw new CryptographicException(CRYPT_E_NO_PROVIDER); + + public Task WrapKeyAsync(KeyWrapAlgorithm algorithm, byte[] key, CancellationToken cancellationToken = default) => throw new CryptographicException(CRYPT_E_NO_PROVIDER); + } +} diff --git a/sdk/keyvault/Azure.Security.KeyVault.Secrets/src/Azure.Security.KeyVault.Secrets.csproj b/sdk/keyvault/Azure.Security.KeyVault.Secrets/src/Azure.Security.KeyVault.Secrets.csproj index fb26e362d59e..9d9fe407b6e4 100644 --- a/sdk/keyvault/Azure.Security.KeyVault.Secrets/src/Azure.Security.KeyVault.Secrets.csproj +++ b/sdk/keyvault/Azure.Security.KeyVault.Secrets/src/Azure.Security.KeyVault.Secrets.csproj @@ -22,7 +22,7 @@ true - + diff --git a/sdk/keyvault/Azure.Security.KeyVault.Shared/Azure.Security.KeyVault.Shared.projitems b/sdk/keyvault/Azure.Security.KeyVault.Shared/src/Azure.Security.KeyVault.Shared.projitems similarity index 100% rename from sdk/keyvault/Azure.Security.KeyVault.Shared/Azure.Security.KeyVault.Shared.projitems rename to sdk/keyvault/Azure.Security.KeyVault.Shared/src/Azure.Security.KeyVault.Shared.projitems diff --git a/sdk/keyvault/Azure.Security.KeyVault.Shared/Azure.Security.KeyVault.Shared.shproj b/sdk/keyvault/Azure.Security.KeyVault.Shared/src/Azure.Security.KeyVault.Shared.shproj similarity index 100% rename from sdk/keyvault/Azure.Security.KeyVault.Shared/Azure.Security.KeyVault.Shared.shproj rename to sdk/keyvault/Azure.Security.KeyVault.Shared/src/Azure.Security.KeyVault.Shared.shproj diff --git a/sdk/keyvault/Azure.Security.KeyVault.Shared/Base64Url.cs b/sdk/keyvault/Azure.Security.KeyVault.Shared/src/Base64Url.cs similarity index 100% rename from sdk/keyvault/Azure.Security.KeyVault.Shared/Base64Url.cs rename to sdk/keyvault/Azure.Security.KeyVault.Shared/src/Base64Url.cs diff --git a/sdk/keyvault/Azure.Security.KeyVault.Shared/ChallengeBasedAuthenticationPolicy.cs b/sdk/keyvault/Azure.Security.KeyVault.Shared/src/ChallengeBasedAuthenticationPolicy.cs similarity index 100% rename from sdk/keyvault/Azure.Security.KeyVault.Shared/ChallengeBasedAuthenticationPolicy.cs rename to sdk/keyvault/Azure.Security.KeyVault.Shared/src/ChallengeBasedAuthenticationPolicy.cs diff --git a/sdk/keyvault/Azure.Security.KeyVault.Shared/ClientOptionsExtensions.cs b/sdk/keyvault/Azure.Security.KeyVault.Shared/src/ClientOptionsExtensions.cs similarity index 100% rename from sdk/keyvault/Azure.Security.KeyVault.Shared/ClientOptionsExtensions.cs rename to sdk/keyvault/Azure.Security.KeyVault.Shared/src/ClientOptionsExtensions.cs diff --git a/sdk/keyvault/Azure.Security.KeyVault.Shared/IJsonSerializable.cs b/sdk/keyvault/Azure.Security.KeyVault.Shared/src/IJsonSerializable.cs similarity index 100% rename from sdk/keyvault/Azure.Security.KeyVault.Shared/IJsonSerializable.cs rename to sdk/keyvault/Azure.Security.KeyVault.Shared/src/IJsonSerializable.cs diff --git a/sdk/keyvault/Azure.Security.KeyVault.Shared/KeyVaultIdentifier.cs b/sdk/keyvault/Azure.Security.KeyVault.Shared/src/KeyVaultIdentifier.cs similarity index 100% rename from sdk/keyvault/Azure.Security.KeyVault.Shared/KeyVaultIdentifier.cs rename to sdk/keyvault/Azure.Security.KeyVault.Shared/src/KeyVaultIdentifier.cs diff --git a/sdk/keyvault/Azure.Security.KeyVault.Shared/KeyVaultPage.cs b/sdk/keyvault/Azure.Security.KeyVault.Shared/src/KeyVaultPage.cs similarity index 100% rename from sdk/keyvault/Azure.Security.KeyVault.Shared/KeyVaultPage.cs rename to sdk/keyvault/Azure.Security.KeyVault.Shared/src/KeyVaultPage.cs diff --git a/sdk/keyvault/Azure.Security.KeyVault.Shared/KeyVaultPipeline.cs b/sdk/keyvault/Azure.Security.KeyVault.Shared/src/KeyVaultPipeline.cs similarity index 100% rename from sdk/keyvault/Azure.Security.KeyVault.Shared/KeyVaultPipeline.cs rename to sdk/keyvault/Azure.Security.KeyVault.Shared/src/KeyVaultPipeline.cs diff --git a/sdk/keyvault/Azure.Security.KeyVault.Shared/tests/Azure.Security.KeyVault.Shared.Tests/Azure.Security.KeyVault.Shared.Tests.projitems b/sdk/keyvault/Azure.Security.KeyVault.Shared/tests/Azure.Security.KeyVault.Shared.Tests/Azure.Security.KeyVault.Shared.Tests.projitems new file mode 100644 index 000000000000..9f220fa5314c --- /dev/null +++ b/sdk/keyvault/Azure.Security.KeyVault.Shared/tests/Azure.Security.KeyVault.Shared.Tests/Azure.Security.KeyVault.Shared.Tests.projitems @@ -0,0 +1,14 @@ + + + + $(MSBuildAllProjects);$(MSBuildThisFileFullPath) + true + 1afa2644-a1d9-419f-b87d-9b519b673f24 + + + Azure.Security.KeyVault.Tests + + + + + \ No newline at end of file diff --git a/sdk/keyvault/Azure.Security.KeyVault.Shared/tests/Azure.Security.KeyVault.Shared.Tests/Azure.Security.KeyVault.Shared.Tests.shproj b/sdk/keyvault/Azure.Security.KeyVault.Shared/tests/Azure.Security.KeyVault.Shared.Tests/Azure.Security.KeyVault.Shared.Tests.shproj new file mode 100644 index 000000000000..27200d0f365b --- /dev/null +++ b/sdk/keyvault/Azure.Security.KeyVault.Shared/tests/Azure.Security.KeyVault.Shared.Tests/Azure.Security.KeyVault.Shared.Tests.shproj @@ -0,0 +1,13 @@ + + + + 1afa2644-a1d9-419f-b87d-9b519b673f24 + 14.0 + + + + + + + + diff --git a/sdk/keyvault/Azure.Security.KeyVault.Shared/tests/Azure.Security.KeyVault.Shared.Tests/TestExtensions.cs b/sdk/keyvault/Azure.Security.KeyVault.Shared/tests/Azure.Security.KeyVault.Shared.Tests/TestExtensions.cs new file mode 100644 index 000000000000..f6c5e4871fd3 --- /dev/null +++ b/sdk/keyvault/Azure.Security.KeyVault.Shared/tests/Azure.Security.KeyVault.Shared.Tests/TestExtensions.cs @@ -0,0 +1,20 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using NUnit.Framework; + +namespace Azure.Security.KeyVault.Tests +{ + internal static class TestExtensions + { + public static TestCaseData ConditionalIgnore(this TestCaseData source, bool condition, string reason) + { + if (condition) + { + return source.Ignore(reason); + } + + return source; + } + } +} diff --git a/sdk/keyvault/Azure.Security.KeyVault.sln b/sdk/keyvault/Azure.Security.KeyVault.sln index e12c0de36a77..a250949f809b 100644 --- a/sdk/keyvault/Azure.Security.KeyVault.sln +++ b/sdk/keyvault/Azure.Security.KeyVault.sln @@ -21,7 +21,7 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Azure.Identity", "..\identi EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Azure.Security.KeyVault.Certificates", "Azure.Security.KeyVault.Certificates\src\Azure.Security.KeyVault.Certificates.csproj", "{E74DABDD-50B0-475C-B83A-44465CF5515C}" EndProject -Project("{D954291E-2A0B-460D-934E-DC6B0785DB48}") = "Azure.Security.KeyVault.Shared", "Azure.Security.KeyVault.Shared\Azure.Security.KeyVault.Shared.shproj", "{1198491F-D3CD-4D23-B536-E89D65B9D965}" +Project("{D954291E-2A0B-460D-934E-DC6B0785DB48}") = "Azure.Security.KeyVault.Shared", "Azure.Security.KeyVault.Shared\src\Azure.Security.KeyVault.Shared.shproj", "{1198491F-D3CD-4D23-B536-E89D65B9D965}" EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Azure.Security.KeyVault.Certificates.Tests", "Azure.Security.KeyVault.Certificates\tests\Azure.Security.KeyVault.Certificates.Tests.csproj", "{B404190B-C1D4-4655-99D4-45CB6532806B}" EndProject @@ -29,9 +29,12 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Azure.Security.KeyVault.Cer EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Azure.Identity.Tests", "..\identity\Azure.Identity\tests\Azure.Identity.Tests.csproj", "{A9EE98A1-2769-4D37-A336-8418184A4448}" EndProject +Project("{D954291E-2A0B-460D-934E-DC6B0785DB48}") = "Azure.Security.KeyVault.Shared.Tests", "Azure.Security.KeyVault.Shared\tests\Azure.Security.KeyVault.Shared.Tests\Azure.Security.KeyVault.Shared.Tests.shproj", "{1AFA2644-A1D9-419F-B87D-9B519B673F24}" +EndProject Global GlobalSection(SharedMSBuildProjectFiles) = preSolution - Azure.Security.KeyVault.Shared\Azure.Security.KeyVault.Shared.projitems*{1198491f-d3cd-4d23-b536-e89d65b9d965}*SharedItemsImports = 13 + Azure.Security.KeyVault.Shared\src\Azure.Security.KeyVault.Shared.projitems*{1198491f-d3cd-4d23-b536-e89d65b9d965}*SharedItemsImports = 13 + Azure.Security.KeyVault.Shared\tests\Azure.Security.KeyVault.Shared.Tests\Azure.Security.KeyVault.Shared.Tests.projitems*{1afa2644-a1d9-419f-b87d-9b519b673f24}*SharedItemsImports = 13 EndGlobalSection GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU From ebcf871a2738b1a45a69d15904cd86dc5d2bebbf Mon Sep 17 00:00:00 2001 From: Heath Stewart Date: Mon, 21 Oct 2019 11:08:24 -0700 Subject: [PATCH 2/3] Resolve PR feedback --- .../Cryptography/AesCryptographyProvider.cs | 4 +- .../Cryptography/EcCryptographyProvider.cs | 12 +---- .../Cryptography/RsaCryptographyProvider.cs | 16 +++---- .../src/KeysEventSource.cs | 2 +- .../tests/KeysEventSourceTests.cs | 44 +++++++++---------- 5 files changed, 34 insertions(+), 44 deletions(-) diff --git a/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Cryptography/AesCryptographyProvider.cs b/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Cryptography/AesCryptographyProvider.cs index ce527bae06b7..e57802a05b44 100644 --- a/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Cryptography/AesCryptographyProvider.cs +++ b/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Cryptography/AesCryptographyProvider.cs @@ -34,7 +34,7 @@ public override UnwrapResult UnwrapKey(KeyWrapAlgorithm algorithm, byte[] encryp int algorithmKeySizeBytes = algorithm.GetKeySizeInBytes(); if (algorithmKeySizeBytes == 0) { - KeysEventSource.Singleton.AlgorithmNotSupported(nameof(UnwrapKey), ref algorithm); + KeysEventSource.Singleton.AlgorithmNotSupported(nameof(UnwrapKey), algorithm); return null; } @@ -66,7 +66,7 @@ public override WrapResult WrapKey(KeyWrapAlgorithm algorithm, byte[] key, Cance int algorithmKeySizeBytes = algorithm.GetKeySizeInBytes(); if (algorithmKeySizeBytes == 0) { - KeysEventSource.Singleton.AlgorithmNotSupported(nameof(WrapKey), ref algorithm); + KeysEventSource.Singleton.AlgorithmNotSupported(nameof(WrapKey), algorithm); return null; } diff --git a/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Cryptography/EcCryptographyProvider.cs b/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Cryptography/EcCryptographyProvider.cs index 6a69cfd9b74f..b33d4cb11908 100644 --- a/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Cryptography/EcCryptographyProvider.cs +++ b/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Cryptography/EcCryptographyProvider.cs @@ -55,11 +55,7 @@ public override SignResult Sign(SignatureAlgorithm algorithm, byte[] digest, Can // The JWK is not supported by this client. Send to the server. if (KeyMaterial is null) { - if (KeysEventSource.Singleton.IsEnabled()) - { - KeysEventSource.Singleton.AlgorithmNotSupported(nameof(Sign), _curve.ToString()); - } - + KeysEventSource.Singleton.AlgorithmNotSupported(nameof(Sign), _curve); return null; } @@ -105,11 +101,7 @@ public override VerifyResult Verify(SignatureAlgorithm algorithm, byte[] digest, // The JWK is not supported by this client. Send to the server. if (KeyMaterial is null) { - if (KeysEventSource.Singleton.IsEnabled()) - { - KeysEventSource.Singleton.AlgorithmNotSupported(nameof(Verify), _curve.ToString()); - } - + KeysEventSource.Singleton.AlgorithmNotSupported(nameof(Verify), _curve); return null; } diff --git a/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Cryptography/RsaCryptographyProvider.cs b/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Cryptography/RsaCryptographyProvider.cs index c5faa891abfb..a48531025411 100644 --- a/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Cryptography/RsaCryptographyProvider.cs +++ b/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Cryptography/RsaCryptographyProvider.cs @@ -35,7 +35,7 @@ public override EncryptResult Encrypt(EncryptionAlgorithm algorithm, byte[] plai RSAEncryptionPadding padding = algorithm.GetRsaEncryptionPadding(); if (padding is null) { - KeysEventSource.Singleton.AlgorithmNotSupported(nameof(Encrypt), ref algorithm); + KeysEventSource.Singleton.AlgorithmNotSupported(nameof(Encrypt), algorithm); return null; } @@ -69,7 +69,7 @@ public override DecryptResult Decrypt(EncryptionAlgorithm algorithm, byte[] ciph RSAEncryptionPadding padding = algorithm.GetRsaEncryptionPadding(); if (padding is null) { - KeysEventSource.Singleton.AlgorithmNotSupported(nameof(Decrypt), ref algorithm); + KeysEventSource.Singleton.AlgorithmNotSupported(nameof(Decrypt), algorithm); return null; } @@ -105,14 +105,14 @@ public override SignResult Sign(SignatureAlgorithm algorithm, byte[] digest, Can HashAlgorithmName hashAlgorithm = algorithm.GetHashAlgorithmName(); if (hashAlgorithm == default) { - KeysEventSource.Singleton.AlgorithmNotSupported(nameof(Sign), ref algorithm); + KeysEventSource.Singleton.AlgorithmNotSupported(nameof(Sign), algorithm); return null; } RSASignaturePadding padding = algorithm.GetRsaSignaturePadding(); if (padding is null) { - KeysEventSource.Singleton.AlgorithmNotSupported(nameof(Sign), ref algorithm); + KeysEventSource.Singleton.AlgorithmNotSupported(nameof(Sign), algorithm); return null; } @@ -135,14 +135,14 @@ public override VerifyResult Verify(SignatureAlgorithm algorithm, byte[] digest, HashAlgorithmName hashAlgorithm = algorithm.GetHashAlgorithmName(); if (hashAlgorithm == default) { - KeysEventSource.Singleton.AlgorithmNotSupported(nameof(Verify), ref algorithm); + KeysEventSource.Singleton.AlgorithmNotSupported(nameof(Verify), algorithm); return null; } RSASignaturePadding padding = algorithm.GetRsaSignaturePadding(); if (padding is null) { - KeysEventSource.Singleton.AlgorithmNotSupported(nameof(Verify), ref algorithm); + KeysEventSource.Singleton.AlgorithmNotSupported(nameof(Verify), algorithm); return null; } @@ -166,7 +166,7 @@ public override WrapResult WrapKey(KeyWrapAlgorithm algorithm, byte[] key, Cance RSAEncryptionPadding padding = algorithm.GetRsaEncryptionPadding(); if (padding is null) { - KeysEventSource.Singleton.AlgorithmNotSupported(nameof(WrapKey), ref algorithm); + KeysEventSource.Singleton.AlgorithmNotSupported(nameof(WrapKey), algorithm); return null; } @@ -200,7 +200,7 @@ public override UnwrapResult UnwrapKey(KeyWrapAlgorithm algorithm, byte[] encryp RSAEncryptionPadding padding = algorithm.GetRsaEncryptionPadding(); if (padding is null) { - KeysEventSource.Singleton.AlgorithmNotSupported(nameof(UnwrapKey), ref algorithm); + KeysEventSource.Singleton.AlgorithmNotSupported(nameof(UnwrapKey), algorithm); return null; } diff --git a/sdk/keyvault/Azure.Security.KeyVault.Keys/src/KeysEventSource.cs b/sdk/keyvault/Azure.Security.KeyVault.Keys/src/KeysEventSource.cs index 2eeed4b932de..e4249066095b 100644 --- a/sdk/keyvault/Azure.Security.KeyVault.Keys/src/KeysEventSource.cs +++ b/sdk/keyvault/Azure.Security.KeyVault.Keys/src/KeysEventSource.cs @@ -24,7 +24,7 @@ private KeysEventSource() : base(EventSourceName, EventSourceSettings.Default, A public static KeysEventSource Singleton { get; } = new KeysEventSource(); [NonEvent] - public void AlgorithmNotSupported(string operation, ref T algorithm) where T : struct + public void AlgorithmNotSupported(string operation, T algorithm) where T : notnull { if (IsEnabled()) { diff --git a/sdk/keyvault/Azure.Security.KeyVault.Keys/tests/KeysEventSourceTests.cs b/sdk/keyvault/Azure.Security.KeyVault.Keys/tests/KeysEventSourceTests.cs index d170c0e5de08..02b7e84560f4 100644 --- a/sdk/keyvault/Azure.Security.KeyVault.Keys/tests/KeysEventSourceTests.cs +++ b/sdk/keyvault/Azure.Security.KeyVault.Keys/tests/KeysEventSourceTests.cs @@ -18,9 +18,16 @@ namespace Azure.Security.KeyVault.Keys.Tests [NonParallelizable] public class KeysEventSourceTests : ClientTestBase { + private static readonly byte[] s_buffer = new byte[32]; + private const string KeyId = "https://localhost/keys/test"; private TestEventListener _listener; + static KeysEventSourceTests() + { + new Random().NextBytes(s_buffer); + } + public KeysEventSourceTests(bool isAsync) : base(isAsync) { } @@ -271,26 +278,20 @@ private CryptographyClient CreateClient(KeyVaultKey key, HttpPipelineTransport t private static IEnumerable GetAesOperations() { - byte[] buffer = new byte[32]; - new Random().NextBytes(buffer); - - yield return new TestCaseData("WrapKey", new Func>(async (client, algorithm) => await client.WrapKeyAsync(algorithm, buffer))); - yield return new TestCaseData("UnwrapKey", new Func>(async (client, algorithm) => await client.UnwrapKeyAsync(algorithm, buffer))); + yield return new TestCaseData("WrapKey", new Func>(async (client, algorithm) => await client.WrapKeyAsync(algorithm, s_buffer))); + yield return new TestCaseData("UnwrapKey", new Func>(async (client, algorithm) => await client.UnwrapKeyAsync(algorithm, s_buffer))); } private static IEnumerable GetEcOperations(bool includePublicKeyMethods, bool ignoreHashingMethods) { - byte[] buffer = new byte[32]; - new Random().NextBytes(buffer); - - yield return new TestCaseData("Sign", null, new Func>(async (client, algorithm) => await client.SignAsync(algorithm, buffer))); - yield return new TestCaseData("SignData", null, new Func>(async (client, algorithm) => await client.SignDataAsync(algorithm, buffer))) + yield return new TestCaseData("Sign", null, new Func>(async (client, algorithm) => await client.SignAsync(algorithm, s_buffer))); + yield return new TestCaseData("SignData", null, new Func>(async (client, algorithm) => await client.SignDataAsync(algorithm, s_buffer))) .ConditionalIgnore(ignoreHashingMethods, "Cannot hash locally with invalid algorithm"); if (includePublicKeyMethods) { - yield return new TestCaseData("Verify", "true", new Func>(async (client, algorithm) => await client.VerifyAsync(algorithm, buffer, buffer))); - yield return new TestCaseData("VerifyData", "true", new Func>(async (client, algorithm) => await client.VerifyDataAsync(algorithm, buffer, buffer))) + yield return new TestCaseData("Verify", "true", new Func>(async (client, algorithm) => await client.VerifyAsync(algorithm, s_buffer, s_buffer))); + yield return new TestCaseData("VerifyData", "true", new Func>(async (client, algorithm) => await client.VerifyDataAsync(algorithm, s_buffer, s_buffer))) .ConditionalIgnore(ignoreHashingMethods, "Cannot create hash locally with invalid algorithm"); } } @@ -298,22 +299,19 @@ private static IEnumerable GetEcOperations(bool includePublicKeyMethods, bool ig // Use RSA operations as a proxy for cryptographic operations since it currently supports them all. private static IEnumerable GetRsaOperations(bool includePublicKeyMethods, bool ignoreHashingMethods) { - byte[] buffer = new byte[32]; - new Random().NextBytes(buffer); - - yield return new TestCaseData("Decrypt", null, new Func>(async (client, algorithm) => await client.DecryptAsync(algorithm ?? EncryptionAlgorithm.RsaOaep, buffer))); - yield return new TestCaseData("Sign", null, new Func>(async (client, algorithm) => await client.SignAsync(algorithm ?? SignatureAlgorithm.RS256, buffer))); - yield return new TestCaseData("SignData", null, new Func>(async (client, algorithm) => await client.SignDataAsync(algorithm ?? SignatureAlgorithm.RS256, buffer))) + yield return new TestCaseData("Decrypt", null, new Func>(async (client, algorithm) => await client.DecryptAsync(algorithm ?? EncryptionAlgorithm.RsaOaep, s_buffer))); + yield return new TestCaseData("Sign", null, new Func>(async (client, algorithm) => await client.SignAsync(algorithm ?? SignatureAlgorithm.RS256, s_buffer))); + yield return new TestCaseData("SignData", null, new Func>(async (client, algorithm) => await client.SignDataAsync(algorithm ?? SignatureAlgorithm.RS256, s_buffer))) .ConditionalIgnore(ignoreHashingMethods, "Cannot hash locally with invalid algorithm"); - yield return new TestCaseData("UnwrapKey", null, new Func>(async (client, algorithm) => await client.UnwrapKeyAsync(algorithm ?? KeyWrapAlgorithm.RsaOaep, buffer))); + yield return new TestCaseData("UnwrapKey", null, new Func>(async (client, algorithm) => await client.UnwrapKeyAsync(algorithm ?? KeyWrapAlgorithm.RsaOaep, s_buffer))); if (includePublicKeyMethods) { - yield return new TestCaseData("Encrypt", null, new Func>(async (client, algorithm) => await client.EncryptAsync(algorithm ?? EncryptionAlgorithm.RsaOaep, buffer))); - yield return new TestCaseData("Verify", "true", new Func>(async (client, algorithm) => await client.VerifyAsync(algorithm ?? SignatureAlgorithm.RS256, buffer, buffer))); - yield return new TestCaseData("VerifyData", "true", new Func>(async (client, algorithm) => await client.VerifyDataAsync(algorithm ?? SignatureAlgorithm.RS256, buffer, buffer))) + yield return new TestCaseData("Encrypt", null, new Func>(async (client, algorithm) => await client.EncryptAsync(algorithm ?? EncryptionAlgorithm.RsaOaep, s_buffer))); + yield return new TestCaseData("Verify", "true", new Func>(async (client, algorithm) => await client.VerifyAsync(algorithm ?? SignatureAlgorithm.RS256, s_buffer, s_buffer))); + yield return new TestCaseData("VerifyData", "true", new Func>(async (client, algorithm) => await client.VerifyDataAsync(algorithm ?? SignatureAlgorithm.RS256, s_buffer, s_buffer))) .ConditionalIgnore(ignoreHashingMethods, "Cannot hash locally with invalid algorithm"); - yield return new TestCaseData("WrapKey", null, new Func>(async (client, algorithm) => await client.WrapKeyAsync(algorithm ?? KeyWrapAlgorithm.RsaOaep, buffer))); + yield return new TestCaseData("WrapKey", null, new Func>(async (client, algorithm) => await client.WrapKeyAsync(algorithm ?? KeyWrapAlgorithm.RsaOaep, s_buffer))); } } } From cf1f2d3033aec01d84be03213616e57601887d8e Mon Sep 17 00:00:00 2001 From: Heath Stewart Date: Mon, 21 Oct 2019 11:26:25 -0700 Subject: [PATCH 3/3] Fix shared test project directory --- .../tests/Azure.Security.KeyVault.Keys.Tests.csproj | 3 +-- .../Azure.Security.KeyVault.Shared.Tests.projitems | 0 .../Azure.Security.KeyVault.Shared.Tests.shproj | 0 .../TestExtensions.cs | 0 sdk/keyvault/Azure.Security.KeyVault.sln | 4 ++-- 5 files changed, 3 insertions(+), 4 deletions(-) rename sdk/keyvault/Azure.Security.KeyVault.Shared/tests/{Azure.Security.KeyVault.Shared.Tests => }/Azure.Security.KeyVault.Shared.Tests.projitems (100%) rename sdk/keyvault/Azure.Security.KeyVault.Shared/tests/{Azure.Security.KeyVault.Shared.Tests => }/Azure.Security.KeyVault.Shared.Tests.shproj (100%) rename sdk/keyvault/Azure.Security.KeyVault.Shared/tests/{Azure.Security.KeyVault.Shared.Tests => }/TestExtensions.cs (100%) diff --git a/sdk/keyvault/Azure.Security.KeyVault.Keys/tests/Azure.Security.KeyVault.Keys.Tests.csproj b/sdk/keyvault/Azure.Security.KeyVault.Keys/tests/Azure.Security.KeyVault.Keys.Tests.csproj index 2863fead4e57..ee2925b6bd01 100644 --- a/sdk/keyvault/Azure.Security.KeyVault.Keys/tests/Azure.Security.KeyVault.Keys.Tests.csproj +++ b/sdk/keyvault/Azure.Security.KeyVault.Keys/tests/Azure.Security.KeyVault.Keys.Tests.csproj @@ -5,9 +5,8 @@ $(RequiredTargetFrameworks);net461;net47 - - + diff --git a/sdk/keyvault/Azure.Security.KeyVault.Shared/tests/Azure.Security.KeyVault.Shared.Tests/Azure.Security.KeyVault.Shared.Tests.projitems b/sdk/keyvault/Azure.Security.KeyVault.Shared/tests/Azure.Security.KeyVault.Shared.Tests.projitems similarity index 100% rename from sdk/keyvault/Azure.Security.KeyVault.Shared/tests/Azure.Security.KeyVault.Shared.Tests/Azure.Security.KeyVault.Shared.Tests.projitems rename to sdk/keyvault/Azure.Security.KeyVault.Shared/tests/Azure.Security.KeyVault.Shared.Tests.projitems diff --git a/sdk/keyvault/Azure.Security.KeyVault.Shared/tests/Azure.Security.KeyVault.Shared.Tests/Azure.Security.KeyVault.Shared.Tests.shproj b/sdk/keyvault/Azure.Security.KeyVault.Shared/tests/Azure.Security.KeyVault.Shared.Tests.shproj similarity index 100% rename from sdk/keyvault/Azure.Security.KeyVault.Shared/tests/Azure.Security.KeyVault.Shared.Tests/Azure.Security.KeyVault.Shared.Tests.shproj rename to sdk/keyvault/Azure.Security.KeyVault.Shared/tests/Azure.Security.KeyVault.Shared.Tests.shproj diff --git a/sdk/keyvault/Azure.Security.KeyVault.Shared/tests/Azure.Security.KeyVault.Shared.Tests/TestExtensions.cs b/sdk/keyvault/Azure.Security.KeyVault.Shared/tests/TestExtensions.cs similarity index 100% rename from sdk/keyvault/Azure.Security.KeyVault.Shared/tests/Azure.Security.KeyVault.Shared.Tests/TestExtensions.cs rename to sdk/keyvault/Azure.Security.KeyVault.Shared/tests/TestExtensions.cs diff --git a/sdk/keyvault/Azure.Security.KeyVault.sln b/sdk/keyvault/Azure.Security.KeyVault.sln index a250949f809b..07a3c1595d2b 100644 --- a/sdk/keyvault/Azure.Security.KeyVault.sln +++ b/sdk/keyvault/Azure.Security.KeyVault.sln @@ -29,12 +29,12 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Azure.Security.KeyVault.Cer EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Azure.Identity.Tests", "..\identity\Azure.Identity\tests\Azure.Identity.Tests.csproj", "{A9EE98A1-2769-4D37-A336-8418184A4448}" EndProject -Project("{D954291E-2A0B-460D-934E-DC6B0785DB48}") = "Azure.Security.KeyVault.Shared.Tests", "Azure.Security.KeyVault.Shared\tests\Azure.Security.KeyVault.Shared.Tests\Azure.Security.KeyVault.Shared.Tests.shproj", "{1AFA2644-A1D9-419F-B87D-9B519B673F24}" +Project("{D954291E-2A0B-460D-934E-DC6B0785DB48}") = "Azure.Security.KeyVault.Shared.Tests", "Azure.Security.KeyVault.Shared\tests\Azure.Security.KeyVault.Shared.Tests.shproj", "{1AFA2644-A1D9-419F-B87D-9B519B673F24}" EndProject Global GlobalSection(SharedMSBuildProjectFiles) = preSolution Azure.Security.KeyVault.Shared\src\Azure.Security.KeyVault.Shared.projitems*{1198491f-d3cd-4d23-b536-e89d65b9d965}*SharedItemsImports = 13 - Azure.Security.KeyVault.Shared\tests\Azure.Security.KeyVault.Shared.Tests\Azure.Security.KeyVault.Shared.Tests.projitems*{1afa2644-a1d9-419f-b87d-9b519b673f24}*SharedItemsImports = 13 + Azure.Security.KeyVault.Shared\tests\Azure.Security.KeyVault.Shared.Tests.projitems*{1afa2644-a1d9-419f-b87d-9b519b673f24}*SharedItemsImports = 13 EndGlobalSection GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU