From 6c42366ac72da8ef334533722a173742995ec950 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". --- 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'])