Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BugFix] Enable AzKeyStore with keyring in Linux #20341

Merged
merged 2 commits into from
Dec 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Accounts/Accounts.Test/AutosaveTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/Accounts/Accounts.Test/ProfileCmdletTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
13 changes: 7 additions & 6 deletions src/Accounts/Accounts/Account/ConnectAzureRmAccount.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
}

Expand All @@ -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)
Expand Down Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions src/Accounts/Accounts/ChangeLog.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 5 additions & 0 deletions src/Accounts/Accounts/Context/ClearAzureRmContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
4 changes: 2 additions & 2 deletions src/Accounts/Accounts/Properties/Resources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/Accounts/Accounts/Properties/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
2 changes: 1 addition & 1 deletion src/Accounts/Authentication.Test/AzKeyStorageTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down
5 changes: 2 additions & 3 deletions src/Accounts/Authentication/AzureSessionInitializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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"))
Expand Down Expand Up @@ -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);
Expand Down
1 change: 0 additions & 1 deletion src/Accounts/Authentication/ContextAutosaveSettings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ public class ContextAutosaveSettings : IExtensibleSettings
/// </summary>
public string KeyStoreFile { get; set; }


/// <summary>
/// Extensible settings for autosave
/// </summary>
Expand Down
11 changes: 11 additions & 0 deletions src/Accounts/Authentication/KeyStore/AzKeyStore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@ public IStorage Storage
set => _storage = value;
}

public bool IsProtected
{
get => Storage.IsProtected;
}

public AzKeyStore()
{

Expand Down Expand Up @@ -152,6 +157,12 @@ public void ClearCache()
_credentials.Clear();
}

public void Clear()
{
ClearCache();
Storage.Clear();
}

public void Flush()
{
IList<KeyStoreElement> serializableKeyStore = new List<KeyStoreElement>();
Expand Down
5 changes: 5 additions & 0 deletions src/Accounts/Authentication/KeyStore/IStorage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,10 @@ public interface IStorage
void WriteData(byte[] data);

Exception GetLastError();

bool IsProtected
{
get;
}
}
}
17 changes: 14 additions & 3 deletions src/Accounts/Authentication/KeyStore/StorageWrapper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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()
Expand All @@ -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
{
Expand Down