phantom/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-01-10 03:47:29 -03:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

Merge bitcoin/bitcoin#28500: Prevent default/invalid CKey objects from allocating secure memory

Browse source 
6ef405ddb1 key: don't allocate secure mem for null (invalid) key (Pieter Wuille)
d9841a7ac6 Add make_secure_unique helper (Anthony Towns)

Pull request description:

  Bitcoin Core has `secure_allocator`, which allocates inside special "secure" (non-swappable) memory pages, which may be limited in availability. Currently, every `CKey` object uses 32 such secure bytes, even when the `CKey` object contains the (invalid) value zero.

  Change this to not use memory when the `CKey` is invalid. This is particularly relevant for `BIP324Cipher` which briefly holds a `CKey`, but after receiving the remote's public key and initializing the encryption ciphers, the key is wiped. In case secure memory usage is in high demand, it'd be silly to waste it on P2P encryption keys instead of wallet keys.

ACKs for top commit:
  ajtowns:
    ACK 6ef405ddb1
  john-moffett:
    ACK 6ef405ddb1

Tree-SHA512: 987f4376ed825daf034ea4d7c4b4952fe664b25b48f1c09fbcfa6257a40b06c4da7c2caaafa35c346c86bdf298ae21f16c68ea4b1039836990d1a205de2034fd
This commit is contained in:
fanquake 2023-10-02 11:58:48 +02:00
commit 0f9307c4cb
No known key found for this signature in database
GPG key ID: 2EEB9F5CC09526C1
3 changed files with 84 additions and 39 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

37
src/key.cpp
View file

@ -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);

62
src/key.h
View file

@ -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);
public:
//! Construct an invalid private key.
CKey()
void MakeKeyData()
{
// Important: vch must be 32 bytes in length to not break serialization
keydata.resize(32);
if (!keydata) keydata = make_secure_unique<KeyType>();
}
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)
{
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; }

24
src/support/allocators/secure.h
View file

@ -57,4 +57,28 @@ struct secure_allocator {
// TODO: Consider finding a way to make incoming RPC request.params[i] mlock()ed as well
typedef std::basic_string<char, std::char_traits<char>, secure_allocator<char> > SecureString;
template<typename T>
struct SecureUniqueDeleter {
void operator()(T* t) noexcept {
secure_allocator<T>().deallocate(t, 1);
}
};
template<typename T>
using secure_unique_ptr = std::unique_ptr<T, SecureUniqueDeleter<T>>;
template<typename T, typename... Args>
secure_unique_ptr<T> make_secure_unique(Args&&... as)
{
T* p = secure_allocator<T>().allocate(1);
// initialize in place, and return as secure_unique_ptr
try {
return secure_unique_ptr<T>(new (p) T(std::forward(as)...));
} catch (...) {
secure_allocator<T>().deallocate(p, 1);
throw;
}
}
#endif // BITCOIN_SUPPORT_ALLOCATORS_SECURE_H