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.

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

Add support for the shared history flag defined in MSC3061 #4700

Merged
merged 8 commits into from
Feb 25, 2025

Conversation

poljar
Copy link
Contributor

@poljar poljar commented Feb 20, 2025

This is the first PR in many for the "Encrypted history sharing" feature.

This specific PR tackles element-hq/element-meta#2720.

There are a couple of commits preparing the introduction of the shared_history flag but the meat of the PR is contained in 97a7705. The latter commits add a bunch of tests ensuring that the flag is correctly handled in all the various scenarios where we receive or send a room key.

Thus a review commit by commit is advised.

One final note, the main constructor for InboundGroupSession now has too many arguments, but I'd like to get rid of that constructor or at least make it private. In many (or perhaps all?) cases this constructor is used to create an InboundGroupSession for testing purposes. I think that the Account::create_group_session_pair() method is better suited for this purpose.

  • Public API changes documented in changelogs (optional)

@poljar poljar force-pushed the poljar/shared_history_flag branch 3 times, most recently from fc77217 to b0918eb Compare February 21, 2025 11:51
Copy link

codecov bot commented Feb 21, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.08%. Comparing base (b2356a0) to head (0d8ed73).
Report is 8 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4700   +/-   ##
=======================================
  Coverage   86.08%   86.08%           
=======================================
  Files         290      290           
  Lines       34038    34072   +34     
=======================================
+ Hits        29300    29332   +32     
- Misses       4738     4740    +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@poljar poljar force-pushed the poljar/shared_history_flag branch from b0918eb to c4c6c6b Compare February 21, 2025 12:36
@poljar poljar marked this pull request as ready for review February 21, 2025 16:08
@poljar poljar requested review from a team as code owners February 21, 2025 16:08
@poljar poljar requested review from jmartinesp and richvdh and removed request for a team February 21, 2025 16:08
Copy link
Contributor

@jmartinesp jmartinesp left a comment

Choose a reason for hiding this comment

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

The code LGTM, but I think I'll leave reviewing the crypto behaviour to the crypto team 😅 .

Copy link
Member

@Hywan Hywan left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks for all the tests and the refactoring.

@poljar poljar force-pushed the poljar/shared_history_flag branch from e464490 to 49b2244 Compare February 25, 2025 15:20
@poljar poljar enabled auto-merge (rebase) February 25, 2025 15:21
…access in some places

In several places, we access almost all the fields of a struct to
create an `InboundGroupSession` from a pure data struct.

When new fields are added to these data structs, it's easy to
overlook updating the `InboundGroupSession` accordingly.

By using struct destructuring, we ensure that newly added fields  are
explicitly considered, making it harder to forget one of the newly added
fields.


This patch adds support for the `shared_history` flag from MSC3061 to
the `m.room_key` content, exported room keys, and backed-up room keys.

The flag is now persisted in our `InboundGroupSession`. Additionally,
when creating a new `InboundGroupSession`, we ensure the
`shared_history`  flag is set appropriately.

MSC3061: matrix-org/matrix-spec-proposals#3061
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants