replace retrieveUnboundSessionKey with retrieveLatestUnboundSessionKey

relaycorp · Jan 16, 2024 · cb6a5f0 · cb6a5f0
1 parent 487eaf9
commit cb6a5f0
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 55 deletions.
diff --git a/src/lib/keyStores/PrivateKeyStore.spec.ts b/src/lib/keyStores/PrivateKeyStore.spec.ts
@@ -125,61 +125,23 @@ describe('Session keys', () => {
     });
   });
 
-  describe('retrieveUnboundSessionKey', () => {
+  describe('retrieveLatestUnboundSessionKey', () => {
     test('Existing key should be returned', async () => {
       await MOCK_STORE.saveSessionKey(
         sessionKeyPair.privateKey,
         sessionKeyPair.sessionKey.keyId,
         NODE_ID,
       );
 
-      const keySerialized = await MOCK_STORE.retrieveUnboundSessionKey(
-        sessionKeyPair.sessionKey.keyId,
-        NODE_ID,
-      );
+      const keySerialized = await MOCK_STORE.retrieveLatestUnboundSessionKey(NODE_ID);
 
-      expect(await derSerializePrivateKey(keySerialized)).toEqual(
+      expect(await derSerializePrivateKey(keySerialized!!)).toEqual(
         await derSerializePrivateKey(sessionKeyPair.privateKey),
       );
     });
 
-    test('UnknownKeyError should be thrown if key id does not exist', async () => {
-      await expect(
-        MOCK_STORE.retrieveUnboundSessionKey(sessionKeyPair.sessionKey.keyId, NODE_ID),
-      ).rejects.toThrowWithMessage(UnknownKeyError, `Key ${sessionKeyIdHex} does not exist`);
-    });
-
-    test('Key should not be returned if owned by different node', async () => {
-      await MOCK_STORE.saveSessionKey(
-        sessionKeyPair.privateKey,
-        sessionKeyPair.sessionKey.keyId,
-        NODE_ID,
-      );
-
-      await expect(
-        MOCK_STORE.retrieveUnboundSessionKey(sessionKeyPair.sessionKey.keyId, `not-${NODE_ID}`),
-      ).rejects.toThrowWithMessage(UnknownKeyError, 'Key is owned by a different node');
-    });
-
-    test('Subsequent session keys should not be returned', async () => {
-      await MOCK_STORE.saveSessionKey(
-        sessionKeyPair.privateKey,
-        sessionKeyPair.sessionKey.keyId,
-        NODE_ID,
-        PEER_ID,
-      );
-
-      await expect(
-        MOCK_STORE.retrieveUnboundSessionKey(sessionKeyPair.sessionKey.keyId, NODE_ID),
-      ).rejects.toThrowWithMessage(UnknownKeyError, `Key ${sessionKeyIdHex} is bound`);
-    });
-
-    test('Errors should be wrapped', async () => {
-      const store = new MockPrivateKeyStore(false, true);
-
-      await expect(
-        store.retrieveUnboundSessionKey(sessionKeyPair.sessionKey.keyId, NODE_ID),
-      ).rejects.toEqual(new KeyStoreError('Failed to retrieve key: Denied'));
+    test('Null should be returned if node has no unbound keys', async () => {
+      await expect(MOCK_STORE.retrieveLatestUnboundSessionKey(NODE_ID)).resolves.toBeNull();
     });
   });
 

diff --git a/src/lib/keyStores/PrivateKeyStore.ts b/src/lib/keyStores/PrivateKeyStore.ts
@@ -59,23 +59,31 @@ export abstract class PrivateKeyStore {
   }
 
   /**
-   * Return the private component of an initial session key pair.
+   * Return the private component of the latest, unbound session key for the specified `nodeId`.
    *
-   * @param keyId The key pair id (typically the serial number)
    * @param nodeId The id of the node that owns the key
    * @throws UnknownKeyError when the key does not exist
    * @throws PrivateKeyStoreError when the look-up could not be done
    */
-  public async retrieveUnboundSessionKey(keyId: Buffer, nodeId: string): Promise<CryptoKey> {
-    const keyData = await this.retrieveSessionKeyDataOrThrowError(keyId, nodeId);
+  public async retrieveLatestUnboundSessionKey(nodeId: string): Promise<CryptoKey | null> {
+    const keySerialised = await this.retrieveLatestUnboundSessionKeySerialised(nodeId);
 
-    if (keyData.peerId) {
-      throw new UnknownKeyError(`Key ${keyId.toString('hex')} is bound`);
+    if (!keySerialised) {
+      return null;
     }
 
-    return derDeserializeECDHPrivateKey(keyData.keySerialized, 'P-256');
+    return derDeserializeECDHPrivateKey(keySerialised, 'P-256');
   }
 
+  /**
+   * Return the data of the latest, unbound session key for the specified `nodeId`.
+   *
+   * @param nodeId The id of the node that owns the key
+   */
+  protected abstract retrieveLatestUnboundSessionKeySerialised(
+    nodeId: string,
+  ): Promise<Buffer | null>;
+
   /**
    * Retrieve private session key, regardless of whether it's an initial key or not.
    *

diff --git a/src/lib/keyStores/testMocks.ts b/src/lib/keyStores/testMocks.ts
@@ -54,6 +54,15 @@ export class MockPrivateKeyStore extends PrivateKeyStore {
 
     return this.sessionKeys[keyId] ?? null;
   }
+
+  protected override async retrieveLatestUnboundSessionKeySerialised(
+    nodeId: string,
+  ): Promise<Buffer | null> {
+    const keyData = Object.values(this.sessionKeys).find(
+      (data) => data.nodeId === nodeId && !data.peerId,
+    );
+    return keyData?.keySerialized ?? null;
+  }
 }
 
 export class MockPublicKeyStore extends PublicKeyStore {

diff --git a/src/lib/nodes/Node.spec.ts b/src/lib/nodes/Node.spec.ts
@@ -111,11 +111,10 @@ describe('generateSessionKey', () => {
 
     const sessionKey = await node.generateSessionKey();
 
-    await expect(
-      derSerializePublicKey(
-        await KEY_STORES.privateKeyStore.retrieveUnboundSessionKey(sessionKey.keyId, node.id),
-      ),
-    ).resolves.toEqual(await derSerializePublicKey(sessionKey.publicKey));
+    const publicKey = await KEY_STORES.privateKeyStore.retrieveLatestUnboundSessionKey(node.id);
+    await expect(derSerializePublicKey(publicKey!!)).resolves.toEqual(
+      await derSerializePublicKey(sessionKey.publicKey),
+    );
   });
 
   test('Key should be bound to a peer if explicitly set', async () => {