From 45963adeb41671a893829c1902b1d003716b8fc3 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 19 Feb 2020 17:02:14 -0700 Subject: [PATCH 1/3] Use binary packing for verification QR codes Fixes https://github.com/vector-im/riot-web/issues/12257 Fixes https://github.com/vector-im/riot-web/issues/12375 We do not remove the existing QR code library in this commit because it is still used elsewhere (like the share dialog). This should be as accurate as possible to what [MSC1543](https://github.com/matrix-org/matrix-doc/pull/1544) asks for. --- package.json | 1 + .../elements/crypto/VerificationQRCode.js | 182 +++++++++++------- yarn.lock | 55 +++++- 3 files changed, 164 insertions(+), 74 deletions(-) diff --git a/package.json b/package.json index a68dcfddb4..32e78f835f 100644 --- a/package.json +++ b/package.json @@ -85,6 +85,7 @@ "pako": "^1.0.5", "png-chunks-extract": "^1.0.0", "prop-types": "^15.5.8", + "qrcode": "^1.4.4", "qrcode-react": "^0.1.16", "qs": "^6.6.0", "react": "^16.9.0", diff --git a/src/components/views/elements/crypto/VerificationQRCode.js b/src/components/views/elements/crypto/VerificationQRCode.js index 1c0fdcbf44..09408fe4dd 100644 --- a/src/components/views/elements/crypto/VerificationQRCode.js +++ b/src/components/views/elements/crypto/VerificationQRCode.js @@ -17,89 +17,87 @@ limitations under the License. import React from "react"; import PropTypes from "prop-types"; import {replaceableComponent} from "../../../../utils/replaceableComponent"; -import * as qs from "qs"; -import QRCode from "qrcode-react"; import {MatrixClientPeg} from "../../../../MatrixClientPeg"; import {VerificationRequest} from "matrix-js-sdk/src/crypto/verification/request/VerificationRequest"; import {ToDeviceChannel} from "matrix-js-sdk/src/crypto/verification/request/ToDeviceChannel"; +import {decodeBase64} from "matrix-js-sdk/src/crypto/olmlib"; +import Spinner from "../Spinner"; +import * as QRCode from "qrcode"; + +const CODE_VERSION = 0x02; +const BINARY_PREFIX = "MATRIX"; // ASCII, used to prefix the +const MODE_VERIFY_OTHER_USER = 0x00; +const MODE_VERIFY_SELF_TRUSTED = 0x01; // We trust the master key +const MODE_VERIFY_SELF_UNTRUSTED = 0x02; // We do not trust the master key @replaceableComponent("views.elements.crypto.VerificationQRCode") export default class VerificationQRCode extends React.PureComponent { static propTypes = { - // Common for all kinds of QR codes - keys: PropTypes.array.isRequired, // array of [Key ID, Base64 Key] pairs - action: PropTypes.string.isRequired, - keyholderUserId: PropTypes.string.isRequired, - - // User verification use case only - secret: PropTypes.string, - otherUserKey: PropTypes.string, // Base64 key being verified - otherUserDeviceKey: PropTypes.string, // Base64 key of the other user's device (or what we think it is; optional) - requestEventId: PropTypes.string, // for DM verification only - }; - - static defaultProps = { - action: "verify", + prefix: PropTypes.string.isRequired, + version: PropTypes.number.isRequired, + mode: PropTypes.number.isRequired, + transactionId: PropTypes.string.isRequired, // or requestEventId + firstKeyB64: PropTypes.string.isRequired, + secondKeyB64: PropTypes.string.isRequired, + secretB64: PropTypes.string.isRequired, }; static async getPropsForRequest(verificationRequest: VerificationRequest) { const cli = MatrixClientPeg.get(); const myUserId = cli.getUserId(); const otherUserId = verificationRequest.otherUserId; - const myDeviceId = cli.getDeviceId(); - const otherDevice = verificationRequest.targetDevice; - const otherDeviceId = otherDevice ? otherDevice.deviceId : null; - const qrProps = { - secret: verificationRequest.encodedSharedSecret, - keyholderUserId: myUserId, - action: "verify", - keys: [], // array of pairs: keyId, base64Key - otherUserKey: "", // base64key - otherUserDeviceKey: "", // base64key - requestEventId: "", // we figure this out in a moment - }; + let mode = MODE_VERIFY_OTHER_USER; + if (myUserId === otherUserId) { + // Mode changes depending on whether or not we trust the master cross signing key + const myTrust = cli.checkUserTrust(myUserId); + if (myTrust.isCrossSigningVerified()) { + mode = MODE_VERIFY_SELF_TRUSTED; + } else { + mode = MODE_VERIFY_SELF_UNTRUSTED; + } + } const requestEvent = verificationRequest.requestEvent; - qrProps.requestEventId = requestEvent.getId() + let transactionId = requestEvent.getId() ? requestEvent.getId() : ToDeviceChannel.getTransactionId(requestEvent); - // Populate the keys we need depending on which direction and users are involved in the verification. - if (myUserId === otherUserId) { - if (!otherDeviceId) { - // Existing scanning New session's QR code - qrProps.otherUserDeviceKey = null; - } else { - // New scanning Existing session's QR code - const myDevices = (await cli.getStoredDevicesForUser(myUserId)) || []; - const device = myDevices.find(d => d.deviceId === otherDeviceId); - if (device) qrProps.otherUserDeviceKey = device.getFingerprint(); - } + const qrProps = { + prefix: BINARY_PREFIX, + version: CODE_VERSION, + mode, + transactionId, + firstKeyB64: '', // worked out shortly + secondKeyB64: '', // worked out shortly + secretB64: verificationRequest.encodedSharedSecret, + }; - // Either direction shares these next few props + const myCrossSigningInfo = cli.getStoredCrossSigningForUser(myUserId); + const myDevices = (await cli.getStoredDevicesForUser(myUserId)) || []; - const xsignInfo = cli.getStoredCrossSigningForUser(myUserId); - qrProps.otherUserKey = xsignInfo.getId("master"); + if (mode === MODE_VERIFY_OTHER_USER) { + // First key is our master cross signing key + qrProps.firstKeyB64 = myCrossSigningInfo.getId("master"); - qrProps.keys = [ - [myDeviceId, cli.getDeviceEd25519Key()], - [xsignInfo.getId("master"), xsignInfo.getId("master")], - ]; - } else { - // Doesn't matter which direction the verification is, we always show the same QR code - // for not-ourself verification. - const myXsignInfo = cli.getStoredCrossSigningForUser(myUserId); - const otherXsignInfo = cli.getStoredCrossSigningForUser(otherUserId); - const otherDevices = (await cli.getStoredDevicesForUser(otherUserId)) || []; - const otherDevice = otherDevices.find(d => d.deviceId === otherDeviceId); + // Second key is the other user's master cross signing key + const otherUserCrossSigningInfo = cli.getStoredCrossSigningForUser(otherUserId); + qrProps.secondKeyB64 = otherUserCrossSigningInfo.getId("master"); + } else if (mode === MODE_VERIFY_SELF_TRUSTED) { + // First key is our master cross signing key + qrProps.firstKeyB64 = myCrossSigningInfo.getId("master"); - qrProps.keys = [ - [myDeviceId, cli.getDeviceEd25519Key()], - [myXsignInfo.getId("master"), myXsignInfo.getId("master")], - ]; - qrProps.otherUserKey = otherXsignInfo.getId("master"); - if (otherDevice) qrProps.otherUserDeviceKey = otherDevice.getFingerprint(); + // Second key is the other device's device key + const otherDevice = verificationRequest.targetDevice; + const otherDeviceId = otherDevice ? otherDevice.deviceId : null; + const device = myDevices.find(d => d.deviceId === otherDeviceId); + qrProps.secondKeyB64 = device.getFingerprint(); + } else if (mode === MODE_VERIFY_SELF_UNTRUSTED) { + // First key is our device's key + qrProps.firstKeyB64 = cli.getDeviceEd25519Key(); + + // Second key is what we think our master cross signing key is + qrProps.secondKeyB64 = myCrossSigningInfo.getId("master"); } return qrProps; @@ -107,21 +105,63 @@ export default class VerificationQRCode extends React.PureComponent { constructor(props) { super(props); + + this.state = { + dataUri: null, + }; + this.generateQrCode(); + } + + componentDidUpdate(prevProps: Readonly

): void { + if (JSON.stringify(this.props) === JSON.stringify(prevProps)) return; // No prop change + + this.generateQRCode(); + } + + async generateQrCode() { + let buf = Buffer.alloc(0); // we'll concat our way through life + + const appendByte = (b: number) => { + const tmpBuf = Buffer.from([b]); + buf = Buffer.concat([buf, tmpBuf]); + }; + const appendInt = (i: number) => { + const tmpBuf = Buffer.alloc(4); + tmpBuf.writeInt8(i, 0); + buf = Buffer.concat([buf, tmpBuf]); + }; + const appendStr = (s: string, enc: string) => { + const tmpBuf = Buffer.from(s, enc); + appendInt(tmpBuf.byteLength); + buf = Buffer.concat([buf, tmpBuf]); + }; + const appendEncBase64 = (b64: string) => { + const b = decodeBase64(b64); + const tmpBuf = Buffer.from(b); + buf = Buffer.concat([buf, tmpBuf]); + }; + + // Actually build the buffer for the QR code + appendStr(this.props.prefix, "ascii"); + appendByte(this.props.version); + appendByte(this.props.mode); + appendStr(this.props.transactionId, "utf-8"); + appendEncBase64(this.props.firstKeyB64); + appendEncBase64(this.props.secondKeyB64); + appendEncBase64(this.props.secretB64); + + // Now actually assemble the QR code's data URI + const uri = await QRCode.toDataURL([{data: buf, mode: 'byte'}], { + errorCorrectionLevel: 'L', // we want it as trivial-looking as possible + }); + this.setState({dataUri: uri}); } render() { - const query = { - request: this.props.requestEventId, - action: this.props.action, - other_user_key: this.props.otherUserKey, - secret: this.props.secret, - }; - for (const key of this.props.keys) { - query[`key_${key[0]}`] = key[1]; + if (!this.state.dataUri) { + return

; } - const uri = `https://matrix.to/#/${this.props.keyholderUserId}?${qs.stringify(query)}`; - - return ; + return ; } } diff --git a/yarn.lock b/yarn.lock index 6924d7dcc0..5aa6a3ed0c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2101,7 +2101,25 @@ bser@2.1.1: dependencies: node-int64 "^0.4.0" -buffer-from@^1.0.0: +buffer-alloc-unsafe@^1.1.0: + version "1.1.0" + resolved "https://registry.yarnpkg.com/buffer-alloc-unsafe/-/buffer-alloc-unsafe-1.1.0.tgz#bd7dc26ae2972d0eda253be061dba992349c19f0" + integrity sha512-TEM2iMIEQdJ2yjPJoSIsldnleVaAk1oW3DBVUykyOLsEsFmEc9kn+SFFPz+gl54KQNxlDnAwCXosOS9Okx2xAg== + +buffer-alloc@^1.2.0: + version "1.2.0" + resolved "https://registry.yarnpkg.com/buffer-alloc/-/buffer-alloc-1.2.0.tgz#890dd90d923a873e08e10e5fd51a57e5b7cce0ec" + integrity sha512-CFsHQgjtW1UChdXgbyJGtnm+O/uLQeZdtbDo8mfUgYXCHSM1wgrVxXm6bSyrUuErEb+4sYVGCzASBRot7zyrow== + dependencies: + buffer-alloc-unsafe "^1.1.0" + buffer-fill "^1.0.0" + +buffer-fill@^1.0.0: + version "1.0.0" + resolved "https://registry.yarnpkg.com/buffer-fill/-/buffer-fill-1.0.0.tgz#f8f78b76789888ef39f205cd637f68e702122b2c" + integrity sha1-+PeLdniYiO858gXNY39o5wISKyw= + +buffer-from@^1.0.0, buffer-from@^1.1.1: version "1.1.1" resolved "https://registry.yarnpkg.com/buffer-from/-/buffer-from-1.1.1.tgz#32713bc028f75c02fdb710d7c7bcec1f2c6070ef" integrity sha512-MQcXEUbCKtEo7bhqEs6560Hyd4XaovZlO/k9V3hjVUF/zwW7KBVdSK4gIt/bzwS9MbR5qob+F5jusZsb0YQK2A== @@ -2120,6 +2138,14 @@ buffer@^4.3.0: ieee754 "^1.1.4" isarray "^1.0.0" +buffer@^5.4.3: + version "5.4.3" + resolved "https://registry.yarnpkg.com/buffer/-/buffer-5.4.3.tgz#3fbc9c69eb713d323e3fc1a895eee0710c072115" + integrity sha512-zvj65TkFeIt3i6aj5bIvJDzjjQQGs4o/sNoezg1F1kYap9Nu2jcUdpwzRSJTHMMzG0H7bZkn4rNQpImhuxWX2A== + dependencies: + base64-js "^1.0.2" + ieee754 "^1.1.4" + builtin-modules@^1.1.1: version "1.1.1" resolved "https://registry.yarnpkg.com/builtin-modules/-/builtin-modules-1.1.1.tgz#270f076c5a72c02f5b65a47df94c5fe3a278892f" @@ -2887,6 +2913,11 @@ diffie-hellman@^5.0.0: miller-rabin "^4.0.0" randombytes "^2.0.0" +dijkstrajs@^1.0.1: + version "1.0.1" + resolved "https://registry.yarnpkg.com/dijkstrajs/-/dijkstrajs-1.0.1.tgz#d3cd81221e3ea40742cfcde556d4e99e98ddc71b" + integrity sha1-082BIh4+pAdCz83lVtTpnpjdxxs= + dir-glob@^2.2.2: version "2.2.2" resolved "https://registry.yarnpkg.com/dir-glob/-/dir-glob-2.2.2.tgz#fa09f0694153c8918b18ba0deafae94769fc50c4" @@ -4882,7 +4913,7 @@ isarray@1.0.0, isarray@^1.0.0, isarray@~1.0.0: resolved "https://registry.yarnpkg.com/isarray/-/isarray-1.0.0.tgz#bb935d48582cba168c06834957a54a3e07124f11" integrity sha1-u5NdSFgsuhaMBoNJV6VKPgcSTxE= -isarray@^2.0.5: +isarray@^2.0.1, isarray@^2.0.5: version "2.0.5" resolved "https://registry.yarnpkg.com/isarray/-/isarray-2.0.5.tgz#8af1e4c1221244cc62459faf38940d4e644a5723" integrity sha512-xHjhDr3cNBK0BzdUJSPXZntQUx/mwMS5Rw4A7lPJ90XGAO6ISP/ePDNuo0vhqOZU+UD5JoodwCAAoZQd3FeAKw== @@ -6728,6 +6759,11 @@ png-chunks-extract@^1.0.0: dependencies: crc-32 "^0.3.0" +pngjs@^3.3.0: + version "3.4.0" + resolved "https://registry.yarnpkg.com/pngjs/-/pngjs-3.4.0.tgz#99ca7d725965fb655814eaf65f38f12bbdbf555f" + integrity sha512-NCrCHhWmnQklfH4MtJMRjZ2a8c80qXeMlQMv2uVp9ISJMTt562SbGd6n2oq0PaPgKm7Z6pL9E2UlLIhC+SHL3w== + posix-character-classes@^0.1.0: version "0.1.1" resolved "https://registry.yarnpkg.com/posix-character-classes/-/posix-character-classes-0.1.1.tgz#01eac0fe3b5af71a2a6c02feabb8c1fef7e00eab" @@ -7018,6 +7054,19 @@ qrcode-react@^0.1.16: dependencies: qr.js "0.0.0" +qrcode@^1.4.4: + version "1.4.4" + resolved "https://registry.yarnpkg.com/qrcode/-/qrcode-1.4.4.tgz#f0c43568a7e7510a55efc3b88d9602f71963ea83" + integrity sha512-oLzEC5+NKFou9P0bMj5+v6Z40evexeE29Z9cummZXZ9QXyMr3lphkURzxjXgPJC5azpxcshoDWV1xE46z+/c3Q== + dependencies: + buffer "^5.4.3" + buffer-alloc "^1.2.0" + buffer-from "^1.1.1" + dijkstrajs "^1.0.1" + isarray "^2.0.1" + pngjs "^3.3.0" + yargs "^13.2.4" + qs@^6.5.2, qs@^6.6.0: version "6.9.1" resolved "https://registry.yarnpkg.com/qs/-/qs-6.9.1.tgz#20082c65cb78223635ab1a9eaca8875a29bf8ec9" @@ -9260,7 +9309,7 @@ yargs@^12.0.5: y18n "^3.2.1 || ^4.0.0" yargs-parser "^11.1.1" -yargs@^13.3.0: +yargs@^13.2.4, yargs@^13.3.0: version "13.3.0" resolved "https://registry.yarnpkg.com/yargs/-/yargs-13.3.0.tgz#4c657a55e07e5f2cf947f8a366567c04a0dedc83" integrity sha512-2eehun/8ALW8TLoIl7MVaRUrg+yCnenu8B4kBlRxj3GJGDKU1Og7sMXPNm1BYyM1DOJmTZ4YeN/Nwxv+8XJsUA== From 3b6a201d4ea80566214e2038adbff80e76c8c8c6 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 19 Feb 2020 17:03:50 -0700 Subject: [PATCH 2/3] Add comments --- src/components/views/elements/crypto/VerificationQRCode.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/views/elements/crypto/VerificationQRCode.js b/src/components/views/elements/crypto/VerificationQRCode.js index 09408fe4dd..407c1adf81 100644 --- a/src/components/views/elements/crypto/VerificationQRCode.js +++ b/src/components/views/elements/crypto/VerificationQRCode.js @@ -24,9 +24,9 @@ import {decodeBase64} from "matrix-js-sdk/src/crypto/olmlib"; import Spinner from "../Spinner"; import * as QRCode from "qrcode"; -const CODE_VERSION = 0x02; -const BINARY_PREFIX = "MATRIX"; // ASCII, used to prefix the -const MODE_VERIFY_OTHER_USER = 0x00; +const CODE_VERSION = 0x02; // the version of binary QR codes we support +const BINARY_PREFIX = "MATRIX"; // ASCII, used to prefix the binary format +const MODE_VERIFY_OTHER_USER = 0x00; // Verifying someone who isn't us const MODE_VERIFY_SELF_TRUSTED = 0x01; // We trust the master key const MODE_VERIFY_SELF_UNTRUSTED = 0x02; // We do not trust the master key From 760a472be51f6c8550ace4f43a51e6691c4b6133 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 19 Feb 2020 17:05:32 -0700 Subject: [PATCH 3/3] Appease the linter --- src/components/views/elements/crypto/VerificationQRCode.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/views/elements/crypto/VerificationQRCode.js b/src/components/views/elements/crypto/VerificationQRCode.js index 407c1adf81..cbbe2912d6 100644 --- a/src/components/views/elements/crypto/VerificationQRCode.js +++ b/src/components/views/elements/crypto/VerificationQRCode.js @@ -59,7 +59,7 @@ export default class VerificationQRCode extends React.PureComponent { } const requestEvent = verificationRequest.requestEvent; - let transactionId = requestEvent.getId() + const transactionId = requestEvent.getId() ? requestEvent.getId() : ToDeviceChannel.getTransactionId(requestEvent); @@ -112,7 +112,7 @@ export default class VerificationQRCode extends React.PureComponent { this.generateQrCode(); } - componentDidUpdate(prevProps: Readonly

): void { + componentDidUpdate(prevProps): void { if (JSON.stringify(this.props) === JSON.stringify(prevProps)) return; // No prop change this.generateQRCode();