patch contrib/pgcrypto for win32 (2)

Korea PostgreSQL Users' Group wrote:

I found that function gen_salt() in contrib/pgcrypto had bug on win32.

I patched contrib/pgcrypto/random.c file.

What is the purpose of this addition?

+ srandom(time(NULL));
+

Is resetting the seed on each call a good idea?

cheers

andrew

"Korea PostgreSQL Users' Group" <pgsql-kr@postgresql.or.kr> writes:

SSBmb3VuZCB0aGF0IGZ1bmN0aW9uIGdlbl9zYWx0KCkgaW4gY29udHJpYi9w
Z2NyeXB0byBoYWQgYnVnIG9uIHdpbjMyLg0KDQpJIHBhdGNoZWQgY29udHJp
Yi9wZ2NyeXB0by9yYW5kb20uYyBmaWxlLg0KDQo=

Unencoded text would be nicer to reply to ...

But anyway, why are you inserting an srandom() call? That changes the
behavior on all platforms not just win32. And I don't think the % 255
change is right either; doesn't that make it impossible to produce 255
as an output byte?

regards, tom lane

On Mon, Dec 06, 2004 at 12:51:28AM +0900, Korea PostgreSQL Users' Group wrote:

I found that function gen_salt() in contrib/pgcrypto had bug on win32.

I patched contrib/pgcrypto/random.c file.

Could you describe the bug bit more?

As for srandom, src/backend/postmaster/postmaster.c does it
already, and doing it more will make matters only worse.

I would not object to just sticking '& 255' there, but if
current code has problems then I imagine lot more code could be
affected. Or are you just silencing some warning?

--
marko

Marko Kreen <marko@l-t.ee> writes:

As for srandom, src/backend/postmaster/postmaster.c does it
already, and doing it more will make matters only worse.

Yes. I think we had some discussion about that already, and concluded
it was a bad idea to insert ad-hoc srandom calls.

I would not object to just sticking '& 255' there,

The patch actually says '% 255' which is a whole different animal;
it still requires explaining though.

regards, tom lane

On Sun, Dec 05, 2004 at 06:36:38PM -0500, Tom Lane wrote:

Marko Kreen <marko@l-t.ee> writes:

I would not object to just sticking '& 255' there,

The patch actually says '% 255' which is a whole different animal;
it still requires explaining though.

Yeah, I was hinting that '& 255' I could accept with less
explaining...

--
marko

this bug is only for win32 system.

On mingw32 random() function have to be initialized by srandom().
so, I put srandom(time(NULL)) line.
and,
Because random() function return integer (2byte), this return integer number need filtering.
so, I changed random() % 255 line.

on win32, original code gen_salt() function allways returned "$1$/2E./2E.".
this string made by same return value by random() function. (sorry, I can't express in good English)

plz, check and properly fix this bug.

I tried "& 255" operation. but this bug is still.

"Korea PostgreSQL Users' Group" <pgsql-kr@postgresql.or.kr> writes:

this bug is only for win32 system.
On mingw32 random() function have to be initialized by srandom().
so, I put srandom(time(NULL)) line.

But there is already an srandom() call during backend startup.

Because random() function return integer (2byte), this return integer number need filtering.
so, I changed random() % 255 line.

But the value will automatically be converted to a single byte when it's
stored into a uint8 variable.

plz, check and properly fix this bug.

I see no bug here.

regards, tom lane

On Tue, Dec 07, 2004 at 01:18:41AM +0900, Korea PostgreSQL Users' Group wrote:

this bug is only for win32 system.

On mingw32 random() function have to be initialized by srandom().
so, I put srandom(time(NULL)) line.
and,
Because random() function return integer (2byte), this return integer number need filtering.
so, I changed random() % 255 line.

on win32, original code gen_salt() function allways returned "$1$/2E./2E.".
this string made by same return value by random() function. (sorry, I can't express in good English)

This seems really suspicious. My explanation would be, that
Win32 starup somehow skips the srandom call.

Or could the (MyProcPid ^ port->session_start.tv_usec) be
constant on win32?

plz, check and properly fix this bug.

I tried "& 255" operation. but this bug is still.

I dont understand. Does that mean that

random()
random() & 255

are buggy, but

random() % 255

is not?

--
marko