Azure / azure-sdk-for-net

This repository is for active development of the Azure SDK for .NET. For consumers of the SDK we recommend visiting our public developer docs at https://learn.microsoft.com/dotnet/azure/ or our versioned developer docs at https://azure.github.io/azure-sdk-for-net.
MIT License
5.57k stars 4.82k forks source link

[BUG] Azure KeyVault Secrets does not validate or sanitize user input #47353

Open henning-moe opened 6 days ago

henning-moe commented 6 days ago

Library name and version

Azure.Security.KeyVault.Secrets 4.7.0

Describe the bug

When doing a Get/Set secret operation the API will send the secret name unsanitized, and unvalidated.

The cause of this behavior is here: https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/keyvault/Azure.Security.KeyVault.Shared/src/KeyVaultPipeline.cs#L86

foreach (var p in path)
{
  request.Uri.AppendPath(p, escape: false);
}

Where it explicitly does not escape path strings. The Secret API's makes no attempt at either validating or sanitizing input.

The following code shows the issue:

client.GetSecret("user-foo-secret/../user-bar-secret")

I'm not sure in what capacity this is a security issue, because you would need a system where the user has control over the secrets, but at the very least it's an annoyance, because other Vault systems (like HashiCorp Vault) allows slashes in secret names. Azure Key Vault Client will just do the wrong thing instead of failing on bad input.

Expected behavior

ArgumentException or 400 Bad Request from Key Vault since the secret name is actually not valid.

Actual behavior

The provided reproduction returns "hunter2", i.e. a different secret.

Reproduction Steps

Create a key vault, and create a secret called "user-foo-secret" and fill it with "**". Create another secret called "user-bar-secret" and fill it with "hunter2".

using Azure.Identity;

var client = new Azure.Security.KeyVault.Secrets.SecretClient(new Uri("https://mhsreprokeyvault.vault.azure.net/"), new DefaultAzureCredential());

var secret = client.GetSecret("user-foo-secret/../user-bar-secret");

Console.WriteLine(secret.Value.Value);

Environment

github-actions[bot] commented 6 days ago

Thank you for your feedback. Tagging and routing to the team member best able to assist.

jsquire commented 5 days ago

Hi @henning-moe. Thanks for reaching out and we regret that you're experiencing difficulties. The Azure SDK clients intentionally do not perform validation of developer-provided parameters outside of client-side concerns. The service is the authority on what parameter data is or is not valid - and we want to ensure that the clients never artificially reject data that the service would allow, as this would block legitimate service scenarios. This is especially important as service rules/validations change over time and should not require that applications update SDK package versions to avoid client/service conflicts. For more information see: Azure SDK Design Guidelines.

There are no security implications here for the same reason; the service has responsibility to ensure that it validates/sanitizes the request being made. Callers are not guaranteed to be using an official Azure SDK package, and the service cannot assume that any sanitization has taken place nor that callers are not malicious. From the SDK perspective, the client must send the actual name of the secret that developers ask for, as the service requires that to perform the query.

If you believe that the service has an implementation flaw and is returning data other than what it should be, this will need to be reported to the Key Vault service team. Unfortunately, they do not monitor this repository nor are we able to transfer inquries to them directly. To ensure that the right team has visibility and can help, your best path forward for would be to open an Azure support request.

github-actions[bot] commented 5 days ago

Hi @henning-moe. Thank you for opening this issue and giving us the opportunity to assist. We believe that this has been addressed. If you feel that further discussion is needed, please add a comment with the text "/unresolve" to remove the "issue-addressed" label and continue the conversation.

henning-moe commented 4 days ago

Thanks for the reply.

If you believe that the service has an implementation flaw and is returning data other than what it should be, this will need to be reported to the Key Vault service team. Unfortunately, they do not monitor this repository nor are we able to transfer inquries to them directly. To ensure that the right team has visibility and can help, your best path forward for would be to open an Azure support request.

This feels very disrespectful of my time.