treat wrong or missing uid as a storage miss, do not nuke inbox

go/chat/storage/basebox.go

@@ -1,12 +1,14 @@
 package storage
 
 import (
+	"bytes"
 	"context"
 	"fmt"
 
 	"github.com/keybase/client/go/chat/globals"
 	"github.com/keybase/client/go/encrypteddb"
 	"github.com/keybase/client/go/libkb"
+	"github.com/keybase/client/go/protocol/gregor1"
 	"github.com/keybase/client/go/protocol/keybase1"
 )
 
@@ -42,6 +44,14 @@ func (i *baseBox) writeDiskBox(ctx context.Context, key libkb.DbKey, data any) e
 	return i.encryptedDB.Put(ctx, key, data)
 }
 
+func (i *baseBox) missIfWrongSessionUID(uid gregor1.UID) Error {
+	me := i.G().ExternalG().ActiveDevice.UID()
+	if uid.IsNil() || !me.Exists() || !bytes.Equal(me.ToBytes(), uid) {
+		return MissError{Msg: "uid mismatch"}
+	}
+	return nil
+}
+
 func (i *baseBox) maybeNuke(err Error, key libkb.DbKey) {
 	if err != nil && err.ShouldClear() {
 		i.G().Log.Debug("nuking %v on err: %v", key, err)

go/chat/storage/inbox.go

@@ -179,6 +179,13 @@ func (i *Inbox) dbConvKey(uid gregor1.UID, convID chat1.ConversationID) libkb.Db
 	}
 }
 
+func (i *Inbox) diskReadError(ctx context.Context, uid gregor1.UID, err error) Error {
+	if _, ok := err.(libkb.LoginRequiredError); ok {
+		return MiscError{Msg: err.Error()}
+	}
+	return NewInternalError(ctx, i.DebugLabeler, "failed to read inbox: uid: %s err: %s", uid, err)
+}
+
 func (i *Inbox) maybeNuke(ctx context.Context, ef func() Error, uid gregor1.UID) {
 	err := ef()
 	if err != nil && err.ShouldClear() {
@@ -195,18 +202,17 @@ func (i *Inbox) readDiskVersions(ctx context.Context, uid gregor1.UID, useInMemo
 	if err := isAbortedRequest(ctx); err != nil {
 		return ibox, err
 	}
+	if err := i.missIfWrongSessionUID(uid); err != nil {
+		return ibox, err
+	}
 	// Check in memory cache first
 	if memibox := inboxMemCache.GetVersions(uid); useInMemory && memibox != nil {
 		i.Debug(ctx, "readDiskVersions: hit in memory cache")
 		ibox = *memibox
 	} else {
 		found, err := i.readDiskBox(ctx, i.dbVersionsKey(uid), &ibox)
 		if err != nil {
-			if _, ok := err.(libkb.LoginRequiredError); ok {
-				return ibox, MiscError{Msg: err.Error()}
-			}
-			return ibox, NewInternalError(ctx, i.DebugLabeler,
-				"failed to read inbox: uid: %d err: %s", uid, err)
+			return ibox, i.diskReadError(ctx, uid, err)
 		}
 		if !found {
 			return ibox, MissError{}
@@ -241,6 +247,9 @@ func (i *Inbox) readDiskVersions(ctx context.Context, uid gregor1.UID, useInMemo
 }
 
 func (i *Inbox) writeDiskVersions(ctx context.Context, uid gregor1.UID, ibox inboxDiskVersions) Error {
+	if err := i.missIfWrongSessionUID(uid); err != nil {
+		return err
+	}
 	// Get latest server version
 	vers, err := i.G().ServerCacheVersions.Fetch(ctx)
 	if err != nil {
@@ -263,18 +272,17 @@ func (i *Inbox) readDiskIndex(ctx context.Context, uid gregor1.UID, useInMemory
 	if err := isAbortedRequest(ctx); err != nil {
 		return ibox, err
 	}
+	if err := i.missIfWrongSessionUID(uid); err != nil {
+		return ibox, err
+	}
 	// Check in memory cache first
 	if memibox := inboxMemCache.GetIndex(uid); useInMemory && memibox != nil {
 		i.Debug(ctx, "readDiskIndex: hit in memory cache")
 		ibox = *memibox
 	} else {
 		found, err := i.readDiskBox(ctx, i.dbIndexKey(uid), &ibox)
 		if err != nil {
-			if _, ok := err.(libkb.LoginRequiredError); ok {
-				return ibox, MiscError{Msg: err.Error()}
-			}
-			return ibox, NewInternalError(ctx, i.DebugLabeler,
-				"failed to read inbox: uid: %d err: %s", uid, err)
+			return ibox, i.diskReadError(ctx, uid, err)
 		}
 		if !found {
 			return ibox, MissError{}
@@ -288,6 +296,9 @@ func (i *Inbox) readDiskIndex(ctx context.Context, uid gregor1.UID, useInMemory
 }
 
 func (i *Inbox) writeDiskIndex(ctx context.Context, uid gregor1.UID, ibox inboxDiskIndex) Error {
+	if err := i.missIfWrongSessionUID(uid); err != nil {
+		return err
+	}
 	i.Debug(ctx, "writeDiskIndex: convs: %d queries: %d", len(ibox.ConversationIDs), len(ibox.Queries))
 	inboxMemCache.PutIndex(uid, &ibox)
 	if err := i.writeDiskBox(ctx, i.dbIndexKey(uid), ibox); err != nil {
@@ -297,6 +308,9 @@ func (i *Inbox) writeDiskIndex(ctx context.Context, uid gregor1.UID, ibox inboxD
 }
 
 func (i *Inbox) readConvs(ctx context.Context, uid gregor1.UID, convIDs []chat1.ConversationID) (res []types.RemoteConversation, err Error) {
+	if err := i.missIfWrongSessionUID(uid); err != nil {
+		return res, err
+	}
 	res = make([]types.RemoteConversation, 0, len(convIDs))
 	memHits := make(map[chat1.ConvIDStr]bool, len(convIDs))
 	for _, convID := range convIDs {
@@ -320,11 +334,7 @@ func (i *Inbox) readConvs(ctx context.Context, uid gregor1.UID, convIDs []chat1.
 		dbReads++
 		found, err := i.readDiskBox(ctx, i.dbConvKey(uid, convID), &conv)
 		if err != nil {
-			if _, ok := err.(libkb.LoginRequiredError); ok {
-				return res, MiscError{Msg: err.Error()}
-			}
-			return res, NewInternalError(ctx, i.DebugLabeler,
-				"failed to read inbox: uid: %d err: %s", uid, err)
+			return res, i.diskReadError(ctx, uid, err)
 		}
 		if !found {
 			return res, MissError{}
@@ -349,6 +359,9 @@ func (i *Inbox) readConv(ctx context.Context, uid gregor1.UID, convID chat1.Conv
 func (i *Inbox) writeConvs(ctx context.Context, uid gregor1.UID, convs []types.RemoteConversation,
 	withVersionCheck bool,
 ) Error {
+	if err := i.missIfWrongSessionUID(uid); err != nil {
+		return err
+	}
 	i.summarizeConvs(convs)
 	for _, conv := range convs {
 		if withVersionCheck {
@@ -716,6 +729,7 @@ func (i *Inbox) clearLocked(ctx context.Context, uid gregor1.UID) (err Error) {
 	var iboxIndex inboxDiskIndex
 	if iboxIndex, err = i.readDiskIndex(ctx, uid, true); err != nil {
 		i.Debug(ctx, "Clear: failed to read index: %s", err)
+		return err
 	}
 	for _, convID := range iboxIndex.ConversationIDs {
 		if ierr := i.G().LocalChatDb.Delete(i.dbConvKey(uid, convID)); ierr != nil {

go/chat/storage/inbox_memcache.go

@@ -1,6 +1,7 @@
 package storage
 
 import (
+	"strings"
 	"sync"
 
 	"github.com/keybase/client/go/chat/types"
@@ -81,7 +82,12 @@ func (i *inboxMemCacheImpl) Clear(uid gregor1.UID) {
 	defer i.Unlock()
 	delete(i.versMap, uid.String())
 	delete(i.indexMap, uid.String())
-	i.convMap = make(map[string]types.RemoteConversation)
+	prefix := uid.String()
+	for k := range i.convMap {
+		if strings.HasPrefix(k, prefix) {
+			delete(i.convMap, k)
+		}
+	}
 }
 
 func (i *inboxMemCacheImpl) clearCache() {

go/chat/storage/inbox_test.go

@@ -743,17 +743,9 @@ func TestInboxMembershipUpdate(t *testing.T) {
 	ctc, inbox, uid := setupInboxTest(t, "membership")
 	defer ctc.Cleanup()
 
-	u2, err := kbtest.CreateAndSignupFakeUser("ib", ctc.G)
-	require.NoError(t, err)
-	uid2 := gregor1.UID(u2.User.GetUID().ToBytes())
-
-	u3, err := kbtest.CreateAndSignupFakeUser("ib", ctc.G)
-	require.NoError(t, err)
-	uid3 := gregor1.UID(u3.User.GetUID().ToBytes())
-
-	u4, err := kbtest.CreateAndSignupFakeUser("ib", ctc.G)
-	require.NoError(t, err)
-	uid4 := gregor1.UID(u4.User.GetUID().ToBytes())
+	uid2 := makeUID(t)
+	uid3 := makeUID(t)
+	uid4 := makeUID(t)
 
 	t.Logf("uid: %s uid2: %s uid3: %s uid4: %s", uid, uid2, uid3, uid4)
 
@@ -932,3 +924,61 @@ func TestUpdateLocalMtime(t *testing.T) {
 	require.Equal(t, mtime1, convs[0].GetMtime())
 	require.Equal(t, mtime2, convs[1].GetMtime())
 }
+
+func TestInboxWrongSessionUIDIsMissNotNuke(t *testing.T) {
+	tc, inbox, uidA := setupInboxTest(t, "decmiss")
+	defer tc.Cleanup()
+
+	conv := makeConvo(gregor1.Time(1), 1, 1)
+	require.NoError(t, inbox.Merge(context.TODO(), uidA, 7, []chat1.Conversation{conv.Conv}, nil))
+
+	_, found, err := tc.G.LocalChatDb.GetRaw(inbox.dbVersionsKey(uidA))
+	require.NoError(t, err)
+	require.True(t, found)
+
+	_, _, err = inbox.Read(context.TODO(), nil, nil)
+	require.ErrorAs(t, err, new(MissError))
+	_, found, err = tc.G.LocalChatDb.GetRaw(inbox.dbVersionsKey(uidA))
+	require.NoError(t, err)
+	require.True(t, found, "empty request uid must not delete inbox versions")
+
+	_, err = kbtest.CreateAndSignupFakeUser("ib", tc.G)
+	require.NoError(t, err)
+
+	_, _, err = inbox.Read(context.TODO(), uidA, nil)
+	require.ErrorAs(t, err, new(MissError))
+
+	_, found, err = tc.G.LocalChatDb.GetRaw(inbox.dbVersionsKey(uidA))
+	require.NoError(t, err)
+	require.True(t, found, "wrong-session uid must not delete inbox versions")
+}
+
+func TestInboxMemCacheClearOnlyUID(t *testing.T) {
+	uidA := gregor1.UID([]byte("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"))
+	uidB := gregor1.UID([]byte("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"))
+	convA := makeConvo(gregor1.Time(1), 1, 1)
+	convB := makeConvo(gregor1.Time(2), 1, 1)
+	inboxMemCache.PutConv(uidA, convA)
+	inboxMemCache.PutConv(uidB, convB)
+	inboxMemCache.Clear(uidA)
+	require.Nil(t, inboxMemCache.GetConv(uidA, convA.GetConvID()))
+	require.NotNil(t, inboxMemCache.GetConv(uidB, convB.GetConvID()))
+	inboxMemCache.clearCache()
+}
+
+func TestInboxClearLockedIndexErrorKeepsVersions(t *testing.T) {
+	tc, inbox, uid := setupInboxTest(t, "clridx")
+	defer tc.Cleanup()
+
+	conv := makeConvo(gregor1.Time(1), 1, 1)
+	require.NoError(t, inbox.Merge(context.TODO(), uid, 3, []chat1.Conversation{conv.Conv}, nil))
+	require.NoError(t, tc.G.LocalChatDb.PutRaw(inbox.dbIndexKey(uid), []byte("not-a-box")))
+	inboxMemCache.Clear(uid)
+
+	err := inbox.clearLocked(context.TODO(), uid)
+	require.Error(t, err)
+
+	_, found, gerr := tc.G.LocalChatDb.GetRaw(inbox.dbVersionsKey(uid))
+	require.NoError(t, gerr)
+	require.True(t, found, "clearLocked must not delete versions when the index cannot be read")
+}

go/chat/storage/outbox_basebox.go

@@ -45,6 +45,9 @@ func (s *outboxBaseboxStorage) clear(ctx context.Context) Error {
 
 func (s *outboxBaseboxStorage) readStorage(ctx context.Context) (res diskOutbox, err Error) {
 	defer func() { s.maybeNuke(err, s.dbKey()) }()
+	if err := s.missIfWrongSessionUID(s.uid); err != nil {
+		return res, err
+	}
 
 	if memobox := outboxMemCache.Get(s.uid); memobox != nil {
 		s.Debug(ctx, "hit in memory cache")
@@ -76,6 +79,9 @@ func (s *outboxBaseboxStorage) readStorage(ctx context.Context) (res diskOutbox,
 
 func (s *outboxBaseboxStorage) writeStorage(ctx context.Context, obox diskOutbox) (err Error) {
 	defer func() { s.maybeNuke(err, s.dbKey()) }()
+	if err := s.missIfWrongSessionUID(s.uid); err != nil {
+		return err
+	}
 	if ierr := s.writeDiskBox(ctx, s.dbKey(), obox); ierr != nil {
 		return NewInternalError(ctx, s.DebugLabeler, "error writing outbox: err: %s", ierr)
 	}

go/chat/storage/readoutbox.go

@@ -59,6 +59,9 @@ func (o *ReadOutbox) clear(ctx context.Context) Error {
 }
 
 func (o *ReadOutbox) readStorage(ctx context.Context) (res diskReadOutbox) {
+	if err := o.missIfWrongSessionUID(o.uid); err != nil {
+		return diskReadOutbox{Version: readOutboxVersion}
+	}
 	if memobox := readOutboxMemCache.Get(o.uid); memobox != nil {
 		o.Debug(ctx, "hit in memory cache")
 		res = *memobox
@@ -87,6 +90,9 @@ func (o *ReadOutbox) readStorage(ctx context.Context) (res diskReadOutbox) {
 }
 
 func (o *ReadOutbox) writeStorage(ctx context.Context, obox diskReadOutbox) (err Error) {
+	if err := o.missIfWrongSessionUID(o.uid); err != nil {
+		return err
+	}
 	if ierr := o.writeDiskBox(ctx, o.dbKey(), obox); ierr != nil {
 		return NewInternalError(ctx, o.DebugLabeler, "error writing outbox: err: %s", ierr)
 	}