Page MenuHomePhabricator

[services] Backup - add relevant opaque apis for server
ClosedPublic

Authored by varun on Feb 10 2022, 8:52 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Dec 27, 6:46 AM
Unknown Object (File)
Fri, Dec 27, 6:46 AM
Unknown Object (File)
Fri, Dec 27, 6:45 AM
Unknown Object (File)
Fri, Dec 27, 6:45 AM
Unknown Object (File)
Fri, Dec 27, 6:45 AM
Unknown Object (File)
Tue, Dec 17, 7:41 AM
Unknown Object (File)
Tue, Dec 17, 7:41 AM
Unknown Object (File)
Tue, Dec 17, 7:41 AM

Details

Summary

this diff exposes the server apis from the opaque-ke library for account registration and login. wanted to get this out before it blocks the backup service.

Test Plan

unit tested, checked that the C++ headers were generated as expected

Diff Detail

Repository
rCOMM Comm
Branch
opaque-ke-server (branched from master)
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

varun requested review of this revision.Feb 10 2022, 8:56 PM

Created a linear task to restructure the cargo project and consolidate some of the unit testing code: https://linear.app/comm/issue/ENG-727/restructure-and-clean-up-the-opaque-ke-cxx-package

jim requested changes to this revision.Feb 16 2022, 2:23 PM
jim added inline comments.
native/cpp/CommonCpp/CryptoTools/opaque-ke-cxx/src/lib.rs
85 ↗(On Diff #9554)

I don't really like how you're reusing MessageState in all of these requests. Prefer to create new structs for each call, mirroring the underlying API.

Why not reuse the struct if the all end up just being a pair of byte vecs? To perserve the typedness of the API. If you have one type of like a ServerRegistrationStartResult there's really only one thing you can do with it -- the next step in the protocol. This is good API design because its hard to misuse.

233 ↗(On Diff #9554)

nit: I wouldn't have introduced password_file_bytes. Just file: s.serialize().

252 ↗(On Diff #9554)

Also consider inlining the message: s.message.serialize()? here

277 ↗(On Diff #9554)

Why does this function exist?

284 ↗(On Diff #9554)

I'd inline this also

This revision now requires changes to proceed.Feb 16 2022, 2:23 PM
varun marked 4 inline comments as done.

addressed feedback

This revision is now accepted and ready to land.Mar 8 2022, 8:49 AM