* Strip the port from the request host when matching cookie domains
GetCookieDomain suffix-matches the request host against each configured
cookie domain. When a reverse proxy rewrites the Host header to the
upstream's address, that host arrives as "host:port" and never matches,
so the request falls through to the "did not match any of the specific
cookie domains" warning and the shortest configured domain is used.
One way to hit this is Traefik's Errors middleware: it fetches a page
from a Service on port 443 with passHostHeader disabled, and forwards
the Host as "<host>:443". The request reaches oauth2-proxy correctly and
the resulting cookie is still usable, but every such request logs an
error that suggests a misconfiguration when none exists.
warnInvalidDomain, a few lines below in the same file, already calls
net.SplitHostPort on the host before comparing it to the cookie domain.
This makes GetCookieDomain consistent with it.
Tests cover a Host header with a port, an X-Forwarded-Host header with a
port, a non-standard port, and a host with a port that matches no
configured domain.
Signed-off-by: Kiril Jurbinsky <kiril@instaffo.com>
* chore: add changelog entry for #3546
Signed-off-by: Kiril Jurbinsky <kiril@instaffo.com>
* test: keep the new port entries at the end of the table
The port cases were inserted between the Host and X-Forwarded-Host
variants of the existing suffix-match case, which split a pair that
reads as one. Move them after the last existing entry instead.
Signed-off-by: Kiril Jurbinsky <kiril@instaffo.com>
* test: pin that a port part can no longer match a cookie domain
SplitHostPort does not require the port to be numeric, so a host such as
"evil.com:.cookies.test" used to suffix-match a configured cookie domain
through its port part, and the cookie was then set for that domain. Matching
now runs against the host alone, so it does not.
Covers both the Host header and X-Forwarded-Host, since GetRequestHost
returns one or the other into the same comparison.
A Host of this shape cannot be expressed through the request URL, because
http.NewRequest rejects it as an invalid port, so the table gains a rawHost
field that assigns req.Host after the request is built. That is how a server
populates it from the wire, and pkg/upstream and pkg/middleware already set
req.Host the same way in their tests.
Thanks to @Lrifton92 for spotting this.
Signed-off-by: Kiril Jurbinsky <kiril@instaffo.com>
---------
Signed-off-by: Kiril Jurbinsky <kiril@instaffo.com>
* Create session cookie when cookie-expire set 0
* Fix format
* add test
* fix lint error
* fix test code
* fix conflicted test case
* update test case of cookie expiration
* update tests of csrf cookies
* update docs
* Update docs/docs/configuration/overview.md
Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk>
---------
Co-authored-by: tanuki884 <morkazuk@fsi.co.jp>
Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk>
* Set and verify a nonce with OIDC
* Create a CSRF object to manage nonces & cookies
* Add missing generic cookie unit tests
* Add config flag to control OIDC SkipNonce
* Send hashed nonces in authentication requests
* Encrypt the CSRF cookie
* Add clarity to naming & add more helper methods
* Make CSRF an interface and keep underlying nonces private
* Add ReverseProxy scope to cookie tests
* Align to new 1.16 SameSite cookie default
* Perform SecretBytes conversion on CSRF cookie crypto
* Make state encoding signatures consistent
* Mock time in CSRF struct via Clock
* Improve InsecureSkipNonce docstring
This includes a fix for our samesite cookie parsing. The behaviour
changed in 1.16 so that the default value now leaves it empty, so it's
equivalent to not setting it (as per spec)
Reorganized the structure of the Request Utils due to their widespread use
resulting in circular imports issues (mostly because of middleware & logger).
* Allow multiple cookie domains to be specified
* Use X-Forwarded-Host, if it exists, when selecting cookie domain
* Perform cookie domain sorting in config validation phase
* Extract get domain cookies to a single function
* Update pkg/cookies/cookies.go
Co-Authored-By: Joel Speed <Joel.speed@hotmail.co.uk>
* Update changelog
Co-authored-by: Marcos Lilljedahl <marcosnils@gmail.com>
Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk>
Values of 'lax' and 'strict' can improve and mitigate
some categories of cross-site traffic tampering.
Given that the nature of this proxy is often to proxy
private tools, this is useful to take advantage of.
See: https://www.owasp.org/index.php/SameSite
kirilju