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
Unknown Object (File)
Mon, Aug 26, 11:19 AM
Unknown Object (File)
Sun, Aug 25, 3:44 PM
Unknown Object (File)
Sun, Aug 25, 2:36 PM
Unknown Object (File)
Sat, Aug 17, 8:16 PM
Unknown Object (File)
Fri, Aug 9, 3:44 PM
Unknown Object (File)
Jul 25 2024, 10:23 PM
Unknown Object (File)
Jul 25 2024, 4:07 PM
Unknown Object (File)
Jul 25 2024, 3:10 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
Branch
qr_followups
Lint
No Lint Coverage
Unit
No Test Coverage

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

this looks a little cleaner to me

27

do we need a try-catch block here?

33

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

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

The colors depend on theme (light or dark mode)

Address @varun's feedback

native/qr-code/qr-code-screen.react.js
27

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

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