Skip to content

Commit

Permalink
fix(key-manager): handle eth_signTransaction with from field (#675)
Browse files Browse the repository at this point in the history
fixes #674
  • Loading branch information
mirceanis authored Aug 25, 2021
1 parent 6b201f5 commit 50f074d
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 13 deletions.
48 changes: 47 additions & 1 deletion __tests__/shared/keyManager.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { IKey, TKeyType } from '@veramo/core'
import { TAgent, IDIDManager, IKeyManager, IAgentOptions } from '../../packages/core/src'
import { serialize } from '@ethersproject/transactions'
import { serialize, computeAddress } from '@ethersproject/transactions'

type ConfiguredAgent = TAgent<IDIDManager & IKeyManager>

Expand Down Expand Up @@ -200,6 +200,52 @@ export default (testContext: {
expect(typeof rawTx).toEqual('string')
})

it('should allow signing EthTX with matching from', async () => {
const key = await agent.keyManagerCreate({
kms: 'local',
type: 'Secp256k1',
})
const keyAddress = computeAddress('0x' + key.publicKeyHex)

const rawTx = await agent.keyManagerSignEthTX({
kid: key.kid,
transaction: {
to: '0xce31a19193d4b23f4e9d6163d7247243bAF801c3',
from: keyAddress,
value: 300000,
gasLimit: 43092000,
gasPrice: 20000000000,
nonce: 1,
},
})

expect(typeof rawTx).toEqual('string')
})

it('should NOT sign EthTX with mismatching from field', async () => {
expect.assertions(1)
const key = await agent.keyManagerCreate({
kms: 'local',
type: 'Secp256k1',
})

await expect(
agent.keyManagerSignEthTX({
kid: key.kid,
transaction: {
to: '0xce31a19193d4b23f4e9d6163d7247243bAF801c3',
from: '0xce31a19193d4b23f4e9d6163d7247243bAF801c3',
value: 300000,
gasLimit: 43092000,
gasPrice: 20000000000,
nonce: 1,
},
}),
).rejects.toThrowError(
'invalid_arguments: keyManagerSignEthTX `from` field does not match the chosen key. `from` field should be omitted.',
)
})

it('Should Encrypt/Decrypt', async () => {
const message = 'foo bar'
const senderKey = await agent.keyManagerCreate({
Expand Down
4 changes: 2 additions & 2 deletions packages/key-manager/src/abstract-key-management-system.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { IKey, TKeyType } from '@veramo/core'
import { arrayify, hexlify } from '@ethersproject/bytes'
import { serialize, Transaction } from '@ethersproject/transactions'
import { serialize } from '@ethersproject/transactions'
import * as u8a from 'uint8arrays'

export abstract class AbstractKeyManagementSystem {
Expand All @@ -9,7 +9,7 @@ export abstract class AbstractKeyManagementSystem {

/**@deprecated please use `sign({key, alg: 'eth_signTransaction', data: arrayify(serialize(transaction))})` instead */
async signEthTX({ key, transaction }: { key: IKey; transaction: object }): Promise<string> {
const { v, r, s, type, ...tx } = <Transaction>transaction
const { v, r, s, from, ...tx } = <any>transaction
const data = arrayify(serialize(tx))
const algorithm = 'eth_signTransaction'
const signedTxHexString = this.sign({ key, data, algorithm })
Expand Down
36 changes: 28 additions & 8 deletions packages/key-manager/src/key-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,11 @@ import {
import * as u8a from 'uint8arrays'
import { JWE, createAnonDecrypter, createAnonEncrypter, createJWE, decryptJWE, ECDH } from 'did-jwt'
import { arrayify, hexlify } from '@ethersproject/bytes'
import { serialize, computeAddress } from '@ethersproject/transactions'
import { toUtf8String, toUtf8Bytes } from '@ethersproject/strings'
import { convertPublicKeyToX25519 } from '@stablelib/ed25519'
import Debug from 'debug'
const debug = Debug('veramo:key-manager')

/**
* Agent plugin that provides {@link @veramo/core#IKeyManager} methods
Expand Down Expand Up @@ -138,9 +141,12 @@ export class KeyManager implements IAgentPlugin {

/** {@inheritDoc @veramo/core#IKeyManager.keyManagerSignJWT} */
async keyManagerSignJWT({ kid, data }: IKeyManagerSignJWTArgs): Promise<string> {
const key = await this.store.get({ kid })
const kms = this.getKms(key.kms)
return kms.signJWT({ key, data })
if (typeof data === 'string') {
return this.keyManagerSign({ keyRef: kid, data, encoding: 'utf-8' })
} else {
const dataString = u8a.toString(data, 'base16')
return this.keyManagerSign({ keyRef: kid, data: dataString, encoding: 'base16' })
}
}

/** {@inheritDoc @veramo/core#IKeyManager.keyManagerSign} */
Expand All @@ -164,9 +170,23 @@ export class KeyManager implements IAgentPlugin {

/** {@inheritDoc @veramo/core#IKeyManager.keyManagerSignEthTX} */
async keyManagerSignEthTX({ kid, transaction }: IKeyManagerSignEthTXArgs): Promise<string> {
const key = await this.store.get({ kid })
const kms = this.getKms(key.kms)
return kms.signEthTX({ key, transaction })
const { v, r, s, from, ...tx } = <any>transaction
if (typeof from === 'string') {
debug('WARNING: executing a transaction signing request with a `from` field.')
const key = await this.store.get({ kid })
if (key.publicKeyHex) {
const address = computeAddress('0x' + key.publicKeyHex)
if (address.toLowerCase() !== from.toLowerCase()) {
const msg =
'invalid_arguments: keyManagerSignEthTX `from` field does not match the chosen key. `from` field should be omitted.'
debug(msg)
throw new Error(msg)
}
}
}
const data = serialize(tx)
const algorithm = 'eth_signTransaction'
return this.keyManagerSign({ keyRef: kid, data, algorithm, encoding: 'base16' })
}

/** {@inheritDoc @veramo/core#IKeyManager.keyManagerSharedKey} */
Expand All @@ -178,11 +198,11 @@ export class KeyManager implements IAgentPlugin {
myKey.type === theirKey.type ||
(['Ed25519', 'X25519'].includes(myKey.type) && ['Ed25519', 'X25519'].includes(theirKey.type))
) {
const kms = this.getKms(myKey.kms)
return kms.sharedSecret({ myKey, theirKey })
} else {
throw new Error('invalid_argument: the key types have to match to be able to compute a shared secret')
}
const kms = this.getKms(myKey.kms)
return kms.sharedSecret({ myKey, theirKey })
}

createX25519ECDH(secretKeyRef: string): ECDH {
Expand Down
12 changes: 10 additions & 2 deletions packages/kms-local/src/key-management-system.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import { parse } from '@ethersproject/transactions'
import { Wallet } from '@ethersproject/wallet'
import { SigningKey } from '@ethersproject/signing-key'
import { randomBytes } from '@ethersproject/random'
import Debug from 'debug'
import { arrayify, hexlify } from '@ethersproject/bytes'
import Debug from 'debug'
const debug = Debug('veramo:kms:local')

export class KeyManagementSystem extends AbstractKeyManagementSystem {
Expand Down Expand Up @@ -161,8 +161,16 @@ export class KeyManagementSystem extends AbstractKeyManagementSystem {
* @returns a `0x` prefixed hex string representing the signed raw transaction
*/
private async eth_signTransaction(privateKeyHex: string, rlpTransaction: Uint8Array) {
const { v, r, s, ...tx } = parse(rlpTransaction)
const { v, r, s, from, ...tx } = parse(rlpTransaction)
const wallet = new Wallet(privateKeyHex)
if (from) {
debug('WARNING: executing a transaction signing request with a `from` field.')
if (wallet.address.toLowerCase() !== from.toLowerCase()) {
const msg = "invalid_arguments: eth_signTransaction `from` field does not match the chosen key. `from` field should be omitted."
debug(msg)
throw new Error(msg)
}
}
const signedRawTransaction = await wallet.signTransaction(<TransactionRequest>tx)
//HEX encoded string, 0x prefixed
return signedRawTransaction
Expand Down

0 comments on commit 50f074d

Please sign in to comment.