From c1c7daea00058fc20b85fd1628bb35ddd7bcef23 Mon Sep 17 00:00:00 2001 From: dec05eba Date: Mon, 21 May 2018 10:09:54 +0200 Subject: Prevent malicious peer from replaying user ping --- include/Channel.hpp | 4 ++-- include/User.hpp | 3 +-- src/Channel.cpp | 34 ++++++++++------------------------ src/User.cpp | 3 +-- src/UsersSidePanel.cpp | 8 ++++---- 5 files changed, 18 insertions(+), 34 deletions(-) diff --git a/include/Channel.hpp b/include/Channel.hpp index 28d174b..26b5286 100644 --- a/include/Channel.hpp +++ b/include/Channel.hpp @@ -63,12 +63,12 @@ namespace dchat void update(); // Returns 0 if we are offline - u64 getSyncedTimestampUtcCombined(); + u32 getSyncedTimestampUtcInSec(); static void setCurrent(Channel *channel); static Channel* getCurrent(); private: - void sendPing(u32 pingCounter, u64 pingTimestamp); + void sendPing(u32 pingTimestampSec); protected: odhtdb::Database *database; odhtdb::DatabaseNode databaseNodeInfo; diff --git a/include/User.hpp b/include/User.hpp index ac08e36..c2874c2 100644 --- a/include/User.hpp +++ b/include/User.hpp @@ -38,8 +38,7 @@ namespace dchat bool isOnlineUser() const override { return true; } std::string name; - u32 pingCounter; - u64 pingTimestamp; + u32 pingTimestampSec; }; class OnlineRemoteUser : public OnlineUser diff --git a/src/Channel.cpp b/src/Channel.cpp index 5043665..1ac34c0 100644 --- a/src/Channel.cpp +++ b/src/Channel.cpp @@ -48,14 +48,10 @@ namespace dchat string unsignedData = userPublicKey.unsign(odhtdb::DataView((void*)deserializer.getBuffer(), deserializer.getSize())); sibs::SafeDeserializer unsignedDeserializer((const u8*)unsignedData.data(), unsignedData.size()); - u32 pingCounter = unsignedDeserializer.extract(); - u64 pingTimestamp = unsignedDeserializer.extract(); - // TODO: A malicious peer can capture the packets and reply them after the user has reconnect and counter has reset, need to fix this somehow. - // One solution is for the user to store the counter locally in file and continue using it when reconnecting - if(pingTimestamp > user->pingTimestamp) + u32 pingTimestampSec = unsignedDeserializer.extract(); + if(pingTimestampSec > user->pingTimestampSec) { - user->pingCounter = pingCounter; - user->pingTimestamp = pingTimestamp; + user->pingTimestampSec = pingTimestampSec; } } catch(std::exception &e) @@ -65,11 +61,7 @@ namespace dchat return result; }); - if(localUser->type == User::Type::ONLINE_LOCAL_USER) - { - auto onlineLocalUser = static_cast(localUser); - sendPing(onlineLocalUser->pingCounter + 1, database->getSyncedTimestampUtc().getCombined()); - } + sendPing(database->getSyncedTimestampUtc().seconds); } } @@ -79,11 +71,7 @@ namespace dchat { database->cancelNodeListener(pingKey, pingListener); database->stopSeeding(*databaseNodeInfo.getRequestHash()); - if(database && localUser->type == User::Type::ONLINE_LOCAL_USER) - { - auto onlineLocalUser = static_cast(localUser); - sendPing(onlineLocalUser->pingCounter + 1, 0); - } + sendPing(0); } for(User *user : users) @@ -266,12 +254,11 @@ namespace dchat if(database && localUser->type == User::Type::ONLINE_LOCAL_USER && pingTimer.getElapsedTime().asMilliseconds() > 5000) { pingTimer.restart(); - auto onlineLocalUser = static_cast(localUser); - sendPing(onlineLocalUser->pingCounter + 1, database->getSyncedTimestampUtc().getCombined()); + sendPing(database->getSyncedTimestampUtc().seconds); } } - void Channel::sendPing(u32 pingCounter, u64 pingTimestamp) + void Channel::sendPing(u32 pingTimestampSec) { if(database && localUser->type == User::Type::ONLINE_LOCAL_USER) { @@ -281,19 +268,18 @@ namespace dchat serializer.add((const u8*)onlineLocalUser->getPublicKey().getData(), onlineLocalUser->getPublicKey().getSize()); sibs::SafeSerializer signedSerializer; - signedSerializer.add(pingCounter); - signedSerializer.add(pingTimestamp); + signedSerializer.add(pingTimestampSec); string signedData = onlineLocalUser->keyPair.getPrivateKey().sign(odhtdb::DataView(signedSerializer.getBuffer().data(), signedSerializer.getBuffer().size())); serializer.add((const u8*)signedData.data(), signedData.size()); database->sendCustomMessage(pingKey, move(serializer.getBuffer())); } } - u64 Channel::getSyncedTimestampUtcCombined() + u32 Channel::getSyncedTimestampUtcInSec() { if(!database) return 0; - return database->getSyncedTimestampUtc().getCombined(); + return database->getSyncedTimestampUtc().seconds; } void Channel::setCurrent(Channel *channel) diff --git a/src/User.cpp b/src/User.cpp index 0cfbb4e..aabfd0b 100644 --- a/src/User.cpp +++ b/src/User.cpp @@ -13,8 +13,7 @@ namespace dchat OnlineUser::OnlineUser(const std::string &_name, Type type) : User(type), name(_name), - pingCounter(0), - pingTimestamp(0) + pingTimestampSec(0) { } diff --git a/src/UsersSidePanel.cpp b/src/UsersSidePanel.cpp index 6ddfbe2..c05a54f 100644 --- a/src/UsersSidePanel.cpp +++ b/src/UsersSidePanel.cpp @@ -85,7 +85,7 @@ namespace dchat const float textHeight = font->getLineSpacing(FONT_SIZE * Settings::getScaling()); - i64 timestamp = currentChannel->getSyncedTimestampUtcCombined(); + i64 timestampSec = currentChannel->getSyncedTimestampUtcInSec(); u32 numOnlineUsers = 0; u32 numOfflineUsers = 0; @@ -95,7 +95,7 @@ namespace dchat if(user->isOnlineUser()) { auto onlineUser = static_cast(user); - i64 pingTimeDiffSec = (timestamp - (i64)onlineUser->pingTimestamp) >> 32LL; + i64 pingTimeDiffSec = timestampSec - (i64)onlineUser->pingTimestampSec; if(pingTimeDiffSec > USER_TIMEOUT_SEC) hasUserTimedOut = true; } @@ -124,7 +124,7 @@ namespace dchat if(user->isOnlineUser()) { auto onlineUser = static_cast(user); - i64 pingTimeDiffSec = (timestamp - (i64)onlineUser->pingTimestamp) >> 32LL; + i64 pingTimeDiffSec = timestampSec - (i64)onlineUser->pingTimestampSec; if(pingTimeDiffSec > USER_TIMEOUT_SEC) isUserOnline = false; } @@ -152,7 +152,7 @@ namespace dchat if(user->isOnlineUser()) { auto onlineUser = static_cast(user); - i64 pingTimeDiffSec = (timestamp - (i64)onlineUser->pingTimestamp) >> 32LL; + i64 pingTimeDiffSec = timestampSec - (i64)onlineUser->pingTimestampSec; if(pingTimeDiffSec > USER_TIMEOUT_SEC) isUserOnline = false; } -- cgit v1.2.3