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

Remove support for asymmetric 4S encryption #1373

Merged
merged 6 commits into from
May 14, 2020
Merged

Conversation

bwindels
Copy link
Contributor

@bwindels bwindels commented May 12, 2020

Removes support for Curve25519 encryption of SSSS secrets. Also removes signing of the SSSS key by the cross-signing master key, as that is not needed for AES encryption.

This is a breaking change, as it removes the secretStorageKeyNeedsUpgrade method on MatrixClient.

Part of element-hq/element-web#13581
Required by matrix-org/matrix-react-sdk#4581

@bwindels bwindels requested review from uhoreg and a team May 12, 2020 13:13
Copy link
Member

@uhoreg uhoreg left a comment

Choose a reason for hiding this comment

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

Mostly looks fine

src/crypto/SecretStorage.js Outdated Show resolved Hide resolved
@@ -1310,7 +1310,6 @@ wrapCryptoFuncs(MatrixClient, [
"bootstrapSecretStorage",
"addSecretStorageKey",
"hasSecretStorageKey",
"secretStorageKeyNeedsUpgrade",
Copy link
Member

Choose a reason for hiding this comment

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

Maybe should keep this in order to not break backwards compat, but just have it return false.

Copy link
Contributor Author

@bwindels bwindels May 13, 2020

Choose a reason for hiding this comment

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

I'd rather remove it and mark this as a breaking change tbh. There will likely be more breaking changes coming anyway when disentangling 4S and cross-signing. Will add this in the PR description 👍

Copy link
Collaborator

Choose a reason for hiding this comment

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

About breaking changes, we've been marking 4S / cross-signing APIs in client.js with the text "The Secure Secret Storage API is currently UNSTABLE and may change without notice.", so I think that may be enough wiggle room to make breaking changes like this without a major version bump.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Filed element-hq/element-web#13651 to track properly stabilising these APIs.

@bwindels
Copy link
Contributor Author

Sorry, had to force-push because I pushed a WIP commit for the next phase by accident. The only thing I changed was bringing back the algorithm parameter.

@bwindels bwindels requested a review from uhoreg May 13, 2020 08:43
@bwindels bwindels merged commit da2ef38 into develop May 14, 2020
@t3chguy t3chguy deleted the bwindels/remove-asym-4s branch May 10, 2022 14:31
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.

3 participants