feat(session): Validate MLS proposal sender and type

session/mls.go

@@ -387,6 +387,24 @@ func (s *session) processProposalBatchLocked(proposals []byte) (bool, error) {
 			return acceptedAny, ErrEmptyProposal
 		}
 
+		allowed, err := s.proposalAllowedByDAVELocked(msg)
+		if err != nil {
+			// fail closed: if we can't inspect the proposal safely, reject it.
+			s.logger.Warn("processProposalBatchLocked: rejecting proposal that failed inspection",
+				"error", err,
+				"index", i)
+			remaining = remaining[len(wire):]
+
+			continue
+		}
+		if !allowed {
+			s.logger.Warn("processProposalBatchLocked: rejecting disallowed proposal sender/type",
+				"index", i)
+			remaining = remaining[len(wire):]
+
+			continue
+		}
+
 		userID, ok, err := s.addProposalUserID(msg)
 		if err != nil {
 			// This is a Proposal we couldn't parse or extract an identity from —
@@ -427,6 +445,42 @@ func (s *session) processProposalBatchLocked(proposals []byte) (bool, error) {
 	return acceptedAny, nil
 }
 
+func (s *session) proposalAllowedByDAVELocked(msg *framing.MLSMessage) (bool, error) {
+	if msg == nil {
+		return false, nil
+	}
+
+	pubMsg, ok := msg.AsPublic()
+	if !ok || pubMsg == nil {
+		return false, nil
+	}
+
+	if pubMsg.Content.Sender.Type != framing.SenderTypeExternal {
+		return false, nil
+	}
+
+	if pubMsg.Content.ContentType() != framing.ContentTypeProposal {
+		return false, nil
+	}
+
+	data, ok := pubMsg.Content.ProposalData()
+	if !ok {
+		return false, nil
+	}
+
+	proposal, err := group.UnmarshalProposal(data)
+	if err != nil {
+		return false, err
+	}
+
+	switch proposal.Type {
+	case group.ProposalTypeAdd, group.ProposalTypeRemove:
+		return true, nil
+	default:
+		return false, nil
+	}
+}
+
 func (s *session) addProposalUserID(msg *framing.MLSMessage) (godave.UserID, bool, error) {
 	if msg == nil {
 		return "", false, nil

session/proposal_validation_test.go

@@ -2,50 +2,69 @@ package session
 
 import (
 	"context"
+	"crypto/ecdsa"
 	"testing"
 
 	"github.com/disgoorg/godave"
 	"github.com/thomas-vilte/mls-go"
 	"github.com/thomas-vilte/mls-go/ciphersuite"
+	"github.com/thomas-vilte/mls-go/framing"
+	"github.com/thomas-vilte/mls-go/group"
+	"github.com/thomas-vilte/mls-go/keypackages"
 	memorystore "github.com/thomas-vilte/mls-go/storage/memory"
 )
 
-func setupActiveSoleMemberSession(t *testing.T) *session {
+func setupActiveSoleMemberSessionWithKey(t *testing.T) (*session, *ecdsa.PrivateKey) {
 	t.Helper()
 
+	pkg, extPriv := buildExternalSenderPackageWithKey(t)
 	s := New(nil, "123456789", testCallbacks{}).(*session)
 	s.SetChannelID(987654321)
 	s.OnSelectProtocolAck(1)
-	s.OnDaveMLSExternalSenderPackage(buildExternalSenderPackage(t))
+	s.OnDaveMLSExternalSenderPackage(pkg)
 	s.OnDavePrepareTransition(0, 1)
 
 	if s.groupID == nil || s.mlsClient == nil || s.activeEpoch == nil {
 		t.Fatal("expected active sole-member MLS epoch to be initialized")
 	}
 
-	return s
+	return s, extPriv
 }
 
-func addProposalBytesForUser(t *testing.T, s *session, userID string) []byte {
+func addProposalBytesForUser(t *testing.T, s *session, extPriv *ecdsa.PrivateKey, userID string) []byte {
 	t.Helper()
 
 	identity, err := userIDToIdentityBytes(godave.UserID(userID))
 	if err != nil {
 		t.Fatalf("userIDToIdentityBytes(%q): %v", userID, err)
 	}
 
-	return addProposalBytesForIdentity(t, s, identity)
+	return addProposalBytesAsExternalSender(t, s, extPriv, identity)
 }
 
-// addProposalBytesForIdentity is the same as addProposalBytesForUser but
+// addProposalBytesAsExternalSender is the same as addProposalBytesForUser but
 // takes raw identity bytes directly, bypassing the 8-byte snowflake
 // encoding — used to build a credential that parses fine at the MLS level
 // but that identityBytesToUserID (which requires exactly 8 bytes) can't
 // turn into a UserID.
-func addProposalBytesForIdentity(t *testing.T, s *session, identity []byte) []byte {
+func addProposalBytesForIdentity(t *testing.T, s *session, extPriv *ecdsa.PrivateKey, identity []byte) []byte {
+	t.Helper()
+
+	return addProposalBytesAsExternalSender(t, s, extPriv, identity)
+}
+
+// addProposalBytesAsExternalSender builds an Add proposal signed by the
+// external sender (the voice gateway), not by a group member. This mirrors
+// how DAVE delivers proposals in reality (protocol.md:166: "Only Add and
+// Remove proposals sent by the voice gateway external sender are allowed").
+// The bot's own ProposeAddMember signs as SenderTypeMember, which the
+// #16 check now rejects; tests must generate proposals the way the gateway
+// actually would.
+func addProposalBytesAsExternalSender(t *testing.T, s *session, extPriv *ecdsa.PrivateKey, identity []byte) []byte {
 	t.Helper()
 
 	ctx := context.Background()
+
 	store := memorystore.NewStore()
 	member, err := mls.NewClient(identity, ciphersuite.MLS128DHKEMP256,
 		mls.WithStorage(store, store),
@@ -55,20 +74,48 @@ func addProposalBytesForIdentity(t *testing.T, s *session, identity []byte) []by
 		t.Fatalf("mls.NewClient(identity=%x): %v", identity, err)
 	}
 
-	keyPackage, err := member.FreshKeyPackageBytes(ctx)
+	keyPackageBytes, err := member.FreshKeyPackageBytes(ctx)
 	if err != nil {
 		t.Fatalf("FreshKeyPackageBytes(identity=%x): %v", identity, err)
 	}
+	keyPackage, err := keypackages.UnmarshalKeyPackage(keyPackageBytes)
+	if err != nil {
+		t.Fatalf("UnmarshalKeyPackage: %v", err)
+	}
+
+	proposal := group.NewAddProposal(keyPackage)
+	proposalData := group.ProposalMarshal(proposal)
+
+	epoch, err := s.mlsClient.client.Epoch(ctx, s.groupID)
+	if err != nil {
+		t.Fatalf("Epoch: %v", err)
+	}
 
-	proposalBytes, err := s.mlsClient.client.ProposeAddMember(ctx, s.groupID, keyPackage)
+	groupInfoBytes, err := s.mlsClient.client.GroupInfo(ctx, s.groupID)
 	if err != nil {
-		t.Fatalf("ProposeAddMember(identity=%x): %v", identity, err)
+		t.Fatalf("GroupInfo: %v", err)
 	}
-	if err := s.mlsClient.client.CancelPendingProposals(ctx, s.groupID); err != nil {
-		t.Fatalf("CancelPendingProposals(identity=%x): %v", identity, err)
+	info, err := group.UnmarshalGroupInfo(groupInfoBytes)
+	if err != nil {
+		t.Fatalf("UnmarshalGroupInfo: %v", err)
+	}
+	gc := info.GroupContext.Marshal()
+
+	content := framing.FramedContent{
+		GroupID:           s.groupID,
+		Epoch:             epoch,
+		Sender:            framing.Sender{Type: framing.SenderTypeExternal, SenderIndex: 0},
+		AuthenticatedData: []byte{},
+		Body:              framing.ProposalBody{Data: proposalData},
+	}
+
+	sigKey := ciphersuite.NewSignaturePrivateKey(extPriv)
+	pm, err := framing.NewPublicMessage(content, sigKey, gc, nil, ciphersuite.MLS128DHKEMP256)
+	if err != nil {
+		t.Fatalf("NewPublicMessage: %v", err)
 	}
 
-	return proposalBytes
+	return framing.NewMLSMessagePublic(pm).Marshal()
 }
 
 func memberCount(t *testing.T, s *session) int {
@@ -82,10 +129,23 @@ func memberCount(t *testing.T, s *session) int {
 	return len(members)
 }
 
+func mutateMLSMessage(t *testing.T, wire []byte, mutate func(*framing.MLSMessage)) []byte {
+	t.Helper()
+
+	msg, err := framing.UnmarshalMLSMessage(wire)
+	if err != nil {
+		t.Fatalf("framing.UnmarshalMLSMessage: %v", err)
+	}
+
+	mutate(msg)
+
+	return msg.Marshal()
+}
+
 func TestOnDaveMLSProposals_AddValidation(t *testing.T) {
 	t.Run("ignores unexpected add", func(t *testing.T) {
-		s := setupActiveSoleMemberSession(t)
-		proposalBytes := addProposalBytesForUser(t, s, "222222222")
+		s, extPriv := setupActiveSoleMemberSessionWithKey(t)
+		proposalBytes := addProposalBytesForUser(t, s, extPriv, "222222222")
 
 		if got := memberCount(t, s); got != 1 {
 			t.Fatalf("initial member count = %d, want 1", got)
@@ -99,10 +159,10 @@ func TestOnDaveMLSProposals_AddValidation(t *testing.T) {
 	})
 
 	t.Run("accepts expected add", func(t *testing.T) {
-		s := setupActiveSoleMemberSession(t)
+		s, extPriv := setupActiveSoleMemberSessionWithKey(t)
 		const expectedUserID = "333333333"
 		s.AddUser(expectedUserID)
-		proposalBytes := addProposalBytesForUser(t, s, expectedUserID)
+		proposalBytes := addProposalBytesForUser(t, s, extPriv, expectedUserID)
 
 		if got := memberCount(t, s); got != 1 {
 			t.Fatalf("initial member count = %d, want 1", got)
@@ -120,8 +180,8 @@ func TestOnDaveMLSProposals_AddValidation(t *testing.T) {
 	// expects. identityBytesToUserID fails on it, and the proposal must be
 	// rejected rather than let through to ProcessPublicMessage unchecked.
 	t.Run("rejects add whose identity fails inspection", func(t *testing.T) {
-		s := setupActiveSoleMemberSession(t)
-		proposalBytes := addProposalBytesForIdentity(t, s, []byte{0x01, 0x02, 0x03})
+		s, extPriv := setupActiveSoleMemberSessionWithKey(t)
+		proposalBytes := addProposalBytesForIdentity(t, s, extPriv, []byte{0x01, 0x02, 0x03})
 
 		if got := memberCount(t, s); got != 1 {
 			t.Fatalf("initial member count = %d, want 1", got)
@@ -134,3 +194,45 @@ func TestOnDaveMLSProposals_AddValidation(t *testing.T) {
 		}
 	})
 }
+
+func TestOnDaveMLSProposals_RejectsNewMemberProposalSender(t *testing.T) {
+	s, extPriv := setupActiveSoleMemberSessionWithKey(t)
+	proposalBytes := addProposalBytesForUser(t, s, extPriv, "222222222")
+
+	mutated := mutateMLSMessage(t, proposalBytes, func(msg *framing.MLSMessage) {
+		pub, _ := msg.AsPublic()
+		pub.Content.Sender.Type = framing.SenderTypeNewMemberProposal
+	})
+
+	if got := memberCount(t, s); got != 1 {
+		t.Fatalf("initial member count = %d, want 1", got)
+	}
+
+	s.OnDaveMLSProposals(buildProposalBatch(mutated))
+
+	if got := memberCount(t, s); got != 1 {
+		t.Fatalf("member count after disallowed sender = %d, want 1", got)
+	}
+}
+
+func TestOnDaveMLSProposals_RejectsDisallowedProposalType(t *testing.T) {
+	s, extPriv := setupActiveSoleMemberSessionWithKey(t)
+	proposalBytes := addProposalBytesForUser(t, s, extPriv, "222222222")
+
+	mutated := mutateMLSMessage(t, proposalBytes, func(msg *framing.MLSMessage) {
+		pub, _ := msg.AsPublic()
+		pub.Content.Body = framing.ProposalBody{
+			Data: group.ProposalMarshal(group.NewPreSharedKeyProposal(1, []byte("psk"))),
+		}
+	})
+
+	if got := memberCount(t, s); got != 1 {
+		t.Fatalf("initial member count = %d, want 1", got)
+	}
+
+	s.OnDaveMLSProposals(buildProposalBatch(mutated))
+
+	if got := memberCount(t, s); got != 1 {
+		t.Fatalf("member count after disallowed proposal type = %d, want 1", got)
+	}
+}

session/sole_member_test.go

@@ -32,6 +32,17 @@ func writeVLBytes(b []byte) []byte {
 // the voice gateway sends it: VL(signature_public_key) || Credential_inline.
 func buildExternalSenderPackage(t *testing.T) []byte {
 	t.Helper()
+	pkg, _ := buildExternalSenderPackageWithKey(t)
+
+	return pkg
+}
+
+// buildExternalSenderPackageWithKey is like buildExternalSenderPackage but also
+// returns the ECDH private key so tests can sign proposals as the external
+// sender (simulating the gateway, which is how DAVE delivers proposals in
+// reality — not as a group member).
+func buildExternalSenderPackageWithKey(t *testing.T) ([]byte, *ecdsa.PrivateKey) {
+	t.Helper()
 
 	priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
 	if err != nil {
@@ -44,7 +55,7 @@ func buildExternalSenderPackage(t *testing.T) []byte {
 
 	cred := credentials.NewBasicCredentialFromUint64(1)
 
-	return append(writeVLBytes(ecdhPub.Bytes()), cred.Marshal()...)
+	return append(writeVLBytes(ecdhPub.Bytes()), cred.Marshal()...), priv
 }
 
 // TestSoleMemberReset_EncryptsWhileAlone reproduces the DAVE "Sole member reset":