Page MenuHomePhabricator
Differential D11073

[identity] Add types for Ed25519-signed nonce challenge
ClosedPublic
Actions

Authored by bartek on Feb 14 2024, 4:43 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Jun 14, 2:11 PM
Unknown Object (File)
Thu, Jun 13, 9:08 PM
Unknown Object (File)
Fri, May 31, 6:24 AM
Unknown Object (File)
Thu, May 23, 5:42 AM
Unknown Object (File)
Mon, May 20, 1:30 AM
Unknown Object (File)
Mon, May 20, 1:30 AM
Unknown Object (File)
Mon, May 20, 1:30 AM
Unknown Object (File)
Mon, May 20, 1:30 AM
View All 20 Files
Subscribers
ashoat
tomek

Details

Reviewers
varun
michal
Commits
rCOMM56be0a1c5e98: [identity] Add types for Ed25519-signed nonce challenge
Summary

For the "Existing Device Login" (5.3.6), we want to introduce the challenge-response approach to protect from replay attacks.
We agreed to re-use nonces from the GenerateNonce RPC which is currently used for wallet login/registration.

This diff introduces types for verifying if the nonce is properly signed by the client using Olm account (ed25519).
The code is similiar to what we do for verifying keyserver messages for reserved usernames.

Test Plan

Added unit test, the data comes from D11037 test plan

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bartek created this revision.Feb 14 2024, 4:43 AM
bartek held this revision as a draft.
Herald added subscribers: tomek, ashoat. · View Herald TranscriptFeb 14 2024, 4:43 AM
bartek added a child revision: D10862: [identity] Implement RPC for secondary device registration.Feb 14 2024, 4:44 AM
bartek published this revision for review.Feb 14 2024, 4:56 AM
Harbormaster completed remote builds in B26824: Diff 37067.Feb 14 2024, 4:58 AM
varun requested changes to this revision.Feb 15 2024, 9:02 PM
varun added inline comments.
services/identity/src/grpc_utils.rs
29 ↗(On Diff #37067)

the whitepaper says the challenge message should contain a nonce and timestamp. wondering why there's no timestamp in this struct

47–63 ↗(On Diff #37067)

i wonder if we should try to dedup this given how similar it is to validate_and_decode_message (can defer this if you'd prefer)

This revision now requires changes to proceed.Feb 15 2024, 9:02 PM
bartek requested review of this revision.Feb 15 2024, 9:53 PM
bartek added inline comments.
services/identity/src/grpc_utils.rs
29 ↗(On Diff #37067)

We do timestamp verification server-side, I'm reusing the wallet login nonces mechanics here - the timestamp is stored in the identity-nonces table and verified if not expired

varun accepted this revision.Feb 15 2024, 9:57 PM
varun added inline comments.
services/identity/src/grpc_utils.rs
47–63 ↗(On Diff #37067)

lmk what you think about this

This revision is now accepted and ready to land.Feb 15 2024, 9:57 PM
bartek added inline comments.Feb 15 2024, 10:08 PM
services/identity/src/grpc_utils.rs
47–63 ↗(On Diff #37067)

Yeah I'll try to dedupe this

bartek added inline comments.Feb 16 2024, 6:23 AM
services/identity/src/grpc_utils.rs
47–63 ↗(On Diff #37067)

Turns out that refactor of validate_and_decode_message is too significant for this diff, I'll put up a follow-up

Closed by commit rCOMM56be0a1c5e98: [identity] Add types for Ed25519-signed nonce challenge (authored by bartek). · Explain WhyFeb 16 2024, 6:26 AM
This revision was automatically updated to reflect the committed changes.
bartek added a commit: rCOMM56be0a1c5e98: [identity] Add types for Ed25519-signed nonce challenge.

Revision Contents
Changeset List

PathSize
services/
identity/
src/
83 lines

Diff 37304

View Options

services/identity/src/grpc_utils.rs

Loading...