Fix logging gap

This commit is contained in:
Jonathon Leight
2026-07-05 18:31:35 -04:00
parent dedfa1d4d4
commit 43bfd6e888
4 changed files with 95 additions and 5 deletions
+9 -3
View File
@@ -156,10 +156,13 @@ func (s *Service) RegisterFinish(w http.ResponseWriter, r *http.Request) {
}
}
// Verify the credential before writing anything durable.
// Verify the credential before writing anything durable. A failure here is a
// client/ceremony error (400), but the underlying go-webauthn detail can leak
// internals, so log it server-side and return only a generic message.
cred, err := s.wa.FinishRegistration(waUser, *sessionData, r)
if err != nil {
httpError(w, r, http.StatusBadRequest, "registration failed: "+err.Error(), nil)
web.LogError(r, "webauthn: finish registration", err)
httpError(w, r, http.StatusBadRequest, "registration failed", nil)
return
}
blob, err := json.Marshal(cred)
@@ -254,9 +257,12 @@ func (s *Service) LoginFinish(w http.ResponseWriter, r *http.Request) {
return
}
// Assertion failure is a client error (401), but the go-webauthn detail can
// leak internals — log it server-side and return only a generic message.
cred, err := s.wa.FinishLogin(waUser, *sessionData, r)
if err != nil {
httpError(w, r, http.StatusUnauthorized, "login failed: "+err.Error(), nil)
web.LogError(r, "webauthn: finish login", err)
httpError(w, r, http.StatusUnauthorized, "login failed", nil)
return
}
// Persist the updated sign counter / clone-warning state.
+76
View File
@@ -0,0 +1,76 @@
package core
import (
"encoding/json"
"io"
"net/http"
"net/http/cookiejar"
"net/http/httptest"
"strings"
"testing"
"github.com/go-webauthn/webauthn/webauthn"
)
// TestLoginFinishDoesNotLeakWebAuthnError drives a passkey assertion to failure
// and asserts the client sees only a generic message — the go-webauthn internal
// detail must be logged server-side, not returned. Regression for the error-leak
// audit (auth/handlers.go LoginFinish).
func TestLoginFinishDoesNotLeakWebAuthnError(t *testing.T) {
t.Parallel()
st, ctx, ts, h := splitServer(t)
user, err := st.CreateUser(ctx, "wauser", "")
if err != nil {
t.Fatalf("create user: %v", err)
}
// A minimal stored credential so LoginBegin has something to challenge; its key
// material is never exercised because the assertion body below fails to parse.
blob, err := json.Marshal(webauthn.Credential{ID: []byte("cred-1")})
if err != nil {
t.Fatalf("marshal credential: %v", err)
}
if err := st.AddCredential(ctx, user.ID, []byte("cred-1"), blob, "key"); err != nil {
t.Fatalf("add credential: %v", err)
}
// One client with a cookie jar so the ceremony stashed at /begin is present at
// /finish (both carry the same session cookie).
jar, _ := cookiejar.New(nil)
client := &http.Client{Jar: jar, CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse }}
begin := authJSON(t, client, ts, h.auth, "/api/login/begin", `{"username":"wauser"}`)
begin.Body.Close()
if begin.StatusCode != http.StatusOK {
t.Fatalf("login begin status = %d, want 200", begin.StatusCode)
}
// A malformed assertion body makes FinishLogin fail. The response must be the
// generic message only.
finish := authJSON(t, client, ts, h.auth, "/api/login/finish", `{}`)
body, _ := io.ReadAll(finish.Body)
finish.Body.Close()
if finish.StatusCode != http.StatusUnauthorized {
t.Fatalf("login finish status = %d, want 401 (body %s)", finish.StatusCode, body)
}
got := strings.TrimSpace(string(body))
if got != `{"error":"login failed"}` {
t.Fatalf("login finish body = %s, want generic {\"error\":\"login failed\"} with no leaked detail", got)
}
}
// authJSON POSTs a JSON body to the given host/path using the provided client.
func authJSON(t *testing.T, client *http.Client, ts *httptest.Server, host, path, jsonBody string) *http.Response {
t.Helper()
req, err := http.NewRequest(http.MethodPost, ts.URL+path, strings.NewReader(jsonBody))
if err != nil {
t.Fatalf("request: %v", err)
}
req.Host = host
req.Header.Set("Content-Type", "application/json")
resp, err := client.Do(req)
if err != nil {
t.Fatalf("post %s%s: %v", host, path, err)
}
return resp
}
+6 -2
View File
@@ -134,6 +134,9 @@ func (s *Handlers) wsConfirm(w http.ResponseWriter, r *http.Request) {
}
if err := modem.Connect(ctx); err != nil {
// The user's own local modem — the detail helps them troubleshoot, so keep it
// in the status frame; also log it so operators see connection failures.
web.LogError(r, "confirm: modem connect", err, "repeater_id", id)
_ = bridge.Status("error", "modem connect: "+err.Error())
return
}
@@ -180,6 +183,7 @@ func (s *Handlers) wsConfirm(w http.ResponseWriter, r *http.Request) {
SF: uint8(rep.RadioSF), //nolint:gosec // G115: radio config value is bounded (preset-constrained)
CR: uint8(rep.RadioCR), //nolint:gosec // G115: radio config value is bounded (preset-constrained)
}); err != nil {
web.LogError(r, "confirm: set radio", err, "repeater_id", id)
_ = bridge.Status("error", "set radio: "+err.Error())
return
}
@@ -205,7 +209,7 @@ func (s *Handlers) wsConfirm(w http.ResponseWriter, r *http.Request) {
if err := s.Store.SetRepeaterConfirmed(ctx, id, uid, lr.IsAdmin, int16(lr.Permissions)); err != nil {
web.LogError(r, "confirm: save confirmation", err, "repeater_id", id)
_ = bridge.Status("error", "could not save confirmation: "+err.Error())
_ = bridge.Status("error", "Could not save the confirmation — please try again.")
return
}
if debug {
@@ -268,7 +272,7 @@ func (s *Handlers) fetchAndStoreLocation(ctx context.Context, r *http.Request, e
}
if err := s.Store.SetRepeaterLocation(ctx, id, lat, lon); err != nil {
web.LogError(r, "confirm: store location", err, "repeater_id", id)
_ = bridge.Status("error", "could not store location: "+err.Error())
_ = bridge.Status("error", "Could not store the location — please try again.")
return 0, 0, false
}
_ = bridge.Status("info", fmt.Sprintf("Stored location: %.5f, %.5f", lat, lon))
+4
View File
@@ -226,6 +226,9 @@ func (s *Handlers) wsConsole(w http.ResponseWriter, r *http.Request) {
}()
if err := modem.Connect(ctx); err != nil {
// The user's own local modem — keep the detail for troubleshooting, and log
// it so operators see connection failures.
web.LogError(r, "console: modem connect", err, "repeater_id", id)
_ = bridge.Status("error", "modem connect: "+err.Error())
return
}
@@ -289,6 +292,7 @@ func (s *Handlers) wsConsole(w http.ResponseWriter, r *http.Request) {
SF: uint8(rep.RadioSF), //nolint:gosec // G115: radio config value is bounded (preset-constrained)
CR: uint8(rep.RadioCR), //nolint:gosec // G115: radio config value is bounded (preset-constrained)
}); err != nil {
web.LogError(r, "console: set radio", err, "repeater_id", id)
_ = bridge.Status("error", "set radio: "+err.Error())
return
}