Page MenuHomeWildfire Games

Verify password in Authenticate
ClosedPublic

Authored by wraitii on Jan 20 2021, 8:12 PM.

Details

Summary

This followed #5913 and rP24728. It validates the password for actually rejoining the game, so even a player that knows the connection data cannot join.

Very similar to D3075, but that one is for the host and will have to go alongside.

Probably a necessary functionality for dedicated servers. Refs #3556

Test Plan

Join lobby games with / without password.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

wraitii created this revision.Jan 20 2021, 8:12 PM

Build is green

builderr-debug-macos.txt
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libnetwork_dbg.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libtinygettext_dbg.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libtinygettext_dbg.a(tinygettext.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/liblobby_dbg.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libglooxwrapper_dbg.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file

See https://jenkins.wildfiregames.com/job/macos-differential/2995/display/redirect for more details.

Build is green

builderr-debug-gcc7.txt
../../../source/network/scripting/JSInterface_Network.cpp: In function 'void JSI_Network::StartNetworkJoinLobby(ScriptInterface::CmptPrivate*, const CStrW&, const CStr8&, const CStr8&)':
../../../source/network/scripting/JSInterface_Network.cpp:171:71: warning: unused parameter 'pCmptPrivate' [-Wunused-parameter]
 void JSI_Network::StartNetworkJoinLobby(ScriptInterface::CmptPrivate* pCmptPrivate, const CStrW& playerName, const CStr& hostJID, const CStr& password)
                                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~
builderr-release-gcc7.txt
../../../source/network/scripting/JSInterface_Network.cpp: In function 'void JSI_Network::StartNetworkJoinLobby(ScriptInterface::CmptPrivate*, const CStrW&, const CStr8&, const CStr8&)':
../../../source/network/scripting/JSInterface_Network.cpp:171:71: warning: unused parameter 'pCmptPrivate' [-Wunused-parameter]
 void JSI_Network::StartNetworkJoinLobby(ScriptInterface::CmptPrivate* pCmptPrivate

See https://jenkins.wildfiregames.com/job/docker-differential/4652/display/redirect for more details.

wraitii requested review of this revision.Jan 20 2021, 9:56 PM
Stan added a subscriber: Stan.Jan 20 2021, 10:59 PM

years

source/network/NetClient.cpp
178 ↗(On Diff #15580)
source/network/NetMessages.h
124 ↗(On Diff #15580)

Where is the TODO fixed?

wraitii added inline comments.Jan 20 2021, 11:02 PM
source/network/NetMessages.h
124 ↗(On Diff #15580)

The TODO is fixed because the passwords are hashed securely client-side, so IMO it's fine to write it in the logs.

wraitii added inline comments.Jan 20 2021, 11:10 PM
source/network/NetMessages.h
124 ↗(On Diff #15580)

As noted on IRC, I'm tired. Since the hashes are consistent, entering the same password will always lead to the same hash, so this is problematic because it's likely people will reuse passwords.

Probably should inject some uniqueness to the hash, need to think about it.

wraitii planned changes to this revision.Jan 20 2021, 11:10 PM
wraitii updated this revision to Diff 15645.Jan 23 2021, 4:59 PM

Make the password not print to the logs. The length doesn't matter since it's hashed to a constant length (or 0 if there is no password, but that's also not secret information).

Build is green

builderr-debug-macos.txt
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libnetwork_dbg.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/liblobby_dbg.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libsimulation2_dbg.a(precompiled.o) has no symbols
../../../source/network/scripting/JSInterface_Network.cpp:171:71: warning: unused parameter 'pCmptPrivate' [-Wunused-parameter]
void JSI_Network::StartNetworkJoinLobby(ScriptInterface::CmptPrivate* pCmptPrivate, const CStrW& playerName, const CStr& hostJID, const CStr& password)
                                                                      ^
1 warning generated.
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/us

See https://jenkins.wildfiregames.com/job/macos-differential/3027/display/redirect for more details.

Build is green

builderr-debug-gcc7.txt
../../../source/network/scripting/JSInterface_Network.cpp: In function 'void JSI_Network::StartNetworkJoinLobby(ScriptInterface::CmptPrivate*, const CStrW&, const CStr8&, const CStr8&)':
../../../source/network/scripting/JSInterface_Network.cpp:171:71: warning: unused parameter 'pCmptPrivate' [-Wunused-parameter]
 void JSI_Network::StartNetworkJoinLobby(ScriptInterface::CmptPrivate* pCmptPrivate, const CStrW& playerName, const CStr& hostJID, const CStr& password)
                                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~
builderr-release-gcc7.txt
../../../source/network/scripting/JSInterface_Network.cpp: In function 'void JSI_Network::StartNetworkJoinLobby(ScriptInterface::CmptPrivate*, const CStrW&, const CStr8&, const CStr8&)':
../../../source/network/scripting/JSInterface_Network.cpp:171:71: warning: unused parameter 'pCmptPrivate' [-Wunused-parameter]
 void JSI_Network::StartNetworkJoinLobby(ScriptInterface::CmptPrivate* pCmptPrivate

See https://jenkins.wildfiregames.com/job/docker-differential/4685/display/redirect for more details.

wraitii updated this revision to Diff 15647.Jan 23 2021, 6:43 PM

Slight formatting tweaks before commit.

Build is green

builderr-debug-macos.txt
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libnetwork_dbg.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/liblobby_dbg.a(precompiled.o) has no symbols
../../../source/network/scripting/JSInterface_Network.cpp:171:71: warning: unused parameter 'pCmptPrivate' [-Wunused-parameter]
void JSI_Network::StartNetworkJoinLobby(ScriptInterface::CmptPrivate* pCmptPrivate, const CStrW& playerName, const CStr& hostJID, const CStr& password)
                                                                      ^
1 warning generated.
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libengine_dbg.a(precompiled.o) has no symbols
ld: warning: text-based stub file /System/Library/Frameworks//CoreAudio.framework/Core

See https://jenkins.wildfiregames.com/job/macos-differential/3028/display/redirect for more details.

Build is green

builderr-debug-gcc7.txt
../../../source/network/scripting/JSInterface_Network.cpp: In function 'void JSI_Network::StartNetworkJoinLobby(ScriptInterface::CmptPrivate*, const CStrW&, const CStr8&, const CStr8&)':
../../../source/network/scripting/JSInterface_Network.cpp:171:71: warning: unused parameter 'pCmptPrivate' [-Wunused-parameter]
 void JSI_Network::StartNetworkJoinLobby(ScriptInterface::CmptPrivate* pCmptPrivate, const CStrW& playerName, const CStr& hostJID, const CStr& password)
                                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~
builderr-release-gcc7.txt
../../../source/network/scripting/JSInterface_Network.cpp: In function 'void JSI_Network::StartNetworkJoinLobby(ScriptInterface::CmptPrivate*, const CStrW&, const CStr8&, const CStr8&)':
../../../source/network/scripting/JSInterface_Network.cpp:171:71: warning: unused parameter 'pCmptPrivate' [-Wunused-parameter]
 void JSI_Network::StartNetworkJoinLobby(ScriptInterface::CmptPrivate* pCmptPrivate

See https://jenkins.wildfiregames.com/job/docker-differential/4686/display/redirect for more details.

This revision was not accepted when it landed; it landed in state Needs Review.Jan 23 2021, 7:04 PM
This revision was automatically updated to reflect the committed changes.