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.
This commit is contained in:
Pieter Wuille
2023-09-18 12:10:19 -04:00
parent d9841a7ac6
commit 6ef405ddb1
2 changed files with 60 additions and 39 deletions

View File

@ -159,21 +159,21 @@ bool CKey::Check(const unsigned char *vch) {
} }
void CKey::MakeNewKey(bool fCompressedIn) { void CKey::MakeNewKey(bool fCompressedIn) {
MakeKeyData();
do { do {
GetStrongRandBytes(keydata); GetStrongRandBytes(*keydata);
} while (!Check(keydata.data())); } while (!Check(keydata->data()));
fValid = true;
fCompressed = fCompressedIn; fCompressed = fCompressedIn;
} }
bool CKey::Negate() bool CKey::Negate()
{ {
assert(fValid); assert(keydata);
return secp256k1_ec_seckey_negate(secp256k1_context_sign, keydata.data()); return secp256k1_ec_seckey_negate(secp256k1_context_sign, keydata->data());
} }
CPrivKey CKey::GetPrivKey() const { CPrivKey CKey::GetPrivKey() const {
assert(fValid); assert(keydata);
CPrivKey seckey; CPrivKey seckey;
int ret; int ret;
size_t seckeylen; size_t seckeylen;
@ -186,7 +186,7 @@ CPrivKey CKey::GetPrivKey() const {
} }
CPubKey CKey::GetPubKey() const { CPubKey CKey::GetPubKey() const {
assert(fValid); assert(keydata);
secp256k1_pubkey pubkey; secp256k1_pubkey pubkey;
size_t clen = CPubKey::SIZE; size_t clen = CPubKey::SIZE;
CPubKey result; 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 { bool CKey::Sign(const uint256 &hash, std::vector<unsigned char>& vchSig, bool grind, uint32_t test_case) const {
if (!fValid) if (!keydata)
return false; return false;
vchSig.resize(CPubKey::SIGNATURE_SIZE); vchSig.resize(CPubKey::SIGNATURE_SIZE);
size_t nSigLen = 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 { bool CKey::SignCompact(const uint256 &hash, std::vector<unsigned char>& vchSig) const {
if (!fValid) if (!keydata)
return false; return false;
vchSig.resize(CPubKey::COMPACT_SIGNATURE_SIZE); vchSig.resize(CPubKey::COMPACT_SIGNATURE_SIZE);
int rec = -1; 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) { 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; return false;
}
fCompressed = vchPubKey.IsCompressed(); fCompressed = vchPubKey.IsCompressed();
fValid = true;
if (fSkipCheck) if (fSkipCheck)
return true; return true;
@ -325,22 +327,21 @@ bool CKey::Derive(CKey& keyChild, ChainCode &ccChild, unsigned int nChild, const
BIP32Hash(cc, nChild, 0, begin(), vout.data()); BIP32Hash(cc, nChild, 0, begin(), vout.data());
} }
memcpy(ccChild.begin(), vout.data()+32, 32); 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()); bool ret = secp256k1_ec_seckey_tweak_add(secp256k1_context_sign, (unsigned char*)keyChild.begin(), vout.data());
keyChild.fCompressed = true; if (!ret) keyChild.ClearKeyData();
keyChild.fValid = ret;
return ret; return ret;
} }
EllSwiftPubKey CKey::EllSwiftCreate(Span<const std::byte> ent32) const EllSwiftPubKey CKey::EllSwiftCreate(Span<const std::byte> ent32) const
{ {
assert(fValid); assert(keydata);
assert(ent32.size() == 32); assert(ent32.size() == 32);
std::array<std::byte, EllSwiftPubKey::size()> encoded_pubkey; std::array<std::byte, EllSwiftPubKey::size()> encoded_pubkey;
auto success = secp256k1_ellswift_create(secp256k1_context_sign, auto success = secp256k1_ellswift_create(secp256k1_context_sign,
UCharCast(encoded_pubkey.data()), UCharCast(encoded_pubkey.data()),
keydata.data(), keydata->data(),
UCharCast(ent32.data())); UCharCast(ent32.data()));
// Should always succeed for valid keys (asserted above). // 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 ECDHSecret CKey::ComputeBIP324ECDHSecret(const EllSwiftPubKey& their_ellswift, const EllSwiftPubKey& our_ellswift, bool initiating) const
{ {
assert(fValid); assert(keydata);
ECDHSecret output; ECDHSecret output;
// BIP324 uses the initiator as party A, and the responder as party B. Remap the inputs // 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(output.data()),
UCharCast(initiating ? our_ellswift.data() : their_ellswift.data()), UCharCast(initiating ? our_ellswift.data() : their_ellswift.data()),
UCharCast(initiating ? their_ellswift.data() : our_ellswift.data()), UCharCast(initiating ? their_ellswift.data() : our_ellswift.data()),
keydata.data(), keydata->data(),
initiating ? 0 : 1, initiating ? 0 : 1,
secp256k1_ellswift_xdh_hash_function_bip324, secp256k1_ellswift_xdh_hash_function_bip324,
nullptr); nullptr);

View File

@ -46,57 +46,77 @@ public:
"COMPRESSED_SIZE is larger than SIZE"); "COMPRESSED_SIZE is larger than SIZE");
private: private:
//! Whether this private key is valid. We check for correctness when modifying the key /** Internal data container for private key material. */
//! data, so fValid should always correspond to the actual state. using KeyType = std::array<unsigned char, 32>;
bool fValid{false};
//! Whether the public key corresponding to this private key is (to be) compressed. //! Whether the public key corresponding to this private key is (to be) compressed.
bool fCompressed{false}; bool fCompressed{false};
//! The actual byte data //! The actual byte data. nullptr for invalid keys.
std::vector<unsigned char, secure_allocator<unsigned char> > keydata; secure_unique_ptr<KeyType> keydata;
//! Check whether the 32-byte array pointed to by vch is valid keydata. //! Check whether the 32-byte array pointed to by vch is valid keydata.
bool static Check(const unsigned char* vch); bool static Check(const unsigned char* vch);
public: void MakeKeyData()
//! Construct an invalid private key.
CKey()
{ {
// Important: vch must be 32 bytes in length to not break serialization if (!keydata) keydata = make_secure_unique<KeyType>();
keydata.resize(32);
} }
void ClearKeyData()
{
keydata.reset();
}
public:
CKey() noexcept = default;
CKey(CKey&&) noexcept = default;
CKey& operator=(CKey&&) noexcept = default;
CKey& operator=(const CKey& other)
{
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) friend bool operator==(const CKey& a, const CKey& b)
{ {
return a.fCompressed == b.fCompressed && return a.fCompressed == b.fCompressed &&
a.size() == b.size() && 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. //! Initialize using begin and end iterators to byte data.
template <typename T> template <typename T>
void Set(const T pbegin, const T pend, bool fCompressedIn) void Set(const T pbegin, const T pend, bool fCompressedIn)
{ {
if (size_t(pend - pbegin) != keydata.size()) { if (size_t(pend - pbegin) != std::tuple_size_v<KeyType>) {
fValid = false; ClearKeyData();
} else if (Check(&pbegin[0])) { } else if (Check(&pbegin[0])) {
memcpy(keydata.data(), (unsigned char*)&pbegin[0], keydata.size()); MakeKeyData();
fValid = true; memcpy(keydata->data(), (unsigned char*)&pbegin[0], keydata->size());
fCompressed = fCompressedIn; fCompressed = fCompressedIn;
} else { } else {
fValid = false; ClearKeyData();
} }
} }
//! Simple read-only vector-like interface. //! Simple read-only vector-like interface.
unsigned int size() const { return (fValid ? keydata.size() : 0); } unsigned int size() const { return keydata ? keydata->size() : 0; }
const std::byte* data() const { return reinterpret_cast<const std::byte*>(keydata.data()); } const std::byte* data() const { return keydata ? reinterpret_cast<const std::byte*>(keydata->data()) : nullptr; }
const unsigned char* begin() const { return keydata.data(); } const unsigned char* begin() const { return keydata ? keydata->data() : nullptr; }
const unsigned char* end() const { return keydata.data() + size(); } const unsigned char* end() const { return begin() + size(); }
//! Check whether this private key is valid. //! 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. //! Check whether the public key corresponding to this private key is (to be) compressed.
bool IsCompressed() const { return fCompressed; } bool IsCompressed() const { return fCompressed; }