[8445] Fixed redundant calculation of v and s on every login Author: arrai

    The SRP-6 specifications clearly say, that v and s are only to be
    calculated on registering a user and changing his password; calculating
    them on every login is plain waste.

--HG--
branch : trunk

7 files changed, 47 insertions, 60 deletions

diff --git a/src/game/AccountMgr.cpp b/src/game/AccountMgr.cpp
index 74bc4cd2dcc..981a5ad07ec 100644
--- a/src/game/AccountMgr.cpp
+++ b/src/game/AccountMgr.cpp
@@ -145,7 +145,8 @@ AccountOpResult AccountMgr::ChangePassword(uint32 accid, std::string new_passwd)
     normalizeString(new_passwd);
 
     loginDatabase.escape_string(new_passwd);
-    if(!loginDatabase.PExecute("UPDATE account SET sha_pass_hash=SHA1(CONCAT(username,':','%s')) WHERE id='%d'", new_passwd.c_str(), accid))
+    // also reset s and v to force update at next realmd login
+    if(!loginDatabase.PExecute("UPDATE account SET v='0', s='0', sha_pass_hash=SHA1("_CONCAT3_("username","':'","'%s'")") WHERE id='%d'", new_passwd.c_str(), accid))
         return AOR_DB_INTERNAL_ERROR;                       // unexpected error
 
     return AOR_OK;
diff --git a/src/game/CharacterHandler.cpp b/src/game/CharacterHandler.cpp
index 7ce02dc6a77..becf81d9c04 100644
--- a/src/game/CharacterHandler.cpp
+++ b/src/game/CharacterHandler.cpp
@@ -126,9 +126,6 @@ class CharacterHandler
 
 void WorldSession::HandleCharEnum(QueryResult * result)
 {
-    // keys can be non cleared if player open realm list and close it by 'cancel'
-    loginDatabase.PExecute("UPDATE account SET v = '0', s = '0' WHERE id = '%u'", GetAccountId());
-
     WorldPacket data(SMSG_CHAR_ENUM, 100);                  // we guess size
 
     uint8 num = 0;
diff --git a/src/game/WorldSocket.cpp b/src/game/WorldSocket.cpp
index ba119637cf9..0413f197877 100644
--- a/src/game/WorldSocket.cpp
+++ b/src/game/WorldSocket.cpp
@@ -683,7 +683,7 @@ int WorldSocket::HandleAuthSession (WorldPacket& recvPacket)
     LocaleConstant locale;
     std::string account;
     Sha1Hash sha1;
-    BigNumber v, s, g, N, x, I;
+    BigNumber v, s, g, N;
     WorldPacket packet, SendAddonPacked;
 
     BigNumber K;
@@ -725,12 +725,11 @@ int WorldSocket::HandleAuthSession (WorldPacket& recvPacket)
                                 "sessionkey, "              //2
                                 "last_ip, "                 //3
                                 "locked, "                  //4
-                                "sha_pass_hash, "           //5
-                                "v, "                       //6
-                                "s, "                       //7
-                                "expansion, "               //8
-                                "mutetime, "                //9
-                                "locale "                   //10
+                                "v, "                       //5
+                                "s, "                       //6
+                                "expansion, "               //7
+                                "mutetime, "                //8
+                                "locale "                   //9
                                 "FROM account "
                                 "WHERE username = '%s'",
                                 safe_account.c_str ());
@@ -749,61 +748,25 @@ int WorldSocket::HandleAuthSession (WorldPacket& recvPacket)
 
     Field* fields = result->Fetch ();
 
-    uint8 expansion = fields[8].GetUInt8();
+    uint8 expansion = fields[7].GetUInt8();
     uint32 world_expansion = sWorld.getConfig(CONFIG_EXPANSION);
     if(expansion > world_expansion)
         expansion = world_expansion;
-    //expansion = ((sWorld.getConfig(CONFIG_EXPANSION) > fields[8].GetUInt8()) ? fields[8].GetUInt8() : sWorld.getConfig(CONFIG_EXPANSION));
+    //expansion = ((sWorld.getConfig(CONFIG_EXPANSION) > fields[7].GetUInt8()) ? fields[7].GetUInt8() : sWorld.getConfig(CONFIG_EXPANSION));
 
     N.SetHexStr ("894B645E89E1535BBDAD5B8B290650530801B18EBFBF5E8FAB3C82872A3E9BB7");
     g.SetDword (7);
-    I.SetHexStr (fields[5].GetString ());
 
-    //In case of leading zeros in the I hash, restore them
-    uint8 mDigest[SHA_DIGEST_LENGTH];
-    memset (mDigest, 0, SHA_DIGEST_LENGTH);
-
-    if (I.GetNumBytes () <= SHA_DIGEST_LENGTH)
-        memcpy (mDigest, I.AsByteArray (), I.GetNumBytes ());
-
-    std::reverse (mDigest, mDigest + SHA_DIGEST_LENGTH);
-
-    s.SetHexStr (fields[7].GetString ());
-    sha1.UpdateData (s.AsByteArray (), s.GetNumBytes ());
-    sha1.UpdateData (mDigest, SHA_DIGEST_LENGTH);
-    sha1.Finalize ();
-    x.SetBinary (sha1.GetDigest (), sha1.GetLength ());
-    v = g.ModExp (x, N);
+    v.SetHexStr(fields[5].GetString());
+    s.SetHexStr (fields[6].GetString ());
 
     const char* sStr = s.AsHexStr ();                       //Must be freed by OPENSSL_free()
     const char* vStr = v.AsHexStr ();                       //Must be freed by OPENSSL_free()
-    const char* vold = fields[6].GetString ();
 
-    DEBUG_LOG ("WorldSocket::HandleAuthSession: (s,v) check s: %s v_old: %s v_new: %s",
+    DEBUG_LOG ("WorldSocket::HandleAuthSession: (s,v) check s: %s v: %s",
                 sStr,
-                vold,
                 vStr);
 
-    loginDatabase.PExecute ("UPDATE account "
-                            "SET "
-                            "v = '0', "
-                            "s = '0' "
-                            "WHERE username = '%s'",
-                            safe_account.c_str ());
-
-    if (!vold || strcmp (vStr, vold))
-    {
-        packet.Initialize (SMSG_AUTH_RESPONSE, 1);
-        packet << uint8 (AUTH_UNKNOWN_ACCOUNT);
-        SendPacket (packet);
-        delete result;
-        OPENSSL_free ((void*) sStr);
-        OPENSSL_free ((void*) vStr);
-
-        sLog.outBasic ("WorldSocket::HandleAuthSession: User not logged.");
-        return -1;
-    }
-
     OPENSSL_free ((void*) sStr);
     OPENSSL_free ((void*) vStr);
 
@@ -831,9 +794,9 @@ int WorldSocket::HandleAuthSession (WorldPacket& recvPacket)
 
     K.SetHexStr (fields[2].GetString ());
 
-    time_t mutetime = time_t (fields[9].GetUInt64 ());
+    time_t mutetime = time_t (fields[8].GetUInt64 ());
 
-    locale = LocaleConstant (fields[10].GetUInt8 ());
+    locale = LocaleConstant (fields[9].GetUInt8 ());
     if (locale >= MAX_LOCALE)
         locale = LOCALE_enUS;
 
diff --git a/src/shared/Auth/BigNumber.cpp b/src/shared/Auth/BigNumber.cpp
index 7872b141f3d..8869d3bb9e5 100644
--- a/src/shared/Auth/BigNumber.cpp
+++ b/src/shared/Auth/BigNumber.cpp
@@ -161,6 +161,11 @@ uint32 BigNumber::AsDword()
     return (uint32)BN_get_word(_bn);
 }
 
+bool BigNumber::isZero() const
+{
+    return BN_is_zero(_bn)!=0;
+}
+
 uint8 *BigNumber::AsByteArray(int minSize)
 {
     int length = (minSize >= GetNumBytes()) ? minSize : GetNumBytes();
diff --git a/src/shared/Auth/BigNumber.h b/src/shared/Auth/BigNumber.h
index b9038e86b13..ba116db3965 100644
--- a/src/shared/Auth/BigNumber.h
+++ b/src/shared/Auth/BigNumber.h
@@ -74,6 +74,8 @@ class BigNumber
             return t %= bn;
         }
 
+        bool isZero() const;
+
         BigNumber ModExp(const BigNumber &bn1, const BigNumber &bn2);
         BigNumber Exp(const BigNumber &);
 
diff --git a/src/trinityrealm/AuthSocket.cpp b/src/trinityrealm/AuthSocket.cpp
index 14dadb1f2e2..88cd3aa0ec9 100644
--- a/src/trinityrealm/AuthSocket.cpp
+++ b/src/trinityrealm/AuthSocket.cpp
@@ -248,7 +248,6 @@ void AuthSocket::OnAccept()
     sLog.outBasic("Accepting connection from '%s:%d'",
         GetRemoteAddress().c_str(), GetRemotePort());
 
-    s.SetRand(s_BYTE_SIZE * 8);
 }
 
 /// Read the packet from the client
@@ -297,6 +296,8 @@ void AuthSocket::OnRead()
 /// Make the SRP6 calculation from hash in dB
 void AuthSocket::_SetVSFields(const std::string& rI)
 {
+    s.SetRand(s_BYTE_SIZE * 8);
+
     BigNumber I;
     I.SetHexStr(rI.c_str());
 
@@ -398,7 +399,7 @@ bool AuthSocket::_HandleLogonChallenge()
         ///- Get the account details from the account table
         // No SQL injection (escaped user name)
 
-        result = loginDatabase.PQuery("SELECT sha_pass_hash,id,locked,last_ip,gmlevel FROM account WHERE username = '%s'",_safelogin.c_str ());
+        result = loginDatabase.PQuery("SELECT sha_pass_hash,id,locked,last_ip,gmlevel,v,s FROM account WHERE username = '%s'",_safelogin.c_str ());
         if( result )
         {
             ///- If the IP is 'locked', check that the player comes indeed from the correct IP address
@@ -448,7 +449,21 @@ bool AuthSocket::_HandleLogonChallenge()
                 {
                     ///- Get the password from the account table, upper it, and make the SRP6 calculation
                     std::string rI = (*result)[0].GetCppString();
-                    _SetVSFields(rI);
+
+                    ///- Don't calculate (v, s) if there are already some in the database
+                    std::string databaseV = (*result)[5].GetCppString();
+                    std::string databaseS = (*result)[6].GetCppString();
+
+                    sLog.outDebug("database authentication values: v='%s' s='%s'", databaseV.c_str(), databaseS.c_str());
+
+                    // multiply with 2, bytes are stored as hexstring
+                    if(databaseV.size() != s_BYTE_SIZE*2 || databaseS.size() != s_BYTE_SIZE*2)
+                        _SetVSFields(rI);
+                    else
+                    {
+                        s.SetHexStr(databaseS.c_str());
+                        v.SetHexStr(databaseV.c_str());
+                    }
 
                     b.SetRand(19 * 8);
                     BigNumber gmod = g.ModExp(b, N);
@@ -593,8 +608,13 @@ bool AuthSocket::_HandleLogonProof()
 
     ///- Continue the SRP6 calculation based on data received from the client
     BigNumber A;
+
     A.SetBinary(lp.A, 32);
 
+    // SRP safeguard: abort if A==0
+    if (A.isZero())
+        return false;
+
     Sha1Hash sha;
     sha.UpdateBigNumbers(&A, &B, NULL);
     sha.Finalize();
@@ -605,7 +625,7 @@ bool AuthSocket::_HandleLogonProof()
     uint8 t[32];
     uint8 t1[16];
     uint8 vK[40];
-    memcpy(t, S.AsByteArray(), 32);
+    memcpy(t, S.AsByteArray(32), 32);
     for (int i = 0; i < 16; ++i)
     {
         t1[i] = t[i * 2];
@@ -906,8 +926,6 @@ bool AuthSocket::_HandleRealmList()
 
     SendBuf((char const*)hdr.contents(), hdr.size());
 
-    // Set check field before possible relogin to realm
-    _SetVSFields(rI);
     return true;
 }
 
diff --git a/src/trinityrealm/Main.cpp b/src/trinityrealm/Main.cpp
index e44fbc60cdb..53f206f2573 100644
--- a/src/trinityrealm/Main.cpp
+++ b/src/trinityrealm/Main.cpp
@@ -148,6 +148,7 @@ extern int main(int argc, char **argv)
         sLog.outError("Could not find configuration file %s.", cfg_file);
         return 1;
     }
+    sLog.Initialize();
 
     sLog.outString( "%s (realm-daemon)", _FULLVERSION );
     sLog.outString( "<Ctrl-C> to stop.\n" );