diff --git a/ykval-sync.php b/ykval-sync.php index 71669a4..32086a0 100644 --- a/ykval-sync.php +++ b/ykval-sync.php @@ -67,6 +67,21 @@ foreach ($syncParams as $param=>$value) { } $myLog->log(LOG_INFO, $tmp_log); +# +# Verify correctness of input parameters +# + +foreach (array('modified', 'yk_counter', 'yk_use', 'yk_high', 'yk_low') as $param) { + if (preg_match("/^[0-9]*$/", $syncParams[$param])==0) { + $myLog->log(LOG_NOTICE, 'Input parameters ' . $param . ' not correct'); + sendResp(S_MISSING_PARAMETER, $apiKey); + exit; + } +} + + + + # # Get local counter data # @@ -86,39 +101,44 @@ if ($localParams['active'] != 1) { } -# -# Compare sync and local counters and generate warnings according to -# -# http://code.google.com/p/yubikey-val-server-php/wiki/ServerReplicationProtocol -# - /* Conditional update local database */ $sync->updateDbCounters($syncParams); $myLog->log(LOG_DEBUG, 'Local params ' , $localParams); $myLog->log(LOG_DEBUG, 'Sync request params ' , $syncParams); +# +# Compare sync and local counters and generate warnings according to +# +# http://code.google.com/p/yubikey-val-server-php/wiki/ServerReplicationProtocol +# + + + if ($sync->countersHigherThan($localParams, $syncParams)) { - /* sync counters are lower than local counters */ $myLog->log(LOG_WARNING, 'Remote server out of sync.'); } + if ($sync->countersEqual($localParams, $syncParams)) { - /* sync counters are equal to local counters. */ - if ($syncParams['modified']==$localParams['modified']) { - /* sync modified is equal to local modified. - Sync request is unnessecarily sent, we log a "light" warning */ + + if ($syncParams['modified']==$localParams['modified'] && + $syncParams['nonce']==$localParams['nonce']) { $myLog->log(LOG_NOTICE, 'Sync request unnessecarily sent'); - } else { - /* sync modified is not equal to local modified. - We have an OTP replay attempt somewhere in the system */ - $myLog->log(LOG_WARNING, 'We might have a replay. 2 events at different times have generated the same counters'); } + + if ($syncParams['modified']!=$localParams['modified'] && + $syncParams['nonce']==$localParams['nonce']) { + $deltaModified = $syncParams['modified'] - $localParams['modified']; + $myLog->log(LOG_WARNING, 'We might have a replay. 2 events at different times have generated the same counters. The time difference is ' . $deltaModified . ' seconds'); + } + if ($syncParams['nonce']!=$localParams['nonce']) { - $myLog->log(LOG_WARNING, 'Remote server has received a request to validate an already validated OTP'); + $myLog->log(LOG_WARNING, 'Remote server has received a request to validate an already validated OTP '); } } + $extra=array('modified'=>$localParams['modified'], 'nonce'=>$localParams['nonce'], diff --git a/ykval-synclib.php b/ykval-synclib.php index 05cb0c0..d9b6ed1 100644 --- a/ykval-synclib.php +++ b/ykval-synclib.php @@ -123,7 +123,7 @@ class SyncLib } function getLocalParams($yk_publicname) { - $this->log(LOG_NOTICE, "searching for yk_publicname " . $yk_publicname . " in local db"); + $this->log(LOG_INFO, "searching for yk_publicname " . $yk_publicname . " in local db"); $res = $this->db->findBy('yubikeys', 'yk_publicname', $yk_publicname,1); if (!$res) { @@ -152,7 +152,7 @@ class SyncLib 'yk_high'=>$res['yk_high'], 'yk_low'=>$res['yk_low']); - $this->log(LOG_NOTICE, "yubikey found in db ", $localParams); + $this->log(LOG_INFO, "yubikey found in db ", $localParams); return $localParams; } else { $this->log(LOG_NOTICE, 'params for yk_publicname ' . $yk_publicname . ' not found in database'); @@ -183,7 +183,7 @@ class SyncLib public function updateDbCounters($params) { $res=$this->db->findBy('yubikeys', 'yk_publicname', $params['yk_publicname'], 1); - $this->log(LOG_NOTICE, 'yk_publicname is ' . $res['yk_publicname']); + $this->log(LOG_INFO, 'yk_publicname is ' . $res['yk_publicname']); if (isset($res['yk_publicname'])) { $condition='('.$params['yk_counter'].'>yk_counter or ('.$params['yk_counter'].'=yk_counter and ' . $params['yk_use'] . '>yk_use))' ; @@ -200,8 +200,8 @@ class SyncLib return false; } else { - if ($this->db->rowCount()>0) $this->log(LOG_NOTICE, "updated database ", $params); - else $this->log(LOG_NOTICE, 'database not updated', $params); + if ($this->db->rowCount()>0) $this->log(LOG_INFO, "updated database ", $params); + else $this->log(LOG_INFO, 'database not updated', $params); return true; } } else return false; @@ -231,7 +231,7 @@ class SyncLib preg_match('/url=(.*)\?/', $answer, $out); $server=$out[1]; - $this->log(LOG_DEBUG, "deleting server=" . $server . + $this->log(LOG_INFO, "deleting server=" . $server . " modified=" . $this->otpParams['modified'] . " server_nonce=" . $this->server_nonce); $this->db->deleteByMultiple('queue', @@ -242,11 +242,11 @@ class SyncLib public function reSync($older_than=60, $timeout) { - $this->log(LOG_NOTICE, 'starting resync'); + $this->log(LOG_INFO, 'starting resync'); /* Loop over all unique servers in queue */ $queued_limit=time()-$older_than; $res=$this->db->customQuery("select distinct server from queue WHERE queued < " . $queued_limit . " or queued is null"); - $this->log(LOG_NOTICE, "found " . $res->rowCount() . " unique servers"); + $this->log(LOG_INFO, "found " . $res->rowCount() . " unique servers"); foreach ($res as $my_server) { $this->log(LOG_INFO, "Sending queue request to server on server " . $my_server['server']); @@ -254,7 +254,7 @@ class SyncLib $this->log(LOG_INFO, "found " . $res->rowCount() . " queue entries"); while ($entry=$res->fetch(PDO::FETCH_ASSOC)) { - $this->log(LOG_NOTICE, "server=" . $entry['server'] . " , info=" . $entry['info']); + $this->log(LOG_INFO, "server=" . $entry['server'] . " , info=" . $entry['info']); $url=$entry['server'] . "?otp=" . $entry['otp'] . "&modified=" . $entry['modified'] . @@ -262,7 +262,7 @@ class SyncLib /* Send out sync request */ - $this->log(LOG_NOTICE, 'url is ' . $url); + $this->log(LOG_DEBUG, 'url is ' . $url); $ch = curl_init($url); curl_setopt($ch, CURLOPT_USERAGENT, "YK-VAL"); curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1); @@ -273,7 +273,7 @@ class SyncLib curl_close($ch); if ($response==False) { - $this->log(LOG_WARNING, 'Timeout. Stopping queue resync for server ' . $my_server['server']); + $this->log(LOG_NOTICE, 'Timeout. Stopping queue resync for server ' . $my_server['server']); break; } @@ -286,40 +286,41 @@ class SyncLib /* Retrieve info from entry info string */ - $localParams=$this->localParamsFromInfoString($entry['info']); + $validationParams=$this->localParamsFromInfoString($entry['info']); $otpParams=$this->otpParamsFromInfoString($entry['info']); + $localParams=$this->getLocalParams($otpParams['yk_publicname']); + + $this->log(LOG_DEBUG, "validation params: ", $validationParams); + $this->log(LOG_DEBUG, "OTP params: ", $otpParams); + + /* Check for warnings */ + + if ($this->countersHigherThan($validationParams, $resParams)) { + $this->log(LOG_NOTICE, "Remote server out of sync compared to counters at validation request time. "); + } + + if ($this->countersHigherThan($resParams, $validationParams)) { + $this->log(LOG_NOTICE, "Local server out of sync compared to counters at validation request time. "); + } - /* Check for warnings - - If received sync response have lower counters than locally saved - last counters (indicating that remote server wasn't synced) - */ if ($this->countersHigherThan($localParams, $resParams)) { - $this->log(LOG_WARNING, "queued:Remote server out of sync, local counters ", $localParams); - $this->log(LOG_WARNING, "queued:Remote server out of sync, remote counters ", $resParams); + $this->log(LOG_WARNING, "Remote server out of sync compared to current local counters. "); } - /* If received sync response have higher counters than locally saved - last counters (indicating that local server wasn't synced) - */ if ($this->countersHigherThan($resParams, $localParams)) { - $this->log(LOG_WARNING, "queued:Local server out of sync, local counters ", $localParams); - $this->log(LOG_WARNING, "queued:Local server out of sync, remote counters ", $resParams); + $this->log(LOG_WARNING, "Local server out of sync compared to current local counters. Local server updated. "); } - if ($this->countersHigherThan($resParams, $otpParams) || - ($this->countersEqual($resParams, $otpParams) && - $resParams['nonce']!=$otpParams['nonce'])) { - - /* If received sync response have higher counters than OTP or same counters with different nonce - (indicating REPLAYED_OTP) - */ - - $this->log(LOG_WARNING, "queued:Remote server has higher or equal counters than OTP. This response would have marked the OTP as invalid. "); - } + if ($this->countersHigherThan($resParams, $otpParams)) { + $this->log(LOG_ERR, "Remote server has higher counters than OTP. This response would have marked the OTP as invalid. "); + } + elseif ($this->countersEqual($resParams, $otpParams) && + $resParams['nonce']!=$otpParams['nonce']) { + $this->log(LOG_ERR, "Remote server has equal counters as OTP and nonce differs. This response would have marked the OTP as invalid."); + } /* Deletion */ - $this->log(LOG_NOTICE, 'deleting queue entry with modified=' . $entry['modified'] . + $this->log(LOG_INFO, 'deleting queue entry with modified=' . $entry['modified'] . ' server_nonce=' . $entry['server_nonce'] . ' server=' . $entry['server']); $this->db->deleteByMultiple('queue', @@ -378,39 +379,47 @@ class SyncLib /* Check for warnings + + See http://code.google.com/p/yubikey-val-server-php/wiki/ServerReplicationProtocol - If received sync response have lower counters than local db - (indicating that remote server wasn't synced) + NOTE: We use localParams for validationParams comparison since they are actually the + same in this situation and we have them at hand. */ + if ($this->countersHigherThan($localParams, $resParams)) { - $this->log(LOG_WARNING, "Remote server out of sync"); + $this->log(LOG_NOTICE, "Remote server out of sync"); } - /* If received sync response have higher counters than local db - (indicating that local server wasn't synced) - */ if ($this->countersHigherThan($resParams, $localParams)) { - $this->log(LOG_WARNING, "Local server out of sync"); + $this->log(LOG_NOTICE, "Local server out of sync"); } - if ($this->countersHigherThan($resParams, $this->otpParams) || - ($this->countersEqual($resParams, $this->otpParams) && - $resParams['nonce']!=$this->otpParams['nonce'])) { + if ($this->CountersEqual($resParams, $localParams) && + $resParams['nonce']!=$localParams['nonce']) { + $this->log(LOG_NOTICE, "Servers out of sync. Nonce differs. "); + } - /* If received sync response have higher counters than OTP or same counters with different nonce - (indicating REPLAYED_OTP) - */ - - $this->log(LOG_WARNING, "Replayed OTP"); + + if ($this->CountersEqual($resParams, $localParams) && + $resParams['modified']!=$localParams['modified']) { + $this->log(LOG_NOTICE, "Servers out of sync. Modified differs. "); + } + + if ($this->countersHigherThan($resParams, $this->otpParams)){ + $this->log(LOG_WARNING, 'OTP is replayed. Sync response counters higher than OTP counters.'); + } + elseif ($this->countersEqual($resParams, $this->otpParams) && + $resParams['nonce']!=$this->otpParams['nonce']) { + $this->log(LOG_WARNING, 'OTP is replayed. Sync response counters equal to OTP counters and nonce differs.'); } else { - + /* The answer is ok since a REPLAY was not indicated */ $this->valid_answers++; } - + /* Delete entry from table */ $this->deleteQueueEntry($answer); @@ -460,7 +469,7 @@ class SyncLib $ch = array(); foreach ($urls as $id => $url) { - $this->log(LOG_INFO, "url in retrieveURLasync is " . $url); + $this->log(LOG_DEBUG, "url in retrieveURLasync is " . $url); $handle = curl_init(); curl_setopt($handle, CURLOPT_URL, $url); diff --git a/ykval-verify.php b/ykval-verify.php index f87a297..09e417c 100644 --- a/ykval-verify.php +++ b/ykval-verify.php @@ -41,7 +41,7 @@ if ($protocol_version>=2.0) { $nonce = getHttpVal('nonce', ''); /* Nonce is required from protocol 2.0 */ - if(!$nonce || strlen($nonce)<16) { + if(!$nonce || strlen($nonce)<16 || strlen($nonce)>32) { $myLog->log(LOG_NOTICE, 'Protocol version >= 2.0. Nonce is missing'); sendResp(S_MISSING_PARAMETER, $apiKey); exit; @@ -55,8 +55,17 @@ if ($protocol_version<2.0) { } -/* Sanity check HTTP parameters */ -/* id, sl, timestamp, timeout, nonce, timestamp */ +/* Sanity check HTTP parameters + + * otp: one-time password + * id: client id + * timeout: timeout in seconds to wait for external answers, optional: if absent the server decides + * nonce: random alphanumeric string, 8 to 32 bytes characters long. Must be non-predictable and changing for each request, but need not be cryptographically strong + * sl: "sync level", percentage of external servers that needs to answer (integer 0 to 100), or "fast" or "secure" to use server-configured values + * h: signature (optional) + * timestamp: requests timestamp/counters in response + + */ if (preg_match("/^[0-9]*$/", $client)==0){ $myLog->log(LOG_NOTICE, 'id provided in request must be an integer'); @@ -64,6 +73,32 @@ if (preg_match("/^[0-9]*$/", $client)==0){ exit; } +if (preg_match("/^[0-9]*$/", $timeout)==0) { + $myLog->log(LOG_NOTICE, 'timeout is provided but not correct'); + sendResp(S_MISSING_PARAMETER, $apiKey); + exit; + } + +if (preg_match("/^[A-Za-z0-9]*$/", $nonce)==0) { + $myLog->log(LOG_NOTICE, 'NONCE is provided but not correct'); + sendResp(S_MISSING_PARAMETER, $apiKey); + exit; + + } + +if (preg_match("/^[0-9]*$/", $sl)==0 || ($sl<0 || $sl>100)) { + $myLog->log(LOG_NOTICE, 'SL is provided but not correct'); + sendResp(S_MISSING_PARAMETER, $apiKey); + exit; + } + +// NOTE: Timestamp parameter is not checked since current protocol says that 1 means request timestamp +// and anything else is discarded. + + + + + //// Get Client info from DB // if ($client <= 0) {