Page MenuHomePhabricator

[lib] expose opaque-ke library to Node
ClosedPublic

Authored by varun on Nov 18 2022, 11:36 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Mar 25, 3:28 PM
Unknown Object (File)
Thu, Mar 13, 6:47 PM
Unknown Object (File)
Sun, Mar 9, 6:39 AM
Unknown Object (File)
Mar 3 2025, 3:54 AM
Unknown Object (File)
Feb 27 2025, 3:21 PM
Unknown Object (File)
Feb 27 2025, 2:59 PM
Unknown Object (File)
Feb 27 2025, 2:59 PM
Unknown Object (File)
Feb 27 2025, 11:40 AM
Subscribers

Details

Summary

Neon provides bindings for writing Node.js addons in Rust. We will call the opaque-ke registration/login functions through this addon.

Adding @ashoat because of the new dependency. More context here: https://linear.app/comm/issue/ENG-2289/use-opaque-ke-to-handle-pake-registrationlogin-on-keyserver-side

Test Plan

run cargo build in the opaque-ke-cxx directory, the library compiles

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ashoat requested changes to this revision.Nov 18 2022, 1:05 PM

Defer to you on whether this is the right approach. We're fast approaching the deadline for this work so want to avoid adding blocks

Two big things need to be changed here:

  1. That our formatting config is being applied here
  2. We should almost certainly move this out of lib, which is a JS workspace that is shared across all of the rest of our JS workspaces. Is this meant to be accessed from keyserver? If yes, why not move it there?
lib/opaque-ke-cxx/src/lib.rs
4 ↗(On Diff #18597)

We always want two-space tabs, can you make sure we're formatting correctly here? It seems like our formatter is not configured to run here or something

This revision now requires changes to proceed.Nov 18 2022, 1:05 PM

fixed formatting and cargo project location

also renamed the package to opaque-ke-node since we're not using cxx

keyserver/addons/opaque-ke-node/Cargo.toml
6 ↗(On Diff #18883)

Doesn't Neon support 2021? I guess this value comes from the create-neon template

keyserver/addons/opaque-ke-node/package.json
8–10 ↗(On Diff #18883)

I know it's generated but shouldn't we use yarn here, for consistency?

varun added inline comments.
keyserver/addons/opaque-ke-node/Cargo.toml
6 ↗(On Diff #18883)

good catch, i'll change this

keyserver/addons/opaque-ke-node/package.json
8–10 ↗(On Diff #18883)

yeah good point

How are we making sure that the linter is running on this code?

switch from npm to yarn, fix rust version in Cargo.toml

I think it would be better to use Yarn workspaces here if possible:

  1. Add keyserver/addons/opaque-ke-node to workspaces in the root package.json
  2. Run yarn cleaninstall
  3. Delete keyserver/addons/opaque-ke-node/yarn.lock and commit the updated root yarn.lock instead

How are we making sure that the linter is running on this code?

the pre-commit hook needs to be modified to cover this cargo project, i've assigned this issue to myself. will add all the rust projects we have now

I think it would be better to use Yarn workspaces here if possible:

  1. Add keyserver/addons/opaque-ke-node to workspaces in the root package.json
  2. Run yarn cleaninstall
  3. Delete keyserver/addons/opaque-ke-node/yarn.lock and commit the updated root yarn.lock instead

ok, let me give this a shot

ashoat requested changes to this revision.Nov 28 2022, 3:22 PM
This revision now requires changes to proceed.Nov 28 2022, 3:22 PM
bartek added inline comments.
keyserver/addons/opaque-ke-node/README.md
1 ↗(On Diff #18996)

I'm wondering if we should:

  • keep this file unchanged
  • keep this file and delete some stuff that quickly goes out of sync
  • delete it
This revision is now accepted and ready to land.Nov 30 2022, 7:01 AM
keyserver/addons/opaque-ke-node/README.md
1 ↗(On Diff #18996)

what do you think might go out of sync here? open to just deleting the readme since it isn't that useful

This revision was automatically updated to reflect the committed changes.