Skip to content

Store Secrets to Encrypted Storage #19929

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

Merged
merged 5 commits into from
Nov 24, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion src/Accounts/Accounts.Test/AutosaveTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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]
Expand Down
21 changes: 21 additions & 0 deletions src/Accounts/Accounts.Test/ProfileCmdletTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -34,6 +36,7 @@ public class ProfileCmdletTests : RMTestBase
{
private MemoryDataStore dataStore;
private MockCommandRuntime commandRuntimeMock;
private AzKeyStore keyStore;

public ProfileCmdletTests(ITestOutputHelper output)
{
Expand All @@ -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";
Expand All @@ -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
Expand All @@ -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";
Expand All @@ -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";
Expand All @@ -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
Expand All @@ -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"]);
Expand Down
20 changes: 12 additions & 8 deletions src/Accounts/Accounts/Account/ConnectAzureRmAccount.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
Expand All @@ -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)
Expand Down Expand Up @@ -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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
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 @@ -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]
Expand Down
11 changes: 10 additions & 1 deletion src/Accounts/Accounts/Properties/Resources.Designer.cs

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

3 changes: 3 additions & 0 deletions src/Accounts/Accounts/Properties/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
31 changes: 30 additions & 1 deletion src/Accounts/Authentication.ResourceManager/AzureRmProfile.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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)
{
}
Expand Down Expand Up @@ -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
{
Expand Down
Loading