From 7ac1ee3077157a557ebf4e180afa86b28cd56608 Mon Sep 17 00:00:00 2001 From: cproudlock Date: Fri, 18 Sep 2026 13:30:24 -0400 Subject: [PATCH] Refuse a kiosk caller wearing an address that belongs to the path Both kiosks admit a check-in or check-out with no login when the caller sits in a configured network. The subtraction that makes that safe was missing. A reverse proxy normally sits ON the plant network a site enters in the allowlist. When the proxy forwards nothing - misconfigured, or something bypassed it - the request arrives wearing the proxy's own address, which matches the allowlist, while carrying no evidence whatever about who sent it. That is the one case that has to refuse, and it was being admitted. The previous fail-closed test only looked closed because the proxy in it happened to sit outside the range. Addresses that belong to the path are now subtracted before the allowlist is consulted: loopback always, since no site's kiosk is the server itself, plus whatever the site declares as its proxies. USB and printedparts get the same treatment - they had diverged for no reason anybody chose. Three more, same surface: A range that admits everybody can no longer be saved by accident. 0.0.0.0/0 and ::/0 are refused outright, anything wider than a site plausibly needs is refused unless an explicit override is set, and the refusal says why. The kiosk token no longer travels in a URL. The provisioning link carries a single-use setup code redeemed on first open, so the token stays out of browser history, referrers and proxy logs. The route is declared public deliberately: the secret is the code, the way a password-reset link is the secret. The badge-to-name lookup is bounded. It answers without a login and is now reachable from a whole network rather than one screen, so a source that keeps missing gets refused rather than being allowed to walk the directory. On by default, budget and kill switch are settings. All four new settings carry controls on their settings pages. A setting that can stop the kiosk, or widen it, must not be reachable only by hand-editing rows. The installer's client-IP advice is corrected: it described the opposite of what the shipped waitress flags do, and named only GE-Enforce as the thing that breaks when the rule is absent, when the two kiosk allowlists depend on it too. --- deploy/windows/installer/shopdb-install.ps1 | 77 ++- plugins/printedparts/api/routes.py | 34 +- .../frontend/views/PrintedPartsSettings.vue | 14 + plugins/printedparts/plugin.py | 15 + plugins/usb/api/routes.py | 483 ++++++++++++++++- plugins/usb/api/selfhosted.py | 37 +- plugins/usb/frontend/views/USBSettings.vue | 59 ++- plugins/usb/plugin.py | 122 ++++- tests/test_core/test_public_read_surface.py | 7 + .../test_kiosk_network_allowlist.py | 16 +- .../test_printedparts_kiosk_admission.py | 92 ++++ .../test_plugins/test_usb_kiosk_admission.py | 499 ++++++++++++++++++ 12 files changed, 1392 insertions(+), 63 deletions(-) create mode 100644 tests/test_plugins/test_printedparts_kiosk_admission.py create mode 100644 tests/test_plugins/test_usb_kiosk_admission.py 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. +

+ + +
+