freeradius-client-1.1.5

[Eivind Naess]

Hi Chris!

And thank you for your reply. Do you have any estimates on when 1.1.6
will be released?

On Nov 7, 2007 2:56 PM, Chris Parker <cparker at starnetusa.net> wrote:
>
> 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.
>

Thanks!

> >
> > 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.

Thanks!

> >
> > 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.

It works in CVS, thanks!

>
> > 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.

Very well, I'll define a macro in my code, that expands to
rc_add_config(..., "config", 0);

Thanks for clarifying this!

>
> > 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'.
>

Thanks!, I just noticed that the functionality is in CVS and not in
1.1.5 when I sent the email, sorry!

> > 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.

Oh well, I don't mean to start any religious debate on this, and
frankly I don't understand radius to the extent that you probably do.
But! I need to add some of the MS-CHAP attributes to the dictionary,
and I think it would be nice if I had the chance to do so with a
function and then use the dictionary as provided as a template.

Also, much of what I intend to do is to use simple username and
password, and ms-chap related Radius attributes. Do I really need to
keep the others around (except those which is specified by an RFC?)
The 'other' vendor specifics seems like an overkill to me, but I might
be wrong.

Just a few comments on the dicitionary file and format:
1) the dictionary seems to be a bit confusing to me, maybe add some
details regarding how one should add vendor specific attributes which
is different from freeradius server.
VENDOR          Microsoft                               311
ATTRIBUTE       MS-CHAP-Response             1       string  vendor=Microsoft
ATTRIBUTE       MS-CHAP-Error                     2       string
vendor=Microsoft

2) the types have string, but not octets as specified by freeradius.
Thus, if octets could be specified then you could perform a
rc_log_avpair() printing octets as hex, and strings as strings. Also,
wouldn't you say that a dictionary compatibility with freeradius would
be a beneficial?

Anyways, I was able to configure and run the client using freeradius
as the server. And, it passed both my tests: user/pass and user +
mschap-challenge, mschapv2-response.

>
> > 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.  :)

It's not stealing! I just tossed you a bone ;p)

>
> > BTW, I think freeradius is a great project, keep up the good work :o)
>
> Thanks for the valuable testing and feedback!

Glad I could be of any help, maybe I'll join your development efforts...

>
> -Chris
>
>
> -
> List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
>