From 6cc547f791e644a5b297a006010b3accbae20a24 Mon Sep 17 00:00:00 2001 From: Olov Danielson Date: Tue, 12 Jan 2010 13:00:28 +0000 Subject: [PATCH] Remove ID column from yubikeys and queue table. Renamed and changed random_key to server_nonce --- tests/DbTest.php | 36 +++++++++++++++++++ tests/syncLibTest.php | 55 +++++++++++++++++++++++++++-- ykval-db.php | 33 ++++++++++++++---- ykval-db.sql | 9 ++--- ykval-synclib.php | 81 +++++++++++++++++++------------------------ ykval-verify.php | 1 + 6 files changed, 154 insertions(+), 61 deletions(-) diff --git a/tests/DbTest.php b/tests/DbTest.php index c1c5d7b..8e7effe 100644 --- a/tests/DbTest.php +++ b/tests/DbTest.php @@ -18,6 +18,7 @@ class DbTest extends PHPUnit_Framework_TestCase $this->db->connect(); $this->db->customQuery("drop table unittest"); $this->db->customQuery("create table unittest (id int,value1 int, value2 int)"); + } public function test_template() { @@ -83,5 +84,40 @@ class DbTest extends PHPUnit_Framework_TestCase 'id'=>1))); $this->assertEquals(1, $this->db->rowCount(), "1 row should have been affected by previous statement"); } + public function testConditionalUpdate() + { + $this->assertTrue($this->db->save('unittest', array('value1'=>100, + 'value2'=>200, + 'id'=>1))); + $condition="(100 > value1 or (100=value1 and 200>value2))"; + $this->assertTrue($this->db->conditionalUpdate('unittest', 1, array('value2'=>201), $condition)); + $this->assertEquals(0, $this->db->rowCount(), "One row should have been affected"); + $condition="(100 > value1 or (100=value1 and 201>value2))"; + $this->assertTrue($this->db->conditionalUpdate('unittest', 1, array('value2'=>201), $condition)); + $this->assertEquals(1, $this->db->rowCount(), "One row should have been affected"); + } + public function testConditionalUpdateBy() + { + $this->assertTrue($this->db->save('unittest', array('value1'=>100, + 'value2'=>200, + 'id'=>1))); + $condition="(100 > value1 or (100=value1 and 201>value2))"; + $this->assertTrue($this->db->conditionalUpdateBy('unittest', 'value1', 100, array('value2'=>201), $condition)); + $this->assertEquals(1, $this->db->rowCount(), "One row should have been affected"); + + + $this->db->customQuery("drop table myunittest"); + $this->db->customQuery("create table myunittest (id int, string1 varchar(10), value1 int)"); + + + $this->assertTrue($this->db->save('myunittest', array('value1'=>100, + 'string1'=>'hej', + 'id'=>1))); + $condition="(101 > value1)"; + $this->assertTrue($this->db->conditionalUpdateBy('myunittest', 'string1', 'hej', array('value1'=>101), $condition)); + $this->assertEquals(1, $this->db->rowCount(), "One row should have been affected"); + + + } } ?> \ No newline at end of file diff --git a/tests/syncLibTest.php b/tests/syncLibTest.php index 5303976..6bf24ad 100644 --- a/tests/syncLibTest.php +++ b/tests/syncLibTest.php @@ -57,7 +57,23 @@ class SyncLibTest extends PHPUnit_Framework_TestCase $this->assertEquals($nr_servers + $queue_length, $sl->getQueueLength()); - $lastSync=$sl->getLast(); + $res=$sl->db->findByMultiple('queue', + array("modified"=>1259585588, + "server_nonce"=>$sl->server_nonce)); + $lastRes=$res[0]; + $info=$sl->otpParamsFromInfoString($lastRes['info']); + $lastSync=array('queued'=>$lastRes['queued'], + 'modified'=>$lastRes['modified'], + 'otp'=>$lastRes['otp'], + 'server'=>$lastRes['server'], + 'nonce'=>$info['nonce'], + 'yk_publicname'=>$info['yk_publicname'], + 'yk_counter'=>$info['yk_counter'], + 'yk_use'=>$info['yk_use'], + 'yk_high'=>$info['yk_high'], + 'yk_low'=>$info['yk_low']); + + $this->assertEquals($lastSync['modified'], 1259585588); $this->assertEquals($lastSync['otp'], "ccccccccccccfrhiutjgfnvgdurgliidceuilikvfhui"); $this->assertEquals($lastSync['yk_publicname'], "cccccccccccc"); @@ -269,12 +285,45 @@ class SyncLibTest extends PHPUnit_Framework_TestCase $this->assertTrue($sl->queue($p1, $p1)); - $res=$sl->getLast(); + + $res=$sl->db->findByMultiple('queue', + array("modified"=>1259585588+1000, + "server_nonce"=>$sl->server_nonce)); + $lastRes=$res[0]; + $info=$sl->otpParamsFromInfoString($lastRes['info']); + $res=array('queued'=>$lastRes['queued'], + 'modified'=>$lastRes['modified'], + 'otp'=>$lastRes['otp'], + 'server'=>$lastRes['server'], + 'nonce'=>$info['nonce'], + 'yk_publicname'=>$info['yk_publicname'], + 'yk_counter'=>$info['yk_counter'], + 'yk_use'=>$info['yk_use'], + 'yk_high'=>$info['yk_high'], + 'yk_low'=>$info['yk_low']); + $this->assertNotNull($res['queued']); $res=$sl->sync(3); $this->assertEquals(1+$start_length, $sl->getQueueLength()); - $res=$sl->getLast(); + + $res=$sl->db->findByMultiple('queue', + array("modified"=>1259585588+1000, + "server_nonce"=>$sl->server_nonce)); + $lastRes=$res[0]; + $info=$sl->otpParamsFromInfoString($lastRes['info']); + $res=array('queued'=>$lastRes['queued'], + 'modified'=>$lastRes['modified'], + 'otp'=>$lastRes['otp'], + 'server'=>$lastRes['server'], + 'nonce'=>$info['nonce'], + 'yk_publicname'=>$info['yk_publicname'], + 'yk_counter'=>$info['yk_counter'], + 'yk_use'=>$info['yk_use'], + 'yk_high'=>$info['yk_high'], + 'yk_low'=>$info['yk_low']); + + $this->assertNull($res['queued']); diff --git a/ykval-db.php b/ykval-db.php index e37b85b..d5b67f3 100644 --- a/ykval-db.php +++ b/ykval-db.php @@ -136,7 +136,7 @@ class Db if($this->dbh) { $this->result = $this->dbh->query($query); if (! $this->result){ - $this->myLog->log(LOG_ERR, 'Database error: ' . print_r($this->dbh->errorInfo(), true)); + $this->myLog->log(LOG_INFO, 'Database error: ' . print_r($this->dbh->errorInfo(), true)); $this->myLog->log(LOG_INFO, 'Query was: ' . $query); return false; } @@ -174,7 +174,7 @@ class Db return true; } - $query = rtrim($query, ",") . " WHERE " . $k . ' = ' . $v; + $query = rtrim($query, ",") . " WHERE " . $k . " = '" . $v . "'"; // Insert UPDATE statement at beginning $query = "UPDATE " . $table . " SET " . $query; @@ -195,18 +195,19 @@ class Db { return $this->updateBy($table, 'id', $id, $values); } - + /** - * function to update row in database + * function to update row in database based on a condition * * @param string $table Database table to update row in - * @param int $id Id on row to update + * @param string $k Column to select row on + * @param string $v Value to select row on * @param array $values Array with key=>values to update * @param string $condition conditional statement * @return boolean True on success, otherwise false. * */ - public function conditional_update($table, $id, $values, $condition) + public function conditionalUpdateBy($table, $k, $v, $values, $condition) { foreach ($values as $key=>$value){ @@ -217,13 +218,30 @@ class Db return true; } - $query = rtrim($query, ",") . " WHERE id = " . $id . " and " . $condition; + $query = rtrim($query, ",") . " WHERE " . $k . " = '" . $v . "' and " . $condition; // Insert UPDATE statement at beginning $query = "UPDATE " . $table . " SET " . $query; $this->myLog->log(LOG_INFO, "query is " . $query); return $this->query($query, false); } + + + /** + * Function to update row in database based on a condition. + * An ID value is passed to select the appropriate column + * + * @param string $table Database table to update row in + * @param int $id Id on row to update + * @param array $values Array with key=>values to update + * @param string $condition conditional statement + * @return boolean True on success, otherwise false. + * + */ + public function conditionalUpdate($table, $id, $values, $condition) + { + return $this->conditionalUpdateBy($table, 'id', $id, $values, $condition); + } /** * function to insert new row in database @@ -311,6 +329,7 @@ or false on failure. if ($rev==1) $query.= " ORDER BY id DESC"; if ($nr!=null) $query.= " LIMIT " . $nr; + $this->myLog->log(LOG_NOTICE, 'query is ' . $query); $result = $this->query($query, true); if (!$result) return false; diff --git a/ykval-db.sql b/ykval-db.sql index 2b806ac..72ca772 100644 --- a/ykval-db.sql +++ b/ykval-db.sql @@ -10,7 +10,6 @@ CREATE TABLE clients ( ); CREATE TABLE yubikeys ( - id INT NOT NULL UNIQUE AUTO_INCREMENT, active BOOLEAN DEFAULT TRUE, created INT NOT NULL, modified INT NOT NULL, @@ -22,16 +21,14 @@ CREATE TABLE yubikeys ( yk_high INT, nonce VARCHAR(32) DEFAULT '', notes VARCHAR(100) DEFAULT '', - PRIMARY KEY (id) + PRIMARY KEY (yk_publicname) ); CREATE TABLE queue ( - id INT NOT NULL UNIQUE AUTO_INCREMENT, queued INT DEFAULT NULL, modified INT DEFAULT NULL, - random_key INT, + server_nonce VARCHAR(32) NOT NULL, otp VARCHAR(100) NOT NULL, server VARCHAR(100) NOT NULL, - info VARCHAR(256) NOT NULL, - PRIMARY KEY (id) + info VARCHAR(256) NOT NULL ); diff --git a/ykval-synclib.php b/ykval-synclib.php index de8c65f..8cffefe 100644 --- a/ykval-synclib.php +++ b/ykval-synclib.php @@ -21,7 +21,8 @@ class SyncLib $baseParams['__YKVAL_DB_PW__'], $baseParams['__YKVAL_DB_OPTIONS__']); $this->isConnected=$this->db->connect(); - $this->random_key=rand(0,1<<16); + $this->server_nonce=md5(uniqid(rand())); + } function isConnected() @@ -49,24 +50,9 @@ class SyncLib if($res->rowCount()>0) return $res->fetch(PDO::FETCH_ASSOC); else return false; } - function getLast() - { - $res=$this->db->last('queue', 1); - $info=$this->otpParamsFromInfoString($res['info']); - return array('queued'=>$res['queued'], - 'modified'=>$res['modified'], - 'otp'=>$res['otp'], - 'server'=>$res['server'], - 'nonce'=>$info['nonce'], - 'yk_publicname'=>$info['yk_publicname'], - 'yk_counter'=>$info['yk_counter'], - 'yk_use'=>$info['yk_use'], - 'yk_high'=>$info['yk_high'], - 'yk_low'=>$info['yk_low']); - } public function getQueueLength() { - return count($this->db->last('queue', null)); + return count($this->db->findBy('queue', null, null, null)); } public function createInfoString($otpParams, $localParams) @@ -111,7 +97,7 @@ class SyncLib 'modified'=>$otpParams['modified'], 'otp'=>$otpParams['otp'], 'server'=>$server, - 'random_key'=>$this->random_key, + 'server_nonce'=>$this->server_nonce, 'info'=>$info))) $res=False; } return $res; @@ -147,11 +133,10 @@ class SyncLib 'yk_counter'=>0, 'yk_use'=>0, 'nonce'=>0)); - $res=$this->db->findBy('yubikeys', 'yk_publicname', $yk_publicname, 1); + $res=$this->db->findBy('yubikeys', 'yk_publicname', $yk_publicname,1); } if ($res) { - $localParams=array('id'=>$res['id'], - 'modified'=>$res['modified'], + $localParams=array('modified'=>$res['modified'], 'otp'=>$res['otp'], 'nonce'=>$res['nonce'], 'active'=>$res['active'], @@ -161,10 +146,10 @@ class SyncLib 'yk_high'=>$res['yk_high'], 'yk_low'=>$res['yk_low']); - $this->log(LOG_NOTICE, "counter found in db ", $localParams); + $this->log(LOG_NOTICE, "yubikey found in db ", $localParams); return $localParams; } else { - $this->log(LOG_NOTICE, 'params for identity ' . $yk_publicname . ' not found in database'); + $this->log(LOG_NOTICE, 'params for yk_publicname ' . $yk_publicname . ' not found in database'); return false; } } @@ -191,27 +176,28 @@ class SyncLib public function updateDbCounters($params) { - $res=$this->db->lastBy('yubikeys', 'yk_publicname', $params['yk_publicname']); - if (isset($res['id'])) { + $res=$this->db->findBy('yubikeys', 'yk_publicname', $params['yk_publicname'], 1); + $this->log(LOG_NOTICE, '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))' ; - if(! $this->db->conditional_update('yubikeys', - $res['id'], - array('modified'=>$params['modified'], - 'yk_counter'=>$params['yk_counter'], - 'yk_use'=>$params['yk_use'], - 'yk_low'=>$params['yk_low'], - 'yk_high'=>$params['yk_high'], - 'nonce'=>$params['nonce']), - $condition)) + if(! $this->db->conditionalUpdateBy('yubikeys', 'yk_publicname', $params['yk_publicname'], + array('modified'=>$params['modified'], + 'yk_counter'=>$params['yk_counter'], + 'yk_use'=>$params['yk_use'], + 'yk_low'=>$params['yk_low'], + 'yk_high'=>$params['yk_high'], + 'nonce'=>$params['nonce']), + $condition)) { $this->log(LOG_CRIT, 'failed to update internal DB with new counters'); return false; - } else { - if ($this->db->rowCount()>0) $this->log(LOG_NOTICE, "updated database ", $params); - else $this->log(LOG_NOTICE, 'database not updated', $params); - return true; - } + } else + { + if ($this->db->rowCount()>0) $this->log(LOG_NOTICE, "updated database ", $params); + else $this->log(LOG_NOTICE, 'database not updated', $params); + return true; + } } else return false; } @@ -241,10 +227,10 @@ class SyncLib $server=$out[1]; $this->log(LOG_DEBUG, "deleting server=" . $server . " modified=" . $this->otpParams['modified'] . - " random_key=" . $this->random_key); + " server_nonce=" . $this->server_nonce); $this->db->deleteByMultiple('queue', array("modified"=>$this->otpParams['modified'], - "random_key"=>$this->random_key, + "server_nonce"=>$this->server_nonce, 'server'=>$server)); } @@ -328,8 +314,13 @@ class SyncLib } /* Deletion */ - $this->log(LOG_NOTICE, 'deleting queue entry with id=' . $entry['id']); - $this->db->deleteByMultiple('queue', array('id'=>$entry['id'])); + $this->log(LOG_NOTICE, 'deleting queue entry with modified=' . $entry['id'] . + ' server_nonce=' . $entry['server_nonce'] . + ' server=' . $entry['server']); + $this->db->deleteByMultiple('queue', + array("modified"=>$entry['modified'], + "server_nonce"=>$entry['server_nonce'], + 'server'=>$entry['nonce'])); } } /* End of loop over each queue entry for a server */ @@ -344,7 +335,7 @@ class SyncLib */ $urls=array(); - $res=$this->db->findByMultiple('queue', array("modified"=>$this->otpParams['modified'], "random_key"=>$this->random_key)); + $res=$this->db->findByMultiple('queue', array("modified"=>$this->otpParams['modified'], "server_nonce"=>$this->server_nonce)); foreach ($res as $row) { $urls[]=$row['server'] . "?otp=" . $row['otp'] . @@ -428,7 +419,7 @@ class SyncLib NULL queued_time for remaining entries in queue, to allow daemon to take care of them as soon as possible. */ - $this->db->updateBy('queue', 'random_key', $this->random_key, + $this->db->updateBy('queue', 'server_nonce', $this->server_nonce, array('queued'=>NULL)); diff --git a/ykval-verify.php b/ykval-verify.php index b2d3a6c..6f35730 100644 --- a/ykval-verify.php +++ b/ykval-verify.php @@ -173,6 +173,7 @@ if ($sync->countersHigherThanOrEqual($localParams, $otpParams)) { /* Valid OTP, update database. */ if(!$sync->updateDbCounters($otpParams)) { + $myLog->log(LOG_CRIT, "Failed to update yubikey counters in database"); sendResp(S_BACKEND_ERROR, $apiKey); exit; }