fix(transparentSessions): improve session hash computation

- Users and Groups are de-duplicated - Users and Groups are sorted - Each id is length prefixed (using an uint32) - Users are separated from Groups by a '|'
TankerHQ · Dec 9, 2022 · e5ec3b2 · e5ec3b2
1 parent 5ad198b
commit e5ec3b2
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 27 deletions.
diff --git a/packages/core/src/DataProtection/DataProtector.ts b/packages/core/src/DataProtection/DataProtector.ts
@@ -6,7 +6,7 @@ import { castData, getDataLength } from '@tanker/types';
 
 import type { Data } from '@tanker/types';
 
-import { _deserializePublicIdentity, _splitProvisionalAndPermanentPublicIdentities, assertTrustchainId } from '../Identity';
+import { _deserializePublicIdentity, _splitProvisionalAndPermanentPublicIdentities, assertTrustchainId, _serializeIdentity } from '../Identity';
 import type { PublicIdentity, PublicProvisionalUser } from '../Identity';
 import type { Client } from '../Network/Client';
 import type LocalUser from '../LocalUser/LocalUser';
@@ -218,8 +218,16 @@ export class DataProtector {
   }
 
   async _getTransparentSession(encryptionOptions: Omit<EncryptionOptions, 'paddingStep'>): Promise<SessionResult> {
+    const shareWithUsers = encryptionOptions.shareWithUsers || [];
+    const shareWithGroups = encryptionOptions.shareWithGroups || [];
+
+    if (encryptionOptions.shareWithSelf) {
+      const selfIdentity = _serializeIdentity(this._handleShareWithSelf([], encryptionOptions.shareWithSelf)[0]!);
+      shareWithUsers.push(selfIdentity);
+    }
+
     return this._sessionManager.getTransparentSession(
-      encryptionOptions,
+      { shareWithUsers, shareWithGroups },
       this._createTransparentSession.bind(this, encryptionOptions),
     );
   }

diff --git a/packages/core/src/TransparentSession/Manager.ts b/packages/core/src/TransparentSession/Manager.ts
@@ -1,6 +1,6 @@
-import { generichash, utils } from '@tanker/crypto';
+import { generichash, number, utils } from '@tanker/crypto';
 import type { b64string } from '@tanker/crypto';
-import type { EncryptionOptions } from '../DataProtection/options';
+import type { SharingOptions } from '../DataProtection/options';
 import { TaskCoalescer } from '../TaskCoalescer';
 import type { TransparentSessionStore, SessionResult } from './SessionStore';
 
@@ -11,14 +11,23 @@ type LookupResult = {
   session: SessionResult;
 };
 
-export const computeRecipientHash = (recipients: EncryptionOptions): Uint8Array => {
-  const recipientList = [
-    ...(recipients.shareWithUsers || []).sort(),
-    ' Users | Groups ',
-    ...(recipients.shareWithGroups || []).sort(),
-    recipients.shareWithSelf ? 'withSelf' : 'withoutSelf',
-  ];
-  return generichash(utils.fromString(recipientList.join('|')));
+const formatIdArray = (ids: Array<string>) => ids.sort()
+  .flatMap(id => [
+    number.toUint32le(id.length),
+    utils.fromString(id),
+  ]);
+
+export const computeRecipientHash = (recipients: Required<SharingOptions>): Uint8Array => {
+  const users = new Set(recipients.shareWithUsers);
+  const groups = new Set(recipients.shareWithGroups);
+
+  const recipientsVector = utils.concatArrays(
+    ...formatIdArray([...users.keys()]),
+    utils.fromString('|'),
+    ...formatIdArray([...groups.keys()]),
+  );
+
+  return generichash(recipientsVector);
 };
 
 export class SessionManager {
@@ -46,7 +55,8 @@ export class SessionManager {
     }),
   );
 
-  async getTransparentSession(recipients: EncryptionOptions, sessionGenerator: SessionGenerator): Promise<SessionResult> {
+  // Precondition: if shareWithSelf was true, the user's public identity must be part of `recipients.shareWithUsers`
+  async getTransparentSession(recipients: Required<SharingOptions>, sessionGenerator: SessionGenerator): Promise<SessionResult> {
     const recipientsHash = utils.toBase64(computeRecipientHash(recipients));
 
     const sessions = await this._keyLookupCoalescer.run(

diff --git a/packages/core/src/TransparentSession/__tests__/TransparentSession.spec.ts b/packages/core/src/TransparentSession/__tests__/TransparentSession.spec.ts
@@ -86,51 +86,72 @@ describe('computeRecipientHash', () => {
 
     for (let index = 0; index < ids.length; index++) {
       expect(
-        computeRecipientHash({ shareWithUsers: ids.slice(index) }),
+        computeRecipientHash({
+          shareWithUsers: ids.slice(index),
+          shareWithGroups: [],
+        }),
       ).to.not.deep.equal(
-        computeRecipientHash({ shareWithGroups: ids.slice(index) }),
+        computeRecipientHash({
+          shareWithUsers: [],
+          shareWithGroups: ids.slice(index),
+        }),
       );
     }
 
     for (let index = 0; index < ids.length; index++) {
       expect(
-        computeRecipientHash({ shareWithUsers: [ids[0]!] }),
+        computeRecipientHash({
+          shareWithUsers: [ids[0]!],
+          shareWithGroups: [],
+        }),
       ).to.not.deep.equal(
-        computeRecipientHash({ shareWithUsers: ids.slice(2) }),
+        computeRecipientHash({
+          shareWithUsers: ids.slice(2),
+          shareWithGroups: [],
+        }),
       );
     }
 
     expect(
-      computeRecipientHash({ shareWithUsers: ids.slice(0, 1) }),
+      computeRecipientHash({
+        shareWithUsers: ids.slice(0, 1),
+        shareWithGroups: [],
+      }),
     ).to.not.deep.equal(
-      computeRecipientHash({ shareWithUsers: ids.slice(1) }),
+      computeRecipientHash({
+        shareWithUsers: ids.slice(1),
+        shareWithGroups: [],
+      }),
     );
   });
 
-  it('ignores order inside arrays', () => {
+  it('ignores order', () => {
     const id1 = utils.toBase64(random(32));
     const id2 = utils.toBase64(random(32));
 
     const hash = computeRecipientHash({
+      shareWithUsers: [],
       shareWithGroups: [id1, id2],
     });
 
     expect(computeRecipientHash({
+      shareWithUsers: [],
       shareWithGroups: [id2, id1],
     })).to.deep.equal(hash);
   });
 
-  it('changes when sharing w/ and w/o self', () => {
-    const groupId = utils.toBase64(random(32));
+  it('ignores duplicates', () => {
+    const id1 = utils.toBase64(random(32));
+    const id2 = utils.toBase64(random(32));
 
     const hash = computeRecipientHash({
-      shareWithGroups: [groupId],
-      shareWithSelf: true,
+      shareWithUsers: [id1, id1],
+      shareWithGroups: [id2, id2],
     });
 
     expect(computeRecipientHash({
-      shareWithGroups: [groupId],
-      shareWithSelf: false,
-    })).to.not.deep.equal(hash);
+      shareWithUsers: [id1],
+      shareWithGroups: [id2],
+    })).to.deep.equal(hash);
   });
 });