From: | Magnus Hagander <magnus(at)hagander(dot)net> |
---|---|
To: | KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: RADIUS authentication |
Date: | 2010-01-25 21:30:51 |
Message-ID: | 9837222c1001251330i598be0f8w61b433f063c3ea45@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
2010/1/25 KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)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/
From | Date | Subject | |
---|---|---|---|
Next Message | Greg Smith | 2010-01-25 21:34:01 | Re: Dividing progress/debug information in pg_standby, and stat before copy |
Previous Message | Magnus Hagander | 2010-01-25 21:24:42 | Re: default pg_hba.conf tabulation |