From 85e98e3b751bd094692f3b7e24945a7394223362 Mon Sep 17 00:00:00 2001 From: Heath Stewart Date: Thu, 9 Jul 2020 14:12:48 -0700 Subject: [PATCH] Return null key if access is denied (#13317) * Return null key if access is denied Fixes #11574. IKeyEncryptionResolver.Resolve is designed to delegate access verification to the resolver, so that any access issues are thrown downstream at the call site instead of upstream when trying to get the resolver/client. * Resolve offline and PR feedback --- .../src/Cryptography/KeyResolver.cs | 17 +++-- .../tests/KeyResolverMockTests.cs | 68 +++++++++++++++++++ 2 files changed, 78 insertions(+), 7 deletions(-) create mode 100644 sdk/keyvault/Azure.Security.KeyVault.Keys/tests/KeyResolverMockTests.cs diff --git a/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Cryptography/KeyResolver.cs b/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Cryptography/KeyResolver.cs index 7506869c2605a..c575e41a3ccbb 100644 --- a/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Cryptography/KeyResolver.cs +++ b/sdk/keyvault/Azure.Security.KeyVault.Keys/src/Cryptography/KeyResolver.cs @@ -6,7 +6,6 @@ using System.Threading.Tasks; using Azure.Core; using Azure.Core.Cryptography; -using Azure.Core.Diagnostics; using Azure.Core.Pipeline; namespace Azure.Security.KeyVault.Keys.Cryptography @@ -63,11 +62,11 @@ public KeyResolver(TokenCredential credential, CryptographyClientOptions options } /// - /// Retrieves a capable of performing cryptographic operations with the key represented by the specfiied . + /// Retrieves a capable of performing cryptographic operations with the key represented by the specified . /// - /// The key idenitifier of the key used by the created . + /// The key identifier of the key used by the created . /// A controlling the request lifetime. - /// A new capable of performing cryptographic operations with the key represented by the specfiied . + /// A new capable of performing cryptographic operations with the key represented by the specified . /// is null. public virtual CryptographyClient Resolve(Uri keyId, CancellationToken cancellationToken = default) { @@ -95,11 +94,11 @@ public virtual CryptographyClient Resolve(Uri keyId, CancellationToken cancellat } /// - /// Retrieves a capable of performing cryptographic operations with the key represented by the specfiied . + /// Retrieves a capable of performing cryptographic operations with the key represented by the specified . /// - /// The key idenitifier of the key used by the created . + /// The key identifier of the key used by the created . /// A controlling the request lifetime. - /// A new capable of performing cryptographic operations with the key represented by the specfiied . + /// A new capable of performing cryptographic operations with the key represented by the specified . /// is null. public virtual async Task ResolveAsync(Uri keyId, CancellationToken cancellationToken = default) { @@ -168,6 +167,10 @@ private Response ParseResponse(Response response, T result) case 204: result.Deserialize(response.ContentStream); return Response.FromValue(result, response); + case 403 when !(result is SecretKey): + // The "get" permission may not be granted on a key, while other key permissions may be granted. + // To use a key contained within a secret, the "get" permission is required to retrieve the key material. + return Response.FromValue(default, response); default: throw _clientDiagnostics.CreateRequestFailedException(response); } diff --git a/sdk/keyvault/Azure.Security.KeyVault.Keys/tests/KeyResolverMockTests.cs b/sdk/keyvault/Azure.Security.KeyVault.Keys/tests/KeyResolverMockTests.cs new file mode 100644 index 0000000000000..12910df06a290 --- /dev/null +++ b/sdk/keyvault/Azure.Security.KeyVault.Keys/tests/KeyResolverMockTests.cs @@ -0,0 +1,68 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Net; +using System.Threading; +using System.Threading.Tasks; +using Azure.Core; +using Azure.Core.Testing; +using Azure.Security.KeyVault.Keys.Cryptography; +using NUnit.Framework; + +namespace Azure.Security.KeyVault.Keys.Tests +{ + public class KeyResolverMockTests : ClientTestBase + { + public KeyResolverMockTests(bool isAsync) : base(isAsync) + { + } + + [Test] + public async Task ShouldNotRequireGetPermissionForKey() + { + // Test for https://github.com/Azure/azure-sdk-for-net/issues/11574 + MockTransport transport = new MockTransport(request => new MockResponse((int)HttpStatusCode.Forbidden, nameof(HttpStatusCode.Forbidden))); + + KeyResolver resolver = GetResolver(transport); + CryptographyClient client = await resolver.ResolveAsync(new Uri("https://mock.vault.azure.net/keys/mock-key")); + + RequestFailedException ex = Assert.ThrowsAsync(() => client.UnwrapKeyAsync(KeyWrapAlgorithm.A256KW, new byte[] { 0, 1, 2, 3 })); + Assert.AreEqual((int)HttpStatusCode.Forbidden, ex.Status); + } + + [Test] + public void ShouldRequireGetPermissionForSecret() + { + // Test for https://github.com/Azure/azure-sdk-for-net/issues/11574 + MockTransport transport = new MockTransport(request => new MockResponse((int)HttpStatusCode.Forbidden, nameof(HttpStatusCode.Forbidden))); + + KeyResolver resolver = GetResolver(transport); + + RequestFailedException ex = Assert.ThrowsAsync(() => resolver.ResolveAsync(new Uri("https://mock.vault.azure.net/secrets/mock-secret"))); + Assert.AreEqual((int)HttpStatusCode.Forbidden, ex.Status); + } + + protected KeyResolver GetResolver(MockTransport transport) + { + Assert.NotNull(transport); + + CryptographyClientOptions options = new CryptographyClientOptions + { + Transport = transport, + }; + + return InstrumentClient( + new KeyResolver(new NullTokenCredential(), options)); + } + + private class NullTokenCredential : TokenCredential + { + public override AccessToken GetToken(TokenRequestContext requestContext, CancellationToken cancellationToken) => + new AccessToken("invalid", DateTimeOffset.Now.AddHours(1)); + + public override ValueTask GetTokenAsync(TokenRequestContext requestContext, CancellationToken cancellationToken) => + new ValueTask(GetToken(requestContext, cancellationToken)); + } + } +}