Re: RADIUS authentication

(2010/01/10 22:25), Magnus Hagander wrote:
> The attached patch implements RADIUS authentication (RFC2865-compatible).
>
> The main usecase for me in this is the ability to use (token based)
> one-time-password systems easily with PostgreSQL. These systems almost
> always support RADIUS, and the implementation is fairly simple. RADIUS
> can of course be used in many other scenarios as well (for example, it
> can be used to implement "only this group"-access with at least Active
> Directory, something our current LDAP doesn't support. We might
> eventually want to support that in our LDAP, but it's not there now)

I checked this patch.

It can be applied on the latest CVS HEAD, and built without any matter.

I tried to work it with freeradius, then it performs well in a few
configurations. (Of course, it is far from comprehensive.)
Because I'm not good at RADIUS protocol, I didn't check correctness
of the protocol.

Hmm, it introduces the format of the UDP packets.
It seems to me this patch is implemented correctly.
http://technet.microsoft.com/en-us/library/cc958030.aspx

Here is a few comments from the initial reviewing.

* Is the feature to be configurable via ./configure scripts?
Currently, we have --with-pam or --with-ldap option, and it allows
users to turn on/off the feature.
Of course, it has dependency on libraries.

* A corresponding comment. This patch implements RADIUS protocol
by itself. Is there any commonly used libraries for the purpose?
It allows us to separate a burden to manage a certain network
protocol within PostgreSQL.

* IIUC, inet_addr() takes only IPv4 address. It is used to translate
"radiusserver" parameter to netaddr format.
Could you document this parameter takes only IPv4 format.

* I think this comment is right.
+ for (i = 0; i < RADIUS_VECTOR_LENGTH; i++)
+ /* XXX: Generate a more secure random string? */
+ packet->vector[i] = random() % 255;

The random seed is initialized at BackendRun() with MyProcPid and
the time of backend process launched.
Then, PostgresMain() -> InitPostgres() -> PerformAuthentication()
will be called, and this random() shall be the first call just after
initialization of the srandom().

Do you have any good idea?
Or, do you think it should be fixed with high priority?

* 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?

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>