[Freeciv-Dev] (PR#10727) [PATCH] Hack challenge protocol issues.

["Vasco Alexandre da Silva Costa" <vasc@xxxxxxxxxxxxxx>]

<URL: http://rt.freeciv.org/Ticket/Display.html?id=10727 >

This patch is a proof of concept for a different way of implementing the
client hack access protocol. It still has several issues (it doesn't
remove the file with the challenge properly on abnormal termination,
doesn't behave well with different protocol client/servers, etc), but
I'm storing it for reference.

Why was this done? Because the current protocol has some issues. Namely,
the fact that evil servers can make an unsuspecting client clobber
files. Also, simultaneous incoming requests may wreak havoc among
themselves.

The current protocol works like this: the server generates a filename.
Then it sends this name to the client. The client writes a challenge
number to that file, and returns this number via the network. The server
reads the number from the file, compares it with the one it got from the
network and if they match it gives access. The server removes the file.

The protocol i'm proposing works like this: the server generates a file
with the challenge number and sends its filename to the client. The
client reads the challenge number from the file and returns it to the
server via the network. The challenge file is removed on server termination.

? foo
Index: client/connectdlg_common.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/connectdlg_common.c,v
retrieving revision 1.26
diff -u -r1.26 connectdlg_common.c
--- client/connectdlg_common.c  24 Oct 2004 23:18:07 -0000      1.26
+++ client/connectdlg_common.c  25 Oct 2004 20:40:40 -0000
@@ -38,6 +38,7 @@
 #endif
 
 #include "fcintl.h"
+#include "log.h"
 #include "mem.h"
 #include "netintf.h"
 #include "rand.h"
@@ -373,29 +374,26 @@
 if the client is capable of 'wanting hack', then the server will 
 send the client a filename in the packet_join_game_reply packet.
 
-this function creates the file with a suitably random number in it 
-and then sends the filename and number to the server. If the server 
-can open and read the number, then the client is given hack access.
+this function reads the challenge number in that file and returns
+it to the server. If the number matches with that the server wrote,
+the client is given hack access.
 *****************************************************************/ 
-void send_client_wants_hack(char * filename)
+void send_client_wants_hack(char *filename)
 {
   struct section_file file;
 
-  /* find some entropy */ 
-  int entropy = myrand(MAX_UINT32) - time(NULL);
+  if (section_file_load_nodup(&file, filename)) {
+    const char *entropy;
 
-  /* we don't want zero */ 
-  if (entropy == 0) {
-    entropy++;
-  }
+    entropy = secfile_lookup_str(&file, "challenge.entropy");
 
-  section_file_init(&file);
-  secfile_insert_int(&file, entropy, "challenge.entropy");
-  section_file_save(&file, filename, 0);      
-  section_file_free(&file);
+    /* tell the server what was in the file */ 
+    dsend_packet_single_want_hack_req(&aconnection, entropy);
 
-  /* tell the server what we put into the file */ 
-  dsend_packet_single_want_hack_req(&aconnection, entropy);
+    section_file_free(&file);
+  } else {
+    freelog(LOG_ERROR, "Couldn't load challenge file: \"%s\".", filename);
+  }
 }
 
 /**************************************************************** 
Index: common/packets.def
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/packets.def,v
retrieving revision 1.57
diff -u -r1.57 packets.def
--- common/packets.def  21 Oct 2004 20:27:28 -0000      1.57
+++ common/packets.def  25 Oct 2004 20:40:42 -0000
@@ -1253,7 +1253,7 @@
  Below are the packets that control single-player mode.  
 *********************************************************/
 PACKET_SINGLE_WANT_HACK_REQ=108;cs,handle-per-conn,no-handle,dsend
- UINT32 challenge;
+ STRING challenge[MAX_LEN_NAME];
 end
 
 PACKET_SINGLE_WANT_HACK_REPLY=109;sc,dsend
Index: server/gamehand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/gamehand.c,v
retrieving revision 1.144
diff -u -r1.144 gamehand.c
--- server/gamehand.c   10 Oct 2004 20:02:02 -0000      1.144
+++ server/gamehand.c   25 Oct 2004 20:40:49 -0000
@@ -35,28 +35,34 @@
 
 #include "gamehand.h"
 
-#define NUMBER_OF_TRIES 500
-#ifndef __VMS
-#  define CHALLENGE_ROOT ".freeciv-tmp"
-#else /*VMS*/
-#  define CHALLENGE_ROOT "freeciv-tmp"
-#endif
+#define CHALLENGE_ROOT "fc-challenge"
 
 static char challenge_filename[MAX_LEN_PATH];
+static bool challenge_init;
+static char challenge_entropy[MAX_LEN_NAME];
+
 
 /****************************************************************************
-  Initialize the game.id variable to a random string of characters.
+  Generate a random string.
 ****************************************************************************/
-static void init_game_id(void)
+static void randomize_string(char *str, size_t n)
 {
   static const char chars[] =
     "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
   int i;
 
-  for (i = 0; i < sizeof(game.id) - 1; i++) {
-    game.id[i] = chars[myrand(sizeof(chars) - 1)];
+  for (i = 0; i < n - 1; i++) {
+    str[i] = chars[myrand(sizeof(chars) - 1)];
   }
-  game.id[i] = '\0';
+  str[i] = '\0';
+}
+
+/****************************************************************************
+  Initialize the game.id variable to a random string of characters.
+****************************************************************************/
+static void init_game_id(void)
+{
+  randomize_string(game.id, sizeof(game.id));
 }
 
 /****************************************************************************
@@ -412,75 +418,76 @@
   return game.timeout;
 }
 
+/**************************************************************************
+  Remove challenge file on server exit.
+**************************************************************************/
+static void challenge_remove_handler(void)
+{
+  if (challenge_init) {
+    remove(challenge_filename);
+  }
+}
+
 /************************************************************************** 
-find a suitably random file that we can write too, and return it's name.
+find a suitably random file that we can write too, and write the challenge
+into it.
 **************************************************************************/
 const char *create_challenge_filename(void)
 {
-  int i;
-  unsigned int tmp = 0;
+  if (!challenge_init) {
+    unsigned num;
+    struct section_file file;
+    bool res;
 
-  /* find a suitable file to place the challenge in, we'll remove the file
-   * once the challenge is  */
 #ifndef CHALLENGE_PATH
-  sz_strlcpy(challenge_filename, user_home_dir());
-  sz_strlcat(challenge_filename, "/");
-  sz_strlcat(challenge_filename, CHALLENGE_ROOT);
+    sz_strlcpy(challenge_filename, user_home_dir());
+#else
+    sz_strlcpy(challenge_filename, CHALLENGE_PATH);
+#endif
+
+    sz_strlcat(challenge_filename, "/");
+    sz_strlcat(challenge_filename, CHALLENGE_ROOT);
+
+    /* find a suitable file to place the challenge in, we'll remove the file
+     * on server exit. */
+#ifndef WIN32_NATIVE
+    num = getpid();
 #else
-  sz_strlcpy(challenge_filename, CHALLENGE_PATH);
+    num = time(NULL);
 #endif
+    cat_snprintf(challenge_filename, sizeof(challenge_filename), "-%d", num);
+
+    /* find some entropy, the RNG has not been init yet, so we do it here. */
+    mysrand(time(NULL));
+    randomize_string(challenge_entropy, sizeof(challenge_entropy));
 
-  for (i = 0; i < NUMBER_OF_TRIES; i++) {
-    char test_str[MAX_LEN_PATH];
+    section_file_init(&file);
+    secfile_insert_str(&file, challenge_entropy, "challenge.entropy");
+    res = section_file_save(&file, challenge_filename, 0);
+    section_file_free(&file);
 
-    sz_strlcpy(test_str, challenge_filename);
-    tmp = time(NULL);
-    cat_snprintf(test_str, MAX_LEN_PATH, "-%d", tmp);
-
-    /* file doesn't exist but we can create one and write to it */
-    if (!is_reg_file_for_access(test_str, FALSE) &&
-        is_reg_file_for_access(test_str, TRUE)) {
-      cat_snprintf(challenge_filename, MAX_LEN_PATH, "-%d", tmp);
-      break;
-    } else {
-      die("we can't seem to write to the filesystem!");
+    if (!res) {
+      freelog(LOG_FATAL, _("Couldn't write to challenge file: \"%s\"."),
+         challenge_filename);
+      exit(EXIT_FAILURE);
     }
+    challenge_init = TRUE;
+
+    atexit(challenge_remove_handler);
   }
 
   return challenge_filename;
 }
 
 /************************************************************************** 
-opens a file specified by the packet and compares the packet values with
-the file values. Sends an answer to the client once it's done.
+compares the client returned challenge number with the one the server
+generated in the first place. Sends an answer to the client once it's done.
 **************************************************************************/
-void handle_single_want_hack_req(struct connection *pc, int challenge)
+void handle_single_want_hack_req(struct connection *pc, const char *challenge)
 {
-  struct section_file file;
-  int entropy = 0;
-  bool could_load = TRUE;
-  bool you_have_hack = FALSE;
-
-  if (is_reg_file_for_access(challenge_filename, FALSE)) {
-    if (section_file_load_nodup(&file, challenge_filename)) {
-      entropy = secfile_lookup_int_default(&file, 0, "challenge.entropy");
-      section_file_free(&file);
-    } else {
-      freelog(LOG_ERROR, "couldn't load temporary file: %s",
-              challenge_filename);
-      could_load = FALSE;
-    }
+  bool you_have_hack;
 
-    /* remove temp file */
-    if (remove(challenge_filename) != 0) {
-      freelog(LOG_ERROR, "couldn't remove temporary file: %s",
-              challenge_filename);
-    }
-  } else {
-    could_load = FALSE;
-  }
-
-  you_have_hack = (could_load && entropy && entropy == challenge);
+  you_have_hack = (challenge_init && strcmp(challenge_entropy, challenge)==0);
 
   if (you_have_hack) {
     pc->access_level = ALLOW_HACK;
@@ -488,3 +495,4 @@
 
   dsend_packet_single_want_hack_reply(pc, you_have_hack);
 }
+
Index: server/gamehand.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/gamehand.h,v
retrieving revision 1.11
diff -u -r1.11 gamehand.h
--- server/gamehand.h   10 Apr 2004 03:47:49 -0000      1.11
+++ server/gamehand.h   25 Oct 2004 20:40:49 -0000
@@ -26,6 +26,6 @@
 int update_timeout(void);
 
 const char *create_challenge_filename(void);
-void handle_single_want_hack_req(struct connection *pc, int challenge);
+void handle_single_want_hack_req(struct connection *pc, const char *challenge);
 
 #endif  /* FC__GAMEHAND_H */