On Fri, 2006-09-22 at 11:55 -0400, Alan DeKok wrote:

Peter Nixon <listuser@peternixon.net> wrote:

...

OK. No replies, so here is there patch.

Looks good to me.

I think many of the current log messages could be updated to use this new API. Each module could have a "log {}" subsection, with messages that can be edited and changed.

It's a bit of work, but very useful.

Another idea is "super duper debugging mode", which would create a directory per request, in that create a directory per authorize/etc section, and then create yet another directory for each module it runs. That final directory would contain the complete list of input/output pairs for check, request, reply, etc.

It's annoyingly huge, but it's a definitive view into which module is mangling what attribute.

It would be nice if the log file name and directory hierarchy could be defined using xlat and could be auto created based on defined criteria as well. This would allow as a simple example : logfile="/var/log/radius/%Y/%m/%d/radius-%{Client-Ip-Address:-log}" I had hoped I would have had time to look at solving this myself but have not had any time.

On Sat 23 Sep 2006 20:06, Nicolas Baradakis wrote:

Peter Nixon wrote:

...

OK. No replies, so here is there patch.

The upside of this patch is that you can use any radius attribute in auth/proxy logs. The existing format can be achieved with:

log_auth_format = "[%{User-Name}] (from client %C port %{NAS-Port} cli %{Calling-Station-Id})"

This should be the default value in radiusd.conf.in instead of the string found in the patch.

...

Is anyone vehmently against this patch or should I go ahead and commit it?

I think the patch could be less intrusive. If the log message is xlat'ed in the rad_authlog() function, you don't need a new logging function. Having one more logging function is more maintaining work, and it makes it more difficult to switch to a newer logging mechanism in the future. (like Alan suggested in a previous email)

ok. Well, this is a first stab at implimenting this :-)

I also note this patch doesn't use the same format for successful and failed login attempts. And it seems to me it doesn't print the password if "log_auth_goodpass" is set to "yes".

Ahhh.. Good point.

OTOH you could put the User-Password attribute directly in the string. Does this make the "log_auth_goodpass" option obsolete?

Possibly...

Until this question is answered, I see two different ways to do configurable logging in radiusd.conf.

The first one uses a set of 4 options:

log_auth = yes log_auth_badpass = yes log_auth_goodpass = yes # Append any attributes after the username and (eventually) the password log_auth_append = "from client %C port %{NAS-Port} cli %{Calling-Station-Id}"

The second one uses only 2 options:

# Log authentication without showing the passwords log_auth_accept = "[%{User-Name:-<no User-Name attribute>}] (from client %C port %{NAS-Port} cli %{Calling-Station-Id})" log_auth_reject = "[%{User-Name:-<no User-Name attribute>}] (from client %C port %{NAS-Port} cli %{Calling-Station-Id})"

# The options "log_auth_goodpass" and "log_auth_badpass" are deprecated # but you can still write log_auth_accept = "[%{User-Name:-<no User-Name attribute>}/%{User-Password:-<CHAP-Password>}] (from client %C port %{NAS-Port} cli %{Calling-Station-Id})" log_auth_reject = "[%{User-Name:-<no User-Name attribute>}/%{User-Password:-<CHAP-Password>}] (from client %C port %{NAS-Port} cli %{Calling-Station-Id})"

# Authentication logging is disabled log_auth_accept = "" log_auth_reject = ""

This was actually my first plan (To have 5 or six different logging config lines) but then we figured that it may not be necessary. You are correct though that its not as clean as it should be so I think we will have another stab at it on monday and go with a different config line for each type of log. Once we do this log_auth_badpass etc will be obselete Cheers -- Peter Nixon http://www.peternixon.net/ PGP Key: http://www.peternixon.net/public.asc