pair_cmp overwrites user file instance data [sec: unclas]

["Ranner, Frank MR" <Frank.Ranner@defence.gov.au>]

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);
                }
        }