Page MenuHomePhabricator

[keyserver] expose APIs to start PAKE registration on client side
ClosedPublic

Authored by varun on Nov 28 2022, 7:39 AM.
Tags
None
Referenced Files
F3386404: D5736.diff
Fri, Nov 29, 4:32 AM
Unknown Object (File)
Fri, Nov 22, 8:00 AM
Unknown Object (File)
Fri, Nov 22, 8:00 AM
Unknown Object (File)
Fri, Nov 22, 8:00 AM
Unknown Object (File)
Fri, Nov 22, 8:00 AM
Unknown Object (File)
Fri, Nov 22, 7:59 AM
Unknown Object (File)
Fri, Nov 22, 7:45 AM
Unknown Object (File)
Mon, Nov 18, 5:14 AM
Subscribers

Details

Summary

using Neon to surface the Rust opaque-ke library to Node.js

Test Plan

called the new APIs from a Node module and confirmed the results

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

varun requested review of this revision.Nov 28 2022, 7:53 AM

Looks more-or-less ok, but I don't fully understand what's happening here.

keyserver/addons/opaque-ke-node/src/lib.rs
27 ↗(On Diff #18879)

Why do we name it params?

41 ↗(On Diff #18879)

Why do we need that?

62–63 ↗(On Diff #18879)

Could you explain what this line does? What is the meaning of 0?

What's the best place to start reading to have more context on the PAKE stuff? I'd like to have a better overview of what's going on here

keyserver/addons/opaque-ke-node/src/lib.rs
62–63 ↗(On Diff #18879)

I think 0 means the first argument of a function: https://neon-bindings.com/docs/functions#accessing-arguments
So I suspect this gets the first argument, which should be of type ClientRegistrationStartResult

83–91 ↗(On Diff #18879)

Shouldn't we use the camelCase for JS function names?

tomek added 1 blocking reviewer(s): bartek.
tomek added inline comments.
keyserver/addons/opaque-ke-node/src/lib.rs
62–63 ↗(On Diff #18879)

Now it makes a lot more sense!

What's the best place to start reading to have more context on the PAKE stuff? I'd like to have a better overview of what's going on here

I think for these diffs this should give you enough context: https://github.com/novifinancial/opaque-ke/blob/v1.2.0/examples/simple_login.rs

This is an example of how OPAQUE is used for login (our use case)

keyserver/addons/opaque-ke-node/src/lib.rs
27 ↗(On Diff #18879)

to keep things consistent with naming on the Identity service side

41 ↗(On Diff #18879)

Finalize is executed on the main JavaScript thread and executed immediately before garbage collection. Values contained by a JsBox must implement Finalize. We don't have anything we need to clean up ourselves so we use the default empty implementation

62–63 ↗(On Diff #18879)

yeah, that's right. sorry, should've annotated this diff

83–91 ↗(On Diff #18879)

yeah, good point, i'll fix that

change js function names to camelCase

I feel like this would be a good candidate to include a unit test. Also demonstrates the ergonomics of "using" the hash method we just introduced.

This revision is now accepted and ready to land.Nov 30 2022, 11:49 AM
In D5736#172373, @jon wrote:

I feel like this would be a good candidate to include a unit test. Also demonstrates the ergonomics of "using" the hash method we just introduced.

https://linear.app/comm/issue/ENG-2383/add-unit-tests-for-hash-function-in-opaque-ke-node