serious problem with 2.0 rlm_sql?

[Hoppál Felicián]

Hello,

I'm new to this list but not to FreeRADIUS. There's a quite big change in 2.0 which might lead to serious issues with more complex FreeRADIUS setups but I'm not sure if it's a bug.

In 1.x you could use any attribute in authorize_group_check_query to validate the user. I mean if you specified a Huntgroup-Name in the groupcheck sql table it worked, the rlm_sql module returned a not found if those pairs didn't match:

        if (paircmp(request, request->packet->vps, check_tmp, &reply_tmp) != 0) {
                radlog(L_INFO, "rlm_sql (%s): No matching entry in the database for request from user [%s]",
                       inst->config->xlat_name, sqlusername);
                /* Remove the username we (maybe) added above */
                pairdelete(&request->packet->vps, PW_SQL_USER_NAME);
                pairfree(&reply_tmp);
                pairfree(&check_tmp);
                return RLM_MODULE_NOTFOUND;
        }

We use this for example the ADSL group has a Huntgroup-Name =~ pppoe|dialup

This changed in 2.0 with separating rlm_sql_process_groups(). If you put these checks in the usercheck table they will still work, FreeRADIUS won't find the user and will return an Access-Reject to the client. But if you put it in the radgroupcheck table see what happens:

The rlm_sql_authorize() checks if these pairs match with the Access-Request and the usercheck table:

                /*
                 *      Only do this if *some* check pairs were returned
                 */
                if (paircompare(request, request->packet->vps, check_tmp, &request->reply->vps) == 0) {
                        found = 1;

They will match because Huntgroup-Name is in the groupcheck table and not in the usercheck. So the user is found and FreeRADIUS will process the groups. And the problem in rlm_sql_process_groups:

                } else if (rows > 0) {
                        /*
                         *      Only do this if *some* check pairs were returned
                         */
                        if (paircompare(request, request->packet->vps, check_tmp, &request->reply->vps) == 0) {
                                found = 1;
                                DEBUG2("rlm_sql (%s): User found in group %s",
                                        inst->config->xlat_name, group_list_tmp->groupname);
                                /*
                                 *      Now get the reply pairs since the paircompare matched
                                 */
                                if (!radius_xlat(querystr, sizeof(querystr), inst->config->authorize_group_reply_query, request, sql_escape_func)) {
                                        radlog(L_ERR, "rlm_sql (%s): Error generating query; rejecting user",
                                               inst->config->xlat_name);
                                        /* Remove the grouup we added above */
                                        pairdelete(&request->packet->vps, PW_SQL_GROUP);
                                        pairfree(&check_tmp);
                                        return -1;
                                }
                                if (sql_getvpdata(inst, sqlsocket, &reply_tmp, querystr) < 0) {
                                        radlog(L_ERR, "rlm_sql (%s): Error retrieving reply pairs for group %s",
                                               inst->config->xlat_name, group_list_tmp->groupname);
                                        /* Remove the grouup we added above */
                                        pairdelete(&request->packet->vps, PW_SQL_GROUP);
                                        pairfree(&check_tmp);
                                        pairfree(&reply_tmp);
                                        return -1;
                                }
                                *dofallthrough = fallthrough(reply_tmp);
                                pairxlatmove(request, &request->reply->vps, &reply_tmp);
                                pairxlatmove(request, &request->config_items, &check_tmp);
                        }

The paircompare() won't be true because the Huntgroup-Name doesn't match, but the USER WON'T GET ACCESS-REJECT! The only thing that happens is the user won't get the pairs specified in the groupreply table because the paircompare() won't be true.

So the rlm_sql_process_groups returns without the groupreply pairs and rlm_sql_authorize lets the user in:

        if (!found) {
                radlog(L_DBG, "rlm_sql (%s): User %s not found",
                       inst->config->xlat_name, sqlusername);
                return RLM_MODULE_NOTFOUND;
        } else {
                return RLM_MODULE_OK;
        }

I think it's a quite serious problem.

Best regards,
Felician Hoppal