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

switch default ciphersuite to 2 #4373

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
Open

switch default ciphersuite to 2 #4373

wants to merge 7 commits into from

Conversation

stefanwire
Copy link
Contributor

@stefanwire stefanwire commented Dec 13, 2024 •
edited
Loading

https://wearezeta.atlassian.net/browse/WPB-15004

Checklist

  • Add a new entry in an appropriate subdirectory of changelog.d
  • Read and follow the PR guidelines

@echoes-hq echoes-hq bot added the echoes/initiative: federation-wire-cloud Activate Federation with MLS on Wire Cloud label Dec 13, 2024
@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Dec 13, 2024
@stefanwire stefanwire force-pushed the WPB-15004-default-mls-ciphersuite branch 2 times, most recently from 999d2a3 to 94c88e0 Compare December 17, 2024 09:18
@stefanwire stefanwire force-pushed the WPB-15004-default-mls-ciphersuite branch from 94c88e0 to 4b38c2e Compare December 17, 2024 11:17
@stefanwire stefanwire marked this pull request as ready for review December 17, 2024 13:37
akshaymankar
akshaymankar reviewed Dec 17, 2024
View reviewed changes
Copy link
Member

@akshaymankar akshaymankar left a comment •
edited
Loading

Choose a reason for hiding this comment

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

We should add release notes about this change because if the operator doesn't notice this, the clients could get into a very confusing state.

integration/test/Test/MLS/KeyPackage.hs Outdated Show resolved Hide resolved
akshaymankar
akshaymankar reviewed Dec 18, 2024
View reviewed changes
libs/wire-api/src/Wire/API/Conversation/Protocol.hs Outdated Show resolved Hide resolved
libs/wire-api/src/Wire/API/MLS/CipherSuite.hs
Comment on lines -125 to +126
defCipherSuite = MLS_128_DHKEMX25519_AES128GCM_SHA256_Ed25519
defCipherSuite = MLS_128_DHKEMP256_AES128GCM_SHA256_P256
Copy link
Member

Choose a reason for hiding this comment

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

This is unused now. As discussed on the call, we should delete this and make a v8 version of the only endpoint which uses defaultCipherSuiteV7 which makes the ciphersuite mandatory.

Copy link
Member

Choose a reason for hiding this comment

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

Also feel free to do this in a separate PR/ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akshaymankar akshaymankar akshaymankar left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
echoes/initiative: federation-wire-cloud Activate Federation with MLS on Wire Cloud ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@stefanwire @akshaymankar @zebot