Skip to content

Commit

Permalink
Kitchen is a component that keeps the password encrypted in memory.…
Browse files Browse the repository at this point in the history
… It (WalletWasabi#13218)

implicitly assumes that an attacker with access to the process memory
cannot extract the password. That assumption is incorrect and Wasabi has to decrypt the
password multiple times to pass it to components that expect it as string.
  • Loading branch information
lontivero authored Jul 2, 2024
1 parent bf0cbd3 commit 432c55a
Show file tree
Hide file tree
Showing 23 changed files with 37 additions and 299 deletions.
4 changes: 2 additions & 2 deletions WalletWasabi.Daemon/Rpc/WasabiJsonRpcService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ public string BuildCancelTransaction(uint256 txId, string password = "")
{
Guard.NotNull(nameof(txId), txId);
var activeWallet = Guard.NotNull(nameof(ActiveWallet), ActiveWallet);
activeWallet.Kitchen.Cook(password);
activeWallet.TryLogin(password, out _);
var mempoolStore = Global.BitcoinStore.TransactionStore.MempoolStore;
if (!mempoolStore.TryGetTransaction(txId, out var smartTransactionToCancel))
{
Expand All @@ -389,7 +389,7 @@ public string SpeedUpTransaction(uint256 txId, string password = "")
{
Guard.NotNull(nameof(txId), txId);
var activeWallet = Guard.NotNull(nameof(ActiveWallet), ActiveWallet);
activeWallet.Kitchen.Cook(password);
activeWallet.TryLogin(password, out _);
var mempoolStore = Global.BitcoinStore.TransactionStore.MempoolStore;
if (!mempoolStore.TryGetTransaction(txId, out var smartTransactionToSpeedUp))
{
Expand Down
4 changes: 2 additions & 2 deletions WalletWasabi.Fluent/Models/Wallets/WalletAuthModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public WalletAuthModel(IWalletModel walletModel, Wallet wallet)
_wallet = wallet;
}

public bool HasPassword => !string.IsNullOrEmpty(_wallet.Kitchen.SaltSoup());
public bool HasPassword => !string.IsNullOrEmpty(_wallet.Password);

public async Task LoginAsync(string password)
{
Expand Down Expand Up @@ -61,7 +61,7 @@ public void Logout()

public bool VerifyRecoveryWords(Mnemonic mnemonic)
{
var saltSoup = _wallet.Kitchen.SaltSoup();
var saltSoup = _wallet.Password;

var recovered = KeyManager.Recover(
mnemonic,
Expand Down
2 changes: 1 addition & 1 deletion WalletWasabi.Fluent/Models/Wallets/WalletCoinsModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public List<ICoinModel> GetSpentCoins(BuildTransactionResult? transaction)

public bool AreEnoughToCreateTransaction(TransactionInfo transactionInfo, IEnumerable<ICoinModel> coins)
{
return TransactionHelpers.TryBuildTransactionWithoutPrevTx(Wallet.KeyManager, transactionInfo, Wallet.Coins, coins.GetSmartCoins(), Wallet.Kitchen.SaltSoup(), out _);
return TransactionHelpers.TryBuildTransactionWithoutPrevTx(Wallet.KeyManager, transactionInfo, Wallet.Coins, coins.GetSmartCoins(), Wallet.Password, out _);
}

protected override Pocket[] GetPockets()
Expand Down
4 changes: 2 additions & 2 deletions WalletWasabi.Fluent/Models/Wallets/WalletInfoModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@ public WalletInfoModel(Wallet wallet)
var network = wallet.Network;
if (!wallet.KeyManager.IsWatchOnly)
{
var secret = PasswordHelper.GetMasterExtKey(wallet.KeyManager, wallet.Kitchen.SaltSoup(), out _);
var secret = PasswordHelper.GetMasterExtKey(wallet.KeyManager, wallet.Password, out _);

ExtendedMasterPrivateKey = secret.GetWif(network).ToWif();
ExtendedAccountPrivateKey = secret.Derive(wallet.KeyManager.SegwitAccountKeyPath).GetWif(network).ToWif();
ExtendedMasterZprv = secret.ToZPrv(network);

// TODO: Should work for every type of wallet, temporarily disabling it.
WpkhOutputDescriptors = wallet.KeyManager.GetOutputDescriptors(wallet.Kitchen.SaltSoup(), network);
WpkhOutputDescriptors = wallet.KeyManager.GetOutputDescriptors(wallet.Password, network);
}

SegWitExtendedAccountPublicKey = wallet.KeyManager.SegwitExtPubKey.ToString(network);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public PrivacyControlViewModel(Wallet wallet, SendFlowModel sendFlow, Transactio
_isSilent = isSilent;
_usedCoins = usedCoins;

LabelSelection = new LabelSelectionViewModel(wallet.KeyManager, wallet.Kitchen.SaltSoup(), _transactionInfo, isSilent);
LabelSelection = new LabelSelectionViewModel(wallet.KeyManager, wallet.Password, _transactionInfo, isSilent);

SetupCancel(enableCancel: false, enableCancelOnEscape: true, enableCancelOnPressed: false);
EnableBack = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public TransactionPreviewViewModel(UiContext uiContext, IWalletModel walletModel
];

DisplayedTransactionSummary = CurrentTransactionSummary;

SetupCancel(enableCancel: true, enableCancelOnEscape: true, enableCancelOnPressed: false);
EnableBack = true;

Expand Down Expand Up @@ -479,7 +479,7 @@ private async Task CheckChangePocketAvailableAsync(BuildTransactionResult transa

var usedCoins = transaction.SpentCoins;
var pockets = _sendFlow.GetPockets();
var labelSelection = new LabelSelectionViewModel(_wallet.KeyManager, _wallet.Kitchen.SaltSoup(), _info, isSilent: true);
var labelSelection = new LabelSelectionViewModel(_wallet.KeyManager, _wallet.Password, _info, isSilent: true);
await labelSelection.ResetAsync(pockets, coinsToExclude: cjManager.CoinsInCriticalPhase[_wallet.WalletId].ToList());

_info.IsOtherPocketSelectionPossible = labelSelection.IsOtherSelectionPossible(usedCoins, _info.Recipient);
Expand Down
4 changes: 2 additions & 2 deletions WalletWasabi.Tests/Helpers/WabiSabiFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ public static BlameRound CreateBlameRound(Round round, WabiSabiConfig cfg)
public static (IKeyChain, SmartCoin, SmartCoin) CreateCoinKeyPairs(KeyManager? keyManager = null)
{
var km = keyManager ?? ServiceFactory.CreateKeyManager("");
var keyChain = new KeyChain(km, new Kitchen(""));
var keyChain = new KeyChain(km,"");

var smartCoin1 = BitcoinFactory.CreateSmartCoin(BitcoinFactory.CreateHdPubKey(km), Money.Coins(1m));
var smartCoin2 = BitcoinFactory.CreateSmartCoin(BitcoinFactory.CreateHdPubKey(km), Money.Coins(2m));
Expand All @@ -321,7 +321,7 @@ public static CoinJoinClient CreateTestCoinJoinClient(
{
return CreateTestCoinJoinClient(
httpClientFactory,
new KeyChain(keyManager, new Kitchen("")),
new KeyChain(keyManager,""),
new OutputProvider(new InternalDestinationProvider(keyManager)),
roundStateUpdater,
keyManager.RedCoinIsolation);
Expand Down
2 changes: 1 addition & 1 deletion WalletWasabi.Tests/RegressionTests/CancelTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ public async Task CancelTestsAsync()
// Wait until the filter our previous transaction is present.
var blockCount = await rpc.GetBlockCountAsync();
await setup.WaitForFiltersToBeProcessedAsync(TimeSpan.FromSeconds(120), blockCount);
wallet.Kitchen.Cook(password);
wallet.Password = password;

TransactionBroadcaster broadcaster = new(network, bitcoinStore, httpClientFactory, walletManager);
broadcaster.Initialize(nodes, rpc);
Expand Down
2 changes: 1 addition & 1 deletion WalletWasabi.Tests/RegressionTests/MaxFeeTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public async Task CalculateMaxFeeTestAsync()
// Wait until the filter our previous transaction is present.
var blockCount = await rpc.GetBlockCountAsync();
await setup.WaitForFiltersToBeProcessedAsync(TimeSpan.FromSeconds(120), blockCount);
wallet.Kitchen.Cook(password);
wallet.Password = password;

TransactionBroadcaster broadcaster = new(network, bitcoinStore, httpClientFactory, walletManager);
broadcaster.Initialize(nodes, rpc);
Expand Down
2 changes: 1 addition & 1 deletion WalletWasabi.Tests/RegressionTests/ReceiveSpeedupTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public async Task ReceiveSpeedupTestsAsync()
var blockCount = await rpc.GetBlockCountAsync();
await setup.WaitForFiltersToBeProcessedAsync(TimeSpan.FromSeconds(120), blockCount);

wallet.Kitchen.Cook(password);
wallet.Password = password;

TransactionBroadcaster broadcaster = new(network, bitcoinStore, httpClientFactory, walletManager);
broadcaster.Initialize(nodes, rpc);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public async Task SelfSpendSpeedupTestsAsync()
var blockCount = await rpc.GetBlockCountAsync();
await setup.WaitForFiltersToBeProcessedAsync(TimeSpan.FromSeconds(120), blockCount);

wallet.Kitchen.Cook(password);
wallet.Password = password;

TransactionBroadcaster broadcaster = new(network, bitcoinStore, httpClientFactory, walletManager);
broadcaster.Initialize(nodes, rpc);
Expand Down
2 changes: 1 addition & 1 deletion WalletWasabi.Tests/RegressionTests/SendSpeedupTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ public async Task SendSpeedupTestsAsync()
// Wait until the filter our previous transaction is present.
var blockCount = await rpc.GetBlockCountAsync();
await setup.WaitForFiltersToBeProcessedAsync(TimeSpan.FromSeconds(120), blockCount);
wallet.Kitchen.Cook(password);
wallet.Password = password;

TransactionBroadcaster broadcaster = new(network, bitcoinStore, httpClientFactory, walletManager);
broadcaster.Initialize(nodes, rpc);
Expand Down
83 changes: 0 additions & 83 deletions WalletWasabi.Tests/UnitTests/Crypto/StringCipherTests.cs

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
using WalletWasabi.WabiSabi.Client.CoinJoin.Client;
using WalletWasabi.WabiSabi.Client.RoundStateAwaiters;
using WalletWasabi.WabiSabi.Models;
using WalletWasabi.Wallets;
using Xunit;

namespace WalletWasabi.Tests.UnitTests.WabiSabi.Backend;
Expand All @@ -35,7 +34,7 @@ public async Task AliceRegistrationTimesOutAsync()
await roundStateUpdater.StartAsync(testDeadlineCts.Token);

// Register Alices.
KeyChain keyChain = new(km, new Kitchen(ingredients: ""));
KeyChain keyChain = new(km, "");

using CancellationTokenSource registrationCts = new();
Task<AliceClient> task = AliceClient.CreateRegisterAndConfirmInputAsync(RoundState.FromRound(round), arenaClient, smartCoin, keyChain, roundStateUpdater, registrationCts.Token, registrationCts.Token, confirmationCancellationToken: testDeadlineCts.Token);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public async Task SignTransactionAsync()
var password = "satoshi";

var km = ServiceFactory.CreateKeyManager(password);
var keyChain = new KeyChain(km, new Kitchen(password));
var keyChain = new KeyChain(km, password);
var destinationProvider = new InternalDestinationProvider(km);

var coins = destinationProvider.GetNextDestinations(2, false)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public async Task RegisterOutputTestAsync()
using RoundStateUpdater roundStateUpdater = new(TimeSpan.FromSeconds(2), wabiSabiApi);
await roundStateUpdater.StartAsync(token);

var keyChain = new KeyChain(km, new Kitchen(""));
var keyChain = new KeyChain(km,"");
var task = AliceClient.CreateRegisterAndConfirmInputAsync(RoundState.FromRound(round), aliceArenaClient, coin1, keyChain, roundStateUpdater, token, token, token);

do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public void SignTransactionTest()
{
var keyManager = KeyManager.CreateNew(out _, "", Network.Main);
var destinationProvider = new InternalDestinationProvider(keyManager);
var keyChain = new KeyChain(keyManager, new Kitchen(""));
var keyChain = new KeyChain(keyManager,"");

var coinDestination = destinationProvider.GetNextDestinations(1, false).First();
var coin = new Coin(BitcoinFactory.CreateOutPoint(), new TxOut(Money.Coins(1.0m), coinDestination));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public static BuildTransactionResult BuildChangelessTransaction(
label);

var txRes = wallet.BuildTransaction(
wallet.Kitchen.SaltSoup(),
wallet.Password,
intent,
FeeStrategy.CreateFromFeeRate(feeRate),
allowUnconfirmed: true,
Expand Down Expand Up @@ -119,7 +119,7 @@ public static BuildTransactionResult BuildTransaction(
label: label);

var txRes = wallet.BuildTransaction(
password: wallet.Kitchen.SaltSoup(),
password: wallet.Password,
payments: intent,
feeStrategy: FeeStrategy.CreateFromFeeRate(feeRate),
allowUnconfirmed: true,
Expand Down Expand Up @@ -172,7 +172,7 @@ public static BuildTransactionResult BuildTransactionForSIB(
label: label);

var txRes = wallet.BuildTransaction(
password: wallet.Kitchen.SaltSoup(),
password: wallet.Password,
payments: intent,
feeStrategy: FeeStrategy.CreateFromConfirmationTarget(2),
allowUnconfirmed: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ ownOutput is null
var allowedInputs = transactionToSpeedUp.WalletInputs.Select(coin => coin.Outpoint);

BuildTransactionResult rbf = wallet.BuildTransaction(
password: wallet.Kitchen.SaltSoup(),
password: wallet.Password,
payments: new PaymentIntent(payments),
feeStrategy: FeeStrategy.CreateFromFeeRate(rbfFeeRate),
allowUnconfirmed: true,
Expand Down
Loading

0 comments on commit 432c55a

Please sign in to comment.