From 6767b41b9749e2f0ed638f2efddea95a7880add2 Mon Sep 17 00:00:00 2001 From: UNITRONIX <36471318+UNITRONIX@users.noreply.github.com> Date: Wed, 29 Jul 2026 20:10:01 +0200 Subject: [PATCH] fix(web-remote): harden SignedId key verify after #313 (Refs #313) Reload id_ed25519.pub per viewer render and verify RelayResponse.pk then SignedId with the correct RustDesk key chain so base64 Keys work; desktop signature-mismatch without Key remains expected config. --- CHANGELOG.md | 1 + web-nodejs/public/js/rdclient/client.js | 41 ++++-- web-nodejs/public/js/rdclient/crypto.js | 105 ++++++++++++--- .../public/js/rdclient/file-connection.js | 18 ++- web-nodejs/routes/remote.routes.js | 20 +-- web-nodejs/tests/rdclient.crypto.test.js | 126 ++++++++++++++++++ 6 files changed, 274 insertions(+), 37 deletions(-) create mode 100644 web-nodejs/tests/rdclient.crypto.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 15c94a57..12e54759 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - **Client Configuration public key masked by default (#319):** Dashboard and Keys page show the KEY as bullets until revealed via an eye toggle; Copy still pastes the raw key. Ships via panel update. ### Fixed +- **Web Remote SignedId MITM check + stale server key (#313):** viewer reads `id_ed25519.pub` on each page render (no empty cache if Go starts later); RdClient accepts base64 Key (and hex) and verifies `RelayResponse.pk` with the server key then `SignedId` with the peer identity key (RustDesk chain). Does not change the 3.4.2 panel-proxy allowlist. Ships via panel update. Note: desktop client `Failed to secure tcp: Signature mismatch in key exchange` when the Key field is empty/wrong is expected client config — set Key from Keys page / `id_ed25519.pub`. - **Cannot delete seed `admin` with false UI success (#315):** panel delete now checks Go Super Admin parity on dual-SQLite, mirrors delete before local removal, and returns 409/502 instead of success when Go refuses last-admin (no silent backfill restore). Last–Super Admin guard and installer `reset-password.js` / menu reset (username `admin`) are unchanged. Ships via panel update. - **Native install TLS self-signed deploy (#325):** `_safe_cp_tls_file` no longer deletes `betterdesk.crt` when source and dest are the same real file (self-signed generated in place). Symlink→copy for Let's Encrypt (#219) is unchanged. Ships via installer / `betterdesk.sh` (not panel-only). Verify: `install.sh --native` completes past “Generating self-signed TLS certificates” with both `/opt/betterdesk/ssl/betterdesk.crt` and `.key` present. - **Enrollment Requests UI (#320):** search icon no longer overlaps the placeholder; row dividers stay continuous under Platform/Actions (`display:flex` moved off ``); Platform/Version/Status/Requested/Actions columns centered. Ships via panel update. diff --git a/web-nodejs/public/js/rdclient/client.js b/web-nodejs/public/js/rdclient/client.js index 023435df..a1a5a3fe 100644 --- a/web-nodejs/public/js/rdclient/client.js +++ b/web-nodejs/public/js/rdclient/client.js @@ -893,19 +893,40 @@ class RDClient { .map(b => b.toString(16).padStart(2, '0')).join(''); this._debugRelay(`[RDClient] Peer ephemeral pk: ${parsed.peerPk.length} bytes [${peerPkHex}...]`); - // Verify Ed25519 signature against server public key (MITM protection) + // MITM chain (matches RustDesk decode_id_pk): + // RelayResponse.pk = IdPk{id, identityPk} signed by server → verify with server Key + // SignedId = IdPk{id, ephemeralBoxPk} signed by peer identity → verify with identityPk const serverPubKey = this.opts.serverPubKey || ''; - if (serverPubKey && serverPubKey.length >= 64) { - const verified = RDCrypto.verifySignedId(parsed.signature, parsed.payload, serverPubKey); - parsed.signatureVerified = verified; - if (verified) { - this._debugRelay('[RDClient] Ed25519 signature VERIFIED — peer identity authenticated'); - this._emit('log', 'Peer identity verified (Ed25519)'); + if (RDCrypto.hasDecodablePublicKey(serverPubKey) && this._peerSignedPk && this._peerSignedPk.length) { + const peerIdentity = RDCrypto.verifyAndDecodeIdPk( + this._peerSignedPk instanceof Uint8Array + ? this._peerSignedPk + : new Uint8Array(this._peerSignedPk), + serverPubKey, + this.proto.types.IdPk + ); + if (!peerIdentity) { + console.warn('[RDClient] RelayResponse.pk failed server-key verification'); + this._emit('signature_warning', 'Server-signed peer identity could not be verified.'); + this._emit('log', 'WARNING: Peer identity (RelayResponse.pk) verification failed'); } else { - console.warn('[RDClient] Ed25519 signature FAILED — possible MITM attack!'); - this._emit('signature_warning', 'Ed25519 signature verification failed. Connection may be intercepted.'); - this._emit('log', 'WARNING: Peer signature verification failed'); + const verified = RDCrypto.verifySignedId( + parsed.signature, + parsed.payload, + peerIdentity.peerPk + ); + parsed.signatureVerified = verified; + if (verified) { + this._debugRelay('[RDClient] Ed25519 signature VERIFIED — peer identity authenticated'); + this._emit('log', 'Peer identity verified (Ed25519)'); + } else { + console.warn('[RDClient] Ed25519 signature FAILED — possible MITM attack!'); + this._emit('signature_warning', 'Ed25519 signature verification failed. Connection may be intercepted.'); + this._emit('log', 'WARNING: Peer signature verification failed'); + } } + } else if (RDCrypto.hasDecodablePublicKey(serverPubKey)) { + this._debugRelay('[RDClient] No RelayResponse.pk — SignedId not verified against peer identity'); } else { this._debugRelay('[RDClient] No server public key available — signature not verified'); } diff --git a/web-nodejs/public/js/rdclient/crypto.js b/web-nodejs/public/js/rdclient/crypto.js index 6b33ae72..806ceee0 100644 --- a/web-nodejs/public/js/rdclient/crypto.js +++ b/web-nodejs/public/js/rdclient/crypto.js @@ -127,38 +127,113 @@ class RDCrypto { } /** - * Verify Ed25519 signature on SignedId payload against the server's public key. - * Prevents MITM attacks: the signal server signs (IdPk) with its Ed25519 key. - * Without verification, an attacker could substitute their own ephemeral key. + * Decode a 32-byte Ed25519 public key from RustDesk formats. + * Prefer base64 (id_ed25519.pub / Key field); also accept hex (API key_hex). + * + * @param {string} encoded + * @returns {Uint8Array|null} 32-byte public key, or null if invalid + */ + static decodeServerPublicKey(encoded) { + if (!encoded || typeof encoded !== 'string') return null; + const trimmed = encoded.trim(); + if (!trimmed) return null; + + // Hex: 64 hex chars = 32 bytes (optional 0x prefix) + const hexBody = trimmed.replace(/^0x/i, ''); + if (/^[0-9a-fA-F]{64}$/.test(hexBody)) { + try { + const bytes = RDCrypto._hexToBytes(hexBody); + return bytes.length === 32 ? bytes : null; + } catch { + return null; + } + } + + // Base64 (standard RustDesk Key / id_ed25519.pub) + try { + let b64 = trimmed.replace(/-/g, '+').replace(/_/g, '/'); + while (b64.length % 4) b64 += '='; + const bin = (typeof atob === 'function') + ? atob(b64) + : Buffer.from(b64, 'base64').toString('binary'); + if (bin.length !== 32) return null; + const out = new Uint8Array(32); + for (let i = 0; i < 32; i++) out[i] = bin.charCodeAt(i); + return out; + } catch { + return null; + } + } + + /** + * True when encoded looks like a usable server/peer Ed25519 public key. + * @param {string} encoded + * @returns {boolean} + */ + static hasDecodablePublicKey(encoded) { + return RDCrypto.decodeServerPublicKey(encoded) != null; + } + + /** + * Verify Ed25519 detached signature over payload with a base64 or hex public key. + * + * RustDesk chain for Web Remote: + * 1. RelayResponse.pk is IdPk signed by the **server** key (identity of target) + * 2. Message.SignedId is IdPk signed by the **peer identity** key (ephemeral box pk) * * @param {Uint8Array} signature - 64-byte Ed25519 signature * @param {Uint8Array} payload - Signed protobuf payload (IdPk bytes after the 64-byte sig) - * @param {string} serverPubKeyHex - Server Ed25519 public key as hex string (64 hex chars = 32 bytes) + * @param {string|Uint8Array} publicKey - Ed25519 public key (base64/hex string or raw 32 bytes) * @returns {boolean} True if signature is valid, false otherwise */ - static verifySignedId(signature, payload, serverPubKeyHex) { - if (!serverPubKeyHex || serverPubKeyHex.length < 64) { - console.warn('[RDCrypto] No server public key for Ed25519 verification'); - return false; - } + static verifySignedId(signature, payload, publicKey) { if (!signature || signature.length !== 64) { console.warn('[RDCrypto] Invalid Ed25519 signature length:', signature?.length); return false; } + if (!payload || !payload.length) { + console.warn('[RDCrypto] Empty signed payload'); + return false; + } + + let keyBytes = null; + if (publicKey instanceof Uint8Array) { + keyBytes = publicKey.length === 32 ? publicKey : null; + } else { + keyBytes = RDCrypto.decodeServerPublicKey(publicKey || ''); + } + if (!keyBytes) { + console.warn('[RDCrypto] No usable public key for Ed25519 verification'); + return false; + } try { - const serverPubKey = RDCrypto._hexToBytes(serverPubKeyHex); - if (serverPubKey.length !== 32) { - console.warn('[RDCrypto] Server public key must be 32 bytes, got:', serverPubKey.length); - return false; - } - return nacl.sign.detached.verify(payload, signature, serverPubKey); + return nacl.sign.detached.verify(payload, signature, keyBytes); } catch (err) { console.warn('[RDCrypto] Ed25519 verification error:', err.message); return false; } } + /** + * Verify a NaCl combined signed blob [64-byte sig][payload] and decode IdPk. + * @param {Uint8Array} signedBytes + * @param {string|Uint8Array} publicKey + * @param {Object} idPkType - protobufjs IdPk type + * @returns {{ peerId: string, peerPk: Uint8Array, signatureVerified: boolean }|null} + */ + static verifyAndDecodeIdPk(signedBytes, publicKey, idPkType) { + const parsed = new RDCrypto().parseSignedId(signedBytes, idPkType); + if (!parsed) return null; + const ok = RDCrypto.verifySignedId(parsed.signature, parsed.payload, publicKey); + if (!ok) return null; + return { + peerId: parsed.peerId, + peerPk: parsed.peerPk, + signatureVerified: true, + }; + } + /** * Convert hex string to Uint8Array * @param {string} hex diff --git a/web-nodejs/public/js/rdclient/file-connection.js b/web-nodejs/public/js/rdclient/file-connection.js index 2fe8a615..e6a6f165 100644 --- a/web-nodejs/public/js/rdclient/file-connection.js +++ b/web-nodejs/public/js/rdclient/file-connection.js @@ -34,6 +34,7 @@ class RDFileConnection { this._connectPromise = null; this._loginResolve = null; this._loginReject = null; + this._peerSignedPk = null; } get state() { return this._state; } @@ -131,6 +132,7 @@ class RDFileConnection { if (rendezvousResponse.error) { throw new Error(rendezvousResponse.error); } + this._peerSignedPk = rendezvousResponse.pk || null; let relayUUID = rendezvousResponse.uuid || ''; let relayServer = rendezvousResponse.relayServer || ''; @@ -147,6 +149,9 @@ class RDFileConnection { if (relayConfirm.error) throw new Error(relayConfirm.error); relayUUID = relayConfirm.uuid || relayUUID; relayServer = relayConfirm.relayServer || relayServer; + if (relayConfirm.pk) { + this._peerSignedPk = relayConfirm.pk; + } } this.conn.closeRendezvous(); @@ -443,8 +448,17 @@ class RDFileConnection { if (!parsed) return; const serverPubKey = this.opts.serverPubKey || ''; - if (serverPubKey && serverPubKey.length >= 64) { - RDCrypto.verifySignedId(parsed.signature, parsed.payload, serverPubKey); + if (RDCrypto.hasDecodablePublicKey(serverPubKey) && this._peerSignedPk && this._peerSignedPk.length) { + const peerIdentity = RDCrypto.verifyAndDecodeIdPk( + this._peerSignedPk instanceof Uint8Array + ? this._peerSignedPk + : new Uint8Array(this._peerSignedPk), + serverPubKey, + this.proto.types.IdPk + ); + if (peerIdentity) { + RDCrypto.verifySignedId(parsed.signature, parsed.payload, peerIdentity.peerPk); + } } this.crypto.generateKeyPair(); diff --git a/web-nodejs/routes/remote.routes.js b/web-nodejs/routes/remote.routes.js index 25e45418..c020a91b 100644 --- a/web-nodejs/routes/remote.routes.js +++ b/web-nodejs/routes/remote.routes.js @@ -5,13 +5,12 @@ const express = require('express'); const router = express.Router(); -const fs = require('fs'); const db = require('../services/database'); -const config = require('../config/config'); const logger = require('../lib/logger').child('REMOTE'); const { requireRdClientAuth, rdClientGuestOnly, normalizeRdClientReturnUrl, roleHasPermission } = require('../middleware/auth'); const { rdClientPageLimiter } = require('../middleware/rateLimiter'); const betterdeskApi = require('../services/betterdeskApi'); +const keyService = require('../services/keyService'); const { getGuestToken, getGuestTokenFromQuery, @@ -79,14 +78,15 @@ function getRemoteRelay() { try { return require('../services/remoteRelay'); } catch { return null; } } -// Read server public key once at startup -let serverPubKey = ''; -try { - if (fs.existsSync(config.pubKeyPath)) { - serverPubKey = fs.readFileSync(config.pubKeyPath, 'utf8').trim(); +// Read server public key on each viewer render (Go may write id_ed25519.pub after +// console start; avoid caching empty/stale key across the process lifetime). +function getServerPubKey() { + try { + return keyService.getPublicKey() || ''; + } catch (err) { + console.warn('Warning: Could not read server public key:', err.message); + return ''; } -} catch (err) { - console.warn('Warning: Could not read server public key:', err.message); } /** @@ -233,7 +233,7 @@ router.get('/remote/:deviceId', rdClientPageLimiter, requireRemoteAccess, async activePage: 'remote', deviceId: deviceId, device: device || { id: deviceId, hostname: '', platform: '', note: '' }, - serverPubKey: serverPubKey, + serverPubKey: getServerPubKey(), capabilities, guestToken: req.guestToken || getGuestTokenFromQuery(req) || '', layout: 'viewer' diff --git a/web-nodejs/tests/rdclient.crypto.test.js b/web-nodejs/tests/rdclient.crypto.test.js new file mode 100644 index 00000000..9ca2be69 --- /dev/null +++ b/web-nodejs/tests/rdclient.crypto.test.js @@ -0,0 +1,126 @@ +'use strict'; + +/** + * Web Remote SignedId / server-key decoding (#313 follow-up). + * id_ed25519.pub is base64; SignedId is peer-identity-signed (not server-signed). + */ + +const fs = require('fs'); +const path = require('path'); +const vm = require('vm'); +const nacl = require('tweetnacl'); +const protobuf = require('protobufjs'); + +function loadRDCrypto() { + const sandbox = { + console, + nacl, + Uint8Array, + atob: (s) => Buffer.from(s, 'base64').toString('binary'), + Buffer, + window: {}, + globalThis: {}, + }; + sandbox.window = sandbox; + sandbox.globalThis = sandbox; + const src = fs.readFileSync( + path.join(__dirname, '..', 'public/js/rdclient/crypto.js'), + 'utf8' + ); + vm.runInNewContext(src + '\nglobalThis.RDCrypto = RDCrypto;', sandbox, { + filename: 'crypto.js', + }); + return sandbox.RDCrypto; +} + +describe('RDCrypto server public key decoding', () => { + let RDCrypto; + + beforeAll(() => { + RDCrypto = loadRDCrypto(); + }); + + it('decodes standard base64 id_ed25519.pub (44 chars)', () => { + const kp = nacl.sign.keyPair(); + const b64 = Buffer.from(kp.publicKey).toString('base64'); + expect(b64.length).toBeLessThan(64); + expect(RDCrypto.hasDecodablePublicKey(b64)).toBe(true); + const decoded = RDCrypto.decodeServerPublicKey(b64); + expect(decoded).toEqual(kp.publicKey); + }); + + it('decodes hex key_hex (64 chars)', () => { + const kp = nacl.sign.keyPair(); + const hex = Buffer.from(kp.publicKey).toString('hex'); + expect(hex.length).toBe(64); + const decoded = RDCrypto.decodeServerPublicKey(hex); + expect(decoded).toEqual(kp.publicKey); + }); + + it('rejects empty / garbage keys', () => { + expect(RDCrypto.decodeServerPublicKey('')).toBeNull(); + expect(RDCrypto.decodeServerPublicKey('not-a-key')).toBeNull(); + expect(RDCrypto.hasDecodablePublicKey('')).toBe(false); + }); +}); + +describe('RDCrypto SignedId verification chain', () => { + let RDCrypto; + let IdPk; + + beforeAll(async () => { + RDCrypto = loadRDCrypto(); + const root = await protobuf.load([ + path.join(__dirname, '../protos/message.proto'), + ]); + IdPk = root.lookupType('hbb.IdPk'); + }); + + function signIdPk(signSecretKey, peerId, boxPk) { + const payload = IdPk.encode( + IdPk.create({ id: peerId, pk: boxPk }) + ).finish(); + const sig = nacl.sign.detached(payload, signSecretKey); + const combined = new Uint8Array(sig.length + payload.length); + combined.set(sig, 0); + combined.set(payload, sig.length); + return combined; + } + + it('verifies RelayResponse.pk with server key and SignedId with peer identity', () => { + const server = nacl.sign.keyPair(); + const peerIdentity = nacl.sign.keyPair(); + const ephemeral = nacl.box.keyPair(); + + // Server signs peer identity public key (RegisterPk / RelayResponse.pk) + const relayPk = signIdPk(server.secretKey, '123456789', peerIdentity.publicKey); + // Peer signs ephemeral box key (Message.SignedId) + const signedId = signIdPk(peerIdentity.secretKey, '123456789', ephemeral.publicKey); + + const serverKeyB64 = Buffer.from(server.publicKey).toString('base64'); + const identity = RDCrypto.verifyAndDecodeIdPk(relayPk, serverKeyB64, IdPk); + expect(identity).not.toBeNull(); + expect(identity.peerId).toBe('123456789'); + expect(identity.peerPk).toEqual(peerIdentity.publicKey); + + const session = new RDCrypto().parseSignedId(signedId, IdPk); + expect(session).not.toBeNull(); + expect( + RDCrypto.verifySignedId(session.signature, session.payload, identity.peerPk) + ).toBe(true); + expect(session.peerPk).toEqual(ephemeral.publicKey); + }); + + it('rejects SignedId when verified with the server key (wrong signer)', () => { + const server = nacl.sign.keyPair(); + const peerIdentity = nacl.sign.keyPair(); + const ephemeral = nacl.box.keyPair(); + const signedId = signIdPk(peerIdentity.secretKey, '123456789', ephemeral.publicKey); + const serverKeyB64 = Buffer.from(server.publicKey).toString('base64'); + + const parsed = new RDCrypto().parseSignedId(signedId, IdPk); + expect( + RDCrypto.verifySignedId(parsed.signature, parsed.payload, serverKeyB64) + ).toBe(false); + }); +});