Re: RADIUS authentication

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/

In response to

Responses

Browse pgsql-hackers by date

  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