Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KeyResolver requires extra "get" permissions #11574

Closed
pakrym opened this issue Apr 24, 2020 · 2 comments
Closed

KeyResolver requires extra "get" permissions #11574

pakrym opened this issue Apr 24, 2020 · 2 comments
Assignees
Labels
blocking-release Blocks release bug This issue requires a change to an existing behavior in the product in order to be resolved. Client This issue points to a problem in the data-plane of the library. KeyVault
Milestone

Comments

@pakrym
Copy link
Contributor

pakrym commented Apr 24, 2020

It seems that IKeyEncryptionKeyResolver.Resolve can be implemented in a way that avoids requiring "get" permission on key.


Azure.RequestFailedException: 
Status: 403 (Forbidden)

Content:
{"error":{"code":"Forbidden","message":"Operation is not allowed.\r\nOperation: \"get\"\r\nCaller: appid=bdc40a9a-30bd-47f0-8e10-8033e8e1c248;oid=2bc41755-ddc3-4b49-a381-7428fa1537d5;numgroups=0;iss=https://sts.windows.net/72f988bf-86f1-41af-91ab-2d7cd011db47/\r\nVault: bdorrans;location=eastus","innererror":{"code":"ForbiddenByPolicy"}}}

Headers:
Cache-Control: no-cache
Pragma: no-cache
Server: Microsoft-IIS/10.0
x-ms-keyvault-region: eastus
x-ms-request-id: de919011-b86f-4004-8878-4bcf9b263c10
x-ms-keyvault-service-version: 1.1.0.898
x-ms-keyvault-network-info: addr=23.99.191.39;act_addr_fam=InterNetwork;
X-AspNet-Version: 4.0.30319
X-Powered-By: ASP.NET
Strict-Transport-Security: max-age=31536000;includeSubDomains
X-Content-Type-Options: nosniff
Date: Fri, 24 Apr 2020 16:29:00 GMT
Content-Length: 340
Content-Type: application/json; charset=utf-8
Expires: -1

   at Azure.Security.KeyVault.Keys.Cryptography.KeyResolver.ParseResponse[T](Response response, T result)
   at Azure.Security.KeyVault.Keys.Cryptography.KeyResolver.GetKeyAsync(Uri keyId, CancellationToken cancellationToken)
   at Azure.Security.KeyVault.Keys.Cryptography.KeyResolver.ResolveAsync(Uri keyId, CancellationToken cancellationToken)
   at Azure.Security.KeyVault.Keys.Cryptography.KeyResolver.Azure.Core.Cryptography.IKeyEncryptionKeyResolver.ResolveAsync(String keyId, CancellationToken cancellationToken)

cc @heaths @schaabs

@pakrym pakrym added KeyVault Client This issue points to a problem in the data-plane of the library. bug This issue requires a change to an existing behavior in the product in order to be resolved. labels Apr 24, 2020
@schaabs
Copy link
Contributor

schaabs commented Apr 24, 2020

try
{
Argument.AssertNotNull(keyId, nameof(keyId));
KeyVaultKey key = await GetKeyAsync(keyId, cancellationToken).ConfigureAwait(false);
KeyVaultPipeline pipeline = new KeyVaultPipeline(keyId, _apiVersion, _pipeline, _clientDiagnostics);
return (key != null) ? new CryptographyClient(key, pipeline) : new CryptographyClient(keyId, pipeline);
}
catch (Exception e)
{
scope.Failed(e);
throw;
}

Looks like Resolve and ResolveAsync expect GetKey and GetKeyAsync to return null when they fail rather than raising an exception. We should be able to wrap GetKey and GetKeyAsync in a try/catch to handle failures and return null.

@heaths
Copy link
Member

heaths commented Apr 24, 2020

Rather than use exceptions for control flow, might be better to handle these in a manner I did for certificate LROs (non-throwing, delegating whether to throw to code further down the stack).

@heaths heaths self-assigned this Apr 24, 2020
@heaths heaths added this to the [2020] July milestone May 20, 2020
@heaths heaths added the blocking-release Blocks release label Jun 15, 2020
@heaths heaths modified the milestones: [2020] August, [2020] July Jul 8, 2020
heaths added a commit to heaths/azure-sdk-for-net that referenced this issue Jul 8, 2020
Fixes Azure#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.
@heaths heaths closed this as completed in 85e98e3 Jul 10, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Mar 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
blocking-release Blocks release bug This issue requires a change to an existing behavior in the product in order to be resolved. Client This issue points to a problem in the data-plane of the library. KeyVault
Projects
None yet
Development

No branches or pull requests

4 participants