From cc19e40e48589f632dcd29e768f20089fbe38121 Mon Sep 17 00:00:00 2001 From: kyranjamie Date: Mon, 26 Feb 2024 17:05:56 +0100 Subject: [PATCH] fix(ledger): stacks message signing, closes #4945 --- package.json | 2 + src/app/common/publish-subscribe.ts | 10 +++++ .../ledger-stacks-sign-msg-container.tsx | 19 +++------- .../stacks-message-signing-event-listeners.ts | 29 +++++++++++++++ .../use-message-type.ts | 22 +++++------ .../ledger/hooks/use-ledger-navigate.ts | 18 ++++----- .../use-sign-stacks-message.ts | 26 +++++++------ .../use-rpc-sign-stacks-message.ts | 20 +++++----- .../use-sign-stacks-message-request.ts | 1 + src/shared/signature/signature-types.ts | 37 ++++++++++++++++++- yarn.lock | 12 ++++++ 11 files changed, 139 insertions(+), 57 deletions(-) create mode 100644 src/app/features/ledger/flows/stacks-message-signing/stacks-message-signing-event-listeners.ts diff --git a/package.json b/package.json index 8e3da47c48c..291721adfc3 100644 --- a/package.json +++ b/package.json @@ -202,6 +202,7 @@ "ledger-bitcoin": "0.2.3", "limiter": "2.1.0", "lodash.get": "4.4.2", + "lodash.isequal": "4.5.0", "lodash.uniqby": "4.7.0", "micro-packed": "0.3.2", "object-hash": "3.0.0", @@ -269,6 +270,7 @@ "@types/html-webpack-plugin": "3.2.9", "@types/jsdom": "21.1.3", "@types/lodash.get": "4.4.7", + "@types/lodash.isequal": "4.5.8", "@types/node": "20.11.19", "@types/prismjs": "1.26.3", "@types/promise-memoize": "1.2.4", diff --git a/src/app/common/publish-subscribe.ts b/src/app/common/publish-subscribe.ts index 4f97b4d5621..4285815f714 100644 --- a/src/app/common/publish-subscribe.ts +++ b/src/app/common/publish-subscribe.ts @@ -1,6 +1,9 @@ import type { Transaction } from '@scure/btc-signer'; +import type { SignatureData } from '@stacks/connect'; import type { StacksTransaction } from '@stacks/transactions'; +import type { UnsignedMessage } from '@shared/signature/signature-types'; + type PubTypeFn = ( event: Key, // Ensures if we have an event with no payload, the second arg can be empty, @@ -59,6 +62,13 @@ export interface GlobalAppEvents { ledgerBitcoinTxSigningCancelled: { unsignedPsbt: string; }; + ledgerStacksMessageSigned: { + unsignedMessage: UnsignedMessage; + messageSignatures: SignatureData; + }; + ledgerStacksMessageSigningCancelled: { + unsignedMessage: UnsignedMessage; + }; } export const appEvents = createPublishSubscribe(); diff --git a/src/app/features/ledger/flows/stacks-message-signing/ledger-stacks-sign-msg-container.tsx b/src/app/features/ledger/flows/stacks-message-signing/ledger-stacks-sign-msg-container.tsx index e3b250b457e..c16b0d498f0 100644 --- a/src/app/features/ledger/flows/stacks-message-signing/ledger-stacks-sign-msg-container.tsx +++ b/src/app/features/ledger/flows/stacks-message-signing/ledger-stacks-sign-msg-container.tsx @@ -6,12 +6,11 @@ import { serializeCV } from '@stacks/transactions'; import { LedgerError } from '@zondax/ledger-stacks'; import get from 'lodash.get'; -import { finalizeMessageSignature } from '@shared/actions/finalize-message-signature'; -import { logger } from '@shared/logger'; import { UnsignedMessage, whenSignableMessageOfType } from '@shared/signature/signature-types'; import { isError } from '@shared/utils'; import { useScrollLock } from '@app/common/hooks/use-scroll-lock'; +import { appEvents } from '@app/common/publish-subscribe'; import { delay } from '@app/common/utils'; import { BaseDrawer } from '@app/components/drawer/base-drawer'; import { @@ -23,7 +22,6 @@ import { } from '@app/features/ledger/utils/stacks-ledger-utils'; import { useCurrentStacksAccount } from '@app/store/accounts/blockchain/stacks/stacks-account.hooks'; import { StacksAccount } from '@app/store/accounts/blockchain/stacks/stacks-account.models'; -import { useSignatureRequestSearchParams } from '@app/store/signatures/requests.hooks'; import { useLedgerAnalytics } from '../../hooks/use-ledger-analytics.hook'; import { useLedgerNavigate } from '../../hooks/use-ledger-navigate'; @@ -57,7 +55,6 @@ function LedgerSignStacksMsg({ account, unsignedMessage }: LedgerSignMsgProps) { const ledgerNavigate = useLedgerNavigate(); const ledgerAnalytics = useLedgerAnalytics(); const verifyLedgerPublicKey = useVerifyMatchingLedgerStacksPublicKey(); - const { tabId, requestToken } = useSignatureRequestSearchParams(); const [latestDeviceResponse, setLatestDeviceResponse] = useLedgerResponseState(); const canUserCancelAction = useActionCancellableByUser(); @@ -86,11 +83,6 @@ function LedgerSignStacksMsg({ account, unsignedMessage }: LedgerSignMsgProps) { return; } - if (!tabId || !requestToken) { - logger.warn('Cannot sign message without corresponding `tabId` or `requestToken'); - return; - } - ledgerNavigate.toDeviceBusyStep(`Verifying public key on Ledger…`); await verifyLedgerPublicKey(stacksApp); @@ -122,7 +114,7 @@ function LedgerSignStacksMsg({ account, unsignedMessage }: LedgerSignMsgProps) { if (resp.returnCode === LedgerError.TransactionRejected) { ledgerNavigate.toOperationRejectedStep(`Message signing operation rejected`); ledgerAnalytics.messageSignedOnLedgerRejected(); - finalizeMessageSignature({ requestPayload: requestToken, tabId, data: 'cancel' }); + appEvents.publish('ledgerStacksMessageSigningCancelled', { unsignedMessage }); return; } if (resp.returnCode !== LedgerError.NoErrors) { @@ -133,13 +125,12 @@ function LedgerSignStacksMsg({ account, unsignedMessage }: LedgerSignMsgProps) { ledgerAnalytics.messageSignedOnLedgerSuccessfully(); - finalizeMessageSignature({ - requestPayload: requestToken, - tabId, - data: { + appEvents.publish('ledgerStacksMessageSigned', { + messageSignatures: { signature: signatureVrsToRsv(resp.signatureVRS.toString('hex')), publicKey: account.stxPublicKey, }, + unsignedMessage, }); await stacksApp.transport.close(); diff --git a/src/app/features/ledger/flows/stacks-message-signing/stacks-message-signing-event-listeners.ts b/src/app/features/ledger/flows/stacks-message-signing/stacks-message-signing-event-listeners.ts new file mode 100644 index 00000000000..c518229ef63 --- /dev/null +++ b/src/app/features/ledger/flows/stacks-message-signing/stacks-message-signing-event-listeners.ts @@ -0,0 +1,29 @@ +import isEqual from 'lodash.isequal'; + +import type { UnsignedMessage } from '@shared/signature/signature-types'; + +import { GlobalAppEvents, appEvents } from '@app/common/publish-subscribe'; + +export async function listenForStacksMessageSigning( + unsignedMessage: UnsignedMessage +): Promise { + return new Promise((resolve, reject) => { + function stacksMessageSignedHandler(msg: GlobalAppEvents['ledgerStacksMessageSigned']) { + if (isEqual(msg.unsignedMessage, unsignedMessage)) { + appEvents.unsubscribe('ledgerStacksMessageSigned', stacksMessageSignedHandler); + appEvents.unsubscribe('ledgerStacksMessageSigningCancelled', signingAbortedHandler); + resolve(msg.messageSignatures); + } + } + appEvents.subscribe('ledgerStacksMessageSigned', stacksMessageSignedHandler); + + function signingAbortedHandler(msg: GlobalAppEvents['ledgerStacksMessageSigningCancelled']) { + if (isEqual(msg.unsignedMessage, unsignedMessage)) { + appEvents.unsubscribe('ledgerStacksMessageSigningCancelled', signingAbortedHandler); + appEvents.unsubscribe('ledgerStacksMessageSigned', stacksMessageSignedHandler); + reject(new Error('User cancelled the signing operation')); + } + } + appEvents.subscribe('ledgerStacksMessageSigningCancelled', signingAbortedHandler); + }); +} diff --git a/src/app/features/ledger/flows/stacks-message-signing/use-message-type.ts b/src/app/features/ledger/flows/stacks-message-signing/use-message-type.ts index e6752641dce..ca35fba1b9b 100644 --- a/src/app/features/ledger/flows/stacks-message-signing/use-message-type.ts +++ b/src/app/features/ledger/flows/stacks-message-signing/use-message-type.ts @@ -1,21 +1,21 @@ -import { ClarityValue } from '@stacks/transactions'; - -import { StructuredMessageDataDomain, UnsignedMessage } from '@shared/signature/signature-types'; +import { + type SignedMessageType, + UnsignedMessage, + deserializeUnsignedMessage, +} from '@shared/signature/signature-types'; import { isString } from '@shared/utils'; import { useLocationStateWithCache } from '@app/common/hooks/use-location-state'; export function useUnsignedMessageType(): UnsignedMessage | null { - const message = useLocationStateWithCache('message'); - const domain = useLocationStateWithCache('domain'); + const messageType = useLocationStateWithCache('messageType'); + const message = useLocationStateWithCache('message'); + const domain = useLocationStateWithCache('domain'); - if (isString(message)) - return { - messageType: 'utf8', - message, - }; + if (messageType === 'utf8' && isString(message)) return { messageType, message }; - if (message && domain) return { messageType: 'structured', message, domain }; + if (messageType === 'structured' && message && !isString(message) && domain) + return deserializeUnsignedMessage({ messageType, message, domain }); return null; } diff --git a/src/app/features/ledger/hooks/use-ledger-navigate.ts b/src/app/features/ledger/hooks/use-ledger-navigate.ts index 49258117ed9..8a62b0f759f 100644 --- a/src/app/features/ledger/hooks/use-ledger-navigate.ts +++ b/src/app/features/ledger/hooks/use-ledger-navigate.ts @@ -2,11 +2,15 @@ import { useMemo } from 'react'; import { useLocation, useNavigate } from 'react-router-dom'; import { bytesToHex } from '@stacks/common'; -import { ClarityValue, StacksTransaction } from '@stacks/transactions'; +import { StacksTransaction } from '@stacks/transactions'; import { SupportedBlockchains } from '@shared/constants'; import { BitcoinInputSigningConfig } from '@shared/crypto/bitcoin/signer-config'; import { RouteUrls } from '@shared/route-urls'; +import { + type UnsignedMessage, + toSerializableUnsignedMessage, +} from '@shared/signature/signature-types'; import { immediatelyAttemptLedgerConnection } from './use-when-reattempt-ledger-connection'; @@ -42,17 +46,11 @@ export function useLedgerNavigate() { }); }, - toConnectAndSignUtf8MessageStep(message: string) { + toConnectAndSignMessageStep(message: UnsignedMessage) { return navigate(RouteUrls.ConnectLedger, { replace: true, - state: { type: 'utf8', message }, - }); - }, - - toConnectAndSignStructuredMessageStep(domain: ClarityValue, message: ClarityValue) { - return navigate(RouteUrls.ConnectLedger, { - replace: true, - state: { type: 'structured', domain, message }, + // Unsigned messages may contain unserializable data, such as bigint + state: { ...toSerializableUnsignedMessage(message) }, }); }, diff --git a/src/app/features/stacks-message-signer/use-sign-stacks-message.ts b/src/app/features/stacks-message-signer/use-sign-stacks-message.ts index 285f562f3ec..0e547994acb 100644 --- a/src/app/features/stacks-message-signer/use-sign-stacks-message.ts +++ b/src/app/features/stacks-message-signer/use-sign-stacks-message.ts @@ -3,7 +3,7 @@ import { useState } from 'react'; import { SignatureData } from '@stacks/connect'; import { logger } from '@shared/logger'; -import { UnsignedMessage, whenSignableMessageOfType } from '@shared/signature/signature-types'; +import { UnsignedMessage } from '@shared/signature/signature-types'; import { useAnalytics } from '@app/common/hooks/analytics/use-analytics'; import { useWalletType } from '@app/common/use-wallet-type'; @@ -13,11 +13,16 @@ import { useMessageSignerStacksSoftwareWallet, } from '@app/features/stacks-message-signer/stacks-message-signing.utils'; +import { listenForStacksMessageSigning } from '../ledger/flows/stacks-message-signing/stacks-message-signing-event-listeners'; + interface SignStacksMessageProps { onSignMessageCompleted(messageSignature: SignatureData): void; + onSignMessageCancelled(): void; } - -export function useSignStacksMessage({ onSignMessageCompleted }: SignStacksMessageProps) { +export function useSignStacksMessage({ + onSignMessageCompleted, + onSignMessageCancelled, +}: SignStacksMessageProps) { const analytics = useAnalytics(); const signSoftwareWalletMessage = useMessageSignerStacksSoftwareWallet(); @@ -46,14 +51,13 @@ export function useSignStacksMessage({ onSignMessageCompleted }: SignStacksMessa async ledger(unsignedMessage: UnsignedMessage) { void analytics.track('request_signature_sign', { type: 'ledger' }); - whenSignableMessageOfType(unsignedMessage)({ - utf8(msg) { - ledgerNavigate.toConnectAndSignUtf8MessageStep(msg); - }, - structured(domain, msg) { - ledgerNavigate.toConnectAndSignStructuredMessageStep(domain, msg); - }, - }); + ledgerNavigate.toConnectAndSignMessageStep(unsignedMessage); + try { + const messageSignature = await listenForStacksMessageSigning(unsignedMessage); + onSignMessageCompleted(messageSignature); + } catch (e) { + onSignMessageCancelled(); + } }, }); diff --git a/src/app/pages/rpc-sign-stacks-message/use-rpc-sign-stacks-message.ts b/src/app/pages/rpc-sign-stacks-message/use-rpc-sign-stacks-message.ts index f0e6e5b42ec..e64dc826d2f 100644 --- a/src/app/pages/rpc-sign-stacks-message/use-rpc-sign-stacks-message.ts +++ b/src/app/pages/rpc-sign-stacks-message/use-rpc-sign-stacks-message.ts @@ -1,5 +1,4 @@ import { useMemo } from 'react'; -import { useSearchParams } from 'react-router-dom'; import { RpcErrorCode } from '@btckit/types'; import { StacksNetwork } from '@stacks/network'; @@ -15,6 +14,7 @@ import { closeWindow } from '@shared/utils'; import { useAnalytics } from '@app/common/hooks/analytics/use-analytics'; import { useDefaultRequestParams } from '@app/common/hooks/use-default-request-search-params'; +import { initialSearchParams } from '@app/common/initial-search-params'; import { StructuredPayload, Utf8Payload, @@ -59,17 +59,16 @@ export function useRpcStacksMessagePayload() { } export function useRpcSignStacksMessageParams() { - const [searchParams] = useSearchParams(); const { origin, tabId } = useDefaultRequestParams(); - const requestId = searchParams.get('requestId'); - const network = searchParams.get('network'); - const appName = searchParams.get('appName'); - const message = searchParams.get('message'); - const messageType = searchParams.get('messageType'); - const domain = searchParams.get('domain'); + const requestId = initialSearchParams.get('requestId'); + const network = initialSearchParams.get('network'); + const appName = initialSearchParams.get('appName'); + const message = initialSearchParams.get('message'); + const messageType = initialSearchParams.get('messageType'); + const domain = initialSearchParams.get('domain'); if (!requestId || !message || !origin || !isSignableMessageType(messageType)) - throw new Error('Invalid params'); + throw new Error('Missing required parameters for Stacks signing message request'); return useMemo( () => ({ @@ -93,7 +92,7 @@ export function useRpcSignStacksMessage() { if (!tabId) throw new Error('Requests can only be made with corresponding tab'); const { isLoading, signMessage } = useSignStacksMessage({ - onSignMessageCompleted: messageSignature => { + onSignMessageCompleted(messageSignature) { chrome.tabs.sendMessage( tabId, makeRpcSuccessResponse('stx_signMessage', { @@ -103,6 +102,7 @@ export function useRpcSignStacksMessage() { ); closeWindow(); }, + onSignMessageCancelled: onCancelMessageSigning, }); function onCancelMessageSigning() { diff --git a/src/app/pages/sign-stacks-message-request/use-sign-stacks-message-request.ts b/src/app/pages/sign-stacks-message-request/use-sign-stacks-message-request.ts index d260811062c..08cab05368a 100644 --- a/src/app/pages/sign-stacks-message-request/use-sign-stacks-message-request.ts +++ b/src/app/pages/sign-stacks-message-request/use-sign-stacks-message-request.ts @@ -55,6 +55,7 @@ export function useSignStacksMessageRequest() { onSignMessageCompleted: messageSignature => { finalizeMessageSignature({ requestPayload: requestToken, tabId, data: messageSignature }); }, + onSignMessageCancelled: onCancelMessageSigning, }); function onCancelMessageSigning() { diff --git a/src/shared/signature/signature-types.ts b/src/shared/signature/signature-types.ts index 5818c984a89..d7fe96e7f6b 100644 --- a/src/shared/signature/signature-types.ts +++ b/src/shared/signature/signature-types.ts @@ -1,4 +1,11 @@ -import { ClarityValue, StringAsciiCV, TupleCV, UIntCV } from '@stacks/transactions'; +import { + ClarityValue, + StringAsciiCV, + TupleCV, + UIntCV, + deserializeCV, + serializeCV, +} from '@stacks/transactions'; export type SignedMessageType = 'utf8' | 'structured'; @@ -23,6 +30,12 @@ export interface UnsignedMessageStructured extends AbstractUnsignedMessage { domain: StructuredMessageDataDomain; } +interface SerializedUnsignedMessageStructured extends AbstractUnsignedMessage { + messageType: 'structured'; + message: Uint8Array; + domain: Uint8Array; +} + export type UnsignedMessage = UnsignedMessageUtf8 | UnsignedMessageStructured; export function isStructuredMessageType( @@ -49,3 +62,25 @@ export function whenSignableMessageOfType(msg: UnsignedMessage) { return structured(msg.domain, msg.message); }; } + +export function toSerializableUnsignedMessage( + unsignedMessage: UnsignedMessage +): UnsignedMessageUtf8 | SerializedUnsignedMessageStructured { + if (unsignedMessage.messageType === 'utf8') return unsignedMessage; + return { + messageType: 'structured', + message: serializeCV(unsignedMessage.message), + domain: serializeCV(unsignedMessage.domain), + }; +} + +export function deserializeUnsignedMessage( + serialisedUnsignedMessage: UnsignedMessageUtf8 | SerializedUnsignedMessageStructured +): UnsignedMessage { + if (serialisedUnsignedMessage.messageType === 'utf8') return serialisedUnsignedMessage; + return { + messageType: 'structured', + message: deserializeCV(serialisedUnsignedMessage.message), + domain: deserializeCV(serialisedUnsignedMessage.domain), + }; +} diff --git a/yarn.lock b/yarn.lock index 150cda42079..6331ebc426b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7288,6 +7288,13 @@ dependencies: "@types/lodash" "*" +"@types/lodash.isequal@4.5.8": + version "4.5.8" + resolved "https://registry.yarnpkg.com/@types/lodash.isequal/-/lodash.isequal-4.5.8.tgz#b30bb6ff6a5f6c19b3daf389d649ac7f7a250499" + integrity sha512-uput6pg4E/tj2LGxCZo9+y27JNyB2OZuuI/T5F+ylVDYuqICLG2/ktjxx0v6GvVntAf8TvEzeQLcV0ffRirXuA== + dependencies: + "@types/lodash" "*" + "@types/lodash.uniqby@4.7.7": version "4.7.7" resolved "https://registry.yarnpkg.com/@types/lodash.uniqby/-/lodash.uniqby-4.7.7.tgz#48dbb652c41cc8fb30aa61a44174368081835ab5" @@ -15593,6 +15600,11 @@ lodash.isboolean@^3.0.3: resolved "https://registry.yarnpkg.com/lodash.isboolean/-/lodash.isboolean-3.0.3.tgz#6c2e171db2a257cd96802fd43b01b20d5f5870f6" integrity sha512-Bz5mupy2SVbPHURB98VAcw+aHh4vRV5IPNhILUCsOzRmsTmSQ17jIuqopAentWoehktxGd9e/hbIXq980/1QJg== +lodash.isequal@4.5.0: + version "4.5.0" + resolved "https://registry.yarnpkg.com/lodash.isequal/-/lodash.isequal-4.5.0.tgz#415c4478f2bcc30120c22ce10ed3226f7d3e18e0" + integrity sha512-pDo3lu8Jhfjqls6GkMgpahsF9kCyayhgykjyLMNFTKWrpVdAQtYyB4muAMWozBB4ig/dtWAmsMxLEI8wuz+DYQ== + lodash.isinteger@^4.0.4: version "4.0.4" resolved "https://registry.yarnpkg.com/lodash.isinteger/-/lodash.isinteger-4.0.4.tgz#619c0af3d03f8b04c31f5882840b77b11cd68343"