Bladeren bron

sending challenge on paste creation, adding logic to store and check it on view requests

El RIDO 7 jaren geleden
bovenliggende
commit
79db7ddafc
13 gewijzigde bestanden met toevoegingen van 247 en 14 verwijderingen
  1. 3 0
      js/pastemeta.jsonld
  2. 7 2
      js/privatebin.js
  3. 3 0
      js/types.jsonld
  4. 14 5
      lib/Controller.php
  5. 7 0
      lib/FormatV2.php
  6. 63 3
      lib/Model/Paste.php
  7. 1 1
      tpl/bootstrap.php
  8. 1 1
      tpl/page.php
  9. 1 1
      tst/Bootstrap.php
  10. 74 0
      tst/ControllerTest.php
  11. 4 0
      tst/FormatV2Test.php
  12. 53 1
      tst/ModelTest.php
  13. 16 0
      tst/RequestTest.php

+ 3 - 0
js/pastemeta.jsonld

@@ -26,6 +26,9 @@
 		},
 		"time_to_live": {
 			"@type": "pb:RemainingSeconds"
+		},
+		"challenge": {
+			"@type": "pb:Challenge"
 		}
 	}
 }

+ 7 - 2
js/privatebin.js

@@ -4229,7 +4229,9 @@ jQuery.PrivateBin = (function($, RawDeflate) {
 
             // prepare server interaction
             ServerInteraction.prepare();
-            ServerInteraction.setCryptParameters(TopNav.getPassword());
+            const key = CryptTool.getSymmetricKey(),
+                  password = TopNav.getPassword();
+            ServerInteraction.setCryptParameters(password, key);
 
             // set success/fail functions
             ServerInteraction.setSuccess(showCreatedPaste);
@@ -4250,7 +4252,10 @@ jQuery.PrivateBin = (function($, RawDeflate) {
                 TopNav.getOpenDiscussion() ? 1 : 0,
                 TopNav.getBurnAfterReading() ? 1 : 0
             ]);
-            ServerInteraction.setUnencryptedData('meta', {'expire': TopNav.getExpiration()});
+            ServerInteraction.setUnencryptedData('meta', {
+                'expire':    TopNav.getExpiration(),
+                'challenge': CryptTool.getCredentials(key, password)
+            });
 
             // prepare PasteViewer for later preview
             PasteViewer.setText(plainText);

+ 3 - 0
js/types.jsonld

@@ -92,6 +92,9 @@
 		"@type": "dp:Second",
 		"@minimum": 1
 	},
+	"Challenge": {
+		"@type": "pb:Base64"
+	},
 	"CipherParameters": {
 		"@container": "@list",
 		"@value": [

+ 14 - 5
lib/Controller.php

@@ -276,9 +276,7 @@ class Controller
                 // accessing this method ensures that the paste would be
                 // deleted if it has already expired
                 $paste->get();
-                if (
-                    Filter::slowEquals($deletetoken, $paste->getDeleteToken())
-                ) {
+                if ($paste->isDeleteTokenCorrect($deletetoken)) {
                     // Paste exists and deletion token is valid: Delete the paste.
                     $paste->delete();
                     $this->_status = 'Paste was properly deleted.';
@@ -315,9 +313,20 @@ class Controller
         try {
             $paste = $this->_model->getPaste($dataid);
             if ($paste->exists()) {
+                // handle challenge response
+                if (!$paste->isTokenCorrect($this->_request->getParam('token'))) {
+                    // we send a generic error to avoid leaking information
+                    // about the existance of a burn after reading pastes
+                    // this avoids an attacker being able to poll, if it has
+                    // been read by the intended recipient or not
+                    $this->_return_message(1, self::GENERIC_ERROR);
+                    return;
+                }
                 $data = $paste->get();
-                if (array_key_exists('salt', $data['meta'])) {
-                    unset($data['meta']['salt']);
+                foreach (array('salt', 'challenge') as $key) {
+                    if (array_key_exists($key, $data['meta'])) {
+                        unset($data['meta'][$key]);
+                    }
                 }
                 $this->_return_message(0, $dataid, (array) $data);
             } else {

+ 7 - 0
lib/FormatV2.php

@@ -67,6 +67,13 @@ class FormatV2
         if (!($ct = base64_decode($message['ct'], true))) {
             return false;
         }
+        // - (optional) challenge
+        if (
+            !$isComment && array_key_exists('challenge', $message['meta']) &&
+            !base64_decode($message['meta']['challenge'], true)
+        ) {
+            return false;
+        }
 
         // Make sure some fields have a reasonable size:
         // - initialization vector

+ 63 - 3
lib/Model/Paste.php

@@ -14,6 +14,7 @@ namespace PrivateBin\Model;
 
 use Exception;
 use PrivateBin\Controller;
+use PrivateBin\Filter;
 use PrivateBin\Persistence\ServerSalt;
 
 /**
@@ -23,6 +24,14 @@ use PrivateBin\Persistence\ServerSalt;
  */
 class Paste extends AbstractModel
 {
+    /**
+     * Token for challenge/response.
+     *
+     * @access protected
+     * @var string
+     */
+    protected $_token = '';
+
     /**
      * Get paste data.
      *
@@ -32,6 +41,11 @@ class Paste extends AbstractModel
      */
     public function get()
     {
+        // return cached result if one is found
+        if (array_key_exists('adata', $this->_data) || array_key_exists('data', $this->_data)) {
+            return $this->_data;
+        }
+
         $data = $this->_store->read($this->getId());
         if ($data === false) {
             throw new Exception(Controller::GENERIC_ERROR, 64);
@@ -48,10 +62,16 @@ class Paste extends AbstractModel
             unset($data['meta']['expire_date']);
         }
 
-        // check if non-expired burn after reading paste needs to be deleted
+        // check if non-expired burn after reading paste needs to be deleted,
+        // but don't delete it if an incorrect token was sent
         if (
-            (array_key_exists('adata', $data) && $data['adata'][3] === 1) ||
-            (array_key_exists('burnafterreading', $data['meta']) && $data['meta']['burnafterreading'])
+            (
+                (array_key_exists('adata', $data) && $data['adata'][3] === 1) ||
+                (array_key_exists('burnafterreading', $data['meta']) && $data['meta']['burnafterreading'])
+            ) && (
+                !array_key_exists('challenge', $data['meta']) ||
+                $this->_token === $data['meta']['challenge']
+            )
         ) {
             $this->delete();
         }
@@ -94,6 +114,12 @@ class Paste extends AbstractModel
 
         $this->_data['meta']['created'] = time();
         $this->_data['meta']['salt']    = serversalt::generate();
+        // if a challenge was sent, we store the HMAC of paste ID & challenge
+        if (array_key_exists('challenge', $this->_data['meta'])) {
+            $this->_data['meta']['challenge'] = hash_hmac(
+                'sha256', $this->getId(), base64_decode($this->_data['meta']['challenge'])
+            );
+        }
 
         // store paste
         if (
@@ -201,6 +227,40 @@ class Paste extends AbstractModel
             (array_key_exists('opendiscussion', $this->_data['meta']) && $this->_data['meta']['opendiscussion']);
     }
 
+    /**
+     * Check if paste challenge matches provided token.
+     *
+     * @access public
+     * @param  string $token
+     * @throws Exception
+     * @return bool
+     */
+    public function isTokenCorrect($token)
+    {
+        $this->_token = $token;
+        if (!array_key_exists('challenge', $this->_data['meta'])) {
+            $this->get();
+        }
+        if (array_key_exists('challenge', $this->_data['meta'])) {
+            return Filter::slowEquals($token, $this->_data['meta']['challenge']);
+        }
+        // paste created without challenge, accept every token sent
+        return true;
+    }
+
+    /**
+     * Check if paste salt based HMAC matches provided delete token.
+     *
+     * @access public
+     * @param  string $deletetoken
+     * @throws Exception
+     * @return bool
+     */
+    public function isDeleteTokenCorrect($deletetoken)
+    {
+        return Filter::slowEquals($deletetoken, $this->getDeleteToken());
+    }
+
     /**
      * Sanitizes data to conform with current configuration.
      *

+ 1 - 1
tpl/bootstrap.php

@@ -71,7 +71,7 @@ if ($MARKDOWN):
 endif;
 ?>
 		<script type="text/javascript" data-cfasync="false" src="js/purify-1.0.11.js" integrity="sha512-p7UyJuyBkhMcMgE4mDsgK0Lz70OvetLefua1oXs1OujWv9gOxh4xy8InFux7bZ4/DAZsTmO4rgVwZW9BHKaTaw==" crossorigin="anonymous"></script>
-		<script type="text/javascript" data-cfasync="false" src="js/privatebin.js?<?php echo rawurlencode($VERSION); ?>" integrity="sha512-JTOr7OLbpnUePy6e/3tj28v3L4Cf1Xvoo1WQVRmJbBAcBK2skOsz4vBkC0RbeRkaObPIYXuv5egrFYseABbuZA==" crossorigin="anonymous"></script>
+		<script type="text/javascript" data-cfasync="false" src="js/privatebin.js?<?php echo rawurlencode($VERSION); ?>" integrity="sha512-tQGC7Jwx3TVRu69OGEmWl0K6lwDmgh5of3xU1Xbo4U0EY1Sv9qyUMQaOgGnMET3WMu9kbuZHsPnf7d9tTXhbsQ==" crossorigin="anonymous"></script>
 		<!--[if IE]>
 		<style type="text/css">body {padding-left:60px;padding-right:60px;} #ienotice {display:block;}</style>
 		<![endif]-->

+ 1 - 1
tpl/page.php

@@ -49,7 +49,7 @@ if ($MARKDOWN):
 endif;
 ?>
 		<script type="text/javascript" data-cfasync="false" src="js/purify-1.0.11.js" integrity="sha512-p7UyJuyBkhMcMgE4mDsgK0Lz70OvetLefua1oXs1OujWv9gOxh4xy8InFux7bZ4/DAZsTmO4rgVwZW9BHKaTaw==" crossorigin="anonymous"></script>
-		<script type="text/javascript" data-cfasync="false" src="js/privatebin.js?<?php echo rawurlencode($VERSION); ?>" integrity="sha512-JTOr7OLbpnUePy6e/3tj28v3L4Cf1Xvoo1WQVRmJbBAcBK2skOsz4vBkC0RbeRkaObPIYXuv5egrFYseABbuZA==" crossorigin="anonymous"></script>
+		<script type="text/javascript" data-cfasync="false" src="js/privatebin.js?<?php echo rawurlencode($VERSION); ?>" integrity="sha512-tQGC7Jwx3TVRu69OGEmWl0K6lwDmgh5of3xU1Xbo4U0EY1Sv9qyUMQaOgGnMET3WMu9kbuZHsPnf7d9tTXhbsQ==" crossorigin="anonymous"></script>
 		<!--[if IE]>
 		<style type="text/css">body {padding-left:60px;padding-right:60px;} #ienotice {display:block;}</style>
 		<![endif]-->

+ 1 - 1
tst/Bootstrap.php

@@ -155,7 +155,7 @@ class Helper
     public static function getPastePost($version = 2, array $meta = array())
     {
         $example         = self::getPaste($version, $meta);
-        $example['meta'] = array('expire' => $example['meta']['expire']);
+        $example['meta'] = array_merge(array('expire' => $example['meta']['expire']), $meta);
         return $example;
     }
 

+ 74 - 0
tst/ControllerTest.php

@@ -366,6 +366,30 @@ class ControllerTest extends PHPUnit_Framework_TestCase
         $this->assertEquals(1, $paste['adata'][2], 'discussion is enabled');
     }
 
+    /**
+     * @runInSeparateProcess
+     */
+    public function testCreateInvalidFormat()
+    {
+        $options                     = parse_ini_file(CONF, true);
+        $options['traffic']['limit'] = 0;
+        Helper::createIniFile(CONF, $options);
+        $paste = Helper::getPasteJson(2, array('challenge' => '$'));
+        $file  = tempnam(sys_get_temp_dir(), 'FOO');
+        file_put_contents($file, $paste);
+        Request::setInputStream($file);
+        $_SERVER['HTTP_X_REQUESTED_WITH'] = 'JSONHttpRequest';
+        $_SERVER['REQUEST_METHOD']        = 'POST';
+        $_SERVER['REMOTE_ADDR']           = '::1';
+        ob_start();
+        new Controller;
+        $content = ob_get_contents();
+        ob_end_clean();
+        $response = json_decode($content, true);
+        $this->assertEquals(1, $response['status'], 'outputs error status');
+        $this->assertFalse($this->_data->exists(Helper::getPasteId()), 'paste exists after posting data');
+    }
+
     /**
      * @runInSeparateProcess
      */
@@ -784,6 +808,56 @@ class ControllerTest extends PHPUnit_Framework_TestCase
         $this->assertFalse($this->_data->exists(Helper::getPasteId()), 'paste successfully deleted');
     }
 
+    /**
+     * @runInSeparateProcess
+     */
+    public function testReadBurnAfterReadingWithToken()
+    {
+        $token = base64_encode(hash_hmac(
+            'sha256', Helper::getPasteId(), random_bytes(32)
+        ));
+        $burnPaste = Helper::getPaste(2, array('challenge' => $token));
+        $burnPaste['adata'][3] = 1;
+        $this->_data->create(Helper::getPasteId(), $burnPaste);
+        $this->assertTrue($this->_data->exists(Helper::getPasteId()), 'paste exists before deleting data');
+        $_SERVER['QUERY_STRING']          = Helper::getPasteId() . '&token=' . $token;
+        $_GET[Helper::getPasteId()]       = '';
+        $_GET['token']                    = $token;
+        $_SERVER['HTTP_X_REQUESTED_WITH'] = 'JSONHttpRequest';
+        ob_start();
+        new Controller;
+        $content = ob_get_contents();
+        ob_end_clean();
+        $response = json_decode($content, true);
+        $this->assertEquals(0, $response['status'], 'outputs status');
+        $this->assertFalse($this->_data->exists(Helper::getPasteId()), 'paste successfully deleted');
+    }
+
+    /**
+     * @runInSeparateProcess
+     */
+    public function testReadBurnAfterReadingWithIncorrectToken()
+    {
+        $token = base64_encode(hash_hmac(
+            'sha256', Helper::getPasteId(), random_bytes(32)
+        ));
+        $burnPaste = Helper::getPaste(2, array('challenge' => base64_encode(random_bytes(32))));
+        $burnPaste['adata'][3] = 1;
+        $this->_data->create(Helper::getPasteId(), $burnPaste);
+        $this->assertTrue($this->_data->exists(Helper::getPasteId()), 'paste exists before deleting data');
+        $_SERVER['QUERY_STRING']          = Helper::getPasteId() . '&token=' . $token;
+        $_GET[Helper::getPasteId()]       = '';
+        $_GET['token']                    = $token;
+        $_SERVER['HTTP_X_REQUESTED_WITH'] = 'JSONHttpRequest';
+        ob_start();
+        new Controller;
+        $content = ob_get_contents();
+        ob_end_clean();
+        $response = json_decode($content, true);
+        $this->assertEquals(1, $response['status'], 'outputs status');
+        $this->assertTrue($this->_data->exists(Helper::getPasteId()), 'paste not deleted');
+    }
+
     /**
      * @runInSeparateProcess
      */

+ 4 - 0
tst/FormatV2Test.php

@@ -21,6 +21,10 @@ class FormatV2Test extends PHPUnit_Framework_TestCase
         $paste['ct'] = '$';
         $this->assertFalse(FormatV2::isValid($paste), 'invalid base64 encoding of ct');
 
+        $paste                      = Helper::getPastePost();
+        $paste['meta']['challenge'] = '$';
+        $this->assertFalse(FormatV2::isValid($paste), 'invalid base64 encoding of ct');
+
         $paste       = Helper::getPastePost();
         $paste['ct'] = 'bm9kYXRhbm9kYXRhbm9kYXRhbm9kYXRhbm9kYXRhCg==';
         $this->assertFalse(FormatV2::isValid($paste), 'low ct entropy');

+ 53 - 1
tst/ModelTest.php

@@ -268,10 +268,48 @@ class ModelTest extends PHPUnit_Framework_TestCase
         $paste->setData($pasteData);
         $paste->store();
 
-        $paste = $paste->get();
+        $paste = $this->_model->getPaste(Helper::getPasteId())->get();
         $this->assertEquals((float) 300, (float) $paste['meta']['time_to_live'], 'remaining time is set correctly', 1.0);
     }
 
+    public function testToken()
+    {
+        $pasteData = Helper::getPastePost();
+        $pasteData['meta']['challenge'] = base64_encode(random_bytes(32));
+        $token = hash_hmac(
+            'sha256', Helper::getPasteId(), base64_decode($pasteData['meta']['challenge'])
+        );
+        $this->_model->getPaste(Helper::getPasteId())->delete();
+        $paste = $this->_model->getPaste(Helper::getPasteId());
+        $this->assertFalse($paste->exists(), 'paste does not yet exist');
+
+        $paste = $this->_model->getPaste();
+        $paste->setData($pasteData);
+        $paste->store();
+
+        $paste = $this->_model->getPaste(Helper::getPasteId());
+        $this->assertTrue(
+            $paste->isTokenCorrect($token),
+            'token is accepted after store and retrieval'
+        );
+    }
+
+    public function testDeleteToken()
+    {
+        $pasteData = Helper::getPastePost();
+        $this->_model->getPaste(Helper::getPasteId())->delete();
+        $paste = $this->_model->getPaste(Helper::getPasteId());
+        $this->assertFalse($paste->exists(), 'paste does not yet exist');
+
+        $paste = $this->_model->getPaste();
+        $paste->setData($pasteData);
+        $paste->store();
+        $deletetoken = $paste->getDeleteToken();
+
+        $paste = $this->_model->getPaste(Helper::getPasteId());
+        $this->assertTrue($paste->isDeleteTokenCorrect($deletetoken), 'delete token is accepted after store and retrieval');
+    }
+
     /**
      * @expectedException Exception
      * @expectedExceptionCode 64
@@ -287,6 +325,20 @@ class ModelTest extends PHPUnit_Framework_TestCase
         $paste->getComment(Helper::getPasteId())->delete();
     }
 
+    /**
+     * @expectedException Exception
+     * @expectedExceptionCode 75
+     */
+    public function testInvalidFormat()
+    {
+        $pasteData = Helper::getPastePost();
+        $pasteData['adata'][1] = 'foo';
+        $this->_model->getPaste(Helper::getPasteId())->delete();
+
+        $paste = $this->_model->getPaste();
+        $paste->setData($pasteData);
+    }
+
     public function testPurge()
     {
         $conf  = new Configuration;

+ 16 - 0
tst/RequestTest.php

@@ -130,6 +130,22 @@ class RequestTest extends PHPUnit_Framework_TestCase
         $this->assertEquals('read', $request->getOperation());
     }
 
+    public function testApiReadWithToken()
+    {
+        $this->reset();
+        $id                        = $this->getRandomId();
+        $_SERVER['REQUEST_METHOD'] = 'GET';
+        $_SERVER['HTTP_ACCEPT']    = 'application/json, text/javascript, */*; q=0.01';
+        $_SERVER['QUERY_STRING']   = $id . '&token=foo';
+        $_GET[$id]                 = '';
+        $_GET['token']             = 'foo';
+        $request                   = new Request;
+        $this->assertTrue($request->isJsonApiCall(), 'is JSON Api call');
+        $this->assertEquals($id, $request->getParam('pasteid'));
+        $this->assertEquals('foo', $request->getParam('token'));
+        $this->assertEquals('read', $request->getOperation());
+    }
+
     public function testApiDelete()
     {
         $this->reset();