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

[PB-2666] use hybrid encryption in index.ts #1445

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

TamaraFinogina
Copy link
Contributor

Description

Switch encryption to a hybrid version with Kyber and ensure that it works as expected

Related Issues

Relates to PB-2666

Related Pull Requests

Checklist

  • Changes have been tested locally.
  • Unit tests have been written or updated as necessary.
  • The code adheres to the repository's coding standards.
  • Relevant documentation has been added or updated.
  • No new warnings or errors have been introduced.
  • SonarCloud issues have been reviewed and addressed.
  • QA Passed

How Has This Been Tested?

Unit tests:

  1. The new hybrid function works as expected with Kyber key
  2. The new hybrid function works as before without Kyber key
  3. Works as expected for old users (without field keys)

Additional Notes

Copy link

vercel bot commented Jan 29, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
drive-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 31, 2025 3:52pm

Copy link
Member

@sg-gs sg-gs left a comment

Choose a reason for hiding this comment

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

Running the QA on this one.

Checklist
Private sharing:

  • Sharing content from the new version to the old one
    • This cannot be done as the new uses a PQC approach the old one cannot understand
  • Sharing content from the new version to the new version
    • User A shares content with user B, user B can see the content.
  • Sharing content from the old version to the new version
    • User C (old version, without Kyber) shares content with user B (using the new version)
    • User C (in the old version) can open shares in the new version that were generated using only the old version

Copy link

cloudflare-workers-and-pages bot commented Jan 30, 2025

Deploying drive-web with  Cloudflare Pages  Cloudflare Pages

Latest commit: dbf459c
Status:⚡️  Build in progress...

View logs

});

const encryptedMnemonicInBase64 = btoa(encryptedMnemonic as string);
const encryptionAlgorithm = publicKyberKey !== '' ? 'hybrid' : 'ed25519';
Copy link
Contributor

Choose a reason for hiding this comment

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

Add this hardcoded string to constants

Comment on lines 120 to +121
publicKey = publicKeyResponse.publicKey;
if (publicKeyResponse.keys) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I understand that the backend keeps sending the public key in the publicKey field for pre-hybrid users and in keys.ecc.publicKey for new hybrid users, is that right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it sends it for everyone now, but for pre-hybrid users kyber key is null (unless they login recently)

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

Successfully merging this pull request may close these issues.

4 participants