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

* 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
azure-sdk · Dec 7, 2022 · d687600 · d687600
1 parent d126c2e
commit d687600
Show file tree

Hide file tree

Showing 13 changed files with 51 additions and 19 deletions.
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
             {