From dc3a387120b4d724d61489ce54dce84560e1d72d Mon Sep 17 00:00:00 2001 From: Reto Brunner Date: Tue, 23 Nov 2021 17:53:00 +0100 Subject: [PATCH 1/2] vapid: keep the file secret Contains a secret key, so we probably should keep it, well, secret. Warn if the file is world readable. --- src/plugins/webpush.js | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/src/plugins/webpush.js b/src/plugins/webpush.js index cf34c10a..09cfb81d 100644 --- a/src/plugins/webpush.js +++ b/src/plugins/webpush.js @@ -11,7 +11,28 @@ class WebPush { constructor() { const vapidPath = path.join(Helper.getHomePath(), "vapid.json"); - if (fs.existsSync(vapidPath)) { + let vapidStat = undefined; + + try { + vapidStat = fs.statSync(vapidPath); + } catch { + // ignored on purpose, node v14.17.0 will give us {throwIfNoEntry: false} + } + + if (vapidStat) { + const isWorldReadable = (vapidStat.mode & 0o004) !== 0; + + if (isWorldReadable) { + log.warn( + vapidPath, + "is world readable. The file contains secrets. Please fix the permissions" + ); + + if (require("os").platform() !== "win32") { + log.warn(`run \`chmod o= ${vapidPath}\` to correct it`); + } + } + const data = fs.readFileSync(vapidPath, "utf-8"); const parsedData = JSON.parse(data); @@ -29,7 +50,9 @@ class WebPush { if (!this.vapidKeys) { this.vapidKeys = WebPushAPI.generateVAPIDKeys(); - fs.writeFileSync(vapidPath, JSON.stringify(this.vapidKeys, null, "\t")); + fs.writeFileSync(vapidPath, JSON.stringify(this.vapidKeys, null, "\t"), { + mode: 0o600, + }); log.info("New VAPID key pair has been generated for use with push subscription."); } From 0ff9703a2809ddd07d56c49f8cf3eff607242d35 Mon Sep 17 00:00:00 2001 From: Reto Brunner Date: Tue, 23 Nov 2021 19:48:59 +0100 Subject: [PATCH 2/2] logs: Set umode to a more restrictive value When TL first creates the log folder, let only the user read the log files, should the admin override that subsequently we'll simply warn about it but respect the decision. Meaning we have private by default, but this can be overriden --- src/helper.js | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/helper.js b/src/helper.js index 27352b53..be106721 100644 --- a/src/helper.js +++ b/src/helper.js @@ -158,6 +158,34 @@ function setHome(newPath) { // Load theme color from the web manifest const manifest = JSON.parse(fs.readFileSync(manifestPath, "utf8")); this.config.themeColor = manifest.theme_color; + + // log dir probably shouldn't be world accessible. + // Create it with the desired permission bits if it doesn't exist yet. + let logsStat = undefined; + + try { + logsStat = fs.statSync(userLogsPath); + } catch { + // ignored on purpose, node v14.17.0 will give us {throwIfNoEntry: false} + } + + if (!logsStat) { + try { + fs.mkdirSync(userLogsPath, {recursive: true, mode: 0o750}); + } catch (e) { + log.error("Unable to create logs directory", e); + } + } else if (logsStat && logsStat.mode & 0o001) { + log.warn( + "contents of", + userLogsPath, + "can be accessed by any user, the log files may be exposed" + ); + + if (os.platform() !== "win32") { + log.warn(`run \`chmod o-x ${userLogsPath}\` to correct it`); + } + } } function getHomePath() {