Page MenuHomePhabricator

[web/native] Retrieve and encode the ed25519 key in the QR code
ClosedPublic

Authored by rohan on Sep 7 2023, 7:18 AM.
Tags
None
Referenced Files
F3134227: D9096.id30868.diff
Sat, Nov 2, 2:28 PM
Unknown Object (File)
Thu, Oct 31, 2:39 PM
Unknown Object (File)
Thu, Oct 31, 2:39 PM
Unknown Object (File)
Thu, Oct 31, 2:39 PM
Unknown Object (File)
Thu, Oct 31, 2:39 PM
Unknown Object (File)
Thu, Oct 31, 2:39 PM
Unknown Object (File)
Thu, Oct 31, 2:39 PM
Unknown Object (File)
Thu, Oct 31, 2:36 PM

Details

Summary

The second key we need to encode into the QR code during secondary device login is the ed25519 key, identified as the device id.

After speaking with @marcin and @kamil (and office hours yesterday), accessing this via the commCoreModule on native is done by making a commCoreModule.getUserPublicKey() call, returning both a ed25519 and curve25519 keypair - the first is the one we need.

On web, it's a little different. @kamil pointed out the way we currently do it is by accessing it through Redux, and it seems like the only way to access the keys for now. I won't need to worry about running the key generation protocol since key generation occurs when the login form is rendered. Since the QR code login screen is accessed by clicking a button on the login form, we know the keys should exist.

Depends on D9093

Addresses ENG-4800

Test Plan

On native and web:

  • Opened the QR code login screen and console.log(url)
  • Copied the string
  • Used the same RegEx in links.js to parse out the keys
  • Confirmed that the ed25519 key was encoded correctly
const url = 
'comm://qr-code/%7B%22aes256%22%3A%7B%220%22%3A213%2C%221%22%3A43%2C%222%22%3A225%2C%223%22%3A116%2C%224%22%3A29%2C%225%22%3A212%2C%226%22%3A144%2C%227%22%3A175%2C%228%22%3A227%2C%229%22%3A166%2C%2210%22%3A225%2C%2211%22%3A42%2C%2212%22%3A77%2C%2213%22%3A29%2C%2214%22%3A189%2C%2215%22%3A156%2C%2216%22%3A93%2C%2217%22%3A204%2C%2218%22%3A38%2C%2219%22%3A3%2C%2220%22%3A246%2C%2221%22%3A18%2C%2222%22%3A139%2C%2223%22%3A50%2C%2224%22%3A220%2C%2225%22%3A238%2C%2226%22%3A64%2C%2227%22%3A24%2C%2228%22%3A149%2C%2229%22%3A191%2C%2230%22%3A7%2C%2231%22%3A229%7D%2C%22ed25519%22%3A%228dKa0Z7j4P4cs1FRGj%2BHvXDzwiaylxqa90PgLM0Muzg%22%7D';

const qrCodeKeysRegex = /qr-code\/(\S+)$/;
const qrCodeKeysMatch = qrCodeKeysRegex.exec(url)[1];

const keys = JSON.parse(decodeURIComponent(qrCodeKeysMatch));

console.log(keys);

Output:

{
  aes256: {
    '0': 213,
    '1': 43,
    '2': 225,
    '3': 116,
    '4': 29,
    '5': 212,
    '6': 144,
    '7': 175,
    '8': 227,
    '9': 166,
    '10': 225,
    '11': 42,
    '12': 77,
    '13': 29,
    '14': 189,
    '15': 156,
    '16': 93,
    '17': 204,
    '18': 38,
    '19': 3,
    '20': 246,
    '21': 18,
    '22': 139,
    '23': 50,
    '24': 220,
    '25': 238,
    '26': 64,
    '27': 24,
    '28': 149,
    '29': 191,
    '30': 7,
    '31': 229
  },
  ed25519: '8dKa0Z7j4P4cs1FRGj+HvXDzwiaylxqa90PgLM0Muzg'
}

Two additional testing plans:

  • I deleted Comm on the simulator then reinstalled and opened it up to confirm that the keys would be generated before the QR code screen is shown
  • Opened Comm on a separate browser to confirm that the key generation occurred before reaching the QR code page on web

Diff Detail

Repository
rCOMM Comm
Branch
qr_followups
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

rohan requested review of this revision.Sep 7 2023, 7:37 AM

LGTM, but letting other reviewers to take a look

native/qr-code/qr-code-screen.react.js
28–31 ↗(On Diff #30833)
web/account/qr-code-login.react.js
20–22 ↗(On Diff #30833)

probably in the future, this check will not be needed, because will need this key to log into identity anyway, so at this point should be present, but for now seems okay

marcin added inline comments.
native/qr-code/qr-code-screen.react.js
28–31 ↗(On Diff #30833)

I agree. We should try to reuse code implemented by others.

This revision is now accepted and ready to land.Sep 8 2023, 2:18 AM
native/qr-code/qr-code-screen.react.js
28–31 ↗(On Diff #30833)

Thanks for mentioning getContentSigningKey, I didn't realize we already had a method!

Use getContentSigningKey

This revision was landed with ongoing or failed builds.Sep 13 2023, 8:41 AM
This revision was automatically updated to reflect the committed changes.