cert rotation (#1192)

* initial commit cert rotation...mainly for testing * Jmprieur/cert rot (#1181) * Small tweaks * Retrying when a certificate expires - without an infinite loop - in all the requests to TokenAcquisition Improving the exceptions * PR feedback (#1184) * PR feedback * update argument exception Co-authored-by: jennyf19 <[email protected]> * Addressing PR feedback. Re-tested Co-authored-by: Jean-Marc Prieur <[email protected]>
AzureAD · May 13, 2021 · a27fc44 · a27fc44
1 parent 330eb11
commit a27fc44
Show file tree

Hide file tree

Showing 5 changed files with 98 additions and 11 deletions.
diff --git a/src/Microsoft.Identity.Web/CertificateManagement/DefaultCertificateLoader.cs b/src/Microsoft.Identity.Web/CertificateManagement/DefaultCertificateLoader.cs
@@ -102,7 +102,7 @@ private static X509Certificate2 LoadFromBase64Encoded(string certificateBase64,
         /// <remarks>This code is inspired by Heath Stewart's code in:
         /// https://github.com/heaths/azsdk-sample-getcert/blob/master/Program.cs#L46-L82.
         /// </remarks>
-        private static X509Certificate2 LoadFromKeyVault(
+        private static X509Certificate2? LoadFromKeyVault(
             string keyVaultUrl,
             string certificateName,
             X509KeyStorageFlags x509KeyStorageFlags)
@@ -116,6 +116,16 @@ private static X509Certificate2 LoadFromKeyVault(
 
             KeyVaultCertificateWithPolicy certificate = certificateClient.GetCertificate(certificateName);
 
+            if (certificate.Properties.NotBefore == null || certificate.Properties.ExpiresOn == null)
+            {
+                return null;
+            }
+
+            if (DateTimeOffset.UtcNow < certificate.Properties.NotBefore || DateTimeOffset.UtcNow > certificate.Properties.ExpiresOn)
+            {
+                return null;
+            }
+
             // Return a certificate with only the public key if the private key is not exportable.
             if (certificate.Policy?.Exportable != true)
             {
@@ -205,6 +215,17 @@ private static X509Certificate2 LoadFromKeyVault(
             return cert;
         }
 
+        internal static void ResetCertificates(IEnumerable<CertificateDescription>? clientCertificates)
+        {
+            if (clientCertificates != null)
+            {
+                foreach (var cert in clientCertificates)
+                {
+                    cert.Certificate = null;
+                }
+            }
+        }
+
         private static X509Certificate2 LoadFromPath(
             string certificateFileName,
             string? password = null)
@@ -266,9 +287,13 @@ private static void ParseStoreLocationAndName(
         internal /*for test only*/ static X509Certificate2? LoadFirstCertificate(IEnumerable<CertificateDescription> certificateDescription)
         {
             DefaultCertificateLoader defaultCertificateLoader = new DefaultCertificateLoader();
-            CertificateDescription certDescription = certificateDescription.First();
-            defaultCertificateLoader.LoadIfNeeded(certDescription);
-            return certDescription.Certificate;
+            CertificateDescription? certDescription = certificateDescription.FirstOrDefault(c =>
+            {
+                defaultCertificateLoader.LoadIfNeeded(c);
+                return c.Certificate != null;
+            });
+
+            return certDescription?.Certificate;
         }
 
         internal /*for test only*/ static IEnumerable<X509Certificate2?> LoadAllCertificates(IEnumerable<CertificateDescription> certificateDescriptions)

diff --git a/src/Microsoft.Identity.Web/Constants/Constants.cs b/src/Microsoft.Identity.Web/Constants/Constants.cs
@@ -103,6 +103,8 @@ public static class Constants
         internal const string ApplicationJson = "application/json";
         internal const string ISessionStore = "ISessionStore";
         internal const string True = "True";
+        internal const string InvalidClient = "invalid_client";
+        internal const string InvalidKeyError = "AADSTS700027";
 
         // Blazor challenge URI
         internal const string BlazorChallengeUri = "MicrosoftIdentity/Account/Challenge?redirectUri=";

diff --git a/src/Microsoft.Identity.Web/Constants/IDWebErrorMessage.cs b/src/Microsoft.Identity.Web/Constants/IDWebErrorMessage.cs
@@ -25,6 +25,7 @@ internal static class IDWebErrorMessage
         public const string ConfigurationOptionRequired = "IDW10106: The '{0}' option must be provided. ";
         public const string ScopesNotConfiguredInConfigurationOrViaDelegate = "IDW10107: Scopes need to be passed-in either by configuration or by the delegate overriding it. ";
         public const string MissingRequiredScopesForAuthorizationFilter = "IDW10108: RequiredScope Attribute does not contain a value. The scopes need to be set on the controller, the page or action. See https://aka.ms/ms-id-web/required-scope-attribute. ";
+        public const string ClientCertificatesHaveExpiredOrCannotBeLoaded = "IDW10109: All client certificates passed to the configuration have expired or can't be loaded. ";
 
         // Authorization IDW10200 = "IDW10200:"
         public const string NeitherScopeOrRolesClaimFoundInToken = "IDW10201: Neither scope or roles claim was found in the bearer token. ";

diff --git a/src/Microsoft.Identity.Web/TokenAcquisition.Logger.cs b/src/Microsoft.Identity.Web/TokenAcquisition.Logger.cs
@@ -13,11 +13,11 @@ internal partial class TokenAcquisition
     {
         private static class Logger
         {
-            private static readonly Action<ILogger, string, Exception> s_tokenAcquisitionError =
-           LoggerMessage.Define<string>(
-               LogLevel.Information,
-               LoggingEventId.TokenAcquisitionError,
-               "[MsIdWeb] An error occured during token acquisition: {MsalErrorMessage}");
+            private static readonly Action<ILogger, string, Exception?> s_tokenAcquisitionError =
+                LoggerMessage.Define<string>(
+                    LogLevel.Information,
+                    LoggingEventId.TokenAcquisitionError,
+                    "[MsIdWeb] An error occured during token acquisition: {MsalErrorMessage}");
 
             /// <summary>
             /// Logger for handling MSAL exceptions in TokenAcquisition.
@@ -28,7 +28,7 @@ private static class Logger
             public static void TokenAcquisitionError(
                 ILogger logger,
                 string msalErrorMessage,
-                Exception ex) => s_tokenAcquisitionError(logger, msalErrorMessage, ex);
+                Exception? ex) => s_tokenAcquisitionError(logger, msalErrorMessage, ex);
         }
     }
 }
diff --git a/src/Microsoft.Identity.Web/TokenAcquisition.cs b/src/Microsoft.Identity.Web/TokenAcquisition.cs
@@ -40,6 +40,7 @@ internal partial class TokenAcquisition : ITokenAcquisitionInternal
         ///  Please call GetOrBuildConfidentialClientApplication instead of accessing this field directly.
         /// </summary>
         private IConfidentialClientApplication? _application;
+        private bool retryClientCertificate;
         private readonly IHttpContextAccessor _httpContextAccessor;
         private HttpContext? CurrentHttpContext => _httpContextAccessor.HttpContext;
         private readonly IMsalHttpClientFactory _httpClientFactory;
@@ -173,11 +174,24 @@ public async Task AddAccountToCacheFromAuthorizationCodeAsync(
 
                 context.HandleCodeRedemption(null, result.IdToken);
             }
+            catch (MsalServiceException exMsal) when (IsInvalidClientCertificateError(exMsal))
+            {
+                DefaultCertificateLoader.ResetCertificates(_microsoftIdentityOptions.ClientCertificates);
+                _application = null;
+
+                // Retry
+                retryClientCertificate = true;
+                await AddAccountToCacheFromAuthorizationCodeAsync(context, scopes).ConfigureAwait(false);
+            }
             catch (MsalException ex)
             {
                 Logger.TokenAcquisitionError(_logger, LogMessages.ExceptionOccurredWhenAddingAnAccountToTheCacheFromAuthCode, ex);
                 throw;
             }
+            finally
+            {
+                retryClientCertificate = false;
+            }
         }
 
         /// <summary>
@@ -243,6 +257,15 @@ public async Task<AuthenticationResult> GetAuthenticationResultForUserAsync(
                      userFlow)
                      .ConfigureAwait(false);
             }
+            catch (MsalServiceException exMsal) when (IsInvalidClientCertificateError(exMsal))
+            {
+                DefaultCertificateLoader.ResetCertificates(_microsoftIdentityOptions.ClientCertificates);
+                _application = null;
+
+                // Retry
+                retryClientCertificate = true;
+                return await GetAuthenticationResultForUserAsync(scopes, tenantId, userFlow, user, tokenAcquisitionOptions).ConfigureAwait(false);
+            }
             catch (MsalUiRequiredException ex)
             {
                 // GetAccessTokenForUserAsync is an abstraction that can be called from a web app or a web API
@@ -252,6 +275,10 @@ public async Task<AuthenticationResult> GetAuthenticationResultForUserAsync(
                 // AuthorizeForScopesAttribute exception filter so that the user can consent, do 2FA, etc ...
                 throw new MicrosoftIdentityWebChallengeUserException(ex, scopes.ToArray(), userFlow);
             }
+            finally
+            {
+                retryClientCertificate = false;
+            }
         }
 
         /// <summary>
@@ -313,7 +340,23 @@ public Task<AuthenticationResult> GetAuthenticationResultForAppAsync(
                 }
             }
 
-            return builder.ExecuteAsync();
+            try
+            {
+                return builder.ExecuteAsync();
+            }
+            catch (MsalServiceException exMsal) when (IsInvalidClientCertificateError(exMsal))
+            {
+                DefaultCertificateLoader.ResetCertificates(_microsoftIdentityOptions.ClientCertificates);
+                _application = null;
+
+                // Retry
+                retryClientCertificate = true;
+                return GetAuthenticationResultForAppAsync(scope, tenant, tokenAcquisitionOptions);
+            }
+            finally
+            {
+                retryClientCertificate = false;
+            }
         }
 
         /// <summary>
@@ -475,6 +518,11 @@ public async Task RemoveAccountAsync(RedirectContext context)
             }
         }
 
+        private bool IsInvalidClientCertificateError(MsalServiceException exMsal)
+        {
+            return !retryClientCertificate && exMsal.ErrorCode == Constants.InvalidClient && exMsal.Message.Contains(Constants.InvalidKeyError);
+        }
+
         private string BuildCurrentUriFromRequest(HttpContext httpContext, HttpRequest request)
         {
             // need to lock to avoid threading issues with code outside of this library
@@ -563,6 +611,17 @@ private IConfidentialClientApplication BuildConfidentialClientApplication()
                 if (_microsoftIdentityOptions.ClientCertificates != null)
                 {
                     X509Certificate2? certificate = DefaultCertificateLoader.LoadFirstCertificate(_microsoftIdentityOptions.ClientCertificates);
+                    if (certificate == null)
+                    {
+                        Logger.TokenAcquisitionError(
+                            _logger,
+                            IDWebErrorMessage.ClientCertificatesHaveExpiredOrCannotBeLoaded,
+                            null);
+                        throw new ArgumentException(
+                            IDWebErrorMessage.ClientCertificatesHaveExpiredOrCannotBeLoaded,
+                            nameof(_microsoftIdentityOptions.ClientCertificates));
+                    }
+
                     builder.WithCertificate(certificate);
                 }