ImdsV2 is detected, even on a UAMI configuration error #5581
Conversation
|
|
||
| if (response.StatusCode != HttpStatusCode.OK) | ||
| { | ||
| if (probeMode) |
There was a problem hiding this comment.
IMDS throttles very aggressively on 200s, but is much more forgiving of 400s.
For that reason we want the IMDSv2 probe to have a single responsibility:
“Does the endpoint exist and respond?”
and nothing else.
Right now GetPlatformMetadata ends up hitting IMDSv2 twice, and the probe path behaves too much like a “real” call:
- It can return 200.
- It runs identity-specific logic (UAMI /
IdentityNotFound) that we only need on the second call.
We’d like to align the probe with the way Azure.Identity does it in ImdsManagedIdentityProbeSource:
- The probe sends a GET to the IMDS endpoint without the
Metadata: trueheader. - IMDS responds with 400 very quickly.
- A 400 of that specific shape is treated as “probe succeeded, endpoint exists”
(they throw an internalProbeRequestResponseExceptionand then do the real call throughMsalManagedIdentityClient). - Only the second call is a full, valid IMDS request that can return 200 and surface identity errors.
Concretely, for MSAL
Probe call
- Build the request so IMDS returns a predictable 400 (e.g., omit the
Metadataheader). - Treat that 400 as “IMDSv2 endpoint is available.”
- Do not rely on 200 responses or run UAMI /
IdentityNotFoundlogic here — the probe is not an identity call.
Second (real) call
- Send the fully valid IMDSv2 request (with
Metadata: true+ identity hints). - This is the only call that should be able to return 200.
- This is also the only place where we classify UAMI vs system-assigned and map
IdentityNotFoundvsManagedIdentityRequestFailed.
That keeps the probe cheap and stable (one responsibility: endpoint exists)
and ensures we’re not burning extra 200s against IMDS just to detect IMDSv2.
IMDS V2 probe spec - https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/blob/main/docs/msi_v2/vm_vmss_credential_probe.md
There was a problem hiding this comment.
I think this should be tracked on a separate GitHub issue. This PR fixes the linked bug, that you were pushing me to fix, so that people using public preview are no longer blocked.
Do you remember our discussions where we decided to combine the probe and the metadata endpoint? We had that discussion very early on.
There was a problem hiding this comment.
If there are partners blocked by this, agreed to take this fix, but I am not sure there are?
Otherwise, it does seem like the 400 error is the correct fix and this can be done alongisde MSIv1 probing
There was a problem hiding this comment.
While onboarding the partner we found they are using UAMI, and UAMI itself is not working in the environment. So no new partner onboarding till end of month.
| public const string InvalidCertificate = "The certificate received from the Imds server is invalid."; | ||
| public const string CannotSwitchBetweenImdsVersionsForPreview = "ImdsV2 is currently experimental - A Bearer token has already been received; Please restart the application to receive a mTLS PoP token."; | ||
| public const string MtlsPopTokenNotSupportedinImdsV1 = "A mTLS PoP token cannot be requested because the application\'s source was determined to be ImdsV1."; | ||
| public const string IdentityNotFound = "Managed Identity not found. To request credentials for this identity, please assign it first. Please see aka.ms/ManagedIdentityNotFound for more details"; |
There was a problem hiding this comment.
IMDS already returns the IdentityNotFound error message, and MSAL simply bubbles up the IMDS payload without rewriting or overriding it. Adding our own string introduces duplication and may cause divergence from IMDS' authoritative error.
There was a problem hiding this comment.
This is the exact error message from Imds - It's used multiple times in this PR, including test mocks, so I made it a constant.
I could return the error message from Imds, but I would still keep this constant for the test mocks.
There was a problem hiding this comment.
Agreed with @gladjohn - we can depend on the error code, but the error messages should from the server.
|
Are we abandoning this @Robbie-Microsoft ? |
Yes, that's what you and Gladwin told me. |
Fixes #5565