Skip to content

Commit

Permalink
Store unboxing errors in EK caches (#13258)
Browse files Browse the repository at this point in the history 
* skip unboxing, fix dek storage

* teamek storage

* refactor to err msg, user ek storage

* tests

* cleanup

* cleanup 2

* fix test

* Bust teamEK caches on newlyAdded/exit team
  • Loading branch information
@joshblum
joshblum committed Aug 14, 2018
1 parent ba254a8 commit 6256145
Show file tree
Hide file tree
Showing 26 changed files with 592 additions and 177 deletions.
4 changes: 4 additions & 0 deletions go/chat/boxer.go
View file
Expand Up @@ -292,6 +292,10 @@ func (b *Boxer) UnboxMessage(ctx context.Context, boxed chat1.MessageBoxed, conv
// If the message is exploding, load the ephemeral key.
var ephemeralSeed *keybase1.TeamEk
if boxed.IsEphemeral() {
// Don't bother if the message is already expired.
if boxed.IsEphemeralExpired(b.clock.Now()) {
return b.makeErrorMessage(ctx, boxed, NewPermanentUnboxingError(NewEphemeralAlreadyExpiredError())), nil
}
ek, ekErr := CtxKeyFinder(ctx, b.G()).EphemeralKeyForDecryption(
ctx, tlfName, boxed.ClientHeader.Conv.Tlfid, conv.GetMembersType(), boxed.ClientHeader.TlfPublic,
boxed.EphemeralMetadata().Generation)
Expand Down
16 changes: 16 additions & 0 deletions go/chat/errors.go
View file
Expand Up @@ -138,6 +138,22 @@ func (e TransientUnboxingError) InternalError() string {

//=============================================================================

type EphemeralAlreadyExpiredError struct{ inner error }

func NewEphemeralAlreadyExpiredError() EphemeralAlreadyExpiredError {
return EphemeralAlreadyExpiredError{}
}

func (e EphemeralAlreadyExpiredError) Error() string {
return "Unable to decrypt already exploded message"
}

func (e EphemeralAlreadyExpiredError) InternalError() string {
return e.Error()
}

//=============================================================================

type EphemeralUnboxingError struct{ inner error }

func NewEphemeralUnboxingError(inner error) EphemeralUnboxingError {
Expand Down
38 changes: 13 additions & 25 deletions go/ephemeral/common.go
View file
Expand Up @@ -16,40 +16,28 @@ const (
TeamEKStr EKType = "teamEK"
)

type EKUnboxErr struct {
boxType EKType
boxGeneration keybase1.EkGeneration
missingType EKType
missingGeneration keybase1.EkGeneration
type EphemeralKeyError struct {
Msg string
}

func newEKUnboxErr(boxType EKType, boxGeneration keybase1.EkGeneration, missingType EKType, missingGeneration keybase1.EkGeneration) EKUnboxErr {
return EKUnboxErr{
missingType: missingType,
boxType: boxType,
missingGeneration: missingGeneration,
boxGeneration: boxGeneration,
}
}

func (e EKUnboxErr) Error() string {
return fmt.Sprintf("Error unboxing %s@generation:%v missing %s@generation:%v", e.boxType, e.boxGeneration, e.missingType, e.missingGeneration)
func newEKUnboxErr(boxType EKType, boxGeneration keybase1.EkGeneration, missingType EKType, missingGeneration keybase1.EkGeneration) EphemeralKeyError {
msg := fmt.Sprintf("Error unboxing %s@generation:%v missing %s@generation:%v", boxType, boxGeneration, missingType, missingGeneration)
return newEphemeralKeyError(msg)
}

type EKMissingBoxErr struct {
boxType EKType
boxGeneration keybase1.EkGeneration
func newEKMissingBoxErr(boxType EKType, boxGeneration keybase1.EkGeneration) EphemeralKeyError {
msg := fmt.Sprintf("Missing box for %s@generation:%v", boxType, boxGeneration)
return newEphemeralKeyError(msg)
}

func newEKMissingBoxErr(boxType EKType, boxGeneration keybase1.EkGeneration) EKMissingBoxErr {
return EKMissingBoxErr{
boxType: boxType,
boxGeneration: boxGeneration,
func newEphemeralKeyError(msg string) EphemeralKeyError {
return EphemeralKeyError{
Msg: msg,
}
}

func (e EKMissingBoxErr) Error() string {
return fmt.Sprintf("Missing box for %s@generation:%v", e.boxType, e.boxGeneration)
func (e EphemeralKeyError) Error() string {
return e.Msg
}

func ctimeIsStale(ctime time.Time, currentMerkleRoot libkb.MerkleRoot) bool {
Expand Down
113 changes: 79 additions & 34 deletions go/ephemeral/device_ek_storage.go
View file
Expand Up @@ -18,21 +18,27 @@ import (
const deviceEKSubDir = "device-eks"
const deviceEKPrefix = "deviceEphemeralKey"

type deviceEKCacheItem struct {
DeviceEK keybase1.DeviceEk
Err error
}

type deviceEKCache map[keybase1.EkGeneration]deviceEKCacheItem
type DeviceEKMap map[keybase1.EkGeneration]keybase1.DeviceEk

type DeviceEKStorage struct {
libkb.Contextified
sync.Mutex
storage erasablekv.ErasableKVStore
cache DeviceEKMap
cache deviceEKCache
indexed bool
}

func NewDeviceEKStorage(g *libkb.GlobalContext) *DeviceEKStorage {
return &DeviceEKStorage{
Contextified: libkb.NewContextified(g),
storage: erasablekv.NewFileErasableKVStore(g, deviceEKSubDir),
cache: make(DeviceEKMap),
cache: make(deviceEKCache),
}
}

Expand Down Expand Up @@ -110,8 +116,7 @@ func (s *DeviceEKStorage) Put(ctx context.Context, generation keybase1.EkGenerat
if deviceEK.Metadata.DeviceCtime == 0 {
deviceEK.Metadata.DeviceCtime = keybase1.ToTime(time.Now())
}
err = s.storage.Put(ctx, key, deviceEK)
if err != nil {
if err = s.storage.Put(ctx, key, deviceEK); err != nil {
return err
}

Expand All @@ -120,7 +125,10 @@ func (s *DeviceEKStorage) Put(ctx context.Context, generation keybase1.EkGenerat
if err != nil {
return err
}
cache[generation] = deviceEK
cache[generation] = deviceEKCacheItem{
DeviceEK: deviceEK,
Err: nil,
}
return nil
}

Expand All @@ -133,18 +141,23 @@ func (s *DeviceEKStorage) Get(ctx context.Context, generation keybase1.EkGenerat
if err != nil {
return deviceEK, err
}
deviceEK, ok := cache[generation]
cacheItem, ok := cache[generation]
if ok {
return deviceEK, nil
return cacheItem.DeviceEK, cacheItem.Err
}
// Try persistent storage.
deviceEK, err = s.get(ctx, generation)
if err != nil {
switch err.(type) {
case nil, erasablekv.UnboxError:
// cache the result
cache[generation] = deviceEKCacheItem{
DeviceEK: deviceEK,
Err: err,
}
return deviceEK, err
default:
return deviceEK, err
}
// cache the result
cache[generation] = deviceEK
return deviceEK, nil
}

func (s *DeviceEKStorage) get(ctx context.Context, generation keybase1.EkGeneration) (deviceEK keybase1.DeviceEk, err error) {
Expand Down Expand Up @@ -190,7 +203,7 @@ func (s *DeviceEKStorage) delete(ctx context.Context, generation keybase1.EkGene
return s.storage.Erase(ctx, key)
}

func (s *DeviceEKStorage) getCache(ctx context.Context) (cache DeviceEKMap, err error) {
func (s *DeviceEKStorage) getCache(ctx context.Context) (cache deviceEKCache, err error) {
defer s.G().CTraceTimed(ctx, "DeviceEKStorage#getCache", func() error { return err })()

if !s.indexed {
Expand All @@ -204,20 +217,19 @@ func (s *DeviceEKStorage) getCache(ctx context.Context) (cache DeviceEKMap, err
return nil, err
}
if generation < 0 {
s.G().Log.CDebugf(ctx, "getCache: invalid generation: %s -> %s", key, generation)
s.G().Log.CDebugf(ctx, "DeviceEKStorage#getCache: invalid generation: %s -> %s", key, generation)
continue
}
deviceEK, err := s.get(ctx, generation)
if err != nil {
switch err.(type) {
case erasablekv.UnboxError:
s.G().Log.Debug("DeviceEKStorage#getCache failed to get item from storage: %v", err)
continue
default:
return nil, err
switch err.(type) {
case nil, erasablekv.UnboxError:
s.cache[generation] = deviceEKCacheItem{
DeviceEK: deviceEK,
Err: err,
}
default:
return nil, err
}
s.cache[generation] = deviceEK
}
s.indexed = true
}
Expand All @@ -231,7 +243,7 @@ func (s *DeviceEKStorage) ClearCache() {
}

func (s *DeviceEKStorage) clearCache() {
s.cache = make(DeviceEKMap)
s.cache = make(deviceEKCache)
s.indexed = false
}

Expand All @@ -241,7 +253,18 @@ func (s *DeviceEKStorage) GetAll(ctx context.Context) (deviceEKs DeviceEKMap, er
s.Lock()
defer s.Unlock()

return s.getCache(ctx)
cache, err := s.getCache(ctx)
if err != nil {
return nil, err
}
deviceEKs = make(DeviceEKMap)
for gen, cacheItem := range cache {
if cacheItem.Err != nil {
continue
}
deviceEKs[gen] = cacheItem.DeviceEK
}
return deviceEKs, nil
}

func (s *DeviceEKStorage) GetAllActive(ctx context.Context, merkleRoot libkb.MerkleRoot) (metadatas []keybase1.DeviceEkMetadata, err error) {
Expand All @@ -256,7 +279,11 @@ func (s *DeviceEKStorage) GetAllActive(ctx context.Context, merkleRoot libkb.Mer
}

activeKeysInOrder := []keybase1.DeviceEkMetadata{}
for _, deviceEK := range cache {
for _, cacheItem := range cache {
if cacheItem.Err != nil {
continue
}
deviceEK := cacheItem.DeviceEK
// Skip expired keys. Expired keys are spared from deletion past for a
// window past their expiry date, in case they're needed for
// decryption, but they're never signed over or used for encryption.
Expand Down Expand Up @@ -308,7 +335,10 @@ func (s *DeviceEKStorage) MaxGeneration(ctx context.Context) (maxGeneration keyb
if err != nil {
return maxGeneration, err
}
for generation := range cache {
for generation, cacheItem := range cache {
if cacheItem.Err != nil {
continue
}
if generation > maxGeneration {
maxGeneration = generation
}
Expand Down Expand Up @@ -337,17 +367,24 @@ func (s *DeviceEKStorage) DeleteExpired(ctx context.Context, merkleRoot libkb.Me
}

keyMap := make(keyExpiryMap)
for generation, deviceEK := range cache {
var ctime keybase1.Time
// If we have a nil root _and_ a valid DeviceCtime, use that. If we're
// missing a DeviceCtime it's better to use the slightly off
// merkleCtime than a 0
if merkleRoot.IsNil() && deviceEK.Metadata.DeviceCtime > 0 {
ctime = deviceEK.Metadata.DeviceCtime
// We delete expired and invalid cache entries but only return the expired.
toDelete := []keybase1.EkGeneration{}
for generation, cacheItem := range cache {
if cacheItem.Err != nil {
toDelete = append(toDelete, generation)
} else {
ctime = deviceEK.Metadata.Ctime
deviceEK := cacheItem.DeviceEK
var ctime keybase1.Time
// If we have a nil root _and_ a valid DeviceCtime, use that. If we're
// missing a DeviceCtime it's better to use the slightly off
// merkleCtime than a 0
if merkleRoot.IsNil() && deviceEK.Metadata.DeviceCtime > 0 {
ctime = deviceEK.Metadata.DeviceCtime
} else {
ctime = deviceEK.Metadata.Ctime
}
keyMap[generation] = ctime
}
keyMap[generation] = ctime
}

expired = s.getExpiredGenerations(context.Background(), keyMap, now)
Expand All @@ -356,6 +393,14 @@ func (s *DeviceEKStorage) DeleteExpired(ctx context.Context, merkleRoot libkb.Me
epick.Push(s.delete(ctx, generation))
}

// Delete any invalid cache entries we were harboring but don't return the
// error to the caller.
for _, generation := range toDelete {
if err := s.delete(ctx, generation); err != nil {
s.G().Log.CDebugf(ctx, "unable to delete generation (with cache err) %v: %v", generation, err)
}
}

epick.Push(s.deletedWrongEldestSeqno(ctx))
return expired, epick.Error()
}
Expand Down
20 changes: 17 additions & 3 deletions go/ephemeral/device_ek_storage_test.go
View file
Expand Up @@ -78,9 +78,9 @@ func TestDeviceEKStorage(t *testing.T) {
}

// Test Get nonexistent
deviceEK, err := s.Get(context.Background(), 5)
nonexistent, err := s.Get(context.Background(), keybase1.EkGeneration(len(testKeys)+1))
require.Error(t, err)
require.Equal(t, keybase1.DeviceEk{}, deviceEK)
require.Equal(t, keybase1.DeviceEk{}, nonexistent)

s.ClearCache()
// Test GetAll
Expand All @@ -97,7 +97,7 @@ func TestDeviceEKStorage(t *testing.T) {
// Test Delete
require.NoError(t, s.Delete(context.Background(), 2))

deviceEK, err = s.Get(context.Background(), 2)
deviceEK, err := s.Get(context.Background(), 2)
require.Error(t, err)
require.Equal(t, keybase1.DeviceEk{}, deviceEK)

Expand Down Expand Up @@ -147,6 +147,20 @@ func TestDeviceEKStorage(t *testing.T) {
err = erasableStorage.Get(context.Background(), badEldestSeqnoKey, &badEldestSeqnoDeviceEK)
require.Error(t, err)
require.Equal(t, badEldestSeqnoDeviceEK, keybase1.DeviceEk{})

// Verify we store failures in the cache
t.Logf("cache failures")
nonexistent, err = s.Get(context.Background(), maxGeneration+1)
require.Error(t, err)
require.Equal(t, keybase1.DeviceEk{}, nonexistent)

cache, err := s.getCache(context.Background())
require.NoError(t, err)
require.Len(t, cache, 1)

cacheItem, ok := cache[maxGeneration+1]
require.True(t, ok)
require.Error(t, cacheItem.Err)
}

// If we change the key format intentionally, we have to introduce some form of
Expand Down
2 changes: 1 addition & 1 deletion go/ephemeral/handler.go
View file
Expand Up @@ -11,7 +11,7 @@ func HandleNewTeamEK(ctx context.Context, g *libkb.GlobalContext, teamID keybase
defer g.CTrace(ctx, "HandleNewTeamEK", func() error { return err })()

if ekLib := g.GetEKLib(); ekLib != nil {
ekLib.PurgeTeamEKGenCache(teamID, generation)
ekLib.PurgeCachesForTeamIDAndGeneration(ctx, teamID, generation)
}
g.NotifyRouter.HandleNewTeamEK(ctx, teamID, generation)
return nil
Expand Down

0 comments on commit 6256145

Please sign in to comment.