diff --git a/pkg/client/client.go b/pkg/client/client.go index 2768f40..005df6a 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -799,7 +799,17 @@ func (c *Client) processFrame(roomID string, info roomView, data []byte, handler if err != nil { return } - if info.Policy.SignMsgs && f.Sig != nil { + // A room with SignMsgs REQUIRES a signature, so an unsigned frame is + // unauthenticated and must be dropped — not silently accepted. The previous + // `&& f.Sig != nil` guard verified the signature only when one was present, so + // an attacker with data-plane access could publish a frame with Sig==nil and a + // forged Sender and have the receiver accept it as authentic in a room that + // demands signatures (audit N3, report 0006). Requiring the signature first + // closes that spoof. + if info.Policy.SignMsgs { + if f.Sig == nil { + return // signature required by room policy but absent: drop + } pub, err := c.signerPub(roomID, f.Sender) if err != nil || !cs.VerifyEd25519(pub, f.SigningBytes(), f.Sig) { return // unauthenticated frame: drop diff --git a/pkg/client/sig_nil_spoof_test.go b/pkg/client/sig_nil_spoof_test.go new file mode 100644 index 0000000..8d1baa5 --- /dev/null +++ b/pkg/client/sig_nil_spoof_test.go @@ -0,0 +1,154 @@ +package client_test + +import ( + "sync" + "testing" + "time" + + "github.com/enmanuel/unibus/pkg/client" + "github.com/enmanuel/unibus/pkg/frame" + "github.com/enmanuel/unibus/pkg/room" + "github.com/nats-io/nats.go" +) + +// TestReaudit_SigNilSpoof ports the re-auditor's N3 (Alto) finding: in a room +// that REQUIRES per-message signatures, an attacker with data-plane access +// publishes a raw frame with Sig==nil and a forged Sender. Before the fix +// processFrame verified the signature only when one was present +// (`SignMsgs && f.Sig != nil`), so the receiver accepted the unsigned, forged +// frame as authentic. The fix drops any unsigned frame in a SignMsgs room. +// +// Coverage: +// - golden: a properly signed frame from a real member IS delivered; +// - error : an unsigned frame with a forged Sender in a SignMsgs room is DROPPED; +// - edge : a room WITHOUT SignMsgs still delivers an unsigned frame (the drop +// is specific to signed rooms, not a blanket reject of unsigned frames). +func TestReaudit_SigNilSpoof(t *testing.T) { + h := newHarness(t) + waitHealth(t, h.ctrlURL) + + alice, err := client.New(h.natsURL, h.ctrlURL, mustIdentity(t)) + if err != nil { + t.Fatalf("connect alice: %v", err) + } + defer alice.Close() + bob, err := client.New(h.natsURL, h.ctrlURL, mustIdentity(t)) + if err != nil { + t.Fatalf("connect bob: %v", err) + } + defer bob.Close() + + // A signed-but-NOT-encrypted room: SignMsgs enforces authorship, and the lack + // of encryption is exactly the case the auditor flagged as Alto (any peer with + // the subject can forge a sender if signatures are not strictly required). + const subject = "room.signed.spoof" + signedPolicy := room.Policy{Encrypt: false, Persist: false, SignMsgs: true} + roomID, err := alice.CreateRoom(subject, signedPolicy) + if err != nil { + t.Fatalf("alice create signed room: %v", err) + } + if err := alice.Invite(roomID, bob.Endpoint()); err != nil { + t.Fatalf("alice invite bob: %v", err) + } + if err := bob.Join(roomID); err != nil { + t.Fatalf("bob join: %v", err) + } + + var mu sync.Mutex + var got []string + sub, err := bob.Subscribe(roomID, func(_ frame.Frame, plaintext []byte) { + mu.Lock() + got = append(got, string(plaintext)) + mu.Unlock() + }) + if err != nil { + t.Fatalf("bob subscribe: %v", err) + } + defer sub.Unsubscribe() + time.Sleep(150 * time.Millisecond) + + // Attacker: a raw NATS connection (the dev harness leaves the data plane open), + // no identity, forged Sender, NO signature. + const spoofMsg = "I am totally the victim" + rawAtk, err := nats.Connect(h.natsURL) + if err != nil { + t.Fatalf("attacker raw connect: %v", err) + } + defer rawAtk.Close() + spoof := frame.Frame{ + Type: frame.PUB, + Subject: subject, + Sender: "victim-forged-endpoint", + MsgID: "spoof-1", + Epoch: 1, + Payload: []byte(spoofMsg), + // Sig intentionally nil — this is the attack. + } + sb, err := spoof.Marshal() + if err != nil { + t.Fatalf("marshal spoof: %v", err) + } + if err := rawAtk.Publish(subject, sb); err != nil { + t.Fatalf("attacker publish: %v", err) + } + _ = rawAtk.Flush() + + // Golden: alice's properly signed frame must be delivered. + const goodMsg = "authentic from alice" + if err := alice.Publish(roomID, []byte(goodMsg)); err != nil { + t.Fatalf("alice publish: %v", err) + } + if !waitFor(&mu, &got, func(rs []string) bool { + for _, r := range rs { + if r == goodMsg { + return true + } + } + return false + }, 2*time.Second) { + t.Fatalf("a properly signed frame should be delivered; got %v", snapshot(&mu, &got)) + } + + // Error path: the unsigned, forged frame must NEVER reach the handler. + for _, r := range snapshot(&mu, &got) { + if r == spoofMsg { + t.Fatalf("SIG-NIL SPOOF: receiver accepted an unsigned frame with a forged Sender in a SignMsgs room") + } + } + + // Edge: a room WITHOUT SignMsgs still delivers an unsigned raw frame, proving + // the drop is scoped to signed rooms and did not break the plain-NATS path. + const subjectOpen = "room.open.nosig" + openRoom, err := alice.CreateRoom(subjectOpen, room.ModeNATS) + if err != nil { + t.Fatalf("alice create open room: %v", err) + } + openCol := subscribeCollect(t, alice, openRoom) + defer openCol.sub.Unsubscribe() + time.Sleep(150 * time.Millisecond) + + const openMsg = "unsigned but allowed here" + openFrame := frame.Frame{ + Type: frame.PUB, + Subject: subjectOpen, + Sender: "anyone", + MsgID: "open-1", + Payload: []byte(openMsg), + // no Sig — fine in a non-signed room + } + ob, _ := openFrame.Marshal() + if err := rawAtk.Publish(subjectOpen, ob); err != nil { + t.Fatalf("publish open frame: %v", err) + } + _ = rawAtk.Flush() + if !waitFor(&openCol.mu, &openCol.msgs, func(rs []string) bool { + for _, r := range rs { + if r == openMsg { + return true + } + } + return false + }, 2*time.Second) { + t.Fatalf("an unsigned frame in a non-signed room should be delivered; got %v", snapshot(&openCol.mu, &openCol.msgs)) + } +}