freeradius-client-1.1.5

[Chris Parker]

On Nov 7, 2007, at 10:59 AM, Eivind Naess wrote:

> Hi Chris,
>
> Thank you for your reply to my question. Now, I did checkout
> freeradius-client from CVS and the function that I asked for exist.
> However, I did run into a few problems / issues. I am trying to
> cross-compile this for an ARM Intel ixp425 platform.
>
> 1) I needed to export ac_cv_func_uname=no into the environment before
> executing ./configure. Configure just didn't like the fact that I was
> cross-compiling with a different toolchain and barfed when it figured
> it couldn't run the test program. This should probably get fixed for
> 1.1.6.

Agreed, cross-compiling isn't something that I have tested, so that  
will likely need some tweaks.

>
> 2) There are some added macro "DEBUG" added to the
> freeradius-client.h. This conflicts with a different define of DEBUG
> defined somewhere else. I'd say either remove it, or rename it. It's
> for debugging anyway.

Yes, that needs to be cleaned up.  That was added to enable some  
debugging of the new functions.

>
> 3) Cross-compiling 1.1.5, I had a problem with sending authentication
> requests where the password got corrupted due to some alignment errors
> in rc_pack_list() and rc_md5_calc(). It appears that the problem is
> gone in CVS, however; the code looks pretty much the same to me. What
> I did to circumvent the problem was to declare a char
> md5[MD5_DIGEST_LENGTH] and use that for MD5Final(), then do a
> memcpy(output, md5, MD5_DIGEST_LENGTH). This works well, when the
> output variable doesn't align with the boundary of my processor, in
> gdb it showed up as 0x...0a and it stomped on 0x...08 (the two first
> bytes got corrupted and my radiusd didn't understand squat).

This is probably due to moving to uint_X rather than uchar and the like.

> 4) I looked at the function signature for rc_add_config(...), and it
> does not seem to require "line", and "filename". It's more an artifact
> of your static functions requiring some input in case they fail. In my
> opinion, since the code is shared between rc_read_config(), the static
> functions should not need to know anything about filename and
> position, that's a responsibility of the caller function. Return codes
> can be used to solve this better refactoring the functions to better
> fit a users (freeradius-client api) need. The filename and line
> arguments simply doesn't have any value when you embed the radius
> client.

I've left those there, as I'm passing values from the config file of  
the application I'm embedding the radius client into.  :)  Also, it  
helps a bit with backwards compatability.

> Please consider the following as comments:
> 5) If you really want to 'embed' freeradius-client, it would be nice
> if I could specify something similar to "authserv
> <ip>:<port>:<secret>" and not needing to write a "server" file which
> declares the servers w/secrets.

You can do that with rc_add_config.

rc_add_config(rad_config, "acctserver", server, filename, lineno)

Where server is a string with 'ip:port:secret'.

> 6) I don't necessarily understand the radius-protocol in detail, but I
> would think it would be nice to be able to specify the dictionary
> attributes/values as you kind of do with rc_add_config(..). This again
> relates to the fact of reducing external dependencies when embedding
> freeradius-client.

There's been discussion about that.  For now, the dictionary is one  
of the few external files still required for the client lib.

> 7) This comment is related to logs, and I haven't studied the use of
> rc_log too closely. Just in general, this is a library and not an
> application. It would be nice to possibly use logs to trace calls
> within the library, but for error reporting it seems like it would be
> more useful to return an error code, and then call a
> rc_geterror_str(errno) or something. I am speaking of this as an
> integrator of OSS and I often like the ability to control the output
> of an application and not to necessarily "pollute" the system's event
> logs.

That's a very good idea.  I think I will steal it and implement it.  :)

> BTW, I think freeradius is a great project, keep up the good work :o)

Thanks for the valuable testing and feedback!

-Chris