Add MSAL_MI_DISABLE_IMDS_V2 kill switch for managed identity - #6178
Conversation
Adds an environment-variable kill switch that disables IMDSv2 as an emergency mitigation. IMDSv2 remains enabled by default; the switch is honored only for the values "true" or "1" (case-insensitive ordinal), and any other value is a no-op so a typo cannot silently degrade a fleet. The variable is read live on every check, so it takes effect and can be reversed without restarting the process. Behavior while the switch is set: - Plain bearer requests fall back to IMDSv1 and keep working. - WithMtlsProofOfPossession and WithRequestOverMtls fail fast with the new MsalError.ImdsV2Disabled rather than silently downgrading to an unbound token the caller did not ask for. This matches what MSAL already does on hosts that genuinely lack IMDSv2 support. - GetManagedIdentityCapabilitiesAsync reports MaxSupportedBindingStrength None with a populated ErrorReason, so credential chains such as DefaultAzureCredential select the bearer path instead of a PoP path that is guaranteed to fail. This is a behavior change on a public API; the signature is unchanged. The switch is enforced at three points rather than once at startup, because MSAL caches both the discovery result (in a process-wide static) and the binding certificate. A discovery-only check would be bypassed by any process that was already warm when the switch was flipped, which is exactly the situation during an incident. The discovery cache is masked on read and is not written while the switch is set. Masking stops a process that cached IMDSv2 beforehand from continuing to advertise PoP; not caching stops a switch-induced "v1-only" result from outliving the mitigation and requiring a restart to recover. Together these keep the switch reversible in place in both directions. Adds 22 unit tests covering value parsing, both cache-bypass paths, the warm certificate cache, bearer fallback, and reversibility. Each gate was verified to be load-bearing by disabling it and confirming the expected tests failed. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Declining to cache a switch-influenced discovery result kept the mitigation reversible but made it expensive: Azure.Identity calls GetManagedIdentityCapabilitiesAsync on every authentication and caches nothing itself, so every token request would have paid a serialized IMDS round trip for the duration of an incident. Record the switch state alongside the cached result instead. Off-to-on is masked on read, so no re-probe. Only on-to-off discards the cache, because that result never probed IMDSv2 and describes the switch rather than the host. Discovery is now O(1) in both modes and still reversible in place. Also: - Warn once per process when the variable is set to an unrecognized value. - Document the ErrorReason contract: non-null does not imply unavailability. - Correct the docs on error-code precedence, add the net462/net472 note, and document that already-cached tokens are not revoked. - Add regression tests for repeated discovery, the all-sources-unavailable verdict, and the token-cache carve-out. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
The new cached-state tracking introduces a potential inconsistent read/write concurrency hazard, and the capabilities masking can omit the kill-switch reason in ErrorReason for cached IMDSv1/None results.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds an emergency, process-wide environment-variable kill switch (MSAL_MI_DISABLE_IMDS_V2) to disable IMDSv2 managed identity behavior at runtime (without redeploying), while preserving IMDSv1 bearer-token availability and introducing a dedicated error code (MsalError.ImdsV2Disabled) for IMDSv2-required requests.
Changes:
- Introduces
MSAL_MI_DISABLE_IMDS_V2parsing + “unrecognized value” warning, and enforces the switch across discovery, routing, and mTLS-binding entry points. - Adds a new public error code (
MsalError.ImdsV2Disabled) with corresponding error message and PublicAPI entries across TFMs. - Adds a comprehensive unit-test suite covering parsing, cache poisoning in both directions, fallback behavior, and warm-cache bypass prevention; adds SDK guidance + dedicated docs page.
File summaries
| File | Description |
|---|---|
| tests/Microsoft.Identity.Test.Unit/ManagedIdentityTests/ImdsV2Tests.cs | Adds unit coverage for kill-switch parsing, routing/discovery interactions, and cache-bypass regressions. |
| src/client/Microsoft.Identity.Client/PublicApi/netstandard2.0/PublicAPI.Unshipped.txt | Declares new public error-code constant in API baseline. |
| src/client/Microsoft.Identity.Client/PublicApi/net8.0/PublicAPI.Unshipped.txt | Declares new public error-code constant in API baseline. |
| src/client/Microsoft.Identity.Client/PublicApi/net8.0-ios/PublicAPI.Unshipped.txt | Declares new public error-code constant in API baseline. |
| src/client/Microsoft.Identity.Client/PublicApi/net8.0-android/PublicAPI.Unshipped.txt | Declares new public error-code constant in API baseline. |
| src/client/Microsoft.Identity.Client/PublicApi/net472/PublicAPI.Unshipped.txt | Declares new public error-code constant in API baseline. |
| src/client/Microsoft.Identity.Client/PublicApi/net462/PublicAPI.Unshipped.txt | Declares new public error-code constant in API baseline. |
| src/client/Microsoft.Identity.Client/MsalErrorMessage.cs | Adds user-facing error strings for IMDSv2-disabled request failures and discovery diagnostics. |
| src/client/Microsoft.Identity.Client/MsalError.cs | Adds new public error-code constant + XML documentation describing mitigation and contrast with host capability limits. |
| src/client/Microsoft.Identity.Client/ManagedIdentity/ManagedIdentityClient.cs | Implements the kill switch across discovery caching, request routing, and mTLS binding acquisition. |
| src/client/Microsoft.Identity.Client/ManagedIdentity/ManagedIdentityCapabilities.cs | Updates ErrorReason doc semantics to cover “reduced capability” scenarios (not only total failure). |
| src/client/Microsoft.Identity.Client/ManagedIdentity/EnvironmentVariables.cs | Adds kill-switch env-var name constant, parsing, and “unrecognized value” detection. |
| docs/msi_v2/imds_v2_kill_switch.md | Documents accepted values, runtime behavior, enforcement points, logging, and testing. |
| docs/msi_v2/guidance_for_sdks_consuming_msal.md | Adds guidance for SDKs that branch on managed identity capabilities in the presence of the kill switch. |
Review details
Suppressed comments (1)
src/client/Microsoft.Identity.Client/ManagedIdentity/ManagedIdentityClient.cs:361
ApplyImdsV2KillSwitchreturns early for cached IMDSv1 +Noneresults, which meansManagedIdentityCapabilities.ErrorReasoncan fail to mention that the kill switch is active (e.g., if the process previously cached an IMDSv1-only discovery result and thenMSAL_MI_DISABLE_IMDS_V2is set). This is inconsistent with the documented behavior thatErrorReasonshould explain the reduced capability while the switch is on.
if (!imdsV2Disabled ||
result.Source != ManagedIdentitySource.Imds ||
(result.DetectedImdsVersion != ImdsVersion.V2 &&
result.MaxSupportedBindingStrength == MtlsBindingStrength.None))
{
- Files reviewed: 14/14 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Conflicts were limited to the six PublicAPI.Unshipped.txt files, where both sides added entries: main added the ICloudConfiguration surface (#6104) and this branch added MsalError.ImdsV2Disabled. Resolved by keeping both sides and restoring the analyzer's ordering (types, then const, then static, each alphabetical). Verified with a clean build on net462, net472, net8.0, and netstandard2.0. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The result and the switch state it was computed under lived in two separate static fields, written and read outside the discovery lock. A reader could observe a fresh result paired with a stale flag, or the reverse, and then misjudge whether the cached value describes the host or merely describes the switch. The two readers also used opposite orderings, which made the pair harder to reason about than it needed to be. Collapse both into an immutable CachedDiscovery snapshot published and read through a single volatile reference, so the pair is untearable by construction rather than by argument. Behavior is unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
There are a couple of concrete behavior/robustness issues (unrecognized-value warning not emitted on token-only flows, and inconsistent double-reading of the env var) that should be fixed to match the stated contract.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 14/14 changed files
- Comments generated: 2
- Review effort level: Lite
…y path Read the kill switch environment variable once per check instead of twice. IsImdsV2Disabled and HasUnrecognizedImdsV2DisableValue each issued their own Environment.GetEnvironmentVariable call, so a caller reading both observed two separate reads that could disagree if the variable changed in between. Both now derive from a single read via a shared private classifier. Emit the unrecognized-value warning from SelectManagedIdentitySourceType as well. It previously fired only from GetManagedIdentityCapabilitiesAsync, so a process that only acquires bearer tokens never ran discovery and never saw the warning - which is the worst case, because a bearer-only process is exactly what is running when IMDSv2 has been disabled in response to an incident. Still logged once per process through the existing Interlocked guard. Add a regression test covering the bearer-only path. It fails when the new call site is removed. Correct the kill switch documentation. The Purpose section asserted an operator persona and a rollout motivation that the originating issue does not state. Rewritten to describe only the specified behavior, with the design reasoning kept but explicitly labeled as inference rather than a stated requirement. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
It introduces security-critical runtime gating and caching behavior in managed identity flows, and should receive final human validation despite only minor actionable review notes.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
src/client/Microsoft.Identity.Client/ManagedIdentity/ManagedIdentityClient.cs:278
- The comments here still refer to
s_cachedSourceResult, but that field was removed and replaced withs_cachedDiscovery. Leaving the old name is misleading when debugging kill-switch routing behavior.
- Files reviewed: 14/14 changed files
- Comments generated: 1
- Review effort level: Lite
Comments across the kill switch change spent too many lines narrating mechanics the code already states. Trimmed to lead with the reason a given decision was made and dropped the restatements. Fix a doc comment defect introduced when EnvironmentVariables was refactored to share a single environment read: the summary belonging to IsImdsV2Disabled was left stranded above ReadImdsV2DisableState, giving that method two stacked summary blocks and leaving IsImdsV2Disabled undocumented. Correct two comments and one doc paragraph that still referred to s_cachedSourceResult, a field renamed to s_cachedDiscovery when the cached result and switch state were collapsed into one snapshot. Remove framing the originating task (#6174) does not state. The issue specifies the variable, its accepted values, and the required behavior; it does not describe who sets it or the circumstances under which they would. Wording that asserted an operator persona, an incident, or an emergency mitigation has been replaced with what the switch does and why the design follows. The design rationale that remains in the documentation is still labeled as inference. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
The kill-switch state is read multiple times per request path, which can produce inconsistent decisions or misleading warnings when the environment variable is flipped at runtime.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
src/client/Microsoft.Identity.Client/ManagedIdentity/ManagedIdentityClient.cs:209
- SelectManagedIdentitySourceType reads MSAL_MI_DISABLE_IMDS_V2 twice (IsImdsV2Disabled, then WarnOnceIfImdsV2DisableValueUnrecognized -> HasUnrecognizedImdsV2DisableValue). If an operator flips the variable between those reads (a supported scenario per the PR design), the warning can become misleading (e.g., logging that IMDSv2 remains enabled while routing has already observed it as disabled). Read and classify the variable once per routing decision, and only attempt the warning when that same snapshot says the value is unrecognized.
bool imdsV2Disabled = EnvironmentVariables.IsImdsV2Disabled;
// Warned here too, because a process that only acquires bearer tokens never runs
// capability discovery and would otherwise never learn its switch value is inert.
WarnOnceIfImdsV2DisableValueUnrecognized(requestContext);
src/client/Microsoft.Identity.Client/ManagedIdentity/ManagedIdentityClient.cs:405
- GetManagedIdentityCapabilitiesAsync also reads MSAL_MI_DISABLE_IMDS_V2 twice (IsImdsV2Disabled, then WarnOnceIfImdsV2DisableValueUnrecognized). Because the kill switch is explicitly live/read-per-check, the variable can change mid-call and produce inconsistent behavior or misleading logging. Use a single read/classification for both the cached-result selection and the unrecognized-value warning in this method.
bool imdsV2Disabled = EnvironmentVariables.IsImdsV2Disabled;
WarnOnceIfImdsV2DisableValueUnrecognized(requestContext);
- Files reviewed: 14/14 changed files
- Comments generated: 1
- Review effort level: Lite
SelectManagedIdentitySourceType and GetManagedIdentityCapabilitiesAsync each read the environment variable twice: once through IsImdsV2Disabled to decide the request, and again inside WarnOnceIfImdsV2DisableValueUnrecognized to decide whether to warn. The switch is deliberately read live so it stays reversible without a restart, so those two reads can straddle a change. The two states are mutually exclusive, so only one interleaving misbehaves: a value changed from "true" to an unrecognized value between the reads blocks the request off the first read while logging "IMDSv2 remains enabled" off the second. Enforcement stays correct either way - the first read governs routing, the downgrade, and the throw for the whole method, so the switch cannot fail open - but the log can contradict the request it describes. Promote the classifier to internal and pass the classification into the warning instead of re-reading, so each request path reads once and the warning can only ever describe the read that decided the request. This mirrors the treatment already applied to the cached discovery result and its switch state, which were collapsed into a single snapshot for the same reason. Remove HasUnrecognizedImdsV2DisableValue, which has no remaining callers and was the API that made the double read possible. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
The kill-switch masking logic can fail to reflect the disabled state for previously cached IMDSv1-only discovery results, and the new doc incorrectly describes net472 behavior.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
src/client/Microsoft.Identity.Client/ManagedIdentity/ManagedIdentityClient.cs:387
- ApplyImdsV2KillSwitch returns the cached discovery result unchanged when it is already (IMDSv1 + MaxSupportedBindingStrength=None). If the host was discovered as IMDSv1-only before MSAL_MI_DISABLE_IMDS_V2 was set, this short-circuit prevents the kill switch from being reflected in the discovery result (e.g., ErrorReason/ImdsV2FailureReason won’t indicate the switch is on). This contradicts the documented/expected “switch takes precedence” diagnostics while disabled.
docs/msi_v2/imds_v2_kill_switch.md:81 - The note about framework support is inaccurate: ManagedIdentityPopExtensions gates these APIs only under
#if NET462, not net472. Also, “throw … at build time” is misleading—on net462 the code compiles but throws when the method is invoked. The doc should reflect the actual conditional behavior so consumers don’t incorrectly assume net472 is excluded.
- Files reviewed: 14/14 changed files
- Comments generated: 0 new
- Review effort level: Lite
Gladwin Johnson VR (gladjohn)
left a comment
There was a problem hiding this comment.
This PR substantially exceeds the task. Existing IMDSv1 behavior already rejects PoP requests. Please implement only the environment-variable check that disables IMDSv2 discovery/routing, preserve the existing IMDSv1 bearer and PoP behavior, and document that restart/recycle is required.
MSAL reads the process environment, which is fixed when the process starts, so the switch cannot change mid-process outside tests. The earlier version carried state and cache handling for transitions that cannot occur in a real host. Enforcement is now two checks. Discovery skips the IMDSv2 probe and reports no binding support. Source selection stops the direct-to-IMDSv2 route by converting an early return into a fall-through, so mTLS requests reach the IMDSv1 guard that already rejects them. An mTLS request does not always run discovery: with no minimum strength floor the binding path skips the capability call entirely, so a check placed only in discovery leaves that route minting bound tokens. Removes the dedicated error code, the unrecognized-value warning, and the reverse-direction cache change. No public API surface changes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Add
MSAL_MI_DISABLE_IMDS_V2kill switchFixes #6174
Environment-variable kill switch that disables IMDSv2 for a process, so a host hitting an IMDSv2 problem can fall back to IMDSv1 without waiting for an MSAL release. Enabled by default; honored for
true(case-insensitive) and1. Anything else is ignored, so a typo cannot quietly weaken token binding.Two product files: one env-var read, and the two places that consult it.
WithMtlsProofOfPossession()MsalError.MtlsPopTokenNotSupportedinImdsV1WithRequestOverMtls()MsalError.MtlsPopTokenNotSupportedinImdsV1WithMtlsProofOfPossession(...)with aMinStrengthfloorMsalError.MinStrengthNotMetGetManagedIdentityCapabilitiesAsync()MaxSupportedBindingStrength = NoneA restart is required
MSAL reads the process environment. Nothing outside the process can modify that block — Windows exposes no supported API, and
/proc/<pid>/environis read-only on Linux — so an edit made at the machine, service, or container level is invisible to an already-running process. Setting or clearing the variable takes effect only after a restart or recycle.This is mechanics rather than a design choice, and it costs nothing in practice: the deployment mechanisms that would set the variable — app restart, container replacement, VM reimage — all restart the process anyway. A process can still change its own block through
Environment.SetEnvironmentVariable, which is how the tests exercise both states, but nothing in a real deployment does. The value is settled before the first call, so there is no warm-cache state to reconcile.mTLS requests fail fast
WithMtlsProofOfPossession()andWithRequestOverMtls()are served exclusively by IMDSv2 and have no IMDSv1 equivalent, so both throw rather than return a token. A plain request getsMtlsPopTokenNotSupportedinImdsV1, the error MSAL already raises on a host with no IMDSv2 support. A request carrying aMinStrengthfloor is measured against the reportedNoneby the pre-existing floor check and getsMinStrengthNotMetinstead.They throw rather than fall back because the caller opted into something IMDSv1 cannot provide and has no way to notice that a weaker result came back instead. The two APIs ask for different things —
WithMtlsProofOfPossession()for a certificate-bound token,WithRequestOverMtls()for a bearer token issued over an mTLS connection — and neither is reachable without IMDSv2. Reusing the existing code is deliberate: to the caller the situation is the same either way — mTLS is unavailable here — and the log records which cause applies. The existing message attributes the failure to the VM image, which is accurate for a genuinely v1-only host but not when the switch is the cause; the log line is what separates the two.Capability discovery reports no binding support
While the switch is set,
GetManagedIdentityCapabilitiesAsync()reportsMaxSupportedBindingStrength = NoneandIsMtlsPopSupportedByHost = false.Callers use this API to decide up front whether to ask for a bound token. Reporting the host's hardware capability while the switch is on would let a chain such as
DefaultAzureCredentialconfidently pick the PoP path and then fail on every token request. Reporting what the caller can actually obtain keeps them on the bearer path. This does narrowMaxSupportedBindingStrengthfrom hardware capability to effective availability; the two already diverge on a v1-only host, which is graded even though it can never mint a bound token.MSAL therefore also skips the IMDSv1 binding-strength probe: with no route to a bound token there is nothing to grade, and the check would only cost an HTTP call.
Two enforcement points
An mTLS request does not always run discovery. With no
MinStrengthfloor, the binding path skips the capability call and routes straight to IMDSv2, so a check placed only in discovery is never reached on that path — with the switch on, the call still returnstokenType=mtls_popand a provisioned certificate. I verified that directly before adding the second check.GetManagedIdentityCapabilitiesAsyncSelectManagedIdentitySourceTypeThe second adds no rejection logic of its own. It converts an early return into a fall-through so the request reaches the IMDSv1 mTLS guard that already exists, and that guard throws.
Environment-detected sources (App Service, Cloud Shell, and the rest) are untouched. IMDSv2 is never involved there, so they keep their existing
MtlsPopNotSupportedForEnvironmenterror, which diagnoses those hosts more accurately.Tests
11 cases across 5 methods in
ImdsV2Tests.cs: value parsing (true/True/1disable; unset, empty,false,yesdo not), discovery resolving to IMDSv1 with no binding support, repeated discovery, bearer over IMDSv1, and both mTLS shapes throwing.The mocks carry most of the assertion. Only IMDSv1 handlers are queued, and
MockHttpManagerfails both on an unmatched request and on an unconsumed mock, so the tests prove that no IMDSv2 probe, CSR, or certificate call was issued and that the binding-strength probe no longer fires.Teeth verified by mutation: with the switch forced to report "not disabled", 7 of the 11 fail. The 4 that still pass are the switch-off rows, which should pass.
Full ManagedIdentity suite: 473 passed, 1 skipped, 0 failed. Build clean, 0 warnings.
Compatibility
No public API change and no new error code, so
PublicAPI.Unshipped.txtis untouched.One intentional behavior change on an existing public API: while the switch is set,
GetManagedIdentityCapabilitiesAsyncreports no binding support on a host that would otherwise advertise it. That is the purpose of the switch — a caller must not be told PoP is available and then be thrown at on every request. It occurs only when the variable is set, so default behavior is unchanged.Checked
Microsoft.Identity.Webfor the affected APIs. It calls none of the capability APIs (GetManagedIdentityCapabilitiesAsync,MaxSupportedBindingStrength,IsMtlsPopSupportedByHost,ManagedIdentityCapabilities,MtlsBindingStrength) and does not useWithRequestOverMtls, so it cannot observe this change. Its threeWithMtlsProofOfPossession()call sites each sit behind an explicit opt-in flag (bindToCertificate,isTokenBinding) and are unaffected unless the variable is set.Docs
docs/msi_v2/imds_v2_kill_switch.mdcovers accepted values, the restart requirement, and both enforcement points.guidance_for_sdks_consuming_msal.mdgets a pointer, since SDK authors branching on capabilities are the audience most affected.assistance: agentic-cli
type: security
agent-tool: copilot-cli
agent-model: claude-opus-5
work-item: AB#n/a