Re: Cancel key now ready - Mailing list pgsql-hackers

Bruce Momjian <maillist@candle.pha.pa.us> writes:
> I have added code to the postmaster to generate a random cancel key by
> calling gettimeofday() on postmaster startup and on the first invocation
> of a backend, and merged the micro-seconds of the two times to seed the
> random number generator.

There were several things I didn't like about Bruce's first cut.
His method for generating a random seed in the postmaster is good,
but there are several security holes elsewhere.

> Not sure if you can
> predict forward, but it is probably impossible to predict randoms
> backward on any of our supported platforms.

Actually, it's not that hard.  Nearly all implementations of random()
are basically just
    seed = (seed * a + b) % c;
    return seed;
for some constants a,b,c --- which a potential attacker could easily
find out.  So if the attacker can execute random() in a backend before
it gets used for anything else, he can back up to the last random value
generated in the postmaster.  The most secure way to prevent this is to
re-execute srandom() during the startup of each backend, so that it will
have a random() sequence that's unrelated to the postmaster's.

Also, Bruce was assuming that the random_seed value wouldn't be visible
in a backend ... but the forked-off backend will have a copy just
sitting there, readily accessible if you can figure out its address.
Backend startup should zero out this variable to be on the safe side.

Attached is a patch that fixes these leaks, and does a couple other
things as well:
  * Computes and saves a cancel key for each backend.
  * fflush before forking, to eliminate double-buffering problems
    between postmaster and backends.
  * Go back to two random() calls instead of one to generate random
    salt.  I'm not sure why Bruce changed that, but it looks much
    less secure to me; the one-call way is exposing twice as many
    bits of the current random seed.
  * Fix "ramdom" typo.

Next is to transmit the PID + cancel key to the frontend and modify
libpq's cancel routine to send it back.  I'll work on that later.

            regards, tom lane


*** postmaster.c~    Mon Jun  8 14:10:39 1998
--- postmaster.c    Mon Jun  8 14:44:15 1998
***************
*** 113,118 ****
--- 113,119 ----
  typedef struct bkend
  {
      int            pid;            /* process id of backend */
+     long        cancel_key;        /* cancel key for cancels for this backend */
  } Backend;

  /* list of active backends.  For garbage collection only now. */
***************
*** 198,204 ****
--- 199,212 ----
  static    int            orgsigmask = sigblock(0);
  #endif

+ /*
+  * State for assigning random salts and cancel keys.
+  * Also, the global MyCancelKey passes the cancel key assigned to a given
+  * backend from the postmaster to that backend (via fork).
+  */
+
  static unsigned int random_seed = 0;
+ long MyCancelKey = 0;

  extern char *optarg;
  extern int    optind,
***************
*** 602,618 ****
              return (STATUS_ERROR);
          }

!         if (random_seed == 0)
          {
              gettimeofday(&later, &tz);

              /*
               *    We are not sure how much precision is in tv_usec, so we
!              *    swap the nibbles of 'later' and XOR them with 'now'
               */
              random_seed = now.tv_usec ^
                      ((later.tv_usec << 16) |
!                     ((unsigned int)(later.tv_usec & 0xffff0000) >> 16));
          }

          /*
--- 610,631 ----
              return (STATUS_ERROR);
          }

!         /*
!          * Select a random seed at the time of first receiving a request.
!          */
!         while (random_seed == 0)
          {
              gettimeofday(&later, &tz);

              /*
               *    We are not sure how much precision is in tv_usec, so we
!              *    swap the nibbles of 'later' and XOR them with 'now'.
!              *  On the off chance that the result is 0, we loop until
!              *  it isn't.
               */
              random_seed = now.tv_usec ^
                      ((later.tv_usec << 16) |
!                     ((later.tv_usec >> 16) & 0xffff));
          }

          /*
***************
*** 1075,1080 ****
--- 1088,1101 ----
      }
  #endif

+     /*
+      * Compute the cancel key that will be assigned to this backend.
+      * The backend will have its own copy in the forked-off process'
+      * value of MyCancelKey, so that it can transmit the key to the
+      * frontend.
+      */
+     MyCancelKey = PostmasterRandom();
+
      if (DebugLvl > 2)
      {
          char      **p;
***************
*** 1088,1104 ****
          fprintf(stderr, "-----------------------------------------\n");
      }

      if ((pid = fork()) == 0)
      {  /* child */
          if (DoBackend(port))
          {
              fprintf(stderr, "%s child[%d]: BackendStartup: backend startup failed\n",
!                     progname, pid);
!             /* use _exit to keep from double-flushing stdio */
!              _exit(1);
          }
          else
!             _exit(0);
      }

      /* in parent */
--- 1109,1129 ----
          fprintf(stderr, "-----------------------------------------\n");
      }

+     /* Flush all stdio channels just before fork,
+      * to avoid double-output problems.
+      */
+     fflush(NULL);
+
      if ((pid = fork()) == 0)
      {  /* child */
          if (DoBackend(port))
          {
              fprintf(stderr, "%s child[%d]: BackendStartup: backend startup failed\n",
!                     progname, (int) getpid());
!              exit(1);
          }
          else
!             exit(0);
      }

      /* in parent */
***************
*** 1130,1135 ****
--- 1155,1161 ----
      }

      bn->pid = pid;
+     bn->cancel_key = MyCancelKey;
      DLAddHead(BackendList, DLNewElem(bn));

      ActiveBackends = TRUE;
***************
*** 1192,1197 ****
--- 1218,1225 ----
      char        dbbuf[ARGV_SIZE + 1];
      int            ac = 0;
      int            i;
+     struct timeval now;
+     struct timezone tz;

      /*
       *    Let's clean up ourselves as the postmaster child
***************
*** 1225,1231 ****
      if (NetServer)
          StreamClose(ServerSock_INET);
      StreamClose(ServerSock_UNIX);
!
      /* Now, on to standard postgres stuff */

      MyProcPid = getpid();
--- 1253,1268 ----
      if (NetServer)
          StreamClose(ServerSock_INET);
      StreamClose(ServerSock_UNIX);
!
!     /*
!      * Don't want backend to be able to see the postmaster random number
!      * generator state.  We have to clobber the static random_seed *and*
!      * start a new random sequence in the random() library function.
!      */
!     random_seed = 0;
!     gettimeofday(&now, &tz);
!     srandom(now.tv_usec);
!
      /* Now, on to standard postgres stuff */

      MyProcPid = getpid();
***************
*** 1365,1374 ****
  static void
  RandomSalt(char *salt)
  {
!     long rand = PostmasterRandom();
!
!     *salt = CharRemap(rand % 62);
!     *(salt + 1) = CharRemap(rand / 62);
  }

  /*
--- 1402,1409 ----
  static void
  RandomSalt(char *salt)
  {
!     *salt = CharRemap(PostmasterRandom());
!     *(salt + 1) = CharRemap(PostmasterRandom());
  }

  /*
***************
*** 1387,1391 ****
          initialized = true;
      }

!     return ramdom() ^ random_seed;
  }
--- 1422,1426 ----
          initialized = true;
      }

!     return random() ^ random_seed;
  }