fix(0005b): drop unsigned frames in SignMsgs rooms (close sig-nil spoof)
client.processFrame verified a frame's signature only when one was present
(`info.Policy.SignMsgs && f.Sig != nil`). In a room whose policy REQUIRES
per-message signatures, an attacker with data-plane access could publish a raw
frame with Sig==nil and a forged Sender, and the receiver accepted it as
authentic because the verification block was skipped (audit N3, report 0006).
On a signed-but-cleartext room any peer that knows the subject could thus
impersonate any sender.
Fix: in a SignMsgs room a missing signature is itself a rejection. processFrame
now drops any frame with Sig==nil before attempting verification:
if info.Policy.SignMsgs {
if f.Sig == nil { return } // signature required but absent: drop
// verify ...
}
Non-signed rooms (ModeNATS) are unaffected: unsigned frames there are still
delivered, so the plain-NATS path is unchanged.
Verification (pkg/client/sig_nil_spoof_test.go, TestReaudit_SigNilSpoof):
- golden: a properly signed frame from a member is delivered.
- error : an unsigned frame with a forged Sender in a SignMsgs room is dropped
(the test fails with "SIG-NIL SPOOF: receiver accepted ..." when the fix is
reverted, confirming it is a real regression guard).
- edge : a non-signed room still delivers an unsigned frame.
- CGO_ENABLED=0 go build ./... && go vet ./... && go test -count=1 ./... green.
Refs: report 0006 N3, issue 0005b.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
+11
-1
@@ -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
|
||||
|
||||
@@ -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))
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user