-
Notifications
You must be signed in to change notification settings - Fork 305
chore: use io-ts to parse vault keycard box + update keycard formatting and QR headers #9225
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
s84krish
wants to merge
1
commit into
master
Choose a base branch
from
WCN-1193-followups
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -56,34 +56,67 @@ function moveDown(y: number, ydelta: number): number { | |
| return y + ydelta; | ||
| } | ||
|
|
||
| /** | ||
| * Prefixes one fragment of a split key with a part header so a QR scanner can reassemble the | ||
| * fragments without relying on scan order. | ||
| * | ||
| * When a key box's payload exceeds {@link QRBinaryMaxLength}, {@link splitKeys} divides it into | ||
| * multiple QR codes. Each fragment's QR payload is encoded as a 1-based part header followed by | ||
| * the fragment: | ||
| * | ||
| * "<index>/<total>|<fragment>" e.g. "1/3|<chunk>", "2/3|<chunk>", "3/3|<chunk>" | ||
| * | ||
| * A single-fragment payload is returned unchanged (no header). | ||
| * | ||
| * Reassembly contract for a consumer (e.g. a future recovery/scan tool): | ||
| * 1. Scan every QR code for a box. | ||
| * 2. Split each payload on the FIRST "|": the left side is "<index>/<total>", the right side | ||
| * is the fragment. | ||
| * 3. Verify parts 1..total are all present (total is identical in every header). | ||
| * 4. Concatenate the fragments in ascending index order to recover the full box payload. | ||
| * 5. Parse/decrypt as usual (for a vault box: JSON.parse, then decrypt each root value). | ||
| * | ||
| * Notes: | ||
| * - The "|" delimiter is safe: base64 ciphertext, JSON, and base58 xpubs never contain it. | ||
| * - This header exists ONLY inside the QR image. The human-readable "Data:" text printed on | ||
| * the card is the full, unheadered payload; the PDF-text parser reads that, so this header | ||
| * does not affect PDF-based recovery. | ||
| */ | ||
| function encodeQrCodePart(fragment: string, index: number, total: number): string { | ||
| return total > 1 ? `${index + 1}/${total}|${fragment}` : fragment; | ||
| } | ||
|
|
||
| // Draws QR codes down the left column, returning the index of the next QR still to draw | ||
| // (for continuation on a later page) and the y-offset just below the drawn QR column (so | ||
| // callers can place content, e.g. a note, under the QR codes). | ||
| function drawOnePageOfQrCodes( | ||
| qrImages: HTMLCanvasElement[], | ||
| doc: jsPDF, | ||
| y: number, | ||
| qrSize: number, | ||
| startIndex | ||
| ): number { | ||
| startIndex: number | ||
| ): { nextIndex: number; endY: number } { | ||
| doc.setFont('helvetica'); | ||
| let qrIndex: number = startIndex; | ||
| for (; qrIndex < qrImages.length; qrIndex++) { | ||
| const image = qrImages[qrIndex]; | ||
| const textBuffer = 15; | ||
| if (y + qrSize + textBuffer >= doc.internal.pageSize.getHeight()) { | ||
| return qrIndex; | ||
| return { nextIndex: qrIndex, endY: y }; | ||
| } | ||
|
|
||
| doc.addImage(image, left(0), y, qrSize, qrSize); | ||
|
|
||
| if (qrImages.length === 1) { | ||
| return qrIndex + 1; | ||
| return { nextIndex: qrIndex + 1, endY: y + qrSize }; | ||
| } | ||
|
|
||
| y = moveDown(y, qrSize + textBuffer); | ||
| doc.setFontSize(font.body).setTextColor(color.black); | ||
| doc.text('Part ' + (qrIndex + 1).toString(), left(0), y); | ||
| y = moveDown(y, 20); | ||
| } | ||
| return qrIndex + 1; | ||
| return { nextIndex: qrIndex, endY: y }; | ||
| } | ||
|
|
||
| function computeKeyCardImageDimensions(keyCardImage: HTMLImageElement) { | ||
|
|
@@ -115,6 +148,7 @@ export async function drawKeycard({ | |
| walletLabel, | ||
| curve, | ||
| pageBreakBeforeIndices = DEFAULT_PAGE_BREAK_INDICES, | ||
| useQrPartHeaders = false, | ||
| }: IDrawKeyCard): Promise<jsPDF> { | ||
| const jsPDFModule = await loadJSPDF(); | ||
|
|
||
|
|
@@ -203,11 +237,26 @@ export async function drawKeycard({ | |
|
|
||
| const qrImages: HTMLCanvasElement[] = []; | ||
| const keys = splitKeys(qr.data, QRBinaryMaxLength); | ||
| for (const key of keys) { | ||
| qrImages.push(await QRCode.toCanvas(key, { errorCorrectionLevel: 'L' })); | ||
| for (let i = 0; i < keys.length; i++) { | ||
| const payload = useQrPartHeaders ? encodeQrCodePart(keys[i], i, keys.length) : keys[i]; | ||
| qrImages.push(await QRCode.toCanvas(payload, { errorCorrectionLevel: 'L' })); | ||
| } | ||
|
|
||
| let nextQrIndex = drawOnePageOfQrCodes(qrImages, doc, y, qrSize, 0); | ||
| const isMultiPart = qr?.data?.length > QRBinaryMaxLength; | ||
|
s84krish marked this conversation as resolved.
|
||
| const { nextIndex, endY: qrColumnEndY } = drawOnePageOfQrCodes(qrImages, doc, y, qrSize, 0); | ||
| let nextQrIndex = nextIndex; | ||
|
|
||
| // For a split key, place the "put all Parts together" note directly under the QR codes. | ||
| // Wrap it to the QR column width so it stays in the left column and never overlaps the | ||
| // Data payload rendered on the right. | ||
| let qrColumnBottom = qrColumnEndY; | ||
| if (isMultiPart) { | ||
| doc.setFontSize(font.body - 2).setTextColor(color.darkgray); | ||
| const noteLines = doc.splitTextToSize('Note: you will need to put all Parts together for the full key', qrSize); | ||
| const noteY = moveDown(qrColumnEndY, 15); | ||
| doc.text(noteLines, left(0), noteY); | ||
| qrColumnBottom = noteY + noteLines.length * doc.getLineHeight(); | ||
| } | ||
|
|
||
| doc.setFontSize(font.subheader).setTextColor(color.black); | ||
| y = moveDown(y, 10); | ||
|
|
@@ -220,11 +269,6 @@ export async function drawKeycard({ | |
| doc.text(qr.description, textLeft, y); | ||
| textHeight += doc.getLineHeight(); | ||
| doc.setFontSize(font.body - 2); | ||
| if (qr?.data?.length > QRBinaryMaxLength) { | ||
| y = moveDown(y, 30); | ||
| textHeight += 30; | ||
| doc.text('Note: you will need to put all Parts together for the full key', textLeft, y); | ||
| } | ||
| y = moveDown(y, 30); | ||
| textHeight += 30; | ||
| doc.text('Data:', textLeft, y); | ||
|
|
@@ -242,7 +286,11 @@ export async function drawKeycard({ | |
| textHeight = 0; | ||
| y = 30; | ||
| topY = y; | ||
| nextQrIndex = drawOnePageOfQrCodes(qrImages, doc, y, qrSize, nextQrIndex); | ||
| // Redraw remaining QRs on the new page and update the column bottom to THIS page, so | ||
| // the rowHeight math below stays same-page (avoids a stale page-1 coordinate). | ||
| const continued = drawOnePageOfQrCodes(qrImages, doc, y, qrSize, nextQrIndex); | ||
| nextQrIndex = continued.nextIndex; | ||
| qrColumnBottom = continued.endY; | ||
| doc.setFont('courier').setFontSize(9).setTextColor(color.black); | ||
| } | ||
| doc.text(lines[line], textLeft, y); | ||
|
|
@@ -273,9 +321,10 @@ export async function drawKeycard({ | |
| } | ||
|
|
||
| doc.setFont('helvetica'); | ||
| // Move down the size of the QR code minus accumulated height on the right side, plus margin | ||
| // if we have a key that spans multiple pages, then exclude QR code size | ||
| const rowHeight = Math.max(qr.data.length > QRBinaryMaxLength ? qrSize + 20 : qrSize, textHeight); | ||
| // Move down past the taller of the two columns: the right-side text (textHeight) or the | ||
| // left-side QR column (plus the note printed under it for split keys), then add a margin. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what with the +10? is there a reasoning for it? if there is -- maybe add a comment? |
||
| const qrColumnHeight = isMultiPart ? qrColumnBottom - topY + 10 : qrSize; | ||
| const rowHeight = Math.max(qrColumnHeight, textHeight); | ||
| const marginBottom = 15; | ||
| y = moveDown(y, rowHeight - (y - topY) + marginBottom); | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -132,16 +132,21 @@ function generateUserMasterPublicKeyQRData(publicKey: string): MasterPublicKeyQr | |
| }; | ||
| } | ||
|
|
||
| // The product a keycard belongs to, used for user-facing wording (e.g. Box D copy). | ||
| type KeycardEntity = 'wallet' | 'vault'; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should this go to the types? wdyt? |
||
|
|
||
| async function generatePasscodeQrData( | ||
| passphrase: string, | ||
| passcodeEncryptionCode: string, | ||
| encryptionVersion?: 1 | 2 | ||
| encryptionVersion?: 1 | 2, | ||
| entityNoun: KeycardEntity = 'wallet' | ||
| ): Promise<QrDataEntry> { | ||
| const encryptedWalletPasscode = await encrypt(passcodeEncryptionCode, passphrase, { encryptionVersion }); | ||
| const encryptedPasscode = await encrypt(passcodeEncryptionCode, passphrase, { encryptionVersion }); | ||
| const titleNoun = entityNoun === 'vault' ? 'Vault' : 'Wallet'; | ||
| return { | ||
| title: 'D: Encrypted Wallet Password', | ||
| description: 'This is the wallet password, encrypted client-side with a key held by BitGo.', | ||
| data: encryptedWalletPasscode, | ||
| title: `D: Encrypted ${titleNoun} Password`, | ||
| description: `This is the ${entityNoun} password, encrypted client-side with a key held by BitGo.`, | ||
| data: encryptedPasscode, | ||
| }; | ||
| } | ||
|
|
||
|
|
@@ -263,12 +268,12 @@ export async function generateVaultQrData(params: GenerateVaultQrDataParams): Pr | |
| const qrData: QrData = { | ||
| user: { | ||
| title: 'A: User Key', | ||
| description: 'Your 4 root private keys, encrypted with your wallet password.', | ||
| description: 'Your 4 root private keys, encrypted with your vault password.', | ||
| data: buildVaultBoxData(roots, (root, slot) => selectRootPrivateKey(root.userKeychain, slot, 'user')), | ||
| }, | ||
| backup: { | ||
| title: 'B: Backup Key', | ||
| description: 'Your 4 root backup keys, encrypted with your wallet password.', | ||
| description: 'Your 4 root backup keys, encrypted with your vault password.', | ||
| data: buildVaultBoxData(roots, (root, slot) => selectRootPrivateKey(root.backupKeychain, slot, 'backup')), | ||
| }, | ||
| bitgo: { | ||
|
|
@@ -282,7 +287,8 @@ export async function generateVaultQrData(params: GenerateVaultQrDataParams): Pr | |
| qrData.passcode = await generatePasscodeQrData( | ||
| params.passphrase, | ||
| params.passcodeEncryptionCode, | ||
| params.encryptionVersion | ||
| params.encryptionVersion, | ||
| 'vault' | ||
| ); | ||
| } | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering what the best way to do this parts encoding would be - if it is even necessary.