[web] "Unpickle" primary OLM account to sign keys and get `SignedIdentityKeysBlob`
ClosedPublic
Actions

Authored by atul on Feb 24 2023, 2:21 AM.

Details

Reviewers

ashoat
tomek
marcin

Commits

rCOMM6fbcb6ddc7d6: [web] "Unpickle" primary OLM account to sign keys and get…

Summary

We unpickle the primary OLM account in order to sign a "concatenation" of the primary and notification keys.

We then bundle up the payload and signature into SignedIdentityKeysBlob which is what will get included in the login request to the keyserver.

Depends on D6883

Test Plan

Logged identity_keys() of "unpickled" account and it matched the keys from the instantiation of the primary account:

signedIdentityKeysBlob is created as expected:

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

atul created this revision.Feb 24 2023, 2:21 AM

atul published this revision for review.Feb 24 2023, 2:23 AM

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

Harbormaster completed remote builds in B16869: Diff 23048.Feb 24 2023, 2:36 AM

Accepting to unblock, but a little confused since I don't see the keyserver side

lib/types/account-types.js
107 ↗	(On Diff #23048)	Why aren't we removing this?
lib/types/crypto-types.js
21 ↗	(On Diff #23048)	We could consider passing the JSON directly instead of the stringified JSON. We'll want to validate that the `payload` matches a specific format anyways, but I guess we can use an `inputValidator` at a later step (after the JSON is parsed)
web/account/traditional-login-form.react.js
113 ↗	(On Diff #23048)	Where is `keyserver` updated to support this?

This revision is now accepted and ready to land.Feb 24 2023, 8:21 AM

atul added inline comments.Feb 24 2023, 1:41 PM

lib/types/account-types.js
107 ↗	(On Diff #23048)	It will be, just sequenced it in a later diff. For the moment there are other places (`native` login) that are still passing `primaryIdentityPublicKey` in their requests. Once `primaryIdentityPublicKey` has been replaced everywhere by `signedIdentityKeysBlob`, I'll rip the old stuff out.
web/account/traditional-login-form.react.js
113 ↗	(On Diff #23048)	It hasn't been yet. It will be before we actually include `signedIdentityKeysBlob` in the `logIn` request.

atul added inline comments.Feb 24 2023, 1:48 PM

lib/types/crypto-types.js
21 ↗	(On Diff #23048)	We want to stringify the JSON before we sign the message on the client. `JSON.stringify(...)` isn't "deterministic" in that the order of keys depends on insertion order, so I didn't want to rely on `JSON.stringify(payload)` on the client matching `JSON.stringify(payload)` on the `keyserver`. There are libraries (eg https://github.com/ljharb/json-stable-stringify) that offer a "stable" stringify, but I thought it was a simpler solution to just stringify and sign on the client and then pass the exact signed payload to the `keyserver`.

Closed by commit rCOMM6fbcb6ddc7d6: [web] "Unpickle" primary OLM account to sign keys and get… (authored by atul). · Explain WhyFeb 24 2023, 2:39 PM

This revision was automatically updated to reflect the committed changes.

atul added a commit: rCOMM6fbcb6ddc7d6: [web] "Unpickle" primary OLM account to sign keys and get….

ashoat added inline comments.Feb 26 2023, 4:55 AM

lib/types/crypto-types.js
21 ↗	(On Diff #23048)	That makes sense, thanks for explaining. We should just make sure we pass the object through an input validator after we `JSON.parse` it on the keyserver side