Skip to content

Commit

Permalink
build: fix build with Boost 1.85 and remove instances of viewkey logg…
Browse files Browse the repository at this point in the history
…ing [RELEASE]

1. Use std::is_standard_layout and std::is_trivially_copyable instead of std::is_pod for KV byte-wise serialization, which fixes compile issue for Boost UUIDs
2. Removed reimplementation of std::hash for boost::uuids::uuid
3. Removed << operator overload for crypto::secret_key
4. Removed instances in code where private view key was dumped to the log in plaintext

Release version of #9450, containing C++14 modified assertions
  • Loading branch information
jeffro256 committed Sep 10, 2024
1 parent b089f9e commit 65568d3
Show file tree
Hide file tree
Showing 20 changed files with 80 additions and 69 deletions.
2 changes: 2 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1077,6 +1077,8 @@ if(STATIC)
endif()
find_package(Boost 1.58 QUIET REQUIRED COMPONENTS system filesystem thread date_time chrono regex serialization program_options locale)
add_definitions(-DBOOST_ASIO_ENABLE_SEQUENTIAL_STRAND_ALLOCATION)
add_definitions(-DBOOST_NO_AUTO_PTR)
add_definitions(-DBOOST_UUID_DISABLE_ALIGNMENT) # This restores UUID's std::has_unique_object_representations property

set(CMAKE_FIND_LIBRARY_SUFFIXES ${OLD_LIB_SUFFIXES})
if(NOT Boost_FOUND)
Expand Down
18 changes: 10 additions & 8 deletions contrib/epee/include/serialization/keyvalue_serialization.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,16 +98,18 @@ public: \
#define KV_SERIALIZE_VAL_POD_AS_BLOB_FORCE_N(varialble, val_name) \
epee::serialization::selector<is_store>::serialize_t_val_as_blob(this_ref.varialble, stg, hparent_section, val_name);

#define KV_SERIALIZE_VAL_POD_AS_BLOB_N(varialble, val_name) \
static_assert(std::is_pod<decltype(this_ref.varialble)>::value, "t_type must be a POD type."); \
KV_SERIALIZE_VAL_POD_AS_BLOB_FORCE_N(varialble, val_name)
#define KV_SERIALIZE_VAL_POD_AS_BLOB_N(variable, val_name) \
static_assert(std::is_trivially_copyable<decltype(this_ref.variable)>(), "t_type must be a trivially copyable type."); \
static_assert(std::is_standard_layout<decltype(this_ref.variable)>(), "t_type must be a standard layout type."); \
KV_SERIALIZE_VAL_POD_AS_BLOB_FORCE_N(variable, val_name)

#define KV_SERIALIZE_VAL_POD_AS_BLOB_OPT_N(varialble, val_name, default_value) \
#define KV_SERIALIZE_VAL_POD_AS_BLOB_OPT_N(variable, val_name, default_value) \
do { \
static_assert(std::is_pod<decltype(this_ref.varialble)>::value, "t_type must be a POD type."); \
bool ret = KV_SERIALIZE_VAL_POD_AS_BLOB_FORCE_N(varialble, val_name); \
static_assert(std::is_trivially_copyable<decltype(this_ref.variable)>(), "t_type must be a trivially copyable type."); \
static_assert(std::is_standard_layout<decltype(this_ref.variable)>(), "t_type must be a standard layout type."); \
bool ret = KV_SERIALIZE_VAL_POD_AS_BLOB_FORCE_N(variable, val_name) \
if (!ret) \
epee::serialize_default(this_ref.varialble, default_value); \
epee::serialize_default(this_ref.variable, default_value); \
} while(0);

#define KV_SERIALIZE_CONTAINER_POD_AS_BLOB_N(varialble, val_name) \
Expand All @@ -118,7 +120,7 @@ public: \
#define KV_SERIALIZE(varialble) KV_SERIALIZE_N(varialble, #varialble)
#define KV_SERIALIZE_VAL_POD_AS_BLOB(varialble) KV_SERIALIZE_VAL_POD_AS_BLOB_N(varialble, #varialble)
#define KV_SERIALIZE_VAL_POD_AS_BLOB_OPT(varialble, def) KV_SERIALIZE_VAL_POD_AS_BLOB_OPT_N(varialble, #varialble, def)
#define KV_SERIALIZE_VAL_POD_AS_BLOB_FORCE(varialble) KV_SERIALIZE_VAL_POD_AS_BLOB_FORCE_N(varialble, #varialble) //skip is_pod compile time check
#define KV_SERIALIZE_VAL_POD_AS_BLOB_FORCE(varialble) KV_SERIALIZE_VAL_POD_AS_BLOB_FORCE_N(varialble, #varialble) //skip is_trivially_copyable and is_standard_layout compile time check
#define KV_SERIALIZE_CONTAINER_POD_AS_BLOB(varialble) KV_SERIALIZE_CONTAINER_POD_AS_BLOB_N(varialble, #varialble)
#define KV_SERIALIZE_OPT(variable,default_value) KV_SERIALIZE_OPT_N(variable, #variable, default_value)

Expand Down
23 changes: 13 additions & 10 deletions contrib/epee/include/span.h
Original file line number Diff line number Diff line change
Expand Up @@ -133,17 +133,14 @@ namespace epee
return {src.data(), src.size()};
}

template<typename T>
constexpr bool has_padding() noexcept
{
return !std::is_standard_layout<T>() || alignof(T) != 1;
}

//! \return Cast data from `src` as `span<const std::uint8_t>`.
template<typename T>
span<const std::uint8_t> to_byte_span(const span<const T> src) noexcept
{
static_assert(!has_padding<T>(), "source type may have padding");
static_assert(!std::is_empty<T>(), "empty value types will not work -> sizeof == 1");
static_assert(std::is_standard_layout<T>(), "type must have standard layout");
static_assert(std::is_trivially_copyable<T>(), "type must be trivially copyable");
static_assert(alignof(T) == 1, "type may have padding");
return {reinterpret_cast<const std::uint8_t*>(src.data()), src.size_bytes()};
}

Expand All @@ -153,7 +150,9 @@ namespace epee
{
using value_type = typename T::value_type;
static_assert(!std::is_empty<value_type>(), "empty value types will not work -> sizeof == 1");
static_assert(!has_padding<value_type>(), "source value type may have padding");
static_assert(std::is_standard_layout<value_type>(), "value type must have standard layout");
static_assert(std::is_trivially_copyable<value_type>(), "value type must be trivially copyable");
static_assert(alignof(value_type) == 1, "value type may have padding");
return {reinterpret_cast<std::uint8_t*>(src.data()), src.size() * sizeof(value_type)};
}

Expand All @@ -162,7 +161,9 @@ namespace epee
span<const std::uint8_t> as_byte_span(const T& src) noexcept
{
static_assert(!std::is_empty<T>(), "empty types will not work -> sizeof == 1");
static_assert(!has_padding<T>(), "source type may have padding");
static_assert(std::is_standard_layout<T>(), "type must have standard layout");
static_assert(std::is_trivially_copyable<T>(), "type must be trivially copyable");
static_assert(alignof(T) == 1, "type may have padding");
return {reinterpret_cast<const std::uint8_t*>(std::addressof(src)), sizeof(T)};
}

Expand All @@ -171,7 +172,9 @@ namespace epee
span<std::uint8_t> as_mut_byte_span(T& src) noexcept
{
static_assert(!std::is_empty<T>(), "empty types will not work -> sizeof == 1");
static_assert(!has_padding<T>(), "source type may have padding");
static_assert(std::is_standard_layout<T>(), "type must have standard layout");
static_assert(std::is_trivially_copyable<T>(), "type must be trivially copyable");
static_assert(alignof(T) == 1, "type may have padding");
return {reinterpret_cast<std::uint8_t*>(std::addressof(src)), sizeof(T)};
}

Expand Down
3 changes: 3 additions & 0 deletions contrib/epee/include/string_tools.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,13 +91,16 @@ namespace string_tools
std::string pod_to_hex(const t_pod_type& s)
{
static_assert(std::is_standard_layout<t_pod_type>(), "expected standard layout type");
static_assert(alignof(t_pod_type) == 1, "type may have padding");
return to_hex::string(as_byte_span(s));
}
//----------------------------------------------------------------------------
template<class t_pod_type>
bool hex_to_pod(const boost::string_ref hex_str, t_pod_type& s)
{
static_assert(std::is_standard_layout<t_pod_type>(), "expected standard layout type");
static_assert(alignof(t_pod_type) == 1, "type may have padding");
static_assert(std::is_trivially_copyable<t_pod_type>(), "type must be trivially copyable");
return from_hex::to_buffer(as_mut_byte_span(s), hex_str);
}
//----------------------------------------------------------------------------
Expand Down
14 changes: 11 additions & 3 deletions src/crypto/crypto.h
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,9 @@ namespace crypto {
/* Generate a value filled with random bytes.
*/
template<typename T>
typename std::enable_if<std::is_pod<T>::value, T>::type rand() {
T rand() {
static_assert(std::is_standard_layout<T>(), "cannot write random bytes into non-standard layout type");
static_assert(std::is_trivially_copyable<T>(), "cannot write random bytes into non-trivially copyable type");
typename std::remove_cv<T>::type res;
generate_random_bytes_thread_safe(sizeof(T), (uint8_t*)&res);
return res;
Expand Down Expand Up @@ -314,8 +316,14 @@ namespace crypto {
inline std::ostream &operator <<(std::ostream &o, const crypto::public_key &v) {
epee::to_hex::formatted(o, epee::as_byte_span(v)); return o;
}
inline std::ostream &operator <<(std::ostream &o, const crypto::secret_key &v) {
epee::to_hex::formatted(o, epee::as_byte_span(v)); return o;
/* Do NOT overload the << operator for crypto::secret_key here. Use secret_key_explicit_print_ref
* instead to prevent accidental implicit dumping of secret key material to the logs (which has
* happened before). For the same reason, do not overload it for crypto::ec_scalar either since
* crypto::secret_key is a subclass. I'm not sorry that it's obtuse; that's the point, bozo.
*/
struct secret_key_explicit_print_ref { const crypto::secret_key &sk; };
inline std::ostream &operator <<(std::ostream &o, const secret_key_explicit_print_ref v) {
epee::to_hex::formatted(o, epee::as_byte_span(unwrap(unwrap(v.sk)))); return o;
}
inline std::ostream &operator <<(std::ostream &o, const crypto::key_derivation &v) {
epee::to_hex::formatted(o, epee::as_byte_span(v)); return o;
Expand Down
4 changes: 2 additions & 2 deletions src/cryptonote_basic/cryptonote_format_utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ namespace cryptonote
bool r = hwdev.generate_key_derivation(tx_public_key, ack.m_view_secret_key, recv_derivation);
if (!r)
{
MWARNING("key image helper: failed to generate_key_derivation(" << tx_public_key << ", " << ack.m_view_secret_key << ")");
MWARNING("key image helper: failed to generate_key_derivation(" << tx_public_key << ", <viewkey>)");
memcpy(&recv_derivation, rct::identity().bytes, sizeof(recv_derivation));
}

Expand All @@ -303,7 +303,7 @@ namespace cryptonote
r = hwdev.generate_key_derivation(additional_tx_public_keys[i], ack.m_view_secret_key, additional_recv_derivation);
if (!r)
{
MWARNING("key image helper: failed to generate_key_derivation(" << additional_tx_public_keys[i] << ", " << ack.m_view_secret_key << ")");
MWARNING("key image helper: failed to generate_key_derivation(" << additional_tx_public_keys[i] << ", <viewkey>)");
}
else
{
Expand Down
4 changes: 2 additions & 2 deletions src/cryptonote_core/cryptonote_tx_utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ namespace cryptonote
crypto::key_derivation derivation = AUTO_VAL_INIT(derivation);
crypto::public_key out_eph_public_key = AUTO_VAL_INIT(out_eph_public_key);
bool r = crypto::generate_key_derivation(miner_address.m_view_public_key, txkey.sec, derivation);
CHECK_AND_ASSERT_MES(r, false, "while creating outs: failed to generate_key_derivation(" << miner_address.m_view_public_key << ", " << txkey.sec << ")");
CHECK_AND_ASSERT_MES(r, false, "while creating outs: failed to generate_key_derivation(" << miner_address.m_view_public_key << ", " << crypto::secret_key_explicit_print_ref{txkey.sec} << ")");

r = crypto::derive_public_key(derivation, no, miner_address.m_spend_public_key, out_eph_public_key);
CHECK_AND_ASSERT_MES(r, false, "while creating outs: failed to derive_public_key(" << derivation << ", " << no << ", "<< miner_address.m_spend_public_key << ")");
Expand Down Expand Up @@ -484,7 +484,7 @@ namespace cryptonote
crypto::generate_ring_signature(tx_prefix_hash, boost::get<txin_to_key>(tx.vin[i]).k_image, keys_ptrs, in_contexts[i].in_ephemeral.sec, src_entr.real_output, sigs.data());
ss_ring_s << "signatures:" << ENDL;
std::for_each(sigs.begin(), sigs.end(), [&](const crypto::signature& s){ss_ring_s << s << ENDL;});
ss_ring_s << "prefix_hash:" << tx_prefix_hash << ENDL << "in_ephemeral_key: " << in_contexts[i].in_ephemeral.sec << ENDL << "real_output: " << src_entr.real_output << ENDL;
ss_ring_s << "prefix_hash:" << tx_prefix_hash << ENDL << "in_ephemeral_key: " << crypto::secret_key_explicit_print_ref{in_contexts[i].in_ephemeral.sec} << ENDL << "real_output: " << src_entr.real_output << ENDL;
i++;
}

Expand Down
13 changes: 2 additions & 11 deletions src/cryptonote_protocol/block_queue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,6 @@
#undef MONERO_DEFAULT_LOG_CATEGORY
#define MONERO_DEFAULT_LOG_CATEGORY "cn.block_queue"

namespace std {
static_assert(sizeof(size_t) <= sizeof(boost::uuids::uuid), "boost::uuids::uuid too small");
template<> struct hash<boost::uuids::uuid> {
std::size_t operator()(const boost::uuids::uuid &_v) const {
return reinterpret_cast<const std::size_t &>(_v);
}
};
}

namespace cryptonote
{

Expand Down Expand Up @@ -472,15 +463,15 @@ bool block_queue::has_spans(const boost::uuids::uuid &connection_id) const
float block_queue::get_speed(const boost::uuids::uuid &connection_id) const
{
boost::unique_lock<boost::recursive_mutex> lock(mutex);
std::unordered_map<boost::uuids::uuid, float> speeds;
std::unordered_map<boost::uuids::uuid, float, boost::hash<boost::uuids::uuid>> speeds;
for (const auto &span: blocks)
{
if (span.blocks.empty())
continue;
// note that the average below does not average over the whole set, but over the
// previous pseudo average and the latest rate: this gives much more importance
// to the latest measurements, which is fine here
std::unordered_map<boost::uuids::uuid, float>::iterator i = speeds.find(span.connection_id);
const auto i = speeds.find(span.connection_id);
if (i == speeds.end())
speeds.insert(std::make_pair(span.connection_id, span.rate));
else
Expand Down
8 changes: 5 additions & 3 deletions src/device/device_default.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -317,13 +317,15 @@ namespace hw {
{
// sending change to yourself; derivation = a*R
r = generate_key_derivation(txkey_pub, sender_account_keys.m_view_secret_key, derivation);
CHECK_AND_ASSERT_MES(r, false, "at creation outs: failed to generate_key_derivation(" << txkey_pub << ", " << sender_account_keys.m_view_secret_key << ")");
CHECK_AND_ASSERT_MES(r, false, "at creation outs: failed to generate_key_derivation(" << txkey_pub << ", <viewkey>)");
}
else
{
// sending to the recipient; derivation = r*A (or s*C in the subaddress scheme)
r = generate_key_derivation(dst_entr.addr.m_view_public_key, dst_entr.is_subaddress && need_additional_txkeys ? additional_txkey.sec : tx_key, derivation);
CHECK_AND_ASSERT_MES(r, false, "at creation outs: failed to generate_key_derivation(" << dst_entr.addr.m_view_public_key << ", " << (dst_entr.is_subaddress && need_additional_txkeys ? additional_txkey.sec : tx_key) << ")");
const crypto::secret_key &tx_privkey{dst_entr.is_subaddress && need_additional_txkeys ? additional_txkey.sec : tx_key};
r = generate_key_derivation(dst_entr.addr.m_view_public_key, tx_privkey, derivation);
CHECK_AND_ASSERT_MES(r, false, "at creation outs: failed to generate_key_derivation("
<< dst_entr.addr.m_view_public_key << ", " << crypto::secret_key_explicit_print_ref{tx_privkey} << ")");
}

if (need_additional_txkeys)
Expand Down
4 changes: 2 additions & 2 deletions src/lmdb/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -127,15 +127,15 @@ namespace lmdb
/*!
A LMDB comparison function that uses `std::memcmp`.
\toaram T is `!epee::has_padding`
\toaram T has standard layout and an alignment of 1
\tparam offset to `T` within the value.
\return The result of `std::memcmp` over the value.
*/
template<typename T, std::size_t offset = 0>
inline int compare(MDB_val const* left, MDB_val const* right) noexcept
{
static_assert(!epee::has_padding<T>(), "memcmp will not work");
static_assert(std::is_standard_layout<T>() && alignof(T) == 1, "memcmp will not work");
if (!left || !right || left->mv_size < sizeof(T) + offset || right->mv_size < sizeof(T) + offset)
{
assert("invalid use of custom comparison" == 0);
Expand Down
6 changes: 3 additions & 3 deletions src/simplewallet/simplewallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1979,7 +1979,7 @@ bool simple_wallet::rpc_payment_info(const std::vector<std::string> &args)
crypto::public_key pkey;
crypto::secret_key_to_public_key(m_wallet->get_rpc_client_secret_key(), pkey);
message_writer() << tr("RPC client ID: ") << pkey;
message_writer() << tr("RPC client secret key: ") << m_wallet->get_rpc_client_secret_key();
message_writer() << tr("RPC client secret key: ") << crypto::secret_key_explicit_print_ref{m_wallet->get_rpc_client_secret_key()};
if (!m_wallet->get_rpc_payment_info(false, payment_required, credits, diff, credits_per_hash_found, hashing_blob, height, seed_height, seed_hash, next_seed_hash, cookie))
{
fail_msg_writer() << tr("Failed to query daemon");
Expand Down Expand Up @@ -8026,9 +8026,9 @@ bool simple_wallet::submit_transfer(const std::vector<std::string> &args_)
std::string get_tx_key_stream(crypto::secret_key tx_key, std::vector<crypto::secret_key> additional_tx_keys)
{
ostringstream oss;
oss << epee::string_tools::pod_to_hex(tx_key);
oss << epee::string_tools::pod_to_hex(unwrap(unwrap(tx_key)));
for (size_t i = 0; i < additional_tx_keys.size(); ++i)
oss << epee::string_tools::pod_to_hex(additional_tx_keys[i]);
oss << epee::string_tools::pod_to_hex(unwrap(unwrap(additional_tx_keys[i])));
return oss.str();
}

Expand Down
8 changes: 4 additions & 4 deletions src/wallet/api/wallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -881,7 +881,7 @@ std::string WalletImpl::integratedAddress(const std::string &payment_id) const

std::string WalletImpl::secretViewKey() const
{
return epee::string_tools::pod_to_hex(m_wallet->get_account().get_keys().m_view_secret_key);
return epee::string_tools::pod_to_hex(unwrap(unwrap(m_wallet->get_account().get_keys().m_view_secret_key)));
}

std::string WalletImpl::publicViewKey() const
Expand All @@ -891,7 +891,7 @@ std::string WalletImpl::publicViewKey() const

std::string WalletImpl::secretSpendKey() const
{
return epee::string_tools::pod_to_hex(m_wallet->get_account().get_keys().m_spend_secret_key);
return epee::string_tools::pod_to_hex(unwrap(unwrap(m_wallet->get_account().get_keys().m_spend_secret_key)));
}

std::string WalletImpl::publicSpendKey() const
Expand Down Expand Up @@ -1878,9 +1878,9 @@ std::string WalletImpl::getTxKey(const std::string &txid_str) const
{
clearStatus();
std::ostringstream oss;
oss << epee::string_tools::pod_to_hex(tx_key);
oss << epee::string_tools::pod_to_hex(unwrap(unwrap(tx_key)));
for (size_t i = 0; i < additional_tx_keys.size(); ++i)
oss << epee::string_tools::pod_to_hex(additional_tx_keys[i]);
oss << epee::string_tools::pod_to_hex(unwrap(unwrap(additional_tx_keys[i])));
return oss.str();
}
else
Expand Down
Loading

0 comments on commit 65568d3

Please sign in to comment.