Page MenuHomePhabricator

[native] remove dependency on identity service package for gRPC client
ClosedPublic

Authored by varun on Sep 6 2022, 9:21 AM.
Tags
None
Referenced Files
F3119659: D5075.id16645.diff
Fri, Nov 1, 2:40 AM
F3119601: D5075.id16381.diff
Fri, Nov 1, 2:30 AM
Unknown Object (File)
Sat, Oct 26, 2:11 AM
Unknown Object (File)
Sat, Oct 19, 1:47 AM
Unknown Object (File)
Sat, Oct 19, 1:47 AM
Unknown Object (File)
Sat, Oct 19, 1:46 AM
Unknown Object (File)
Sat, Oct 19, 1:39 AM
Unknown Object (File)
Thu, Oct 10, 6:34 AM

Details

Summary

The location of the protos directory is still in flux. That means that it's difficult to take a dependency on the Identity service crate, even though it would be ideal to not repeat OPAQUE config code on client and server. This issue tracks restoring the dependency on the Identity crate: https://linear.app/comm/issue/ENG-1746/remove-opaquers-from-grpc-client

Added ashoat since there are new dependencies (these are all in the Identity service as well)

Test Plan

cargo build

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

native/cpp/CommonCpp/grpc/grpc_client/Cargo.toml
20–22

we don't use the latest version of sha2 or digest because the API has changed and I want to keep things consistent with the Identity service. these dependencies will all be removed anyway, once we can take a dependency on the Identity service crate again

varun requested review of this revision.Sep 6 2022, 9:31 AM

wrt Test Plan I can figure out where to run cargo build, but it's helpful for this sort of thing to give reviewers something they can copy and paste. Something like:

cd native/cpp/CommonCpp/grpc/grpc_client && cargo clean && cargo build
native/cpp/CommonCpp/grpc/grpc_client/src/opaque.rs
1–30

nit: was able to get an idea of what was going on from the Linear link, but would've been good to at least minimally mention what's going on here in the Summary section

In D5075#148029, @atul wrote:

wrt Test Plan I can figure out where to run cargo build, but it's helpful for this sort of thing to give reviewers something they can copy and paste. Something like:

cd native/cpp/CommonCpp/grpc/grpc_client && cargo clean && cargo build

Fair enough

native/cpp/CommonCpp/grpc/grpc_client/src/opaque.rs
1–30

additional context: this file currently exists in the identity service cargo project. we don't want to depend directly on that crate until the .proto file location issue is resolved. once resolved, we can remove this file and the new dependencies in Cargo.toml and just use the identity crate directly

tomek added inline comments.
native/cpp/CommonCpp/grpc/grpc_client/src/opaque.rs
26

Is it a good idea to use hardcoded salt?

This revision is now accepted and ready to land.Sep 12 2022, 5:36 AM
native/cpp/CommonCpp/grpc/grpc_client/src/opaque.rs
26

opaque-ke already takes care of the salting but argon2 requires a salt of minimum 8 bytes so we just zero out an 8-byte array