implemented Identicon library as new default for comment icons, made Vizhash an optional alternative, refactored Vizhash and removed string lenghtening

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

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

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(

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 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 = 'data:image/png;base64,' . base64_encode($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.
     }
 }

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(

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);
         }
     }

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');
+    }
 }

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')));
     }
 }