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

Enable certificate pinning for all requests to login.microsoftonline.com #4646

Open svrooij opened 6 months ago

svrooij commented 6 months ago

Summary

The current implementation uses the default httpclienthandler, which trusts the root certificates on the machine. If the machine is compromised, critical security information from the app might be shared with a traffic intercepting proxy.

Motivation and goals

In scope

An opt-in way on the clients EnforceRootCA() might be a good first step, after that it might become default with an opt-out.

Out of scope

...

Risks / unknowns

There are some valid use cases for intercepting traffic in debug situations, those will stop to work.

Examples

var httpClientHandler = new HttpClientHandler();
httpClientHandler.ServerCertificateCustomValidationCallback = (message, cert, chain, errors) =>
{
  // Can someone tell me if this is the default?
  bool defaultResult = errors == System.Net.Security.SslPolicyErrors.None;
  if (!defaultResult) {
    return false; //Fail fast, if the default validation fails.
  }

  if (message.RequestUri!.Host == "login.microsoftonline.com")
  {
    // Microsoft uses DigiCert, so we can check the thumbprint of the root certificate.
    bool msResult = chain!.ChainElements.Last().Certificate.Thumbprint == "A8985D3A65E5E5C4B2D7D66D40C6DD2FB19C5436";
    if (!msResult )
    {
        Console.WriteLine("Got cert with name {0} and thumb {1} for login.microsoftonline.com", cert!.Subject, cert.Thumbprint);
        foreach (var element in chain!.ChainElements)
        {
            Console.WriteLine("Element: {0} thumb: {1}", element.Certificate.Subject, element.Certificate.Thumbprint);
        }
    }
    return msResult;
  }

  return true;
};
var httpClient = new HttpClient(httpClientHandler);

Give brief examples of possible developer experiences (e.g., code they would write).

Don't be deeply concerned with how it would be implemented yet. Your examples could even be from other technology stacks.

bgavrilMS commented 6 months ago

Well, you can certainly do that by using the HttpClient extensiblity point we created - https://learn.microsoft.com/en-us/dotnet/api/microsoft.identity.client.imsalhttpclientfactory?view=msal-dotnet-latest

svrooij commented 6 months ago

@bgavrilMS this would require each developer to implement this, instead of the library providing the most secure option possible.

And on the topic of the IMsalHttpClientFactory, why is it different from the IHttpClientFactory that other libraries are advised to use? The IHttpClientFactory is what you get when you call .AddHttpClient() on the service collection in Microsoft.Extensions.DependemcyIntjection

bgavrilMS commented 6 months ago

MSAL is a foundational library and we decided not to depend on any of Microsoft.Extensions.* libs. Maybe in the next major version of MSAL we will change this.

bgavrilMS commented 6 months ago

If I understand your blog correctly, an attacker would need to

In case of the connection between client and IdP, this would allow the attacker to steal any credentials that the client passes on to the identity provider.

Would it allow the attacker to intercept the response from the Identity Provider, i.e. the actual tokens?

svrooij commented 6 months ago

Yes they would have some sort of MITM in place, in an enterprise environment where all devices are managed, adding a root cert is not that hard as an adversary. You also have questionable regimes that tried this before, if all those attempts would be blocked by the library, that would be a great improvement to security.

Once they have a questionable root cert in place, you could redirect the dns to the malicious server, that pretends to be login.microsoftonline.com. And see/modify/redirect all (incoming and outbound) traffic, including tokens or client secrets.

sequenceDiagram
    participant User
    participant Attacker
    participant LoginServer
    User->>Attacker: Give me a token here are my secrets
    Attacker->>LoginServer: Give me a token here are my secrets
    LoginServer->>Attacker: Access token and refresh Token
    Attacker->>Attacker: Save details in database
    Attacker->>User: option 1 Here you go
    Attacker->>User: option 2 401 error

No sure, why the mermaid diagram is not loading.

nbevans commented 6 months ago

If the machine or domain controller has been compromised sufficiently to insert an attacker's own Root CA to use for MITM then you can also assume the machine or the domain's machines may have a in-memory token string scanner installed. It's not like token strings are hard to spot in memory, they're just a JWT.

Pinning a certificate like this can be problematic in the long term. I can recall numerous times in the past 20 years when legit Root CAs have been revoked years ahead of their expiry. If Digicert needed to reissue their cert, all MSALs which had pinned the old cert will break. And you'd need to firstly wait for a new MSAL build with the new cert in it and then recompile and redistribute your app to get the fix to your users.

bgavrilMS commented 6 months ago

I agree, @nbevans, if you can compromise the root certs on a machine, you likely have admin / sudo access and can do much more damage.

I am not aware of any prior art around this. I have reached out to Azure SDK and they do not have this sort of protection, they rely on root certs.

The certificate stores that have the root CA have a bit more flexibility inbuilt, with support for revocation and rotation. Pinning to an exact certificate can have unintended consequences.

svrooij commented 6 months ago

What about making it opt-in as specified above, with a big notice "Only enable this if you have a way in place to update the code in the unfortunate event that DigiCert has to revoke its root certificates".

Does the same count for mobile devices? Those can be hacked and we should do anything possible to make it as hard as possible to extract tokens, checking the exact root cert won't prevent it completely, but it will make it a lot harder.

svrooij commented 5 months ago

I understand the need to be able to rollover the certificates without the need to update all the apps. I recently came across DANE which seems to utilize DNS records (type TLSA) secured by DNSSEC to communicate which certificate shall be trusted for a particular website.

I even build a proof-of-concept that does just this. With a secure way to rollover certificates in case of changes would this be something you would reconsider?