From 82dad90ff3fe1438ce5d578dcf50bc82e5c96e9b Mon Sep 17 00:00:00 2001 From: Nils Maier Date: Sat, 19 Apr 2014 19:10:06 +0200 Subject: [PATCH] Validate token using PBKDF2-HMAC-SHA1. This change should make token validation more resilient to: - timing attacks (constant time array compare) - brute-force/dictionary attacks (PBKDF2) Closes #220 --- src/DownloadEngine.cc | 92 ++++++++++++++++++++++++++++++++++++++++++- src/DownloadEngine.h | 16 ++++++++ src/RpcMethod.cc | 6 +-- test/RpcMethodTest.cc | 2 +- 4 files changed, 110 insertions(+), 6 deletions(-) diff --git a/src/DownloadEngine.cc b/src/DownloadEngine.cc index efa86950..36b67469 100644 --- a/src/DownloadEngine.cc +++ b/src/DownloadEngine.cc @@ -73,6 +73,15 @@ #ifdef ENABLE_WEBSOCKET # include "WebSocketSessionMan.h" #endif // ENABLE_WEBSOCKET +#include "Option.h" +#include "util_security.h" + +// Lower time limit for PBKDF2 operations in validateToken. +static const double kTokenTimeLower = 0.055; +// Upper time limit for PBKDF2 operations in validateToken. +static const double kTokenTimeUpper = 0.120; +// Sweet spot time for PBKDF2 operations in validateToken. +static const double kTokenTimeSweetspot = 0.072; namespace aria2 { @@ -102,7 +111,8 @@ DownloadEngine::DownloadEngine(std::unique_ptr eventPoll) asyncDNSServers_(nullptr), #endif // HAVE_ARES_ADDR_NODE dnsCache_(make_unique()), - option_(nullptr) + option_(nullptr), + tokenIterations_(5000) { unsigned char sessionId[20]; util::generateRandomKey(sessionId); @@ -631,4 +641,84 @@ void DownloadEngine::setWebSocketSessionMan } #endif // ENABLE_WEBSOCKET +bool DownloadEngine::validateToken(const std::string& token) +{ + using namespace util::security; + + if (!option_->defined(PREF_RPC_SECRET)) { + return true; + } + + if (!tokenHMAC_ || tokenAverageDuration_ > kTokenTimeUpper || + tokenAverageDuration_ < kTokenTimeLower) { + + // Setup our stuff. + if (tokenHMAC_) { + A2_LOG_INFO(fmt("Recalculating iterations because avg. duration is %.4f", + tokenAverageDuration_)); + } + + tokenHMAC_ = HMAC::createRandom(); + if (!tokenHMAC_) { + A2_LOG_ERROR("Failed to create HMAC"); + return false; + } + + // This should still be pretty fast on a modern system... Well, too fast + // with the initial 5000 iterations, and that is why we adjust it. + // XXX We should run this setup high priorty, so that other processes on the + // system don't mess up our results and let us underestimate the iterations. + std::deque mm; + for (auto i = 0; i < 10; ++i) { + auto c = std::clock(); + tokenExpected_ = make_unique( + PBKDF2(tokenHMAC_.get(), option_->get(PREF_RPC_SECRET), + tokenIterations_)); + mm.push_back((std::clock() - c) / (double)CLOCKS_PER_SEC); + } + std::sort(mm.begin(), mm.end()); + // Pop outliers. + mm.pop_front(); + mm.pop_back(); + mm.pop_back(); + auto duration = std::accumulate(mm.begin(), mm.end(), 0.0) / mm.size(); + + A2_LOG_INFO(fmt("Took us %.4f secs on average to perform PBKDF2 with %zu " + "iterations during setup", + duration, tokenIterations_)); + + // Adjust iterations so that an op takes about |kTokenTimeSpeetspot| sec, + // which would allow for a couple attempts per second (instead of + // potentially thousands without PBKDF2). + // We might overestimate the performance a bit, but should not perform + // worse than |kTokenTimeUpper| secs per attempt on a normally loaded system + // and no better than |kTokenTimeLower|. If this does not hold true anymore, + // the |tokenAverageDuration_| checks will force a re-calcuation. + tokenIterations_ *= kTokenTimeSweetspot / duration; + + auto c = std::clock(); + tokenExpected_ = make_unique( + PBKDF2(tokenHMAC_.get(), option_->get(PREF_RPC_SECRET), + tokenIterations_)); + duration = (std::clock() - c) / (double)CLOCKS_PER_SEC; + A2_LOG_INFO(fmt("Took us %.4f secs to perform PBKDF2 with %zu iterations", + duration, tokenIterations_)); + + // Seed average duration. + tokenAverageDuration_ = duration; + } + + auto c = std::clock(); + bool rv = *tokenExpected_ == PBKDF2(tokenHMAC_.get(), token, + tokenIterations_); + auto duration = (std::clock() - c) / (double)CLOCKS_PER_SEC; + A2_LOG_DEBUG(fmt("Took us %.4f secs to perform token compare with %zu " + "iterations", + duration, tokenIterations_)); + + // Update rolling hash. + tokenAverageDuration_ = tokenAverageDuration_ * 0.9 + duration * 0.1; + return rv; +} + } // namespace aria2 diff --git a/src/DownloadEngine.h b/src/DownloadEngine.h index eb827b97..eadb2f46 100644 --- a/src/DownloadEngine.h +++ b/src/DownloadEngine.h @@ -74,6 +74,13 @@ class WebSocketSessionMan; } // namespace rpc #endif // ENABLE_WEBSOCKET +namespace util { + namespace security { + class HMAC; + class HMACResult; + } // namespace security +} // namespace util + class DownloadEngine { private: void waitData(); @@ -173,6 +180,13 @@ private: // deleted. std::deque> routineCommands_; std::deque> commands_; + + std::unique_ptr tokenHMAC_; + std::unique_ptr tokenExpected_; + size_t tokenIterations_; + + double tokenAverageDuration_; + public: DownloadEngine(std::unique_ptr eventPoll); @@ -360,6 +374,8 @@ public: return webSocketSessionMan_; } #endif // ENABLE_WEBSOCKET + + bool validateToken(const std::string& token); }; } // namespace aria2 diff --git a/src/RpcMethod.cc b/src/RpcMethod.cc index e7b2750f..1f7ea20b 100644 --- a/src/RpcMethod.cc +++ b/src/RpcMethod.cc @@ -85,10 +85,8 @@ void RpcMethod::authorize(RpcRequest& req, DownloadEngine* e) } } } - if(e && e->getOption()->defined(PREF_RPC_SECRET)) { - if(token != e->getOption()->get(PREF_RPC_SECRET)) { - throw DL_ABORT_EX("Unauthorized"); - } + if (!e || !e->validateToken(token)) { + throw DL_ABORT_EX("Unauthorized"); } } diff --git a/test/RpcMethodTest.cc b/test/RpcMethodTest.cc index deea7ba4..d5614b4e 100644 --- a/test/RpcMethodTest.cc +++ b/test/RpcMethodTest.cc @@ -723,7 +723,7 @@ void RpcMethodTest::testChangeGlobalOption_withNotAllowedOption() void RpcMethodTest::testNoSuchMethod() { NoSuchMethodRpcMethod m; - auto res = m.execute(createReq("make.hamburger"), nullptr); + auto res = m.execute(createReq("make.hamburger"), e_.get()); CPPUNIT_ASSERT_EQUAL(1, res.code); CPPUNIT_ASSERT_EQUAL(std::string("No such method: make.hamburger"), getString(downcast(res.param), "faultString"));