Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Introduce a secret inbox #2227

Merged
merged 2 commits into from
Jul 19, 2023
Merged

Introduce a secret inbox #2227

merged 2 commits into from
Jul 19, 2023

Conversation

poljar
Copy link
Contributor

@poljar poljar commented Jul 5, 2023

No description provided.

@poljar poljar requested a review from a team as a code owner July 5, 2023 15:05
@poljar poljar requested review from jplatte and removed request for a team July 5, 2023 15:05
@codecov
Copy link

codecov bot commented Jul 5, 2023 •
edited
Loading

Codecov Report

Patch coverage: 51.51% and project coverage change: +0.11 🎉

Comparison is base (340e0b7) 76.79% compared to head (eedf737) 76.90%.

❗ Current head eedf737 differs from pull request most recent head 643ad2c. Consider uploading reports for the commit 643ad2c to get more accurate results

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #2227      +/-   ##
==========================================
+ Coverage   76.79%   76.90%   +0.11%     
==========================================
  Files         170      170              
  Lines       18088    18150      +62     
==========================================
+ Hits        13891    13959      +68     
+ Misses       4197     4191       -6
Impacted Files Coverage Δ
...atrix-sdk-crypto/src/file_encryption/key_export.rs 97.46% <ø> (ø)
crates/matrix-sdk-crypto/src/gossiping/machine.rs 63.58% <0.00%> (+0.37%) ⬆️
crates/matrix-sdk-crypto/src/gossiping/mod.rs 70.19% <ø> (-0.69%) ⬇️
crates/matrix-sdk-crypto/src/lib.rs 100.00% <ø> (ø)
crates/matrix-sdk-crypto/src/store/traits.rs 70.14% <0.00%> (-4.46%) ⬇️
.../matrix-sdk-crypto/src/types/events/secret_send.rs 66.66% <ø> (ø)
crates/matrix-sdk-crypto/src/store/mod.rs 75.20% <26.66%> (+0.30%) ⬆️
crates/matrix-sdk-crypto/src/store/memorystore.rs 83.44% <50.00%> (-3.77%) ⬇️
crates/matrix-sdk-crypto/src/machine.rs 81.17% <66.66%> (ø)
...s/matrix-sdk-crypto/src/store/integration_tests.rs 100.00% <100.00%> (ø)
... and 1 more

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@Hywan Hywan self-requested a review July 6, 2023 05:52
jplatte
jplatte reviewed Jul 6, 2023
View reviewed changes
crates/matrix-sdk-crypto/src/store/memorystore.rs Outdated Show resolved Hide resolved
crates/matrix-sdk-crypto/src/store/mod.rs Outdated Show resolved Hide resolved
crates/matrix-sdk-crypto/src/store/mod.rs Outdated Show resolved Hide resolved
@poljar poljar mentioned this pull request Jul 7, 2023
Closed
1 task
AndrewFerr added a commit that referenced this pull request Jul 14, 2023
@AndrewFerr
Merge pull request #2227 from poljar/secret-send-inbox
177b042
AndrewFerr added a commit to matrix-org/matrix-rust-sdk-crypto-nodejs that referenced this pull request Jul 14, 2023
@AndrewFerr
HACK: depend on revision that stores backup keys
1e4a8f6 
Depends once this PR is merged:
matrix-org/matrix-rust-sdk#2227
AndrewFerr added a commit to matrix-org/matrix-rust-sdk-crypto-nodejs that referenced this pull request Jul 14, 2023
@AndrewFerr
HACK: depend on revision that stores backup keys
2987de0 
Remove once this PR is merged:
matrix-org/matrix-rust-sdk#2227
jplatte
jplatte reviewed Jul 19, 2023
View reviewed changes
crates/matrix-sdk-crypto/src/gossiping/machine.rs Outdated Show resolved Hide resolved
crates/matrix-sdk-crypto/src/gossiping/machine.rs Outdated Show resolved Hide resolved
crates/matrix-sdk-crypto/src/store/mod.rs Outdated Show resolved Hide resolved
jplatte
jplatte approved these changes Jul 19, 2023
View reviewed changes
Copy link
Collaborator

@jplatte jplatte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't have a lot of context on this, but the code looks good.

poljar and others added 2 commits July 19, 2023 14:10
@poljar
Introduce a secret inbox
b7ca548 
Up until now, users had to listen for to-device events to check for
secrets that were received as an `m.secret.send` event.

This has a bunch of shortcomings:
    1. Once the has been given to the consumer, it's gone and can't be
       retrieved anymore. Secrets may get lost if an app restart happens
       before the consumer decides what to do with it.
    2. The consumer can't be sure if the event was received in a
       secure manner.

This commit ads a inbox for our received secrets where we will long-term
store all secrets we receive until the user decides to delete them.

It's deemed fine to store all secrets, since we only accept secrets we
have requested and if they have been received from a verified device of
ours.
@poljar @jplatte
Broadcast valid secrets we receive over m.secret.send
643ad2c 
Co-authored-by: Jonas Platte <jplatte@matrix.org>
@poljar poljar enabled auto-merge (rebase) July 19, 2023 12:11
@poljar poljar merged commit 9f69c32 into main Jul 19, 2023
38 checks passed
@poljar poljar deleted the poljar/secret-send-inbox branch July 19, 2023 12:27
richvdh added a commit to matrix-org/matrix-rust-sdk-crypto-wasm that referenced this pull request Jul 19, 2023
@richvdh
Update matrix-rust-sdk
1e2970b 
... to pull in matrix-org/matrix-rust-sdk#2227, which
is a pre-requisite for element-hq/crypto-internal#100
@richvdh richvdh mentioned this pull request Jul 19, 2023
Merged
richvdh added a commit to matrix-org/matrix-rust-sdk-crypto-wasm that referenced this pull request Jul 19, 2023
@richvdh
Update matrix-rust-sdk (#9)
225342b 
... to pull in matrix-org/matrix-rust-sdk#2227, which
is a pre-requisite for element-hq/crypto-internal#100
AndrewFerr added a commit to matrix-org/matrix-rust-sdk-crypto-nodejs that referenced this pull request Jul 25, 2023
@AndrewFerr
Update matrix-sdk version
02be195 
Pull in matrix-org/matrix-rust-sdk#2227, which
allows storing backup keys in the memory store
AndrewFerr added a commit to matrix-org/matrix-rust-sdk-crypto-nodejs that referenced this pull request Jul 28, 2023
@AndrewFerr
Update matrix-sdk version
8cc3853 
Pull in matrix-org/matrix-rust-sdk#2227, which
allows storing backup keys in the memory store
@poljar poljar mentioned this pull request Jul 31, 2023
Closed
AndrewFerr added a commit to matrix-org/matrix-rust-sdk-crypto-nodejs that referenced this pull request Aug 1, 2023
@AndrewFerr
Update matrix-sdk version
5cb065b 
Pull in matrix-org/matrix-rust-sdk#2227, which
allows storing backup keys in the memory store
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jplatte jplatte jplatte approved these changes

@Hywan Hywan Awaiting requested review from Hywan

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@poljar @jplatte