diff --git a/go/chat/storage/basebox.go b/go/chat/storage/basebox.go index 16216e8c6d72..9ea146b54054 100644 --- a/go/chat/storage/basebox.go +++ b/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) diff --git a/go/chat/storage/inbox.go b/go/chat/storage/inbox.go index ed4b3b0a97a9..9ad9407c2bf5 100644 --- a/go/chat/storage/inbox.go +++ b/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,6 +202,9 @@ 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") @@ -202,11 +212,7 @@ func (i *Inbox) readDiskVersions(ctx context.Context, uid gregor1.UID, useInMemo } 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,6 +272,9 @@ 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") @@ -270,11 +282,7 @@ func (i *Inbox) readDiskIndex(ctx context.Context, uid gregor1.UID, useInMemory } 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 { diff --git a/go/chat/storage/inbox_memcache.go b/go/chat/storage/inbox_memcache.go index aa8b5aff1d5b..45829da6b610 100644 --- a/go/chat/storage/inbox_memcache.go +++ b/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() { diff --git a/go/chat/storage/inbox_test.go b/go/chat/storage/inbox_test.go index 39ae9c6038f1..c3ab38e8b165 100644 --- a/go/chat/storage/inbox_test.go +++ b/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") +} diff --git a/go/chat/storage/outbox_basebox.go b/go/chat/storage/outbox_basebox.go index 3adb70cc8be3..4d9daca2abaf 100644 --- a/go/chat/storage/outbox_basebox.go +++ b/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) } diff --git a/go/chat/storage/readoutbox.go b/go/chat/storage/readoutbox.go index bcace03aab76..f794a19b8f43 100644 --- a/go/chat/storage/readoutbox.go +++ b/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) }