[services/commtest] Tunnelbroker - Adding of helper function for the string signing with the private key
ClosedPublic
Actions

Authored by • max on Dec 19 2022, 6:30 AM.

Details

Reviewers

• jon
bartek

Commits

rCOMM49c7d86bc331: [services/commtest] Tunnelbroker - Adding of helper function for the string…

Summary

This diff introduces the function to make a signature for the string by the private key. The signature is needed to prove that the public key provided to the Tunnelbroker server belongs to the user during the session creation call.
This function is called during the session creating a call in D5931.

Test Plan

The commtest app is successfully built and the tests are successfully passed in the following D5931.

Diff Detail

Repository

rCOMM Comm

Branch

commtest-helpers-sign-string

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

• max created this revision.Dec 19 2022, 6:30 AM

• max held this revision as a draft.

Herald added a reviewer: • jon. · View Herald TranscriptDec 19 2022, 6:31 AM

Herald added subscribers: atul, tomek, ashoat. · View Herald Transcript

• max added a parent revision: D5837: [services/commtest] Tunnelbroker - Adding of helper function for the `SessionSignature` client request.Dec 19 2022, 6:32 AM

• max added a child revision: D5838: [services/commtest] Tunnelbroker - Adding of helper function for the `NewSession` client request.Dec 19 2022, 6:38 AM

Harbormaster completed remote builds in B14309: Diff 19549.Dec 19 2022, 6:41 AM

• max mentioned this in D5926: [services/commtest] Adding of openssl, base64, anyhow to dependencies.Dec 19 2022, 7:00 AM

• max published this revision for review.Dec 19 2022, 7:07 AM

• max edited the summary of this revision. (Show Details)

• max edited the test plan for this revision. (Show Details)

• max added a reviewer: bartek. • max added 1 blocking reviewer(s): • jon.

• jon requested changes to this revision.Dec 19 2022, 12:07 PM

• jon added inline comments.

services/commtest/src/tunnelbroker/new_session.rs
27–29 ↗	(On Diff #19549)	Not a fan of the `unwrap()`s, would really like to see with a `Result<String, Error>` being returned and we can just the try `?` operator if possible. If that's going to be a pain (e.g. multiple return types) then we should at least use `expect()`s which we can search for if something does fail in the future.

This revision now requires changes to proceed.Dec 19 2022, 12:07 PM

Fixing MessageDigest, using Anyhow::Result to pass the error to the caller.

• max planned changes to this revision.Dec 20 2022, 9:24 AM

Harbormaster failed remote builds in B14525: Diff 19782!Dec 20 2022, 9:31 AM

• max requested review of this revision.Dec 20 2022, 9:32 AM

• max marked an inline comment as done.

• max added inline comments.

services/commtest/src/tunnelbroker/new_session.rs
27–29 ↗	(On Diff #19549)	Not a fan of the `unwrap()`s, would really like to see with a `Result<String, Error>` being returned and we can just the try `?` operator if possible. If that's going to be a pain (e.g. multiple return types) then we should at least use `expect()`s which we can search for if something does fail in the future. I agree, it's better to use `Anyhow::Result` here and pass the errors to the caller. Thanks, @jon! Changed it.