key: don't allocate secure mem for null (invalid) key

Instead of storing the key material as an std::vector (with secure allocator),
use a secure_unique_ptr to a 32-byte array, and use nullptr for invalid keys.
This means a smaller CKey type, and no secure/dynamic memory usage for invalid
keys.

2 files changed, 59 insertions, 38 deletions

diff --git a/src/key.cpp b/src/key.cpp
index efaea5b1b3..0f283ca3e3 100644
--- a/src/key.cpp
+++ b/src/key.cpp
@@ -159,21 +159,21 @@ bool CKey::Check(const unsigned char *vch) {
 }
 
 void CKey::MakeNewKey(bool fCompressedIn) {
+    MakeKeyData();
     do {
-        GetStrongRandBytes(keydata);
-    } while (!Check(keydata.data()));
-    fValid = true;
+        GetStrongRandBytes(*keydata);
+    } while (!Check(keydata->data()));
     fCompressed = fCompressedIn;
 }
 
 bool CKey::Negate()
 {
-    assert(fValid);
-    return secp256k1_ec_seckey_negate(secp256k1_context_sign, keydata.data());
+    assert(keydata);
+    return secp256k1_ec_seckey_negate(secp256k1_context_sign, keydata->data());
 }
 
 CPrivKey CKey::GetPrivKey() const {
-    assert(fValid);
+    assert(keydata);
     CPrivKey seckey;
     int ret;
     size_t seckeylen;
@@ -186,7 +186,7 @@ CPrivKey CKey::GetPrivKey() const {
 }
 
 CPubKey CKey::GetPubKey() const {
-    assert(fValid);
+    assert(keydata);
     secp256k1_pubkey pubkey;
     size_t clen = CPubKey::SIZE;
     CPubKey result;
@@ -212,7 +212,7 @@ bool SigHasLowR(const secp256k1_ecdsa_signature* sig)
 }
 
 bool CKey::Sign(const uint256 &hash, std::vector<unsigned char>& vchSig, bool grind, uint32_t test_case) const {
-    if (!fValid)
+    if (!keydata)
         return false;
     vchSig.resize(CPubKey::SIGNATURE_SIZE);
     size_t nSigLen = CPubKey::SIGNATURE_SIZE;
@@ -253,7 +253,7 @@ bool CKey::VerifyPubKey(const CPubKey& pubkey) const {
 }
 
 bool CKey::SignCompact(const uint256 &hash, std::vector<unsigned char>& vchSig) const {
-    if (!fValid)
+    if (!keydata)
         return false;
     vchSig.resize(CPubKey::COMPACT_SIGNATURE_SIZE);
     int rec = -1;
@@ -301,10 +301,12 @@ bool CKey::SignSchnorr(const uint256& hash, Span<unsigned char> sig, const uint2
 }
 
 bool CKey::Load(const CPrivKey &seckey, const CPubKey &vchPubKey, bool fSkipCheck=false) {
-    if (!ec_seckey_import_der(secp256k1_context_sign, (unsigned char*)begin(), seckey.data(), seckey.size()))
+    MakeKeyData();
+    if (!ec_seckey_import_der(secp256k1_context_sign, (unsigned char*)begin(), seckey.data(), seckey.size())) {
+        ClearKeyData();
         return false;
+    }
     fCompressed = vchPubKey.IsCompressed();
-    fValid = true;
 
     if (fSkipCheck)
         return true;
@@ -325,22 +327,21 @@ bool CKey::Derive(CKey& keyChild, ChainCode &ccChild, unsigned int nChild, const
         BIP32Hash(cc, nChild, 0, begin(), vout.data());
     }
     memcpy(ccChild.begin(), vout.data()+32, 32);
-    memcpy((unsigned char*)keyChild.begin(), begin(), 32);
+    keyChild.Set(begin(), begin() + 32, true);
     bool ret = secp256k1_ec_seckey_tweak_add(secp256k1_context_sign, (unsigned char*)keyChild.begin(), vout.data());
-    keyChild.fCompressed = true;
-    keyChild.fValid = ret;
+    if (!ret) keyChild.ClearKeyData();
     return ret;
 }
 
 EllSwiftPubKey CKey::EllSwiftCreate(Span<const std::byte> ent32) const
 {
-    assert(fValid);
+    assert(keydata);
     assert(ent32.size() == 32);
     std::array<std::byte, EllSwiftPubKey::size()> encoded_pubkey;
 
     auto success = secp256k1_ellswift_create(secp256k1_context_sign,
                                              UCharCast(encoded_pubkey.data()),
-                                             keydata.data(),
+                                             keydata->data(),
                                              UCharCast(ent32.data()));
 
     // Should always succeed for valid keys (asserted above).
@@ -350,7 +351,7 @@ EllSwiftPubKey CKey::EllSwiftCreate(Span<const std::byte> ent32) const
 
 ECDHSecret CKey::ComputeBIP324ECDHSecret(const EllSwiftPubKey& their_ellswift, const EllSwiftPubKey& our_ellswift, bool initiating) const
 {
-    assert(fValid);
+    assert(keydata);
 
     ECDHSecret output;
     // BIP324 uses the initiator as party A, and the responder as party B. Remap the inputs
@@ -359,7 +360,7 @@ ECDHSecret CKey::ComputeBIP324ECDHSecret(const EllSwiftPubKey& their_ellswift, c
                                           UCharCast(output.data()),
                                           UCharCast(initiating ? our_ellswift.data() : their_ellswift.data()),
                                           UCharCast(initiating ? their_ellswift.data() : our_ellswift.data()),
-                                          keydata.data(),
+                                          keydata->data(),
                                           initiating ? 0 : 1,
                                           secp256k1_ellswift_xdh_hash_function_bip324,
                                           nullptr);
diff --git a/src/key.h b/src/key.h
index 8382b0a670..785059da02 100644
--- a/src/key.h
+++ b/src/key.h
@@ -46,57 +46,77 @@ public:
         "COMPRESSED_SIZE is larger than SIZE");
 
 private:
-    //! Whether this private key is valid. We check for correctness when modifying the key
-    //! data, so fValid should always correspond to the actual state.
-    bool fValid{false};
+    /** Internal data container for private key material. */
+    using KeyType = std::array<unsigned char, 32>;
 
     //! Whether the public key corresponding to this private key is (to be) compressed.
     bool fCompressed{false};
 
-    //! The actual byte data
-    std::vector<unsigned char, secure_allocator<unsigned char> > keydata;
+    //! The actual byte data. nullptr for invalid keys.
+    secure_unique_ptr<KeyType> keydata;
 
     //! Check whether the 32-byte array pointed to by vch is valid keydata.
     bool static Check(const unsigned char* vch);
 
+    void MakeKeyData()
+    {
+        if (!keydata) keydata = make_secure_unique<KeyType>();
+    }
+
+    void ClearKeyData()
+    {
+        keydata.reset();
+    }
+
 public:
-    //! Construct an invalid private key.
-    CKey()
+    CKey() noexcept = default;
+    CKey(CKey&&) noexcept = default;
+    CKey& operator=(CKey&&) noexcept = default;
+
+    CKey& operator=(const CKey& other)
     {
-        // Important: vch must be 32 bytes in length to not break serialization
-        keydata.resize(32);
+        if (other.keydata) {
+            MakeKeyData();
+            *keydata = *other.keydata;
+        } else {
+            ClearKeyData();
+        }
+        fCompressed = other.fCompressed;
+        return *this;
     }
 
+    CKey(const CKey& other) { *this = other; }
+
     friend bool operator==(const CKey& a, const CKey& b)
     {
         return a.fCompressed == b.fCompressed &&
             a.size() == b.size() &&
-            memcmp(a.keydata.data(), b.keydata.data(), a.size()) == 0;
+            memcmp(a.data(), b.data(), a.size()) == 0;
     }
 
     //! Initialize using begin and end iterators to byte data.
     template <typename T>
     void Set(const T pbegin, const T pend, bool fCompressedIn)
     {
-        if (size_t(pend - pbegin) != keydata.size()) {
-            fValid = false;
+        if (size_t(pend - pbegin) != std::tuple_size_v<KeyType>) {
+            ClearKeyData();
         } else if (Check(&pbegin[0])) {
-            memcpy(keydata.data(), (unsigned char*)&pbegin[0], keydata.size());
-            fValid = true;
+            MakeKeyData();
+            memcpy(keydata->data(), (unsigned char*)&pbegin[0], keydata->size());
             fCompressed = fCompressedIn;
         } else {
-            fValid = false;
+            ClearKeyData();
         }
     }
 
     //! Simple read-only vector-like interface.
-    unsigned int size() const { return (fValid ? keydata.size() : 0); }
-    const std::byte* data() const { return reinterpret_cast<const std::byte*>(keydata.data()); }
-    const unsigned char* begin() const { return keydata.data(); }
-    const unsigned char* end() const { return keydata.data() + size(); }
+    unsigned int size() const { return keydata ? keydata->size() : 0; }
+    const std::byte* data() const { return keydata ? reinterpret_cast<const std::byte*>(keydata->data()) : nullptr; }
+    const unsigned char* begin() const { return keydata ? keydata->data() : nullptr; }
+    const unsigned char* end() const { return begin() + size(); }
 
     //! Check whether this private key is valid.
-    bool IsValid() const { return fValid; }
+    bool IsValid() const { return !!keydata; }
 
     //! Check whether the public key corresponding to this private key is (to be) compressed.
     bool IsCompressed() const { return fCompressed; }