Page MenuHomePhabricator

[lib/web/native] Generate and encode an ephemeral AES-256 key in the QR code
ClosedPublic

Authored by rohan on Sep 6 2023, 8:51 AM.
Tags
None
Referenced Files
F3349874: D9093.id30798.diff
Fri, Nov 22, 7:33 PM
Unknown Object (File)
Sun, Nov 10, 5:40 PM
Unknown Object (File)
Sat, Nov 9, 8:14 AM
Unknown Object (File)
Mon, Nov 4, 2:51 AM
Unknown Object (File)
Sun, Nov 3, 4:58 AM
Unknown Object (File)
Sun, Nov 3, 4:45 AM
Unknown Object (File)
Thu, Oct 31, 2:39 PM
Unknown Object (File)
Thu, Oct 31, 2:39 PM

Details

Summary

One of the pieces of information we want to encode in the QR code for secondary devices attempting to log-in is an ephemeral AES-256 key. We already have utils written for generating the key on both web and native by @bartek I believe, so I just made the calls and encoded the key into the QR code.

The device ID generation/encoding process will be the next step in the diff, so for now it's just going to be the same placeholder string as before.

Addresses ENG-4797.

Test Plan

Tested this two ways:

  1. Scanned the barcode with the native app and confirmed that it still parses correctly
  1. Extracted the URL I was encoding into the QR code and ran the following code on it:
const url = 
'comm://qr-code/%7B%22aes256%22%3A%7B%220%22%3A176%2C%221%22%3A145%2C%222%22%3A3%2C%223%22%3A167%2C%224%22%3A51%2C%225%22%3A23%2C%226%22%3A2%2C%227%22%3A22%2C%228%22%3A235%2C%229%22%3A123%2C%2210%22%3A153%2C%2211%22%3A42%2C%2212%22%3A26%2C%2213%22%3A114%2C%2214%22%3A149%2C%2215%22%3A68%2C%2216%22%3A245%2C%2217%22%3A175%2C%2218%22%3A76%2C%2219%22%3A248%2C%2220%22%3A142%2C%2221%22%3A10%2C%2222%22%3A94%2C%2223%22%3A220%2C%2224%22%3A239%2C%2225%22%3A90%2C%2226%22%3A162%2C%2227%22%3A160%2C%2228%22%3A124%2C%2229%22%3A183%2C%2230%22%3A190%2C%2231%22%3A249%7D%2C%22ed25519%22%3A%22device_ed25519_key%22%7D';

// This is the regex we use to parse the QR code link in `links.js`
const qrCodeKeysRegex = /qr-code\/(\S+)$/;
const qrCodeKeysMatch = qrCodeKeysRegex.exec(url)[1];

// Decode and parse the keys from the URL
const keys = JSON.parse(decodeURIComponent(qrCodeKeysMatch));

// Log the keys to verify
console.log(keys);

The result is as expected

{
  aes256: {
    '0': 176,
    '1': 145,
    '2': 3,
    '3': 167,
    '4': 51,
    '5': 23,
    '6': 2,
    '7': 22,
    '8': 235,
    '9': 123,
    '10': 153,
    '11': 42,
    '12': 26,
    '13': 114,
    '14': 149,
    '15': 68,
    '16': 245,
    '17': 175,
    '18': 76,
    '19': 248,
    '20': 142,
    '21': 10,
    '22': 94,
    '23': 220,
    '24': 239,
    '25': 90,
    '26': 162,
    '27': 160,
    '28': 124,
    '29': 183,
    '30': 190,
    '31': 249
  },
  ed25519: 'device_ed25519_key'
}

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

rohan requested review of this revision.Sep 6 2023, 9:09 AM
varun requested changes to this revision.Sep 8 2023, 10:19 AM

looks pretty good, but have some q's/suggestions

native/qr-code/qr-code-screen.react.js
25–31 ↗(On Diff #30798)

this looks a little cleaner to me

27 ↗(On Diff #30798)

do we need a try-catch block here?

33 ↗(On Diff #30798)

still new to react hooks. could you explain why we pass unboundStyles here? the styles don't seem to depend on any props or state in this component

40 ↗(On Diff #30798)

what happens if qrCodeValue remains undefined?

This revision now requires changes to proceed.Sep 8 2023, 10:19 AM
native/qr-code/qr-code-screen.react.js
33 ↗(On Diff #30798)

The colors depend on theme (light or dark mode)

Address @varun's feedback

native/qr-code/qr-code-screen.react.js
27 ↗(On Diff #30798)

I'm not sure, it looks like in some places we use a try-catch, but in others we don't. It could be good to have generally though

40 ↗(On Diff #30798)

A QR code appears on the screen, but no data is encoded into it

This revision is now accepted and ready to land.Sep 12 2023, 9:21 AM
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.

This popped up while discussing QR code format with @anunay. @rohan, wondering why we encoded the AES key in this weird way, rather than just as a string (with optional encoding if necessary?)

This popped up while discussing QR code format with @anunay. @rohan, wondering why we encoded the AES key in this weird way, rather than just as a string (with optional encoding if necessary?)

I agree that it was a weird way. I also realized it was more 'space efficient' in the QR codes to not encode an entire array. I ended up addressing this in D9202 across all web/native/keyserver