Thread: patch contrib/pgcrypto for win32 (2)
I found that function gen_salt() in contrib/pgcrypto had bug on win32.
I patched contrib/pgcrypto/random.c file.
Attachment
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