diff --git a/docs/web-configuration.md b/docs/web-configuration.md index c3b1cf12..f4121207 100644 --- a/docs/web-configuration.md +++ b/docs/web-configuration.md @@ -150,9 +150,17 @@ which will look something like: The cost (10 in the example) influences the time it takes for computing the hash. A higher cost will end up slowing down the authentication process. Depending on the machine, a cost of 10 will take about ~70ms, whereas a cost of -18 can take up to a few seconds. That hash will be computed on the first +18 can take tens of seconds. That hash will be computed on the first authenticated HTTP request and then cached. +The cost also bounds how expensive an unauthenticated request can be. A request +naming a user that is not configured is compared against a decoy hash generated +at the highest cost in use, so that it cannot be told apart from a request for a +configured user by how long it takes. This means a single expensive user makes +every rejected request that expensive too. Keep the cost the same for all users, +pick it with that in mind, and consider configuring `rate_limit` alongside +`basic_auth_users`. + ## Performance Basic authentication is meant for simple use cases, with a few users. If you diff --git a/web/decoy_test.go b/web/decoy_test.go new file mode 100644 index 00000000..33641447 --- /dev/null +++ b/web/decoy_test.go @@ -0,0 +1,111 @@ +// Copyright The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package web + +import ( + "testing" + + config_util "github.com/prometheus/common/config" + "golang.org/x/crypto/bcrypt" +) + +func hashAtCost(t *testing.T, cost int) config_util.Secret { + t.Helper() + h, err := bcrypt.GenerateFromPassword([]byte("password"), cost) + if err != nil { + t.Fatalf("GenerateFromPassword: %v", err) + } + return config_util.Secret(h) +} + +// TestMaxBcryptCost checks that the decoy cost tracks the most expensive +// configured user, so that a request for an unknown user is never answered +// faster than one for any configured user. +func TestMaxBcryptCost(t *testing.T) { + cheap := hashAtCost(t, bcrypt.MinCost) + mid := hashAtCost(t, bcrypt.MinCost+2) + + for _, tc := range []struct { + name string + users map[string]config_util.Secret + expected int + }{ + { + name: "no users", + users: nil, + expected: bcrypt.MinCost, + }, + { + name: "single user", + users: map[string]config_util.Secret{"alice": mid}, + expected: bcrypt.MinCost + 2, + }, + { + name: "highest cost wins", + users: map[string]config_util.Secret{"alice": cheap, "bob": mid}, + expected: bcrypt.MinCost + 2, + }, + { + name: "unparseable hashes are skipped", + users: map[string]config_util.Secret{"alice": "not a hash", "bob": mid}, + expected: bcrypt.MinCost + 2, + }, + { + name: "only unparseable hashes", + users: map[string]config_util.Secret{"alice": "not a hash"}, + expected: bcrypt.MinCost, + }, + } { + t.Run(tc.name, func(t *testing.T) { + if got := maxBcryptCost(tc.users); got != tc.expected { + t.Errorf("maxBcryptCost() = %d, expected %d", got, tc.expected) + } + }) + } +} + +// TestDecoyHashesForCost checks that the decoy carries the requested cost, which +// is what makes its comparison take as long as a configured user's, and that it +// is generated only once per cost. +func TestDecoyHashesForCost(t *testing.T) { + d := newDecoyHashes() + + hash, err := d.forCost(bcrypt.MinCost) + if err != nil { + t.Fatalf("forCost: %v", err) + } + cost, err := bcrypt.Cost(hash) + if err != nil { + t.Fatalf("Cost: %v", err) + } + if cost != bcrypt.MinCost { + t.Errorf("decoy cost = %d, expected %d", cost, bcrypt.MinCost) + } + + again, err := d.forCost(bcrypt.MinCost) + if err != nil { + t.Fatalf("forCost: %v", err) + } + if string(again) != string(hash) { + t.Error("forCost generated a second hash for a cost it had already produced") + } +} + +// TestDecoyHashesRejectsInvalidCost checks that an unusable cost is reported +// rather than silently producing a hash at some other cost. +func TestDecoyHashesRejectsInvalidCost(t *testing.T) { + if _, err := newDecoyHashes().forCost(bcrypt.MaxCost + 1); err == nil { + t.Error("expected an error for a cost above bcrypt.MaxCost") + } +} diff --git a/web/handler.go b/web/handler.go index 0a2718d5..51118b7b 100644 --- a/web/handler.go +++ b/web/handler.go @@ -23,10 +23,16 @@ import ( "strings" "sync" + config_util "github.com/prometheus/common/config" "golang.org/x/crypto/bcrypt" "golang.org/x/time/rate" ) +// decoyPassword is hashed to produce the hash that requests for unknown users +// are compared against. Its value is irrelevant; only the cost of hashing it +// matters. +const decoyPassword = "fakepassword" + // extraHTTPHeaders is a map of HTTP headers that can be added to HTTP // responses. // This is private on purpose to ensure consistency in the Prometheus ecosystem. @@ -76,17 +82,80 @@ HeadersLoop: return nil } +// maxBcryptCost returns the highest bcrypt cost among the configured users, or +// bcrypt.MinCost if none of them can be parsed. Hashes are validated by +// validateUsers before the server starts, so an unparseable hash here means the +// configuration was changed to an invalid one after startup. +func maxBcryptCost(users map[string]config_util.Secret) int { + cost := bcrypt.MinCost + for _, hash := range users { + c, err := bcrypt.Cost([]byte(hash)) + if err != nil { + continue + } + if c > cost { + cost = c + } + } + return cost +} + +// decoyHashes caches one bcrypt hash per cost, used for requests naming a user +// that is not configured. +// +// The cache is process-wide rather than per handler: the hashes are derived +// from a fixed password and hold no configuration, and generating one at a high +// cost is expensive enough to be worth doing only once. +type decoyHashes struct { + mtx sync.Mutex + hashes map[int][]byte +} + +var defaultDecoyHashes = newDecoyHashes() + +func newDecoyHashes() *decoyHashes { + return &decoyHashes{hashes: make(map[int][]byte)} +} + +// forCost returns a hash with the given cost, generating it on first use. The +// generation happens at most once per cost for the lifetime of the process. +func (d *decoyHashes) forCost(cost int) ([]byte, error) { + d.mtx.Lock() + defer d.mtx.Unlock() + + if hash, ok := d.hashes[cost]; ok { + return hash, nil + } + hash, err := bcrypt.GenerateFromPassword([]byte(decoyPassword), cost) + if err != nil { + return nil, err + } + d.hashes[cost] = hash + return hash, nil +} + type webHandler struct { tlsConfigPath string handler http.Handler logger *slog.Logger cache *cache limiter *rate.Limiter + // decoys is nil in the default configuration and is then read from the + // process-wide cache. Tests set it to isolate themselves from it. + decoys *decoyHashes // bcryptMtx is there to ensure that bcrypt.CompareHashAndPassword is run // only once in parallel as this is CPU intensive. bcryptMtx sync.Mutex } +// decoyHashes returns the cache this handler draws decoy hashes from. +func (u *webHandler) decoyHashes() *decoyHashes { + if u.decoys != nil { + return u.decoys + } + return defaultDecoyHashes +} + func (u *webHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { c, err := getConfig(u.tlsConfigPath) if err != nil { @@ -115,10 +184,24 @@ func (u *webHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { hashedPassword, validUser := c.Users[user] if !validUser { - // The user is not found. Use a fixed password hash to - // prevent user enumeration by timing requests. - // This is a bcrypt-hashed version of "fakepassword". - hashedPassword = "$2y$10$QOauhQNbBCuQDKes6eFzPeMqBSjb7Mr5DUmpZ/VcEd00UAV/LDeSi" + // The user is not found. Compare against a decoy hash so + // that the request takes about as long as one naming a + // configured user, which prevents user enumeration by + // timing requests. + // + // The decoy is generated at the highest cost in use, so a + // request for an unknown user is never answered faster + // than one for a configured user. Where the configured + // costs differ, the cheaper users are still answered + // faster than the decoy; using a single cost for every + // user avoids that. + decoy, err := u.decoyHashes().forCost(maxBcryptCost(c.Users)) + if err != nil { + u.logger.Error("Unable to generate decoy password hash", "err", err.Error()) + http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) + return + } + hashedPassword = config_util.Secret(decoy) } cacheKey := strings.Join( diff --git a/web/testdata/web_config_users.good.yml b/web/testdata/web_config_users.good.yml index feeee187..a970d89b 100644 --- a/web/testdata/web_config_users.good.yml +++ b/web/testdata/web_config_users.good.yml @@ -3,6 +3,6 @@ tls_server_config: key_file: "server.key" basic_auth_users: alice: $2y$12$1DpfPeqF9HzHJt.EWswy1exHluGfbhnn3yXhR7Xes6m3WJqFg0Wby - bob: $2y$18$4VeFDzXIoPHKnKTU3O3GH.N.vZu06CVqczYZ8WvfzrddFU6tGqjR. + bob: $2y$14$y7HJ017Lo/wKPDdzdHahNevjaqE.WRk3iKVLLt1yKO7RtIrNbvy6. carol: $2y$10$qRTBuFoULoYNA7AQ/F3ck.trZBPyjV64.oA4ZsSBCIWvXuvQlQTuu dave: $2y$10$2UXri9cIDdgeKjBo4Rlpx.U3ZLDV8X1IxKmsfOvhcM5oXQt/mLmXq diff --git a/web/testdata/web_config_users_noTLS.good.yml b/web/testdata/web_config_users_noTLS.good.yml index d3a79871..9641fcb7 100644 --- a/web/testdata/web_config_users_noTLS.good.yml +++ b/web/testdata/web_config_users_noTLS.good.yml @@ -1,5 +1,5 @@ basic_auth_users: alice: $2y$12$1DpfPeqF9HzHJt.EWswy1exHluGfbhnn3yXhR7Xes6m3WJqFg0Wby - bob: $2y$18$4VeFDzXIoPHKnKTU3O3GH.N.vZu06CVqczYZ8WvfzrddFU6tGqjR. + bob: $2y$14$y7HJ017Lo/wKPDdzdHahNevjaqE.WRk3iKVLLt1yKO7RtIrNbvy6. carol: $2y$10$qRTBuFoULoYNA7AQ/F3ck.trZBPyjV64.oA4ZsSBCIWvXuvQlQTuu dave: $2y$10$2UXri9cIDdgeKjBo4Rlpx.U3ZLDV8X1IxKmsfOvhcM5oXQt/mLmXq