diff --git a/CHANGELOG.md b/CHANGELOG.md index 4cabfab1..fd443af0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,123 @@ # TLSPROXY Release Notes +## next + +This release fixes a number of security issues found during a security review. Some of the fixes change the +behavior of the proxy in ways that may require config changes. Please read the breaking changes below before +upgrading. + +### :warning: Breaking changes + +* **PKI server certificates must be allowed explicitly.** Users can no longer request server certificates for + arbitrary DNS names. The new `serverCertificates` option of `pki` lists the DNS names (or one-label wildcards) + that can be requested, and optionally which users or groups can request them. Without it, only client + certificates can be issued. For example: + ```yaml + pki: + - name: "EXAMPLE CA" + endpoint: https://pki-internal.example.com/certs + serverCertificates: + - dnsNames: + - "*.internal.example.com" + acl: + - bob@example.com + ``` +* **`jwksUri` must be an https URL** in `trustedIssuers`. +* **Bearer tokens and ID token cookies must have an `exp` claim.** Tokens issued by TLSPROXY always have one. +* **The Host header must match one of the backend's `serverNames`.** This was already enforced for `HTTP` and + `HTTPS` backends, but only after authentication. It is now checked first, and also for `LOCAL` and `CONSOLE` + backends. Requests with a different Host header get `421 Misdirected Request`. +* **Forwarded headers are set by the proxy.** `X-Forwarded-Host` and `X-Forwarded-Proto` are now set on forwarded + requests, and the client's `Forwarded`, `X-Forwarded-Host`, and `X-Forwarded-Proto` headers are removed. Client + headers with the same name as `X-Forwarded-For`, `X-Forwarded-Client-Cert`, `X-tlsproxy-user-id`, or any + `forwardHttpHeaders` key, ignoring case and with `_` instead of `-` (e.g. `X_tlsproxy_user_id`), are removed. + `forwardHttpHeaders` can still override or remove `X-Forwarded-Host` and `X-Forwarded-Proto`. +* **Backends can't set TLSPROXY's cookies.** `Set-Cookie` headers from backends for `TLSPROXYAUTH`, + `TLSPROXYIDTOKEN`, `TLSPROXYNONCE`, `TLSPROXYSAMLNONCE`, or `__tlsproxySid` are dropped. +* **SSH certificate authority:** + * DSA keys are rejected. + * A `ttl` that is not a positive number of seconds is rejected with `400 Bad Request`. Larger values are still + capped at `maximumCertificateLifetime`. +* **Login links expire.** The links used on the login, logout, and permission denied pages expire after a week, + and must point to the same backend. +* **Device authorization page.** The device verification page now has two steps: after entering the user code, + the page shows which client is requesting access and with which scopes, before the user can approve or deny the + request. +* **Local OIDC server:** + * The `redirect_uri` sent to the token endpoint must be the same as in the authorization request. + * When the client sends a PKCE `code_challenge`, the token request must include a matching `code_verifier`. + * An authorization request must be approved by the same user who made it. +* **Passkeys:** + * The origin of passkey assertions is checked, like it already was for registrations. + * Authenticators with a signature counter must increase it on every login. Authenticators that don't implement + a counter are not affected. +* **Connection limits:** + * The TLS handshake timeout is reduced from 2 minutes to 30 seconds. + * Connections and requests that would wait more than 30 seconds for `forwardRateLimit` are rejected instead of + waiting indefinitely. + * Each stream of a non-HTTP/3 QUIC connection counts toward `forwardRateLimit`. + * The plain HTTP server (`httpAddr`) has 10-second timeouts, and accepts at most 1000 concurrent connections, or + `maxOpen` if it is lower. + * At most 50000 pending logins are kept per identity provider, and the local OIDC server keeps at most 10000 + pending authorization or device authorization requests. URLs longer than 4096 bytes can't be used as the + destination of a login. +* **ACLs:** an ACL entry that is the name of a group is no longer also compared to the user's email address. +* **`tlsclient --ocsp`** verifies OCSP responses against the verified certificate chain, and rejects responses + signed by certificates that aren't authorized to sign OCSP responses. + +### :lock: Security fixes + +* SSO rules and exceptions were matched against the raw request path. A path like `/public/../admin` matched an + exception for `/public/`, and was then forwarded as `/admin` without authentication. +* Bearer tokens signed by any trusted issuer's key were accepted as if they had been issued by TLSPROXY. +* The SSH certificate authority issued certificates that never expire when given a negative or very large `ttl`. +* Forwarding a request to a backend with HTTP/3 crashed the proxy. +* ID tokens issued for one backend could be used on another backend with a different Host header. +* OCSP responses, stapled or fetched, could be signed by any certificate issued by the same CA, including the + revoked certificate itself. Fetched responses weren't checked for the serial number or freshness. +* Any PKI user could get a server certificate for any DNS name. +* The plain HTTP server had no timeouts, and idle connections could use up all the available file descriptors. +* Unauthenticated device authorization requests could make the local OIDC server very slow. +* Open redirects after login and after setting the ID token cookie. +* SAML responses weren't tied to the browser that started the login, which allowed an attacker to log a victim + into the attacker's account. +* Client-supplied headers could spoof the user identity header for CGI-style backends. +* The scopes required by built-in endpoints (pki, ssh, etc) weren't checked when no SSO rule matched the path. +* The CSRF check was skipped for any request with an `Authorization` header, even when cookies were used for + authentication. +* Passkey logins were accepted from other origins, and the existence of passkeys for an email address could be + detected. +* A user could approve another user's OIDC authorization request and get tokens despite the client's ACL. +* Revoking a certificate issued by the built-in PKI didn't close connections already using it. +* The built-in PKI could serve a CRL that didn't include the latest revocations. +* Memory use could be increased without limit with ECH client hellos with arbitrary server names, and with + pending login requests. + +### :star2: New features + +* New `maxOpenPerIP` option to limit the number of open connections from a single client IP address. There is no + limit by default. +* New `serverCertificates` option for `pki` (see above). +* The local OIDC server supports PKCE (RFC 7636), and advertises `code_challenge_methods_supported`. + +### :wrench: Misc + +* Hardening: + * Constant-time comparison of OIDC client secrets. + * Client-provided server names are quoted in QUIC logs. + * More secrets are redacted from the config shown on the metrics page: static `forwardHttpHeaders` values and + ECH webhook URLs. + * `proxy.mjs` only sends the CSRF token to the same origin. + * A warning is logged when `--passphrase` is used. `$TLSPROXY_PASSPHRASE` should be used instead. +* Bug fixes: + * Data races on the ECH keys and in the PKI's OCSP/CRL signing certificate rotation. + * Panic on `/.sso/` for backends without SSO. + * Possible panic when re-authorizing connections after a backend was removed. + * The session chain claim (`th`) wasn't carried over after an OIDC login. + * The ephemeral certificate manager (for testing) keeps a bounded number of certificates in memory, and reissues + expired certificates. +* New tests for all the changes above. + ## v0.25.12 ### :wrench: Misc diff --git a/README.md b/README.md index 467b11cb..f9f4ef81 100644 --- a/README.md +++ b/README.md @@ -196,6 +196,8 @@ The main configuration options are: * `hwBacked`: (Optional) Boolean. Enables hardware-backed cryptographic keys (e.g., with a TPM). * `cacheDir`: (Optional) String. Directory for storing TLS certificates, OCSP responses, etc. Defaults to a system cache directory. * `defaultServerName`: (Optional) String. Server name to use when SNI is not provided by the client. +* `maxOpen`: (Optional) Integer. Maximum number of open incoming connections. Defaults to half of the open file limit. +* `maxOpenPerIP`: (Optional) Integer. Maximum number of open incoming connections from a single client IP address. No limit by default. Keep in mind that many clients can share one IP address, e.g. behind NAT. * `logFilter`: (Optional) Object. Controls what gets logged (connections, requests, errors). * `groups`: (Optional) List of `Group` objects. Defines user groups for access control. * `backends`: (Required) List of `Backend` objects. Defines the services TLSPROXY will forward traffic to. @@ -236,7 +238,7 @@ Each `Backend` object defines a service and its behavior: * `forwardServerName`: (Optional) String. ServerName to send in TLS handshake with backend. * `forwardRootCAs`: (Optional) List of strings. CA names or PEM-encoded certificates for backend verification. * `forwardTimeout`: (Optional) Duration. Connection timeout to backend servers. -* `forwardHttpHeaders`: (Optional) Map of strings. HTTP headers to add to forwarded requests. +* `forwardHttpHeaders`: (Optional) Map of strings. HTTP headers to add to forwarded requests. Headers with the same names sent by clients are removed, including variants with `_` instead of `-`. `X-Forwarded-For`, `X-Forwarded-Host`, and `X-Forwarded-Proto` are always set by the proxy, and `Forwarded` is removed. They can be overridden here. * `forwardECH`: (Optional) Object. ECH parameters for connecting to the backend. * `pathOverrides`: (Optional) List of `PathOverride` objects. Defines different backend parameters for specific path prefixes. * `proxyProtocolVersion`: (Optional) String. Enables PROXY protocol on this backend (`v1` or `v2`). @@ -343,7 +345,7 @@ In this example: The `TrustedIssuer` object defines an external identity provider whose tokens are accepted by `tlsproxy`. This is useful for distributed authentication where multiple proxies trust each other's user identity tokens. * `issuer`: (Required) String. The expected "iss" claim value (e.g., "https://auth.example.com/"). -* `jwksUri`: (Required) String. The URL to fetch the JSON Web Key Set (JWKS). +* `jwksUri`: (Required) String. The `https` URL to fetch the JSON Web Key Set (JWKS). **Example:** @@ -489,6 +491,9 @@ Defines a local Certificate Authority: * `ocspServers`: List of strings. URLs for OCSP. * `endpoint`: String. URL for certificate management. * `admins`: List of strings. Users allowed to perform administrative tasks. +* `serverCertificates`: (Optional) List of objects. Defines which DNS names users can request in server certificates. Without it, only client certificates can be requested. + * `dnsNames`: List of strings. DNS names (e.g. `foo.example.com`) or wildcards (e.g. `*.example.com`). A wildcard matches exactly one label. + * `acl`: (Optional) List of strings. Users or groups who can request these names. If not set, all users with access to the `endpoint` can. ### SSH Certificate Authority Configuration (`ConfigSSHCertificateAuthority`) @@ -573,7 +578,7 @@ To run TLSPROXY, use the `tlsproxy` executable with the `--config` flag pointing * `--config `: Specifies the path to the configuration YAML file. * `--revoke-all-certificates `: Revokes all cached certificates. `reason` can be `unspecified`, `keyCompromise`, `superseded`, or `cessationOfOperation`. -* `--passphrase `: The passphrase to encrypt TLS keys on disk. Can also be set via `TLSPROXY_PASSPHRASE` environment variable. +* `--passphrase `: The passphrase to encrypt TLS keys on disk. Prefer the `TLSPROXY_PASSPHRASE` environment variable: command line arguments are visible to other users on the host. * `--shutdown-grace-period `: Graceful shutdown period (e.g., `1m`, `30s`). * `--use-ephemeral-certificate-manager`: (For testing) Uses an ephemeral certificate manager. * `--stdout`: Logs output to STDOUT. @@ -708,6 +713,12 @@ pki: # Optional: Admins can revoke anybody's certificates. admins: - bob@example.com + # Optional: Allow server certificates for some DNS names. + serverCertificates: + - dnsNames: + - "*.internal.example.com" + acl: + - bob@example.com ``` This example configures a local Certificate Authority (CA) named "EXAMPLE CA". This CA can be used to issue X.509 certificates for client and backend authentication within your environment. @@ -718,6 +729,7 @@ Here is a breakdown of the configuration: * `ocspServers`: Provides the endpoint for the Online Certificate Status Protocol (OCSP), offering a real-time method for checking certificate validity. * `endpoint`: Sets up a web interface at `https://pki-internal.example.com/certs` where authenticated users can request and manage their own certificates. * `admins`: Grants administrative privileges to `bob@example.com`, allowing this user to perform actions like revoking any user's certificate. +* `serverCertificates`: Allows `bob@example.com` to request server certificates for names like `foo.internal.example.com`. Without this, users can only request client certificates. You can then use this CA to enforce client certificate-based authorization for a backend. In the following example, only clients presenting a valid certificate issued by "EXAMPLE CA" for the identity `user@example.com` are allowed access. diff --git a/certmanager/cache_test.go b/certmanager/cache_test.go new file mode 100644 index 00000000..4037da82 --- /dev/null +++ b/certmanager/cache_test.go @@ -0,0 +1,58 @@ +// MIT License +// +// Copyright (c) 2026 TTBT Enterprises LLC +// Copyright (c) 2026 Robin Thellend +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all +// copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +// Package certmanager implements an X509 certificate manager that can replace +// https://pkg.go.dev/golang.org/x/crypto/acme/autocert#Manager for testing +// purposes. +// This certificate manager is a self-signed certificate authority that is not +// and should not be trusted for securing any real life communication. +package certmanager + +import ( + "fmt" + "testing" +) + +func TestGetCertCache(t *testing.T) { + cm, err := New("test", t.Logf) + if err != nil { + t.Fatalf("New: %v", err) + } + // Generating keys is slow. Use a small cache. + cm.maxCachedCerts = 5 + c1, err := cm.GetCert("foo.example.com") + if err != nil { + t.Fatalf("GetCert: %v", err) + } + if c2, err := cm.GetCert("foo.example.com"); err != nil || c2 != c1 { + t.Errorf("GetCert didn't return cached cert: %v", err) + } + for i := range cm.maxCachedCerts + 1 { + if _, err := cm.GetCert(fmt.Sprintf("n%d.example.com", i)); err != nil { + t.Fatalf("GetCert: %v", err) + } + } + if n := len(cm.certs); n > cm.maxCachedCerts { + t.Errorf("len(certs) = %d", n) + } +} diff --git a/certmanager/certmanager.go b/certmanager/certmanager.go index b80cbad7..a2030095 100644 --- a/certmanager/certmanager.go +++ b/certmanager/certmanager.go @@ -59,8 +59,13 @@ type CertManager struct { mu sync.Mutex certs map[string]*tls.Certificate + // maxCachedCerts is the maximum number of certificates kept in memory. + maxCachedCerts int } +// defaultMaxCachedCerts is the default value of maxCachedCerts. +const defaultMaxCachedCerts = 1000 + // New returns a new ephemeral certificate manager. func New(name string, logger func(string, ...interface{})) (*CertManager, error) { if logger == nil { @@ -107,6 +112,8 @@ func New(name string, logger func(string, ...interface{})) (*CertManager, error) pool: pool, logger: logger, certs: make(map[string]*tls.Certificate), + + maxCachedCerts: defaultMaxCachedCerts, }, nil } @@ -234,9 +241,12 @@ func (cm *CertManager) GetCert(name string) (*tls.Certificate, error) { } cm.mu.Lock() defer cm.mu.Unlock() - if c := cm.certs[name]; c != nil { + if c := cm.certs[name]; c != nil && time.Now().Before(c.Leaf.NotAfter.Add(-5*time.Minute)) { return c, nil } + if len(cm.certs) >= cm.maxCachedCerts { + clear(cm.certs) + } cm.logger("[%s] GetCert(%q)", cm.name, name) key, err := rsa.GenerateKey(rand.Reader, 2048) diff --git a/main.go b/main.go index 16d7fcab..ea21dd60 100644 --- a/main.go +++ b/main.go @@ -70,6 +70,11 @@ func main() { log.Fatal("--config must be set") } log.Printf("INF tlsproxy %s %s %s/%s", Version, runtime.Version(), runtime.GOOS, runtime.GOARCH) + flag.Visit(func(f *flag.Flag) { + if f.Name == "passphrase" { + log.Print("WRN --passphrase is visible to other users on this host. Use $TLSPROXY_PASSPHRASE instead.") + } + }) cfg, err := proxy.ReadConfig(*configFile) if err != nil { log.Fatalf("ERR %v", err) diff --git a/proxy/backend-http.go b/proxy/backend-http.go index 05fd3871..50f1a02f 100644 --- a/proxy/backend-http.go +++ b/proxy/backend-http.go @@ -49,15 +49,19 @@ import ( "github.com/c2FmZQ/tlsproxy/proxy/internal/cookiemanager" "github.com/c2FmZQ/tlsproxy/proxy/internal/csrf" "github.com/c2FmZQ/tlsproxy/proxy/internal/fromctx" + "github.com/c2FmZQ/tlsproxy/proxy/internal/sid" ) const ( hstsHeader = "Strict-Transport-Security" hstsValue = "max-age=2592000" // 30 days - viaHeader = "Via" - hostHeader = "Host" - xForwardedForHeader = "X-Forwarded-For" + viaHeader = "Via" + hostHeader = "Host" + xForwardedForHeader = "X-Forwarded-For" + xForwardedHostHeader = "X-Forwarded-Host" + xForwardedProtoHeader = "X-Forwarded-Proto" + forwardedHeader = "Forwarded" ) type ctxURLKeyType int @@ -86,6 +90,9 @@ func (be *Backend) localHandler() http.Handler { be.logPanic(req, r) } }() + if !be.checkRequestHost(w, req) { + return + } if !be.authenticateUser(w, &req) { return } @@ -96,6 +103,24 @@ func (be *Backend) localHandler() http.Handler { }) } +// checkRequestHost verifies that the HTTP request is directed at a server name +// that's configured for this backend. This prevents clients from using one +// server name in the TLS handshake, and then a different server name in the +// request. It must be called before anything uses req.Host. +func (be *Backend) checkRequestHost(w http.ResponseWriter, req *http.Request) bool { + if req.Host == "" { + req.Host = connServerName(req.Context().Value(connCtxKey).(anyConn)) + } + if !slices.Contains(be.ServerNames, hostFromReq(req)) { + if req.Body != nil { + req.Body.Close() + } + http.Error(w, "Misdirected Request", http.StatusMisdirectedRequest) + return false + } + return true +} + func (be *Backend) redirectPermanently(w http.ResponseWriter, req *http.Request, path string) { code := http.StatusMovedPermanently if req.Method != http.MethodGet && req.Method != http.MethodHead { @@ -208,6 +233,9 @@ func (be *Backend) reverseProxy() http.Handler { be.logPanic(req, r) } }() + if !be.checkRequestHost(w, req) { + return + } if !be.authenticateUser(w, &req) { return } @@ -215,31 +243,15 @@ func (be *Backend) reverseProxy() http.Handler { return } - // Verify that the HTTP request is directed at a server name - // that's configured for this backend. This prevents clients - // from using one server name in the TLS handshake, and then - // a different server name in the request. ctx := req.Context() serverName := connServerName(ctx.Value(connCtxKey).(anyConn)) - host := req.Host - if host == "" { - host = serverName - } - req.URL.Host = host - req.Header.Set(hostHeader, host) + req.URL.Host = req.Host + req.Header.Set(hostHeader, req.Host) req.URL.Scheme = "https" if be.Mode == ModeHTTP { req.URL.Scheme = "http" } - - if !slices.Contains(be.ServerNames, req.URL.Hostname()) { - if req.Body != nil { - req.Body.Close() - } - http.Error(w, "Misdirected Request", http.StatusMisdirectedRequest) - return - } ctx = context.WithValue(ctx, ctxURLKey, req.URL.String()) // Apply the forward rate limit. The first request was already @@ -247,7 +259,7 @@ func (be *Backend) reverseProxy() http.Handler { if conn, ok := ctx.Value(connCtxKey).(annotatedConnection); ok { if !conn.Annotation(requestFlagKey, false).(bool) { conn.SetAnnotation(requestFlagKey, true) - } else if err := be.connLimit.Wait(ctx); err != nil { + } else if err := be.waitConnLimit(ctx); err != nil { http.Error(w, "ctx", http.StatusInternalServerError) return } @@ -323,15 +335,21 @@ func (be *Backend) reverseProxy() http.Handler { if sanitizePath { req.URL.Path = cleanPath } + delHeaderVariants(req.Header, xForwardedForHeader) + delHeaderVariants(req.Header, xFCCHeader) + delHeaderVariants(req.Header, forwardedHeader) + delHeaderVariants(req.Header, xForwardedHostHeader) + delHeaderVariants(req.Header, xForwardedProtoHeader) + req.Header.Set(xForwardedHostHeader, req.Host) + req.Header.Set(xForwardedProtoHeader, "https") for k, v := range httpHeaders { + delHeaderVariants(req.Header, k) v = expandVars(v, req) if v != "" { req.Header.Set(k, v) if strings.ToLower(k) == strings.ToLower(hostHeader) { req.Host = v } - } else { - req.Header.Del(k) } } // A value of -1 for ContentLength indicates that the size of @@ -501,8 +519,6 @@ func (be *Backend) handleLocalEndpointsAndAuthorize(w http.ResponseWriter, req * } func (be *Backend) reverseProxyDirector(req *http.Request) { - req.Header.Del(xForwardedForHeader) - req.Header.Del(xFCCHeader) if req.TLS != nil && len(req.TLS.PeerCertificates) > 0 && be.ClientAuth != nil && len(be.ClientAuth.AddClientCertHeader) > 0 { addXFCCHeader(req, be.ClientAuth.AddClientCertHeader) } @@ -582,8 +598,28 @@ func (be *Backend) reverseProxyModifyResponse(resp *http.Response) error { if resp.StatusCode != http.StatusMisdirectedRequest && resp.Header.Get(hstsHeader) == "" { resp.Header.Set(hstsHeader, hstsValue) } + filterOutProxyCookies(resp.Header) if resp.StatusCode >= 200 && resp.StatusCode < 400 && resp.Header.Get("Alt-Svc") == "" { be.setAltSvc(resp.Header, req) } return nil } + +// filterOutProxyCookies removes the Set-Cookie headers that would set the +// proxy's own cookies. Backends must not be able to set them, e.g. to log users +// into another account. +func filterOutProxyCookies(h http.Header) { + cookies := h.Values("Set-Cookie") + if len(cookies) == 0 { + return + } + h.Del("Set-Cookie") + for _, c := range cookies { + name, _, _ := strings.Cut(c, "=") + name = strings.TrimSpace(name) + if cookiemanager.IsProxyCookie(name) || sid.IsCookie(name) { + continue + } + h.Add("Set-Cookie", c) + } +} diff --git a/proxy/backend-http_test.go b/proxy/backend-http_test.go index 4992dcb5..b522fe7f 100644 --- a/proxy/backend-http_test.go +++ b/proxy/backend-http_test.go @@ -24,13 +24,21 @@ package proxy import ( + "bufio" "context" + "crypto/tls" + "fmt" "net" "net/http" + "slices" + "strings" "testing" + "time" jwt "github.com/golang-jwt/jwt/v5" + "golang.org/x/time/rate" + "github.com/c2FmZQ/tlsproxy/certmanager" "github.com/c2FmZQ/tlsproxy/proxy/internal/fromctx" ) @@ -122,3 +130,359 @@ func (mockConn) ByteRateSent() float64 { func (mockConn) ByteRateReceived() float64 { return 0 } + +func TestMisdirectedRequest(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + extCA, err := certmanager.New("root-ca.example.com", t.Logf) + if err != nil { + t.Fatalf("certmanager.New: %v", err) + } + be := newHTTPServer(t, ctx, "https", nil) + + proxy := newTestProxy( + &Config{ + HTTPAddr: newPtr("localhost:0"), + TLSAddr: newPtr("localhost:0"), + CacheDir: newPtr(t.TempDir()), + MaxOpen: newPtr(100), + Backends: []*Backend{ + { + ServerNames: Strings{"local.example.com"}, + Mode: "LOCAL", + DocumentRoot: ".", + }, + { + ServerNames: Strings{"http.example.com"}, + Mode: "HTTP", + Addresses: Strings{be.String()}, + }, + { + ServerNames: Strings{"other.example.com"}, + Mode: "LOCAL", + DocumentRoot: ".", + }, + }, + }, + extCA, + ) + if err := proxy.Start(ctx); err != nil { + t.Fatalf("proxy.Start: %v", err) + } + defer proxy.Stop() + + for _, tc := range []struct { + sni, host, want string + }{ + {"local.example.com", "local.example.com", "HTTP/1.1 200 OK"}, + {"local.example.com", "local.example.com:443", "HTTP/1.1 200 OK"}, + {"local.example.com", "other.example.com", "HTTP/1.1 421 Misdirected Request"}, + {"http.example.com", "http.example.com", "HTTP/1.1 200 OK"}, + {"http.example.com", "other.example.com", "HTTP/1.1 421 Misdirected Request"}, + } { + msg := "GET /proxy.go HTTP/1.1\r\nHost: " + tc.host + "\r\nConnection: close\r\n\r\n" + got, _, err := tlsGet(tc.sni, proxy.listener.Addr().String(), msg, extCA, nil, []string{"http/1.1"}) + if err != nil { + t.Fatalf("tlsGet(%q, %q): %v", tc.sni, tc.host, err) + } + if !strings.HasPrefix(got, tc.want) { + first, _, _ := strings.Cut(got, "\r\n") + t.Errorf("SNI %q Host %q: got %q, want %q", tc.sni, tc.host, first, tc.want) + } + } +} + +func TestForwardedHeaders(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + extCA, err := certmanager.New("root-ca.example.com", t.Logf) + if err != nil { + t.Fatalf("certmanager.New: %v", err) + } + + // A backend that returns all the request headers. + l, err := net.Listen("tcp", "localhost:0") + if err != nil { + t.Fatalf("net.Listen: %v", err) + } + beServer := &http.Server{ + Handler: http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + var lines []string + for k, v := range req.Header { + lines = append(lines, fmt.Sprintf("%s: %s", k, strings.Join(v, ","))) + } + slices.Sort(lines) + fmt.Fprintln(w, strings.Join(lines, "\n")) + }), + } + go beServer.Serve(l) + defer beServer.Close() + + proxy := newTestProxy( + &Config{ + HTTPAddr: newPtr("localhost:0"), + TLSAddr: newPtr("localhost:0"), + CacheDir: newPtr(t.TempDir()), + MaxOpen: newPtr(100), + Backends: []*Backend{ + { + ServerNames: Strings{"http.example.com"}, + Mode: "HTTP", + Addresses: Strings{l.Addr().String()}, + ForwardHTTPHeaders: map[string]string{ + "X-Server-Name": "${SERVER_NAME}", + }, + }, + }, + }, + extCA, + ) + if err := proxy.Start(ctx); err != nil { + t.Fatalf("proxy.Start: %v", err) + } + defer proxy.Stop() + + msg := "GET / HTTP/1.1\r\n" + + "Host: http.example.com\r\n" + + "X_tlsproxy_user_id: admin@example.com\r\n" + + "x-tlsproxy-user-id: admin@example.com\r\n" + + "X_Server_Name: evil.example.com\r\n" + + "X_Forwarded_For: 1.2.3.4\r\n" + + "X-Forwarded-Host: evil.example.com\r\n" + + "X-Forwarded-Proto: http\r\n" + + "Forwarded: for=1.2.3.4\r\n" + + "X-Other: foo\r\n" + + "Connection: close\r\n\r\n" + got, _, err := tlsGet("http.example.com", proxy.listener.Addr().String(), msg, extCA, nil, []string{"http/1.1"}) + if err != nil { + t.Fatalf("tlsGet: %v", err) + } + _, body, _ := strings.Cut(got, "\r\n\r\n") + headers := make(map[string]string) + for _, line := range strings.Split(strings.TrimSpace(body), "\n") { + if k, v, ok := strings.Cut(line, ": "); ok { + headers[k] = v + } + } + for k, want := range map[string]string{ + "X-Server-Name": "http.example.com", + "X-Forwarded-Host": "http.example.com", + "X-Forwarded-Proto": "https", + "X-Other": "foo", + } { + if got := headers[k]; got != want { + t.Errorf("%s = %q, want %q", k, got, want) + } + } + for _, k := range []string{"X_tlsproxy_user_id", "X-Tlsproxy-User-Id", "X_server_name", "X_forwarded_for", "Forwarded"} { + if v, ok := headers[k]; ok { + t.Errorf("unexpected header %s: %q", k, v) + } + } + if got := headers["X-Forwarded-For"]; got == "" || strings.Contains(got, "1.2.3.4") { + t.Errorf("X-Forwarded-For = %q", got) + } + t.Logf("Headers:\n%s", body) +} + +func TestMaxOpenPerIP(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + extCA, err := certmanager.New("root-ca.example.com", t.Logf) + if err != nil { + t.Fatalf("certmanager.New: %v", err) + } + proxy := newTestProxy( + &Config{ + HTTPAddr: newPtr("localhost:0"), + TLSAddr: newPtr("localhost:0"), + CacheDir: newPtr(t.TempDir()), + MaxOpen: newPtr(100), + MaxOpenPerIP: newPtr(2), + Backends: []*Backend{ + { + ServerNames: Strings{"local.example.com"}, + Mode: "LOCAL", + DocumentRoot: ".", + }, + }, + }, + extCA, + ) + if err := proxy.Start(ctx); err != nil { + t.Fatalf("proxy.Start: %v", err) + } + defer proxy.Stop() + + dial := func() (*tls.Conn, error) { + conn, err := tls.Dial("tcp", proxy.listener.Addr().String(), &tls.Config{ + ServerName: "local.example.com", + RootCAs: extCA.RootCACertPool(), + NextProtos: []string{"http/1.1"}, + }) + if err != nil { + return nil, err + } + // Make sure the proxy accepted the connection. + if _, err := conn.Write([]byte("HEAD /proxy.go HTTP/1.1\r\nHost: local.example.com\r\n\r\n")); err != nil { + conn.Close() + return nil, err + } + resp, err := http.ReadResponse(bufio.NewReader(conn), &http.Request{Method: "HEAD"}) + if err != nil { + conn.Close() + return nil, err + } + resp.Body.Close() + return conn, nil + } + + c1, err := dial() + if err != nil { + t.Fatalf("dial 1: %v", err) + } + c2, err := dial() + if err != nil { + t.Fatalf("dial 2: %v", err) + } + defer c2.Close() + if c3, err := dial(); err == nil { + c3.Close() + t.Fatal("dial 3: unexpected success") + } + c1.Close() + // Wait for the proxy to see that c1 is closed. + var c4 *tls.Conn + for range 50 { + if c4, err = dial(); err == nil { + break + } + time.Sleep(100 * time.Millisecond) + } + if err != nil { + t.Fatalf("dial 4: %v", err) + } + c4.Close() +} + +func TestWaitConnLimit(t *testing.T) { + // One token every 100 seconds. + be := &Backend{connLimit: rate.NewLimiter(rate.Every(100*time.Second), 1)} + if err := be.waitConnLimit(context.Background()); err != nil { + t.Fatalf("waitConnLimit: %v", err) + } + // The next token would take longer than maxConnLimitWait. + start := time.Now() + if err := be.waitConnLimit(context.Background()); err == nil { + t.Fatal("waitConnLimit: unexpected success") + } + if d := time.Since(start); d > time.Second { + t.Errorf("waitConnLimit took %s", d) + } +} + +func TestRedactConfig(t *testing.T) { + poHeaders := map[string]string{"Authorization": "Bearer S3CR3T3"} + cfg := &Config{ + ECH: &ECH{ + WebHooks: Strings{"https://hooks.example.com/services/S3CR3T1?token=S3CR3T2"}, + }, + OIDCProviders: []*ConfigOIDC{{ClientSecret: "S3CR3T4"}}, + Backends: []*Backend{ + { + ForwardHTTPHeaders: map[string]string{ + "X-Api-Key": "S3CR3T5", + "X-User": "${JWT:email}", + }, + PathOverrides: []*PathOverride{{ForwardHTTPHeaders: &poHeaders}}, + SSO: &BackendSSO{ + LocalOIDCServer: &LocalOIDCServer{ + Clients: []*LocalOIDCClient{{ID: "foo", Secret: "S3CR3T6"}}, + }, + }, + }, + }, + } + redactConfig(cfg) + out := string(cfg.serialize()) + if strings.Contains(out, "S3CR3T") { + t.Errorf("redacted config contains secrets:\n%s", out) + } + for _, want := range []string{"https://hooks.example.com/", "${JWT:email}"} { + if !strings.Contains(out, want) { + t.Errorf("redacted config doesn't contain %q:\n%s", want, out) + } + } +} + +func TestBackendCannotSetProxyCookies(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + extCA, err := certmanager.New("root-ca.example.com", t.Logf) + if err != nil { + t.Fatalf("certmanager.New: %v", err) + } + l, err := net.Listen("tcp", "localhost:0") + if err != nil { + t.Fatalf("net.Listen: %v", err) + } + beServer := &http.Server{ + Handler: http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + for _, c := range []string{ + "TLSPROXYNONCE=evil; Domain=example.com; Path=/", + "TLSPROXYSAMLNONCE=evil; Domain=example.com; Path=/", + "TLSPROXYAUTH=evil; Path=/", + "tlsproxyidtoken=evil; Path=/", + "__tlsproxySid=evil; Path=/", + "app=ok; Path=/", + } { + w.Header().Add("Set-Cookie", c) + } + w.Write([]byte("ok")) + }), + } + go beServer.Serve(l) + defer beServer.Close() + + proxy := newTestProxy( + &Config{ + HTTPAddr: newPtr("localhost:0"), + TLSAddr: newPtr("localhost:0"), + CacheDir: newPtr(t.TempDir()), + MaxOpen: newPtr(100), + Backends: []*Backend{ + { + ServerNames: Strings{"http.example.com"}, + Mode: "HTTP", + Addresses: Strings{l.Addr().String()}, + }, + }, + }, + extCA, + ) + if err := proxy.Start(ctx); err != nil { + t.Fatalf("proxy.Start: %v", err) + } + defer proxy.Stop() + + msg := "GET / HTTP/1.1\r\nHost: http.example.com\r\nConnection: close\r\n\r\n" + got, _, err := tlsGet("http.example.com", proxy.listener.Addr().String(), msg, extCA, nil, []string{"http/1.1"}) + if err != nil { + t.Fatalf("tlsGet: %v", err) + } + resp, err := http.ReadResponse(bufio.NewReader(strings.NewReader(got)), nil) + if err != nil { + t.Fatalf("http.ReadResponse: %v", err) + } + var names []string + for _, c := range resp.Cookies() { + names = append(names, c.Name) + } + if want := []string{"app"}; !slices.Equal(names, want) { + t.Errorf("Cookies = %v, want %v", names, want) + } +} diff --git a/proxy/backend-sso.go b/proxy/backend-sso.go index 738b520c..84257b11 100644 --- a/proxy/backend-sso.go +++ b/proxy/backend-sso.go @@ -80,12 +80,15 @@ func init() { // available. It modifies the request headers and context. // It returns true if processing of the request should continue. func (be *Backend) authenticateUser(w http.ResponseWriter, req **http.Request) bool { - (*req).Header.Del(xTLSProxyUserIDHeader) + delHeaderVariants((*req).Header, xTLSProxyUserIDHeader) if be.SSO != nil { - claims, tokenHash, cont := be.checkCookies(w, *req) + claims, tokenHash, fromHeader, cont := be.checkCookies(w, *req) if !cont { return false } + if fromHeader { + *req = (*req).WithContext(fromctx.WithBearerAuth((*req).Context())) + } if claims != nil { if email, ok := claims["email"].(string); ok && email != "" { if be.SSO.SetUserIDHeader { @@ -112,7 +115,10 @@ func (be *Backend) authenticateUser(w http.ResponseWriter, req **http.Request) b return true } -func (be *Backend) checkCookies(w http.ResponseWriter, req *http.Request) (jwt.MapClaims, string, bool) { +// checkCookies returns the user's claims from the authorization header or the +// auth cookies. fromHeader is true when the claims came from a valid bearer +// token in the authorization header. +func (be *Backend) checkCookies(w http.ResponseWriter, req *http.Request) (claims jwt.MapClaims, tokenHash string, fromHeader bool, cont bool) { // If a valid ID Token is in the authorization header, use it and // ignore the cookies. if tok, err := be.SSO.cm.ValidateAuthorizationHeader(req); err == nil { @@ -121,43 +127,48 @@ func (be *Backend) checkCookies(w http.ResponseWriter, req *http.Request) (jwt.M if clientID, ok := c["client_id"].(string); ok { if email, ok := c["email"].(string); !ok || be.SSO.oidcServer == nil || !be.SSO.oidcServer.AuthorizeClient(clientID, email) { w.WriteHeader(http.StatusForbidden) - return nil, "", false + return nil, "", false, false } } - return c, "", true + return c, "", true, true } authToken, tokenHash, err := be.SSO.cm.ValidateAuthTokenCookie(req) if err != nil { - return nil, "", true + return nil, "", false, true } authClaims, ok := authToken.Claims.(jwt.MapClaims) if !ok { - return nil, "", true + return nil, "", false, true } email, ok := authClaims["email"].(string) if !ok || email == "" { - return nil, "", true + return nil, "", false, true } if !be.SSO.GenerateIDTokens { - return authClaims, tokenHash, true + return authClaims, tokenHash, false, true } if !slices.Contains(be.ServerNames, hostFromReq(req)) { - return authClaims, tokenHash, true + return authClaims, tokenHash, false, true } if err := be.SSO.cm.ValidateIDTokenCookie(req, authToken); err == nil { // Token is already set, and is valid. - return authClaims, tokenHash, true + return authClaims, tokenHash, false, true } if err := be.SSO.cm.SetIDTokenCookie(w, req, authClaims, be.aclMatcher.groupsForEmail(email)); err != nil { http.Error(w, "internal error", http.StatusInternalServerError) - return nil, "", false - } - http.Redirect(w, req, req.URL.String(), http.StatusFound) - return nil, "", false + return nil, "", false, false + } + // Use an absolute URL. A relative URL like //evil.com/ would redirect + // to another host. + u := *req.URL + u.Scheme = "https" + u.Host = req.Host + http.Redirect(w, req, u.String(), http.StatusFound) + return nil, "", false, false } func serveStatic(w http.ResponseWriter, req *http.Request, content []byte, contentType string) { @@ -212,6 +223,10 @@ func (be *Backend) serveSSOStatus(w http.ResponseWriter, req *http.Request) { enc.Encode(out) return } + if be.SSO == nil { + http.Error(w, "sso is not enabled", http.StatusNotFound) + return + } var keys []string for k := range claims { keys = append(keys, k) @@ -266,7 +281,7 @@ func (be *Backend) serveLogin(w http.ResponseWriter, req *http.Request) { return } url, claims, err := be.tm.ValidateURLToken(req, tok) - if err != nil { + if err != nil || !slices.Contains(be.ServerNames, url.Hostname()) { http.Error(w, "invalid request", http.StatusBadRequest) return } @@ -292,7 +307,7 @@ func (be *Backend) serveLogout(w http.ResponseWriter, req *http.Request) { req.ParseForm() if tokenStr := req.Form.Get("u"); tokenStr != "" { url, _, err := be.tm.ValidateURLToken(req, tokenStr) - if err != nil { + if err != nil || !slices.Contains(be.ServerNames, url.Hostname()) { http.Error(w, "invalid request", http.StatusBadRequest) return } @@ -359,6 +374,11 @@ func (be *Backend) enforceSSOPolicy(w http.ResponseWriter, req *http.Request, ov } rule := be.findSSORule(req) if rule == nil { + // No rule applies, but the scopes of local handlers still + // apply to authenticated users. + if overrideScopes != nil && fromctx.Claims(req.Context()) != nil { + return be.checkScopes(overrideScopes, w, req) + } return true } claims := fromctx.Claims(req.Context()) @@ -447,9 +467,11 @@ func pathMatches(prefixes []string, path string) bool { if len(prefixes) == 0 { return true } + // Only match the clean path. Matching the raw path too would let + // requests like /public/../admin match an exception for /public/. cleanPath := pathClean(path) for _, p := range prefixes { - if strings.HasPrefix(path, p) || strings.HasPrefix(cleanPath, p) { + if strings.HasPrefix(cleanPath, p) { return true } } diff --git a/proxy/backend-sso_test.go b/proxy/backend-sso_test.go index 0d2bad69..5294b6d6 100644 --- a/proxy/backend-sso_test.go +++ b/proxy/backend-sso_test.go @@ -31,6 +31,8 @@ import ( "net" "net/http" "net/http/httptest" + "net/url" + "strings" "testing" "time" @@ -112,6 +114,28 @@ func TestAuthenticateUser(t *testing.T) { } } +func TestIDTokenRedirect(t *testing.T) { + proxy := newBackendSSOTestProxy(t) + + req := httptest.NewRequest("GET", "https://example.com//evil.example.org/foo?a=b", nil) + // Server requests don't have a scheme and host in the URL. + req.URL.Scheme = "" + req.URL.Host = "" + if err := setAuthCookie(req, "bob@", "example.com", "https://example.com/", proxy.tokenManager); err != nil { + t.Fatalf("setAuthCookie: %v", err) + } + w := httptest.NewRecorder() + if got, want := proxy.cfg.Backends[0].authenticateUser(w, &req), false; got != want { + t.Fatalf("authenticateUser() = %v, want %v", got, want) + } + if got, want := w.Code, 302; got != want { + t.Errorf("response code = %d, want %d", got, want) + } + if got, want := w.Header().Get("Location"), "https://example.com//evil.example.org/foo?a=b"; got != want { + t.Errorf("Location = %q, want %q", got, want) + } +} + func TestAuthenticateDevice(t *testing.T) { proxy := newBackendSSOTestProxy(t) @@ -293,6 +317,56 @@ func TestEnforceSSOPolicy(t *testing.T) { t.Fatalf("response code = %d, want %d", got, want) } + // No rule matches, but the local handler's scopes still apply. + proxy.cfg.Backends[0].SSO.Rules = []*SSORule{{ + Paths: Strings{"/bar"}, + }} + for _, tc := range []struct { + scope any + want bool + }{ + {nil, true}, + {[]any{"pki"}, true}, + {[]any{"openid"}, false}, + } { + claims := jwt.MapClaims{"email": "bob@example.org"} + if tc.scope != nil { + claims["scope"] = tc.scope + } + scopeReq := req.WithContext(fromctx.WithClaims(req.Context(), claims)) + w = httptest.NewRecorder() + if got := proxy.cfg.Backends[0].enforceSSOPolicy(w, scopeReq, Strings{"pki"}); got != tc.want { + t.Errorf("enforceSSOPolicy(scope=%v) = %v, want %v", tc.scope, got, tc.want) + } + } + + // Dot-segments can't be used to match an exception or a more + // permissive rule. + proxy.cfg.Backends[0].SSO.Rules = []*SSORule{ + { + Paths: Strings{"/public/"}, + }, + { + Exceptions: Strings{"/favicon.ico"}, + ACL: &Strings{"alice@example.org"}, + }, + } + for _, p := range []string{ + "/public/../foo", + "/public/%2e%2e/foo", + "/favicon.ico/../foo", + "/favicon.ico/%2E%2E/foo", + } { + dotReq := httptest.NewRequest("GET", "https://example.com"+p, nil).WithContext(req.Context()) + w = httptest.NewRecorder() + if got, want := proxy.cfg.Backends[0].enforceSSOPolicy(w, dotReq, nil), false; got != want { + t.Errorf("encorceSSOPolicy(%q) = %v, want %v", p, got, want) + } + if got, want := w.Code, 403; got != want { + t.Errorf("response code for %q = %d, want %d", p, got, want) + } + } + // ForceReAuth fail proxy.cfg.Backends[0].SSO.Rules = []*SSORule{{ ForceReAuth: 5 * time.Minute, @@ -322,6 +396,47 @@ func TestEnforceSSOPolicy(t *testing.T) { } } +func TestServeLoginRedirectHost(t *testing.T) { + proxy := newBackendSSOTestProxy(t) + be := proxy.cfg.Backends[0] + + for _, tc := range []struct { + url string + want int + }{ + {"https://example.com/foo", http.StatusFound}, + {"https://evil.example.org/foo", http.StatusBadRequest}, + } { + req := httptest.NewRequest("GET", "https://example.com/", nil) + w := httptest.NewRecorder() + sid.SetSessionID(w, req, "") + req.Header.Set("Cookie", w.Header().Get("Set-Cookie")) + + u, err := url.Parse(tc.url) + if err != nil { + t.Fatalf("url.Parse: %v", err) + } + token, _, err := be.tm.URLToken(httptest.NewRecorder(), req, u, nil) + if err != nil { + t.Fatalf("URLToken: %v", err) + } + + for _, path := range []string{"/.sso/login?redirect=", "/.sso/logout?u="} { + req := httptest.NewRequest("GET", "https://example.com"+path+url.QueryEscape(token), nil) + req.Header.Set("Cookie", w.Header().Get("Set-Cookie")) + rec := httptest.NewRecorder() + if strings.HasPrefix(path, "/.sso/login") { + be.serveLogin(rec, req) + } else { + be.serveLogout(rec, req) + } + if got := rec.Code; got != tc.want { + t.Errorf("%s%s: code %d, want %d", path, tc.url, got, tc.want) + } + } + } +} + func newBackendSSOTestProxy(t *testing.T) *Proxy { return newTestProxy( &Config{ @@ -469,3 +584,19 @@ func (testAddr) Network() string { func (testAddr) String() string { return "12.34.56.78:90" } + +func TestSSOStatusWithoutSSO(t *testing.T) { + be := &Backend{} + + w := httptest.NewRecorder() + be.serveSSOStatus(w, httptest.NewRequest("GET", "https://example.com/.sso/", nil)) + if got, want := w.Code, http.StatusNotFound; got != want { + t.Errorf("GET: code %d, want %d", got, want) + } + + w = httptest.NewRecorder() + be.serveSSOStatus(w, httptest.NewRequest("POST", "https://example.com/.sso/", nil)) + if got, want := w.Body.String(), "null\n"; got != want { + t.Errorf("POST: body %q, want %q", got, want) + } +} diff --git a/proxy/backend.go b/proxy/backend.go index 3bc6b939..320802c0 100644 --- a/proxy/backend.go +++ b/proxy/backend.go @@ -42,6 +42,24 @@ import ( "github.com/c2FmZQ/tlsproxy/proxy/internal/netw" ) +const ( + // tlsHandshakeTimeout is the maximum amount of time allowed to + // complete a TLS handshake with a client. + tlsHandshakeTimeout = 30 * time.Second + // maxConnLimitWait is the maximum amount of time that a connection or + // request waits for the forward rate limiter. + maxConnLimitWait = 30 * time.Second +) + +// waitConnLimit waits until the forward rate limit allows a new connection or +// request. It returns an error right away if that would take longer than +// maxConnLimitWait. +func (be *Backend) waitConnLimit(ctx context.Context) error { + ctx, cancel := context.WithTimeout(ctx, maxConnLimitWait) + defer cancel() + return be.connLimit.Wait(ctx) +} + func (be *Backend) incInFlight(delta int) int { be.state.mu.Lock() defer be.state.mu.Unlock() diff --git a/proxy/config.go b/proxy/config.go index f2c97d75..bba9c330 100644 --- a/proxy/config.go +++ b/proxy/config.go @@ -180,6 +180,10 @@ type Config struct { RevokeUnusedCertificates *bool `yaml:"revokeUnusedCertificates,omitempty"` // MaxOpen is the maximum number of open incoming connections. MaxOpen *int `yaml:"maxOpen,omitempty"` + // MaxOpenPerIP is the maximum number of open incoming connections + // from a single IP address. The default is no limit. Many clients + // can share an IP address, e.g. behind NAT. + MaxOpenPerIP *int `yaml:"maxOpenPerIP,omitempty"` // AcceptTOS indicates acceptance of the Let's Encrypt Terms of Service. // See https://letsencrypt.org/repository/ AcceptTOS *bool `yaml:"acceptTOS"` @@ -501,6 +505,8 @@ type Backend struct { quicTransport io.Closer defaultLogFilter LogFilter + // tlsConfig returns the backend's TLS config. It acquires p.mu, which + // must not be held by the caller. tlsConfig func(isQUIC bool) *tls.Config clientCAs *x509.CertPool forwardRootCAs *x509.CertPool @@ -699,6 +705,22 @@ type ConfigPKI struct { // Admins is a list of users who are allowed to perform administrative // tasks on the CA, e.g. revoke any certificate. Admins Strings `yaml:"admins"` + // ServerCertificates defines which DNS names users can request in + // server certificates. When empty, server certificates can't be + // requested. + ServerCertificates []*PKIServerCertificates `yaml:"serverCertificates,omitempty"` +} + +// PKIServerCertificates defines which DNS names can be requested in server +// certificates, and by whom. +type PKIServerCertificates struct { + // DNSNames is a list of DNS names or wildcards, e.g. foo.example.com + // or *.example.com. A wildcard matches exactly one label. + DNSNames Strings `yaml:"dnsNames"` + // ACL is a list of users and groups who are allowed to request these + // DNS names. When ACL is nil, all users with access to the PKI + // endpoint can request them. + ACL *Strings `yaml:"acl,omitempty"` } // ConfigSSHCertificateAuthority defines a certificate authority. @@ -985,9 +1007,13 @@ func validateTrustedIssuers(issuers []*TrustedIssuer) error { if ti.JWKSURI == "" { return fmt.Errorf("[%d].JWKSURI must be set", i) } - if _, err := url.Parse(ti.JWKSURI); err != nil { + u, err := url.Parse(ti.JWKSURI) + if err != nil { return fmt.Errorf("[%d].JWKSURI: %v", i, err) } + if u.Scheme != "https" { + return fmt.Errorf("[%d].JWKSURI must be an https URL", i) + } } return nil } @@ -1239,6 +1265,16 @@ func (cfg *Config) Check() error { return fmt.Errorf("pki[%d].Endpoint %q: backend must have mode %s or %s, found %s", i, p.Endpoint, ModeLocal, ModeConsole, mode) } } + for j, sc := range p.ServerCertificates { + if len(sc.DNSNames) == 0 { + return fmt.Errorf("pki[%d].ServerCertificates[%d].DNSNames: must be set", i, j) + } + for _, n := range sc.DNSNames { + if !validDNSNamePattern(n) { + return fmt.Errorf("pki[%d].ServerCertificates[%d].DNSNames: invalid name %q", i, j, n) + } + } + } } sshCAs := make(map[string]bool) @@ -1589,3 +1625,23 @@ func reflectMerge(v1, v2 reflect.Value) error { return nil } } + +// validDNSNamePattern returns true if p is a DNS name, or a wildcard that +// matches one label, e.g. *.example.com. +func validDNSNamePattern(p string) bool { + name, _ := strings.CutPrefix(p, "*.") + if name == "" || len(name) > 253 { + return false + } + for _, label := range strings.Split(name, ".") { + if label == "" || len(label) > 63 || strings.HasPrefix(label, "-") || strings.HasSuffix(label, "-") { + return false + } + for _, c := range label { + if !(c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z' || c >= '0' && c <= '9' || c == '-') { + return false + } + } + } + return true +} diff --git a/proxy/config_test.go b/proxy/config_test.go index a101197b..99506cd3 100644 --- a/proxy/config_test.go +++ b/proxy/config_test.go @@ -461,3 +461,45 @@ func TestReadSplitConfig(t *testing.T) { } } } + +func TestValidateTrustedIssuers(t *testing.T) { + for _, tc := range []struct { + uri string + wantErr bool + }{ + {"https://example.com/.sso/jwks", false}, + {"http://example.com/.sso/jwks", true}, + {"example.com/.sso/jwks", true}, + } { + err := validateTrustedIssuers([]*TrustedIssuer{{Issuer: "https://example.com", JWKSURI: tc.uri}}) + if gotErr := err != nil; gotErr != tc.wantErr { + t.Errorf("validateTrustedIssuers(%q) = %v, wantErr %v", tc.uri, err, tc.wantErr) + } + } +} + +func TestValidDNSNamePattern(t *testing.T) { + for _, tc := range []struct { + pattern string + want bool + }{ + {"example.com", true}, + {"foo.example.com", true}, + {"*.example.com", true}, + {"xn--bcher-kva.example", true}, + {"", false}, + {"*", false}, + {"*.", false}, + {"**.example.com", false}, + {"foo.*.example.com", false}, + {"*foo.example.com", false}, + {"foo..example.com", false}, + {"-foo.example.com", false}, + {"foo_bar.example.com", false}, + {"foo.example.com.", false}, + } { + if got := validDNSNamePattern(tc.pattern); got != tc.want { + t.Errorf("validDNSNamePattern(%q) = %v, want %v", tc.pattern, got, tc.want) + } + } +} diff --git a/proxy/conntracker.go b/proxy/conntracker.go index d8a9bcb5..c613dd46 100644 --- a/proxy/conntracker.go +++ b/proxy/conntracker.go @@ -24,6 +24,7 @@ package proxy import ( + "net" "sync" ) @@ -69,3 +70,34 @@ func (t *connTracker) remove(c annotatedConnection) int { delete(t.conns, connKey{src: cc.LocalAddr().String(), dst: cc.RemoteAddr().String()}) return len(t.conns) } + +// ipCounter counts open connections per client IP address. +type ipCounter struct { + mu sync.Mutex + m map[string]int +} + +// inc increments the number of connections for addr's IP address. It returns +// the IP address and its new number of connections. +func (c *ipCounter) inc(addr net.Addr) (string, int) { + ip := addr.String() + if h, _, err := net.SplitHostPort(ip); err == nil { + ip = h + } + c.mu.Lock() + defer c.mu.Unlock() + if c.m == nil { + c.m = make(map[string]int) + } + c.m[ip]++ + return ip, c.m[ip] +} + +// dec decrements the number of connections for ip. +func (c *ipCounter) dec(ip string) { + c.mu.Lock() + defer c.mu.Unlock() + if c.m[ip]--; c.m[ip] <= 0 { + delete(c.m, ip) + } +} diff --git a/proxy/ech_test.go b/proxy/ech_test.go index 6e0b3d34..12361661 100644 --- a/proxy/ech_test.go +++ b/proxy/ech_test.go @@ -30,7 +30,9 @@ import ( "crypto/tls" "encoding/base64" "errors" + "fmt" "io" + "strings" "testing" "github.com/c2FmZQ/ech" @@ -237,3 +239,63 @@ func echGetQUIC(name, addr, msg string, rootCA *certmanager.CertManager, configL } return string(b), nil } + +func TestECHEventsUnknownName(t *testing.T) { + ctx := t.Context() + + extCA, err := certmanager.New("root-ca.example.com", t.Logf) + if err != nil { + t.Fatalf("certmanager.New: %v", err) + } + proxy := newTestProxy( + &Config{ + HTTPAddr: newPtr("localhost:0"), + TLSAddr: newPtr("localhost:0"), + CacheDir: newPtr(t.TempDir()), + MaxOpen: newPtr(100), + Backends: []*Backend{ + { + ServerNames: Strings{"local.example.com"}, + Mode: "LOCAL", + DocumentRoot: ".", + }, + }, + }, + extCA, + ) + if err := proxy.Start(ctx); err != nil { + t.Fatalf("proxy.Start: %v", err) + } + defer proxy.Stop() + + // ECH configs with public names that the proxy doesn't know. The proxy + // can't decrypt them, so it sees the public names in the outer + // ClientHello. + for i := range 5 { + _, conf, err := ech.NewConfig(uint8(i), []byte(fmt.Sprintf("unknown-%d.example.net", i))) + if err != nil { + t.Fatalf("ech.NewConfig: %v", err) + } + configList, err := ech.ConfigList([]ech.Config{conf}) + if err != nil { + t.Fatalf("ech.ConfigList: %v", err) + } + conn, err := tls.Dial("tcp", proxy.listener.Addr().String(), &tls.Config{ + ServerName: "local.example.com", + RootCAs: extCA.RootCACertPool(), + EncryptedClientHelloConfigList: configList, + }) + if err == nil { + conn.Close() + t.Fatalf("tls.Dial: unexpected success") + } + } + + proxy.eventsmu.Lock() + defer proxy.eventsmu.Unlock() + for k := range proxy.events { + if strings.Contains(k, "unknown-") { + t.Errorf("unexpected event %q", k) + } + } +} diff --git a/proxy/groups.go b/proxy/groups.go index 55f8bd44..8718241f 100644 --- a/proxy/groups.go +++ b/proxy/groups.go @@ -47,8 +47,17 @@ func (m *aclMatcher) groupsForEmail(email string) []string { } func (m *aclMatcher) emailMatches(acl []string, email string) bool { + if email == "" { + return false + } _, userDomain, ok := strings.Cut(email, "@") if slices.ContainsFunc(acl, func(group string) bool { + // Group names are not matched against the email directly. + // Otherwise, an identity provider that asserts an email + // equal to a group name would match all its members' ACLs. + if m.findGroup(group) != nil { + return false + } return group == email || (ok && group == "@"+userDomain) }) { return true diff --git a/proxy/groups_test.go b/proxy/groups_test.go index 9ad04491..845c731a 100644 --- a/proxy/groups_test.go +++ b/proxy/groups_test.go @@ -77,6 +77,9 @@ func TestGroups(t *testing.T) { {email: "carol@example.com", groups: []string{"group3", "group4"}}, {email: "oscar@example.com", groups: []string{"group4"}}, {email: "mike@example.NET", groups: nil}, + // An email equal to a group name doesn't match the group. + {email: "group1", groups: nil}, + {email: "", groups: nil}, } { if got, want := matcher.groupsForEmail(tc.email), tc.groups; !reflect.DeepEqual(got, want) { t.Errorf("groupsForEmail(%q) = %v, want %v", tc.email, got, want) diff --git a/proxy/http.go b/proxy/http.go index f511d554..0a357930 100644 --- a/proxy/http.go +++ b/proxy/http.go @@ -30,12 +30,37 @@ import ( "net/http" "sync" "time" + + "golang.org/x/net/netutil" ) type ctxKey int var connCtxKey ctxKey = 1 +var ( + // plainHTTPTimeout is the read and write timeout of the plain HTTP + // server, i.e. the one serving ACME challenges and redirects. + plainHTTPTimeout = 10 * time.Second + // plainHTTPMaxConns is the maximum number of concurrent connections + // to the plain HTTP server. + plainHTTPMaxConns = 1000 +) + +// startPlainHTTPServer starts the HTTP server that serves ACME challenges and +// redirects to HTTPS. +func startPlainHTTPServer(handler http.Handler, l net.Listener, maxConns int) *http.Server { + s := &http.Server{ + Handler: handler, + ReadHeaderTimeout: plainHTTPTimeout, + ReadTimeout: plainHTTPTimeout, + WriteTimeout: plainHTTPTimeout, + } + s.SetKeepAlivesEnabled(false) + go serveHTTP(s, netutil.LimitListener(l, min(maxConns, plainHTTPMaxConns))) + return s +} + func startInternalHTTPServer(handler http.Handler, conns <-chan net.Conn) *http.Server { l := &proxyListener{ ch: conns, diff --git a/proxy/http_test.go b/proxy/http_test.go new file mode 100644 index 00000000..6fb3f0bc --- /dev/null +++ b/proxy/http_test.go @@ -0,0 +1,80 @@ +// MIT License +// +// Copyright (c) 2026 TTBT Enterprises LLC +// Copyright (c) 2026 Robin Thellend +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all +// copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +package proxy + +import ( + "bufio" + "io" + "net" + "net/http" + "testing" + "time" +) + +func TestPlainHTTPServer(t *testing.T) { + defer func(d time.Duration) { plainHTTPTimeout = d }(plainHTTPTimeout) + plainHTTPTimeout = 500 * time.Millisecond + + l, err := net.Listen("tcp", "localhost:0") + if err != nil { + t.Fatalf("net.Listen: %v", err) + } + s := startPlainHTTPServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + w.Write([]byte("ok")) + }), l, 2) + defer s.Close() + + // Idle connections are closed by the server. + idle := make([]net.Conn, 2) + for i := range idle { + if idle[i], err = net.Dial("tcp", l.Addr().String()); err != nil { + t.Fatalf("net.Dial: %v", err) + } + defer idle[i].Close() + } + // The third connection is accepted after the idle ones time out. + c, err := net.Dial("tcp", l.Addr().String()) + if err != nil { + t.Fatalf("net.Dial: %v", err) + } + defer c.Close() + if _, err := c.Write([]byte("GET / HTTP/1.1\r\nHost: example.com\r\n\r\n")); err != nil { + t.Fatalf("Write: %v", err) + } + c.SetReadDeadline(time.Now().Add(5 * time.Second)) + resp, err := http.ReadResponse(bufio.NewReader(c), nil) + if err != nil { + t.Fatalf("ReadResponse: %v", err) + } + if resp.StatusCode != http.StatusOK { + t.Errorf("StatusCode = %d, want 200", resp.StatusCode) + } + + for i, ic := range idle { + ic.SetReadDeadline(time.Now().Add(5 * time.Second)) + if _, err := io.ReadAll(ic); err != nil { + t.Errorf("idle conn %d: ReadAll: %v", i, err) + } + } +} diff --git a/proxy/internal/cookiemanager/cookiemanager.go b/proxy/internal/cookiemanager/cookiemanager.go index db4661e4..4708f6e2 100644 --- a/proxy/internal/cookiemanager/cookiemanager.go +++ b/proxy/internal/cookiemanager/cookiemanager.go @@ -45,6 +45,7 @@ const ( tlsProxyAuthCookie = "TLSPROXYAUTH" tlsProxyIDTokenCookie = "TLSPROXYIDTOKEN" tlsProxyNonce = "TLSPROXYNONCE" + tlsProxySAMLNonce = "TLSPROXYSAMLNONCE" expiredAuthTokenLeeway = 7 * 24 * time.Hour defaultTokenLifetime = 20 * time.Hour @@ -230,6 +231,39 @@ func (cm *CookieManager) Nonce(w http.ResponseWriter, req *http.Request) string return "" } +// SetSAMLNonce sets a cookie that binds a SAML request to the browser. The SAML +// response is a cross-site POST, so the cookie must be SameSite=None. +func (cm *CookieManager) SetSAMLNonce(w http.ResponseWriter, nonce string) { + http.SetCookie(w, &http.Cookie{ + Name: tlsProxySAMLNonce, + Value: nonce, + Domain: cm.domain, + Path: "/", + MaxAge: 600, + SameSite: http.SameSiteNoneMode, + Secure: true, + HttpOnly: true, + }) +} + +// SAMLNonce returns the value of the cookie set by SetSAMLNonce, and deletes +// the cookie. +func (cm *CookieManager) SAMLNonce(w http.ResponseWriter, req *http.Request) string { + http.SetCookie(w, &http.Cookie{ + Name: tlsProxySAMLNonce, + Domain: cm.domain, + Path: "/", + MaxAge: -1, + SameSite: http.SameSiteNoneMode, + Secure: true, + HttpOnly: true, + }) + if c, err := req.Cookie(tlsProxySAMLNonce); err == nil { + return c.Value + } + return "" +} + func (cm *CookieManager) ClearCookies(w http.ResponseWriter) error { cookie := &http.Cookie{ Name: tlsProxyAuthCookie, @@ -271,24 +305,26 @@ func (cm *CookieManager) validateAuthToken(req *http.Request, leeway time.Durati return nil, "", errors.New("unknown key") } - var expectedIssuer string + var tok *jwt.Token if issuer == "" { // Local - expectedIssuer = cm.issuer + tok, err = cm.tm.ValidateToken(cookie.Value, + jwt.WithIssuer(cm.issuer), + jwt.WithAudience(cm.issuer), + jwt.WithExpirationRequired(), + jwt.WithLeeway(leeway), + ) } else { // Trusted if !slices.Contains(cm.trustedIssuers, issuer) { return nil, "", fmt.Errorf("issuer %q is not trusted", issuer) } - expectedIssuer = issuer + tok, err = cm.tm.ValidateRemoteToken(cookie.Value, issuer, + jwt.WithAudience(issuer), + jwt.WithExpirationRequired(), + jwt.WithLeeway(leeway), + ) } - - tok, err := cm.tm.ValidateToken(cookie.Value, - jwt.WithIssuer(expectedIssuer), - jwt.WithAudience(expectedIssuer), - jwt.WithExpirationRequired(), - jwt.WithLeeway(leeway), - ) if err != nil { return nil, "", err } @@ -347,7 +383,7 @@ func (cm *CookieManager) ValidateIDTokenCookie(req *http.Request, authToken *jwt if err != nil { return err } - tok, err := cm.tm.ValidateToken(cookie.Value, jwt.WithIssuer(cm.issuer), jwt.WithAudience(audience)) + tok, err := cm.tm.ValidateToken(cookie.Value, jwt.WithIssuer(cm.issuer), jwt.WithAudience(audience), jwt.WithExpirationRequired()) if err != nil { return err } @@ -363,7 +399,7 @@ func (cm *CookieManager) ValidateAuthorizationHeader(req *http.Request) (*jwt.To if len(h) < 7 || strings.ToUpper(h[:7]) != "BEARER " { return nil, errors.New("invalid authorization header") } - tok, err := cm.tm.ValidateToken(h[7:], jwt.WithIssuer(cm.issuer), jwt.WithAudience(audienceFromReq(req))) + tok, err := cm.tm.ValidateToken(h[7:], jwt.WithIssuer(cm.issuer), jwt.WithAudience(audienceFromReq(req)), jwt.WithExpirationRequired()) if err != nil { return nil, err } @@ -373,6 +409,17 @@ func (cm *CookieManager) ValidateAuthorizationHeader(req *http.Request) (*jwt.To return tok, nil } +// IsProxyCookie returns true if name is the name of one of the cookies set by +// the cookie manager. +func IsProxyCookie(name string) bool { + for _, n := range []string{tlsProxyAuthCookie, tlsProxyIDTokenCookie, tlsProxyNonce, tlsProxySAMLNonce} { + if strings.EqualFold(name, n) { + return true + } + } + return false +} + func FilterOutAuthTokenCookie(req *http.Request) { cookies := req.Cookies() req.Header.Del("Cookie") diff --git a/proxy/internal/cookiemanager/trusted_issuers_test.go b/proxy/internal/cookiemanager/trusted_issuers_test.go index a1d7740c..85508af1 100644 --- a/proxy/internal/cookiemanager/trusted_issuers_test.go +++ b/proxy/internal/cookiemanager/trusted_issuers_test.go @@ -154,4 +154,45 @@ func TestTrustedIssuers(t *testing.T) { if err == nil { t.Error("ValidateAuthTokenCookie(wrong proxyauth): expected error, got nil") } + + // Case E: Trusted Issuer's key, local issuer claim, as bearer token + tokenLocalIss := jwt.NewWithClaims(jwt.SigningMethodES256, jwt.MapClaims{ + "iss": "https://idp.example.com", + "aud": "https://example.com/", + "sub": "user123", + "email": "admin@example.com", + "exp": time.Now().Add(time.Hour).Unix(), + }) + tokenLocalIss.Header["kid"] = "test-kid-1" + signedTokenLocalIss, _ := tokenLocalIss.SignedString(privKey) + + reqBearer, _ := http.NewRequest("GET", "https://example.com", nil) + reqBearer.Header.Set("Authorization", "Bearer "+signedTokenLocalIss) + if _, err := cm.ValidateAuthorizationHeader(reqBearer); err == nil { + t.Error("ValidateAuthorizationHeader(trusted key, local iss): expected error, got nil") + } + + // Case F: Local key, bearer token with and without exp + localClaims := jwt.MapClaims{ + "iss": "https://idp.example.com", + "aud": "https://example.com/", + "sub": "user123", + } + signedNoExp, err := tm.CreateToken(localClaims, "") + if err != nil { + t.Fatalf("CreateToken: %v", err) + } + reqBearer.Header.Set("Authorization", "Bearer "+signedNoExp) + if _, err := cm.ValidateAuthorizationHeader(reqBearer); err == nil { + t.Error("ValidateAuthorizationHeader(no exp): expected error, got nil") + } + localClaims["exp"] = time.Now().Add(time.Hour).Unix() + signedWithExp, err := tm.CreateToken(localClaims, "") + if err != nil { + t.Fatalf("CreateToken: %v", err) + } + reqBearer.Header.Set("Authorization", "Bearer "+signedWithExp) + if _, err := cm.ValidateAuthorizationHeader(reqBearer); err != nil { + t.Errorf("ValidateAuthorizationHeader(local key, exp): %v", err) + } } diff --git a/proxy/internal/csrf/csrf.go b/proxy/internal/csrf/csrf.go index dee797ba..8e7caebb 100644 --- a/proxy/internal/csrf/csrf.go +++ b/proxy/internal/csrf/csrf.go @@ -34,7 +34,10 @@ func Check(w http.ResponseWriter, req *http.Request) bool { if req.Method != http.MethodPost { return true } - if req.Header.Get("authorization") != "" { + // Requests authenticated with a bearer token don't use cookies. + // Browsers can send other authorization headers automatically, e.g. + // cached Basic credentials. + if fromctx.BearerAuth(req.Context()) { return true } if id := sid.SessionID(nil, req); id != "" { diff --git a/proxy/internal/csrf/csrf_test.go b/proxy/internal/csrf/csrf_test.go new file mode 100644 index 00000000..d3b3ecc9 --- /dev/null +++ b/proxy/internal/csrf/csrf_test.go @@ -0,0 +1,59 @@ +// MIT License +// +// Copyright (c) 2026 TTBT Enterprises LLC +// Copyright (c) 2026 Robin Thellend +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all +// copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +package csrf + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/c2FmZQ/tlsproxy/proxy/internal/fromctx" +) + +func TestCheck(t *testing.T) { + for _, tc := range []struct { + name string + method string + auth string + bearer bool + want bool + }{ + {"GET", http.MethodGet, "", false, true}, + {"POST without token", http.MethodPost, "", false, false}, + {"POST with Basic auth", http.MethodPost, "Basic Zm9vOmJhcg==", false, false}, + {"POST with invalid bearer", http.MethodPost, "Bearer foo", false, false}, + {"POST with valid bearer", http.MethodPost, "Bearer foo", true, true}, + } { + req := httptest.NewRequest(tc.method, "https://example.com/", nil) + if tc.auth != "" { + req.Header.Set("Authorization", tc.auth) + } + if tc.bearer { + req = req.WithContext(fromctx.WithBearerAuth(req.Context())) + } + if got := Check(httptest.NewRecorder(), req); got != tc.want { + t.Errorf("%s: Check() = %v, want %v", tc.name, got, tc.want) + } + } +} diff --git a/proxy/internal/fromctx/fromctx.go b/proxy/internal/fromctx/fromctx.go index a097eaf8..17e63d81 100644 --- a/proxy/internal/fromctx/fromctx.go +++ b/proxy/internal/fromctx/fromctx.go @@ -35,6 +35,7 @@ var ( claimsKey = ctxKeyType(1) expiredClaimsKey = ctxKeyType(2) tokenHashKey = ctxKeyType(3) + bearerAuthKey = ctxKeyType(4) ) func WithClaims(ctx context.Context, v jwt.MapClaims) context.Context { @@ -69,3 +70,15 @@ func TokenHash(ctx context.Context) string { } return "" } + +// WithBearerAuth marks the request as authenticated with a bearer token. +func WithBearerAuth(ctx context.Context) context.Context { + return context.WithValue(ctx, bearerAuthKey, true) +} + +// BearerAuth returns true if the request was authenticated with a bearer +// token. +func BearerAuth(ctx context.Context) bool { + v, _ := ctx.Value(bearerAuthKey).(bool) + return v +} diff --git a/proxy/internal/idp/idp.go b/proxy/internal/idp/idp.go index 6a0ba14d..852e1a87 100644 --- a/proxy/internal/idp/idp.go +++ b/proxy/internal/idp/idp.go @@ -23,6 +23,8 @@ package idp +import "time" + type LoginOptions struct { loginHint string selectAccount bool @@ -68,3 +70,76 @@ func ApplyOptions(opts []Option) LoginOptions { } return lo } + +const ( + // MaxPendingLogins is the maximum number of pending login requests + // that an identity provider keeps track of. + MaxPendingLogins = 50000 + // MaxOriginalURLLength is the maximum length of the URL that users + // are sent back to after logging in. + MaxOriginalURLLength = 4096 + // pruneInterval is how often pending login requests are pruned. + pruneInterval = 30 * time.Second + // fullPruneInterval is how often pending login requests are pruned + // when the limit is reached. It bounds the cost of scanning the whole + // map when a flood of requests keeps it full. + fullPruneInterval = time.Second +) + +// PendingLogins keeps track of pending login requests, with a limited lifetime +// and size. It is not safe for concurrent use. +type PendingLogins[V any] struct { + ttl time.Duration + created func(V) time.Time + m map[string]V + lastPrune time.Time +} + +// NewPendingLogins returns a new PendingLogins. Entries expire ttl after the +// time returned by created. +func NewPendingLogins[V any](ttl time.Duration, created func(V) time.Time) *PendingLogins[V] { + return &PendingLogins[V]{ + ttl: ttl, + created: created, + m: make(map[string]V), + } +} + +func (p *PendingLogins[V]) expired(v V) bool { + return time.Since(p.created(v)) > p.ttl +} + +// Add adds a new entry. It returns false if there are too many pending +// requests. +func (p *PendingLogins[V]) Add(key string, v V) bool { + sincePrune := time.Since(p.lastPrune) + if sincePrune > pruneInterval || (len(p.m) >= MaxPendingLogins && sincePrune > fullPruneInterval) { + p.lastPrune = time.Now() + for k, v := range p.m { + if p.expired(v) { + delete(p.m, k) + } + } + } + if len(p.m) >= MaxPendingLogins { + return false + } + p.m[key] = v + return true +} + +// Get returns the entry for key, if it exists and isn't expired. +func (p *PendingLogins[V]) Get(key string) (V, bool) { + v, ok := p.m[key] + if ok && p.expired(v) { + delete(p.m, key) + var zero V + return zero, false + } + return v, ok +} + +// Delete deletes the entry for key. +func (p *PendingLogins[V]) Delete(key string) { + delete(p.m, key) +} diff --git a/proxy/internal/idp/idp_test.go b/proxy/internal/idp/idp_test.go new file mode 100644 index 00000000..a3f41845 --- /dev/null +++ b/proxy/internal/idp/idp_test.go @@ -0,0 +1,80 @@ +// MIT License +// +// Copyright (c) 2026 TTBT Enterprises LLC +// Copyright (c) 2026 Robin Thellend +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all +// copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +package idp + +import ( + "fmt" + "testing" + "time" +) + +func TestPendingLogins(t *testing.T) { + p := NewPendingLogins(5*time.Minute, func(t time.Time) time.Time { return t }) + + now := time.Now() + if !p.Add("fresh", now) { + t.Fatal("Add(fresh) = false") + } + if !p.Add("old", now.Add(-10*time.Minute)) { + t.Fatal("Add(old) = false") + } + if _, ok := p.Get("fresh"); !ok { + t.Error("Get(fresh) = false") + } + if _, ok := p.Get("old"); ok { + t.Error("Get(old) = true") + } + p.Delete("fresh") + if _, ok := p.Get("fresh"); ok { + t.Error("Get(fresh) after Delete = true") + } + + // Expired entries are pruned when the limit is reached, at most once + // every fullPruneInterval. + for i := range MaxPendingLogins { + if !p.Add(fmt.Sprintf("old-%d", i), now.Add(-10*time.Minute)) { + t.Fatalf("Add(old-%d) = false", i) + } + } + if p.Add("too-soon", now) { + t.Error("Add(too-soon) = true") + } + p.lastPrune = now.Add(-2 * fullPruneInterval) + if !p.Add("new", now) { + t.Fatal("Add(new) = false") + } + if got := len(p.m); got != 1 { + t.Errorf("len = %d, want 1", got) + } + + // Fresh entries are not. + for i := 1; i < MaxPendingLogins; i++ { + if !p.Add(fmt.Sprintf("new-%d", i), now) { + t.Fatalf("Add(new-%d) = false", i) + } + } + if p.Add("one-too-many", now) { + t.Error("Add(one-too-many) = true") + } +} diff --git a/proxy/internal/netw/quic.go b/proxy/internal/netw/quic.go index 8666415d..0b4e5e5a 100644 --- a/proxy/internal/netw/quic.go +++ b/proxy/internal/netw/quic.go @@ -231,11 +231,11 @@ func (c *QUICConn) ByteRateReceived() float64 { } func (c *QUICConn) HandshakeComplete() <-chan struct{} { - return c.HandshakeComplete() + return c.qc.HandshakeComplete() } func (c *QUICConn) NextConnection(ctx context.Context) (quicapi.Conn, error) { - return c.NextConnection(ctx) + return c.qc.NextConnection(ctx) } func (c *QUICConn) TLSConnectionState() tls.ConnectionState { diff --git a/proxy/internal/ocspcache/ocsp.go b/proxy/internal/ocspcache/ocsp.go index b349136a..dc9a6c0c 100644 --- a/proxy/internal/ocspcache/ocsp.go +++ b/proxy/internal/ocspcache/ocsp.go @@ -30,8 +30,10 @@ import ( "crypto/x509" "encoding/hex" "errors" + "fmt" "io" "net/http" + "slices" "time" "github.com/c2FmZQ/storage" @@ -43,6 +45,7 @@ import ( const ( ocspCacheSize = 200 ocspFile = "ocsp-cache" + ocspMaxSkew = 5 * time.Minute ) var ( @@ -134,13 +137,17 @@ func (c *OCSPCache) flush() error { func (c *OCSPCache) VerifyChains(ctx context.Context, chains [][]*x509.Certificate, stapled []byte) error { if stapled != nil && len(chains) > 0 && len(chains[0]) > 1 { cert, issuer := chains[0][0], chains[0][1] - if resp, err := ocsp.ParseResponseForCert(stapled, cert, issuer); err == nil && time.Now().Before(resp.NextUpdate) && resp.Status == ocsp.Good { + if resp, err := parseResponse(stapled, cert, issuer); err == nil && !resp.NextUpdate.IsZero() && resp.Status == ocsp.Good { hash := certHash(cert.Raw) - if resp, ok := c.cache.Get(hash); ok && resp.Status == ocsp.Revoked { + cached, ok := c.cache.Get(hash) + if ok && cached.Status == ocsp.Revoked { // Someone is playing tricks on us. return errOCSPRevoked } - c.cache.Add(hash, resp) + // Only replace the cached response with a newer one. + if !ok || resp.ThisUpdate.After(cached.ThisUpdate) { + c.cache.Add(hash, resp) + } } } var lastError error @@ -248,10 +255,34 @@ func (c *OCSPCache) fetchOneOCSP(ctx context.Context, cert, issuer *x509.Certifi c.logger.Errorf("ERR body: %v", err) return nil, errOCSPProtocol } - ocspResp, err := ocsp.ParseResponse(body, issuer) + ocspResp, err := parseResponse(body, cert, issuer) if err != nil { - c.logger.Errorf("ERR ocsp.ParseResponse for %s from %s: %v", cert.Subject, server, err) + c.logger.Errorf("ERR OCSP response for %s from %s: %v", cert.Subject, server, err) return nil, errOCSPProtocol } return ocspResp, nil } + +// parseResponse parses an OCSP response for cert, and verifies that it is +// signed by issuer, or by a delegated responder that is authorized to sign +// OCSP responses on behalf of issuer, and that it is current. +func parseResponse(raw []byte, cert, issuer *x509.Certificate) (*ocsp.Response, error) { + resp, err := ocsp.ParseResponseForCert(raw, cert, issuer) + if err != nil { + return nil, err + } + // ParseResponseForCert verifies that the responder certificate is + // signed by issuer, but not that it is authorized to sign OCSP + // responses (RFC 6960 section 4.2.2.2). + if rc := resp.Certificate; rc != nil && !rc.Equal(issuer) && !slices.Contains(rc.ExtKeyUsage, x509.ExtKeyUsageOCSPSigning) { + return nil, errors.New("responder certificate is not authorized to sign OCSP responses") + } + now := time.Now() + if resp.ThisUpdate.After(now.Add(ocspMaxSkew)) { + return nil, fmt.Errorf("response is not yet valid: %v", resp.ThisUpdate) + } + if !resp.NextUpdate.IsZero() && !now.Before(resp.NextUpdate) { + return nil, fmt.Errorf("response is expired: %v", resp.NextUpdate) + } + return resp, nil +} diff --git a/proxy/internal/ocspcache/ocsp_test.go b/proxy/internal/ocspcache/ocsp_test.go new file mode 100644 index 00000000..cf999664 --- /dev/null +++ b/proxy/internal/ocspcache/ocsp_test.go @@ -0,0 +1,241 @@ +// MIT License +// +// Copyright (c) 2026 TTBT Enterprises LLC +// Copyright (c) 2026 Robin Thellend +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all +// copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +package ocspcache + +import ( + "context" + "crypto" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" + "io" + "math/big" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/c2FmZQ/storage" + storagecrypto "github.com/c2FmZQ/storage/crypto" + "golang.org/x/crypto/ocsp" +) + +type testLogger struct { + t *testing.T +} + +func (l testLogger) Errorf(f string, args ...any) { l.t.Logf(f, args...) } +func (l testLogger) Fatalf(f string, args ...any) { l.t.Fatalf(f, args...) } + +type testCert struct { + cert *x509.Certificate + key crypto.Signer +} + +func newTestCert(t *testing.T, tmpl *x509.Certificate, parent *testCert) *testCert { + t.Helper() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("ecdsa.GenerateKey: %v", err) + } + sn, err := rand.Int(rand.Reader, big.NewInt(1<<62)) + if err != nil { + t.Fatalf("rand.Int: %v", err) + } + tmpl.SerialNumber = sn + tmpl.NotBefore = time.Now().Add(-time.Hour) + tmpl.NotAfter = time.Now().Add(time.Hour) + parentCert, parentKey := tmpl, crypto.Signer(key) + if parent != nil { + parentCert, parentKey = parent.cert, parent.key + } + der, err := x509.CreateCertificate(rand.Reader, tmpl, parentCert, key.Public(), parentKey) + if err != nil { + t.Fatalf("x509.CreateCertificate: %v", err) + } + cert, err := x509.ParseCertificate(der) + if err != nil { + t.Fatalf("x509.ParseCertificate: %v", err) + } + return &testCert{cert: cert, key: key} +} + +func newOCSPResponse(t *testing.T, cert, issuer, responder *testCert, status int, thisUpdate, nextUpdate time.Time) []byte { + t.Helper() + tmpl := ocsp.Response{ + Status: status, + SerialNumber: cert.cert.SerialNumber, + ThisUpdate: thisUpdate, + NextUpdate: nextUpdate, + } + if status == ocsp.Revoked { + tmpl.RevokedAt = thisUpdate + } + if responder != issuer { + tmpl.Certificate = responder.cert + } + raw, err := ocsp.CreateResponse(issuer.cert, responder.cert, tmpl, responder.key) + if err != nil { + t.Fatalf("ocsp.CreateResponse: %v", err) + } + return raw +} + +func TestVerifyChains(t *testing.T) { + now := time.Now() + + var serverResponse []byte + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + io.Copy(io.Discard, req.Body) + w.Header().Set("content-type", "application/ocsp-response") + w.Write(serverResponse) + })) + defer server.Close() + + ca := newTestCert(t, &x509.Certificate{ + Subject: pkix.Name{CommonName: "Test CA"}, + IsCA: true, + BasicConstraintsValid: true, + KeyUsage: x509.KeyUsageCertSign, + }, nil) + newLeaf := func() *testCert { + return newTestCert(t, &x509.Certificate{ + Subject: pkix.Name{CommonName: "leaf"}, + OCSPServer: []string{server.URL}, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth}, + }, ca) + } + other := newLeaf() + responder := newTestCert(t, &x509.Certificate{ + Subject: pkix.Name{CommonName: "OCSP responder"}, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageOCSPSigning}, + }, ca) + + for _, tc := range []struct { + name string + // The stapled response, if any. + staple func(leaf *testCert) []byte + // The response returned by the OCSP server. + fetched func(leaf *testCert) []byte + wantErr error + }{ + { + name: "Fetched good", + fetched: func(leaf *testCert) []byte { + return newOCSPResponse(t, leaf, ca, ca, ocsp.Good, now, now.Add(time.Hour)) + }, + }, + { + name: "Fetched revoked", + fetched: func(leaf *testCert) []byte { + return newOCSPResponse(t, leaf, ca, ca, ocsp.Revoked, now, now.Add(time.Hour)) + }, + wantErr: errOCSPRevoked, + }, + { + name: "Fetched good from delegated responder", + fetched: func(leaf *testCert) []byte { + return newOCSPResponse(t, leaf, ca, responder, ocsp.Good, now, now.Add(time.Hour)) + }, + }, + { + name: "Fetched good signed by leaf", + fetched: func(leaf *testCert) []byte { + return newOCSPResponse(t, leaf, ca, leaf, ocsp.Good, now, now.Add(time.Hour)) + }, + wantErr: errOCSPProtocol, + }, + { + name: "Fetched good for other cert", + fetched: func(leaf *testCert) []byte { + return newOCSPResponse(t, other, ca, ca, ocsp.Good, now, now.Add(time.Hour)) + }, + wantErr: errOCSPProtocol, + }, + { + name: "Fetched expired good", + fetched: func(leaf *testCert) []byte { + return newOCSPResponse(t, leaf, ca, ca, ocsp.Good, now.Add(-2*time.Hour), now.Add(-time.Hour)) + }, + wantErr: errOCSPProtocol, + }, + { + name: "Fetched future good", + fetched: func(leaf *testCert) []byte { + return newOCSPResponse(t, leaf, ca, ca, ocsp.Good, now.Add(time.Hour), now.Add(2*time.Hour)) + }, + wantErr: errOCSPProtocol, + }, + { + name: "Stapled good, fetched revoked", + staple: func(leaf *testCert) []byte { + return newOCSPResponse(t, leaf, ca, ca, ocsp.Good, now, now.Add(time.Hour)) + }, + fetched: func(leaf *testCert) []byte { + return newOCSPResponse(t, leaf, ca, ca, ocsp.Revoked, now, now.Add(time.Hour)) + }, + }, + { + name: "Stapled good signed by leaf, fetched revoked", + staple: func(leaf *testCert) []byte { + return newOCSPResponse(t, leaf, ca, leaf, ocsp.Good, now, now.Add(365*24*time.Hour)) + }, + fetched: func(leaf *testCert) []byte { + return newOCSPResponse(t, leaf, ca, ca, ocsp.Revoked, now, now.Add(time.Hour)) + }, + wantErr: errOCSPRevoked, + }, + { + name: "Stapled good for other cert, fetched revoked", + staple: func(leaf *testCert) []byte { + return newOCSPResponse(t, other, ca, ca, ocsp.Good, now, now.Add(time.Hour)) + }, + fetched: func(leaf *testCert) []byte { + return newOCSPResponse(t, leaf, ca, ca, ocsp.Revoked, now, now.Add(time.Hour)) + }, + wantErr: errOCSPRevoked, + }, + } { + t.Run(tc.name, func(t *testing.T) { + mk, err := storagecrypto.CreateAESMasterKeyForTest() + if err != nil { + t.Fatalf("CreateAESMasterKeyForTest: %v", err) + } + c := New(storage.New(t.TempDir(), mk), testLogger{t}) + + leaf := newLeaf() + var staple []byte + if tc.staple != nil { + staple = tc.staple(leaf) + } + serverResponse = tc.fetched(leaf) + chains := [][]*x509.Certificate{{leaf.cert, ca.cert}} + if err := c.VerifyChains(context.Background(), chains, staple); err != tc.wantErr { + t.Errorf("VerifyChains() = %v, want %v", err, tc.wantErr) + } + }) + } +} diff --git a/proxy/internal/oidc/client.go b/proxy/internal/oidc/client.go index efe32e4a..8e25f064 100644 --- a/proxy/internal/oidc/client.go +++ b/proxy/internal/oidc/client.go @@ -94,7 +94,7 @@ type ProviderClient struct { er EventRecorder mu sync.Mutex - states map[string]*oauthState + states *idp.PendingLogins[*oauthState] } type oauthState struct { @@ -112,7 +112,7 @@ func New(cfg Config, er EventRecorder, cm CookieManager) (*ProviderClient, error cfg: cfg, cm: cm, er: er, - states: make(map[string]*oauthState), + states: idp.NewPendingLogins(5*time.Minute, func(s *oauthState) time.Time { return s.Created }), } if p.cfg.DiscoveryURL != "" { resp, err := http.Get(p.cfg.DiscoveryURL) @@ -149,6 +149,10 @@ func New(cfg Config, er EventRecorder, cm CookieManager) (*ProviderClient, error func (p *ProviderClient) RequestLogin(w http.ResponseWriter, req *http.Request, originalURL string, opts ...idp.Option) { loginOptions := idp.ApplyOptions(opts) + if len(originalURL) > idp.MaxOriginalURLLength { + http.Error(w, "URL too long", http.StatusRequestURITooLong) + return + } ou, err := url.Parse(originalURL) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) @@ -168,14 +172,18 @@ func (p *ProviderClient) RequestLogin(w http.ResponseWriter, req *http.Request, codeVerifierStr := base64.RawURLEncoding.EncodeToString(codeVerifier[:]) cvh := sha256.Sum256([]byte(codeVerifierStr)) p.mu.Lock() - p.states[nonceStr] = &oauthState{ + added := p.states.Add(nonceStr, &oauthState{ Created: time.Now(), OriginalURL: originalURL, Host: ou.Host, CodeVerifier: codeVerifierStr, Depth: loginOptions.Depth(), - } + }) p.mu.Unlock() + if !added { + http.Error(w, "too many pending login requests", http.StatusServiceUnavailable) + return + } scopes := p.cfg.Scopes if len(scopes) == 0 { scopes = []string{"openid", "email"} @@ -208,13 +216,8 @@ func (p *ProviderClient) HandleCallback(w http.ResponseWriter, req *http.Request req.ParseForm() p.mu.Lock() - for k, v := range p.states { - if time.Since(v.Created) > 5*time.Minute { - delete(p.states, k) - } - } nonce := req.Form.Get("state") - state, ok := p.states[nonce] + state, ok := p.states.Get(nonce) invalid := !ok || state.Seen || nonce != p.cm.Nonce(w, req) if ok { state.Seen = true @@ -236,14 +239,14 @@ func (p *ProviderClient) HandleCallback(w http.ResponseWriter, req *http.Request form.Add("grant_type", "authorization_code") form.Add("code_verifier", state.CodeVerifier) - req, err := http.NewRequest(http.MethodPost, p.cfg.TokenEndpoint, strings.NewReader(form.Encode())) + tokenReq, err := http.NewRequest(http.MethodPost, p.cfg.TokenEndpoint, strings.NewReader(form.Encode())) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return } - req.Header.Set("content-type", "application/x-www-form-urlencoded") - req.Header.Set("accept", "application/json") - resp, err := http.DefaultClient.Do(req) + tokenReq.Header.Set("content-type", "application/x-www-form-urlencoded") + tokenReq.Header.Set("accept", "application/json") + resp, err := http.DefaultClient.Do(tokenReq) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return @@ -280,14 +283,14 @@ func (p *ProviderClient) HandleCallback(w http.ResponseWriter, req *http.Request return } } else if p.cfg.UserinfoEndpoint != "" && (data.TokenType == "" || strings.ToLower(data.TokenType) == "bearer") { - req, err := http.NewRequest(http.MethodGet, p.cfg.UserinfoEndpoint, nil) + userinfoReq, err := http.NewRequest(http.MethodGet, p.cfg.UserinfoEndpoint, nil) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return } - req.Header.Set("authorization", "Bearer "+data.AccessToken) - req.Header.Set("accept", "application/json") - resp, err := http.DefaultClient.Do(req) + userinfoReq.Header.Set("authorization", "Bearer "+data.AccessToken) + userinfoReq.Header.Set("accept", "application/json") + resp, err := http.DefaultClient.Do(userinfoReq) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return @@ -306,8 +309,8 @@ func (p *ProviderClient) HandleCallback(w http.ResponseWriter, req *http.Request claims.Nonce = nonce } p.mu.Lock() - state, ok = p.states[claims.Nonce] - delete(p.states, claims.Nonce) + state, ok = p.states.Get(claims.Nonce) + p.states.Delete(claims.Nonce) p.mu.Unlock() if !ok { p.er.Record("invalid nonce") diff --git a/proxy/internal/oidc/deviceauth.go b/proxy/internal/oidc/deviceauth.go index 8426e14d..a1489f7f 100644 --- a/proxy/internal/oidc/deviceauth.go +++ b/proxy/internal/oidc/deviceauth.go @@ -99,6 +99,12 @@ func (s *ProviderServer) ServeDeviceAuthorization(w http.ResponseWriter, req *ht now := time.Now().UTC() s.mu.Lock() + if len(s.deviceTokens) >= maxPendingRequests || len(s.deviceCodes) >= maxPendingRequests { + s.mu.Unlock() + s.opts.Logger.Errorf("ERR ServeDeviceAuthorization: too many pending requests") + http.Error(w, "too many pending requests", http.StatusServiceUnavailable) + return + } s.deviceCodes[userCode] = &deviceCodeData{ created: now, clientID: clientID, @@ -169,10 +175,27 @@ func (s *ProviderServer) ServeDeviceVerification(w http.ResponseWriter, req *htt data := struct { Email string UserCode string + Invalid bool + ClientID string + Scopes string }{ Email: email, UserCode: req.Form.Get("user_code"), } + // Show which client is requesting access, and to what, before + // the user approves the request. + if data.UserCode != "" { + s.mu.Lock() + if dc, ok := s.deviceCodes[strings.ToUpper(data.UserCode)]; ok && !expired(dc.created) { + if dt, ok := s.deviceTokens[dc.deviceCode]; ok { + data.ClientID = dc.clientID + data.Scopes = strings.Join(dt.scope, ",") + } + } + s.mu.Unlock() + data.Invalid = data.ClientID == "" + } + setNoFrameHeaders(w) w.Header().Set("content-type", "text/html; charset=utf-8") verifyTemplate.Execute(w, data) return @@ -189,8 +212,10 @@ func (s *ProviderServer) ServeDeviceVerification(w http.ResponseWriter, req *htt data, ok := s.deviceCodes[userCode] delete(s.deviceCodes, userCode) var devToken *deviceToken - if ok { + if ok && !expired(data.created) { devToken, ok = s.deviceTokens[data.deviceCode] + } else { + ok = false } if !ok { http.Error(w, "request expired", http.StatusBadRequest) diff --git a/proxy/internal/oidc/deviceauth_test.go b/proxy/internal/oidc/deviceauth_test.go new file mode 100644 index 00000000..1eee36ed --- /dev/null +++ b/proxy/internal/oidc/deviceauth_test.go @@ -0,0 +1,153 @@ +// MIT License +// +// Copyright (c) 2026 TTBT Enterprises LLC +// Copyright (c) 2026 Robin Thellend +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all +// copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +package oidc + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + "time" + + jwt "github.com/golang-jwt/jwt/v5" + + "github.com/c2FmZQ/tlsproxy/proxy/internal/fromctx" +) + +type nopRecorder struct{} + +func (nopRecorder) Record(string) {} + +func TestDeviceAuthorizationLimits(t *testing.T) { + s := NewServer(ServerOptions{ + Clients: []Client{{ID: "client"}}, + ACLMatcher: func([]string, string) bool { return true }, + EventRecorder: nopRecorder{}, + Logger: nopLogger{}, + }) + + deviceAuth := func() (int, string) { + req := httptest.NewRequest(http.MethodPost, "https://idp.example.com/device/authorization", strings.NewReader("client_id=client")) + req.Header.Set("content-type", "application/x-www-form-urlencoded") + w := httptest.NewRecorder() + s.ServeDeviceAuthorization(w, req) + var resp struct { + UserCode string `json:"user_code"` + } + json.NewDecoder(w.Body).Decode(&resp) + return w.Code, resp.UserCode + } + + // An expired request can't be used, even before it is vacuumed. + code, userCode := deviceAuth() + if code != http.StatusOK { + t.Fatalf("deviceAuth: code %d", code) + } + s.mu.Lock() + s.deviceCodes[userCode].created = time.Now().Add(-2 * codeExpiration) + s.mu.Unlock() + + form := url.Values{"user_code": {userCode}, "approve": {"true"}} + req := httptest.NewRequest(http.MethodPost, "https://idp.example.com/device/verify", strings.NewReader(form.Encode())) + req.Header.Set("content-type", "application/x-www-form-urlencoded") + req = req.WithContext(fromctx.WithClaims(req.Context(), jwt.MapClaims{"email": "bob@example.com"})) + w := httptest.NewRecorder() + s.ServeDeviceVerification(w, req) + if got, want := w.Code, http.StatusBadRequest; got != want { + t.Errorf("ServeDeviceVerification(expired) = %d, want %d", got, want) + } + + // The number of pending requests is limited. + for range maxPendingRequests { + if code, _ := deviceAuth(); code != http.StatusOK { + s.mu.Lock() + n := len(s.deviceTokens) + s.mu.Unlock() + if code != http.StatusServiceUnavailable || n < maxPendingRequests { + t.Fatalf("deviceAuth: code %d with %d pending", code, n) + } + break + } + } + if code, _ := deviceAuth(); code != http.StatusServiceUnavailable { + t.Errorf("deviceAuth: code %d, want %d", code, http.StatusServiceUnavailable) + } +} + +type nopLogger struct{} + +func (nopLogger) Errorf(string, ...any) {} + +func TestDeviceVerificationPage(t *testing.T) { + s := NewServer(ServerOptions{ + Clients: []Client{{ID: "my-client"}}, + Scopes: []string{"openid", "email"}, + ACLMatcher: func([]string, string) bool { return true }, + EventRecorder: nopRecorder{}, + Logger: nopLogger{}, + }) + + req := httptest.NewRequest(http.MethodPost, "https://idp.example.com/device/authorization", strings.NewReader("client_id=my-client&scope=openid+email")) + req.Header.Set("content-type", "application/x-www-form-urlencoded") + w := httptest.NewRecorder() + s.ServeDeviceAuthorization(w, req) + var resp struct { + UserCode string `json:"user_code"` + } + if err := json.NewDecoder(w.Body).Decode(&resp); err != nil { + t.Fatalf("json.Decode: %v", err) + } + + verify := func(userCode string) *httptest.ResponseRecorder { + req := httptest.NewRequest(http.MethodGet, "https://idp.example.com/device/verify?user_code="+url.QueryEscape(userCode), nil) + req = req.WithContext(fromctx.WithClaims(req.Context(), jwt.MapClaims{"email": "bob@example.com"})) + w := httptest.NewRecorder() + s.ServeDeviceVerification(w, req) + return w + } + + // A valid code shows the client and scopes, and the approve button. + w = verify(strings.ToLower(resp.UserCode)) + if got, want := w.Header().Get("X-Frame-Options"), "DENY"; got != want { + t.Errorf("X-Frame-Options = %q, want %q", got, want) + } + for _, want := range []string{"
my-client
", "
(openid,email)
", "oidc-approve-button"} { + if body := w.Body.String(); !strings.Contains(body, want) { + t.Errorf("Body doesn't contain %q", want) + } + } + + // An invalid code doesn't. + w = verify("0000-0000-0000") + for _, notWant := range []string{"my-client", "oidc-approve-button"} { + if body := w.Body.String(); strings.Contains(body, notWant) { + t.Errorf("Body contains %q", notWant) + } + } + if body := w.Body.String(); !strings.Contains(body, `class="invalid"`) { + t.Error("Body doesn't contain invalid class") + } +} diff --git a/proxy/internal/oidc/server.go b/proxy/internal/oidc/server.go index 927472d7..90725d97 100644 --- a/proxy/internal/oidc/server.go +++ b/proxy/internal/oidc/server.go @@ -26,6 +26,7 @@ package oidc import ( "crypto/rand" "crypto/sha256" + "crypto/subtle" _ "embed" "encoding/base64" "encoding/hex" @@ -62,6 +63,10 @@ const ( defaultTokenLifetime = time.Hour codeExpiration = 10 * time.Minute pollInterval = 5 * time.Second + vacuumInterval = 30 * time.Second + // maxPendingRequests is the maximum number of pending authorization + // requests and device authorization requests. + maxPendingRequests = 10000 ) var ( @@ -88,18 +93,26 @@ type openIDConfiguration struct { IDTokenSigningAlgValuesSupported []string `json:"id_token_signing_alg_values_supported"` ScopesSupported []string `json:"scopes_supported"` ClaimsSupported []string `json:"claims_supported"` + CodeChallengeMethodsSupported []string `json:"code_challenge_methods_supported"` } type codeData struct { created time.Time requestID string clientID string + email string + sub string redirectURI *url.URL - nonce string - state string - scopes []string - accessToken string - idToken string + // origRedirectURI is the redirect_uri of the authorization request. + origRedirectURI string + // codeChallenge and codeChallengeMethod are used for PKCE (RFC 7636). + codeChallenge string + codeChallengeMethod string + nonce string + state string + scopes []string + accessToken string + idToken string } // ServerOptions contains the parameters needed to configure a ProviderServer. @@ -154,6 +167,7 @@ type ProviderServer struct { opts ServerOptions mu sync.Mutex + lastVacuum time.Time codes map[string]*codeData deviceCodes map[string]*deviceCodeData deviceTokens map[string]*deviceToken @@ -166,22 +180,53 @@ type Client struct { ACL *[]string } +// verifyCodeChallenge verifies the PKCE code verifier, if a code challenge was +// sent with the authorization request. +func (d *codeData) verifyCodeChallenge(verifier string) bool { + if d.codeChallenge == "" { + return true + } + want := verifier + if d.codeChallengeMethod == "S256" { + h := sha256.Sum256([]byte(verifier)) + want = base64.RawURLEncoding.EncodeToString(h[:]) + } + return verifier != "" && subtle.ConstantTimeCompare([]byte(want), []byte(d.codeChallenge)) == 1 +} + +// setNoFrameHeaders prevents the page from being framed by other pages, e.g. for +// clickjacking. +func setNoFrameHeaders(w http.ResponseWriter) { + w.Header().Set("X-Frame-Options", "DENY") + w.Header().Set("Content-Security-Policy", "frame-ancestors 'none'") +} + +func expired(created time.Time) bool { + return created.Add(codeExpiration).Before(time.Now().UTC()) +} + +// vacuum removes expired requests. It scans all the pending requests at most +// once every vacuumInterval. Lookups must still check expiration. func (s *ProviderServer) vacuum() { s.mu.Lock() defer s.mu.Unlock() now := time.Now().UTC() + if now.Sub(s.lastVacuum) < vacuumInterval { + return + } + s.lastVacuum = now for k, v := range s.codes { - if v.created.Add(codeExpiration).Before(now) { + if expired(v.created) { delete(s.codes, k) } } for k, v := range s.deviceCodes { - if v.created.Add(codeExpiration).Before(now) { + if expired(v.created) { delete(s.deviceCodes, k) } } for k, v := range s.deviceTokens { - if v.created.Add(codeExpiration).Before(now) { + if expired(v.created) { delete(s.deviceTokens, k) } } @@ -209,7 +254,8 @@ func (s *ProviderServer) ServeConfig(w http.ResponseWriter, req *http.Request) { "RS256", "ES256", }, - ScopesSupported: s.opts.Scopes, + ScopesSupported: s.opts.Scopes, + CodeChallengeMethodsSupported: []string{"S256", "plain"}, ClaimsSupported: []string{ "aud", "email", @@ -268,7 +314,7 @@ func (s *ProviderServer) ServeAuthorization(w http.ResponseWriter, req *http.Req var code string var data *codeData for k, v := range s.codes { - if v.requestID == requestID { + if v.requestID == requestID && !expired(v.created) { code = k data = v break @@ -278,6 +324,13 @@ func (s *ProviderServer) ServeAuthorization(w http.ResponseWriter, req *http.Req http.Error(w, "request expired", http.StatusBadRequest) return } + // The request must be approved by the same user who made it. + // The client ACL and scopes were checked for that user. + if sub, _ := userClaims["sub"].(string); data.email != email || data.sub != sub { + s.opts.EventRecorder.Record("openid auth request approved by wrong user for " + data.clientID) + http.Error(w, "request expired", http.StatusBadRequest) + return + } if req.Form.Get("approve") != "true" && !AutoApproveForTests { s.opts.EventRecorder.Record("denied openid auth request for " + data.clientID) http.Error(w, "request was denied", http.StatusForbidden) @@ -357,6 +410,7 @@ func (s *ProviderServer) ServeAuthorization(w http.ResponseWriter, req *http.Req } // GET + sub, _ := userClaims["sub"].(string) if rt := req.Form.Get("response_type"); rt != "code" { s.opts.Logger.Errorf("ERR ServeAuthorization: invalid response_type %q", rt) http.Error(w, "invalid response_type", http.StatusBadRequest) @@ -388,6 +442,16 @@ func (s *ProviderServer) ServeAuthorization(w http.ResponseWriter, req *http.Req http.Error(w, "invalid redirect_uri", http.StatusBadRequest) return } + codeChallenge := req.Form.Get("code_challenge") + codeChallengeMethod := req.Form.Get("code_challenge_method") + if codeChallenge != "" && codeChallengeMethod == "" { + codeChallengeMethod = "plain" + } + if codeChallenge != "" && codeChallengeMethod != "S256" && codeChallengeMethod != "plain" { + s.opts.Logger.Errorf("ERR ServeAuthorization: invalid code_challenge_method %q", codeChallengeMethod) + http.Error(w, "invalid code_challenge_method", http.StatusBadRequest) + return + } b := make([]byte, 12) if _, err := io.ReadFull(rand.Reader, b); err != nil { @@ -424,14 +488,26 @@ func (s *ProviderServer) ServeAuthorization(w http.ResponseWriter, req *http.Req } s.mu.Lock() + if len(s.codes) >= maxPendingRequests { + s.mu.Unlock() + s.opts.Logger.Errorf("ERR ServeAuthorization: too many pending requests") + http.Error(w, "too many pending requests", http.StatusServiceUnavailable) + return + } s.codes[code] = &codeData{ created: time.Now().UTC(), clientID: clientID, requestID: requestID, + email: email, + sub: sub, redirectURI: ru, state: req.Form.Get("state"), nonce: req.Form.Get("nonce"), scopes: scopes, + + origRedirectURI: redirectURI, + codeChallenge: codeChallenge, + codeChallengeMethod: codeChallengeMethod, } s.mu.Unlock() @@ -452,6 +528,7 @@ func (s *ProviderServer) ServeAuthorization(w http.ResponseWriter, req *http.Req RequestID: requestID, Scopes: strings.Join(scopes, ","), } + setNoFrameHeaders(w) w.Header().Set("content-type", "text/html; charset=utf-8") authorizeTemplate.Execute(w, data) return @@ -473,7 +550,7 @@ func (s *ProviderServer) ServeToken(w http.ResponseWriter, req *http.Request) { var found bool for _, client := range s.opts.Clients { - if client.ID == clientID && client.Secret != "" && client.Secret == clientSecret && redirectURI != "" && slices.Contains(client.RedirectURI, redirectURI) { + if client.ID == clientID && client.Secret != "" && subtle.ConstantTimeCompare([]byte(client.Secret), []byte(clientSecret)) == 1 && redirectURI != "" && slices.Contains(client.RedirectURI, redirectURI) { found = true break } @@ -488,7 +565,7 @@ func (s *ProviderServer) ServeToken(w http.ResponseWriter, req *http.Request) { delete(s.codes, code) s.mu.Unlock() - if !ok || data.clientID != clientID { + if !ok || expired(data.created) || data.clientID != clientID || data.origRedirectURI != redirectURI || !data.verifyCodeChallenge(req.Form.Get("code_verifier")) { http.Error(w, "invalid request", http.StatusBadRequest) return } @@ -534,6 +611,10 @@ func (s *ProviderServer) ServeToken(w http.ResponseWriter, req *http.Request) { } s.mu.Lock() data, ok = s.deviceTokens[deviceCode] + if ok && expired(data.created) { + delete(s.deviceTokens, deviceCode) + ok = false + } defer s.mu.Unlock() var resp struct { diff --git a/proxy/internal/oidc/server_test.go b/proxy/internal/oidc/server_test.go index 4d3faf41..4b14cb8a 100644 --- a/proxy/internal/oidc/server_test.go +++ b/proxy/internal/oidc/server_test.go @@ -24,9 +24,22 @@ package oidc import ( + "crypto/sha256" + "encoding/base64" + "encoding/json" + "net/http" + "net/http/httptest" + "net/url" + "strings" "testing" + "github.com/c2FmZQ/storage" + "github.com/c2FmZQ/storage/crypto" jwt "github.com/golang-jwt/jwt/v5" + + "github.com/c2FmZQ/tlsproxy/proxy/internal/cookiemanager" + "github.com/c2FmZQ/tlsproxy/proxy/internal/fromctx" + "github.com/c2FmZQ/tlsproxy/proxy/internal/tokenmanager" ) func TestRewriteRules(t *testing.T) { @@ -87,3 +100,166 @@ func TestRewriteRules(t *testing.T) { t.Errorf("username2 = %q, want %q", got, want) } } + +func TestAuthorizationSameUser(t *testing.T) { + mk, err := crypto.CreateAESMasterKeyForTest() + if err != nil { + t.Fatalf("crypto.CreateAESMasterKeyForTest: %v", err) + } + tm, err := tokenmanager.New(storage.New(t.TempDir(), mk), nil, nil) + if err != nil { + t.Fatalf("tokenmanager.New: %v", err) + } + s := NewServer(ServerOptions{ + CookieManager: cookiemanager.New(tm, "idp", "example.com", "https://idp.example.com", 0, nil), + Clients: []Client{{ID: "client", RedirectURI: []string{"https://app.example.com/callback"}}}, + ACLMatcher: func([]string, string) bool { return true }, + GroupsForEmail: func(string) []string { return nil }, + EventRecorder: nopRecorder{}, + Logger: nopLogger{}, + }) + + withUser := func(req *http.Request, email string) *http.Request { + return req.WithContext(fromctx.WithClaims(req.Context(), jwt.MapClaims{"email": email, "sub": email})) + } + + // Alice makes the request. + args := url.Values{ + "response_type": {"code"}, + "client_id": {"client"}, + "redirect_uri": {"https://app.example.com/callback"}, + "scope": {"openid email"}, + } + req := withUser(httptest.NewRequest(http.MethodGet, "https://idp.example.com/authorization?"+args.Encode(), nil), "alice@example.com") + w := httptest.NewRecorder() + s.ServeAuthorization(w, req) + if w.Code != http.StatusOK { + t.Fatalf("GET: code %d", w.Code) + } + var requestID string + s.mu.Lock() + for _, v := range s.codes { + requestID = v.requestID + } + s.mu.Unlock() + + approve := func(email string) int { + form := url.Values{"request_id": {requestID}, "approve": {"true"}} + req := httptest.NewRequest(http.MethodPost, "https://idp.example.com/authorization", strings.NewReader(form.Encode())) + req.Header.Set("content-type", "application/x-www-form-urlencoded") + w := httptest.NewRecorder() + s.ServeAuthorization(w, withUser(req, email)) + return w.Code + } + // Bob can't approve it. + if got, want := approve("bob@example.com"), http.StatusBadRequest; got != want { + t.Errorf("approve(bob) = %d, want %d", got, want) + } + // Alice can. + if got, want := approve("alice@example.com"), http.StatusOK; got != want { + t.Errorf("approve(alice) = %d, want %d", got, want) + } +} + +func TestTokenRedirectURIAndPKCE(t *testing.T) { + mk, err := crypto.CreateAESMasterKeyForTest() + if err != nil { + t.Fatalf("crypto.CreateAESMasterKeyForTest: %v", err) + } + tm, err := tokenmanager.New(storage.New(t.TempDir(), mk), nil, nil) + if err != nil { + t.Fatalf("tokenmanager.New: %v", err) + } + redirect1 := "https://app.example.com/callback1" + redirect2 := "https://app.example.com/callback2" + s := NewServer(ServerOptions{ + CookieManager: cookiemanager.New(tm, "idp", "example.com", "https://idp.example.com", 0, nil), + Clients: []Client{{ID: "client", Secret: "secret", RedirectURI: []string{redirect1, redirect2}}}, + Scopes: []string{"openid"}, + ACLMatcher: func([]string, string) bool { return true }, + GroupsForEmail: func(string) []string { return nil }, + EventRecorder: nopRecorder{}, + Logger: nopLogger{}, + }) + withUser := func(req *http.Request) *http.Request { + return req.WithContext(fromctx.WithClaims(req.Context(), jwt.MapClaims{"email": "alice@example.com", "sub": "alice"})) + } + + verifier := "this-is-a-long-enough-code-verifier-0123456789" + h := sha256.Sum256([]byte(verifier)) + challenge := base64.RawURLEncoding.EncodeToString(h[:]) + + // getCode does the authorization request and returns the code. + getCode := func() string { + args := url.Values{ + "response_type": {"code"}, + "client_id": {"client"}, + "redirect_uri": {redirect1}, + "scope": {"openid"}, + "code_challenge": {challenge}, + "code_challenge_method": {"S256"}, + } + w := httptest.NewRecorder() + s.ServeAuthorization(w, withUser(httptest.NewRequest(http.MethodGet, "https://idp.example.com/authorization?"+args.Encode(), nil))) + if w.Code != http.StatusOK { + t.Fatalf("GET: code %d", w.Code) + } + var requestID string + s.mu.Lock() + for _, v := range s.codes { + if v.accessToken == "" { + requestID = v.requestID + } + } + s.mu.Unlock() + + form := url.Values{"request_id": {requestID}, "approve": {"true"}} + req := httptest.NewRequest(http.MethodPost, "https://idp.example.com/authorization", strings.NewReader(form.Encode())) + req.Header.Set("content-type", "application/x-www-form-urlencoded") + w = httptest.NewRecorder() + s.ServeAuthorization(w, withUser(req)) + var resp struct { + Redirect string `json:"redirect"` + } + if err := json.NewDecoder(w.Body).Decode(&resp); err != nil { + t.Fatalf("json.Decode: %v", err) + } + u, err := url.Parse(resp.Redirect) + if err != nil { + t.Fatalf("url.Parse: %v", err) + } + return u.Query().Get("code") + } + + token := func(code, redirectURI, verifier string) int { + form := url.Values{ + "grant_type": {"authorization_code"}, + "code": {code}, + "client_id": {"client"}, + "client_secret": {"secret"}, + "redirect_uri": {redirectURI}, + } + if verifier != "" { + form.Set("code_verifier", verifier) + } + req := httptest.NewRequest(http.MethodPost, "https://idp.example.com/token", strings.NewReader(form.Encode())) + req.Header.Set("content-type", "application/x-www-form-urlencoded") + w := httptest.NewRecorder() + s.ServeToken(w, req) + return w.Code + } + + for _, tc := range []struct { + name, redirectURI, verifier string + want int + }{ + {"wrong redirect_uri", redirect2, verifier, http.StatusBadRequest}, + {"missing verifier", redirect1, "", http.StatusBadRequest}, + {"wrong verifier", redirect1, "wrong-" + verifier, http.StatusBadRequest}, + {"ok", redirect1, verifier, http.StatusOK}, + } { + if got := token(getCode(), tc.redirectURI, tc.verifier); got != tc.want { + t.Errorf("%s: code %d, want %d", tc.name, got, tc.want) + } + } +} diff --git a/proxy/internal/oidc/verify-template.html b/proxy/internal/oidc/verify-template.html index 84ae844d..a7ab4798 100644 --- a/proxy/internal/oidc/verify-template.html +++ b/proxy/internal/oidc/verify-template.html @@ -32,6 +32,9 @@ #usercode { text-align: center; } +#usercode.invalid { + border: 2px solid red; +}