Limit login attempt frequency based on IP address

The previous method was broken because it set the "last time" to the
current time when the client connected. So login would fail when the
username and password were sent within a second from connecting, which
is not desirable.

If I'd have fixed this by setting the "last time" to login time minus
one second, then an attacker would just need to reconnect for each login
attempt. So now it uses an IP address based approach, where each IP can
only try to log in once per second.

6 files changed, 28 insertions, 60 deletions

diff --git a/src/account-server/account.cpp b/src/account-server/account.cpp
index 246442fd..55f521b9 100644
--- a/src/account-server/account.cpp
+++ b/src/account-server/account.cpp
@@ -22,9 +22,6 @@
 
 #include "account-server/account.hpp"
 
-/**
- * Destructor.
- */
 Account::~Account()
 {
     for (Characters::iterator i = mCharacters.begin(),
@@ -34,20 +31,11 @@ Account::~Account()
     }
 }
 
-
-/**
- * Set the characters.
- */
-void
-Account::setCharacters(const Characters& characters)
+void Account::setCharacters(const Characters& characters)
 {
     mCharacters = characters;
 }
 
-
-/**
- * Add a new character.
- */
 void Account::addCharacter(Character *character)
 {
     mCharacters.push_back(character);
diff --git a/src/account-server/account.hpp b/src/account-server/account.hpp
index b3758a2e..71575d78 100644
--- a/src/account-server/account.hpp
+++ b/src/account-server/account.hpp
@@ -45,7 +45,6 @@ class Account
          */
         ~Account();
 
-
         /**
          * Set the user name.
          *
@@ -54,7 +53,6 @@ class Account
         void setName(const std::string &name)
         { mName = name; }
 
-
         /**
          * Get the user name.
          *
@@ -63,7 +61,6 @@ class Account
         const std::string &getName() const
         { return mName; }
 
-
         /**
          * Set the user password. The password is expected to be already
          * hashed with a salt.
@@ -114,7 +111,6 @@ class Account
         void setLevel(int level)
         { mLevel = level; }
 
-
         /**
          * Get the account level.
          *
@@ -123,18 +119,15 @@ class Account
         int getLevel() const
         { return mLevel; }
 
-
         /**
          * Set the characters.
          *
          * @param characters a list of characters.
          */
-        void
-        setCharacters(const Characters& characters);
-
+        void setCharacters(const Characters& characters);
 
         /**
-         * Add a new character.
+         * Adds a new character.
          *
          * @param character the new character.
          */
@@ -147,7 +140,6 @@ class Account
          */
         void delCharacter(int i);
 
-
         /**
          * Get all the characters.
          *
diff --git a/src/account-server/accountclient.cpp b/src/account-server/accountclient.cpp
index 18eab583..3b9f35e8 100644
--- a/src/account-server/accountclient.cpp
+++ b/src/account-server/accountclient.cpp
@@ -26,7 +26,6 @@ AccountClient::AccountClient(ENetPeer *peer):
     status(CLIENT_LOGIN),
     mAccount(NULL)
 {
-    time(&lastLoginAttempt);
 }
 
 AccountClient::~AccountClient()
@@ -45,8 +44,3 @@ void AccountClient::unsetAccount()
     delete mAccount;
     mAccount = NULL;
 }
-
-void AccountClient::updateLoginAttempt()
-{
-    time(&lastLoginAttempt);
-}
diff --git a/src/account-server/accountclient.hpp b/src/account-server/accountclient.hpp
index dbb24aca..43f8c629 100644
--- a/src/account-server/accountclient.hpp
+++ b/src/account-server/accountclient.hpp
@@ -68,23 +68,11 @@ class AccountClient : public NetComputer
         Account *getAccount() const
         { return mAccount; }
 
-        /**
-         * Update lastLoginAttempt
-         */
-        void updateLoginAttempt();
-
-        /**
-         * Returns the time of the last login attempt.
-         */
-        int getLastLoginAttempt() const
-        { return lastLoginAttempt; }
-
         int status;
 
     private:
         /** Account associated with connection */
         Account *mAccount;
-        time_t lastLoginAttempt;
 };
 
 #endif
diff --git a/src/account-server/accounthandler.cpp b/src/account-server/accounthandler.cpp
index 6ab5b890..9ee5fc46 100644
--- a/src/account-server/accounthandler.cpp
+++ b/src/account-server/accounthandler.cpp
@@ -93,6 +93,9 @@ private:
     void handleCharacterCreateMessage(AccountClient &client, MessageIn &msg);
     void handleCharacterSelectMessage(AccountClient &client, MessageIn &msg);
     void handleCharacterDeleteMessage(AccountClient &client, MessageIn &msg);
+
+    typedef std::map<int, time_t> IPsToTime;
+    IPsToTime mLastLoginAttemptForIP;
 };
 
 static AccountHandler *accountHandler;
@@ -175,7 +178,7 @@ void AccountHandler::handleLoginMessage(AccountClient &client, MessageIn &msg)
         return;
     }
 
-    int clientVersion = msg.readLong();
+    const int clientVersion = msg.readLong();
 
     if (clientVersion < Configuration::getValue("clientVersion", 0))
     {
@@ -184,24 +187,24 @@ void AccountHandler::handleLoginMessage(AccountClient &client, MessageIn &msg)
         return;
     }
 
-    // get the IP address
-    //int address = client.getIP();
-
-    // TODO: Check IP against blacklist
-
-    time_t lastAttempt = client.getLastLoginAttempt();
-    if ((time(NULL) - lastAttempt) < 1)
+    // Check whether the last login attempt for this IP is still too fresh
+    const int address = client.getIP();
+    const time_t now = time(NULL);
+    IPsToTime::const_iterator it = mLastLoginAttemptForIP.find(address);
+    if (it != mLastLoginAttemptForIP.end())
     {
-        reply.writeByte(LOGIN_INVALID_TIME);
-        client.send(reply);
-        return;
+        const time_t lastAttempt = it->second;
+        if (now < lastAttempt + 1)
+        {
+            reply.writeByte(LOGIN_INVALID_TIME);
+            client.send(reply);
+            return;
+        }
     }
+    mLastLoginAttemptForIP[address] = now;
 
-    // updates the time last attempted to login
-    client.updateLoginAttempt();
-
-    std::string username = msg.readString();
-    std::string password = msg.readString();
+    const std::string username = msg.readString();
+    const std::string password = msg.readString();
 
     if (stringFilter->findDoubleQuotes(username))
     {
@@ -210,7 +213,9 @@ void AccountHandler::handleLoginMessage(AccountClient &client, MessageIn &msg)
         return;
     }
 
-    unsigned maxClients = (unsigned)Configuration::getValue("net_maxClients", 1000);
+    const unsigned maxClients =
+            (unsigned) Configuration::getValue("net_maxClients", 1000);
+
     if (getClientNumber() >= maxClients)
     {
         reply.writeByte(ERRMSG_SERVER_FULL);
@@ -237,13 +242,14 @@ void AccountHandler::handleLoginMessage(AccountClient &client, MessageIn &msg)
         return;
     }
 
+    // The client succesfully logged in
+
     // set lastLogin date of the account
     time_t login;
     time(&login);
     acc->setLastLogin(login);
     storage->updateLastLogin(acc);
 
-
     // Associate account with connection
     client.setAccount(acc);
     client.status = CLIENT_CONNECTED;
diff --git a/src/utils/stringfilter.cpp b/src/utils/stringfilter.cpp
index 1e2eaa54..b36ad71d 100644
--- a/src/utils/stringfilter.cpp
+++ b/src/utils/stringfilter.cpp
@@ -120,7 +120,7 @@ bool StringFilter::isEmailValid(const std::string& email)
         (email.find_first_of(' ') == std::string::npos);
 }
 
-bool StringFilter::findDoubleQuotes(const std::string& text)
+bool StringFilter::findDoubleQuotes(const std::string &text)
 {
     return (text.find('"', 0) != std::string::npos);
 }