From abb34c1c1c6c05b81dd29332921d72647a8954fa Mon Sep 17 00:00:00 2001 From: Alexey Yurchenko Date: Sun, 22 Mar 2015 13:51:58 +0200 Subject: [PATCH 1/2] Refs codership/galera-features#49 - beefed up unit test for CC event --- gcs/src/unit_tests/gcs_act_cchange_test.cpp | 28 ++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/gcs/src/unit_tests/gcs_act_cchange_test.cpp b/gcs/src/unit_tests/gcs_act_cchange_test.cpp index bd6f9c2ce..cf7cdfeae 100644 --- a/gcs/src/unit_tests/gcs_act_cchange_test.cpp +++ b/gcs/src/unit_tests/gcs_act_cchange_test.cpp @@ -10,6 +10,7 @@ #include "gcs_act_cchange_test.hpp" #include "../gcs.hpp" #include "gu_uuid.hpp" +#include "gu_utils.hpp" #include @@ -62,7 +63,18 @@ START_TEST (serialization) cc_src.repl_proto_ver = 4; cc_src.appl_proto_ver = 5; - // TODO - add memb array + for (int i(0); i < 128; ++i) // make really big cluster ;) + { + gcs_act_cchange::member m; + + gu_uuid_generate(&m.uuid_, &i, sizeof(i)); + m.name_ = std::string("node") + gu::to_string(i); + m.incoming_ = std::string("192.168.0.") + gu::to_string(i) + ":4567"; + m.cached_ = i % 7 + 47; // some random number + m.state_ = gcs_node_state(i % GCS_NODE_STATE_MAX); + + cc_src.memb.push_back(m); + } size = cc_src.write(&buf); @@ -80,6 +92,20 @@ START_TEST (serialization) fail_if(cc_dst.repl_proto_ver != cc_src.repl_proto_ver); fail_if(cc_dst.appl_proto_ver != cc_src.appl_proto_ver); } + + /* another buffer corruption, exception must be thrown */ + try + { + static_cast(buf)[size/2] += 1; + + gcs_act_cchange const cc_dst(buf, size); + + fail_if(true, "exception must be thrown"); + } + catch (gu::Exception& e) + {} + + ::free(buf); } END_TEST From b4b07b72a101a140327b00090db5ac0e0d12991d Mon Sep 17 00:00:00 2001 From: Alexey Yurchenko Date: Mon, 23 Mar 2015 01:27:06 +0200 Subject: [PATCH 2/2] Refs codership/galera-features#34 1. Added 8-byte checksums to IST message headers. 2. Some cleanups in CC serialization and checksumming code. --- galera/src/SConscript | 1 + galera/src/ist_proto.cpp | 31 ++++++++++++ galera/src/ist_proto.hpp | 64 +++++++++++++++++------- galera/src/trx_handle.cpp | 2 +- galerautils/src/gu_digest.hpp | 91 +++++++++++++++++++++++++--------- gcs/src/gcs_act_cchange.cpp | 93 +++++++++++++++++++++-------------- 6 files changed, 203 insertions(+), 79 deletions(-) create mode 100644 galera/src/ist_proto.cpp diff --git a/galera/src/SConscript b/galera/src/SConscript index de79cada9..6aac0a7b8 100644 --- a/galera/src/SConscript +++ b/galera/src/SConscript @@ -19,6 +19,7 @@ libgaleraxx_srcs = [ 'gcs_action_source.cpp', 'galera_info.cpp', 'replicator.cpp', + 'ist_proto.cpp', 'ist.cpp', 'gcs_dummy.cpp', 'saved_state.cpp' ] diff --git a/galera/src/ist_proto.cpp b/galera/src/ist_proto.cpp new file mode 100644 index 000000000..2b483907e --- /dev/null +++ b/galera/src/ist_proto.cpp @@ -0,0 +1,31 @@ +// +// Copyright (C) 2015 Codership Oy +// + +#include "ist_proto.hpp" + +std::ostream& +galera::ist::operator<< (std::ostream& os, const Message& m) +{ + os << "ver: " << m.version() + << ", type: " << m.type() + << ", flags: " << m.flags() + << ", ctrl: " << m.ctrl() + << ", len: " << m.len() + << ", seqno: " << m.seqno(); + + return os; +} + +void +galera::ist::Message::throw_invalid_version(uint8_t const v) +{ + gu_throw_error(EPROTO) << "invalid protocol version " << int(v) + << ", expected " << version_; +} + +void +galera::ist::Message::throw_corrupted_header() +{ + gu_throw_error(EINVAL) << "Corrupted IST message header: " << *this; +} diff --git a/galera/src/ist_proto.hpp b/galera/src/ist_proto.hpp index bd8f9c452..6cb43f794 100644 --- a/galera/src/ist_proto.hpp +++ b/galera/src/ist_proto.hpp @@ -1,5 +1,5 @@ // -// Copyright (C) 2011-2014 Codership Oy +// Copyright (C) 2011-2015 Codership Oy // #ifndef GALERA_IST_PROTO_HPP @@ -75,12 +75,12 @@ namespace galera ctrl_ (ctrl ) {} - int version() const { return version_; } - Type type() const { return type_ ; } - uint8_t flags() const { return flags_ ; } - int8_t ctrl() const { return ctrl_ ; } - uint32_t len() const { return len_ ; } - wsrep_seqno_t seqno() const { return seqno_; } + int version() const { return version_; } + Type type() const { return type_ ; } + uint8_t flags() const { return flags_ ; } + int8_t ctrl() const { return ctrl_ ; } + uint32_t len() const { return len_ ; } + wsrep_seqno_t seqno() const { return seqno_ ; } void set_type_seqno(Type t, wsrep_seqno_t s) { @@ -93,7 +93,7 @@ namespace galera { // header: version 1 byte, type 1 byte, flags 1 byte, // ctrl field 1 byte, length 4 bytes, seqno 8 bytes - return 4 + 4 + 8; + return 4 + 4 + 8 + sizeof(checksum_t); } else { @@ -106,9 +106,9 @@ namespace galera size_t serialize(gu::byte_t* buf, size_t buflen, size_t offset)const { assert(version_ >= 4); -#ifndef NDEBUG - size_t orig_offset(offset); -#endif // NDEBUG + + size_t const orig_offset(offset); + offset = gu::serialize1(uint8_t(version_), buf, buflen, offset); offset = gu::serialize1(uint8_t(type_), buf, buflen, offset); offset = gu::serialize1(flags_, buf, buflen, offset); @@ -118,6 +118,11 @@ namespace galera { offset = gu::serialize4(len_, buf, buflen, offset); offset = gu::serialize8(seqno_, buf, buflen, offset); + + *reinterpret_cast(buf + offset) = + htog_checksum(buf + orig_offset, offset - orig_offset); + + offset += sizeof(checksum_t); } else /**/ { @@ -134,18 +139,13 @@ namespace galera size_t offset) { assert(version_ >= 4); -#ifndef NDEBUG + size_t orig_offset(offset); -#endif // NDEBUG + uint8_t u8; offset = gu::unserialize1(buf, buflen, offset, u8); - if (u8 != version_) - { - gu_throw_error(EPROTO) << "invalid protocol version " - << int(u8) - << ", expected " << version_; - } + if (gu_unlikely(u8 != version_)) throw_invalid_version(u8); offset = gu::unserialize1(buf, buflen, offset, u8); type_ = static_cast(u8); @@ -156,6 +156,16 @@ namespace galera { offset = gu::unserialize4(buf, buflen, offset, len_); offset = gu::unserialize8(buf, buflen, offset, seqno_); + + checksum_t const computed(htog_checksum(buf + orig_offset, + offset-orig_offset)); + const checksum_t* expected + (reinterpret_cast(buf + offset)); + + if (gu_unlikely(computed != *expected)) + throw_corrupted_header(); + + offset += sizeof(checksum_t); } else { @@ -178,8 +188,24 @@ namespace galera uint8_t version_; uint8_t flags_; int8_t ctrl_; + + typedef uint64_t checksum_t; + + // returns endian-adjusted checksum of buf + static checksum_t + htog_checksum(const void* const buf, size_t const size) + { + return + gu::htog(gu::FastHash::digest(buf, + size)); + } + + void throw_invalid_version(uint8_t v); + void throw_corrupted_header(); }; + std::ostream& operator<< (std::ostream& os, const Message& m); + class Handshake : public Message { public: diff --git a/galera/src/trx_handle.cpp b/galera/src/trx_handle.cpp index 96e94f735..e00e547c9 100644 --- a/galera/src/trx_handle.cpp +++ b/galera/src/trx_handle.cpp @@ -204,7 +204,7 @@ galera::TrxHandleSlave::sanity_checks() const (F_ROLLBACK | F_BEGIN))) { log_warn << "Both F_BEGIN and F_ROLLBACK are set on trx. " - << "This trx should not have been replicated at all: " << this; + << "This trx should not have been replicated at all: " << *this; } } diff --git a/galerautils/src/gu_digest.hpp b/galerautils/src/gu_digest.hpp index dd8fb5724..bee6b6ca9 100644 --- a/galerautils/src/gu_digest.hpp +++ b/galerautils/src/gu_digest.hpp @@ -28,7 +28,7 @@ class MMH3 ~MMH3 () {} template static int - digest (const void* const in, size_t size, T& out) + digest (const void* const in, size_t const size, T& out) { byte_t tmp[16]; gu_mmh128(in, size, tmp); @@ -39,7 +39,7 @@ class MMH3 /* experimental */ template static T - digest (const void* const in, size_t size) + digest (const void* const in, size_t const size) { switch (sizeof(T)) { @@ -56,7 +56,7 @@ class MMH3 gu_mmh128_append (&ctx_, buf, size); } - template + template int gather (void* const buf) const { GU_COMPILE_ASSERT(size >= 16, wrong_buf_size); @@ -90,25 +90,25 @@ class MMH3 }; /* class MMH3 */ template <> inline int -MMH3::digest (const void* const in, size_t size, uint8_t& out) +MMH3::digest (const void* const in, size_t const size, uint8_t& out) { out = gu_mmh128_32(in, size); return sizeof(out); } template <> inline int -MMH3::digest (const void* const in, size_t size, uint16_t& out) +MMH3::digest (const void* const in, size_t const size, uint16_t& out) { out = gu_mmh128_32(in, size); return sizeof(out); } template <> inline int -MMH3::digest (const void* const in, size_t size, uint32_t& out) +MMH3::digest (const void* const in, size_t const size, uint32_t& out) { out = gu_mmh128_32(in, size); return sizeof(out); } template <> inline int -MMH3::digest (const void* const in, size_t size, uint64_t& out) +MMH3::digest (const void* const in, size_t const size, uint64_t& out) { out = gu_mmh128_64(in, size); return sizeof(out); } @@ -133,7 +133,7 @@ class FastHash public: template static int - digest (const void* const in, size_t size, T& out) + digest (const void* const in, size_t const size, T& out) { byte_t tmp[16]; gu_fast_hash128(in, size, tmp); @@ -144,43 +144,88 @@ class FastHash /* experimental */ template static T - digest (const void* const in, size_t size) - { - switch (sizeof(T)) - { - case 1: return gu_fast_hash32(in, size); - case 2: return gu_fast_hash32(in, size); - case 4: return gu_fast_hash32(in, size); - case 8: return gu_fast_hash64(in, size); - } - throw; - } + digest (const void* const in, size_t const size); + /* The above is undefined and should cause linking error in case that + * template gets instantiated instead of specialized ones below. + * Unfortunately GU_COMPILE_ASSERT() is unusable here - causes compilation + * errors in every unit that only includes this header (probably because + * method is static). + * Perhaps templating the class would have done the trick */ + }; /* FastHash */ template <> inline int -FastHash::digest (const void* const in, size_t size, uint8_t& out) +FastHash::digest (const void* const in, size_t const size, uint8_t& out) { out = gu_fast_hash32(in, size); return sizeof(out); } template <> inline int -FastHash::digest (const void* const in, size_t size, uint16_t& out) +FastHash::digest (const void* const in, size_t const size, uint16_t& out) { out = gu_fast_hash32(in, size); return sizeof(out); } template <> inline int -FastHash::digest (const void* const in, size_t size, uint32_t& out) +FastHash::digest (const void* const in, size_t const size, uint32_t& out) { out = gu_fast_hash32(in, size); return sizeof(out); } template <> inline int -FastHash::digest (const void* const in, size_t size, uint64_t& out) +FastHash::digest (const void* const in, size_t const size, uint64_t& out) { out = gu_fast_hash64(in, size); return sizeof(out); } +template <> inline uint8_t +FastHash::digest(const void* const in, size_t const size) +{ + return gu_fast_hash32(in, size); +} + +template <> inline uint16_t +FastHash::digest(const void* const in, size_t const size) +{ + return gu_fast_hash32(in, size); +} + +template <> inline uint32_t +FastHash::digest(const void* const in, size_t const size) +{ + return gu_fast_hash32(in, size); +} + +template <> inline uint64_t +FastHash::digest(const void* const in, size_t const size) +{ + return gu_fast_hash64(in, size); +} + +template <> inline int8_t +FastHash::digest(const void* const in, size_t const size) +{ + return gu_fast_hash32(in, size); +} + +template <> inline int16_t +FastHash::digest(const void* const in, size_t const size) +{ + return gu_fast_hash32(in, size); +} + +template <> inline int32_t +FastHash::digest(const void* const in, size_t const size) +{ + return gu_fast_hash32(in, size); +} + +template <> inline int64_t +FastHash::digest(const void* const in, size_t const size) +{ + return gu_fast_hash64(in, size); +} + } /* namespace gu */ #endif /* GU_DIGEST_HPP */ diff --git a/gcs/src/gcs_act_cchange.cpp b/gcs/src/gcs_act_cchange.cpp index e917d5738..25d020e98 100644 --- a/gcs/src/gcs_act_cchange.cpp +++ b/gcs/src/gcs_act_cchange.cpp @@ -6,6 +6,7 @@ #include "gu_digest.hpp" #include "gu_hexdump.hpp" #include "gu_uuid.hpp" +#include "gu_macros.hpp" #include #include @@ -21,38 +22,65 @@ gcs_act_cchange::gcs_act_cchange() appl_proto_ver(-1) {} -static int -_checksum_len(int ver) +enum Version +{ + VER0 = 0 +}; + +static Version +_version(int ver) { switch(ver) { - case 0: return sizeof(uint64_t); + case VER0: return VER0; default: gu_throw_error(EPROTO) << "Unsupported CC action version"; throw; } } +// sufficiently big array to cover all potential checksum sizes +typedef char checksum_t[16]; + +static inline int +_checksum_len(Version const ver) +{ + int ret(0); + + switch(ver) + { + case VER0: ret = 8; break; + default: assert(0); + } + + assert(ret < int(sizeof(checksum_t))); + + return ret; +} + +static void +_checksum(Version const ver, const void* const buf, size_t const size, + checksum_t& res) +{ + switch(ver) + { + case VER0: gu::FastHash::digest(buf, size, res); return; + default: assert(0); + } +} + static inline gcs_node_state_t _int_to_node_state(int const s) { - if (s < 0 || s >= GCS_NODE_STATE_MAX) + if (gu_unlikely(s < 0 || s >= GCS_NODE_STATE_MAX)) { + assert(0); gu_throw_error(EINVAL) << "No such node state: " << s; } return gcs_node_state_t(s); } -/* >> wrapper for gcs_node_state enum */ -inline std::istream& operator>>(std::istream& is, gcs_node_state_t& ns) -{ - int s; - is >> s; - ns = _int_to_node_state(s); - return is; -} - gcs_act_cchange::gcs_act_cchange(const void* const cc_buf, int const cc_size) : memb(), @@ -63,18 +91,19 @@ gcs_act_cchange::gcs_act_cchange(const void* const cc_buf, int const cc_size) appl_proto_ver() { const char* b(static_cast(cc_buf)); - int const cc_ver(b[0]); - int const check_len(cc_size - _checksum_len(cc_ver)); + Version const cc_ver(_version(b[0])); + int const check_len(_checksum_len(cc_ver)); + int const check_offset(cc_size - check_len); - char check[16]; - gu::FastHash::digest(cc_buf, check_len, check); + checksum_t check; + _checksum(cc_ver, cc_buf, check_offset, check); - if (!std::equal(b + check_len, b + cc_size, check)) + if (gu_unlikely(!std::equal(b + check_offset, b + cc_size, check))) { gu_throw_error(EINVAL) << "CC action checksum mismatch. Expected " - << gu::Hexdump(b + check_len, cc_size - check_len) - << ", found " - << gu::Hexdump(check, cc_size - check_len); + << gu::Hexdump(b + check_offset, check_len) + << ", computed " + << gu::Hexdump(check, check_len); } b += 1; // skip version byte @@ -117,21 +146,13 @@ gcs_act_cchange::gcs_act_cchange(const void* const cc_buf, int const cc_size) m.cached_ = gu::gtoh(*cached); b += sizeof(gcs_seqno_t); - if (b[0] >= GCS_NODE_STATE_NON_PRIM && b[0] < GCS_NODE_STATE_MAX) - { - m.state_ = gcs_node_state(b[0]); - ++b; - } - else - { - assert(0); - gu_throw_error(EINVAL) << "Unrecognized node state in CC: " << b[0]; - } + m.state_ = _int_to_node_state(b[0]); + ++b; memb.push_back(m); } - assert(b - static_cast(cc_buf) == check_len); + assert(b - static_cast(cc_buf) == check_offset); } static size_t @@ -161,7 +182,7 @@ _strcopy(const std::string& str, char* ptr) int gcs_act_cchange::write(void** buf) const { - int const cc_ver(0); + Version const cc_ver(VER0); std::ostringstream os; os << cc_ver << ',' @@ -175,7 +196,7 @@ gcs_act_cchange::write(void** buf) const int const payload_len(str.length() + 1 + _memb_size(memb)); int const check_offset(1 + payload_len); // version byte + payload - int const check_len(_checksum_len(cc_ver)); // checksum hash + int const check_len(_checksum_len(cc_ver)); // checksum length int const ret(check_offset + check_len); // total message length /* using malloc() for C compatibility */ @@ -215,8 +236,8 @@ gcs_act_cchange::write(void** buf) const assert(static_cast(*buf) + check_offset == b); - gu::byte_t check[16]; - gu::FastHash::digest(*buf, check_offset, check); + checksum_t check; + _checksum(cc_ver, *buf, check_offset, check); std::copy(check, check + check_len, b); b += check_len;