search

AzureAD / microsoft-authentication-library-for-dotnet

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

Correct IMDS resource ID query parameter #4911

Open christothes opened 2 months ago

christothes commented 2 months ago

Specifying an identity by resource ID doesn't work on Azure Container Instances (and maybe some other platforms) because we specify the resource ID with query parameter "mi_res_id". ACI observes only "msi_res_id", which we should prefer anyway because although IMDS observes both parameters, it documents only msi_res_id.

rayluo commented 2 months ago

Interesting.

We (MSAL teams) were mostly referencing public docs from https://learn.microsoft.com, but indeed they seem inconsistent, such as this App Service doc mentions mi_res_id and this VM doc mentions msi_res_id (and IIRC we manually tested the mi_res_id before and it also worked).

Question:

  1. What is the single source of truth of this topic? That repo (https://github.com/Azure/azure-rest-api-specs) sounds promising but is it also used by all the Managed Identity services?
  2. MSALs currently support these flavors of managed identities. Is Azure Container Instance considered one of them, or a new one?
christothes commented 2 months ago

Interesting.

We (MSAL teams) were mostly referencing public docs from https://learn.microsoft.com, but indeed they seem inconsistent, such as this App Service doc mentions mi_res_id and this VM doc mentions msi_res_id (and I thought we manually tested the mi_res_id before).

Question:

  1. What is the single source of truth of this topic? That repo (https://github.com/Azure/azure-rest-api-specs) sounds promising but is it also used by all the Managed Identity services?

Good question, and I don't know the right answer. We would need to validate with the resource owners.

  1. MSALs currently support these flavors of managed identities. Is Azure Container Instance considered one of them, or a new one?

ACI is an IMDS-like endpoint, but I'm not sure if it is an actual IDMS implementation, or similar to it like with other hosting platforms. In such cases, we typically rely on the IMDS fallback as long as they adhere to the same protocol.

bgavrilMS commented 2 months ago

Hi @christothes - do I understand correctly that Azure SDK has the same issue today? Is this blocking Azure SDK adoption of MSAL for MSI ?

christothes commented 2 months ago

Hi @christothes - do I understand correctly that Azure SDK has the same issue today? Is this blocking Azure SDK adoption of MSAL for MSI ?

Correct - we could fix it but as we've already migrated to MSAL in preview, it wouldn't take effect until MSAL addresses it.

chlowell commented 2 months ago

What is the single source of truth of this topic? That repo (https://github.com/Azure/azure-rest-api-specs) sounds promising but is it also used by all the Managed Identity services?

All our flavors of managed identity are owned by different teams. They have sometimes imitated each other but their implementations are separate. mi_res_id is correct for App Service. It happens to work for IMDS as well, however the source of truth for IMDS is its REST spec, which documents only msi_res_id. We should follow the IMDS spec if only because at least one service (ACI) apparently did the same and observes only msi_res_id.

rayluo commented 2 months ago

What is the single source of truth of this topic? That repo (https://github.com/Azure/azure-rest-api-specs) sounds promising but is it also used by all the Managed Identity services?

All our flavors of managed identity are owned by different teams. They have sometimes imitated each other but their implementations are separate. mi_res_id is correct for App Service. It happens to work for IMDS as well, however the source of truth for IMDS is its REST spec, which documents only msi_res_id. We should follow the IMDS spec if only because at least one service (ACI) apparently did the same and observes only msi_res_id.

  1. Agreed that there are many flavors of Managed Identity (MI). I guess that means we the SDKs can no longer operate with an assumption that "we expect a parameter foo to work across all MIs". We need to refactor our implementation to allow each flavor having its own parameters and fall back into the IMDS as a default.
  2. The Azure Rest Api Specs repo is meant to be an archive of truth: "The REST API descriptions for all Azure services should be published in the Azure REST API specs GitHub repositories". We will use it as a reference from now on. FWIW, it seems that it does not contain all the API versions of the 5 or 6 MI flavors MSAL currently support.
  3. Back to the Azure Container Instance (ACI) topic. Do we have further docs to define the ACI environment detection and its protocol? Or shall MSAL simply serve it by going with the default code path for Azure VM?
chlowell commented 2 months ago
  1. Agreed that there are many flavors of Managed Identity (MI). I guess that means we the SDKs can no longer operate with an assumption that "we expect a parameter foo to work across all MIs". We need to refactor our implementation to allow each flavor having its own parameters and fall back into the IMDS as a default.

👍 it's never been safe to assume uniformity across these APIs. They're all snowflakes.

  1. The Azure Rest Api Specs repo is meant to be an archive of truth: "The REST API descriptions for all Azure services should be published in the Azure REST API specs GitHub repositories". We will use it as a reference from now on. FWIW, it seems that it does not contain all the API versions of the 5 or 6 MI flavors MSAL currently support.

So far as I know, only IMDS has a formal spec. Other endpoints are described less formally in their public docs, if at all, and those docs are sometimes wrong. The Azure SDK may be the best example of what works.

  1. Back to the Azure Container Instance (ACI) topic. Do we have further docs to define the ACI environment detection and its protocol? Or shall MSAL simply serve it by going with the default code path for Azure VM?

It's indistinguishable from IMDS at runtime. See e.g. the ACI docs. The same code can support both IMDs and ACI. I don't know any practical difference other than the resource ID parameter.

bgavrilMS commented 2 months ago

How about we test this out in MSAL Golang, which is being worked on, and then make the changes in the other MSALs too? It will help with testing. But it will take longer to fix.

CC @4gust @AndyOHart

christothes commented 1 month ago

It appears there is another scenario that we haven't accounted for on ACI. As described in this article, for Windows containers, the normal IMDS endpoint (169.254.169.254) isn't available. In that case, the IDENTITY_ENDPOINT environment variable will contain the endpoint URI and the IDENTITY_HEADER env var will contain a header value that must also be sent.

Here are some details from an issue where a customer hit this. https://github.com/Azure/azure-sdk-for-net/issues/45547#issuecomment-2359079021

rayluo commented 1 month ago

It appears there is another scenario that we haven't accounted for on ACI. As described in this article, for Windows containers, the normal IMDS endpoint (169.254.169.254) isn't available. In that case, the IDENTITY_ENDPOINT environment variable will contain the endpoint URI and the IDENTITY_HEADER env var will contain a header value that must also be sent.

Here are some details from an issue where a customer hit this. Azure/azure-sdk-for-net#45547 (comment)

Then we might have reached the limitation of the current Managed Identity v1 design. With the current heuristic algorithm, the existence of both IDENTITY_ENDPOINT and IDENTITY_HEADER will trigger the App Services code path, however App Services uses a different version of api-version which also does not mention anything about principalId. How can we differentiate App Services from ACI? Or is that even possible?

jaysondin commented 1 week ago

It appears there is another scenario that we haven't accounted for on ACI. As described in this article, for Windows containers, the normal IMDS endpoint (169.254.169.254) isn't available. In that case, the IDENTITY_ENDPOINT environment variable will contain the endpoint URI and the IDENTITY_HEADER env var will contain a header value that must also be sent.

Here are some details from an issue where a customer hit this. Azure/azure-sdk-for-net#45547 (comment)

Hi all, one of my customers is running into similar issues as they try to get managed identity in their ACI implementation. Do we have any updates / guidance please?