From 3988b860b0150fd9944a634d2b93dbd6706ae957 Mon Sep 17 00:00:00 2001 From: El RIDO Date: Wed, 10 Aug 2016 17:41:46 +0200 Subject: [PATCH] implemented Identicon library as new default for comment icons, made Vizhash an optional alternative, refactored Vizhash and removed string lenghtening --- CHANGELOG.md | 6 ++- cfg/conf.ini.sample | 13 ++--- lib/Configuration.php | 4 +- lib/Model/Comment.php | 30 +++++++---- lib/Persistence/TrafficLimiter.php | 17 +++--- lib/Vizhash16x16.php | 87 ++++++++++++------------------ tst/ModelTest.php | 86 +++++++++++++++++++++++++++-- tst/Vizhash16x16Test.php | 6 +-- 8 files changed, 162 insertions(+), 87 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b2c0bdc..7ebaef21 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ * ADDED: re-introduced URL shortener support (optional), which was removed back in version 0.16 for privacy concerns * ADDED: Preview tab, helpful for writing markdown code or check source code rendering * ADDED: Automatic purging of expired pastes, done on paste creation - * ADDED: Option to disable vizhashs in discussions (will only affect newly created pastes) + * ADDED: Option to disable icons in discussions (will only affect newly created pastes) * ADDED: Composer support * CHANGED: Renamed the ZeroBin fork to PrivateBin * CHANGED: Removed unmaintained RainTPL template engine, replacing the templates with straight forward PHP files @@ -14,7 +14,9 @@ * CHANGED: Switched to GCM instead CCM mode for AES encryption for newly created pastes * CHANGED: Switched to a SHA256 HMAC of the IP in traffic limiter instead of storing it in plain text on the server * CHANGED: Introduced content security policy header to reduce cross site scripting (XSS) risks - * CHANGED: Refactored PHP code to conform to PSR-4 and PSR-2 standards. + * CHANGED: Refactored PHP code to conform to PSR-4 and PSR-2 standards + * CHANGED: Switched to Identicons as the default for comments with nicknames + * CHANGED: Vizhash is now optional and based on (128 byte) SHA512 HMAC instead of (144 bytes) combination of MD5, SHA1 and a reversal of that string * FIXED: Content-type negociation for HTML in certain uncommon browser configurations * FIXED: JavaScript error displayed before page is loaded or during attachment load * FIXED: Don't strip space characters at beginning or end of optional password diff --git a/cfg/conf.ini.sample b/cfg/conf.ini.sample index 96062647..9a8cd938 100644 --- a/cfg/conf.ini.sample +++ b/cfg/conf.ini.sample @@ -53,18 +53,19 @@ languageselection = false ; the pastes encryption key ; urlshortener = "https://shortener.example.com/api?link=" -; (optional) vizhash is a weak mechanism to detect if a comment was from a -; different user when the same username was used in a comment. It is based on -; the IP and might be used to get the posters IP if the server salt is leaked -; and a rainbow table is generated for all IPs. Enabled by default. -; vizhash = false +; (optional) IP based icons are a weak mechanism to detect if a comment was from +; a different user when the same username was used in a comment. It might be +; used to get the IP of a non anonymous comment poster if the server salt is +; leaked and a SHA256 HMAC rainbow table is generated for all (relevant) IPs. +; Can be set to one these values: none / vizhash / identicon (default). +; icon = none ; Content Security Policy headers allow a website to restrict what sources are ; allowed to be accessed in its context. You need to change this if you added ; custom scripts from third-party domains to your templates, e.g. tracking ; scripts or run your site behind certain DDoS-protection services. ; Check the documentation at https://content-security-policy.com/ -cspheader = "default-src 'none'; connect-src *; script-src 'self'; style-src 'self'; font-src 'self'; img-src 'self';" +cspheader = "default-src 'none'; connect-src *; script-src 'self'; style-src 'self'; font-src 'self'; img-src 'self' data:;" ; stay compatible with PrivateBin Alpha 0.19, less secure ; if enabled will use base64.js version 1.7 instead of 2.1.9 and sha1 instead of diff --git a/lib/Configuration.php b/lib/Configuration.php index 44a35d63..9d979eb8 100644 --- a/lib/Configuration.php +++ b/lib/Configuration.php @@ -50,8 +50,8 @@ class Configuration 'languageselection' => false, 'languagedefault' => '', 'urlshortener' => '', - 'vizhash' => true, - 'cspheader' => 'default-src \'none\'; connect-src *; script-src \'self\'; style-src \'self\'; font-src \'self\'; img-src \'self\';', + 'icon' => 'identicon', + 'cspheader' => 'default-src \'none\'; connect-src *; script-src \'self\'; style-src \'self\'; font-src \'self\'; img-src \'self\' data:;', 'zerobincompatibility' => false, ), 'expire' => array( diff --git a/lib/Model/Comment.php b/lib/Model/Comment.php index 44d346f0..600e61e4 100644 --- a/lib/Model/Comment.php +++ b/lib/Model/Comment.php @@ -15,6 +15,7 @@ namespace PrivateBin\Model; use PrivateBin\Sjcl; use PrivateBin\Persistence\TrafficLimiter; use PrivateBin\Vizhash16x16; +use Identicon\Identicon; use Exception; /** @@ -192,17 +193,26 @@ class Comment extends AbstractModel } $this->_data->meta->nickname = $nickname; - if ($this->_conf->getKey('vizhash')) { - // Generation of the anonymous avatar (Vizhash): - // If a nickname is provided, we generate a Vizhash. - // (We assume that if the user did not enter a nickname, he/she wants - // to be anonymous and we will not generate the vizhash.) - $vh = new Vizhash16x16(); - $pngdata = $vh->generate(TrafficLimiter::getIp()); - if ($pngdata != '') { - $this->_data->meta->vizhash = 'data:image/png;base64,' . base64_encode($pngdata); + // If a nickname is provided, we generate an icon based on a SHA512 HMAC + // of the users IP. (We assume that if the user did not enter a nickname, + // the user wants to be anonymous and we will not generate an icon.) + $icon = $this->_conf->getKey('icon'); + if ($icon != 'none') { + $pngdata = ''; + $hmac = TrafficLimiter::getHash(); + if ($icon == 'identicon') { + $identicon = new Identicon(); + $pngdata = $identicon->getImageDataUri($hmac, 16); + } elseif ($icon == 'vizhash') { + $vh = new Vizhash16x16(); + $pngdata = 'data:image/png;base64,' . base64_encode( + $vh->generate($hmac) + ); + } + if ($pngdata != '') { + $this->_data->meta->vizhash = $pngdata; } - // Once the avatar is generated, we do not keep the IP address, nor its hash. } + // Once the icon is generated, we do not keep the IP address hash. } } diff --git a/lib/Persistence/TrafficLimiter.php b/lib/Persistence/TrafficLimiter.php index 36c7025b..ee4efd0a 100644 --- a/lib/Persistence/TrafficLimiter.php +++ b/lib/Persistence/TrafficLimiter.php @@ -73,15 +73,16 @@ class TrafficLimiter extends AbstractPersistence } /** - * get the current visitors IP address + * get a HMAC of the current visitors IP address * * @access public * @static + * @param string $algo * @return string */ - public static function getIp() + public static function getHash($algo = 'sha512') { - return $_SERVER[self::$_ipKey]; + return hash_hmac($algo, $_SERVER[self::$_ipKey], ServerSalt::get()); } /** @@ -101,8 +102,6 @@ class TrafficLimiter extends AbstractPersistence return true; } - $ip = hash_hmac('sha256', self::getIp(), ServerSalt::get()); - $file = 'traffic_limiter.php'; if (!self::_exists($file)) { self::_store( @@ -117,17 +116,19 @@ class TrafficLimiter extends AbstractPersistence $now = time(); $tl = $GLOBALS['traffic_limiter']; - // purge file of expired IPs to keep it small + // purge file of expired hashes to keep it small foreach ($tl as $key => $time) { if ($time + self::$_limit < $now) { unset($tl[$key]); } } - if (array_key_exists($ip, $tl) && ($tl[$ip] + self::$_limit >= $now)) { + // this hash is used as an array key, hence a shorter hash is used + $hash = self::getHash('sha256'); + if (array_key_exists($hash, $tl) && ($tl[$hash] + self::$_limit >= $now)) { $result = false; } else { - $tl[$ip] = time(); + $tl[$hash] = time(); $result = true; } self::_store( diff --git a/lib/Vizhash16x16.php b/lib/Vizhash16x16.php index 53f03464..8428975a 100644 --- a/lib/Vizhash16x16.php +++ b/lib/Vizhash16x16.php @@ -13,8 +13,6 @@ namespace PrivateBin; -use PrivateBin\Persistence\ServerSalt; - /** * Vizhash16x16 * @@ -60,14 +58,6 @@ class Vizhash16x16 */ private $height; - /** - * salt used when generating the image - * - * @access private - * @var string - */ - private $salt; - /** * constructor * @@ -78,12 +68,13 @@ class Vizhash16x16 { $this->width = 16; $this->height = 16; - $this->salt = ServerSalt::get(); } /** * Generate a 16x16 png corresponding to $text. * + * The given text should to be 128 to 150 characters long + * * @access public * @param string $text * @return string PNG data. Or empty string if GD is not available. @@ -94,44 +85,35 @@ class Vizhash16x16 return ''; } - // We hash the input string. - $hash=hash('sha1', $text.$this->salt).hash('md5', $text.$this->salt); - $hash=$hash.strrev($hash); # more data to make graphics - $hashlen=strlen($hash); + $textlen=strlen($text); // We convert the hash into an array of integers. - $this->VALUES=array(); - for ($i=0; $i<$hashlen; $i=$i+2) { - array_push($this->VALUES, hexdec(substr($hash, $i, 2))); + $this->VALUES = array(); + for ($i = 0; $i < $textlen; $i = $i + 2) { + array_push($this->VALUES, hexdec(substr($text, $i, 2))); } - $this->VALUES_INDEX=0; // to walk the array. + $this->VALUES_INDEX = 0; // to walk the array. // Then use these integers to drive the creation of an image. $image = imagecreatetruecolor($this->width, $this->height); - $r0 = $this->getInt(); - $r=$r0; - $g0 = $this->getInt(); - $g=$g0; - $b0 = $this->getInt(); - $b=$b0; + $r = $r0 = $this->getInt(); + $g = $g0 = $this->getInt(); + $b = $b0 = $this->getInt(); // First, create an image with a specific gradient background. - $op='v'; - if (($this->getInt()%2)==0) { - $op='h'; + $op = 'v'; + if (($this->getInt() % 2) == 0) { + $op = 'h'; }; $image = $this->degrade($image, $op, array($r0, $g0, $b0), array(0, 0, 0)); - for ($i=0; $i<7; $i=$i+1) { - $action=$this->getInt(); + for ($i = 0; $i < 7; ++$i) { + $action = $this->getInt(); $color = imagecolorallocate($image, $r, $g, $b); - $r = ($r0 + $this->getInt()/25)%256; - $g = ($g0 + $this->getInt()/25)%256; - $b = ($b0 + $this->getInt()/25)%256; - $r0=$r; - $g0=$g; - $b0=$b; + $r = $r0 = ($r0 + $this->getInt() / 25) % 256; + $g = $g0 = ($g0 + $this->getInt() / 25) % 256; + $b = $b0 = ($b0 + $this->getInt() / 25) % 256; $this->drawshape($image, $action, $color); } @@ -154,8 +136,8 @@ class Vizhash16x16 */ private function getInt() { - $v= $this->VALUES[$this->VALUES_INDEX]; - $this->VALUES_INDEX++; + $v = $this->VALUES[$this->VALUES_INDEX]; + ++$this->VALUES_INDEX; $this->VALUES_INDEX %= count($this->VALUES); // Warp around the array return $v; } @@ -168,7 +150,7 @@ class Vizhash16x16 */ private function getX() { - return $this->width*$this->getInt()/256; + return $this->width * $this->getInt() / 256; } /** @@ -179,7 +161,7 @@ class Vizhash16x16 */ private function getY() { - return $this->height*$this->getInt()/256; + return $this->height * $this->getInt() / 256; } /** @@ -197,7 +179,7 @@ class Vizhash16x16 */ private function degrade($img, $direction, $color1, $color2) { - if ($direction=='h') { + if ($direction == 'h') { $size = imagesx($img); $sizeinv = imagesy($img); } else { @@ -205,15 +187,15 @@ class Vizhash16x16 $sizeinv = imagesx($img); } $diffs = array( - (($color2[0]-$color1[0])/$size), - (($color2[1]-$color1[1])/$size), - (($color2[2]-$color1[2])/$size) - ); - for ($i=0;$i<$size;$i++) { - $r = $color1[0]+($diffs[0]*$i); - $g = $color1[1]+($diffs[1]*$i); - $b = $color1[2]+($diffs[2]*$i); - if ($direction=='h') { + (($color2[0] - $color1[0]) / $size), + (($color2[1] - $color1[1]) / $size), + (($color2[2] - $color1[2]) / $size) + ); + for ($i = 0; $i < $size; ++$i) { + $r = $color1[0] + ($diffs[0] * $i); + $g = $color1[1] + ($diffs[1] * $i); + $b = $color1[2] + ($diffs[2] * $i); + if ($direction == 'h') { imageline($img, $i, 0, $i, $sizeinv, imagecolorallocate($img, $r, $g, $b)); } else { imageline($img, 0, $i, $sizeinv, $i, imagecolorallocate($img, $r, $g, $b)); @@ -233,7 +215,7 @@ class Vizhash16x16 */ private function drawshape($image, $action, $color) { - switch ($action%7) { + switch ($action % 7) { case 0: ImageFilledRectangle($image, $this->getX(), $this->getY(), $this->getX(), $this->getY(), $color); break; @@ -246,7 +228,8 @@ class Vizhash16x16 ImageFilledPolygon($image, $points, 4, $color); break; default: - $start=$this->getInt()*360/256; $end=$start+$this->getInt()*180/256; + $start = $this->getInt() * 360 /256; + $end = $start + $this->getInt() * 180 / 256; ImageFilledArc($image, $this->getX(), $this->getY(), $this->getX(), $this->getY(), $start, $end, $color, IMG_ARC_PIE); } } diff --git a/tst/ModelTest.php b/tst/ModelTest.php index 62136904..9ddfd172 100644 --- a/tst/ModelTest.php +++ b/tst/ModelTest.php @@ -5,7 +5,9 @@ use PrivateBin\Data\Database; use PrivateBin\Model; use PrivateBin\Model\Paste; use PrivateBin\Persistence\ServerSalt; +use PrivateBin\Persistence\TrafficLimiter; use PrivateBin\Vizhash16x16; +use Identicon\Identicon; class ModelTest extends PHPUnit_Framework_TestCase { @@ -167,13 +169,13 @@ class ModelTest extends PHPUnit_Framework_TestCase $paste->setOpendiscussion(); $paste->store(); - $vz = new Vizhash16x16(); - $pngdata = 'data:image/png;base64,' . base64_encode($vz->generate($_SERVER['REMOTE_ADDR'])); $comment = $paste->getComment(Helper::getPasteId()); $comment->setData($commentData['data']); $comment->setNickname($commentData['meta']['nickname']); $comment->store(); + $identicon = new Identicon(); + $pngdata = $identicon->getImageDataUri(TrafficLimiter::getHash(), 16); $comment = $paste->getComment(Helper::getPasteId(), Helper::getCommentId())->get(); $this->assertEquals($pngdata, $comment->meta->vizhash, 'nickname triggers vizhash to be set'); } @@ -260,9 +262,9 @@ class ModelTest extends PHPUnit_Framework_TestCase public function testCommentWithDisabledVizhash() { $options = parse_ini_file(CONF, true); - $options['main']['vizhash'] = false; + $options['main']['icon'] = 'none'; $options['model'] = array( - 'class' => 'privatebin_db', + 'class' => 'Database', ); $options['model_options'] = array( 'dsn' => 'sqlite::memory:', @@ -311,4 +313,80 @@ class ModelTest extends PHPUnit_Framework_TestCase $this->assertEquals($commentData['meta']['nickname'], $comment->meta->nickname); $this->assertFalse(property_exists($comment->meta, 'vizhash'), 'vizhash was not generated'); } + + public function testCommentIdenticon() + { + $options = parse_ini_file(CONF, true); + $options['main']['icon'] = 'identicon'; + $options['model'] = array( + 'class' => 'Database', + ); + $options['model_options'] = array( + 'dsn' => 'sqlite::memory:', + 'usr' => null, + 'pwd' => null, + 'opt' => array(PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION), + ); + Helper::confBackup(); + Helper::createIniFile(CONF, $options); + $model = new Model(new Configuration); + + $pasteData = Helper::getPaste(); + $commentData = Helper::getComment(); + $model->getPaste(Helper::getPasteId())->delete(); + + $paste = $model->getPaste(); + $paste->setData($pasteData['data']); + $paste->setOpendiscussion(); + $paste->setFormatter($pasteData['meta']['formatter']); + $paste->store(); + + $comment = $paste->getComment(Helper::getPasteId()); + $comment->setData($commentData['data']); + $comment->setNickname($commentData['meta']['nickname']); + $comment->store(); + + $identicon = new Identicon(); + $pngdata = $identicon->getImageDataUri(TrafficLimiter::getHash(), 16); + $comment = $paste->getComment(Helper::getPasteId(), Helper::getCommentId())->get(); + $this->assertEquals($pngdata, $comment->meta->vizhash, 'nickname triggers vizhash to be set'); + } + + public function testCommentVizhash() + { + $options = parse_ini_file(CONF, true); + $options['main']['icon'] = 'vizhash'; + $options['model'] = array( + 'class' => 'Database', + ); + $options['model_options'] = array( + 'dsn' => 'sqlite::memory:', + 'usr' => null, + 'pwd' => null, + 'opt' => array(PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION), + ); + Helper::confBackup(); + Helper::createIniFile(CONF, $options); + $model = new Model(new Configuration); + + $pasteData = Helper::getPaste(); + $commentData = Helper::getComment(); + $model->getPaste(Helper::getPasteId())->delete(); + + $paste = $model->getPaste(); + $paste->setData($pasteData['data']); + $paste->setOpendiscussion(); + $paste->setFormatter($pasteData['meta']['formatter']); + $paste->store(); + + $comment = $paste->getComment(Helper::getPasteId()); + $comment->setData($commentData['data']); + $comment->setNickname($commentData['meta']['nickname']); + $comment->store(); + + $vz = new Vizhash16x16(); + $pngdata = 'data:image/png;base64,' . base64_encode($vz->generate(TrafficLimiter::getHash())); + $comment = $paste->getComment(Helper::getPasteId(), Helper::getCommentId())->get(); + $this->assertEquals($pngdata, $comment->meta->vizhash, 'nickname triggers vizhash to be set'); + } } diff --git a/tst/Vizhash16x16Test.php b/tst/Vizhash16x16Test.php index fbab0eca..76d2f95a 100644 --- a/tst/Vizhash16x16Test.php +++ b/tst/Vizhash16x16Test.php @@ -30,11 +30,11 @@ class Vizhash16x16Test extends PHPUnit_Framework_TestCase public function testVizhashGeneratesUniquePngsPerIp() { $vz = new Vizhash16x16(); - $pngdata = $vz->generate('127.0.0.1'); + $pngdata = $vz->generate(hash('sha512', '127.0.0.1')); file_put_contents($this->_file, $pngdata); $finfo = new finfo(FILEINFO_MIME_TYPE); $this->assertEquals('image/png', $finfo->file($this->_file)); - $this->assertNotEquals($pngdata, $vz->generate('2001:1620:2057:dead:beef::cafe:babe')); - $this->assertEquals($pngdata, $vz->generate('127.0.0.1')); + $this->assertNotEquals($pngdata, $vz->generate(hash('sha512', '2001:1620:2057:dead:beef::cafe:babe'))); + $this->assertEquals($pngdata, $vz->generate(hash('sha512', '127.0.0.1'))); } }