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

Feature/bca/fix 5906 #5939

Merged
merged 5 commits into from
May 13, 2022
Merged

Feature/bca/fix 5906 #5939

merged 5 commits into from
May 13, 2022

Conversation

BillCarsonFr
Copy link
Member

@BillCarsonFr BillCarsonFr commented May 5, 2022

Fixes #5906
Fixes #1260
Fixes #3368

Type of change

  • Bugfix

Content

Change to keep 4S in sync with megolm backup:

If 4S is setup and you try to delete then create a new megolm backup, you will now be prompted to enter the 4S passphrase. A new random key will be generated for the backup and will be then saved in the 4S (replacing the old one), and will also be saved locally for gossiping.
Code Change => The SharedSecureStorageActivity has been modified to support 2 modes now, read and write. So all the code to request the password/key is the same. In the write mode you can now pass a secretName/Value to be stored.

Sign & Verify backup using MSK signature.

When the backup is created we now also add the MSK signature if possible (cross signing enabled and we have private part).
The signature is also checked in order to trust the backup .
Code Change => KeysBackupVersionTrustSignature is changed to a sealed class of DeviceSignatureor UserSignature
The backup settings screens have been modified to show the new signatures.
A new class CrossSigningOlm has been extracted (session scope) and holds the olmPkSigning objects than can be then injected in both CrossSigningService and KeyBackupService

Screenshots / GIFs

Before After
before after

Tests

See step to reproduce from #5906
Unit test has been updated to check signatures

Tested devices

  • Physical
  • Emulator
  • OS version(s):

Checklist

@BillCarsonFr BillCarsonFr requested review from a team, yostyle and ariskotsomitopoulos and removed request for a team May 5, 2022 07:35
@github-actions
Copy link

github-actions bot commented May 5, 2022

Unit Test Results

122 files  ±0  122 suites  ±0   1m 58s ⏱️ -10s
205 tests ±0  205 ✔️ ±0  0 💤 ±0  0 ±0 
690 runs  ±0  690 ✔️ ±0  0 💤 ±0  0 ±0 

Results for commit 7d5570f. ± Comparison against base commit 3f8ddbe.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@ariskotsomitopoulos ariskotsomitopoulos left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, some minor comments.

@michaelkaye
Copy link
Contributor

This may fix some of the crypto integration tests that are going to be ignored in #6025 - will wait for this to merge before deciding the final set of tests to ignore.

Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

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

LGTM (static review)

@BillCarsonFr BillCarsonFr merged commit d40f8b0 into develop May 13, 2022
@BillCarsonFr BillCarsonFr deleted the feature/bca/fix_5906 branch May 13, 2022 13:43
@github-actions
Copy link

Matrix SDK

Integration Tests Results:

  • [org.matrix.android.sdk.session]
    = passed=15 failures=5 errors=0 skipped=3
  • [org.matrix.android.sdk.account]
    = passed=3 failures=0 errors=0 skipped=2
  • [org.matrix.android.sdk.internal]
    = passed=90 failures=57 errors=0 skipped=13
  • [org.matrix.android.sdk.ordering]
    = passed=16 failures=0 errors=0 skipped=0
  • [org.matrix.android.sdk.PermalinkParserTest]
    = passed=2 failures=0 errors=0 skipped=0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants