microsoft-authentication-library-for-python icon indicating copy to clipboard operation
microsoft-authentication-library-for-python copied to clipboard

[Feature Request] Add cert validation for MI auth in Service Fabric

Open Avery-Dunn opened this issue 1 year ago • 5 comments

MSAL client type

Managed identity

Problem Statement

According to the managed identity docs for Service Fabric, there is an environment variable called "IDENTITY_SERVER_THUMBPRINT": https://learn.microsoft.com/en-us/azure/service-fabric/how-to-managed-identity-service-fabric-app-code#acquiring-an-access-token-using-rest-api

And per those docs, the thumbprint should be used as part of the steps to retrieve a token:

To obtain a token, the client performs the following steps:

  • forms a URI by concatenating the managed identity endpoint (IDENTITY_ENDPOINT value) with the API version and the resource (audience) required for the token
  • creates a GET http(s) request for the specified URI
  • adds appropriate server certificate validation logic
  • adds the authentication code (IDENTITY_HEADER value) as a header to the request
  • submits the request

Proposed solution

The docs don't describe what 'appropriate' validation entails, however Azure Identity does perform this step for the Java and .NET libraries:

  • For Java, the thumbprint is added to the HttpsURLConnection object that manages the HTTP requests
    • Related issue in MSAL Java repo: https://github.com/AzureAD/microsoft-authentication-library-for-java/issues/758
  • For .NET, a callback is added that compares the server's environment variable to another cert
    • Related issue in MSAL .NET repo: https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/issues/4462

Unfortunately for Python they have not implemented any validation logic to copy, and simply hardcode the relevant 'connection_verify' flag to false.

Avery-Dunn avatar Jan 23 '24 19:01 Avery-Dunn

Unfortunately for Python they have not implemented any validation logic to copy, and simply hardcode the relevant 'connection_verify' flag to false.

No wonder we were not aware of this feature requirement when we researched the prior art in Azure Identity for Python. And, that also means MSAL Python's current Managed Identity behavior is on par with Azure Identity's status quo.

rayluo avatar Jan 23 '24 19:01 rayluo

Marking this a bug, as it is inconsistent with Azure SDK's managed identity implementation, which we try to replace. This is a GA blocker.

bgavrilMS avatar Feb 09 '24 11:02 bgavrilMS

For Java and .NET it's a bug since the behavior doesn't match what's in Azure SDK, but for Python (and Node) Azure SDK's implementation doesn't currently validate the cert so the current behavior technically is consistent.

However, they also have an issue on their backlog to add this validation, and I believe it's a blocker for them as well: https://github.com/Azure/azure-sdk-for-python/issues/33431

Avery-Dunn avatar Feb 09 '24 17:02 Avery-Dunn

Disabling cert validation is our behavior today. Given that, I don't think this is a GA blocker.

xiangyan99 avatar Feb 23 '24 22:02 xiangyan99

And if you want to discuss more details, let's use emails.

xiangyan99 avatar Feb 23 '24 22:02 xiangyan99