Skip to content

Commit

Permalink
Cache temp key and crypter scrypt in unlockwallet
Browse files Browse the repository at this point in the history
Cache the aesKey instead of the password in unlock wallet method,
avoiding extra overhead of deriving it from the temp password in the
lock method.

The KeyCrypterScrypt used in the unlock method must also be cached for
use in the manual lock method.  Creating a new KeyCrypterScrypt instance
in the manual lock method would have a different random salt value,
invalidating the password used in the unlock method.
  • Loading branch information
ghubstan committed May 6, 2020
1 parent fbb025a commit 4262f29
Showing 1 changed file with 36 additions and 12 deletions.
48 changes: 36 additions & 12 deletions core/src/main/java/bisq/core/grpc/CoreWalletService.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ class CoreWalletService {
private final WalletsManager walletsManager;

@Nullable
private String tempLockWalletPassword;
private KeyParameter tempAesKey;

@Nullable
KeyCrypterScrypt tempKeyCrypterScrypt;

@Inject
public CoreWalletService(Balances balances, WalletsManager walletsManager) {
Expand Down Expand Up @@ -77,30 +80,51 @@ public void setWalletPassword(String password, String newPassword) {
}

public void lockWallet() {
if (tempLockWalletPassword == null)
if (tempKeyCrypterScrypt == null && tempAesKey == null)
throw new IllegalStateException("wallet is already locked");

setWalletPassword(tempLockWalletPassword, null);
tempLockWalletPassword = null;
if (walletsManager.areWalletsEncrypted()) {
// This should never happen.
log.error("The lockwallet method found the wallet already encrypted, "
+ "while the tempKeyCrypterScrypt and tempAesKey values that are "
+ " supposed to be used on re-encrypt the wallet are not null.");
tempKeyCrypterScrypt = null;
tempAesKey = null;
throw new IllegalStateException("wallet is already locked");
}

walletsManager.encryptWallets(tempKeyCrypterScrypt, tempAesKey);
tempKeyCrypterScrypt = null;
tempAesKey = null;
}

public void unlockWallet(String password, long timeout) {
removeWalletPassword(password);
verifyWalletIsAvailableAndEncrypted();

// We need to cache a temporary KeyCrypterScrypt for use in the manual lock method.
// Using a different crypter instance there would invalidate the password used here
// because it would have a different random salt value.
tempKeyCrypterScrypt = getKeyCrypterScrypt();
// The aesKey is also cached for timeout (secs) after being used to decrypt the
// wallet, in case the user wants to manually lock the wallet before the timeout.
tempAesKey = tempKeyCrypterScrypt.deriveKey(password);

if (!walletsManager.checkAESKey(tempAesKey))
throw new IllegalStateException("incorrect password");

walletsManager.decryptWallets(tempAesKey);
TimerTask timerTask = new TimerTask() {
@Override
public void run() {
log.info("Locking wallet");
setWalletPassword(password, null);
tempLockWalletPassword = null;
if (tempKeyCrypterScrypt != null && tempAesKey != null) {
// Do not try to lock wallet after timeout if the user already has via 'lockwallet'.
log.info("Locking wallet after {} second timeout expired.", timeout);
lockWallet();
}
}
};
Timer timer = new Timer("Lock Wallet Timer");
timer.schedule(timerTask, SECONDS.toMillis(timeout));

// Cache wallet password for timeout (secs), in case
// user wants to lock the wallet for timeout expires.
tempLockWalletPassword = password;
}

// Provided for automated wallet protection method testing, despite the
Expand Down

0 comments on commit 4262f29

Please sign in to comment.