diff --git a/README.md b/README.md index 5bb03685..582d86f8 100644 --- a/README.md +++ b/README.md @@ -142,6 +142,54 @@ stripped path prefixes should specify their excluded paths in the un-prefixed form. +### Password-protecting a service + +To put a service behind an HTTP Basic password prompt, deploy it with +`--basic-auth`: + + kamal-proxy deploy service1 --target web-1:3000 --tls --host app.example.com --basic-auth admin:s3cr3t + +Requests without valid credentials get a `401` and a browser password prompt. +The password is hashed by the CLI before it is sent to the proxy, so neither +the RPC socket nor the saved state file ever sees the plaintext. + +Things worth knowing: + +* **Use it with TLS.** Basic credentials are replayable and are re-sent on + every request. On a service deployed with `--tls`, plaintext requests are + redirected to HTTPS *before* any challenge is issued, so the password is + never solicited in the clear. If you turn that redirect off with + `--tls-redirect=false`, or deploy without `--tls` at all, the proxy logs a + warning and challenges over plaintext — only do that when TLS is terminated + in front of the proxy. +* **The health check path stays open.** `GET` and `HEAD` on the configured + `--health-check-path` are served without credentials, so downstream load + balancers can still see the service drain during a deploy. Deploying with + both `--basic-auth` and a health check path of `/` is rejected, since that + would leave the service's index page public. +* **The credential is removed before forwarding.** Your application never sees + the proxy's `Authorization` header, so it cannot be logged by + `--log-request-header authorization` or read by the upstream. +* **Rollout targets inherit it.** `kamal-proxy rollout deploy` reuses the + service's stored options, so rollout traffic stays protected. +* **Redeploying without the flag removes protection.** The credential is not + sticky; a deploy that omits `--basic-auth` leaves the service open. +* **Rolling the proxy image back removes protection silently.** A binary older + than this feature ignores the stored credential and the next state save drops + it. Redeploy with `--basic-auth` after any proxy rollback. +* **The password reaches the deploy host's process table.** It is an ordinary + command-line argument, so it is visible to `ps` and to anything that logs the + command. +* **A per-path prefix is routing, not a security boundary.** You can protect + part of a site by deploying it as its own `--path-prefix` service with its own + credential, but prefix matching does not normalize paths — give the protected + prefix a target that does not also serve the same content under an + unprotected root. + +If you use `--error-pages`, add a `401.html` to that directory; otherwise the +challenge falls back to the proxy's built-in plain response. + + ### Automatic TLS Kamal Proxy can automatically obtain and renew TLS certificates for your diff --git a/ROADMAP.md b/ROADMAP.md index 804ba198..ada6f694 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -24,7 +24,7 @@ Proxy-side roadmap for the dash fork. The cross-repo release sequencing, strateg | Item | Evidence | Anchor | |---|---|---| -| Basic auth per service/path | port PR #216 (open); kamal#1604 | new `ServiceOptions` field + middleware in `createMiddleware` (`service.go:458`) | +| Basic auth per service | port PR #216 (open); kamal#1604 | DONE — `--basic-auth :` (`internal/server/basic_auth.go`). Deliberately **not** a `createMiddleware` middleware: that chain wraps `serviceRequestWithTarget`, which is where the HTTPS redirect lives, so a middleware there challenges before the 301 and the browser sends the password in cleartext. The check sits inline after `handleRedirectsIfNeeded`. Credentials are hashed CLI-side (salted SHA-256), so no plaintext crosses the RPC socket or reaches the state file. Per-path scoping is served by deploying a `--path-prefix` service with its own credential | | IP allow/deny (CIDR) | discussions #143/#144 | middleware; client addr extraction exists (`logging_middleware.go:70`) | | Per-IP rate limiting (token bucket + burst + allowlist) | rejected #20 | global chain (`server.go:211 buildHandler`) or per-service; `golang.org/x/time/rate` | | PROXY protocol | rejected #31, discussion #41 | `go-proxyproto` listener wrap in `server.go`; `run` flag | diff --git a/internal/cmd/deploy.go b/internal/cmd/deploy.go index a8259753..721dd9ce 100644 --- a/internal/cmd/deploy.go +++ b/internal/cmd/deploy.go @@ -3,6 +3,7 @@ package cmd import ( "fmt" "net/rpc" + "strings" "time" "github.com/spf13/cobra" @@ -16,6 +17,7 @@ type deployCommand struct { tlsStaging bool pathTimeouts map[string]string pathRequestTimeouts map[string]string + basicAuth string } func newDeployCommand() *deployCommand { @@ -79,6 +81,7 @@ func newDeployCommand() *deployCommand { deployCommand.cmd.Flags().Int64Var(&deployCommand.args.TargetOptions.MaxResponseBodySize, "max-response-body", server.DefaultMaxResponseBodySize, "Max size of response body when buffering (default of 0 means unlimited)") deployCommand.cmd.Flags().StringVar(&deployCommand.args.ServiceOptions.ErrorPagePath, "error-pages", "", "Path to custom error pages") deployCommand.cmd.Flags().IntSliceVar(&deployCommand.args.ServiceOptions.InterceptErrorStatuses, "intercept-errors", nil, "Replace these response statuses from the target with the proxy's error pages, as 4xx or 5xx codes (e.g. 502,503,504; default none)") + deployCommand.cmd.Flags().StringVar(&deployCommand.basicAuth, "basic-auth", "", "Require HTTP Basic credentials on every request to this service, as :. The health check path stays open. Use with --tls, or terminate TLS in front of the proxy -- Basic credentials are replayable and are sent on every request") deployCommand.cmd.Flags().StringSliceVar(&deployCommand.args.TargetOptions.LogRequestHeaders, "log-request-header", nil, "Additional request header to log (may be specified multiple times)") deployCommand.cmd.Flags().StringSliceVar(&deployCommand.args.TargetOptions.LogResponseHeaders, "log-response-header", nil, "Additional response header to log (may be specified multiple times)") @@ -130,6 +133,20 @@ func (c *deployCommand) preRun(cmd *cobra.Command, args []string) error { return err } + if c.basicAuth != "" { + username, password, err := parseBasicAuthFlag(c.basicAuth) + if err != nil { + return err + } + + // Hash here, so the plaintext credential never crosses the RPC socket + // and never reaches the state file. + c.args.ServiceOptions.BasicAuth, err = server.EncodeBasicAuthCredential(username, password) + if err != nil { + return err + } + } + if err := c.args.TargetOptions.Validate(); err != nil { return err } @@ -141,6 +158,21 @@ func (c *deployCommand) preRun(cmd *cobra.Command, args []string) error { return nil } +// parseBasicAuthFlag splits a : flag value. It cuts at the +// first colon, matching how net/http decodes the credentials a client sends: +// passwords may contain colons, usernames may not. +func parseBasicAuthFlag(value string) (string, string, error) { + username, password, found := strings.Cut(value, ":") + if !found { + return "", "", fmt.Errorf("%w: basic-auth must be given as :", server.ErrServiceOptionsInvalid) + } + if username == "" || password == "" { + return "", "", fmt.Errorf("%w: basic-auth needs both a username and a password", server.ErrServiceOptionsInvalid) + } + + return username, password, nil +} + // parsePathTimeouts converts the = flag pairs into the // server's normalized, longest-prefix-first form. The map's iteration order is // random, so normalizing here is what makes the deployed order deterministic. diff --git a/internal/cmd/deploy_test.go b/internal/cmd/deploy_test.go index 4ef84c49..9661ca59 100644 --- a/internal/cmd/deploy_test.go +++ b/internal/cmd/deploy_test.go @@ -1,6 +1,7 @@ package cmd import ( + "strings" "testing" "time" @@ -388,3 +389,82 @@ func TestDeployCommand_TargetPoolFlags(t *testing.T) { }) } } + +func TestParseBasicAuthFlag(t *testing.T) { + tests := []struct { + name string + value string + expectedUsername string + expectedPassword string + expectError bool + }{ + { + name: "a simple credential", + value: "admin:s3cr3t", + expectedUsername: "admin", + expectedPassword: "s3cr3t", + }, + { + // Passwords may contain colons; usernames may not. Splitting anywhere + // but the first colon corrupts the password. + name: "splits on the first colon only", + value: "admin:pa:ss:word", + expectedUsername: "admin", + expectedPassword: "pa:ss:word", + }, + {name: "no colon", value: "adminpass", expectError: true}, + {name: "empty username", value: ":pass", expectError: true}, + {name: "empty password", value: "admin:", expectError: true}, + {name: "colon only", value: ":", expectError: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + username, password, err := parseBasicAuthFlag(tt.value) + + if tt.expectError { + require.Error(t, err) + return + } + + require.NoError(t, err) + assert.Equal(t, tt.expectedUsername, username) + assert.Equal(t, tt.expectedPassword, password) + }) + } +} + +func TestDeployCommand_BasicAuthEncodesCredential(t *testing.T) { + cmd := newDeployCommand() + require.NoError(t, cmd.cmd.Flags().Parse([]string{"--target=web:3000", "--basic-auth=admin:s3cr3t"})) + require.NoError(t, cmd.preRun(cmd.cmd, []string{"test-service"})) + + encoded := cmd.args.ServiceOptions.BasicAuth + require.NotEmpty(t, encoded) + + // What crosses the RPC socket must be a hash, never the credential. + assert.NotContains(t, encoded, "admin") + assert.NotContains(t, encoded, "s3cr3t") + assert.True(t, strings.HasPrefix(encoded, "sha256:")) + + // And it must be something the server can read back. + require.NoError(t, server.ServiceOptions{BasicAuth: encoded}.Validate()) +} + +func TestDeployCommand_BasicAuthRejectsMalformedValues(t *testing.T) { + cmd := newDeployCommand() + require.NoError(t, cmd.cmd.Flags().Parse([]string{"--target=web:3000", "--basic-auth=adminpass"})) + + err := cmd.preRun(cmd.cmd, []string{"test-service"}) + + require.ErrorIs(t, err, server.ErrServiceOptionsInvalid) + require.ErrorContains(t, err, "basic-auth") +} + +func TestDeployCommand_BasicAuthAbsentLeavesServiceUnprotected(t *testing.T) { + cmd := newDeployCommand() + require.NoError(t, cmd.cmd.Flags().Parse([]string{"--target=web:3000"})) + + require.NoError(t, cmd.preRun(cmd.cmd, []string{"test-service"})) + assert.Empty(t, cmd.args.ServiceOptions.BasicAuth) +} diff --git a/internal/server/basic_auth.go b/internal/server/basic_auth.go new file mode 100644 index 00000000..10554014 --- /dev/null +++ b/internal/server/basic_auth.go @@ -0,0 +1,204 @@ +package server + +import ( + "crypto/rand" + "crypto/sha256" + "crypto/subtle" + "encoding/hex" + "fmt" + "log/slog" + "net/http" + "strings" +) + +const ( + // basicAuthScheme prefixes every encoded credential, so a future switch to a + // different hash can be recognized rather than guessed. + basicAuthScheme = "sha256" + + basicAuthSaltSize = 16 + basicAuthDigestSize = sha256.Size + + basicAuthRealm = `Basic realm="Restricted", charset="UTF-8"` +) + +// basicAuthCredential is a service's stored credential, ready to compare +// against what a request supplies. +// +// Deliberately not an adaptive hash: verification runs on the *unauthenticated* +// request path, so a deliberately slow one would hand an uncacheable +// CPU-exhaustion primitive to exactly the people the password is there to keep +// out. The salt is what defeats precomputation against a leaked state file; it +// does not defend a weak password against an offline dictionary attack. +type basicAuthCredential struct { + salt []byte + digest []byte + + // denyAll marks a credential that could not be read back from saved state. + // Such a service must refuse every request rather than serve unprotected. + denyAll bool +} + +// EncodeBasicAuthCredential hashes a username and password for storage. The CLI +// calls this, so the plaintext credential never crosses the RPC socket and +// never reaches the state file. +func EncodeBasicAuthCredential(username, password string) (string, error) { + if username == "" { + return "", fmt.Errorf("%w: basic auth username cannot be empty", ErrServiceOptionsInvalid) + } + if password == "" { + return "", fmt.Errorf("%w: basic auth password cannot be empty", ErrServiceOptionsInvalid) + } + if strings.Contains(username, ":") { + return "", fmt.Errorf("%w: basic auth username cannot contain a colon", ErrServiceOptionsInvalid) + } + + salt := make([]byte, basicAuthSaltSize) + if _, err := rand.Read(salt); err != nil { + return "", fmt.Errorf("unable to generate a basic auth salt: %w", err) + } + + return fmt.Sprintf("%s:%s:%s", basicAuthScheme, + hex.EncodeToString(salt), + hex.EncodeToString(basicAuthDigest(salt, username, password)), + ), nil +} + +// Private + +func basicAuthDigest(salt []byte, username, password string) []byte { + hash := sha256.New() + hash.Write(salt) + hash.Write([]byte(username)) + hash.Write([]byte{':'}) + hash.Write([]byte(password)) + + return hash.Sum(nil) +} + +func parseBasicAuthCredential(encoded string) (*basicAuthCredential, error) { + scheme, rest, found := strings.Cut(encoded, ":") + if !found || scheme != basicAuthScheme { + return nil, fmt.Errorf("basic auth credential is not a %s value", basicAuthScheme) + } + + rawSalt, rawDigest, found := strings.Cut(rest, ":") + if !found { + return nil, fmt.Errorf("basic auth credential is missing its digest") + } + + salt, err := hex.DecodeString(rawSalt) + if err != nil || len(salt) != basicAuthSaltSize { + return nil, fmt.Errorf("basic auth credential has an unreadable salt") + } + + digest, err := hex.DecodeString(rawDigest) + if err != nil || len(digest) != basicAuthDigestSize { + return nil, fmt.Errorf("basic auth credential has an unreadable digest") + } + + return &basicAuthCredential{salt: salt, digest: digest}, nil +} + +// matches reports whether the supplied credentials are the stored ones. The +// comparison is constant-time, and both sides are fixed-length digests, so +// neither the username nor the password leaks its length. +func (c *basicAuthCredential) matches(username, password string) bool { + if c == nil || c.denyAll { + return false + } + + return subtle.ConstantTimeCompare(basicAuthDigest(c.salt, username, password), c.digest) == 1 +} + +func (so ServiceOptions) validateBasicAuth() error { + if so.BasicAuth == "" { + return nil + } + + if _, err := parseBasicAuthCredential(so.BasicAuth); err != nil { + return fmt.Errorf("%w: %s", ErrServiceOptionsInvalid, err) + } + + return nil +} + +// validateBasicAuthHealthCheck rejects the one health check path that would +// quietly unprotect the service. The carve-out below keys off the health check +// path, so pointing it at the root would exempt the service's index page. Note +// that a stripped --path-prefix also resolves to the root, so this covers both. +func validateBasicAuthHealthCheck(options ServiceOptions, targetOptions TargetOptions) error { + if options.BasicAuth == "" { + return nil + } + + path := targetOptions.HealthCheckConfig.Path + if path == "" || path == rootPath { + return fmt.Errorf("%w: health-check-path cannot be %q when basic auth is enabled, as that path is served without credentials", ErrServiceOptionsInvalid, rootPath) + } + + return nil +} + +// resolveBasicAuth prepares the stored credential for serving. It never returns +// an error: this runs from initialize, which runs while decoding saved state, +// and failing there would abort the decode of every other service too. +func (s *Service) resolveBasicAuth(options ServiceOptions) *basicAuthCredential { + if options.BasicAuth == "" { + return nil + } + + credential, err := parseBasicAuthCredential(options.BasicAuth) + if err != nil { + slog.Error("Unable to read the stored basic auth credential; denying every request to this service", "service", s.name, "error", err) + return &basicAuthCredential{denyAll: true} + } + + switch { + case !options.TLSEnabled: + slog.Warn("Basic auth is enabled without TLS: credentials will cross the wire in cleartext", "service", s.name) + case !options.TLSRedirect: + slog.Warn("Basic auth is enabled with TLS redirection off: plaintext requests will be challenged in the clear", "service", s.name) + default: + slog.Info("Basic auth enabled", "service", s.name) + } + + return credential +} + +// rejectUnauthenticated challenges a request that does not carry this service's +// credentials, reporting whether it handled the response. +// +// It is called from serviceRequestWithTarget rather than from a middleware in +// createMiddleware, and the position is load-bearing: the HTTPS redirect lives +// inside that same handler, so every middleware slot runs *before* it. A +// challenge issued from up there would make the browser send the password in +// cleartext before the redirect to https ever fired. +func (s *Service) rejectUnauthenticated(w http.ResponseWriter, r *http.Request) bool { + if s.basicAuth == nil { + return false + } + + // Take the credential off the request before anything can forward or log it, + // and before the exemptions below return: browsers replay cached credentials + // to every path in the protection space, including the health check one. + // HTTP carries a single Authorization header, so a service behind basic auth + // cannot also pass one through to its target. + username, password, hasCredentials := r.BasicAuth() + r.Header.Del("Authorization") + + // The ACME and TLS on-demand probes address the proxy itself, and downstream + // load balancers must be able to see this service drain during a deploy. + if isInternalRequest(r) || s.targetOptions.IsHealthCheckRequest(r) { + return false + } + + if hasCredentials && s.basicAuth.matches(username, password) { + return false + } + + w.Header().Set("WWW-Authenticate", basicAuthRealm) + SetErrorResponse(w, r, http.StatusUnauthorized, nil) + + return true +} diff --git a/internal/server/basic_auth_test.go b/internal/server/basic_auth_test.go new file mode 100644 index 00000000..e5008fcd --- /dev/null +++ b/internal/server/basic_auth_test.go @@ -0,0 +1,557 @@ +package server + +import ( + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "sync/atomic" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ( + testAuthUser = "admin" + testAuthPassword = "s3cr3t" +) + +func testEncodedCredential(t testing.TB, username, password string) string { + t.Helper() + + encoded, err := EncodeBasicAuthCredential(username, password) + require.NoError(t, err) + + return encoded +} + +// testProtectedService deploys a service behind basic auth and returns a +// handler wired the way the real server wires one. +func testProtectedService(t *testing.T, options ServiceOptions, handler http.HandlerFunc) http.Handler { + t.Helper() + + router := testRouter(t) + _, target := testBackendWithHandler(t, handler) + + if options.BasicAuth == "" { + options.BasicAuth = testEncodedCredential(t, testAuthUser, testAuthPassword) + } + + require.NoError(t, router.DeployService("service1", []string{target}, defaultEmptyReaders, + options, defaultTargetOptions, defaultDeploymentOptions)) + + return testRoutedHandler(t, router) +} + +func testAuthRequest(handler http.Handler, req *http.Request) *http.Response { + w := httptest.NewRecorder() + handler.ServeHTTP(w, req) + + return w.Result() +} + +func testAuthBody(t testing.TB, resp *http.Response) string { + t.Helper() + + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + + return string(body) +} + +func TestBasicAuthCredential_EncodeAndMatch(t *testing.T) { + tests := []struct { + name string + username string + password string + suppliedUsername string + suppliedPassword string + expectedMatch bool + }{ + {"correct credentials", testAuthUser, testAuthPassword, testAuthUser, testAuthPassword, true}, + {"wrong password", testAuthUser, testAuthPassword, testAuthUser, "nope", false}, + {"wrong username", testAuthUser, testAuthPassword, "root", testAuthPassword, false}, + {"empty supplied password", testAuthUser, testAuthPassword, testAuthUser, "", false}, + {"password containing colons", testAuthUser, "pa:ss:word", testAuthUser, "pa:ss:word", true}, + {"unicode password", testAuthUser, "hüñtér2·√", testAuthUser, "hüñtér2·√", true}, + {"username is not swappable with password", "a", "b", "b", "a", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + credential, err := parseBasicAuthCredential(testEncodedCredential(t, tt.username, tt.password)) + require.NoError(t, err) + + assert.Equal(t, tt.expectedMatch, credential.matches(tt.suppliedUsername, tt.suppliedPassword)) + }) + } +} + +func TestBasicAuthCredential_SaltDiffersPerEncode(t *testing.T) { + first := testEncodedCredential(t, testAuthUser, testAuthPassword) + second := testEncodedCredential(t, testAuthUser, testAuthPassword) + + // A shared salt would let one leaked state file confirm that two services + // use the same password. + assert.NotEqual(t, first, second) + + for _, encoded := range []string{first, second} { + credential, err := parseBasicAuthCredential(encoded) + require.NoError(t, err) + assert.True(t, credential.matches(testAuthUser, testAuthPassword)) + } +} + +func TestBasicAuthCredential_EncodeRejectsInvalidInput(t *testing.T) { + tests := []struct { + name string + username string + password string + }{ + {"empty username", "", testAuthPassword}, + {"empty password", testAuthUser, ""}, + {"username containing a colon", "ad:min", testAuthPassword}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := EncodeBasicAuthCredential(tt.username, tt.password) + require.Error(t, err) + }) + } +} + +func TestBasicAuthCredential_ParseRejectsMalformedValues(t *testing.T) { + tests := []struct { + name string + encoded string + }{ + {"garbage", "garbage"}, + {"too few parts", "sha256:abcd"}, + {"unknown scheme", "argon2id:aabb:ccdd"}, + {"non-hex salt", "sha256:zzzz:" + strings.Repeat("ab", 32)}, + {"short salt", "sha256:abcd:" + strings.Repeat("ab", 32)}, + {"non-hex digest", "sha256:" + strings.Repeat("ab", 16) + ":zzzz"}, + {"short digest", "sha256:" + strings.Repeat("ab", 16) + ":abcd"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := parseBasicAuthCredential(tt.encoded) + require.Error(t, err) + }) + } +} + +func TestServiceOptions_ValidateBasicAuth(t *testing.T) { + tests := []struct { + name string + basicAuth string + expectError bool + }{ + {"absent", "", false}, + {"valid", testEncodedCredential(t, testAuthUser, testAuthPassword), false}, + {"malformed", "sha256:zz:zz", true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + options := defaultServiceOptions + options.BasicAuth = tt.basicAuth + + err := options.Validate() + + if tt.expectError { + require.ErrorIs(t, err, ErrServiceOptionsInvalid) + return + } + require.NoError(t, err) + }) + } +} + +func TestBasicAuth_ChallengesUnauthenticatedRequests(t *testing.T) { + var reachedTarget atomic.Int64 + + handler := testProtectedService(t, defaultServiceOptions, func(w http.ResponseWriter, r *http.Request) { + // Deploying runs a health check against the backend, so only count the + // requests this test actually proxies. + if r.URL.Path != DefaultHealthCheckPath { + reachedTarget.Add(1) + } + w.Write([]byte("secret")) + }) + + resp := testAuthRequest(handler, httptest.NewRequest(http.MethodGet, "http://example.com/", nil)) + + assert.Equal(t, http.StatusUnauthorized, resp.StatusCode) + assert.Equal(t, basicAuthRealm, resp.Header.Get("WWW-Authenticate")) + assert.Zero(t, reachedTarget.Load(), "the target must never see an unauthenticated request") +} + +func TestBasicAuth_AllowsCorrectCredentials(t *testing.T) { + handler := testProtectedService(t, defaultServiceOptions, func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte("secret")) + }) + + req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil) + req.SetBasicAuth(testAuthUser, testAuthPassword) + resp := testAuthRequest(handler, req) + + assert.Equal(t, http.StatusOK, resp.StatusCode) + assert.Equal(t, "secret", testAuthBody(t, resp)) +} + +func TestBasicAuth_RejectsWrongCredentials(t *testing.T) { + tests := []struct { + name string + username string + password string + }{ + {"wrong password", testAuthUser, "nope"}, + {"wrong username", "root", testAuthPassword}, + {"both wrong", "root", "nope"}, + {"empty", "", ""}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + handler := testProtectedService(t, defaultServiceOptions, func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte("secret")) + }) + + req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil) + req.SetBasicAuth(tt.username, tt.password) + resp := testAuthRequest(handler, req) + + assert.Equal(t, http.StatusUnauthorized, resp.StatusCode) + assert.Equal(t, basicAuthRealm, resp.Header.Get("WWW-Authenticate")) + }) + } +} + +func TestBasicAuth_ChallengeSurvivesInterceptErrors(t *testing.T) { + // --intercept-errors accepts 4xx, so an operator can name 401. The challenge + // header must still reach the client, or the browser never prompts. + options := defaultServiceOptions + options.InterceptErrorStatuses = []int{401, 503} + + handler := testProtectedService(t, options, func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte("secret")) + }) + + resp := testAuthRequest(handler, httptest.NewRequest(http.MethodGet, "http://example.com/", nil)) + + assert.Equal(t, http.StatusUnauthorized, resp.StatusCode) + assert.Equal(t, basicAuthRealm, resp.Header.Get("WWW-Authenticate")) +} + +func TestBasicAuth_ChallengeRendersCustomErrorPage(t *testing.T) { + pagesDir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(pagesDir, "401.html"), []byte("

go away

"), 0644)) + + options := defaultServiceOptions + options.ErrorPagePath = pagesDir + + handler := testProtectedService(t, options, func(w http.ResponseWriter, r *http.Request) {}) + + resp := testAuthRequest(handler, httptest.NewRequest(http.MethodGet, "http://example.com/", nil)) + + assert.Equal(t, http.StatusUnauthorized, resp.StatusCode) + assert.Equal(t, basicAuthRealm, resp.Header.Get("WWW-Authenticate")) + assert.Contains(t, testAuthBody(t, resp), "go away") +} + +func TestBasicAuth_ChallengeSurvivesErrorPagesWithout401Template(t *testing.T) { + // The service-level middleware only parses the operator's directory, so a + // dir with no 401.html falls through to the root middleware. The client must + // still get a usable challenge. + pagesDir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(pagesDir, "404.html"), []byte("

nope

"), 0644)) + + options := defaultServiceOptions + options.ErrorPagePath = pagesDir + + handler := testProtectedService(t, options, func(w http.ResponseWriter, r *http.Request) {}) + + resp := testAuthRequest(handler, httptest.NewRequest(http.MethodGet, "http://example.com/", nil)) + + assert.Equal(t, http.StatusUnauthorized, resp.StatusCode) + assert.Equal(t, basicAuthRealm, resp.Header.Get("WWW-Authenticate")) +} + +func TestBasicAuth_RedirectsBeforeChallenging(t *testing.T) { + // The core security property: a plaintext request to a TLS service must be + // redirected, never challenged, or the browser sends the password in the + // clear before it ever reaches https. This is the regression guard against + // anyone later "tidying" the check into createMiddleware. + options := defaultServiceOptions + options.TLSEnabled = true + options.TLSRedirect = true + options.Hosts = []string{"example.com"} + + handler := testProtectedService(t, options, func(w http.ResponseWriter, r *http.Request) {}) + + resp := testAuthRequest(handler, httptest.NewRequest(http.MethodGet, "http://example.com/", nil)) + + assert.Equal(t, http.StatusMovedPermanently, resp.StatusCode) + assert.Equal(t, "https://example.com/", resp.Header.Get("Location")) + assert.Empty(t, resp.Header.Get("WWW-Authenticate"), "must not solicit credentials over plaintext") +} + +func TestBasicAuth_RedirectsToCanonicalHostBeforeChallenging(t *testing.T) { + options := defaultServiceOptions + options.Hosts = []string{"example.com", "www.example.com"} + options.CanonicalHost = "example.com" + + handler := testProtectedService(t, options, func(w http.ResponseWriter, r *http.Request) {}) + + resp := testAuthRequest(handler, httptest.NewRequest(http.MethodGet, "http://www.example.com/", nil)) + + assert.Equal(t, http.StatusMovedPermanently, resp.StatusCode) + assert.Empty(t, resp.Header.Get("WWW-Authenticate")) +} + +func TestBasicAuth_StripsAuthorizationBeforeForwarding(t *testing.T) { + var forwarded atomic.Value + forwarded.Store("") + + handler := testProtectedService(t, defaultServiceOptions, func(w http.ResponseWriter, r *http.Request) { + forwarded.Store(r.Header.Get("Authorization")) + }) + + req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil) + req.SetBasicAuth(testAuthUser, testAuthPassword) + resp := testAuthRequest(handler, req) + + require.Equal(t, http.StatusOK, resp.StatusCode) + assert.Empty(t, forwarded.Load(), "the proxy's credential must not reach the target") +} + +func TestBasicAuth_StripsAuthorizationOnExemptPaths(t *testing.T) { + // Browsers preemptively replay cached credentials to every path under the + // protection space, so an exempt path must not become a forwarding hole. + var forwarded atomic.Value + forwarded.Store("") + + handler := testProtectedService(t, defaultServiceOptions, func(w http.ResponseWriter, r *http.Request) { + forwarded.Store(r.Header.Get("Authorization")) + }) + + req := httptest.NewRequest(http.MethodGet, "http://example.com"+DefaultHealthCheckPath, nil) + req.SetBasicAuth(testAuthUser, testAuthPassword) + resp := testAuthRequest(handler, req) + + require.Equal(t, http.StatusOK, resp.StatusCode) + assert.Empty(t, forwarded.Load()) +} + +func TestBasicAuth_ExemptsHealthCheckRequests(t *testing.T) { + tests := []struct { + name string + method string + path string + expectedStatus int + }{ + {"GET on the health check path", http.MethodGet, DefaultHealthCheckPath, http.StatusOK}, + {"HEAD on the health check path", http.MethodHead, DefaultHealthCheckPath, http.StatusOK}, + {"POST on the health check path", http.MethodPost, DefaultHealthCheckPath, http.StatusUnauthorized}, + {"any other path", http.MethodGet, "/", http.StatusUnauthorized}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + handler := testProtectedService(t, defaultServiceOptions, func(w http.ResponseWriter, r *http.Request) {}) + + resp := testAuthRequest(handler, httptest.NewRequest(tt.method, "http://example.com"+tt.path, nil)) + + assert.Equal(t, tt.expectedStatus, resp.StatusCode) + }) + } +} + +func TestBasicAuth_RejectsRootHealthCheckPath(t *testing.T) { + // A health check path of "/" would exempt the protected service's index. + router := testRouter(t) + _, target := testBackendWithHandler(t, func(w http.ResponseWriter, r *http.Request) {}) + + options := defaultServiceOptions + options.BasicAuth = testEncodedCredential(t, testAuthUser, testAuthPassword) + + targetOptions := defaultTargetOptions + targetOptions.HealthCheckConfig.Path = "/" + + err := router.DeployService("service1", []string{target}, defaultEmptyReaders, + options, targetOptions, defaultDeploymentOptions) + + require.ErrorIs(t, err, ErrServiceOptionsInvalid) + require.ErrorContains(t, err, "health-check-path") +} + +func TestBasicAuth_StateWrittenBeforeTheOptionStaysUnprotected(t *testing.T) { + state := ` + { + "name": "my-app", + "hosts": ["app.example.com"], + "active_target": "localhost:3000", + "rollout_target": "", + "options": {}, + "target_options": { + "health_check_config": {"path": "/up", "interval": 1000000000, "timeout": 5000000000}, + "response_timeout": 30000000000, + "forward_headers": true + }, + "pause_controller": {"state": 0, "stop_message": "", "fail_after": 0}, + "rollout_controller": null + } + ` + + var service Service + require.NoError(t, json.NewDecoder(strings.NewReader(state)).Decode(&service)) + t.Cleanup(service.Dispose) + + // Never accidentally locked out: a state file written before this feature + // existed must load a service that requires no credentials. + assert.Empty(t, service.options.BasicAuth) + assert.Nil(t, service.basicAuth) +} + +func TestBasicAuth_SurvivesStateRoundTrip(t *testing.T) { + options := defaultServiceOptions + options.BasicAuth = testEncodedCredential(t, testAuthUser, testAuthPassword) + + service := testCreateService(t, options, defaultTargetOptions) + t.Cleanup(service.Dispose) + + encoded, err := json.Marshal(service) + require.NoError(t, err) + + // The state file must never carry the plaintext credential. + assert.NotContains(t, string(encoded), testAuthUser) + assert.NotContains(t, string(encoded), testAuthPassword) + + var restored Service + require.NoError(t, json.Unmarshal(encoded, &restored)) + t.Cleanup(restored.Dispose) + + require.NotNil(t, restored.basicAuth) + assert.True(t, restored.basicAuth.matches(testAuthUser, testAuthPassword)) + assert.False(t, restored.basicAuth.matches(testAuthUser, "nope")) +} + +func TestBasicAuth_UnreadableStoredCredentialFailsClosed(t *testing.T) { + // An unparseable credential must not abort the decode of the whole state + // file, and must deny rather than open the service it belongs to. + state := ` + { + "name": "my-app", + "hosts": ["app.example.com"], + "active_target": "localhost:3000", + "options": {"basic_auth": "argon2id:aa:bb"}, + "target_options": { + "health_check_config": {"path": "/up", "interval": 1000000000, "timeout": 5000000000}, + "response_timeout": 30000000000 + }, + "pause_controller": {"state": 0, "stop_message": "", "fail_after": 0}, + "rollout_controller": null + } + ` + + var service Service + require.NoError(t, json.NewDecoder(strings.NewReader(state)).Decode(&service)) + t.Cleanup(service.Dispose) + + require.NotNil(t, service.basicAuth) + assert.False(t, service.basicAuth.matches(testAuthUser, testAuthPassword)) + assert.False(t, service.basicAuth.matches("", "")) +} + +func TestBasicAuth_MalformedCredentialFailsTheDeploy(t *testing.T) { + router := testRouter(t) + _, target := testBackendWithHandler(t, func(w http.ResponseWriter, r *http.Request) {}) + + options := defaultServiceOptions + options.BasicAuth = "sha256:zz:zz" + + err := router.DeployService("service1", []string{target}, defaultEmptyReaders, + options, defaultTargetOptions, defaultDeploymentOptions) + + require.ErrorIs(t, err, ErrServiceOptionsInvalid) +} + +func TestBasicAuth_RedeployWithoutTheCredentialRemovesProtection(t *testing.T) { + router := testRouter(t) + _, target := testBackendWithHandler(t, func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte("ok")) + }) + + protected := defaultServiceOptions + protected.BasicAuth = testEncodedCredential(t, testAuthUser, testAuthPassword) + + require.NoError(t, router.DeployService("service1", []string{target}, defaultEmptyReaders, + protected, defaultTargetOptions, defaultDeploymentOptions)) + + handler := testRoutedHandler(t, router) + resp := testAuthRequest(handler, httptest.NewRequest(http.MethodGet, "http://example.com/", nil)) + require.Equal(t, http.StatusUnauthorized, resp.StatusCode) + + require.NoError(t, router.DeployService("service1", []string{target}, defaultEmptyReaders, + defaultServiceOptions, defaultTargetOptions, defaultDeploymentOptions)) + + resp = testAuthRequest(handler, httptest.NewRequest(http.MethodGet, "http://example.com/", nil)) + assert.Equal(t, http.StatusOK, resp.StatusCode) +} + +func TestBasicAuth_IsNotExposedByListActiveServices(t *testing.T) { + router := testRouter(t) + _, target := testBackendWithHandler(t, func(w http.ResponseWriter, r *http.Request) {}) + + options := defaultServiceOptions + options.BasicAuth = testEncodedCredential(t, testAuthUser, testAuthPassword) + + require.NoError(t, router.DeployService("service1", []string{target}, defaultEmptyReaders, + options, defaultTargetOptions, defaultDeploymentOptions)) + + listed, err := json.Marshal(router.ListActiveServices()) + require.NoError(t, err) + + assert.NotContains(t, string(listed), options.BasicAuth) + assert.NotContains(t, string(listed), testAuthPassword) +} + +func TestRouter_StateFileIsNotWorldReadable(t *testing.T) { + statePath := filepath.Join(t.TempDir(), "state.json") + router := NewRouter(statePath) + _, target := testBackendWithHandler(t, func(w http.ResponseWriter, r *http.Request) {}) + + options := defaultServiceOptions + options.BasicAuth = testEncodedCredential(t, testAuthUser, testAuthPassword) + + require.NoError(t, router.DeployService("service1", []string{target}, defaultEmptyReaders, + options, defaultTargetOptions, defaultDeploymentOptions)) + + // The state file now carries a credential digest, so it must not be readable + // by every process that can reach the config volume. + info, err := os.Stat(statePath) + require.NoError(t, err) + assert.Zero(t, info.Mode().Perm()&0o077, "state file mode is %v", info.Mode().Perm()) +} + +func TestBasicAuth_ExemptsInternalRequests(t *testing.T) { + // The TLS on-demand probe addresses the proxy itself. Challenging it would + // silently refuse a certificate to every on-demand host. + handler := testProtectedService(t, defaultServiceOptions, func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte("ok")) + }) + + req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil) + req = req.WithContext(markInternalRequest(req.Context())) + + resp := testAuthRequest(handler, req) + + assert.Equal(t, http.StatusOK, resp.StatusCode) +} diff --git a/internal/server/router.go b/internal/server/router.go index 0cbe7916..3076461e 100644 --- a/internal/server/router.go +++ b/internal/server/router.go @@ -167,7 +167,7 @@ func (r *Router) RestoreLastSavedState() error { } } else { // Keep a last-known-good copy to recover from a future torn write. - if err := writeFileAtomic(r.backupPath(), data, 0644); err != nil { + if err := writeFileAtomic(r.backupPath(), data, 0600); err != nil { slog.Warn("Failed to write state backup", "path", r.backupPath(), "error", err) } } @@ -209,7 +209,7 @@ func (r *Router) restoreFromBackup(cause error) ([]*Service, error) { slog.Warn("Restored state from backup after decode failure", "path", r.backupPath()) // Repair the primary so subsequent boots restore cleanly again. - if err := writeFileAtomic(r.statePath, data, 0644); err != nil { + if err := writeFileAtomic(r.statePath, data, 0600); err != nil { slog.Warn("Failed to repair state file from backup", "path", r.statePath, "error", err) } @@ -271,6 +271,10 @@ func (r *Router) DeployService(name string, targetURLs, readerURLs []string, opt return err } + if err := validateBasicAuthHealthCheck(options, targetOptions); err != nil { + return err + } + options.Normalize() slog.Info("Deploying", "service", name, "targets", targetURLs, "hosts", options.Hosts, "paths", options.PathPrefixes, "tls", options.TLSEnabled) @@ -588,7 +592,7 @@ func (r *Router) saveStateSnapshot() error { r.saveLock.Lock() defer r.saveLock.Unlock() - if err := writeFileAtomic(r.statePath, data, 0644); err != nil { + if err := writeFileAtomic(r.statePath, data, 0600); err != nil { slog.Error("Unable to save state", "error", err, "path", r.statePath) return err } diff --git a/internal/server/service.go b/internal/server/service.go index f0be6c3c..7ae32d28 100644 --- a/internal/server/service.go +++ b/internal/server/service.go @@ -126,6 +126,13 @@ type ServiceOptions struct { // TargetTryInterval is the pause between those attempts. Zero uses // DefaultTargetTryInterval. TargetTryInterval time.Duration `json:"target_try_interval,omitempty"` + + // BasicAuth requires HTTP Basic credentials on every request to this + // service, stored as "::" over + // ":". The CLI does the hashing, so no plaintext + // credential crosses the RPC socket or reaches the state file. Empty (the + // default) leaves the service open. + BasicAuth string `json:"basic_auth,omitempty"` } func (so *ServiceOptions) ShouldExcludeMetrics(r *http.Request) bool { @@ -184,6 +191,10 @@ func (so ServiceOptions) Validate() error { return err } + if err := so.validateBasicAuth(); err != nil { + return err + } + return so.validateDynamicDomains() } @@ -283,6 +294,7 @@ type Service struct { sanCertManager *SANCertManager certManager CertManager middleware http.Handler + basicAuth *basicAuthCredential } func NewService(name string, options ServiceOptions, targetOptions TargetOptions, sanCertManager *SANCertManager) (*Service, error) { @@ -524,6 +536,7 @@ func (s *Service) initialize(options ServiceOptions, targetOptions TargetOptions s.targetOptions = targetOptions s.certManager = certManager s.middleware = middleware + s.basicAuth = s.resolveBasicAuth(options) return nil } @@ -683,6 +696,13 @@ func (s *Service) serviceRequestWithTarget(w http.ResponseWriter, r *http.Reques return } + // After the redirect, so credentials are never solicited over plaintext on a + // service that redirects to HTTPS. Before the pause check, so protection + // does not lapse while a service is paused or stopped. + if s.rejectUnauthenticated(w, r) { + return + } + if s.handlePausedAndStoppedRequests(w, r) { return } diff --git a/internal/server/testing.go b/internal/server/testing.go index d635560c..83101999 100644 --- a/internal/server/testing.go +++ b/internal/server/testing.go @@ -11,8 +11,23 @@ import ( "time" "github.com/stretchr/testify/require" + + "github.com/basecamp/kamal-proxy/internal/pages" ) +// testRoutedHandler wraps a router the way Server.buildHandler does, so that +// proxy-generated statuses render through the default error pages instead of +// falling back to http.Error. Tests asserting on a response body need this; +// the bare router has no error page middleware in its chain. +func testRoutedHandler(t testing.TB, router *Router) http.Handler { + t.Helper() + + handler, err := WithErrorPageMiddleware(pages.DefaultErrorPages, true, router) + require.NoError(t, err) + + return handler +} + var ( defaultHealthCheckConfig = HealthCheckConfig{Path: DefaultHealthCheckPath, Port: DefaultHealthCheckPort, Interval: DefaultHealthCheckInterval, Timeout: time.Second * 5} defaultEmptyReaders = []string{} diff --git a/internal/server/util.go b/internal/server/util.go index 1f2f97fd..886c1d25 100644 --- a/internal/server/util.go +++ b/internal/server/util.go @@ -25,6 +25,14 @@ func writeFileAtomic(path string, data []byte, perm os.FileMode) error { return err } + // O_CREATE only applies perm to a file it creates, and umask masks it even + // then. Set it explicitly, so a temp file left behind by an earlier run + // cannot keep looser permissions than the caller asked for. + if err := f.Chmod(perm); err != nil { + f.Close() + return err + } + if _, err := f.Write(data); err != nil { f.Close() return err