Support for fallback keys #3451
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3451 +/- ##
==========================================
- Coverage 49.37% 49.34% -0.03%
==========================================
Files 522 524 +2
Lines 59488 59674 +186
==========================================
+ Hits 29373 29448 +75
- Misses 26651 26745 +94
- Partials 3464 3481 +17
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
neilalexander
force-pushed
the
fallback-keys
branch
2 times, most recently
from
650d72d to
fcb75ac
Compare
| CONSTRAINT keyserver_fallback_keys_unique UNIQUE (user_id, device_id, algorithm) | ||
| ); | ||
|
|
||
| CREATE INDEX IF NOT EXISTS keyserver_fallback_keys_idx ON keyserver_fallback_keys (user_id, device_id); |
There was a problem hiding this comment.
I think this index is already covered by the constraint above.
(See this, where we have similar patterns for indexes.)
There was a problem hiding this comment.
It's not a given, depending on the index type, that an aggregate index will optimise lookups for constituent columns without all keys present. Therefore a lookup by (user_id, device_id) only is not necessarily optimised by the aggregate (user_id, device_id, algorithm). I think btree indices fall under this category.
| used BOOLEAN NOT NULL | ||
| ); | ||
| CREATE UNIQUE INDEX IF NOT EXISTS keyserver_fallback_keys_unique_idx ON keyserver_fallback_keys(user_id, device_id, algorithm); | ||
| CREATE INDEX IF NOT EXISTS keyserver_fallback_keys_idx ON keyserver_fallback_keys (user_id, device_id); |
There was a problem hiding this comment.
Similar, I think this is covered by the unique index.
userapi/internal/key_api.go
Outdated
| if err := a.KeyDatabase.DeleteFallbackKeys(ctx, req.UserID, req.DeviceID); err != nil { | ||
| res.KeyError(req.UserID, req.DeviceID, &api.KeyError{ | ||
| Err: fmt.Sprintf("%s device %s : failed to clear fallback keys: %s", req.UserID, req.DeviceID, err.Error()), | ||
| }) | ||
| continue | ||
| } |
There was a problem hiding this comment.
Not totally sure why we need to clear existing fallback keys, but doing that in this loop seems odd.
We clear the keys, store a new one, clear the keys, store a new one, etc. Don't we end up with just one key?
neilalexander
force-pushed
the
fallback-keys
branch
2 times, most recently
from
da787e7 to
f3c90b7
Compare
neilalexander
force-pushed
the
fallback-keys
branch
from
f3c90b7 to
b302ef0
Compare
Backports support for fallback keys from Harmony, which should make E2EE more reliable in the face of OTK exhaustion.
Signed-off-by: Neil Alexander [email protected]