Input hardening

This commit is contained in:
Jonathon Leight committed 2026-07-05 12:09:49 -04:00
1 parent 3af089aada
commit 8a34898ac4
9 files changed
+179 -17

No files matched your search

+2 -2
View File
@@ -402,8 +402,8 @@ func (s *Handlers) handleChangePassword(w http.ResponseWriter, r *http.Request)
}
newPassword := r.FormValue("new_password")
if len(newPassword) < 8 {
accountRedirect(w, r, "error", "New password must be at least 8 characters.")
if !ValidPassword(newPassword) {
accountRedirect(w, r, "error", "New password must be 8–72 characters.")
return
}
// When a password already exists, require the current one to change it.
+14
View File
@@ -364,6 +364,20 @@ func NormalizePasskeyName(s string) string {
return s
}
// Password length bounds. The lower bound is a basic strength floor; the upper
// bound is bcrypt's hard limit — GenerateFromPassword rejects inputs longer than
// 72 bytes, so we reject them at the form boundary with a clear message rather
// than surfacing a generic hashing failure.
const (
MinPasswordLen = 8
MaxPasswordLen = 72
)
// ValidPassword reports whether p is within the accepted length bounds.
func ValidPassword(p string) bool {
return len(p) >= MinPasswordLen && len(p) <= MaxPasswordLen
}
// ValidUsername reports whether s is 3–32 chars of [a-z0-9_.-].
func ValidUsername(s string) bool {
if len(s) < 3 || len(s) > 32 {
+2 -2
View File
@@ -13,8 +13,8 @@ func (s *Service) SignupPassword(w http.ResponseWriter, r *http.Request) {
username := NormalizeUsername(r.FormValue("username"))
displayName := NormalizeDisplayName(r.FormValue("display_name"))
password := r.FormValue("password")
if !ValidUsername(username) || len(password) < 8 {
redirectErr(w, r, "/signup", "Choose a username (3–32 chars: letters, digits, _ . -) and a password of at least 8 characters.")
if !ValidUsername(username) || !ValidPassword(password) {
redirectErr(w, r, "/signup", "Choose a username (3–32 chars: letters, digits, _ . -) and a password of 8–72 characters.")
return
}
+21 -1
View File
@@ -1,6 +1,26 @@
package auth
import "testing"
import (
"strings"
"testing"
)
// TestValidPassword pins the accepted length window: an 8-char floor and a
// 72-byte ceiling (bcrypt's hard input limit). Reject just outside both ends.
func TestValidPassword(t *testing.T) {
t.Parallel()
cases := []struct {
n int
want bool
}{
{7, false}, {8, true}, {72, true}, {73, false},
}
for _, c := range cases {
if got := ValidPassword(strings.Repeat("a", c.n)); got != c.want {
t.Errorf("ValidPassword(len=%d) = %v, want %v", c.n, got, c.want)
}
}
}
func TestSafeLocalPath(t *testing.T) {
t.Parallel()
+56
View File
@@ -6,6 +6,7 @@ import (
"net/http"
"net/url"
"strconv"
"strings"
"testing"
meshcore "github.com/meshcore-go/meshcore-go"
@@ -13,6 +14,61 @@ import (
"github.com/jleight/meshtender/internal/store"
)
// TestCleanRepeaterName pins the firmware-derived byte bound: MeshCore stores a
// node name in a 32-byte buffer and NUL-terminates it, leaving 31 usable bytes.
func TestCleanRepeaterName(t *testing.T) {
t.Parallel()
cases := []struct {
in string
want string
wantOK bool
}{
{" Repeater One ", "Repeater One", true},
{"", "", false},
{" ", "", false},
{strings.Repeat("x", 31), strings.Repeat("x", 31), true},
{strings.Repeat("x", 32), strings.Repeat("x", 32), false},
{" " + strings.Repeat("x", 31) + " ", strings.Repeat("x", 31), true}, // trimmed to the limit
}
for _, c := range cases {
got, ok := cleanRepeaterName(c.in)
if got != c.want || ok != c.wantOK {
t.Errorf("cleanRepeaterName(%q) = (%q, %v), want (%q, %v)", c.in, got, ok, c.want, c.wantOK)
}
}
}
// TestRepeaterNameLengthBound: a name over MeshCore's 31-byte limit is rejected
// at create (it would be silently truncated on the device); exactly-at-limit
// succeeds. Regression for the pre-release audit finding that names were
// unbounded.
func TestRepeaterNameLengthBound(t *testing.T) {
t.Parallel()
st, ctx, ts, h := splitServer(t)
_, sess := appLogin(t, ts, st, ctx, h.app, "namelen")
form := func(name, key string) url.Values {
return url.Values{
"name": {name}, "public_key": {key},
"radio_freq_mhz": {"869.525"}, "radio_bw_khz": {"250"}, "radio_sf": {"11"}, "radio_cr": {"5"},
}
}
overID, _ := meshcore.GenerateLocalIdentity(rand.Reader)
over := post(t, ts, h.app, "/repeaters", form(strings.Repeat("x", maxRepeaterNameLen+1), overID.String()), sess)
over.Body.Close()
if loc, _ := url.Parse(over.Header.Get("Location")); over.StatusCode != http.StatusSeeOther || loc.Path != "/repeaters/add" || loc.Query().Get("error") == "" {
t.Fatalf("over-long create = %d %q, want 303 → /repeaters/add?error", over.StatusCode, over.Header.Get("Location"))
}
atID, _ := meshcore.GenerateLocalIdentity(rand.Reader)
at := post(t, ts, h.app, "/repeaters", form(strings.Repeat("x", maxRepeaterNameLen), atID.String()), sess)
at.Body.Close()
if loc, _ := url.Parse(at.Header.Get("Location")); at.StatusCode != http.StatusSeeOther || !strings.HasSuffix(loc.Path, "/added") {
t.Fatalf("at-limit create = %d %q, want 303 → /repeaters/{id}/added", at.StatusCode, at.Header.Get("Location"))
}
}
// Black-box coverage for the repeater, sharing, invite, and admin POST endpoints.
// All redirect (303) and do not render; each test asserts the redirect target and,
// where cheap, a store side-effect.
+7 -6
View File
@@ -54,11 +54,12 @@ func (s *Handlers) handleSetupCommands(w http.ResponseWriter, r *http.Request) {
http.Error(w, "bad request", http.StatusBadRequest)
return
}
req.Name = strings.TrimSpace(req.Name)
if req.Name == "" {
http.Error(w, "name is required", http.StatusBadRequest)
name, ok := cleanRepeaterName(req.Name)
if !ok {
http.Error(w, "name is required and must be 31 characters or fewer", http.StatusBadRequest)
return
}
req.Name = name
var cmds []string
cmds = append(cmds, "set name "+req.Name)
@@ -223,10 +224,10 @@ func (s *Handlers) profileSteps(ctx context.Context, orgID int64, name string) (
func (s *Handlers) handleSetupComplete(w http.ResponseWriter, r *http.Request) {
uid := s.Auth.CurrentUserID(r.Context())
name := strings.TrimSpace(r.FormValue("name"))
name, ok := cleanRepeaterName(r.FormValue("name"))
pubHex := strings.ToLower(strings.TrimSpace(r.FormValue("public_key")))
if name == "" {
http.Error(w, "name is required", http.StatusBadRequest)
if !ok {
http.Error(w, "name is required and must be 31 characters or fewer", http.StatusBadRequest)
return
}
if _, err := meshcore.NewIdentityFromHex(pubHex); err != nil {
+22 -6
View File
@@ -161,14 +161,30 @@ func (s *Handlers) pageRepeaterAdded(w http.ResponseWriter, r *http.Request) {
})
}
// maxRepeaterNameLen bounds a repeater name to what MeshCore firmware can store
// and advertise. A node's name lives in a 32-byte buffer (`node_name[32]` in
// CommonCLI.h) and `set name` copies into it with StrHelper::strncpy(...,
// sizeof(node_name)), whose loop runs while buf_sz > 1 and then writes the NUL
// terminator — so 31 usable bytes. A longer name is silently truncated on the
// device, so we reject it here rather than record a name the hardware won't
// actually advertise. (Verified against the MeshCore firmware source.)
const maxRepeaterNameLen = 31
// cleanRepeaterName trims s and reports whether it is a usable repeater name:
// non-empty and within the firmware's byte limit.
func cleanRepeaterName(s string) (string, bool) {
s = strings.TrimSpace(s)
return s, s != "" && len(s) <= maxRepeaterNameLen
}
// handleAddRepeater registers a new repeater (unconfirmed) for the current user.
func (s *Handlers) handleAddRepeater(w http.ResponseWriter, r *http.Request) {
uid := s.Auth.CurrentUserID(r.Context())
name := strings.TrimSpace(r.FormValue("name"))
name, ok := cleanRepeaterName(r.FormValue("name"))
pubHex := strings.ToLower(strings.TrimSpace(r.FormValue("public_key")))
if name == "" {
addErr(w, r, "Repeater name is required.")
if !ok {
addErr(w, r, "Repeater name is required and must be 31 characters or fewer.")
return
}
// Validate the public key is a real 32-byte MeshCore key.
@@ -245,9 +261,9 @@ func (s *Handlers) handleEditRepeater(w http.ResponseWriter, r *http.Request) {
editErr := func(msg string) {
web.RedirectErr(w, r, "/repeaters/"+repeaterParam(r)+"/edit", msg)
}
name := strings.TrimSpace(r.FormValue("name"))
if name == "" {
editErr("Repeater name is required.")
name, ok := cleanRepeaterName(r.FormValue("name"))
if !ok {
editErr("Repeater name is required and must be 31 characters or fewer.")
return
}
freq, bw, sf, cr, valid := parseRadioForm(r)
+19
View File
@@ -239,9 +239,28 @@ func (e *Env) CommonMiddleware(r chi.Router) {
r.Use(CaptureRemoteAddr) // preserve the true TCP peer before we resolve
r.Use(e.resolveClientIP) // trusted-proxy-aware X-Forwarded-For resolution
r.Use(e.securityHeaders) // CSP (+ per-request script nonce) and hardening headers
r.Use(limitBody) // cap request bodies before any handler reads them
r.Use(middleware.Recoverer)
}
// maxRequestBody caps the request body every surface will read. The app has no
// file uploads — the largest bodies are form posts and small JSON (serial-setup
// command lists, markdown docs) — so 1 MiB is generous while stopping a client
// from streaming an arbitrarily large body into memory before per-field limits
// can apply. WebSocket upgrades carry no request body, so this doesn't affect
// the console/confirm sockets.
const maxRequestBody = 1 << 20 // 1 MiB
// limitBody caps r.Body so an oversized request fails fast — a read past the
// limit errors and the handler surfaces its generic 400/500 — instead of
// buffering unbounded data.
func limitBody(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
r.Body = http.MaxBytesReader(w, r.Body, maxRequestBody)
next.ServeHTTP(w, r)
})
}
// SharedRoutes registers endpoints every surface needs (health, static assets).
func (e *Env) SharedRoutes(r chi.Router) {
r.Get("/healthz", func(w http.ResponseWriter, _ *http.Request) {
+36
View File
@@ -0,0 +1,36 @@
package web
import (
"io"
"net/http"
"net/http/httptest"
"strings"
"testing"
)
// TestLimitBody: a request body within the cap reads fully, one past the cap
// errors when the handler drains it. Regression for the pre-release audit
// finding that no request-body size limit existed.
func TestLimitBody(t *testing.T) {
t.Parallel()
// A handler that drains the body and reports whether the read succeeded.
var readErr error
h := limitBody(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
_, readErr = io.ReadAll(r.Body)
}))
post := func(size int) {
req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(strings.Repeat("x", size)))
h.ServeHTTP(httptest.NewRecorder(), req)
}
post(maxRequestBody) // exactly at the cap
if readErr != nil {
t.Fatalf("body at the cap should read cleanly, got %v", readErr)
}
post(maxRequestBody + 1) // one byte over
if readErr == nil {
t.Fatal("body over the cap should fail to read, got nil error")
}
}