From 1beb96cb2059f9699c23f529aa693af7cebeb789 Mon Sep 17 00:00:00 2001 From: elexis Date: Fri, 24 Aug 2018 11:29:38 +0000 Subject: [PATCH] Lobby to optionally require TLS certificate and certificate verification, refs #4737, #5257. These are config options because developers should be able to test a local lobby server quickly without going through the hassle to create a valid or invalid certificate or modify and compile the client. To protect from malicious JS mods reducing these security config options, these options as well as the hostname would have to be protected from JS access. The user might still connect to other lobbies through a hypothetical UI if there were a non-modifiable GUI confirmation dialog prior to the connection. Proofreading and feature design discussion by Vladislav and Dunedan on irc on 2018-08-19 and 2018-08-23. This was SVN commit r21875. --- binaries/data/config/default.cfg | 2 ++ source/lobby/XmppClient.cpp | 21 +++++++++++++++------ 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/binaries/data/config/default.cfg b/binaries/data/config/default.cfg index bcdc83eee6..717e73bd65 100644 --- a/binaries/data/config/default.cfg +++ b/binaries/data/config/default.cfg @@ -413,6 +413,8 @@ extended = true ; Whether to display the chat history history = 0 ; Number of past messages to display on join room = "arena23" ; Default MUC room to join server = "lobby.wildfiregames.com" ; Address of lobby server +require_tls = true ; Whether to reject connecting to the lobby if TLS encryption is unavailable. +verify_certificate = false ; Whether to reject connecting to the lobby if the TLS certificate is invalid (TODO get a valid certificate) terms_of_service = "0" ; Version (hash) of the Terms of Service that the user has accepted terms_of_use = "0" ; Version (hash) of the Terms of Use that the user has accepted xpartamupp = "wfgbot23" ; Name of the server-side XMPP-account that manage games diff --git a/source/lobby/XmppClient.cpp b/source/lobby/XmppClient.cpp index 7e685cfd62..19b248c22a 100644 --- a/source/lobby/XmppClient.cpp +++ b/source/lobby/XmppClient.cpp @@ -16,6 +16,7 @@ */ #include "precompiled.h" + #include "XmppClient.h" #include "StanzaExtensions.h" @@ -24,7 +25,6 @@ #endif #include "i18n/L10n.h" - #include "lib/external_libraries/enet.h" #include "lib/utf8.h" #include "network/NetServer.h" @@ -34,6 +34,8 @@ #include "ps/Pyrogenesis.h" #include "scriptinterface/ScriptInterface.h" +#include + //debug #if 1 #define DbgXMPP(x) @@ -96,8 +98,11 @@ XmppClient::XmppClient(const std::string& sUsername, const std::string& sPasswor else m_client = new glooxwrapper::Client(m_server); - // Disable TLS as we haven't set a certificate on the server yet - m_client->setTls(gloox::TLSDisabled); + // Optionally join without a TLS certificate, so a local server can be tested quickly. + // Security risks from malicious JS mods can be mitigated if this option and also the hostname and login are shielded from JS access. + bool require_tls = true; + CFG_GET_VAL("lobby.require_tls", require_tls); + m_client->setTls(require_tls ? gloox::TLSRequired : gloox::TLSOptional); // Disable use of the SASL PLAIN mechanism, to prevent leaking credentials // if the server doesn't list any supported SASL mechanism or the response @@ -248,11 +253,10 @@ void XmppClient::onDisconnect(gloox::ConnectionError error) } /** - * Handle TLS connection + * Handle TLS connection. */ bool XmppClient::onTLSConnect(const glooxwrapper::CertInfo& info) { - UNUSED2(info); DbgXMPP("onTLSConnect"); DbgXMPP( "status: " << info.status << @@ -262,7 +266,12 @@ bool XmppClient::onTLSConnect(const glooxwrapper::CertInfo& info) "\nmac: " << info.mac << "\ncipher: " << info.cipher << "\ncompression: " << info.compression ); - return true; + + // Optionally accept invalid certificates, see require_tls option. + bool verify_certificate = true; + CFG_GET_VAL("lobby.verify_certificate", verify_certificate); + + return info.status == gloox::CertOk || !verify_certificate; } /**