Store Secrets to Encrypted Storage (#19929)

* Store secrets to encrypted storage * Uses unprotected files in Linux * Update Changelog.md * Fix the pipeline issue * Update src/Accounts/Authentication/KeyStore/AzKeyStore.cs Co-authored-by: Beisi Zhou <[email protected]> Co-authored-by: Beisi Zhou <[email protected]>
Azure · Nov 24, 2022 · e692a81 · e692a81
1 parent 49564bd
commit e692a81
Show file tree

Hide file tree

Showing 20 changed files with 733 additions and 37 deletions.
diff --git a/src/Accounts/Accounts.Test/AutosaveTests.cs b/src/Accounts/Accounts.Test/AutosaveTests.cs
@@ -22,23 +22,38 @@
 using Xunit.Abstractions;
 using Microsoft.Azure.Commands.Common.Authentication.Abstractions;
 using System;
+using System.Security;
 using Microsoft.Azure.Commands.Profile.Context;
 using Microsoft.Azure.Commands.ScenarioTest;
 using Microsoft.Azure.Commands.ResourceManager.Common;
 using Microsoft.Azure.Commands.TestFx.Mocks;
+using Moq;
 
 namespace Microsoft.Azure.Commands.Profile.Test
 {
     public class AutosaveTests
     {
         private MemoryDataStore dataStore;
         private MockCommandRuntime commandRuntimeMock;
+        private AzKeyStore keyStore;
         public AutosaveTests(ITestOutputHelper output)
         {
             XunitTracingInterceptor.AddToContext(new XunitTracingInterceptor(output));
             commandRuntimeMock = new MockCommandRuntime();
             dataStore = new MemoryDataStore();
-            ResetState();
+            keyStore = SetMockedAzKeyStore();
+        }
+
+        private AzKeyStore SetMockedAzKeyStore()
+        {
+            var storageMocker = new Mock<IStorage>();
+            storageMocker.Setup(f => f.Create()).Returns(storageMocker.Object);
+            storageMocker.Setup(f => f.ReadData()).Returns(new byte[0]);
+            storageMocker.Setup(f => f.WriteData(It.IsAny<byte[]>())).Callback((byte[] s) => {});
+            var keyStore = new AzKeyStore(AzureSession.Instance.ARMProfileDirectory, "keystore.cache", false, false, storageMocker.Object);
+            AzKeyStore.RegisterJsonConverter(typeof(ServicePrincipalKey), typeof(ServicePrincipalKey).Name);
+            AzKeyStore.RegisterJsonConverter(typeof(SecureString), typeof(SecureString).Name, new SecureStringConverter());
+            return keyStore;
         }
 
         void ResetState()
@@ -54,6 +69,7 @@ void ResetState()
             Environment.SetEnvironmentVariable("Azure_PS_Data_Collection", "false");
             PowerShellTokenCacheProvider tokenProvider = new InMemoryTokenCacheProvider();
             AzureSession.Instance.RegisterComponent(PowerShellTokenCacheProvider.PowerShellTokenCacheProviderKey, () => tokenProvider, true);
+            AzureSession.Instance.RegisterComponent(AzKeyStore.Name, () => keyStore, true);
         }
 
         [Fact]

diff --git a/src/Accounts/Accounts.Test/ProfileCmdletTests.cs b/src/Accounts/Accounts.Test/ProfileCmdletTests.cs
@@ -15,13 +15,15 @@
 using Microsoft.Azure.Commands.Common.Authentication;
 using Microsoft.Azure.Commands.Common.Authentication.Abstractions;
 using Microsoft.Azure.Commands.Common.Authentication.Models;
+using Microsoft.Azure.Commands.ResourceManager.Common;
 using Microsoft.Azure.Commands.ScenarioTest;
 using Microsoft.Azure.Commands.TestFx.Mocks;
 using Microsoft.Azure.ServiceManagement.Common.Models;
 using Microsoft.WindowsAzure.Commands.Common.Test.Mocks;
 using Microsoft.WindowsAzure.Commands.ScenarioTest;
 using Microsoft.WindowsAzure.Commands.Test.Utilities.Common;
 using Microsoft.WindowsAzure.Commands.Utilities.Common;
+using Moq;
 using System;
 using System.Linq;
 using System.Management.Automation;
@@ -34,6 +36,7 @@ public class ProfileCmdletTests : RMTestBase
     {
         private MemoryDataStore dataStore;
         private MockCommandRuntime commandRuntimeMock;
+        private AzKeyStore keyStore;
 
         public ProfileCmdletTests(ITestOutputHelper output)
         {
@@ -43,12 +46,25 @@ public ProfileCmdletTests(ITestOutputHelper output)
             AzureSession.Instance.DataStore = dataStore;
             commandRuntimeMock = new MockCommandRuntime();
             AzureSession.Instance.AuthenticationFactory = new MockTokenAuthenticationFactory();
+            keyStore = SetMockedAzKeyStore();
+        }
+
+        private AzKeyStore SetMockedAzKeyStore()
+        {
+            var storageMocker = new Mock<IStorage>();
+            storageMocker.Setup(f => f.Create()).Returns(storageMocker.Object);
+            storageMocker.Setup(f => f.ReadData()).Returns(new byte[0]);
+            storageMocker.Setup(f => f.WriteData(It.IsAny<byte[]>())).Callback((byte[] s) => { });
+            var keyStore = new AzKeyStore(AzureSession.Instance.ARMProfileDirectory, "keystore.cache", false, false, storageMocker.Object);
+            return keyStore;
         }
 
         [Fact]
         [Trait(Category.AcceptanceType, Category.CheckIn)]
         public void SelectAzureProfileInMemory()
         {
+            AzureSession.Instance.RegisterComponent(AzKeyStore.Name, () => keyStore, true);
+
             var profile = new AzureRmProfile { DefaultContext = new AzureContext() };
             var env = new AzureEnvironment(AzureEnvironment.PublicEnvironments.Values.FirstOrDefault());
             env.Name = "foo";
@@ -71,6 +87,7 @@ public void SelectAzureProfileInMemory()
         [Trait(Category.AcceptanceType, Category.CheckIn)]
         public void SelectAzureProfileBadPath()
         {
+            AzureSession.Instance.RegisterComponent(AzKeyStore.Name, () => keyStore, true);
 #pragma warning disable CS0618 // Suppress obsolescence warning: cmdlet name is changing
             ImportAzureRMContextCommand cmdlt = new ImportAzureRMContextCommand();
 #pragma warning restore CS0618 // Suppress obsolescence warning: cmdlet name is changing
@@ -88,6 +105,7 @@ public void SelectAzureProfileBadPath()
         [Trait(Category.AcceptanceType, Category.CheckIn)]
         public void SelectAzureProfileFromDisk()
         {
+            AzureSession.Instance.RegisterComponent(AzKeyStore.Name, () => keyStore, true);
             var profile = new AzureRmProfile();
             profile.EnvironmentTable.Add("foo", new AzureEnvironment(new AzureEnvironment( AzureEnvironment.PublicEnvironments.Values.FirstOrDefault())));
             profile.EnvironmentTable["foo"].Name = "foo";
@@ -110,6 +128,7 @@ public void SelectAzureProfileFromDisk()
         [Trait(Category.AcceptanceType, Category.CheckIn)]
         public void SaveAzureProfileInMemory()
         {
+            AzureSession.Instance.RegisterComponent(AzKeyStore.Name, () => keyStore, true);
             var profile = new AzureRmProfile();
             profile.EnvironmentTable.Add("foo", new AzureEnvironment(AzureEnvironment.PublicEnvironments.Values.FirstOrDefault()));
             profile.EnvironmentTable["foo"].Name = "foo";
@@ -134,6 +153,7 @@ public void SaveAzureProfileInMemory()
         [Trait(Category.AcceptanceType, Category.CheckIn)]
         public void SaveAzureProfileNull()
         {
+            AzureSession.Instance.RegisterComponent(AzKeyStore.Name, () => keyStore, true);
 #pragma warning disable CS0618 // Suppress obsolescence warning: cmdlet name is changing
             SaveAzureRMContextCommand cmdlt = new SaveAzureRMContextCommand();
 #pragma warning restore CS0618 // Suppress obsolescence warning: cmdlet name is changing
@@ -150,6 +170,7 @@ public void SaveAzureProfileNull()
         [Trait(Category.AcceptanceType, Category.CheckIn)]
         public void SaveAzureProfileFromDefault()
         {
+            AzureSession.Instance.RegisterComponent(AzKeyStore.Name, () => keyStore, true);
             var profile = new AzureRmProfile();
             profile.EnvironmentTable.Add("foo", new AzureEnvironment(AzureEnvironment.PublicEnvironments.Values.FirstOrDefault()));
             profile.DefaultContext = new AzureContext(new AzureSubscription(), new AzureAccount(), profile.EnvironmentTable["foo"]);

diff --git a/src/Accounts/Accounts/Account/ConnectAzureRmAccount.cs b/src/Accounts/Accounts/Account/ConnectAzureRmAccount.cs
@@ -41,7 +41,6 @@
 using Microsoft.Azure.PowerShell.Common.Config;
 using Microsoft.Identity.Client;
 using Microsoft.WindowsAzure.Commands.Common;
-using Microsoft.WindowsAzure.Commands.Common.CustomAttributes;
 using Microsoft.WindowsAzure.Commands.Common.Utilities;
 using Microsoft.WindowsAzure.Commands.Utilities.Common;
 using Microsoft.Azure.PowerShell.Common.Share.Survey;
@@ -426,7 +425,6 @@ public override void ExecuteCmdlet()
                 azureAccount.SetProperty(AzureAccount.Property.CertificatePath, resolvedPath);
                 if (CertificatePassword != null)
                 {
-                    azureAccount.SetProperty(AzureAccount.Property.CertificatePassword, CertificatePassword.ConvertToString());
                     keyStore?.SaveKey(new ServicePrincipalKey(AzureAccount.Property.CertificatePassword, azureAccount.Id, Tenant), CertificatePassword);
                 }
             }
@@ -449,7 +447,6 @@ public override void ExecuteCmdlet()
 
             if (azureAccount.Type == AzureAccount.AccountType.ServicePrincipal && password != null)
             {
-                azureAccount.SetProperty(AzureAccount.Property.ServicePrincipalSecret, password.ConvertToString());
                 keyStore?.SaveKey(new ServicePrincipalKey(AzureAccount.Property.ServicePrincipalSecret
                     ,azureAccount.Id, Tenant), password);
                 if (GetContextModificationScope() == ContextModificationScope.CurrentUser)
@@ -713,16 +710,23 @@ public void OnImport()
                     WriteInitializationWarnings(Resources.FallbackContextSaveModeDueCacheCheckError.FormatInvariant(ex.Message));
                 }
 
-                if(!InitializeProfileProvider(autoSaveEnabled))
+                AzKeyStore keyStore = null;
+                //AzureSession.Instance.KeyStoreFile
+                keyStore = new AzKeyStore(AzureSession.Instance.ARMProfileDirectory, "keystore.cache", false, autoSaveEnabled);
+                AzKeyStore.RegisterJsonConverter(typeof(ServicePrincipalKey), typeof(ServicePrincipalKey).Name);
+                AzKeyStore.RegisterJsonConverter(typeof(SecureString), typeof(SecureString).Name, new SecureStringConverter());
+                AzureSession.Instance.RegisterComponent(AzKeyStore.Name, () => keyStore);
+
+                if (!InitializeProfileProvider(autoSaveEnabled))
                 {
                     AzureSession.Instance.ARMContextSaveMode = ContextSaveMode.Process;
                     autoSaveEnabled = false;
                 }
 
-#pragma warning disable CS0618 // Type or member is obsolete
-                var keyStore = new AzKeyStore(AzureRmProfileProvider.Instance.Profile);
-#pragma warning restore CS0618 // Type or member is obsolete
-                AzureSession.Instance.RegisterComponent(AzKeyStore.Name, () => keyStore);
+                if (!keyStore.LoadStorage())
+                {
+                    WriteInitializationWarnings(Resources.KeyStoreLoadingError);
+                }
 
                 IAuthenticatorBuilder builder = null;
                 if (!AzureSession.Instance.TryGetComponent(AuthenticatorBuilder.AuthenticatorBuilderKey, out builder))

diff --git a/src/Accounts/Accounts/AutoSave/DisableAzureRmContextAutosave.cs b/src/Accounts/Accounts/AutoSave/DisableAzureRmContextAutosave.cs
@@ -92,6 +92,11 @@ void DisableAutosave(IAzureSession session, bool writeAutoSaveFile, out ContextA
                 builder.Reset();
             }
 
+            if (AzureSession.Instance.TryGetComponent(AzKeyStore.Name, out AzKeyStore keystore))
+            {
+                keystore.DisableAutoSaving();
+            }
+
             if (writeAutoSaveFile)
             {
                 FileUtilities.EnsureDirectoryExists(session.ProfileDirectory);

diff --git a/src/Accounts/Accounts/AutoSave/EnableAzureRmContextAutosave.cs b/src/Accounts/Accounts/AutoSave/EnableAzureRmContextAutosave.cs
@@ -102,6 +102,12 @@ void EnableAutosave(IAzureSession session, bool writeAutoSaveFile, out ContextAu
                 AzureSession.Instance.RegisterComponent(PowerShellTokenCacheProvider.PowerShellTokenCacheProviderKey, () => newCacheProvider, true);
             }
 
+            if (AzureSession.Instance.TryGetComponent(AzKeyStore.Name, out AzKeyStore keystore))
+            {
+                keystore.Flush();
+                keystore.DisableAutoSaving();
+            }
+
 
             if (writeAutoSaveFile)
             {

diff --git a/src/Accounts/Accounts/ChangeLog.md b/src/Accounts/Accounts/ChangeLog.md
@@ -21,6 +21,7 @@
 ## Upcoming Release
 * Enabled caching tokens when logging in with a service principal. This could reduce network traffic and improve performance.
 * Upgraded target framework of Microsoft.Identity.Client to net461 [#20189]
+* Stored `ServicePrincipalSecret` and `CertificatePassword` into `AzKeyStore`.
 
 ## Version 2.10.3
 * Updated `Get-AzSubscription` to retrieve subscription by Id rather than listed all the subscriptions from server if subscription Id is provided. [#19115]

diff --git a/src/Accounts/Accounts/Properties/Resources.Designer.cs b/src/Accounts/Accounts/Properties/Resources.Designer.cs
diff --git a/src/Accounts/Accounts/Properties/Resources.resx b/src/Accounts/Accounts/Properties/Resources.resx
@@ -571,4 +571,7 @@
     <value>The command {0} is part of Azure PowerShell module "{1}" and it is not installed. Run "Install-Module {1}" to install it.</value>
     <comment>0: command being not found; 1: its module</comment>
   </data>
+  <data name="KeyStoreLoadingError" xml:space="preserve">
+    <value>KeyStore cannot be loaded from storage. Please check the keystore file integrity or system compablity. The functions relate to context autosaving may be affected.</value>
+  </data>
 </root>
diff --git a/src/Accounts/Authentication.ResourceManager/AzureRmProfile.cs b/src/Accounts/Authentication.ResourceManager/AzureRmProfile.cs
@@ -25,6 +25,7 @@
 using Microsoft.Azure.Commands.Common.Authentication.ResourceManager.Properties;
 using Microsoft.Azure.Commands.ResourceManager.Common;
 using Microsoft.Azure.Commands.ResourceManager.Common.Serialization;
+using Microsoft.WindowsAzure.Commands.Common;
 
 using Newtonsoft.Json;
 
@@ -205,15 +206,39 @@ private void Initialize(AzureRmProfile profile)
                     EnvironmentTable[environment.Key] = environment.Value;
                 }
 
+                AzKeyStore keystore = null;
+                AzureSession.Instance.TryGetComponent(AzKeyStore.Name, out keystore);
+
                 foreach (var context in profile.Contexts)
                 {
-                    this.Contexts.Add(context.Key, context.Value);
+                    this.Contexts.Add(context.Key, MigrateSecretToKeyStore(context.Value, keystore));
                 }
 
                 DefaultContextKey = profile.DefaultContextKey ?? (profile.Contexts.Any() ? null : "Default");
             }
         }
 
+        private IAzureContext MigrateSecretToKeyStore(IAzureContext context, AzKeyStore keystore)
+        {
+            if (keystore != null)
+            {
+                var account = context.Account;
+                if (account.IsPropertySet(AzureAccount.Property.ServicePrincipalSecret))
+                {
+                    keystore?.SaveKey(new ServicePrincipalKey(AzureAccount.Property.ServicePrincipalSecret, account.Id, account.GetTenants().First())
+                        , account.ExtendedProperties.GetProperty(AzureAccount.Property.ServicePrincipalSecret).ConvertToSecureString());
+                    account.ExtendedProperties.Remove(AzureAccount.Property.ServicePrincipalSecret);
+                }
+                if (account.IsPropertySet(AzureAccount.Property.CertificatePassword))
+                {
+                    keystore?.SaveKey(new ServicePrincipalKey(AzureAccount.Property.CertificatePassword, account.Id, account.GetTenants().First())
+    , account.ExtendedProperties.GetProperty(AzureAccount.Property.CertificatePassword).ConvertToSecureString());
+                    account.ExtendedProperties.Remove(AzureAccount.Property.CertificatePassword);
+                }
+            }
+            return context;
+        }
+
         private void LoadImpl(string contents)
         {
         }
@@ -311,6 +336,10 @@ public void Save(IFileProvider provider, bool serializeCache = true)
                     // so that previous data is overwritten
                     provider.Stream.SetLength(provider.Stream.Position);
                 }
+
+                AzKeyStore keystore = null;
+                AzureSession.Instance.TryGetComponent(AzKeyStore.Name, out keystore);
+                keystore?.Flush();
             }
             finally
             {