Clean up some auth packet handling related code

diff --git a/src/clientiface.cpp b/src/clientiface.cpp
index a4bfb824259724015a2c2f2c14ad49306409b6d4..5733b05def440d7df18bebbd2fe71979581a4e3c 100644 (file)
--- a/src/clientiface.cpp
+++ b/src/clientiface.cpp
@@ -472,20 +472,14 @@ void RemoteClient::notifyEvent(ClientStateEvent event)
                {
                case CSE_AuthAccept:
                        m_state = CS_AwaitingInit2;
-                       if (chosen_mech == AUTH_MECHANISM_SRP ||
-                                       chosen_mech == AUTH_MECHANISM_LEGACY_PASSWORD)
-                               srp_verifier_delete((SRPVerifier *) auth_data);
-                       chosen_mech = AUTH_MECHANISM_NONE;
+                       resetChosenMech();
                        break;
                case CSE_Disconnect:
                        m_state = CS_Disconnecting;
                        break;
                case CSE_SetDenied:
                        m_state = CS_Denied;
-                       if (chosen_mech == AUTH_MECHANISM_SRP ||
-                                       chosen_mech == AUTH_MECHANISM_LEGACY_PASSWORD)
-                               srp_verifier_delete((SRPVerifier *) auth_data);
-                       chosen_mech = AUTH_MECHANISM_NONE;
+                       resetChosenMech();
                        break;
                default:
                        myerror << "HelloSent: Invalid client state transition! " << event;
@@ -561,9 +555,7 @@ void RemoteClient::notifyEvent(ClientStateEvent event)
                        break;
                case CSE_SudoSuccess:
                        m_state = CS_SudoMode;
-                       if (chosen_mech == AUTH_MECHANISM_SRP)
-                               srp_verifier_delete((SRPVerifier *) auth_data);
-                       chosen_mech = AUTH_MECHANISM_NONE;
+                       resetChosenMech();
                        break;
                /* Init GotInit2 SetDefinitionsSent SetMediaSent SetDenied */
                default:
@@ -598,7 +590,7 @@ void RemoteClient::notifyEvent(ClientStateEvent event)
 
 void RemoteClient::resetChosenMech()
 {
-       if (chosen_mech == AUTH_MECHANISM_SRP) {
+       if (auth_data) {
                srp_verifier_delete((SRPVerifier *) auth_data);
                auth_data = nullptr;
        }

diff --git a/src/network/clientpackethandler.cpp b/src/network/clientpackethandler.cpp
index 15b5766404d3a06b031ee988876745f656b7d490..55d20d673ab5c4378b9b659efe2105a53aa6cfc8 100644 (file)
--- a/src/network/clientpackethandler.cpp
+++ b/src/network/clientpackethandler.cpp
@@ -183,7 +183,7 @@ void Client::handleCommand_AccessDenied(NetworkPacket* pkt)
        m_access_denied_reason = "Unknown";
 
        if (pkt->getCommand() != TOCLIENT_ACCESS_DENIED) {
-               // 13/03/15 Legacy code from 0.4.12 and lesser but is still used
+               // Legacy code from 0.4.12 and older but is still used
                // in some places of the server code
                if (pkt->getSize() >= 2) {
                        std::wstring wide_reason;
@@ -196,14 +196,14 @@ void Client::handleCommand_AccessDenied(NetworkPacket* pkt)
        if (pkt->getSize() < 1)
                return;
 
-       u8 denyCode = SERVER_ACCESSDENIED_UNEXPECTED_DATA;
+       u8 denyCode;
        *pkt >> denyCode;
+
        if (denyCode == SERVER_ACCESSDENIED_SHUTDOWN ||
                        denyCode == SERVER_ACCESSDENIED_CRASH) {
                *pkt >> m_access_denied_reason;
-               if (m_access_denied_reason.empty()) {
+               if (m_access_denied_reason.empty())
                        m_access_denied_reason = accessDeniedStrings[denyCode];
-               }
                u8 reconnect;
                *pkt >> reconnect;
                m_access_denied_reconnect = reconnect & 1;
@@ -220,9 +220,8 @@ void Client::handleCommand_AccessDenied(NetworkPacket* pkt)
                // Until then (which may be never), this is outside
                // of the defined protocol.
                *pkt >> m_access_denied_reason;
-               if (m_access_denied_reason.empty()) {
+               if (m_access_denied_reason.empty())
                        m_access_denied_reason = "Unknown";
-               }
        }
 }
 

diff --git a/src/network/networkprotocol.h b/src/network/networkprotocol.h
index f98a829ba68f4221b7db5edd9e11793003db8d73..3923cb85812e61dfc31d9a3e2ace8ff2522095c6 100644 (file)
--- a/src/network/networkprotocol.h
+++ b/src/network/networkprotocol.h
@@ -1006,7 +1006,7 @@ enum AuthMechanism
        AUTH_MECHANISM_FIRST_SRP = 1 << 2,
 };
 
-enum AccessDeniedCode {
+enum AccessDeniedCode : u8 {
        SERVER_ACCESSDENIED_WRONG_PASSWORD,
        SERVER_ACCESSDENIED_UNEXPECTED_DATA,
        SERVER_ACCESSDENIED_SINGLEPLAYER,
@@ -1029,18 +1029,18 @@ enum NetProtoCompressionMode {
 
 const static std::string accessDeniedStrings[SERVER_ACCESSDENIED_MAX] = {
        "Invalid password",
-       "Your client sent something the server didn't expect.  Try reconnecting or updating your client",
+       "Your client sent something the server didn't expect.  Try reconnecting or updating your client.",
        "The server is running in simple singleplayer mode.  You cannot connect.",
-       "Your client's version is not supported.\nPlease contact server administrator.",
-       "Player name contains disallowed characters.",
-       "Player name not allowed.",
-       "Too many users.",
+       "Your client's version is not supported.\nPlease contact the server administrator.",
+       "Player name contains disallowed characters",
+       "Player name not allowed",
+       "Too many users",
        "Empty passwords are disallowed.  Set a password and try again.",
        "Another client is connected with this name.  If your client closed unexpectedly, try again in a minute.",
-       "Server authentication failed.  This is likely a server error.",
+       "Internal server error",
        "",
-       "Server shutting down.",
-       "This server has experienced an internal error. You will now be disconnected."
+       "Server shutting down",
+       "The server has experienced an internal error.  You will now be disconnected."
 };
 
 enum PlayerListModifer : u8

diff --git a/src/network/serveropcodes.cpp b/src/network/serveropcodes.cpp
index 44b65e8daa2821933cbcefb28ce172dd152d10f0..12665e7f17fb0176e3095b5783a868657b0c83d5 100644 (file)
--- a/src/network/serveropcodes.cpp
+++ b/src/network/serveropcodes.cpp
@@ -176,7 +176,7 @@ const ClientCommandFactory clientCommandFactoryTable[TOCLIENT_NUM_MSG_TYPES] =
        { "TOCLIENT_ACTIVE_OBJECT_MESSAGES",   0, true }, // 0x32 (may be sent as unrel over channel 1 too)
        { "TOCLIENT_HP",                       0, true }, // 0x33
        { "TOCLIENT_MOVE_PLAYER",              0, true }, // 0x34
-       { "TOCLIENT_ACCESS_DENIED_LEGACY",     0, true }, // 0x35
+       null_command_factory, // 0x35
        { "TOCLIENT_FOV",                      0, true }, // 0x36
        { "TOCLIENT_DEATHSCREEN",              0, true }, // 0x37
        { "TOCLIENT_MEDIA",                    2, true }, // 0x38

diff --git a/src/network/serverpackethandler.cpp b/src/network/serverpackethandler.cpp
index 125e85cabc91755f42dd3b39b770848471db2ae8..4b9de488c76b168cbb3e6f563177fe823c63e22d 100644 (file)
--- a/src/network/serverpackethandler.cpp
+++ b/src/network/serverpackethandler.cpp
@@ -227,7 +227,7 @@ void Server::handleCommand_Init(NetworkPacket* pkt)
                Compose auth methods for answer
        */
        std::string encpwd; // encrypted Password field for the user
-       bool has_auth = m_script->getAuth(playername, &encpwd, NULL);
+       bool has_auth = m_script->getAuth(playername, &encpwd, nullptr);
        u32 auth_mechs = 0;
 
        client->chosen_mech = AUTH_MECHANISM_NONE;
@@ -1461,11 +1461,9 @@ void Server::handleCommand_FirstSrp(NetworkPacket* pkt)
        session_t peer_id = pkt->getPeerId();
        RemoteClient *client = getClient(peer_id, CS_Invalid);
        ClientState cstate = client->getState();
+       const std::string playername = client->getName();
 
-       std::string playername = client->getName();
-
-       std::string salt;
-       std::string verification_key;
+       std::string salt, verification_key;
 
        std::string addr_s = getPeerAddress(peer_id).serializeString();
        u8 is_empty;
@@ -1551,8 +1549,6 @@ void Server::handleCommand_SrpBytesA(NetworkPacket* pkt)
        RemoteClient *client = getClient(peer_id, CS_Invalid);
        ClientState cstate = client->getState();
 
-       bool wantSudo = (cstate == CS_Active);
-
        if (!((cstate == CS_HelloSent) || (cstate == CS_Active))) {
                actionstream << "Server: got SRP _A packet in wrong state " << cstate <<
                        " from " << getPeerAddress(peer_id).serializeString() <<
@@ -1560,6 +1556,8 @@ void Server::handleCommand_SrpBytesA(NetworkPacket* pkt)
                return;
        }
 
+       const bool wantSudo = (cstate == CS_Active);
+
        if (client->chosen_mech != AUTH_MECHANISM_NONE) {
                actionstream << "Server: got SRP _A packet, while auth is already "
                        "going on with mech " << client->chosen_mech << " from " <<
@@ -1606,8 +1604,7 @@ void Server::handleCommand_SrpBytesA(NetworkPacket* pkt)
 
        client->chosen_mech = chosen;
 
-       std::string salt;
-       std::string verifier;
+       std::string salt, verifier;
 
        if (based_on == 0) {
 
@@ -1657,10 +1654,10 @@ void Server::handleCommand_SrpBytesM(NetworkPacket* pkt)
        session_t peer_id = pkt->getPeerId();
        RemoteClient *client = getClient(peer_id, CS_Invalid);
        ClientState cstate = client->getState();
-       std::string addr_s = getPeerAddress(pkt->getPeerId()).serializeString();
-       std::string playername = client->getName();
+       const std::string addr_s = client->getAddress().serializeString();
+       const std::string playername = client->getName();
 
-       bool wantSudo = (cstate == CS_Active);
+       const bool wantSudo = (cstate == CS_Active);
 
        verbosestream << "Server: Received TOSERVER_SRP_BYTES_M." << std::endl;
 
@@ -1720,8 +1717,7 @@ void Server::handleCommand_SrpBytesM(NetworkPacket* pkt)
        if (client->create_player_on_auth_success) {
                m_script->createAuth(playername, client->enc_pwd);
 
-               std::string checkpwd; // not used, but needed for passing something
-               if (!m_script->getAuth(playername, &checkpwd, NULL)) {
+               if (!m_script->getAuth(playername, nullptr, nullptr)) {
                        errorstream << "Server: " << playername <<
                                " cannot be authenticated (auth handler does not work?)" <<
                                std::endl;

diff --git a/src/script/lua_api/l_server.cpp b/src/script/lua_api/l_server.cpp
index 88ab5e16b4b8dc41a51c8bcd24c1e28bce052a91..5b3054d179ff168acd87dfdf4ffa3554bb619041 100644 (file)
--- a/src/script/lua_api/l_server.cpp
+++ b/src/script/lua_api/l_server.cpp
@@ -325,12 +325,15 @@ int ModApiServer::l_disconnect_player(lua_State *L)
        else
                message.append("Disconnected.");
 
-       RemotePlayer *player = dynamic_cast<ServerEnvironment *>(getEnv(L))->getPlayer(name);
-       if (player == NULL) {
+       Server *server = getServer(L);
+
+       RemotePlayer *player = server->getEnv().getPlayer(name);
+       if (!player) {
                lua_pushboolean(L, false); // No such player
                return 1;
        }
-       getServer(L)->DenyAccess_Legacy(player->getPeerId(), utf8_to_wide(message));
+
+       server->DenyAccess(player->getPeerId(), SERVER_ACCESSDENIED_CUSTOM_STRING, message);
        lua_pushboolean(L, true);
        return 1;
 }

diff --git a/src/server.cpp b/src/server.cpp
index 8ca8a9bdaeb8ff4bb43634a8546327ea270ea745..9d7e8e56347a00f0a2655ee10030e2c2b215b34d 100644 (file)
--- a/src/server.cpp
+++ b/src/server.cpp
@@ -1038,8 +1038,7 @@ void Server::Receive()
                } catch (const ClientStateError &e) {
                        errorstream << "ProcessData: peer=" << peer_id << " what()="
                                         << e.what() << std::endl;
-                       DenyAccess_Legacy(peer_id, L"Your client sent something server didn't expect."
-                                       L"Try reconnecting or updating your client");
+                       DenyAccess(peer_id, SERVER_ACCESSDENIED_UNEXPECTED_DATA);
                } catch (const con::PeerNotFoundException &e) {
                        // Do nothing
                } catch (const con::NoIncomingDataException &e) {
@@ -1068,15 +1067,13 @@ PlayerSAO* Server::StageTwoClientInit(session_t peer_id)
                if (player && player->getPeerId() != PEER_ID_INEXISTENT) {
                        actionstream << "Server: Failed to emerge player \"" << playername
                                        << "\" (player allocated to an another client)" << std::endl;
-                       DenyAccess_Legacy(peer_id, L"Another client is connected with this "
-                                       L"name. If your client closed unexpectedly, try again in "
-                                       L"a minute.");
+                       DenyAccess(peer_id, SERVER_ACCESSDENIED_ALREADY_CONNECTED);
                } else {
                        errorstream << "Server: " << playername << ": Failed to emerge player"
                                        << std::endl;
-                       DenyAccess_Legacy(peer_id, L"Could not allocate player.");
+                       DenyAccess(peer_id, SERVER_ACCESSDENIED_SERVER_FAIL);
                }
-               return NULL;
+               return nullptr;
        }
 
        /*
@@ -1141,18 +1138,16 @@ void Server::ProcessData(NetworkPacket *pkt)
                Address address = getPeerAddress(peer_id);
                std::string addr_s = address.serializeString();
 
-               if(m_banmanager->isIpBanned(addr_s)) {
+               // FIXME: Isn't it a bit excessive to check this for every packet?
+               if (m_banmanager->isIpBanned(addr_s)) {
                        std::string ban_name = m_banmanager->getBanName(addr_s);
                        infostream << "Server: A banned client tried to connect from "
-                                       << addr_s << "; banned name was "
-                                       << ban_name << std::endl;
-                       // This actually doesn't seem to transfer to the client
-                       DenyAccess_Legacy(peer_id, L"Your ip is banned. Banned name was "
-                                       + utf8_to_wide(ban_name));
+                                       << addr_s << "; banned name was " << ban_name << std::endl;
+                       DenyAccess(peer_id, SERVER_ACCESSDENIED_CUSTOM_STRING,
+                               "Your IP is banned. Banned name was " + ban_name);
                        return;
                }
-       }
-       catch(con::PeerNotFoundException &e) {
+       } catch (con::PeerNotFoundException &e) {
                /*
                 * no peer for this packet found
                 * most common reason is peer timeout, e.g. peer didn't
@@ -1406,13 +1401,6 @@ void Server::SendAccessDenied(session_t peer_id, AccessDeniedCode reason,
        Send(&pkt);
 }
 
-void Server::SendAccessDenied_Legacy(session_t peer_id,const std::wstring &reason)
-{
-       NetworkPacket pkt(TOCLIENT_ACCESS_DENIED_LEGACY, 0, peer_id);
-       pkt << reason;
-       Send(&pkt);
-}
-
 void Server::SendDeathscreen(session_t peer_id, bool set_camera_point_target,
                v3f camera_point_target)
 {
@@ -2777,29 +2765,10 @@ void Server::DenySudoAccess(session_t peer_id)
 }
 
 
-void Server::DenyAccessVerCompliant(session_t peer_id, u16 proto_ver, AccessDeniedCode reason,
-               const std::string &str_reason, bool reconnect)
-{
-       SendAccessDenied(peer_id, reason, str_reason, reconnect);
-
-       m_clients.event(peer_id, CSE_SetDenied);
-       DisconnectPeer(peer_id);
-}
-
-
 void Server::DenyAccess(session_t peer_id, AccessDeniedCode reason,
-               const std::string &custom_reason)
-{
-       SendAccessDenied(peer_id, reason, custom_reason);
-       m_clients.event(peer_id, CSE_SetDenied);
-       DisconnectPeer(peer_id);
-}
-
-// 13/03/15: remove this function when protocol version 25 will become
-// the minimum version for MT users, maybe in 1 year
-void Server::DenyAccess_Legacy(session_t peer_id, const std::wstring &reason)
+               const std::string &custom_reason, bool reconnect)
 {
-       SendAccessDenied_Legacy(peer_id, reason);
+       SendAccessDenied(peer_id, reason, custom_reason, reconnect);
        m_clients.event(peer_id, CSE_SetDenied);
        DisconnectPeer(peer_id);
 }
@@ -2985,8 +2954,8 @@ std::wstring Server::handleChat(const std::string &name,
                        return ws.str();
                }
                case RPLAYER_CHATRESULT_KICK:
-                       DenyAccess_Legacy(player->getPeerId(),
-                                       L"You have been kicked due to message flooding.");
+                       DenyAccess(player->getPeerId(), SERVER_ACCESSDENIED_CUSTOM_STRING,
+                               "You have been kicked due to message flooding.");
                        return L"";
                case RPLAYER_CHATRESULT_OK:
                        break;

diff --git a/src/server.h b/src/server.h
index c0539329163c3dcf063e77317dfbc734693e5c87..008213c5d6e66dd8893d33412f8bd3dac3c35f5d 100644 (file)
--- a/src/server.h
+++ b/src/server.h
@@ -341,12 +341,9 @@ class Server : public con::PeerHandler, public MapEventReceiver,
        void deletingPeer(con::Peer *peer, bool timeout);
 
        void DenySudoAccess(session_t peer_id);
-       void DenyAccessVerCompliant(session_t peer_id, u16 proto_ver, AccessDeniedCode reason,
-               const std::string &str_reason = "", bool reconnect = false);
        void DenyAccess(session_t peer_id, AccessDeniedCode reason,
-               const std::string &custom_reason = "");
+               const std::string &custom_reason = "", bool reconnect = false);
        void acceptAuth(session_t peer_id, bool forSudoMode);
-       void DenyAccess_Legacy(session_t peer_id, const std::wstring &reason);
        void DisconnectPeer(session_t peer_id);
        bool getClientConInfo(session_t peer_id, con::rtt_stat_type type, float *retval);
        bool getClientInfo(session_t peer_id, ClientInfo &ret);

diff --git a/src/serverenvironment.cpp b/src/serverenvironment.cpp
index 34a1e33e5491cf2e27ea2670085f9d11c91b2e40..f8d84604b0bcc5a16077c09660c5c5ea2e79abbf 100644 (file)
--- a/src/serverenvironment.cpp
+++ b/src/serverenvironment.cpp
@@ -552,10 +552,8 @@ bool ServerEnvironment::removePlayerFromDatabase(const std::string &name)
 void ServerEnvironment::kickAllPlayers(AccessDeniedCode reason,
        const std::string &str_reason, bool reconnect)
 {
-       for (RemotePlayer *player : m_players) {
-               m_server->DenyAccessVerCompliant(player->getPeerId(),
-                       player->protocol_version, reason, str_reason, reconnect);
-       }
+       for (RemotePlayer *player : m_players)
+               m_server->DenyAccess(player->getPeerId(), reason, str_reason, reconnect);
 }
 
 void ServerEnvironment::saveLoadedPlayers(bool force)