2010/1/25 KaiGai Kohei <kai...@ak.jp.nec.com>: > (2010/01/24 23:29), Magnus Hagander wrote: >> There is one more option here - use OpenSSL if available. It has >> functions for secure random number generations >> (http://www.openssl.org/docs/crypto/RAND_bytes.html). That seems easy >> enough when OpenSSL is available. > > In just my opinion (so, committer may have different one), it is an > option to utilize openSSL library when available. However, it should > be moved to PostmasterRandom() and used to provide more randomness > for srandom(). And, srandom() in the head of BackendRun() should be > replaced by PostmasterRandom().
That is a feature separate from this. And note that PostmasterRandom() and friends still deal with pseudo-random numbers AFAIK. Not crytographically strong ones. Which migh tactually be something we'd want to do in other places (like generating salts), but again that's a completely different scope from this. > I also want any opinions from others. Agreed, me too. I suggest a separate thread discussing random generations in general for that. >> The question then becomes what do we do if we don't have OpenSSL >> available? Do we document that it's not secure, or refuse to run it? >> I'd vote for document it.. If you don't have SSL enabled, then you >> clearly don't use SSL for the libpq connection, which means the >> password goes in cleartext in that stream... > > The seed of random is a different issue from safeness of password on > the stream between client and server. For example, if admin set up > IPsec/ESP between them, OpenSSL is not must-requirement. Exactly, which is why I suggest a note in the docs only. > Even if OpenSSL is not available, as long as both of PostgreSQL and > RADIUS server are set up in trusted network, we can consider it is > secure. So, all we can do is to introduce the risk, and the decisions > are depending on end-users. Agreed. >>>>> * It casts char array (such as radius_buffer) into radius_packet >>>>> structure. The radius_packet structure represents the format of >>>>> RADIUS network packet as is. >>>>> It may be preferable to give compiler a hint not to align this >>>>> structure. >>>>> In GCC, we can use "__attribute__((packed))" to suggest not to >>>>> align the member of structure. Is there any portable way for this? >>>> >>>> This I can't answer, I don't know this well enough. Somebody else? >>> >>> What manner is applied to handle network protocol in other part? >>> >>> The radius_packet is declared as follows: >>> >>> + typedef struct >>> + { >>> + unsigned char code; +0 >>> + unsigned char id; +1 >>> + unsigned short length; +2 >>> + unsigned char vector[RADIUS_VECTOR_LENGTH]; +4? +8? >>> + } radius_packet; >>> >>> It may be a bit nervous, except for possible alignment of the vector >>> on 64bit architecture. >>> >>> And, one more. It seems to me uint8 and uint16 are more preferable than >>> unsigned char/short in this context. >> >> Yeha, that is probably right. I copied that off a reference >> implementation of the struct. Will change accordingly. >> >> FWIW, I tested it on Win64 and it didn't have any issues there at least. > > Just to be safe, could you inject an Assert() here? > If a minor compiler aligned it unintentionally, it will be a bug not easy > to find out. > > /* check whether the compiler aligned it unintentionally, or not */ > Assert(offsetof(radius_packet, vector) == 4); Yeah, good point. I'll add that. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers