From f24aed77a658a572eb351908a13560808e96e07a Mon Sep 17 00:00:00 2001 From: Scott Sakai Date: Wed, 2 Mar 2016 22:55:30 -0800 Subject: [PATCH] Fix what looks like a logic error in sync logic. Only $req_answers sync peers would get polled. When $req_answers is less than $nr_servers, some servers (that return replayed counters) will get ignored, since retrieveURLasync() stops after $req_answers responses. The fix requires $nr_servers responses from retrieveURLasync, causing all sync peers to get polled and processed by sync(). This arrangement also allows a two-server sync pool to operate when one peer is gone or unreachable, something that cannot be done before these modifications. Set the sync_level to 0, which means "try everyone, but if you get no valid responses, it's okay to proceed". Prior to the modifications, it means "don't even try syncing". Also, added ykval-cron, which can be fired off from a cron job to make sure ykval-queue stays running. This is example code, as your enviroment and usernames may differ. --- ykval-cron | 22 ++++++++++++++++++++++ ykval-verify.php | 32 +++++++++++++++++++++++--------- 2 files changed, 45 insertions(+), 9 deletions(-) create mode 100755 ykval-cron diff --git a/ykval-cron b/ykval-cron new file mode 100755 index 0000000..f6500d0 --- /dev/null +++ b/ykval-cron @@ -0,0 +1,22 @@ +#!/bin/bash + +# Something to keep ykval-queue going +# run this in cron +# restarts if necessary + +RUNAS=ykval + +function start +{ + su -s /bin/bash -c '/usr/sbin/ykval-queue &>/dev/null &' - ${RUNAS} + exit 0 +} + + +FOUND=$( ps -C ykval-queue | wc -l ) + +if [[ ${FOUND} -lt 2 ]]; then + start +fi + +exit 0 diff --git a/ykval-verify.php b/ykval-verify.php index 0956197..f89be5f 100644 --- a/ykval-verify.php +++ b/ykval-verify.php @@ -378,9 +378,13 @@ if (!$sync->queue($otpParams, $localParams)) $nr_servers = $sync->getNumberOfServers(); $req_answers = ceil($nr_servers * $sl / 100.0); -if ($req_answers > 0) +// try syncing anyway +if ($nr_servers > 0) { - $syncres = $sync->sync($req_answers, $timeout); + // we need to attempt syncing with all servers, since one may + // return replayed counters. Using $req_answers means such a server + // may get ignored. + $syncres = $sync->sync($nr_servers, $timeout); $nr_answers = $sync->getNumberOfAnswers(); $nr_valid_answers = $sync->getNumberOfValidAnswers(); $sl_success_rate = floor(100.0 * $nr_valid_answers / $nr_servers); @@ -405,16 +409,26 @@ $myLog->log(LOG_INFO, '', array( if ($syncres == False) { - /* sync returned false, indicating that - either at least 1 answer marked OTP as invalid or - there were not enough answers */ - $myLog->log(LOG_WARNING, 'Sync failed'); + /* sync returned false, indicating that not all servers returned valid answers; + some may have returned NO answer (host down...). Sort this out here, + it's not the end of the world yet! */ - if ($nr_valid_answers != $nr_answers) + if ($nr_answers != $nr_valid_answers) + { + /* at least 1 answers marked OTP as invalid */ + $myLog->log(LOG_WARNING, 'Sync failed'); sendResp(S_REPLAYED_OTP, $myLog, $apiKey, $extra); + } + + if ($nr_valid_answers < $req_answers) + { + /* not enough valid answers */ + $myLog->log(LOG_WARNING, 'Sync failed'); + + $extra['sl'] = $sl_success_rate; + sendResp(S_NOT_ENOUGH_ANSWERS, $myLog, $apiKey, $extra); + } - $extra['sl'] = $sl_success_rate; - sendResp(S_NOT_ENOUGH_ANSWERS, $myLog, $apiKey, $extra); } if ($otpParams['yk_counter'] == $localParams['yk_counter'] && $otpParams['yk_use'] > $localParams['yk_use'])