From cbb598e017f8808949953dc57e42c8cf4e9aeb92 Mon Sep 17 00:00:00 2001 From: Paul Mackerras Date: Thu, 28 Oct 2004 00:24:40 +0000 Subject: [PATCH] Patch from Robert Vogelgesang: This patch does two things: o It adds some debugging messages. o "cleanup()" will no longer be added to the link_down_notifier chain. The debugging messages are obvious. The problem with cleanup() in the link_down_notifier chain is only half-way that there could be cases where the link would go up again and without a further authentication -- I just don't know if this can happen. But this part of the patch is a work-around for a _real_ problem/bug in the radius plugin (not the radattr plugin): The radius plugin calls functions registered via the radius_attributes_hook after each PAP authentication (which is correct), but only after the _first_ successful CHAP authentication during a session. Subsequent CHAP authentications are performed, but the radius_attributes_hook will not be processed again. This can happen in a setup where pppd is used as a back-end by a L2TP daemon, in case the PPP session at the other side of the L2TP tunnel reconnects, but the L2TP daemon at that side just reuses the old L2TP tunnel instead of creating a new one. In such situations, an incomming follow-up session via an existing T2TP tunnel would re-use the same instance of pppd; the incomming CHAP authentication would first tear down the old session, which in turn would call the link_down_notifier. When the _subsequent_ CHAP authentication succeeds, there is currently no call to the function assigned to radius_attributes_hook (here: print_attributes(); THIS BUG REMAINS AND NEEDS TO BE FIXED). To summarize: The radius plugin calls the function registered via the radius_attributes_hook after _each_ successful PAP authentication, but only after the _first_ successful CHAP authentication; radius_attributes_hook _should_ be processed after _each_ successful CHAP authentication. I have currently no patch for this bug; furthermore, I should first contact the author of the radius plugin and ask him, _why_ he has programmed a special handling of subsequent CHAP authentications. With the following patch, the follow-up session can re-use the radattr-file left over from the previous session, which is OK in our application, but may cause problems in others. Note: This is only a problem when CHAP is used; subsequent sessions authenticated with PAP are OK, with and without this patch. --- pppd/plugins/radius/radattr.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/pppd/plugins/radius/radattr.c b/pppd/plugins/radius/radattr.c index 77fc61d..1fe7daa 100644 --- a/pppd/plugins/radius/radattr.c +++ b/pppd/plugins/radius/radattr.c @@ -15,7 +15,7 @@ ***********************************************************************/ static char const RCSID[] = -"$Id: radattr.c,v 1.1 2002/01/22 16:03:00 dfs Exp $"; +"$Id: radattr.c,v 1.2 2004/10/28 00:24:40 paulus Exp $"; #include "pppd.h" #include "radiusclient.h" @@ -41,7 +41,12 @@ plugin_init(void) { radius_attributes_hook = print_attributes; +#if 0 + /* calling cleanup() on link down is problematic because print_attributes() + is called only after PAP or CHAP authentication, but not when the link + should go up again for any other reason */ add_notifier(&link_down_notifier, cleanup, NULL); +#endif /* Just in case... */ add_notifier(&exitnotify, cleanup, NULL); @@ -65,6 +70,7 @@ print_attributes(VALUE_PAIR *vp) char fname[512]; char name[2048]; char value[2048]; + int cnt = 0; slprintf(fname, sizeof(fname), "/var/run/radattr.%s", ifname); fp = fopen(fname, "w"); @@ -78,8 +84,10 @@ print_attributes(VALUE_PAIR *vp) continue; } fprintf(fp, "%s %s\n", name, value); + cnt++; } fclose(fp); + dbglog("RADATTR plugin wrote %d line(s) to file %s.", cnt, fname); } /********************************************************************** @@ -99,4 +107,5 @@ cleanup(void *opaque, int arg) slprintf(fname, sizeof(fname), "/var/run/radattr.%s", ifname); (void) remove(fname); + dbglog("RADATTR plugin removed file %s.", fname); } -- 2.39.2