Skip to content

Commit b44d2d2

Browse files
muke1908claude
andcommitted
security: harden E2EE, sanitise errors, fix types, and real crypto tests
P1 security fixes: - SEC-1: RSA-wrap AES key before sharing — relay server no longer sees plaintext AES key; shareEncryptedAesKey() encrypts JWK with peer's RSA public key; receiving side decrypts with own RSA private key - SEC-2: asyncHandler now returns generic 'Internal server error' in production and console.error()s the full error server-side only - SEC-3: share-public-key endpoint returns 409 if a key is already registered for (channel, user), preventing key-slot overwrite attacks P2 bug fixes: - BUG-1: request body limit reduced from 10 mb to 64 kb; urlencoded bodies also capped at 64 kb Tests (TEST-1): - Replaced all mocked window.crypto stubs with real Web Crypto API - 6 new tests: RSA keypair generation, encrypt/decrypt round-trip, wrong-key rejection; AES key caching, encrypt/decrypt round-trip, missing-remote-key error - jest.config.js: explicit testEnvironment: 'node' Code quality (CODE-1): - webrtc.ts: replaced every `any` cast with proper interfaces (RTCRtpSenderWithStreams, RTCRtpReceiverWithStreams, RTCPeerConnectionWithInsertableStreams, IceCandidateSignalData); fixed typo appenVideoStreamToDom → appendVideoStreamToDom - socket.ts: typed handler() args and markDelivered() param - cryptoAES.ts: removed spurious `iv as any` cast (Uint8Array is already a valid BufferSource) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent ae57ffe commit b44d2d2

9 files changed

Lines changed: 216 additions & 118 deletions

File tree

app.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@ require("dotenv").config();
1010
const app = express();
1111
app.disable("x-powered-by");
1212
app.use(cors());
13-
app.use(bodyParser.json({ limit: "10mb" }));
13+
app.use(bodyParser.json({ limit: '64kb' }));
14+
app.use(bodyParser.urlencoded({ extended: true, limit: '64kb' }));
1415
app.use((req, res, next) => {
1516
res.header("Access-Control-Allow-Origin", "*");
1617
next();

backend/api/messaging/index.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,10 @@ router.post(
6666
if (!valid) {
6767
return res.sendStatus(404);
6868
}
69-
// TODO: do not store if already exists
69+
const existing = await db.findOneFromDB({ channel, user: sender }, PUBLIC_KEY_COLLECTION);
70+
if (existing) {
71+
return res.status(409).send({ error: "Key already registered for this session" });
72+
}
7073
await db.insertInDb({ aesKey, publicKey, user: sender, channel }, PUBLIC_KEY_COLLECTION);
7174
return res.send({ status: "ok" });
7275
})

backend/middleware/asyncHandler.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import { Request, Response } from 'express';
22
export default (fn) => (req: Request, res: Response, next) => {
33
Promise.resolve(fn(req, res, next)).catch((e) => {
4-
return res.status(500).send({ error: e.message });
4+
console.error(e);
5+
const message = process.env.NODE_ENV === 'production' ? 'Internal server error' : e.message;
6+
return res.status(500).send({ error: message });
57
});
68
};

service/jest.config.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ module.exports = {
44
collectCoverage: true,
55
collectCoverageFrom: ['**/*.ts'],
66
preset: 'ts-jest',
7+
testEnvironment: 'node',
78
transform: {
89
'^.+\\.(ts|tsx)?$': 'ts-jest',
910
},

service/src/crypto.test.ts

Lines changed: 121 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -1,81 +1,126 @@
1-
import { cryptoUtils } from "./cryptoRSA";
2-
3-
describe('cryptoUtils', () => {
4-
const mockBase64String = 'ZW5jcnlwdGVkLXRleHQ='; // decoded = encrypted-text
5-
const subtle = {
6-
// Generates a new key (for symmetric algorithms) or key pair (for public-key algorithms).
7-
// Returns a promise that fulfills with a CryptoKey (for symmetric algorithms) or a CryptoKeyPair (for public-key algorithms).
8-
generateKey: jest.fn().mockResolvedValue({
9-
publicKey: 'public-key',
10-
privateKey: 'private-key',
11-
}),
12-
// Takes as input a CryptoKey object and gives you the key in an external, portable format.
13-
// Returns a promise - If format was jwk, then the promise fulfills with a JSON object containing the key.
14-
exportKey: jest.fn().mockImplementation((type, str) => str),
15-
// Takes as input a key in an external, portable format and gives you a CryptoKey object that you can use in the Web Crypto API.
16-
// Returns a promise that fulfills with the imported key as a CryptoKey object.
17-
importKey: jest.fn().mockImplementation((type, str) => str),
18-
// It takes as its arguments a key to encrypt with, some algorithm-specific parameters, and the data to encrypt (also known as "plaintext").
19-
// Returns a promise that fulfills with an ArrayBuffer containing the "ciphertext".
20-
encrypt: jest.fn().mockImplementation((algorithm, publicKey, encoded) => encoded),
21-
decrypt: jest.fn().mockImplementation((algorithm, privateKey, ciphertext) => ciphertext)
22-
};
23-
24-
// Receives a Uint8Array to encode to base-64
25-
// Returns ASCII string containing the Base64 representation of stringToEncode.
26-
const btoa = jest.fn().mockImplementation(function (str) {
27-
return mockBase64String;
28-
});
29-
// Receives a base-64 encoded string to decode to Uint8Array
30-
const atob = jest.fn().mockImplementation(function (str) {
31-
return "This is another message";
32-
});
33-
34-
let window = {} as any;
35-
window.crypto = {
36-
subtle: subtle
37-
} as any;
38-
39-
window.btoa = btoa;
40-
window.atob = atob;
41-
42-
beforeEach(() => {
43-
global.window = window;
44-
jest.clearAllMocks();
45-
});
46-
47-
describe('generateKeypairs', () => {
48-
it('should generate an object with a private and a public key', async () => {
49-
const keyPair = await cryptoUtils.generateKeypairs();
50-
51-
expect(window.crypto.subtle.exportKey).toHaveBeenCalledWith('jwk', expect.any(String));
52-
expect(keyPair.privateKey).toBe(JSON.stringify('private-key'));
53-
expect(keyPair.publicKey).toBe(JSON.stringify('public-key'));
1+
/**
2+
* Crypto tests using the real Web Crypto API.
3+
*
4+
* Node 19+ exposes `globalThis.crypto.subtle` natively.
5+
* The production code accesses crypto via `window.crypto`, so we alias
6+
* `globalThis.window` to `globalThis` once before all tests so that
7+
* `window.crypto`, `window.btoa`, and `window.atob` resolve correctly
8+
* without any mocking.
9+
*/
10+
11+
import { webcrypto } from 'crypto';
12+
13+
// Polyfill for Node versions < 19 that do not expose globalThis.crypto
14+
if (!globalThis.crypto) {
15+
(globalThis as any).crypto = webcrypto;
16+
}
17+
18+
// cryptoRSA.ts accesses `window.crypto`, `window.btoa`, and `window.atob`.
19+
// In a Node (non-jsdom) environment `window` is undefined, so we point it at
20+
// globalThis which already has btoa/atob (Node 16+) and crypto (Node 19+).
21+
if (typeof window === 'undefined') {
22+
(globalThis as any).window = globalThis;
23+
}
24+
25+
import { cryptoUtils } from './cryptoRSA';
26+
import { AesGcmEncryption } from './cryptoAES';
27+
28+
// ---------------------------------------------------------------------------
29+
// RSA round-trip tests
30+
// ---------------------------------------------------------------------------
31+
describe('cryptoUtils (RSA-OAEP) – real Web Crypto', () => {
32+
it('generateKeypairs() returns non-empty serialised public and private keys', async () => {
33+
const keyPair = await cryptoUtils.generateKeypairs();
34+
35+
expect(typeof keyPair.publicKey).toBe('string');
36+
expect(typeof keyPair.privateKey).toBe('string');
37+
expect(keyPair.publicKey.length).toBeGreaterThan(0);
38+
expect(keyPair.privateKey.length).toBeGreaterThan(0);
39+
40+
// Keys should be valid JSON (JWK format wrapped in JSON.stringify)
41+
expect(() => JSON.parse(keyPair.publicKey)).not.toThrow();
42+
expect(() => JSON.parse(keyPair.privateKey)).not.toThrow();
5443
});
55-
});
5644

57-
describe('encryptMessage', () => {
58-
it('should encrypt plaintext using a public key and return a base64-encoded string', async () => {
59-
const plaintext = 'This is a message';
60-
const { publicKey } = await cryptoUtils.generateKeypairs();
61-
const ciphertext = await cryptoUtils.encryptMessage(plaintext, publicKey);
45+
it('encryptMessage() + decryptMessage() recover the original plaintext', async () => {
46+
const plaintext = 'Hello, end-to-end encryption!';
47+
48+
const { publicKey, privateKey } = await cryptoUtils.generateKeypairs();
6249

63-
expect(window.crypto.subtle.importKey).toHaveBeenCalledWith('jwk', expect.any(String), { name: 'RSA-OAEP', hash: 'SHA-256' }, true, ['encrypt']);
64-
expect(window.btoa).toHaveBeenCalledWith(expect.any(String));
65-
expect(ciphertext).toBe('ZW5jcnlwdGVkLXRleHQ=');
50+
const ciphertext = await cryptoUtils.encryptMessage(plaintext, publicKey);
51+
expect(typeof ciphertext).toBe('string');
52+
expect(ciphertext).not.toBe(plaintext);
53+
54+
const recovered = await cryptoUtils.decryptMessage(ciphertext, privateKey);
55+
expect(recovered).toBe(plaintext);
6656
});
67-
});
68-
69-
describe('decryptMessage', () => {
70-
it('should decrypt a ciphertext using a private key', async () => {
71-
const plaintext = 'This is another message';
72-
const keyPair = await cryptoUtils.generateKeypairs();
73-
const ciphertext = await cryptoUtils.encryptMessage(plaintext, keyPair.publicKey);
74-
const decryptedText = await cryptoUtils.decryptMessage(ciphertext, keyPair.privateKey);
75-
76-
expect(window.crypto.subtle.importKey).toHaveBeenCalledWith('jwk', expect.any(String), { name: 'RSA-OAEP', hash: 'SHA-256' }, true, ['encrypt']);
77-
expect(window.atob).toHaveBeenCalledWith(expect.any(String));
78-
expect(decryptedText).toBe("This is another message");
57+
58+
it('decryptMessage() fails when using the wrong private key', async () => {
59+
const plaintext = 'Secret message';
60+
61+
const { publicKey } = await cryptoUtils.generateKeypairs();
62+
const { privateKey: wrongPrivateKey } = await cryptoUtils.generateKeypairs();
63+
64+
const ciphertext = await cryptoUtils.encryptMessage(plaintext, publicKey);
65+
66+
await expect(
67+
cryptoUtils.decryptMessage(ciphertext, wrongPrivateKey)
68+
).rejects.toThrow();
69+
});
70+
});
71+
72+
// ---------------------------------------------------------------------------
73+
// AES-GCM round-trip tests
74+
// ---------------------------------------------------------------------------
75+
describe('AesGcmEncryption (AES-GCM) – real Web Crypto', () => {
76+
it('int() generates a CryptoKey and returns it on subsequent calls', async () => {
77+
const aes = new AesGcmEncryption();
78+
const key1 = await aes.int();
79+
const key2 = await aes.int();
80+
81+
expect(key1).toBeDefined();
82+
// Second call should return the same cached instance
83+
expect(key1).toBe(key2);
84+
});
85+
86+
it('encryptData() + decryptData() recover the original data', async () => {
87+
const aes = new AesGcmEncryption();
88+
89+
// Initialise the local key (used for encryption)
90+
await aes.int();
91+
92+
// Export the local key and re-import it as the "remote" key so that
93+
// decryptData() (which uses aesKeyRemote) can decrypt what encryptData()
94+
// produced.
95+
const exportedKey = await aes.getRawAesKeyToExport();
96+
await aes.setRemoteAesKey(exportedKey);
97+
98+
const originalText = 'AES test payload 🔒';
99+
const originalData = new TextEncoder().encode(originalText).buffer;
100+
101+
const { encryptedData, iv } = await aes.encryptData(originalData);
102+
103+
expect(encryptedData).toBeInstanceOf(Uint8Array);
104+
expect(iv).toBeInstanceOf(Uint8Array);
105+
expect(iv.length).toBe(12); // AES-GCM IV is 12 bytes
106+
107+
const decryptedBuffer = await aes.decryptData(encryptedData, iv);
108+
const decryptedText = new TextDecoder().decode(decryptedBuffer);
109+
110+
expect(decryptedText).toBe(originalText);
111+
});
112+
113+
it('decryptData() throws when remote key has not been set', async () => {
114+
const aes = new AesGcmEncryption();
115+
await aes.int();
116+
117+
const { encryptedData, iv } = await aes.encryptData(
118+
new TextEncoder().encode('data').buffer
119+
);
120+
121+
// No setRemoteAesKey() call → should throw
122+
await expect(aes.decryptData(encryptedData, iv)).rejects.toThrow(
123+
'Remote AES key not set.'
124+
);
79125
});
80-
});
81-
});
126+
});

service/src/cryptoAES.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ export class AesGcmEncryption {
5959
const encryptedData = await crypto.subtle.encrypt(
6060
{
6161
name: "AES-GCM",
62-
iv: iv as any
62+
iv: iv
6363
},
6464
this.aesKeyLocal, // Symmetric key for encryption
6565
data // The frame data to be encrypted

service/src/sdk.ts

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,13 @@ class ChatE2EE implements IChatE2EE {
7777
this.privateKey = privateKey;
7878
this.publicKey = publicKey;
7979

80-
this.on('on-alice-join', () => {
80+
this.on('on-alice-join', async () => {
8181
evetLogger.log("Receiver connected.");
82-
this.getPublicKey(initLogger);
82+
await this.getPublicKey(initLogger);
83+
// Now that we have the receiver's RSA public key, share AES key encrypted with it
84+
if (this.receiverPublicKey) {
85+
await this.shareEncryptedAesKey();
86+
}
8387
})
8488

8589
this.on("on-alice-disconnect", () => {
@@ -145,11 +149,14 @@ class ChatE2EE implements IChatE2EE {
145149
this.channelId = channelId;
146150
this.userId = userId;
147151

148-
const aesPlain = await this.symEncryption.getRawAesKeyToExport();
149-
150-
await sharePublicKey({ aesKey: aesPlain, publicKey: this.publicKey, sender: this.userId, channelId: this.channelId});
152+
// Share RSA public key (without AES key until we have receiver's RSA public key)
153+
await sharePublicKey({ aesKey: null, publicKey: this.publicKey, sender: this.userId, channelId: this.channelId});
151154
this.socket.joinChat({ publicKey: this.publicKey, userID: this.userId, channelID: this.channelId})
152155
await this.getPublicKey(logger);
156+
// If the receiver's RSA public key is now known, share AES key encrypted with it
157+
if (this.receiverPublicKey) {
158+
await this.shareEncryptedAesKey();
159+
}
153160
return;
154161
}
155162

@@ -255,11 +262,20 @@ class ChatE2EE implements IChatE2EE {
255262
logger.log(`setPublicKey() - ${!!receiverPublicKey?.publicKey}`);
256263
this.receiverPublicKey = receiverPublicKey?.publicKey;
257264
if(receiverPublicKey.aesKey) {
258-
await this.symEncryption.setRemoteAesKey(receiverPublicKey.aesKey)
265+
// AES key is RSA-encrypted ciphertext; decrypt it with our RSA private key
266+
const decryptedAesKeyJwk = await _cryptoUtils.decryptMessage(receiverPublicKey.aesKey, this.privateKey);
267+
await this.symEncryption.setRemoteAesKey(decryptedAesKeyJwk);
259268
}
260269
return;
261270
}
262271

272+
// Encrypt local AES key with receiver's RSA public key and share it
273+
private async shareEncryptedAesKey(): Promise<void> {
274+
const aesKeyJwk = await this.symEncryption.getRawAesKeyToExport();
275+
const encryptedAesKey = await _cryptoUtils.encryptMessage(aesKeyJwk, this.receiverPublicKey);
276+
await sharePublicKey({ aesKey: encryptedAesKey, publicKey: this.publicKey, sender: this.userId, channelId: this.channelId });
277+
}
278+
263279
private createSocketSubcription(): void {
264280
const subscriptionContext = () => this.subscriptions as SubscriptionType;
265281
this.socket = new SocketInstance(subscriptionContext, logger.createChild('Socket'));

service/src/socket/socket.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,14 +52,14 @@ export class SocketInstance {
5252
this.socket.disconnect();
5353
}
5454

55-
private handler(listener: SocketListenerType, args) {
55+
private handler(listener: SocketListenerType, args: unknown[]): void {
5656
const loggerWithCount = this.eventHandlerLogger.count();
5757
loggerWithCount.log(`handler called for ${listener}`);
5858
const callbacks = this.subscriptionContext().get(listener);
5959
callbacks?.forEach(fn => fn(...args));
6060
}
6161

62-
private markDelivered(msg) {
62+
private markDelivered(msg: { channel: string; sender: string; id: string }): void {
6363
this.logger.log(`markDelivered()`);
6464
this.socket.emit('received', { channel: msg.channel, sender: msg.sender, id: msg.id })
6565
}

0 commit comments

Comments
 (0)