Cancel key now ready

Started by Bruce Momjianover 27 years ago19 messages
#1Bruce Momjian
maillist@candle.pha.pa.us

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.

I added a PostmasterRandom() function which returns a random that is
XOR'ed with the original random seed, so it it not possible to take a
given cancel key and predict future random keys.

The only way you could do it would be to call random in your backend,
and somehow find the PREVIOUS random value. You could XOR it with your
cancel key to find the original seed, and then try going forward to
predict the next cancel value. Seems impossible to me.

This fulfills two goals, to make the random seed truly random, so the
cancel keys are not guess-able, and to make seeing your own cancel key
almost useless in computing other cancel keys. Not sure if you can
predict forward, but it is probably impossible to predict randoms
backward on any of our supported platforms.

Patch is posted to patches list.

Now I need help in passing the value to the font-end, and having the
front-end pass it to the backend for a cancel. I do not recommend
passing the pid because I will store the cancel key in the per-backend
structure, so having the pid does not help me find the backend. Might
as well just scan the table to find the matching cancel key, and kill
that backend. We will have to store the pid in the structure, but that
is easy to do.

-- 
Bruce Momjian                          |  830 Blythe Avenue
maillist@candle.pha.pa.us              |  Drexel Hill, Pennsylvania 19026
  +  If your life is a hard drive,     |  (610) 353-9879(w)
  +  Christ can be your backup.        |  (610) 853-3000(h)
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#1)
Re: Cancel key now ready

Bruce Momjian <maillist@candle.pha.pa.us> writes:

Now I need help in passing the value to the font-end, and having the
front-end pass it to the backend for a cancel.

I can work on that. Have you checked the postmaster changes into cvs?

I do not recommend passing the pid because I will store the cancel key
in the per-backend structure, so having the pid does not help me find
the backend. Might as well just scan the table to find the matching
cancel key, and kill that backend. We will have to store the pid in
the structure, but that is easy to do.

I don't like this. Backend PIDs are guaranteed unique (among active
backends); cancel keys are not guaranteed unique, unless you took some
special measure to make them so. So you could hit the wrong backend
if you only compare cancel keys. Since you must store the PID anyway to
send the signal, you may as well use both to verify that you have found
the right backend.

regards, tom lane

#3Bruce Momjian
maillist@candle.pha.pa.us
In reply to: Tom Lane (#2)
Re: [HACKERS] Re: Cancel key now ready

Bruce Momjian <maillist@candle.pha.pa.us> writes:

Now I need help in passing the value to the font-end, and having the
front-end pass it to the backend for a cancel.

I can work on that. Have you checked the postmaster changes into cvs?

Always. You bet.

I do not recommend passing the pid because I will store the cancel key
in the per-backend structure, so having the pid does not help me find
the backend. Might as well just scan the table to find the matching
cancel key, and kill that backend. We will have to store the pid in
the structure, but that is easy to do.

I don't like this. Backend PIDs are guaranteed unique (among active
backends); cancel keys are not guaranteed unique, unless you took some
special measure to make them so. So you could hit the wrong backend
if you only compare cancel keys. Since you must store the PID anyway to
send the signal, you may as well use both to verify that you have found
the right backend.

OK, sure, pass the pid. However, one problem is that the postmaster
does not know the pid until after it forks the backend, so if you want
to send the pid with the cancel key, you will have to send the pid from
the backend.

Also, the odds of two backends have the same cancel key, when random
returns a long() is so astonomically small, that I am willing to live
with the risk, to take the advantage of cleaner, more modular code.

Considering the problem of sending the cancel key from the backend and
not the postmaster, I dropped the pid. Remember, you have to store the
cancel key in the postmaster so sending it to the client at that point
made sense. Setting the pid after the fork is easy because there is no
communication required.

-- 
Bruce Momjian                          |  830 Blythe Avenue
maillist@candle.pha.pa.us              |  Drexel Hill, Pennsylvania 19026
  +  If your life is a hard drive,     |  (610) 353-9879(w)
  +  Christ can be your backup.        |  (610) 853-3000(h)
#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#3)
Re: Cancel key now ready

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;
}

#5Egon Schmid
eschmid@delos.stuttgart.netsurf.de
In reply to: Bruce Momjian (#3)
Re: [HACKERS] Re: Cancel key now ready

In the current CVS may be a little bug or a big typo. IMHO should line
1390 in postmaster.c return random and not ramdom.

Since the change around 06/04 I have trouble starting the postmaster. I
can only access db's with postgres.

-Egon

On Mon, 8 Jun 1998, Bruce Momjian wrote:

Show quoted text

Bruce Momjian <maillist@candle.pha.pa.us> writes:

Now I need help in passing the value to the font-end, and having the
front-end pass it to the backend for a cancel.

I can work on that. Have you checked the postmaster changes into cvs?

Always. You bet.

#6Bruce Momjian
maillist@candle.pha.pa.us
In reply to: Egon Schmid (#5)
Re: [HACKERS] Re: Cancel key now ready

In the current CVS may be a little bug or a big typo. IMHO should line
1390 in postmaster.c return random and not ramdom.

Since the change around 06/04 I have trouble starting the postmaster. I
can only access db's with postgres.

Fixing now.

-- 
Bruce Momjian                          |  830 Blythe Avenue
maillist@candle.pha.pa.us              |  Drexel Hill, Pennsylvania 19026
  +  If your life is a hard drive,     |  (610) 353-9879(w)
  +  Christ can be your backup.        |  (610) 853-3000(h)
#7Bruce Momjian
maillist@candle.pha.pa.us
In reply to: Tom Lane (#4)
Re: [HACKERS] Re: Cancel key now ready

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.

I thought about this. I can force a re-seeding of random in the
backend on first use. Didn't get that far yet. Could re-seed on every
startup, but again, could be an expensive function.

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.

If they have access the backend address space, they can see the entire
postmaster backend structure at time of fork(), so seeing the seed is
meanless. Basically, for any user who is installing their own functions
or stuff is already able to do more severe damage than just cancel.
They can write directly into the database.

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.

Can you elaborate on what this fixes?

* 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.

The code is similar to taking a random() and doing:

rand % 10

(rand / 10) % 10

which for a random of 123456 returns 6 and 5. In the postmaster case
the values are 62 and not 10, but the concept is the same. No reason to
call random() twice. May be an expensive function on some platforms.

-- 
Bruce Momjian                          |  830 Blythe Avenue
maillist@candle.pha.pa.us              |  Drexel Hill, Pennsylvania 19026
  +  If your life is a hard drive,     |  (610) 353-9879(w)
  +  Christ can be your backup.        |  (610) 853-3000(h)
#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#7)
Re: [HACKERS] Re: Cancel key now ready

Bruce Momjian <maillist@candle.pha.pa.us> writes:

OK, sure, pass the pid. However, one problem is that the postmaster
does not know the pid until after it forks the backend, so if you want
to send the pid with the cancel key, you will have to send the pid from
the backend.

Ah, I see: you were planning to send the cancel authorization data to
the FE as part of the AuthenticationOK message. I was assuming it
should be sent by the backend as part of backend startup.
It could be done either way I suppose. The transmission of the cancel
key to the backend is essentially free (see my recent patch), so really
it boils down to whether we'd rather add version-dependent fields to
AuthenticationOK or just add a whole new message type.

Also, the odds of two backends have the same cancel key, when random
returns a long() is so astonomically small, that I am willing to live
with the risk, to take the advantage of cleaner, more modular code.

It would only take a few more lines to make it safe: generate a key,
check for a duplicate in the list of active backends, repeat if there
is a duplicate. However I think that using PID+key is better, because
it makes it that much harder for an attacker to guess a valid cancel
request.

regards, tom lane

#9Egon Schmid
eschmid@delos.stuttgart.netsurf.de
In reply to: Bruce Momjian (#6)
Re: [HACKERS] Re: Cancel key now ready

Already fixed, compiled and started postmaster I see the following:

marliesle$ postmaster -i &
[1]: + Exit 1 postmaster -i &
marliesle$ No such file or directory
PostmasterMain execv failed on postmaster

[1]: + Exit 1 postmaster -i &

I'm on debian (bo) and till July 4 it worked everyday :)

-Egon

Show quoted text

On Mon, 8 Jun 1998, Bruce Momjian wrote:

Fixing now.

#10Bruce Momjian
maillist@candle.pha.pa.us
In reply to: Tom Lane (#8)
Re: [HACKERS] Re: Cancel key now ready

It would only take a few more lines to make it safe: generate a key,
check for a duplicate in the list of active backends, repeat if there
is a duplicate. However I think that using PID+key is better, because
it makes it that much harder for an attacker to guess a valid cancel
request.

Another way to do it is that when scanning looking for a match of a
cancel key, do not execute the cancel if there is more than one match.
Simple, and we are already scannig the structure. I see no reason to
scan it at cancel assignment time because the odds are so small.

But the PID is clearly visible to an attacker, so I see no benefit. If
it can be sent easily, lets do it. I am not sure where/how to send it,
so do it the way you think is best. Again, if you send the pid, you
can't have duplicates, you are right. Also, remember if we send the
cancel and the pid, we have to store each value in every interface. It
is not just libpq.

-- 
Bruce Momjian                          |  830 Blythe Avenue
maillist@candle.pha.pa.us              |  Drexel Hill, Pennsylvania 19026
  +  If your life is a hard drive,     |  (610) 353-9879(w)
  +  Christ can be your backup.        |  (610) 853-3000(h)
#11Bruce Momjian
maillist@candle.pha.pa.us
In reply to: Egon Schmid (#9)
Re: [HACKERS] Re: Cancel key now ready

Already fixed, compiled and started postmaster I see the following:

marliesle$ postmaster -i &
[1] 22619
marliesle$ No such file or directory
PostmasterMain execv failed on postmaster

[1]+ Exit 1 postmaster -i &

I'm on debian (bo) and till July 4 it worked everyday :)

OK, this is another issue. To properly show status on the command line,
I have the postmaster re-exec() itself with at least three args. For
some reason, on your platform, this is not working. I have just
committed a patch that shows the file name on exec failure. Please
re-sync and tell me what it displays.

-- 
Bruce Momjian                          |  830 Blythe Avenue
maillist@candle.pha.pa.us              |  Drexel Hill, Pennsylvania 19026
  +  If your life is a hard drive,     |  (610) 353-9879(w)
  +  Christ can be your backup.        |  (610) 853-3000(h)
#12Egon Schmid
eschmid@delos.stuttgart.netsurf.de
In reply to: Bruce Momjian (#11)
Re: [HACKERS] Re: Cancel key now ready

Sadly to say the same.

-Egon

On Mon, 8 Jun 1998, Bruce Momjian wrote:

Show quoted text

marliesle$ postmaster -i &
[1] 22619
marliesle$ No such file or directory
PostmasterMain execv failed on postmaster

OK, this is another issue. To properly show status on the command line,
I have the postmaster re-exec() itself with at least three args. For
some reason, on your platform, this is not working. I have just
committed a patch that shows the file name on exec failure. Please
re-sync and tell me what it displays.

#13Bruce Momjian
maillist@candle.pha.pa.us
In reply to: Egon Schmid (#12)
Re: [HACKERS] Re: Cancel key now ready

Sadly to say the same.

-Egon

On Mon, 8 Jun 1998, Bruce Momjian wrote:

marliesle$ postmaster -i &
[1] 22619
marliesle$ No such file or directory

^^^^^^^^^^^^^^

There should be some more information in the error message at this
point.

-- 
Bruce Momjian                          |  830 Blythe Avenue
maillist@candle.pha.pa.us              |  Drexel Hill, Pennsylvania 19026
  +  If your life is a hard drive,     |  (610) 353-9879(w)
  +  Christ can be your backup.        |  (610) 853-3000(h)
#14Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#13)
Re: [HACKERS] Re: Cancel key now ready

Bruce Momjian <maillist@candle.pha.pa.us> writes:

I thought about this. I can force a re-seeding of random in the
backend on first use.

No you can't; you might make PostmasterRandom behave that way, but
that doesn't stop an installed function from just calling random()
directly. You really need to wipe out the state saved by the random
library function.

Could re-seed on every
startup, but again, could be an expensive function.

srandom() is generally not much more than a store into a
static variable. If there's anything expensive about this,
it'd be the gettimeofday() call to produce the new seed value.

If they have access the backend address space, they can see the entire
postmaster backend structure at time of fork(), so seeing the seed is
meanless.

That's a good point --- in particular, they could trace the postmaster
backend-process list to find out everyone else's cancel keys. This
sort of thing is one of the disadvantages of not using an exec().

What do you think of freeing that process list as part of backend startup?

Basically, for any user who is installing their own functions
or stuff is already able to do more severe damage than just cancel.
They can write directly into the database.

That's certainly true ... but last week we were trying to make the
cancel protocol proof against someone with the ability to spy on
TCP packets in transit (which is not that easy) and now you seem
to be unworried about attacks that only require loading a function
into one of the backends. I'd prefer to do whatever we can easily
do to defend against that.

* fflush before forking, to eliminate double-buffering problems
between postmaster and backends.

Can you elaborate on what this fixes?

I have not seen any failure cases, if that's what you mean; but I
haven't yet done anything with the new no-exec code. The risk is
that if any data is waiting in a postmaster stdio output buffer,
it will eventually get written twice, once by the postmaster and
once by the backend. You want to flush it out before forking
to ensure that doesn't happen. This wasn't an issue before with
the exec-based code, because the child process' copy of the postmaster's
stdio buffers got thrown away when the exec() occurred. With no
exec, the unflushed buffers are still there and still valid as far
as the stdio library in the child knows.

The code is similar to taking a random() and doing:
rand % 10
(rand / 10) % 10
which for a random of 123456 returns 6 and 5. In the postmaster case
the values are 62 and not 10, but the concept is the same. No reason to
call random() twice. May be an expensive function on some platforms.

It's not that expensive (you were doing it twice before, with no visible
problem). I'm concerned that the new way exposes more info about the
current state of the postmaster's random sequence. For that matter,
I'm probably going to want to change the computation of the cancel key
later on --- the code I just sent in was only
MyCancelKey = PostmasterRandom();
but I think it would be better to synthesize the cancel key from
fragments of a couple of random values. This code will do to get the
protocol working but I don't think it's cryptographically secure.

regards, tom lane

#15Egon Schmid
eschmid@delos.stuttgart.netsurf.de
In reply to: Bruce Momjian (#13)
Re: [HACKERS] Re: Cancel key now ready

No, postmaster jumps from main/main.c

if (len >= 10 && !strcmp(argv[0] + len - 10, "postmaster"))
exit(PostmasterMain(argc, argv));

to postmaster/postmaster.c

int
PostmasterMain(int argc, char *argv[])
{ ..
if (argc < 4)
.. /* How did we get here, error! */
fprintf(stderr, "PostmasterMain execv failed on %s\n", argv[0]);

I tried this today and after the fix the message is the same. Will start a
next time tomorrow.

-Egon

On Mon, 8 Jun 1998, Bruce Momjian wrote:

Show quoted text

Sadly to say the same.

-Egon

On Mon, 8 Jun 1998, Bruce Momjian wrote:

marliesle$ postmaster -i &
[1] 22619
marliesle$ No such file or directory

^^^^^^^^^^^^^^

There should be some more information in the error message at this
point.

#16Bruce Momjian
maillist@candle.pha.pa.us
In reply to: Tom Lane (#14)
Re: [HACKERS] Re: Cancel key now ready

Bruce Momjian <maillist@candle.pha.pa.us> writes:

I thought about this. I can force a re-seeding of random in the
backend on first use.

No you can't; you might make PostmasterRandom behave that way, but
that doesn't stop an installed function from just calling random()
directly. You really need to wipe out the state saved by the random
library function.

You can't just call random directly. You have to call an install
function with a pg_proc entry for it to work. If we set that one up to
initialize itself, it should work.

Could re-seed on every
startup, but again, could be an expensive function.

srandom() is generally not much more than a store into a
static variable. If there's anything expensive about this,
it'd be the gettimeofday() call to produce the new seed value.

But we don't call that for every backend startup, just twice for the
life of the postmaster, once for postmaster startup, and once for the
startup of the first backend. What I don't want is to profile the
backend and find that random/srandom() is showing up as significant.

If they have access the backend address space, they can see the entire
postmaster backend structure at time of fork(), so seeing the seed is
meanless.

That's a good point --- in particular, they could trace the postmaster
backend-process list to find out everyone else's cancel keys. This
sort of thing is one of the disadvantages of not using an exec().

What do you think of freeing that process list as part of backend startup?

Again, being able to connect to the backend, and accessing its address
space are two separate privs. Only the postgres user can do such
access.

Basically, for any user who is installing their own functions
or stuff is already able to do more severe damage than just cancel.
They can write directly into the database.

That's certainly true ... but last week we were trying to make the
cancel protocol proof against someone with the ability to spy on
TCP packets in transit (which is not that easy) and now you seem
to be unworried about attacks that only require loading a function
into one of the backends. I'd prefer to do whatever we can easily
do to defend against that.

Only the postgres super-user can load functions. This is something we
have protection against. Someone snooping the wire may not even have
permissions to access the database.

p

* fflush before forking, to eliminate double-buffering problems
between postmaster and backends.

Can you elaborate on what this fixes?

I have not seen any failure cases, if that's what you mean; but I
haven't yet done anything with the new no-exec code. The risk is
that if any data is waiting in a postmaster stdio output buffer,
it will eventually get written twice, once by the postmaster and
once by the backend. You want to flush it out before forking
to ensure that doesn't happen. This wasn't an issue before with
the exec-based code, because the child process' copy of the postmaster's
stdio buffers got thrown away when the exec() occurred. With no
exec, the unflushed buffers are still there and still valid as far
as the stdio library in the child knows.

Yes. Excellent point.

The code is similar to taking a random() and doing:
rand % 10
(rand / 10) % 10
which for a random of 123456 returns 6 and 5. In the postmaster case
the values are 62 and not 10, but the concept is the same. No reason to
call random() twice. May be an expensive function on some platforms.

It's not that expensive (you were doing it twice before, with no visible
problem). I'm concerned that the new way exposes more info about the
current state of the postmaster's random sequence. For that matter,
I'm probably going to want to change the computation of the cancel key
later on --- the code I just sent in was only
MyCancelKey = PostmasterRandom();
but I think it would be better to synthesize the cancel key from
fragments of a couple of random values. This code will do to get the
protocol working but I don't think it's cryptographically secure.

Again, XOR'ing with the seed should do what we need.

-- 
Bruce Momjian                          |  830 Blythe Avenue
maillist@candle.pha.pa.us              |  Drexel Hill, Pennsylvania 19026
  +  If your life is a hard drive,     |  (610) 353-9879(w)
  +  Christ can be your backup.        |  (610) 853-3000(h)
#17Bruce Momjian
maillist@candle.pha.pa.us
In reply to: Egon Schmid (#15)
Re: [HACKERS] Re: Cancel key now ready

No, postmaster jumps from main/main.c

if (len >= 10 && !strcmp(argv[0] + len - 10, "postmaster"))
exit(PostmasterMain(argc, argv));

to postmaster/postmaster.c

int
PostmasterMain(int argc, char *argv[])
{ ..
if (argc < 4)
.. /* How did we get here, error! */
fprintf(stderr, "PostmasterMain execv failed on %s\n", argv[0]);

I tried this today and after the fix the message is the same. Will start a
next time tomorrow.

-Egon

OK, I can recreate it here now. Fixing now.

-- 
Bruce Momjian                          |  830 Blythe Avenue
maillist@candle.pha.pa.us              |  Drexel Hill, Pennsylvania 19026
  +  If your life is a hard drive,     |  (610) 353-9879(w)
  +  Christ can be your backup.        |  (610) 853-3000(h)
#18Bruce Momjian
maillist@candle.pha.pa.us
In reply to: Egon Schmid (#15)
Re: [HACKERS] Re: Cancel key now ready

No, postmaster jumps from main/main.c

if (len >= 10 && !strcmp(argv[0] + len - 10, "postmaster"))
exit(PostmasterMain(argc, argv));

to postmaster/postmaster.c

int
PostmasterMain(int argc, char *argv[])
{ ..
if (argc < 4)
.. /* How did we get here, error! */
fprintf(stderr, "PostmasterMain execv failed on %s\n", argv[0]);

I tried this today and after the fix the message is the same. Will start a
next time tomorrow.

Fixed. Thank you for finding this.

-- 
Bruce Momjian                          |  830 Blythe Avenue
maillist@candle.pha.pa.us              |  Drexel Hill, Pennsylvania 19026
  +  If your life is a hard drive,     |  (610) 353-9879(w)
  +  Christ can be your backup.        |  (610) 853-3000(h)
#19Bruce Momjian
maillist@candle.pha.pa.us
In reply to: Tom Lane (#14)
Re: [HACKERS] Re: Cancel key now ready

Here is a patch that will auto-seed any request for random from the
user. This will prevent users from seeing random values that use our
postmaster cancel seed.

---------------------------------------------------------------------------

Index: src/backend/utils/adt/misc.c
===================================================================
RCS file: /usr/local/cvsroot/pgsql/src/backend/utils/adt/misc.c,v
retrieving revision 1.12
diff -c -r1.12 misc.c
*** misc.c	1998/02/24 03:47:26	1.12
--- misc.c	1998/06/09 19:16:16
***************
*** 13,18 ****
--- 13,19 ----
   */
  #include <sys/types.h>
  #include <sys/file.h>
+ #include <time.h>
  #include "postgres.h"
  #include "utils/datum.h"
  #include "catalog/pg_type.h"
***************
*** 60,65 ****
--- 61,69 ----
   * will return about 1/10 of the tuples in TEMP
   *
   */
+ 
+ static bool random_initialized = false;
+ 
  bool
  oidrand(Oid o, int32 X)
  {
***************
*** 68,73 ****
--- 72,88 ----
  	if (X == 0)
  		return true;
+ 	/*
+ 	 *	We do this because the cancel key is actually a random, so we don't
+ 	 *	want them to be able to request random numbers using our postmaster
+ 	 *	seeded value.
+ 	 */
+ 	if (!random_initialized)
+ 	{
+ 		srandom((unsigned int)time(NULL));
+ 		random_initialized = true;
+ 	}
+ 
  	result = (random() % X == 0);
  	return result;
  }
***************
*** 81,86 ****
--- 96,102 ----
  oidsrand(int32 X)
  {
  	srand(X);
+ 	random_initialized = true;
  	return true;
  }
-- 
Bruce Momjian                          |  830 Blythe Avenue
maillist@candle.pha.pa.us              |  Drexel Hill, Pennsylvania 19026
  +  If your life is a hard drive,     |  (610) 353-9879(w)
  +  Christ can be your backup.        |  (610) 853-3000(h)