From 96a1020134225ce187e31be85f25310d2d3c2143 Mon Sep 17 00:00:00 2001 From: saubyk <39208279+saubyk@users.noreply.github.com> Date: Mon, 3 Aug 2026 12:36:53 -0700 Subject: [PATCH] Reduce exposure of authentication secrets in logs and config responses --- backend/controllers/shared/RTLConf.js | 4 ++ backend/utils/common.js | 18 +++++-- backend/utils/config.js | 4 +- release-notes/Release-notes-0.15.10.md | 7 +++ server/controllers/shared/RTLConf.ts | 2 + server/utils/common.ts | 16 ++++-- server/utils/config.ts | 4 +- test/backend/common.test.mjs | 41 +++++++++++++++ test/backend/rtlconf.test.mjs | 71 ++++++++++++++++++++++++++ 9 files changed, 155 insertions(+), 12 deletions(-) create mode 100644 test/backend/common.test.mjs diff --git a/backend/controllers/shared/RTLConf.js b/backend/controllers/shared/RTLConf.js index 35270a0a..ac5c9251 100644 --- a/backend/controllers/shared/RTLConf.js +++ b/backend/controllers/shared/RTLConf.js @@ -299,6 +299,10 @@ export const updateApplicationSettings = (req, res, next) => { delete fileConfig.rtlConfFilePath; delete fileConfig.rtlPass; delete fileConfig.multiPass; + // Runtime-only SSO bearer; must not be persisted with the config. + if (fileConfig.SSO) { + delete fileConfig.SSO.cookieValue; + } fileConfig.nodes?.forEach((node) => { delete node.authentication?.options; delete node.authentication?.runeValue; diff --git a/backend/utils/common.js b/backend/utils/common.js index 9de430f9..ceff3000 100644 --- a/backend/utils/common.js +++ b/backend/utils/common.js @@ -32,7 +32,8 @@ export class CommonService { if (typeof keys[i] === 'string' && ((keys[i].toLowerCase().includes('password') && keys[i] !== 'allowPasswordUpdate') || keys[i].toLowerCase().includes('multipass') || keys[i].toLowerCase().includes('rpcpass') || keys[i].toLowerCase().includes('rpcpassword') || - keys[i].toLowerCase().includes('rpcuser'))) { + keys[i].toLowerCase().includes('rpcuser') || keys[i].toLowerCase().includes('secret2fa') || + keys[i].toLowerCase().includes('cookievalue'))) { obj[keys[i]] = '*'.repeat(20); } } @@ -55,6 +56,10 @@ export class CommonService { delete config.multiPass; delete config.multiPassHashed; delete config.secret2FA; + // The SSO cookie is a live bearer credential; it must never leave the server. + if (config.SSO) { + delete config.SSO.cookieValue; + } config.nodes?.forEach((node) => this.removeAuthSecureData(node)); return config; }; @@ -63,6 +68,9 @@ export class CommonService { config.rtlPass = this.appConfig.rtlPass; config.multiPassHashed = this.appConfig.multiPassHashed; config.SSO.rtlCookiePath = this.appConfig.SSO.rtlCookiePath; + // cookieValue is stripped from client responses, so a settings save can never echo it; + // restore the server-held value or the save would silently wipe the live SSO cookie. + config.SSO.cookieValue = this.appConfig.SSO.cookieValue; if (this.appConfig.multiPass) { config.multiPass = this.appConfig.multiPass; } @@ -103,7 +111,7 @@ export class CommonService { this.logger.log({ selectedNode: this.selectedNode, level: 'ERROR', fileName: 'Common', msg: 'Loop macaroon Error', error: err }); } } - this.logger.log({ selectedNode: this.selectedNode, level: 'INFO', fileName: 'Common', msg: 'Swap Options', data: swapOptions }); + this.logger.log({ selectedNode: this.selectedNode, level: 'INFO', fileName: 'Common', msg: 'Swap Options Set' }); return swapOptions; }; this.getBoltzServerOptions = (req) => { @@ -121,7 +129,7 @@ export class CommonService { this.logger.log({ selectedNode: this.selectedNode, level: 'ERROR', fileName: 'Common', msg: 'Boltz macaroon Error', error: err }); } } - this.logger.log({ selectedNode: this.selectedNode, level: 'INFO', fileName: 'Common', msg: 'Boltz Options', data: boltzOptions }); + this.logger.log({ selectedNode: this.selectedNode, level: 'INFO', fileName: 'Common', msg: 'Boltz Options Set' }); return boltzOptions; }; this.getOptions = (req) => { @@ -167,7 +175,7 @@ export class CommonService { } } if (req.session.selectedNode) { - this.logger.log({ selectedNode: this.selectedNode, level: 'INFO', fileName: 'Common', msg: 'Updated Node Options for ' + req.session.selectedNode.lnNode, data: req.session.selectedNode.authentication.options }); + this.logger.log({ selectedNode: this.selectedNode, level: 'INFO', fileName: 'Common', msg: 'Updated Node Options for ' + req.session.selectedNode.lnNode }); } return { status: 200, message: 'Updated Successfully' }; } @@ -237,7 +245,7 @@ export class CommonService { form: '' }; } - this.logger.log({ selectedNode: this.selectedNode, level: 'INFO', fileName: 'Common', msg: 'Set Node Options for ' + node.lnNode, data: node.authentication.options }); + this.logger.log({ selectedNode: this.selectedNode, level: 'INFO', fileName: 'Common', msg: 'Set Node Options for ' + node.lnNode }); }); this.updateSelectedNodeOptions(req); } diff --git a/backend/utils/config.js b/backend/utils/config.js index b001d38f..318e080b 100644 --- a/backend/utils/config.js +++ b/backend/utils/config.js @@ -302,7 +302,9 @@ export class ConfigService { this.logger.log({ selectedNode: this.common.selectedNode, level: 'ERROR', fileName: 'Config', msg: 'Something went wrong while creating the backup directory: \n' + err }); } this.common.nodes[idx].settings.logFile = config.rtlConfFilePath + '/logs/RTL-Node-' + node.index + '.log'; - this.logger.log({ selectedNode: this.common.selectedNode, level: 'INFO', fileName: 'Config', msg: 'Node Config: ' + JSON.stringify(this.common.nodes[idx]) }); + // maskPasswords keeps paths visible for debugging while redacting lnApiPassword + // and any other credential fields before they reach the log file. + this.logger.log({ selectedNode: this.common.selectedNode, level: 'INFO', fileName: 'Config', msg: 'Node Config: ' + JSON.stringify(this.common.maskPasswords(JSON.parse(JSON.stringify(this.common.nodes[idx])))) }); const log_file = this.common.nodes[idx].settings.logFile; if (fs.existsSync(log_file || '')) { fs.writeFile((log_file || ''), '', () => { }); diff --git a/release-notes/Release-notes-0.15.10.md b/release-notes/Release-notes-0.15.10.md index 3082992c..84f462e3 100644 --- a/release-notes/Release-notes-0.15.10.md +++ b/release-notes/Release-notes-0.15.10.md @@ -11,6 +11,13 @@ this release should add its entry under the appropriate section below. (`test/backend/authenticate.test.mjs`). Users who have two-factor authentication enabled are encouraged to update promptly. +- **Config & logging: reduce exposure of authentication secrets** + ([#TBD](https://github.com/Ride-The-Lightning/RTL/pull/TBD)). + Tightens redaction of authentication material in node logs and configuration API + responses, and keeps runtime-only SSO state out of the persisted config file. Adds + regression coverage (`test/backend/common.test.mjs`). Users are encouraged to update + promptly. + ## Code Health - **Batch dependency update resolving the open Dependabot security PRs** diff --git a/server/controllers/shared/RTLConf.ts b/server/controllers/shared/RTLConf.ts index bc86d75c..e84513ce 100644 --- a/server/controllers/shared/RTLConf.ts +++ b/server/controllers/shared/RTLConf.ts @@ -299,6 +299,8 @@ export const updateApplicationSettings = (req, res, next) => { delete fileConfig.rtlConfFilePath; delete fileConfig.rtlPass; delete fileConfig.multiPass; + // Runtime-only SSO bearer; must not be persisted with the config. + if (fileConfig.SSO) { delete fileConfig.SSO.cookieValue; } fileConfig.nodes?.forEach((node) => { delete node.authentication?.options; delete node.authentication?.runeValue; diff --git a/server/utils/common.ts b/server/utils/common.ts index 79a23584..c9145f90 100644 --- a/server/utils/common.ts +++ b/server/utils/common.ts @@ -37,7 +37,8 @@ export class CommonService { if (typeof keys[i] === 'string' && ((keys[i].toLowerCase().includes('password') && keys[i] !== 'allowPasswordUpdate') || keys[i].toLowerCase().includes('multipass') || keys[i].toLowerCase().includes('rpcpass') || keys[i].toLowerCase().includes('rpcpassword') || - keys[i].toLowerCase().includes('rpcuser')) + keys[i].toLowerCase().includes('rpcuser') || keys[i].toLowerCase().includes('secret2fa') || + keys[i].toLowerCase().includes('cookievalue')) ) { obj[keys[i]] = '*'.repeat(20); } @@ -63,6 +64,8 @@ export class CommonService { delete config.multiPass; delete config.multiPassHashed; delete config.secret2FA; + // The SSO cookie is a live bearer credential; it must never leave the server. + if (config.SSO) { delete config.SSO.cookieValue; } config.nodes?.forEach((node) => this.removeAuthSecureData(node)); return config; }; @@ -72,6 +75,9 @@ export class CommonService { config.rtlPass = this.appConfig.rtlPass; config.multiPassHashed = this.appConfig.multiPassHashed; config.SSO.rtlCookiePath = this.appConfig.SSO.rtlCookiePath; + // cookieValue is stripped from client responses, so a settings save can never echo it; + // restore the server-held value or the save would silently wipe the live SSO cookie. + config.SSO.cookieValue = this.appConfig.SSO.cookieValue; if (this.appConfig.multiPass) { config.multiPass = this.appConfig.multiPass; } @@ -112,7 +118,7 @@ export class CommonService { this.logger.log({ selectedNode: this.selectedNode, level: 'ERROR', fileName: 'Common', msg: 'Loop macaroon Error', error: err }); } } - this.logger.log({ selectedNode: this.selectedNode, level: 'INFO', fileName: 'Common', msg: 'Swap Options', data: swapOptions }); + this.logger.log({ selectedNode: this.selectedNode, level: 'INFO', fileName: 'Common', msg: 'Swap Options Set' }); return swapOptions; }; @@ -130,7 +136,7 @@ export class CommonService { this.logger.log({ selectedNode: this.selectedNode, level: 'ERROR', fileName: 'Common', msg: 'Boltz macaroon Error', error: err }); } } - this.logger.log({ selectedNode: this.selectedNode, level: 'INFO', fileName: 'Common', msg: 'Boltz Options', data: boltzOptions }); + this.logger.log({ selectedNode: this.selectedNode, level: 'INFO', fileName: 'Common', msg: 'Boltz Options Set' }); return boltzOptions; }; @@ -179,7 +185,7 @@ export class CommonService { } } if (req.session.selectedNode) { - this.logger.log({ selectedNode: this.selectedNode, level: 'INFO', fileName: 'Common', msg: 'Updated Node Options for ' + req.session.selectedNode.lnNode, data: req.session.selectedNode.authentication.options }); + this.logger.log({ selectedNode: this.selectedNode, level: 'INFO', fileName: 'Common', msg: 'Updated Node Options for ' + req.session.selectedNode.lnNode }); } return { status: 200, message: 'Updated Successfully' }; } catch (err) { @@ -247,7 +253,7 @@ export class CommonService { form: '' }; } - this.logger.log({ selectedNode: this.selectedNode, level: 'INFO', fileName: 'Common', msg: 'Set Node Options for ' + node.lnNode, data: node.authentication.options }); + this.logger.log({ selectedNode: this.selectedNode, level: 'INFO', fileName: 'Common', msg: 'Set Node Options for ' + node.lnNode }); }); this.updateSelectedNodeOptions(req); } diff --git a/server/utils/config.ts b/server/utils/config.ts index e0d101ce..ec3b8b85 100644 --- a/server/utils/config.ts +++ b/server/utils/config.ts @@ -284,7 +284,9 @@ export class ConfigService { this.logger.log({ selectedNode: this.common.selectedNode, level: 'ERROR', fileName: 'Config', msg: 'Something went wrong while creating the backup directory: \n' + err }); } this.common.nodes[idx].settings.logFile = config.rtlConfFilePath + '/logs/RTL-Node-' + node.index + '.log'; - this.logger.log({ selectedNode: this.common.selectedNode, level: 'INFO', fileName: 'Config', msg: 'Node Config: ' + JSON.stringify(this.common.nodes[idx]) }); + // maskPasswords keeps paths visible for debugging while redacting lnApiPassword + // and any other credential fields before they reach the log file. + this.logger.log({ selectedNode: this.common.selectedNode, level: 'INFO', fileName: 'Config', msg: 'Node Config: ' + JSON.stringify(this.common.maskPasswords(JSON.parse(JSON.stringify(this.common.nodes[idx])))) }); const log_file = this.common.nodes[idx].settings.logFile; if (fs.existsSync(log_file || '')) { fs.writeFile((log_file || ''), '', () => { }); diff --git a/test/backend/common.test.mjs b/test/backend/common.test.mjs new file mode 100644 index 00000000..6fe47d7d --- /dev/null +++ b/test/backend/common.test.mjs @@ -0,0 +1,41 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; + +import { Common } from '../../backend/utils/common.js'; + +test('maskPasswords masks TOTP and SSO cookie secrets along with passwords', () => { + const config = { + secret2FA: 'JBSWY3DPEHPK3PXP', + multiPassHashed: 'password-hash', + SSO: { rtlSSO: 1, rtlCookiePath: '/cookie-path', cookieValue: 'live-sso-cookie' }, + nodes: [{ index: 1, authentication: { lnApiPassword: 'eclair-pass', macaroonPath: '/macaroon/path' } }] + }; + const masked = Common.maskPasswords(config); + assert.equal(masked.secret2FA, '*'.repeat(20)); + assert.equal(masked.SSO.cookieValue, '*'.repeat(20)); + assert.equal(masked.multiPassHashed, '*'.repeat(20)); + assert.equal(masked.nodes[0].authentication.lnApiPassword, '*'.repeat(20)); + // Paths are configuration, not secrets — they must stay visible for the settings UI. + assert.equal(masked.nodes[0].authentication.macaroonPath, '/macaroon/path'); + assert.equal(masked.SSO.rtlCookiePath, '/cookie-path'); +}); + +test('removeSecureData strips the SSO cookie along with the other secrets', () => { + const config = { + rtlConfFilePath: '/conf', + rtlPass: 'password-hash', + multiPassHashed: 'password-hash', + secret2FA: 'JBSWY3DPEHPK3PXP', + SSO: { rtlSSO: 1, rtlCookiePath: '/cookie-path', logoutRedirectLink: '', cookieValue: 'live-sso-cookie' }, + nodes: [{ index: 1, authentication: { macaroonPath: '/macaroon/path', runeValue: 'rune', options: {} } }] + }; + const cleaned = Common.removeSecureData(config); + assert.equal(cleaned.rtlConfFilePath, undefined); + assert.equal(cleaned.rtlPass, undefined); + assert.equal(cleaned.multiPassHashed, undefined); + assert.equal(cleaned.secret2FA, undefined); + assert.equal(cleaned.SSO.cookieValue, undefined); + // Non-secret SSO settings survive — the settings UI renders them. + assert.equal(cleaned.SSO.rtlCookiePath, '/cookie-path'); + assert.equal(cleaned.nodes[0].authentication.macaroonPath, undefined); +}); diff --git a/test/backend/rtlconf.test.mjs b/test/backend/rtlconf.test.mjs index 05546949..8bf42826 100644 --- a/test/backend/rtlconf.test.mjs +++ b/test/backend/rtlconf.test.mjs @@ -137,3 +137,74 @@ test('updateApplicationSettings preserves indexed node auth and sanitizes only p rmSync(tempDir, { force: true, recursive: true }); } }); + +test('updateApplicationSettings keeps the SSO cookie server-side without exposing or persisting it', () => { + const tempDir = mkdtempSync(join(tmpdir(), 'rtlconf-sso-')); + const oldConfig = { + defaultNodeIndex: 0, + dbDirectoryPath: '/db', + SSO: { rtlSSO: 1, rtlCookiePath: '/cookie-path', logoutRedirectLink: '' }, + nodes: [ + { + index: 0, + lnNode: 'lnd-main', + lnImplementation: 'LND', + authentication: { macaroonPath: '/lnd/admin' }, + settings: { userPersona: 'OPERATOR', themeMode: 'DAY' } + } + ] + }; + const runtimeConfig = clone({ + ...oldConfig, + selectedNodeIndex: 0, + enable2FA: false, + allowPasswordUpdate: true, + rtlConfFilePath: tempDir, + rtlPass: 'hashed-password', + SSO: { rtlSSO: 1, rtlCookiePath: '/cookie-path', logoutRedirectLink: '', cookieValue: 'live-sso-cookie' } + }); + // The request carries only what the sanitized client can have seen: no cookieValue. + // The server must re-attach it — a settings save must never wipe the live cookie — + // while keeping it out of both the response and the persisted file. + const requestBody = { + ...clone(oldConfig), + selectedNodeIndex: 0, + enable2FA: false, + allowPasswordUpdate: true, + SSO: { rtlSSO: 1, rtlCookiePath: '/cookie-path', logoutRedirectLink: '' } + }; + + try { + Common.appConfig = clone(runtimeConfig); + Common.nodes = clone(runtimeConfig.nodes); + Common.selectedNode = Common.nodes[0]; + writeFileSync(join(tempDir, 'RTL-Config.json'), JSON.stringify(oldConfig, null, 2), 'utf-8'); + + let responseStatus; + let responseBody; + updateApplicationSettings( + { body: clone(requestBody), session: { selectedNode: Common.selectedNode } }, + { + status: (status) => { + responseStatus = status; + return { + json: (body) => { + responseBody = body; + } + }; + } + }, + null + ); + + assert.equal(responseStatus, 201); + assert.equal(Common.appConfig.SSO.cookieValue, 'live-sso-cookie'); + assert.equal(Common.appConfig.SSO.rtlCookiePath, '/cookie-path'); + const fileConfig = JSON.parse(readFileSync(join(tempDir, 'RTL-Config.json'), 'utf-8')); + assert.equal(fileConfig.SSO.cookieValue, undefined); + assert.equal(responseBody.SSO.cookieValue, undefined); + } finally { + clearInterval(WSServer.pingInterval); + rmSync(tempDir, { force: true, recursive: true }); + } +});