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 encryptFileWithKeys #580

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from
Draft

Add encryptFileWithKeys #580

wants to merge 1 commit into from

Conversation

ben-kaufman
Copy link
Contributor

@ben-kaufman ben-kaufman commented Oct 23, 2023

Summary by CodeRabbit

  • New Feature: Added a function encryptFileWithKeys in the SubmissionFormPage. This feature enhances the security of file submissions by encrypting them using one or more public keys. The encrypted file and session key are returned, ensuring secure transmission and storage of sensitive user data.

@github-actions
Copy link

github-actions bot commented Oct 23, 2023

Image description CodeRabbit

Walkthrough

The introduced changeset adds a new function encryptFileWithKeys in the SubmissionFormPage. This function is designed to encrypt a file using one or more public keys, ensuring secure data transmission. It reads and verifies the public keys, generates a session key, and then encrypts the file.

Changes

File Summary
packages/web/src/pages/Submissions/SubmissionFormPage/encrypt.ts A new function encryptFileWithKeys has been added. This function encrypts a file using one or more public keys. It reads the public keys, verifies them, generates a session key, and then encrypts the file using the keys and session key. The encrypted file is returned along with the session key.

🐇💻

In the land of code, where the shadows lie,

One function to rule them all, one function to find them,

One function to bring them all, and in encryption bind them,

In the land of Submissions where the shadows lie. 🎉🔒


Uplevel your code reviews with CodeRabbit Pro

CodeRabbit Pro

If you like this project, please support us by purchasing the Pro version. The Pro version has advanced context, superior noise reduction and several proprietary improvements compared to the open source version. Moreover, CodeRabbit Pro is free for open source projects.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Image description CodeRabbit

Commits Files that changed from the base of the PR and between 4f24184 and 54ce3a8 commits.
Files selected (1)
  • packages/web/src/pages/Submissions/SubmissionFormPage/encrypt.ts (2)
Files ignored due to filter (2)
  • packages/web/package.json
  • yarn.lock
Review comments generated (2)
  • Review: 1
  • LGTM: 1

Tips

Chat with Image description CodeRabbit Bot (@coderabbitai)

  • Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
  • Invite the bot into a review comment chain by tagging @coderabbitai in a reply.

Code suggestions

  • The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
  • You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.

Pausing incremental reviews

  • Add @coderabbitai: ignore anywhere in the PR description to pause further reviews from the bot.

Comment on lines +43 to +82
export async function encryptFileWithKeys(publicKeyOrKeys: string | string[], fileToEncrypt: File) {
let encryptionKeys: MaybeArray<Key>;

if (Array.isArray(publicKeyOrKeys)) {
encryptionKeys = [];

const encryptionKeysList = await Promise.all(publicKeyOrKeys.map((key) => readKey({ armoredKey: key })));
for (let key of encryptionKeysList) {
try {
await key.verifyPrimaryKey();
encryptionKeys.push(key);
} catch {
continue;
}
}

if (encryptionKeys.length === 0) return undefined;
} else {
encryptionKeys = await readKey({ armoredKey: publicKeyOrKeys });

try {
await encryptionKeys.verifyPrimaryKey();
} catch (error) {
return undefined;
}
}

const sessionKey = await generateSessionKey({ encryptionKeys });

const message = await encrypt({
message: await createMessage({ binary: new Uint8Array(await fileToEncrypt.arrayBuffer() as ArrayBuffer) }),
encryptionKeys,
sessionKey,
});

const encryptedData = await readToEnd(message);

const encryptedFile = new File([encryptedData], fileToEncrypt.name, {type: fileToEncrypt.type})

return { encryptedFile, sessionKey };

Choose a reason for hiding this comment

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

Image description CodeRabbit

The function encryptFileWithKeys is quite large and does a lot of things. It would be beneficial to break it down into smaller, more manageable functions. This will improve readability, maintainability, and testability.

For example, the key reading and verification process could be extracted into its own function:

+ async function readAndVerifyKeys(publicKeyOrKeys: string | string[]): Promise<MaybeArray<Key> | undefined> {
+   let encryptionKeys: MaybeArray<Key>;
+
+   if (Array.isArray(publicKeyOrKeys)) {
+     encryptionKeys = [];
+
+     const encryptionKeysList = await Promise.all(publicKeyOrKeys.map((key) => readKey({ armoredKey: key })));
+     for (let key of encryptionKeysList) {
+       try {
+         await key.verifyPrimaryKey();
+         encryptionKeys.push(key);
+       } catch {
+         continue;
+       }
+     }
+
+     if (encryptionKeys.length === 0) return undefined;
+   } else {
+     encryptionKeys = await readKey({ armoredKey: publicKeyOrKeys });
+
+     try {
+       await encryptionKeys.verifyPrimaryKey();
+     } catch (error) {
+       return undefined;
+     }
+   }
+
+   return encryptionKeys;
+ }

- export async function encryptFileWithKeys(publicKeyOrKeys: string | string[], fileToEncrypt: File) {
-   let encryptionKeys: MaybeArray<Key>;
-
-   if (Array.isArray(publicKeyOrKeys)) {
-     encryptionKeys = [];
-
-     const encryptionKeysList = await Promise.all(publicKeyOrKeys.map((key) => readKey({ armoredKey: key })));
-     for (let key of encryptionKeysList) {
-       try {
-         await key.verifyPrimaryKey();
-         encryptionKeys.push(key);
-       } catch {
-         continue;
-       }
-     }
-
-     if (encryptionKeys.length === 0) return undefined;
-   } else {
-     encryptionKeys = await readKey({ armoredKey: publicKeyOrKeys });
-
-     try {
-       await encryptionKeys.verifyPrimaryKey();
-     } catch (error) {
-       return undefined;
-     }
-   }
+   const encryptionKeys = await readAndVerifyKeys(publicKeyOrKeys);
+   if (!encryptionKeys) return undefined;

    const sessionKey = await generateSessionKey({ encryptionKeys });

    const message = await encrypt({
      message: await createMessage({ binary: new Uint8Array(await fileToEncrypt.arrayBuffer() as ArrayBuffer) }),
      encryptionKeys,
      sessionKey,
    });

    const encryptedData = await readToEnd(message);

    const encryptedFile = new File([encryptedData], fileToEncrypt.name, {type: fileToEncrypt.type})

    return { encryptedFile, sessionKey };
}

Also, it's important to note that the function currently swallows any errors that occur during the key verification process. This could make debugging difficult in the future. It might be better to let these errors propagate up to the caller, or at least log them for future reference.

@cloudflare-workers-and-pages
Copy link

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 54ce3a8
Status: ✅  Deploy successful!
Preview URL: https://be62f623.dapp-a9y.pages.dev
Branch Preview URL: https://encrypt-files.dapp-a9y.pages.dev

View logs

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.

1 participant