liudf0716 / apfree-wifidog

apfree-wifidog is a high-performance captive portal solution that serves as a gateway between your wireless networks and the Internet. Optimized for both HTTP and HTTPS traffic, it ensures secure border control while enabling seamless user authentication and efficient network management.
http://wifidogx.online
GNU General Public License v3.0
833 stars 301 forks source link

Authenticated repeatedly. #230

Closed bobbyz3g closed 4 months ago

bobbyz3g commented 3 years ago

When device A is successfully authenticated, device B also completes the authentication. At this point, both device A and device B need to be authenticated repeatedly. And has the following error message:

Thu Sep  2 07:59:30 2021 daemon.err wifidogx[6456]: iptables_fw_counters_update(): Could not find 0 in client list, this should not happen unless if the gateway crashed
Thu Sep  2 07:59:30 2021 daemon.err wifidogx[6456]: Preventively deleting firewall rules for 0 in table WiFiDog_$ID$_Outgoing
Thu Sep  2 07:59:30 2021 daemon.err wifidogx[6456]: Preventively deleting firewall rules for 0 in table WiFiDog_$ID$_Incoming
Thu Sep  2 08:04:30 2021 daemon.err wifidogx[6456]: iptables command failed(0): iptables -t mangle -D WiFiDog_br-lan_Outgoing -s 192.168.4.110 -m mac --mac-source e6:b9:ab:33:a5:f6 -j MARK --set-mark 0x020000/0xff0000

Here is my config

config wifidog
    option gateway_interface 'br-lan'
    option auth_server_hostname '192.168.1.1' #'change wifidog.kunteng.org.cn to your auth server domain or ip'
    option auth_server_port 8000
    option auth_server_path '/wifidog/'
    option check_interval 60
    option client_timeout 5
    option pool_mode 1
    option thread_number 5
    option queue_size 20
    option wired_passed 0
    option disabled 0
xscode-auto-reply[bot] commented 3 years ago

Thanks for opening a new issue. The team has been notified and will review it as soon as possible. For urgent issues and priority support, visit https://xscode.com/liudf0716/apfree_wifidog

bobbyz3g commented 3 years ago

The system version is the cause, after downgrading openwrt to 19.07, it is back to normal.

liudf0716 commented 2 years ago

The system version is the cause, after downgrading openwrt to 19.07, it is back to normal.

can u provide the system version which cause above problem? thx

wsch32 commented 2 years ago

The system version is the cause, after downgrading openwrt to 19.07, it is back to normal.

can u provide the system version which cause above problem? thx

I have the same problem in Openwrt 21.02, and I fixed.

The problem is in the function of iptables_fw_counters_update() ( fw_iptables.c line 1228)

The reason why this problem occur is as following:

  1. fscanf format string problem(last %*s );
  2. because of reason 1, the the ip string got a result of "0",and the C function inet_aton() take the result of "0" for granted (return true).
  3. The funcion client_list_find_by_ip() can't find the ip of "0",and trigger the reset of firwall logic.

That's All!

Talk is cheap, show the code(fw_iptables.c line 1228) .........

` /* Update the counters of all the clients in the client list / int iptables_fw_counters_update(void) { FILE output; char script, ip[16] = {0}, rc; unsigned long long int counter; t_client *p1; struct in_addr tempaddr;

/* Look for outgoing traffic */
safe_asprintf(&script, "%s %s", "iptables", "-v -n -x -t mangle -L " CHAIN_OUTGOING);
iptables_insert_gateway_id(&script);
debug(LOG_DEBUG, "Run iptables Command: %s", script);
output = popen(script, "r");
free(script);
if (!output) {
    debug(LOG_ERR, "popen(): %s", strerror(errno));
    return -1;
}

LOCK_CLIENT_LIST();
reset_client_list();
UNLOCK_CLIENT_LIST();

/* skip the first two lines */
while (('\n' != fgetc(output)) && !feof(output)) ;
while (('\n' != fgetc(output)) && !feof(output)) ;

while (output && !(feof(output))) {
    rc = fscanf(output, "%*s %llu %*s %*s %*s %*s %*s %15[0-9.] %*s %*s %*s %*s %*s", &counter, ip);
    //rc = fscanf(output, "%*s %llu %*s %*s %*s %*s %*s %15[0-9.] %*s %*s %*s %*s %*s 0x%*u", &counter, ip);
    debug(LOG_DEBUG, "Read from iptables format string output: ip = %s  count = %llu", ip, counter);
    if (2 == rc && EOF != rc) {
        /* Sanity */
        if ( (!inet_aton(ip, &tempaddr)) || (0 == strcmp(ip,"0")) ) {
            debug(LOG_WARNING, "I was supposed to read an IP address but instead got [%s] - ignoring it", ip);
            continue;
        }
        debug(LOG_DEBUG, "Read outgoing traffic for %s: Bytes=%llu", ip, counter);
        LOCK_CLIENT_LIST();
        if ((p1 = client_list_find_by_ip(ip))) {
            if ((p1->counters.outgoing - p1->counters.outgoing_history) < counter) {
                p1->counters.outgoing_delta = p1->counters.outgoing_history + counter - p1->counters.outgoing;
                p1->counters.outgoing = p1->counters.outgoing_history + counter;
                p1->counters.last_updated = time(NULL);
                debug(LOG_DEBUG, "%s - Outgoing traffic %llu bytes, updated counter.outgoing to %llu bytes.  Updated last_updated to %d", ip,
                      counter, p1->counters.outgoing, p1->counters.last_updated);
                p1->is_online = 1;
            }

            // get client name
            if(p1->name == NULL)
                __get_client_name(p1);

            if(p1->wired == -1) {
                p1->wired = br_is_device_wired(p1->mac);
            }
            UNLOCK_CLIENT_LIST();
        } else {
            UNLOCK_CLIENT_LIST();
            debug(LOG_ERR,
                  "iptables_fw_counters_update(): Could not find %s in client list, this should not happen unless if the gateway crashed",
                  ip);
            // debug(LOG_ERR, "Preventively deleting firewall rules for %s in table %s", ip, CHAIN_OUTGOING);
            // __iptables_fw_destroy_mention("mangle", CHAIN_OUTGOING, ip, NULL, 5);
            // debug(LOG_ERR, "Preventively deleting firewall rules for %s in table %s", ip, CHAIN_INCOMING);
            // __iptables_fw_destroy_mention("mangle", CHAIN_INCOMING, ip, NULL, 5);
        }

    }
}
pclose(output);

/* Look for incoming traffic */
safe_asprintf(&script, "%s %s", "iptables", "-v -n -x -t mangle -L " CHAIN_INCOMING);
iptables_insert_gateway_id(&script);
debug(LOG_DEBUG, "Run iptables Command: %s", script);
output = popen(script, "r");
free(script);
if (!output) {
    debug(LOG_ERR, "popen(): %s", strerror(errno));
    return -1;
}

/* skip the first two lines */
while (('\n' != fgetc(output)) && !feof(output)) ;
while (('\n' != fgetc(output)) && !feof(output)) ;
while (output && !(feof(output))) {
    rc = fscanf(output, "%*s %llu %*s %*s %*s %*s %*s %*s %15[0-9.]", &counter, ip);
    if (2 == rc && EOF != rc) {
        /* Sanity */
        if ( (!inet_aton(ip, &tempaddr)) || (0 == strcmp(ip,"0")) ) {
            debug(LOG_WARNING, "I was supposed to read an IP address but instead got [%s] - ignoring it", ip);
            continue;
        }
        debug(LOG_DEBUG, "Read incoming traffic for %s: Bytes=%llu", ip, counter);
        LOCK_CLIENT_LIST();
        if ((p1 = client_list_find_by_ip(ip))) {
            if ((p1->counters.incoming - p1->counters.incoming_history) < counter) {
                p1->counters.incoming_delta = p1->counters.incoming_history + counter - p1->counters.incoming;
                p1->counters.incoming = p1->counters.incoming_history + counter;
                debug(LOG_DEBUG, "%s - Incoming traffic %llu bytes, Updated counter.incoming to %llu bytes", ip, counter, p1->counters.incoming);
            /*  p1->counters.last_updated = time(NULL); */
            }

            UNLOCK_CLIENT_LIST();
        } else {
            UNLOCK_CLIENT_LIST();
            debug(LOG_ERR,
                  "iptables_fw_counters_update(): Could not find %s in client list, this should not happen unless if the gateway crashed",
                  ip);
            // debug(LOG_ERR, "Preventively deleting firewall rules for %s in table %s", ip, CHAIN_OUTGOING);
            // __iptables_fw_destroy_mention("mangle", CHAIN_OUTGOING, ip, NULL, 5);
            // debug(LOG_ERR, "Preventively deleting firewall rules for %s in table %s", ip, CHAIN_INCOMING);
            // __iptables_fw_destroy_mention("mangle", CHAIN_INCOMING, ip, NULL, 5);
        }
    }
}
pclose(output);

return 1;

} `

liudf0716 commented 2 years ago

@wsch32 very good, can u provide PR to fix that bug?

wsch32 commented 2 years ago

@wsch32 very good, can u provide PR to fix that bug?

of course I can do this,but the way I fixed is temporary,when the iptables return info changes again, it will go wrong again. I don't think it's a good idea that use popen() and fscanf() C function for execute iptables command and got the statistics of each client, We should use the iptc lib rewrite this part of code.