Require secrets via headers only: drop ?key= and ?password= query fallbacks (#137, #141)
CI / test (pull_request) Successful in 28s
CI / docker (pull_request) Skipped

Query strings leak into Traefik access logs, browser history, and Referer
headers. Admin key is now accepted only via X-Admin-Key; paste and can
passwords only via X-Paste-Password (or the POST unlock form). Tests
updated; new negative cases assert 401 for the query paths.
This commit is contained in:
fen
2026-09-09 23:23:10 -05:00
parent 98222e762f
commit d013f3965f
8 changed files with 29 additions and 29 deletions
+4 -5
View File
@@ -160,13 +160,12 @@ func HandleResetAdminKey(dbPath string) {
} }
// adminKeyOK reports whether the request carries the correct admin key via // adminKeyOK reports whether the request carries the correct admin key via
// X-Admin-Key header or ?key=. Constant-time compare; failures and successes // the X-Admin-Key header only. The ?key= query fallback was removed (#137):
// are both logged (#40). // query strings land in access logs, browser history, and Referer headers,
// so accepting the key there leaked the admin secret. Constant-time compare;
// failures and successes are both logged (#40).
func (a *apiServer) adminKeyOK(r *http.Request, key string) bool { func (a *apiServer) adminKeyOK(r *http.Request, key string) bool {
given := r.Header.Get("X-Admin-Key") given := r.Header.Get("X-Admin-Key")
if given == "" {
given = r.URL.Query().Get("key")
}
return subtle.ConstantTimeCompare([]byte(given), []byte(key)) == 1 return subtle.ConstantTimeCompare([]byte(given), []byte(key)) == 1
} }
+3 -2
View File
@@ -37,11 +37,12 @@ func TestAdminAuth(t *testing.T) {
t.Fatalf("wrong key: expected 401, got %d", rec.Code) t.Fatalf("wrong key: expected 401, got %d", rec.Code)
} }
// #137: the ?key= query fallback was removed; keys must go via header.
req = httptest.NewRequest("GET", "/admin/api/settings?key=test-admin-key", nil) req = httptest.NewRequest("GET", "/admin/api/settings?key=test-admin-key", nil)
rec = httptest.NewRecorder() rec = httptest.NewRecorder()
h.ServeHTTP(rec, req) h.ServeHTTP(rec, req)
if rec.Code != 200 { if rec.Code != 401 {
t.Fatalf("query key: expected 200, got %d", rec.Code) t.Fatalf("query key: expected 401 after #137 removal, got %d", rec.Code)
} }
req = httptest.NewRequest("GET", "/admin/api/settings", nil) req = httptest.NewRequest("GET", "/admin/api/settings", nil)
-6
View File
@@ -206,9 +206,6 @@ func (a *apiServer) handleGetCan(w http.ResponseWriter, r *http.Request) {
} }
if can.PasswordHash.Valid { if can.PasswordHash.Valid {
pw := r.Header.Get("X-Paste-Password") pw := r.Header.Get("X-Paste-Password")
if pw == "" {
pw = r.URL.Query().Get("password")
}
if pw == "" || !store.CheckPassword(can.PasswordHash.String, pw) { if pw == "" || !store.CheckPassword(can.PasswordHash.String, pw) {
writeErr(w, 401, "password required") writeErr(w, 401, "password required")
return return
@@ -256,9 +253,6 @@ func (a *apiServer) handleCanItem(w http.ResponseWriter, r *http.Request) {
can, _ := a.store.GetCan(row.CanID.String) can, _ := a.store.GetCan(row.CanID.String)
if can != nil && can.PasswordHash.Valid { if can != nil && can.PasswordHash.Valid {
pw := r.Header.Get("X-Paste-Password") pw := r.Header.Get("X-Paste-Password")
if pw == "" {
pw = r.URL.Query().Get("password")
}
if pw == "" || !store.CheckPassword(can.PasswordHash.String, pw) { if pw == "" || !store.CheckPassword(can.PasswordHash.String, pw) {
// fall back to the browser's unlock cookie for this can // fall back to the browser's unlock cookie for this can
c, cerr := r.Cookie("pw_" + can.ID) c, cerr := r.Cookie("pw_" + can.ID)
+2 -1
View File
@@ -294,7 +294,8 @@ func TestCanItemCookieParity(t *testing.T) {
} }
// item id from API (with password query) // item id from API (with password query)
req = httptest.NewRequest("GET", "/api/cans/"+created.ID+"?password=pw123", nil) req = httptest.NewRequest("GET", "/api/cans/"+created.ID, nil)
req.Header.Set("X-Paste-Password", "pw123")
rec = httptest.NewRecorder() rec = httptest.NewRecorder()
h.ServeHTTP(rec, req) h.ServeHTTP(rec, req)
var can struct { var can struct {
+4 -2
View File
@@ -112,7 +112,8 @@ func TestCanPasswordInheritedByItems(t *testing.T) {
} }
// get item id with pw // get item id with pw
req = httptest.NewRequest("GET", "/api/cans/"+created.ID+"?password=pw123", nil) req = httptest.NewRequest("GET", "/api/cans/"+created.ID, nil)
req.Header.Set("X-Paste-Password", "pw123")
rec = httptest.NewRecorder() rec = httptest.NewRecorder()
h.ServeHTTP(rec, req) h.ServeHTTP(rec, req)
var can struct { var can struct {
@@ -130,7 +131,8 @@ func TestCanPasswordInheritedByItems(t *testing.T) {
} }
// item with pw -> 200 // item with pw -> 200
req = httptest.NewRequest("GET", "/api/cans/"+created.ID+"/items/"+itemID+"?password=pw123", nil) req = httptest.NewRequest("GET", "/api/cans/"+created.ID+"/items/"+itemID, nil)
req.Header.Set("X-Paste-Password", "pw123")
rec = httptest.NewRecorder() rec = httptest.NewRecorder()
h.ServeHTTP(rec, req) h.ServeHTTP(rec, req)
if rec.Code != 200 { if rec.Code != 200 {
@@ -2,7 +2,7 @@ package api
// #81: ALL password verification attempts (GET query param, header, POST // #81: ALL password verification attempts (GET query param, header, POST
// form) must go through the per-IP unlock limiter. Regression: N wrong // form) must go through the per-IP unlock limiter. Regression: N wrong
// passwords via GET ?password= must eventually yield 429. // passwords via X-Paste-Password must eventually yield 429.
import ( import (
"encoding/json" "encoding/json"
@@ -26,7 +26,7 @@ func createPasswordPaste(t *testing.T, s *apiServer, pw string) string {
} }
// TestRateLimitGetPasswordQuery: repeated wrong passwords via GET // TestRateLimitGetPasswordQuery: repeated wrong passwords via GET
// ?password= must eventually return 429 (unlock limiter: burst 5). // X-Paste-Password wrong attempts must eventually return 429 (unlock limiter: burst 5).
func TestRateLimitGetPasswordQuery(t *testing.T) { func TestRateLimitGetPasswordQuery(t *testing.T) {
s := testServer(t) s := testServer(t)
h := s.routes() h := s.routes()
@@ -35,7 +35,8 @@ func TestRateLimitGetPasswordQuery(t *testing.T) {
var saw429 bool var saw429 bool
// more attempts than the unlock burst (5) // more attempts than the unlock burst (5)
for i := 0; i < 10; i++ { for i := 0; i < 10; i++ {
req := httptest.NewRequest("GET", "/api/pastes/"+id+"?password=wrong"+string(rune('a'+i)), nil) req := httptest.NewRequest("GET", "/api/pastes/"+id, nil)
req.Header.Set("X-Paste-Password", "wrong"+string(rune('a'+i)))
rec := httptest.NewRecorder() rec := httptest.NewRecorder()
h.ServeHTTP(rec, req) h.ServeHTTP(rec, req)
if rec.Code == 429 { if rec.Code == 429 {
@@ -47,7 +48,7 @@ func TestRateLimitGetPasswordQuery(t *testing.T) {
} }
} }
if !saw429 { if !saw429 {
t.Fatal("expected 429 after repeated wrong ?password= attempts, never got one") t.Fatal("expected 429 after repeated wrong password attempts, never got one")
} }
} }
@@ -83,7 +84,8 @@ func TestRateLimitGetPasswordCorrectStillAllowed(t *testing.T) {
h := s.routes() h := s.routes()
id := createPasswordPaste(t, s, "hunter2") id := createPasswordPaste(t, s, "hunter2")
req := httptest.NewRequest("GET", "/api/pastes/"+id+"?password=hunter2", nil) req := httptest.NewRequest("GET", "/api/pastes/"+id, nil)
req.Header.Set("X-Paste-Password", "hunter2")
rec := httptest.NewRecorder() rec := httptest.NewRecorder()
h.ServeHTTP(rec, req) h.ServeHTTP(rec, req)
if rec.Code != 200 { if rec.Code != 200 {
+4 -2
View File
@@ -89,7 +89,8 @@ func TestPasswordProtection(t *testing.T) {
} }
// with password -> 200 // with password -> 200
req = httptest.NewRequest("GET", "/api/pastes/"+created.ID+"?password=hunter2", nil) req = httptest.NewRequest("GET", "/api/pastes/"+created.ID, nil)
req.Header.Set("X-Paste-Password", "hunter2")
rec = httptest.NewRecorder() rec = httptest.NewRecorder()
h.ServeHTTP(rec, req) h.ServeHTTP(rec, req)
if rec.Code != 200 { if rec.Code != 200 {
@@ -97,7 +98,8 @@ func TestPasswordProtection(t *testing.T) {
} }
// wrong password -> 401 // wrong password -> 401
req = httptest.NewRequest("GET", "/api/pastes/"+created.ID+"?password=nope", nil) req = httptest.NewRequest("GET", "/api/pastes/"+created.ID, nil)
req.Header.Set("X-Paste-Password", "nope")
rec = httptest.NewRecorder() rec = httptest.NewRecorder()
h.ServeHTTP(rec, req) h.ServeHTTP(rec, req)
if rec.Code != 401 { if rec.Code != 401 {
+5 -6
View File
@@ -267,18 +267,17 @@ func (a *apiServer) handleGetPaste(w http.ResponseWriter, r *http.Request) {
return return
} }
if row.PasswordHash.Valid { if row.PasswordHash.Valid {
// #81: every password verification (header, query param, or empty) // #81/#141: every password verification (header or empty) goes
// goes through the same per-IP+paste unlock limiter as the POST form // through the same per-IP+paste unlock limiter as the POST form
// path, so brute-force via GET ?password= or X-Paste-Password gets 429. // path, so brute-force via X-Paste-Password gets 429. The
// ?password= query fallback was removed (#141): query strings
// leak into access logs, browser history, and Referer headers.
if !rateLimitUnlock(row.ID, r) { if !rateLimitUnlock(row.ID, r) {
writeRateLimited(w, 60) writeRateLimited(w, 60)
return return
} }
// require password via header or query // require password via header or query
pw := r.Header.Get("X-Paste-Password") pw := r.Header.Get("X-Paste-Password")
if pw == "" {
pw = r.URL.Query().Get("password")
}
if pw == "" || !store.CheckPassword(row.PasswordHash.String, pw) { if pw == "" || !store.CheckPassword(row.PasswordHash.String, pw) {
writeErr(w, 401, "password required") writeErr(w, 401, "password required")
return return