Propagate KeyGuard attestation failures and bridge native MAA logs to the MSAL logger

CHANGELOG.md

@@ -19,6 +19,7 @@
 - Fixed `WithExtraQueryParameters` on `ManagedIdentityApplicationBuilder` bypassing token caching. [#6035](https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/pull/6035)
 - Guarded HTTP status codes on discovery endpoints in `KnownInstanceMetadataIsUpToDateAsync`. [#6048](https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/pull/6048)
 - Detect orphaned KeyGuard certificates via public-key modulus comparison. [#6020](https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/pull/6020)
+- KeyGuard/Credential Guard attestation failures now surface as an `MsalServiceException("attestation_failed")` (carrying the MAA status/native error/reason) instead of silently sending a non-attested certificate request to IMDS, and native `AttestationClientLib` (MAA) logs are bridged into the MSAL `ILoggerAdapter`. **Behavior change:** when an attestation provider is configured via `WithAttestationSupport()` and returns no token for a KeyGuard key, the request now fails closed rather than falling back to a non-attested request. [#6081](https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/pull/6081)
 
 4.84.1
 ======

src/client/Microsoft.Identity.Client.KeyAttestation/Attestation/AttestationClient.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Runtime.InteropServices;
+using Microsoft.Identity.Client.Core;
 using Microsoft.Win32.SafeHandles;
 
 namespace Microsoft.Identity.Client.KeyAttestation.Attestation
@@ -15,19 +16,33 @@ internal sealed class AttestationClient : IDisposable
     {
         private bool _initialized;
 
+        // Strong reference to the native log callback for the lifetime of this client. The native library
+        // stores the function pointer during InitAttestationLib and invokes it until UninitAttestationLib;
+        // keeping the delegate referenced here prevents the GC from collecting it (which would crash the
+        // native callback).
+        private readonly AttestationClientLib.LogFunc _logBridge;
+
         /// <summary>
         /// AttestationClient constructor. Relies on the default OS loader to locate the native DLL.
         /// </summary>
+        /// <param name="logger">
+        /// Optional MSAL logger. When supplied, native <c>AttestationClientLib.dll</c> log lines are
+        /// forwarded into the MSAL logging pipeline so MAA diagnostics appear in MSAL logs. When null,
+        /// native logs fall back to <see cref="System.Diagnostics.Trace"/>.
+        /// </param>
         /// <exception cref="InvalidOperationException"></exception>
-        public AttestationClient()
+        public AttestationClient(ILoggerAdapter logger = null)
         {
             string dllError = NativeDiagnostics.ProbeNativeDll();
             // intentionally not throwing on dllError
 
+            // Bridge native logs into the MSAL logger when available; otherwise Trace.
+            _logBridge = AttestationLogger.CreateLoggerBridge(logger);
+
             // Load & initialize (logger is required by native lib)
             var info = new AttestationClientLib.AttestationLogInfo
             {
-                Log = AttestationLogger.ConsoleLogger,
+                Log = _logBridge,
                 Ctx = IntPtr.Zero
             };
 
@@ -58,7 +73,13 @@ public AttestationResult Attest(string endpoint,
                     endpoint, null, null, keyHandle, out buf, clientId);
 
                 if (rc != 0)
-                    return new(AttestationStatus.NativeError, null, null, rc, null);
+                {
+                    // Translate the native return code into a human-readable reason so the failure
+                    // (e.g. MAA policy evaluation / dbx validation) propagates back to the caller
+                    // instead of being silently dropped.
+                    string reason = AttestationErrors.Describe((AttestationResultErrorCode)rc);
+                    return new(AttestationStatus.NativeError, null, null, rc, reason);
+                }
 
                 if (buf == IntPtr.Zero)
                     return new(AttestationStatus.TokenEmpty, null, null, 0,

src/client/Microsoft.Identity.Client.KeyAttestation/Attestation/AttestationLogger.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Diagnostics;
 using System.Runtime.InteropServices;
+using Microsoft.Identity.Client.Core;
 
 namespace Microsoft.Identity.Client.KeyAttestation.Attestation
 {
@@ -16,11 +17,7 @@ internal static class AttestationLogger
         {
             try
             {
-                string sTag = ToText(tag);
-                string sFunc = ToText(func);
-                string sMsg = ToText(msg);
-
-                var lineText = $"[MtlsPop][{lvl}] {sTag} {sFunc}:{line}  {sMsg}";
+                var lineText = FormatNativeLog(tag, lvl, func, line, msg);
 
                 // Default: Trace (respects listeners; safe for all app types)
                 Trace.WriteLine(lineText);
@@ -30,6 +27,86 @@ internal static class AttestationLogger
             }
         };
 
+        /// <summary>
+        /// Builds a native log callback (<see cref="AttestationClientLib.LogFunc"/>) that forwards
+        /// <c>AttestationClientLib.dll</c> log lines into the MSAL <see cref="ILoggerAdapter"/> so that
+        /// native MAA diagnostics (e.g. policy-evaluation failures) appear in MSAL logs instead of only
+        /// going to <see cref="Trace"/>. When <paramref name="logger"/> is null, falls back to the
+        /// <see cref="ConsoleLogger"/> (Trace) so behavior is preserved for callers without a logger.
+        /// </summary>
+        /// <remarks>
+        /// The returned delegate must never throw: a managed exception escaping back into the native
+        /// caller would corrupt the interop boundary. All work is wrapped in a try/catch that swallows.
+        /// Callers must keep a strong reference to the returned delegate for as long as the native library
+        /// holds the function pointer (i.e. until <c>UninitAttestationLib</c>), otherwise the GC may collect
+        /// it and the native callback will crash.
+        /// </remarks>
+        internal static AttestationClientLib.LogFunc CreateLoggerBridge(ILoggerAdapter logger)
+        {
+            if (logger is null)
+            {
+                return ConsoleLogger;
+            }
+
+            return (ctx, tag, lvl, func, line, msg) =>
+            {
+                try
+                {
+                    LogLevel mapped = MapLevel(lvl);
+                    if (!logger.IsLoggingEnabled(mapped))
+                    {
+                        return;
+                    }
+
+                    string lineText = FormatNativeLog(tag, lvl, func, line, msg);
+
+                    // Native Error/Warning/Info lines (MAA policy-evaluation failures, endpoint
+                    // URLs, TPM cert issuer) are operational diagnostics, not user PII, so they are
+                    // logged as the scrubbed message and stay visible in standard MSAL logs. The
+                    // most verbose native output (Debug -> Verbose, and any unknown level) can carry
+                    // richer payload fragments, so it is routed through the PII slot and only
+                    // surfaces when the caller opts into PII logging.
+                    if (mapped == LogLevel.Verbose)
+                    {
+                        logger.Log(mapped, lineText, string.Empty);
+                    }
+                    else
+                    {
+                        logger.Log(mapped, string.Empty, lineText);
+                    }
+                }
+                catch
+                {
+                    // A logging callback must never throw back into native code.
+                }
+            };
+        }
+
+        /// <summary>
+        /// Maps a native <see cref="AttestationClientLib.LogLevel"/> to the MSAL <see cref="LogLevel"/>.
+        /// Unknown values map to <see cref="LogLevel.Verbose"/>.
+        /// </summary>
+        internal static LogLevel MapLevel(AttestationClientLib.LogLevel lvl) => lvl switch
+        {
+            AttestationClientLib.LogLevel.Error => LogLevel.Error,
+            AttestationClientLib.LogLevel.Warn => LogLevel.Warning,
+            AttestationClientLib.LogLevel.Info => LogLevel.Info,
+            AttestationClientLib.LogLevel.Debug => LogLevel.Verbose,
+            _ => LogLevel.Verbose,
+        };
+
+        /// <summary>
+        /// Formats a single native log line into a human-readable string, robust to null/IntPtr inputs.
+        /// </summary>
+        internal static string FormatNativeLog(string tag, AttestationClientLib.LogLevel lvl, string func, int line, string msg)
+        {
+            string sTag = ToText(tag);
+            string sFunc = ToText(func);
+            string sMsg = ToText(msg);
+
+            return $"[MtlsPop][AttestationClientLib][{lvl}] {sTag} {sFunc}:{line}  {sMsg}";
+        }
+
         // Converts either string or IntPtr (char*) to text. Works with any LogFunc variant.
         private static string ToText(object value)
         {

src/client/Microsoft.Identity.Client.KeyAttestation/ManagedIdentityAttestationExtensions.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Threading.Tasks;
 using Microsoft.Identity.Client.Core;
+using Microsoft.Identity.Client.KeyAttestation.Attestation;
 
 namespace Microsoft.Identity.Client.KeyAttestation
 {
@@ -12,6 +13,11 @@ namespace Microsoft.Identity.Client.KeyAttestation
     /// </summary>
     public static class ManagedIdentityAttestationExtensions
     {
+        // Error code surfaced when Credential Guard / KeyGuard attestation is requested but fails.
+        // The failure originates from the attestation (MAA) service, so it is surfaced as a service error.
+        // Matches the error code used by the IMDSv2 consumer so callers see a single, consistent code.
+        private const string AttestationFailedErrorCode = "attestation_failed";
+
         /// <summary>
         /// Enables Credential Guard attestation support for managed identity mTLS Proof-of-Possession flows.
         /// This method should be called after <see cref="ManagedIdentityPopExtensions.WithMtlsProofOfPossession"/>.
@@ -28,16 +34,31 @@ public static AcquireTokenForManagedIdentityParameterBuilder WithAttestationSupp
 
             builder.CommonParameters.AttestationTokenProvider = async (endpoint, keyHandle, clientId, keyId, logger, ct) =>
             {
-                var result = await PopKeyAttestor.AttestCredentialGuardAsync(
+                AttestationResult result = await PopKeyAttestor.AttestCredentialGuardAsync(
                     endpoint,
                     keyHandle,
                     clientId,
                     keyId,
                     logger,
                     ct).ConfigureAwait(false);
 
-                // Return JWT on success, null for non-attested flow on failure
-                return result.Status == Attestation.AttestationStatus.Success ? result.Jwt : null;
+                if (result.Status == AttestationStatus.Success && !string.IsNullOrWhiteSpace(result.Jwt))
+                {
+                    return result.Jwt;
+                }
+
+                // Attestation failed. Surface the reason to the caller instead of returning null —
+                // returning null would cause an empty/non-attested certificate request to be sent to
+                // IMDS, silently dropping the real failure (e.g. an MAA policy-evaluation deny). The
+                // failure originates from the attestation (MAA) service, so it is a service exception.
+                string reason = string.IsNullOrEmpty(result.ErrorMessage)
+                    ? "(no additional detail available)"
+                    : result.ErrorMessage;
+
+                throw new MsalServiceException(
+                    AttestationFailedErrorCode,
+                    $"Key Guard attestation failed; no attestation token was produced. " +
+                    $"Status: {result.Status}, NativeErrorCode: {result.NativeErrorCode}, Reason: {reason}");
             };
 
             return builder;

src/client/Microsoft.Identity.Client.KeyAttestation/PopKeyAttestor.cs

@@ -138,7 +138,7 @@ public static async Task<AttestationResult> AttestCredentialGuardAsync(
                     {
                         try
                         {
-                            using var client = new AttestationClient();
+                            using var client = new AttestationClient(logger);
                             return client.Attest(endpoint, safeNCryptKeyHandle, clientId ?? string.Empty);
                         }
                         catch (Exception ex)
@@ -183,7 +183,7 @@ public static async Task<AttestationResult> AttestCredentialGuardAsync(
                     {
                         try
                         {
-                            using var client = new AttestationClient();
+                            using var client = new AttestationClient(logger);
                             return client.Attest(endpoint, safeNCryptKeyHandle, clientId ?? string.Empty);
                         }
                         catch (Exception ex)

src/client/Microsoft.Identity.Client/ManagedIdentity/V2/ImdsV2ManagedIdentitySource.cs

@@ -1,4 +1,4 @@
-// Copyright (c) Microsoft Corporation. All rights reserved.
+// Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
 using System;
@@ -490,18 +490,41 @@ private async Task<string> GetAttestationJwtAsync(
 
                 if (string.IsNullOrWhiteSpace(attestationJwt))
                 {
-                    _requestContext.Logger.Info("[ImdsV2] Attestation provider returned null/empty JWT. Proceeding with non-attested flow.");
-                    return null;
+                    // A provider was configured (we passed the _attestationTokenProvider null-check above)
+                    // and this is a KeyGuard key, for which attestation is mandatory. A null/empty token
+                    // means attestation did not succeed — refuse to fall back to a non-attested request,
+                    // which would silently send an empty attestation token to IMDS.
+                    throw MsalServiceExceptionFactory.CreateManagedIdentityException(
+                        "attestation_failed",
+                        "[ImdsV2] Attestation was required for this KeyGuard key but the attestation provider " +
+                        "returned no token. Refusing to send a non-attested certificate request to IMDS.",
+                        null,
+                        ManagedIdentitySource.Imds,
+                        null);
                 }
 
                 return attestationJwt;
             }
+            catch (MsalServiceException)
+            {
+                // Already a well-formed attestation service failure (from the provider or the check above) —
+                // propagate as-is rather than double-wrapping.
+                throw;
+            }
+            catch (OperationCanceledException)
+            {
+                // Cancellation (including TaskCanceledException) must propagate unchanged rather than
+                // being masked as an attestation_failed service error.
+                throw;
+            }
             catch (Exception ex)
             {
-                throw new MsalClientException(
+                throw MsalServiceExceptionFactory.CreateManagedIdentityException(
                     "attestation_failed",
                     $"[ImdsV2] Attestation token provider failed: {ex.Message}",
-                    ex);
+                    ex,
+                    ManagedIdentitySource.Imds,
+                    null);
             }
         }