Header auth previously trusted any request that carried the configured
user header, with no check that the request came from a known upstream
proxy. Anyone reaching rdpgw directly could mint an authenticated
session as any user by setting the header.
Add `Header.TrustedProxies` (CIDR list) checked against `RemoteAddr`
before reading the user header. Refuse the request with 401 when the
remote is outside the allow-list. Refuse to start when header
authentication is enabled but `Header.TrustedProxies` is empty.
The CIDR allow-list gates the immediate upstream only; operators must
still configure their proxy to strip duplicate inbound copies of the
user header so a client cannot smuggle one through the trusted hop.
Documented in docs/header-authentication.md.
TestHeaderAuthRequiresTrustedProxy is a 3-case table covering: no
allow-list (refused), outside allow-list (refused), inside allow-list
(allowed). Existing TestHeaderAuthenticated cases updated to declare
trust for httptest.NewRequest's default RemoteAddr (192.0.2.1).
The .rdp file format is line-delimited (key:type:value\r\n), and an
unfiltered \r, \n, or NUL inside a string field is reinterpreted by RDP
clients as a directive boundary. A username flowing in from any of OIDC,
header auth, NTLM, or the URL-override path could therefore inject
arbitrary additional directives — e.g. `alternate shell:s:cmd.exe` — and
when RDP signing is enabled the malicious payload is signed as
authentic, producing a one-click client-side RCE on every user who
opens the file.
Strip bytes < 0x20 and 0x7F at the renderer chokepoint
(addStructToString), so every source path — caller, template file,
ApplyOverrides, anything future — passes through the same filter.
Legitimate values (usernames, base64url tokens, hostnames) contain no
such bytes, so the filter is a no-op for normal input. Stripping is
logged so operators can spot rejected input.
Adds TestStringFieldBoundaryHygiene covering CRLF in username, domain
and full address; bare LF in alternate shell; and embedded NUL.
Co-authored-by: Bolke de Bruin <bolke.debruin@metyis.com>
Introduce a generic, allow-list-gated way for the /connect endpoint to
accept RDP setting overrides via URL query parameters. Operators opt in
via client.rdpoverridablekeys; absent that allow-list, URL-driven
overrides are rejected with 400.
Override values are routed through rdp.Builder.ApplyOverrides, which
matches query keys against the rdp struct tags of RdpSettings and
validates per Go field type. Overridden fields are tracked so explicit
values always serialize even when they equal the field default. The
override pass runs before authoritative server-controlled fields
(gateway address, access token, full address, username) so those
always win.
This replaces the per-option string-splice approach considered in #181:
multimon now works via ?usemultimon=1 against an operator allow-list
containing "use multimon", and any other RDP key follows the same path
without bespoke handler code.
* Fix protocol fallback
* Use token-aware header matching, drop dead fallback log
Connection is a list header; plain equality misses legitimate
"keep-alive, Upgrade" clients. Switch to case-insensitive token
matching for the Connection/Upgrade checks.
Remove the "falling back to old protocol" log on upgrade failure.
upgrader.Upgrade commits an HTTP error response before returning, so
the follow-up legacy path cannot produce a coherent reply. The real
fallback happens at the header pre-check for clients and reverse
proxies that strip the upgrade tokens.
Add tests for the header helper and RDGOUT routing.
Running the gateway as non-tls, but using an external TLS gateway in
kubernetes+istio, I determined that the istio TLS gateway would join
messages frames into a single TCP packet. The packet read code assumed
that a single packet is a message. This is not the case for a TCP
stream, since you don't know how the frames are segmented via proxies,
etc.
The fix turned out more complex that I would have liked, but added a
number of unit tests to cover all the corner cases. Likely fragmentation
was not working correctly as well, as there was some cases that were
previously not handled.
Note that this might address issue #126 as well.
* Support for NTLM authentication added
To support NTLM authentication, a database is added as an authentication source.
Currently, only the configuration file is supported as a database.
Database authentication supports Basic and NTLM authentication protcols.
ServerConfig.BasicAuthEnabled renamed to LocalEnabled as Basic auth can be used with NTLM or Local.
