[BugFix] Enable AzKeyStore with keyring in Linux (Azure#20341)

msJinLei · web-flow · commit d687600213b5 · 2022-12-08T02:20:53.000+08:00
* Enable AzKeyStore with keyring in Linux (Azure#20296) * Enable AzKeystore with keyring in Linux * Optimize the warning message for customers * Update common library version. * Address review comments
diff --git a/src/Accounts/Accounts.Test/AutosaveTests.cs b/src/Accounts/Accounts.Test/AutosaveTests.cs
@@ -51,7 +51,7 @@ private AzKeyStore SetMockedAzKeyStore()
             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);
+            var keyStore = new AzKeyStore(AzureSession.Instance.ARMProfileDirectory, "azkeystore", false, false, storageMocker.Object);
             AzKeyStore.RegisterJsonConverter(typeof(ServicePrincipalKey), typeof(ServicePrincipalKey).Name);
             AzKeyStore.RegisterJsonConverter(typeof(SecureString), typeof(SecureString).Name, new SecureStringConverter());
             return keyStore;
diff --git a/src/Accounts/Accounts.Test/ProfileCmdletTests.cs b/src/Accounts/Accounts.Test/ProfileCmdletTests.cs
@@ -55,7 +55,7 @@ private AzKeyStore SetMockedAzKeyStore()
             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);
+            var keyStore = new AzKeyStore(AzureSession.Instance.ARMProfileDirectory, "azkeystore", false, false, storageMocker.Object);
             return keyStore;
         }
 
diff --git a/src/Accounts/Accounts/Account/ConnectAzureRmAccount.cs b/src/Accounts/Accounts/Account/ConnectAzureRmAccount.cs
@@ -426,6 +426,10 @@ public override void ExecuteCmdlet()
                 if (CertificatePassword != null)
                 {
                     keyStore?.SaveKey(new ServicePrincipalKey(AzureAccount.Property.CertificatePassword, azureAccount.Id, Tenant), CertificatePassword);
+                    if (GetContextModificationScope() == ContextModificationScope.CurrentUser && !keyStore.IsProtected)
+                    {
+                        WriteWarning(string.Format(Resources.ServicePrincipalWarning, AzureSession.Instance.KeyStoreFile, AzureSession.Instance.ARMProfileDirectory));
+                    }
                 }
             }
 
@@ -449,11 +453,9 @@ public override void ExecuteCmdlet()
             {
                 keyStore?.SaveKey(new ServicePrincipalKey(AzureAccount.Property.ServicePrincipalSecret
                     ,azureAccount.Id, Tenant), password);
-                if (GetContextModificationScope() == ContextModificationScope.CurrentUser)
+                if (GetContextModificationScope() == ContextModificationScope.CurrentUser && !keyStore.IsProtected)
                 {
-                    var file = AzureSession.Instance.ARMProfileFile;
-                    var directory = AzureSession.Instance.ARMProfileDirectory;
-                    WriteWarning(string.Format(Resources.ServicePrincipalWarning, file, directory));
+                    WriteWarning(string.Format(Resources.ServicePrincipalWarning, AzureSession.Instance.KeyStoreFile, AzureSession.Instance.ARMProfileDirectory));
                 }
             }
             if (azureAccount.Type == "ClientAssertion" && FederatedToken != null)
@@ -711,8 +713,7 @@ public void OnImport()
                 }
 
                 AzKeyStore keyStore = null;
-                //AzureSession.Instance.KeyStoreFile
-                keyStore = new AzKeyStore(AzureSession.Instance.ARMProfileDirectory, "keystore.cache", false, autoSaveEnabled);
+                keyStore = new AzKeyStore(AzureSession.Instance.ARMProfileDirectory, AzureSession.Instance.KeyStoreFile, false, autoSaveEnabled);
                 AzKeyStore.RegisterJsonConverter(typeof(ServicePrincipalKey), typeof(ServicePrincipalKey).Name);
                 AzKeyStore.RegisterJsonConverter(typeof(SecureString), typeof(SecureString).Name, new SecureStringConverter());
                 AzureSession.Instance.RegisterComponent(AzKeyStore.Name, () => keyStore);
diff --git a/src/Accounts/Accounts/ChangeLog.md b/src/Accounts/Accounts/ChangeLog.md
@@ -19,6 +19,7 @@
 -->
 
 ## Upcoming Release
+* Enabled AzKeyStore with keyring in Linux.
 
 ## Version 2.10.4
 * Enabled caching tokens when logging in with a client assertion. This fixed the incorrectly short lifespan of tokens.
diff --git a/src/Accounts/Accounts/Context/ClearAzureRmContext.cs b/src/Accounts/Accounts/Context/ClearAzureRmContext.cs
@@ -83,6 +83,11 @@ void ClearContext(AzureRmProfile profile, RMProfileClient client)
                     profile.TrySetDefaultContext(defaultContext);
                     result = true;
                 }
+                if (AzureSession.Instance.TryGetComponent(AzKeyStore.Name, out AzKeyStore keyStore))
+                {
+                    keyStore?.Clear();
+                }
+
             }
 
             AzureSession.Instance.RaiseContextClearedEvent();
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
@@ -424,7 +424,7 @@
     <value>Unable to set profile because environment variable '${0}' is null.</value>
   </data>
   <data name="ServicePrincipalWarning" xml:space="preserve">
-    <value>The provided service principal secret will be included in the '{0}' file found in the user profile ( {1} ). Please ensure that this directory has appropriate protections.</value>
+    <value>The provided service principal secret or certifcate password will be included in the '{0}' file found in the user profile ( {1} ). Please ensure that this directory has appropriate protections.</value>
   </data>
   <data name="ClientAssertionWarning" xml:space="preserve">
     <value>The provided client id and assertion will be included in the '{0}' file found in the user profile ( {1} ). Please ensure that this directory has appropriate protections.</value>
diff --git a/src/Accounts/Authentication.Test/AzKeyStorageTest.cs b/src/Accounts/Authentication.Test/AzKeyStorageTest.cs
@@ -30,7 +30,7 @@ public class AzKeyStorageTest
         private Mock<IStorage> storageMocker = null;
         private List<byte> storageChecker = null;
         private string dummpyPath = "/home/dummy/.Azure";
-        private string keyStoreFileName = "keystore.cache";
+        private string keyStoreFileName = "azkeystore";
 
         public AzKeyStorageTest()
         {
diff --git a/src/Accounts/Authentication/AzureSessionInitializer.cs b/src/Accounts/Authentication/AzureSessionInitializer.cs
@@ -160,8 +160,7 @@ static ContextAutosaveSettings InitializeSessionSettings(IDataStore store, strin
                 ContextDirectory = profileDirectory,
                 Mode = ContextSaveMode.Process,
                 CacheFile = "msal.cache",
-                ContextFile = "AzureRmContext.json",
-                KeyStoreFile = "keystore.cache"
+                ContextFile = "AzureRmContext.json"
             };
 
             var settingsPath = Path.Combine(profileDirectory, settingsFile);
@@ -177,7 +176,6 @@ static ContextAutosaveSettings InitializeSessionSettings(IDataStore store, strin
                     result.ContextDirectory = migrated ? profileDirectory : settings.ContextDirectory ?? result.ContextDirectory;
                     result.Mode = settings.Mode;
                     result.ContextFile = settings.ContextFile ?? result.ContextFile;
-                    result.KeyStoreFile = settings.KeyStoreFile ?? result.KeyStoreFile;
                     result.Settings = settings.Settings;
                     bool updateSettings = false;
                     if (!settings.Settings.ContainsKey("InstallationId"))
@@ -270,6 +268,7 @@ static IAzureSession CreateInstance(IDataStore dataStore = null, Action<string>
             session.ARMProfileFile = autoSave.ContextFile;
             session.TokenCacheDirectory = autoSave.CacheDirectory;
             session.TokenCacheFile = autoSave.CacheFile;
+            session.KeyStoreFile = "azkeystore.cache";
             autoSave.Settings.TryGetValue("InstallationId", out string installationId);
             session.ExtendedProperties.Add("InstallationId", installationId);
             InitializeConfigs(session, profilePath, writeWarning);
diff --git a/src/Accounts/Authentication/ContextAutosaveSettings.cs b/src/Accounts/Authentication/ContextAutosaveSettings.cs
@@ -56,7 +56,6 @@ public class ContextAutosaveSettings : IExtensibleSettings
         /// </summary>
         public string KeyStoreFile { get; set; }
 
-
         /// <summary>
         /// Extensible settings for autosave
         /// </summary>
diff --git a/src/Accounts/Authentication/KeyStore/AzKeyStore.cs b/src/Accounts/Authentication/KeyStore/AzKeyStore.cs
@@ -71,6 +71,11 @@ public IStorage Storage
             set => _storage = value;
         }
 
+        public bool IsProtected
+        {
+            get => Storage.IsProtected;
+        }
+
         public AzKeyStore()
         {
 
@@ -152,6 +157,12 @@ public void ClearCache()
             _credentials.Clear();
         }
 
+        public void Clear()
+        {
+            ClearCache();
+            Storage.Clear();
+        }
+
         public void Flush()
         {
             IList<KeyStoreElement> serializableKeyStore = new List<KeyStoreElement>();
diff --git a/src/Accounts/Authentication/KeyStore/IStorage.cs b/src/Accounts/Authentication/KeyStore/IStorage.cs
@@ -28,5 +28,10 @@ public interface IStorage
         void WriteData(byte[] data);
 
         Exception GetLastError();
+
+        bool IsProtected
+        {
+            get;
+        }
     }
 }
diff --git a/src/Accounts/Authentication/KeyStore/StorageWrapper.cs b/src/Accounts/Authentication/KeyStore/StorageWrapper.cs
@@ -14,6 +14,7 @@
 using Microsoft.Azure.Commands.Common.Authentication.Properties;
 using Microsoft.Identity.Client.Extensions.Msal;
 using System;
+using System.Collections.Generic;
 using System.Threading;
 
 namespace Microsoft.Azure.Commands.ResourceManager.Common
@@ -29,6 +30,13 @@ class StorageWrapper : IStorage
 
         private Storage _storage = null;
 
+        private bool _protected;
+        public bool IsProtected
+        {
+            get => _protected;
+            private set => _protected = value;
+        }
+
         static ReaderWriterLockSlim storageLock = new ReaderWriterLockSlim(LockRecursionPolicy.SupportsRecursion);
 
         public StorageWrapper()
@@ -47,16 +55,19 @@ public IStorage Create()
             {
                 storageProperties = new StorageCreationPropertiesBuilder(FileName, Directory)
                     .WithMacKeyChain(KeyChainServiceName + ".other_secrets", FileName)
-                    .WithLinuxUnprotectedFile();
+                    .WithLinuxKeyring(FileName, "default", "AzKeyStoreCache",
+                    new KeyValuePair<string, string>("AzureClientID", "Microsoft.Developer.Azure.PowerShell"),
+                    new KeyValuePair<string, string>("Microsoft.Developer.Azure.PowerShell", "1.0.0.0"));
                 _storage = Storage.Create(storageProperties.Build());
                 VerifyPersistence();
+                _protected = true;
             }
-            catch (MsalCachePersistenceException e)
+            catch (Exception e)
             {
                 _lastError = e;
-                _storage.Clear();
                 storageProperties = new StorageCreationPropertiesBuilder(FileName, Directory).WithUnprotectedFile();
                 _storage = Storage.Create(storageProperties.Build());
+                _protected = false;
             }
             finally
             {

Original file line number	Diff line number	Diff line change
`@@ -55,7 +55,7 @@ private AzKeyStore SetMockedAzKeyStore()`
`55`	`55`	`storageMocker.Setup(f => f.Create()).Returns(storageMocker.Object);`
`56`	`56`	`storageMocker.Setup(f => f.ReadData()).Returns(new byte[0]);`
`57`	`57`	`storageMocker.Setup(f => f.WriteData(It.IsAny<byte[]>())).Callback((byte[] s) => { });`
`58`		`- var keyStore = new AzKeyStore(AzureSession.Instance.ARMProfileDirectory, "keystore.cache", false, false, storageMocker.Object);`
	`58`	`+ var keyStore = new AzKeyStore(AzureSession.Instance.ARMProfileDirectory, "azkeystore", false, false, storageMocker.Object);`
`59`	`59`	`return keyStore;`
`60`	`60`	`}`
`61`	`61`
Original file line number	Diff line number	Diff line change
`@@ -426,6 +426,10 @@ public override void ExecuteCmdlet()`
`426`	`426`	`if (CertificatePassword != null)`
`427`	`427`	`{`
`428`	`428`	`keyStore?.SaveKey(new ServicePrincipalKey(AzureAccount.Property.CertificatePassword, azureAccount.Id, Tenant), CertificatePassword);`
	`429`	`+ if (GetContextModificationScope() == ContextModificationScope.CurrentUser && !keyStore.IsProtected)`
	`430`	`+ {`
	`431`	`+ WriteWarning(string.Format(Resources.ServicePrincipalWarning, AzureSession.Instance.KeyStoreFile, AzureSession.Instance.ARMProfileDirectory));`
	`432`	`+ }`
`429`	`433`	`}`
`430`	`434`	`}`
`431`	`435`
`@@ -449,11 +453,9 @@ public override void ExecuteCmdlet()`
`449`	`453`	`{`
`450`	`454`	`keyStore?.SaveKey(new ServicePrincipalKey(AzureAccount.Property.ServicePrincipalSecret`
`451`	`455`	`,azureAccount.Id, Tenant), password);`
`452`		`- if (GetContextModificationScope() == ContextModificationScope.CurrentUser)`
	`456`	`+ if (GetContextModificationScope() == ContextModificationScope.CurrentUser && !keyStore.IsProtected)`
`453`	`457`	`{`
`454`		`- var file = AzureSession.Instance.ARMProfileFile;`
`455`		`- var directory = AzureSession.Instance.ARMProfileDirectory;`
`456`		`- WriteWarning(string.Format(Resources.ServicePrincipalWarning, file, directory));`
	`458`	`+ WriteWarning(string.Format(Resources.ServicePrincipalWarning, AzureSession.Instance.KeyStoreFile, AzureSession.Instance.ARMProfileDirectory));`
`457`	`459`	`}`
`458`	`460`	`}`
`459`	`461`	`if (azureAccount.Type == "ClientAssertion" && FederatedToken != null)`
`@@ -711,8 +713,7 @@ public void OnImport()`
`711`	`713`	`}`
`712`	`714`
`713`	`715`	`AzKeyStore keyStore = null;`
`714`		`- //AzureSession.Instance.KeyStoreFile`
`715`		`- keyStore = new AzKeyStore(AzureSession.Instance.ARMProfileDirectory, "keystore.cache", false, autoSaveEnabled);`
	`716`	`+ keyStore = new AzKeyStore(AzureSession.Instance.ARMProfileDirectory, AzureSession.Instance.KeyStoreFile, false, autoSaveEnabled);`
`716`	`717`	`AzKeyStore.RegisterJsonConverter(typeof(ServicePrincipalKey), typeof(ServicePrincipalKey).Name);`
`717`	`718`	`AzKeyStore.RegisterJsonConverter(typeof(SecureString), typeof(SecureString).Name, new SecureStringConverter());`
`718`	`719`	`AzureSession.Instance.RegisterComponent(AzKeyStore.Name, () => keyStore);`
Original file line number	Diff line number	Diff line change
`@@ -83,6 +83,11 @@ void ClearContext(AzureRmProfile profile, RMProfileClient client)`
`83`	`83`	`profile.TrySetDefaultContext(defaultContext);`
`84`	`84`	`result = true;`
`85`	`85`	`}`
	`86`	`+ if (AzureSession.Instance.TryGetComponent(AzKeyStore.Name, out AzKeyStore keyStore))`
	`87`	`+ {`
	`88`	`+ keyStore?.Clear();`
	`89`	`+ }`
	`90`	`+`
`86`	`91`	`}`
`87`	`92`
`88`	`93`	`AzureSession.Instance.RaiseContextClearedEvent();`
Original file line number	Diff line number	Diff line change
`@@ -30,7 +30,7 @@ public class AzKeyStorageTest`
`30`	`30`	`private Mock<IStorage> storageMocker = null;`
`31`	`31`	`private List<byte> storageChecker = null;`
`32`	`32`	`private string dummpyPath = "/home/dummy/.Azure";`
`33`		`- private string keyStoreFileName = "keystore.cache";`
	`33`	`+ private string keyStoreFileName = "azkeystore";`
`34`	`34`
`35`	`35`	`public AzKeyStorageTest()`
`36`	`36`	`{`