Tolerate packet reordering in the early init process - minetest.git

diff options

author	est31 <MTest31@outlook.com>	2016-05-18 06:18:08 +0200
committer	est31 <MTest31@outlook.com>	2016-05-22 15:56:54 +0200
commit	423d8c1b0d3841f6ce9756ab0d6b2e10408fc89b (patch)
tree	88a75511309d8bd1d27adb27f5d5f5d3845475cf /src/database-redis.h
parent	f64a6259b2b5e79103cfd4bc151304e6da39bca9 (diff)
download	minetest-423d8c1b0d3841f6ce9756ab0d6b2e10408fc89b.tar.gz minetest-423d8c1b0d3841f6ce9756ab0d6b2e10408fc89b.tar.bz2 minetest-423d8c1b0d3841f6ce9756ab0d6b2e10408fc89b.zip

Tolerate packet reordering in the early init process

Fixes a bug where packet reordering made the server give the client two peer ids instead of one. This in turn confused reliable packet sending and made connecting to the server fail. The client usually sends three packets at init: one "dummy" packet consisting of two 0 bytes, and the init packet as well as its legacy counterpart. The last one can be turned off since commit af30183124d40a969040d7de4b3a487feec466e4, but this is of lower relevance for the bug. The relevant part here is that network packet reorder (which is a normal occurence) can make the packets reach the server in different order. If reorder puts the dummy packet further behind, the following would happen before the patch: 1. The server will get one of the init packets on channel 1 and assign the client a peer id, as the packet will have zero as peer id. 2. The server sends a CONTROLTYPE_SET_PEER_ID packet to inform the client of the peer id. 3. The next packet from the client will contain the peer id set by the server. 4. The server sets the m_has_sent_with_id member for the client's peer structure to true. 5. Now the dummy packet arrives. It has a peer id of zero, therefore the server searches whether it already has a peer id for the address the packet was sent from. The search fails because m_has_sent_with_id was set to true and the server only searched for peers with m_has_sent_with_id set to false. 6. In a working setup, the server would assign the dummy packet to the correct peer id. However the server instead now assigns a second peer id and peer structure to the peer, and assign the packet to that new peer. 7. In order to inform the peer of its peer id, the server sends a CONTROLTYPE_SET_PEER_ID command packet, reliably, to the peer. This packet uses the new peer id. 8. The client sends an ack to that packet, not with the new peer id but with the peer id sent in 2. 9. This packet reaches the server, but it drops the ACK as the peer id does not map to any un-ACK-ed packets with that seqnum. The same time, the server still waits for an ACK with the new peer id, which of course won't come. This causes the server to periodically re-try sending that packet, and the client ACKing it each time. Steps 7-9 cause annoyances and erroneous output, but don't cause the connection failure itself. The actual mistake that causes the connection failure happens in 6: The server does not assign the dummy packet to the correct peer, but to a newly created one. Therefore, all further packets sent by the client on channel 0 are now buffered by the server as it waits for the dummy packet to reach the peer, which of course doesn't happen as the server assigned that packet to the second peer it created for the client. This makes the connection code indefinitely buffer the TOSERVER_CLIENT_READY packet, not passing it to higher level code, which stalls the continuation of the further init process indefinitely and causes the actual bug. Maybe this can be caused by reordered init packets as well, the only studied case was where network has reliably reordered the dummy packet to get sent after the init packets. The patch fixes the bug by not ignoring peers where m_has_sent_with_id has been set anymore. The other changes of the patch are just cleanups of unused methods and fields and additional explanatory comments. One could think of alternate ways to fix the bug: * The client could simply take the new peer id and continue communicating with that. This is however worse than the fix as it requires the peer id set command to be sent reliably (which currently happens, but it cant be changed anymore). Also, such a change would require both server and client to be patched in order for the bug to be fixed, as right now the client ignores peer id set commands after the peer id is different from PEER_ID_INEXISTENT and the server requires modification too to change the peer id internally. And, most importantly, right now we guarantee higher level server code that the peer id for a certain peer does not change. This guarantee would have to be broken, and it would require much larger changes to the server than this patch means. * One could stop sending the dummy packet. One may be unsure whether this is a good idea, as the meaning of the dummy packet is not known (it might be there for something important), and as it is possible that the init packets may cause this problem as well (although it may be possible too that they can't cause this). Thanks to @auouymous who had originally reported this bug and who has helped patiently in finding its cause.

Diffstat (limited to 'src/database-redis.h')

0 files changed, 0 insertions, 0 deletions

/* Minetest Copyright (C) 2013 celeron55, Perttu Ahola <celeron55@gmail.com> This program is free software; you can redistribute it and/or modify it under the terms of the GNU Lesser General Public License as published by the Free Software Foundation; either version 2.1 of the License, or (at your option) any later version. This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more details. You should have received a copy of the GNU Lesser General Public License along with this program; if not, write to the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ #include "cpp_api/s_server.h" #include "cpp_api/s_internal.h" #include "common/c_converter.h" #include "util/numeric.h" // myrand bool ScriptApiServer::getAuth(const std::string &playername, std::string *dst_password, std::set<std::string> *dst_privs, s64 *dst_last_login) { SCRIPTAPI_PRECHECKHEADER int error_handler = PUSH_ERROR_HANDLER(L); getAuthHandler(); lua_getfield(L, -1, "get_auth"); if (lua_type(L, -1) != LUA_TFUNCTION) throw LuaError("Authentication handler missing get_auth"); lua_pushstring(L, playername.c_str()); PCALL_RES(lua_pcall(L, 1, 1, error_handler)); lua_remove(L, -2); // Remove auth handler lua_remove(L, error_handler); // nil = login not allowed if (lua_isnil(L, -1)) return false; luaL_checktype(L, -1, LUA_TTABLE); std::string password; if (!getstringfield(L, -1, "password", password)) throw LuaError("Authentication handler didn't return password"); if (dst_password) *dst_password = password; lua_getfield(L, -1, "privileges"); if (!lua_istable(L, -1)) throw LuaError("Authentication handler didn't return privilege table"); if (dst_privs) readPrivileges(-1, *dst_privs); lua_pop(L, 1); // Remove key from privs table s64 last_login; if(!getintfield(L, -1, "last_login", last_login)) throw LuaError("Authentication handler didn't return last_login"); if (dst_last_login) *dst_last_login = (s64)last_login; return true; } void ScriptApiServer::getAuthHandler() { lua_State *L = getStack(); lua_getglobal(L, "core"); lua_getfield(L, -1, "registered_auth_handler"); if (lua_isnil(L, -1)){ lua_pop(L, 1); lua_getfield(L, -1, "builtin_auth_handler"); } setOriginFromTable(-1); lua_remove(L, -2); // Remove core if (lua_type(L, -1) != LUA_TTABLE) throw LuaError("Authentication handler table not valid"); } void ScriptApiServer::readPrivileges(int index, std::set<std::string> &result) { lua_State *L = getStack(); result.clear(); lua_pushnil(L); if (index < 0) index -= 1; while (lua_next(L, index) != 0) { // key at index -2 and value at index -1 std::string key = luaL_checkstring(L, -2); bool value = readParam<bool>(L, -1); if (value) result.insert(key); // removes value, keeps key for next iteration lua_pop(L, 1); } } void ScriptApiServer::createAuth(const std::string &playername, const std::string &password) { SCRIPTAPI_PRECHECKHEADER int error_handler = PUSH_ERROR_HANDLER(L); getAuthHandler(); lua_getfield(L, -1, "create_auth"); lua_remove(L, -2); // Remove auth handler if (lua_type(L, -1) != LUA_TFUNCTION) throw LuaError("Authentication handler missing create_auth"); lua_pushstring(L, playername.c_str()); lua_pushstring(L, password.c_str()); PCALL_RES(lua_pcall(L, 2, 0, error_handler)); lua_pop(L, 1); // Pop error handler } bool ScriptApiServer::setPassword(const std::string &playername, const std::string &password) { SCRIPTAPI_PRECHECKHEADER int error_handler = PUSH_ERROR_HANDLER(L); getAuthHandler(); lua_getfield(L, -1, "set_password"); lua_remove(L, -2); // Remove auth handler if (lua_type(L, -1) != LUA_TFUNCTION) throw LuaError("Authentication handler missing set_password"); lua_pushstring(L, playername.c_str()); lua_pushstring(L, password.c_str()); PCALL_RES(lua_pcall(L, 2, 1, error_handler)); lua_remove(L, error_handler); return lua_toboolean(L, -1); } bool ScriptApiServer::on_chat_message(const std::string &name, const std::string &message) { SCRIPTAPI_PRECHECKHEADER // Get core.registered_on_chat_messages lua_getglobal(L, "core"); lua_getfield(L, -1, "registered_on_chat_messages"); // Call callbacks lua_pushstring(L, name.c_str()); lua_pushstring(L, message.c_str()); runCallbacks(2, RUN_CALLBACKS_MODE_OR_SC); return readParam<bool>(L, -1); } void ScriptApiServer::on_mods_loaded() { SCRIPTAPI_PRECHECKHEADER // Get registered shutdown hooks lua_getglobal(L, "core"); lua_getfield(L, -1, "registered_on_mods_loaded"); // Call callbacks runCallbacks(0, RUN_CALLBACKS_MODE_FIRST); } void ScriptApiServer::on_shutdown() { SCRIPTAPI_PRECHECKHEADER // Get registered shutdown hooks lua_getglobal(L, "core"); lua_getfield(L, -1, "registered_on_shutdown"); // Call callbacks runCallbacks(0, RUN_CALLBACKS_MODE_FIRST); } std::string ScriptApiServer::formatChatMessage(const std::string &name, const std::string &message) { SCRIPTAPI_PRECHECKHEADER // Push function onto stack lua_getglobal(L, "core"); lua_getfield(L, -1, "format_chat_message"); // Push arguments onto stack lua_pushstring(L, name.c_str()); lua_pushstring(L, message.c_str()); // Actually call the function lua_call(L, 2, 1); // Fetch return value std::string ret = lua_tostring(L, -1); lua_pop(L, 1); return ret; } u32 ScriptApiServer::allocateDynamicMediaCallback(lua_State *L, int f_idx) { if (f_idx < 0) f_idx = lua_gettop(L) + f_idx + 1; lua_getglobal(L, "core"); lua_getfield(L, -1, "dynamic_media_callbacks"); luaL_checktype(L, -1, LUA_TTABLE); // Find a randomly generated token that doesn't exist yet int tries = 100; u32 token; while (1) { token = myrand(); lua_rawgeti(L, -2, token); bool is_free = lua_isnil(L, -1); lua_pop(L, 1); if (is_free) break; if (--tries < 0) FATAL_ERROR("Ran out of callbacks IDs?!"); } // core.dynamic_media_callbacks[token] = callback_func lua_pushvalue(L, f_idx); lua_rawseti(L, -2, token); lua_pop(L, 2); verbosestream << "allocateDynamicMediaCallback() = " << token << std::endl; return token; } void ScriptApiServer::freeDynamicMediaCallback(u32 token) { SCRIPTAPI_PRECHECKHEADER verbosestream << "freeDynamicMediaCallback(" << token << ")" << std::endl; // core.dynamic_media_callbacks[token] = nil lua_getglobal(L, "core"); lua_getfield(L, -1, "dynamic_media_callbacks"); luaL_checktype(L, -1, LUA_TTABLE); lua_pushnil(L); lua_rawseti(L, -2, token); lua_pop(L, 2); } void ScriptApiServer::on_dynamic_media_added(u32 token, const char *playername) { SCRIPTAPI_PRECHECKHEADER int error_handler = PUSH_ERROR_HANDLER(L); lua_getglobal(L, "core"); lua_getfield(L, -1, "dynamic_media_callbacks"); luaL_checktype(L, -1, LUA_TTABLE); lua_rawgeti(L, -1, token); luaL_checktype(L, -1, LUA_TFUNCTION); lua_pushstring(L, playername);


context:
space:
mode: