wallet: avoid returning a reference to vMasterKey after releasing the…

… mutex that guards it

`CWallet::GetEncryptionKey()` would return a reference to the internal
`CWallet::vMasterKey`, guarded by `CWallet::cs_wallet`, which is unsafe.

Returning a copy would be a shorter solution, but could have security
implications of the master key remaining somewhere in the memory even
after `CWallet::Lock()` (the current calls to
`CWallet::GetEncryptionKey()` are safe, but that is not future proof).

So, instead of `EncryptSecret(m_storage.GetEncryptionKey(), ...`
introduce a wallet method `CWallet::EncryptSecret()` which locks the
mutex and calls the external `EncryptSecret()` with the master key. Same
for `DecryptKey()`.

This silences the following (clang 18):

```
wallet/wallet.cpp:3520:12: error: returning variable 'vMasterKey' by reference requires holding mutex 'cs_wallet' [-Werror,-Wthread-safety-reference-return]
 3520 |     return vMasterKey;
      |            ^
```

src/wallet/scriptpubkeyman.cpp

@@ -810,7 +810,7 @@ bool LegacyScriptPubKeyMan::AddKeyPubKeyInner(const CKey& key, const CPubKey &pu
 
     std::vector<unsigned char> vchCryptedSecret;
     CKeyingMaterial vchSecret(key.begin(), key.end());
-    if (!EncryptSecret(m_storage.GetEncryptionKey(), vchSecret, pubkey.GetHash(), vchCryptedSecret)) {
+    if (!m_storage.EncryptSecret(vchSecret, pubkey.GetHash(), vchCryptedSecret)) {
         return false;
     }
 
@@ -996,7 +996,7 @@ bool LegacyScriptPubKeyMan::GetKey(const CKeyID &address, CKey& keyOut) const
     {
         const CPubKey &vchPubKey = (*mi).second.first;
         const std::vector<unsigned char> &vchCryptedSecret = (*mi).second.second;
-        return DecryptKey(m_storage.GetEncryptionKey(), vchCryptedSecret, vchPubKey, keyOut);
+        return m_storage.DecryptKey(vchCryptedSecret, vchPubKey, keyOut);
     }
     return false;
 }
@@ -2126,7 +2126,7 @@ std::map<CKeyID, CKey> DescriptorScriptPubKeyMan::GetKeys() const
             const CPubKey& pubkey = key_pair.second.first;
             const std::vector<unsigned char>& crypted_secret = key_pair.second.second;
             CKey key;
-            DecryptKey(m_storage.GetEncryptionKey(), crypted_secret, pubkey, key);
+            m_storage.DecryptKey(crypted_secret, pubkey, key);
             keys[pubkey.GetID()] = key;
         }
         return keys;
@@ -2252,7 +2252,7 @@ bool DescriptorScriptPubKeyMan::AddDescriptorKeyWithDB(WalletBatch& batch, const
 
         std::vector<unsigned char> crypted_secret;
         CKeyingMaterial secret(key.begin(), key.end());
-        if (!EncryptSecret(m_storage.GetEncryptionKey(), secret, pubkey.GetHash(), crypted_secret)) {
+        if (!m_storage.EncryptSecret(secret, pubkey.GetHash(), crypted_secret)) {
             return false;
         }
 

src/wallet/scriptpubkeyman.h

@@ -46,7 +46,8 @@ class WalletStorage
     virtual void UnsetBlankWalletFlag(WalletBatch&) = 0;
     virtual bool CanSupportFeature(enum WalletFeature) const = 0;
     virtual void SetMinVersion(enum WalletFeature, WalletBatch* = nullptr) = 0;
-    virtual const CKeyingMaterial& GetEncryptionKey() const = 0;
+    virtual bool EncryptSecret(const CKeyingMaterial& plaintext, const uint256& iv, std::vector<unsigned char>& ciphertext) const = 0;
+    virtual bool DecryptKey(const std::vector<unsigned char>& crypted_secret, const CPubKey& pub_key, CKey& key) const = 0;
     virtual bool HasEncryptionKeys() const = 0;
     virtual bool IsLocked() const = 0;
 };

src/wallet/wallet.cpp

@@ -3515,9 +3515,16 @@ void CWallet::SetupLegacyScriptPubKeyMan()
     AddScriptPubKeyMan(id, std::move(spk_manager));
 }
 
-const CKeyingMaterial& CWallet::GetEncryptionKey() const
+bool CWallet::EncryptSecret(const CKeyingMaterial& plaintext, const uint256& iv, std::vector<unsigned char>& ciphertext) const
 {
-    return vMasterKey;
+    LOCK(cs_wallet);
+    return ::wallet::EncryptSecret(vMasterKey, plaintext, iv, ciphertext);
+}
+
+bool CWallet::DecryptKey(const std::vector<unsigned char>& crypted_secret, const CPubKey& pub_key, CKey& key) const
+{
+    LOCK(cs_wallet);
+    return ::wallet::DecryptKey(vMasterKey, crypted_secret, pub_key, key);
 }
 
 bool CWallet::HasEncryptionKeys() const

src/wallet/wallet.h

@@ -949,7 +949,10 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
     //! Make a LegacyScriptPubKeyMan and set it for all types, internal, and external.
     void SetupLegacyScriptPubKeyMan();
 
-    const CKeyingMaterial& GetEncryptionKey() const override;
+    bool EncryptSecret(const CKeyingMaterial& plaintext, const uint256& iv, std::vector<unsigned char>& ciphertext) const override;
+
+    bool DecryptKey(const std::vector<unsigned char>& crypted_secret, const CPubKey& pub_key, CKey& key) const override;
+
     bool HasEncryptionKeys() const override;
 
     /** Get last block processed height */