Skip to content

Commit

Permalink
[13970] Implement totp auth
Browse files Browse the repository at this point in the history
Credit goes to TC/Chaosvex for reversing structures, Laizerox for implementing it and Killerwife for porting it
  • Loading branch information
Laizerox authored and killerwife committed Nov 11, 2017
1 parent f162a0a commit 12ce14f
Show file tree
Hide file tree
Showing 10 changed files with 231 additions and 31 deletions.
3 changes: 2 additions & 1 deletion sql/base/realmd.sql
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

DROP TABLE IF EXISTS `realmd_db_version`;
CREATE TABLE `realmd_db_version` (
`required_10008_01_realmd_realmd_db_version` bit(1) DEFAULT NULL
`required_13970_01_realmd_totp` bit(1) DEFAULT NULL
) ENGINE=MyISAM DEFAULT CHARSET=utf8 ROW_FORMAT=DYNAMIC COMMENT='Last applied sql update to DB';

--
Expand Down Expand Up @@ -58,6 +58,7 @@ CREATE TABLE `account` (
`expansion` tinyint(3) unsigned NOT NULL DEFAULT '0',
`mutetime` bigint(40) unsigned NOT NULL DEFAULT '0',
`locale` tinyint(3) unsigned NOT NULL DEFAULT '0',
`token` text,
PRIMARY KEY (`id`),
UNIQUE KEY `idx_username` (`username`),
KEY `idx_gmlevel` (`gmlevel`)
Expand Down
3 changes: 3 additions & 0 deletions sql/updates/realmd/13970_01_realmd_totp.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
ALTER TABLE realmd_db_version CHANGE COLUMN required_10008_01_realmd_realmd_db_version required_13970_01_realmd_totp bit;

ALTER TABLE account ADD COLUMN token text AFTER locale;
134 changes: 105 additions & 29 deletions src/realmd/AuthSocket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
*/

#include "Common.h"
#include "Auth/HMACSHA1.h"
#include "Auth/base32.h"
#include "Database/DatabaseEnv.h"
#include "Config/Config.h"
#include "Log.h"
Expand All @@ -29,6 +31,8 @@
#include "AuthCodes.h"

#include <openssl/md5.h>
#include <ctime>

//#include "Util.h" -- for commented utf8ToUpperOnlyLatin

extern DatabaseType LoginDatabase;
Expand All @@ -40,6 +44,14 @@ enum AccountFlags
ACCOUNT_FLAG_PROPASS = 0x00800000,
};

enum SecurityFlags
{
SECURITY_FLAG_NONE = 0x00,
SECURITY_FLAG_PIN = 0x01,
SECURITY_FLAG_UNK = 0x02,
SECURITY_FLAG_AUTHENTICATOR = 0x04
};

// GCC have alternative #pragma pack(N) syntax and old gcc version not support pack(push,N), also any gcc version not support it at some paltform
#if defined( __GNUC__ )
#pragma pack(1)
Expand All @@ -66,6 +78,18 @@ typedef struct AUTH_LOGON_CHALLENGE_C
uint8 I[1];
} sAuthLogonChallenge_C;

typedef struct AUTH_LOGON_PIN_DATA_C
{
uint8 salt[16];
uint8 hash[20];
} sAuthLogonPinData_C;

typedef struct AUTH_LOGON_AUTHENTICATOR_DATA_C
{
uint8 unk; // Has to be 0x01
uint8 keys[6]; // Valid code must be 6 digits

This comment has been minimized.

Copy link
@Shauren

Shauren Nov 13, 2017

Contributor

This can be any length - your "unk" field is the length.

google authenticator spits out 6 digit codes but battle.net one uses 8 digits

This comment has been minimized.

Copy link
@killerwife

killerwife Nov 13, 2017

Contributor

So it can be made into more than 8 digits for example?

This comment has been minimized.

Copy link
@Laizerox

Laizerox Nov 13, 2017

Author Member

Ooh, awesome. Thanks let's fix this!

This comment has been minimized.

Copy link
@Shauren

Shauren Nov 13, 2017

Contributor

It can, if you change the modulo in generateToken (both on server and authenticator itself)

} sAuthLogonAuthenticatorData_C;

// typedef sAuthLogonChallenge_C sAuthReconnectChallenge_C;
/*
typedef struct
Expand Down Expand Up @@ -265,7 +289,7 @@ void AuthSocket::SendProof(Sha1Hash sha)
case 6005: // 1.12.2
case 6141: // 1.12.3
{
sAuthLogonProof_S_BUILD_6005 proof;
sAuthLogonProof_S_BUILD_6005 proof{};
memcpy(proof.M2, sha.GetDigest(), 20);
proof.cmd = CMD_AUTH_LOGON_PROOF;
proof.error = 0;
Expand All @@ -282,7 +306,7 @@ void AuthSocket::SendProof(Sha1Hash sha)
case 12340: // 3.3.5a
default: // or later
{
sAuthLogonProof_S proof;
sAuthLogonProof_S proof{};
memcpy(proof.M2, sha.GetDigest(), 20);
proof.cmd = CMD_AUTH_LOGON_PROOF;
proof.error = 0;
Expand Down Expand Up @@ -358,30 +382,35 @@ bool AuthSocket::_HandleLogonChallenge()

///- Verify that this IP is not in the ip_banned table
// No SQL injection possible (paste the IP address as passed by the socket)
QueryResult* result = LoginDatabase.PQuery("SELECT unbandate FROM ip_banned WHERE "
// permanent still banned
"(unbandate = bandate OR unbandate > UNIX_TIMESTAMP()) AND ip = '%s'", m_address.c_str());
if (result)
std::unique_ptr<QueryResult> ip_banned_result(LoginDatabase.PQuery("SELECT unbandate FROM ip_banned "
"WHERE (unbandate = bandate OR unbandate > UNIX_TIMESTAMP()) AND ip = '%s'", m_address.c_str()));

std::unique_ptr<QueryResult> account_banned_result(LoginDatabase.PQuery(
"SELECT ab.unbandate FROM account_banned ab LEFT JOIN account a ON a.id = ab.id "
"WHERE active = 1 AND a.username = '%s' AND (ab.unbandate = ab.bandate OR ab.unbandate > UNIX_TIMESTAMP())", _safelogin.c_str()));

if (ip_banned_result || account_banned_result)
{
pkt << (uint8)WOW_FAIL_BANNED;
BASIC_LOG("[AuthChallenge] Banned ip %s tries to login!", m_address.c_str());
delete result;
}
else
{
///- 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,v,s FROM account WHERE username = '%s'", _safelogin.c_str());
QueryResult* result = LoginDatabase.PQuery("SELECT sha_pass_hash,id,locked,last_ip,gmlevel,v,s,token FROM account WHERE username = '%s'", _safelogin.c_str());
if (result)
{
Field* fields = result->Fetch();

///- If the IP is 'locked', check that the player comes indeed from the correct IP address
bool locked = false;
if ((*result)[2].GetUInt8() == 1) // if ip is locked
if (fields[2].GetUInt8() == 1) // if ip is locked
{
DEBUG_LOG("[AuthChallenge] Account '%s' is locked to IP - '%s'", _login.c_str(), (*result)[3].GetString());
DEBUG_LOG("[AuthChallenge] Account '%s' is locked to IP - '%s'", _login.c_str(), fields[3].GetString());
DEBUG_LOG("[AuthChallenge] Player address is '%s'", m_address.c_str());
if (strcmp((*result)[3].GetString(), m_address.c_str()))
if (strcmp(fields[3].GetString(), m_address.c_str()))
{
DEBUG_LOG("[AuthChallenge] Account IP differs");
pkt << (uint8) WOW_FAIL_SUSPENDED;
Expand All @@ -401,7 +430,7 @@ bool AuthSocket::_HandleLogonChallenge()
{
///- If the account is banned, reject the logon attempt
QueryResult* banresult = LoginDatabase.PQuery("SELECT bandate,unbandate FROM account_banned WHERE "
"id = %u AND active = 1 AND (unbandate > UNIX_TIMESTAMP() OR unbandate = bandate)", (*result)[1].GetUInt32());
"id = %u AND active = 1 AND (unbandate > UNIX_TIMESTAMP() OR unbandate = bandate)", fields[1].GetUInt32());
if (banresult)
{
if ((*banresult)[0].GetUInt64() == (*banresult)[1].GetUInt64())
Expand All @@ -420,11 +449,11 @@ bool AuthSocket::_HandleLogonChallenge()
else
{
///- Get the password from the account table, upper it, and make the SRP6 calculation
std::string rI = (*result)[0].GetCppString();
std::string rI = fields[0].GetCppString();

///- 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();
std::string databaseV = fields[5].GetCppString();
std::string databaseS = fields[6].GetCppString();

DEBUG_LOG("database authentication values: v='%s' s='%s'", databaseV.c_str(), databaseS.c_str());

Expand Down Expand Up @@ -458,15 +487,21 @@ bool AuthSocket::_HandleLogonChallenge()
pkt.append(s.AsByteArray(), s.GetNumBytes());// 32 bytes
pkt.append(unk3.AsByteArray(16), 16);
uint8 securityFlags = 0;
pkt << uint8(securityFlags); // security flags (0x0...0x04)

if (securityFlags & 0x01) // PIN input
_token = fields[7].GetCppString();
if (!_token.empty() && _build >= 8606) // authenticator was added in 2.4.3
securityFlags = SECURITY_FLAG_AUTHENTICATOR;

pkt << uint8(securityFlags); // security flags (0x0...0x04)

if (securityFlags & SECURITY_FLAG_PIN) // PIN input
{
pkt << uint32(0);
pkt << uint64(0) << uint64(0); // 16 bytes hash?
pkt << uint64(0);
pkt << uint64(0);
}

if (securityFlags & 0x02) // Matrix input
if (securityFlags & SECURITY_FLAG_UNK) // Matrix input
{
pkt << uint8(0);
pkt << uint8(0);
Expand All @@ -475,12 +510,10 @@ bool AuthSocket::_HandleLogonChallenge()
pkt << uint64(0);
}

if (securityFlags & 0x04) // Security token input
{
if (securityFlags & SECURITY_FLAG_AUTHENTICATOR) // Authenticator input
pkt << uint8(1);
}

uint8 secLevel = (*result)[4].GetUInt8();
uint8 secLevel = fields[4].GetUInt8();
_accountSecurityLevel = secLevel <= SEC_ADMINISTRATOR ? AccountTypes(secLevel) : SEC_ADMINISTRATOR;

_localizationName.resize(4);
Expand All @@ -496,10 +529,9 @@ bool AuthSocket::_HandleLogonChallenge()
delete result;
}
else // no account
{
pkt << (uint8) WOW_FAIL_UNKNOWN_ACCOUNT;
}
}

Write((const char *)pkt.contents(), pkt.size());
return true;
}
Expand All @@ -509,7 +541,7 @@ bool AuthSocket::_HandleLogonProof()
{
DEBUG_LOG("Entering _HandleLogonProof");
///- Read the packet
sAuthLogonProof_C lp;
sAuthLogonProof_C lp{};
if (!Read((char*)&lp, sizeof(sAuthLogonProof_C)))
return false;

Expand All @@ -519,7 +551,6 @@ bool AuthSocket::_HandleLogonProof()
/// <ul><li> If the client has no valid version
if (!FindBuildInfo(_build))
{

// no patch found
ByteBuffer pkt;
pkt << (uint8) CMD_AUTH_LOGON_CHALLENGE;
Expand All @@ -533,7 +564,6 @@ 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
Expand Down Expand Up @@ -611,6 +641,28 @@ bool AuthSocket::_HandleLogonProof()
///- Check if SRP6 results match (password is correct), else send an error
if (!memcmp(M.AsByteArray(), lp.M1, 20))
{
if (lp.securityFlags & SECURITY_FLAG_AUTHENTICATOR || !_token.empty())
{
sAuthLogonAuthenticatorData_C authData{};
if (!Read((char*) &authData, sizeof(sAuthLogonAuthenticatorData_C)))
{
const char data[4] = {CMD_AUTH_LOGON_PROOF, WOW_FAIL_UNKNOWN_ACCOUNT, 3, 0};
Write(data, sizeof(data));
return true;
}

auto ServerToken = generateToken(_token.c_str());
auto clientToken = atoi((const char*) authData.keys);
if (ServerToken != clientToken)
{
BASIC_LOG("[AuthChallenge] Account %s tried to login with wrong pincode! Given %u Expected %u", _login.c_str(), clientToken, ServerToken);

const char data[4] = { CMD_AUTH_LOGON_PROOF, WOW_FAIL_UNKNOWN_ACCOUNT, 3, 0};
Write(data, sizeof(data));
return true;
}
}

BASIC_LOG("User '%s' successfully authenticated", _login.c_str());

///- Update the sessionkey, last_ip, last login time and reset number of failed logins in the account table for this account
Expand Down Expand Up @@ -839,7 +891,6 @@ bool AuthSocket::_HandleRealmList()
hdr.append(pkt);

Write((const char *)hdr.contents(), hdr.size());

return true;
}

Expand Down Expand Up @@ -1004,4 +1055,29 @@ bool AuthSocket::_HandleXferAccept()
ReadSkip(1);

return true;
}

int32 AuthSocket::generateToken(char const* b32key)
{
size_t keySize = strlen(b32key);
size_t bufSize = (keySize + 7) / 8 * 5;
char* encoded = new char[bufSize];
memset(encoded, 0, bufSize);
unsigned int hmac_result_size = HMAC_RES_SIZE;
unsigned char hmac_result[HMAC_RES_SIZE];
unsigned long timestamp = time(nullptr)/30;
unsigned char challenge[8];

for (int i = 8; i--; timestamp >>= 8)
challenge[i] = timestamp;

base32_decode(b32key, encoded, bufSize);
HMAC(EVP_sha1(), encoded, bufSize, challenge, 8, hmac_result, &hmac_result_size);
unsigned int offset = hmac_result[19] & 0xF;
unsigned int truncHash = (hmac_result[offset] << 24) | (hmac_result[offset + 1] << 16 ) | (hmac_result[offset + 2] << 8) | (hmac_result[offset + 3]);
truncHash &= 0x7FFFFFFF;

delete[] encoded;

return truncHash % 1000000;
}
4 changes: 4 additions & 0 deletions src/realmd/AuthSocket.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@

#include <functional>

#define HMAC_RES_SIZE 20

class AuthSocket : public MaNGOS::Socket
{
public:
Expand All @@ -43,6 +45,7 @@ class AuthSocket : public MaNGOS::Socket

void SendProof(Sha1Hash sha);
void LoadRealmlist(ByteBuffer& pkt, uint32 acctid);
int32 generateToken(char const* b32key);

bool _HandleLogonChallenge();
bool _HandleLogonProof();
Expand Down Expand Up @@ -77,6 +80,7 @@ class AuthSocket : public MaNGOS::Socket

std::string _login;
std::string _safelogin;
std::string _token;

// Since GetLocaleByName() is _NOT_ bijective, we have to store the locale as a string. Otherwise we can't differ
// between enUS and enGB, which is important for the patch system
Expand Down
12 changes: 12 additions & 0 deletions src/shared/Auth/HMACSHA1.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,18 @@
#include "BigNumber.h"

HMACSHA1::HMACSHA1(uint32 len, uint8* seed)
{
memcpy(&m_key, seed, len);
#if defined(OPENSSL_VERSION_NUMBER) && OPENSSL_VERSION_NUMBER >= 0x10100000L
m_ctx = HMAC_CTX_new();
HMAC_Init_ex(m_ctx, &m_key, len, EVP_sha1(), nullptr);
#else
HMAC_CTX_init(&m_ctx);
HMAC_Init_ex(&m_ctx, &m_key, len, EVP_sha1(), nullptr);
#endif
}

HMACSHA1::HMACSHA1(uint32 len, uint8* seed, bool) // to get over the default constructor
{
#if defined(OPENSSL_VERSION_NUMBER) && OPENSSL_VERSION_NUMBER >= 0x10100000L
m_ctx = HMAC_CTX_new();
Expand Down
1 change: 1 addition & 0 deletions src/shared/Auth/HMACSHA1.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ class HMACSHA1
{
public:
HMACSHA1(uint32 len, uint8* seed);
HMACSHA1(uint32 len, uint8* seed, bool);
~HMACSHA1();
void UpdateBigNumber(BigNumber* bn);
void UpdateData(const uint8* data, int length);
Expand Down

8 comments on commit 12ce14f

@Fra298
Copy link

@Fra298 Fra298 commented on 12ce14f Nov 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't compile under VS2017 : 

1>C:\mangos\mangos-wotlk\src\shared\Auth\HMACSHA1.cpp(24): error C2065: 'm_key' : undeclared identifier

@NeatElves
Copy link

@NeatElves NeatElves commented on 12ce14f Nov 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VS2015 - Can't compile similarly.(

@Phatcat
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ehhh, Chaos also implemented it in his PR (cmangos/mangos-classic#193), so if anything he actually went the whole 9 yards, and not like you make it out as he only did the reversing part. So you guys have been letting his PR hang forever and then implemented it yourselves and pretended that you guys done the work load. How is this not plagiarism? because you give him a nod for the reversing of the protocols?

You seriously going out of your way to state, practially steal credit from someone else's work, for people who merely merge and port now?

@cyberium
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Phatcat its always easy to point some error from someone that work so much here. For me its a mistake and its on discussion to avoid same error in future.
Btw in this code there is only part of @Chaosvex code. So proper commit order had to be, merge his PR(or crypt part of it) then add @Laizerox code(authentificator).

Anyway the author order is perfectly correct in term of this commit (its not pin implementation!), afaik its not possible to set two author for same commit.

@Phatcat
Copy link
Contributor

@Phatcat Phatcat commented on 12ce14f Nov 17, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Phatcat its always easy to point some error from someone that work so much here

What, because I didn't use to put in a lot of hours and get my fair share of comments regarding my work when I was a member of the team? - that doesn't excuse plagiarism.

afaik its not possible to set two author for same commit.

So why try to squeeze it all into one commit? Also looking at the PR and comparing it with the merged code it's easy to see where the merged code comes from.

Not only that, but doing stuff like this kills any incentive for outside people to want to open PRs when the treatment they get is that they'll be practically ignored for a year and then when their work finally gets merged someone else takes the credit. You tell me who wants to contribute to such a project.

@cyberium
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What, because I didn't use to put in a lot of hours and get my fair share of comments regarding my work when I was a member of the team? - that doesn't excuse plagiarism.

No, it doesn't excuse, but only guys that work can do mistake (finally its not the case here so big kudo to @killerwife for his excellent job as always). Some are better at bashing then doing anything...

So why try to squeeze it all into one commit? Also looking at the PR and comparing it with the merged code it's easy to see where the merged code comes from.

Yes after reviewing those code its easy to see that the code is not from @Chaosvex PR (his code is better than this commit btw). Only 2 included files are the same... So better try to learn to read the code to be able to see the difference. This code come from trinity...

blabla...

We don't care much about your opinion, you already expressed yourself a bit too much here. So try to be more constructive or we don't need anymore any of your reaction here.

@Phatcat
Copy link
Contributor

@Phatcat Phatcat commented on 12ce14f Nov 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So ban me some more, Cyberium. Like I care at this point.

Yes after reviewing those code its easy to see that the code is not from @Chaosvex PR (his code is better than this commit btw).

So, you outright implemented an inferiour version with less functionality on purpose, and yes, looking at his PR it is very clear to see where laizer and killer got their inspiration and denying it / trying to underplay that fact is messed up. So after letting his PR hang for over a year on what's supposed to be an 'educational project', someone else implements an inferiour version and you're up in arms to defend them?, hell, it isn't more than a few months ago you were about to close his PR as a mere 'proof of concept' that no-one in your mind needed;

Does someone really need this? I mean its nice proof of concept but its a big amount of code to add for this feature that i myself doubt anyone will use.
I will go to close it if no answer.

What a joke.

@cyberium
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok if you say so...
Yes i still doubt of the need of this code but why not, im not he only user of that code... And i was not against if some want it i was open.

So ban me some more, Cyberium. Like I care at this point.

Be my guest :)

Please sign in to comment.