Skip to content

Commit

Permalink
Encrypted connections (#132)
Browse files Browse the repository at this point in the history
* Implement our own network code

If we want to encrypt the traffic with the server, we need to be able to work with the socket. SDL_net provides
a nice interface, but in the process abstracts away most of the details we need to be able to use SSL. So implement
our own socket type. Furthermore, split off the networking part of the message handling from multiplayer.c,
into a separate file, and use the new TCPSocket.

In time we may get rid of other uses of SDL_net in the code too. This is early code.

* Initial version of encrypted connections

Encrypt connection using TLS (or the highest encryption scheme both client and server support). This
requires server support to function, so it does not yet work. Lightly tested with a toy server.

* Move IPAddress struct to separate file

* If encryption is used, encrypt directly from the start, don't wait for invitation from the server

* Add peek() method to socket, to check incoming network data without removing it.

* Verify server certificates

* Add function to set blocking mode of sockets, make client sockets blocking by default

* Add (optiona;) encryption field to servers.lst

Add an optional Encryption column to the servers.lst file between the Port and Description fields that
determines whether the connection should be encrypted. If the columns contains "crypt", "encrypt", or "encrypted",
the connection should be encrypted, if it contains "plain" or "clear", or is left empty, the connection is
not encrypted. The match is case-insensitive. Please note that the field is optional, so any other value
than the 5 listed above will be parsed as part of the description. Conversely, server descriptions should
not start with any of the five reserved words unless the encryption column is explicitly set.

Since the field is optional, existing servers,lst files should still work. Note that the new field is only
parsed when the client is compiled with USE_SSL defined. Otherwise, any contents of the encryption
column will be added to the description.

* Fix parsing over servers.lst when USE_SSL is off.

* Fix for parsing servers.lst

The field width for strings in a sscanf() format string does not include the terminating NULL byte. Leave
room for that. Found by address sanitizer.

* Encrypted connection fixes:
*) Make connecting and setting up encryption a single operation. Otherwise, on reconnect, the reader thread
   will start reading after the connection is made, butbefore encryption is set up, wreaking havoc on the
   TLS handshake.
*) Get rid of the mutex on incoming data. It does not protect anything, as only the reader thread accesses
   the incoming data buffer.
*) Set a flag on login, so that on reconnect the login information is automatically sent again
*) Properly close the SSL session before closing the socket, if possible
*) Protect the SSL object by a mutex to prevent simultaneous reads and writes which OpenSSL cannot handle

* Documentation updates

* Add C++ abstraction for windows

Add a C++ Window class that wraps the C window_info struct, and can automatically register event handlers
and handle capturing lambdas as callbacks. This class does not yet expose the full set of window and
widget capabilties, and will probably evolve further. But it is a base to start from.

* Add text popup

Add a new class for text popups that can display a text message and a set of buttons with various callbacks
underneath. This is heavily inspired by the serverpopup window, but adds the ability to add multiple buttons.
The server popup could be trivially reimplemented in terms of this class.

* Show a warning popup when certificate check fails

If the certificate sent by the server over an encrypted connection cannot be verified, show a popup
dialog to the user with a warning emssage, and options to either disconnect, or continue accepting the risks.

* Clean up connection before exit.

Welcome to the wonderful world of global statics. The destructor of the Connection singleton may be called
after the context menu container singleton. This is a problem since the Connection can contain a (popup) window,
that will try to clean up its (non-existant) context menu upon destruction, and tries to look itself up
in the destroyed container. Ensure the popup is destroyed before the program finsihes.

* Add CMake support for building with SSL

* Load certificates from game directories

Load SSL certificates from both the game data directory, and the user's configuration directory instead of
the current working directory.

* Open servers.lst in binary mode

Although servers.lst is a text file, open it in binary mode as otherwise the check on the number of bytes
read will fail on Windows (or other platforms that use multiple characters to specify end of line).

* Changes to make the new network code compile on Windows

* Make connection actually work on windows

Fixes to make connections actually work on windows: initialize the
winsock library and fix an invalid check on the file descriptor leading
to the socket not actually calling connect().

* More network initialization/deinitialization

Clean up after the last socket is destroyed. Initialize in the constructor, not on connect, and don't throw
an exception on failure to initialize. We might be able to catch it, but if the initialization fails, we
will refuse to connect anyway.

* Proper cleanup after socket

Actually clean up when the last socket is destroyed, and properly serialize access to the shared network state.

* Make unverified certificate warning translatable

* Don't expand unverified signature popup to full screen width.

The popup window becomes very wide with the current warning message, limit it to ~80 characters of text.

* Let's not destroy the invalid certificate warning window in one of its event handlers. Although it might work
in this case, that's asking for trouble. Simply hide the window when a button is clicked, but keep it alive.

* Include fix for FreeBSD. Apparently these are pulled in somehow on Linux, but not FreeBSD

* The disconnected variable is only used when USE_SSL is not defined, move it inside #ifdef guard

* Add hostname verification for encrypted connections

* More translatable strings

* Log the TLS version used to set up the encryption on the channel

* Log the encryption cipher used for encrypted connections

* Generate password keys more securely.

When using SSL, generate the key for the password file using a cyptographically secure random number generator.
Otherwise, on non-Windows systems, try to seed the random number generator with a truly random seed. Doing
the same on Windows is a bit problematic: rand_s() is not supported by gcc on MSYS, using RtlGenRandom()
directly is a pain, and pulling in the full crypto system (either the old and deprecated one or the new one)
feel like overkill if we already have OpenSSL support. So for now, the situation on Windows remain unchanged
when USE_SSL is not enabled.

This only affects the generation of new password keys. Also the key format hasn't changed, so existing clients
can decode newly generated keys.
  • Loading branch information
gvissers authored Jul 21, 2021
1 parent c256cc8 commit 67b537a
Show file tree
Hide file tree
Showing 35 changed files with 3,013 additions and 137 deletions.
18 changes: 17 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ OPTION(LOCAL_NLOHMANN_JSON "Enable if you want to use an embedded copy of nlohma
OPTION(MARCHNATIVE "Enable -march=native compiler flag, for when not building a portable executable." Off)
OPTION(ADDRSANITISER "Enable the gcc address sanitisation options to do run-time checking." Off)
OPTION(TTF "Add the ability to use True Type system fonts." On)
OPTION(USE_SSL "Encrypt the traffic with the server." On)

# standard options
add_definitions(
Expand Down Expand Up @@ -130,6 +131,9 @@ include(cmake/FindCal3d.cmake)
if (TTF)
include(cmake/FindSDL2TTF.cmake)
endif()
if (USE_SSL)
find_package(OpenSSL REQUIRED)
endif()

# require the nlohmann_json package if building for json files
if (JSON_FILES)
Expand Down Expand Up @@ -175,7 +179,7 @@ set(SOURCES
# list the C++ source files
set(SOURCES ${SOURCES}
${SD}achievements.cpp ${SD}actor_init.cpp ${SD}books.cpp ${SD}cal3d_wrapper.cpp ${SD}command_queue.cpp ${SD}context_menu.cpp
${SD}elloggingwrapper.cpp ${SD}font.cpp ${SD}hud_indicators.cpp ${SD}hud_timer.cpp ${SD}icon_window.cpp
${SD}cppwindows.cpp ${SD}elloggingwrapper.cpp ${SD}font.cpp ${SD}hud_indicators.cpp ${SD}hud_timer.cpp ${SD}icon_window.cpp
${SD}io/cal3d_io_wrapper.cpp ${SD}item_info.cpp ${SD}item_lists.cpp ${SD}named_colours.cpp
${SD}optimizer.cpp ${SD}password_manager.cpp ${SD}quest_log.cpp ${SD}select.cpp ${SD}sendvideoinfo.cpp
${SD}trade_log.cpp ${SD}user_menus.cpp ${SD}xor_cipher.cpp ${SD}engine/hardwarebuffer.cpp ${SD}engine/logging.cpp
Expand Down Expand Up @@ -230,6 +234,11 @@ if (TTF)
add_definitions(-DTTF)
endif()

if (USE_SSL)
add_definitions(-DUSE_SSL)
set(SOURCES ${SOURCES} ${SD}connection.cpp cppwindows.cpp ipaddress.cpp socket.cpp textpopup.cpp)
endif()

# set the compiler flags for DEBUG and RELEASE builds
set(CMAKE_C_FLAGS "${EXTRA_C_FLAGS} -Wall -Wdeclaration-after-statement -W -Wno-unused-parameter -Wno-sign-compare -D_GNU_SOURCE=1 -D_REENTRANT")
set(CMAKE_C_FLAGS_DEBUG "-Og -ggdb -pipe -fno-strict-aliasing")
Expand Down Expand Up @@ -336,6 +345,13 @@ if (TTF)
target_include_directories(${EXEC} SYSTEM PUBLIC ${SDL2TTF_INCLUDE_DIR})
endif()

if (USE_SSL)
if (CMAKE_SYSTEM_NAME MATCHES "Windows")
target_link_libraries(${EXEC} -lws2_32)
endif()
target_link_libraries(${EXEC} ${OPENSSL_SSL_LIBRARY} ${OPENSSL_CRYPTO_LIBRARY})
endif()

install(
TARGETS ${EXEC} RUNTIME DESTINATION bin
)
2 changes: 1 addition & 1 deletion Makefile.bsd
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ COBJS=2d_objects.o 3d_objects.o \

JSON_FILES_CXXOBJ = json_io.o
CXXOBJS=achievements.o actor_init.o books.o cal3d_wrapper.o command_queue.o \
context_menu.o elloggingwrapper.o engine/logging.o exceptions/extendedexception.o \
context_menu.o cppwindows.o elloggingwrapper.o engine/logging.o exceptions/extendedexception.o \
font.o hud_indicators.o hud_timer.o icon_window.o io/cal3d_io_wrapper.o item_info.o item_lists.o \
named_colours.o password_manager.o optimizer.o quest_log.o select.o \
sendvideoinfo.o trade_log.o user_menus.o xml/xmlhelper.o eye_candy_wrapper.o \
Expand Down
7 changes: 5 additions & 2 deletions Makefile.linux
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@
-include make.conf

TTF_CFLAGS = $(shell pkg-config SDL2_ttf --cflags)
USE_SSL_CFLAGS = $(shell pkg-config openssl --cflags)
JSON_FILES_CFLAGS = -I nlohmann_json/single_include/
ADDRSANITISER_CFLAGS=-fsanitize=address

PNG_SCREENSHOT_LIBS = $(shell pkg-config libpng --libs)
TTF_LIBS = $(shell pkg-config SDL2_ttf --libs)
USE_SSL_LIBS = $(shell pkg-config openssl --libs)
ADDRSANITISER_LIBS=-fsanitize=address
PAWN_LIBS = -ldl

Expand Down Expand Up @@ -92,11 +94,12 @@ COBJS=2d_objects.o 3d_objects.o \
$(foreach FEATURE, $(FEATURES), $($(FEATURE)_COBJ))

JSON_FILES_CXXOBJ = json_io.o
USE_SSL_CXXOBJ = connection.o ipaddress.o socket.o
CXXOBJS=achievements.o actor_init.o books.o cal3d_wrapper.o command_queue.o \
context_menu.o elloggingwrapper.o engine/logging.o exceptions/extendedexception.o \
context_menu.o cppwindows.o elloggingwrapper.o engine/logging.o exceptions/extendedexception.o \
font.o hud_indicators.o hud_timer.o icon_window.o io/cal3d_io_wrapper.o item_info.o item_lists.o \
named_colours.o password_manager.o optimizer.o quest_log.o select.o \
sendvideoinfo.o trade_log.o user_menus.o xml/xmlhelper.o eye_candy_wrapper.o \
sendvideoinfo.o textpopup.o trade_log.o user_menus.o xml/xmlhelper.o eye_candy_wrapper.o \
engine/hardwarebuffer.o xor_cipher.o \
eye_candy/eye_candy.o eye_candy/math_cache.o eye_candy/effect_lamp.o \
eye_candy/effect_candle.o \
Expand Down
2 changes: 1 addition & 1 deletion Makefile.win
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ COBJS=2d_objects.o 3d_objects.o \

JSON_FILES_CXXOBJ = json_io.o
CXXOBJS=achievements.o actor_init.o books.o cal3d_wrapper.o command_queue.o \
context_menu.o elloggingwrapper.o engine/logging.o exceptions/extendedexception.o \
context_menu.o cppwindows.o elloggingwrapper.o engine/logging.o exceptions/extendedexception.o \
font.o hud_indicators.o hud_timer.o icon_window.o io/cal3d_io_wrapper.o item_info.o item_lists.o \
named_colours.o password_manager.o optimizer.o quest_log.o select.o \
sendvideoinfo.o trade_log.o user_menus.o xml/xmlhelper.o eye_candy_wrapper.o \
Expand Down
Loading

0 comments on commit 67b537a

Please sign in to comment.