RCON - use 64 bit time on 32 bit windows systems

[ciscon]

No description provided.

[VVD]

There are no __TIMESIZE and __time64_t in FreeBSD.

[ciscon]

updated to affect bsd as well, i can confirm this works for the win32 and linux builds, but @VVD hasn't confirmed it fixes the issue with bsd (though i see no reason it shouldn't).

[tcsabina]

@VVD , could you confirm that this works now for BSD?

[VVD]

$ ./build_cmake.sh
bash: ./build_cmake.sh: cannot execute: required file not found
$ head -n1 build_cmake.sh 
#!/bin/bash -e

🤦
WHY U THINK bash ALWAYS IN /bin/ ?!?!?!?!
Use env: #!/usr/bin/env bash
Or better use classic shell: #!/bin/sh

-- The C compiler identification is unknown
CMake Error at CMakeLists.txt:6 (project):
  The CMAKE_C_COMPILER:

    x86_64-linux-gnu-gcc

  is not a full path and was not found in the PATH.

  Tell CMake where to find the compiler by setting either the environment
  variable "CC" or the CMake cache entry CMAKE_C_COMPILER to the full path to
  the compiler, or to the compiler name if it is in the PATH.


-- Configuring incomplete, errors occurred!

If you don't know how to write build system for cross platform projects - just keep it as it was before with gmake.
It work even on SunOS and ofc on Linux, FreeBSD, OpenBSD, NetBSD and DragonflyBSD. Now it just doesn't work at all.

[tcsabina]

Hey @VVD ,

As we use github as build environment, we tune our build scripts to work on the github ecosystem, and not on a randomly selected distro.
And I can tell you that the build scripts are working on many linux distro...

I am sorry to hear you frustration that your beloved whateverBSD distro is that special, that is not able to handle the build scripts.
Maybe you should stick to an OS that works ;).

[VVD]

Just for information (if you don't know): I was the project leader of the mvdsv for 4 or 5 years (~2003-2008) and fixed build system to support everything I have access to - different Linux distros, *BSD, SunOS, MacOS. And now I see how somebody break all this work and got answer "Maybe you should stick to an OS that works". WTF?!
Just return back old build system if u can't write cross platform build system self.
But I see CODE_OF_CONDUCT.md! Ofc it's more important (no).

freebsd-amd64.cmake:

# the name of the target operating system
set(CMAKE_SYSTEM_NAME FreeBSD)
set(CMAKE_SYSTEM_PROCESSOR amd64)

# which compilers to use for C and C++
set(CMAKE_C_COMPILER clang)
set(CMAKE_CXX_COMPILER clang++)

# adjust the default behaviour of the FIND_XXX() commands:
# search headers and libraries in the target environment, search
# programs in the host environment
set(CMAKE_FIND_ROOT_PATH_MODE_PROGRAM NEVER)
set(CMAKE_FIND_ROOT_PATH_MODE_LIBRARY ONLY)
set(CMAKE_FIND_ROOT_PATH_MODE_INCLUDE ONLY)

[47/49] Building C object CMakeFiles/mvdsv.dir/src/sv_main.c.o
src/sv_main.c:1566:15: warning: passing 'unsigned char *' to parameter of type 'char *' converts between pointers to integer types where one is of the unique plain 'char' type and the other is not [-Wpointer-sign]
                SHA1_Update((unsigned char*)Cmd_Argv(0));
                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~
src/sha1.h:61:25: note: passing argument to parameter 'data' here
void SHA1_Update (char* data);
                        ^
src/sv_main.c:1567:15: warning: passing 'unsigned char *' to parameter of type 'char *' converts between pointers to integer types where one is of the unique plain 'char' type and the other is not [-Wpointer-sign]
                SHA1_Update((unsigned char*)" ");
                            ^~~~~~~~~~~~~~~~~~~

Why changed in function call, but not changed in implementation?

./src/sha1.c:void SHA1_Update (char *string)
./src/sha1.h:void SHA1_Update (char* data);
./src/sv_main.c:                SHA1_Update((unsigned char*)Cmd_Argv(0));
./src/sv_main.c:                SHA1_Update((unsigned char*)" ");

[VVD]

Same with ktx - removed cross platform build, but added CODE_OF_CONDUCT.md. 🤦
And same freebsd-amd64.cmake work for it too.

What genius wrote this code:

src/race.c:3579:42: note: change size argument to be the size of the destination
                        strlcpy(race.records[cnt].date, line, strlen(line));
                                                              ^~~~~~~~~~~~
                                                              sizeof(race.records[cnt].date)

🤦🤦🤦🤦🤦

[ciscon]

are we still on the subject of the pr here? i don't know what's going on.

[VVD]

Subject is crypt_rcon is broken.

In ezQuake hardcoded size of the string for time:

char    message[1024], client_time_str[9];

But in mvdsv it calculated from sizeof(time_t):

                if (strlen(digest) < DIGEST_SIZE * 2 + sizeof(time_t) * 2) {
                        return 0;
                }
                if ((int)sv_timestamplen.value) {
                        time_t server_time, client_time = 0;
                        double difftime_server_client;

                        time(&server_time);
                        for (i = 0; i < sizeof(client_time) * 2; i += 2) {
                                client_time += (char2int((unsigned char)time_start[i]) << (4 + i * 4)) +
                                                        (char2int((unsigned char)time_start[i + 1]) << (i * 4));
                        }
                        difftime_server_client = difftime(server_time, client_time);

                        if (difftime_server_client > (double)sv_timestamplen.value || difftime_server_client < -(double)sv_times
tamplen.value) {
                                return 0;
                        }
                }

Fix is replace sizeof(time_t) * 2 and sizeof(client_time) * 2 with constant 8.

[ciscon]

that's essentially what this pr does except it still uses the size of our time int which is now always uint64, which it should probably also be in mvdsv, that way if we ever change it (say 128?) it's still the one place.

[VVD]

Move from 32bit to 64bit time_t broke the "protocol" of the crypt_rcon: older clients can't use it with newer mvdsv and newer clients can't use it with older mvdsv.
So if you not set timestamp's size to constant you will get broken protocol during migration from 64 to 128 bit too.

[ciscon]

correct, that's why we're setting it to something that's always the same size, uint64, that's no different than hardcoding a number

[VVD]

So your suggestion is to break compatibility with older versions and fixate protocol with 64bit time_t.
Why not keep 32bit and compatibility with older versions? Overflow in 2038?

[ciscon]

it still won't work on all the current mvdsv servers that are using 64 bit time, which is like 99% of them. may as well just start setting all time on both sides to 64 bit.

[VVD]

time(&client_time);

time want time_t, if time_t == int128_t and we try to call it with pointer to 64bit => segmentation fault and/or broken stack.
So always use time_t and fixed size (32 or 64 bit) for hex in rcon is better way.
ezQuake with this patch work for me with current mvdsv without any changes:

--- common.h.orig
+++ common.h
@@ -436,5 +436,7 @@
 unsigned char *Q_redtext(unsigned char *str);
 unsigned char *Q_yelltext(unsigned char *str);
 
+#define TIME_T_SIZE    8
+
 #endif /* !__COMMON_H__ */
 
--- cl_cmd.c.orig
+++ cl_cmd.c
@@ -84,7 +84,7 @@
 // don't forward the first argument
 void CL_ForwardToServer_f (void) {
 // Added by VVD {
-       char            *server_string, client_time_str[9];
+       char            *server_string, client_time_str[TIME_T_SIZE * 2 + 1];
        int             i, server_string_len;
        extern cvar_t   cl_crypt_rcon;
        time_t          client_time;
@@ -131,13 +131,13 @@
                if (cl_crypt_rcon.value && strcasecmp(Cmd_Argv(1), "techlogin") == 0 && Cmd_Argc() > 2)
                {
                        time(&client_time);
-                       for (client_time_str[0] = i = 0; i < sizeof(client_time); i++) {
+                       for (client_time_str[0] = i = 0; i < TIME_T_SIZE; i++) {
                                char tmp[3];
                                snprintf(tmp, sizeof(tmp), "%02X", (unsigned int)((client_time >> (i * 8)) & 0xFF));
                                strlcat(client_time_str, tmp, sizeof(client_time_str));
                        }
 
-                       server_string_len = Cmd_Argc() + strlen(Cmd_Argv(1)) + DIGEST_SIZE * 2 + 16;
+                       server_string_len = Cmd_Argc() + strlen(Cmd_Argv(1)) + DIGEST_SIZE * 2 + TIME_T_SIZE * 2;
                        for (i = 3; i < Cmd_Argc(); ++i)
                                server_string_len += strlen(Cmd_Argv(i));
                        server_string = (char *) Q_malloc(server_string_len);
@@ -548,7 +548,7 @@
 //Send the rest of the command line over as an unconnected command.
 void CL_Rcon_f (void) {
 
-       char    message[1024], client_time_str[9];
+       char    message[1024], client_time_str[TIME_T_SIZE * 2 + 1];
        int             i, i_from;
        netadr_t        to;
        extern cvar_t   rcon_password, rcon_address, cl_crypt_rcon;
@@ -565,7 +565,7 @@
        if (cl_crypt_rcon.value)
        {
                time(&client_time);
-               for (client_time_str[0] = i = 0; i < sizeof(client_time); i++) {
+               for (client_time_str[0] = i = 0; i < TIME_T_SIZE; i++) {
                        char tmp[3];
                        snprintf(tmp, sizeof(tmp), "%02X", (unsigned int)((client_time >> (i * 8)) & 0xFF));
                        strlcat(client_time_str, tmp, sizeof(client_time_str));
--- sv_main.c.orig
+++ sv_main.c
@@ -1499,7 +1499,7 @@
 
        if ((int)sv_crypt_rcon.value) {
                time(&server_time);
-               for (i = 0; i < sizeof(client_time) * 2; i += 2) {
+               for (i = 0; i < TIME_T_SIZE * 2; i += 2) {
                        client_time += (char2int((unsigned char)(Cmd_Argv(1) + DIGEST_SIZE * 2)[i]) << (4 + i * 4)) +
                                       (char2int((unsigned char)(Cmd_Argv(1) + DIGEST_SIZE * 2)[i + 1]) << (i * 4));
                }

[ciscon]

if you want to change it again feel free to make a pr for it, i'm fine with leaving it as it fixes 32 bit clients.

[VVD]

#726