diff --git a/src/dashbls/include/dashbls/chaincode.hpp b/src/dashbls/include/dashbls/chaincode.hpp index bd3e57b4d779..146df7f13f44 100644 --- a/src/dashbls/include/dashbls/chaincode.hpp +++ b/src/dashbls/include/dashbls/chaincode.hpp @@ -38,6 +38,8 @@ class ChainCode { static ChainCode FromBytes(const Bytes& bytes); ChainCode(const ChainCode &cc); + ChainCode& operator=(const ChainCode& cc); + ~ChainCode(); // Comparator implementation. friend bool operator==(ChainCode const &a, ChainCode const &b); @@ -48,7 +50,7 @@ class ChainCode { std::vector Serialize() const; // Prevent direct construction, use static constructor - ChainCode() {} + ChainCode(); private: bn_t chainCode; @@ -56,4 +58,3 @@ class ChainCode { } // end namespace bls #endif // SRC_CHAINCODE_HPP_ - diff --git a/src/dashbls/include/dashbls/util.hpp b/src/dashbls/include/dashbls/util.hpp index 37fd8547a983..8422757e0cf6 100644 --- a/src/dashbls/include/dashbls/util.hpp +++ b/src/dashbls/include/dashbls/util.hpp @@ -95,6 +95,17 @@ class Util { secureFreeCallback(ptr); } + /* + * Overwrite sensitive memory through a volatile pointer so the writes are + * not removed as dead stores by the compiler. + */ + static void SecureWipe(void* ptr, size_t size) noexcept { + volatile uint8_t* bytes = static_cast(ptr); + while (size-- > 0) { + *bytes++ = 0; + } + } + /* * Converts one hex character to an int. */ diff --git a/src/dashbls/src/chaincode.cpp b/src/dashbls/src/chaincode.cpp index efacdc95ae29..75c493ff1e04 100644 --- a/src/dashbls/src/chaincode.cpp +++ b/src/dashbls/src/chaincode.cpp @@ -20,17 +20,44 @@ ChainCode ChainCode::FromBytes(const Bytes& bytes) { if (bytes.size() != ChainCode::SIZE) { throw std::invalid_argument("ChainCode::FromBytes: Invalid size"); } - ChainCode c = ChainCode(); - bn_new(c.chainCode); + ChainCode c; bn_read_bin(c.chainCode, bytes.begin(), ChainCode::SIZE); return c; } -ChainCode::ChainCode(const ChainCode &cc) { - uint8_t bytes[ChainCode::SIZE]; - cc.Serialize(bytes); +ChainCode::ChainCode() { + bn_null(chainCode); bn_new(chainCode); - bn_read_bin(chainCode, bytes, ChainCode::SIZE); + bn_zero(chainCode); +} + +ChainCode::ChainCode(const ChainCode &cc) : ChainCode() { + bn_copy(chainCode, cc.chainCode); +} + +ChainCode& ChainCode::operator=(const ChainCode& cc) { + if (this != &cc) { +#if ALLOC == DYNAMIC + if (chainCode->dp != nullptr && chainCode->alloc > 0) { + Util::SecureWipe(chainCode->dp, chainCode->alloc * sizeof(dig_t)); + } +#elif ALLOC == AUTO + Util::SecureWipe(chainCode->dp, sizeof(chainCode->dp)); +#endif + bn_copy(chainCode, cc.chainCode); + } + return *this; +} + +ChainCode::~ChainCode() { +#if ALLOC == DYNAMIC + if (chainCode != nullptr && chainCode->dp != nullptr && chainCode->alloc > 0) { + Util::SecureWipe(chainCode->dp, chainCode->alloc * sizeof(dig_t)); + } + bn_free(chainCode); +#elif ALLOC == AUTO + Util::SecureWipe(chainCode, sizeof(chainCode)); +#endif } // Comparator implementation. diff --git a/src/dashbls/src/extendedprivatekey.cpp b/src/dashbls/src/extendedprivatekey.cpp index d5fe746ee690..7ed3b3af8308 100644 --- a/src/dashbls/src/extendedprivatekey.cpp +++ b/src/dashbls/src/extendedprivatekey.cpp @@ -13,47 +13,95 @@ // limitations under the License. #include +#include #include "bls.hpp" namespace bls { +namespace { + +class SecureBytes { + public: + explicit SecureBytes(size_t size) : m_size(size), m_data(Util::SecAlloc(size)) { + if (m_data == nullptr) { + throw std::bad_alloc(); + } + } + + ~SecureBytes() { + Util::SecureWipe(m_data, m_size); + Util::SecFree(m_data); + } + + SecureBytes(const SecureBytes&) = delete; + SecureBytes& operator=(const SecureBytes&) = delete; + + uint8_t* data() { return m_data; } + + private: + const size_t m_size; + uint8_t* const m_data; +}; + +class SecureRelicBn { + public: + SecureRelicBn() { + bn_null(m_value); + bn_new(m_value); + } + + ~SecureRelicBn() { +#if ALLOC == DYNAMIC + if (m_value != nullptr && m_value->dp != nullptr && m_value->alloc > 0) { + Util::SecureWipe(m_value->dp, m_value->alloc * sizeof(dig_t)); + } + bn_free(m_value); +#elif ALLOC == AUTO + Util::SecureWipe(m_value, sizeof(m_value)); +#endif + } + + SecureRelicBn(const SecureRelicBn&) = delete; + SecureRelicBn& operator=(const SecureRelicBn&) = delete; + + bn_st* get() { return m_value; } + + private: + bn_t m_value; +}; + +} // namespace ExtendedPrivateKey ExtendedPrivateKey::FromSeed(const Bytes& bytes) { // "BLS HD seed" in ascii const uint8_t prefix[] = {66, 76, 83, 32, 72, 68, 32, 115, 101, 101, 100}; - uint8_t* hashInput = Util::SecAlloc(bytes.size() + 1); - std::memcpy(hashInput, bytes.begin(), bytes.size()); + SecureBytes hashInput(bytes.size() + 1); + std::memcpy(hashInput.data(), bytes.begin(), bytes.size()); // 32 bytes for secret key, and 32 bytes for chaincode - uint8_t* ILeft = Util::SecAlloc( - PrivateKey::PRIVATE_KEY_SIZE); - uint8_t IRight[ChainCode::SIZE]; + SecureBytes ILeft(PrivateKey::PRIVATE_KEY_SIZE); + SecureBytes IRight(ChainCode::SIZE); // Hash the seed into 64 bytes, half will be sk, half will be cc - hashInput[bytes.size()] = 0; - md_hmac(ILeft, hashInput, bytes.size() + 1, prefix, sizeof(prefix)); + hashInput.data()[bytes.size()] = 0; + md_hmac(ILeft.data(), hashInput.data(), bytes.size() + 1, prefix, sizeof(prefix)); - hashInput[bytes.size()] = 1; - md_hmac(IRight, hashInput, bytes.size() + 1, prefix, sizeof(prefix)); + hashInput.data()[bytes.size()] = 1; + md_hmac(IRight.data(), hashInput.data(), bytes.size() + 1, prefix, sizeof(prefix)); // Make sure private key is less than the curve order - bn_t* skBn = Util::SecAlloc(1); - bn_t order; - bn_new(order); - g1_get_ord(order); + SecureRelicBn skBn; + SecureRelicBn order; + g1_get_ord(order.get()); - bn_new(*skBn); - bn_read_bin(*skBn, ILeft, PrivateKey::PRIVATE_KEY_SIZE); - bn_mod_basic(*skBn, *skBn, order); - bn_write_bin(ILeft, PrivateKey::PRIVATE_KEY_SIZE, *skBn); + bn_read_bin(skBn.get(), ILeft.data(), PrivateKey::PRIVATE_KEY_SIZE); + bn_mod_basic(skBn.get(), skBn.get(), order.get()); + bn_write_bin(ILeft.data(), PrivateKey::PRIVATE_KEY_SIZE, skBn.get()); ExtendedPrivateKey esk(ExtendedPublicKey::REVISION, 0, 0, 0, - ChainCode::FromBytes(Bytes(IRight, ChainCode::SIZE)), - PrivateKey::FromBytes(Bytes(ILeft, PrivateKey::PRIVATE_KEY_SIZE))); + ChainCode::FromBytes(Bytes(IRight.data(), ChainCode::SIZE)), + PrivateKey::FromBytes(Bytes(ILeft.data(), PrivateKey::PRIVATE_KEY_SIZE))); - Util::SecFree(skBn); - Util::SecFree(ILeft); - Util::SecFree(hashInput); return esk; } @@ -79,48 +127,45 @@ ExtendedPrivateKey ExtendedPrivateKey::PrivateChild(uint32_t i, const bool fLega uint32_t cmp = (1 << 31); bool hardened = i >= cmp; - uint8_t* ILeft = Util::SecAlloc(PrivateKey::PRIVATE_KEY_SIZE); - uint8_t IRight[ChainCode::SIZE]; + SecureBytes ILeft(PrivateKey::PRIVATE_KEY_SIZE); + SecureBytes IRight(ChainCode::SIZE); // Chain code is used as hmac key - uint8_t hmacKey[ChainCode::SIZE]; - chainCode.Serialize(hmacKey); + SecureBytes hmacKey(ChainCode::SIZE); + chainCode.Serialize(hmacKey.data()); size_t inputLen = hardened ? PrivateKey::PRIVATE_KEY_SIZE + 4 + 1 : G1Element::SIZE + 4 + 1; // Hmac input includes sk or pk, int i, and byte with 0 or 1 - uint8_t* hmacInput = Util::SecAlloc(inputLen); + SecureBytes hmacInput(inputLen); // Fill the input with the required data if (hardened) { - sk.Serialize(hmacInput); - Util::IntToFourBytes(hmacInput + PrivateKey::PRIVATE_KEY_SIZE, i); + sk.Serialize(hmacInput.data()); + Util::IntToFourBytes(hmacInput.data() + PrivateKey::PRIVATE_KEY_SIZE, i); } else { - memcpy(hmacInput, sk.GetG1Element().Serialize(fLegacy).data(), G1Element::SIZE); - Util::IntToFourBytes(hmacInput + G1Element::SIZE, i); + memcpy(hmacInput.data(), sk.GetG1Element().Serialize(fLegacy).data(), G1Element::SIZE); + Util::IntToFourBytes(hmacInput.data() + G1Element::SIZE, i); } - hmacInput[inputLen - 1] = 0; + hmacInput.data()[inputLen - 1] = 0; - md_hmac(ILeft, hmacInput, inputLen, - hmacKey, ChainCode::SIZE); + md_hmac(ILeft.data(), hmacInput.data(), inputLen, + hmacKey.data(), ChainCode::SIZE); // Change 1 byte to generate a different sequence for chaincode - hmacInput[inputLen - 1] = 1; + hmacInput.data()[inputLen - 1] = 1; - md_hmac(IRight, hmacInput, inputLen, - hmacKey, ChainCode::SIZE); + md_hmac(IRight.data(), hmacInput.data(), inputLen, + hmacKey.data(), ChainCode::SIZE); - PrivateKey newSk = PrivateKey::FromBytes(Bytes(ILeft, PrivateKey::PRIVATE_KEY_SIZE), true); + PrivateKey newSk = PrivateKey::FromBytes(Bytes(ILeft.data(), PrivateKey::PRIVATE_KEY_SIZE), true); newSk = PrivateKey::Aggregate({sk, newSk}); ExtendedPrivateKey esk(version, depth + 1, sk.GetG1Element().GetFingerprint(), i, - ChainCode::FromBytes(Bytes(IRight, ChainCode::SIZE)), + ChainCode::FromBytes(Bytes(IRight.data(), ChainCode::SIZE)), newSk); - Util::SecFree(ILeft); - Util::SecFree(hmacInput); - return esk; } diff --git a/src/dashbls/src/privatekey.cpp b/src/dashbls/src/privatekey.cpp index d07246d8575e..90cf2f535984 100644 --- a/src/dashbls/src/privatekey.cpp +++ b/src/dashbls/src/privatekey.cpp @@ -16,6 +16,20 @@ #include "legacy.hpp" namespace bls { +namespace { + +void SecureWipePrivateKey(bn_st* keydata) noexcept +{ +#if ALLOC == DYNAMIC + if (keydata->dp != nullptr && keydata->alloc > 0) { + Util::SecureWipe(keydata->dp, keydata->alloc * sizeof(dig_t)); + } +#elif ALLOC == AUTO + Util::SecureWipe(keydata->dp, sizeof(keydata->dp)); +#endif +} + +} // namespace const size_t PrivateKey::PRIVATE_KEY_SIZE; @@ -119,6 +133,7 @@ PrivateKey::~PrivateKey() void PrivateKey::DeallocateKeyData() { if(keydata != nullptr) { + SecureWipePrivateKey(keydata); Util::SecFree(keydata); keydata = nullptr; } @@ -133,9 +148,13 @@ void PrivateKey::InvalidateCaches() PrivateKey& PrivateKey::operator=(const PrivateKey& other) { + if (this == &other) { + return *this; + } CheckKeyData(); other.CheckKeyData(); InvalidateCaches(); + SecureWipePrivateKey(keydata); bn_copy(keydata, other.keydata); return *this; } diff --git a/src/dashbls/src/test.cpp b/src/dashbls/src/test.cpp index 7e9d5280b3b5..03e139d66323 100644 --- a/src/dashbls/src/test.cpp +++ b/src/dashbls/src/test.cpp @@ -14,6 +14,7 @@ // limitations under the License. #define CATCH_CONFIG_RUNNER +#include #include #include "bls.hpp" @@ -73,6 +74,8 @@ TEST_CASE("class PrivateKey") { REQUIRE(pk1.GetG1Element() == pk2.GetG1Element()); REQUIRE(pk1.GetG2Element() == pk2.GetG2Element()); REQUIRE(pk3 != pk2); + pk2 = pk2; + REQUIRE(pk1 == pk2); } SECTION("Move {constructor|assignment operator}") { PrivateKey pk1 = PrivateKey::RandomPrivateKey(); @@ -1267,6 +1270,32 @@ TEST_CASE("Schemes") { } TEST_CASE("Legacy HD keys") { + SECTION("Chain code copy construction and assignment preserve value") { + std::array first{}; + std::array second{}; + first.front() = 1; + second.back() = 2; + + const ChainCode source = ChainCode::FromBytes(Bytes(first)); + ChainCode copy{source}; + ChainCode assigned = ChainCode::FromBytes(Bytes(second)); + + assigned = source; + REQUIRE(copy == source); + REQUIRE(assigned == source); + + assigned = assigned; + REQUIRE(assigned == source); + +#if ALLOC == AUTO + alignas(ChainCode) std::array storage; + storage.fill(0xa5); + auto* stored = new (storage.data()) ChainCode{source}; + stored->~ChainCode(); + REQUIRE(std::all_of(storage.begin(), storage.end(), [](uint8_t byte) { return byte == 0; })); +#endif + } + SECTION("Should create an extended private key from seed") { std::vector seed{1, 50, 6, 244, 24, 199, 1, 25}; ExtendedPrivateKey esk = ExtendedPrivateKey::FromSeed(Bytes(seed));