On Tue, Mar 15, 2011 at 08:44:22AM +0000, Brian Candler wrote:
Alan DeKok wrote:
The "update" section should behave identically to the "if".
It doesn't at the moment.
I understand it a little bit more now. Here's a version which combines both behaviours in one. testing_module { update reply { Reply-Message += "Foo" } if (1) { update reply { Reply-Message += "Bar" } } ok } And here's the debug (it doesn't reach the 'ok', but it does add both Reply-Message attributes): Tue Mar 15 10:56:48 2011 : Info: ++- entering policy testing_module {...} Tue Mar 15 10:56:48 2011 : Info: +++[reply] returns reject Tue Mar 15 10:56:48 2011 : Info: +++? if (1) Tue Mar 15 10:56:48 2011 : Info: ? Evaluating (1) -> TRUE Tue Mar 15 10:56:48 2011 : Info: +++? if (1) -> TRUE Tue Mar 15 10:56:48 2011 : Info: +++- entering if (1) {...} Tue Mar 15 10:56:48 2011 : Info: ++++[reply] returns reject Tue Mar 15 10:56:48 2011 : Info: +++- if (1) returns reject Tue Mar 15 10:56:48 2011 : Info: ++- policy testing_module returns reject Tue Mar 15 10:56:48 2011 : Info: Failed to authenticate the user. It seems that two things are going on here: 1. The 'update reply' statement is returning 'reject' 2. At the top level of the policy group, the 'reject' is not causing an immediate return - but it does at the end of an 'if' block. Taking point (1) first, one thing I notice is here: if (child->type == MOD_UPDATE) { ... if (rcode != RLM_MODULE_UPDATED) { myresult = rcode; } else { /* * FIXME: Set priority based on * previous priority, so that we * don't stop on reject when the * default priority was to * continue... * */ } goto handle_result; So the 'updated' status from MOD_UPDATE is being explicitly ignored. In that case, myresult defaults to... well, at the top of modcall it's initialised to: myresult = stack.result[0] = default_component_results[component]; Since we're being called from module_authenticate (from rad_check_password) we're passed component == 0, and the default is RLM_MODULE_REJECT. However I don't see the reason why myresult = RLM_MODULE_UPDATED can't be set after an update statement. It's certainly fine for authentication; in auth.c the only return value from module_authenticate which causes an Access-Accept is RLM_MODULE_OK. So there must be some other case this was intended to deal with. As for point (2), this appears to be fortuitously because I had mapped the 'reject' result to a different code: Auth-Type PAP { pap { ok = return reject = 1 } testing_module } Which in turn begs the question, why does the 'if' statement still act on the reject? I'm wondering if the child->actions table is not inherited inside the 'if' body, because I can make it work like this: testing_module { update reply { Reply-Message += "Foo" } if (1) { reject = 1 <<<<< EXTRA LINE update reply { Reply-Message += "Bar" } } ok } But then, the expected actions seemed to get from the pap { ... } block to testing_module... Here's what I actually see on entry to modcall (without the extra line shown above): (gdb) print ((modgroup*)c)->mc $1 = {parent = 0x0, next = 0x0, name = 0x66f482 "PAP", type = MOD_GROUP, method = 0, actions = {-1, 1, -1, -1, 1, -1, -1, 1, 1}} (gdb) print ((modgroup*)c)->children $2 = (modcallable *) 0x7a3030 (gdb) print *(((modgroup*)c)->children) $3 = {parent = 0x7a2640, next = 0x7a25d0, name = 0x66f638 "pap", type = MOD_SINGLE, method = 0, actions = {1, -1, -1, -1, -1, -1, 1, 2, 4}} (gdb) print *(((modgroup*)c)->children->next) $4 = {parent = 0x7a2640, next = 0x0, name = 0x6543f8 "testing_module", type = MOD_POLICY, method = 0, actions = {-1, -1, 3, -1, -1, -1, 1, 2, 4}} (gdb) print ((modgroup*)(((modgroup*)c)->children->next))->mc $5 = {parent = 0x7a2640, next = 0x0, name = 0x6543f8 "testing_module", type = MOD_POLICY, method = 0, actions = {-1, -1, 3, -1, -1, -1, 1, 2, 4}} (gdb) print ((modgroup*)(((modgroup*)c)->children->next))->children $6 = (modcallable *) 0x7a3740 (gdb) print *(((modgroup*)(((modgroup*)c)->children->next))->children) $7 = {parent = 0x7a25d0, next = 0x7a37b0, name = 0x65457f "reply", type = MOD_UPDATE, method = 0, actions = {0, 0, 0, 0, 0, 0, 0, 0, 0}} (gdb) print *(((modgroup*)(((modgroup*)c)->children->next))->children->next) $8 = {parent = 0x7a25d0, next = 0x7a4460, name = 0x6547cb "(1)", type = MOD_IF, method = 0, actions = {-1, -1, 3, -1, -1, -1, 1, 2, 4}} (gdb) print *(((modgroup*)(((modgroup*)c)->children->next))->children->next->next) $9 = {parent = 0x7a25d0, next = 0x0, name = 0x654b50 "ok", type = MOD_SINGLE, method = 0, actions = {-1, -1, 3, -1, -1, -1, 1, 2, 4}} (gdb) print *((modgroup*)(((modgroup*)(((modgroup*)c)->children->next))->children->next))->children $10 = {parent = 0x7a37b0, next = 0x0, name = 0x65494f "reply", type = MOD_UPDATE, method = 0, actions = {0, 0, 0, 0, 0, 0, 0, 0, 0}} I see the actions table isn't set for MOD_UPDATE. As far as I can tell, the reject action is actions[0]. This is 1 in the pap{} section, but is -1 in testing_module, both at the top level and inside the MOD_IF node. So this rather spoils my theory :-( Regards, Brian.