AzureAD / microsoft-authentication-library-for-dotnet

Microsoft Authentication Library (MSAL) for .NET
https://aka.ms/msal-net
MIT License
1.37k stars 337 forks source link

[Feature Request] Support for KeyVault sign when using ClientCredentials #3355

Open svrooij opened 2 years ago

svrooij commented 2 years ago

Is your feature request related to a problem? Please describe. All samples related to Client Credentials suggest to put a certificate in the Key Vault, registering that certificate in the application and accessing the certificate using a managed identity. Which is all great, but I think no one is aware that the certificate might be exported either by a hack changing the application code or by getting in from memory.

Not even talking about what would happen if a malicious developer would extract the certificate.

All samples show something like:

using Azure.Identity;
using Azure.Security.KeyVault.Secrets;

public async Task<string> GetToken(...)
{
  const string ceyVaultUri = "https://{kv-domain}.vault.azure.net/";
  const string certName = "{some-certificate-name}";
  var tokenCredential = new DefaultAzureCredential();
  var secretClient = new SecretClient(new Uri(keyVaultUri), tokenCredential);
  var secretResult = await secretClient.GetSecretAsync(certName).ExecuteAsync();
  // Certificate is now present on client machine
  var certificate = new X509Certificate2(Convert.FromBase64String(secretResult.Value.Value));

  const string clientId = "d294e746-425b-44fa-896c-dacf2c7938b8";
  const string tenantId = "42a26c5d-b8ed-4f1b-8760-655f98154373";

  var app = ConfidentialClientApplicationBuilder
    .Create(clientId)
    .WithAuthority(AzureCloudInstance.AzurePublic, tenantId)
    .WithCertificate(certificate)
    .Build();

  var tokenResult = await app
    .AcquireTokenForClient(new[] { "https://graph.microsoft.com/.default" })
    .ExecuteAsync();

  return tokenResult.AccessToken;
}

Describe the solution you'd like

I would like to see a warning in the documentation about the possibility that people can extract the certificate and possibly gain persistent access to customer data. The solution is in the KeyVault, instead of downloading the certificate and signing the client assertion on the application side, you can also use the Key Vault to sign the unsigned assertion in the cloud, mitigating the possibility to extract the data.

I created a poc to do exactly that and I could use some assistance in getting this in MSAL.net if you're open to that.

// Seeing support for 'WithKeyVaultKey' instead of 'WithCertificate' is the main goal here
IConfidentialClientApplication app = ConfidentialClientApplicationBuilder
  .Create(clientId)
  .WithAuthority(AzureCloudInstance.AzurePublic, tenantId)
  //.WithCertificate(certificate)
  .WithKeyVaultKey(keyId, hash, cred)
  //.WithRedirectUri(redirectUri )
  .Build();

Describe alternatives you've considered Making people aware of the possibility of certificate extraction is the first part that can be tackled.

Additional context

svrooij commented 2 years ago

Possibly related to #3352 because in the poc the developer has to re-specify the tenant and the client id. And the authority isn't configurable at the moment.

@bgavrilMS and @jmprieur what do you guys think of this idea?

bgavrilMS commented 2 years ago

Yes, it's a good point. The only downside is that it involves at least an extra HTTP request to KeyVault for the Sign operation. And a malicious developer can always steal the access token if they do not have access to the certificate.

From MSAL's perspective, this is achievable now using WithClientAssertion no? Microsoft.Identity.Web could provide a higher level API to help with this, as it already uses KeyVault, but I think the future here is to focus "certificate-less" auth, based on Managed Identity.

I believe Microsoft.Identity.Web already implements a version of this, where it calls Managed Identity to create a client assertion which can then be used to get a token. @jmprieur - do you have docs or a sample for this?

svrooij commented 2 years ago

Not really... In the https://github.com/Smartersoft/identity-client-assertion/blob/main/src/Smartersoft.Identity.Client.Assertion/ConfidentialClientApplicationBuilderExtensions.cs#L42 needs the client ID and the tenant id. That data cannot be extracted from the builder I think.

bgavrilMS commented 2 years ago

I'm going to close this because it is not the path forward that Identity organization wants to take. Instead, we want to focus on Managed Identity all up.

Happy to support any extensibility though.

bgavrilMS commented 2 years ago

Just realized that we do have support for this actually. Microsoft.Identity.Web supports this - https://github.com/AzureAD/microsoft-identity-web/wiki/Certificates#getting-certificates-from-key-vault

svrooij commented 2 years ago

@bgavrilMS It's exactly doing what I'm trying to avoid (and everybody else should as well). It's is downloading the certificate from the key vault and using it to sign the client assertion on the client side (while thus making the certificate exportable, by this app or a malicious (or hacked) admin/developer).

I tried to explain the differences between the two options in this post

bgavrilMS commented 2 years ago

@svrooij - yes, your are right, you use KV to sign. This increases the security posture, at the expense of performance - extra HTTP call(s) to KV.

The solution we are trying to come up with is to not have to use KeyVault at all, i.e. rely on Managed Identity to produce the signed assertion (is there a prototype for this in Microsoft.Identity.Web @jmprieur ?). Or go even further, and have Managed Identity produce give tokens.

svrooij commented 2 years ago

@bgavrilMS the proposed solution would also work during development. You give the developer sign permission to the key vault and the developer would not be able to extract the certificate, but would be able to request access tokens. I know managed identities for multi tenant apps are coming, but they are not here yet. And they don't allow usage during development.

I'm all for creating an extension on the ConfidentialClientApplicationBuilder, want to do that myself and host it on Github, but is would be really useful if we would have access to the payload claims from the .WithClientAssertion((cancellationToken) => string) method.

Something like .WithClientAssertion((CancellationToken token, Dictionary<string, object> payload) => string). That would allow hooking into the mechanics of the ConfidentialClientBuilder is so many ways. And that would stop this extension asking for all the duplicate data already in the builder.

svrooij commented 1 year ago

@bgavrilMS you wanted to go "managed identity everywhere"? How about support for the following?


var app = ConfidentialClientApplicationBuilder
  .Create(clientId)
  .WithAuthority(AzureCloudInstance.AzurePublic, tenantId)
  .WithManagedIdentity(federatedScope)
  .Build();

I figured out a way on how to use managed identities as federated credential and use them in multi tenant applications. So this frees the managed identity from the "own tenant only" constraint. More details in this post.

bgavrilMS commented 1 year ago

That is a great post @svrooij ! I think there are more scenarios that the federated stuff enables, but certainly the "multi-tenant" has great value proposition.

A few notes:

  1. We are considering implementing a higher level API that does what you just described. MSAL has not yet decided to do it yet, as we're kind of low level / protocol library and as you just posted, it works fine with existing primitives.
  2. Note that we've built a higher level experience around this in Microsoft.Idenity.Web - see https://github.com/AzureAD/microsoft-identity-web/tree/master/src/Microsoft.Identity.Web.Certificateless (it does what you just showed but does some assertion caching, because IMDS is very "throttly"). But it's not really advertised / samples etc. because the federated stuff I think is still in preview.
  3. We are bringing the pure Managed Identity support in MSAL. So to get the original token you won't need Azure SDK. Have a look at MSAL 4.49.1 (out today) - AcquireTokenForClient will have a WithManagedIdentity API (feedback on the API welcome, we are thinking of creating an AcquireTokenFromManagedIdentity API instead)

CC @jmprieur @schaabs @gladjohn

bgavrilMS commented 1 year ago

@jmprieur @jennyf19 - we've had a few people over the years discussing this. Wouldn't it fit into Microsoft.Identity.Web.Certificate ?

msundman78 commented 6 months ago

+1 for this feature request. I can't believe there are so many examples and implementation out there recommending to store RSA key-pairs in Azure KeyVault and then just exporting the private key to be used within the application. I goes against all basic PKI fundamentals. The private key should simply never leave the key vault. Azure Key Vault has all the functionality we need for a proper design where it used for the signing operations, even with HSM backed non-exportable keys. So it makes perfect sense to implement it as described in this request.

Yes, an attacker could use other methods to steal the ACCESS token, but the ACCESS token is short lived, so it's a much less exposure than having the private key exportable by any engineer with access to the key vault, who could silently export the key and use half a year later after leaving the company. Yes it will require one extra API call to Key Vault for the signing, but that's well worth it in most scenarios where you want decent security.

My specific use case for this is for cross-tenant authentications. I have Azure functions or automation runbooks in Tenant A using a Managed Identity in tenant A which needs to call custom APIs and MsGraph APIs in tenant B authenticated with certificate-based ClientCredentials stored in my tenant A's key vault. In this process I want the key vault to perform the signing of the OIDC JWT auth token during the authentication so my private key never has to leave the key vault.

I have this requirement from apps developed in both PowerShell (using Connect-MgGraph/Connect-AzAccount) and .NET, so it makes all sense to have this implemented in the base MSAL library so it can be leveraged by both MSAL.NET and PowerShell SDKs.

svrooij commented 6 months ago

@msundman78 I got you covered https://github.com/Smartersoft/identity-client-assertion/blob/main/docs%2FSmartersoft.Identity.Client.Assertion.md

This shows you how to use the key to sign the assertion in the cloud, the key will NEVER leave the keyvault.

But still this should be in msal

msundman78 commented 6 months ago

@msundman78 I got you covered https://github.com/Smartersoft/identity-client-assertion/blob/main/docs%2FSmartersoft.Identity.Client.Assertion.md

This shows you how to use the key to sign the assertion in the cloud, the key will NEVER leave the keyvault.

But still this should be in msal

That's awesome! Many thanks. I was just about to start writing my own module for this but thought I just can't be the first wanting this basic functionality.

I just wonder from the MSAL team, how is a managed identity going to solve my use case above? Will we have cross tenant managed identities somehow?

svrooij commented 6 months ago

@msundman78 at some point in time, you were able to use a token you got from managed identity as a federated credential for a multi tenant app.

I've build a PoC on this years ago, and then they blocked it..... https://svrooij.io/2022/06/21/managed-identity-multi-tenant-app/

And

https://svrooij.io/2022/12/16/poc-multi-tenant-managed-identity/

This does not longer work, unfortunately