diff --git a/deploy/windows/installer/shopdb-install.ps1 b/deploy/windows/installer/shopdb-install.ps1 index afe9e052..3701897e 100644 --- a/deploy/windows/installer/shopdb-install.ps1 +++ b/deploy/windows/installer/shopdb-install.ps1 @@ -124,8 +124,11 @@ param( # the proxy, so applying it would DESTROY the real client IP. # # Without one or the other, IIS sends no X-Forwarded-For at all and every - # client reads as 127.0.0.1: the GE-Enforce IP allowlist, the dashboard - # visitor-location lookup and per-host login rate limiting all break quietly. + # client reads as 127.0.0.1: the GE-Enforce IP allowlist, BOTH logged-out + # kiosk allowlists (usb_kiosk_allowed_cidrs, printedparts_kiosk_allowed_cidrs + # - these admit writes with no login), the dashboard visitor-location lookup + # and per-host login rate limiting all break quietly. Stage 4 verifies this + # in both modes; see the client-IP check there. [ValidateSet('direct','proxy')] [string] $ClientIpSource = 'direct', # Load the shared vendor/model catalog during stage 3. Offered by the # wizard rather than assumed: a site that machines nothing does not want @@ -2500,27 +2503,71 @@ public class ShopdbSmokeTestCertPolicy : ICertificatePolicy { } } - # Is the client-IP rule actually in effect? It is the control that makes - # remote_addr trustworthy: without it IIS forwards whatever X-Forwarded-For - # the CALLER sent, waitress trusts it because it arrives from loopback, and - # the GE-Enforce IP allowlist becomes bypassable from anywhere on the network. - # It does not fail closed. Verified here rather than assumed, because the - # failure is silent and looks exactly like a working server. + # Is the client-IP control actually in effect? Something has to make the + # address the app sees the CALLER's: waitress is launched with + # --trusted-proxy=127.0.0.1 --trusted-proxy-headers=x-forwarded-for, and + # HttpPlatformHandler always connects from loopback, so waitress ALWAYS + # believes the X-Forwarded-For it is handed. Whether that header is worth + # believing is decided in front of it, and nothing fails closed if it is not. + # + # THREE allowlists now ride on this, not one: the GE-Enforce client + # allowlist and the two kiosk allowlists (usb_kiosk_allowed_cidrs, + # printedparts_kiosk_allowed_cidrs), which admit UNAUTHENTICATED writes by + # network position. Naming only GE-Enforce here sent people to the wrong + # place - and left the kiosks out of the decision entirely. + # + # Checked in BOTH modes. It used to run only under -ClientIpSource direct, + # so the proxy deployment - the one where the header really does arrive from + # outside - had nothing verified at all. + $liveCfg = Join-Path $AppRoot 'web.config' + $cfgText = if (Test-Path $liveCfg) { Get-Content $liveCfg -Raw } else { '' } + $ruleLive = ($cfgText -match '') -and ($cfgText -notmatch 'SHOPDB-CLIENTIP-BEGIN') + $moduleThere = Test-Path (Join-Path $env:windir 'system32\inetsrv\rewrite.dll') if ($ClientIpSource -eq 'direct') { - $liveCfg = Join-Path $AppRoot 'web.config' - $cfgText = if (Test-Path $liveCfg) { Get-Content $liveCfg -Raw } else { '' } - $ruleLive = ($cfgText -match '') -and ($cfgText -notmatch 'SHOPDB-CLIENTIP-BEGIN') - $moduleThere = Test-Path (Join-Path $env:windir 'system32\inetsrv\rewrite.dll') if ($ruleLive -and $moduleThere) { Write-Log 'client-IP rule is live; remote_addr is trustworthy' 'OK' } else { Write-Log 'CLIENT-IP RULE IS NOT IN EFFECT' 'WARN' if (-not $moduleThere) { Write-Log ' URL Rewrite module is not installed' 'WARN' } if (-not $ruleLive) { Write-Log ' web.config does not have an active block' 'WARN' } - Write-Log ' every client will be recorded as 127.0.0.1, and the GE-Enforce IP' 'WARN' - Write-Log ' allowlist is BYPASSABLE - a caller can send its own X-Forwarded-For' 'WARN' - Write-Log ' re-run with -ClientIpSource direct, or use token authentication' 'WARN' + Write-Log ' a caller can send its own X-Forwarded-For and be believed, so EVERY' 'WARN' + Write-Log ' address-based control is BYPASSABLE from anywhere on the network:' 'WARN' + Write-Log ' - the GE-Enforce client IP allowlist' 'WARN' + Write-Log ' - usb_kiosk_allowed_cidrs (logged-out USB kiosk writes)' 'WARN' + Write-Log ' - printedparts_kiosk_allowed_cidrs (logged-out parts kiosk writes)' 'WARN' + Write-Log ' and with no header at all every client is recorded as 127.0.0.1.' 'WARN' + Write-Log ' the USB kiosk refuses a loopback caller outright rather than' 'WARN' + Write-Log ' admitting one, so that case reads as "not set up", not as access.' 'WARN' + Write-Log ' install URL Rewrite and enable the SHOPDB-CLIENTIP block, or leave the' 'WARN' + Write-Log ' two kiosk allowlists EMPTY and provision each screen with its token' 'WARN' } + } else { + # Proxy mode. The rule must be OFF here - it would overwrite the real + # client with the proxy's address - and the thing that has to be true + # instead is about the proxy, which this script cannot reach out and + # test. So: check what can be checked, and state the rest plainly. + if ($ruleLive) { + Write-Log 'web.config OVERWRITES X-Forwarded-For but -ClientIpSource is proxy' 'WARN' + Write-Log ' every request will be attributed to the proxy, not the caller:' 'WARN' + Write-Log ' the GE-Enforce and kiosk allowlists then match nothing at all' 'WARN' + Write-Log ' remove the block from web.config' 'WARN' + } else { + Write-Log 'client IP comes from the proxy; no rewrite rule in web.config' 'OK' + } + Write-Log 'PROXY MODE: two things this installer cannot verify for you' 'WARN' + Write-Log ' 1. the proxy must APPEND the caller to X-Forwarded-For (ARR does).' 'WARN' + Write-Log ' waitress takes the LAST hop and puts it in REMOTE_ADDR, so a proxy' 'WARN' + Write-Log ' that passes a caller-sent header through unchanged makes every' 'WARN' + Write-Log ' address control bypassable from anywhere that can reach the proxy.' 'WARN' + Write-Log ' 2. set usb_kiosk_trusted_proxies (Settings > USB) to where the proxy' 'WARN' + Write-Log ' is, BEFORE filling in usb_kiosk_allowed_cidrs. The app does not' 'WARN' + Write-Log ' read forwarding headers itself - waitress already promoted the' 'WARN' + Write-Log ' caller into REMOTE_ADDR, so the allowlist matches the CALLER, not' 'WARN' + Write-Log ' the proxy. What it is for is the opposite case: a request that' 'WARN' + Write-Log ' arrives wearing the PROXY address, because the proxy forwarded' 'WARN' + Write-Log ' nothing. A proxy normally sits on the same plant network the' 'WARN' + Write-Log ' allowlist names, so that request would otherwise be admitted on no' 'WARN' + Write-Log ' evidence at all. Naming the proxy here subtracts it and fails closed.' 'WARN' } # The self-hosted API documentation. It is the thing an operator - or the diff --git a/plugins/printedparts/api/routes.py b/plugins/printedparts/api/routes.py index 5d895bfc..85a54126 100644 --- a/plugins/printedparts/api/routes.py +++ b/plugins/printedparts/api/routes.py @@ -80,7 +80,7 @@ import re from flask import current_app from werkzeug.utils import secure_filename -from shopdb.api import Setting +from shopdb.api import Setting, client_ip, ip_in_cidrs IMAGE_EXTENSIONS = {'.png', '.jpg', '.jpeg', '.gif', '.webp'} IMAGE_URL_PREFIX = '/api/printedparts/image/' @@ -518,12 +518,36 @@ def adjust_item(item_id: int): import hmac -from shopdb.api import request_ip_allowlisted, audit_log +from shopdb.api import audit_log # Largest single take the kiosk records. A shop-floor grab is single digits; 25 # leaves real use untouched and denies draining a bin in one request. KIOSK_MAX_TAKE_QUANTITY = 25 +# Addresses that belong to the PATH, not to a caller. Subtracted from the +# allowlist before it is consulted. +# +# WHY, and it is the half that was missing here: a reverse proxy normally sits +# ON the network a site enters in printedparts_kiosk_allowed_cidrs. A request +# that arrives wearing the proxy's own address - because the proxy forwarded +# nothing, or because nothing forwarded at all - therefore MATCHED the allowlist +# and was admitted while carrying no evidence about who actually sent it. Being +# inside the allowlist is not enough; the address also has to be one a client +# could plausibly have had. +# +# Loopback is never a client at any site, so it needs no configuration. The USB +# kiosk closed this on 2026-09-18 and this one did not, which is the only reason +# the two read differently. +PARTS_KIOSK_TRUSTED_PROXIES = 'printedparts_kiosk_trusted_proxies' +PARTS_KIOSK_NEVER_A_CLIENT = '127.0.0.0/8, ::1/128' + + +def _kiosk_address_is_infrastructure(address): + """True when this address belongs to the path rather than to a caller.""" + if ip_in_cidrs(address, PARTS_KIOSK_NEVER_A_CLIENT): + return True + return ip_in_cidrs(address, Setting.get(PARTS_KIOSK_TRUSTED_PROXIES)) + def _kiosk_token_presented(): """True when the caller carries this kiosk's device token.""" @@ -553,8 +577,10 @@ def _kiosk_open(): return False if _kiosk_token_presented(): return True - return request_ip_allowlisted( - request, Setting.get('printedparts_kiosk_allowed_cidrs')) + address = client_ip(request) + if _kiosk_address_is_infrastructure(address): + return False + return ip_in_cidrs(address, Setting.get('printedparts_kiosk_allowed_cidrs')) @printedparts_bp.route('/kiosk/provisioning-link', methods=['GET']) diff --git a/plugins/printedparts/frontend/views/PrintedPartsSettings.vue b/plugins/printedparts/frontend/views/PrintedPartsSettings.vue index c635fac7..649f893f 100644 --- a/plugins/printedparts/frontend/views/PrintedPartsSettings.vue +++ b/plugins/printedparts/frontend/views/PrintedPartsSettings.vue @@ -86,6 +86,18 @@

+
+ + +

+ Only needed when a proxy sits between the kiosk and this server. A + request arriving FROM one of these addresses is never treated as a + kiosk, so a proxy inside an allowed network above cannot admit itself. + Loopback is always excluded and needs no entry. Leave empty when + browsers reach this server directly. +

+
+

- Provision each screen once by opening - /usb-kiosk?token=<this value> on it. That browser - keeps it; the address bar does not. Turning the kiosk on is a + Provision each screen once with Copy setup link + above and open that link on the screen. The link carries a single-use + setup code, not the token, so the token never reaches an address bar, + a browser history or a proxy log. That browser keeps the token + afterwards. Turning the kiosk on is a decision, not a credential - without this token the routes answer 404, so nobody who merely finds the URL can move a drive and sign it with a badge they guessed. @@ -61,6 +63,18 @@

+
+ + +

+ Only fill this in when a proxy sits between the kiosk screens and this + server. A request arriving FROM one of these addresses is never + treated as a kiosk itself, so a proxy that sits inside an allowed + network below cannot admit itself. Leave empty when browsers reach + the server directly. +

+
+
@@ -78,6 +92,34 @@

+
+ +

+ Off, a range wider than a site normally uses is refused when you save, + and 0.0.0.0/0 is refused outright. Turn this on only to enter a wide + range deliberately: every address in it can move drives with no token + and no login. +

+
+ +
+ +

+ The kiosk turns a badge number into an employee name without a login, + so an allowed network can otherwise be walked to enumerate staff. On, + a source that misses too many times in a row is refused for a while. +

+ + +
+