pair_cmp overwrites user file instance data [sec: unclas]

[Ranner, Frank MR]

Hello,
 
In my users file I have rules that link ldap groups to hunt groups,
possibly with suffixes.
 
They look something like this:
 
DEFAULT Ldap-Group == `%{Huntgroup-Name}`
        Access-Level := RW,
        Service-Type = Administrative-User,
        Cisco-AVPair := "shell:priv-lvl=15",
        Passport-Command-Impact = configuration
 
# Check passport access groups
DEFAULT Ldap-Group == `%{Huntgroup-Name}_configuration`
        Passport-Command-Impact = configuration
 
DEFAULT Ldap-Group == `%{Huntgroup-Name}_systemadmin`
        Passport-Command-Impact = systemAdministration
 
DEFAULT Ldap-Group == `%{Huntgroup-Name}_passive`
        Passport-Command-Impact = passive
 
# These checks cover Nortel switches, et al.
DEFAULT Ldap-Group == `%{Huntgroup-Name}_RWA`
        Service-Type = Administrative-User,
        Access-Level := RWA
 
DEFAULT Ldap-Group == `%{Huntgroup-Name}_RO`
        Access-Level := RO,
        Service-Type = Nas-Prompt-User
 
On the first request that comes in, the system worked fine. In the
radiusd output 
you would see and ldap group search for, say, QLD_South, then
QLD_South_configuration, 
etc. Eventually one would match (or all would fail) and the request
would complete.
 
Now a second request for a different huntgroup would come in, but the
ldap search would stil be for 
QLD_South and so on.
 
After tracing through with gdb I discovered that pair_cmp when xlating
the config item was actually 
overwriting the source data with the xlat result. I didn't want to
fiddle with pair_cmp so i looked at 
rlm_files with the intention of supplying pair_cmp with a copy of the
user record rather than the 
original. It turns out that pl->check was already being copied, so all
that needed to be done was make 
the copy earlier, and supply check_tmp rather than pl->check to
pair_cmp.
 
Long story short, this fixes the problem. here is the patch:
 
*** src/modules/rlm_files/rlm_files.c~  Thu Oct 21 03:14:38 2004
--- src/modules/rlm_files/rlm_files.c   Tue Aug 15 12:48:22 2006
***************
*** 313,324 ****
                        continue;
                }
 
                /*
                 *      If the current request matches against the
                 *      check pairs, then add the reply pairs from the
                 *      entry to the current list of reply pairs.
                 */
!               if ((paircmp(request, request_pairs, pl->check,
reply_pairs) == 0)) {
                        if ((mainconfig.do_usercollide) &&
                            (strcmp(pl->name, "DEFAULT"))) {
 
--- 313,327 ----
                        continue;
                }
 
+               /* Don't let paircmp overwrite original record when
expanding variables */
+               check_tmp = paircopy(pl->check);
+
                /*
                 *      If the current request matches against the
                 *      check pairs, then add the reply pairs from the
                 *      entry to the current list of reply pairs.
                 */
!               if ((paircmp(request, request_pairs, check_tmp,
reply_pairs) == 0)) {
                        if ((mainconfig.do_usercollide) &&
                            (strcmp(pl->name, "DEFAULT"))) {
 
***************
*** 331,337 ****
                                check_save =
paircopy(request->config_items);
 
                                /* Copy this users check pairs to the
request */
-                               check_tmp = paircopy(pl->check);
                                pairmove(check_pairs, &check_tmp);
                                pairfree(&check_tmp);
 
--- 334,339 ----
***************
*** 367,373 ****
 
                                DEBUG2("    users: Matched entry %s at
line %d", pl->name, pl->lineno);
                                found = 1;
-                               check_tmp = paircopy(pl->check);
                                reply_tmp = paircopy(pl->reply);
                                pairxlatmove(request, reply_pairs,
&reply_tmp);
                                pairmove(check_pairs, &check_tmp);
--- 369,374 ----
***************
*** 379,384 ****
--- 380,387 ----
                         */
                        if (!fallthrough(pl->reply))
                                break;
+               } else {
+                 pairfree(&check_tmp);
                }
        }
 
 
 
 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freeradius.org/pipermail/freeradius-users/attachments/20060816/df55da82/attachment.html>