dataforge/unibus
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(membership): owner binding, pre-auth nonce-cache fix, generic errors

Browse Source 
Three medium audit findings.

H6 (owner spoof): handleCreateRoom now binds the body's declared owner to the
authenticated signer — both the endpoint id and the signing key must be the
signer's — so a registered peer cannot create rooms in another identity's name.
Enforced only when an authenticated signer is present.

H7 (nonce-cache poison pre-auth): IsAuthorized now runs BEFORE the replay cache
is touched, so an unregistered identity (Ed25519 keys are free) can no longer
seed nonces into it. The cache is rewritten with O(expired) pruning (insertion
order equals expiry order under a constant TTL) instead of the previous O(n)
full-map scan under the mutex, plus a size cap with oldest-eviction. This is the
prerequisite the 0003 replicated nonce store builds on.

H12 (error leak): internal store/blob errors are logged and replaced with a
generic client message via writeServerErr, so SQL fragments and filesystem paths
no longer reach the caller. Crafted 4xx messages (owner-sig, validation) are kept.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Egutierrez
2026-06-07 14:36:22 +02:00
parent 957b728160
commit 0aa2caae43
2 changed files with 89 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/membership/auth.go
+59 -18
View File
@@ -75,6 +75,11 @@ const (
const (
clockSkew = 30 * time.Second
nonceTTL = 60 * time.Second
// maxNonceCacheEntries bounds the replay cache so it cannot grow without limit
// (audit H7). With IsAuthorized now gating insertion, only authorized traffic
// is cached, so this ceiling is only approached under a legitimate burst; at
// the cap the oldest nonce is evicted (its TTL is nearly up anyway).
maxNonceCacheEntries = 100_000
)
// CanonicalRequest returns the exact bytes that are signed and verified for a
@@ -91,34 +96,64 @@ func CanonicalRequest(method, path, ts, nonce string, body []byte) []byte {
}
// nonceCache remembers recently-seen nonces to reject replays. It is an
// in-memory map guarded by a mutex with lazy expiry — sufficient for a single
// membershipd process (the spec's chosen tradeoff over a server-issued nonce
// round-trip). A distributed deployment would need a shared store.
// in-memory store guarded by a mutex — sufficient for a single membershipd
// process (the spec's chosen tradeoff over a server-issued nonce round-trip). A
// distributed deployment would need a shared store (tracked for issue 0003).
//
// Pruning is O(expired), not O(n): because the TTL is constant, insertion order
// equals expiry order, so the oldest entries (front of `order`) are exactly the
// ones that expire first (audit H7 — the previous full-map scan under the mutex
// was a CPU-amplification vector). A size cap bounds memory.
type nonceCache struct {
mu sync.Mutex
seen map[string]time.Time
ttl time.Duration
mu sync.Mutex
seen map[string]time.Time // nonce -> expiry
order []string // nonces in insertion order == expiry order
ttl time.Duration
cap int
}
func newNonceCache(ttl time.Duration) *nonceCache {
return &nonceCache{seen: make(map[string]time.Time), ttl: ttl}
func newNonceCache(ttl time.Duration, capacity int) *nonceCache {
return &nonceCache{seen: make(map[string]time.Time), ttl: ttl, cap: capacity}
}
// rememberOrReject records nonce and returns true if it was unseen, or false if
// it is a replay (still live in the cache). Expired entries are pruned lazily on
// each call so the map cannot grow without bound under steady traffic.
// it is a replay (still live in the cache).
func (n *nonceCache) rememberOrReject(nonce string, now time.Time) bool {
n.mu.Lock()
defer n.mu.Unlock()
for k, exp := range n.seen {
if exp.Before(now) {
delete(n.seen, k)
// Prune expired entries from the front (oldest first). The first live entry
// ends the scan — everything behind it was inserted later and is newer.
cut := 0
for cut < len(n.order) {
exp, ok := n.seen[n.order[cut]]
if !ok {
cut++ // already evicted by the cap path below
continue
}
if !exp.Before(now) {
break
}
delete(n.seen, n.order[cut])
cut++
}
if cut > 0 {
n.order = append(n.order[:0], n.order[cut:]...)
}
if exp, ok := n.seen[nonce]; ok && !exp.Before(now) {
return false
return false // a live replay
}
// Bound memory: at capacity, evict the oldest entry (its TTL is nearly up).
for len(n.seen) >= n.cap && len(n.order) > 0 {
oldest := n.order[0]
n.order = n.order[1:]
delete(n.seen, oldest)
}
n.seen[nonce] = now.Add(n.ttl)
n.order = append(n.order, nonce)
return true
}
@@ -172,10 +207,9 @@ func (s *Server) authenticate(r *http.Request, body []byte, now time.Time) (auth
return authResult{}, fmt.Errorf("invalid signature")
}
if !s.nonces.rememberOrReject(nonce, now) {
return authResult{}, fmt.Errorf("replayed nonce")
}
// Authorize BEFORE touching the replay cache (audit H7): an unregistered
// identity can mint valid signatures for free, so caching its nonces would let
// it poison/grow the cache pre-auth. Only authorized identities are remembered.
if !s.store.IsAuthorized(pubHex) {
return authResult{}, fmt.Errorf("identity not authorized")
}
@@ -185,5 +219,12 @@ func (s *Server) authenticate(r *http.Request, body []byte, now time.Time) (auth
// IsAuthorized passed but the row vanished (race with revoke): fail closed.
return authResult{}, fmt.Errorf("identity not authorized")
}
// Anti-replay last: a replayed request from an authorized identity is still
// rejected here (the nonce is already live in the cache from its first use).
if !s.nonces.rememberOrReject(nonce, now) {
return authResult{}, fmt.Errorf("replayed nonce")
}
return authResult{pubHex: pubHex, endpoint: frame.EndpointID(pub), user: user}, nil
}
pkg/membership/server.go
+30 -10
View File
@@ -19,6 +19,7 @@ import (
"golang.org/x/time/rate"
"github.com/enmanuel/unibus/pkg/blobstore"
"github.com/enmanuel/unibus/pkg/frame"
)
// Body-size ceilings for the control plane. They bound how much an unauthenticated
@@ -84,7 +85,7 @@ func NewServer(store *Store, blobs *blobstore.Store, authMode AuthMode) *Server
blobs: blobs,
mux: http.NewServeMux(),
authMode: authMode,
nonces: newNonceCache(nonceTTL),
nonces: newNonceCache(nonceTTL, maxNonceCacheEntries),
limiter: newIPRateLimiter(defaultRatePerSec, defaultRateBurst, rateBucketTTL),
}
s.routes()
@@ -307,6 +308,15 @@ func writeErr(w http.ResponseWriter, code int, msg string) {
writeJSON(w, code, map[string]string{"error": msg})
}
// writeServerErr logs the internal error detail and returns ONLY a generic
// message to the client (audit H12): raw store/blob errors embed SQL fragments
// and filesystem paths, which must not leak to a caller. Use it for any error
// that originates inside the server (5xx, or a not-found wrapping a store error).
func writeServerErr(w http.ResponseWriter, r *http.Request, code int, publicMsg string, err error) {
log.Printf("[handler] %s %s -> %d: %v", r.Method, r.URL.Path, code, err)
writeErr(w, code, publicMsg)
}
// canonicalSig returns the bytes to verify for a request: the request struct
// re-marshaled with its Sig field cleared. The caller passes a copy with Sig
// already zeroed. This is symmetric with how the client signs.
@@ -359,6 +369,16 @@ func (s *Server) handleCreateRoom(w http.ResponseWriter, r *http.Request) {
"cleartext rooms are disabled on this deployment; create an encrypted (Matrix-policy) room")
return
}
// Owner binding (audit H6): the declared owner must BE the authenticated
// signer — both the endpoint id and the signing key. Otherwise a registered
// peer could create rooms in another identity's name. Enforced only when an
// authenticated signer is present (AuthOff/dev trusts the caller).
if signer, ok := signerEndpoint(r); ok {
if req.Owner.Endpoint != signer || frame.EndpointID(req.Owner.SignPub) != signer {
writeErr(w, http.StatusForbidden, "forbidden: room owner must be the authenticated signer")
return
}
}
roomID := newULID()
info := RoomInfo{
RoomID: roomID,
@@ -369,7 +389,7 @@ func (s *Server) handleCreateRoom(w http.ResponseWriter, r *http.Request) {
OwnerEndpoint: req.Owner.Endpoint,
}
if err := s.store.CreateRoom(info, req.Owner.SignPub, req.Owner.KexPub, req.SealedKeySelf); err != nil {
writeErr(w, http.StatusInternalServerError, err.Error())
writeServerErr(w, r, http.StatusInternalServerError, "internal error", err)
return
}
writeJSON(w, http.StatusCreated, createRoomResp{RoomID: roomID})
@@ -391,7 +411,7 @@ func (s *Server) handleInvite(w http.ResponseWriter, r *http.Request) {
}
info, err := s.store.GetRoom(roomID)
if err != nil {
writeErr(w, http.StatusNotFound, err.Error())
writeServerErr(w, r, http.StatusNotFound, "room not found", err)
return
}
m := Member{
@@ -401,7 +421,7 @@ func (s *Server) handleInvite(w http.ResponseWriter, r *http.Request) {
KexPub: req.Member.KexPub,
}
if err := s.store.AddMember(roomID, m, info.Epoch, req.SealedKey); err != nil {
writeErr(w, http.StatusInternalServerError, err.Error())
writeServerErr(w, r, http.StatusInternalServerError, "internal error", err)
return
}
writeJSON(w, http.StatusOK, map[string]string{"status": "invited"})
@@ -441,7 +461,7 @@ func (s *Server) handleGetKey(w http.ResponseWriter, r *http.Request) {
"not invited to this encrypted room: no key has been sealed for your identity. Ask the room owner to invite you before joining.")
return
}
writeErr(w, http.StatusInternalServerError, err.Error())
writeServerErr(w, r, http.StatusInternalServerError, "internal error", err)
return
}
writeJSON(w, http.StatusOK, keyResp{Epoch: ep, SealedKey: sealed})
@@ -533,7 +553,7 @@ func (s *Server) handleRekey(w http.ResponseWriter, r *http.Request) {
// Bump epoch, then store the fresh sealed keys for the remaining members,
// then remove the kicked/left members.
if err := s.store.BumpEpoch(roomID, req.NewEpoch); err != nil {
writeErr(w, http.StatusInternalServerError, err.Error())
writeServerErr(w, r, http.StatusInternalServerError, "internal error", err)
return
}
keys := make(map[string][]byte, len(req.Keys))
@@ -542,13 +562,13 @@ func (s *Server) handleRekey(w http.ResponseWriter, r *http.Request) {
}
if len(keys) > 0 {
if err := s.store.PutSealedKeys(roomID, req.NewEpoch, keys); err != nil {
writeErr(w, http.StatusInternalServerError, err.Error())
writeServerErr(w, r, http.StatusInternalServerError, "internal error", err)
return
}
}
for _, ep := range req.Remove {
if err := s.store.RemoveMember(roomID, ep); err != nil {
writeErr(w, http.StatusInternalServerError, err.Error())
writeServerErr(w, r, http.StatusInternalServerError, "internal error", err)
return
}
}
@@ -572,7 +592,7 @@ func (s *Server) handlePutBlob(w http.ResponseWriter, r *http.Request) {
}
hash, err := s.blobs.Put(data)
if err != nil {
writeErr(w, http.StatusInternalServerError, err.Error())
writeServerErr(w, r, http.StatusInternalServerError, "internal error", err)
return
}
writeJSON(w, http.StatusOK, blobResp{Hash: hash})
@@ -586,7 +606,7 @@ func (s *Server) handleGetBlob(w http.ResponseWriter, r *http.Request) {
}
data, err := s.blobs.Get(hash)
if err != nil {
writeErr(w, http.StatusNotFound, err.Error())
writeServerErr(w, r, http.StatusNotFound, "not found", err)
return
}
w.Header().Set("Content-Type", "application/octet-stream")