-
Notifications
You must be signed in to change notification settings - Fork 87
Add HTTP Basic Auth support to deploy #216
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ import ( | |
| "fmt" | ||
| "net/rpc" | ||
| "slices" | ||
| "strings" | ||
|
|
||
| "github.com/spf13/cobra" | ||
|
|
||
|
|
@@ -14,6 +15,7 @@ type deployCommand struct { | |
| cmd *cobra.Command | ||
| args server.DeployArgs | ||
| tlsStaging bool | ||
| basicAuth string | ||
| } | ||
|
|
||
| func newDeployCommand() *deployCommand { | ||
|
|
@@ -66,6 +68,8 @@ func newDeployCommand() *deployCommand { | |
| deployCommand.cmd.Flags().BoolVar(&deployCommand.args.TargetOptions.ForwardHeaders, "forward-headers", false, "Forward X-Forwarded headers to target (default false if TLS enabled; otherwise true)") | ||
| deployCommand.cmd.Flags().BoolVar(&deployCommand.args.TargetOptions.ScopeCookiePaths, "scope-cookie-paths", false, "Scope cookie paths to match path prefix") | ||
|
|
||
| deployCommand.cmd.Flags().StringVar(&deployCommand.basicAuth, "basic-auth", "", "Require HTTP Basic Auth, in the form username:password") | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point on the leak surface, and I've added a README warning. I kept it as a single --basic-auth flag for now because (a) this command is invoked by Kamal over SSH via docker exec, not typed interactively, so shell history doesn't really apply, and (b) every other kamal-proxy deploy option is a plain flag — a --basic-auth-password-stdin/env/file variant would be an inconsistent interface here and wouldn't fully close the gap unless Kamal adopted stdin on its side too. I'm happy to add a stdin/env/file input as a follow-up if you'd like it; just wanted to keep this PR's surface consistent with the existing CLI. |
||
|
|
||
| deployCommand.cmd.MarkFlagRequired("target") | ||
| deployCommand.cmd.MarkFlagsRequiredTogether("tls-certificate-path", "tls-private-key-path") | ||
|
|
||
|
|
@@ -88,6 +92,15 @@ func (c *deployCommand) run(cmd *cobra.Command, args []string) error { | |
| func (c *deployCommand) preRun(cmd *cobra.Command, args []string) error { | ||
| c.args.ServiceOptions.Normalize() | ||
|
|
||
| if c.basicAuth != "" { | ||
| username, password, found := strings.Cut(c.basicAuth, ":") | ||
| if !found || username == "" || password == "" { | ||
| return fmt.Errorf("basic-auth must be in the form username:password") | ||
| } | ||
| c.args.ServiceOptions.BasicAuthUsername = username | ||
| c.args.ServiceOptions.BasicAuthPasswordHash = server.HashBasicAuthCredential(password) | ||
| } | ||
|
Comment on lines
+95
to
+102
|
||
|
|
||
| if cmd.Flags().Changed("max-request-body") && !cmd.Flags().Changed("buffer-requests") { | ||
| return fmt.Errorf("max-request-body can only be set when request buffering is enabled") | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| package server | ||
|
|
||
| import ( | ||
| "crypto/sha256" | ||
| "crypto/subtle" | ||
| "encoding/hex" | ||
| "net/http" | ||
| ) | ||
|
|
||
| const basicAuthRealm = `Basic realm="Restricted", charset="UTF-8"` | ||
|
|
||
| // HashBasicAuthCredential returns the hex-encoded SHA-256 digest of a Basic Auth | ||
| // credential. The password is hashed before it is stored in the service options, | ||
| // so the proxy never persists it in plaintext. | ||
| func HashBasicAuthCredential(value string) string { | ||
| sum := sha256.Sum256([]byte(value)) | ||
| return hex.EncodeToString(sum[:]) | ||
| } | ||
|
Comment on lines
+12
to
+18
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Deliberately keeping SHA-256 here. This isn't a password database — it's a single, operator-chosen shared credential whose plaintext already lives in the deploy config, so the hash is just defense-in-depth for the persisted state file rather than protection against mass offline cracking. The bigger issue with an adaptive hash is cost: bcrypt/Argon2 would run on every proxied request on the hot path (tens of ms each), which is a self-inflicted DoS vector for a reverse proxy. Comparisons are already constant-time, so the timing-safety goal is met. Happy to revisit if you'd prefer an adaptive hash with a verification cache, but that seemed like more machinery than this feature warrants — open to your call.
Comment on lines
+12
to
+18
|
||
|
|
||
| type BasicAuthMiddleware struct { | ||
| usernameHash [sha256.Size]byte | ||
| passwordHash []byte | ||
| next http.Handler | ||
| } | ||
|
|
||
| func WithBasicAuthMiddleware(username, passwordHash string, next http.Handler) http.Handler { | ||
| // If the stored hash is malformed (bad hex or not a SHA-256 digest), fall back | ||
| // to a fixed-length zero hash. No real password hashes to all zeroes, so every | ||
| // request fails closed while comparisons still run over equal-length inputs. | ||
| decoded, err := hex.DecodeString(passwordHash) | ||
| if err != nil || len(decoded) != sha256.Size { | ||
| decoded = make([]byte, sha256.Size) | ||
| } | ||
|
Comment on lines
+26
to
+33
Comment on lines
+30
to
+33
|
||
|
|
||
| return &BasicAuthMiddleware{ | ||
| usernameHash: sha256.Sum256([]byte(username)), | ||
| passwordHash: decoded, | ||
| next: next, | ||
| } | ||
| } | ||
|
r4mbo7 marked this conversation as resolved.
|
||
|
|
||
| func (h *BasicAuthMiddleware) ServeHTTP(w http.ResponseWriter, r *http.Request) { | ||
| if h.authenticated(r) { | ||
| h.next.ServeHTTP(w, r) | ||
| return | ||
| } | ||
|
|
||
| w.Header().Set("WWW-Authenticate", basicAuthRealm) | ||
| SetErrorResponse(w, r, http.StatusUnauthorized, nil) | ||
| } | ||
|
|
||
| // Private | ||
|
|
||
| func (h *BasicAuthMiddleware) authenticated(r *http.Request) bool { | ||
| username, password, ok := r.BasicAuth() | ||
| if !ok { | ||
| return false | ||
| } | ||
|
|
||
| // Hash both sides so the comparisons run in constant time over equal-length | ||
| // inputs, regardless of the supplied credential lengths. | ||
| givenUser := sha256.Sum256([]byte(username)) | ||
| givenPassword := sha256.Sum256([]byte(password)) | ||
|
r4mbo7 marked this conversation as resolved.
|
||
|
|
||
| userMatch := subtle.ConstantTimeCompare(givenUser[:], h.usernameHash[:]) == 1 | ||
| passwordMatch := subtle.ConstantTimeCompare(givenPassword[:], h.passwordHash) == 1 | ||
|
|
||
| return userMatch && passwordMatch | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| package server | ||
|
|
||
| import ( | ||
| "net/http" | ||
| "net/http/httptest" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| ) | ||
|
|
||
| func TestBasicAuthMiddleware(t *testing.T) { | ||
| reached := false | ||
| next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| reached = true | ||
| w.WriteHeader(http.StatusOK) | ||
| }) | ||
| handler := WithBasicAuthMiddleware("admin", HashBasicAuthCredential("secret"), next) | ||
|
r4mbo7 marked this conversation as resolved.
|
||
|
|
||
| send := func(setAuth func(*http.Request)) *httptest.ResponseRecorder { | ||
| reached = false | ||
| r := httptest.NewRequest("GET", "/", nil) | ||
| if setAuth != nil { | ||
| setAuth(r) | ||
| } | ||
| w := httptest.NewRecorder() | ||
| handler.ServeHTTP(w, r) | ||
| return w | ||
| } | ||
|
|
||
| t.Run("allows requests with correct credentials", func(t *testing.T) { | ||
| w := send(func(r *http.Request) { r.SetBasicAuth("admin", "secret") }) | ||
|
|
||
| assert.True(t, reached) | ||
| assert.Equal(t, http.StatusOK, w.Code) | ||
| }) | ||
|
|
||
| t.Run("rejects requests with no credentials", func(t *testing.T) { | ||
| w := send(nil) | ||
|
|
||
| assert.False(t, reached) | ||
| assert.Equal(t, http.StatusUnauthorized, w.Code) | ||
| assert.Equal(t, basicAuthRealm, w.Header().Get("WWW-Authenticate")) | ||
| }) | ||
|
|
||
| t.Run("rejects requests with a wrong password", func(t *testing.T) { | ||
| w := send(func(r *http.Request) { r.SetBasicAuth("admin", "wrong") }) | ||
|
|
||
| assert.False(t, reached) | ||
| assert.Equal(t, http.StatusUnauthorized, w.Code) | ||
| }) | ||
|
|
||
| t.Run("rejects requests with a wrong username", func(t *testing.T) { | ||
| w := send(func(r *http.Request) { r.SetBasicAuth("root", "secret") }) | ||
|
|
||
| assert.False(t, reached) | ||
| assert.Equal(t, http.StatusUnauthorized, w.Code) | ||
| }) | ||
| } | ||
|
|
||
| func TestBasicAuthMiddleware_FailsClosedWithInvalidHash(t *testing.T) { | ||
| for name, passwordHash := range map[string]string{ | ||
| "non-hex hash": "not-a-valid-hash", | ||
| "wrong-length hash": "abcd", | ||
| "empty hash": "", | ||
| } { | ||
| t.Run(name, func(t *testing.T) { | ||
| reached := false | ||
| next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| reached = true | ||
| }) | ||
| handler := WithBasicAuthMiddleware("admin", passwordHash, next) | ||
|
|
||
| r := httptest.NewRequest("GET", "/", nil) | ||
| r.SetBasicAuth("admin", "secret") | ||
| w := httptest.NewRecorder() | ||
| handler.ServeHTTP(w, r) | ||
|
|
||
| assert.False(t, reached, "request must not reach the backend") | ||
| assert.Equal(t, http.StatusUnauthorized, w.Code) | ||
| assert.Equal(t, basicAuthRealm, w.Header().Get("WWW-Authenticate")) | ||
| }) | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.