fix: make trustProxy changes instant + clarify security settings requiring restart

scarlett
NGPixel 5 days ago
parent d39f2eff0f
commit e8b6fcb3fc
No known key found for this signature in database

@ -53,8 +53,8 @@ async function routes(app: FastifyInstance) {
// parses, so the JSON routes below are unaffected. // parses, so the JSON routes below are unaffected.
// //
// The limit is read once, here, because a route's body limit is fixed when it is registered — // The limit is read once, here, because a route's body limit is fixed when it is registered —
// changing it in the admin area takes effect on the next restart, as the rest of the security // changing it in the admin area takes effect on the next restart, which is why that field is
// settings do. // one of the ones the security view marks as needing one.
app.addContentTypeParser( app.addContentTypeParser(
'*', '*',
{ parseAs: 'buffer', bodyLimit: WIKI.config.security?.uploadMaxFileSize ?? 10485760 }, { parseAs: 'buffer', bodyLimit: WIKI.config.security?.uploadMaxFileSize ?? 10485760 },

@ -314,7 +314,7 @@ async function routes(app: FastifyInstance) {
schema: { schema: {
summary: 'Get the security configuration', summary: 'Get the security configuration',
description: description:
'Most of this is applied when the HTTP server starts, so changing it takes effect on the next restart.', 'Response headers, CSP, HSTS, CORS and `uploadMaxFileSize` are applied when the HTTP server starts, so changing those takes effect on the next restart. The rest is read per request.',
tags: ['System'], tags: ['System'],
response: { response: {
200: { $ref: 'SecurityConfig#' } 200: { $ref: 'SecurityConfig#' }
@ -338,7 +338,7 @@ async function routes(app: FastifyInstance) {
schema: { schema: {
summary: 'Update the security configuration', summary: 'Update the security configuration',
description: description:
'Accepts any subset of the fields. Header, CORS and proxy settings are read when the HTTP server starts and therefore apply after a restart.', 'Accepts any subset of the fields. Response headers, CSP, HSTS, CORS and `uploadMaxFileSize` are read when the HTTP server starts and therefore apply after a restart; `trustProxy`, `forceAssetDownload` and the rate limit are read per request and apply at once.',
tags: ['System'], tags: ['System'],
body: { $ref: 'SecurityConfig#' }, body: { $ref: 'SecurityConfig#' },
response: { response: {

@ -304,9 +304,31 @@ async function initHTTPServer() {
logger: { logger: {
level: 'error' level: 'error'
}, },
// -> `securityTrustProxy` was the 2.x name: the setting is `trustProxy`, so this read never /*
// matched and the option was permanently off no matter what the admin area showed A function rather than the boolean itself, so that the setting is answered per request instead
trustProxy: WIKI.config.security.trustProxy ?? false, of being baked in here: the value fastify is handed at construction can never change, and this
is the one security setting whose effect an administrator checks immediately an audit entry
or a rate limit recorded against the proxy rather than the visitor. Turning it on and being
told to restart before it means anything is how a wrong address gets read as a bug.
Fastify only asks whether a given hop is trusted, and it asks per hop per request, so
returning false for all of them is exactly the `false` behaviour: `proxy-addr` truncates the
chain at the socket, and `req.ip` / `req.host` / `req.protocol` come from the connection and
the Host header as they would with the option off. The truthiness of the function is what
matters at construction time, not the setting.
Read through the `WIKI` global on every call, and that is load-bearing rather than incidental:
an HA instance that did not serve the save learns about it from the `reloadConfig` event, whose
handler REPLACES `WIKI.config` with a merged copy rather than mutating it. Hoisting this to a
captured `WIKI.config.security` would keep working on the instance the administrator happened to
hit and silently freeze on every other one. Verified across two instances sharing a database.
Note that `true` trusts the whole chain and therefore takes the LEFTMOST `X-Forwarded-For`
entry, which a client can put anything it likes into. That is the meaning of the setting as it
stands; a deployment where clients can reach the wiki without passing the proxy needs a hop
count or a CIDR list, which this setting cannot express yet.
*/
trustProxy: () => WIKI.config.security?.trustProxy === true,
routerOptions: { routerOptions: {
ignoreTrailingSlash: true ignoreTrailingSlash: true
} }
@ -354,8 +376,10 @@ async function initHTTPServer() {
// Security // Security
// ---------------------------------------- // ----------------------------------------
// -> Every setting below comes from the admin area's security view. They are read once, here, so a // -> Every setting below comes from the admin area's security view, and every one of them is read
// change takes effect on the next restart — the view says as much. // once, here, so a change takes effect on the next restart. Not every setting in that view is:
// `trustProxy` above and `forceAssetDownload` and the rate limit elsewhere are read per request,
// which is why the view marks the restart on the individual options rather than on the page.
const security = WIKI.config.security const security = WIKI.config.security
app.register(fastifyHelmet, { app.register(fastifyHelmet, {

@ -929,7 +929,7 @@
"admin.security.rateLimitRecommended": "Keeping rate limiting enabled is highly recommended: without it, passwords, second factors and page passwords can be guessed as fast as the server will answer.", "admin.security.rateLimitRecommended": "Keeping rate limiting enabled is highly recommended: without it, passwords, second factors and page passwords can be guessed as fast as the server will answer.",
"admin.security.rateLimitWindow": "Time Window", "admin.security.rateLimitWindow": "Time Window",
"admin.security.rateLimitWindowHint": "How long attempts are counted over, as a duration — 30s, 5m, 2h, 1d. Going over the limit within it earns a ban.", "admin.security.rateLimitWindowHint": "How long attempts are counted over, as a duration — 30s, 5m, 2h, 1d. Going over the limit within it earns a ban.",
"admin.security.restartRequired": "Header, CORS and proxy settings are applied when the server starts, so they take effect after a restart.", "admin.security.restartRequired": "Takes effect after a server restart.",
"admin.security.saveFailed": "Failed to save the security configuration.", "admin.security.saveFailed": "Failed to save the security configuration.",
"admin.security.saveSuccess": "Security configuration updated successfully.", "admin.security.saveSuccess": "Security configuration updated successfully.",
"admin.security.scanSVG": "Scan and Sanitize SVG Uploads", "admin.security.scanSVG": "Scan and Sanitize SVG Uploads",
@ -941,7 +941,7 @@
"admin.security.trustProxyHint": "Should be enabled when using a reverse-proxy like nginx, apache, CloudFlare, etc in front of Wiki.js. Turn off otherwise.", "admin.security.trustProxyHint": "Should be enabled when using a reverse-proxy like nginx, apache, CloudFlare, etc in front of Wiki.js. Turn off otherwise.",
"admin.security.uploads": "Uploads", "admin.security.uploads": "Uploads",
"admin.security.uploadsInfo": "These settings only affect Wiki.js. If you're using a reverse-proxy (e.g. nginx, Apache, Cloudflare), you must also change its settings to match.", "admin.security.uploadsInfo": "These settings only affect Wiki.js. If you're using a reverse-proxy (e.g. nginx, Apache, Cloudflare), you must also change its settings to match.",
"admin.security.uploadsNotEnforced": "These limits are saved but not enforced yet: uploading is not implemented.", "admin.security.uploadsNotEnforced": "The batch limit and the SVG scan are saved but not enforced yet. The maximum file size is.",
"admin.security.warn": "Make sure to understand the implications before turning on / off a security feature.", "admin.security.warn": "Make sure to understand the implications before turning on / off a security feature.",
"admin.sites.activate": "Activate Site", "admin.sites.activate": "Activate Site",
"admin.sites.activateConfirm": "Are you sure you want activate site {siteTitle}? The site will become accessible to users with read access.", "admin.sites.activateConfirm": "Are you sure you want activate site {siteTitle}? The site will become accessible to users with read access.",

@ -28,9 +28,12 @@ const DURATION_PATTERN = /^\d+[smhdwy]$/
/** /**
* Security model * Security model
* *
* The admin area's security view, which is exactly the `security` settings blob. Most of it is read * The admin area's security view, which is exactly the `security` settings blob. Saving does not mean
* when the HTTP server starts see the `Security` section of `index.ts` so saving here takes * the same thing for all of it, which is why the view marks the restart per option rather than once
* effect on the next restart. * at the top: the response headers, CSP, HSTS and CORS are read by the `Security` section of
* `index.ts` when the HTTP server starts, and `uploadMaxFileSize` by the upload route's body limit
* when it is registered, so those take effect on the next restart. `trustProxy`, the rate limit
* fields and `forceAssetDownload` are read per request and apply as soon as they are saved.
*/ */
class Security { class Security {
/** /**

@ -5,7 +5,9 @@
<img class="admin-icon animated fadeInLeft" src="/_assets/icons/fluent-protect.svg" /> <img class="admin-icon animated fadeInLeft" src="/_assets/icons/fluent-protect.svg" />
</div> </div>
<div class="min-w-0 flex-1 pl-4"> <div class="min-w-0 flex-1 pl-4">
<div class="text-h5 admin-page-title animated fadeInLeft">{{ t('admin.security.title') }}</div> <div class="text-h5 admin-page-title animated fadeInLeft">
{{ t('admin.security.title') }}
</div>
<div class="text-subtitle1 text-grey animated fadeInLeft wait-p2s"> <div class="text-subtitle1 text-grey animated fadeInLeft wait-p2s">
{{ t('admin.security.subtitle') }} {{ t('admin.security.subtitle') }}
</div> </div>
@ -57,8 +59,6 @@
</w-card-section> </w-card-section>
<w-card-section class="text-caption"> <w-card-section class="text-caption">
<div>{{ t('admin.security.warn') }}</div> <div>{{ t('admin.security.warn') }}</div>
<!-- These are read when the HTTP server builds its plugin chain, not per request -->
<div class="mt-1">{{ t('admin.security.restartRequired') }}</div>
</w-card-section> </w-card-section>
</w-card-section> </w-card-section>
</w-card> </w-card>
@ -69,6 +69,10 @@
<w-item-section> <w-item-section>
<w-item-label>{{ t(`admin.security.disallowIframe`) }}</w-item-label> <w-item-label>{{ t(`admin.security.disallowIframe`) }}</w-item-label>
<w-item-label caption>{{ t(`admin.security.disallowIframeHint`) }}</w-item-label> <w-item-label caption>{{ t(`admin.security.disallowIframeHint`) }}</w-item-label>
<w-item-label class="text-caption text-negative flex items-center">
<w-icon class="mr-1" name="la:exclamation-triangle" size="xs" />
{{ t('admin.security.restartRequired') }}
</w-item-label>
</w-item-section> </w-item-section>
<w-item-section avatar> <w-item-section avatar>
<w-toggle <w-toggle
@ -84,6 +88,10 @@
<w-item-label caption>{{ <w-item-label caption>{{
t(`admin.security.enforceSameOriginReferrerPolicyHint`) t(`admin.security.enforceSameOriginReferrerPolicyHint`)
}}</w-item-label> }}</w-item-label>
<w-item-label class="text-caption text-negative flex items-center">
<w-icon class="mr-1" name="la:exclamation-triangle" size="xs" />
{{ t('admin.security.restartRequired') }}
</w-item-label>
</w-item-section> </w-item-section>
<w-item-section avatar> <w-item-section avatar>
<w-toggle <w-toggle
@ -143,6 +151,10 @@
<w-item-section> <w-item-section>
<w-item-label>{{ t(`admin.security.enforceHsts`) }}</w-item-label> <w-item-label>{{ t(`admin.security.enforceHsts`) }}</w-item-label>
<w-item-label caption>{{ t(`admin.security.enforceHstsHint`) }}</w-item-label> <w-item-label caption>{{ t(`admin.security.enforceHstsHint`) }}</w-item-label>
<w-item-label class="text-caption text-negative flex items-center">
<w-icon class="mr-1" name="la:exclamation-triangle" size="xs" />
{{ t('admin.security.restartRequired') }}
</w-item-label>
</w-item-section> </w-item-section>
<w-item-section avatar> <w-item-section avatar>
<w-toggle <w-toggle
@ -294,6 +306,10 @@
<w-item-section> <w-item-section>
<w-item-label>{{ t(`admin.security.maxUploadSize`) }}</w-item-label> <w-item-label>{{ t(`admin.security.maxUploadSize`) }}</w-item-label>
<w-item-label caption>{{ t(`admin.security.maxUploadSizeHint`) }}</w-item-label> <w-item-label caption>{{ t(`admin.security.maxUploadSizeHint`) }}</w-item-label>
<w-item-label class="text-caption text-negative flex items-center">
<w-icon class="mr-1" name="la:exclamation-triangle" size="xs" />
{{ t('admin.security.restartRequired') }}
</w-item-label>
</w-item-section> </w-item-section>
<w-item-section style="flex: 0 0 200px"> <w-item-section style="flex: 0 0 200px">
<w-input <w-input
@ -343,6 +359,10 @@
<w-item-section> <w-item-section>
<w-item-label>{{ t(`admin.security.corsMode`) }}</w-item-label> <w-item-label>{{ t(`admin.security.corsMode`) }}</w-item-label>
<w-item-label caption>{{ t(`admin.security.corsModeHint`) }}</w-item-label> <w-item-label caption>{{ t(`admin.security.corsModeHint`) }}</w-item-label>
<w-item-label class="text-caption text-negative flex items-center">
<w-icon class="mr-1" name="la:exclamation-triangle" size="xs" />
{{ t('admin.security.restartRequired') }}
</w-item-label>
</w-item-section> </w-item-section>
<w-item-section> <w-item-section>
<w-select <w-select

Loading…
Cancel
Save