Small patch for proxy code - listen.c
Hi, I have a small patch for the proxy code, in particular for listen.c in CVS HEAD. When the proxy reply comes back, only the cl->ipaddr is checked against the reply source address, however it is possible to configure cl->acct_ipaddr differently to cl->ipaddr (ie different auth and acct home servers for a single realm entry), and thus the active status and last reply time may never be updated for an accounting home server. I'm not sure what functional detriment the current behaviour might have, as I only stumbled across this while trying a small modification to proxy.c so that it would choose one of the home servers, even if they are all marked dead. Unfortunately I can't get CVS HEAD to compile under Solaris 10 x86 using gcc, so I have to admit I haven't compiled this code yet, but I have test the equivalent for 1.0.1 at work today. Sorry. Actually, this brings up another point that just crossed my mind, should cl->last_reply be split between authentication and accounting? Probably not a big deal, but I think it could result in for example the home auth server not being marked dead for the realm if a reply had been received from the home accounting server? (files.c around line 500) regards, Mike -bash-3.00$ diff -u listen.c.orig listen.c --- listen.c.orig Thu Sep 1 23:28:11 2005 +++ listen.c Fri Sep 2 00:00:48 2005 @@ -916,17 +916,24 @@ * as garbage. */ for (cl = mainconfig.realms; cl != NULL; cl = cl->next) { - if (oldreq->proxy_reply->src_ipaddr.af != cl->ipaddr.af) continue; + if ((oldreq->proxy_reply->src_ipaddr.af != cl->ipaddr.af) && + (oldreq->proxy_reply->src_ipaddr.af != cl->acct_ipaddr.af)) + continue; + if (cl->ipaddr.af != AF_INET) continue; /* FIXME */ - if (oldreq->proxy_reply->src_ipaddr.ipaddr.ip4addr.s_addr == cl->ipaddr.ipaddr.ip4addr.s_addr) { - if (oldreq->proxy_reply->src_port == cl->auth_port) { - cl->active = TRUE; - cl->last_reply = oldreq->timestamp; - } else if (oldreq->proxy_reply->src_port == cl->acct_port) { - cl->acct_active = TRUE; - cl->last_reply = oldreq->timestamp; - } + if ((oldreq->proxy_reply->src_ipaddr.ipaddr.ip4addr.s_addr == cl->ipaddr.ipaddr.ip4addr.s_addr) && + (oldreq->proxy_reply->src_port == cl->auth_port)) { + + cl->active = TRUE; + cl->last_reply = oldreq->timestamp; + + } else if ((oldreq->proxy_reply->src_ipaddr.ipaddr.ip4addr.s_addr == cl->acct_ipaddr.ipaddr.ip4addr.s_addr) && + (oldreq->proxy_reply->src_port == cl->acct_port)) { + + cl->acct_active = TRUE; + cl->last_reply = oldreq->timestamp; + } }
Michael Mitchell wrote:
When the proxy reply comes back, only the cl->ipaddr is checked against the reply source address, however it is possible to configure cl->acct_ipaddr differently to cl->ipaddr (ie different auth and acct home servers for a single realm entry), and thus the active status and last reply time may never be updated for an accounting home server.
Thanks for spotting this. This is a problem indeed. Can you please fill a bug report on the bugzilla, so your patch doesn't get lost? http://bugs.freeradius.org/enter_bug.cgi -- Nicolas Baradakis
Nicolas Baradakis <nbk@sitadelle.com> wrote:
Thanks for spotting this. This is a problem indeed. Can you please fill a bug report on the bugzilla, so your patch doesn't get lost?
For 2.0, I'd like to fix the realm handling, to separate realms from home servers. e.g. realm foo.com { nostrip home_server = radius-for-foo.com } home_server radius-for-foo.com { type = acct host = ... port = ... secret = ... } That will fix a number of issues in the code, and enable multiple realms to share a fail-over list of home servers, without the current problem of typing in the same home server multiple times. Alan DeKok.
participants (3)
-
Alan DeKok -
Michael Mitchell -
Nicolas Baradakis