feat: authenticate document and parent IDs of comments

- client appends the document and parent IDs to the cipher parameters
  in the comment adata, so they are covered by the AEAD tag
- client skips comments whose authenticated IDs don't match the
  document or their parent; comments without them are still shown
- FormatV2 accepts comment adata with 8 elements, or 10 if the last two
  equal the unencrypted pasteid and parentid
- new option discussionrequireids (default false) rejects new comments
  without authenticated IDs (77)
- document the extended comment adata in the JSON-LD types
- tests for the format validation, the option and the IDs being
  authenticated by the cipher
- SRI hash, sample configuration and CHANGELOG updated

CHANGELOG.md

@@ -1,6 +1,7 @@
 # PrivateBin version history
 
 ## 2.1.1 (not yet released)
+* ADDED: Comments authenticate the IDs of their document and parent comment by default. Older comments without these IDs are still accepted and shown, unless the new `discussionrequireids` option is enabled
 * FIXED: Creating a document failed with a JavaScript error when the `discussion`, `password`, e-mail or QR code option is disabled, as the top navigation accessed the omitted elements
 * ADDED: Translation for Korean (#2071)
 * FIXED: Slovenian plural forms were stored in the wrong order, i.e. "1 uri" instead of "1 ura" for one hour (#1951)

cfg/conf.sample.php

@@ -23,6 +23,11 @@ opendiscussion = false
 ; the comments by creation time, but you can choose not to display them.
 ; discussiondatedisplay = false
 
+; require new comments to authenticate the IDs of their document and parent
+; comment, defaults to false. Comments sent by clients that don't, are rejected.
+; This prevents copies of comments from getting moved elsewhere in the discussion.
+; discussionrequireids = true
+
 ; enable or disable the password feature, defaults to true
 password = true
 

js/comment.jsonld

@@ -26,7 +26,7 @@
 			"@type": "pb:CipherText"
 		},
 		"adata": {
-			"@type": "pb:CipherParameters"
+			"@type": "cm:AuthenticatedData"
 		},
 		"meta": {
 			"@type": "cm:MetaData"

js/commentmeta.jsonld

@@ -3,6 +3,43 @@
 		"so": "https://schema.org/",
 		"pb": "?jsonld=types#"
 	},
+	"AuthenticatedData": {
+		"@container": "@list",
+		"@value": [
+			{
+				"@type": "pb:InitializationVector"
+			},
+			{
+				"@type": "pb:Salt"
+			},
+			{
+				"@type": "pb:Iterations"
+			},
+			{
+				"@type": "pb:KeySize"
+			},
+			{
+				"@type": "pb:TagSize"
+			},
+			{
+				"@type": "pb:Algorithm"
+			},
+			{
+				"@type": "pb:Mode"
+			},
+			{
+				"@type": "pb:Compression"
+			},
+			{
+				"@type": "pb:DocumentId",
+				"@optional": true
+			},
+			{
+				"@type": "pb:ParentId",
+				"@optional": true
+			}
+		]
+	},
 	"MetaData": {
 		"created": {
 			"@type": "CreationTime"

js/privatebin.js

@@ -229,6 +229,25 @@ window.PrivateBin = (function () {
         this.getIcon = function () {
             return this.meta.icon || '';
         };
+
+        /**
+         * checks if the comment belongs to the given document and its parent
+         *
+         * Comments without authenticated IDs predate this check and are
+         * accepted.
+         *
+         * @name Comment.isBoundTo
+         * @function
+         * @param  {string} pasteId
+         * @return {bool}
+         */
+        this.isBoundTo = function (pasteId) {
+            return this.adata.length === 8 || (
+                this.adata.length === 10 &&
+                this.adata[8] === pasteId &&
+                this.adata[9] === this.parentid
+            );
+        };
     }
 
     /**
@@ -1235,12 +1254,12 @@ window.PrivateBin = (function () {
             for (let i = 0; i < spec.length; ++i) {
                 encodedSpec[i] = i < 2 ? btoa(spec[i]) : spec[i];
             }
-            if (adata.length === 0) {
-                // comment
-                adata = encodedSpec;
-            } else if (adata[0] === null) {
+            if (adata[0] === null) {
                 // paste
                 adata[0] = encodedSpec;
+            } else {
+                // comment, optionally followed by its document and parent IDs
+                adata = encodedSpec.concat(adata);
             }
 
             // finally, encrypt message
@@ -5279,14 +5298,16 @@ window.PrivateBin = (function () {
             });
 
             // fill it with unencrypted params
-            ServerInteraction.setUnencryptedData('pasteid', Model.getPasteId());
+            const pasteid = Model.getPasteId();
             if (typeof parentid === 'undefined') {
                 // if parent id is not set, this is the top-most comment, so use
                 // document id as parent, as the root element of the discussion tree
-                ServerInteraction.setUnencryptedData('parentid', Model.getPasteId());
-            } else {
-                ServerInteraction.setUnencryptedData('parentid', parentid);
+                parentid = pasteid;
             }
+            ServerInteraction.setUnencryptedData('pasteid', pasteid);
+            ServerInteraction.setUnencryptedData('parentid', parentid);
+            // authenticate the IDs, so the comment can't be moved elsewhere
+            ServerInteraction.setUnencryptedData('adata', [pasteid, parentid]);
 
             // prepare cypher message
             let cipherMessage = {
@@ -5542,11 +5563,15 @@ window.PrivateBin = (function () {
             const commentDecryptionPromises = [];
             // iterate over comments
             for (let i = 0; i < paste.comments.length; ++i) {
-                const comment = new Comment(paste.comments[i]),
-                    commentPromise = CryptTool.decipher(key, password, comment.getCipherData());
+                const comment = new Comment(paste.comments[i]);
                 paste.comments[i] = comment;
+                // skip comments that were moved from another document or parent
+                if (!comment.isBoundTo(Model.getPasteId())) {
+                    commentDecryptionPromises.push(Promise.resolve(['', '']));
+                    continue;
+                }
                 commentDecryptionPromises.push(
-                    commentPromise.then(function (commentJson) {
+                    CryptTool.decipher(key, password, comment.getCipherData()).then(function (commentJson) {
                         const commentMessage = JSON.parse(commentJson);
                         return [
                             commentMessage.comment || '',

js/test/CryptTool.js

@@ -70,6 +70,35 @@ describe('CryptTool', function () {
             assert.strictEqual(message, plaintext);
         });
 
+        it('authenticates the document and parent IDs of comments', async function () {
+            const clean = globalThis.cleanup();
+            Object.defineProperty(window, 'crypto', {
+                value: new WebCrypto(),
+                configurable: true,
+                enumerable: true,
+                writable: false
+            });
+            // ensure zlib is getting loaded
+            PrivateBin.Controller.initZlib();
+            global.atob = common.atob;
+            global.btoa = common.btoa;
+            const cipherMessage = await PrivateBin.CryptTool.cipher(
+                    'foo', 'bar', 'baz', ['5b65a01b43987bc2', '33a5671d11b29d01']
+                ),
+                plaintext = await PrivateBin.CryptTool.decipher(
+                    'foo', 'bar', cipherMessage
+                );
+            cipherMessage[1][9] = '1111111111111111';
+            const tampered = await PrivateBin.CryptTool.decipher(
+                'foo', 'bar', cipherMessage
+            );
+            clean();
+            assert.strictEqual(cipherMessage[1].length, 10);
+            assert.strictEqual(cipherMessage[1][8], '5b65a01b43987bc2');
+            assert.strictEqual(plaintext, 'baz');
+            assert.strictEqual(tampered, '');
+        });
+
         it('can en- and decrypt a particular message (#260)', async function () {
             await fc.assert(fc.asyncProperty(
                 fc.string(),

js/types.jsonld

@@ -92,6 +92,12 @@
 		"@type": "dp:Second",
 		"@minimum": 1
 	},
+	"DocumentId": {
+		"@type": "so:name"
+	},
+	"ParentId": {
+		"@type": "so:name"
+	},
 	"CipherParameters": {
 		"@container": "@list",
 		"@value": [

lib/Configuration.php

@@ -40,6 +40,7 @@ class Configuration
             'discussion'               => true,
             'opendiscussion'           => false,
             'discussiondatedisplay'    => true,
+            'discussionrequireids'     => false,
             'password'                 => true,
             'fileupload'               => false,
             'burnafterreadingselected' => false,
@@ -118,7 +119,7 @@ class Configuration
             'js/kjua-0.10.0.js'      => 'sha512-BYj4xggowR7QD150VLSTRlzH62YPfhpIM+b/1EUEr7RQpdWAGKulxWnOvjFx1FUlba4m6ihpNYuQab51H6XlYg==',
             'js/legacy.js'           => 'sha512-pRofxsrf5UItjiP22Dcjh3FAcBjF/n7h8U9/W5xqJk17U0N2U1oajhXypq/omo9jhwS1iVGOhWrRepoPeFns+w==',
             'js/prettify.js'         => 'sha512-puO0Ogy++IoA2Pb9IjSxV1n4+kQkKXYAEUtVzfZpQepyDPyXk8hokiYDS7ybMogYlyyEIwMLpZqVhCkARQWLMg==',
-            'js/privatebin.js'       => 'sha512-jXV3mKXtezHb3zaJ/Ijhj4EVT9IoB/RQGhrVxFE6ID8zAZIE3otvIL3Nf+L7AR78xNNCTm0jLJSvHeNPVxoJIw==',
+            'js/privatebin.js'       => 'sha512-TX8LhsibraqXsqOZtS0gxj06dMPtZxfoZiIZ6GuULrpGhS6/DwotD8HT3lfl3HMrKYbBmjUkK7qA3ZW5IxjMiw==',
             'js/purify-3.4.17.js'    => 'sha512-udF2zPdEyVCAxIGeBbeSGFK1Fj5c4eHGi6NqOSsr8M75lGs39POzgUNxO3SFxOg4cHfAoZJgR2EaXdmDi6v6AQ==',
             'js/showdown-2.1.0.js'   => 'sha512-WYXZgkTR0u/Y9SVIA4nTTOih0kXMEd8RRV6MLFdL6YU8ymhR528NLlYQt1nlJQbYz4EW+ZsS0fx1awhiQJme1Q==',
             'js/zlib-1.3.2.js'       => 'sha512-RAhJgxg9siMIA8ky4c10Rc2zUgnK80olHB8Tt1IOYWY4Eh1WmrviQkDn+sgBlb38ZHq3tzufGC41kP360gmosQ==',

lib/FormatV2.php

@@ -63,6 +63,19 @@ class FormatV2
             return false;
         }
 
+        // Comments may authenticate their document and parent IDs, by
+        // appending them to the cipher parameters. These must match the
+        // unencrypted IDs, which are validated by the comment model.
+        if ($isComment && count($cipherParams) !== 8 && (
+            count($cipherParams) !== 10 ||
+            !is_string($cipherParams[8]) ||
+            !is_string($cipherParams[9]) ||
+            $cipherParams[8] !== $message['pasteid'] ||
+            $cipherParams[9] !== $message['parentid']
+        )) {
+            return false;
+        }
+
         // Make sure the ciphertext and the cipher parameters used in the
         // string operations below are actually strings, so that malformed
         // input yields "Invalid data." instead of a fatal type error.

lib/Model/Comment.php

@@ -138,6 +138,21 @@ class Comment extends AbstractModel
         return $this->_data['parentid'];
     }
 
+    /**
+     * Validate data.
+     *
+     * @access protected
+     * @param  array $data
+     * @throws TranslatedException
+     */
+    protected function _validate(array &$data)
+    {
+        // reject comments without authenticated IDs, if required
+        if ($this->_conf->getKey('discussionrequireids') && count($data['adata']) !== 10) {
+            throw new TranslatedException(self::INVALID_DATA_ERROR, 77);
+        }
+    }
+
     /**
      * Sanitizes data to conform with current configuration.
      *

tst/FormatV2Test.php

@@ -108,5 +108,35 @@ class FormatV2Test extends TestCase
         $comment = Helper::getCommentPost();
         $comment['ct'] = 42;
         $this->assertFalse(FormatV2::isValid($comment, true), 'non-string comment ciphertext');
+
+        $comment = Helper::getCommentPost();
+        $comment['adata'][] = $comment['pasteid'];
+        $comment['adata'][] = $comment['parentid'];
+        $this->assertTrue(FormatV2::isValid($comment, true), 'comment with authenticated IDs');
+
+        $comment = Helper::getCommentPost();
+        $comment['adata'][] = $comment['pasteid'];
+        $this->assertFalse(FormatV2::isValid($comment, true), 'comment with incomplete authenticated IDs');
+
+        $comment = Helper::getCommentPost();
+        $comment['adata'][] = $comment['pasteid'];
+        $comment['adata'][] = $comment['parentid'];
+        $comment['adata'][] = $comment['parentid'];
+        $this->assertFalse(FormatV2::isValid($comment, true), 'comment with too many cipher parameters');
+
+        $comment = Helper::getCommentPost();
+        $comment['adata'][] = '1111111111111111';
+        $comment['adata'][] = $comment['parentid'];
+        $this->assertFalse(FormatV2::isValid($comment, true), 'comment with mismatched document ID');
+
+        $comment = Helper::getCommentPost();
+        $comment['adata'][] = $comment['pasteid'];
+        $comment['adata'][] = '1111111111111111';
+        $this->assertFalse(FormatV2::isValid($comment, true), 'comment with mismatched parent ID');
+
+        $comment = Helper::getCommentPost();
+        $comment['adata'][] = $comment['pasteid'];
+        $comment['adata'][] = [];
+        $this->assertFalse(FormatV2::isValid($comment, true), 'comment with non-string parent ID');
     }
 }

tst/ModelTest.php

@@ -221,6 +221,36 @@ class ModelTest extends TestCase
         $comment->store();
     }
 
+    public function testCommentRequireIds()
+    {
+        $options = parse_ini_file(CONF, true);
+        $options['main']['discussionrequireids'] = 'true';
+        Helper::createIniFile(CONF, $options);
+        $model = new Model(new Configuration);
+
+        $pasteData = Helper::getPastePost();
+        $commentData = Helper::getCommentPost();
+        $model->getPaste(Helper::getPasteId())->delete();
+
+        $paste = $model->getPaste();
+        $paste->setData($pasteData);
+        $paste->store();
+
+        $commentData['adata'][] = $commentData['pasteid'];
+        $commentData['adata'][] = $commentData['parentid'];
+        $comment = $paste->getComment(Helper::getPasteId());
+        $comment->setData($commentData);
+        $comment->store();
+        $this->assertTrue($comment->exists(), 'comment with authenticated IDs can be stored');
+
+        $commentData = Helper::getCommentPost();
+        $commentData['ct'] = base64_encode('other ciphertext');
+        $comment = $paste->getComment(Helper::getPasteId());
+        $this->expectException(Exception::class);
+        $this->expectExceptionCode(77);
+        $comment->setData($commentData);
+    }
+
     public function testCommentDuplicate()
     {
         $pasteData = Helper::getPastePost();