Re: [HACKERS] Re: Cancel key now ready - Mailing list pgsql-hackers

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